From 8f18db8162a494d2d6c806afb57a4637333eaae4 Mon Sep 17 00:00:00 2001 From: Rahul Chanila <rchanila@gitlab.com> Date: Fri, 25 Oct 2024 17:56:43 +0000 Subject: [PATCH] Extract package errors alert to separate component This is continuation of https://gitlab.com/gitlab-org/gitlab/-/merge_requests/151550 This MR is prep for https://gitlab.com/gitlab-org/gitlab/-/issues/460583\#solution --- .../components/list/package_errors_count.vue | 90 ++++++++++++++ .../components/list/packages_list.vue | 83 ++----------- locale/gitlab.pot | 8 +- .../list/package_errors_count_spec.js | 117 ++++++++++++++++++ .../components/list/packages_list_spec.js | 94 +++----------- 5 files changed, 233 insertions(+), 159 deletions(-) create mode 100644 app/assets/javascripts/packages_and_registries/package_registry/components/list/package_errors_count.vue create mode 100644 spec/frontend/packages_and_registries/package_registry/components/list/package_errors_count_spec.js diff --git a/app/assets/javascripts/packages_and_registries/package_registry/components/list/package_errors_count.vue b/app/assets/javascripts/packages_and_registries/package_registry/components/list/package_errors_count.vue new file mode 100644 index 0000000000000..a8208b6a43506 --- /dev/null +++ b/app/assets/javascripts/packages_and_registries/package_registry/components/list/package_errors_count.vue @@ -0,0 +1,90 @@ +<script> +import { GlAlert, GlButton } from '@gitlab/ui'; +import { mergeUrlParams } from '~/lib/utils/url_utility'; +import { s__, sprintf } from '~/locale'; + +export default { + name: 'PackageErrorsCount', + components: { + GlAlert, + GlButton, + }, + props: { + errorPackages: { + type: Array, + required: false, + default: () => [], + }, + }, + computed: { + errorTitleAlert() { + if (this.singleErrorPackage) { + return sprintf(s__('PackageRegistry|There was an error publishing %{packageName}'), { + packageName: this.singleErrorPackage.name, + }); + } + return sprintf(s__('PackageRegistry|There was an error publishing %{count} packages'), { + count: this.errorPackages.length, + }); + }, + errorMessageBodyAlert() { + if (this.singleErrorPackage) { + return this.singleErrorPackage.statusMessage || this.$options.i18n.errorMessageBodyAlert; + } + + return sprintf( + s__( + 'PackageRegistry|Failed to publish %{count} packages. Delete these packages and try again.', + ), + { + count: this.errorPackages.length, + }, + ); + }, + singleErrorPackage() { + if (this.errorPackages.length === 1) { + const [errorPackage] = this.errorPackages; + return errorPackage; + } + + return null; + }, + showErrorPackageAlert() { + return this.errorPackages.length > 0; + }, + errorPackagesHref() { + // For reactivity we depend on showErrorPackageAlert so we update accordingly + if (!this.showErrorPackageAlert) { + return ''; + } + + const pageParams = { after: null, before: null }; + return mergeUrlParams({ status: 'error', ...pageParams }, window.location.href); + }, + }, + methods: { + handleClick() { + this.$emit('confirm-delete', [this.singleErrorPackage]); + }, + }, + i18n: { + errorMessageBodyAlert: s__( + 'PackageRegistry|There was a timeout and the package was not published. Delete this package and try again.', + ), + }, +}; +</script> + +<template> + <gl-alert v-if="showErrorPackageAlert" class="gl-mt-5" variant="danger" :title="errorTitleAlert"> + {{ errorMessageBodyAlert }} + <template #actions> + <gl-button v-if="singleErrorPackage" variant="confirm" @click="handleClick">{{ + s__('PackageRegistry|Delete this package') + }}</gl-button> + <gl-button v-else :href="errorPackagesHref" variant="confirm">{{ + s__('PackageRegistry|Show packages with errors') + }}</gl-button> + </template> + </gl-alert> +</template> diff --git a/app/assets/javascripts/packages_and_registries/package_registry/components/list/packages_list.vue b/app/assets/javascripts/packages_and_registries/package_registry/components/list/packages_list.vue index b93ea37e7dc47..3dcaeb66a66cd 100644 --- a/app/assets/javascripts/packages_and_registries/package_registry/components/list/packages_list.vue +++ b/app/assets/javascripts/packages_and_registries/package_registry/components/list/packages_list.vue @@ -1,11 +1,10 @@ <script> -import { GlAlert, GlButton } from '@gitlab/ui'; -import { s__, sprintf, n__ } from '~/locale'; -import { mergeUrlParams } from '~/lib/utils/url_utility'; +import { n__ } from '~/locale'; import PackagesListRow from '~/packages_and_registries/package_registry/components/list/package_list_row.vue'; import PackagesListLoader from '~/packages_and_registries/shared/components/packages_list_loader.vue'; import RegistryList from '~/packages_and_registries/shared/components/registry_list.vue'; import DeleteModal from '~/packages_and_registries/package_registry/components/delete_modal.vue'; +import PackageErrorsCount from '~/packages_and_registries/package_registry/components/list/package_errors_count.vue'; import { DELETE_PACKAGE_TRACKING_ACTION, DELETE_PACKAGES_TRACKING_ACTION, @@ -30,9 +29,8 @@ const forwardingFieldToPackageTypeMapping = { export default { name: 'PackagesList', components: { - GlAlert, - GlButton, DeleteModal, + PackageErrorsCount, PackagesListLoader, PackagesListRow, RegistryList, @@ -90,51 +88,6 @@ export default { category, }; }, - errorTitleAlert() { - if (this.singleErrorPackage) { - return sprintf( - s__('PackageRegistry|There was an error publishing a %{packageName} package'), - { packageName: this.singleErrorPackage.name }, - ); - } - return sprintf(s__('PackageRegistry|There was an error publishing %{count} packages'), { - count: this.errorPackages.length, - }); - }, - errorMessageBodyAlert() { - if (this.singleErrorPackage) { - return this.singleErrorPackage.statusMessage || this.$options.i18n.errorMessageBodyAlert; - } - - return sprintf( - s__( - 'PackageRegistry|%{count} packages were not published to the registry. Remove these packages and try again.', - ), - { - count: this.errorPackages.length, - }, - ); - }, - singleErrorPackage() { - if (this.errorPackages.length === 1) { - const [errorPackage] = this.errorPackages; - return errorPackage; - } - - return null; - }, - showErrorPackageAlert() { - return this.errorPackages.length > 0 && !this.hideErrorAlert; - }, - errorPackagesHref() { - // For reactivity we depend on showErrorPackageAlert so we update accordingly - if (!this.showErrorPackageAlert) { - return ''; - } - - const pageParams = { after: null, before: null }; - return mergeUrlParams({ status: 'error', ...pageParams }, window.location.href); - }, packageTypesWithForwardingEnabled() { return Object.keys(this.groupSettings) .filter((field) => this.groupSettings[field]) @@ -183,15 +136,6 @@ export default { } this.itemsToBeDeleted = []; }, - showConfirmationModal() { - this.setItemsToBeDeleted([this.singleErrorPackage]); - }, - }, - i18n: { - errorMessageBodyAlert: s__( - 'PackageRegistry|There was a timeout and the package was not published. Delete this package and try again.', - ), - deleteThisPackage: s__('PackageRegistry|Delete this package'), }, }; </script> @@ -205,22 +149,11 @@ export default { </div> <template v-else> - <gl-alert - v-if="showErrorPackageAlert" - class="gl-mt-5" - variant="danger" - :title="errorTitleAlert" - > - {{ errorMessageBodyAlert }} - <template #actions> - <gl-button v-if="singleErrorPackage" variant="confirm" @click="showConfirmationModal">{{ - $options.i18n.deleteThisPackage - }}</gl-button> - <gl-button v-else :href="errorPackagesHref" variant="confirm">{{ - s__('PackageRegistry|Show packages with errors') - }}</gl-button> - </template> - </gl-alert> + <package-errors-count + v-if="!hideErrorAlert" + :error-packages="errorPackages" + @confirm-delete="setItemsToBeDeleted" + /> <registry-list data-testid="packages-table" :hidden-delete="!canDeletePackages" diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 76bc4c2bff7cf..4eb1f6f0e49a1 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -38679,9 +38679,6 @@ msgstr "" msgid "Package type must be Terraform Module" msgstr "" -msgid "PackageRegistry|%{count} packages were not published to the registry. Remove these packages and try again." -msgstr "" - msgid "PackageRegistry|%{linkStart}Wildcards%{linkEnd} such as `my-package-*` are supported." msgstr "" @@ -38895,6 +38892,9 @@ msgstr "" msgid "PackageRegistry|Failed to load version data" msgstr "" +msgid "PackageRegistry|Failed to publish %{count} packages. Delete these packages and try again." +msgstr "" + msgid "PackageRegistry|For more information on Composer packages in GitLab, %{linkStart}see the documentation.%{linkEnd}" msgstr "" @@ -39206,7 +39206,7 @@ msgstr "" msgid "PackageRegistry|There was an error publishing %{count} packages" msgstr "" -msgid "PackageRegistry|There was an error publishing a %{packageName} package" +msgid "PackageRegistry|There was an error publishing %{packageName}" msgstr "" msgid "PackageRegistry|This NuGet package has no dependencies." diff --git a/spec/frontend/packages_and_registries/package_registry/components/list/package_errors_count_spec.js b/spec/frontend/packages_and_registries/package_registry/components/list/package_errors_count_spec.js new file mode 100644 index 0000000000000..01ebbec0f8420 --- /dev/null +++ b/spec/frontend/packages_and_registries/package_registry/components/list/package_errors_count_spec.js @@ -0,0 +1,117 @@ +import { GlAlert, GlButton } from '@gitlab/ui'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import setWindowLocation from 'helpers/set_window_location_helper'; +import { TEST_HOST } from 'spec/test_constants'; +import PackageErrorsCount from '~/packages_and_registries/package_registry/components/list/package_errors_count.vue'; +import { packageData } from '../../mock_data'; + +describe('PackageErrorsCount', () => { + let wrapper; + + const firstPackage = packageData(); + const errorPackage = { + ...packageData(), + id: 'gid://gitlab/Packages::Package/121', + status: 'ERROR', + name: 'error package', + }; + + const findErrorPackageAlert = () => wrapper.findComponent(GlAlert); + const findErrorAlertButton = () => findErrorPackageAlert().findComponent(GlButton); + + const mountComponent = ({ props = {}, stubs = {} } = {}) => { + wrapper = shallowMountExtended(PackageErrorsCount, { + propsData: { + ...props, + }, + stubs: { + ...stubs, + }, + }); + }; + + describe('when an error package is present', () => { + beforeEach(() => { + mountComponent({ props: { errorPackages: [errorPackage] } }); + }); + + it('should display an alert with default body message', () => { + expect(findErrorPackageAlert().exists()).toBe(true); + expect(findErrorPackageAlert().props('title')).toBe( + 'There was an error publishing error package', + ); + expect(findErrorPackageAlert().text()).toBe( + 'There was a timeout and the package was not published. Delete this package and try again.', + ); + }); + + it('should display alert body with message set in `statusMessage`', () => { + mountComponent({ + props: { + errorPackages: [{ ...errorPackage, statusMessage: 'custom error message' }], + }, + }); + + expect(findErrorPackageAlert().exists()).toBe(true); + expect(findErrorPackageAlert().props('title')).toBe( + 'There was an error publishing error package', + ); + expect(findErrorPackageAlert().text()).toBe('custom error message'); + }); + + describe('`Delete this package` button', () => { + beforeEach(() => { + mountComponent({ + props: { errorPackages: [errorPackage] }, + stubs: { GlAlert }, + }); + }); + + it('displays the button within the alert', () => { + expect(findErrorAlertButton().text()).toBe('Delete this package'); + }); + + it('when clicked emits `confirm-delete` event', () => { + findErrorAlertButton().vm.$emit('click'); + + expect(wrapper.emitted('confirm-delete')[0][0]).toStrictEqual([errorPackage]); + }); + }); + }); + + describe('when multiple error packages are present', () => { + beforeEach(() => { + mountComponent({ + props: { errorPackages: [{ ...firstPackage, status: errorPackage.status }, errorPackage] }, + }); + }); + + it('should display an alert with default body message', () => { + expect(findErrorPackageAlert().props('title')).toBe( + 'There was an error publishing 2 packages', + ); + expect(findErrorPackageAlert().text()).toBe( + 'Failed to publish 2 packages. Delete these packages and try again.', + ); + }); + + describe('`Show packages with errors` button', () => { + beforeEach(() => { + setWindowLocation(`${TEST_HOST}/foo?type=maven&after=1234`); + mountComponent({ + props: { + errorPackages: [{ ...firstPackage, status: errorPackage.status }, errorPackage], + }, + stubs: { GlAlert }, + }); + }); + + it('is shown with correct href within the alert', () => { + expect(findErrorAlertButton().text()).toBe('Show packages with errors'); + expect(findErrorAlertButton().attributes('href')).toBe( + `${TEST_HOST}/foo?type=maven&status=error`, + ); + }); + }); + }); +}); diff --git a/spec/frontend/packages_and_registries/package_registry/components/list/packages_list_spec.js b/spec/frontend/packages_and_registries/package_registry/components/list/packages_list_spec.js index 554c1cc333408..23c5f09d847d2 100644 --- a/spec/frontend/packages_and_registries/package_registry/components/list/packages_list_spec.js +++ b/spec/frontend/packages_and_registries/package_registry/components/list/packages_list_spec.js @@ -1,13 +1,12 @@ -import { GlAlert, GlButton } from '@gitlab/ui'; import { nextTick } from 'vue'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; -import { TEST_HOST } from 'spec/test_constants'; import { stubComponent } from 'helpers/stub_component'; import PackagesListRow from '~/packages_and_registries/package_registry/components/list/package_list_row.vue'; import PackagesListLoader from '~/packages_and_registries/shared/components/packages_list_loader.vue'; import DeleteModal from '~/packages_and_registries/package_registry/components/delete_modal.vue'; import RegistryList from '~/packages_and_registries/shared/components/registry_list.vue'; -import setWindowLocation from 'helpers/set_window_location_helper'; +import PackageErrorsCount from '~/packages_and_registries/package_registry/components/list/package_errors_count.vue'; + import { DELETE_PACKAGE_TRACKING_ACTION, DELETE_PACKAGES_TRACKING_ACTION, @@ -52,8 +51,7 @@ describe('packages_list', () => { const findEmptySlot = () => wrapper.findComponent(EmptySlotStub); const findRegistryList = () => wrapper.findComponent(RegistryList); const findPackagesListRow = () => wrapper.findComponent(PackagesListRow); - const findErrorPackageAlert = () => wrapper.findComponent(GlAlert); - const findErrorAlertButton = () => findErrorPackageAlert().findComponent(GlButton); + const findPackageErrorsCount = () => wrapper.findComponent(PackageErrorsCount); const findDeletePackagesModal = () => wrapper.findComponent(DeleteModal); const showMock = jest.fn(); @@ -138,8 +136,8 @@ describe('packages_list', () => { expect(findDeletePackagesModal().props('showRequestForwardingContent')).toBe(false); }); - it('does not have an error alert displayed', () => { - expect(findErrorPackageAlert().exists()).toBe(false); + it('renders PackageErrorsCount component', () => { + expect(findPackageErrorsCount().props('errorPackages')).toEqual([]); }); }); @@ -284,51 +282,23 @@ describe('packages_list', () => { }); }); - describe('when an error package is present', () => { + describe('when error packages are present', () => { beforeEach(() => { mountComponent({ props: { list: [firstPackage, errorPackage] } }); }); - it('should display an alert with default body message', () => { - expect(findErrorPackageAlert().exists()).toBe(true); - expect(findErrorPackageAlert().props('title')).toBe( - 'There was an error publishing a error package package', - ); - expect(findErrorPackageAlert().text()).toBe( - 'There was a timeout and the package was not published. Delete this package and try again.', - ); + it('renders PackageErrorsCount component with props', () => { + expect(findPackageErrorsCount().props('errorPackages')).toStrictEqual([errorPackage]); }); - it('should display alert body with message set in `statusMessage`', () => { - mountComponent({ - props: { list: [firstPackage, { ...errorPackage, statusMessage: 'custom error message' }] }, - }); + it('and PackageErrorsCount component emits `confirm-delete`, modal component is shown', async () => { + findPackageErrorsCount().vm.$emit('confirm-delete', [errorPackage]); - expect(findErrorPackageAlert().exists()).toBe(true); - expect(findErrorPackageAlert().props('title')).toBe( - 'There was an error publishing a error package package', - ); - expect(findErrorPackageAlert().text()).toBe('custom error message'); - }); - - describe('`Delete this package` button', () => { - beforeEach(() => { - mountComponent({ props: { list: [firstPackage, errorPackage] }, stubs: { GlAlert } }); - }); - - it('displays the button within the alert', () => { - expect(findErrorAlertButton().text()).toBe('Delete this package'); - }); - - it('should display the deletion modal when clicked on the `Delete this package` button', async () => { - findErrorAlertButton().vm.$emit('click'); - - await nextTick(); + expect(showMock).toHaveBeenCalledTimes(1); - expect(showMock).toHaveBeenCalledTimes(1); + await nextTick(); - expect(findDeletePackagesModal().props('itemsToBeDeleted')).toStrictEqual([errorPackage]); - }); + expect(findDeletePackagesModal().props('itemsToBeDeleted')).toStrictEqual([errorPackage]); }); describe('when `hideErrorAlert` is true', () => { @@ -339,43 +309,7 @@ describe('packages_list', () => { }); it('does not display alert message', () => { - expect(findErrorPackageAlert().exists()).toBe(false); - }); - }); - }); - - describe('when multiple error packages are present', () => { - beforeEach(() => { - mountComponent({ - props: { list: [{ ...firstPackage, status: errorPackage.status }, errorPackage] }, - }); - }); - - it('should display an alert with default body message', () => { - expect(findErrorPackageAlert().props('title')).toBe( - 'There was an error publishing 2 packages', - ); - expect(findErrorPackageAlert().text()).toBe( - '2 packages were not published to the registry. Remove these packages and try again.', - ); - }); - - describe('`Show packages with errors` button', () => { - beforeEach(() => { - setWindowLocation(`${TEST_HOST}/foo?type=maven&after=1234`); - mountComponent({ - props: { - list: [{ ...firstPackage, status: errorPackage.status }, errorPackage], - }, - stubs: { GlAlert }, - }); - }); - - it('is shown with correct href within the alert', () => { - expect(findErrorAlertButton().text()).toBe('Show packages with errors'); - expect(findErrorAlertButton().attributes('href')).toBe( - `${TEST_HOST}/foo?type=maven&status=error`, - ); + expect(findPackageErrorsCount().exists()).toBe(false); }); }); }); -- GitLab