Commit 3302888c authored by Hannes Rosenögger's avatar Hannes Rosenögger
Browse files

Merge branch 'fix-side-by-side-diff' into 'master'

Fix broken side-by-side diff view on merge request page

### What does this MR do?

This MR fixes the side-by-side diff that was broken in v7.9.0. The "Inline" and "Side-by-Side" buttons are now always rendered with HTML links instead of changing depending on whether the code diff was requested in JSON format.

### Are there points in the code the reviewer needs to double check?

Is there a better way to fix this? The links in JSON format may be desirable if there were a JavaScript hook to handle the buttons, but as far as I can tell there isn't at the moment.

### Why was this MR needed?

The issue is that the code diffs can be generated either by the JavaScript handler or the regular HTML. With the forcing of JSON request format in 1f79ff1a22f5, the JavaScript handler requests the page in JSON format, which it then uses to render the page. However, this has the unintended side effect of requesting that the "Inline" and "Side-by-Side" buttons have references to a JSON-formatted link.

### What are the relevant issue numbers / [Feature requests](http://feedback.gitlab.com/)?

#1294

See merge request !460
Please view this file on the master branch, on stable branches it's out of date. Please view this file on the master branch, on stable branches it's out of date.
v 7.10.0 (unreleased) v 7.10.0 (unreleased)
- Fix broken side-by-side diff view on merge request page (Stan Hu)
- Allow HTML tags in Markdown input - Allow HTML tags in Markdown input
- Fix code unfold not working on Compare commits page (Stan Hu) - Fix code unfold not working on Compare commits page (Stan Hu)
- Include missing events and fix save functionality in admin service template settings form (Stan Hu) - Include missing events and fix save functionality in admin service template settings form (Stan Hu)
......
...@@ -121,6 +121,8 @@ def organize_comments(type_left, type_right, line_code_left, line_code_right) ...@@ -121,6 +121,8 @@ def organize_comments(type_left, type_right, line_code_left, line_code_right)
def inline_diff_btn def inline_diff_btn
params_copy = params.dup params_copy = params.dup
params_copy[:view] = 'inline' params_copy[:view] = 'inline'
# Always use HTML to handle case where JSON diff rendered this button
params_copy.delete(:format)
link_to url_for(params_copy), id: "commit-diff-viewtype", class: (params[:view] != 'parallel' ? 'btn btn-sm active' : 'btn btn-sm') do link_to url_for(params_copy), id: "commit-diff-viewtype", class: (params[:view] != 'parallel' ? 'btn btn-sm active' : 'btn btn-sm') do
'Inline' 'Inline'
...@@ -130,6 +132,8 @@ def inline_diff_btn ...@@ -130,6 +132,8 @@ def inline_diff_btn
def parallel_diff_btn def parallel_diff_btn
params_copy = params.dup params_copy = params.dup
params_copy[:view] = 'parallel' params_copy[:view] = 'parallel'
# Always use HTML to handle case where JSON diff rendered this button
params_copy.delete(:format)
link_to url_for(params_copy), id: "commit-diff-viewtype", class: (params[:view] == 'parallel' ? 'btn active btn-sm' : 'btn btn-sm') do link_to url_for(params_copy), id: "commit-diff-viewtype", class: (params[:view] == 'parallel' ? 'btn active btn-sm' : 'btn btn-sm') do
'Side-by-side' 'Side-by-side'
......
...@@ -166,6 +166,13 @@ Feature: Project Merge Requests ...@@ -166,6 +166,13 @@ Feature: Project Merge Requests
And I click Side-by-side Diff tab And I click Side-by-side Diff tab
Then I should see comments on the side-by-side diff page Then I should see comments on the side-by-side diff page
@javascript
Scenario: I view diffs on a merge request
Given project "Shop" have "Bug NS-05" open merge request with diffs inside
And I visit merge request page "Bug NS-05"
And I click on the Changes tab via Javascript
Then I should see the proper Inline and Side-by-side links
# Task status in issues list # Task status in issues list
Scenario: Merge requests list should display task status Scenario: Merge requests list should display task status
......
...@@ -117,6 +117,20 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps ...@@ -117,6 +117,20 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request) visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)
end end
step 'I click on the Changes tab via Javascript' do
find('.diffs-tab').click
sleep 2
end
step 'I should see the proper Inline and Side-by-side links' do
buttons = all('#commit-diff-viewtype')
expect(buttons.count).to eq(2)
buttons.each do |b|
expect(b['href']).should_not have_content('json')
end
end
step 'I switch to the merge request\'s comments tab' do step 'I switch to the merge request\'s comments tab' do
visit namespace_project_merge_request_path(project.namespace, project, merge_request) visit namespace_project_merge_request_path(project.namespace, project, merge_request)
end end
......
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