From 0dcbc736ab94a48cad482702f56a19c7bdb9b207 Mon Sep 17 00:00:00 2001 From: Stanislav Lashmanov Date: Tue, 5 Dec 2023 02:38:31 +0400 Subject: [PATCH] Add diff_by_file_hash merge request action Changelog: added --- .../merge_requests/diffs_controller.rb | 12 ++- app/helpers/diff_helper.rb | 2 +- config/routes/merge_requests.rb | 1 + .../merge_requests/diffs_controller_spec.rb | 89 ++++++++++++++++++- 4 files changed, 97 insertions(+), 7 deletions(-) diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 5bd0063ab95a2f..ee7b1303b32878 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -9,11 +9,11 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic before_action :commit before_action :define_diff_vars before_action :define_diff_comment_vars, except: [:diffs_batch, :diffs_metadata] - before_action :update_diff_discussion_positions! + before_action :update_diff_discussion_positions!, except: [:diff_by_file_hash] around_action :allow_gitaly_ref_name_caching - after_action :track_viewed_diffs_events, only: [:diffs_batch, :diff_for_path] + after_action :track_viewed_diffs_events, only: [:diffs_batch, :diff_for_path, :diff_by_file_hash] urgency :low, [ :show, @@ -26,6 +26,14 @@ def show render_diffs end + def diff_by_file_hash + diff_file = @compare.diffs.diff_files.find { |file| file.file_hash == params[:file_hash] } + params[:old_path] = diff_file&.old_path + params[:new_path] = diff_file&.new_path + + render_diffs + end + def diff_for_path render_diffs end diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 9031d0556dacf6..6069e4e64a1b21 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -30,7 +30,7 @@ def show_only_context_commits? def diff_options options = { ignore_whitespace_change: hide_whitespace?, expanded: diffs_expanded?, use_extra_viewer_as_main: true } - if action_name == 'diff_for_path' + if action_name == 'diff_for_path' || action_name == 'diff_by_file_hash' options[:expanded] = true options[:paths] = params.values_at(:old_path, :new_path) options[:use_extra_viewer_as_main] = false diff --git a/config/routes/merge_requests.rb b/config/routes/merge_requests.rb index adfc9bcd19b262..a5d89e74980566 100644 --- a/config/routes/merge_requests.rb +++ b/config/routes/merge_requests.rb @@ -41,6 +41,7 @@ end get :diff_for_path, controller: 'merge_requests/diffs' + get 'diff_by_file_hash/:file_hash', to: 'merge_requests/diffs#diff_by_file_hash', as: :diff_by_file_hash scope controller: 'merge_requests/conflicts' do get :conflicts, action: :show diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index 80c68d3e09125f..3ebca0994999ed 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -138,10 +138,16 @@ end let(:project) { create(:project, :repository) } - let(:user) { create(:user) } let(:maintainer) { true } let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } + let_it_be_with_reload(:user) { create(:user) } + let_it_be(:other_project) { create(:project) } + + before_all do + other_project.add_maintainer(user) + end + before do project.add_maintainer(user) if maintainer sign_in(user) @@ -429,10 +435,7 @@ def diff_for_path(extra_params = {}) end context 'when the merge request belongs to a different project' do - let(:other_project) { create(:project) } - before do - other_project.add_maintainer(user) diff_for_path(old_path: existing_path, new_path: existing_path, project_id: other_project) end @@ -442,6 +445,84 @@ def diff_for_path(extra_params = {}) end end + describe 'GET diff_by_file_hash' do + def diff_by_file_hash(extra_params = {}) + params = { + namespace_id: project.namespace.to_param, + project_id: project, + id: merge_request.iid, + format: 'json' + } + + get :diff_by_file_hash, params: params.merge(extra_params) + end + + let(:file) { merge_request.merge_request_diff.diffs.diff_files.first } + let(:file_hash) { file.file_hash } + + context 'when the merge request exists' do + context 'when the user can view the merge request' do + context 'when the path exists in the diff' do + include_examples 'diff tracking' do + let(:method_call) { diff_by_file_hash(file_hash: file_hash) } + end + + it 'enables diff notes' do + diff_by_file_hash(file_hash: file_hash) + + expect(assigns(:diff_notes_disabled)).to be_falsey + expect(assigns(:new_diff_note_attrs)).to eq( + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + commit_id: nil + ) + end + + it 'only renders diff for the hash given' do + diff_by_file_hash(file_hash: file_hash) + + diffs = json_response['diff_files'] + + expect(diffs.count).to eq(1) + expect(diffs.first['file_hash']).to eq(file_hash) + end + end + end + + context 'when the user cannot view the merge request' do + let(:maintainer) { false } + + before do + diff_by_file_hash(file_hash: file_hash) + end + + it 'returns a 404' do + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when the merge request does not exist' do + before do + diff_by_file_hash(id: merge_request.iid.succ, file_hash: file_hash) + end + + it 'returns a 404' do + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when the merge request belongs to a different project' do + before do + diff_by_file_hash(project_id: other_project, file_hash: file_hash) + end + + it 'returns a 404' do + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + describe 'GET diffs_batch' do shared_examples_for 'serializes diffs with expected arguments' do it 'serializes paginated merge request diff collection' do -- GitLab