From 7ee28604490321354c46f635ef0ac8d35eb478d5 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 18 Apr 2022 14:20:26 -0600 Subject: [PATCH 1/2] Swap from an implicit attention request toggle to explicit set/remove --- .../components/attention_requested_toggle.vue | 22 ++++++++++- .../remove_attention_request.mutation.graphql | 7 ++++ .../request_attention.mutation.graphql | 5 +++ ...oggle_attention_requested.mutation.graphql | 7 ---- .../sidebar/services/sidebar_service.js | 17 +++++++-- .../javascripts/sidebar/sidebar_mediator.js | 11 ++++-- .../attention_requested_toggle_spec.js | 1 + .../frontend/sidebar/sidebar_mediator_spec.js | 38 +++++++++++++------ 8 files changed, 81 insertions(+), 27 deletions(-) create mode 100644 app/assets/javascripts/sidebar/queries/remove_attention_request.mutation.graphql create mode 100644 app/assets/javascripts/sidebar/queries/request_attention.mutation.graphql delete mode 100644 app/assets/javascripts/sidebar/queries/toggle_attention_requested.mutation.graphql diff --git a/app/assets/javascripts/sidebar/components/attention_requested_toggle.vue b/app/assets/javascripts/sidebar/components/attention_requested_toggle.vue index cd1aae155d29db..031de6694890f4 100644 --- a/app/assets/javascripts/sidebar/components/attention_requested_toggle.vue +++ b/app/assets/javascripts/sidebar/components/attention_requested_toggle.vue @@ -47,6 +47,23 @@ export default { return this.$options.i18n.noAttentionRequestedNoPermission; }, + request() { + const state = { + variant: 'default', + icon: 'attention', + direction: 'add', + }; + + if (this.user.attention_requested) { + Object.assign(state, { + variant: 'warning', + icon: 'attention-solid', + direction: 'remove', + }); + } + + return state; + }, }, methods: { toggleAttentionRequired() { @@ -57,6 +74,7 @@ export default { this.$emit('toggle-attention-requested', { user: this.user, callback: this.toggleAttentionRequiredComplete, + direction: this.request.direction, }); }, toggleAttentionRequiredComplete() { @@ -74,8 +92,8 @@ export default { > this.service.requestAttention(id), + remove: (id) => this.service.removeAttentionRequest(id), + }; + try { const isReviewer = type === 'reviewer'; const reviewerOrAssignee = isReviewer ? this.store.findReviewer(user) : this.store.findAssignee(user); - await this.service.toggleAttentionRequested(user.id); + await mutations[direction]?.(user.id); if (reviewerOrAssignee.attention_requested) { toast( @@ -138,7 +143,7 @@ export default class SidebarMediator { captureError: true, actionConfig: { title: __('Try again'), - clickHandler: () => this.toggleAttentionRequired(type, { user, callback }), + clickHandler: () => this.toggleAttentionRequired(type, { user, callback, direction }), }, }); } diff --git a/spec/frontend/sidebar/components/attention_requested_toggle_spec.js b/spec/frontend/sidebar/components/attention_requested_toggle_spec.js index e3eabc6a0f6ab8..959fa799eb737c 100644 --- a/spec/frontend/sidebar/components/attention_requested_toggle_spec.js +++ b/spec/frontend/sidebar/components/attention_requested_toggle_spec.js @@ -68,6 +68,7 @@ describe('Attention require toggle', () => { { user: { attention_requested: true, can_update_merge_request: true }, callback: expect.anything(), + direction: 'remove', }, ]); }); diff --git a/spec/frontend/sidebar/sidebar_mediator_spec.js b/spec/frontend/sidebar/sidebar_mediator_spec.js index c472a98bf0b11c..7b8b1baafac953 100644 --- a/spec/frontend/sidebar/sidebar_mediator_spec.js +++ b/spec/frontend/sidebar/sidebar_mediator_spec.js @@ -122,25 +122,39 @@ describe('Sidebar mediator', () => { }); describe('toggleAttentionRequested', () => { - let attentionRequiredService; + let requestAttentionMock; + let removeAttentionRequestMock; beforeEach(() => { - attentionRequiredService = jest - .spyOn(mediator.service, 'toggleAttentionRequested') + requestAttentionMock = jest.spyOn(mediator.service, 'requestAttention').mockResolvedValue(); + removeAttentionRequestMock = jest + .spyOn(mediator.service, 'removeAttentionRequest') .mockResolvedValue(); }); - it('calls attentionRequired service method', async () => { - mediator.store.reviewers = [{ id: 1, attention_requested: false, username: 'root' }]; + it.each` + attentionIsCurrentlyRequested | serviceMethod + ${true} | ${'remove'} + ${false} | ${'add'} + `( + "calls the $serviceMethod service method when the user's attention request is set to $attentionIsCurrentlyRequested", + async ({ serviceMethod }) => { + const methods = { + add: requestAttentionMock, + remove: removeAttentionRequestMock, + }; + mediator.store.reviewers = [{ id: 1, attention_requested: false, username: 'root' }]; - await mediator.toggleAttentionRequested('reviewer', { - user: { id: 1, username: 'root' }, - callback: jest.fn(), - }); + await mediator.toggleAttentionRequested('reviewer', { + user: { id: 1, username: 'root' }, + callback: jest.fn(), + direction: serviceMethod, + }); - expect(attentionRequiredService).toHaveBeenCalledWith(1); - expect(refreshUserMergeRequestCounts).toHaveBeenCalled(); - }); + expect(methods[serviceMethod]).toHaveBeenCalledWith(1); + expect(refreshUserMergeRequestCounts).toHaveBeenCalled(); + }, + ); it.each` type | method -- GitLab From 4eb8504cb1a3ce34bdb1468ab9db39f872f0dcce Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 2 May 2022 12:44:10 -0600 Subject: [PATCH 2/2] Add a test for showing the error flash if updating fails --- .../frontend/sidebar/sidebar_mediator_spec.js | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/spec/frontend/sidebar/sidebar_mediator_spec.js b/spec/frontend/sidebar/sidebar_mediator_spec.js index 7b8b1baafac953..82fb10ab1d27bc 100644 --- a/spec/frontend/sidebar/sidebar_mediator_spec.js +++ b/spec/frontend/sidebar/sidebar_mediator_spec.js @@ -1,4 +1,5 @@ import MockAdapter from 'axios-mock-adapter'; +import createFlash from '~/flash'; import axios from '~/lib/utils/axios_utils'; import * as urlUtility from '~/lib/utils/url_utility'; import SidebarService, { gqClient } from '~/sidebar/services/sidebar_service'; @@ -8,6 +9,7 @@ import toast from '~/vue_shared/plugins/global_toast'; import { refreshUserMergeRequestCounts } from '~/commons/nav/user_merge_requests'; import Mock from './mock_data'; +jest.mock('~/flash'); jest.mock('~/vue_shared/plugins/global_toast'); jest.mock('~/commons/nav/user_merge_requests'); @@ -186,5 +188,27 @@ describe('Sidebar mediator', () => { expect(toast).toHaveBeenCalledWith(toastMessage); }, ); + + describe('errors', () => { + beforeEach(() => { + jest + .spyOn(mediator.service, 'removeAttentionRequest') + .mockRejectedValueOnce(new Error('Something went wrong')); + }); + + it('shows an error message', async () => { + await mediator.toggleAttentionRequested('reviewer', { + user: { id: 1, username: 'root' }, + callback: jest.fn(), + direction: 'remove', + }); + + expect(createFlash).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Updating the attention request for root failed.', + }), + ); + }); + }); }); }); -- GitLab