From 577d290f1395797cfe59e4136d3842ed34d695d2 Mon Sep 17 00:00:00 2001 From: Paul Gascou-Vaillancourt Date: Thu, 25 Feb 2021 16:20:34 -0500 Subject: [PATCH 1/8] Add ability to pick enabled ref types Adds a enabledRefTypes prop to the RefSelector component to pick what ref types should be fetched and presented to the user. --- .../ref/components/ref_selector.vue | 11 +++++-- app/assets/javascripts/ref/constants.js | 5 ++++ app/assets/javascripts/ref/stores/actions.js | 17 ++++++++--- .../javascripts/ref/stores/mutation_types.js | 2 ++ .../javascripts/ref/stores/mutations.js | 3 ++ app/assets/javascripts/ref/stores/state.js | 25 +++++++--------- .../ref/components/ref_selector_spec.js | 30 ++++++++++++++++++- spec/frontend/ref/stores/actions_spec.js | 22 +++++++++++++- spec/frontend/ref/stores/mutations_spec.js | 11 ++++++- 9 files changed, 102 insertions(+), 24 deletions(-) diff --git a/app/assets/javascripts/ref/components/ref_selector.vue b/app/assets/javascripts/ref/components/ref_selector.vue index 8f2805b36f623b..150937c2317136 100644 --- a/app/assets/javascripts/ref/components/ref_selector.vue +++ b/app/assets/javascripts/ref/components/ref_selector.vue @@ -10,7 +10,7 @@ import { } from '@gitlab/ui'; import { debounce } 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 } from '../constants'; import createStore from '../stores'; import RefResultsSection from './ref_results_section.vue'; @@ -28,6 +28,12 @@ export default { RefResultsSection, }, props: { + enabledRefTypes: { + type: Array, + required: false, + default: () => ALL_REF_TYPES, + validator: (refTypes) => refTypes.every((refType) => ALL_REF_TYPES.includes(refType)), + }, value: { type: String, required: false, @@ -96,11 +102,12 @@ export default { this.search(this.query); }, SEARCH_DEBOUNCE_MS); + this.setEnabledRefTypes(this.enabledRefTypes); this.setProjectId(this.projectId); this.search(this.query); }, methods: { - ...mapActions(['setProjectId', 'setSelectedRef', 'search']), + ...mapActions(['setEnabledRefTypes', 'setProjectId', 'setSelectedRef', 'search']), focusSearchBox() { this.$refs.searchBox.$el.querySelector('input').focus(); }, diff --git a/app/assets/javascripts/ref/constants.js b/app/assets/javascripts/ref/constants.js index ca82b951377bd7..c9d8e16ba14648 100644 --- a/app/assets/javascripts/ref/constants.js +++ b/app/assets/javascripts/ref/constants.js @@ -1,5 +1,10 @@ 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 = [REF_TYPE_BRANCHES, REF_TYPE_TAGS, REF_TYPE_COMMITS]; + export const X_TOTAL_HEADER = 'x-total'; export const SEARCH_DEBOUNCE_MS = 250; diff --git a/app/assets/javascripts/ref/stores/actions.js b/app/assets/javascripts/ref/stores/actions.js index d9bdd64ace59b2..3832cc0c21df26 100644 --- a/app/assets/javascripts/ref/stores/actions.js +++ b/app/assets/javascripts/ref/stores/actions.js @@ -1,17 +1,26 @@ 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 }) => { diff --git a/app/assets/javascripts/ref/stores/mutation_types.js b/app/assets/javascripts/ref/stores/mutation_types.js index 9f6195f5f3ff77..c26f4fa00c784d 100644 --- a/app/assets/javascripts/ref/stores/mutation_types.js +++ b/app/assets/javascripts/ref/stores/mutation_types.js @@ -1,3 +1,5 @@ +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'; diff --git a/app/assets/javascripts/ref/stores/mutations.js b/app/assets/javascripts/ref/stores/mutations.js index 4dc73dabfe2beb..f91cbae8462ca4 100644 --- a/app/assets/javascripts/ref/stores/mutations.js +++ b/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; }, diff --git a/app/assets/javascripts/ref/stores/state.js b/app/assets/javascripts/ref/stores/state.js index 65b9d6449d7dc4..3affa8f8d03982 100644 --- a/app/assets/javascripts/ref/stores/state.js +++ b/app/assets/javascripts/ref/stores/state.js @@ -1,23 +1,18 @@ +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, diff --git a/spec/frontend/ref/components/ref_selector_spec.js b/spec/frontend/ref/components/ref_selector_spec.js index 27ada131ed6075..8d8d8059691090 100644 --- a/spec/frontend/ref/components/ref_selector_spec.js +++ b/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,24 @@ 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()); + }, + ); + }); }); diff --git a/spec/frontend/ref/stores/actions_spec.js b/spec/frontend/ref/stores/actions_spec.js index 11acec27165959..099ce062a3ac7b 100644 --- a/spec/frontend/ref/stores/actions_spec.js +++ b/spec/frontend/ref/stores/actions_spec.js @@ -1,4 +1,5 @@ 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 })), ); }); }); diff --git a/spec/frontend/ref/stores/mutations_spec.js b/spec/frontend/ref/stores/mutations_spec.js index cda130897666c6..11d4fe0e206250 100644 --- a/spec/frontend/ref/stores/mutations_spec.js +++ b/spec/frontend/ref/stores/mutations_spec.js @@ -1,4 +1,4 @@ -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'; -- GitLab From 04237c10dbfeb12973b5c4e3b31e2285d1d2e0cd Mon Sep 17 00:00:00 2001 From: Paul Gascou-Vaillancourt Date: Thu, 25 Feb 2021 16:38:01 -0500 Subject: [PATCH 2/8] Add ability to hide section headers Hides section headers when a single ref type is enabled. --- .../ref/components/ref_results_section.vue | 8 +++++++- .../ref/components/ref_selector.vue | 6 ++++++ .../ref/components/ref_selector_spec.js | 20 +++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/ref/components/ref_results_section.vue b/app/assets/javascripts/ref/components/ref_results_section.vue index 87ce4f1a49ce9b..4fa2a92ff03eda 100644 --- a/app/assets/javascripts/ref/components/ref_results_section.vue +++ b/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 {