Merge branch '196526-bad-ux-of-instance-level-security-dashboard-project-selector' into 'master'

Add pagination support to ILSD project selector

See merge request gitlab-org/gitlab!26138

ee/app/assets/javascripts/security_dashboard/components/project_manager.vue

@@ -12,6 +12,7 @@ export default {
   },
   computed: {
     ...mapState('projectSelector', [
+      'pageInfo',
       'projects',
       'selectedProjects',
       'projectSearchResults',
@@ -26,6 +27,7 @@ export default {
   methods: {
     ...mapActions('projectSelector', [
       'fetchSearchResults',
+      'fetchSearchResultsNextPage',
       'addProjects',
       'clearSearchResults',
       'toggleSelectedProject',
@@ -62,8 +64,10 @@ export default {
             :show-loading-indicator="isSearchingProjects"
             :show-minimum-search-query-message="messages.minimumQuery"
             :show-search-error-message="messages.searchError"
+            :total-results="pageInfo.total"
             @searched="searched"
             @projectClicked="projectClicked"
+            @bottomReached="fetchSearchResultsNextPage"
           />
           <div class="mb-3">
             <gl-button :disabled="!canAddProjects" variant="success" @click="addProjects">

ee/app/assets/javascripts/security_dashboard/store/modules/project_selector/actions.js

@@ -2,10 +2,14 @@ import Api from '~/api';
 import axios from '~/lib/utils/axios_utils';
 import createFlash from '~/flash';
 import { __, s__, sprintf } from '~/locale';
+import addPageInfo from './utils/add_page_info';
 import * as types from './mutation_types';
 
 const API_MINIMUM_QUERY_LENGTH = 3;
 
+const searchProjects = (searchQuery, searchOptions) =>
+  Api.projects(searchQuery, searchOptions).then(addPageInfo);
+
 export const toggleSelectedProject = ({ commit, state }, project) => {
   const isProject = ({ id }) => id === project.id;
 
@@ -141,7 +145,7 @@ export const receiveRemoveProjectError = ({ commit }) => {
   createFlash(__('Something went wrong, unable to remove project'));
 };
 
-export const fetchSearchResults = ({ state, dispatch }) => {
+export const fetchSearchResults = ({ state, dispatch, commit }) => {
   const { searchQuery } = state;
   dispatch('requestSearchResults');
 
@@ -149,17 +153,30 @@ export const fetchSearchResults = ({ state, dispatch }) => {
     return dispatch('setMinimumQueryMessage');
   }
 
-  return Api.projects(searchQuery, {})
-    .then(results => dispatch('receiveSearchResultsSuccess', results))
+  return searchProjects(searchQuery)
+    .then(payload => commit(types.RECEIVE_SEARCH_RESULTS_SUCCESS, payload))
     .catch(() => dispatch('receiveSearchResultsError'));
 };
 
-export const requestSearchResults = ({ commit }) => {
-  commit(types.REQUEST_SEARCH_RESULTS);
+export const fetchSearchResultsNextPage = ({ state, dispatch, commit }) => {
+  const {
+    searchQuery,
+    pageInfo: { totalPages, page, nextPage },
+  } = state;
+
+  if (totalPages <= page) {
+    return Promise.resolve();
+  }
+
+  const searchOptions = { page: nextPage };
+
+  return searchProjects(searchQuery, searchOptions)
+    .then(payload => commit(types.RECEIVE_SEARCH_RESULTS_SUCCESS, payload))
+    .catch(() => dispatch('receiveSearchResultsError'));
 };
 
-export const receiveSearchResultsSuccess = ({ commit }, results) => {
-  commit(types.RECEIVE_SEARCH_RESULTS_SUCCESS, results);
+export const requestSearchResults = ({ commit }) => {
+  commit(types.REQUEST_SEARCH_RESULTS);
 };
 
 export const receiveSearchResultsError = ({ commit }) => {

ee/app/assets/javascripts/security_dashboard/store/modules/project_selector/mutation_types.js

@@ -24,3 +24,5 @@ export const RECEIVE_SEARCH_RESULTS_SUCCESS = 'RECEIVE_SEARCH_RESULTS_SUCCESS';
 export const RECEIVE_SEARCH_RESULTS_ERROR = 'RECEIVE_SEARCH_RESULTS_ERROR';
 
 export const SET_MINIMUM_QUERY_MESSAGE = 'SET_MINIMUM_QUERY_MESSAGE';
+
+export const RECEIVE_NEXT_PAGE_SUCCESS = 'RECEIVE_NEXT_PAGE_SUCCESS';

ee/app/assets/javascripts/security_dashboard/store/modules/project_selector/mutations.js

@@ -53,8 +53,9 @@ export default {
     state.messages.minimumQuery = false;
     state.searchCount += 1;
   },
-  [types.RECEIVE_SEARCH_RESULTS_SUCCESS](state, results) {
-    state.projectSearchResults = results.data;
+  [types.RECEIVE_SEARCH_RESULTS_SUCCESS](state, { data, pageInfo }) {
+    state.projectSearchResults = data;
+    state.pageInfo = pageInfo;
 
     state.messages.noResults = state.projectSearchResults.length === 0;
     state.messages.searchError = false;
@@ -73,6 +74,7 @@ export default {
   },
   [types.SET_MINIMUM_QUERY_MESSAGE](state) {
     state.projectSearchResults = [];
+    state.pageInfo.total = 0;
 
     state.messages.noResults = false;
     state.messages.searchError = false;
@@ -80,4 +82,9 @@ export default {
 
     state.searchCount = Math.max(0, state.searchCount - 1);
   },
+  [types.RECEIVE_NEXT_PAGE_SUCCESS](state, { data, pageInfo }) {
+    state.projectSearchResults = state.projectSearchResults.concat(data);
+
+    state.pageInfo = pageInfo;
+  },
 };

ee/app/assets/javascripts/security_dashboard/store/modules/project_selector/state.js

@@ -17,4 +17,10 @@ export default () => ({
     minimumQuery: false,
   },
   searchCount: 0,
+  pageInfo: {
+    page: 0,
+    nextPage: 0,
+    total: 0,
+    totalPages: 0,
+  },
 });

ee/app/assets/javascripts/security_dashboard/store/modules/project_selector/utils/add_page_info.js

+import { flow } from 'lodash';
+import { parseIntPagination, normalizeHeaders } from '~/lib/utils/common_utils';
+
+/**
+ * Takes an object containing pagination related data (eg.: page, nextPage, ...)
+ * and returns a new object which has this data grouped in a 'pageInfo' property
+ *
+ * @param {{page: *, nextPage: *, total: *, totalPages: *}}
+ * @returns {{pageInfo: {total: *, nextPage: *, totalPages: *, page: *}}}
+ */
+const groupPageInfo = ({ page, nextPage, total, totalPages }) => ({
+  pageInfo: { page, nextPage, total, totalPages },
+});
+
+/**
+ * Returns an XHR-response's headers property
+ *
+ * @param {{headers}} res
+ * @returns {*}
+ */
+const getHeaders = res => res.headers;
+
+/**
+ * Takes an XHR-response object and returns an object containing pagination related
+ * data
+ *
+ * @param {{headers}}
+ * @returns {{pageInfo: {}}}
+ */
+const pageInfo = flow(
+  getHeaders,
+  normalizeHeaders,
+  parseIntPagination,
+  groupPageInfo,
+);
+
+/**
+ * Takes an XHR-response object and adds pagination related data do it
+ * (eg.: page, nextPage, total, totalPages)
+ *
+ * @param {Object} res
+ * @return {Object}
+ */
+const addPageInfo = res => (res?.headers ? { ...res, ...pageInfo(res) } : res);
+
+export default addPageInfo;

ee/changelogs/unreleased/196526-bad-ux-of-instance-level-security-dashboard-project-selector.yml

+---
+title: Add pagination to Instance Level Security Dashboard project selector
+merge_request: 26138
+author:
+type: fixed

ee/spec/frontend/security_dashboard/components/project_manager_spec.js

@@ -30,6 +30,7 @@ describe('Project Manager component', () => {
           actions: {
             setSearchQuery: jest.fn(),
             fetchSearchResults: jest.fn(),
+            fetchSearchResultsNextPage: jest.fn(),
             addProjects: jest.fn(),
             clearSearchResults: jest.fn(),
             toggleSelectedProject: jest.fn(),
@@ -104,11 +105,24 @@ describe('Project Manager component', () => {
       expect(getProjectList().exists()).toBe(true);
     });
 
-    it('dispatches the right actions when the project-list emits a projectRemoved event', () => {
+    it('dispatches the right actions when the project-list emits a "bottomReached" event', () => {
+      const projectSelector = getProjectSelector();
+      const fetchSearchResultsNextPageAction = getMockAction('fetchSearchResultsNextPage');
+
+      expect(fetchSearchResultsNextPageAction).toHaveBeenCalledTimes(0);
+
+      projectSelector.vm.$emit('bottomReached');
+
+      expect(fetchSearchResultsNextPageAction).toHaveBeenCalledTimes(1);
+    });
+
+    it('dispatches the right actions when the project-list emits a "projectRemoved" event', () => {
       const mockProject = { remove_path: 'foo' };
-      const projectList = wrapper.find(ProjectList);
+      const projectList = getProjectList();
       const removeProjectAction = getMockAction('removeProject');
 
+      expect(removeProjectAction).toHaveBeenCalledTimes(0);
+
       projectList.vm.$emit('projectRemoved', mockProject);
 
       expect(removeProjectAction).toHaveBeenCalledTimes(1);

ee/spec/frontend/security_dashboard/store/modules/project_selector/actions_spec.js

 import MockAdapter from 'axios-mock-adapter';
 import testAction from 'helpers/vuex_action_helper';
-
+import waitForPromises from 'helpers/wait_for_promises';
 import createState from 'ee/security_dashboard/store/modules/project_selector/state';
 import * as types from 'ee/security_dashboard/store/modules/project_selector/mutation_types';
 import * as actions from 'ee/security_dashboard/store/modules/project_selector/actions';
 import createFlash from '~/flash';
-import Api from '~/api';
 import axios from '~/lib/utils/axios_utils';
 
-jest.mock('~/api');
 jest.mock('~/flash');
 
-describe('projectSelector actions', () => {
+describe('EE projectSelector actions', () => {
   const getMockProjects = n => [...Array(n).keys()].map(i => ({ id: i, name: `project-${i}` }));
 
   const mockAddEndpoint = 'mock-add_endpoint';
@@ -22,6 +20,20 @@ describe('projectSelector actions', () => {
   let mockDispatchContext;
   let state;
 
+  const pageInfo = {
+    page: 1,
+    nextPage: 2,
+    total: 50,
+    totalPages: 5,
+  };
+
+  const responseHeaders = {
+    'X-Next-Page': pageInfo.nextPage,
+    'X-Page': pageInfo.page,
+    'X-Total': pageInfo.total,
+    'X-Total-Pages': pageInfo.totalPages,
+  };
+
   beforeEach(() => {
     mockAxios = new MockAdapter(axios);
     mockDispatchContext = { dispatch: () => {}, commit: () => {}, state };
@@ -71,7 +83,23 @@ describe('projectSelector actions', () => {
   });
 
   describe('addProjects', () => {
-    it('posts selected project ids to project add endpoint', () => {
+    it(`posts the selected project's ids to the add-endpoint`, () => {
+      const projectIds = ['1', '2'];
+
+      state.selectedProjects = [{ id: projectIds[0], name: '1' }, { id: projectIds[1], name: '2' }];
+      state.projectEndpoints.add = mockAddEndpoint;
+
+      mockAxios.onPost(mockAddEndpoint).replyOnce(200, mockResponse);
+
+      actions.addProjects({ state, dispatch: () => {} });
+
+      return waitForPromises().then(() => {
+        const requestData = mockAxios.history.post[0].data;
+        expect(requestData).toBe(JSON.stringify({ project_ids: projectIds }));
+      });
+    });
+
+    it('dispatches the correct actions when the request is successful', () => {
       state.projectEndpoints.add = mockAddEndpoint;
 
       mockAxios.onPost(mockAddEndpoint).replyOnce(200, mockResponse);
@@ -406,6 +434,25 @@ describe('projectSelector actions', () => {
     });
   });
 
+  describe('setSearchQuery', () => {
+    it('commits the REQUEST_SEARCH_RESULTS mutation', () => {
+      const payload = 'search-query';
+
+      return testAction(
+        actions.setSearchQuery,
+        payload,
+        state,
+        [
+          {
+            type: types.SET_SEARCH_QUERY,
+            payload,
+          },
+        ],
+        [],
+      );
+    });
+  });
+
   describe('fetchSearchResults', () => {
     it.each([null, undefined, false, NaN, 0, ''])(
       'dispatches setMinimumQueryMessage if the search query is falsy',
@@ -452,11 +499,35 @@ describe('projectSelector actions', () => {
     );
 
     it('dispatches the correct actions when the query is valid', () => {
-      const mockProjects = [{ id: 0, name: 'mock-name1' }];
-      Api.projects.mockResolvedValueOnce(mockProjects);
+      const projects = [{ id: 0, name: 'mock-name1' }];
+
+      mockAxios.onGet().replyOnce(200, projects, responseHeaders);
       state.searchQuery = 'mock-query';
 
       return testAction(
+        actions.fetchSearchResults,
+        null,
+        state,
+        [
+          {
+            type: types.RECEIVE_SEARCH_RESULTS_SUCCESS,
+            payload: { data: projects, headers: responseHeaders, pageInfo },
+          },
+        ],
+        [
+          {
+            type: 'requestSearchResults',
+          },
+        ],
+      );
+    });
+
+    it('dispatches the correct actions when the request is not successful', () => {
+      mockAxios.onGet(mockListEndpoint).replyOnce(500);
+
+      state.searchQuery = 'mock-query';
+
+      testAction(
         actions.fetchSearchResults,
         null,
         state,
@@ -466,8 +537,7 @@ describe('projectSelector actions', () => {
             type: 'requestSearchResults',
           },
           {
-            type: 'receiveSearchResultsSuccess',
-            payload: mockProjects,
+            type: 'receiveSearchResultsError',
           },
         ],
       );
@@ -489,25 +559,6 @@ describe('projectSelector actions', () => {
       ));
   });
 
-  describe('receiveSearchResultsSuccess', () => {
-    it('commits the RECEIVE_SEARCH_RESULTS_SUCCESS mutation', () => {
-      const mockProjects = [{ id: 0, name: 'mock-project1' }];
-
-      return testAction(
-        actions.receiveSearchResultsSuccess,
-        mockProjects,
-        state,
-        [
-          {
-            type: types.RECEIVE_SEARCH_RESULTS_SUCCESS,
-            payload: mockProjects,
-          },
-        ],
-        [],
-      );
-    });
-  });
-
   describe('receiveSearchResultsError', () => {
     it('commits the RECEIVE_SEARCH_RESULTS_ERROR mutation', () =>
       testAction(
@@ -523,6 +574,59 @@ describe('projectSelector actions', () => {
       ));
   });
 
+  describe('fetchSearchResultsNextPage', () => {
+    describe('when the current page-index is smaller than the number of total pages', () => {
+      beforeEach(() => {
+        state.pageInfo.totalPages = 2;
+        state.pageInfo.page = 1;
+      });
+
+      it('dispatches the "receiveNextPageSuccess" action if the request is successful', () => {
+        const projects = [{ id: 0, name: 'mock-name1' }];
+
+        mockAxios.onGet().replyOnce(200, projects, responseHeaders);
+
+        return testAction(
+          actions.fetchSearchResultsNextPage,
+          null,
+          state,
+          [
+            {
+              type: types.RECEIVE_SEARCH_RESULTS_SUCCESS,
+              payload: { data: projects, headers: responseHeaders, pageInfo },
+            },
+          ],
+          [],
+        );
+      });
+
+      it('dispatches the "receiveSearchResultsError" action if the request is not successful', () => {
+        mockAxios.onGet(mockListEndpoint).replyOnce(500);
+
+        return testAction(
+          actions.fetchSearchResultsNextPage,
+          null,
+          state,
+          [],
+          [
+            {
+              type: 'receiveSearchResultsError',
+            },
+          ],
+        );
+      });
+    });
+
+    describe('when the current page-index is equal to the number of total pages', () => {
+      it('does not commit any mutations or dispatch any actions', () => {
+        state.pageInfo.totalPages = 1;
+        state.pageInfo.page = 1;
+
+        return testAction(actions.fetchSearchResultsNextPage, [], state);
+      });
+    });
+  });
+
   describe('setProjectEndpoints', () => {
     it('commits project list and add endpoints', () => {
       const payload = {

ee/spec/frontend/security_dashboard/store/modules/project_selector/folder/add_page_info_spec.js

+import addPageInfo from 'ee/security_dashboard/store/modules/project_selector/utils/add_page_info';
+
+describe('EE Project Selector store utils', () => {
+  describe('addPageInfo', () => {
+    it('takes an API response and adds a "pageInfo" property that contains the headers pagination data', () => {
+      const responseData = {
+        data: { foo: 'bar ' },
+        headers: {
+          'X-Next-Page': 0,
+          'X-Page': 0,
+          'X-Total': 0,
+          'X-Total-Pages': 0,
+        },
+      };
+
+      const responseDataWithPageInfo = addPageInfo(responseData);
+
+      expect(responseDataWithPageInfo).toStrictEqual({
+        ...responseData,
+        pageInfo: {
+          page: 0,
+          nextPage: 0,
+          total: 0,
+          totalPages: 0,
+        },
+      });
+    });
+
+    it.each([{}, { foo: 'foo' }, null, undefined, false])(
+      'returns the original input if it does not contain a header property',
+      input => {
+        expect(addPageInfo(input)).toBe(input);
+      },
+    );
+  });
+});

ee/spec/frontend/security_dashboard/store/modules/project_selector/mutations_spec.js

@@ -2,7 +2,7 @@ import createState from 'ee/security_dashboard/store/modules/project_selector/st
 import mutations from 'ee/security_dashboard/store/modules/project_selector/mutations';
 import * as types from 'ee/security_dashboard/store/modules/project_selector/mutation_types';
 
-describe('projectsSelector mutations', () => {
+describe('EE projectsSelector mutations', () => {
   let state;
 
   beforeEach(() => {
@@ -332,6 +332,14 @@ describe('projectsSelector mutations', () => {
       expect(state.projectSearchResults).toHaveLength(0);
     });
 
+    it('sets "pageInfo.total" to be 0', () => {
+      state.pageInfo.total = 1;
+
+      mutations[types.SET_MINIMUM_QUERY_MESSAGE](state);
+
+      expect(state.pageInfo.total).toBe(0);
+    });
+
     it('sets "messages.noResults" to be false', () => {
       state.messages.noResults = true;
 
@@ -364,4 +372,24 @@ describe('projectsSelector mutations', () => {
       expect(state.searchCount).toBe(0);
     });
   });
+
+  describe('RECEIVE_NEXT_PAGE_SUCCESS', () => {
+    it('appends the data from the payload to "projectsSearchResults"', () => {
+      state.projectSearchResults = ['foo'];
+
+      const payload = { data: ['bar'] };
+
+      mutations[types.RECEIVE_NEXT_PAGE_SUCCESS](state, payload);
+
+      expect(state.projectSearchResults).toEqual(['foo', 'bar']);
+    });
+
+    it('sets "pageInfo" to be the given payload', () => {
+      const payload = { pageInfo: { foo: 'foo', bar: 'bar' } };
+
+      mutations[types.RECEIVE_NEXT_PAGE_SUCCESS](state, payload);
+
+      expect(state.pageInfo).toEqual(payload.pageInfo);
+    });
+  });
 });