Merge branch 'rs-issue-1887' into 'master'

Reorder MergeRequestTabs constructor

The tab was being activated before we set the `[commits|diffs]Loaded`
variable, so even when the `/diffs` route is accessed directly, like
from the "Side-by-side" link, the tab was being loaded from its default
source.

Fixes #1887

See merge request !892

4 files changed, 27 insertions, 28 deletions

diff --git a/app/assets/javascripts/merge_request_tabs.js.coffee b/app/assets/javascripts/merge_request_tabs.js.coffee
index de9a4c2cc2f..5dc441f64b5 100644
--- a/app/assets/javascripts/merge_request_tabs.js.coffee
+++ b/app/assets/javascripts/merge_request_tabs.js.coffee
@@ -49,13 +49,15 @@ class @MergeRequestTabs
     # Store the `location` object, allowing for easier stubbing in tests
     @_location = location
 
+    switch @opts.action
+      when 'commits'
+        @commitsLoaded = true
+      when 'diffs'
+        @diffsLoaded = true
+
     @bindEvents()
     @activateTab(@opts.action)
 
-    switch @opts.action
-      when 'commits' then @commitsLoaded = true
-      when 'diffs'   then @diffsLoaded = true
-
   bindEvents: ->
     $(document).on 'shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', @tabShown
 
@@ -67,6 +69,7 @@ class @MergeRequestTabs
       @loadCommits($target.attr('href'))
     else if action == 'diffs'
       @loadDiff($target.attr('href'))
+      @stickyDiffHeaders()
 
     @setCurrentAction(action)
 
@@ -134,12 +137,15 @@ class @MergeRequestTabs
       url: "#{source}.json"
       success: (data) =>
         document.getElementById('diffs').innerHTML = data.html
-        $('.diff-header').trigger('sticky_kit:recalc')
+        @stickyDiffHeaders()
         @diffsLoaded = true
 
   toggleLoading: ->
     $('.mr-loading-status .loading').toggle()
 
+  stickyDiffHeaders: ->
+    $('.diff-header').trigger('sticky_kit:recalc')
+
   _get: (options) ->
     defaults = {
       beforeSend: @toggleLoading
diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature
index 35ffa9fc6e1..947f668e432 100644
--- a/features/project/merge_requests.feature
+++ b/features/project/merge_requests.feature
@@ -63,7 +63,7 @@ Feature: Project Merge Requests
   Scenario: I comment on a merge request diff
     Given project "Shop" have "Bug NS-05" open merge request with diffs inside
     And I visit merge request page "Bug NS-05"
-    And I switch to the diff tab
+    And I click on the Changes tab
     And I leave a comment like "Line is wrong" on diff
     And I switch to the merge request's comments tab
     Then I should see a discussion has started on diff
@@ -114,7 +114,7 @@ Feature: Project Merge Requests
   Scenario: I hide comments on a merge request diff with comments in a single file
     Given project "Shop" have "Bug NS-05" open merge request with diffs inside
     And I visit merge request page "Bug NS-05"
-    And I switch to the diff tab
+    And I click on the Changes tab
     And I leave a comment like "Line is wrong" on line 39 of the second file
     And I click link "Hide inline discussion" of the second file
     Then I should not see a comment like "Line is wrong here" in the second file
@@ -123,7 +123,7 @@ Feature: Project Merge Requests
   Scenario: I show comments on a merge request diff with comments in a single file
     Given project "Shop" have "Bug NS-05" open merge request with diffs inside
     And I visit merge request page "Bug NS-05"
-    And I switch to the diff tab
+    And I click on the Changes tab
     And I leave a comment like "Line is wrong" on line 39 of the second file
     Then I should see a comment like "Line is wrong" in the second file
 
@@ -131,7 +131,7 @@ Feature: Project Merge Requests
   Scenario: I hide comments on a merge request diff with comments in multiple files
     Given project "Shop" have "Bug NS-05" open merge request with diffs inside
     And I visit merge request page "Bug NS-05"
-    And I switch to the diff tab
+    And I click on the Changes tab
     And I leave a comment like "Line is correct" on line 12 of the first file
     And I leave a comment like "Line is wrong" on line 39 of the second file
     And I click link "Hide inline discussion" of the second file
@@ -142,7 +142,7 @@ Feature: Project Merge Requests
   Scenario: I show comments on a merge request diff with comments in multiple files
     Given project "Shop" have "Bug NS-05" open merge request with diffs inside
     And I visit merge request page "Bug NS-05"
-    And I switch to the diff tab
+    And I click on the Changes tab
     And I leave a comment like "Line is correct" on line 12 of the first file
     And I leave a comment like "Line is wrong" on line 39 of the second file
     And I click link "Hide inline discussion" of the second file
@@ -154,7 +154,7 @@ Feature: Project Merge Requests
   Scenario: I unfold diff
     Given project "Shop" have "Bug NS-05" open merge request with diffs inside
     And I visit merge request page "Bug NS-05"
-    And I switch to the diff tab
+    And I click on the Changes tab
     And I unfold diff
     Then I should see additional file lines
 
@@ -162,7 +162,7 @@ Feature: Project Merge Requests
   Scenario: I show comments on a merge request side-by-side diff with comments in multiple files
     Given project "Shop" have "Bug NS-05" open merge request with diffs inside
     And I visit merge request page "Bug NS-05"
-    And I switch to the diff tab
+    And I click on the Changes tab
     And I leave a comment like "Line is correct" on line 12 of the first file
     And I leave a comment like "Line is wrong" on line 39 of the second file
     And I click Side-by-side Diff tab
@@ -172,7 +172,7 @@ Feature: Project Merge Requests
   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
+    And I click on the Changes tab
     Then I should see the proper Inline and Side-by-side links
 
   # Description preview
diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb
index f11edb659d5..7cfde37de55 100644
--- a/features/steps/project/merge_requests.rb
+++ b/features/steps/project/merge_requests.rb
@@ -118,25 +118,17 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
            author: project.users.first)
   end
 
-  step 'I switch to the diff tab' do
-    visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)
-  end
-
-  step 'I click on the Changes tab via Javascript' do
+  step 'I click on the Changes tab' do
     page.within '.merge-request-tabs' do
       click_link 'Changes'
     end
 
-    sleep 2
+    # Waits for load
+    expect(page).to have_css('.tab-content #diffs.active')
   end
 
   step 'I should see the proper Inline and Side-by-side links' do
-    buttons = page.all('#commit-diff-viewtype')
-    expect(buttons.count).to eq(2)
-
-    buttons.each do |b|
-      expect(b['href']).not_to have_content('json')
-    end
+    expect(page).to have_css('#commit-diff-viewtype', count: 2)
   end
 
   step 'I switch to the merge request\'s comments tab' do
@@ -301,6 +293,8 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
   end
 
   step 'I unfold diff' do
+    expect(page).to have_css('.js-unfold')
+
     first('.js-unfold').click
   end
 
diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb
index 12ab4b844d8..ad37b589b84 100644
--- a/spec/features/notes_on_merge_requests_spec.rb
+++ b/spec/features/notes_on_merge_requests_spec.rb
@@ -223,8 +223,7 @@ describe 'Comments' do
     sample_compare.changes.last[:line_code]
   end
 
-  def click_diff_line(data = nil)
-    data ||= line_code
-    find("button[data-line-code=\"#{data}\"]").click
+  def click_diff_line(data = line_code)
+    page.find(%Q{button[data-line-code="#{data}"]}, visible: false).click
   end
 end