Merge branch 'nmezzopera-add-feature-test-container-registry-details-edge-cases' into 'master'

Better path-generator and tests for container registry details

See merge request gitlab-org/gitlab!47495

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

@@ -72,7 +72,7 @@ export default {
     <template #left-primary>
       <router-link
         class="gl-text-body gl-font-weight-bold"
-        data-testid="detailsLink"
+        data-testid="details-link"
         :to="{ name: 'details', params: { id: item.id } }"
       >
         {{ item.path }}

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

@@ -56,7 +56,7 @@ export const requestTagsList = ({ commit, dispatch, state: { imageDetails } }, p
       dispatch('receiveTagsListSuccess', { data, headers });
     })
     .catch(() => {
-      createFlash(FETCH_TAGS_LIST_ERROR_MESSAGE);
+      createFlash({ message: FETCH_TAGS_LIST_ERROR_MESSAGE });
     })
     .finally(() => {
       commit(types.SET_MAIN_LOADING, false);
@@ -71,7 +71,7 @@ export const requestImageDetailsAndTagsList = ({ dispatch, commit }, id) => {
       dispatch('requestTagsList');
     })
     .catch(() => {
-      createFlash(FETCH_IMAGE_DETAILS_ERROR_MESSAGE);
+      createFlash({ message: FETCH_IMAGE_DETAILS_ERROR_MESSAGE });
       commit(types.SET_MAIN_LOADING, false);
     });
 };

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

 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.name
-    ? imageDetails.path.replace(`/${imageDetails.name}`, '')
-    : imageDetails.path;
+
+  const splitPath = imageDetails.path.split('/').reverse();
+  const splitName = imageDetails.name ? imageDetails.name.split('/').reverse() : [];
+  const basePath = splitPath
+    .reduce((acc, curr, index) => {
+      if (splitPath[index] !== splitName[index]) {
+        acc.unshift(curr);
+      }
+      return acc;
+    }, [])
+    .join('/');
   return `/${basePath}/registry/repository/${imageDetails.id}/tags${ending}`;
 };

spec/features/groups/container_registry_spec.rb

@@ -89,6 +89,20 @@ RSpec.describe 'Container Registry', :js do
     end
   end
 
+  context 'when an image has the same name as the subgroup' do
+    before do
+      stub_container_registry_tags(tags: %w[latest], with_manifest: true)
+      project.container_repositories <<  create(:container_repository, name: group.name)
+      visit_container_registry
+    end
+
+    it 'details page loads properly' do
+      find('a[data-testid="details-link"]').click
+
+      expect(page).to have_content 'latest'
+    end
+  end
+
   def visit_container_registry
     visit group_container_registries_path(group)
   end

spec/features/projects/container_registry_spec.rb

@@ -10,6 +10,10 @@ RSpec.describe 'Container Registry', :js do
     create(:container_repository, name: 'my/image')
   end
 
+  let(:nameless_container_repository) do
+    create(:container_repository, name: '')
+  end
+
   before do
     sign_in(user)
     project.add_developer(user)
@@ -96,6 +100,20 @@ RSpec.describe 'Container Registry', :js do
     end
   end
 
+  describe 'image repo details when image has no name' do
+    before do
+      stub_container_registry_tags(tags: %w[latest], with_manifest: true)
+      project.container_repositories << nameless_container_repository
+      visit_container_registry
+    end
+
+    it 'renders correctly' do
+      find('a[data-testid="details-link"]').click
+
+      expect(page).to have_content 'latest'
+    end
+  end
+
   context 'when there are more than 10 images' do
     before do
       create_list(:container_repository, 12, project: project)

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

@@ -19,7 +19,7 @@ describe('Image List Row', () => {
   let wrapper;
   const item = imagesListResponse.data[0];
 
-  const findDetailsLink = () => wrapper.find('[data-testid="detailsLink"]');
+  const findDetailsLink = () => wrapper.find('[data-testid="details-link"]');
   const findTagsCount = () => wrapper.find('[data-testid="tagsCount"]');
   const findDeleteBtn = () => wrapper.find(DeleteButton);
   const findClipboardButton = () => wrapper.find(ClipboardButton);

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

@@ -8,6 +8,11 @@ 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';
+import {
+  FETCH_IMAGES_LIST_ERROR_MESSAGE,
+  FETCH_TAGS_LIST_ERROR_MESSAGE,
+  FETCH_IMAGE_DETAILS_ERROR_MESSAGE,
+} from '~/registry/explorer/constants/index';
 
 jest.mock('~/flash.js');
 jest.mock('~/registry/explorer/utils');
@@ -129,7 +134,7 @@ describe('Actions RegistryExplorer Store', () => {
         ],
         [],
         () => {
-          expect(createFlash).toHaveBeenCalled();
+          expect(createFlash).toHaveBeenCalledWith({ message: FETCH_IMAGES_LIST_ERROR_MESSAGE });
           done();
         },
       );
@@ -169,7 +174,7 @@ describe('Actions RegistryExplorer Store', () => {
         ],
         [],
         () => {
-          expect(createFlash).toHaveBeenCalled();
+          expect(createFlash).toHaveBeenCalledWith({ message: FETCH_TAGS_LIST_ERROR_MESSAGE });
           done();
         },
       );
@@ -261,7 +266,7 @@ describe('Actions RegistryExplorer Store', () => {
         ],
         [],
         () => {
-          expect(createFlash).toHaveBeenCalled();
+          expect(createFlash).toHaveBeenCalledWith({ message: FETCH_IMAGE_DETAILS_ERROR_MESSAGE });
           done();
         },
       );

spec/frontend/registry/explorer/utils_spec.js

@@ -16,6 +16,20 @@ describe('Utils', () => {
       expect(pathGenerator(imageDetails, '/foo')).toBe('/foo/bar/registry/repository/1/tags/foo');
     });
 
+    it.each`
+      path                     | name         | result
+      ${'foo/foo'}             | ${''}        | ${'/foo/foo/registry/repository/1/tags?format=json'}
+      ${'foo/foo/foo'}         | ${'foo'}     | ${'/foo/foo/registry/repository/1/tags?format=json'}
+      ${'baz/foo/foo/foo'}     | ${'foo'}     | ${'/baz/foo/foo/registry/repository/1/tags?format=json'}
+      ${'baz/foo/foo/foo'}     | ${'foo'}     | ${'/baz/foo/foo/registry/repository/1/tags?format=json'}
+      ${'foo/foo/baz/foo/foo'} | ${'foo/foo'} | ${'/foo/foo/baz/registry/repository/1/tags?format=json'}
+      ${'foo/foo/baz/foo/bar'} | ${'foo/bar'} | ${'/foo/foo/baz/registry/repository/1/tags?format=json'}
+      ${'baz/foo/foo'}         | ${'foo'}     | ${'/baz/foo/registry/repository/1/tags?format=json'}
+      ${'baz/foo/bar'}         | ${'foo'}     | ${'/baz/foo/bar/registry/repository/1/tags?format=json'}
+    `('returns the correct path when path is $path and name is $name', ({ name, path, result }) => {
+      expect(pathGenerator({ id: 1, name, path })).toBe(result);
+    });
+
     it('returns the url unchanged when imageDetails have no name', () => {
       const imageDetailsWithoutName = {
         path: 'foo/bar/baz',