Add MergeRequestTabs specsrs-dev-issue-2355

3 files changed, 193 insertions, 44 deletions

diff --git a/app/assets/javascripts/merge_request_tabs.js.coffee b/app/assets/javascripts/merge_request_tabs.js.coffee
index 4daace89771..de9a4c2cc2f 100644
--- a/app/assets/javascripts/merge_request_tabs.js.coffee
+++ b/app/assets/javascripts/merge_request_tabs.js.coffee
@@ -1,42 +1,79 @@
+# MergeRequestTabs
+#
+# Handles persisting and restoring the current tab selection and lazily-loading
+# content on the MergeRequests#show page.
+#
+# ### Example Markup
+#
+#   <ul class="nav nav-tabs merge-request-tabs">
+#     <li class="notes-tab active">
+#       <a data-action="notes" data-target="#notes" data-toggle="tab" href="/foo/bar/merge_requests/1">
+#         Discussion
+#       </a>
+#     </li>
+#     <li class="commits-tab">
+#       <a data-action="commits" data-target="#commits" data-toggle="tab" href="/foo/bar/merge_requests/1/commits">
+#         Commits
+#       </a>
+#     </li>
+#     <li class="diffs-tab">
+#       <a data-action="diffs" data-target="#diffs" data-toggle="tab" href="/foo/bar/merge_requests/1/diffs">
+#         Diffs
+#       </a>
+#     </li>
+#   </ul>
+#
+#   <div class="tab-content">
+#     <div class="notes tab-pane active" id="notes">
+#       Notes Content
+#     </div>
+#     <div class="commits tab-pane" id="commits">
+#       Commits Content
+#     </div>
+#     <div class="diffs tab-pane" id="diffs">
+#       Diffs Content
+#     </div>
+#   </div>
+#
+#   <div class="mr-loading-status">
+#     <div class="loading">
+#       Loading Animation
+#     </div>
+#   </div>
+#
 class @MergeRequestTabs
   diffsLoaded: false
   commitsLoaded: false
 
-  constructor: (@opts) ->
+  constructor: (@opts = {}) ->
+    # Store the `location` object, allowing for easier stubbing in tests
+    @_location = location
+
     @bindEvents()
-    @activateTabFromPath()
+    @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"]', @tabShow
+    $(document).on 'shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', @tabShown
 
-  tabShow: (event) =>
+  tabShown: (event) =>
     $target = $(event.target)
     action = $target.data('action')
 
-    # Lazy-load commits
-    if action == 'commits' && !@commitsLoaded
-      @loadCommits()
-
-    # Lazy-load diffs
-    if action == 'diffs' && !@diffsLoaded
-      @loadDiff()
+    if action == 'commits'
+      @loadCommits($target.attr('href'))
+    else if action == 'diffs'
+      @loadDiff($target.attr('href'))
 
     @setCurrentAction(action)
 
-  # Activate a tab based on the current URL path
-  #
-  # If the current action is 'show' or 'new' (i.e., initial page load),
-  # activates the first tab, otherwise activates the tab corresponding to the
-  # current action (diffs, commits).
-  activateTabFromPath: ->
-    if @opts.action == 'show' || @opts.action == 'new'
-      $('.merge-request-tabs a[data-toggle="tab"]:first').tab('show')
-    else
-      $(".merge-request-tabs a[data-action='#{@opts.action}']").tab('show')
+  # Activate a tab based on the current action
+  activateTab: (action) ->
+    action = 'notes' if action == 'show'
+    $(".merge-request-tabs a[data-action='#{action}']").tab('show')
 
   # Replaces the current Merge Request-specific action in the URL with a new one
   #
@@ -56,19 +93,21 @@ class @MergeRequestTabs
   #   location.pathname # => "/namespace/project/merge_requests/1/diffs"
   #   setCurrentAction('commits')
   #   location.pathname # => "/namespace/project/merge_requests/1/commits"
-  setCurrentAction: (action) ->
+  #
+  # Returns the new URL String
+  setCurrentAction: (action) =>
     # Normalize action, just to be safe
     action = 'notes' if action == 'show'
 
     # Remove a trailing '/commits' or '/diffs'
-    new_state = location.pathname.replace(/\/(commits|diffs)\/?$/, '')
+    new_state = @_location.pathname.replace(/\/(commits|diffs)\/?$/, '')
 
     # Append the new action if we're on a tab other than 'notes'
     unless action == 'notes'
       new_state += "/#{action}"
 
     # Ensure parameters and hash come along for the ride
-    new_state += location.search + location.hash
+    new_state += @_location.search + @_location.hash
 
     # Replace the current history state with the new one without breaking
     # Turbolinks' history.
@@ -76,33 +115,39 @@ class @MergeRequestTabs
     # See https://github.com/rails/turbolinks/issues/363
     history.replaceState {turbolinks: true, url: new_state}, document.title, new_state
 
-  loadCommits: ->
-    $.ajax
-      type: 'GET'
-      dataType: 'json'
-      url: $('.merge-request-tabs .commits-tab a').attr('href') + ".json"
-      beforeSend: @toggleLoading
-      complete: =>
-        @commits_loaded = true
-        @toggleLoading()
+    new_state
+
+  loadCommits: (source) ->
+    return if @commitsLoaded
+
+    @_get
+      url: "#{source}.json"
       success: (data) =>
         document.getElementById('commits').innerHTML = data.html
-
         $('.js-timeago').timeago()
+        @commitsLoaded = true
 
-  loadDiff: ->
-    $.ajax
-      type: 'GET'
-      dataType: 'json'
-      url: $('.merge-request-tabs .diffs-tab a').attr('href') + ".json"
-      beforeSend: => @toggleLoading()
-      complete: =>
-        @diffs_loaded = true
-        @toggleLoading()
+  loadDiff: (source) ->
+    return if @diffsLoaded
+
+    @_get
+      url: "#{source}.json"
       success: (data) =>
         document.getElementById('diffs').innerHTML = data.html
-
         $('.diff-header').trigger('sticky_kit:recalc')
+        @diffsLoaded = true
 
   toggleLoading: ->
     $('.mr-loading-status .loading').toggle()
+
+  _get: (options) ->
+    defaults = {
+      beforeSend: @toggleLoading
+      complete: @toggleLoading
+      dataType: 'json'
+      type: 'GET'
+    }
+
+    options = $.extend({}, defaults, options)
+
+    $.ajax(options)
diff --git a/spec/javascripts/fixtures/merge_request_tabs.html.haml b/spec/javascripts/fixtures/merge_request_tabs.html.haml
new file mode 100644
index 00000000000..7624a713948
--- /dev/null
+++ b/spec/javascripts/fixtures/merge_request_tabs.html.haml
@@ -0,0 +1,22 @@
+%ul.nav.nav-tabs.merge-request-tabs
+  %li.notes-tab
+    %a{href: '/foo/bar/merge_requests/1', data: {target: '#notes', action: 'notes', toggle: 'tab'}}
+      Discussion
+  %li.commits-tab
+    %a{href: '/foo/bar/merge_requests/1/commits', data: {target: '#commits', action: 'commits', toggle: 'tab'}}
+      Commits
+  %li.diffs-tab
+    %a{href: '/foo/bar/merge_requests/1/diffs', data: {target: '#diffs', action: 'diffs', toggle: 'tab'}}
+      Diffs
+
+.tab-content
+  #notes.notes.tab-pane
+    Notes Content
+  #commits.commits.tab-pane
+    Commits Content
+  #diffs.diffs.tab-pane
+    Diffs Content
+
+.mr-loading-status
+  .loading
+    Loading Animation
diff --git a/spec/javascripts/merge_request_tabs_spec.js.coffee b/spec/javascripts/merge_request_tabs_spec.js.coffee
new file mode 100644
index 00000000000..6cc96fb68a0
--- /dev/null
+++ b/spec/javascripts/merge_request_tabs_spec.js.coffee
@@ -0,0 +1,82 @@
+#= require merge_request_tabs
+
+describe 'MergeRequestTabs', ->
+  stubLocation = (stubs) ->
+    defaults = {pathname: '', search: '', hash: ''}
+    $.extend(defaults, stubs)
+
+  fixture.preload('merge_request_tabs.html')
+
+  beforeEach ->
+    @class = new MergeRequestTabs()
+    @spies = {
+      ajax:    spyOn($, 'ajax').and.callFake ->
+      history: spyOn(history, 'replaceState').and.callFake ->
+    }
+
+  describe '#activateTab', ->
+    beforeEach ->
+      fixture.load('merge_request_tabs.html')
+      @subject = @class.activateTab
+
+    it 'shows the first tab when action is show', ->
+      @subject('show')
+      expect($('#notes')).toHaveClass('active')
+
+    it 'shows the notes tab when action is notes', ->
+      @subject('notes')
+      expect($('#notes')).toHaveClass('active')
+
+    it 'shows the commits tab when action is commits', ->
+      @subject('commits')
+      expect($('#commits')).toHaveClass('active')
+
+    it 'shows the diffs tab when action is diffs', ->
+      @subject('diffs')
+      expect($('#diffs')).toHaveClass('active')
+
+  describe '#setCurrentAction', ->
+    beforeEach ->
+      @subject = @class.setCurrentAction
+
+    it 'changes from commits', ->
+      @class._location = stubLocation(pathname: '/foo/bar/merge_requests/1/commits')
+
+      expect(@subject('notes')).toBe('/foo/bar/merge_requests/1')
+      expect(@subject('diffs')).toBe('/foo/bar/merge_requests/1/diffs')
+
+    it 'changes from diffs', ->
+      @class._location = stubLocation(pathname: '/foo/bar/merge_requests/1/diffs')
+
+      expect(@subject('notes')).toBe('/foo/bar/merge_requests/1')
+      expect(@subject('commits')).toBe('/foo/bar/merge_requests/1/commits')
+
+    it 'changes from notes', ->
+      @class._location = stubLocation(pathname: '/foo/bar/merge_requests/1')
+
+      expect(@subject('diffs')).toBe('/foo/bar/merge_requests/1/diffs')
+      expect(@subject('commits')).toBe('/foo/bar/merge_requests/1/commits')
+
+    it 'includes search parameters and hash string', ->
+      @class._location = stubLocation({
+        pathname: '/foo/bar/merge_requests/1/diffs'
+        search:   '?view=parallel'
+        hash:     '#L15-35'
+      })
+
+      expect(@subject('show')).toBe('/foo/bar/merge_requests/1?view=parallel#L15-35')
+
+    it 'replaces the current history state', ->
+      @class._location = stubLocation(pathname: '/foo/bar/merge_requests/1')
+      new_state = @subject('commits')
+
+      expect(@spies.history).toHaveBeenCalledWith(
+        {turbolinks: true, url: new_state},
+        document.title,
+        new_state
+      )
+
+    it 'treats "show" like "notes"', ->
+      @class._location = stubLocation(pathname: '/foo/bar/merge_requests/1/commits')
+
+      expect(@subject('show')).toBe('/foo/bar/merge_requests/1')