diff options
author | Rémy Coutable <remy@rymai.me> | 2018-05-24 19:23:13 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2018-05-24 19:23:13 +0300 |
commit | 0279a72752e17d773c81c46c602204c318fd1ecd (patch) | |
tree | ddf3acb6fb6fdd296481c83f262bced2168a7621 | |
parent | 6d2a6139721df93c633c58c24ed9a88ac59a080b (diff) | |
parent | aee2b235499f9ea179aed82d6beb8ea955986aa3 (diff) |
Merge branch 'issue_38418' into 'master'
Fix sidebar issue count
Closes #38418
See merge request gitlab-org/gitlab-ce!19022
-rw-r--r-- | app/models/project.rb | 4 | ||||
-rw-r--r-- | app/services/projects/open_issues_count_service.rb | 38 | ||||
-rw-r--r-- | app/views/layouts/nav/sidebar/_project.html.haml | 4 | ||||
-rw-r--r-- | changelogs/unreleased/issue_38418.yml | 5 | ||||
-rw-r--r-- | spec/services/projects/open_issues_count_service_spec.rb | 51 |
5 files changed, 84 insertions, 18 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index 0e727664d39..0fe9f8880b4 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1431,8 +1431,8 @@ class Project < ActiveRecord::Base self.runners_token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.runners_token) end - def open_issues_count - Projects::OpenIssuesCountService.new(self).count + def open_issues_count(current_user = nil) + Projects::OpenIssuesCountService.new(self, current_user).count end def open_merge_requests_count diff --git a/app/services/projects/open_issues_count_service.rb b/app/services/projects/open_issues_count_service.rb index a975a06a05c..0a004677417 100644 --- a/app/services/projects/open_issues_count_service.rb +++ b/app/services/projects/open_issues_count_service.rb @@ -2,14 +2,42 @@ module Projects # Service class for counting and caching the number of open issues of a # project. class OpenIssuesCountService < Projects::CountService + include Gitlab::Utils::StrongMemoize + + def initialize(project, user = nil) + @user = user + + super(project) + end + def cache_key_name - 'open_issues_count' + public_only? ? 'public_open_issues_count' : 'total_open_issues_count' + end + + def public_only? + !user_is_at_least_reporter? + end + + def relation_for_count + self.class.query(@project, public_only: public_only?) + end + + def user_is_at_least_reporter? + strong_memoize(:user_is_at_least_reporter) do + @user && @project.team.member?(@user, Gitlab::Access::REPORTER) + end end - def self.query(project_ids) - # We don't include confidential issues in this number since this would - # expose the number of confidential issues to non project members. - Issue.opened.public_only.where(project: project_ids) + # We only show total issues count for reporters + # which are allowed to view confidential issues + # This will still show a discrepancy on issues number but should be less than before. + # Check https://gitlab.com/gitlab-org/gitlab-ce/issues/38418 description. + def self.query(projects, public_only: true) + if public_only + Issue.opened.public_only.where(project: projects) + else + Issue.opened.where(project: projects) + end end end end diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index f3af15d771d..99e4113bd5a 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -89,7 +89,7 @@ = _('Issues') - if @project.issues_enabled? %span.badge.count.issue_counter - = number_with_delimiter(@project.open_issues_count) + = number_with_delimiter(@project.open_issues_count(current_user)) %ul.sidebar-sub-level-items = nav_link(controller: :issues, html_options: { class: "fly-out-top-item" } ) do @@ -98,7 +98,7 @@ = _('Issues') - if @project.issues_enabled? %span.badge.count.issue_counter.fly-out-badge - = number_with_delimiter(@project.open_issues_count) + = number_with_delimiter(@project.open_issues_count(current_user)) %li.divider.fly-out-top-item = nav_link(controller: :issues, action: :index) do = link_to project_issues_path(@project), title: 'Issues' do diff --git a/changelogs/unreleased/issue_38418.yml b/changelogs/unreleased/issue_38418.yml new file mode 100644 index 00000000000..79452b27e4b --- /dev/null +++ b/changelogs/unreleased/issue_38418.yml @@ -0,0 +1,5 @@ +--- +title: Fix issue count on sidebar +merge_request: +author: +type: other diff --git a/spec/services/projects/open_issues_count_service_spec.rb b/spec/services/projects/open_issues_count_service_spec.rb index f964f9972cd..06b470849b3 100644 --- a/spec/services/projects/open_issues_count_service_spec.rb +++ b/spec/services/projects/open_issues_count_service_spec.rb @@ -2,20 +2,53 @@ require 'spec_helper' describe Projects::OpenIssuesCountService do describe '#count' do - it 'returns the number of open issues' do - project = create(:project) - create(:issue, :opened, project: project) + let(:project) { create(:project) } - expect(described_class.new(project).count).to eq(1) + context 'when user is nil' do + it 'does not include confidential issues in the issue count' do + create(:issue, :opened, project: project) + create(:issue, :opened, confidential: true, project: project) + + expect(described_class.new(project).count).to eq(1) + end end - it 'does not include confidential issues in the issue count' do - project = create(:project) + context 'when user is provided' do + let(:user) { create(:user) } + + context 'when user can read confidential issues' do + before do + project.add_reporter(user) + end + + it 'returns the right count with confidential issues' do + create(:issue, :opened, project: project) + create(:issue, :opened, confidential: true, project: project) + + expect(described_class.new(project, user).count).to eq(2) + end + + it 'uses total_open_issues_count cache key' do + expect(described_class.new(project, user).cache_key_name).to eq('total_open_issues_count') + end + end + + context 'when user cannot read confidential issues' do + before do + project.add_guest(user) + end + + it 'does not include confidential issues' do + create(:issue, :opened, project: project) + create(:issue, :opened, confidential: true, project: project) - create(:issue, :opened, project: project) - create(:issue, :opened, confidential: true, project: project) + expect(described_class.new(project, user).count).to eq(1) + end - expect(described_class.new(project).count).to eq(1) + it 'uses public_open_issues_count cache key' do + expect(described_class.new(project, user).cache_key_name).to eq('public_open_issues_count') + end + end end end end |