Commit 0e7a4d07 authored by Nathan Friend's avatar Nathan Friend Committed by Phil Hughes

Fix Releases page pagination after sort change

parent c87f2d89
<script>
import { GlButton } from '@gitlab/ui';
import createFlash from '~/flash';
import { getParameterByName } from '~/lib/utils/common_utils';
import { historyPushState, getParameterByName } from '~/lib/utils/common_utils';
import { scrollUp } from '~/lib/utils/scroll_utils';
import { setUrlParams } from '~/lib/utils/url_utility';
import { __ } from '~/locale';
import { PAGE_SIZE, RELEASED_AT_DESC } from '~/releases/constants';
import { PAGE_SIZE, DEFAULT_SORT } from '~/releases/constants';
import allReleasesQuery from '~/releases/graphql/queries/all_releases.query.graphql';
import { convertAllReleasesGraphQLResponse } from '~/releases/util';
import ReleaseBlock from './release_block.vue';
......@@ -58,7 +59,7 @@ export default {
before: getParameterByName('before'),
after: getParameterByName('after'),
},
sort: RELEASED_AT_DESC,
sort: DEFAULT_SORT,
};
},
computed: {
......@@ -145,6 +146,29 @@ export default {
// scroll to the top of the page every time a pagination button is pressed.
scrollUp();
},
onSortChanged(newSort) {
if (this.sort === newSort) {
return;
}
// Remove the "before" and "after" query parameters from the URL,
// effectively placing the user back on page 1 of the results.
// This prevents the frontend from requesting the results sorted
// by one field (e.g. `released_at`) while using a pagination cursor
// intended for a different field (e.g.) `created_at`).
// For more details, see the MR that introduced this change:
// https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63434
historyPushState(
setUrlParams({
before: null,
after: null,
}),
);
this.updateQueryParamsFromUrl();
this.sort = newSort;
},
},
i18n: {
newRelease: __('New release'),
......@@ -155,7 +179,7 @@ export default {
<template>
<div class="flex flex-column mt-2">
<div class="gl-align-self-end gl-mb-3">
<releases-sort-apollo-client v-model="sort" class="gl-mr-2" />
<releases-sort-apollo-client :value="sort" class="gl-mr-2" @input="onSortChanged" />
<gl-button
v-if="newReleasePath"
......
......@@ -47,3 +47,5 @@ export const SORT_MAP = {
[DESCENDING_ORDER]: CREATED_DESC,
},
};
export const DEFAULT_SORT = RELEASED_AT_DESC;
......@@ -4,13 +4,14 @@ import VueApollo from 'vue-apollo';
import createMockApollo from 'helpers/mock_apollo_helper';
import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
import createFlash from '~/flash';
import { historyPushState } from '~/lib/utils/common_utils';
import ReleasesIndexApolloClientApp from '~/releases/components/app_index_apollo_client.vue';
import ReleaseBlock from '~/releases/components/release_block.vue';
import ReleaseSkeletonLoader from '~/releases/components/release_skeleton_loader.vue';
import ReleasesEmptyState from '~/releases/components/releases_empty_state.vue';
import ReleasesPaginationApolloClient from '~/releases/components/releases_pagination_apollo_client.vue';
import ReleasesSortApolloClient from '~/releases/components/releases_sort_apollo_client.vue';
import { PAGE_SIZE, RELEASED_AT_DESC, CREATED_ASC } from '~/releases/constants';
import { PAGE_SIZE, CREATED_ASC, DEFAULT_SORT } from '~/releases/constants';
import allReleasesQuery from '~/releases/graphql/queries/all_releases.query.graphql';
Vue.use(VueApollo);
......@@ -23,6 +24,7 @@ jest.mock('~/lib/utils/common_utils', () => ({
getParameterByName: jest
.fn()
.mockImplementation((parameterName) => mockQueryParams[parameterName]),
historyPushState: jest.fn(),
}));
describe('app_index_apollo_client.vue', () => {
......@@ -225,7 +227,7 @@ describe('app_index_apollo_client.vue', () => {
expect(allReleasesQueryMock).toHaveBeenCalledWith({
first: PAGE_SIZE,
fullPath: projectPath,
sort: RELEASED_AT_DESC,
sort: DEFAULT_SORT,
});
});
});
......@@ -241,7 +243,7 @@ describe('app_index_apollo_client.vue', () => {
before,
last: PAGE_SIZE,
fullPath: projectPath,
sort: RELEASED_AT_DESC,
sort: DEFAULT_SORT,
});
});
});
......@@ -257,7 +259,7 @@ describe('app_index_apollo_client.vue', () => {
after,
first: PAGE_SIZE,
fullPath: projectPath,
sort: RELEASED_AT_DESC,
sort: DEFAULT_SORT,
});
});
});
......@@ -273,7 +275,7 @@ describe('app_index_apollo_client.vue', () => {
after,
first: PAGE_SIZE,
fullPath: projectPath,
sort: RELEASED_AT_DESC,
sort: DEFAULT_SORT,
});
});
});
......@@ -320,21 +322,82 @@ describe('app_index_apollo_client.vue', () => {
createComponent();
});
it(`sorts by ${RELEASED_AT_DESC} by default`, () => {
it(`sorts by ${DEFAULT_SORT} by default`, () => {
expect(allReleasesQueryMock.mock.calls).toEqual([
[expect.objectContaining({ sort: RELEASED_AT_DESC })],
[expect.objectContaining({ sort: DEFAULT_SORT })],
]);
});
it('requeries the GraphQL endpoint when the sort is changed', async () => {
it('requeries the GraphQL endpoint and updates the URL when the sort is changed', async () => {
findSort().vm.$emit('input', CREATED_ASC);
await wrapper.vm.$nextTick();
expect(allReleasesQueryMock.mock.calls).toEqual([
[expect.objectContaining({ sort: RELEASED_AT_DESC })],
[expect.objectContaining({ sort: DEFAULT_SORT })],
[expect.objectContaining({ sort: CREATED_ASC })],
]);
// URL manipulation is tested in more detail in the `describe` block below
expect(historyPushState).toHaveBeenCalled();
});
it('does not requery the GraphQL endpoint or update the URL if the sort is updated to the same value', async () => {
findSort().vm.$emit('input', DEFAULT_SORT);
await wrapper.vm.$nextTick();
expect(allReleasesQueryMock.mock.calls).toEqual([
[expect.objectContaining({ sort: DEFAULT_SORT })],
]);
expect(historyPushState).not.toHaveBeenCalled();
});
});
describe('sorting + pagination interaction', () => {
const nonPaginationQueryParam = 'nonPaginationQueryParam';
beforeEach(() => {
historyPushState.mockImplementation((newUrl) => {
mockQueryParams = Object.fromEntries(new URL(newUrl).searchParams);
});
});
describe.each`
queryParamsBefore | paramName | paramInitialValue
${{ before, nonPaginationQueryParam }} | ${'before'} | ${before}
${{ after, nonPaginationQueryParam }} | ${'after'} | ${after}
`(
'when the URL contains a "$paramName" pagination cursor',
({ queryParamsBefore, paramName, paramInitialValue }) => {
beforeEach(async () => {
mockQueryParams = queryParamsBefore;
createComponent();
findSort().vm.$emit('input', CREATED_ASC);
await wrapper.vm.$nextTick();
});
it(`resets the page's "${paramName}" pagination cursor when the sort is changed`, () => {
const firstRequestVariables = allReleasesQueryMock.mock.calls[0][0];
const secondRequestVariables = allReleasesQueryMock.mock.calls[1][0];
expect(firstRequestVariables[paramName]).toBe(paramInitialValue);
expect(secondRequestVariables[paramName]).toBeUndefined();
});
it(`updates the URL to not include the "${paramName}" URL query parameter`, () => {
expect(historyPushState).toHaveBeenCalledTimes(1);
const updatedUrlQueryParams = Object.fromEntries(
new URL(historyPushState.mock.calls[0][0]).searchParams,
);
expect(updatedUrlQueryParams[paramName]).toBeUndefined();
});
},
);
});
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment