From 11748290cc507ba384a98a9df6e61b52165223e8 Mon Sep 17 00:00:00 2001 From: Gerardo Date: Tue, 5 Mar 2024 11:31:09 +0100 Subject: [PATCH 1/3] Protected packages: Delete protection rules in project settings ui - Adding button "Unprotect" for each row containing a protection rule - Sending a graphql mutation "deleteContainerProtectionRule" when the button "Unprotect" is clicked - Adding an alert when the graphql mutation is not successful - Adding tests for button 'Unprotect' Changelog: added --- .../components/container_protection_rules.vue | 81 ++++++++ ...container_protection_rule.mutation.graphql | 13 ++ locale/gitlab.pot | 9 + .../container_protection_rules_spec.js | 178 +++++++++++++++++- .../settings/project/settings/mock_data.js | 12 ++ 5 files changed, 287 insertions(+), 6 deletions(-) create mode 100644 app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/delete_container_protection_rule.mutation.graphql diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue index c28e94322e5afa..ed3729bf224f1a 100644 --- a/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue +++ b/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue @@ -5,12 +5,14 @@ import { GlCard, GlKeysetPagination, GlLoadingIcon, + GlModal, GlModalDirective, GlTable, } from '@gitlab/ui'; import protectionRulesQuery from '~/packages_and_registries/settings/project/graphql/queries/get_container_protection_rules.query.graphql'; import SettingsBlock from '~/packages_and_registries/shared/components/settings_block.vue'; import ContainerProtectionRuleForm from '~/packages_and_registries/settings/project/components/container_protection_rule_form.vue'; +import deleteContainerProtectionRuleMutation from '~/packages_and_registries/settings/project/graphql/mutations/delete_container_protection_rule.mutation.graphql'; import { s__, __ } from '~/locale'; const PAGINATION_DEFAULT_PER_PAGE = 10; @@ -36,6 +38,7 @@ export default { GlCard, GlKeysetPagination, GlLoadingIcon, + GlModal, GlTable, SettingsBlock, }, @@ -48,6 +51,14 @@ export default { settingBlockDescription: s__( 'ContainerRegistry|When a container is protected then only certain user roles are able to update and delete the protected container. This helps to avoid tampering with the container.', ), + protectionRuleDeletionConfirmModal: { + title: s__( + 'ContainerRegistry|Are you sure you want to delete the container protection rule?', + ), + description: s__( + 'ContainerRegistry|Users with at least the Developer role for this project will be able to push and delete container images.', + ), + }, }, apollo: { protectionRulesQueryPayload: { @@ -81,6 +92,7 @@ export default { tableItems() { return this.protectionRulesQueryResult.map((protectionRule) => { return { + id: protectionRule.id, deleteProtectedUpToAccessLevel: ACCESS_LEVEL_GRAPHQL_VALUE_TO_LABEL[protectionRule.deleteProtectedUpToAccessLevel], pushProtectedUpToAccessLevel: @@ -107,6 +119,19 @@ export default { isAddProtectionRuleButtonDisabled() { return this.protectionRuleFormVisibility; }, + modalActionPrimary() { + return { + text: __('Delete'), + attributes: { + variant: 'danger', + }, + }; + }, + modalActionCancel() { + return { + text: __('Cancel'), + }; + }, }, methods: { showProtectionRuleForm() { @@ -150,6 +175,32 @@ export default { isProtectionRuleMutationInProgress(item) { return this.protectionRuleMutationItem === item && this.protectionRuleMutationInProgress; }, + deleteProtectionRule(protectionRule) { + this.clearAlertMessage(); + + this.protectionRuleMutationInProgress = true; + + return this.$apollo + .mutate({ + mutation: deleteContainerProtectionRuleMutation, + variables: { input: { id: protectionRule.id } }, + }) + .then(({ data }) => { + const [errorMessage] = data?.deleteContainerRegistryProtectionRule?.errors ?? []; + if (errorMessage) { + this.alertErrorMessage = errorMessage; + return; + } + this.refetchProtectionRules(); + this.$toast.show(s__('ContainerRegistry|Protection rule deleted.')); + }) + .catch((e) => { + this.alertErrorMessage = e.message; + }) + .finally(() => { + this.resetProtectionRuleMutation(); + }); + }, }, fields: [ { @@ -167,7 +218,14 @@ export default { label: I18N_DELETE_PROTECTED_UP_TO_ACCESS_LEVEL, tdClass: 'gl-vertical-align-middle!', }, + { + key: 'rowActions', + label: '', + thClass: 'gl-display-none', + tdClass: 'gl-vertical-align-middle! gl-text-right', + }, ], + modal: { id: 'delete-protection-rule-confirmation-modal' }, }; @@ -227,6 +285,18 @@ export default { + +
@@ -241,6 +311,17 @@ export default {
+ + +

{{ $options.i18n.protectionRuleDeletionConfirmModal.description }}

+ diff --git a/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/delete_container_protection_rule.mutation.graphql b/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/delete_container_protection_rule.mutation.graphql new file mode 100644 index 00000000000000..36b76ad2639de2 --- /dev/null +++ b/app/assets/javascripts/packages_and_registries/settings/project/graphql/mutations/delete_container_protection_rule.mutation.graphql @@ -0,0 +1,13 @@ +mutation deleteContainerRegistryProtectionRule( + $input: DeleteContainerRegistryProtectionRuleInput! +) { + deleteContainerRegistryProtectionRule(input: $input) { + containerRegistryProtectionRule { + id + repositoryPathPattern + pushProtectedUpToAccessLevel + deleteProtectedUpToAccessLevel + } + errors + } +} diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e032315abf9054..1a670607c8cf23 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -13877,6 +13877,9 @@ msgstr "" msgid "ContainerRegistry|Add rule" msgstr "" +msgid "ContainerRegistry|Are you sure you want to delete the container protection rule?" +msgstr "" + msgid "ContainerRegistry|Build an image" msgstr "" @@ -14051,6 +14054,9 @@ msgstr "" msgid "ContainerRegistry|Protected containers" msgstr "" +msgid "ContainerRegistry|Protection rule deleted." +msgstr "" + msgid "ContainerRegistry|Published %{timeInfo}" msgstr "" @@ -14209,6 +14215,9 @@ msgstr "" msgid "ContainerRegistry|To widen your search, change or remove the filters above." msgstr "" +msgid "ContainerRegistry|Users with at least the Developer role for this project will be able to push and delete container images." +msgstr "" + msgid "ContainerRegistry|We are having trouble connecting to the Container Registry. Please try refreshing the page. If this error persists, please review %{docLinkStart}the troubleshooting documentation%{docLinkEnd}." msgstr "" diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rules_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rules_spec.js index 44fe8baa1d59b0..b19c1d32059b8d 100644 --- a/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rules_spec.js +++ b/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rules_spec.js @@ -1,14 +1,20 @@ -import { GlLoadingIcon, GlKeysetPagination } from '@gitlab/ui'; +import { GlLoadingIcon, GlKeysetPagination, GlModal } from '@gitlab/ui'; import Vue from 'vue'; import VueApollo from 'vue-apollo'; import { mountExtended, extendedWrapper } from 'helpers/vue_test_utils_helper'; +import { getBinding } from 'helpers/vue_mock_directive'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import ContainerProtectionRuleForm from '~/packages_and_registries/settings/project/components/container_protection_rule_form.vue'; import ContainerProtectionRules from '~/packages_and_registries/settings/project/components/container_protection_rules.vue'; import SettingsBlock from '~/packages_and_registries/shared/components/settings_block.vue'; import ContainerProtectionRuleQuery from '~/packages_and_registries/settings/project/graphql/queries/get_container_protection_rules.query.graphql'; -import { containerProtectionRulesData, containerProtectionRuleQueryPayload } from '../mock_data'; +import deleteContainerProtectionRuleMutation from '~/packages_and_registries/settings/project/graphql/mutations/delete_container_protection_rule.mutation.graphql'; +import { + containerProtectionRulesData, + containerProtectionRuleQueryPayload, + deleteContainerProtectionRuleMutationPayload, +} from '../mock_data'; Vue.use(VueApollo); @@ -26,12 +32,15 @@ describe('Container protection rules project settings', () => { const findTable = () => extendedWrapper(wrapper.findByRole('table', { name: /protected containers/i })); const findTableBody = () => extendedWrapper(findTable().findAllByRole('rowgroup').at(1)); - const findTableRow = (i) => extendedWrapper(findTableBody().findAllByRole('row').at(i)); const findTableLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); + const findTableRow = (i) => extendedWrapper(findTableBody().findAllByRole('row').at(i)); + const findTableRowButtonDelete = (i) => + findTableRow(i).findByRole('button', { name: /delete rule/i }); const findAddProtectionRuleForm = () => wrapper.findComponent(ContainerProtectionRuleForm); const findAddProtectionRuleFormSubmitButton = () => wrapper.findByRole('button', { name: /add protection rule/i }); const findAlert = () => wrapper.findByRole('alert'); + const findModal = () => wrapper.findComponent(GlModal); const mountComponent = (mountFn = mountExtended, provide = defaultProvidedValues, config) => { wrapper = mountFn(ContainerProtectionRules, { @@ -53,9 +62,15 @@ describe('Container protection rules project settings', () => { containerProtectionRuleQueryResolver = jest .fn() .mockResolvedValue(containerProtectionRuleQueryPayload()), + deleteContainerProtectionRuleMutationResolver = jest + .fn() + .mockResolvedValue(deleteContainerProtectionRuleMutationPayload()), config = {}, } = {}) => { - const requestHandlers = [[ContainerProtectionRuleQuery, containerProtectionRuleQueryResolver]]; + const requestHandlers = [ + [ContainerProtectionRuleQuery, containerProtectionRuleQueryResolver], + [deleteContainerProtectionRuleMutation, deleteContainerProtectionRuleMutationResolver], + ]; fakeApollo = createMockApollo(requestHandlers); @@ -77,7 +92,7 @@ describe('Container protection rules project settings', () => { describe('table "container protection rules"', () => { const findTableRowCell = (i, j) => findTableRow(i).findAllByRole('cell').at(j); - it('renders table with Container protection rules', async () => { + it('renders table with container protection rules', async () => { createComponent(); await waitForPromises(); @@ -86,7 +101,6 @@ describe('Container protection rules project settings', () => { containerProtectionRuleQueryPayload().data.project.containerRegistryProtectionRules.nodes.forEach( (protectionRule, i) => { - expect(findTableRow(i).findAllByRole('cell').length).toBe(3); expect(findTableRowCell(i, 0).text()).toBe(protectionRule.repositoryPathPattern); expect(findTableRowCell(i, 1).text()).toBe('Maintainer'); expect(findTableRowCell(i, 2).text()).toBe('Maintainer'); @@ -255,6 +269,158 @@ describe('Container protection rules project settings', () => { }); }); }); + + describe('column "rowActions"', () => { + describe('button "Delete"', () => { + it('exists in table', async () => { + createComponent(); + + await waitForPromises(); + + expect(findTableRowButtonDelete(0).exists()).toBe(true); + }); + + describe('when button is clicked', () => { + it('binds modal "confirmation for delete action"', async () => { + createComponent(); + + await waitForPromises(); + + const modalId = getBinding(findTableRowButtonDelete(0).element, 'gl-modal'); + + expect(findModal().props('modal-id')).toBe(modalId); + expect(findModal().props('title')).toBe( + 'Are you sure you want to delete the container protection rule?', + ); + expect(findModal().text()).toBe( + 'Users with at least the Developer role for this project will be able to push and delete container images.', + ); + }); + }); + }); + }); + }); + + describe('modal "confirmation for delete action"', () => { + const createComponentAndClickButtonDeleteInTableRow = async ({ + tableRowIndex = 0, + deleteContainerProtectionRuleMutationResolver = jest + .fn() + .mockResolvedValue(deleteContainerProtectionRuleMutationPayload()), + } = {}) => { + createComponent({ deleteContainerProtectionRuleMutationResolver }); + + await waitForPromises(); + + findTableRowButtonDelete(tableRowIndex).trigger('click'); + }; + + describe('when modal button "primary" clicked', () => { + const clickOnModalPrimaryBtn = () => findModal().vm.$emit('primary'); + + it('disables the button when graphql mutation is executed', async () => { + await createComponentAndClickButtonDeleteInTableRow(); + + await clickOnModalPrimaryBtn(); + + expect(findTableRowButtonDelete(0).props().disabled).toBe(true); + + expect(findTableRowButtonDelete(1).props().disabled).toBe(false); + }); + + it('sends graphql mutation', async () => { + const deleteContainerProtectionRuleMutationResolver = jest + .fn() + .mockResolvedValue(deleteContainerProtectionRuleMutationPayload()); + + await createComponentAndClickButtonDeleteInTableRow({ + deleteContainerProtectionRuleMutationResolver, + }); + + await clickOnModalPrimaryBtn(); + + expect(deleteContainerProtectionRuleMutationResolver).toHaveBeenCalledTimes(1); + expect(deleteContainerProtectionRuleMutationResolver).toHaveBeenCalledWith({ + input: { id: containerProtectionRulesData[0].id }, + }); + }); + + it('handles erroneous graphql mutation', async () => { + const alertErrorMessage = 'Client error message'; + const deleteContainerProtectionRuleMutationResolver = jest + .fn() + .mockRejectedValue(new Error(alertErrorMessage)); + + await createComponentAndClickButtonDeleteInTableRow({ + deleteContainerProtectionRuleMutationResolver, + }); + + await clickOnModalPrimaryBtn(); + + await waitForPromises(); + + expect(findAlert().isVisible()).toBe(true); + expect(findAlert().text()).toBe(alertErrorMessage); + }); + + it('handles graphql mutation with error response', async () => { + const alertErrorMessage = 'Server error message'; + const deleteContainerProtectionRuleMutationResolver = jest.fn().mockResolvedValue( + deleteContainerProtectionRuleMutationPayload({ + containerRegistryProtectionRule: null, + errors: [alertErrorMessage], + }), + ); + + await createComponentAndClickButtonDeleteInTableRow({ + deleteContainerProtectionRuleMutationResolver, + }); + + await clickOnModalPrimaryBtn(); + + await waitForPromises(); + + expect(findAlert().isVisible()).toBe(true); + expect(findAlert().text()).toBe(alertErrorMessage); + }); + + it('refetches package protection rules after successful graphql mutation', async () => { + const deleteContainerProtectionRuleMutationResolver = jest + .fn() + .mockResolvedValue(deleteContainerProtectionRuleMutationPayload()); + + const containerProtectionRuleQueryResolver = jest + .fn() + .mockResolvedValue(containerProtectionRuleQueryPayload()); + + createComponent({ + containerProtectionRuleQueryResolver, + deleteContainerProtectionRuleMutationResolver, + }); + + await waitForPromises(); + + expect(containerProtectionRuleQueryResolver).toHaveBeenCalledTimes(1); + + await findTableRowButtonDelete(0).trigger('click'); + + await clickOnModalPrimaryBtn(); + + await waitForPromises(); + + expect(containerProtectionRuleQueryResolver).toHaveBeenCalledTimes(2); + }); + + it('shows a toast with success message', async () => { + await createComponentAndClickButtonDeleteInTableRow(); + + await clickOnModalPrimaryBtn(); + + await waitForPromises(); + + expect($toast.show).toHaveBeenCalledWith('Protection rule deleted.'); + }); + }); }); describe('button "Add protection rule"', () => { diff --git a/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js b/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js index 4674b666c4d066..8cd37bd7b938e2 100644 --- a/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js +++ b/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js @@ -224,3 +224,15 @@ export const createContainerProtectionRuleMutationInput = { export const createContainerProtectionRuleMutationPayloadErrors = [ 'Repository path pattern has already been taken', ]; + +export const deleteContainerProtectionRuleMutationPayload = ({ + containerRegistryProtectionRule = { ...containerProtectionRulesData[0] }, + errors = [], +} = {}) => ({ + data: { + deleteContainerRegistryProtectionRule: { + containerRegistryProtectionRule, + errors, + }, + }, +}); -- GitLab From 926e8a268cdf98e7b8995586f21389db50d385ca Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Tue, 16 Apr 2024 14:20:02 +0000 Subject: [PATCH 2/3] refactor: Apply suggestion from @ekigbo --- .../settings/components/container_protection_rules_spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rules_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rules_spec.js index b19c1d32059b8d..40333dc3bdb475 100644 --- a/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rules_spec.js +++ b/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rules_spec.js @@ -281,7 +281,7 @@ describe('Container protection rules project settings', () => { }); describe('when button is clicked', () => { - it('binds modal "confirmation for delete action"', async () => { + it('renders the "delete container protection rule" confirmation modal', async () => { createComponent(); await waitForPromises(); -- GitLab From 90227e3e3228e3dc9fba46b60bc2c625f8df6ba0 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Tue, 16 Apr 2024 16:25:59 +0200 Subject: [PATCH 3/3] ci: Fix the ci pipeline --- .../project/components/container_protection_rules.vue | 4 ++-- locale/gitlab.pot | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue index ed3729bf224f1a..d5b964f07ab70d 100644 --- a/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue +++ b/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue @@ -49,7 +49,7 @@ export default { i18n: { settingBlockTitle: s__('ContainerRegistry|Protected containers'), settingBlockDescription: s__( - 'ContainerRegistry|When a container is protected then only certain user roles are able to update and delete the protected container. This helps to avoid tampering with the container.', + 'ContainerRegistry|When a container is protected then only certain user roles are able to push and delete the protected container image. This helps to avoid tampering with the container image.', ), protectionRuleDeletionConfirmModal: { title: s__( @@ -294,7 +294,7 @@ export default { size="small" :disabled="isProtectionRuleDeleteButtonDisabled(item)" @click="showProtectionRuleDeletionConfirmModal(item)" - >{{ __('Delete rule') }}{{ s__('ContainerRegistry|Delete rule') }} diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 1a670607c8cf23..15044da0c8ab76 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -13964,6 +13964,9 @@ msgstr "" msgid "ContainerRegistry|Delete protected up to access level" msgstr "" +msgid "ContainerRegistry|Delete rule" +msgstr "" + msgid "ContainerRegistry|Delete selected tags" msgstr "" @@ -14221,7 +14224,7 @@ msgstr "" msgid "ContainerRegistry|We are having trouble connecting to the Container Registry. Please try refreshing the page. If this error persists, please review %{docLinkStart}the troubleshooting documentation%{docLinkEnd}." msgstr "" -msgid "ContainerRegistry|When a container is protected then only certain user roles are able to update and delete the protected container. This helps to avoid tampering with the container." +msgid "ContainerRegistry|When a container is protected then only certain user roles are able to push and delete the protected container image. This helps to avoid tampering with the container image." msgstr "" msgid "ContainerRegistry|While the rename is in progress, new uploads to the container registry are blocked. Ongoing uploads may fail and need to be retried." -- GitLab