diff --git a/app/assets/javascripts/commons/nav/user_merge_requests.js b/app/assets/javascripts/commons/nav/user_merge_requests.js index 1e2cf756059dd97b2b478e08b3b1df7f4347aa8a..eab2b5a31d75c044fac73160fecf0498a2470fc1 100644 --- a/app/assets/javascripts/commons/nav/user_merge_requests.js +++ b/app/assets/javascripts/commons/nav/user_merge_requests.js @@ -11,7 +11,17 @@ function broadcastCount(newCount) { } function updateUserMergeRequestCounts(newCount) { - const mergeRequestsCountEl = document.querySelector('.merge-requests-count'); + const mergeRequestsCountEl = document.querySelector('.js-assigned-mr-count'); + mergeRequestsCountEl.textContent = newCount.toLocaleString(); +} + +function updateReviewerMergeRequestCounts(newCount) { + const mergeRequestsCountEl = document.querySelector('.js-reviewer-mr-count'); + mergeRequestsCountEl.textContent = newCount.toLocaleString(); +} + +function updateMergeRequestCounts(newCount) { + const mergeRequestsCountEl = document.querySelector('.js-merge-requests-count'); mergeRequestsCountEl.textContent = newCount.toLocaleString(); mergeRequestsCountEl.classList.toggle('hidden', Number(newCount) === 0); } @@ -22,10 +32,14 @@ function updateUserMergeRequestCounts(newCount) { export function refreshUserMergeRequestCounts() { return Api.userCounts() .then(({ data }) => { - const count = data.merge_requests; + const assignedMergeRequests = data.assigned_merge_requests; + const reviewerMergeRequests = data.review_requested_merge_requests; + const fullCount = assignedMergeRequests + reviewerMergeRequests; - updateUserMergeRequestCounts(count); - broadcastCount(count); + updateUserMergeRequestCounts(assignedMergeRequests); + updateReviewerMergeRequestCounts(reviewerMergeRequests); + updateMergeRequestCounts(fullCount); + broadcastCount(fullCount); }) .catch((ex) => { console.error(ex); // eslint-disable-line no-console @@ -60,7 +74,7 @@ export function openUserCountsBroadcast() { if (currentUserId) { channel = new BroadcastChannel(`mr_count_channel_${currentUserId}`); channel.onmessage = (ev) => { - updateUserMergeRequestCounts(ev.data); + updateMergeRequestCounts(ev.data); }; } } diff --git a/app/assets/javascripts/sidebar/components/reviewers/sidebar_reviewers.vue b/app/assets/javascripts/sidebar/components/reviewers/sidebar_reviewers.vue index aee94a55134e45d0c17ce9ce9017831f39420126..d9228450371fc557ef0e3e93ecedb23e63024dbf 100644 --- a/app/assets/javascripts/sidebar/components/reviewers/sidebar_reviewers.vue +++ b/app/assets/javascripts/sidebar/components/reviewers/sidebar_reviewers.vue @@ -2,6 +2,7 @@ // NOTE! For the first iteration, we are simply copying the implementation of Assignees // It will soon be overhauled in Issue https://gitlab.com/gitlab-org/gitlab/-/issues/233736 import { deprecatedCreateFlash as Flash } from '~/flash'; +import { refreshUserMergeRequestCounts } from '~/commons/nav/user_merge_requests'; import eventHub from '~/sidebar/event_hub'; import Store from '~/sidebar/stores/sidebar_store'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; @@ -80,8 +81,7 @@ export default { .saveReviewers(this.field) .then(() => { this.loading = false; - // Uncomment once this issue has been addressed > https://gitlab.com/gitlab-org/gitlab/-/issues/237922 - // refreshUserMergeRequestCounts(); + refreshUserMergeRequestCounts(); }) .catch(() => { this.loading = false; diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index e2335c184b0b11809e0f8eef1586f8f6df07e2c7..c5ffcb48814d7d6f2d4c766f5f9649a7fa322894 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -796,6 +796,14 @@ .navbar-gitlab { li.dropdown { position: static; + + &.user-counter { + margin-left: 8px !important; + + > a { + padding: 0 4px !important; + } + } } } diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index a6a01c7b09021df6e007e98c8931692797c59869..730e10114c3b2ae6d884c820f5813ea36c5f307d 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -172,7 +172,7 @@ } li { - .badge.badge-pill { + .badge.badge-pill:not(.merge-request-badge) { box-shadow: none; font-weight: $gl-font-weight-bold; } @@ -438,7 +438,7 @@ .title-container, .navbar-nav { - .badge.badge-pill { + .badge.badge-pill:not(.merge-request-badge) { position: inherit; font-weight: $gl-font-weight-normal; margin-left: -6px; diff --git a/app/helpers/dashboard_helper.rb b/app/helpers/dashboard_helper.rb index 195b31620391188ab9d7446d5fdce5e2125d245e..08f357916b504c9b7ac4f9ee4a9d2097d1d42f5d 100644 --- a/app/helpers/dashboard_helper.rb +++ b/app/helpers/dashboard_helper.rb @@ -11,6 +11,10 @@ def assigned_mrs_dashboard_path merge_requests_dashboard_path(assignee_username: current_user.username) end + def reviewer_mrs_dashboard_path + merge_requests_dashboard_path(reviewer_username: current_user.username) + end + def dashboard_nav_links @dashboard_nav_links ||= get_dashboard_nav_links end diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 37e701c1c2b7cdc5ce1c3a624f48c24c78acaa4f..bbfd06ca06daa87535d1f2abe860f6f187d0ae86 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -159,6 +159,32 @@ def toggle_draft_merge_request_path(issuable) issuable_path(issuable, { merge_request: { wip_event: wip_event } }) end + + def user_merge_requests_counts + @user_merge_requests_counts ||= begin + assigned_count = assigned_issuables_count(:merge_requests) + review_requested_count = review_requested_merge_requests_count + total_count = assigned_count + review_requested_count + + { + assigned: assigned_count, + review_requested: review_requested_count, + total: total_count + } + end + end + + def merge_request_reviewers_enabled? + Feature.enabled?(:merge_request_reviewers, default_enabled: :yaml) + end + + private + + def review_requested_merge_requests_count + return 0 unless merge_request_reviewers_enabled? + + current_user.review_requested_open_merge_requests_count + end end MergeRequestsHelper.prepend_if_ee('EE::MergeRequestsHelper') diff --git a/app/models/user.rb b/app/models/user.rb index c735f20b92cfe0299ffa0d7fc5996bd33ba1c98b..284182797c03cc594e005b442fb9d126dc0e6a07 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1564,6 +1564,12 @@ def assigned_open_merge_requests_count(force: false) end end + def review_requested_open_merge_requests_count(force: false) + Rails.cache.fetch(['users', id, 'review_requested_open_merge_requests_count'], force: force, expires_in: 20.minutes) do + MergeRequestsFinder.new(self, reviewer_id: id, state: 'opened', non_archived: true).execute.count + end + end + def assigned_open_issues_count(force: false) Rails.cache.fetch(['users', id, 'assigned_open_issues_count'], force: force, expires_in: 20.minutes) do IssuesFinder.new(self, assignee_id: self.id, state: 'opened', non_archived: true).execute.count @@ -1607,6 +1613,7 @@ def invalidate_issue_cache_counts def invalidate_merge_request_cache_counts Rails.cache.delete(['users', id, 'assigned_open_merge_requests_count']) + Rails.cache.delete(['users', id, 'review_requested_open_merge_requests_count']) end def invalidate_todos_done_count diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 1d0ab403f12b22fb1f8d58d309428f40f8e0f42d..d2e5a2a1619c2ef5e114eb83b4ab62e995d6af9d 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -112,9 +112,11 @@ def handle_assignees_change(merge_request, old_assignees) end def handle_reviewers_change(merge_request, old_reviewers) + affected_reviewers = (old_reviewers + merge_request.reviewers) - (old_reviewers & merge_request.reviewers) create_reviewer_note(merge_request, old_reviewers) notification_service.async.changed_reviewer_of_merge_request(merge_request, current_user, old_reviewers) todo_service.reassigned_reviewable(merge_request, current_user, old_reviewers) + invalidate_cache_counts(merge_request, users: affected_reviewers.compact) end def create_branch_change_note(issuable, branch_type, old_branch, new_branch) diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 70ab0a5658124385966589c6da01d5f4d24b704a..1394f15cd1515c68650d994959ad3c29ea89a6ec 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -47,17 +47,36 @@ %span.badge.badge-pill.issues-count.green-badge{ class: ('hidden' if issues_count == 0) } = number_with_delimiter(issues_count) - if header_link?(:merge_requests) - = nav_link(path: 'dashboard#merge_requests', html_options: { class: "user-counter" }) do - = link_to assigned_mrs_dashboard_path, title: _('Merge requests'), class: 'dashboard-shortcuts-merge_requests', aria: { label: _('Merge requests') }, - data: { qa_selector: 'merge_requests_shortcut_button', toggle: 'tooltip', placement: 'bottom', + - reviewers_enabled = merge_request_reviewers_enabled? + = nav_link(path: 'dashboard#merge_requests', html_options: { class: "user-counter #{reviewers_enabled ? 'dropdown' : ''}" }) do + = link_to assigned_mrs_dashboard_path, class: 'dashboard-shortcuts-merge_requests', title: _('Merge requests'), aria: { label: _('Merge requests') }, + data: { qa_selector: 'merge_requests_shortcut_button', + toggle: reviewers_enabled ? "dropdown" : "tooltip", + placement: 'bottom', track_label: 'main_navigation', track_event: 'click_merge_link', track_property: 'navigation', container: 'body' } do = sprite_icon('git-merge') - - merge_requests_count = assigned_issuables_count(:merge_requests) - %span.badge.badge-pill.merge-requests-count{ class: ('hidden' if merge_requests_count == 0) } - = number_with_delimiter(merge_requests_count) + %span.badge.badge-pill.merge-requests-count.js-merge-requests-count{ class: ('hidden' if user_merge_requests_counts[:total] == 0) } + = number_with_delimiter(user_merge_requests_counts[:total]) + - if reviewers_enabled + = sprite_icon('chevron-down', css_class: 'caret-down gl-mx-0!') + - if reviewers_enabled + .dropdown-menu.dropdown-menu-right + %ul + %li.dropdown-header + = _('Merge requests') + %li + = link_to assigned_mrs_dashboard_path, class: 'gl-display-flex! gl-align-items-center' do + = _('Assigned to you') + %span.badge.gl-badge.badge-pill.badge-muted.merge-request-badge.gl-ml-auto.js-assigned-mr-count{ class: "" } + = user_merge_requests_counts[:assigned] + %li + = link_to reviewer_mrs_dashboard_path, class: 'gl-display-flex! gl-align-items-center' do + = _('Review requests for you') + %span.badge.gl-badge.badge-pill.badge-muted.merge-request-badge.gl-ml-auto.js-reviewer-mr-count{ class: "" } + = user_merge_requests_counts[:review_requested] - if header_link?(:todos) = nav_link(controller: 'dashboard/todos', html_options: { class: "user-counter" }) do = link_to dashboard_todos_path, title: _('To-Do List'), aria: { label: _('To-Do List') }, class: 'shortcuts-todos', diff --git a/lib/api/user_counts.rb b/lib/api/user_counts.rb index 3071f08e1de108ad4b0d9cc4a2391c494b5365c7..31c923a219a48d4bb5e9d762228c45696c123cc1 100644 --- a/lib/api/user_counts.rb +++ b/lib/api/user_counts.rb @@ -12,7 +12,9 @@ class UserCounts < ::API::Base unauthorized! unless current_user { - merge_requests: current_user.assigned_open_merge_requests_count + merge_requests: current_user.assigned_open_merge_requests_count, # @deprecated + assigned_merge_requests: current_user.assigned_open_merge_requests_count, + review_requested_merge_requests: current_user.review_requested_open_merge_requests_count } end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 88d9f3f3cbfd0055c917ee45376e7e616f4400dd..9a93cb3e8f0343ca9c953012caaa9fd4ce22cebf 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3940,6 +3940,9 @@ msgstr "" msgid "Assigned to me" msgstr "" +msgid "Assigned to you" +msgstr "" + msgid "Assignee" msgid_plural "%d Assignees" msgstr[0] "" @@ -24008,6 +24011,9 @@ msgstr "" msgid "Review requested from %{name}" msgstr "" +msgid "Review requests for you" +msgstr "" + msgid "Review the changes locally" msgstr "" diff --git a/spec/features/dashboard/merge_requests_spec.rb b/spec/features/dashboard/merge_requests_spec.rb index 0e76b5478a1d17e52bf09cbf0fefe8d76ebbe1a2..26b376be660488276a273b0be9ede5fde0e45131 100644 --- a/spec/features/dashboard/merge_requests_spec.rb +++ b/spec/features/dashboard/merge_requests_spec.rb @@ -110,6 +110,12 @@ visit merge_requests_dashboard_path(assignee_username: current_user.username) end + it 'includes assigned and reviewers in badge' do + expect(find('.merge-requests-count')).to have_content('3') + expect(find('.js-assigned-mr-count')).to have_content('2') + expect(find('.js-reviewer-mr-count')).to have_content('1') + end + it 'shows assigned merge requests' do expect(page).to have_content(assigned_merge_request.title) expect(page).to have_content(assigned_merge_request_from_fork.title) diff --git a/spec/frontend/commons/nav/user_merge_requests_spec.js b/spec/frontend/commons/nav/user_merge_requests_spec.js index 4da6d53557a315bb73d98b01221a0a045fecc502..5f81067f84f5d9ea4cb61f19321d4ffdc01f59f8 100644 --- a/spec/frontend/commons/nav/user_merge_requests_spec.js +++ b/spec/frontend/commons/nav/user_merge_requests_spec.js @@ -8,7 +8,7 @@ import Api from '~/api'; jest.mock('~/api'); const TEST_COUNT = 1000; -const MR_COUNT_CLASS = 'merge-requests-count'; +const MR_COUNT_CLASS = 'js-merge-requests-count'; describe('User Merge Requests', () => { let channelMock; @@ -24,7 +24,9 @@ describe('User Merge Requests', () => { newBroadcastChannelMock = jest.fn().mockImplementation(() => channelMock); global.BroadcastChannel = newBroadcastChannelMock; - setFixtures(`