Commit d5d9a74f authored by Felipe Artur's avatar Felipe Artur
Browse files

Fix sidebar issue count

parent 6b0ed6788afb
......@@ -1417,8 +1417,8 @@ def valid_runners_token?(token)
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
......
......@@ -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
......@@ -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
......
---
title: Fix issue count on sidebar
merge_request:
author:
type: other
......@@ -2,20 +2,53 @@
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
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