Merge branch 'led/288313-fix-storage-usage-quotas-pagination' into 'master'

Fix namespace usage quotas storage pagination

See merge request gitlab-org/gitlab!80752

app/helpers/pagination_helper.rb

@@ -22,4 +22,8 @@ module PaginationHelper
   def paginate_with_count(collection, remote: nil, total_pages: nil)
     paginate(collection, remote: remote, theme: 'gitlab', total_pages: total_pages)
   end
+
+  def page_size
+    Kaminari.config.default_per_page
+  end
 end

ee/app/assets/javascripts/usage_quotas/storage/components/namespace_storage_app.vue

@@ -9,7 +9,7 @@ import {
 } from '@gitlab/ui';
 import { parseBoolean } from '~/lib/utils/common_utils';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
-import { PROJECTS_PER_PAGE, PROJECT_TABLE_LABEL_STORAGE_USAGE } from '../constants';
+import { PROJECT_TABLE_LABEL_STORAGE_USAGE } from '../constants';
 import query from '../queries/namespace_storage.query.graphql';
 import { formatUsageSize, parseGetStorageResults } from '../utils';
 import ProjectList from './project_list.vue';
@@ -39,7 +39,13 @@ export default {
     PROJECT_TABLE_LABEL_STORAGE_USAGE,
   },
   mixins: [glFeatureFlagsMixin()],
-  inject: ['namespacePath', 'purchaseStorageUrl', 'isTemporaryStorageIncreaseVisible', 'helpLinks'],
+  inject: [
+    'namespacePath',
+    'purchaseStorageUrl',
+    'isTemporaryStorageIncreaseVisible',
+    'helpLinks',
+    'defaultPerPage',
+  ],
   apollo: {
     namespace: {
       query,
@@ -48,7 +54,7 @@ export default {
           fullPath: this.namespacePath,
           searchTerm: this.searchTerm,
           withExcessStorageData: this.isAdditionalStorageFlagEnabled,
-          first: PROJECTS_PER_PAGE,
+          first: this.defaultPerPage,
         };
       },
       update: parseGetStorageResults,
@@ -121,7 +127,6 @@ export default {
         variables: {
           fullPath: this.namespacePath,
           withExcessStorageData: this.isAdditionalStorageFlagEnabled,
-          first: PROJECTS_PER_PAGE,
           ...vars,
         },
         updateQuery(previousResult, { fetchMoreResult }) {
@@ -131,12 +136,12 @@ export default {
     },
     onPrev(before) {
       if (this.pageInfo?.hasPreviousPage) {
-        this.fetchMoreProjects({ before });
+        this.fetchMoreProjects({ before, last: this.defaultPerPage, first: undefined });
       }
     },
     onNext(after) {
       if (this.pageInfo?.hasNextPage) {
-        this.fetchMoreProjects({ after });
+        this.fetchMoreProjects({ after, first: this.defaultPerPage });
       }
     },
   },

ee/app/assets/javascripts/usage_quotas/storage/constants.js

@@ -78,8 +78,6 @@ export const STORAGE_USAGE_THRESHOLDS = {
   [ERROR_THRESHOLD]: 1.0,
 };
 
-export const PROJECTS_PER_PAGE = 20;
-
 export const projectHelpLinks = {
   usageQuotas: helpPagePath('user/usage_quotas'),
   buildArtifacts: helpPagePath('ci/pipelines/job_artifacts', {

ee/app/assets/javascripts/usage_quotas/storage/init_namespace_storage.js

@@ -13,6 +13,7 @@ export default () => {
     purchaseStorageUrl,
     buyAddonTargetAttr,
     isTemporaryStorageIncreaseVisible,
+    defaultPerPage,
   } = el.dataset;
 
   const apolloProvider = new VueApollo({
@@ -28,6 +29,7 @@ export default () => {
       buyAddonTargetAttr,
       isTemporaryStorageIncreaseVisible,
       helpLinks,
+      defaultPerPage: Number(defaultPerPage),
     },
     render(createElement) {
       return createElement(NamespaceStorageApp);

ee/app/assets/javascripts/usage_quotas/storage/queries/namespace_storage.query.graphql

@@ -4,7 +4,8 @@ query getNamespaceStorageStatistics(
   $fullPath: ID!
   $withExcessStorageData: Boolean = false
   $searchTerm: String = ""
-  $first: Int!
+  $first: Int
+  $last: Int
   $after: String
   $before: String
 ) {
@@ -33,6 +34,7 @@ query getNamespaceStorageStatistics(
       includeSubgroups: true
       search: $searchTerm
       first: $first
+      last: $last
       after: $after
       before: $before
       sort: STORAGE

ee/app/views/groups/usage_quotas/index.html.haml

@@ -43,4 +43,4 @@
   .tab-pane#shared-runners-usage-quota-tab
     #js-shared-runner-usage-quota{ data: { namespace_id: @group.id } }
   .tab-pane#storage-quota-tab
-    #js-storage-counter-app{ data: { namespace_path: @group.full_path, purchase_storage_url: url_to_purchase_storage, buy_addon_target_attr: buy_addon_target_attr, is_temporary_storage_increase_visible: temporary_storage_increase_visible?(@group).to_s } }
+    #js-storage-counter-app{ data: { namespace_path: @group.full_path, purchase_storage_url: url_to_purchase_storage, buy_addon_target_attr: buy_addon_target_attr, is_temporary_storage_increase_visible: temporary_storage_increase_visible?(@group).to_s, default_per_page: page_size } }

ee/app/views/profiles/usage_quotas/index.html.haml

@@ -28,4 +28,4 @@
     = render "namespaces/pipelines_quota/list",
       locals: { namespace: @namespace, projects: @projects }
   .tab-pane#storage-quota-tab
-    #js-storage-counter-app{ data: { namespace_path: @namespace.full_path, purchase_storage_url: url_to_purchase_storage, buy_addon_target_attr: buy_addon_target_attr, is_temporary_storage_increase_visible: temporary_storage_increase_visible?(@namespace).to_s } }
+    #js-storage-counter-app{ data: { namespace_path: @namespace.full_path, purchase_storage_url: url_to_purchase_storage, buy_addon_target_attr: buy_addon_target_attr, is_temporary_storage_increase_visible: temporary_storage_increase_visible?(@namespace).to_s, default_per_page: page_size } }

ee/spec/features/groups/usage_quotas_spec.rb

@@ -53,7 +53,7 @@ RSpec.describe 'Groups > Usage Quotas' do
       end
 
       it 'renders a 404' do
-        visit_pipeline_quota_page
+        visit_usage_quotas_page
 
         expect(page).to have_gitlab_http_status(:not_found)
       end
@@ -66,7 +66,7 @@ RSpec.describe 'Groups > Usage Quotas' do
     include_examples 'linked in group settings dropdown'
 
     it 'shows correct group quota info' do
-      visit_pipeline_quota_page
+      visit_usage_quotas_page
 
       page.within('.pipeline-quota') do
         expect(page).to have_content("400 / Unlimited minutes")
@@ -82,7 +82,7 @@ RSpec.describe 'Groups > Usage Quotas' do
     include_examples 'linked in group settings dropdown'
 
     it 'shows correct group quota info' do
-      visit_pipeline_quota_page
+      visit_usage_quotas_page
 
       page.within('.pipeline-quota') do
         expect(page).to have_content("0%")
@@ -115,7 +115,7 @@ RSpec.describe 'Groups > Usage Quotas' do
     include_examples 'linked in group settings dropdown'
 
     it 'shows correct group quota info' do
-      visit_pipeline_quota_page
+      visit_usage_quotas_page
 
       page.within('.pipeline-quota') do
         expect(page).to have_content("300 / 500 minutes")
@@ -135,14 +135,14 @@ RSpec.describe 'Groups > Usage Quotas' do
       let(:gitlab_dot_com) { false }
 
       it "does not show 'Buy additional minutes' button" do
-        visit_pipeline_quota_page
+        visit_usage_quotas_page
 
         expect(page).not_to have_content('Buy additional minutes')
       end
     end
 
     it 'has correct tracking setup and shows correct group quota and projects info' do
-      visit_pipeline_quota_page
+      visit_usage_quotas_page
 
       page.within('.pipeline-quota') do
         expect(page).to have_content("1000 / 500 minutes")
@@ -168,7 +168,7 @@ RSpec.describe 'Groups > Usage Quotas' do
     let(:group) { create(:group, parent: root_ancestor) }
 
     it 'does not show subproject' do
-      visit_pipeline_quota_page
+      visit_usage_quotas_page
 
       expect(page).to have_gitlab_http_status(:not_found)
     end
@@ -179,7 +179,7 @@ RSpec.describe 'Groups > Usage Quotas' do
     let!(:subproject) { create(:project, namespace: subgroup, shared_runners_enabled: true) }
 
     it 'does show projects of subgroup' do
-      visit_pipeline_quota_page
+      visit_usage_quotas_page
 
       expect(page).to have_content(project.full_name)
       expect(page).to have_content(subproject.full_name)
@@ -188,13 +188,56 @@ RSpec.describe 'Groups > Usage Quotas' do
 
   context 'when purchasing CI minutes' do
     it 'points to GitLab CI minutes purchase flow' do
-      visit_pipeline_quota_page
+      visit_usage_quotas_page
 
       expect(page).to have_link('Buy additional minutes', href: buy_minutes_subscriptions_path(selected_group: group.id))
     end
   end
 
-  def visit_pipeline_quota_page
-    visit group_usage_quotas_path(group)
+  context 'pagination', :js do
+    let(:per_page) { 1 }
+    let!(:projects) { create_list(:project, 3, namespace: group) }
+
+    before do
+      allow(Kaminari.config).to receive(:default_per_page).and_return(per_page)
+
+      visit_usage_quotas_page('storage-quota-tab')
+    end
+
+    it 'paginates correctly to page 3 and back' do
+      expect(page).to have_selector('.js-project-link', count: per_page)
+      page1_el_text = page.find('.js-project-link').text
+      click_next_page
+
+      expect(page).to have_selector('.js-project-link', count: per_page)
+      page2_el_text = page.find('.js-project-link').text
+      click_next_page
+
+      expect(page).to have_selector('.js-project-link', count: per_page)
+      page3_el_text = page.find('.js-project-link').text
+      click_prev_page
+
+      expect(page3_el_text).not_to eql(page2_el_text)
+      expect(page.find('.js-project-link').text).to eql(page2_el_text)
+
+      click_prev_page
+
+      expect(page.find('.js-project-link').text).to eql(page1_el_text)
+      expect(page).to have_selector('.js-project-link', count: per_page)
+    end
+  end
+
+  def visit_usage_quotas_page(anchor = 'seats-quota-tab')
+    visit group_usage_quotas_path(group, anchor: anchor)
+  end
+
+  def click_next_page
+    page.find('[data-testid="nextButton"]').click
+    wait_for_requests
+  end
+
+  def click_prev_page
+    page.find('[data-testid="prevButton"]').click
+    wait_for_requests
   end
 end

ee/spec/frontend/usage_quotas/storage/components/namespace_storage_app_spec.js

 import { mount } from '@vue/test-utils';
-import { nextTick } from 'vue';
 import NamespaceStorageApp from 'ee/usage_quotas/storage/components/namespace_storage_app.vue';
 import CollapsibleProjectStorageDetail from 'ee/usage_quotas/storage/components/collapsible_project_storage_detail.vue';
 import ProjectList from 'ee/usage_quotas/storage/components/project_list.vue';
@@ -19,6 +18,7 @@ const TEST_LIMIT = 1000;
 
 describe('NamespaceStorageApp', () => {
   let wrapper;
+  let $apollo;
 
   const findTotalUsage = () => wrapper.find("[data-testid='total-usage']");
   const findPurchaseStorageLink = () => wrapper.find("[data-testid='purchase-storage-link']");
@@ -37,7 +37,7 @@ describe('NamespaceStorageApp', () => {
     additionalRepoStorageByNamespace = false,
     namespace = {},
   } = {}) => {
-    const $apollo = {
+    $apollo = {
       queries: {
         namespace: {
           loading,
@@ -66,7 +66,9 @@ describe('NamespaceStorageApp', () => {
   };
 
   beforeEach(() => {
-    createComponent();
+    createComponent({
+      namespace: namespaceData,
+    });
   });
 
   afterEach(() => {
@@ -74,97 +76,55 @@ describe('NamespaceStorageApp', () => {
   });
 
   it('renders the 2 projects', async () => {
-    // setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details
-    // eslint-disable-next-line no-restricted-syntax
-    wrapper.setData({
-      namespace: namespaceData,
-    });
-
-    await nextTick();
-
     expect(wrapper.findAllComponents(CollapsibleProjectStorageDetail)).toHaveLength(3);
   });
 
   describe('limit', () => {
     it('when limit is set it renders limit information', async () => {
-      // setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details
-      // eslint-disable-next-line no-restricted-syntax
-      wrapper.setData({
-        namespace: namespaceData,
-      });
-
-      await nextTick();
-
       expect(wrapper.text()).toContain(formatUsageSize(namespaceData.limit));
     });
 
     it('when limit is 0 it does not render limit information', async () => {
-      // setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details
-      // eslint-disable-next-line no-restricted-syntax
-      wrapper.setData({
+      createComponent({
         namespace: { ...namespaceData, limit: 0 },
       });
 
-      await nextTick();
-
       expect(wrapper.text()).not.toContain(formatUsageSize(0));
     });
   });
 
-  describe('with rootStorageStatistics information', () => {
-    it('renders total usage', async () => {
-      // setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details
-      // eslint-disable-next-line no-restricted-syntax
-      wrapper.setData({
-        namespace: withRootStorageStatistics,
-      });
-
-      await nextTick();
-
-      expect(findTotalUsage().text()).toContain(withRootStorageStatistics.totalUsage);
-    });
+  it('renders N/A for totalUsage when no rootStorageStatistics is provided', async () => {
+    expect(findTotalUsage().text()).toContain('N/A');
   });
 
-  describe('with additional_repo_storage_by_namespace feature', () => {
-    it('usage_graph component hidden is when feature is false', async () => {
-      // setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details
-      // eslint-disable-next-line no-restricted-syntax
-      wrapper.setData({
-        namespace: withRootStorageStatistics,
-      });
-
-      await nextTick();
-
-      expect(findUsageGraph().exists()).toBe(true);
-      expect(findUsageStatistics().exists()).toBe(false);
-      expect(findStorageInlineAlert().exists()).toBe(false);
-    });
-
-    it('usage_statistics component is rendered when feature is true', async () => {
+  describe('with rootStorageStatistics information', () => {
+    beforeEach(() => {
       createComponent({
-        additionalRepoStorageByNamespace: true,
         namespace: withRootStorageStatistics,
       });
+    });
 
-      await nextTick();
-
-      expect(findUsageStatistics().exists()).toBe(true);
-      expect(findUsageGraph().exists()).toBe(false);
-      expect(findStorageInlineAlert().exists()).toBe(true);
+    it('renders total usage', async () => {
+      expect(findTotalUsage().text()).toContain(withRootStorageStatistics.totalUsage);
     });
-  });
 
-  describe('without rootStorageStatistics information', () => {
-    it('renders N/A', async () => {
-      // setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details
-      // eslint-disable-next-line no-restricted-syntax
-      wrapper.setData({
-        namespace: namespaceData,
+    describe('with additional_repo_storage_by_namespace feature', () => {
+      it('usage_graph component hidden is when feature is false', async () => {
+        expect(findUsageGraph().exists()).toBe(true);
+        expect(findUsageStatistics().exists()).toBe(false);
+        expect(findStorageInlineAlert().exists()).toBe(false);
       });
 
-      await nextTick();
+      it('usage_statistics component is rendered when feature is true', async () => {
+        createComponent({
+          additionalRepoStorageByNamespace: true,
+          namespace: withRootStorageStatistics,
+        });
 
-      expect(findTotalUsage().text()).toContain('N/A');
+        expect(findUsageStatistics().exists()).toBe(true);
+        expect(findUsageGraph().exists()).toBe(false);
+        expect(findStorageInlineAlert().exists()).toBe(true);
+      });
     });
   });
 
@@ -207,10 +167,8 @@ describe('NamespaceStorageApp', () => {
 
     describe('when temporary storage increase is visible', () => {
       beforeEach(() => {
-        createComponent({ provide: { isTemporaryStorageIncreaseVisible: 'true' } });
-        // setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details
-        // eslint-disable-next-line no-restricted-syntax
-        wrapper.setData({
+        createComponent({
+          provide: { isTemporaryStorageIncreaseVisible: 'true' },
           namespace: {
             ...namespaceData,
             limit: TEST_LIMIT,
@@ -280,29 +238,61 @@ describe('NamespaceStorageApp', () => {
     });
   });
 
-  describe('renders projects table pagination component', () => {
-    const namespaceWithPageInfo = {
+  describe('projects table pagination component', () => {
+    const namespaceWithPageInfo = (
+      pageInfo = {
+        hasPreviousPage: false,
+        hasNextPage: true,
+      },
+    ) => ({
       namespace: {
         ...withRootStorageStatistics,
         projects: {
           ...withRootStorageStatistics.projects,
-          pageInfo: {
-            hasPreviousPage: false,
-            hasNextPage: true,
-          },
+          pageInfo,
         },
       },
-    };
+    });
     beforeEach(() => {
-      createComponent(namespaceWithPageInfo);
+      createComponent(namespaceWithPageInfo());
     });
 
-    it('with disabled "Prev" button', () => {
+    it('has "Prev" button disabled', () => {
       expect(findPrevButton().attributes().disabled).toBe('disabled');
     });
 
-    it('with enabled "Next" button', () => {
+    it('has "Next" button enabled', () => {
       expect(findNextButton().attributes().disabled).toBeUndefined();
     });
+
+    describe('apollo calls', () => {
+      beforeEach(() => {
+        createComponent(
+          namespaceWithPageInfo({
+            hasPreviousPage: true,
+            hasNextPage: true,
+          }),
+        );
+        $apollo.queries.namespace.fetchMore = jest.fn().mockResolvedValue();
+      });
+
+      it('contains correct `first` and `last` values when clicking "Prev" button', () => {
+        findPrevButton().trigger('click');
+        expect($apollo.queries.namespace.fetchMore).toHaveBeenCalledWith(
+          expect.objectContaining({
+            variables: expect.objectContaining({ first: undefined, last: expect.any(Number) }),
+          }),
+        );
+      });
+
+      it('contains `first` value when clicking "Next" button', () => {
+        findNextButton().trigger('click');
+        expect($apollo.queries.namespace.fetchMore).toHaveBeenCalledWith(
+          expect.objectContaining({
+            variables: expect.objectContaining({ first: expect.any(Number) }),
+          }),
+        );
+      });
+    });
   });
 });

ee/spec/frontend/usage_quotas/storage/mock_data.js

@@ -154,6 +154,7 @@ export const defaultProjectProvideValues = {
 };
 
 export const defaultNamespaceProvideValues = {
+  defaultPerPage: 20,
   namespacePath: 'h5bp',
   purchaseStorageUrl: '',
   buyAddonTargetAttr: '_blank',