diff --git a/ee/app/assets/javascripts/api.js b/ee/app/assets/javascripts/api.js index 04889479225f439895595a9ed9efb07f10c014c3..3cb29b74424ffb335e4d3afc1eec4ebbbd028cb8 100644 --- a/ee/app/assets/javascripts/api.js +++ b/ee/app/assets/javascripts/api.js @@ -253,19 +253,23 @@ export default { }); }, - deploymentApproval({ id, deploymentId, approve, comment }) { + deploymentApproval({ id, deploymentId, representedAs, approve, comment }) { const url = Api.buildUrl(this.environmentApprovalPath) .replace(':id', encodeURIComponent(id)) .replace(':deployment_id', encodeURIComponent(deploymentId)); - return axios.post(url, { status: approve ? 'approved' : 'rejected', comment }); + return axios.post(url, { + status: approve ? 'approved' : 'rejected', + represented_as: representedAs, + comment, + }); }, - approveDeployment({ id, deploymentId, comment }) { - return this.deploymentApproval({ id, deploymentId, approve: true, comment }); + approveDeployment({ id, deploymentId, representedAs, comment }) { + return this.deploymentApproval({ id, deploymentId, representedAs, approve: true, comment }); }, - rejectDeployment({ id, deploymentId, comment }) { - return this.deploymentApproval({ id, deploymentId, approve: false, comment }); + rejectDeployment({ id, deploymentId, representedAs, comment }) { + return this.deploymentApproval({ id, deploymentId, approve: false, representedAs, comment }); }, protectedEnvironments(id, params = {}) { diff --git a/ee/app/assets/javascripts/deployments/components/deployment_approvals.vue b/ee/app/assets/javascripts/deployments/components/deployment_approvals.vue index a34519b5f024ab8f3da0114a6451735271ee297c..df60452ecbf7cea18e67ac5fe650b51f3a9c7d81 100644 --- a/ee/app/assets/javascripts/deployments/components/deployment_approvals.vue +++ b/ee/app/assets/javascripts/deployments/components/deployment_approvals.vue @@ -41,6 +41,7 @@ export default { errorMessage: '', approveLoading: false, rejectLoading: false, + selectedRule: null, }; }, computed: { @@ -130,6 +131,7 @@ export default { input: { id: this.deployment.id, comment: this.comment, + representedAs: this.selectedRule, status, }, }, @@ -147,6 +149,9 @@ export default { captureException(err); }); }, + onRuleSelected($event) { + this.selectedRule = $event; + }, }, i18n: { approvalCommentLabel: s__('Deployment|Add approval comment'), @@ -173,7 +178,7 @@ export default { > <gl-icon name="approval" class="gl-mr-2" /> <span>{{ header }}</span> </div> - <multiple-approval-rules-table :rules="approvalSummary.rules" /> + <multiple-approval-rules-table :rules="approvalSummary.rules" @select-rule="onRuleSelected" /> <template v-if="needsApproval"> <div class="gl-display-flex gl-flex-direction-column gl-m-5"> diff --git a/ee/app/assets/javascripts/deployments/graphql/fragments/approval_summary.fragment.graphql b/ee/app/assets/javascripts/deployments/graphql/fragments/approval_summary.fragment.graphql index 8d7eedc5682ffff45b25eeddc7bb34772769c735..147001538c2972c7d01778a33482e94bb05f3492 100644 --- a/ee/app/assets/javascripts/deployments/graphql/fragments/approval_summary.fragment.graphql +++ b/ee/app/assets/javascripts/deployments/graphql/fragments/approval_summary.fragment.graphql @@ -25,6 +25,7 @@ fragment ApprovalSummary on Deployment { approvals { ...DeploymentApprovalData } + canApprove } } userPermissions { diff --git a/ee/app/assets/javascripts/environments/components/environment_approval.vue b/ee/app/assets/javascripts/environments/components/environment_approval.vue index e8574f1db85732449631de828d8e35421fa20b96..c10cb5cb1cd579b5c112c780e1299f5ba4214af8 100644 --- a/ee/app/assets/javascripts/environments/components/environment_approval.vue +++ b/ee/app/assets/javascripts/environments/components/environment_approval.vue @@ -77,6 +77,7 @@ export default { approvalSummary: { rules: [] }, }, }, + selectedRule: null, }; }, apollo: { @@ -207,6 +208,7 @@ export default { return action({ id: this.projectId, deploymentId: this.deploymentId, + representedAs: this.selectedRule, comment: this.comment, }) .then(() => { @@ -237,6 +239,9 @@ export default { return this.$options.i18n.rejectedAt; }, + onRuleSelected($event) { + this.selectedRule = $event; + }, }, i18n: { button: s__('DeploymentApproval|Approval options'), @@ -328,6 +333,7 @@ export default { v-if="hasApprovalRules" :rules="deployment.approvalSummary.rules" class="gl-my-4 gl-pt-4" + @select-rule="onRuleSelected" /> <div v-else class="gl-my-4 gl-pt-4"> <gl-sprintf :message="$options.i18n.current"> diff --git a/ee/app/assets/javascripts/environments/components/multiple_approval_rules_table.vue b/ee/app/assets/javascripts/environments/components/multiple_approval_rules_table.vue index 131276dc6c1007eebf62e2b6b84179d18014bc7b..6853239f2eccdf7f5ddfbe657d5697e7e1ed820f 100644 --- a/ee/app/assets/javascripts/environments/components/multiple_approval_rules_table.vue +++ b/ee/app/assets/javascripts/environments/components/multiple_approval_rules_table.vue @@ -5,6 +5,7 @@ import { GlLink, GlTableLite, GlTooltipDirective as GlTooltip, + GlFormRadio, } from '@gitlab/ui'; import { s__ } from '~/locale'; @@ -20,6 +21,7 @@ export default { GlAvatarLink, GlLink, GlTableLite, + GlFormRadio, }, directives: { GlTooltip, @@ -31,24 +33,40 @@ export default { }, }, fields: [ - { - key: 'approvers', - label: s__('DeploymentApprovals|Approvers'), - }, + { key: 'approvers', label: s__('DeploymentApprovals|Approvers') }, { key: 'approvals', label: s__('DeploymentApprovals|Approvals') }, { key: 'approvedBy', label: s__('DeploymentApprovals|Approved By') }, + { key: 'giveApproval', label: s__('DeploymentApprovals|Give approval') }, ], + data() { + return { selected: null }; + }, computed: { items() { return this.rules.map((rule) => ({ - approvers: this.getRuleName(rule), + approvers: this.getRuleData(rule), approvals: `${rule.approvedCount}/${rule.requiredApprovals}`, approvedBy: rule.approvals, + giveApproval: { + canApprove: rule.canApprove, + ruleName: this.getRuleName(rule), + }, })); }, + currentUserApproved() { + return Boolean(this.rules.find((rule) => this.hasApproval(rule))); + }, + }, + mounted() { + if (!this.selected) { + const firstRuleToApprove = this.rules.find((rule) => rule.canApprove); + this.selected = this.getRuleName(firstRuleToApprove); + + this.onChange(this.selected); + } }, methods: { - getRuleName(rule) { + getRuleData(rule) { if (rule.group) { return { name: rule.group.name, link: rule.group.webUrl }; } @@ -58,6 +76,21 @@ export default { return { name: accessLevelDisplay[rule.accessLevel.stringValue] }; }, + getRuleName(rule) { + return this.getRuleData(rule).name; + }, + hasApproval(rule) { + const result = Boolean( + rule.approvals.find(({ user }) => user.username === gon.current_username), + ); + if (result) { + this.selected = this.getRuleName(rule); + } + return result; + }, + onChange($event) { + this.$emit('select-rule', $event); + }, }, }; </script> @@ -75,8 +108,18 @@ export default { :href="approval.user.webUrl" :title="approval.user.name" > - <gl-avatar :src="approval.user.avatarUrl" :size="24" /> + <gl-avatar :src="approval.user.avatarUrl" :size="24" aria-hidden="true" /> </gl-avatar-link> </template> + <template #cell(giveApproval)="{ value }"> + <gl-form-radio + v-if="value.canApprove" + v-model="selected" + :value="value.ruleName" + :disabled="currentUserApproved" + @input="onChange" + >{{ s__('DeploymentApprovals|Approve') }}</gl-form-radio + > + </template> </gl-table-lite> </template> diff --git a/ee/app/assets/javascripts/environments/graphql/queries/deployment.query.graphql b/ee/app/assets/javascripts/environments/graphql/queries/deployment.query.graphql index 14ff71572c21bf8ce49c70d36707a5a78e7dbab9..025e855d39ce04be1708a45d7254ce13342b7e22 100644 --- a/ee/app/assets/javascripts/environments/graphql/queries/deployment.query.graphql +++ b/ee/app/assets/javascripts/environments/graphql/queries/deployment.query.graphql @@ -38,6 +38,7 @@ query deployment($fullPath: ID!, $iid: ID!) { approvals { ...DeploymentApprovalData } + canApprove } } } diff --git a/ee/spec/frontend/api_spec.js b/ee/spec/frontend/api_spec.js index a8888e7704785a6504a5021b73bca9cd385ac48a..76f812298f654833de0883773129826481f07584 100644 --- a/ee/spec/frontend/api_spec.js +++ b/ee/spec/frontend/api_spec.js @@ -255,24 +255,45 @@ describe('Api', () => { const projectId = 1; const deploymentId = 2; const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects/${projectId}/deployments/${deploymentId}/approval`; + const representedAs = 'Maintainers'; const comment = 'comment'; it('sends an approval when approve is true', async () => { - mock.onPost(expectedUrl, { status: 'approved', comment }).replyOnce(HTTP_STATUS_OK); - - await Api.deploymentApproval({ id: projectId, deploymentId, approve: true, comment }); + mock + .onPost(expectedUrl, { status: 'approved', represented_as: representedAs, comment }) + .replyOnce(HTTP_STATUS_OK); + + await Api.deploymentApproval({ + id: projectId, + deploymentId, + approve: true, + representedAs, + comment, + }); expect(mock.history.post.length).toBe(1); - expect(mock.history.post[0].data).toBe(JSON.stringify({ status: 'approved', comment })); + expect(mock.history.post[0].data).toBe( + JSON.stringify({ status: 'approved', represented_as: representedAs, comment }), + ); }); it('sends a rejection when approve is false', async () => { - mock.onPost(expectedUrl, { status: 'rejected', comment }).replyOnce(HTTP_STATUS_OK); - - await Api.deploymentApproval({ id: projectId, deploymentId, approve: false, comment }); + mock + .onPost(expectedUrl, { status: 'rejected', represented_as: representedAs, comment }) + .replyOnce(HTTP_STATUS_OK); + + await Api.deploymentApproval({ + id: projectId, + deploymentId, + approve: false, + representedAs, + comment, + }); expect(mock.history.post.length).toBe(1); - expect(mock.history.post[0].data).toBe(JSON.stringify({ status: 'rejected', comment })); + expect(mock.history.post[0].data).toBe( + JSON.stringify({ status: 'rejected', represented_as: representedAs, comment }), + ); }); }); diff --git a/ee/spec/frontend/deployments/deployment_approvals_spec.js b/ee/spec/frontend/deployments/deployment_approvals_spec.js index f9dbb6ce5e6c25ce9956c51380a51e4713f568e5..d22d8064eb88292ae095087f1f329070c0b05c47 100644 --- a/ee/spec/frontend/deployments/deployment_approvals_spec.js +++ b/ee/spec/frontend/deployments/deployment_approvals_spec.js @@ -43,6 +43,7 @@ describe('ee/deployments/components/deployment_approvals.vue', () => { const findRejectButton = () => findButtons().wrappers.at(-1); const findAlert = () => wrapper.findComponent(GlAlert); const findCharacterCount = () => wrapper.findByTestId('approval-character-count'); + const findMultipleApprovalRulesTable = () => wrapper.findComponent(MultipleApprovalRulesTable); it('shows a header listing how many approvals remain', () => { createComponent(); @@ -55,9 +56,7 @@ describe('ee/deployments/components/deployment_approvals.vue', () => { it('shows the approval table when deployment needs approval', () => { createComponent(); - expect(wrapper.findComponent(MultipleApprovalRulesTable).props('rules')).toBe( - deployment.approvalSummary.rules, - ); + expect(findMultipleApprovalRulesTable().props('rules')).toBe(deployment.approvalSummary.rules); }); describe('has approved', () => { @@ -118,6 +117,7 @@ describe('ee/deployments/components/deployment_approvals.vue', () => { input: { comment: '', id: deployment.id, + representedAs: null, status, }, }); @@ -181,6 +181,21 @@ describe('ee/deployments/components/deployment_approvals.vue', () => { expect(countText.attributes('title')).toBe(tooltip); }, ); + + it('uses the correct `representedAs` when get `selectedRule` from the multiple approval rules table', () => { + const selectedRule = 'Maintainers'; + findMultipleApprovalRulesTable().vm.$emit('select-rule', selectedRule); + findApproveButton().vm.$emit('click'); + + expect(mockApprove).toHaveBeenCalledWith({ + input: { + comment: '', + id: deployment.id, + representedAs: selectedRule, + status: 'APPROVED', + }, + }); + }); }); describe('can not approve', () => { diff --git a/ee/spec/frontend/environments/environment_approval_spec.js b/ee/spec/frontend/environments/environment_approval_spec.js index 0a492314cfb6e10959e91090491c65fd7428c021..d62bdbd0fc450a8104ab7fa3ea9777e44be6a834 100644 --- a/ee/spec/frontend/environments/environment_approval_spec.js +++ b/ee/spec/frontend/environments/environment_approval_spec.js @@ -45,6 +45,8 @@ describe('ee/environments/components/environment_approval.vue', () => { const findModal = () => extendedWrapper(wrapper.findComponent(GlModal)); const findButton = () => extendedWrapper(wrapper.findComponent(GlButton)); + const findMultipleApprovalRulesTable = () => + extendedWrapper(wrapper.findComponent(MultipleApprovalRulesTable)); const setComment = (comment) => wrapper @@ -197,7 +199,7 @@ describe('ee/environments/components/environment_approval.vue', () => { if (approvals.length > 0) { const { user } = deploymentFixture.data.project.deployment.approvals[0]; user.username = approvals[0].user.username; - user.id = `${user.id}1`; // we need to bump the id, as mock appollo client maintains the proper cache inside. + user.id = `${user.id}1`; // we need to bump the id, as mock apollo client maintains the proper cache inside. } deploymentFixture.data.project.deployment.userPermissions.approveDeployment = canApproveDeployment; @@ -229,7 +231,9 @@ describe('ee/environments/components/environment_approval.vue', () => { it(`should ${ref} the deployment when ${text} is clicked`, async () => { const projectId = getIdFromGraphQLId(mockDeploymentFixture.data.project.id); - const deploymentId = getIdFromGraphQLId(mockDeploymentFixture.data.project.deployment.id); + const { deployment } = mockDeploymentFixture.data.project; + const deploymentId = getIdFromGraphQLId(deployment.id); + const representedAs = deployment.approvalSummary.rules[0].group.name; api.mockResolvedValue(); @@ -240,6 +244,7 @@ describe('ee/environments/components/environment_approval.vue', () => { expect(api).toHaveBeenCalledWith({ id: projectId, deploymentId, + representedAs, comment: 'comment', }); @@ -248,7 +253,20 @@ describe('ee/environments/components/environment_approval.vue', () => { expect(wrapper.emitted('change')).toEqual([[]]); }); - it(`should toast a messsage when ${ref} is clicked`, async () => { + it('should use the correct `representedAs` when get `selectedRule` from the multiple approval rules table', () => { + const selectedRule = 'Maintainers'; + + findMultipleApprovalRulesTable().vm.$emit('select-rule', selectedRule); + button.trigger('click'); + + expect(api).toHaveBeenCalledWith( + expect.objectContaining({ + representedAs: selectedRule, + }), + ); + }); + + it(`should toast a message when ${ref} is clicked`, async () => { api.mockResolvedValue(); await button.trigger('click'); @@ -310,7 +328,7 @@ describe('ee/environments/components/environment_approval.vue', () => { }); it('should pass the approval rules to the table', () => { - const table = wrapper.findComponent(MultipleApprovalRulesTable); + const table = findMultipleApprovalRulesTable(); expect(table.props('rules')).toEqual( mockDeploymentFixture.data.project.deployment.approvalSummary.rules, ); diff --git a/ee/spec/frontend/environments/multiple_approval_rules_table_spec.js b/ee/spec/frontend/environments/multiple_approval_rules_table_spec.js index 0407e27a502881438233b1094cdfb7872b95cd3f..8b8b6b3dce5dabe8457295525db841baecc4a1ce 100644 --- a/ee/spec/frontend/environments/multiple_approval_rules_table_spec.js +++ b/ee/spec/frontend/environments/multiple_approval_rules_table_spec.js @@ -1,8 +1,9 @@ -import { GlTableLite, GlAvatar, GlAvatarLink } from '@gitlab/ui'; +import { GlTableLite, GlAvatar, GlAvatarLink, GlFormRadio } from '@gitlab/ui'; import mockDeploymentFixture from 'test_fixtures/graphql/environments/graphql/queries/deployment.query.graphql.json'; import { mountExtended } from 'helpers/vue_test_utils_helper'; import MultipleApprovalRulesTable from 'ee/environments/components/multiple_approval_rules_table.vue'; import { s__ } from '~/locale'; +import { stubComponent } from 'helpers/stub_component'; describe('ee/environments/components/multiple_approval_rules_table.vue', () => { let wrapper; @@ -11,6 +12,11 @@ describe('ee/environments/components/multiple_approval_rules_table.vue', () => { const createWrapper = ({ propsData = {} } = {}) => mountExtended(MultipleApprovalRulesTable, { propsData: { rules, ...propsData }, + stubs: { + GlFormRadio: stubComponent(GlFormRadio, { + props: ['checked'], + }), + }, }); const findTable = () => wrapper.findComponent(GlTableLite); @@ -22,6 +28,8 @@ describe('ee/environments/components/multiple_approval_rules_table.vue', () => { return rows; }; + const findRadioButtons = () => wrapper.findAllComponents(GlFormRadio); + describe('rules', () => { beforeEach(() => { wrapper = createWrapper(); @@ -85,4 +93,68 @@ describe('ee/environments/components/multiple_approval_rules_table.vue', () => { }); }); }); + + describe('approvals', () => { + const getRuleName = (index) => rules[index].group?.name || rules[index].user.name; + const firstRuleName = getRuleName(0); + const secondRuleName = getRuleName(1); + + describe('default', () => { + beforeEach(() => { + wrapper = createWrapper(); + }); + + it('should render a column for giving approval', () => { + const column = wrapper.findByRole('columnheader', { name: 'Give approval' }); + + expect(column.exists()).toBe(true); + }); + + it('should render radio button for each rule the user can approve as', () => { + const canApproveRules = rules.filter((rule) => rule.canApprove); + + expect(findRadioButtons()).toHaveLength(canApproveRules.length); + }); + + it('should preselect first radio button if no value was selected', () => { + expect(findRadioButtons().at(0).props('checked')).toBe(firstRuleName); + }); + + it('should emit first applicable rule as selected by default', () => { + expect(wrapper.emitted('select-rule')).toEqual([[firstRuleName]]); + }); + }); + + describe('when user selects rule', () => { + beforeEach(() => { + wrapper = createWrapper(); + findRadioButtons().at(1).vm.$emit('input', secondRuleName); + }); + + it('should deselect first radio button if no value was selected', () => { + expect(findRadioButtons().at(0).props('checked')).not.toBe(firstRuleName); + }); + + it('should emit selected rule', () => { + expect(wrapper.emitted('select-rule').at(-1)).toEqual([secondRuleName]); + }); + }); + + describe('when user already approved', () => { + beforeEach(() => { + gon.current_username = 'administrator'; + wrapper = createWrapper(); + }); + + it('should disable radio buttons', () => { + findRadioButtons().wrappers.forEach((button) => { + expect(button.attributes('disabled')).toBeDefined(); + }); + }); + + it('should not emit any selected rule', () => { + expect(wrapper.emitted('select-rule')).toBeUndefined(); + }); + }); + }); }); diff --git a/locale/gitlab.pot b/locale/gitlab.pot index cb280e5ce8a85149ed7bc2437eff35c1278256d9..70de7dcc6062ed7988efbbf841cf231b5d68ecf6 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -17783,6 +17783,9 @@ msgstr "" msgid "DeploymentApprovals|Approvals" msgstr "" +msgid "DeploymentApprovals|Approve" +msgstr "" + msgid "DeploymentApprovals|Approved By" msgstr "" @@ -17792,6 +17795,9 @@ msgstr "" msgid "DeploymentApprovals|Developers + Maintainers" msgstr "" +msgid "DeploymentApprovals|Give approval" +msgstr "" + msgid "DeploymentApprovals|Maintainers" msgstr ""