From 173b677f6b851d568083b0500ad3fea5a6d0b5d1 Mon Sep 17 00:00:00 2001 From: Anatoli Babenia Date: Sat, 9 Sep 2023 07:43:55 +0000 Subject: [PATCH 1/4] Remove internal var that is used only once --- app/controllers/concerns/creates_commit.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb index 896004045f487a..2a9032f70f7275 100644 --- a/app/controllers/concerns/creates_commit.rb +++ b/app/controllers/concerns/creates_commit.rb @@ -18,10 +18,8 @@ def create_commit(service, success_path:, failure_path:, failure_view: nil, succ @start_branch ||= @ref || @branch_name - start_project = @project_to_commit_into - commit_params = @commit_params.merge( - start_project: start_project, + start_project: @project_to_commit_into, start_branch: @start_branch, source_project: @project, target_project: target_project, -- GitLab From b617cfdbe6df46c23a26d8b384403109026cf2b4 Mon Sep 17 00:00:00 2001 From: Anatoli Babenia Date: Mon, 11 Sep 2023 15:03:31 +0000 Subject: [PATCH 2/4] Replace different_project? method with instance variable Simplifies logic by removing one level of indirection. --- app/controllers/concerns/creates_commit.rb | 10 ++++------ app/controllers/projects/blob_controller.rb | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb index 2a9032f70f7275..27f1d1f5528793 100644 --- a/app/controllers/concerns/creates_commit.rb +++ b/app/controllers/concerns/creates_commit.rb @@ -10,9 +10,11 @@ def create_commit(service, success_path:, failure_path:, failure_view: nil, succ if user_access(target_project).can_push_to_branch?(branch_name_or_ref) @project_to_commit_into = target_project + @different_project = false @branch_name ||= @ref else @project_to_commit_into = current_user.fork_of(target_project) + @different_project = true @branch_name ||= @project_to_commit_into.repository.next_branch('patch') end @@ -72,7 +74,7 @@ def update_flash_notice(success_notice) nil else mr_message = - if different_project? + if @different_project # rubocop:disable Gitlab/ModuleWithInstanceVariables _("You can now submit a merge request to get this change into the original project.") else _("You can now submit a merge request to get this change into the original branch.") @@ -126,16 +128,12 @@ def merge_request_exists? # rubocop: enable CodeReuse/ActiveRecord # rubocop:enable Gitlab/ModuleWithInstanceVariables - def different_project? - @project_to_commit_into != @project # rubocop:disable Gitlab/ModuleWithInstanceVariables - end - def create_merge_request? # Even if the field is set, if we're checking the same branch # as the target branch in the same project, # we don't want to create a merge request. params[:create_merge_request].present? && - (different_project? || @start_branch != @branch_name) # rubocop:disable Gitlab/ModuleWithInstanceVariables + (@different_project || @start_branch != @branch_name) # rubocop:disable Gitlab/ModuleWithInstanceVariables end def branch_name_or_ref diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 56e4b22ded2788..2c21fdea09813d 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -101,7 +101,7 @@ def update ) rescue Files::UpdateService::FileChangedError @conflict = true - @different_project = different_project? + # @different_project is set by create_commit render :edit end -- GitLab From 3f914d171baa9825b9544a8a442e08e02c41c678 Mon Sep 17 00:00:00 2001 From: Anatoli Babenia Date: Sat, 16 Sep 2023 06:38:18 +0000 Subject: [PATCH 3/4] Explicitly pass instance variables from concern First step to factor out CreatesCommit concern into its own class --- app/controllers/projects/blob_controller.rb | 7 ++++--- app/views/projects/blob/edit.html.haml | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 2c21fdea09813d..406bb9c46f5967 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -100,9 +100,10 @@ def update failure_path: project_blob_path(@project, @id) ) rescue Files::UpdateService::FileChangedError - @conflict = true - # @different_project is set by create_commit - render :edit + render :edit, locals: { + conflict: true, + commit_to_fork: @different_project + } end def preview diff --git a/app/views/projects/blob/edit.html.haml b/app/views/projects/blob/edit.html.haml index 195dc03632ab19..b716cd7374b975 100644 --- a/app/views/projects/blob/edit.html.haml +++ b/app/views/projects/blob/edit.html.haml @@ -3,7 +3,7 @@ - content_for :prefetch_asset_tags do - webpack_preload_asset_tag('monaco') - add_page_specific_style 'page_bundles/editor' -- if @conflict +- if conflict = render Pajamas::AlertComponent.new(alert_options: { class: 'gl-mb-5 gl-mt-5' }, variant: :danger, dismissible: false) do |c| @@ -12,7 +12,7 @@ - link_end = ''.html_safe - external_link_icon = content_tag 'span', { aria: { label: _('Opens new window') }} do - sprite_icon('external-link', css_class: 'gl-icon').html_safe - - if @different_project + - if commit_to_fork = _("Error: Can't edit this file. The fork and upstream project have diverged. %{link_start}Edit the file on the fork %{icon}%{link_end}, and create a merge request.").html_safe % {link_start: blob_link_start % { url: project_blob_path(@project_to_commit_into, @id) } , link_end: link_end, icon: external_link_icon } - else - blob_url = project_blob_path(@project, @id) -- GitLab From a723a89876b7fc6463b29133718b2b3b0f02676b Mon Sep 17 00:00:00 2001 From: Anatoli Babenia Date: Thu, 21 Sep 2023 14:35:46 +0000 Subject: [PATCH 4/4] Get back to previous state to make test happy --- app/controllers/projects/blob_controller.rb | 4 ++-- app/views/projects/blob/edit.html.haml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 406bb9c46f5967..18ed617e20cdf0 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -100,8 +100,8 @@ def update failure_path: project_blob_path(@project, @id) ) rescue Files::UpdateService::FileChangedError - render :edit, locals: { - conflict: true, + @conflict = true + render "edit", locals: { commit_to_fork: @different_project } end diff --git a/app/views/projects/blob/edit.html.haml b/app/views/projects/blob/edit.html.haml index b716cd7374b975..74f1688a2db017 100644 --- a/app/views/projects/blob/edit.html.haml +++ b/app/views/projects/blob/edit.html.haml @@ -3,7 +3,7 @@ - content_for :prefetch_asset_tags do - webpack_preload_asset_tag('monaco') - add_page_specific_style 'page_bundles/editor' -- if conflict +- if @conflict = render Pajamas::AlertComponent.new(alert_options: { class: 'gl-mb-5 gl-mt-5' }, variant: :danger, dismissible: false) do |c| -- GitLab