Commit 0279a727 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'issue_38418' into 'master'

Fix sidebar issue count

Closes #38418

See merge request gitlab-org/gitlab-ce!19022
parents 6d2a6139 aee2b235
...@@ -1431,8 +1431,8 @@ class Project < ActiveRecord::Base ...@@ -1431,8 +1431,8 @@ class Project < ActiveRecord::Base
self.runners_token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.runners_token) self.runners_token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.runners_token)
end end
def open_issues_count def open_issues_count(current_user = nil)
Projects::OpenIssuesCountService.new(self).count Projects::OpenIssuesCountService.new(self, current_user).count
end end
def open_merge_requests_count def open_merge_requests_count
......
...@@ -2,14 +2,42 @@ module Projects ...@@ -2,14 +2,42 @@ module Projects
# Service class for counting and caching the number of open issues of a # Service class for counting and caching the number of open issues of a
# project. # project.
class OpenIssuesCountService < Projects::CountService class OpenIssuesCountService < Projects::CountService
include Gitlab::Utils::StrongMemoize
def initialize(project, user = nil)
@user = user
super(project)
end
def cache_key_name 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 end
def self.query(project_ids) # We only show total issues count for reporters
# We don't include confidential issues in this number since this would # which are allowed to view confidential issues
# expose the number of confidential issues to non project members. # This will still show a discrepancy on issues number but should be less than before.
Issue.opened.public_only.where(project: project_ids) # 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 end
end end
...@@ -89,7 +89,7 @@ ...@@ -89,7 +89,7 @@
= _('Issues') = _('Issues')
- if @project.issues_enabled? - if @project.issues_enabled?
%span.badge.count.issue_counter %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 %ul.sidebar-sub-level-items
= nav_link(controller: :issues, html_options: { class: "fly-out-top-item" } ) do = nav_link(controller: :issues, html_options: { class: "fly-out-top-item" } ) do
...@@ -98,7 +98,7 @@ ...@@ -98,7 +98,7 @@
= _('Issues') = _('Issues')
- if @project.issues_enabled? - if @project.issues_enabled?
%span.badge.count.issue_counter.fly-out-badge %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 %li.divider.fly-out-top-item
= nav_link(controller: :issues, action: :index) do = nav_link(controller: :issues, action: :index) do
= link_to project_issues_path(@project), title: 'Issues' 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 @@ require 'spec_helper' ...@@ -2,20 +2,53 @@ require 'spec_helper'
describe Projects::OpenIssuesCountService do describe Projects::OpenIssuesCountService do
describe '#count' do describe '#count' do
it 'returns the number of open issues' do let(:project) { create(:project) }
project = create(:project)
create(:issue, :opened, project: 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 end
it 'does not include confidential issues in the issue count' do context 'when user is provided' do
project = create(:project) 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) expect(described_class.new(project, user).count).to eq(1)
create(:issue, :opened, confidential: true, project: project) 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 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