From 9d94facbc3ae8ccf7f892e8a571a36f01b754b33 Mon Sep 17 00:00:00 2001
From: Rahul Chanila <rchanila@gitlab.com>
Date: Sat, 22 Jul 2023 11:18:35 +0000
Subject: [PATCH] Fixes container registry image list sorting UI inconsistency

Updates the apollo cache policy for images list query

Removed explicit calls to fetchMore by updating pageParams instead

Removes deprecated updateQuery method

Changelog: fixed
---
 .../explorer/pages/list.vue                   | 54 ++++++-------------
 .../projects/container_registry_spec.rb       |  8 ---
 .../explorer/pages/list_spec.js               | 23 ++++++--
 3 files changed, 34 insertions(+), 51 deletions(-)

diff --git a/app/assets/javascripts/packages_and_registries/container_registry/explorer/pages/list.vue b/app/assets/javascripts/packages_and_registries/container_registry/explorer/pages/list.vue
index fe29fa8fdd7f..6881f914e314 100644
--- a/app/assets/javascripts/packages_and_registries/container_registry/explorer/pages/list.vue
+++ b/app/assets/javascripts/packages_and_registries/container_registry/explorer/pages/list.vue
@@ -12,6 +12,7 @@ import { get } from 'lodash';
 import getContainerRepositoriesQuery from 'shared_queries/container_registry/get_container_repositories.query.graphql';
 import { createAlert } from '~/alert';
 import { WORKSPACE_GROUP, WORKSPACE_PROJECT } from '~/issues/constants';
+import { fetchPolicies } from '~/lib/graphql';
 import Tracking from '~/tracking';
 import PersistedSearch from '~/packages_and_registries/shared/components/persisted_search.vue';
 import { FILTERED_SEARCH_TERM } from '~/vue_shared/components/filtered_search_bar/constants';
@@ -87,6 +88,7 @@ export default {
         return !this.fetchBaseQuery;
       },
       query: getContainerRepositoriesQuery,
+      fetchPolicy: fetchPolicies.CACHE_AND_NETWORK,
       variables() {
         return this.queryVariables;
       },
@@ -109,6 +111,7 @@ export default {
         return !this.fetchAdditionalDetails;
       },
       query: getContainerRepositoriesDetails,
+      fetchPolicy: fetchPolicies.CACHE_AND_NETWORK,
       variables() {
         return this.queryVariables;
       },
@@ -133,6 +136,7 @@ export default {
       mutationLoading: false,
       fetchBaseQuery: false,
       fetchAdditionalDetails: false,
+      pageParams: {},
     };
   },
   computed: {
@@ -158,6 +162,7 @@ export default {
         fullPath: this.config.isGroupPage ? this.config.groupPath : this.config.projectPath,
         isGroupPage: this.config.isGroupPage,
         first: GRAPHQL_PAGE_SIZE,
+        ...this.pageParams,
       };
     },
     tracking() {
@@ -193,54 +198,25 @@ export default {
       this.deleteAlertType = null;
       this.itemToDelete = {};
     },
-    updateQuery(_, { fetchMoreResult }) {
-      return fetchMoreResult;
-    },
     async fetchNextPage() {
-      if (this.pageInfo?.hasNextPage) {
-        const variables = {
-          after: this.pageInfo?.endCursor,
-          first: GRAPHQL_PAGE_SIZE,
-        };
-
-        this.$apollo.queries.baseImages.fetchMore({
-          variables,
-          updateQuery: this.updateQuery,
-        });
-
-        await this.$nextTick();
-
-        this.$apollo.queries.additionalDetails.fetchMore({
-          variables,
-          updateQuery: this.updateQuery,
-        });
-      }
+      this.pageParams = {
+        after: this.pageInfo?.endCursor,
+        first: GRAPHQL_PAGE_SIZE,
+      };
     },
     async fetchPreviousPage() {
-      if (this.pageInfo?.hasPreviousPage) {
-        const variables = {
-          first: null,
-          before: this.pageInfo?.startCursor,
-          last: GRAPHQL_PAGE_SIZE,
-        };
-        this.$apollo.queries.baseImages.fetchMore({
-          variables,
-          updateQuery: this.updateQuery,
-        });
-
-        await this.$nextTick();
-
-        this.$apollo.queries.additionalDetails.fetchMore({
-          variables,
-          updateQuery: this.updateQuery,
-        });
-      }
+      this.pageParams = {
+        first: null,
+        before: this.pageInfo?.startCursor,
+        last: GRAPHQL_PAGE_SIZE,
+      };
     },
     startDelete() {
       this.track('confirm_delete');
       this.mutationLoading = true;
     },
     handleSearchUpdate({ sort, filters }) {
+      this.pageParams = {};
       this.sorting = sort;
 
       const search = filters.find((i) => i.type === FILTERED_SEARCH_TERM);
diff --git a/spec/features/projects/container_registry_spec.rb b/spec/features/projects/container_registry_spec.rb
index 493435d34399..c3c660e5ebbb 100644
--- a/spec/features/projects/container_registry_spec.rb
+++ b/spec/features/projects/container_registry_spec.rb
@@ -185,14 +185,6 @@
       visit_next_page
       expect(page).to have_content 'my/image'
     end
-
-    it 'pagination is preserved after navigating back from details' do
-      visit_next_page
-      click_link 'my/image'
-      breadcrumb = find '.breadcrumbs'
-      breadcrumb.click_link 'Container Registry'
-      expect(page).to have_content 'my/image'
-    end
   end
 
   def visit_container_registry
diff --git a/spec/frontend/packages_and_registries/container_registry/explorer/pages/list_spec.js b/spec/frontend/packages_and_registries/container_registry/explorer/pages/list_spec.js
index 1823bbfe5330..211c52a25bee 100644
--- a/spec/frontend/packages_and_registries/container_registry/explorer/pages/list_spec.js
+++ b/spec/frontend/packages_and_registries/container_registry/explorer/pages/list_spec.js
@@ -15,6 +15,7 @@ import RegistryHeader from '~/packages_and_registries/container_registry/explore
 import {
   DELETE_IMAGE_SUCCESS_MESSAGE,
   DELETE_IMAGE_ERROR_MESSAGE,
+  GRAPHQL_PAGE_SIZE,
   SORT_FIELDS,
   SETTINGS_TEXT,
 } from '~/packages_and_registries/container_registry/explorer/constants';
@@ -474,10 +475,18 @@ describe('List Page', () => {
         await waitForPromises();
 
         expect(resolver).toHaveBeenCalledWith(
-          expect.objectContaining({ before: pageInfo.startCursor }),
+          expect.objectContaining({
+            before: pageInfo.startCursor,
+            first: null,
+            last: GRAPHQL_PAGE_SIZE,
+          }),
         );
         expect(detailsResolver).toHaveBeenCalledWith(
-          expect.objectContaining({ before: pageInfo.startCursor }),
+          expect.objectContaining({
+            before: pageInfo.startCursor,
+            first: null,
+            last: GRAPHQL_PAGE_SIZE,
+          }),
         );
       });
 
@@ -494,10 +503,16 @@ describe('List Page', () => {
         await waitForPromises();
 
         expect(resolver).toHaveBeenCalledWith(
-          expect.objectContaining({ after: pageInfo.endCursor }),
+          expect.objectContaining({
+            after: pageInfo.endCursor,
+            first: GRAPHQL_PAGE_SIZE,
+          }),
         );
         expect(detailsResolver).toHaveBeenCalledWith(
-          expect.objectContaining({ after: pageInfo.endCursor }),
+          expect.objectContaining({
+            after: pageInfo.endCursor,
+            first: GRAPHQL_PAGE_SIZE,
+          }),
         );
       });
     });
-- 
GitLab