Commit a6f5bc54 authored by Mark Florian's avatar Mark Florian Committed by Kushal Pandya

Work around missing pagination headers

The backend currently is not sending the expected pagination headers
necessary for the pagination UI in the Dependency List[0].

This change works around that. It:

 - fetches the full, un-paginated list of dependencies on load, to find
   out the total number of them (this information is sufficient to build
   the pagination UI);
 - persists/updates the current page number in the store, rather than
   based on the (missing) response headers.

This should be reverted once the fix[1] for the backend is merged,
though there shouldn't be any problem with them coexisting; the frontend
will then simply be making one unnecessary request.

[0]: https://gitlab.com/gitlab-org/gitlab-ee/issues/10075
[1]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/14115
parent 9cfb979f
......@@ -55,10 +55,12 @@ export default {
},
created() {
this.setDependenciesEndpoint(this.endpoint);
this.fetchDependencies();
this.fetchDependenciesPagination()
.then(() => this.fetchDependencies())
.catch(() => {});
},
methods: {
...mapActions(['setDependenciesEndpoint', 'fetchDependencies']),
...mapActions(['setDependenciesEndpoint', 'fetchDependenciesPagination', 'fetchDependencies']),
fetchPage(page) {
this.fetchDependencies({ page });
},
......
......@@ -9,7 +9,43 @@ import { __ } from '~/locale';
export const setDependenciesEndpoint = ({ commit }, endpoint) =>
commit(types.SET_DEPENDENCIES_ENDPOINT, endpoint);
export const requestDependencies = ({ commit }) => commit(types.REQUEST_DEPENDENCIES);
export const requestDependenciesPagination = ({ commit }) =>
commit(types.REQUEST_DEPENDENCIES_PAGINATION);
export const receiveDependenciesPaginationSuccess = ({ commit }, payload) =>
commit(types.RECEIVE_DEPENDENCIES_PAGINATION_SUCCESS, payload);
export const receiveDependenciesPaginationError = ({ commit }) =>
commit(types.RECEIVE_DEPENDENCIES_PAGINATION_ERROR);
export const fetchDependenciesPagination = ({ state, dispatch }) => {
const onError = error => {
dispatch('receiveDependenciesPaginationError', error);
createFlash(FETCH_ERROR_MESSAGE);
throw error;
};
dispatch('requestDependenciesPagination');
if (!state.endpoint) {
return Promise.reject(new Error('endpoint_missing')).catch(onError);
}
return axios
.get(state.endpoint)
.then(response => {
if (isValidResponse(response)) {
const total = response.data.dependencies.length;
dispatch('receiveDependenciesPaginationSuccess', total);
} else {
throw new Error(__('Invalid server response'));
}
})
.catch(onError);
};
export const requestDependencies = ({ commit }, payload) =>
commit(types.REQUEST_DEPENDENCIES, payload);
export const receiveDependenciesSuccess = ({ commit }, { headers, data }) => {
const normalizedHeaders = normalizeHeaders(headers);
......@@ -27,14 +63,15 @@ export const fetchDependencies = ({ state, dispatch }, params = {}) => {
return;
}
dispatch('requestDependencies');
const page = params.page || state.pageInfo.page || 1;
dispatch('requestDependencies', { page });
axios
.get(state.endpoint, {
params: {
sort_by: state.sortField,
sort: state.sortOrder,
page: state.pageInfo.page || 1,
page,
...params,
},
})
......
......@@ -21,3 +21,5 @@ export const REPORT_STATUS = {
export const FETCH_ERROR_MESSAGE = __(
'Error fetching the dependency list. Please check your network connection and try again.',
);
export const DEPENDENCIES_PER_PAGE = 20;
export const SET_DEPENDENCIES_ENDPOINT = 'SET_DEPENDENCIES_ENDPOINT';
export const REQUEST_DEPENDENCIES_PAGINATION = 'REQUEST_DEPENDENCIES_PAGINATION';
export const RECEIVE_DEPENDENCIES_PAGINATION_SUCCESS = 'RECEIVE_DEPENDENCIES_PAGINATION_SUCCESS';
export const RECEIVE_DEPENDENCIES_PAGINATION_ERROR = 'RECEIVE_DEPENDENCIES_PAGINATION_ERROR';
export const REQUEST_DEPENDENCIES = 'REQUEST_DEPENDENCIES';
export const RECEIVE_DEPENDENCIES_SUCCESS = 'RECEIVE_DEPENDENCIES_SUCCESS';
export const RECEIVE_DEPENDENCIES_ERROR = 'RECEIVE_DEPENDENCIES_ERROR';
......
......@@ -5,13 +5,30 @@ export default {
[types.SET_DEPENDENCIES_ENDPOINT](state, payload) {
state.endpoint = payload;
},
[types.REQUEST_DEPENDENCIES](state) {
[types.REQUEST_DEPENDENCIES_PAGINATION](state) {
state.isLoading = true;
},
[types.RECEIVE_DEPENDENCIES_PAGINATION_SUCCESS](state, total) {
state.pageInfo.total = total;
},
[types.RECEIVE_DEPENDENCIES_PAGINATION_ERROR](state) {
state.isLoading = false;
state.errorLoading = true;
state.dependencies = [];
state.pageInfo = {};
state.reportInfo = {
status: REPORT_STATUS.ok,
jobPath: '',
};
state.initialized = true;
},
[types.REQUEST_DEPENDENCIES](state, { page }) {
state.isLoading = true;
state.errorLoading = false;
state.pageInfo.page = page;
},
[types.RECEIVE_DEPENDENCIES_SUCCESS](state, { dependencies, reportInfo, pageInfo }) {
[types.RECEIVE_DEPENDENCIES_SUCCESS](state, { dependencies, reportInfo }) {
state.dependencies = dependencies;
state.pageInfo = pageInfo;
state.isLoading = false;
state.errorLoading = false;
state.reportInfo.status = reportInfo.status;
......
import { REPORT_STATUS, SORT_FIELDS, SORT_ORDER } from './constants';
import { REPORT_STATUS, SORT_FIELDS, SORT_ORDER, DEPENDENCIES_PER_PAGE } from './constants';
export default () => ({
endpoint: '',
......@@ -6,7 +6,11 @@ export default () => ({
isLoading: false,
errorLoading: false,
dependencies: [],
pageInfo: {},
pageInfo: {
page: 1,
perPage: DEPENDENCIES_PER_PAGE,
total: 0,
},
reportInfo: {
status: REPORT_STATUS.ok,
jobPath: '',
......
......@@ -22,7 +22,7 @@ describe('DependenciesApp component', () => {
const localVue = createLocalVue();
store = createStore();
jest.spyOn(store, 'dispatch').mockImplementation();
jest.spyOn(store, 'dispatch').mockImplementation(() => Promise.resolve());
wrapper = shallowMount(DependenciesApp, {
localVue,
......@@ -52,6 +52,7 @@ describe('DependenciesApp component', () => {
it('dispatches the correct initial actions', () => {
expect(store.dispatch.mock.calls).toEqual([
['setDependenciesEndpoint', basicAppProps.endpoint],
['fetchDependenciesPagination'],
['fetchDependencies'],
]);
});
......
......@@ -52,15 +52,108 @@ describe('Dependencies actions', () => {
));
});
describe('requestDependenciesPagination', () => {
it('commits the REQUEST_DEPENDENCIES_PAGINATION mutation', () =>
testAction(
actions.requestDependenciesPagination,
undefined,
getInitialState(),
[
{
type: types.REQUEST_DEPENDENCIES_PAGINATION,
},
],
[],
));
});
describe('receiveDependenciesPaginationSuccess', () => {
const total = 123;
it('commits the RECEIVE_DEPENDENCIES_PAGINATION_SUCCESS mutation', () =>
testAction(
actions.receiveDependenciesPaginationSuccess,
total,
getInitialState(),
[
{
type: types.RECEIVE_DEPENDENCIES_PAGINATION_SUCCESS,
payload: total,
},
],
[],
));
});
describe('receiveDependenciesPaginationError', () => {
it('commits the RECEIVE_DEPENDENCIES_PAGINATION_ERROR mutation', () =>
testAction(
actions.receiveDependenciesPaginationError,
undefined,
getInitialState(),
[
{
type: types.RECEIVE_DEPENDENCIES_PAGINATION_ERROR,
},
],
[],
));
});
describe('fetchDependenciesPagination', () => {
let mock;
let state;
beforeEach(() => {
state = getInitialState();
state.endpoint = `${TEST_HOST}/dependencies`;
mock = new MockAdapter(axios);
});
afterEach(() => {
mock.restore();
});
describe('on success', () => {
beforeEach(() => {
mock.onGet(state.endpoint).replyOnce(200, mockDependenciesResponse);
});
it('dispatches the correct actions', () =>
testAction(
actions.fetchDependenciesPagination,
undefined,
state,
[],
[
{
type: 'requestDependenciesPagination',
},
{
type: 'receiveDependenciesPaginationSuccess',
payload: mockDependenciesResponse.dependencies.length,
},
],
));
});
/**
* Tests for error conditions are in
* `ee/spec/javascripts/dependencies/store/actions_spec.js`. They cannot be
* tested here due to https://gitlab.com/gitlab-org/gitlab-ce/issues/63225.
*/
});
describe('requestDependencies', () => {
const page = 4;
it('commits the REQUEST_DEPENDENCIES mutation', () =>
testAction(
actions.requestDependencies,
undefined,
{ page },
getInitialState(),
[
{
type: types.REQUEST_DEPENDENCIES,
payload: { page },
},
],
[],
......@@ -134,10 +227,11 @@ describe('Dependencies actions', () => {
describe('on success', () => {
describe('given no params', () => {
let paramsDefault;
beforeEach(() => {
state.pageInfo = { ...pageInfo };
const paramsDefault = {
paramsDefault = {
sort_by: state.sortField,
sort: state.sortOrder,
page: state.pageInfo.page,
......@@ -157,6 +251,7 @@ describe('Dependencies actions', () => {
[
{
type: 'requestDependencies',
payload: { page: paramsDefault.page },
},
{
type: 'receiveDependenciesSuccess',
......@@ -184,6 +279,7 @@ describe('Dependencies actions', () => {
[
{
type: 'requestDependencies',
payload: { page: paramsGiven.page },
},
{
type: 'receiveDependenciesSuccess',
......@@ -199,7 +295,9 @@ describe('Dependencies actions', () => {
${'an invalid response'} | ${[200, { foo: 'bar' }]}
${'a response error'} | ${[500]}
`('given $responseType', ({ responseDetails }) => {
let page;
beforeEach(() => {
({ page } = state.pageInfo);
mock.onGet(state.endpoint).replyOnce(...responseDetails);
});
......@@ -212,6 +310,7 @@ describe('Dependencies actions', () => {
[
{
type: 'requestDependencies',
payload: { page },
},
{
type: 'receiveDependenciesError',
......
......@@ -7,6 +7,18 @@ import { TEST_HOST } from 'helpers/test_constants';
describe('Dependencies mutations', () => {
let state;
const errorLoadingState = () => ({
isLoading: false,
errorLoading: true,
dependencies: [],
pageInfo: {},
initialized: true,
reportInfo: {
status: REPORT_STATUS.ok,
jobPath: '',
},
});
beforeEach(() => {
state = getInitialState();
});
......@@ -19,14 +31,47 @@ describe('Dependencies mutations', () => {
});
});
describe(types.REQUEST_DEPENDENCIES_PAGINATION, () => {
beforeEach(() => {
mutations[types.REQUEST_DEPENDENCIES_PAGINATION](state);
});
it('correctly mutates the state', () => {
expect(state.isLoading).toBe(true);
});
});
describe(types.RECEIVE_DEPENDENCIES_PAGINATION_SUCCESS, () => {
const total = 123;
beforeEach(() => {
mutations[types.RECEIVE_DEPENDENCIES_PAGINATION_SUCCESS](state, total);
});
it('correctly mutates the state', () => {
expect(state.pageInfo.total).toBe(total);
});
});
describe(types.RECEIVE_DEPENDENCIES_PAGINATION_ERROR, () => {
beforeEach(() => {
mutations[types.RECEIVE_DEPENDENCIES_PAGINATION_ERROR](state);
});
it('correctly mutates the state', () => {
expect(state).toEqual(expect.objectContaining(errorLoadingState()));
});
});
describe(types.REQUEST_DEPENDENCIES, () => {
const page = 4;
beforeEach(() => {
mutations[types.REQUEST_DEPENDENCIES](state);
mutations[types.REQUEST_DEPENDENCIES](state, { page });
});
it('correctly mutates the state', () => {
expect(state.isLoading).toBe(true);
expect(state.errorLoading).toBe(false);
expect(state.pageInfo.page).toBe(page);
});
});
......@@ -37,21 +82,27 @@ describe('Dependencies mutations', () => {
status: REPORT_STATUS.jobFailed,
job_path: 'foo',
};
let originalPageInfo;
beforeEach(() => {
originalPageInfo = state.pageInfo;
mutations[types.RECEIVE_DEPENDENCIES_SUCCESS](state, { dependencies, reportInfo, pageInfo });
});
it('correctly mutates the state', () => {
expect(state.isLoading).toBe(false);
expect(state.errorLoading).toBe(false);
expect(state.dependencies).toBe(dependencies);
expect(state.pageInfo).toBe(pageInfo);
expect(state.initialized).toBe(true);
expect(state.reportInfo).toEqual({
status: REPORT_STATUS.jobFailed,
jobPath: 'foo',
});
expect(state).toEqual(
expect.objectContaining({
isLoading: false,
errorLoading: false,
dependencies,
pageInfo: originalPageInfo,
initialized: true,
reportInfo: {
status: REPORT_STATUS.jobFailed,
jobPath: 'foo',
},
}),
);
});
});
......@@ -61,15 +112,7 @@ describe('Dependencies mutations', () => {
});
it('correctly mutates the state', () => {
expect(state.isLoading).toBe(false);
expect(state.errorLoading).toBe(true);
expect(state.dependencies).toEqual([]);
expect(state.pageInfo).toEqual({});
expect(state.initialized).toBe(true);
expect(state.reportInfo).toEqual({
status: REPORT_STATUS.ok,
jobPath: '',
});
expect(state).toEqual(expect.objectContaining(errorLoadingState()));
});
});
......
import axios from 'axios';
import MockAdapter from 'axios-mock-adapter';
import testAction from 'spec/helpers/vuex_action_helper';
import { TEST_HOST } from 'spec/test_constants';
import actionsModule, * as actions from 'ee/dependencies/store/actions';
import getInitialState from 'ee/dependencies/store/state';
import { FETCH_ERROR_MESSAGE } from 'ee/dependencies/store/constants';
describe('Dependencies actions', () => {
/**
* This file only contains tests for failure conditions. Tests for success
* conditions are in `ee/spec/frontend/dependencies/store/actions_spec.js`.
* The split is due to https://gitlab.com/gitlab-org/gitlab-ce/issues/63225.
*/
describe('fetchDependenciesPagination', () => {
const failureScenarios = [
{
context: 'an invalid response',
endpoint: TEST_HOST,
responseDetails: [200, { foo: 'bar' }],
},
{
context: 'a response error',
endpoint: TEST_HOST,
responseDetails: [500],
},
{
context: 'no endpoint',
endpoint: '',
responseDetails: [],
},
];
failureScenarios.forEach(({ context, endpoint, responseDetails }) => {
describe(`given ${context}`, () => {
let state;
let flashSpy;
let mock;
beforeEach(() => {
mock = new MockAdapter(axios);
state = getInitialState();
flashSpy = spyOnDependency(actionsModule, 'createFlash');
state.endpoint = endpoint;
if (endpoint) {
mock.onGet(state.endpoint).replyOnce(...responseDetails);
}
});
afterEach(() => {
mock.restore();
});
it('dispatches the correct actions and creates a flash', done => {
testAction(
actions.fetchDependenciesPagination,
undefined,
state,
[],
[
{
type: 'requestDependenciesPagination',
},
{
type: 'receiveDependenciesPaginationError',
payload: jasmine.any(Error),
},
],
)
.then(done.fail)
.catch(() => {
expect(flashSpy).toHaveBeenCalledTimes(1);
expect(flashSpy).toHaveBeenCalledWith(FETCH_ERROR_MESSAGE);
done();
});
});
});
});
});
});
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