Commit 54613b6a authored by Rémy Coutable's avatar Rémy Coutable

Fix the "x of y" displayed at the top of Issuables' 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.
parent c29517aa
...@@ -280,76 +280,6 @@ module ApplicationHelper ...@@ -280,76 +280,6 @@ module ApplicationHelper
end end
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) def state_filters_text_for(entity, project)
titles = { titles = {
opened: "Open" opened: "Open"
......
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
...@@ -3,18 +3,6 @@ module NavHelper ...@@ -3,18 +3,6 @@ module NavHelper
cookies[:collapsed_nav] == 'true' cookies[:collapsed_nav] == 'true'
end 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 def nav_sidebar_class
if nav_menu_collapsed? if nav_menu_collapsed?
"sidebar-collapsed" "sidebar-collapsed"
...@@ -32,9 +20,9 @@ module NavHelper ...@@ -32,9 +20,9 @@ module NavHelper
end end
def page_gutter_class def page_gutter_class
if current_path?('merge_requests#show') || if current_path?('merge_requests#show') ||
current_path?('merge_requests#diffs') || current_path?('merge_requests#diffs') ||
current_path?('merge_requests#commits') || current_path?('merge_requests#commits') ||
current_path?('issues#show') current_path?('issues#show')
if cookies[:collapsed_gutter] == 'true' if cookies[:collapsed_gutter] == 'true'
"page-gutter right-sidebar-collapsed" "page-gutter right-sidebar-collapsed"
......
...@@ -168,6 +168,16 @@ module Issuable ...@@ -168,6 +168,16 @@ module Issuable
self.class.to_s.underscore self.class.to_s.underscore
end 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 # Returns a Hash of attributes to be used for Twitter card metadata
def card_attributes def card_attributes
{ {
......
...@@ -2,23 +2,20 @@ ...@@ -2,23 +2,20 @@
.issuable-sidebar .issuable-sidebar
.block .block
%span.issuable-count.pull-left %span.issuable-count.pull-left
= issuable.iid = issuable_index(issuable)
of of
= issuable_count(:all, @project) = issuables_count(issuable)
%span.pull-right %span.pull-right
%a.gutter-toggle{href: '#'} %a.gutter-toggle{href: '#'}
- if sidebar_gutter_collapsed? = sidebar_gutter_toggle_icon
= icon('angle-double-left')
- else
= icon('angle-double-right')
.issuable-nav.pull-right.btn-group{role: 'group', "aria-label" => '...'} .issuable-nav.pull-right.btn-group{role: 'group', "aria-label" => '...'}
- if has_prev_issuable?(@project, issuable.id) - if prev_issuable = prev_issuable_for(issuable)
= link_to 'Prev', issuable_link_prev(@project, issuable), class: 'btn btn-default prev-btn' = link_to 'Prev', [@project.namespace.becomes(Namespace), @project, prev_issuable], class: 'btn btn-default prev-btn'
- else - else
%a.btn.btn-default.disabled{href: '#'} %a.btn.btn-default.disabled{href: '#'}
Prev Prev
- if has_next_issuable?(@project, issuable.id) - if next_issuable = next_issuable_for(issuable)
= link_to 'Next', issuable_link_next(@project, issuable), class: 'btn btn-default next-btn' = link_to 'Next', [@project.namespace.becomes(Namespace), @project, next_issuable], class: 'btn btn-default next-btn'
- else - else
%a.btn.btn-default.disabled{href: '#'} %a.btn.btn-default.disabled{href: '#'}
Next Next
......
...@@ -25,9 +25,16 @@ Feature: Project Issues ...@@ -25,9 +25,16 @@ Feature: Project Issues
Scenario: I visit issue page Scenario: I visit issue page
Given I click link "Release 0.4" Given I click link "Release 0.4"
Then I should see issue "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 @javascript
Scenario: I visit issue page Scenario: I filter by author
Given I add a user to project "Shop" Given I add a user to project "Shop"
And I click "author" dropdown And I click "author" dropdown
Then I see current user as the first user Then I see current user as the first user
......
...@@ -39,6 +39,7 @@ Feature: Project Merge Requests ...@@ -39,6 +39,7 @@ Feature: Project Merge Requests
Scenario: I visit merge request page Scenario: I visit merge request page
Given I click link "Bug NS-04" Given I click link "Bug NS-04"
Then I should see merge request "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 Scenario: I close merge request page
Given I click link "Bug NS-04" Given I click link "Bug NS-04"
......
...@@ -54,6 +54,10 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps ...@@ -54,6 +54,10 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps
expect(page).to have_content "Release 0.4" expect(page).to have_content "Release 0.4"
end end
step 'I should see issue "Tweet control"' do
expect(page).to have_content "Tweet control"
end
step 'I click link "New Issue"' do step 'I click link "New Issue"' do
click_link "New Issue" click_link "New Issue"
end end
...@@ -301,4 +305,5 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps ...@@ -301,4 +305,5 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps
def filter_issue(text) def filter_issue(text)
fill_in 'issue_search', with: text fill_in 'issue_search', with: text
end end
end end
...@@ -119,6 +119,24 @@ module SharedIssuable ...@@ -119,6 +119,24 @@ module SharedIssuable
end end
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) def create_issuable_for_project(project_name:, title:, type: :issue)
project = Project.find_by(name: project_name) project = Project.find_by(name: project_name)
...@@ -159,4 +177,10 @@ module SharedIssuable ...@@ -159,4 +177,10 @@ module SharedIssuable
expect(page).to have_content("mentioned in #{issuable.class.to_s.titleize.downcase} #{issuable.to_reference(project)}") expect(page).to have_content("mentioned in #{issuable.class.to_s.titleize.downcase} #{issuable.to_reference(project)}")
end end
def expect_sidebar_content(content)
page.within '.issuable-sidebar' do
expect(page).to have_content content
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment