Commit 5a8873f3 authored by Rémy Coutable's avatar Rémy Coutable

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
parents 988dad46 8530ce4c
......@@ -83,8 +83,7 @@ def require_branch_head
end
def apply_diff_view_cookie!
view = params[:view] || cookies[:diff_view]
cookies.permanent[:diff_view] = params[:view] = view if view
cookies.permanent[:diff_view] = params.delete(:view) if params[:view].present?
end
def builds_enabled
......
......@@ -9,7 +9,13 @@ def mark_inline_diffs(old_line, new_line)
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?
......
......@@ -3,7 +3,7 @@
- page_card_attributes @merge_request.card_attributes
- header_title project_title(@project, "Merge Requests", namespace_project_merge_requests_path(@project.namespace, @project))
- if params[:view] == 'parallel'
- if diff_view == 'parallel'
- fluid_layout true
.merge-request{'data-url' => merge_request_path(@merge_request)}
......
......@@ -300,14 +300,6 @@ def go(extra_params = {})
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
......
......@@ -11,6 +11,26 @@
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 } }
......
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