Commit 78cccb7d authored by Phil Hughes's avatar Phil Hughes

Adds attention request count to navigation dropdown

Updates the merge request navigation count to use the
attention requests count.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/343328/
parent 96358b71
...@@ -23,7 +23,18 @@ function updateReviewerMergeRequestCounts(newCount) { ...@@ -23,7 +23,18 @@ function updateReviewerMergeRequestCounts(newCount) {
function updateMergeRequestCounts(newCount) { function updateMergeRequestCounts(newCount) {
const mergeRequestsCountEl = document.querySelector('.js-merge-requests-count'); const mergeRequestsCountEl = document.querySelector('.js-merge-requests-count');
mergeRequestsCountEl.textContent = newCount.toLocaleString(); mergeRequestsCountEl.textContent = newCount.toLocaleString();
mergeRequestsCountEl.classList.toggle('hidden', Number(newCount) === 0); mergeRequestsCountEl.classList.toggle('gl-display-none', Number(newCount) === 0);
}
function updateAttentionRequestsCount(count) {
const attentionCountEl = document.querySelector('.js-attention-count');
attentionCountEl.textContent = count.toLocaleString();
if (Number(count) === 0) {
attentionCountEl.classList.replace('badge-warning', 'badge-neutral');
} else {
attentionCountEl.classList.replace('badge-neutral', 'badge-warning');
}
} }
/** /**
...@@ -32,14 +43,22 @@ function updateMergeRequestCounts(newCount) { ...@@ -32,14 +43,22 @@ function updateMergeRequestCounts(newCount) {
export function refreshUserMergeRequestCounts() { export function refreshUserMergeRequestCounts() {
return getUserCounts() return getUserCounts()
.then(({ data }) => { .then(({ data }) => {
const attentionRequestsEnabled = window.gon?.features?.mrAttentionRequests;
const assignedMergeRequests = data.assigned_merge_requests; const assignedMergeRequests = data.assigned_merge_requests;
const reviewerMergeRequests = data.review_requested_merge_requests; const reviewerMergeRequests = data.review_requested_merge_requests;
const fullCount = assignedMergeRequests + reviewerMergeRequests; const attentionRequests = data.attention_requests;
const fullCount = attentionRequestsEnabled
? attentionRequests
: assignedMergeRequests + reviewerMergeRequests;
updateUserMergeRequestCounts(assignedMergeRequests); updateUserMergeRequestCounts(assignedMergeRequests);
updateReviewerMergeRequestCounts(reviewerMergeRequests); updateReviewerMergeRequestCounts(reviewerMergeRequests);
updateMergeRequestCounts(fullCount); updateMergeRequestCounts(fullCount);
broadcastCount(fullCount); broadcastCount(fullCount);
if (attentionRequestsEnabled) {
updateAttentionRequestsCount(attentionRequests);
}
}) })
.catch((ex) => { .catch((ex) => {
console.error(ex); // eslint-disable-line no-console console.error(ex); // eslint-disable-line no-console
......
...@@ -27,6 +27,7 @@ import { DropdownVariant } from '~/vue_shared/components/sidebar/labels_select_v ...@@ -27,6 +27,7 @@ import { DropdownVariant } from '~/vue_shared/components/sidebar/labels_select_v
import LabelsSelectWidget from '~/vue_shared/components/sidebar/labels_select_widget/labels_select_root.vue'; import LabelsSelectWidget from '~/vue_shared/components/sidebar/labels_select_widget/labels_select_root.vue';
import { LabelType } from '~/vue_shared/components/sidebar/labels_select_widget/constants'; import { LabelType } from '~/vue_shared/components/sidebar/labels_select_widget/constants';
import eventHub from '~/sidebar/event_hub'; import eventHub from '~/sidebar/event_hub';
import { refreshUserMergeRequestCounts } from '~/commons/nav/user_merge_requests';
import Translate from '../vue_shared/translate'; import Translate from '../vue_shared/translate';
import SidebarAssignees from './components/assignees/sidebar_assignees.vue'; import SidebarAssignees from './components/assignees/sidebar_assignees.vue';
import CopyEmailToClipboard from './components/copy_email_to_clipboard.vue'; import CopyEmailToClipboard from './components/copy_email_to_clipboard.vue';
...@@ -619,9 +620,10 @@ export function mountSidebar(mediator, store) { ...@@ -619,9 +620,10 @@ export function mountSidebar(mediator, store) {
mountSeverityComponent(); mountSeverityComponent();
if (window.gon?.features?.mrAttentionRequests) { if (window.gon?.features?.mrAttentionRequests) {
eventHub.$on('removeCurrentUserAttentionRequested', () => eventHub.$on('removeCurrentUserAttentionRequested', () => {
mediator.removeCurrentUserAttentionRequested(), mediator.removeCurrentUserAttentionRequested();
); refreshUserMergeRequestCounts();
});
} }
} }
......
...@@ -2,6 +2,7 @@ import Store from '~/sidebar/stores/sidebar_store'; ...@@ -2,6 +2,7 @@ import Store from '~/sidebar/stores/sidebar_store';
import createFlash from '~/flash'; import createFlash from '~/flash';
import { __, sprintf } from '~/locale'; import { __, sprintf } from '~/locale';
import toast from '~/vue_shared/plugins/global_toast'; import toast from '~/vue_shared/plugins/global_toast';
import { refreshUserMergeRequestCounts } from '~/commons/nav/user_merge_requests';
import { visitUrl } from '../lib/utils/url_utility'; import { visitUrl } from '../lib/utils/url_utility';
import Service from './services/sidebar_service'; import Service from './services/sidebar_service';
...@@ -125,6 +126,7 @@ export default class SidebarMediator { ...@@ -125,6 +126,7 @@ export default class SidebarMediator {
this.store.updateReviewer(user.id, 'attention_requested'); this.store.updateReviewer(user.id, 'attention_requested');
this.store.updateAssignee(user.id, 'attention_requested'); this.store.updateAssignee(user.id, 'attention_requested');
refreshUserMergeRequestCounts();
callback(); callback();
} catch (error) { } catch (error) {
callback(); callback();
......
...@@ -15,6 +15,10 @@ module DashboardHelper ...@@ -15,6 +15,10 @@ module DashboardHelper
merge_requests_dashboard_path(reviewer_username: current_user.username) merge_requests_dashboard_path(reviewer_username: current_user.username)
end end
def attention_requested_mrs_dashboard_path
merge_requests_dashboard_path(attention: current_user.username)
end
def dashboard_nav_links def dashboard_nav_links
@dashboard_nav_links ||= get_dashboard_nav_links @dashboard_nav_links ||= get_dashboard_nav_links
end end
......
...@@ -150,11 +150,20 @@ module MergeRequestsHelper ...@@ -150,11 +150,20 @@ module MergeRequestsHelper
review_requested_count = review_requested_merge_requests_count review_requested_count = review_requested_merge_requests_count
total_count = assigned_count + review_requested_count total_count = assigned_count + review_requested_count
{ counts = {
assigned: assigned_count, assigned: assigned_count,
review_requested: review_requested_count, review_requested: review_requested_count,
total: total_count total: total_count
} }
if Feature.enabled?(:mr_attention_requests, default_enabled: :yaml)
attention_requested_count = attention_requested_merge_requests_count
counts[:attention_requested_count] = attention_requested_count
counts[:total] = attention_requested_count
end
counts
end end
end end
...@@ -205,6 +214,10 @@ module MergeRequestsHelper ...@@ -205,6 +214,10 @@ module MergeRequestsHelper
current_user.review_requested_open_merge_requests_count current_user.review_requested_open_merge_requests_count
end end
def attention_requested_merge_requests_count
current_user.attention_requested_open_merge_requests_count
end
def default_suggestion_commit_message def default_suggestion_commit_message
@project.suggestion_commit_message.presence || Gitlab::Suggestions::CommitMessage::DEFAULT_SUGGESTION_COMMIT_MESSAGE @project.suggestion_commit_message.presence || Gitlab::Suggestions::CommitMessage::DEFAULT_SUGGESTION_COMMIT_MESSAGE
end end
......
...@@ -1692,6 +1692,12 @@ class User < ApplicationRecord ...@@ -1692,6 +1692,12 @@ class User < ApplicationRecord
end end
end end
def attention_requested_open_merge_requests_count(force: false)
Rails.cache.fetch(attention_request_cache_key, force: force, expires_in: COUNT_CACHE_VALIDITY_PERIOD) do
MergeRequestsFinder.new(self, attention: self.username, state: 'opened', non_archived: true).execute.count
end
end
def assigned_open_issues_count(force: false) def assigned_open_issues_count(force: false)
Rails.cache.fetch(['users', id, 'assigned_open_issues_count'], force: force, expires_in: COUNT_CACHE_VALIDITY_PERIOD) do Rails.cache.fetch(['users', id, 'assigned_open_issues_count'], force: force, expires_in: COUNT_CACHE_VALIDITY_PERIOD) do
IssuesFinder.new(self, assignee_id: self.id, state: 'opened', non_archived: true).execute.count IssuesFinder.new(self, assignee_id: self.id, state: 'opened', non_archived: true).execute.count
...@@ -1735,6 +1741,11 @@ class User < ApplicationRecord ...@@ -1735,6 +1741,11 @@ class User < ApplicationRecord
def invalidate_merge_request_cache_counts def invalidate_merge_request_cache_counts
Rails.cache.delete(['users', id, 'assigned_open_merge_requests_count']) Rails.cache.delete(['users', id, 'assigned_open_merge_requests_count'])
Rails.cache.delete(['users', id, 'review_requested_open_merge_requests_count']) Rails.cache.delete(['users', id, 'review_requested_open_merge_requests_count'])
invalidate_attention_requested_count
end
def invalidate_attention_requested_count
Rails.cache.delete(attention_request_cache_key)
end end
def invalidate_todos_cache_counts def invalidate_todos_cache_counts
...@@ -1746,6 +1757,10 @@ class User < ApplicationRecord ...@@ -1746,6 +1757,10 @@ class User < ApplicationRecord
Rails.cache.delete(['users', id, 'personal_projects_count']) Rails.cache.delete(['users', id, 'personal_projects_count'])
end end
def attention_request_cache_key
['users', id, 'attention_requested_open_merge_requests_count']
end
# This is copied from Devise::Models::Lockable#valid_for_authentication?, as our auth # This is copied from Devise::Models::Lockable#valid_for_authentication?, as our auth
# flow means we don't call that automatically (and can't conveniently do so). # flow means we don't call that automatically (and can't conveniently do so).
# #
......
...@@ -14,7 +14,7 @@ module MergeRequests ...@@ -14,7 +14,7 @@ module MergeRequests
create_approval_note(merge_request) create_approval_note(merge_request)
mark_pending_todos_as_done(merge_request) mark_pending_todos_as_done(merge_request)
execute_approval_hooks(merge_request, current_user) execute_approval_hooks(merge_request, current_user)
remove_attention_requested(merge_request, current_user) remove_attention_requested(merge_request)
merge_request_activity_counter.track_approve_mr_action(user: current_user) merge_request_activity_counter.track_approve_mr_action(user: current_user)
success success
......
...@@ -60,7 +60,7 @@ module MergeRequests ...@@ -60,7 +60,7 @@ module MergeRequests
merge_request_activity_counter.track_reviewers_changed_action(user: current_user) merge_request_activity_counter.track_reviewers_changed_action(user: current_user)
unless new_reviewers.include?(current_user) unless new_reviewers.include?(current_user)
remove_attention_requested(merge_request, current_user) remove_attention_requested(merge_request)
merge_request.merge_request_reviewers_with(new_reviewers).update_all(updated_state_by_user_id: current_user.id) merge_request.merge_request_reviewers_with(new_reviewers).update_all(updated_state_by_user_id: current_user.id)
end end
...@@ -253,10 +253,10 @@ module MergeRequests ...@@ -253,10 +253,10 @@ module MergeRequests
::MergeRequests::BulkRemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request, users: users.uniq).execute ::MergeRequests::BulkRemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request, users: users.uniq).execute
end end
def remove_attention_requested(merge_request, user) def remove_attention_requested(merge_request)
return unless merge_request.attention_requested_enabled? return unless merge_request.attention_requested_enabled?
::MergeRequests::RemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request, user: user).execute ::MergeRequests::RemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request).execute
end end
end end
end end
......
...@@ -19,6 +19,8 @@ module MergeRequests ...@@ -19,6 +19,8 @@ module MergeRequests
merge_request.merge_request_assignees.where(user_id: users).update_all(state: :reviewed) merge_request.merge_request_assignees.where(user_id: users).update_all(state: :reviewed)
merge_request.merge_request_reviewers.where(user_id: users).update_all(state: :reviewed) merge_request.merge_request_reviewers.where(user_id: users).update_all(state: :reviewed)
users.each { |user| user.invalidate_attention_requested_count }
success success
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
...@@ -26,7 +26,7 @@ module MergeRequests ...@@ -26,7 +26,7 @@ module MergeRequests
execute_assignees_hooks(merge_request, old_assignees) if options[:execute_hooks] execute_assignees_hooks(merge_request, old_assignees) if options[:execute_hooks]
unless new_assignees.include?(current_user) unless new_assignees.include?(current_user)
remove_attention_requested(merge_request, current_user) remove_attention_requested(merge_request)
end end
end end
......
...@@ -17,7 +17,7 @@ module MergeRequests ...@@ -17,7 +17,7 @@ module MergeRequests
reset_approvals_cache(merge_request) reset_approvals_cache(merge_request)
create_note(merge_request) create_note(merge_request)
merge_request_activity_counter.track_unapprove_mr_action(user: current_user) merge_request_activity_counter.track_unapprove_mr_action(user: current_user)
remove_attention_requested(merge_request, current_user) remove_attention_requested(merge_request)
end end
success success
......
...@@ -2,13 +2,12 @@ ...@@ -2,13 +2,12 @@
module MergeRequests module MergeRequests
class RemoveAttentionRequestedService < MergeRequests::BaseService class RemoveAttentionRequestedService < MergeRequests::BaseService
attr_accessor :merge_request, :user attr_accessor :merge_request
def initialize(project:, current_user:, merge_request:, user:) def initialize(project:, current_user:, merge_request:)
super(project: project, current_user: current_user) super(project: project, current_user: current_user)
@merge_request = merge_request @merge_request = merge_request
@user = user
end end
def execute def execute
...@@ -18,6 +17,8 @@ module MergeRequests ...@@ -18,6 +17,8 @@ module MergeRequests
update_state(reviewer) update_state(reviewer)
update_state(assignee) update_state(assignee)
current_user.invalidate_attention_requested_count
success success
else else
error("User is not a reviewer or assignee of the merge request") error("User is not a reviewer or assignee of the merge request")
...@@ -27,11 +28,11 @@ module MergeRequests ...@@ -27,11 +28,11 @@ module MergeRequests
private private
def assignee def assignee
merge_request.find_assignee(user) merge_request.find_assignee(current_user)
end end
def reviewer def reviewer
merge_request.find_reviewer(user) merge_request.find_reviewer(current_user)
end end
def update_state(reviewer_or_assignee) def update_state(reviewer_or_assignee)
......
...@@ -6,6 +6,8 @@ module MergeRequests ...@@ -6,6 +6,8 @@ module MergeRequests
return merge_request unless can?(current_user, :reopen_merge_request, merge_request) return merge_request unless can?(current_user, :reopen_merge_request, merge_request)
if merge_request.reopen if merge_request.reopen
users = merge_request.assignees | merge_request.reviewers
create_event(merge_request) create_event(merge_request)
create_note(merge_request, 'reopened') create_note(merge_request, 'reopened')
merge_request_activity_counter.track_reopen_mr_action(user: current_user) merge_request_activity_counter.track_reopen_mr_action(user: current_user)
...@@ -13,11 +15,13 @@ module MergeRequests ...@@ -13,11 +15,13 @@ module MergeRequests
execute_hooks(merge_request, 'reopen') execute_hooks(merge_request, 'reopen')
merge_request.reload_diff(current_user) merge_request.reload_diff(current_user)
merge_request.mark_as_unchecked merge_request.mark_as_unchecked
invalidate_cache_counts(merge_request, users: merge_request.assignees | merge_request.reviewers) invalidate_cache_counts(merge_request, users: users)
merge_request.update_project_counter_caches merge_request.update_project_counter_caches
merge_request.cache_merge_request_closes_issues!(current_user) merge_request.cache_merge_request_closes_issues!(current_user)
merge_request.cleanup_schedule&.destroy merge_request.cleanup_schedule&.destroy
merge_request.update_column(:merge_ref_sha, nil) merge_request.update_column(:merge_ref_sha, nil)
users.each { |user| user.invalidate_attention_requested_count }
end end
merge_request merge_request
......
...@@ -18,12 +18,14 @@ module MergeRequests ...@@ -18,12 +18,14 @@ module MergeRequests
update_state(reviewer) update_state(reviewer)
update_state(assignee) update_state(assignee)
user.invalidate_attention_requested_count
if reviewer&.attention_requested? || assignee&.attention_requested? if reviewer&.attention_requested? || assignee&.attention_requested?
create_attention_request_note create_attention_request_note
notity_user notity_user
if current_user.id != user.id if current_user.id != user.id
remove_attention_requested(merge_request, current_user) remove_attention_requested(merge_request)
end end
else else
create_remove_attention_request_note create_remove_attention_request_note
......
...@@ -68,7 +68,8 @@ ...@@ -68,7 +68,8 @@
= number_with_delimiter(issues_count) = number_with_delimiter(issues_count)
- if header_link?(:merge_requests) - if header_link?(:merge_requests)
= nav_link(path: 'dashboard#merge_requests', html_options: { class: "user-counter dropdown" }) do = nav_link(path: 'dashboard#merge_requests', html_options: { class: "user-counter dropdown" }) do
= link_to assigned_mrs_dashboard_path, class: 'dashboard-shortcuts-merge_requests', title: _('Merge requests'), aria: { label: _('Merge requests') }, - top_level_link = Feature.enabled?(:mr_attention_requests, default_enabled: :yaml) ? attention_requested_mrs_dashboard_path : assigned_mrs_dashboard_path
= link_to top_level_link, class: 'dashboard-shortcuts-merge_requests', title: _('Merge requests'), aria: { label: _('Merge requests') },
data: { qa_selector: 'merge_requests_shortcut_button', data: { qa_selector: 'merge_requests_shortcut_button',
toggle: "dropdown", toggle: "dropdown",
placement: 'bottom', placement: 'bottom',
...@@ -84,6 +85,12 @@ ...@@ -84,6 +85,12 @@
%ul %ul
%li.dropdown-header %li.dropdown-header
= _('Merge requests') = _('Merge requests')
- if Feature.enabled?(:mr_attention_requests, default_enabled: :yaml)
%li
= link_to attention_requested_mrs_dashboard_path, class: 'gl-display-flex! gl-align-items-center js-prefetch-document' do
= _('Need your attention')
= gl_badge_tag user_merge_requests_counts[:attention_requested_count], { size: :sm, variant: user_merge_requests_counts[:attention_requested_count] == 0 ? :neutral : :warning }, { class: 'merge-request-badge gl-ml-auto js-attention-count' }
%li.divider
%li %li
= link_to assigned_mrs_dashboard_path, class: 'gl-display-flex! gl-align-items-center js-prefetch-document' do = link_to assigned_mrs_dashboard_path, class: 'gl-display-flex! gl-align-items-center js-prefetch-document' do
= _('Assigned to you') = _('Assigned to you')
......
...@@ -11,13 +11,19 @@ module API ...@@ -11,13 +11,19 @@ module API
get do get do
unauthorized! unless current_user unauthorized! unless current_user
{ counts = {
merge_requests: current_user.assigned_open_merge_requests_count, # @deprecated merge_requests: current_user.assigned_open_merge_requests_count, # @deprecated
assigned_issues: current_user.assigned_open_issues_count, assigned_issues: current_user.assigned_open_issues_count,
assigned_merge_requests: current_user.assigned_open_merge_requests_count, assigned_merge_requests: current_user.assigned_open_merge_requests_count,
review_requested_merge_requests: current_user.review_requested_open_merge_requests_count, review_requested_merge_requests: current_user.review_requested_open_merge_requests_count,
todos: current_user.todos_pending_count todos: current_user.todos_pending_count
} }
if Feature.enabled?(:mr_attention_requests, default_enabled: :yaml)
counts[:attention_requests] = current_user.attention_requested_open_merge_requests_count
end
counts
end end
end end
end end
......
...@@ -24103,6 +24103,9 @@ msgstr "" ...@@ -24103,6 +24103,9 @@ msgstr ""
msgid "Need help?" msgid "Need help?"
msgstr "" msgstr ""
msgid "Need your attention"
msgstr ""
msgid "Needs" msgid "Needs"
msgstr "" msgstr ""
......
...@@ -8,41 +8,68 @@ RSpec.describe 'Navigation bar counter', :use_clean_rails_memory_store_caching d ...@@ -8,41 +8,68 @@ RSpec.describe 'Navigation bar counter', :use_clean_rails_memory_store_caching d
let(:issue) { create(:issue, project: project) } let(:issue) { create(:issue, project: project) }
let(:merge_request) { create(:merge_request, source_project: project) } let(:merge_request) { create(:merge_request, source_project: project) }
before do describe 'feature flag mr_attention_requests is disabled' do
issue.assignees = [user] before do
merge_request.update!(assignees: [user]) stub_feature_flags(mr_attention_requests: false)
sign_in(user)
end
it 'reflects dashboard issues count' do issue.assignees = [user]
visit issues_path merge_request.update!(assignees: [user])
sign_in(user)
end
expect_counters('issues', '1', n_("%d assigned issue", "%d assigned issues", 1) % 1) it 'reflects dashboard issues count' do
visit issues_path
issue.assignees = [] expect_counters('issues', '1', n_("%d assigned issue", "%d assigned issues", 1) % 1)
user.invalidate_cache_counts issue.assignees = []
travel_to(3.minutes.from_now) do user.invalidate_cache_counts
visit issues_path
travel_to(3.minutes.from_now) do
visit issues_path
expect_counters('issues', '0', n_("%d assigned issue", "%d assigned issues", 0) % 0) expect_counters('issues', '0', n_("%d assigned issue", "%d assigned issues", 0) % 0)
end
end
it 'reflects dashboard merge requests count', :js do
visit merge_requests_path
expect_counters('merge_requests', '1', n_("%d merge request", "%d merge requests", 1) % 1)
merge_request.update!(assignees: [])
user.invalidate_cache_counts
travel_to(3.minutes.from_now) do
visit merge_requests_path
expect_counters('merge_requests', '0', n_("%d merge request", "%d merge requests", 0) % 0)
end
end end
end end
it 'reflects dashboard merge requests count' do describe 'feature flag mr_attention_requests is enabled' do
visit merge_requests_path before do
merge_request.update!(assignees: [user])
sign_in(user)
end
expect_counters('merge_requests', '1', n_("%d merge request", "%d merge requests", 1) % 1) it 'reflects dashboard merge requests count', :js do
visit merge_requests_attention_path
merge_request.update!(assignees: []) expect_counters('merge_requests', '1', n_("%d merge request", "%d merge requests", 1) % 1)
user.invalidate_cache_counts merge_request.find_assignee(user).update!(state: :reviewed)
travel_to(3.minutes.from_now) do user.invalidate_attention_requested_count
visit merge_requests_path
expect_counters('merge_requests', '0', n_("%d merge request", "%d merge requests", 0) % 0) travel_to(3.minutes.from_now) do
visit merge_requests_attention_path
expect_counters('merge_requests', '0', n_("%d merge request", "%d merge requests", 0) % 0)
end
end end
end end
...@@ -54,14 +81,15 @@ RSpec.describe 'Navigation bar counter', :use_clean_rails_memory_store_caching d ...@@ -54,14 +81,15 @@ RSpec.describe 'Navigation bar counter', :use_clean_rails_memory_store_caching d
merge_requests_dashboard_path(assignee_username: user.username) merge_requests_dashboard_path(assignee_username: user.username)
end end
def merge_requests_attention_path
merge_requests_dashboard_path(attention: user.username)
end
def expect_counters(issuable_type, count, badge_label) def expect_counters(issuable_type, count, badge_label)
dashboard_count = find('.gl-tabs-nav li a.active') dashboard_count = find('.gl-tabs-nav li a.active')
nav_count = find(".dashboard-shortcuts-#{issuable_type}")
expect(dashboard_count).to have_content(count) expect(dashboard_count).to have_content(count)
expect(nav_count).to have_content(count) expect(page).to have_css(".dashboard-shortcuts-#{issuable_type}", visible: :all, text: count)
within("span[aria-label='#{badge_label}']") do expect(page).to have_css("span[aria-label='#{badge_label}']", visible: :all, text: count)
expect(page).to have_content(count)
end
end end
end end
...@@ -5,9 +5,11 @@ import SidebarService, { gqClient } from '~/sidebar/services/sidebar_service'; ...@@ -5,9 +5,11 @@ import SidebarService, { gqClient } from '~/sidebar/services/sidebar_service';
import SidebarMediator from '~/sidebar/sidebar_mediator'; import SidebarMediator from '~/sidebar/sidebar_mediator';
import SidebarStore from '~/sidebar/stores/sidebar_store'; import SidebarStore from '~/sidebar/stores/sidebar_store';
import toast from '~/vue_shared/plugins/global_toast'; import toast from '~/vue_shared/plugins/global_toast';
import { refreshUserMergeRequestCounts } from '~/commons/nav/user_merge_requests';
import Mock from './mock_data'; import Mock from './mock_data';
jest.mock('~/vue_shared/plugins/global_toast'); jest.mock('~/vue_shared/plugins/global_toast');
jest.mock('~/commons/nav/user_merge_requests');
describe('Sidebar mediator', () => { describe('Sidebar mediator', () => {
const { mediator: mediatorMockData } = Mock; const { mediator: mediatorMockData } = Mock;
...@@ -137,6 +139,7 @@ describe('Sidebar mediator', () => { ...@@ -137,6 +139,7 @@ describe('Sidebar mediator', () => {
}); });
expect(attentionRequiredService).toHaveBeenCalledWith(1); expect(attentionRequiredService).toHaveBeenCalledWith(1);
expect(refreshUserMergeRequestCounts).toHaveBeenCalled();
}); });
it.each` it.each`
......
...@@ -72,7 +72,8 @@ RSpec.describe MergeRequestsHelper do ...@@ -72,7 +72,8 @@ RSpec.describe MergeRequestsHelper do
let(:user) do let(:user) do
double( double(
assigned_open_merge_requests_count: 1, assigned_open_merge_requests_count: 1,
review_requested_open_merge_requests_count: 2 review_requested_open_merge_requests_count: 2,
attention_requested_open_merge_requests_count: 3
) )
end end
...@@ -82,12 +83,29 @@ RSpec.describe MergeRequestsHelper do ...@@ -82,12 +83,29 @@ RSpec.describe MergeRequestsHelper do
allow(helper).to receive(:current_user).and_return(user) allow(helper).to receive(:current_user).and_return(user)
end end
it "returns assigned, review requested and total merge request counts" do describe 'mr_attention_requests disabled' do
expect(subject).to eq( before do
assigned: user.assigned_open_merge_requests_count, stub_feature_flags(mr_attention_requests: false)
review_requested: user.review_requested_open_merge_requests_count, end
total: user.assigned_open_merge_requests_count + user.review_requested_open_merge_requests_count
) it "returns assigned, review requested and total merge request counts" do
expect(subject).to eq(
assigned: user.assigned_open_merge_requests_count,
review_requested: user.review_requested_open_merge_requests_count,
total: user.assigned_open_merge_requests_count + user.review_requested_open_merge_requests_count
)
end
end
describe 'mr_attention_requests enabled' do
it "returns assigned, review requested, attention requests and total merge request counts" do
expect(subject).to eq(
assigned: user.assigned_open_merge_requests_count,
review_requested: user.review_requested_open_merge_requests_count,
attention_requested_count: user.attention_requested_open_merge_requests_count,
total: user.attention_requested_open_merge_requests_count
)
end
end end
end end
......
...@@ -4712,6 +4712,7 @@ RSpec.describe User do ...@@ -4712,6 +4712,7 @@ RSpec.describe User do
expect(cache_mock).to receive(:delete).with(['users', user.id, 'assigned_open_merge_requests_count']) expect(cache_mock).to receive(:delete).with(['users', user.id, 'assigned_open_merge_requests_count'])
expect(cache_mock).to receive(:delete).with(['users', user.id, 'review_requested_open_merge_requests_count']) expect(cache_mock).to receive(:delete).with(['users', user.id, 'review_requested_open_merge_requests_count'])
expect(cache_mock).to receive(:delete).with(['users', user.id, 'attention_requested_open_merge_requests_count'])
allow(Rails).to receive(:cache).and_return(cache_mock) allow(Rails).to receive(:cache).and_return(cache_mock)
...@@ -4719,6 +4720,20 @@ RSpec.describe User do ...@@ -4719,6 +4720,20 @@ RSpec.describe User do
end end
end end
describe '#invalidate_attention_requested_count' do
let(:user) { build_stubbed(:user) }
it 'invalidates cache for issue counter' do
cache_mock = double
expect(cache_mock).to receive(:delete).with(['users', user.id, 'attention_requested_open_merge_requests_count'])
allow(Rails).to receive(:cache).and_return(cache_mock)
user.invalidate_attention_requested_count
end
end
describe '#invalidate_personal_projects_count' do describe '#invalidate_personal_projects_count' do
let(:user) { build_stubbed(:user) } let(:user) { build_stubbed(:user) }
...@@ -4805,6 +4820,20 @@ RSpec.describe User do ...@@ -4805,6 +4820,20 @@ RSpec.describe User do
end end
end end
describe '#attention_requested_open_merge_requests_count' do
it 'returns number of open merge requests from non-archived projects' do
user = create(:user)
project = create(:project, :public)
archived_project = create(:project, :public, :archived)
create(:merge_request, source_project: project, author: user, reviewers: [user])
create(:merge_request, :closed, source_project: project, author: user, reviewers: [user])
create(:merge_request, source_project: archived_project, author: user, reviewers: [user])
expect(user.attention_requested_open_merge_requests_count(force: true)).to eq 1
end
end
describe '#assigned_open_issues_count' do describe '#assigned_open_issues_count' do
it 'returns number of open issues from non-archived projects' do it 'returns number of open issues from non-archived projects' do
user = create(:user) user = create(:user)
......
...@@ -43,6 +43,21 @@ RSpec.describe API::UserCounts do ...@@ -43,6 +43,21 @@ RSpec.describe API::UserCounts do
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to be_a Hash expect(json_response).to be_a Hash
expect(json_response['merge_requests']).to eq(2) expect(json_response['merge_requests']).to eq(2)
expect(json_response['attention_requests']).to eq(2)
end
describe 'mr_attention_requests is disabled' do
before do
stub_feature_flags(mr_attention_requests: false)
end
it 'does not include attention_requests count' do
create(:merge_request, source_project: project, author: user, assignees: [user])
get api('/user_counts', user)
expect(json_response.key?('attention_requests')).to be(false)
end
end end
end end
......
...@@ -61,7 +61,7 @@ RSpec.describe MergeRequests::ApprovalService do ...@@ -61,7 +61,7 @@ RSpec.describe MergeRequests::ApprovalService do
it 'removes attention requested state' do it 'removes attention requested state' do
expect(MergeRequests::RemoveAttentionRequestedService).to receive(:new) expect(MergeRequests::RemoveAttentionRequestedService).to receive(:new)
.with(project: project, current_user: user, merge_request: merge_request, user: user) .with(project: project, current_user: user, merge_request: merge_request)
.and_call_original .and_call_original
service.execute(merge_request) service.execute(merge_request)
......
...@@ -40,6 +40,10 @@ RSpec.describe MergeRequests::BulkRemoveAttentionRequestedService do ...@@ -40,6 +40,10 @@ RSpec.describe MergeRequests::BulkRemoveAttentionRequestedService do
expect(reviewer.state).to eq 'reviewed' expect(reviewer.state).to eq 'reviewed'
expect(assignee.state).to eq 'reviewed' expect(assignee.state).to eq 'reviewed'
end end
it_behaves_like 'invalidates attention request cache' do
let(:users) { [assignee_user, user] }
end
end end
end end
end end
...@@ -89,7 +89,7 @@ RSpec.describe MergeRequests::HandleAssigneesChangeService do ...@@ -89,7 +89,7 @@ RSpec.describe MergeRequests::HandleAssigneesChangeService do
it 'removes attention requested state' do it 'removes attention requested state' do
expect(MergeRequests::RemoveAttentionRequestedService).to receive(:new) expect(MergeRequests::RemoveAttentionRequestedService).to receive(:new)
.with(project: project, current_user: user, merge_request: merge_request, user: user) .with(project: project, current_user: user, merge_request: merge_request)
.and_call_original .and_call_original
execute execute
......
...@@ -4,23 +4,20 @@ require 'spec_helper' ...@@ -4,23 +4,20 @@ require 'spec_helper'
RSpec.describe MergeRequests::RemoveAttentionRequestedService do RSpec.describe MergeRequests::RemoveAttentionRequestedService do
let(:current_user) { create(:user) } let(:current_user) { create(:user) }
let(:user) { create(:user) } let(:merge_request) { create(:merge_request, reviewers: [current_user], assignees: [current_user]) }
let(:assignee_user) { create(:user) } let(:reviewer) { merge_request.find_reviewer(current_user) }
let(:merge_request) { create(:merge_request, reviewers: [user], assignees: [assignee_user]) } let(:assignee) { merge_request.find_assignee(current_user) }
let(:reviewer) { merge_request.find_reviewer(user) }
let(:assignee) { merge_request.find_assignee(assignee_user) }
let(:project) { merge_request.project } let(:project) { merge_request.project }
let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request, user: user) } let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request) }
let(:result) { service.execute } let(:result) { service.execute }
before do before do
project.add_developer(current_user) project.add_developer(current_user)
project.add_developer(user)
end end
describe '#execute' do describe '#execute' do
context 'invalid permissions' do context 'invalid permissions' do
let(:service) { described_class.new(project: project, current_user: create(:user), merge_request: merge_request, user: user) } let(:service) { described_class.new(project: project, current_user: create(:user), merge_request: merge_request) }
it 'returns an error' do it 'returns an error' do
expect(result[:status]).to eq :error expect(result[:status]).to eq :error
...@@ -28,7 +25,7 @@ RSpec.describe MergeRequests::RemoveAttentionRequestedService do ...@@ -28,7 +25,7 @@ RSpec.describe MergeRequests::RemoveAttentionRequestedService do
end end
context 'reviewer does not exist' do context 'reviewer does not exist' do
let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request, user: create(:user)) } let(:service) { described_class.new(project: project, current_user: create(:user), merge_request: merge_request) }
it 'returns an error' do it 'returns an error' do
expect(result[:status]).to eq :error expect(result[:status]).to eq :error
...@@ -46,10 +43,14 @@ RSpec.describe MergeRequests::RemoveAttentionRequestedService do ...@@ -46,10 +43,14 @@ RSpec.describe MergeRequests::RemoveAttentionRequestedService do
expect(reviewer.state).to eq 'reviewed' expect(reviewer.state).to eq 'reviewed'
end end
it_behaves_like 'invalidates attention request cache' do
let(:users) { [current_user] }
end
end end
context 'assignee exists' do context 'assignee exists' do
let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request, user: assignee_user) } let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request) }
before do before do
assignee.update!(state: :reviewed) assignee.update!(state: :reviewed)
...@@ -65,12 +66,16 @@ RSpec.describe MergeRequests::RemoveAttentionRequestedService do ...@@ -65,12 +66,16 @@ RSpec.describe MergeRequests::RemoveAttentionRequestedService do
expect(assignee.state).to eq 'reviewed' expect(assignee.state).to eq 'reviewed'
end end
it_behaves_like 'invalidates attention request cache' do
let(:users) { [current_user] }
end
end end
context 'assignee is the same as reviewer' do context 'assignee is the same as reviewer' do
let(:merge_request) { create(:merge_request, reviewers: [user], assignees: [user]) } let(:merge_request) { create(:merge_request, reviewers: [current_user], assignees: [current_user]) }
let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request, user: user) } let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request) }
let(:assignee) { merge_request.find_assignee(user) } let(:assignee) { merge_request.find_assignee(current_user) }
it 'updates reviewers and assignees state' do it 'updates reviewers and assignees state' do
service.execute service.execute
......
...@@ -80,11 +80,21 @@ RSpec.describe MergeRequests::ToggleAttentionRequestedService do ...@@ -80,11 +80,21 @@ RSpec.describe MergeRequests::ToggleAttentionRequestedService do
it 'removes attention requested state' do it 'removes attention requested state' do
expect(MergeRequests::RemoveAttentionRequestedService).to receive(:new) expect(MergeRequests::RemoveAttentionRequestedService).to receive(:new)
.with(project: project, current_user: current_user, merge_request: merge_request, user: current_user) .with(project: project, current_user: current_user, merge_request: merge_request)
.and_call_original .and_call_original
service.execute service.execute
end end
it 'invalidates cache' do
cache_mock = double
expect(cache_mock).to receive(:delete).with(['users', user.id, 'attention_requested_open_merge_requests_count'])
allow(Rails).to receive(:cache).and_return(cache_mock)
service.execute
end
end end
context 'assignee exists' do context 'assignee exists' do
...@@ -119,11 +129,15 @@ RSpec.describe MergeRequests::ToggleAttentionRequestedService do ...@@ -119,11 +129,15 @@ RSpec.describe MergeRequests::ToggleAttentionRequestedService do
it 'removes attention requested state' do it 'removes attention requested state' do
expect(MergeRequests::RemoveAttentionRequestedService).to receive(:new) expect(MergeRequests::RemoveAttentionRequestedService).to receive(:new)
.with(project: project, current_user: current_user, merge_request: merge_request, user: current_user) .with(project: project, current_user: current_user, merge_request: merge_request)
.and_call_original .and_call_original
service.execute service.execute
end end
it_behaves_like 'invalidates attention request cache' do
let(:users) { [assignee_user] }
end
end end
context 'assignee is the same as reviewer' do context 'assignee is the same as reviewer' do
......
# frozen_string_literal: true
RSpec.shared_examples 'invalidates attention request cache' do
it 'invalidates the merge requests requiring attention count' do
cache_mock = double
users.each do |user|
expect(cache_mock).to receive(:delete).with(['users', user.id, 'attention_requested_open_merge_requests_count'])
end
allow(Rails).to receive(:cache).and_return(cache_mock)
service.execute
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