Fix the "x of y" displayed at the top of Issuables' sidebarfix/13356-issuable-index-of-total-in-sidebar

1. We now display the index of the current issuable among all its project's
issuables, of the same type and with the same state.
2. Also, refactored a bit the Issuable helpers into a new IssuablesHelper
module.
3. Added acceptance specs for the sidebar counter.

9 files changed, 99 insertions, 96 deletions

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 02357e2f23e..ecefa9b006d 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -280,76 +280,6 @@ module ApplicationHelper
     end
   end
 
-  def issuable_link_next(project,issuable)
-    if project.nil?
-      nil
-    elsif current_controller?(:issues)
-      namespace_project_issue_path(project.namespace, project, next_issuable_for(project, issuable.id).try(:iid))
-    elsif current_controller?(:merge_requests)
-      namespace_project_merge_request_path(project.namespace, project, next_issuable_for(project, issuable.id).try(:iid))
-    end
-  end
-
-  def issuable_link_prev(project,issuable)
-    if project.nil?
-      nil
-    elsif current_controller?(:issues)
-      namespace_project_issue_path(project.namespace, project, prev_issuable_for(project, issuable.id).try(:iid))
-    elsif current_controller?(:merge_requests)
-      namespace_project_merge_request_path(project.namespace, project, prev_issuable_for(project, issuable.id).try(:iid))
-    end
-  end
-
-  def issuable_count(entity, project)
-    if project.nil?
-      0
-    elsif current_controller?(:issues)
-      project.issues.send(entity).count
-    elsif current_controller?(:merge_requests)
-      project.merge_requests.send(entity).count
-    end
-  end
-
-  def next_issuable_for(project, id)
-    if project.nil?
-      nil
-    elsif current_controller?(:issues)
-      project.issues.where("id > ?", id).last
-    elsif current_controller?(:merge_requests)
-      project.merge_requests.where("id > ?", id).last
-    end
-  end
-
-  def has_next_issuable?(project, id)
-    if project.nil?
-      nil
-    elsif current_controller?(:issues)
-      project.issues.where("id > ?", id).last
-    elsif current_controller?(:merge_requests)
-      project.merge_requests.where("id > ?", id).last
-    end
-  end
-
-  def prev_issuable_for(project, id)
-    if project.nil?
-      nil
-    elsif current_controller?(:issues)
-      project.issues.where("id < ?", id).first
-    elsif current_controller?(:merge_requests)
-      project.merge_requests.where("id < ?", id).first
-    end
-  end
-
-  def has_prev_issuable?(project, id)
-    if project.nil?
-      nil
-    elsif current_controller?(:issues)
-      project.issues.where("id < ?", id).first
-    elsif current_controller?(:merge_requests)
-      project.merge_requests.where("id < ?", id).first
-    end
-  end
-
   def state_filters_text_for(entity, project)
     titles = {
       opened: "Open"
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
new file mode 100644
index 00000000000..ccbd93967a2
--- /dev/null
+++ b/app/helpers/issuables_helper.rb
@@ -0,0 +1,41 @@
+module IssuablesHelper
+
+  def sidebar_gutter_toggle_icon
+    sidebar_gutter_collapsed? ? icon('angle-double-left') : icon('angle-double-right')
+  end
+
+  def sidebar_gutter_collapsed_class
+    "right-sidebar-#{sidebar_gutter_collapsed? ? 'collapsed' : 'expanded'}"
+  end
+
+  def issuable_index(issuable)
+    base_issuable_scope(issuable).where('id < ?', issuable.id).size + 1
+  end
+
+  def issuables_count(issuable)
+    base_issuable_scope(issuable).size
+  end
+
+  def next_issuable_for(issuable)
+    base_issuable_scope(issuable).where('id > ?', issuable.id).last
+  end
+
+  def prev_issuable_for(issuable)
+    base_issuable_scope(issuable).where('id < ?', issuable.id).first
+  end
+
+  private
+
+  def sidebar_gutter_collapsed?
+    cookies[:collapsed_gutter] == 'true'
+  end
+
+  def base_issuable_scope(issuable)
+    issuable.project.send(issuable.to_scope_name).send(issuable_state_scope(issuable))
+  end
+
+  def issuable_state_scope(issuable)
+    issuable.open? ? :opened : :closed
+  end
+
+end
diff --git a/app/helpers/nav_helper.rb b/app/helpers/nav_helper.rb
index 75f2ed5e054..29cb753e62c 100644
--- a/app/helpers/nav_helper.rb
+++ b/app/helpers/nav_helper.rb
@@ -3,18 +3,6 @@ module NavHelper
     cookies[:collapsed_nav] == 'true'
   end
 
-  def sidebar_gutter_collapsed_class
-    if cookies[:collapsed_gutter] == 'true'
-      "right-sidebar-collapsed"
-    else
-      "right-sidebar-expanded"
-    end
-  end
-
-  def sidebar_gutter_collapsed?
-    cookies[:collapsed_gutter] == 'true'
-  end
-
   def nav_sidebar_class
     if nav_menu_collapsed?
       "sidebar-collapsed"
@@ -32,9 +20,9 @@ module NavHelper
   end
 
   def page_gutter_class
-    if current_path?('merge_requests#show') || 
-      current_path?('merge_requests#diffs') || 
-      current_path?('merge_requests#commits') || 
+    if current_path?('merge_requests#show') ||
+      current_path?('merge_requests#diffs') ||
+      current_path?('merge_requests#commits') ||
       current_path?('issues#show')
       if cookies[:collapsed_gutter] == 'true'
         "page-gutter right-sidebar-collapsed"
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index cf6aa592e2a..e08687135d6 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -168,6 +168,16 @@ module Issuable
     self.class.to_s.underscore
   end
 
+  # Convert this Issuable class name to a format usable for scoping
+  #
+  # Examples:
+  #
+  #   issuable.class         # => MergeRequest
+  #   issuable.to_scope_name # => "merge_requests"
+  def to_scope_name
+    self.class.to_s.tableize
+  end
+
   # Returns a Hash of attributes to be used for Twitter card metadata
   def card_attributes
     {
diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml
index ae96a45453f..cb5378964bc 100644
--- a/app/views/shared/issuable/_sidebar.html.haml
+++ b/app/views/shared/issuable/_sidebar.html.haml
@@ -2,23 +2,20 @@
   .issuable-sidebar
     .block
       %span.issuable-count.pull-left
-        = issuable.iid
+        = issuable_index(issuable)
         of
-        = issuable_count(:all, @project)
+        = issuables_count(issuable)
       %span.pull-right
         %a.gutter-toggle{href: '#'}
-          - if sidebar_gutter_collapsed?
-            = icon('angle-double-left')
-          - else
-            = icon('angle-double-right')
+          = sidebar_gutter_toggle_icon
       .issuable-nav.pull-right.btn-group{role: 'group', "aria-label" => '...'}
-        - if has_prev_issuable?(@project, issuable.id)
-          = link_to 'Prev', issuable_link_prev(@project, issuable), class: 'btn btn-default prev-btn'
+        - if prev_issuable = prev_issuable_for(issuable)
+          = link_to 'Prev', [@project.namespace.becomes(Namespace), @project, prev_issuable], class: 'btn btn-default prev-btn'
         - else
           %a.btn.btn-default.disabled{href: '#'}
             Prev
-        - if has_next_issuable?(@project, issuable.id)
-          = link_to 'Next', issuable_link_next(@project, issuable), class: 'btn btn-default next-btn'
+        - if next_issuable = next_issuable_for(issuable)
+          = link_to 'Next', [@project.namespace.becomes(Namespace), @project, next_issuable], class: 'btn btn-default next-btn'
         - else
           %a.btn.btn-default.disabled{href: '#'}
             Next
diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature
index 0b3d03aa2a5..ca2399d85a9 100644
--- a/features/project/issues/issues.feature
+++ b/features/project/issues/issues.feature
@@ -25,9 +25,16 @@ Feature: Project Issues
   Scenario: I visit issue page
     Given I click link "Release 0.4"
     Then I should see issue "Release 0.4"
+    And I should see "1 of 2" in the sidebar
+
+  Scenario: I navigate between issues
+    Given I click link "Release 0.4"
+    Then I click link "Next" in the sidebar
+    Then I should see issue "Tweet control"
+    And I should see "2 of 2" in the sidebar
 
   @javascript
-  Scenario: I visit issue page
+  Scenario: I filter by author
     Given I add a user to project "Shop"
     And I click "author" dropdown
     Then I see current user as the first user
diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature
index ca1ee6b3c2b..5995e787961 100644
--- a/features/project/merge_requests.feature
+++ b/features/project/merge_requests.feature
@@ -39,6 +39,7 @@ Feature: Project Merge Requests
   Scenario: I visit merge request page
     Given I click link "Bug NS-04"
     Then I should see merge request "Bug NS-04"
+    And I should see "1 of 1" in the sidebar
 
   Scenario: I close merge request page
     Given I click link "Bug NS-04"
diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb
index d556b73f9fd..09a89e99831 100644
--- a/features/steps/project/issues/issues.rb
+++ b/features/steps/project/issues/issues.rb
@@ -54,6 +54,10 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps
     expect(page).to have_content "Release 0.4"
   end
 
+  step 'I should see issue "Tweet control"' do
+    expect(page).to have_content "Tweet control"
+  end
+
   step 'I click link "New Issue"' do
     click_link "New Issue"
   end
@@ -301,4 +305,5 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps
   def filter_issue(text)
     fill_in 'issue_search', with: text
   end
+
 end
diff --git a/features/steps/shared/issuable.rb b/features/steps/shared/issuable.rb
index 25c2b476f43..2117feaedb8 100644
--- a/features/steps/shared/issuable.rb
+++ b/features/steps/shared/issuable.rb
@@ -119,6 +119,24 @@ module SharedIssuable
     end
   end
 
+  step 'I should see "1 of 1" in the sidebar' do
+    expect_sidebar_content('1 of 1')
+  end
+
+  step 'I should see "1 of 2" in the sidebar' do
+    expect_sidebar_content('1 of 2')
+  end
+
+  step 'I should see "2 of 2" in the sidebar' do
+    expect_sidebar_content('2 of 2')
+  end
+
+  step 'I click link "Next" in the sidebar' do
+    page.within '.issuable-sidebar' do
+      click_link 'Next'
+    end
+  end
+
   def create_issuable_for_project(project_name:, title:, type: :issue)
     project = Project.find_by(name: project_name)
 
@@ -159,4 +177,10 @@ module SharedIssuable
     expect(page).to have_content("mentioned in #{issuable.class.to_s.titleize.downcase} #{issuable.to_reference(project)}")
   end
 
+  def expect_sidebar_content(content)
+    page.within '.issuable-sidebar' do
+      expect(page).to have_content content
+    end
+  end
+
 end