From 6f3b0814e8e9af0df9fe57c9525edbaae74eeeb6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=B2=20Maria=20Mezzopera?= <nmezzopera@gitlab.com>
Date: Tue, 9 Mar 2021 20:55:17 +0000
Subject: [PATCH] Set breadcrumb to Root image when name missing
- source
- tests
---
.../details_page/details_header.vue | 32 ++++++++----
.../components/list_page/image_list_row.vue | 6 ++-
.../registry/explorer/constants/common.js | 3 ++
.../registry/explorer/constants/details.js | 8 ++-
.../registry/explorer/constants/index.js | 1 +
.../registry/explorer/pages/details.vue | 7 ++-
...r-page-says-image-repository-not-found.yml | 5 ++
locale/gitlab.pot | 9 ++--
.../groups/container_registry_spec.rb | 8 ++-
.../projects/container_registry_spec.rb | 8 ++-
.../details_page/details_header_spec.js | 51 +++++++++++++++----
.../list_page/image_list_row_spec.js | 11 +++-
.../registry/explorer/pages/details_spec.js | 22 ++++++++
13 files changed, 138 insertions(+), 33 deletions(-)
create mode 100644 app/assets/javascripts/registry/explorer/constants/common.js
create mode 100644 changelogs/unreleased/320901-container-page-says-image-repository-not-found.yml
diff --git a/app/assets/javascripts/registry/explorer/components/details_page/details_header.vue b/app/assets/javascripts/registry/explorer/components/details_page/details_header.vue
index a4b4c08bc348..f46068acd681 100644
--- a/app/assets/javascripts/registry/explorer/components/details_page/details_header.vue
+++ b/app/assets/javascripts/registry/explorer/components/details_page/details_header.vue
@@ -1,11 +1,10 @@
<script>
-import { GlSprintf, GlButton } from '@gitlab/ui';
+import { GlButton, GlIcon, GlTooltipDirective } from '@gitlab/ui';
import { sprintf, n__ } from '~/locale';
import MetadataItem from '~/vue_shared/components/registry/metadata_item.vue';
import TitleArea from '~/vue_shared/components/registry/title_area.vue';
import timeagoMixin from '~/vue_shared/mixins/timeago';
import {
- DETAILS_PAGE_TITLE,
UPDATED_AT,
CLEANUP_UNSCHEDULED_TEXT,
CLEANUP_SCHEDULED_TEXT,
@@ -20,11 +19,16 @@ import {
UNSCHEDULED_STATUS,
SCHEDULED_STATUS,
ONGOING_STATUS,
+ ROOT_IMAGE_TEXT,
+ ROOT_IMAGE_TOOLTIP,
} from '../../constants/index';
export default {
name: 'DetailsHeader',
- components: { GlSprintf, GlButton, TitleArea, MetadataItem },
+ components: { GlButton, GlIcon, TitleArea, MetadataItem },
+ directives: {
+ GlTooltip: GlTooltipDirective,
+ },
mixins: [timeagoMixin],
props: {
image: {
@@ -73,9 +77,12 @@ export default {
deleteButtonDisabled() {
return this.disabled || !this.image.canDelete;
},
- },
- i18n: {
- DETAILS_PAGE_TITLE,
+ rootImageTooltip() {
+ return !this.image.name ? ROOT_IMAGE_TOOLTIP : '';
+ },
+ imageName() {
+ return this.image.name || ROOT_IMAGE_TEXT;
+ },
},
};
</script>
@@ -84,12 +91,15 @@ export default {
<title-area :metadata-loading="metadataLoading">
<template #title>
<span data-testid="title">
- <gl-sprintf :message="$options.i18n.DETAILS_PAGE_TITLE">
- <template #imageName>
- {{ image.name }}
- </template>
- </gl-sprintf>
+ {{ imageName }}
</span>
+ <gl-icon
+ v-if="rootImageTooltip"
+ v-gl-tooltip="rootImageTooltip"
+ class="gl-text-blue-600"
+ name="information-o"
+ :aria-label="rootImageTooltip"
+ />
</template>
<template #metadata-tags-count>
<metadata-item icon="tag" :text="tagCountText" data-testid="tags-count" />
diff --git a/app/assets/javascripts/registry/explorer/components/list_page/image_list_row.vue b/app/assets/javascripts/registry/explorer/components/list_page/image_list_row.vue
index a09844b59bc1..0373a84b271b 100644
--- a/app/assets/javascripts/registry/explorer/components/list_page/image_list_row.vue
+++ b/app/assets/javascripts/registry/explorer/components/list_page/image_list_row.vue
@@ -13,6 +13,7 @@ import {
CLEANUP_TIMED_OUT_ERROR_MESSAGE,
IMAGE_DELETE_SCHEDULED_STATUS,
IMAGE_FAILED_DELETED_STATUS,
+ ROOT_IMAGE_TEXT,
} from '../../constants/index';
import DeleteButton from '../delete_button.vue';
@@ -74,6 +75,9 @@ export default {
}
return null;
},
+ imageName() {
+ return this.item.name ? this.item.path : `${this.item.path}/ ${ROOT_IMAGE_TEXT}`;
+ },
},
};
</script>
@@ -95,7 +99,7 @@ export default {
data-qa-selector="registry_image_content"
:to="{ name: 'details', params: { id } }"
>
- {{ item.path }}
+ {{ imageName }}
</router-link>
<clipboard-button
v-if="item.location"
diff --git a/app/assets/javascripts/registry/explorer/constants/common.js b/app/assets/javascripts/registry/explorer/constants/common.js
new file mode 100644
index 000000000000..dc71ef8450b2
--- /dev/null
+++ b/app/assets/javascripts/registry/explorer/constants/common.js
@@ -0,0 +1,3 @@
+import { s__ } from '~/locale';
+
+export const ROOT_IMAGE_TEXT = s__('ContainerRegistry|Root image');
diff --git a/app/assets/javascripts/registry/explorer/constants/details.js b/app/assets/javascripts/registry/explorer/constants/details.js
index 3f04538a18bb..7220f9646db8 100644
--- a/app/assets/javascripts/registry/explorer/constants/details.js
+++ b/app/assets/javascripts/registry/explorer/constants/details.js
@@ -2,7 +2,6 @@ import { helpPagePath } from '~/helpers/help_page_helper';
import { s__, __ } from '~/locale';
// Translations strings
-export const DETAILS_PAGE_TITLE = s__('ContainerRegistry|%{imageName} tags');
export const DELETE_TAG_ERROR_MESSAGE = s__(
'ContainerRegistry|Something went wrong while marking the tag for deletion.',
);
@@ -53,7 +52,8 @@ export const MISSING_OR_DELETED_IMAGE_TITLE = s__(
export const MISSING_OR_DELETED_IMAGE_MESSAGE = s__(
'ContainerRegistry|The requested image repository does not exist or has been deleted. If you think this is an error, try refreshing the page.',
);
-export const MISSING_OR_DELETE_IMAGE_BREADCRUMB = s__(
+
+export const MISSING_OR_DELETED_IMAGE_BREADCRUMB = s__(
'ContainerRegistry|Image repository not found',
);
@@ -112,6 +112,10 @@ export const FAILED_DELETION_STATUS_MESSAGE = s__(
'ContainerRegistry|This image repository has failed to be deleted',
);
+export const ROOT_IMAGE_TOOLTIP = s__(
+ 'ContainerRegistry|Image repository with no name located at the project URL.',
+);
+
// Parameters
export const DEFAULT_PAGE = 1;
diff --git a/app/assets/javascripts/registry/explorer/constants/index.js b/app/assets/javascripts/registry/explorer/constants/index.js
index 10816e12ead3..6886356d8e29 100644
--- a/app/assets/javascripts/registry/explorer/constants/index.js
+++ b/app/assets/javascripts/registry/explorer/constants/index.js
@@ -1,3 +1,4 @@
+export * from './common';
export * from './expiration_policies';
export * from './quick_start';
export * from './list';
diff --git a/app/assets/javascripts/registry/explorer/pages/details.vue b/app/assets/javascripts/registry/explorer/pages/details.vue
index 0403467468ad..2f515356fa7f 100644
--- a/app/assets/javascripts/registry/explorer/pages/details.vue
+++ b/app/assets/javascripts/registry/explorer/pages/details.vue
@@ -24,7 +24,8 @@ import {
GRAPHQL_PAGE_SIZE,
FETCH_IMAGES_LIST_ERROR_MESSAGE,
UNFINISHED_STATUS,
- MISSING_OR_DELETE_IMAGE_BREADCRUMB,
+ MISSING_OR_DELETED_IMAGE_BREADCRUMB,
+ ROOT_IMAGE_TEXT,
} from '../constants/index';
import deleteContainerRepositoryTagsMutation from '../graphql/mutations/delete_container_repository_tags.mutation.graphql';
import getContainerRepositoryDetailsQuery from '../graphql/queries/get_container_repository_details.query.graphql';
@@ -116,7 +117,9 @@ export default {
},
methods: {
updateBreadcrumb() {
- const name = this.image?.name || MISSING_OR_DELETE_IMAGE_BREADCRUMB;
+ const name = this.image?.id
+ ? this.image?.name || ROOT_IMAGE_TEXT
+ : MISSING_OR_DELETED_IMAGE_BREADCRUMB;
this.breadCrumbState.updateName(name);
},
deleteTags(toBeDeleted) {
diff --git a/changelogs/unreleased/320901-container-page-says-image-repository-not-found.yml b/changelogs/unreleased/320901-container-page-says-image-repository-not-found.yml
new file mode 100644
index 000000000000..a9494b43b2e8
--- /dev/null
+++ b/changelogs/unreleased/320901-container-page-says-image-repository-not-found.yml
@@ -0,0 +1,5 @@
+---
+title: Use Root Image for images with missing name
+merge_request: 54693
+author:
+type: changed
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 18eae551b0c6..e089cb1191e1 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -7958,9 +7958,6 @@ msgid_plural "ContainerRegistry|%{count} Tags"
msgstr[0] ""
msgstr[1] ""
-msgid "ContainerRegistry|%{imageName} tags"
-msgstr ""
-
msgid "ContainerRegistry|%{strongStart}Disabled%{strongEnd} - Tags will not be automatically deleted."
msgstr ""
@@ -8063,6 +8060,9 @@ msgstr ""
msgid "ContainerRegistry|Image repository will be deleted"
msgstr ""
+msgid "ContainerRegistry|Image repository with no name located at the project URL."
+msgstr ""
+
msgid "ContainerRegistry|Image tags"
msgstr ""
@@ -8128,6 +8128,9 @@ msgstr ""
msgid "ContainerRegistry|Remove these tags"
msgstr ""
+msgid "ContainerRegistry|Root image"
+msgstr ""
+
msgid "ContainerRegistry|Run cleanup:"
msgstr ""
diff --git a/spec/features/groups/container_registry_spec.rb b/spec/features/groups/container_registry_spec.rb
index cacabdda22d7..65374263f450 100644
--- a/spec/features/groups/container_registry_spec.rb
+++ b/spec/features/groups/container_registry_spec.rb
@@ -67,7 +67,13 @@
end
it 'shows the image title' do
- expect(page).to have_content 'my/image tags'
+ expect(page).to have_content 'my/image'
+ end
+
+ it 'shows the image tags' do
+ expect(page).to have_content 'Image tags'
+ first_tag = first('[data-testid="name"]')
+ expect(first_tag).to have_content 'latest'
end
it 'user removes a specific tag from container repository' do
diff --git a/spec/features/projects/container_registry_spec.rb b/spec/features/projects/container_registry_spec.rb
index d0ad6668c076..40d0260eafdd 100644
--- a/spec/features/projects/container_registry_spec.rb
+++ b/spec/features/projects/container_registry_spec.rb
@@ -82,7 +82,13 @@
end
it 'shows the image title' do
- expect(page).to have_content 'my/image tags'
+ expect(page).to have_content 'my/image'
+ end
+
+ it 'shows the image tags' do
+ expect(page).to have_content 'Image tags'
+ first_tag = first('[data-testid="name"]')
+ expect(first_tag).to have_content '1'
end
it 'user removes a specific tag from container repository' do
diff --git a/spec/frontend/registry/explorer/components/details_page/details_header_spec.js b/spec/frontend/registry/explorer/components/details_page/details_header_spec.js
index 3fa3a2ae1de7..b50ed87a5633 100644
--- a/spec/frontend/registry/explorer/components/details_page/details_header_spec.js
+++ b/spec/frontend/registry/explorer/components/details_page/details_header_spec.js
@@ -1,9 +1,9 @@
-import { GlSprintf, GlButton } from '@gitlab/ui';
+import { GlButton, GlIcon } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils';
import { useFakeDate } from 'helpers/fake_date';
+import { createMockDirective, getBinding } from 'helpers/vue_mock_directive';
import component from '~/registry/explorer/components/details_page/details_header.vue';
import {
- DETAILS_PAGE_TITLE,
UNSCHEDULED_STATUS,
SCHEDULED_STATUS,
ONGOING_STATUS,
@@ -13,6 +13,8 @@ import {
CLEANUP_SCHEDULED_TOOLTIP,
CLEANUP_ONGOING_TOOLTIP,
CLEANUP_UNFINISHED_TOOLTIP,
+ ROOT_IMAGE_TEXT,
+ ROOT_IMAGE_TOOLTIP,
} from '~/registry/explorer/constants';
import TitleArea from '~/vue_shared/components/registry/title_area.vue';
@@ -41,6 +43,7 @@ describe('Details Header', () => {
const findTagsCount = () => findByTestId('tags-count');
const findCleanup = () => findByTestId('cleanup');
const findDeleteButton = () => wrapper.find(GlButton);
+ const findInfoIcon = () => wrapper.find(GlIcon);
const waitForMetadataItems = async () => {
// Metadata items are printed by a loop in the title-area and it takes two ticks for them to be available
@@ -51,8 +54,10 @@ describe('Details Header', () => {
const mountComponent = (propsData = { image: defaultImage }) => {
wrapper = shallowMount(component, {
propsData,
+ directives: {
+ GlTooltip: createMockDirective(),
+ },
stubs: {
- GlSprintf,
TitleArea,
},
});
@@ -62,15 +67,41 @@ describe('Details Header', () => {
wrapper.destroy();
wrapper = null;
});
+ describe('image name', () => {
+ describe('missing image name', () => {
+ it('root image ', () => {
+ mountComponent({ image: { ...defaultImage, name: '' } });
- it('has the correct title ', () => {
- mountComponent({ image: { ...defaultImage, name: '' } });
- expect(findTitle().text()).toMatchInterpolatedText(DETAILS_PAGE_TITLE);
- });
+ expect(findTitle().text()).toBe(ROOT_IMAGE_TEXT);
+ });
- it('shows imageName in the title', () => {
- mountComponent();
- expect(findTitle().text()).toContain('foo');
+ it('has an icon', () => {
+ mountComponent({ image: { ...defaultImage, name: '' } });
+
+ expect(findInfoIcon().exists()).toBe(true);
+ expect(findInfoIcon().props('name')).toBe('information-o');
+ });
+
+ it('has a tooltip', () => {
+ mountComponent({ image: { ...defaultImage, name: '' } });
+
+ const tooltip = getBinding(findInfoIcon().element, 'gl-tooltip');
+ expect(tooltip.value).toBe(ROOT_IMAGE_TOOLTIP);
+ });
+ });
+
+ describe('with image name present', () => {
+ it('shows image.name ', () => {
+ mountComponent();
+ expect(findTitle().text()).toContain('foo');
+ });
+
+ it('has no icon', () => {
+ mountComponent();
+
+ expect(findInfoIcon().exists()).toBe(false);
+ });
+ });
});
describe('delete button', () => {
diff --git a/spec/frontend/registry/explorer/components/list_page/image_list_row_spec.js b/spec/frontend/registry/explorer/components/list_page/image_list_row_spec.js
index d6ee871341b6..6c897b983f7f 100644
--- a/spec/frontend/registry/explorer/components/list_page/image_list_row_spec.js
+++ b/spec/frontend/registry/explorer/components/list_page/image_list_row_spec.js
@@ -12,6 +12,7 @@ import {
CLEANUP_TIMED_OUT_ERROR_MESSAGE,
IMAGE_DELETE_SCHEDULED_STATUS,
IMAGE_FAILED_DELETED_STATUS,
+ ROOT_IMAGE_TEXT,
} from '~/registry/explorer/constants';
import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
import ListItem from '~/vue_shared/components/registry/list_item.vue';
@@ -73,8 +74,8 @@ describe('Image List Row', () => {
mountComponent();
const link = findDetailsLink();
- expect(link.html()).toContain(item.path);
- expect(link.props('to')).toMatchObject({
+ expect(link.text()).toBe(item.path);
+ expect(findDetailsLink().props('to')).toMatchObject({
name: 'details',
params: {
id: getIdFromGraphQLId(item.id),
@@ -82,6 +83,12 @@ describe('Image List Row', () => {
});
});
+ it(`when the image has no name appends ${ROOT_IMAGE_TEXT} to the path`, () => {
+ mountComponent({ item: { ...item, name: '' } });
+
+ expect(findDetailsLink().text()).toBe(`${item.path}/ ${ROOT_IMAGE_TEXT}`);
+ });
+
it('contains a clipboard button', () => {
mountComponent();
const button = findClipboardButton();
diff --git a/spec/frontend/registry/explorer/pages/details_spec.js b/spec/frontend/registry/explorer/pages/details_spec.js
index 65c58bf98747..76baf4f72c96 100644
--- a/spec/frontend/registry/explorer/pages/details_spec.js
+++ b/spec/frontend/registry/explorer/pages/details_spec.js
@@ -17,6 +17,8 @@ import {
UNFINISHED_STATUS,
DELETE_SCHEDULED,
ALERT_DANGER_IMAGE,
+ MISSING_OR_DELETED_IMAGE_BREADCRUMB,
+ ROOT_IMAGE_TEXT,
} from '~/registry/explorer/constants';
import deleteContainerRepositoryTagsMutation from '~/registry/explorer/graphql/mutations/delete_container_repository_tags.mutation.graphql';
import getContainerRepositoryDetailsQuery from '~/registry/explorer/graphql/queries/get_container_repository_details.query.graphql';
@@ -515,6 +517,26 @@ describe('Details Page', () => {
expect(breadCrumbState.updateName).toHaveBeenCalledWith(containerRepositoryMock.name);
});
+
+ it(`when the image is missing set the breadcrumb to ${MISSING_OR_DELETED_IMAGE_BREADCRUMB}`, async () => {
+ mountComponent({ resolver: jest.fn().mockResolvedValue(graphQLEmptyImageDetailsMock) });
+
+ await waitForApolloRequestRender();
+
+ expect(breadCrumbState.updateName).toHaveBeenCalledWith(MISSING_OR_DELETED_IMAGE_BREADCRUMB);
+ });
+
+ it(`when the image has no name set the breadcrumb to ${ROOT_IMAGE_TEXT}`, async () => {
+ mountComponent({
+ resolver: jest
+ .fn()
+ .mockResolvedValue(graphQLImageDetailsMock({ ...containerRepositoryMock, name: null })),
+ });
+
+ await waitForApolloRequestRender();
+
+ expect(breadCrumbState.updateName).toHaveBeenCalledWith(ROOT_IMAGE_TEXT);
+ });
});
describe('when the image has a status different from null', () => {
--
GitLab