From b70e12069c78be745737d20c697a2e8ee490f897 Mon Sep 17 00:00:00 2001 From: Stanislav Lashmanov Date: Thu, 21 Nov 2024 12:47:46 +0400 Subject: [PATCH 1/3] Fix wrong changes counter on merge request commit diffs Fixes: https://gitlab.com/gitlab-org/gitlab/-/issues/483020 Changelog: fixed --- .../projects/merge_requests_controller.rb | 13 ++++++++++++ .../user_views_diffs_commit_spec.rb | 20 +++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 624248b6c61ccd..f497e2805c6999 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -476,7 +476,20 @@ def render_html_page render end + # rubocop: disable CodeReuse/ActiveRecord + def commit + commit_id = params[:commit_id].presence + return unless commit_id + + return unless @merge_request.all_commits.exists?(sha: commit_id) || + @merge_request.recent_context_commits.map(&:id).include?(commit_id) + + @commit ||= @project.commit(commit_id) + end + # rubocop: enable CodeReuse/ActiveRecord + def get_diffs_count + return @commit.raw_diffs.size if commit return @merge_request.context_commits_diff.raw_diffs.size if show_only_context_commits? return @merge_request.merge_request_diffs.find_by_id(params[:diff_id])&.size if params[:diff_id] return @merge_request.merge_head_diff.size if @merge_request.diffable_merge_ref? && params[:start_sha].blank? diff --git a/spec/features/merge_request/user_views_diffs_commit_spec.rb b/spec/features/merge_request/user_views_diffs_commit_spec.rb index ba1b41982c9246..b36fdeb39b5ee4 100644 --- a/spec/features/merge_request/user_views_diffs_commit_spec.rb +++ b/spec/features/merge_request/user_views_diffs_commit_spec.rb @@ -8,12 +8,28 @@ end let(:project) { create(:project, :public, :repository) } + let(:commit_id) { merge_request.diff_head_sha } before do - visit(diffs_project_merge_request_path(project, merge_request, commit_id: merge_request.diff_head_sha)) + visit(diffs_project_merge_request_path(project, merge_request, commit_id: commit_id)) end it 'shows full commit description by default' do - expect(page).to have_selector('.commit-row-description', visible: true) + within_testid('commit-content') do + expect(page).to have_content("Add submodule from gitlab.com") + end + end + + it 'shows correct commit metadata' do + expect(page).to have_content("Viewing commit #{commit_id[..7]}") + within_testid('diffs-tab') do + expect(page).to have_content('Changes 2') + end + page.within('#diffs') do + expect(page).to have_content('2 files') + end + within_testid('file-tree-container') do + expect(page).to have_content('Files 2') + end end end -- GitLab From f4c48fe2543717ee19000ae2e5be44f171087d72 Mon Sep 17 00:00:00 2001 From: Stanislav Lashmanov Date: Fri, 22 Nov 2024 13:02:20 +0400 Subject: [PATCH 2/3] Speed up MR commit lookup --- app/controllers/projects/merge_requests_controller.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index f497e2805c6999..afccda96e9deb2 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -476,17 +476,16 @@ def render_html_page render end - # rubocop: disable CodeReuse/ActiveRecord def commit commit_id = params[:commit_id].presence return unless commit_id - return unless @merge_request.all_commits.exists?(sha: commit_id) || + merge_request = MergeRequest.from_project(@project).by_commit_sha(commit_id).sole + return unless merge_request&.id == @merge_request.id || @merge_request.recent_context_commits.map(&:id).include?(commit_id) @commit ||= @project.commit(commit_id) end - # rubocop: enable CodeReuse/ActiveRecord def get_diffs_count return @commit.raw_diffs.size if commit -- GitLab From fa33e7f2023c9119a3b71be450d196623a5f6a9e Mon Sep 17 00:00:00 2001 From: Stanislav Lashmanov Date: Mon, 25 Nov 2024 14:27:46 +0400 Subject: [PATCH 3/3] Restore old commit method variant Move the shared code to a base controller --- .../merge_requests/application_controller.rb | 12 ++++++++++++ .../projects/merge_requests/diffs_controller.rb | 10 ---------- .../projects/merge_requests_controller.rb | 11 ----------- 3 files changed, 12 insertions(+), 21 deletions(-) diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb index a448cc562f25cf..ae93f61018a193 100644 --- a/app/controllers/projects/merge_requests/application_controller.rb +++ b/app/controllers/projects/merge_requests/application_controller.rb @@ -85,6 +85,18 @@ def close_merge_request_if_no_source_project @merge_request.close end + + # rubocop: disable CodeReuse/ActiveRecord + def commit + commit_id = params[:commit_id].presence + return unless commit_id + + return unless @merge_request.all_commits.exists?(sha: commit_id) || + @merge_request.recent_context_commits.map(&:id).include?(commit_id) + + @commit ||= @project.commit(commit_id) + end + # rubocop: enable CodeReuse/ActiveRecord end Projects::MergeRequests::ApplicationController.prepend_mod_with('Projects::MergeRequests::ApplicationController') diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 8e6a8ee19a7dd7..cce2ad3d823a4b 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -134,16 +134,6 @@ def define_diff_vars render_404 unless @compare end - # rubocop: disable CodeReuse/ActiveRecord - def commit - return unless commit_id = params[:commit_id].presence - return unless @merge_request.all_commits.exists?(sha: commit_id) || - @merge_request.recent_context_commits.map(&:id).include?(commit_id) - - @commit ||= @project.commit(commit_id) - end - # rubocop: enable CodeReuse/ActiveRecord - # rubocop: disable CodeReuse/ActiveRecord # # Deprecated: https://gitlab.com/gitlab-org/gitlab/issues/37735 diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index afccda96e9deb2..4e366758acd296 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -476,17 +476,6 @@ def render_html_page render end - def commit - commit_id = params[:commit_id].presence - return unless commit_id - - merge_request = MergeRequest.from_project(@project).by_commit_sha(commit_id).sole - return unless merge_request&.id == @merge_request.id || - @merge_request.recent_context_commits.map(&:id).include?(commit_id) - - @commit ||= @project.commit(commit_id) - end - def get_diffs_count return @commit.raw_diffs.size if commit return @merge_request.context_commits_diff.raw_diffs.size if show_only_context_commits? -- GitLab