From 16db46fdabef8dae6e16b9f318db742d49daedf4 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 15 Dec 2020 08:41:11 +0000 Subject: [PATCH 01/13] Added extended dropdown to merge requests header icon Adds a dropdown to the merge requests count icon in the header that will allow users to see both assigned and reviewer merge requests. Closes https://gitlab.com/gitlab-org/gitlab/-/issues/290355 --- app/helpers/dashboard_helper.rb | 4 +++ app/helpers/issuables_helper.rb | 4 +++ app/models/user.rb | 6 +++++ app/views/layouts/header/_default.html.haml | 30 ++++++++++++++++----- locale/gitlab.pot | 6 +++++ 5 files changed, 44 insertions(+), 6 deletions(-) diff --git a/app/helpers/dashboard_helper.rb b/app/helpers/dashboard_helper.rb index 195b3162039118..08f357916b504c 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/issuables_helper.rb b/app/helpers/issuables_helper.rb index 15842dec3dd771..c7d698c7a7c0a5 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -249,6 +249,10 @@ def assigned_issuables_count(issuable_type) end end + def reviewer_issuables_count + current_user.reviewer_open_merge_requests_count + end + def issuable_reference(issuable) @show_full_reference ? issuable.to_reference(full: true) : issuable.to_reference(@group || @project) end diff --git a/app/models/user.rb b/app/models/user.rb index c735f20b92cfe0..bcbd0f027c99a4 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 reviewer_open_merge_requests_count(force: false) + Rails.cache.fetch(['users', id, 'reviewer_open_merge_requests_count'], force: force, expires_in: 20.minutes) do + MergeRequestsFinder.new(self, reviewer_id: self.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 diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 70ab0a56581243..a38c554ce5a029 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -47,17 +47,35 @@ %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', + - merge_requests_assigned_count = assigned_issuables_count(:merge_requests) + - merge_requests_reviewer_count = reviewer_issuables_count + - merge_requests_total_count = merge_requests_assigned_count + merge_requests_reviewer_count + = nav_link(path: 'dashboard#merge_requests', html_options: { class: "user-counter dropdown" }) do + = link_to assigned_mrs_dashboard_path, class: 'dashboard-shortcuts-merge_requests', aria: { label: _('Merge requests') }, + data: { qa_selector: 'merge_requests_shortcut_button', + toggle: "dropdown", 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{ class: ('hidden' if merge_requests_total_count == 0) } + = number_with_delimiter(merge_requests_total_count) + = sprite_icon('chevron-down', css_class: 'caret-down') + .dropdown-menu.dropdown-menu-right + %ul + %li.dropdown-header + = _('Merge requests') + %li + = link_to assigned_mrs_dashboard_path do + = _('Assigned to you') + %span.badge.gl-badge.badge-pill.badge-muted.float-right{ class: "gl-text-gray-300!" } + = merge_requests_assigned_count + %li + = link_to reviewer_mrs_dashboard_path do + = _('Review requests for you') + %span.badge.gl-badge.badge-pill.badge-muted.float-right{ class: "gl-text-gray-300!" } + = merge_requests_reviewer_count - 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/locale/gitlab.pot b/locale/gitlab.pot index 88d9f3f3cbfd00..9a93cb3e8f0343 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 "" -- GitLab From b586ed8d8b6070caec759fb314e0a5e35cf5982f Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 16 Dec 2020 09:18:54 +0000 Subject: [PATCH 02/13] Fixed cache not reseting Fixed the badge counts not updating in realtime --- .../commons/nav/user_merge_requests.js | 21 +++++++++++++++---- .../reviewers/sidebar_reviewers.vue | 4 ++-- app/models/user.rb | 1 + app/services/merge_requests/update_service.rb | 2 ++ app/views/layouts/header/_default.html.haml | 8 +++---- lib/api/user_counts.rb | 3 ++- 6 files changed, 28 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/commons/nav/user_merge_requests.js b/app/assets/javascripts/commons/nav/user_merge_requests.js index 1e2cf756059dd9..25cc883da665ca 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,13 @@ function updateUserMergeRequestCounts(newCount) { export function refreshUserMergeRequestCounts() { return Api.userCounts() .then(({ data }) => { - const count = data.merge_requests; + const assignedMergeRequests = data.merge_requests; + const reviewerMergeRequests = data.reviewer_merge_requests; - updateUserMergeRequestCounts(count); - broadcastCount(count); + updateUserMergeRequestCounts(assignedMergeRequests); + updateReviewerMergeRequestCounts(reviewerMergeRequests); + updateMergeRequestCounts(assignedMergeRequests + reviewerMergeRequests); + broadcastCount(assignedMergeRequests); }) .catch((ex) => { console.error(ex); // eslint-disable-line no-console diff --git a/app/assets/javascripts/sidebar/components/reviewers/sidebar_reviewers.vue b/app/assets/javascripts/sidebar/components/reviewers/sidebar_reviewers.vue index aee94a55134e45..d9228450371fc5 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/models/user.rb b/app/models/user.rb index bcbd0f027c99a4..ae6151da472359 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1613,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, 'reviewer_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 1d0ab403f12b22..d2e5a2a1619c2e 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 a38c554ce5a029..c12c2c43e1d055 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -59,9 +59,9 @@ track_property: 'navigation', container: 'body' } do = sprite_icon('git-merge') - %span.badge.badge-pill.merge-requests-count{ class: ('hidden' if merge_requests_total_count == 0) } + %span.badge.badge-pill.merge-requests-count.js-merge-requests-count{ class: ('hidden' if merge_requests_total_count == 0) } = number_with_delimiter(merge_requests_total_count) - = sprite_icon('chevron-down', css_class: 'caret-down') + = sprite_icon('chevron-down', css_class: 'caret-down gl-mx-0') .dropdown-menu.dropdown-menu-right %ul %li.dropdown-header @@ -69,12 +69,12 @@ %li = link_to assigned_mrs_dashboard_path do = _('Assigned to you') - %span.badge.gl-badge.badge-pill.badge-muted.float-right{ class: "gl-text-gray-300!" } + %span.badge.gl-badge.badge-pill.badge-muted.float-right.js-assigned-mr-count{ class: "gl-text-gray-500! gl-font-weight-normal! gl-font-sm! gl-line-height-normal!" } = merge_requests_assigned_count %li = link_to reviewer_mrs_dashboard_path do = _('Review requests for you') - %span.badge.gl-badge.badge-pill.badge-muted.float-right{ class: "gl-text-gray-300!" } + %span.badge.gl-badge.badge-pill.badge-muted.float-right.js-reviewer-mr-count{ class: "gl-text-gray-500! gl-font-weight-normal! gl-font-sm! gl-line-height-normal!" } = merge_requests_reviewer_count - if header_link?(:todos) = nav_link(controller: 'dashboard/todos', html_options: { class: "user-counter" }) do diff --git a/lib/api/user_counts.rb b/lib/api/user_counts.rb index 3071f08e1de108..e6c3fe6f570974 100644 --- a/lib/api/user_counts.rb +++ b/lib/api/user_counts.rb @@ -12,7 +12,8 @@ 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, + reviewer_merge_requests: current_user.reviewer_open_merge_requests_count, } end end -- GitLab From 267b120d7099aa227d11c41932009f5c75fe4a4c Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 16 Dec 2020 09:26:24 +0000 Subject: [PATCH 03/13] Updated design of the badges --- app/assets/stylesheets/framework/header.scss | 4 ++-- app/views/layouts/header/_default.html.haml | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index a6a01c7b09021d..730e10114c3b2a 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/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index c12c2c43e1d055..3ac88bc11000ec 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -67,14 +67,14 @@ %li.dropdown-header = _('Merge requests') %li - = link_to assigned_mrs_dashboard_path do + = 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.float-right.js-assigned-mr-count{ class: "gl-text-gray-500! gl-font-weight-normal! gl-font-sm! gl-line-height-normal!" } + %span.badge.gl-badge.badge-pill.badge-muted.merge-request-badge.gl-ml-auto.js-assigned-mr-count{ class: "" } = merge_requests_assigned_count %li - = link_to reviewer_mrs_dashboard_path do + = 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.float-right.js-reviewer-mr-count{ class: "gl-text-gray-500! gl-font-weight-normal! gl-font-sm! gl-line-height-normal!" } + %span.badge.gl-badge.badge-pill.badge-muted.merge-request-badge.gl-ml-auto.js-reviewer-mr-count{ class: "" } = merge_requests_reviewer_count - if header_link?(:todos) = nav_link(controller: 'dashboard/todos', html_options: { class: "user-counter" }) do -- GitLab From f21bfc89c065dd6b9271b1cc0b1cb0683953b021 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 16 Dec 2020 10:56:14 +0000 Subject: [PATCH 04/13] Fixed failing user merge requests Jest spec --- .../javascripts/commons/nav/user_merge_requests.js | 9 +++++---- lib/api/user_counts.rb | 2 +- .../frontend/commons/nav/user_merge_requests_spec.js | 12 +++++++----- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/commons/nav/user_merge_requests.js b/app/assets/javascripts/commons/nav/user_merge_requests.js index 25cc883da665ca..778b8e59abfa9a 100644 --- a/app/assets/javascripts/commons/nav/user_merge_requests.js +++ b/app/assets/javascripts/commons/nav/user_merge_requests.js @@ -34,11 +34,12 @@ export function refreshUserMergeRequestCounts() { .then(({ data }) => { const assignedMergeRequests = data.merge_requests; const reviewerMergeRequests = data.reviewer_merge_requests; + const fullCount = assignedMergeRequests + reviewerMergeRequests; updateUserMergeRequestCounts(assignedMergeRequests); updateReviewerMergeRequestCounts(reviewerMergeRequests); - updateMergeRequestCounts(assignedMergeRequests + reviewerMergeRequests); - broadcastCount(assignedMergeRequests); + updateMergeRequestCounts(fullCount); + broadcastCount(fullCount); }) .catch((ex) => { console.error(ex); // eslint-disable-line no-console @@ -72,8 +73,8 @@ export function openUserCountsBroadcast() { const currentUserId = typeof gon !== 'undefined' && gon && gon.current_user_id; if (currentUserId) { channel = new BroadcastChannel(`mr_count_channel_${currentUserId}`); - channel.onmessage = (ev) => { - updateUserMergeRequestCounts(ev.data); + channel.onmessage = ev => { + updateMergeRequestCounts(ev.data); }; } } diff --git a/lib/api/user_counts.rb b/lib/api/user_counts.rb index e6c3fe6f570974..5ad73204d41437 100644 --- a/lib/api/user_counts.rb +++ b/lib/api/user_counts.rb @@ -13,7 +13,7 @@ class UserCounts < ::API::Base { merge_requests: current_user.assigned_open_merge_requests_count, - reviewer_merge_requests: current_user.reviewer_open_merge_requests_count, + reviewer_merge_requests: current_user.reviewer_open_merge_requests_count } end end diff --git a/spec/frontend/commons/nav/user_merge_requests_spec.js b/spec/frontend/commons/nav/user_merge_requests_spec.js index 4da6d53557a315..be8e234d95d133 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(`
0
`); + setFixtures( + `
0
`, + ); }); const findMRCountText = () => document.body.querySelector(`.${MR_COUNT_CLASS}`).textContent; @@ -33,7 +35,7 @@ describe('User Merge Requests', () => { beforeEach(() => { Api.userCounts.mockReturnValue( Promise.resolve({ - data: { merge_requests: TEST_COUNT }, + data: { merge_requests: TEST_COUNT, reviewer_merge_requests: TEST_COUNT }, }), ); }); @@ -46,7 +48,7 @@ describe('User Merge Requests', () => { }); it('updates the top count of merge requests', () => { - expect(findMRCountText()).toEqual(TEST_COUNT.toLocaleString()); + expect(findMRCountText()).toEqual(Number(TEST_COUNT + TEST_COUNT).toLocaleString()); }); it('calls the API', () => { @@ -54,7 +56,7 @@ describe('User Merge Requests', () => { }); it('posts count to BroadcastChannel', () => { - expect(channelMock.postMessage).toHaveBeenCalledWith(TEST_COUNT); + expect(channelMock.postMessage).toHaveBeenCalledWith(TEST_COUNT + TEST_COUNT); }); }); -- GitLab From f0b271934168c773b9f37d23c90b9f31666db870 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 16 Dec 2020 12:53:18 +0000 Subject: [PATCH 05/13] Fixed failing user model spec --- spec/models/user_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index fb05c9e8052ecb..87eae31dd71050 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -4084,6 +4084,7 @@ def add_user(access) cache_mock = double 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, 'reviewer_open_merge_requests_count']) allow(Rails).to receive(:cache).and_return(cache_mock) -- GitLab From de13ce3254bc1eedabb0dd62ee43a070f7f9a93b Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 16 Dec 2020 16:14:56 +0000 Subject: [PATCH 06/13] Fixed margin on the dropdown caret Fixed mobile styling --- .../stylesheets/framework/dropdowns.scss | 8 ++++ app/views/layouts/header/_default.html.haml | 42 ++++++++++--------- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index e2335c184b0b11..c5ffcb48814d7d 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/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 3ac88bc11000ec..1ba08097654383 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -47,13 +47,15 @@ %span.badge.badge-pill.issues-count.green-badge{ class: ('hidden' if issues_count == 0) } = number_with_delimiter(issues_count) - if header_link?(:merge_requests) + - reviewers_enabled = Feature.enabled?(:merge_request_reviewers) - merge_requests_assigned_count = assigned_issuables_count(:merge_requests) - - merge_requests_reviewer_count = reviewer_issuables_count + - merge_requests_reviewer_count = reviewers_enabled ? reviewer_issuables_count : 0 - merge_requests_total_count = merge_requests_assigned_count + merge_requests_reviewer_count - = nav_link(path: 'dashboard#merge_requests', html_options: { class: "user-counter dropdown" }) do - = link_to assigned_mrs_dashboard_path, class: 'dashboard-shortcuts-merge_requests', aria: { label: _('Merge requests') }, + = 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: "dropdown", + toggle: reviewers_enabled ? "dropdown" : "tooltip", + placement: 'bottom', track_label: 'main_navigation', track_event: 'click_merge_link', track_property: 'navigation', @@ -61,21 +63,23 @@ = sprite_icon('git-merge') %span.badge.badge-pill.merge-requests-count.js-merge-requests-count{ class: ('hidden' if merge_requests_total_count == 0) } = number_with_delimiter(merge_requests_total_count) - = sprite_icon('chevron-down', css_class: 'caret-down gl-mx-0') - .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: "" } - = merge_requests_assigned_count - %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: "" } - = merge_requests_reviewer_count + - 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: "" } + = merge_requests_assigned_count + %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: "" } + = merge_requests_reviewer_count - 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', -- GitLab From a9d0580fb82b0780930ad29a5c84a41b2ef18a72 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 17 Dec 2020 11:53:18 +0000 Subject: [PATCH 07/13] Added a feature spec Added spec for reviewer count --- spec/features/dashboard/merge_requests_spec.rb | 6 ++++++ spec/models/user_spec.rb | 14 ++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/spec/features/dashboard/merge_requests_spec.rb b/spec/features/dashboard/merge_requests_spec.rb index 0e76b5478a1d17..26b376be660488 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/models/user_spec.rb b/spec/models/user_spec.rb index 87eae31dd71050..66f170f9f95da5 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -4164,6 +4164,20 @@ def add_user(access) end end + describe '#reviewer_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.reviewer_open_merge_requests_count(force: true)).to eq 1 + end + end + describe '#assigned_open_issues_count' do it 'returns number of open issues from non-archived projects' do user = create(:user) -- GitLab From ade1eabb39594140b84c9e1099fdbcac3fd52b67 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 17 Dec 2020 12:15:27 +0000 Subject: [PATCH 08/13] Changed method names to better match what it is Updated API to return better named properties --- app/assets/javascripts/commons/nav/user_merge_requests.js | 4 ++-- app/helpers/issuables_helper.rb | 4 ---- app/helpers/merge_requests_helper.rb | 4 ++++ app/models/user.rb | 6 +++--- app/views/layouts/header/_default.html.haml | 2 +- lib/api/user_counts.rb | 5 +++-- spec/frontend/commons/nav/user_merge_requests_spec.js | 5 ++++- spec/models/user_spec.rb | 6 +++--- 8 files changed, 20 insertions(+), 16 deletions(-) diff --git a/app/assets/javascripts/commons/nav/user_merge_requests.js b/app/assets/javascripts/commons/nav/user_merge_requests.js index 778b8e59abfa9a..de3b01c72a1854 100644 --- a/app/assets/javascripts/commons/nav/user_merge_requests.js +++ b/app/assets/javascripts/commons/nav/user_merge_requests.js @@ -32,8 +32,8 @@ function updateMergeRequestCounts(newCount) { export function refreshUserMergeRequestCounts() { return Api.userCounts() .then(({ data }) => { - const assignedMergeRequests = data.merge_requests; - const reviewerMergeRequests = data.reviewer_merge_requests; + const assignedMergeRequests = data.assigned_merge_requests; + const reviewerMergeRequests = data.review_requested_merge_requests; const fullCount = assignedMergeRequests + reviewerMergeRequests; updateUserMergeRequestCounts(assignedMergeRequests); diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index c7d698c7a7c0a5..15842dec3dd771 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -249,10 +249,6 @@ def assigned_issuables_count(issuable_type) end end - def reviewer_issuables_count - current_user.reviewer_open_merge_requests_count - end - def issuable_reference(issuable) @show_full_reference ? issuable.to_reference(full: true) : issuable.to_reference(@group || @project) end diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 37e701c1c2b7cd..0841da7f340d0e 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -159,6 +159,10 @@ def toggle_draft_merge_request_path(issuable) issuable_path(issuable, { merge_request: { wip_event: wip_event } }) end + + def review_requested_merge_requests_count + 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 ae6151da472359..7284167fb27111 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1564,8 +1564,8 @@ def assigned_open_merge_requests_count(force: false) end end - def reviewer_open_merge_requests_count(force: false) - Rails.cache.fetch(['users', id, 'reviewer_open_merge_requests_count'], force: force, expires_in: 20.minutes) do + 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: self.id, state: 'opened', non_archived: true).execute.count end end @@ -1613,7 +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, 'reviewer_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/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 1ba08097654383..500d52a9ec2f39 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -49,7 +49,7 @@ - if header_link?(:merge_requests) - reviewers_enabled = Feature.enabled?(:merge_request_reviewers) - merge_requests_assigned_count = assigned_issuables_count(:merge_requests) - - merge_requests_reviewer_count = reviewers_enabled ? reviewer_issuables_count : 0 + - merge_requests_reviewer_count = reviewers_enabled ? review_requested_merge_requests_count : 0 - merge_requests_total_count = merge_requests_assigned_count + merge_requests_reviewer_count = 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') }, diff --git a/lib/api/user_counts.rb b/lib/api/user_counts.rb index 5ad73204d41437..31c923a219a48d 100644 --- a/lib/api/user_counts.rb +++ b/lib/api/user_counts.rb @@ -12,8 +12,9 @@ class UserCounts < ::API::Base unauthorized! unless current_user { - merge_requests: current_user.assigned_open_merge_requests_count, - reviewer_merge_requests: current_user.reviewer_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/spec/frontend/commons/nav/user_merge_requests_spec.js b/spec/frontend/commons/nav/user_merge_requests_spec.js index be8e234d95d133..5f81067f84f5d9 100644 --- a/spec/frontend/commons/nav/user_merge_requests_spec.js +++ b/spec/frontend/commons/nav/user_merge_requests_spec.js @@ -35,7 +35,10 @@ describe('User Merge Requests', () => { beforeEach(() => { Api.userCounts.mockReturnValue( Promise.resolve({ - data: { merge_requests: TEST_COUNT, reviewer_merge_requests: TEST_COUNT }, + data: { + assigned_merge_requests: TEST_COUNT, + review_requested_merge_requests: TEST_COUNT, + }, }), ); }); diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 66f170f9f95da5..3c5bc1250110d4 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -4084,7 +4084,7 @@ def add_user(access) cache_mock = double 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, 'reviewer_open_merge_requests_count']) + expect(cache_mock).to receive(:delete).with(['users', user.id, 'review_requested_open_merge_requests_count']) allow(Rails).to receive(:cache).and_return(cache_mock) @@ -4164,7 +4164,7 @@ def add_user(access) end end - describe '#reviewer_open_merge_requests_count' do + describe '#review_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) @@ -4174,7 +4174,7 @@ def add_user(access) create(:merge_request, :closed, source_project: project, author: user, reviewers: [user]) create(:merge_request, source_project: archived_project, author: user, reviewers: [user]) - expect(user.reviewer_open_merge_requests_count(force: true)).to eq 1 + expect(user.review_requested_open_merge_requests_count(force: true)).to eq 1 end end -- GitLab From 4fca2a05e9a46b3149d22ad01867ed067cc3f75b Mon Sep 17 00:00:00 2001 From: Patrick Bajao Date: Tue, 22 Dec 2020 17:58:26 +0800 Subject: [PATCH 09/13] Add missing specs for backend changes This also includes changes in views to move some logic to helpers. --- app/helpers/merge_requests_helper.rb | 4 ++++ app/models/user.rb | 2 +- app/views/layouts/header/_default.html.haml | 4 ++-- spec/helpers/dashboard_helper_spec.rb | 6 ++++++ spec/services/merge_requests/update_service_spec.rb | 13 +++++++++++++ 5 files changed, 26 insertions(+), 3 deletions(-) diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 0841da7f340d0e..9ba0ca8191f989 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -163,6 +163,10 @@ def toggle_draft_merge_request_path(issuable) def review_requested_merge_requests_count current_user.review_requested_open_merge_requests_count end + + def merge_request_reviewers_enabled? + Feature.enabled?(:merge_request_reviewers) + end end MergeRequestsHelper.prepend_if_ee('EE::MergeRequestsHelper') diff --git a/app/models/user.rb b/app/models/user.rb index 7284167fb27111..284182797c03cc 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1566,7 +1566,7 @@ def assigned_open_merge_requests_count(force: false) 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: self.id, state: 'opened', non_archived: true).execute.count + MergeRequestsFinder.new(self, reviewer_id: id, state: 'opened', non_archived: true).execute.count end end diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 500d52a9ec2f39..50d2b066b99c20 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -47,7 +47,7 @@ %span.badge.badge-pill.issues-count.green-badge{ class: ('hidden' if issues_count == 0) } = number_with_delimiter(issues_count) - if header_link?(:merge_requests) - - reviewers_enabled = Feature.enabled?(:merge_request_reviewers) + - reviewers_enabled = merge_request_reviewers_enabled? - merge_requests_assigned_count = assigned_issuables_count(:merge_requests) - merge_requests_reviewer_count = reviewers_enabled ? review_requested_merge_requests_count : 0 - merge_requests_total_count = merge_requests_assigned_count + merge_requests_reviewer_count @@ -61,7 +61,7 @@ track_property: 'navigation', container: 'body' } do = sprite_icon('git-merge') - %span.badge.badge-pill.merge-requests-count.js-merge-requests-count{ class: ('hidden' if merge_requests_total_count == 0) } + %span.badge.badge-pill.merge-requests-count.js-merge-requests-count{ class: ('hidden' if merge_requests_total_count.zero?) } = number_with_delimiter(merge_requests_total_count) - if reviewers_enabled = sprite_icon('chevron-down', css_class: 'caret-down gl-mx-0!') diff --git a/spec/helpers/dashboard_helper_spec.rb b/spec/helpers/dashboard_helper_spec.rb index 65182dcb7299a4..8a76771be0ab89 100644 --- a/spec/helpers/dashboard_helper_spec.rb +++ b/spec/helpers/dashboard_helper_spec.rb @@ -89,4 +89,10 @@ it { is_expected.to eq(false) } end + + describe '#reviewer_mrs_dashboard_path' do + subject { helper.reviewer_mrs_dashboard_path } + + it { is_expected.to eq(merge_requests_dashboard_path(reviewer_username: user.username)) } + end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index be802c430ff227..fdda1515cce942 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -521,6 +521,19 @@ def update_merge_request(opts) should_email(user2) should_email(user3) end + + it 'updates open merge request counter for reviewers', :use_clean_rails_memory_store_caching do + merge_request.reviewers = [user3] + + # Cache them to ensure the cache gets invalidated on update + expect(user2.review_requested_open_merge_requests_count).to eq(0) + expect(user3.review_requested_open_merge_requests_count).to eq(1) + + update_merge_request(reviewer_ids: [user2.id]) + + expect(user2.review_requested_open_merge_requests_count).to eq(1) + expect(user3.review_requested_open_merge_requests_count).to eq(0) + end end context 'when the milestone is removed' do -- GitLab From 5f7b44ba82a1ef6fabdc9d01a4e096b3f9ee76ce Mon Sep 17 00:00:00 2001 From: Patrick Bajao Date: Tue, 22 Dec 2020 18:28:13 +0800 Subject: [PATCH 10/13] Set feature flag default_enabled to false Also reverted the change for using zero as linter is complaining. --- app/helpers/merge_requests_helper.rb | 2 +- app/views/layouts/header/_default.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 9ba0ca8191f989..1cd599cf346000 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -165,7 +165,7 @@ def review_requested_merge_requests_count end def merge_request_reviewers_enabled? - Feature.enabled?(:merge_request_reviewers) + Feature.enabled?(:merge_request_reviewers, default_enabled: false) end end diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 50d2b066b99c20..f0f8a0b8340ccb 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -61,7 +61,7 @@ track_property: 'navigation', container: 'body' } do = sprite_icon('git-merge') - %span.badge.badge-pill.merge-requests-count.js-merge-requests-count{ class: ('hidden' if merge_requests_total_count.zero?) } + %span.badge.badge-pill.merge-requests-count.js-merge-requests-count{ class: ('hidden' if merge_requests_total_count == 0) } = number_with_delimiter(merge_requests_total_count) - if reviewers_enabled = sprite_icon('chevron-down', css_class: 'caret-down gl-mx-0!') -- GitLab From dde4f0ddb793a2c60f715366d103e0400d0cae5d Mon Sep 17 00:00:00 2001 From: Patrick Bajao Date: Tue, 22 Dec 2020 19:11:37 +0800 Subject: [PATCH 11/13] Set feature flag default_enabled to yaml This is to match whatever is defined in feature flag's yaml file. --- app/helpers/merge_requests_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 1cd599cf346000..eeb4f9c8e9051d 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -165,7 +165,7 @@ def review_requested_merge_requests_count end def merge_request_reviewers_enabled? - Feature.enabled?(:merge_request_reviewers, default_enabled: false) + Feature.enabled?(:merge_request_reviewers, default_enabled: :yaml) end end -- GitLab From d06d0d7e4603481eb9cbbcbfffd73ed219a4ae11 Mon Sep 17 00:00:00 2001 From: Patrick Bajao Date: Mon, 4 Jan 2021 18:32:12 +0800 Subject: [PATCH 12/13] Add user_merge_requests_counts helper This is to consolidate the logic for getting the assigned, review requested and total counts used in the view. --- app/helpers/merge_requests_helper.rb | 22 ++++++++++++-- app/views/layouts/header/_default.html.haml | 11 +++---- spec/helpers/merge_requests_helper_spec.rb | 33 +++++++++++++++++++++ 3 files changed, 57 insertions(+), 9 deletions(-) diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index eeb4f9c8e9051d..bbfd06ca06daa8 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -160,13 +160,31 @@ def toggle_draft_merge_request_path(issuable) issuable_path(issuable, { merge_request: { wip_event: wip_event } }) end - def review_requested_merge_requests_count - current_user.review_requested_open_merge_requests_count + 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/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index f0f8a0b8340ccb..1394f15cd1515c 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -48,9 +48,6 @@ = number_with_delimiter(issues_count) - if header_link?(:merge_requests) - reviewers_enabled = merge_request_reviewers_enabled? - - merge_requests_assigned_count = assigned_issuables_count(:merge_requests) - - merge_requests_reviewer_count = reviewers_enabled ? review_requested_merge_requests_count : 0 - - merge_requests_total_count = merge_requests_assigned_count + merge_requests_reviewer_count = 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', @@ -61,8 +58,8 @@ track_property: 'navigation', container: 'body' } do = sprite_icon('git-merge') - %span.badge.badge-pill.merge-requests-count.js-merge-requests-count{ class: ('hidden' if merge_requests_total_count == 0) } - = number_with_delimiter(merge_requests_total_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 @@ -74,12 +71,12 @@ = 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: "" } - = merge_requests_assigned_count + = 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: "" } - = merge_requests_reviewer_count + = 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/spec/helpers/merge_requests_helper_spec.rb b/spec/helpers/merge_requests_helper_spec.rb index 377e2c43a72bff..821faaab194a7d 100644 --- a/spec/helpers/merge_requests_helper_spec.rb +++ b/spec/helpers/merge_requests_helper_spec.rb @@ -67,4 +67,37 @@ end end end + + describe '#user_merge_requests_counts' do + let(:user) do + double( + assigned_open_merge_requests_count: 1, + review_requested_open_merge_requests_count: 2 + ) + end + + subject { helper.user_merge_requests_counts } + + before do + allow(helper).to receive(:current_user).and_return(user) + end + + 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 + + context 'when merge_request_reviewers is disabled' do + before do + stub_feature_flags(merge_request_reviewers: false) + end + + it 'returns review_requested as 0' do + expect(subject[:review_requested]).to eq(0) + end + end + end end -- GitLab From 7f106c0345bf3a05ed1a6c0748cabc59b4542758 Mon Sep 17 00:00:00 2001 From: Patrick Bajao Date: Mon, 4 Jan 2021 19:02:24 +0800 Subject: [PATCH 13/13] Prettify user_merge_requests.js file --- app/assets/javascripts/commons/nav/user_merge_requests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/commons/nav/user_merge_requests.js b/app/assets/javascripts/commons/nav/user_merge_requests.js index de3b01c72a1854..eab2b5a31d75c0 100644 --- a/app/assets/javascripts/commons/nav/user_merge_requests.js +++ b/app/assets/javascripts/commons/nav/user_merge_requests.js @@ -73,7 +73,7 @@ export function openUserCountsBroadcast() { const currentUserId = typeof gon !== 'undefined' && gon && gon.current_user_id; if (currentUserId) { channel = new BroadcastChannel(`mr_count_channel_${currentUserId}`); - channel.onmessage = ev => { + channel.onmessage = (ev) => { updateMergeRequestCounts(ev.data); }; } -- GitLab