diff options
author | Rémy Coutable <remy@rymai.me> | 2016-02-15 15:19:23 +0100 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-02-15 15:40:24 +0100 |
commit | 54613b6af56008588a3cc8a9e9f2ee642ca59a36 (patch) | |
tree | 1e5c08d6ce26d4fb839302eac3911ba59f227cd9 | |
parent | c29517aaf420b0d83f21d468b371260f4887cf00 (diff) | |
download | gitlab-ce-54613b6af56008588a3cc8a9e9f2ee642ca59a36.tar.gz |
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.
-rw-r--r-- | app/helpers/application_helper.rb | 70 | ||||
-rw-r--r-- | app/helpers/issuables_helper.rb | 41 | ||||
-rw-r--r-- | app/helpers/nav_helper.rb | 18 | ||||
-rw-r--r-- | app/models/concerns/issuable.rb | 10 | ||||
-rw-r--r-- | app/views/shared/issuable/_sidebar.html.haml | 17 | ||||
-rw-r--r-- | features/project/issues/issues.feature | 9 | ||||
-rw-r--r-- | features/project/merge_requests.feature | 1 | ||||
-rw-r--r-- | features/steps/project/issues/issues.rb | 5 | ||||
-rw-r--r-- | features/steps/shared/issuable.rb | 24 |
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 |