Merge branch 'rs-diff_view' into 'master'

Always read diff_view setting from the cookie

Prior, when the user had their view set to "parallel" and then visited a
merge request's changes tab _without_ passing the `view` parameter via
query string, the view would be parallel but the `Notes` class was
always instantiated with the default value from `diff_view` ("inline"),
resulting in broken markup when the form to add a line note was
dynamically inserted.

The cookie is set whenever the view is changed, so this value should
always be up-to-date.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/14557 and https://gitlab.com/gitlab-org/gitlab-ce/issues/15285

See merge request !3732

2 files changed, 20 insertions, 8 deletions

diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index c54e83339a1..c0a1f45195f 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -300,14 +300,6 @@ describe Projects::MergeRequestsController do
 
       expect(response.cookies['diff_view']).to eq('parallel')
     end
-
-    it 'assigns :view param based on cookie' do
-      request.cookies['diff_view'] = 'parallel'
-
-      go
-
-      expect(controller.params[:view]).to eq 'parallel'
-    end
   end
 
   describe 'GET commits' do
diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb
index 982c113e84b..b7810185d16 100644
--- a/spec/helpers/diff_helper_spec.rb
+++ b/spec/helpers/diff_helper_spec.rb
@@ -11,6 +11,26 @@ describe DiffHelper do
   let(:diff_refs) { [commit.parent, commit] }
   let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs) }
 
+  describe 'diff_view' do
+    it 'returns a valid value when cookie is set' do
+      helper.request.cookies[:diff_view] = 'parallel'
+
+      expect(helper.diff_view).to eq 'parallel'
+    end
+
+    it 'returns a default value when cookie is invalid' do
+      helper.request.cookies[:diff_view] = 'invalid'
+
+      expect(helper.diff_view).to eq 'inline'
+    end
+
+    it 'returns a default value when cookie is nil' do
+      expect(helper.request.cookies).to be_empty
+
+      expect(helper.diff_view).to eq 'inline'
+    end
+  end
+
   describe 'diff_hard_limit_enabled?' do
     it 'should return true if param is provided' do
       allow(controller).to receive(:params) { { force_show_diff: true } }