From 94c811429bbab7a44062127e9d9a92113090b873 Mon Sep 17 00:00:00 2001 From: Nataliia Radina Date: Thu, 3 Nov 2022 10:14:06 +0100 Subject: [PATCH 1/4] Improve delete merged branches modal UX Add explanation text and input to confirm amount of branches Changelog: fixed --- .../components/delete_merged_branches.vue | 236 +++++++ .../branches/init_delete_merged_branches.js | 24 + .../pages/projects/branches/index/index.js | 2 + app/views/projects/branches/index.html.haml | 16 +- locale/gitlab.pot | 25 +- qa/qa/page/project/branches/show.rb | 16 +- .../add_list_delete_branches_spec.rb | 2 +- .../delete_merged_branches_spec.js.snap | 588 ++++++++++++++++++ .../components/delete_merged_branches_spec.js | 193 ++++++ spec/frontend/branches/mock_data.js | 23 + 10 files changed, 1105 insertions(+), 20 deletions(-) create mode 100644 app/assets/javascripts/branches/components/delete_merged_branches.vue create mode 100644 app/assets/javascripts/branches/init_delete_merged_branches.js create mode 100644 spec/frontend/branches/components/__snapshots__/delete_merged_branches_spec.js.snap create mode 100644 spec/frontend/branches/components/delete_merged_branches_spec.js create mode 100644 spec/frontend/branches/mock_data.js diff --git a/app/assets/javascripts/branches/components/delete_merged_branches.vue b/app/assets/javascripts/branches/components/delete_merged_branches.vue new file mode 100644 index 00000000000000..85bd8fc0df36f4 --- /dev/null +++ b/app/assets/javascripts/branches/components/delete_merged_branches.vue @@ -0,0 +1,236 @@ + + + diff --git a/app/assets/javascripts/branches/init_delete_merged_branches.js b/app/assets/javascripts/branches/init_delete_merged_branches.js new file mode 100644 index 00000000000000..4c137d8c4851e2 --- /dev/null +++ b/app/assets/javascripts/branches/init_delete_merged_branches.js @@ -0,0 +1,24 @@ +import Vue from 'vue'; +import DeleteMergedBranches from '~/branches/components/delete_merged_branches.vue'; + +export default function initDeleteMergedBranchesModal() { + const el = document.querySelector('.js-delete-merged-branches'); + if (!el) { + return false; + } + + const { formPath, defaultBranch, branches } = el.dataset; + + return new Vue({ + el, + render(createComponent) { + return createComponent(DeleteMergedBranches, { + props: { + branches: JSON.parse(branches), + formPath, + defaultBranch, + }, + }); + }, + }); +} diff --git a/app/assets/javascripts/pages/projects/branches/index/index.js b/app/assets/javascripts/pages/projects/branches/index/index.js index f3530b468450f6..ac5e0b28dd120c 100644 --- a/app/assets/javascripts/pages/projects/branches/index/index.js +++ b/app/assets/javascripts/pages/projects/branches/index/index.js @@ -3,6 +3,7 @@ import BranchSortDropdown from '~/branches/branch_sort_dropdown'; import initDiverganceGraph from '~/branches/divergence_graph'; import initDeleteBranchButton from '~/branches/init_delete_branch_button'; import initDeleteBranchModal from '~/branches/init_delete_branch_modal'; +import initDeleteMergedBranches from '~/branches/init_delete_merged_branches'; const { divergingCountsEndpoint, defaultBranch } = document.querySelector( '.js-branch-list', @@ -11,6 +12,7 @@ const { divergingCountsEndpoint, defaultBranch } = document.querySelector( initDiverganceGraph(divergingCountsEndpoint, defaultBranch); BranchSortDropdown(); initDeprecatedRemoveRowBehavior(); +initDeleteMergedBranches(); document .querySelectorAll('.js-delete-branch-button') diff --git a/app/views/projects/branches/index.html.haml b/app/views/projects/branches/index.html.haml index 82276938d45d47..4d944a2e9cca2a 100644 --- a/app/views/projects/branches/index.html.haml +++ b/app/views/projects/branches/index.html.haml @@ -13,16 +13,12 @@ #js-branches-sort-dropdown{ data: { project_branches_filtered_path: project_branches_path(@project, state: 'all'), sort_options: branches_sort_options_hash.to_json, mode: @mode } } - if can? current_user, :push_code, @project - = link_to project_merged_branches_path(@project), - class: 'gl-button btn btn-danger btn-danger-secondary has-tooltip', - title: s_("Branches|Delete all branches that are merged into '%{default_branch}'") % { default_branch: @project.repository.root_ref }, - method: :delete, - aria: { label: s_('Branches|Delete merged branches') }, - data: { confirm: s_('Branches|Deleting the merged branches cannot be undone. Are you sure?'), - confirm_btn_variant: 'danger', - container: 'body', - qa_selector: 'delete_merged_branches_link' } do - = s_('Branches|Delete merged branches') + - if repository.merged_branch_names && repository.merged_branch_names.any? + .js-delete-merged-branches{ data: { + default_branch: @project.repository.root_ref, + branches: repository.merged_branch_names.to_a, + form_path: project_merged_branches_path(@project) } + } = link_to new_project_branch_path(@project), class: 'gl-button btn btn-confirm' do = s_('Branches|New branch') diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0bf01382655a92..2720f9a861281c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6996,6 +6996,9 @@ msgstr "" msgid "Branches: %{source_branch} → %{target_branch}" msgstr "" +msgid "Branches|A branch won't be deleted if it is protected or assosiated with an open merge request." +msgstr "" + msgid "Branches|Active" msgstr "" @@ -7017,7 +7020,10 @@ msgstr "" msgid "Branches|Compare" msgstr "" -msgid "Branches|Delete all branches that are merged into '%{default_branch}'" +msgid "Branches|Delete all branches that are merged into '%{defaultBranch}'" +msgstr "" + +msgid "Branches|Delete all merged branches?" msgstr "" msgid "Branches|Delete branch" @@ -7038,7 +7044,7 @@ msgstr "" msgid "Branches|Deleting the %{strongStart}%{branchName}%{strongEnd} branch cannot be undone. Are you sure?" msgstr "" -msgid "Branches|Deleting the merged branches cannot be undone. Are you sure?" +msgid "Branches|Enter the number of merged branches to be deleted to confirm:" msgstr "" msgid "Branches|Filter by branch name" @@ -7062,12 +7068,18 @@ msgstr "" msgid "Branches|Please type the following to confirm:" msgstr "" +msgid "Branches|Show Less" +msgstr "" + msgid "Branches|Show active branches" msgstr "" msgid "Branches|Show all branches" msgstr "" +msgid "Branches|Show more" +msgstr "" + msgid "Branches|Show more active branches" msgstr "" @@ -7095,6 +7107,9 @@ msgstr "" msgid "Branches|This branch hasn't been merged into %{defaultBranchName}. To avoid data loss, consider merging this branch before deleting it." msgstr "" +msgid "Branches|This bulk action is %{strongStart}permanent and cannot be undone or recovered%{strongEnd}." +msgstr "" + msgid "Branches|To discard the local changes and overwrite the branch with the upstream version, delete it here and choose 'Update Now' above." msgstr "" @@ -7107,6 +7122,12 @@ msgstr "" msgid "Branches|Yes, delete protected branch" msgstr "" +msgid "Branches|You are about to %{strongStart} delete %{branchesAmount} branch %{strongEnd} that was merged into %{codeStart}%{defaultBranch}%{codeEnd}." +msgstr "" + +msgid "Branches|You are about to %{strongStart} delete %{branchesAmount} branches %{strongEnd} that were merged into %{codeStart}%{defaultBranch}%{codeEnd}." +msgstr "" + msgid "Branches|You're about to permanently delete the branch %{branchName}." msgstr "" diff --git a/qa/qa/page/project/branches/show.rb b/qa/qa/page/project/branches/show.rb index 22b960b47ceead..7163bc7464d120 100644 --- a/qa/qa/page/project/branches/show.rb +++ b/qa/qa/page/project/branches/show.rb @@ -5,8 +5,6 @@ module Page module Project module Branches class Show < Page::Base - include Page::Component::ConfirmModal - view 'app/assets/javascripts/branches/components/delete_branch_button.vue' do element :delete_branch_button end @@ -25,8 +23,10 @@ class Show < Page::Base element :all_branches_container end - view 'app/views/projects/branches/index.html.haml' do - element :delete_merged_branches_link + view 'app/assets/javascripts/branches/components/delete_merged_branches.vue' do + element :delete_merged_branches_button + element :delete_merged_branches_input + element :delete_merged_branches_confirmation_button end def delete_branch(branch_name) @@ -53,9 +53,11 @@ def has_branch_with_badge?(branch_name, badge) end end - def delete_merged_branches - click_element(:delete_merged_branches_link) - click_confirmation_ok_button + def delete_merged_branches(branches_length) + click_element(:delete_merged_branches_button) + fill_element(:delete_merged_branches_input, branches_length) + click_element(:delete_merged_branches_confirmation_button) + finished_loading? end end end diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/add_list_delete_branches_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/add_list_delete_branches_spec.rb index 849022f5a937f1..8864431b139ae8 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/add_list_delete_branches_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/add_list_delete_branches_spec.rb @@ -76,7 +76,7 @@ module QA expect(branches_page).to have_no_branch(third_branch) - branches_page.delete_merged_branches + branches_page.delete_merged_branches(1) expect(branches_page).to have_content( 'Merged branches are being deleted. This can take some time depending on the number of branches. Please refresh the page to see changes.' diff --git a/spec/frontend/branches/components/__snapshots__/delete_merged_branches_spec.js.snap b/spec/frontend/branches/components/__snapshots__/delete_merged_branches_spec.js.snap new file mode 100644 index 00000000000000..92180bb884f4d7 --- /dev/null +++ b/spec/frontend/branches/components/__snapshots__/delete_merged_branches_spec.js.snap @@ -0,0 +1,588 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Delete merged branches component Delete merged branches confirmation modal renders correct message and number of collapsed items and matches the snapshot 1`] = ` +
+ + + + + + + Delete merged branches + + + + +
+
+

+ You are about to + + delete 1 branch + + that was merged into + + master + + . +

+ +
+
    +
  • + + branch + +
    • +
+ + +
+ +

+ + A branch won't be deleted if it is protected or assosiated with an open merge request. + +

+ +

+ This bulk action is + + permanent and cannot be undone or recovered + + . +

+ +

+ + Enter the number of merged branches to be deleted to confirm: + + +

+ + + + +
+
+ + + + + + + + Cancel + + + + + + + + + + + Delete merged branches + + +
+
+
+`; + +exports[`Delete merged branches component Delete merged branches confirmation modal renders correct message and number of collapsed items and matches the snapshot 2`] = ` +
+ + + + + + + Delete merged branches + + + + +
+
+

+ You are about to + + delete 5 branches + + that were merged into + + master + + . +

+ +
+
    +
  • + + branch1 + +
    • +
  • + + branch2 + +
    • +
  • + + branch3 + +
    • +
  • + + branch4 + +
    • +
  • + + branch5 + +
    • +
+ + +
+ +

+ + A branch won't be deleted if it is protected or assosiated with an open merge request. + +

+ +

+ This bulk action is + + permanent and cannot be undone or recovered + + . +

+ +

+ + Enter the number of merged branches to be deleted to confirm: + + +

+ + + + +
+
+ + + + + + + + Cancel + + + + + + + + + + + Delete merged branches + + +
+
+
+`; + +exports[`Delete merged branches component Delete merged branches confirmation modal renders correct message and number of collapsed items and matches the snapshot 3`] = ` +
+ + + + + + + Delete merged branches + + + + +
+
+

+ You are about to + + delete 12 branches + + that were merged into + + master + + . +

+ +
+
    +
  • + + branch1 + +
    • +
  • + + branch2 + +
    • +
  • + + branch3 + +
    • +
  • + + branch4 + +
    • +
  • + + branch5 + +
    • +
  • + + branch6 + +
    • +
  • + + branch7 + +
    • +
  • + + branch8 + +
    • +
  • + + branch9 + +
    • +
  • + + branch10 + +
    • +
+ + +
    +
  • + + branch11 + +
    • +
  • + + branch12 + +
    • +
+ + +
+ + + + + + + + Show more + + + +
+
+ +

+ + A branch won't be deleted if it is protected or assosiated with an open merge request. + +

+ +

+ This bulk action is + + permanent and cannot be undone or recovered + + . +

+ +

+ + Enter the number of merged branches to be deleted to confirm: + + +

+ + + + +
+
+ + + + + + + + Cancel + + + + + + + + + + + Delete merged branches + + +
+
+
+`; diff --git a/spec/frontend/branches/components/delete_merged_branches_spec.js b/spec/frontend/branches/components/delete_merged_branches_spec.js new file mode 100644 index 00000000000000..27ec9e46efa2d3 --- /dev/null +++ b/spec/frontend/branches/components/delete_merged_branches_spec.js @@ -0,0 +1,193 @@ +import { GlButton, GlModal, GlFormInput, GlSprintf } from '@gitlab/ui'; +import { mount } from '@vue/test-utils'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import { stubComponent } from 'helpers/stub_component'; +import waitForPromises from 'helpers/wait_for_promises'; +import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; +import DeleteMergedBranches, { + i18n, + maxAmountOfVisibleBranches, +} from '~/branches/components/delete_merged_branches.vue'; +import { + formPath, + branches, + singleBranch, + branchesWithCollapse, + propsDataMock, +} from '../mock_data'; + +jest.mock('~/lib/utils/csrf', () => ({ token: 'mock-csrf-token' })); + +let wrapper; + +const stubsData = { + GlModal: stubComponent(GlModal, { + template: + '
', + }), + GlButton, + GlFormInput, + GlSprintf, +}; + +const createComponent = (props = {}, mountFn = shallowMountExtended, stubs = {}) => { + wrapper = mountFn(DeleteMergedBranches, { + propsData: { + ...propsDataMock, + ...props, + }, + directives: { + GlTooltip: createMockDirective(), + }, + stubs, + }); +}; + +const findDeleteButton = () => wrapper.findComponent(GlButton); +const findModal = () => wrapper.findComponent(GlModal); +const findConfirmationButton = () => + wrapper.findByTestId('delete-merged-branches-confirmation-button'); +const findCancelButton = () => wrapper.findByTestId('delete-merged-branches-cancel-button'); +const findCollapse = () => wrapper.findByTestId('collapse'); +const findVisibleBranches = () => wrapper.findAllByTestId('visible-branch'); +const findCollapsedBranches = () => wrapper.findAllByTestId('collapsed-branch'); +const findCollapseToggle = () => wrapper.findByTestId('collapse-toggle'); +const findFormInput = () => wrapper.findComponent(GlFormInput); +const findForm = () => wrapper.find('form'); + +describe('Delete merged branches component', () => { + beforeEach(() => { + createComponent({ branches }); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + describe('Delete merged branches button', () => { + it('has correct attributes, text and tooltip', () => { + expect(findDeleteButton().attributes()).toMatchObject({ + category: 'secondary', + variant: 'danger', + }); + + expect(findDeleteButton().text()).toBe(i18n.deleteButtonText); + }); + + it('displays a tooltip', () => { + const tooltip = getBinding(findDeleteButton().element, 'gl-tooltip'); + + expect(tooltip).toBeDefined(); + expect(tooltip.value).toBe(wrapper.vm.buttonTooltipText); + }); + + it('opens modal when clicked', () => { + createComponent({ branches }, mount); + jest.spyOn(wrapper.vm.$refs.modal, 'show'); + findDeleteButton().trigger('click'); + + expect(wrapper.vm.$refs.modal.show).toHaveBeenCalled(); + }); + }); + + describe('Delete merged branches confirmation modal', () => { + beforeEach(() => { + createComponent({ branches }, shallowMountExtended, stubsData); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('renders correct modal title and text', () => { + const permanentEffectWarning = + 'This bulk action is permanent and cannot be undone or recovered.'; + const modalText = findModal().text(); + expect(findModal().props('title')).toBe(i18n.modalTitle); + expect(modalText).toContain(i18n.protectedBranchWarning); + expect(modalText).toContain(permanentEffectWarning); + expect(modalText).toContain(i18n.confirmationMessage); + }); + + it('renders confirm and cancel buttons with correct text', () => { + expect(findConfirmationButton().text()).toContain(i18n.deleteButtonText); + expect(findCancelButton().text()).toContain(i18n.cancelButtonText); + }); + + it('renders form with correct attributes and hiden inputs', () => { + const form = findForm(); + expect(form.attributes()).toEqual({ + action: formPath, + method: 'post', + }); + expect(form.find('input[name="_method"]').attributes('value')).toBe('delete'); + expect(form.find('input[name="authenticity_token"]').attributes('value')).toBe( + 'mock-csrf-token', + ); + }); + + it.each` + branchesData | modalMessage | collapsedBranchesLength + ${singleBranch} | ${'You are about to delete 1 branch that was merged into master.'} | ${0} + ${branches} | ${'You are about to delete 5 branches that were merged into master.'} | ${0} + ${branchesWithCollapse} | ${'You are about to delete 12 branches that were merged into master.'} | ${2} + `( + 'renders correct message and number of collapsed items and matches the snapshot', + ({ branchesData, modalMessage, collapsedBranchesLength }) => { + createComponent({ branches: branchesData }, shallowMountExtended, stubsData); + expect(findModal().text()).toContain(modalMessage); + expect(findCollapsedBranches().length).toBe(collapsedBranchesLength); + expect(wrapper.element).toMatchSnapshot(); + }, + ); + + it('has a disabled confirm button by default', () => { + expect(findConfirmationButton().props('disabled')).toBe(true); + }); + + it('keeps disabled state when wrong input is provided', async () => { + findFormInput().vm.$emit('input', 3); + await waitForPromises(); + expect(findConfirmationButton().props('disabled')).toBe(true); + }); + + it('submits form when correct amount is provided and the confirm button is clicked', async () => { + const submitFormSpy = jest.spyOn(wrapper.vm.$refs.form, 'submit'); + findFormInput().vm.$emit('input', branches.length); + await waitForPromises(); + expect(findDeleteButton().props('disabled')).not.toBe(true); + findConfirmationButton().trigger('click'); + expect(submitFormSpy).toHaveBeenCalled(); + }); + + it('calls hide on the modal when cancel button is clicked', () => { + const closeModalSpy = jest.spyOn(wrapper.vm.$refs.modal, 'hide'); + findCancelButton().trigger('click'); + expect(closeModalSpy).toHaveBeenCalled(); + }); + + describe('when the number of branches is more than maxAmountOfVisibleBranches', () => { + beforeEach(() => { + createComponent({ branches: branchesWithCollapse }, shallowMountExtended, stubsData); + }); + + it('renders collapse component', () => { + expect(findCollapse().exists()).toBe(true); + }); + + it('renders a toggle button with correct text', () => { + expect(findCollapseToggle().exists()).toBe(true); + expect(findCollapseToggle().text()).toBe(i18n.showMore); + }); + + it('renders only specified amount of branches at initial state', () => { + expect(findVisibleBranches().length).toBe(maxAmountOfVisibleBranches); + }); + + it('expands collapse when you click on a toggle', async () => { + await findCollapseToggle().trigger('click'); + expect(findCollapseToggle().text()).toBe(i18n.showLess); + }); + }); + }); +}); diff --git a/spec/frontend/branches/mock_data.js b/spec/frontend/branches/mock_data.js new file mode 100644 index 00000000000000..075fddc4aa3164 --- /dev/null +++ b/spec/frontend/branches/mock_data.js @@ -0,0 +1,23 @@ +export const formPath = '/namespace/project/-/merged_branches'; +const defaultBranch = 'master'; +export const singleBranch = ['branch']; +export const branches = ['branch1', 'branch2', 'branch3', 'branch4', 'branch5']; +export const branchesWithCollapse = [ + 'branch1', + 'branch2', + 'branch3', + 'branch4', + 'branch5', + 'branch6', + 'branch7', + 'branch8', + 'branch9', + 'branch10', + 'branch11', + 'branch12', +]; + +export const propsDataMock = { + formPath, + defaultBranch, +}; -- GitLab From 1eb5ab96235ad6e7e7824e48f7e7462c74e556b2 Mon Sep 17 00:00:00 2001 From: Nataliia Radina Date: Mon, 7 Nov 2022 17:54:34 +0100 Subject: [PATCH 2/4] Fix typo, translations handling improvement --- .../components/delete_merged_branches.vue | 19 ++++++------------- locale/gitlab.pot | 12 +++++++----- .../delete_merged_branches_spec.js.snap | 6 +++--- 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/app/assets/javascripts/branches/components/delete_merged_branches.vue b/app/assets/javascripts/branches/components/delete_merged_branches.vue index 85bd8fc0df36f4..cdfbe40937fb12 100644 --- a/app/assets/javascripts/branches/components/delete_merged_branches.vue +++ b/app/assets/javascripts/branches/components/delete_merged_branches.vue @@ -8,7 +8,7 @@ import { GlCollapse, } from '@gitlab/ui'; import csrf from '~/lib/utils/csrf'; -import { sprintf, s__, __ } from '~/locale'; +import { sprintf, s__, __, n__ } from '~/locale'; export const maxAmountOfVisibleBranches = 10; export const i18n = { @@ -17,13 +17,10 @@ export const i18n = { modalTitle: s__('Branches|Delete all merged branches?'), modalTitleProtectedBranch: s__('Branches|Delete protected branch. Are you ABSOLUTELY SURE?'), modalMessage: s__( - 'Branches|You are about to %{strongStart} delete %{branchesAmount} branches %{strongEnd} that were merged into %{codeStart}%{defaultBranch}%{codeEnd}.', - ), - modalMessageSingle: s__( - 'Branches|You are about to %{strongStart} delete %{branchesAmount} branch %{strongEnd} that was merged into %{codeStart}%{defaultBranch}%{codeEnd}.', + 'Branches|You are about to %{strongStart} delete %{branchesAmount} %{subject} %{strongEnd} that %{verb} merged into %{codeStart}%{defaultBranch}%{codeEnd}.', ), protectedBranchWarning: s__( - "Branches|A branch won't be deleted if it is protected or assosiated with an open merge request.", + "Branches|A branch won't be deleted if it is protected or associated with an open merge request.", ), permanentEffectWarning: s__( 'Branches|This bulk action is %{strongStart}permanent and cannot be undone or recovered%{strongEnd}.', @@ -56,7 +53,6 @@ export default { formPath: { type: String, required: true, - default: '', }, defaultBranch: { type: String, @@ -91,14 +87,11 @@ export default { return sprintf(this.$options.i18n.buttonTooltipText, { defaultBranch: this.defaultBranch }); }, modalMessage() { - const message = - this.branchesAmount === 1 - ? this.$options.i18n.modalMessageSingle - : this.$options.i18n.modalMessage; - - return sprintf(message, { + return sprintf(this.$options.i18n.modalMessage, { defaultBranch: this.defaultBranch, branchesAmount: this.branchesAmount, + subject: n__('branch', 'branches', this.branchesAmount), + verb: n__('was', 'were', this.branchesAmount), }); }, branchesAmountConfirmed() { diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2720f9a861281c..83110a1860ef45 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6996,7 +6996,7 @@ msgstr "" msgid "Branches: %{source_branch} → %{target_branch}" msgstr "" -msgid "Branches|A branch won't be deleted if it is protected or assosiated with an open merge request." +msgid "Branches|A branch won't be deleted if it is protected or associated with an open merge request." msgstr "" msgid "Branches|Active" @@ -7122,10 +7122,7 @@ msgstr "" msgid "Branches|Yes, delete protected branch" msgstr "" -msgid "Branches|You are about to %{strongStart} delete %{branchesAmount} branch %{strongEnd} that was merged into %{codeStart}%{defaultBranch}%{codeEnd}." -msgstr "" - -msgid "Branches|You are about to %{strongStart} delete %{branchesAmount} branches %{strongEnd} that were merged into %{codeStart}%{defaultBranch}%{codeEnd}." +msgid "Branches|You are about to %{strongStart} delete %{branchesAmount} %{subject} %{strongEnd} that %{verb} merged into %{codeStart}%{defaultBranch}%{codeEnd}." msgstr "" msgid "Branches|You're about to permanently delete the branch %{branchName}." @@ -49425,6 +49422,11 @@ msgstr "" msgid "vulnerability|dismissed" msgstr "" +msgid "was" +msgid_plural "were" +msgstr[0] "" +msgstr[1] "" + msgid "was scheduled to merge after pipeline succeeds by" msgstr "" diff --git a/spec/frontend/branches/components/__snapshots__/delete_merged_branches_spec.js.snap b/spec/frontend/branches/components/__snapshots__/delete_merged_branches_spec.js.snap index 92180bb884f4d7..e80d0104f21123 100644 --- a/spec/frontend/branches/components/__snapshots__/delete_merged_branches_spec.js.snap +++ b/spec/frontend/branches/components/__snapshots__/delete_merged_branches_spec.js.snap @@ -61,7 +61,7 @@ exports[`Delete merged branches component Delete merged branches confirmation mo

- A branch won't be deleted if it is protected or assosiated with an open merge request. + A branch won't be deleted if it is protected or associated with an open merge request.

@@ -239,7 +239,7 @@ exports[`Delete merged branches component Delete merged branches confirmation mo

- A branch won't be deleted if it is protected or assosiated with an open merge request. + A branch won't be deleted if it is protected or associated with an open merge request.

@@ -498,7 +498,7 @@ exports[`Delete merged branches component Delete merged branches confirmation mo

- A branch won't be deleted if it is protected or assosiated with an open merge request. + A branch won't be deleted if it is protected or associated with an open merge request.

-- GitLab From 83bb78ab783a40df94ae049c3566af71758accb9 Mon Sep 17 00:00:00 2001 From: Nataliia Radina Date: Tue, 15 Nov 2022 19:56:41 +0100 Subject: [PATCH 3/4] Update modal according to new design --- .../components/delete_merged_branches.vue | 109 +--- .../branches/init_delete_merged_branches.js | 3 +- app/views/projects/branches/index.html.haml | 10 +- locale/gitlab.pot | 18 +- .../add_list_delete_branches_spec.rb | 2 +- .../delete_merged_branches_spec.js.snap | 469 +----------------- .../components/delete_merged_branches_spec.js | 76 +-- spec/frontend/branches/mock_data.js | 16 - 8 files changed, 57 insertions(+), 646 deletions(-) diff --git a/app/assets/javascripts/branches/components/delete_merged_branches.vue b/app/assets/javascripts/branches/components/delete_merged_branches.vue index cdfbe40937fb12..8531e2dec76cec 100644 --- a/app/assets/javascripts/branches/components/delete_merged_branches.vue +++ b/app/assets/javascripts/branches/components/delete_merged_branches.vue @@ -1,23 +1,17 @@ @@ -134,7 +97,6 @@ export default { @@ -149,36 +111,9 @@ export default {

-
-
    -
  • - {{ branch }} -
    • -
- -
+

+ {{ $options.i18n.notVisibleBranchesWarning }} +

{{ $options.i18n.protectedBranchWarning }}

@@ -190,12 +125,16 @@ export default {

- {{ $options.i18n.confirmationMessage }} + + + -

+
You are about to - delete 1 branch + delete all branches - that was merged into + that were merged into master .

-
-
    -
  • - - branch - -
    • -
- - -
- -

- - A branch won't be deleted if it is protected or associated with an open merge request. - -

-

- This bulk action is - - permanent and cannot be undone or recovered - - . -

- -

- - Enter the number of merged branches to be deleted to confirm: - - -

- - - - -
-
- - - - - - - - Cancel - - - - - - - - - - Delete merged branches - - -
-
-
-`; - -exports[`Delete merged branches component Delete merged branches confirmation modal renders correct message and number of collapsed items and matches the snapshot 2`] = ` -
- - - - + This may include merged branches that are not visible on the current screen. - - Delete merged branches - - - - -
-
-

- You are about to - - delete 5 branches - - that were merged into - - master - - .

-
-
    -
  • - - branch1 - -
    • -
  • - - branch2 - -
    • -
  • - - branch3 - -
    • -
  • - - branch4 - -
    • -
  • - - branch5 - -
    • -
- - -
-

A branch won't be deleted if it is protected or associated with an open merge request. @@ -252,276 +60,19 @@ exports[`Delete merged branches component Delete merged branches confirmation mo

- - Enter the number of merged branches to be deleted to confirm: - - -

- - - - -
-
- - - - - - - - Cancel - - - - - - - - - - - Delete merged branches - - -
-
-
-`; - -exports[`Delete merged branches component Delete merged branches confirmation modal renders correct message and number of collapsed items and matches the snapshot 3`] = ` -
- - - - - - - Delete merged branches - - - - -
-
-

- You are about to - - delete 12 branches - - that were merged into + Plese type the following to confirm: - master + delete - . -

- -
-
    -
  • - - branch1 - -
    • -
  • - - branch2 - -
    • -
  • - - branch3 - -
    • -
  • - - branch4 - -
    • -
  • - - branch5 - -
    • -
  • - - branch6 - -
    • -
  • - - branch7 - -
    • -
  • - - branch8 - -
    • -
  • - - branch9 - -
    • -
  • - - branch10 - -
    • -
- - -
    -
  • - - branch11 - -
    • -
  • - - branch12 - -
    • -
- - -
- - - - - - - - Show more - - - -
-
- -

- - A branch won't be deleted if it is protected or associated with an open merge request. - -

- -

- This bulk action is - - permanent and cannot be undone or recovered - - . -

- -

- - Enter the number of merged branches to be deleted to confirm: - + .

diff --git a/spec/frontend/branches/components/delete_merged_branches_spec.js b/spec/frontend/branches/components/delete_merged_branches_spec.js index 27ec9e46efa2d3..67a1207ea623aa 100644 --- a/spec/frontend/branches/components/delete_merged_branches_spec.js +++ b/spec/frontend/branches/components/delete_merged_branches_spec.js @@ -4,17 +4,8 @@ import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import { stubComponent } from 'helpers/stub_component'; import waitForPromises from 'helpers/wait_for_promises'; import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; -import DeleteMergedBranches, { - i18n, - maxAmountOfVisibleBranches, -} from '~/branches/components/delete_merged_branches.vue'; -import { - formPath, - branches, - singleBranch, - branchesWithCollapse, - propsDataMock, -} from '../mock_data'; +import DeleteMergedBranches, { i18n } from '~/branches/components/delete_merged_branches.vue'; +import { formPath, propsDataMock } from '../mock_data'; jest.mock('~/lib/utils/csrf', () => ({ token: 'mock-csrf-token' })); @@ -30,11 +21,10 @@ const stubsData = { GlSprintf, }; -const createComponent = (props = {}, mountFn = shallowMountExtended, stubs = {}) => { +const createComponent = (mountFn = shallowMountExtended, stubs = {}) => { wrapper = mountFn(DeleteMergedBranches, { propsData: { ...propsDataMock, - ...props, }, directives: { GlTooltip: createMockDirective(), @@ -48,16 +38,12 @@ const findModal = () => wrapper.findComponent(GlModal); const findConfirmationButton = () => wrapper.findByTestId('delete-merged-branches-confirmation-button'); const findCancelButton = () => wrapper.findByTestId('delete-merged-branches-cancel-button'); -const findCollapse = () => wrapper.findByTestId('collapse'); -const findVisibleBranches = () => wrapper.findAllByTestId('visible-branch'); -const findCollapsedBranches = () => wrapper.findAllByTestId('collapsed-branch'); -const findCollapseToggle = () => wrapper.findByTestId('collapse-toggle'); const findFormInput = () => wrapper.findComponent(GlFormInput); const findForm = () => wrapper.find('form'); describe('Delete merged branches component', () => { beforeEach(() => { - createComponent({ branches }); + createComponent(); }); afterEach(() => { @@ -82,7 +68,7 @@ describe('Delete merged branches component', () => { }); it('opens modal when clicked', () => { - createComponent({ branches }, mount); + createComponent(mount); jest.spyOn(wrapper.vm.$refs.modal, 'show'); findDeleteButton().trigger('click'); @@ -92,7 +78,7 @@ describe('Delete merged branches component', () => { describe('Delete merged branches confirmation modal', () => { beforeEach(() => { - createComponent({ branches }, shallowMountExtended, stubsData); + createComponent(shallowMountExtended, stubsData); }); afterEach(() => { @@ -100,13 +86,10 @@ describe('Delete merged branches component', () => { }); it('renders correct modal title and text', () => { - const permanentEffectWarning = - 'This bulk action is permanent and cannot be undone or recovered.'; const modalText = findModal().text(); expect(findModal().props('title')).toBe(i18n.modalTitle); + expect(modalText).toContain(i18n.notVisibleBranchesWarning); expect(modalText).toContain(i18n.protectedBranchWarning); - expect(modalText).toContain(permanentEffectWarning); - expect(modalText).toContain(i18n.confirmationMessage); }); it('renders confirm and cancel buttons with correct text', () => { @@ -126,34 +109,23 @@ describe('Delete merged branches component', () => { ); }); - it.each` - branchesData | modalMessage | collapsedBranchesLength - ${singleBranch} | ${'You are about to delete 1 branch that was merged into master.'} | ${0} - ${branches} | ${'You are about to delete 5 branches that were merged into master.'} | ${0} - ${branchesWithCollapse} | ${'You are about to delete 12 branches that were merged into master.'} | ${2} - `( - 'renders correct message and number of collapsed items and matches the snapshot', - ({ branchesData, modalMessage, collapsedBranchesLength }) => { - createComponent({ branches: branchesData }, shallowMountExtended, stubsData); - expect(findModal().text()).toContain(modalMessage); - expect(findCollapsedBranches().length).toBe(collapsedBranchesLength); - expect(wrapper.element).toMatchSnapshot(); - }, - ); + it('matches snapshot', () => { + expect(wrapper.element).toMatchSnapshot(); + }); it('has a disabled confirm button by default', () => { expect(findConfirmationButton().props('disabled')).toBe(true); }); it('keeps disabled state when wrong input is provided', async () => { - findFormInput().vm.$emit('input', 3); + findFormInput().vm.$emit('input', 'hello'); await waitForPromises(); expect(findConfirmationButton().props('disabled')).toBe(true); }); it('submits form when correct amount is provided and the confirm button is clicked', async () => { const submitFormSpy = jest.spyOn(wrapper.vm.$refs.form, 'submit'); - findFormInput().vm.$emit('input', branches.length); + findFormInput().vm.$emit('input', 'delete'); await waitForPromises(); expect(findDeleteButton().props('disabled')).not.toBe(true); findConfirmationButton().trigger('click'); @@ -165,29 +137,5 @@ describe('Delete merged branches component', () => { findCancelButton().trigger('click'); expect(closeModalSpy).toHaveBeenCalled(); }); - - describe('when the number of branches is more than maxAmountOfVisibleBranches', () => { - beforeEach(() => { - createComponent({ branches: branchesWithCollapse }, shallowMountExtended, stubsData); - }); - - it('renders collapse component', () => { - expect(findCollapse().exists()).toBe(true); - }); - - it('renders a toggle button with correct text', () => { - expect(findCollapseToggle().exists()).toBe(true); - expect(findCollapseToggle().text()).toBe(i18n.showMore); - }); - - it('renders only specified amount of branches at initial state', () => { - expect(findVisibleBranches().length).toBe(maxAmountOfVisibleBranches); - }); - - it('expands collapse when you click on a toggle', async () => { - await findCollapseToggle().trigger('click'); - expect(findCollapseToggle().text()).toBe(i18n.showLess); - }); - }); }); }); diff --git a/spec/frontend/branches/mock_data.js b/spec/frontend/branches/mock_data.js index 075fddc4aa3164..9e8839d8ce9dbf 100644 --- a/spec/frontend/branches/mock_data.js +++ b/spec/frontend/branches/mock_data.js @@ -1,21 +1,5 @@ export const formPath = '/namespace/project/-/merged_branches'; const defaultBranch = 'master'; -export const singleBranch = ['branch']; -export const branches = ['branch1', 'branch2', 'branch3', 'branch4', 'branch5']; -export const branchesWithCollapse = [ - 'branch1', - 'branch2', - 'branch3', - 'branch4', - 'branch5', - 'branch6', - 'branch7', - 'branch8', - 'branch9', - 'branch10', - 'branch11', - 'branch12', -]; export const propsDataMock = { formPath, -- GitLab From 8bd81a80a4fe4eb5ab4cb064a149b4f88ee28c65 Mon Sep 17 00:00:00 2001 From: Nataliia Radina Date: Wed, 16 Nov 2022 09:50:50 +0100 Subject: [PATCH 4/4] Remove obsolete wrapper destroy --- .../branches/components/delete_merged_branches.vue | 7 +++++-- .../components/delete_merged_branches_spec.js | 14 ++++++++------ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/branches/components/delete_merged_branches.vue b/app/assets/javascripts/branches/components/delete_merged_branches.vue index 8531e2dec76cec..70974f2e725c20 100644 --- a/app/assets/javascripts/branches/components/delete_merged_branches.vue +++ b/app/assets/javascripts/branches/components/delete_merged_branches.vue @@ -73,7 +73,9 @@ export default { this.$refs.modal.show(); }, submitForm() { - this.$refs.form.submit(); + if (!this.isDeleteButtonDisabled) { + this.$refs.form.submit(); + } }, closeModal() { this.$refs.modal.hide(); @@ -100,7 +102,7 @@ export default { modal-id="delete-merged-branches" :title="$options.i18n.modalTitle" > - +