Commit 90a67a76 authored by Robert Speicher's avatar Robert Speicher

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
parent ba18e0b2
...@@ -9,7 +9,13 @@ module DiffHelper ...@@ -9,7 +9,13 @@ module DiffHelper
end end
def diff_view 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 end
def diff_hard_limit_enabled? def diff_hard_limit_enabled?
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
= render "header_title" = render "header_title"
- if params[:view] == 'parallel' - if diff_view == 'parallel'
- fluid_layout true - fluid_layout true
.merge-request{'data-url' => merge_request_path(@merge_request)} .merge-request{'data-url' => merge_request_path(@merge_request)}
......
...@@ -11,6 +11,26 @@ describe DiffHelper do ...@@ -11,6 +11,26 @@ describe DiffHelper do
let(:diff_refs) { [commit.parent, commit] } let(:diff_refs) { [commit.parent, commit] }
let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs) } 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 describe 'diff_hard_limit_enabled?' do
it 'should return true if param is provided' do it 'should return true if param is provided' do
allow(controller).to receive(:params) { { force_show_diff: true } } allow(controller).to receive(:params) { { force_show_diff: true } }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment