From 4de6931b301d2b3962ea3bcff34fbfca2663d885 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro <gerardo@b310.de> Date: Tue, 6 Feb 2024 21:33:05 +0000 Subject: [PATCH] feature: Add keyset pagination to the list of package protection rules This MR includes: - Adding (keyset) pagination for the list of package protection rules - Integrating keyset pagination logic with GraphQL API - Add tests for pagination Changelog: added --- .../components/packages_protection_rules.vue | 73 +++++++--- ...et_packages_protection_rules.query.graphql | 15 +- .../packages_protection_rules_spec.js | 133 +++++++++++++++++- .../settings/project/settings/mock_data.js | 28 ++-- 4 files changed, 206 insertions(+), 43 deletions(-) diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue index d7219e3a91c3..2b85a787c933 100644 --- a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue +++ b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue @@ -1,5 +1,5 @@ <script> -import { GlButton, GlCard, GlTable, GlLoadingIcon } from '@gitlab/ui'; +import { GlButton, GlCard, GlTable, GlLoadingIcon, GlKeysetPagination } from '@gitlab/ui'; import packagesProtectionRuleQuery from '~/packages_and_registries/settings/project/graphql/queries/get_packages_protection_rules.query.graphql'; import SettingsBlock from '~/packages_and_registries/shared/components/settings_block.vue'; import PackagesProtectionRuleForm from '~/packages_and_registries/settings/project/components/packages_protection_rule_form.vue'; @@ -15,6 +15,7 @@ export default { GlTable, GlLoadingIcon, PackagesProtectionRuleForm, + GlKeysetPagination, }, inject: ['projectPath'], i18n: { @@ -28,11 +29,13 @@ export default { fetchSettingsError: false, packageProtectionRules: [], protectionRuleFormVisibility: false, + packageProtectionRulesQueryPayload: { nodes: [], pageInfo: {} }, + packageProtectionRulesQueryPaginationParams: { first: PAGINATION_DEFAULT_PER_PAGE }, }; }, computed: { tableItems() { - return this.packageProtectionRules.map((packagesProtectionRule) => { + return this.packageProtectionRulesQueryResult.map((packagesProtectionRule) => { return { col_1_package_name_pattern: packagesProtectionRule.packageNamePattern, col_2_package_type: packagesProtectionRule.packageType, @@ -41,44 +44,36 @@ export default { }; }); }, - totalItems() { - return this.packageProtectionRules.length; + packageProtectionRulesQueryPageInfo() { + return this.packageProtectionRulesQueryPayload.pageInfo; + }, + packageProtectionRulesQueryResult() { + return this.packageProtectionRulesQueryPayload.nodes; }, isLoadingPackageProtectionRules() { - return this.$apollo.queries.packageProtectionRules.loading; + return this.$apollo.queries.packageProtectionRulesQueryPayload.loading; }, isAddProtectionRuleButtonDisabled() { return this.protectionRuleFormVisibility; }, }, apollo: { - packageProtectionRules: { + packageProtectionRulesQueryPayload: { query: packagesProtectionRuleQuery, variables() { return { projectPath: this.projectPath, - first: PAGINATION_DEFAULT_PER_PAGE, + ...this.packageProtectionRulesQueryPaginationParams, }; }, - update: (data) => { - return data.project?.packagesProtectionRules?.nodes || []; + update(data) { + return data.project?.packagesProtectionRules ?? this.packageProtectionRulesQueryPayload; }, error(e) { this.fetchSettingsError = e; }, }, }, - fields: [ - { - key: 'col_1_package_name_pattern', - label: s__('PackageRegistry|Package name pattern'), - }, - { key: 'col_2_package_type', label: s__('PackageRegistry|Package type') }, - { - key: 'col_3_push_protected_up_to_access_level', - label: s__('PackageRegistry|Push protected up to access level'), - }, - ], methods: { showProtectionRuleForm() { this.protectionRuleFormVisibility = true; @@ -87,10 +82,34 @@ export default { this.protectionRuleFormVisibility = false; }, refetchProtectionRules() { - this.$apollo.queries.packageProtectionRules.refetch(); + this.$apollo.queries.packageProtectionRulesQueryPayload.refetch(); this.hideProtectionRuleForm(); }, + onNextPage() { + this.packageProtectionRulesQueryPaginationParams = { + after: this.packageProtectionRulesQueryPageInfo.endCursor, + first: PAGINATION_DEFAULT_PER_PAGE, + }; + }, + + onPrevPage() { + this.packageProtectionRulesQueryPaginationParams = { + before: this.packageProtectionRulesQueryPageInfo.startCursor, + last: PAGINATION_DEFAULT_PER_PAGE, + }; + }, }, + fields: [ + { + key: 'col_1_package_name_pattern', + label: s__('PackageRegistry|Package name pattern'), + }, + { key: 'col_2_package_type', label: s__('PackageRegistry|Package type') }, + { + key: 'col_3_push_protected_up_to_access_level', + label: s__('PackageRegistry|Push protected up to access level'), + }, + ], }; </script> @@ -135,7 +154,7 @@ export default { :fields="$options.fields" show-empty stacked="md" - class="mb-3" + class="gl-mb-5!" :aria-label="$options.i18n.settingBlockTitle" :busy="isLoadingPackageProtectionRules" > @@ -143,6 +162,16 @@ export default { <gl-loading-icon size="sm" class="gl-my-5" /> </template> </gl-table> + + <div class="gl-display-flex gl-justify-content-center gl-mb-3"> + <gl-keyset-pagination + v-bind="packageProtectionRulesQueryPageInfo" + :prev-text="__('Previous')" + :next-text="__('Next')" + @prev="onPrevPage" + @next="onNextPage" + /> + </div> </template> </gl-card> </template> diff --git a/app/assets/javascripts/packages_and_registries/settings/project/graphql/queries/get_packages_protection_rules.query.graphql b/app/assets/javascripts/packages_and_registries/settings/project/graphql/queries/get_packages_protection_rules.query.graphql index e0a072b93e4c..c90c00b4d1a3 100644 --- a/app/assets/javascripts/packages_and_registries/settings/project/graphql/queries/get_packages_protection_rules.query.graphql +++ b/app/assets/javascripts/packages_and_registries/settings/project/graphql/queries/get_packages_protection_rules.query.graphql @@ -1,13 +1,24 @@ -query getProjectPackageProtectionRules($projectPath: ID!, $first: Int) { +#import "~/graphql_shared/fragments/page_info.fragment.graphql" + +query getProjectPackageProtectionRules( + $projectPath: ID! + $first: Int + $last: Int + $after: String + $before: String +) { project(fullPath: $projectPath) { id - packagesProtectionRules(first: $first) { + packagesProtectionRules(first: $first, last: $last, after: $after, before: $before) { nodes { id packageNamePattern packageType pushProtectedUpToAccessLevel } + pageInfo { + ...PageInfo + } } } } diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js index 27b7ec44db32..507711a0764c 100644 --- a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js +++ b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js @@ -1,4 +1,4 @@ -import { GlLoadingIcon } from '@gitlab/ui'; +import { GlLoadingIcon, GlKeysetPagination } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import Vue from 'vue'; import VueApollo from 'vue-apollo'; @@ -60,7 +60,7 @@ describe('Packages protection rules project settings', () => { expect(findTable().exists()).toBe(true); }); - describe('table package protection rules', () => { + describe('table "package protection rules"', () => { it('renders table with packages protection rules', async () => { createComponent({ mountFn: mountExtended }); @@ -68,11 +68,13 @@ describe('Packages protection rules project settings', () => { expect(findTable().exists()).toBe(true); - packagesProtectionRulesData.forEach((protectionRule, i) => { - expect(findTableRow(i).text()).toContain(protectionRule.packageNamePattern); - expect(findTableRow(i).text()).toContain(protectionRule.packageType); - expect(findTableRow(i).text()).toContain(protectionRule.pushProtectedUpToAccessLevel); - }); + packagesProtectionRuleQueryPayload().data.project.packagesProtectionRules.nodes.forEach( + (protectionRule, i) => { + expect(findTableRow(i).text()).toContain(protectionRule.packageNamePattern); + expect(findTableRow(i).text()).toContain(protectionRule.packageType); + expect(findTableRow(i).text()).toContain(protectionRule.pushProtectedUpToAccessLevel); + }, + ); }); it('displays table in busy state and shows loading icon inside table', async () => { @@ -88,6 +90,123 @@ describe('Packages protection rules project settings', () => { expect(findTableLoadingIcon().exists()).toBe(false); expect(findTable().attributes('aria-busy')).toBe('false'); }); + + it('calls graphql api query', () => { + const resolver = jest.fn().mockResolvedValue(packagesProtectionRuleQueryPayload()); + createComponent({ resolver }); + + expect(resolver).toHaveBeenCalledWith( + expect.objectContaining({ projectPath: defaultProvidedValues.projectPath }), + ); + }); + + describe('table pagination', () => { + const findPagination = () => wrapper.findComponent(GlKeysetPagination); + + it('renders pagination', async () => { + createComponent({ mountFn: mountExtended }); + + await waitForPromises(); + + expect(findPagination().exists()).toBe(true); + expect(findPagination().props()).toMatchObject({ + endCursor: '10', + startCursor: '0', + hasNextPage: true, + hasPreviousPage: false, + }); + }); + + it('calls initial graphql api query with pagination information', () => { + const resolver = jest.fn().mockResolvedValue(packagesProtectionRuleQueryPayload()); + createComponent({ resolver }); + + expect(resolver).toHaveBeenCalledWith( + expect.objectContaining({ + projectPath: defaultProvidedValues.projectPath, + first: 10, + }), + ); + }); + + describe('when button "Previous" is clicked', () => { + const resolver = jest + .fn() + .mockResolvedValueOnce( + packagesProtectionRuleQueryPayload({ + nodes: packagesProtectionRulesData.slice(10), + pageInfo: { + hasNextPage: false, + hasPreviousPage: true, + startCursor: '10', + endCursor: '16', + }, + }), + ) + .mockResolvedValueOnce(packagesProtectionRuleQueryPayload()); + + const findPaginationButtonPrev = () => + extendedWrapper(findPagination()).findByRole('button', { name: 'Previous' }); + + beforeEach(async () => { + createComponent({ mountFn: mountExtended, resolver }); + + await waitForPromises(); + + findPaginationButtonPrev().trigger('click'); + }); + + it('sends a second graphql api query with new pagination params', () => { + expect(resolver).toHaveBeenCalledTimes(2); + expect(resolver).toHaveBeenLastCalledWith( + expect.objectContaining({ + before: '10', + last: 10, + projectPath: 'path', + }), + ); + }); + }); + + describe('when button "Next" is clicked', () => { + const resolver = jest + .fn() + .mockResolvedValueOnce(packagesProtectionRuleQueryPayload()) + .mockResolvedValueOnce( + packagesProtectionRuleQueryPayload({ + nodes: packagesProtectionRulesData.slice(10), + pageInfo: { + hasNextPage: true, + hasPreviousPage: false, + startCursor: '1', + endCursor: '10', + }, + }), + ); + + const findPaginationButtonNext = () => + extendedWrapper(findPagination()).findByRole('button', { name: 'Next' }); + + beforeEach(async () => { + createComponent({ mountFn: mountExtended, resolver }); + + await waitForPromises(); + + findPaginationButtonNext().trigger('click'); + }); + + it('sends a second graphql api query with new pagination params', () => { + expect(resolver).toHaveBeenCalledTimes(2); + expect(resolver).toHaveBeenLastCalledWith( + expect.objectContaining({ + after: '10', + first: 10, + projectPath: 'path', + }), + ); + }); + }); + }); }); it('does not initially render package protection form', async () => { diff --git a/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js b/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js index a8133c0ace6a..e49bf8c6131e 100644 --- a/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js +++ b/spec/frontend/packages_and_registries/settings/project/settings/mock_data.js @@ -81,18 +81,12 @@ export const packagesCleanupPolicyMutationPayload = ({ override, errors = [] } = }); export const packagesProtectionRulesData = [ - { - id: `gid://gitlab/Packages::Protection::Rule/14`, - packageNamePattern: `@flight/flight-maintainer-14-*`, + ...Array.from(Array(15)).map((_e, i) => ({ + id: `gid://gitlab/Packages::Protection::Rule/${i}`, + packageNamePattern: `@flight/flight-maintainer-${i}-*`, packageType: 'NPM', pushProtectedUpToAccessLevel: 'MAINTAINER', - }, - { - id: `gid://gitlab/Packages::Protection::Rule/15`, - packageNamePattern: `@flight/flight-maintainer-15-*`, - packageType: 'NPM', - pushProtectedUpToAccessLevel: 'MAINTAINER', - }, + })), { id: 'gid://gitlab/Packages::Protection::Rule/16', packageNamePattern: '@flight/flight-owner-16-*', @@ -101,12 +95,22 @@ export const packagesProtectionRulesData = [ }, ]; -export const packagesProtectionRuleQueryPayload = ({ override, errors = [] } = {}) => ({ +export const packagesProtectionRuleQueryPayload = ({ + errors = [], + nodes = packagesProtectionRulesData.slice(0, 10), + pageInfo = { + hasNextPage: true, + hasPreviousPage: false, + startCursor: '0', + endCursor: '10', + }, +} = {}) => ({ data: { project: { id: '1', packagesProtectionRules: { - nodes: override || packagesProtectionRulesData, + nodes, + pageInfo: { __typename: 'PageInfo', ...pageInfo }, }, errors, }, -- GitLab