diff --git a/.rubocop_todo/rspec/factory_bot/avoid_create.yml b/.rubocop_todo/rspec/factory_bot/avoid_create.yml index 8ead0688816cd9b0ab62f0bb7f4dcf4db1b885bf..e8a8f50e999cdbadb7dbb91aa0920a2faf4515b3 100644 --- a/.rubocop_todo/rspec/factory_bot/avoid_create.yml +++ b/.rubocop_todo/rspec/factory_bot/avoid_create.yml @@ -265,7 +265,6 @@ RSpec/FactoryBot/AvoidCreate: - 'spec/helpers/gitlab_routing_helper_spec.rb' - 'spec/helpers/graph_helper_spec.rb' - 'spec/helpers/groups/group_members_helper_spec.rb' - - 'spec/helpers/groups/settings_helper_spec.rb' - 'spec/helpers/groups_helper_spec.rb' - 'spec/helpers/ide_helper_spec.rb' - 'spec/helpers/import_helper_spec.rb' diff --git a/.rubocop_todo/rspec/feature_category.yml b/.rubocop_todo/rspec/feature_category.yml index eea383d73997fd6d0e43800a080c1c5f9be4cd20..f0d250bf9eff1d59c81feeb0f383589ca1cc4de8 100644 --- a/.rubocop_todo/rspec/feature_category.yml +++ b/.rubocop_todo/rspec/feature_category.yml @@ -1947,7 +1947,6 @@ RSpec/FeatureCategory: - 'spec/helpers/gitlab_script_tag_helper_spec.rb' - 'spec/helpers/graph_helper_spec.rb' - 'spec/helpers/groups/group_members_helper_spec.rb' - - 'spec/helpers/groups/settings_helper_spec.rb' - 'spec/helpers/hooks_helper_spec.rb' - 'spec/helpers/icons_helper_spec.rb' - 'spec/helpers/import_helper_spec.rb' diff --git a/app/assets/javascripts/init_confirm_danger.js b/app/assets/javascripts/init_confirm_danger.js index 6c6cadedf00f055ac6c7cf4d4d47c7eadab920ea..0e5071cf4d831b01bbe341b5bed8af44f897a3e0 100644 --- a/app/assets/javascripts/init_confirm_danger.js +++ b/app/assets/javascripts/init_confirm_danger.js @@ -4,49 +4,52 @@ import { parseBoolean } from './lib/utils/common_utils'; import ConfirmDanger from './vue_shared/components/confirm_danger/confirm_danger.vue'; export default () => { - const el = document.querySelector('.js-confirm-danger'); - if (!el) return null; + const elements = document.querySelectorAll('.js-confirm-danger'); - const { - removeFormId = null, - phrase, - buttonText, - buttonClass = '', - buttonTestid, - buttonVariant, - confirmDangerMessage, - confirmButtonText = null, - disabled, - additionalInformation, - htmlConfirmationMessage, - } = el.dataset; + if (!elements.length) return; - return new Vue({ - el, - provide: pickBy( - { - htmlConfirmationMessage, - confirmDangerMessage, - additionalInformation, - confirmButtonText, - }, - (v) => Boolean(v), - ), - render: (createElement) => - createElement(ConfirmDanger, { - props: { - phrase, - buttonText, - buttonClass, - buttonVariant, - buttonTestid, - disabled: parseBoolean(disabled), + elements.forEach((element) => { + const { + removeFormId = null, + phrase, + buttonText, + buttonClass = '', + buttonTestid, + buttonVariant, + confirmDangerMessage, + confirmButtonText = null, + disabled, + additionalInformation, + htmlConfirmationMessage, + } = element.dataset; + + return new Vue({ + el: element, + provide: pickBy( + { + htmlConfirmationMessage, + confirmDangerMessage, + additionalInformation, + confirmButtonText, }, - on: { - confirm: () => { - if (removeFormId) document.getElementById(removeFormId)?.submit(); + (v) => Boolean(v), + ), + render: (createElement) => + createElement(ConfirmDanger, { + props: { + phrase, + buttonText, + buttonClass, + buttonVariant, + buttonTestid, + disabled: parseBoolean(disabled), }, - }, - }), + on: { + confirm: () => { + if (removeFormId) document.getElementById(removeFormId)?.submit(); + }, + }, + }), + }); }); }; diff --git a/app/assets/javascripts/pages/admin/groups/index/index.js b/app/assets/javascripts/pages/admin/groups/index/index.js new file mode 100644 index 0000000000000000000000000000000000000000..36c70a8664376e3368a86413597a2b88c9592f86 --- /dev/null +++ b/app/assets/javascripts/pages/admin/groups/index/index.js @@ -0,0 +1,3 @@ +import initConfirmDanger from '~/init_confirm_danger'; + +initConfirmDanger(); diff --git a/app/helpers/groups/settings_helper.rb b/app/helpers/groups/settings_helper.rb deleted file mode 100644 index 29c615285f37572586152a1c73575c0f82c31831..0000000000000000000000000000000000000000 --- a/app/helpers/groups/settings_helper.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -module Groups - module SettingsHelper - include GroupsHelper - - def group_settings_confirm_modal_data(group, remove_form_id = nil, button_text = nil) - { - remove_form_id: remove_form_id, - button_text: button_text.presence || _('Delete group'), - button_testid: 'remove-group-button', - disabled: group.prevent_delete?.to_s, - confirm_danger_message: remove_group_message(group), - phrase: group.full_path, - html_confirmation_message: 'true' - } - end - end -end - -Groups::SettingsHelper.prepend_mod_with('Groups::SettingsHelper') diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index f2249c98ee8da3b0ab96d595c8d3482ff785f419..95f197e5095cede72c16153a16f9e5a07a988cff 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -98,8 +98,20 @@ def group_lfs_status(group) end end + def group_confirm_modal_data(group:, remove_form_id: nil, permanently_remove: false, button_text: nil) + { + remove_form_id: remove_form_id, + button_text: button_text.nil? ? _('Delete group') : button_text, + button_testid: 'remove-group-button', + disabled: group.prevent_delete?.to_s, + confirm_danger_message: remove_group_message(group, permanently_remove), + phrase: group.full_path, + html_confirmation_message: 'true' + } + end + # Overridden in EE - def remove_group_message(group) + def remove_group_message(group, permanently_remove) content_tag :div do content = ''.html_safe content << content_tag(:span, _("You are about to delete the group %{group_name}.") % { group_name: group.name }) diff --git a/app/views/admin/groups/_group.html.haml b/app/views/admin/groups/_group.html.haml index 68c2bbb3b35d1ecd1f8db9e8f2a46d2b0da67ab2..23959348ba8daea95ec6d33cfa48bd72e91adadd 100644 --- a/app/views/admin/groups/_group.html.haml +++ b/app/views/admin/groups/_group.html.haml @@ -1,4 +1,5 @@ - group = local_assigns.fetch(:group) +- remove_form_id = "js-remove-group-#{group.id}-form" %li.group-row.gl-py-3.gl-align-items-center{ class: 'gl-display-flex!', data: { testid: 'group-row-content' } } = render Pajamas::AvatarComponent.new(group, size: 32, alt: '') @@ -29,8 +30,9 @@ %span.gl-ml-5.visibility-icon.has-tooltip{ data: { container: 'body', placement: 'left' }, title: visibility_icon_description(group) } = visibility_level_icon(group.visibility_level) - .controls.gl-flex-shrink-0.gl-ml-5 + .controls.gl-flex.gl-flex-shrink-0.gl-ml-5 = render Pajamas::ButtonComponent.new(href: admin_group_edit_path(group), button_options: { id: "edit_#{dom_id(group)}" }) do = _('Edit') - = render Pajamas::ButtonComponent.new(href: [:admin, group], variant: :danger, button_options: { data: { confirm: _("Are you sure you want to delete %{group_name}?") % { group_name: group.name }, confirm_btn_variant: 'danger', method: :delete } }) do - = _('Delete') + + = form_tag([:admin, group], method: :delete, id: remove_form_id) do + .js-confirm-danger{ data: group_confirm_modal_data(group: group, remove_form_id: remove_form_id, permanently_remove: true, button_text: _('Delete')) } diff --git a/app/views/groups/settings/_remove_button.html.haml b/app/views/groups/settings/_remove_button.html.haml index 5e407b40966bfc5b32007b4403aa6b46ef3ac205..2aa9c860b4df5ae3ad6dd1eda294b72ae9245f96 100644 --- a/app/views/groups/settings/_remove_button.html.haml +++ b/app/views/groups/settings/_remove_button.html.haml @@ -6,4 +6,4 @@ - c.with_body do = html_escape(_("This group can't be removed because it is linked to a subscription. To remove this group, %{linkStart}link the subscription%{linkEnd} with a different group.")) % { linkStart: "<a href=\"#{help_page_path('subscriptions/gitlab_com/index', anchor: 'change-the-linked-namespace')}\">".html_safe, linkEnd: '</a>'.html_safe } -.js-confirm-danger{ data: group_settings_confirm_modal_data(group, remove_form_id, button_text) } +.js-confirm-danger{ data: group_confirm_modal_data(group: group, remove_form_id: remove_form_id, button_text: button_text) } diff --git a/ee/app/helpers/ee/groups_helper.rb b/ee/app/helpers/ee/groups_helper.rb index 4f36d18d8f3d0b9ba181d5ad426226571820b7c6..3c8d6b4440967de4889e8bfae1fae26dd22de914 100644 --- a/ee/app/helpers/ee/groups_helper.rb +++ b/ee/app/helpers/ee/groups_helper.rb @@ -32,7 +32,8 @@ def can_admin_service_accounts?(group) end override :remove_group_message - def remove_group_message(group) + def remove_group_message(group, permanently_remove) + return super if permanently_remove return super unless group.licensed_feature_available?(:adjourned_deletion_for_projects_and_groups) return super if group.marked_for_deletion? return super unless group.adjourned_deletion? diff --git a/ee/spec/helpers/ee/groups_helper_spec.rb b/ee/spec/helpers/ee/groups_helper_spec.rb index 5b03dd041e4ad646290aef7c109f02d08225e7ce..14b595de8d25585fbea7d4d4cddf670ed68636c9 100644 --- a/ee/spec/helpers/ee/groups_helper_spec.rb +++ b/ee/spec/helpers/ee/groups_helper_spec.rb @@ -86,18 +86,20 @@ end describe '#remove_group_message' do - subject { helper.remove_group_message(group) } + let(:delayed_deletion_message) { "The contents of this group, its subgroups and projects will be permanently deleted after" } + let(:permanent_deletion_message) { ["You are about to delete the group #{group.name}", "After you delete a group, you <strong>cannot</strong> restore it or its components."] } + + subject { helper.remove_group_message(group, false) } shared_examples 'permanent deletion message' do it 'returns the message related to permanent deletion' do - expect(subject).to include("You are about to delete the group #{group.name}") - expect(subject).to include("After you delete a group, you <strong>cannot</strong> restore it or its components.") + expect(subject).to include(*permanent_deletion_message) end end shared_examples 'delayed deletion message' do it 'returns the message related to delayed deletion' do - expect(subject).to include("The contents of this group, its subgroups and projects will be permanently deleted after") + expect(subject).to include(delayed_deletion_message) end end @@ -140,6 +142,21 @@ it_behaves_like 'permanent deletion message' end + + context "group has not been marked for deletion" do + let(:group) { build(:group) } + + context "'permanently_remove' argument is set to 'true'" do + it "displays permanent deletion message" do + allow(group).to receive(:licensed_feature_available?).with(:adjourned_deletion_for_projects_and_groups).and_return(true) + allow(group).to receive(:marked_for_deletion?).and_return(false) + allow(group).to receive(:adjourned_deletion?).and_return(true) + + expect(subject).to include(delayed_deletion_message) + expect(helper.remove_group_message(group, true)).to include(*permanent_deletion_message) + end + end + end end context 'delayed deletion feature is not available' do diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f1b18b67575c2ecc49f31a7e5aea083f84df04bc..3d0fc1e06ffc887c094228c4c47c9e7a524594ae 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6961,9 +6961,6 @@ msgstr "" msgid "Are you sure you want to continue?" msgstr "" -msgid "Are you sure you want to delete %{group_name}?" -msgstr "" - msgid "Are you sure you want to delete %{name}?" msgstr "" diff --git a/spec/helpers/groups/settings_helper_spec.rb b/spec/helpers/groups/settings_helper_spec.rb deleted file mode 100644 index e1ab03c8af9c7276221c01acf2ed5f62c202d47f..0000000000000000000000000000000000000000 --- a/spec/helpers/groups/settings_helper_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Groups::SettingsHelper do - include GroupsHelper - include NumbersHelper - - let_it_be(:group) { create(:group, path: "foo") } - - describe('#group_settings_confirm_modal_data') do - using RSpec::Parameterized::TableSyntax - - fake_form_id = "fake_form_id" - where(:prevent_delete_response, :is_button_disabled, :form_value_id) do - true | "true" | nil - true | "true" | fake_form_id - false | "false" | nil - false | "false" | fake_form_id - end - - with_them do - it "returns expected parameters" do - allow(group).to receive(:prevent_delete?).and_return(prevent_delete_response) - - expected = helper.group_settings_confirm_modal_data(group, form_value_id) - expect(expected).to eq({ - button_text: "Delete group", - confirm_danger_message: remove_group_message(group), - remove_form_id: form_value_id, - phrase: group.full_path, - button_testid: "remove-group-button", - disabled: is_button_disabled, - html_confirmation_message: 'true' - }) - end - end - end -end diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index 11c80f133ca1c189c814fb61388099e1ce58ee01..259f74c793128779e47e7e876988b547e1ffb440 100644 --- a/spec/helpers/groups_helper_spec.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -5,6 +5,7 @@ RSpec.describe GroupsHelper, feature_category: :groups_and_projects do include ApplicationHelper include AvatarsHelper + include NumbersHelper describe '#group_icon_url' do it 'returns an url for the avatar' do @@ -761,4 +762,35 @@ expect(helper.show_prevent_inviting_groups_outside_hierarchy_setting?(subgroup)).to eq(false) end end + + describe('#group_confirm_modal_data') do + using RSpec::Parameterized::TableSyntax + + let_it_be(:group) { create(:group, path: "foo") } + + fake_form_id = "fake_form_id" + where(:prevent_delete_response, :is_button_disabled, :form_value_id, :permanently_remove, :button_text) do + true | "true" | nil | false | "Delete" + true | "true" | fake_form_id | true | nil + false | "false" | nil | false | "Delete group" + false | "false" | fake_form_id | true | nil + end + + with_them do + it "returns expected parameters" do + allow(group).to receive(:prevent_delete?).and_return(prevent_delete_response) + + expected = helper.group_confirm_modal_data(group: group, remove_form_id: form_value_id, button_text: button_text) + expect(expected).to eq({ + button_text: button_text.nil? ? "Delete group" : button_text, + confirm_danger_message: remove_group_message(group, permanently_remove), + remove_form_id: form_value_id, + phrase: group.full_path, + button_testid: "remove-group-button", + disabled: is_button_disabled, + html_confirmation_message: 'true' + }) + end + end + end end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index 1a97e5b7730668add0975e5a313cae3fd0210c5c..0635bb7542b0685a58125bac3ed16b8084f7263c 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -4224,7 +4224,6 @@ - './spec/helpers/graph_helper_spec.rb' - './spec/helpers/groups/group_members_helper_spec.rb' - './spec/helpers/groups_helper_spec.rb' -- './spec/helpers/groups/settings_helper_spec.rb' - './spec/helpers/hooks_helper_spec.rb' - './spec/helpers/icons_helper_spec.rb' - './spec/helpers/import_helper_spec.rb'