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

3 files changed, 28 insertions, 2 deletions

diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb
index 6a3ec83b8c0..97466d532f4 100644
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@@ -9,7 +9,13 @@ module DiffHelper
   end
 
   def diff_view
-    params[:view] == 'parallel' ? 'parallel' : 'inline'
+    diff_views = %w(inline parallel)
+
+    if diff_views.include?(cookies[:diff_view])
+      cookies[:diff_view]
+    else
+      diff_views.first
+    end
   end
 
   def diff_hard_limit_enabled?
diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml
index 1dd8f721f7e..1733622e9b7 100644
--- a/app/views/projects/merge_requests/_show.html.haml
+++ b/app/views/projects/merge_requests/_show.html.haml
@@ -4,7 +4,7 @@
 
 = render "header_title"
 
-- if params[:view] == 'parallel'
+- if diff_view == 'parallel'
   - fluid_layout true
 
 .merge-request{'data-url' => merge_request_path(@merge_request)}
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 } }