Merge branch 'tab-persistance' into 'master'

Make new tabs persistent when changing view.

## What does this MR do?

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

## Why was this MR needed?

## What are the relevant issue numbers?

## Screenshots (if relevant)

## Does this MR meet the acceptance criteria?

- [ ] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- Tests
  - [ ] Added for this feature/bug
  - [ ] All builds are passing
- [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

cc @stanhu @DouweM @rdavila 

See merge request !5859

5 files changed, 15 insertions, 15 deletions

diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js
index 1bba69a255a..4e2273f5aa8 100644
--- a/app/assets/javascripts/merge_request_tabs.js
+++ b/app/assets/javascripts/merge_request_tabs.js
@@ -15,6 +15,7 @@
 
     function MergeRequestTabs(opts) {
       this.opts = opts != null ? opts : {};
+      this.opts.setUrl = this.opts.setUrl !== undefined ? this.opts.setUrl : true;
       this.setCurrentAction = bind(this.setCurrentAction, this);
       this.tabShown = bind(this.tabShown, this);
       this.showTab = bind(this.showTab, this);
@@ -58,7 +59,9 @@
       } else {
         this.expandView();
       }
-      return this.setCurrentAction(action);
+      if (this.opts.setUrl) {
+        this.setCurrentAction(action);
+      }
     };
 
     MergeRequestTabs.prototype.scrollToElement = function(container) {
diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb
index 996909a28c6..91315a07deb 100644
--- a/app/controllers/projects/application_controller.rb
+++ b/app/controllers/projects/application_controller.rb
@@ -83,6 +83,7 @@ class Projects::ApplicationController < ApplicationController
   end
 
   def apply_diff_view_cookie!
+    @show_changes_tab = params[:view].present?
     cookies.permanent[:diff_view] = params.delete(:view) if params[:view].present?
   end
 
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 00a3022cbf7..4c1f38be9f3 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -216,7 +216,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @base_commit = @merge_request.diff_base_commit
     @diffs = @merge_request.diffs(diff_options) if @merge_request.compare
     @diff_notes_disabled = true
-
     @pipeline = @merge_request.pipeline
     @statuses = @pipeline.statuses.relevant if @pipeline
 
diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml
index 598bd743676..00bd4e143df 100644
--- a/app/views/projects/merge_requests/_new_submit.html.haml
+++ b/app/views/projects/merge_requests/_new_submit.html.haml
@@ -20,7 +20,7 @@
 .mr-compare.merge-request
   %ul.merge-request-tabs.nav-links.no-top.no-bottom
     %li.commits-tab
-      = link_to url_for(params), data: {target: 'div#commits', action: 'commits', toggle: 'tab'} do
+      = link_to url_for(params), data: {target: 'div#commits', action: 'new', toggle: 'tab'} do
         Commits
         %span.badge= @commits.size
     - if @pipeline
@@ -52,11 +52,8 @@
     $('#merge_request_assignee_id').val("#{current_user.id}").trigger("change");
     e.preventDefault();
   });
-
 :javascript
-  var merge_request
-  merge_request = new MergeRequest({
-    action: 'new',
-    diffs_loaded: true,
-    commits_loaded: true
+  var merge_request = new MergeRequest({
+    action: "#{(@show_changes_tab ? 'diffs' : 'new')}",
+    setUrl: false
   });
diff --git a/spec/features/merge_requests/create_new_mr_spec.rb b/spec/features/merge_requests/create_new_mr_spec.rb
index b63931d9d35..b963d1305b5 100644
--- a/spec/features/merge_requests/create_new_mr_spec.rb
+++ b/spec/features/merge_requests/create_new_mr_spec.rb
@@ -49,14 +49,14 @@ feature 'Create New Merge Request', feature: true, js: true do
 
     click_link 'Changes'
 
-    expect(page.find_link('Inline')[:class]).to match(/\bactive\b/)
-    expect(page.find_link('Side-by-side')[:class]).not_to match(/\bactive\b/)
+    expect(page).to have_css('a.btn.active', text: 'Inline')
+    expect(page).not_to have_css('a.btn.active', text: 'Side-by-side')
 
     click_link 'Side-by-side'
 
-    click_link 'Changes'
-
-    expect(page.find_link('Inline')[:class]).not_to match(/\bactive\b/)
-    expect(page.find_link('Side-by-side')[:class]).to match(/\bactive\b/)
+    within '.merge-request' do
+      expect(page).not_to have_css('a.btn.active', text: 'Inline')
+      expect(page).to have_css('a.btn.active', text: 'Side-by-side')
+    end
   end
 end