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 27e75344, 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

4 files changed, 26 insertions, 0 deletions

diff --git a/CHANGELOG b/CHANGELOG
index d16f20266cb..8284688efdb 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 7.10.0 (unreleased)
+  - Fix broken side-by-side diff view on merge request page (Stan Hu)
   - Allow HTML tags in Markdown input
   - 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)
diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb
index f81504991d3..b56f21c7a18 100644
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@@ -121,6 +121,8 @@ module DiffHelper
   def inline_diff_btn
     params_copy = params.dup
     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
       'Inline'
@@ -130,6 +132,8 @@ module DiffHelper
   def parallel_diff_btn
     params_copy = params.dup
     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
       'Side-by-side'
diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature
index 91dc576f8b4..cbb5c8eb39b 100644
--- a/features/project/merge_requests.feature
+++ b/features/project/merge_requests.feature
@@ -166,6 +166,13 @@ Feature: Project Merge Requests
     And I click Side-by-side Diff tab
     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
 
   Scenario: Merge requests list should display task status
diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb
index 6f6ce439f3e..40c102833a4 100644
--- a/features/steps/project/merge_requests.rb
+++ b/features/steps/project/merge_requests.rb
@@ -117,6 +117,20 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
     visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)
   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
     visit namespace_project_merge_request_path(project.namespace, project, merge_request)
   end