Merge branch '322890-enhance-ref-selector-component' into 'master'

Enhance the RefSelector component

See merge request gitlab-org/gitlab!55245

app/assets/javascripts/ref/components/ref_results_section.vue

@@ -11,6 +11,12 @@ export default {
     GlIcon,
   },
   props: {
+    showHeader: {
+      type: Boolean,
+      required: false,
+      default: true,
+    },
+
     sectionTitle: {
       type: String,
       required: true,
@@ -84,7 +90,7 @@ export default {
 
 <template>
   <div>
-    <gl-dropdown-section-header>
+    <gl-dropdown-section-header v-if="showHeader">
       <div class="gl-display-flex align-items-center" data-testid="section-header">
         <span class="gl-mr-2 gl-mb-1">{{ sectionTitle }}</span>
         <gl-badge variant="neutral">{{ totalCountText }}</gl-badge>

app/assets/javascripts/ref/components/ref_selector.vue

@@ -8,9 +8,16 @@ import {
   GlIcon,
   GlLoadingIcon,
 } from '@gitlab/ui';
-import { debounce } from 'lodash';
+import { debounce, isArray } from 'lodash';
 import { mapActions, mapGetters, mapState } from 'vuex';
-import { SEARCH_DEBOUNCE_MS, DEFAULT_I18N } from '../constants';
+import {
+  ALL_REF_TYPES,
+  SEARCH_DEBOUNCE_MS,
+  DEFAULT_I18N,
+  REF_TYPE_BRANCHES,
+  REF_TYPE_TAGS,
+  REF_TYPE_COMMITS,
+} from '../constants';
 import createStore from '../stores';
 import RefResultsSection from './ref_results_section.vue';
 
@@ -28,6 +35,20 @@ export default {
     RefResultsSection,
   },
   props: {
+    enabledRefTypes: {
+      type: Array,
+      required: false,
+      default: () => ALL_REF_TYPES,
+      validator: (val) =>
+        // It has to be an arrray
+        isArray(val) &&
+        // with at least one item
+        val.length > 0 &&
+        // and only "REF_TYPE_BRANCHES", "REF_TYPE_TAGS", and "REF_TYPE_COMMITS" are allowed
+        val.every((item) => ALL_REF_TYPES.includes(item)) &&
+        // and no duplicates are allowed
+        val.length === new Set(val).size,
+    },
     value: {
       type: String,
       required: false,
@@ -62,17 +83,29 @@ export default {
       };
     },
     showBranchesSection() {
-      return Boolean(this.matches.branches.totalCount > 0 || this.matches.branches.error);
+      return (
+        this.enabledRefTypes.includes(REF_TYPE_BRANCHES) &&
+        Boolean(this.matches.branches.totalCount > 0 || this.matches.branches.error)
+      );
     },
     showTagsSection() {
-      return Boolean(this.matches.tags.totalCount > 0 || this.matches.tags.error);
+      return (
+        this.enabledRefTypes.includes(REF_TYPE_TAGS) &&
+        Boolean(this.matches.tags.totalCount > 0 || this.matches.tags.error)
+      );
     },
     showCommitsSection() {
-      return Boolean(this.matches.commits.totalCount > 0 || this.matches.commits.error);
+      return (
+        this.enabledRefTypes.includes(REF_TYPE_COMMITS) &&
+        Boolean(this.matches.commits.totalCount > 0 || this.matches.commits.error)
+      );
     },
     showNoResults() {
       return !this.showBranchesSection && !this.showTagsSection && !this.showCommitsSection;
     },
+    showSectionHeaders() {
+      return this.enabledRefTypes.length > 1;
+    },
   },
   watch: {
     // Keep the Vuex store synchronized if the parent
@@ -97,10 +130,18 @@ export default {
     }, SEARCH_DEBOUNCE_MS);
 
     this.setProjectId(this.projectId);
-    this.search(this.query);
+
+    this.$watch(
+      'enabledRefTypes',
+      () => {
+        this.setEnabledRefTypes(this.enabledRefTypes);
+        this.search(this.query);
+      },
+      { immediate: true },
+    );
   },
   methods: {
-    ...mapActions(['setProjectId', 'setSelectedRef', 'search']),
+    ...mapActions(['setEnabledRefTypes', 'setProjectId', 'setSelectedRef', 'search']),
     focusSearchBox() {
       this.$refs.searchBox.$el.querySelector('input').focus();
     },
@@ -170,6 +211,7 @@ export default {
               :selected-ref="selectedRef"
               :error="matches.branches.error"
               :error-message="i18n.branchesErrorMessage"
+              :show-header="showSectionHeaders"
               data-testid="branches-section"
               @selected="selectRef($event)"
             />
@@ -185,6 +227,7 @@ export default {
               :selected-ref="selectedRef"
               :error="matches.tags.error"
               :error-message="i18n.tagsErrorMessage"
+              :show-header="showSectionHeaders"
               data-testid="tags-section"
               @selected="selectRef($event)"
             />
@@ -200,6 +243,7 @@ export default {
               :selected-ref="selectedRef"
               :error="matches.commits.error"
               :error-message="i18n.commitsErrorMessage"
+              :show-header="showSectionHeaders"
               data-testid="commits-section"
               @selected="selectRef($event)"
             />

app/assets/javascripts/ref/constants.js

 import { __ } from '~/locale';
 
+export const REF_TYPE_BRANCHES = 'REF_TYPE_BRANCHES';
+export const REF_TYPE_TAGS = 'REF_TYPE_TAGS';
+export const REF_TYPE_COMMITS = 'REF_TYPE_COMMITS';
+export const ALL_REF_TYPES = Object.freeze([REF_TYPE_BRANCHES, REF_TYPE_TAGS, REF_TYPE_COMMITS]);
+
 export const X_TOTAL_HEADER = 'x-total';
 
 export const SEARCH_DEBOUNCE_MS = 250;

app/assets/javascripts/ref/stores/actions.js

 import Api from '~/api';
+import { REF_TYPE_BRANCHES, REF_TYPE_TAGS, REF_TYPE_COMMITS } from '../constants';
 import * as types from './mutation_types';
 
+export const setEnabledRefTypes = ({ commit }, refTypes) =>
+  commit(types.SET_ENABLED_REF_TYPES, refTypes);
+
 export const setProjectId = ({ commit }, projectId) => commit(types.SET_PROJECT_ID, projectId);
 
 export const setSelectedRef = ({ commit }, selectedRef) =>
   commit(types.SET_SELECTED_REF, selectedRef);
 
-export const search = ({ dispatch, commit }, query) => {
+export const search = ({ state, dispatch, commit }, query) => {
   commit(types.SET_QUERY, query);
 
-  dispatch('searchBranches');
-  dispatch('searchTags');
-  dispatch('searchCommits');
+  const dispatchIfRefTypeEnabled = (refType, action) => {
+    if (state.enabledRefTypes.includes(refType)) {
+      dispatch(action);
+    }
+  };
+  dispatchIfRefTypeEnabled(REF_TYPE_BRANCHES, 'searchBranches');
+  dispatchIfRefTypeEnabled(REF_TYPE_TAGS, 'searchTags');
+  dispatchIfRefTypeEnabled(REF_TYPE_COMMITS, 'searchCommits');
 };
 
 export const searchBranches = ({ commit, state }) => {

app/assets/javascripts/ref/stores/mutation_types.js

+export const SET_ENABLED_REF_TYPES = 'SET_ENABLED_REF_TYPES';
+
 export const SET_PROJECT_ID = 'SET_PROJECT_ID';
 export const SET_SELECTED_REF = 'SET_SELECTED_REF';
 export const SET_QUERY = 'SET_QUERY';

app/assets/javascripts/ref/stores/mutations.js

@@ -4,6 +4,9 @@ import { X_TOTAL_HEADER } from '../constants';
 import * as types from './mutation_types';
 
 export default {
+  [types.SET_ENABLED_REF_TYPES](state, refTypes) {
+    state.enabledRefTypes = refTypes;
+  },
   [types.SET_PROJECT_ID](state, projectId) {
     state.projectId = projectId;
   },

app/assets/javascripts/ref/stores/state.js

+const createRefTypeState = () => ({
+  list: [],
+  totalCount: 0,
+  error: null,
+});
+
 export default () => ({
+  enabledRefTypes: [],
   projectId: null,
 
   query: '',
   matches: {
-    branches: {
-      list: [],
-      totalCount: 0,
-      error: null,
-    },
-    tags: {
-      list: [],
-      totalCount: 0,
-      error: null,
-    },
-    commits: {
-      list: [],
-      totalCount: 0,
-      error: null,
-    },
+    branches: createRefTypeState(),
+    tags: createRefTypeState(),
+    commits: createRefTypeState(),
   },
   selectedRef: null,
   requestCount: 0,

spec/frontend/ref/components/ref_selector_spec.js

@@ -7,7 +7,13 @@ import { trimText } from 'helpers/text_helper';
 import { ENTER_KEY } from '~/lib/utils/keys';
 import { sprintf } from '~/locale';
 import RefSelector from '~/ref/components/ref_selector.vue';
-import { X_TOTAL_HEADER, DEFAULT_I18N } from '~/ref/constants';
+import {
+  X_TOTAL_HEADER,
+  DEFAULT_I18N,
+  REF_TYPE_BRANCHES,
+  REF_TYPE_TAGS,
+  REF_TYPE_COMMITS,
+} from '~/ref/constants';
 import createStore from '~/ref/stores/';
 
 const localVue = createLocalVue();
@@ -26,6 +32,7 @@ describe('Ref selector component', () => {
   let branchesApiCallSpy;
   let tagsApiCallSpy;
   let commitApiCallSpy;
+  let requestSpies;
 
   const createComponent = (props = {}, attrs = {}) => {
     wrapper = mount(RefSelector, {
@@ -58,6 +65,7 @@ describe('Ref selector component', () => {
       .mockReturnValue([200, fixtures.branches, { [X_TOTAL_HEADER]: '123' }]);
     tagsApiCallSpy = jest.fn().mockReturnValue([200, fixtures.tags, { [X_TOTAL_HEADER]: '456' }]);
     commitApiCallSpy = jest.fn().mockReturnValue([200, fixtures.commit]);
+    requestSpies = { branchesApiCallSpy, tagsApiCallSpy, commitApiCallSpy };
 
     mock
       .onGet(`/api/v4/projects/${projectId}/repository/branches`)
@@ -592,4 +600,86 @@ describe('Ref selector component', () => {
       });
     });
   });
+
+  describe('with non-default ref types', () => {
+    it.each`
+      enabledRefTypes                      | reqsCalled                | reqsNotCalled
+      ${[REF_TYPE_BRANCHES]}               | ${['branchesApiCallSpy']} | ${['tagsApiCallSpy', 'commitApiCallSpy']}
+      ${[REF_TYPE_TAGS]}                   | ${['tagsApiCallSpy']}     | ${['branchesApiCallSpy', 'commitApiCallSpy']}
+      ${[REF_TYPE_COMMITS]}                | ${[]}                     | ${['branchesApiCallSpy', 'tagsApiCallSpy', 'commitApiCallSpy']}
+      ${[REF_TYPE_TAGS, REF_TYPE_COMMITS]} | ${['tagsApiCallSpy']}     | ${['branchesApiCallSpy', 'commitApiCallSpy']}
+    `(
+      'only calls $reqsCalled requests when $enabledRefTypes are enabled',
+      async ({ enabledRefTypes, reqsCalled, reqsNotCalled }) => {
+        createComponent({ enabledRefTypes });
+
+        await waitForRequests();
+
+        reqsCalled.forEach((req) => expect(requestSpies[req]).toHaveBeenCalledTimes(1));
+        reqsNotCalled.forEach((req) => expect(requestSpies[req]).not.toHaveBeenCalled());
+      },
+    );
+
+    it('only calls commitApiCallSpy when REF_TYPE_COMMITS is enabled', async () => {
+      createComponent({ enabledRefTypes: [REF_TYPE_COMMITS] });
+      updateQuery('abcd1234');
+
+      await waitForRequests();
+
+      expect(commitApiCallSpy).toHaveBeenCalledTimes(1);
+      expect(branchesApiCallSpy).not.toHaveBeenCalled();
+      expect(tagsApiCallSpy).not.toHaveBeenCalled();
+    });
+
+    it('triggers another search if enabled ref types change', async () => {
+      createComponent({ enabledRefTypes: [REF_TYPE_BRANCHES] });
+      await waitForRequests();
+
+      expect(branchesApiCallSpy).toHaveBeenCalledTimes(1);
+      expect(tagsApiCallSpy).not.toHaveBeenCalled();
+
+      wrapper.setProps({
+        enabledRefTypes: [REF_TYPE_BRANCHES, REF_TYPE_TAGS],
+      });
+      await waitForRequests();
+
+      expect(branchesApiCallSpy).toHaveBeenCalledTimes(2);
+      expect(tagsApiCallSpy).toHaveBeenCalledTimes(1);
+    });
+
+    it('if a ref type becomes disabled, its section is hidden, even if it had some results in store', async () => {
+      createComponent({ enabledRefTypes: [REF_TYPE_BRANCHES, REF_TYPE_COMMITS] });
+      updateQuery('abcd1234');
+      await waitForRequests();
+
+      expect(findBranchesSection().exists()).toBe(true);
+      expect(findCommitsSection().exists()).toBe(true);
+
+      wrapper.setProps({ enabledRefTypes: [REF_TYPE_COMMITS] });
+      await waitForRequests();
+
+      expect(findBranchesSection().exists()).toBe(false);
+      expect(findCommitsSection().exists()).toBe(true);
+    });
+
+    it.each`
+      enabledRefType       | findVisibleSection     | findHiddenSections
+      ${REF_TYPE_BRANCHES} | ${findBranchesSection} | ${[findTagsSection, findCommitsSection]}
+      ${REF_TYPE_TAGS}     | ${findTagsSection}     | ${[findBranchesSection, findCommitsSection]}
+      ${REF_TYPE_COMMITS}  | ${findCommitsSection}  | ${[findBranchesSection, findTagsSection]}
+    `(
+      'hides section headers if a single ref type is enabled',
+      async ({ enabledRefType, findVisibleSection, findHiddenSections }) => {
+        createComponent({ enabledRefTypes: [enabledRefType] });
+        updateQuery('abcd1234');
+        await waitForRequests();
+
+        expect(findVisibleSection().exists()).toBe(true);
+        expect(findVisibleSection().find('[data-testid="section-header"]').exists()).toBe(false);
+        findHiddenSections.forEach((findHiddenSection) =>
+          expect(findHiddenSection().exists()).toBe(false),
+        );
+      },
+    );
+  });
 });

spec/frontend/ref/stores/actions_spec.js

 import testAction from 'helpers/vuex_action_helper';
+import { ALL_REF_TYPES, REF_TYPE_BRANCHES, REF_TYPE_TAGS, REF_TYPE_COMMITS } from '~/ref/constants';
 import * as actions from '~/ref/stores/actions';
 import * as types from '~/ref/stores/mutation_types';
 import createState from '~/ref/stores/state';
@@ -25,6 +26,14 @@ describe('Ref selector Vuex store actions', () => {
     state = createState();
   });
 
+  describe('setEnabledRefTypes', () => {
+    it(`commits ${types.SET_ENABLED_REF_TYPES} with the enabled ref types`, () => {
+      testAction(actions.setProjectId, ALL_REF_TYPES, state, [
+        { type: types.SET_PROJECT_ID, payload: ALL_REF_TYPES },
+      ]);
+    });
+  });
+
   describe('setProjectId', () => {
     it(`commits ${types.SET_PROJECT_ID} with the new project ID`, () => {
       const projectId = '4';
@@ -46,12 +55,23 @@ describe('Ref selector Vuex store actions', () => {
   describe('search', () => {
     it(`commits ${types.SET_QUERY} with the new search query`, () => {
       const query = 'hello';
+      testAction(actions.search, query, state, [{ type: types.SET_QUERY, payload: query }]);
+    });
+
+    it.each`
+      enabledRefTypes                                         | expectedActions
+      ${[REF_TYPE_BRANCHES]}                                  | ${['searchBranches']}
+      ${[REF_TYPE_COMMITS]}                                   | ${['searchCommits']}
+      ${[REF_TYPE_BRANCHES, REF_TYPE_TAGS, REF_TYPE_COMMITS]} | ${['searchBranches', 'searchTags', 'searchCommits']}
+    `(`dispatches fetch actions for enabled ref types`, ({ enabledRefTypes, expectedActions }) => {
+      const query = 'hello';
+      state.enabledRefTypes = enabledRefTypes;
       testAction(
         actions.search,
         query,
         state,
         [{ type: types.SET_QUERY, payload: query }],
-        [{ type: 'searchBranches' }, { type: 'searchTags' }, { type: 'searchCommits' }],
+        expectedActions.map((type) => ({ type })),
       );
     });
   });

spec/frontend/ref/stores/mutations_spec.js

-import { X_TOTAL_HEADER } from '~/ref/constants';
+import { X_TOTAL_HEADER, ALL_REF_TYPES } from '~/ref/constants';
 import * as types from '~/ref/stores/mutation_types';
 import mutations from '~/ref/stores/mutations';
 import createState from '~/ref/stores/state';
@@ -13,6 +13,7 @@ describe('Ref selector Vuex store mutations', () => {
   describe('initial state', () => {
     it('is created with the correct structure and initial values', () => {
       expect(state).toEqual({
+        enabledRefTypes: [],
         projectId: null,
 
         query: '',
@@ -39,6 +40,14 @@ describe('Ref selector Vuex store mutations', () => {
     });
   });
 
+  describe(`${types.SET_ENABLED_REF_TYPES}`, () => {
+    it('sets the enabled ref types', () => {
+      mutations[types.SET_ENABLED_REF_TYPES](state, ALL_REF_TYPES);
+
+      expect(state.enabledRefTypes).toBe(ALL_REF_TYPES);
+    });
+  });
+
   describe(`${types.SET_PROJECT_ID}`, () => {
     it('updates the project ID', () => {
       const newProjectId = '4';