Merge branch 'nfriend-add-graphql-releases-page-pagination' into 'master'

Step 4/4: Add pagination to GraphQL release page

See merge request gitlab-org/gitlab!41441

app/assets/javascripts/releases/components/app_index.vue

@@ -6,14 +6,10 @@ import {
   GlLink,
   GlButton,
 } from '@gitlab/ui';
-import {
-  getParameterByName,
-  historyPushState,
-  buildUrlWithCurrentLocation,
-} from '~/lib/utils/common_utils';
+import { getParameterByName } from '~/lib/utils/common_utils';
 import { __ } from '~/locale';
-import TablePagination from '~/vue_shared/components/pagination/table_pagination.vue';
 import ReleaseBlock from './release_block.vue';
+import ReleasesPagination from './releases_pagination.vue';
 
 export default {
   name: 'ReleasesApp',
@@ -21,7 +17,7 @@ export default {
     GlSkeletonLoading,
     GlEmptyState,
     ReleaseBlock,
-    TablePagination,
+    ReleasesPagination,
     GlLink,
     GlButton,
   },
@@ -33,7 +29,6 @@ export default {
       'isLoading',
       'releases',
       'hasError',
-      'pageInfo',
     ]),
     shouldRenderEmptyState() {
       return !this.releases.length && !this.hasError && !this.isLoading;
@@ -48,15 +43,23 @@ export default {
     },
   },
   created() {
-    this.fetchReleases({
-      page: getParameterByName('page'),
-    });
+    this.fetchReleases();
+
+    window.addEventListener('popstate', this.fetchReleases);
   },
   methods: {
-    ...mapActions('list', ['fetchReleases']),
-    onChangePage(page) {
-      historyPushState(buildUrlWithCurrentLocation(`?page=${page}`));
-      this.fetchReleases({ page });
+    ...mapActions('list', {
+      fetchReleasesStoreAction: 'fetchReleases',
+    }),
+    fetchReleases() {
+      this.fetchReleasesStoreAction({
+        // these two parameters are only used in "GraphQL mode"
+        before: getParameterByName('before'),
+        after: getParameterByName('after'),
+
+        // this parameter is only used when in "REST mode"
+        page: getParameterByName('page'),
+      });
     },
   },
 };
@@ -105,7 +108,7 @@ export default {
       />
     </div>
 
-    <table-pagination v-if="!isLoading" :change="onChangePage" :page-info="pageInfo" />
+    <releases-pagination v-if="!isLoading" />
   </div>
 </template>
 <style>

app/assets/javascripts/releases/components/releases_pagination_graphql.vue

@@ -13,14 +13,14 @@ export default {
     },
   },
   methods: {
-    ...mapActions('list', ['fetchReleasesGraphQl']),
+    ...mapActions('list', ['fetchReleases']),
     onPrev(before) {
       historyPushState(buildUrlWithCurrentLocation(`?before=${before}`));
-      this.fetchReleasesGraphQl({ before });
+      this.fetchReleases({ before });
     },
     onNext(after) {
       historyPushState(buildUrlWithCurrentLocation(`?after=${after}`));
-      this.fetchReleasesGraphQl({ after });
+      this.fetchReleases({ after });
     },
   },
 };

app/assets/javascripts/releases/components/releases_pagination_rest.vue

@@ -7,18 +7,18 @@ export default {
   name: 'ReleasesPaginationRest',
   components: { TablePagination },
   computed: {
-    ...mapState('list', ['pageInfo']),
+    ...mapState('list', ['restPageInfo']),
   },
   methods: {
-    ...mapActions('list', ['fetchReleasesRest']),
+    ...mapActions('list', ['fetchReleases']),
     onChangePage(page) {
       historyPushState(buildUrlWithCurrentLocation(`?page=${page}`));
-      this.fetchReleasesRest({ page });
+      this.fetchReleases({ page });
     },
   },
 };
 </script>
 
 <template>
-  <table-pagination :change="onChangePage" :page-info="pageInfo" />
+  <table-pagination :change="onChangePage" :page-info="restPageInfo" />
 </template>

app/assets/javascripts/releases/constants.js

@@ -10,3 +10,5 @@ export const ASSET_LINK_TYPE = Object.freeze({
 });
 
 export const DEFAULT_ASSET_LINK_TYPE = ASSET_LINK_TYPE.OTHER;
+
+export const PAGE_SIZE = 20;

app/assets/javascripts/releases/queries/all_releases.query.graphql

-query allReleases($fullPath: ID!) {
+query allReleases($fullPath: ID!, $first: Int, $last: Int, $before: String, $after: String) {
   project(fullPath: $fullPath) {
-    releases(first: 20) {
-      count
+    releases(first: $first, last: $last, before: $before, after: $after) {
       nodes {
         name
         tagName
@@ -64,6 +63,12 @@ query allReleases($fullPath: ID!) {
           }
         }
       }
+      pageInfo {
+        startCursor
+        hasPreviousPage
+        hasNextPage
+        endCursor
+      }
     }
   }
 }

app/assets/javascripts/releases/stores/getters.js

+/**
+ * @returns {Boolean} `true` if all the feature flags
+ * required to enable the GraphQL endpoint are enabled
+ */
+export const useGraphQLEndpoint = rootState => {
+  return Boolean(
+    rootState.featureFlags.graphqlReleaseData &&
+      rootState.featureFlags.graphqlReleasesPage &&
+      rootState.featureFlags.graphqlMilestoneStats,
+  );
+};

app/assets/javascripts/releases/stores/index.js

 import Vuex from 'vuex';
+import * as getters from './getters';
 
 export default ({ modules, featureFlags }) =>
   new Vuex.Store({
     modules,
     state: { featureFlags },
+    getters,
   });

app/assets/javascripts/releases/stores/modules/list/actions.js

@@ -9,54 +9,89 @@ import {
 } from '~/lib/utils/common_utils';
 import allReleasesQuery from '~/releases/queries/all_releases.query.graphql';
 import { gqClient, convertGraphQLResponse } from '../../../util';
+import { PAGE_SIZE } from '../../../constants';
 
 /**
- * Commits a mutation to update the state while the main endpoint is being requested.
+ * Gets a paginated list of releases from the server
+ *
+ * @param {Object} vuexParams
+ * @param {Object} actionParams
+ * @param {Number} [actionParams.page] The page number of results to fetch
+ * (this parameter is only used when fetching results from the REST API)
+ * @param {String} [actionParams.before] A GraphQL cursor. If provided,
+ * the items returned will proceed the provided cursor (this parameter is only
+ * used when fetching results from the GraphQL API).
+ * @param {String} [actionParams.after] A GraphQL cursor. If provided,
+ * the items returned will follow the provided cursor (this parameter is only
+ * used when fetching results from the GraphQL API).
  */
-export const requestReleases = ({ commit }) => commit(types.REQUEST_RELEASES);
+export const fetchReleases = ({ dispatch, rootGetters }, { page = 1, before, after }) => {
+  if (rootGetters.useGraphQLEndpoint) {
+    dispatch('fetchReleasesGraphQl', { before, after });
+  } else {
+    dispatch('fetchReleasesRest', { page });
+  }
+};
 
 /**
- * Fetches the main endpoint.
- * Will dispatch requestNamespace action before starting the request.
- * Will dispatch receiveNamespaceSuccess if the request is successful
- * Will dispatch receiveNamesapceError if the request returns an error
- *
- * @param {String} projectId
+ * Gets a paginated list of releases from the GraphQL endpoint
  */
-export const fetchReleases = ({ dispatch, rootState, state }, { page = '1' }) => {
-  dispatch('requestReleases');
+export const fetchReleasesGraphQl = (
+  { dispatch, commit, state },
+  { before = null, after = null },
+) => {
+  commit(types.REQUEST_RELEASES);
+
+  let paginationParams;
+  if (!before && !after) {
+    paginationParams = { first: PAGE_SIZE };
+  } else if (before && !after) {
+    paginationParams = { last: PAGE_SIZE, before };
+  } else if (!before && after) {
+    paginationParams = { first: PAGE_SIZE, after };
+  } else {
+    throw new Error(
+      'Both a `before` and an `after` parameter were provided to fetchReleasesGraphQl. These parameters cannot be used together.',
+    );
+  }
 
-  if (
-    rootState.featureFlags.graphqlReleaseData &&
-    rootState.featureFlags.graphqlReleasesPage &&
-    rootState.featureFlags.graphqlMilestoneStats
-  ) {
   gqClient
     .query({
       query: allReleasesQuery,
       variables: {
         fullPath: state.projectPath,
+        ...paginationParams,
       },
     })
     .then(response => {
-        dispatch('receiveReleasesSuccess', convertGraphQLResponse(response));
+      const { data, paginationInfo: graphQlPageInfo } = convertGraphQLResponse(response);
+
+      commit(types.RECEIVE_RELEASES_SUCCESS, {
+        data,
+        graphQlPageInfo,
+      });
     })
     .catch(() => dispatch('receiveReleasesError'));
-  } else {
-    api
-      .releases(state.projectId, { page })
-      .then(response => dispatch('receiveReleasesSuccess', response))
-      .catch(() => dispatch('receiveReleasesError'));
-  }
 };
 
-export const receiveReleasesSuccess = ({ commit }, { data, headers }) => {
-  const pageInfo = parseIntPagination(normalizeHeaders(headers));
+/**
+ * Gets a paginated list of releases from the REST endpoint
+ */
+export const fetchReleasesRest = ({ dispatch, commit, state }, { page }) => {
+  commit(types.REQUEST_RELEASES);
+
+  api
+    .releases(state.projectId, { page })
+    .then(({ data, headers }) => {
+      const restPageInfo = parseIntPagination(normalizeHeaders(headers));
       const camelCasedReleases = convertObjectPropsToCamelCase(data, { deep: true });
+
       commit(types.RECEIVE_RELEASES_SUCCESS, {
         data: camelCasedReleases,
-    pageInfo,
+        restPageInfo,
       });
+    })
+    .catch(() => dispatch('receiveReleasesError'));
 };
 
 export const receiveReleasesError = ({ commit }) => {

app/assets/javascripts/releases/stores/modules/list/mutations.js

@@ -17,11 +17,12 @@ export default {
    * @param {Object} state
    * @param {Object} resp
    */
-  [types.RECEIVE_RELEASES_SUCCESS](state, { data, pageInfo }) {
+  [types.RECEIVE_RELEASES_SUCCESS](state, { data, restPageInfo, graphQlPageInfo }) {
     state.hasError = false;
     state.isLoading = false;
     state.releases = data;
-    state.pageInfo = pageInfo;
+    state.restPageInfo = restPageInfo;
+    state.graphQlPageInfo = graphQlPageInfo;
   },
 
   /**
@@ -35,5 +36,7 @@ export default {
     state.isLoading = false;
     state.releases = [];
     state.hasError = true;
+    state.restPageInfo = {};
+    state.graphQlPageInfo = {};
   },
 };

app/assets/javascripts/releases/stores/modules/list/state.js

@@ -14,5 +14,6 @@ export default ({
   isLoading: false,
   hasError: false,
   releases: [],
-  pageInfo: {},
+  restPageInfo: {},
+  graphQlPageInfo: {},
 });

app/assets/javascripts/releases/util.js

@@ -126,5 +126,9 @@ export const convertGraphQLResponse = response => {
     ...convertMilestones(r),
   }));
 
-  return { data: releases };
+  const paginationInfo = {
+    ...response.data.project.releases.pageInfo,
+  };
+
+  return { data: releases, paginationInfo };
 };

app/controllers/projects/releases_controller.rb

@@ -13,7 +13,7 @@ class Projects::ReleasesController < Projects::ApplicationController
     push_frontend_feature_flag(:release_asset_link_type, project, default_enabled: true)
     push_frontend_feature_flag(:graphql_release_data, project, default_enabled: true)
     push_frontend_feature_flag(:graphql_milestone_stats, project, default_enabled: true)
-    push_frontend_feature_flag(:graphql_releases_page, project, default_enabled: false)
+    push_frontend_feature_flag(:graphql_releases_page, project, default_enabled: true)
   end
   before_action :authorize_update_release!, only: %i[edit update]
   before_action :authorize_create_release!, only: :new

spec/frontend/releases/__snapshots__/util_spec.js.snap

@@ -109,5 +109,11 @@ Object {
       "upcomingRelease": false,
     },
   ],
+  "paginationInfo": Object {
+    "endCursor": "eyJpZCI6IjMiLCJyZWxlYXNlZF9hdCI6IjIwMjAtMDctMDkgMjA6MTE6MzMuODA0OTYxMDAwIFVUQyJ9",
+    "hasNextPage": true,
+    "hasPreviousPage": false,
+    "startCursor": "eyJpZCI6IjQ0IiwicmVsZWFzZWRfYXQiOiIyMDMwLTAzLTE1IDA4OjAwOjAwLjAwMDAwMDAwMCBVVEMifQ",
+  },
 }
 `;

spec/frontend/releases/components/app_index_spec.js

@@ -13,7 +13,14 @@ import {
   releases,
 } from '../mock_data';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
-import TablePagination from '~/vue_shared/components/pagination/table_pagination.vue';
+import ReleasesPagination from '~/releases/components/releases_pagination.vue';
+
+jest.mock('~/lib/utils/common_utils', () => ({
+  ...jest.requireActual('~/lib/utils/common_utils'),
+  getParameterByName: jest.fn().mockImplementation(paramName => {
+    return `${paramName}_param_value`;
+  }),
+}));
 
 const localVue = createLocalVue();
 localVue.use(Vuex);
@@ -22,7 +29,7 @@ describe('Releases App ', () => {
   let wrapper;
   let fetchReleaseSpy;
 
-  const releasesPagination = rge(21).map(index => ({
+  const paginatedReleases = rge(21).map(index => ({
     ...convertObjectPropsToCamelCase(release, { deep: true }),
     tagName: `${index}.00`,
   }));
@@ -70,9 +77,13 @@ describe('Releases App ', () => {
       createComponent();
     });
 
-    it('calls fetchRelease with the page parameter', () => {
+    it('calls fetchRelease with the page, before, and after parameters', () => {
       expect(fetchReleaseSpy).toHaveBeenCalledTimes(1);
-      expect(fetchReleaseSpy).toHaveBeenCalledWith(expect.anything(), { page: null });
+      expect(fetchReleaseSpy).toHaveBeenCalledWith(expect.anything(), {
+        page: 'page_param_value',
+        before: 'before_param_value',
+        after: 'after_param_value',
+      });
     });
   });
 
@@ -91,7 +102,7 @@ describe('Releases App ', () => {
       expect(wrapper.contains('.js-loading')).toBe(true);
       expect(wrapper.contains('.js-empty-state')).toBe(false);
       expect(wrapper.contains('.js-success-state')).toBe(false);
-      expect(wrapper.contains(TablePagination)).toBe(false);
+      expect(wrapper.contains(ReleasesPagination)).toBe(false);
     });
   });
 
@@ -108,7 +119,7 @@ describe('Releases App ', () => {
       expect(wrapper.contains('.js-loading')).toBe(false);
       expect(wrapper.contains('.js-empty-state')).toBe(false);
       expect(wrapper.contains('.js-success-state')).toBe(true);
-      expect(wrapper.contains(TablePagination)).toBe(true);
+      expect(wrapper.contains(ReleasesPagination)).toBe(true);
     });
   });
 
@@ -116,7 +127,7 @@ describe('Releases App ', () => {
     beforeEach(() => {
       jest
         .spyOn(api, 'releases')
-        .mockResolvedValue({ data: releasesPagination, headers: pageInfoHeadersWithPagination });
+        .mockResolvedValue({ data: paginatedReleases, headers: pageInfoHeadersWithPagination });
 
       createComponent();
     });
@@ -125,7 +136,7 @@ describe('Releases App ', () => {
       expect(wrapper.contains('.js-loading')).toBe(false);
       expect(wrapper.contains('.js-empty-state')).toBe(false);
       expect(wrapper.contains('.js-success-state')).toBe(true);
-      expect(wrapper.contains(TablePagination)).toBe(true);
+      expect(wrapper.contains(ReleasesPagination)).toBe(true);
     });
   });
 
@@ -154,7 +165,7 @@ describe('Releases App ', () => {
       const newReleasePath = 'path/to/new/release';
 
       beforeEach(() => {
-        createComponent({ ...defaultInitialState, newReleasePath });
+        createComponent({ newReleasePath });
       });
 
       it('renders the "New release" button', () => {
@@ -174,4 +185,27 @@ describe('Releases App ', () => {
       });
     });
   });
+
+  describe('when the back button is pressed', () => {
+    beforeEach(() => {
+      jest
+        .spyOn(api, 'releases')
+        .mockResolvedValue({ data: releases, headers: pageInfoHeadersWithoutPagination });
+
+      createComponent();
+
+      fetchReleaseSpy.mockClear();
+
+      window.dispatchEvent(new PopStateEvent('popstate'));
+    });
+
+    it('calls fetchRelease with the page parameter', () => {
+      expect(fetchReleaseSpy).toHaveBeenCalledTimes(1);
+      expect(fetchReleaseSpy).toHaveBeenCalledWith(expect.anything(), {
+        page: 'page_param_value',
+        before: 'before_param_value',
+        after: 'after_param_value',
+      });
+    });
+  });
 });

spec/frontend/releases/components/releases_pagination_graphql_spec.js

@@ -29,7 +29,7 @@ describe('~/releases/components/releases_pagination_graphql.vue', () => {
 
     listModule.state.graphQlPageInfo = pageInfo;
 
-    listModule.actions.fetchReleasesGraphQl = jest.fn();
+    listModule.actions.fetchReleases = jest.fn();
 
     wrapper = mount(ReleasesPaginationGraphql, {
       store: createStore({
@@ -141,8 +141,8 @@ describe('~/releases/components/releases_pagination_graphql.vue', () => {
         findNextButton().trigger('click');
       });
 
-      it('calls fetchReleasesGraphQl with the correct after cursor', () => {
-        expect(listModule.actions.fetchReleasesGraphQl.mock.calls).toEqual([
+      it('calls fetchReleases with the correct after cursor', () => {
+        expect(listModule.actions.fetchReleases.mock.calls).toEqual([
           [expect.anything(), { after: cursors.endCursor }],
         ]);
       });
@@ -159,8 +159,8 @@ describe('~/releases/components/releases_pagination_graphql.vue', () => {
         findPrevButton().trigger('click');
       });
 
-      it('calls fetchReleasesGraphQl with the correct before cursor', () => {
-        expect(listModule.actions.fetchReleasesGraphQl.mock.calls).toEqual([
+      it('calls fetchReleases with the correct before cursor', () => {
+        expect(listModule.actions.fetchReleases.mock.calls).toEqual([
           [expect.anything(), { before: cursors.startCursor }],
         ]);
       });

spec/frontend/releases/components/releases_pagination_rest_spec.js

@@ -20,9 +20,9 @@ describe('~/releases/components/releases_pagination_rest.vue', () => {
   const createComponent = pageInfo => {
     listModule = createListModule({ projectId });
 
-    listModule.state.pageInfo = pageInfo;
+    listModule.state.restPageInfo = pageInfo;
 
-    listModule.actions.fetchReleasesRest = jest.fn();
+    listModule.actions.fetchReleases = jest.fn();
 
     wrapper = mount(ReleasesPaginationRest, {
       store: createStore({
@@ -57,8 +57,8 @@ describe('~/releases/components/releases_pagination_rest.vue', () => {
       findGlPagination().vm.$emit('input', newPage);
     });
 
-    it('calls fetchReleasesRest with the correct page', () => {
-      expect(listModule.actions.fetchReleasesRest.mock.calls).toEqual([
+    it('calls fetchReleases with the correct page', () => {
+      expect(listModule.actions.fetchReleases.mock.calls).toEqual([
         [expect.anything(), { page: newPage }],
       ]);
     });

spec/frontend/releases/mock_data.js

@@ -346,6 +346,14 @@ export const graphqlReleasesResponse = {
             },
           },
         ],
+        pageInfo: {
+          startCursor:
+            'eyJpZCI6IjQ0IiwicmVsZWFzZWRfYXQiOiIyMDMwLTAzLTE1IDA4OjAwOjAwLjAwMDAwMDAwMCBVVEMifQ',
+          hasPreviousPage: false,
+          hasNextPage: true,
+          endCursor:
+            'eyJpZCI6IjMiLCJyZWxlYXNlZF9hdCI6IjIwMjAtMDctMDkgMjA6MTE6MzMuODA0OTYxMDAwIFVUQyJ9',
+        },
       },
     },
   },

spec/frontend/releases/stores/getters_spec.js

+import * as getters from '~/releases/stores/getters';
+
+describe('~/releases/stores/getters.js', () => {
+  it.each`
+    graphqlReleaseData | graphqlReleasesPage | graphqlMilestoneStats | result
+    ${false}           | ${false}            | ${false}              | ${false}
+    ${false}           | ${false}            | ${true}               | ${false}
+    ${false}           | ${true}             | ${false}              | ${false}
+    ${false}           | ${true}             | ${true}               | ${false}
+    ${true}            | ${false}            | ${false}              | ${false}
+    ${true}            | ${false}            | ${true}               | ${false}
+    ${true}            | ${true}             | ${false}              | ${false}
+    ${true}            | ${true}             | ${true}               | ${true}
+  `(
+    'returns $result with feature flag values graphqlReleaseData=$graphqlReleaseData, graphqlReleasesPage=$graphqlReleasesPage, and graphqlMilestoneStats=$graphqlMilestoneStats',
+    ({ result: expectedResult, ...featureFlags }) => {
+      const actualResult = getters.useGraphQLEndpoint({ featureFlags });
+
+      expect(actualResult).toBe(expectedResult);
+    },
+  );
+});

spec/frontend/releases/stores/modules/list/mutations_spec.js

@@ -2,15 +2,22 @@ import createState from '~/releases/stores/modules/list/state';
 import mutations from '~/releases/stores/modules/list/mutations';
 import * as types from '~/releases/stores/modules/list/mutation_types';
 import { parseIntPagination } from '~/lib/utils/common_utils';
-import { pageInfoHeadersWithoutPagination, releases } from '../../../mock_data';
+import {
+  pageInfoHeadersWithoutPagination,
+  releases,
+  graphqlReleasesResponse,
+} from '../../../mock_data';
+import { convertGraphQLResponse } from '~/releases/util';
 
 describe('Releases Store Mutations', () => {
   let stateCopy;
-  let pageInfo;
+  let restPageInfo;
+  let graphQlPageInfo;
 
   beforeEach(() => {
     stateCopy = createState({});
-    pageInfo = parseIntPagination(pageInfoHeadersWithoutPagination);
+    restPageInfo = parseIntPagination(pageInfoHeadersWithoutPagination);
+    graphQlPageInfo = convertGraphQLResponse(graphqlReleasesResponse).paginationInfo;
   });
 
   describe('REQUEST_RELEASES', () => {
@@ -23,7 +30,11 @@ describe('Releases Store Mutations', () => {
 
   describe('RECEIVE_RELEASES_SUCCESS', () => {
     beforeEach(() => {
-      mutations[types.RECEIVE_RELEASES_SUCCESS](stateCopy, { pageInfo, data: releases });
+      mutations[types.RECEIVE_RELEASES_SUCCESS](stateCopy, {
+        restPageInfo,
+        graphQlPageInfo,
+        data: releases,
+      });
     });
 
     it('sets is loading to false', () => {
@@ -38,18 +49,29 @@ describe('Releases Store Mutations', () => {
       expect(stateCopy.releases).toEqual(releases);
     });
 
-    it('sets pageInfo', () => {
-      expect(stateCopy.pageInfo).toEqual(pageInfo);
+    it('sets restPageInfo', () => {
+      expect(stateCopy.restPageInfo).toEqual(restPageInfo);
+    });
+
+    it('sets graphQlPageInfo', () => {
+      expect(stateCopy.graphQlPageInfo).toEqual(graphQlPageInfo);
     });
   });
 
   describe('RECEIVE_RELEASES_ERROR', () => {
     it('resets data', () => {
+      mutations[types.RECEIVE_RELEASES_SUCCESS](stateCopy, {
+        restPageInfo,
+        graphQlPageInfo,
+        data: releases,
+      });
+
       mutations[types.RECEIVE_RELEASES_ERROR](stateCopy);
 
       expect(stateCopy.isLoading).toEqual(false);
       expect(stateCopy.releases).toEqual([]);
-      expect(stateCopy.pageInfo).toEqual({});
+      expect(stateCopy.restPageInfo).toEqual({});
+      expect(stateCopy.graphQlPageInfo).toEqual({});
     });
   });
 });