Merge branch '296962-container-registry-details-split-details-from-tags-call' into 'master'

Defer tag count loading in container registry details

See merge request gitlab-org/gitlab!60595

app/assets/javascripts/registry/explorer/components/details_page/details_header.vue

 <script>
 import { GlButton, GlIcon, GlTooltipDirective } from '@gitlab/ui';
-import { sprintf, n__ } from '~/locale';
+import { sprintf, n__, s__ } 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';
@@ -23,6 +23,8 @@ import {
   ROOT_IMAGE_TOOLTIP,
 } from '../../constants/index';
 
+import getContainerRepositoryTagsCountQuery from '../../graphql/queries/get_container_repository_tags_count.query.graphql';
+
 export default {
   name: 'DetailsHeader',
   components: { GlButton, GlIcon, TitleArea, MetadataItem },
@@ -35,60 +37,77 @@ export default {
       type: Object,
       required: true,
     },
-    metadataLoading: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
     disabled: {
       type: Boolean,
       default: false,
       required: false,
     },
   },
+  data() {
+    return {
+      containerRepository: {},
+      fetchTagsCount: false,
+    };
+  },
+  apollo: {
+    containerRepository: {
+      query: getContainerRepositoryTagsCountQuery,
+      variables() {
+        return {
+          id: this.image.id,
+        };
+      },
+    },
+  },
   computed: {
+    imageDetails() {
+      return { ...this.image, ...this.containerRepository };
+    },
     visibilityIcon() {
-      return this.image?.project?.visibility === 'public' ? 'eye' : 'eye-slash';
+      return this.imageDetails?.project?.visibility === 'public' ? 'eye' : 'eye-slash';
     },
     timeAgo() {
-      return this.timeFormatted(this.image.updatedAt);
+      return this.timeFormatted(this.imageDetails.updatedAt);
     },
     updatedText() {
       return sprintf(UPDATED_AT, { time: this.timeAgo });
     },
     tagCountText() {
-      return n__('%d tag', '%d tags', this.image.tagsCount);
+      if (this.$apollo.queries.containerRepository.loading) {
+        return s__('ContainerRegistry|-- tags');
+      }
+      return n__('%d tag', '%d tags', this.imageDetails.tagsCount);
     },
     cleanupTextAndTooltip() {
-      if (!this.image.project.containerExpirationPolicy?.enabled) {
+      if (!this.imageDetails.project.containerExpirationPolicy?.enabled) {
         return { text: CLEANUP_DISABLED_TEXT, tooltip: CLEANUP_DISABLED_TOOLTIP };
       }
       return {
         [UNSCHEDULED_STATUS]: {
           text: sprintf(CLEANUP_UNSCHEDULED_TEXT, {
-            time: this.timeFormatted(this.image.project.containerExpirationPolicy.nextRunAt),
+            time: this.timeFormatted(this.imageDetails.project.containerExpirationPolicy.nextRunAt),
           }),
         },
         [SCHEDULED_STATUS]: { text: CLEANUP_SCHEDULED_TEXT, tooltip: CLEANUP_SCHEDULED_TOOLTIP },
         [ONGOING_STATUS]: { text: CLEANUP_ONGOING_TEXT, tooltip: CLEANUP_ONGOING_TOOLTIP },
         [UNFINISHED_STATUS]: { text: CLEANUP_UNFINISHED_TEXT, tooltip: CLEANUP_UNFINISHED_TOOLTIP },
-      }[this.image?.expirationPolicyCleanupStatus];
+      }[this.imageDetails?.expirationPolicyCleanupStatus];
     },
     deleteButtonDisabled() {
-      return this.disabled || !this.image.canDelete;
+      return this.disabled || !this.imageDetails.canDelete;
     },
     rootImageTooltip() {
-      return !this.image.name ? ROOT_IMAGE_TOOLTIP : '';
+      return !this.imageDetails.name ? ROOT_IMAGE_TOOLTIP : '';
     },
     imageName() {
-      return this.image.name || ROOT_IMAGE_TEXT;
+      return this.imageDetails.name || ROOT_IMAGE_TEXT;
     },
   },
 };
 </script>
 
 <template>
-  <title-area :metadata-loading="metadataLoading">
+  <title-area>
     <template #title>
       <span data-testid="title">
         {{ imageName }}
@@ -124,12 +143,7 @@ export default {
       />
     </template>
     <template #right-actions>
-      <gl-button
-        v-if="!metadataLoading"
-        variant="danger"
-        :disabled="deleteButtonDisabled"
-        @click="$emit('delete')"
-      >
+      <gl-button variant="danger" :disabled="deleteButtonDisabled" @click="$emit('delete')">
         {{ __('Delete image repository') }}
       </gl-button>
     </template>

app/assets/javascripts/registry/explorer/graphql/queries/get_container_repository_details.query.graphql

@@ -8,7 +8,6 @@ query getContainerRepositoryDetails($id: ID!) {
     canDelete
     createdAt
     updatedAt
-    tagsCount
     expirationPolicyStartedAt
     expirationPolicyCleanupStatus
     project {

app/assets/javascripts/registry/explorer/graphql/queries/get_container_repository_tags.query.graphql

 #import "~/graphql_shared/fragments/pageInfo.fragment.graphql"
 
-query getContainerRepositoryDetails(
+query getContainerRepositoryTags(
   $id: ID!
   $first: Int
   $last: Int

app/assets/javascripts/registry/explorer/graphql/queries/get_container_repository_tags_count.query.graphql

+query getContainerRepositoryTagsCount($id: ID!) {
+  containerRepository(id: $id) {
+    id
+    tagsCount
+  }
+}

app/assets/javascripts/registry/explorer/pages/details.vue

@@ -48,14 +48,11 @@ export default {
   mixins: [Tracking.mixin()],
   inject: ['breadCrumbState', 'config'],
   apollo: {
-    image: {
+    containerRepository: {
       query: getContainerRepositoryDetailsQuery,
       variables() {
         return this.queryVariables;
       },
-      update(data) {
-        return data.containerRepository;
-      },
       result() {
         this.updateBreadcrumb();
       },
@@ -66,7 +63,7 @@ export default {
   },
   data() {
     return {
-      image: {},
+      containerRepository: {},
       itemsToBeDeleted: [],
       isMobile: false,
       mutationLoading: false,
@@ -82,12 +79,12 @@ export default {
       };
     },
     isLoading() {
-      return this.$apollo.queries.image.loading || this.mutationLoading;
+      return this.$apollo.queries.containerRepository.loading || this.mutationLoading;
     },
     showPartialCleanupWarning() {
       return (
         this.config.showUnfinishedTagCleanupCallout &&
-        this.image?.expirationPolicyCleanupStatus === UNFINISHED_STATUS &&
+        this.containerRepository?.expirationPolicyCleanupStatus === UNFINISHED_STATUS &&
         !this.hidePartialCleanupWarning
       );
     },
@@ -98,13 +95,13 @@ export default {
       };
     },
     pageActionsAreDisabled() {
-      return Boolean(this.image?.status);
+      return Boolean(this.containerRepository?.status);
     },
   },
   methods: {
     updateBreadcrumb() {
-      const name = this.image?.id
-        ? this.image?.name || ROOT_IMAGE_TEXT
+      const name = this.containerRepository?.id
+        ? this.containerRepository?.name || ROOT_IMAGE_TEXT
         : MISSING_OR_DELETED_IMAGE_BREADCRUMB;
       this.breadCrumbState.updateName(name);
     },
@@ -164,7 +161,7 @@ export default {
     },
     deleteImage() {
       this.deleteImageAlert = true;
-      this.itemsToBeDeleted = [{ path: this.image.path }];
+      this.itemsToBeDeleted = [{ path: this.containerRepository.path }];
       this.$refs.deleteModal.show();
     },
     deleteImageError() {
@@ -180,7 +177,7 @@ export default {
 
 <template>
   <div v-gl-resize-observer="handleResize" class="gl-my-3">
-    <template v-if="image">
+    <template v-if="containerRepository">
       <delete-alert
         v-model="deleteAlertType"
         :garbage-collection-help-page-path="config.garbageCollectionHelpPagePath"
@@ -195,11 +192,11 @@ export default {
         @dismiss="dismissPartialCleanupWarning"
       />
 
-      <status-alert v-if="image.status" :status="image.status" />
+      <status-alert v-if="containerRepository.status" :status="containerRepository.status" />
 
       <details-header
-        :image="image"
-        :metadata-loading="isLoading"
+        v-if="!isLoading"
+        :image="containerRepository"
         :disabled="pageActionsAreDisabled"
         @delete="deleteImage"
       />
@@ -215,7 +212,7 @@ export default {
       />
 
       <delete-image
-        :id="image.id"
+        :id="containerRepository.id"
         ref="deleteImage"
         use-update-fn
         @start="deleteImageIniit"

locale/gitlab.pot

@@ -8564,6 +8564,9 @@ msgstr ""
 msgid "ContainerRegistry|%{title} was successfully scheduled for deletion"
 msgstr ""
 
+msgid "ContainerRegistry|-- tags"
+msgstr ""
+
 msgid "ContainerRegistry|Build an image"
 msgstr ""
 

spec/frontend/registry/explorer/components/details_page/details_header_spec.js

 import { GlButton, GlIcon } from '@gitlab/ui';
-import { shallowMount } from '@vue/test-utils';
+import { shallowMount, createLocalVue } from '@vue/test-utils';
+import VueApollo from 'vue-apollo';
 import { useFakeDate } from 'helpers/fake_date';
+import createMockApollo from 'helpers/mock_apollo_helper';
 import { createMockDirective, getBinding } from 'helpers/vue_mock_directive';
+import waitForPromises from 'helpers/wait_for_promises';
 import component from '~/registry/explorer/components/details_page/details_header.vue';
 import {
   UNSCHEDULED_STATUS,
@@ -16,15 +19,18 @@ import {
   ROOT_IMAGE_TEXT,
   ROOT_IMAGE_TOOLTIP,
 } from '~/registry/explorer/constants';
+import getContainerRepositoryTagCountQuery from '~/registry/explorer/graphql/queries/get_container_repository_tags_count.query.graphql';
 import TitleArea from '~/vue_shared/components/registry/title_area.vue';
+import { imageTagsCountMock } from '../../mock_data';
 
 describe('Details Header', () => {
   let wrapper;
+  let apolloProvider;
+  let localVue;
 
   const defaultImage = {
     name: 'foo',
     updatedAt: '2020-11-03T13:29:21Z',
-    tagsCount: 10,
     canDelete: true,
     project: {
       visibility: 'public',
@@ -51,12 +57,31 @@ describe('Details Header', () => {
     await wrapper.vm.$nextTick();
   };
 
-  const mountComponent = (propsData = { image: defaultImage }) => {
+  const mountComponent = ({
+    propsData = { image: defaultImage },
+    resolver = jest.fn().mockResolvedValue(imageTagsCountMock()),
+    $apollo = undefined,
+  } = {}) => {
+    const mocks = {};
+
+    if ($apollo) {
+      mocks.$apollo = $apollo;
+    } else {
+      localVue = createLocalVue();
+      localVue.use(VueApollo);
+
+      const requestHandlers = [[getContainerRepositoryTagCountQuery, resolver]];
+      apolloProvider = createMockApollo(requestHandlers);
+    }
+
     wrapper = shallowMount(component, {
+      localVue,
+      apolloProvider,
       propsData,
       directives: {
         GlTooltip: createMockDirective(),
       },
+      mocks,
       stubs: {
         TitleArea,
       },
@@ -64,41 +89,48 @@ describe('Details Header', () => {
   };
 
   afterEach(() => {
+    // if we want to mix createMockApollo and manual mocks we need to reset everything
     wrapper.destroy();
+    apolloProvider = undefined;
+    localVue = undefined;
     wrapper = null;
   });
+
   describe('image name', () => {
     describe('missing image name', () => {
-      it('root image ', () => {
-        mountComponent({ image: { ...defaultImage, name: '' } });
+      beforeEach(() => {
+        mountComponent({ propsData: { image: { ...defaultImage, name: '' } } });
+
+        return waitForPromises();
+      });
 
+      it('root image ', () => {
         expect(findTitle().text()).toBe(ROOT_IMAGE_TEXT);
       });
 
       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 ', () => {
+      beforeEach(() => {
         mountComponent();
+
+        return waitForPromises();
+      });
+
+      it('shows image.name ', () => {
         expect(findTitle().text()).toContain('foo');
       });
 
       it('has no icon', () => {
-        mountComponent();
-
         expect(findInfoIcon().exists()).toBe(false);
       });
     });
@@ -111,12 +143,6 @@ describe('Details Header', () => {
       expect(findDeleteButton().exists()).toBe(true);
     });
 
-    it('is hidden while loading', () => {
-      mountComponent({ image: defaultImage, metadataLoading: true });
-
-      expect(findDeleteButton().exists()).toBe(false);
-    });
-
     it('has the correct text', () => {
       mountComponent();
 
@@ -149,7 +175,7 @@ describe('Details Header', () => {
     `(
       'when canDelete is $canDelete and disabled is $disabled is $isDisabled that the button is disabled',
       ({ canDelete, disabled, isDisabled }) => {
-        mountComponent({ image: { ...defaultImage, canDelete }, disabled });
+        mountComponent({ propsData: { image: { ...defaultImage, canDelete }, disabled } });
 
         expect(findDeleteButton().props('disabled')).toBe(isDisabled);
       },
@@ -158,15 +184,32 @@ describe('Details Header', () => {
 
   describe('metadata items', () => {
     describe('tags count', () => {
+      it('displays "-- tags" while loading', async () => {
+        // here we are forced to mock apollo because `waitForMetadataItems` waits
+        // for two ticks, de facto allowing the promise to resolve, so there is
+        // no way to catch the component as both rendered and in loading state
+        mountComponent({ $apollo: { queries: { containerRepository: { loading: true } } } });
+
+        await waitForMetadataItems();
+
+        expect(findTagsCount().props('text')).toBe('-- tags');
+      });
+
       it('when there is more than one tag has the correct text', async () => {
         mountComponent();
+
+        await waitForPromises();
         await waitForMetadataItems();
 
-        expect(findTagsCount().props('text')).toBe('10 tags');
+        expect(findTagsCount().props('text')).toBe('13 tags');
       });
 
       it('when there is one tag has the correct text', async () => {
-        mountComponent({ image: { ...defaultImage, tagsCount: 1 } });
+        mountComponent({
+          resolver: jest.fn().mockResolvedValue(imageTagsCountMock({ tagsCount: 1 })),
+        });
+
+        await waitForPromises();
         await waitForMetadataItems();
 
         expect(findTagsCount().props('text')).toBe('1 tag');
@@ -208,6 +251,7 @@ describe('Details Header', () => {
         'when the status is $status the text is $text and the tooltip is $tooltip',
         async ({ status, text, tooltip }) => {
           mountComponent({
+            propsData: {
               image: {
                 ...defaultImage,
                 expirationPolicyCleanupStatus: status,
@@ -215,6 +259,7 @@ describe('Details Header', () => {
                   containerExpirationPolicy: { enabled: true, nextRunAt: '2021-01-03T14:29:21Z' },
                 },
               },
+            },
           });
           await waitForMetadataItems();
 
@@ -242,7 +287,9 @@ describe('Details Header', () => {
           expect(findLastUpdatedAndVisibility().props('icon')).toBe('eye');
         });
         it('shows an eye slashed when the project is not public', async () => {
-          mountComponent({ image: { ...defaultImage, project: { visibility: 'private' } } });
+          mountComponent({
+            propsData: { image: { ...defaultImage, project: { visibility: 'private' } } },
+          });
           await waitForMetadataItems();
 
           expect(findLastUpdatedAndVisibility().props('icon')).toBe('eye-slash');

spec/frontend/registry/explorer/mock_data.js

@@ -113,7 +113,6 @@ export const containerRepositoryMock = {
   canDelete: true,
   createdAt: '2020-11-03T13:29:21Z',
   updatedAt: '2020-11-03T13:29:21Z',
-  tagsCount: 13,
   expirationPolicyStartedAt: null,
   expirationPolicyCleanupStatus: 'UNSCHEDULED',
   project: {
@@ -175,6 +174,16 @@ export const imageTagsMock = (nodes = tagsMock) => ({
   },
 });
 
+export const imageTagsCountMock = (override) => ({
+  data: {
+    containerRepository: {
+      id: containerRepositoryMock.id,
+      tagsCount: 13,
+      ...override,
+    },
+  },
+});
+
 export const graphQLImageDetailsMock = (override) => ({
   data: {
     containerRepository: {

spec/frontend/registry/explorer/pages/details_spec.js

@@ -292,7 +292,6 @@ describe('Details Page', () => {
 
       await waitForApolloRequestRender();
       expect(findDetailsHeader().props()).toMatchObject({
-        metadataLoading: false,
         image: {
           name: containerRepositoryMock.name,
           project: {