Merge branch 'ph/fixDiffViewCookieBeingSetIncorrectly' into 'master'

Fixes diff view cookie being set on wrong endpoint

See merge request gitlab-org/gitlab!41109

app/controllers/projects/merge_requests/diffs_controller.rb

@@ -4,7 +4,6 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
   include DiffHelper
   include RendersNotes
 
-  before_action :apply_diff_view_cookie!
   before_action :commit
   before_action :define_diff_vars
   before_action :define_diff_comment_vars, except: [:diffs_batch, :diffs_metadata]

app/controllers/projects/merge_requests_controller.rb

@@ -10,8 +10,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   include IssuableCollections
   include RecordUserLastActivity
   include SourcegraphDecorator
+  include DiffHelper
 
   skip_before_action :merge_request, only: [:index, :bulk_update]
+  before_action :apply_diff_view_cookie!, only: [:show]
   before_action :whitelist_query_limiting, only: [:assign_related_issues, :update]
   before_action :authorize_update_issuable!, only: [:close, :edit, :update, :remove_wip, :sort]
   before_action :authorize_read_actual_head_pipeline!, only: [

spec/controllers/projects/merge_requests/diffs_controller_spec.rb

@@ -58,39 +58,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do
     end
   end
 
-  shared_examples 'persisted preferred diff view cookie' do
-    context 'with view param' do
-      before do
-        go(view: 'parallel')
-      end
-
-      it 'saves the preferred diff view in a cookie' do
-        expect(response.cookies['diff_view']).to eq('parallel')
-      end
-
-      it 'only renders the required view', :aggregate_failures do
-        diff_files_without_deletions = json_response['diff_files'].reject { |f| f['deleted_file'] }
-        have_no_inline_diff_lines = satisfy('have no inline diff lines') do |diff_file|
-          !diff_file.has_key?('highlighted_diff_lines')
-        end
-
-        expect(diff_files_without_deletions).to all(have_key('parallel_diff_lines'))
-        expect(diff_files_without_deletions).to all(have_no_inline_diff_lines)
-      end
-    end
-
-    context 'when the user cannot view the merge request' do
-      before do
-        project.team.truncate
-        go
-      end
-
-      it 'returns a 404' do
-        expect(response).to have_gitlab_http_status(:not_found)
-      end
-    end
-  end
-
   shared_examples "diff note on-demand position creation" do
     it "updates diff discussion positions" do
       service = double("service")
@@ -155,7 +122,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do
       it_behaves_like 'forked project with submodules'
     end
 
-    it_behaves_like 'persisted preferred diff view cookie'
     it_behaves_like 'cached diff collection'
     it_behaves_like 'diff note on-demand position creation'
   end
@@ -511,7 +477,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do
     end
 
     it_behaves_like 'forked project with submodules'
-    it_behaves_like 'persisted preferred diff view cookie'
     it_behaves_like 'cached diff collection'
 
     context 'diff unfolding' do

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -44,6 +44,16 @@ RSpec.describe Projects::MergeRequestsController do
       get :show, params: params.merge(extra_params)
     end
 
+    context 'with view param' do
+      before do
+        go(view: 'parallel')
+      end
+
+      it 'saves the preferred diff view in a cookie' do
+        expect(response.cookies['diff_view']).to eq('parallel')
+      end
+    end
+
     context 'when merge request is unchecked' do
       before do
         merge_request.mark_as_unchecked!