diff --git a/app/assets/javascripts/merge_requests/components/reviewers/reviewer_dropdown.vue b/app/assets/javascripts/merge_requests/components/reviewers/reviewer_dropdown.vue index 62106936db838b1366ea9db503612182036c43a4..90df79503369a83005d9045a581b5399d9273b91 100644 --- a/app/assets/javascripts/merge_requests/components/reviewers/reviewer_dropdown.vue +++ b/app/assets/javascripts/merge_requests/components/reviewers/reviewer_dropdown.vue @@ -42,6 +42,11 @@ export default { required: false, default: () => [], }, + visibleReviewers: { + type: Array, + required: false, + default: () => [], + }, }, data() { return { @@ -57,10 +62,10 @@ export default { const items = []; let users; - if (this.selectedReviewers.length && !this.search) { + if (this.selectedReviewersToShow.length && !this.search) { items.push({ text: __('Reviewers'), - options: this.selectedReviewers.map((user) => this.mapUser(user)), + options: this.selectedReviewersToShow.map((user) => this.mapUser(user)), }); } @@ -82,6 +87,11 @@ export default { return items; }, + selectedReviewersToShow() { + return this.selectedReviewers.filter((user) => + this.visibleReviewers.map((gqlUser) => gqlUser.id).includes(user.id), + ); + }, }, watch: { selectedReviewers(newVal) { diff --git a/ee/app/assets/javascripts/merge_requests/components/reviewers/approval_rules.vue b/ee/app/assets/javascripts/merge_requests/components/reviewers/approval_rules.vue index d0cc7f85e340ef7cea792286b63061c2a5033411..012d7c30c93eaba4107d68530a43c966ba460eb7 100644 --- a/ee/app/assets/javascripts/merge_requests/components/reviewers/approval_rules.vue +++ b/ee/app/assets/javascripts/merge_requests/components/reviewers/approval_rules.vue @@ -69,6 +69,15 @@ export default { toggleApprovalSections() { this.showApprovalSections = !this.showApprovalSections; }, + visibleReviewersForRule(rule) { + let visible = rule.reviewers; + + if (rule.type.toLowerCase() === RULE_TYPE_ANY_APPROVER) { + visible = this.reviewers; + } + + return visible; + }, }, ANY_APPROVER: RULE_TYPE_ANY_APPROVER.toUpperCase(), CODE_OWNERS: RULE_TYPE_CODE_OWNER.toUpperCase(), @@ -148,6 +157,7 @@ export default {
diff --git a/ee/spec/frontend/merge_requests/components/reviewers/approval_rules_spec.js b/ee/spec/frontend/merge_requests/components/reviewers/approval_rules_spec.js index 5f09f344f19ae55aa2a97559dd2c27e8c2683fb9..63b446dfcc00910b1288bc3a2902fbec2aa466a2 100644 --- a/ee/spec/frontend/merge_requests/components/reviewers/approval_rules_spec.js +++ b/ee/spec/frontend/merge_requests/components/reviewers/approval_rules_spec.js @@ -39,6 +39,7 @@ describe('Reviewer drawer approval rules component', () => { { approvalsRequired, name: 'Optional rule', + type: 'any_approver', approvedBy: { nodes: [], }, @@ -46,6 +47,7 @@ describe('Reviewer drawer approval rules component', () => { { approvalsRequired, name: 'Required rule', + type: 'code_owner', approvedBy: { nodes: [], }, @@ -53,6 +55,7 @@ describe('Reviewer drawer approval rules component', () => { { approvalsRequired, name: 'Approved rule', + type: 'regular', approvedBy: { nodes: [{ id: 1 }], }, @@ -108,6 +111,7 @@ describe('Reviewer drawer approval rules component', () => { approvalsRequired: 1, name: 'Approved rule', section: 'Frontend', + type: 'code_owner', approvedBy: { nodes: [{ id: 1 }], }, @@ -123,6 +127,7 @@ describe('Reviewer drawer approval rules component', () => { approvalsRequired: 1, name: 'Approved rule', section: 'codeowners', + type: 'code_owner', approvedBy: { nodes: [{ id: 1 }], }, diff --git a/spec/frontend/merge_requests/components/reviewers/reviewer_dropdown_spec.js b/spec/frontend/merge_requests/components/reviewers/reviewer_dropdown_spec.js index 3ca5fc9de95381e6bb1529f8538fed27e5eb7b39..4d881ccf51758930351ff417d95a3564255056c9 100644 --- a/spec/frontend/merge_requests/components/reviewers/reviewer_dropdown_spec.js +++ b/spec/frontend/merge_requests/components/reviewers/reviewer_dropdown_spec.js @@ -16,19 +16,19 @@ let setReviewersMutationMock; Vue.use(VueApollo); -const createMockUser = () => ({ +const createMockUser = ({ id = 1, name = 'Administrator', username = 'root' } = {}) => ({ __typename: 'UserCore', - id: 'gid://gitlab/User/1', + id: `gid://gitlab/User/${id}`, avatarUrl: 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', - name: 'Administrator', - username: 'root', - webUrl: '/root', - webPath: '/root', + webUrl: `/${username}`, + webPath: `/${username}`, status: null, mergeRequestInteraction: { canMerge: true, }, + username, + name, }); function createComponent( @@ -39,7 +39,7 @@ function createComponent( data: { workspace: { id: 1, - users: [createMockUser()], + users: [createMockUser(), createMockUser({ id: 2, name: 'Nonadmin', username: 'bob' })], }, }, }); @@ -131,25 +131,62 @@ describe('Reviewer dropdown component', () => { }); }); - it('renders users from autocomplete endpoint', async () => { - findDropdown().vm.$emit('shown'); + describe('with the user already selected', () => { + it('renders users from autocomplete endpoint and skips "ineligible" pre-selected reviewers', async () => { + findDropdown().vm.$emit('shown'); + + await waitForPromises(); + + expect(findDropdown().props('items')).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + options: expect.arrayContaining([ + expect.objectContaining({ + secondaryText: '@bob', + text: 'Nonadmin', + value: 'bob', + mergeRequestInteraction: { canMerge: true }, + }), + ]), + }), + ]), + ); + }); + }); - await waitForPromises(); + describe('with the user not already selected', () => { + beforeEach(async () => { + createComponent(true, {}); - expect(findDropdown().props('items')).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - options: expect.arrayContaining([ - expect.objectContaining({ - secondaryText: '@root', - text: 'Administrator', - value: 'root', - mergeRequestInteraction: { canMerge: true }, - }), - ]), - }), - ]), - ); + await waitForPromises(); + }); + + it('renders users from autocomplete endpoint', async () => { + findDropdown().vm.$emit('shown'); + + await waitForPromises(); + + expect(findDropdown().props('items')).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + options: expect.arrayContaining([ + expect.objectContaining({ + secondaryText: '@root', + text: 'Administrator', + value: 'root', + mergeRequestInteraction: { canMerge: true }, + }), + expect.objectContaining({ + secondaryText: '@bob', + text: 'Nonadmin', + value: 'bob', + mergeRequestInteraction: { canMerge: true }, + }), + ]), + }), + ]), + ); + }); }); it('updates reviewers when dropdown is closed', () => {