Merge branch 'nmezzopera-refactor-details-fetch-2' into 'master'

Use new image details API in container registry details

See merge request gitlab-org/gitlab!47054

app/assets/javascripts/registry/explorer/components/list_page/image_list_row.vue

@@ -37,15 +37,6 @@ export default {
     ROW_SCHEDULED_FOR_DELETION,
   },
   computed: {
-    encodedItem() {
-      const params = JSON.stringify({
-        name: this.item.path,
-        tags_path: this.item.tags_path,
-        id: this.item.id,
-        cleanup_policy_started_at: this.item.cleanup_policy_started_at,
-      });
-      return window.btoa(params);
-    },
     disabledDelete() {
       return !this.item.destroy_path || this.item.deleting;
     },
@@ -82,7 +73,7 @@ export default {
       <router-link
         class="gl-text-body gl-font-weight-bold"
         data-testid="detailsLink"
-        :to="{ name: 'details', params: { id: encodedItem } }"
+        :to="{ name: 'details', params: { id: item.id } }"
       >
         {{ item.path }}
       </router-link>

app/assets/javascripts/registry/explorer/components/registry_breadcrumb.vue

@@ -30,7 +30,7 @@ export default {
       return {
         tagName,
         className,
-        text: this.$route.meta.nameGenerator(this.$route),
+        text: this.$route.meta.nameGenerator(this.$store.state),
         path: { to: this.$route.name },
       };
     },
@@ -48,7 +48,7 @@ export default {
     ></li>
     <li v-if="!isRootRoute">
       <router-link ref="rootRouteLink" :to="rootRoute.path">
-        {{ rootRoute.meta.nameGenerator(rootRoute) }}
+        {{ rootRoute.meta.nameGenerator($store.state) }}
       </router-link>
       <component :is="divider.tagName" v-safe-html="divider.innerHTML" :class="divider.classList" />
     </li>

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

@@ -11,7 +11,6 @@ import TagsList from '../components/details_page/tags_list.vue';
 import TagsLoader from '../components/details_page/tags_loader.vue';
 import EmptyTagsState from '../components/details_page/empty_tags_state.vue';
 
-import { decodeAndParse } from '../utils';
 import {
   ALERT_SUCCESS_TAG,
   ALERT_DANGER_TAG,
@@ -43,12 +42,9 @@ export default {
     };
   },
   computed: {
-    ...mapState(['tagsPagination', 'isLoading', 'config', 'tags']),
-    queryParameters() {
-      return decodeAndParse(this.$route.params.id);
-    },
+    ...mapState(['tagsPagination', 'isLoading', 'config', 'tags', 'imageDetails']),
     showPartialCleanupWarning() {
-      return this.queryParameters.cleanup_policy_started_at && !this.dismissPartialCleanupWarning;
+      return this.imageDetails?.cleanup_policy_started_at && !this.dismissPartialCleanupWarning;
     },
     tracking() {
       return {
@@ -61,15 +57,20 @@ export default {
         return this.tagsPagination.page;
       },
       set(page) {
-        this.requestTagsList({ pagination: { page }, params: this.$route.params.id });
+        this.requestTagsList({ page });
       },
     },
   },
   mounted() {
-    this.requestTagsList({ params: this.$route.params.id });
+    this.requestImageDetailsAndTagsList(this.$route.params.id);
   },
   methods: {
-    ...mapActions(['requestTagsList', 'requestDeleteTag', 'requestDeleteTags']),
+    ...mapActions([
+      'requestTagsList',
+      'requestDeleteTag',
+      'requestDeleteTags',
+      'requestImageDetailsAndTagsList',
+    ]),
     deleteTags(toBeDeleted) {
       this.itemsToBeDeleted = this.tags.filter(tag => toBeDeleted[tag.name]);
       this.track('click_button');
@@ -78,7 +79,7 @@ export default {
     handleSingleDelete() {
       const [itemToDelete] = this.itemsToBeDeleted;
       this.itemsToBeDeleted = [];
-      return this.requestDeleteTag({ tag: itemToDelete, params: this.$route.params.id })
+      return this.requestDeleteTag({ tag: itemToDelete })
         .then(() => {
           this.deleteAlertType = ALERT_SUCCESS_TAG;
         })
@@ -92,7 +93,6 @@ export default {
 
       return this.requestDeleteTags({
         ids: itemsToBeDeleted.map(x => x.name),
-        params: this.$route.params.id,
       })
         .then(() => {
           this.deleteAlertType = ALERT_SUCCESS_TAGS;
@@ -132,7 +132,7 @@ export default {
       @dismiss="dismissPartialCleanupWarning = true"
     />
 
-    <details-header :image-name="queryParameters.name" />
+    <details-header :image-name="imageDetails.name" />
 
     <tags-loader v-if="isLoading" />
     <template v-else>

app/assets/javascripts/registry/explorer/router.js

@@ -2,7 +2,6 @@ import Vue from 'vue';
 import VueRouter from 'vue-router';
 import List from './pages/list.vue';
 import Details from './pages/details.vue';
-import { decodeAndParse } from './utils';
 import { CONTAINER_REGISTRY_TITLE } from './constants/index';
 
 Vue.use(VueRouter);
@@ -26,7 +25,7 @@ export default function createRouter(base) {
         path: '/:id',
         component: Details,
         meta: {
-          nameGenerator: route => decodeAndParse(route.params.id).name,
+          nameGenerator: ({ imageDetails }) => imageDetails?.name,
         },
       },
     ],

app/assets/javascripts/registry/explorer/stores/actions.js

@@ -9,7 +9,7 @@ import {
   FETCH_TAGS_LIST_ERROR_MESSAGE,
   FETCH_IMAGE_DETAILS_ERROR_MESSAGE,
 } from '../constants/index';
-import { decodeAndParse } from '../utils';
+import { pathGenerator } from '../utils';
 
 export const setInitialState = ({ commit }, data) => commit(types.SET_INITIAL_STATE, data);
 export const setShowGarbageCollectionTip = ({ commit }, data) =>
@@ -45,13 +45,13 @@ export const requestImagesList = (
     });
 };
 
-export const requestTagsList = ({ commit, dispatch }, { pagination = {}, params }) => {
+export const requestTagsList = ({ commit, dispatch, state: { imageDetails } }, pagination = {}) => {
   commit(types.SET_MAIN_LOADING, true);
-  const { tags_path } = decodeAndParse(params);
+  const tagsPath = pathGenerator(imageDetails);
 
   const { page = DEFAULT_PAGE, perPage = DEFAULT_PAGE_SIZE } = pagination;
   return axios
-    .get(tags_path, { params: { page, per_page: perPage } })
+    .get(tagsPath, { params: { page, per_page: perPage } })
     .then(({ data, headers }) => {
       dispatch('receiveTagsListSuccess', { data, headers });
     })
@@ -76,30 +76,30 @@ export const requestImageDetailsAndTagsList = ({ dispatch, commit }, id) => {
     });
 };
 
-export const requestDeleteTag = ({ commit, dispatch, state }, { tag, params }) => {
+export const requestDeleteTag = ({ commit, dispatch, state }, { tag }) => {
   commit(types.SET_MAIN_LOADING, true);
   return axios
     .delete(tag.destroy_path)
     .then(() => {
       dispatch('setShowGarbageCollectionTip', true);
-      return dispatch('requestTagsList', { pagination: state.tagsPagination, params });
+
+      return dispatch('requestTagsList', state.tagsPagination);
     })
     .finally(() => {
       commit(types.SET_MAIN_LOADING, false);
     });
 };
 
-export const requestDeleteTags = ({ commit, dispatch, state }, { ids, params }) => {
+export const requestDeleteTags = ({ commit, dispatch, state }, { ids }) => {
   commit(types.SET_MAIN_LOADING, true);
-  const { tags_path } = decodeAndParse(params);
 
-  const url = tags_path.replace('?format=json', '/bulk_destroy');
+  const tagsPath = pathGenerator(state.imageDetails, '/bulk_destroy');
 
   return axios
-    .delete(url, { params: { ids } })
+    .delete(tagsPath, { params: { ids } })
     .then(() => {
       dispatch('setShowGarbageCollectionTip', true);
-      return dispatch('requestTagsList', { pagination: state.tagsPagination, params });
+      return dispatch('requestTagsList', state.tagsPagination);
     })
     .finally(() => {
       commit(types.SET_MAIN_LOADING, false);

app/assets/javascripts/registry/explorer/utils.js

-export const decodeAndParse = param => JSON.parse(window.atob(param));
-
-// eslint-disable-next-line @gitlab/require-i18n-strings
-export const pathGenerator = (imageDetails, ending = 'tags?format=json') => {
+export const pathGenerator = (imageDetails, ending = '?format=json') => {
   // this method is a temporary workaround, to be removed with graphql implementation
   // https://gitlab.com/gitlab-org/gitlab/-/issues/276432
   const basePath = imageDetails.path.replace(`/${imageDetails.name}`, '');
-  return `/${basePath}/registry/repository/${imageDetails.id}/${ending}`;
+  return `/${basePath}/registry/repository/${imageDetails.id}/tags${ending}`;
 };

changelogs/unreleased/nmezzopera-refactor-details-fetch-2.yml

+---
+title: Use new image details API in container registry details
+merge_request: 47054
+author:
+type: changed

spec/frontend/registry/explorer/components/list_page/image_list_row_spec.js

@@ -67,7 +67,12 @@ describe('Image List Row', () => {
       mountComponent();
       const link = findDetailsLink();
       expect(link.html()).toContain(item.path);
-      expect(link.props('to').name).toBe('details');
+      expect(link.props('to')).toMatchObject({
+        name: 'details',
+        params: {
+          id: item.id,
+        },
+      });
     });
 
     it('contains a clipboard button', () => {

spec/frontend/registry/explorer/components/registry_breadcrumb_spec.js

@@ -32,6 +32,10 @@ describe('Registry Breadcrumb', () => {
     { name: 'baz', meta: { nameGenerator } },
   ];
 
+  const state = {
+    imageDetails: { foo: 'bar' },
+  };
+
   const findDivider = () => wrapper.find('.js-divider');
   const findRootRoute = () => wrapper.find({ ref: 'rootRouteLink' });
   const findChildRoute = () => wrapper.find({ ref: 'childRouteLink' });
@@ -52,6 +56,9 @@ describe('Registry Breadcrumb', () => {
             routes,
           },
         },
+        $store: {
+          state,
+        },
       },
     });
   };
@@ -80,7 +87,7 @@ describe('Registry Breadcrumb', () => {
     });
 
     it('the link text is calculated by nameGenerator', () => {
-      expect(nameGenerator).toHaveBeenCalledWith(routes[0]);
+      expect(nameGenerator).toHaveBeenCalledWith(state);
       expect(nameGenerator).toHaveBeenCalledTimes(1);
     });
   });
@@ -104,7 +111,7 @@ describe('Registry Breadcrumb', () => {
     });
 
     it('the link text is calculated by nameGenerator', () => {
-      expect(nameGenerator).toHaveBeenCalledWith(routes[1]);
+      expect(nameGenerator).toHaveBeenCalledWith(state);
       expect(nameGenerator).toHaveBeenCalledTimes(2);
     });
   });

spec/frontend/registry/explorer/mock_data.js

@@ -97,3 +97,14 @@ export const imagePagination = {
   totalPages: 2,
   nextPage: 2,
 };
+
+export const imageDetailsMock = {
+  id: 1,
+  name: 'rails-32309',
+  path: 'gitlab-org/gitlab-test/rails-32309',
+  project_id: 1,
+  location: '0.0.0.0:5000/gitlab-org/gitlab-test/rails-32309',
+  created_at: '2020-06-29T10:23:47.838Z',
+  cleanup_policy_started_at: null,
+  delete_api_path: 'http://0.0.0.0:3000/api/v4/projects/1/registry/repositories/1',
+};

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

@@ -14,9 +14,10 @@ import {
   SET_TAGS_LIST_SUCCESS,
   SET_TAGS_PAGINATION,
   SET_INITIAL_STATE,
+  SET_IMAGE_DETAILS,
 } from '~/registry/explorer/stores/mutation_types';
 
-import { tagsListResponse } from '../mock_data';
+import { tagsListResponse, imageDetailsMock } from '../mock_data';
 import { DeleteModal } from '../stubs';
 
 describe('Details Page', () => {
@@ -33,8 +34,7 @@ describe('Details Page', () => {
   const findEmptyTagsState = () => wrapper.find(EmptyTagsState);
   const findPartialCleanupAlert = () => wrapper.find(PartialCleanupAlert);
 
-  const routeIdGenerator = override =>
-    window.btoa(JSON.stringify({ name: 'foo', tags_path: 'bar', ...override }));
+  const routeId = 1;
 
   const tagsArrayToSelectedTags = tags =>
     tags.reduce((acc, c) => {
@@ -42,7 +42,7 @@ describe('Details Page', () => {
       return acc;
     }, {});
 
-  const mountComponent = ({ options, routeParams } = {}) => {
+  const mountComponent = ({ options } = {}) => {
     wrapper = shallowMount(component, {
       store,
       stubs: {
@@ -51,7 +51,7 @@ describe('Details Page', () => {
       mocks: {
         $route: {
           params: {
-            id: routeIdGenerator(routeParams),
+            id: routeId,
           },
         },
       },
@@ -65,6 +65,7 @@ describe('Details Page', () => {
     dispatchSpy.mockResolvedValue();
     store.commit(SET_TAGS_LIST_SUCCESS, tagsListResponse.data);
     store.commit(SET_TAGS_PAGINATION, tagsListResponse.headers);
+    store.commit(SET_IMAGE_DETAILS, imageDetailsMock);
     jest.spyOn(Tracking, 'event');
   });
 
@@ -73,6 +74,13 @@ describe('Details Page', () => {
     wrapper = null;
   });
 
+  describe('lifecycle events', () => {
+    it('calls the appropriate action on mount', () => {
+      mountComponent();
+      expect(dispatchSpy).toHaveBeenCalledWith('requestImageDetailsAndTagsList', routeId);
+    });
+  });
+
   describe('when isLoading is true', () => {
     beforeEach(() => {
       store.commit(SET_MAIN_LOADING, true);
@@ -194,8 +202,7 @@ describe('Details Page', () => {
       dispatchSpy.mockResolvedValue();
       findPagination().vm.$emit(GlPagination.model.event, 2);
       expect(store.dispatch).toHaveBeenCalledWith('requestTagsList', {
-        params: wrapper.vm.$route.params.id,
-        pagination: { page: 2 },
+        page: 2,
       });
     });
   });
@@ -227,7 +234,6 @@ describe('Details Page', () => {
           findDeleteModal().vm.$emit('confirmDelete');
           expect(dispatchSpy).toHaveBeenCalledWith('requestDeleteTag', {
             tag: store.state.tags[0],
-            params: routeIdGenerator(),
           });
         });
       });
@@ -242,7 +248,6 @@ describe('Details Page', () => {
           findDeleteModal().vm.$emit('confirmDelete');
           expect(dispatchSpy).toHaveBeenCalledWith('requestDeleteTags', {
             ids: store.state.tags.map(t => t.name),
-            params: routeIdGenerator(),
           });
         });
       });
@@ -257,7 +262,7 @@ describe('Details Page', () => {
 
     it('has the correct props', () => {
       mountComponent();
-      expect(findDetailsHeader().props()).toEqual({ imageName: 'foo' });
+      expect(findDetailsHeader().props()).toEqual({ imageName: imageDetailsMock.name });
     });
   });
 
@@ -293,10 +298,14 @@ describe('Details Page', () => {
     };
 
     describe('when expiration_policy_started is not null', () => {
-      const routeParams = { cleanup_policy_started_at: Date.now().toString() };
-
+      beforeEach(() => {
+        store.commit(SET_IMAGE_DETAILS, {
+          ...imageDetailsMock,
+          cleanup_policy_started_at: Date.now().toString(),
+        });
+      });
       it('exists', () => {
-        mountComponent({ routeParams });
+        mountComponent();
 
         expect(findPartialCleanupAlert().exists()).toBe(true);
       });
@@ -304,13 +313,13 @@ describe('Details Page', () => {
       it('has the correct props', () => {
         store.commit(SET_INITIAL_STATE, { ...config });
 
-        mountComponent({ routeParams });
+        mountComponent();
 
         expect(findPartialCleanupAlert().props()).toEqual({ ...config });
       });
 
       it('dismiss hides the component', async () => {
-        mountComponent({ routeParams });
+        mountComponent();
 
         expect(findPartialCleanupAlert().exists()).toBe(true);
         findPartialCleanupAlert().vm.$emit('dismiss');

spec/frontend/registry/explorer/stores/actions_spec.js

@@ -7,13 +7,18 @@ import axios from '~/lib/utils/axios_utils';
 import * as actions from '~/registry/explorer/stores/actions';
 import * as types from '~/registry/explorer/stores/mutation_types';
 import { reposServerResponse, registryServerResponse } from '../mock_data';
+import * as utils from '~/registry/explorer/utils';
 
 jest.mock('~/flash.js');
+jest.mock('~/registry/explorer/utils');
 
 describe('Actions RegistryExplorer Store', () => {
   let mock;
   const endpoint = `${TEST_HOST}/endpoint.json`;
 
+  const url = `${endpoint}/1}`;
+  jest.spyOn(utils, 'pathGenerator').mockReturnValue(url);
+
   beforeEach(() => {
     mock = new MockAdapter(axios);
   });
@@ -132,15 +137,12 @@ describe('Actions RegistryExplorer Store', () => {
   });
 
   describe('fetch tags list', () => {
-    const url = `${endpoint}/1}`;
-    const params = window.btoa(JSON.stringify({ tags_path: `${endpoint}/1}` }));
-
     it('sets the tagsList', done => {
       mock.onGet(url).replyOnce(200, registryServerResponse, {});
 
       testAction(
         actions.requestTagsList,
-        { params },
+        {},
         {},
         [
           { type: types.SET_MAIN_LOADING, payload: true },
@@ -159,7 +161,7 @@ describe('Actions RegistryExplorer Store', () => {
     it('should create flash on error', done => {
       testAction(
         actions.requestTagsList,
-        { params },
+        {},
         {},
         [
           { type: types.SET_MAIN_LOADING, payload: true },
@@ -177,8 +179,6 @@ describe('Actions RegistryExplorer Store', () => {
   describe('request delete single tag', () => {
     it('successfully performs the delete request', done => {
       const deletePath = 'delete/path';
-      const params = window.btoa(JSON.stringify({ tags_path: `${endpoint}/1}`, id: 1 }));
-
       mock.onDelete(deletePath).replyOnce(200);
 
       testAction(
@@ -187,7 +187,6 @@ describe('Actions RegistryExplorer Store', () => {
           tag: {
             destroy_path: deletePath,
           },
-          params,
         },
         {
           tagsPagination: {},
@@ -203,7 +202,7 @@ describe('Actions RegistryExplorer Store', () => {
           },
           {
             type: 'requestTagsList',
-            payload: { pagination: {}, params },
+            payload: {},
           },
         ],
         done,
@@ -270,17 +269,13 @@ describe('Actions RegistryExplorer Store', () => {
   });
 
   describe('request delete multiple tags', () => {
-    const url = `project-path/registry/repository/foo/tags`;
-    const params = window.btoa(JSON.stringify({ tags_path: `${url}?format=json` }));
-
     it('successfully performs the delete request', done => {
-      mock.onDelete(`${url}/bulk_destroy`).replyOnce(200);
+      mock.onDelete(url).replyOnce(200);
 
       testAction(
         actions.requestDeleteTags,
         {
           ids: [1, 2],
-          params,
         },
         {
           tagsPagination: {},
@@ -296,7 +291,7 @@ describe('Actions RegistryExplorer Store', () => {
           },
           {
             type: 'requestTagsList',
-            payload: { pagination: {}, params },
+            payload: {},
           },
         ],
         done,
@@ -310,7 +305,6 @@ describe('Actions RegistryExplorer Store', () => {
         actions.requestDeleteTags,
         {
           ids: [1, 2],
-          params,
         },
         {
           tagsPagination: {},

spec/frontend/registry/explorer/utils_spec.js

@@ -13,7 +13,7 @@ describe('Utils', () => {
     });
 
     it('returns the url with an ending when is passed', () => {
-      expect(pathGenerator(imageDetails, 'foo')).toBe('/foo/bar/registry/repository/1/foo');
+      expect(pathGenerator(imageDetails, '/foo')).toBe('/foo/bar/registry/repository/1/tags/foo');
     });
   });
 });