From 374e3f8461588f8f0e2e720d566813bb346c3a2d Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 5 Dec 2022 16:02:35 -0700 Subject: [PATCH 1/3] Removed pre-set branch in revert modal This should avoid pre-filtering the dropdown, which in turn should make it easier to see that you can pick a different branch. Changelog: fixed --- .../projects/commit/components/branches_dropdown.vue | 7 ++++++- .../projects/commit/components/form_modal.vue | 12 +++++++++++- .../projects/commit/init_revert_commit_modal.js | 1 + 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/projects/commit/components/branches_dropdown.vue b/app/assets/javascripts/projects/commit/components/branches_dropdown.vue index 52da8aaba4d841..a037e721677f20 100644 --- a/app/assets/javascripts/projects/commit/components/branches_dropdown.vue +++ b/app/assets/javascripts/projects/commit/components/branches_dropdown.vue @@ -28,6 +28,11 @@ export default { required: false, default: '', }, + blanked: { + type: Boolean, + required: false, + default: false, + }, }, i18n: { noResultsMessage: I18N_NO_RESULTS_MESSAGE, @@ -36,7 +41,7 @@ export default { }, data() { return { - searchTerm: this.value, + searchTerm: this.blanked ? '' : this.value, }; }, computed: { diff --git a/app/assets/javascripts/projects/commit/components/form_modal.vue b/app/assets/javascripts/projects/commit/components/form_modal.vue index d9aaa574fecab2..1febe8ceaabb2d 100644 --- a/app/assets/javascripts/projects/commit/components/form_modal.vue +++ b/app/assets/javascripts/projects/commit/components/form_modal.vue @@ -41,6 +41,11 @@ export default { required: false, default: false, }, + isRevert: { + type: Boolean, + required: false, + default: false, + }, primaryActionEventName: { type: String, required: false, @@ -150,7 +155,12 @@ export default { > - + Date: Tue, 6 Dec 2022 14:26:07 -0700 Subject: [PATCH 2/3] Pass branches_dropdown test props as an object --- .../components/branches_dropdown_spec.js | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/spec/frontend/projects/commit/components/branches_dropdown_spec.js b/spec/frontend/projects/commit/components/branches_dropdown_spec.js index e2848e615c308d..6a339f3fe93a2f 100644 --- a/spec/frontend/projects/commit/components/branches_dropdown_spec.js +++ b/spec/frontend/projects/commit/components/branches_dropdown_spec.js @@ -13,7 +13,7 @@ describe('BranchesDropdown', () => { let store; const spyFetchBranches = jest.fn(); - const createComponent = (term, state = { isFetching: false }) => { + const createComponent = (props, state = { isFetching: false }) => { store = new Vuex.Store({ getters: { joinedBranches: () => ['_main_', '_branch_1_', '_branch_2_'], @@ -28,7 +28,8 @@ describe('BranchesDropdown', () => { shallowMount(BranchesDropdown, { store, propsData: { - value: term, + value: props.term, + blanked: props.blanked || false, }, }), ); @@ -48,7 +49,7 @@ describe('BranchesDropdown', () => { describe('On mount', () => { beforeEach(() => { - createComponent(''); + createComponent({ term: '' }); }); it('invokes fetchBranches', () => { @@ -58,13 +59,13 @@ describe('BranchesDropdown', () => { describe('Loading states', () => { it('shows loading icon while fetching', () => { - createComponent('', { isFetching: true }); + createComponent({ term: '' }, { isFetching: true }); expect(findLoading().isVisible()).toBe(true); }); it('does not show loading icon', () => { - createComponent(''); + createComponent({ term: '' }); expect(findLoading().isVisible()).toBe(false); }); @@ -72,7 +73,7 @@ describe('BranchesDropdown', () => { describe('No branches found', () => { beforeEach(() => { - createComponent('_non_existent_branch_'); + createComponent({ term: '_non_existent_branch_' }); }); it('renders empty results message', () => { @@ -90,7 +91,7 @@ describe('BranchesDropdown', () => { describe('Search term is empty', () => { beforeEach(() => { - createComponent(''); + createComponent({ term: '' }); }); it('renders all branches when search term is empty', () => { @@ -107,7 +108,7 @@ describe('BranchesDropdown', () => { describe('When searching', () => { beforeEach(() => { - createComponent(''); + createComponent({ term: '' }); }); it('invokes fetchBranches', async () => { @@ -124,7 +125,7 @@ describe('BranchesDropdown', () => { describe('Branches found', () => { beforeEach(() => { - createComponent('_branch_1_', { branch: '_branch_1_' }); + createComponent({ term: '_branch_1_' }, { branch: '_branch_1_' }); }); it('renders only the branch searched for', () => { @@ -156,7 +157,7 @@ describe('BranchesDropdown', () => { describe('Case insensitive for search term', () => { beforeEach(() => { - createComponent('_BrAnCh_1_'); + createComponent({ term: '_BrAnCh_1_' }); }); it('renders only the branch searched for', () => { -- GitLab From 8946b4de80530f06086db1c9204c004017b733df Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 6 Dec 2022 15:13:08 -0700 Subject: [PATCH 3/3] Make sure that blanked branches_dropdown renders all branches But also selects the right branch --- .../components/branches_dropdown_spec.js | 35 ++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/spec/frontend/projects/commit/components/branches_dropdown_spec.js b/spec/frontend/projects/commit/components/branches_dropdown_spec.js index 6a339f3fe93a2f..a84dd246f5d0a6 100644 --- a/spec/frontend/projects/commit/components/branches_dropdown_spec.js +++ b/spec/frontend/projects/commit/components/branches_dropdown_spec.js @@ -28,7 +28,7 @@ describe('BranchesDropdown', () => { shallowMount(BranchesDropdown, { store, propsData: { - value: props.term, + value: props.value, blanked: props.blanked || false, }, }), @@ -49,23 +49,40 @@ describe('BranchesDropdown', () => { describe('On mount', () => { beforeEach(() => { - createComponent({ term: '' }); + createComponent({ value: '' }); }); it('invokes fetchBranches', () => { expect(spyFetchBranches).toHaveBeenCalled(); }); + + describe('with a value but visually blanked', () => { + beforeEach(() => { + createComponent({ value: '_main_', blanked: true }, { branch: '_main_' }); + }); + + it('renders all branches', () => { + expect(findAllDropdownItems()).toHaveLength(3); + expect(findDropdownItemByIndex(0).text()).toBe('_main_'); + expect(findDropdownItemByIndex(1).text()).toBe('_branch_1_'); + expect(findDropdownItemByIndex(2).text()).toBe('_branch_2_'); + }); + + it('selects the active branch', () => { + expect(wrapper.vm.isSelected('_main_')).toBe(true); + }); + }); }); describe('Loading states', () => { it('shows loading icon while fetching', () => { - createComponent({ term: '' }, { isFetching: true }); + createComponent({ value: '' }, { isFetching: true }); expect(findLoading().isVisible()).toBe(true); }); it('does not show loading icon', () => { - createComponent({ term: '' }); + createComponent({ value: '' }); expect(findLoading().isVisible()).toBe(false); }); @@ -73,7 +90,7 @@ describe('BranchesDropdown', () => { describe('No branches found', () => { beforeEach(() => { - createComponent({ term: '_non_existent_branch_' }); + createComponent({ value: '_non_existent_branch_' }); }); it('renders empty results message', () => { @@ -91,7 +108,7 @@ describe('BranchesDropdown', () => { describe('Search term is empty', () => { beforeEach(() => { - createComponent({ term: '' }); + createComponent({ value: '' }); }); it('renders all branches when search term is empty', () => { @@ -108,7 +125,7 @@ describe('BranchesDropdown', () => { describe('When searching', () => { beforeEach(() => { - createComponent({ term: '' }); + createComponent({ value: '' }); }); it('invokes fetchBranches', async () => { @@ -125,7 +142,7 @@ describe('BranchesDropdown', () => { describe('Branches found', () => { beforeEach(() => { - createComponent({ term: '_branch_1_' }, { branch: '_branch_1_' }); + createComponent({ value: '_branch_1_' }, { branch: '_branch_1_' }); }); it('renders only the branch searched for', () => { @@ -157,7 +174,7 @@ describe('BranchesDropdown', () => { describe('Case insensitive for search term', () => { beforeEach(() => { - createComponent({ term: '_BrAnCh_1_' }); + createComponent({ value: '_BrAnCh_1_' }); }); it('renders only the branch searched for', () => { -- GitLab