From 5b7c7090bd1716e382ac32f31c7d2fe44616e841 Mon Sep 17 00:00:00 2001 From: Phil Hughes <me@iamphill.com> Date: Tue, 17 Nov 2020 09:42:47 +0000 Subject: [PATCH] Fixes diff metadata StartupJS URL not matching With unified diffs we always send the view param as inline this wasn't taken into account when setting up the startup JS url. Closes https://gitlab.com/gitlab-org/gitlab/-/issues/280579 --- .../merge_requests/diffs_controller.rb | 2 +- .../projects/merge_requests_controller.rb | 2 +- app/helpers/diff_helper.rb | 8 ++++++ .../ph-280579-fixDiffMetadataStartupJS.yml | 5 ++++ .../merge_requests_controller_spec.rb | 3 ++- spec/helpers/diff_helper_spec.rb | 26 +++++++++++++++++++ 6 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/ph-280579-fixDiffMetadataStartupJS.yml diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 15e16687c02..7fbeac12644 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -69,7 +69,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic } options = additional_attributes.merge( - diff_view: Feature.enabled?(:unified_diff_lines, @merge_request.project, default_enabled: true) ? "inline" : diff_view, + diff_view: unified_diff_lines_view_type(@merge_request.project), merge_ref_head_diff: render_merge_ref_head_diff? ) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index b6a6e1e775a..f2b41294a85 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -481,7 +481,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo def endpoint_metadata_url(project, merge_request) params = request.query_parameters - params[:view] = cookies[:diff_view] if params[:view].blank? && cookies[:diff_view].present? + params[:view] = unified_diff_lines_view_type(project) if Feature.enabled?(:default_merge_ref_for_diffs, project) params = params.merge(diff_head: true) diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 69a2efebb1f..d6d06434590 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -203,6 +203,14 @@ module DiffHelper set_secure_cookie(:diff_view, params.delete(:view), type: CookiesHelper::COOKIE_TYPE_PERMANENT) if params[:view].present? end + def unified_diff_lines_view_type(project) + if Feature.enabled?(:unified_diff_lines, project, default_enabled: true) + 'inline' + else + diff_view + end + end + private def diff_btn(title, name, selected) diff --git a/changelogs/unreleased/ph-280579-fixDiffMetadataStartupJS.yml b/changelogs/unreleased/ph-280579-fixDiffMetadataStartupJS.yml new file mode 100644 index 00000000000..ee1faca8310 --- /dev/null +++ b/changelogs/unreleased/ph-280579-fixDiffMetadataStartupJS.yml @@ -0,0 +1,5 @@ +--- +title: Fixed diff metadata endpoint being called twice +merge_request: 47265 +author: +type: fixed diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index d7d1278159b..f159f0e6099 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -95,7 +95,8 @@ RSpec.describe Projects::MergeRequestsController do project, merge_request, 'json', - diff_head: true)) + diff_head: true, + view: 'inline')) end end diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index 3580959fde0..c085c3bdbd9 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -358,4 +358,30 @@ RSpec.describe DiffHelper do expect(diff_file_path_text(diff_file, max: 10)).to eq("...open.rb") end end + + describe 'unified_diff_lines_view_type' do + before do + controller.params[:view] = 'parallel' + end + + describe 'unified diffs enabled' do + before do + stub_feature_flags(unified_diff_lines: true) + end + + it 'returns inline view' do + expect(helper.unified_diff_lines_view_type(project)).to eq 'inline' + end + end + + describe 'unified diffs disabled' do + before do + stub_feature_flags(unified_diff_lines: false) + end + + it 'returns parallel view' do + expect(helper.unified_diff_lines_view_type(project)).to eq :parallel + end + end + end end -- 2.30.9