diff --git a/app/models/user.rb b/app/models/user.rb index 0499b6954d464540baa830fe4489da308b4ae8a6..6e4a39dadba072143291be7cf93c39f2a9b2b9f0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1970,10 +1970,12 @@ def contributed_projects end # Returns true if the user can be removed, false otherwise. - # A user can be removed if they do not own any groups where they are the sole owner + # A user can be removed if they do not own any groups or organizations where they are the sole owner # Method `none?` is used to ensure faster retrieval, See https://gitlab.com/gitlab-org/gitlab/-/issues/417105 def can_be_removed? + return solo_owned_groups.none? && solo_owned_organizations.none? if Feature.enabled?(:ui_for_organizations) + solo_owned_groups.none? end diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index f6093604bac99f1dfeaed488511c63d81db490dc..ae236655f06e684293d2b2a6070c6f5eb57b380b 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -84,14 +84,26 @@ confirm_with_password: ('true' if current_user.confirm_deletion_with_password?), username: current_user.username } } - else - - if current_user.solo_owned_groups.present? + - if !current_user.can_be_removed? %p - = s_('Profiles|Your account is currently an owner in these groups:') - %ul - - current_user.solo_owned_groups.each do |group| - %li= group.name + = s_('Profiles|Your account is currently the sole owner in the following:') + + - if Feature.enabled?(:ui_for_organizations, current_user) && current_user.solo_owned_organizations.present? + %p.gl-font-bold + = _('Organizations') + %ul + - current_user.solo_owned_organizations.each do |organization| + %li= link_to organization.name, organization.web_url, target: '_blank', rel: 'noopener noreferrer' + + - if current_user.solo_owned_groups.present? + %p.gl-font-bold + = _('Groups') + %ul + - current_user.solo_owned_groups.each do |group| + %li= link_to group.name, group.web_url, target: '_blank', rel: 'noopener noreferrer' + %p - = s_('Profiles|You must transfer ownership or delete these groups before you can delete your account.') + = s_('Profiles|You must transfer ownership or delete these before you can delete your account.') - elsif !current_user.can_remove_self? %p = s_('Profiles|GitLab is unable to verify your identity automatically. For security purposes, you must set a password by %{openingTag}resetting your password%{closingTag} to delete your account.').html_safe % { openingTag: "<a href='#{reset_user_settings_password_path}' rel=\"nofollow\" data-method=\"put\">".html_safe, closingTag: '</a>'.html_safe} diff --git a/ee/app/views/profiles/accounts/_group_saml_unlink_buttons.html.haml b/ee/app/views/profiles/accounts/_group_saml_unlink_buttons.html.haml index 6645c8840e71babf336b90311bd4554bbda0e4c9..dcabc375ac690fe3bfa0be3e496f4ddc05a9aef4 100644 --- a/ee/app/views/profiles/accounts/_group_saml_unlink_buttons.html.haml +++ b/ee/app/views/profiles/accounts/_group_saml_unlink_buttons.html.haml @@ -1,4 +1,4 @@ -- group_saml_identities.each do |identity| +- group_saml_identities&.each do |identity| - group = identity.saml_provider.group .provider-btn-group .provider-btn-image diff --git a/locale/gitlab.pot b/locale/gitlab.pot index fa62a64eee6f28ed03aa4a788017e30ddcfc846e..b58f8f959d447c106d38f2b455d80f4af569bf8a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -42529,7 +42529,7 @@ msgstr "" msgid "Profiles|You must transfer ownership or delete groups you are an owner of before you can delete your account" msgstr "" -msgid "Profiles|You must transfer ownership or delete these groups before you can delete your account." +msgid "Profiles|You must transfer ownership or delete these before you can delete your account." msgstr "" msgid "Profiles|Your Discord user ID." @@ -42538,7 +42538,7 @@ msgstr "" msgid "Profiles|Your LinkedIn profile name from linkedin.com/in/profilename" msgstr "" -msgid "Profiles|Your account is currently an owner in these groups:" +msgid "Profiles|Your account is currently the sole owner in the following:" msgstr "" msgid "Profiles|Your email address was automatically set based on your %{provider_label} account" diff --git a/spec/features/profile_spec.rb b/spec/features/profile_spec.rb index 8790b81cfdaf7a784738141d7e5e67a629fd8f38..7813b5099870826779b9e80ce499a2da9f70a23e 100644 --- a/spec/features/profile_spec.rb +++ b/spec/features/profile_spec.rb @@ -61,7 +61,20 @@ visit profile_account_path expect(page).not_to have_button('Delete account') - expect(page).to have_content("Your account is currently an owner in these groups: #{group.name}") + expect(page).to have_content("Your account is currently the sole owner in the following:") + expect(page).to have_link(group.name, href: group.web_url) + end + + it 'does not show delete button when user owns an organization and feature flag ui_for_organizations is enabled' do + stub_feature_flags(ui_for_organizations: true) + organization = create(:organization) + create(:organization_owner, user: user, organization: organization) + + visit profile_account_path + + expect(page).not_to have_button('Delete account') + expect(page).to have_content("Your account is currently the sole owner in the following:") + expect(page).to have_link(organization.name, href: organization.web_url) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f2ccce453cac0e1c5a7655b3ce085f6e8744c9b0..6542d02b953ccea0896972ba6a78d3df458fdb61 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -4839,17 +4839,49 @@ def login_method(login) describe '#can_be_removed?' do subject { create(:user) } - context 'no owned groups' do - it { expect(subject.can_be_removed?).to be_truthy } + let_it_be(:group) { create(:group) } + let_it_be(:organization) { create(:organization) } + + context 'when feature flag :ui_for_organizations is enabled' do + where(:solo_owned_groups, :solo_owned_organizations, :result) do + [ + [[], [], true], + [[ref(:group)], [], false], + [[], [ref(:organization)], false], + [[ref(:group)], [ref(:organization)], false] + ] + end + + with_them do + before do + stub_feature_flags(ui_for_organizations: true) + allow(subject).to receive(:solo_owned_groups).and_return(solo_owned_groups) + allow(subject).to receive(:solo_owned_organizations).and_return(solo_owned_organizations) + end + + it { expect(subject.can_be_removed?).to be(result) } + end end - context 'has owned groups' do - before do - group = create(:group) - group.add_owner(subject) + context 'when feature flag :ui_for_organizations is disabled' do + where(:solo_owned_groups, :solo_owned_organizations, :result) do + [ + [[], [], true], + [[ref(:group)], [], false], + [[], [ref(:organization)], true], + [[ref(:group)], [ref(:organization)], false] + ] end - it { expect(subject.can_be_removed?).to be_falsey } + with_them do + before do + stub_feature_flags(ui_for_organizations: false) + allow(subject).to receive(:solo_owned_groups).and_return(solo_owned_groups) + allow(subject).to receive(:solo_owned_organizations).and_return(solo_owned_organizations) + end + + it { expect(subject.can_be_removed?).to be(result) } + end end end diff --git a/spec/views/profiles/accounts/show.html.haml_spec.rb b/spec/views/profiles/accounts/show.html.haml_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..a3a3741e850a2498a16f7b90dc69e98b7cf0e803 --- /dev/null +++ b/spec/views/profiles/accounts/show.html.haml_spec.rb @@ -0,0 +1,151 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'profiles/accounts/show', feature_category: :user_profile do + let_it_be(:user) { build_stubbed(:user) } + + before do + assign(:user, user) + allow(view).to receive(:current_user).and_return(user) + end + + context 'for account deletion' do + context 'when user can delete profile' do + before do + allow(user).to receive(:can_be_removed?).and_return(true) + allow(view) + .to receive(:can?) + .with(user, :destroy_user, user) + .and_return(true) + + render + end + + it 'renders delete button' do + expect(rendered).to have_css("[data-testid='delete-account-button']") + end + end + + context 'when user does not have permissions to delete their own profile' do + before do + allow(user).to receive(:can_be_removed?).and_return(true) + allow(view) + .to receive(:can?) + .with(user, :destroy_user, user) + .and_return(false) + + render + end + + it 'does not render delete button' do + expect(rendered).not_to have_css("[data-testid='delete-account-button']") + end + + it 'renders correct help text' do + expect(rendered).to have_text("You don't have access to delete this user.") + end + end + + context 'when user cannot be verified automatically' do + before do + allow(user).to receive_messages(can_be_removed?: true, can_remove_self?: false) + allow(view) + .to receive(:can?) + .with(user, :destroy_user, user) + .and_return(false) + + render + end + + it 'does not render delete button' do + expect(rendered).not_to have_css("[data-testid='delete-account-button']") + end + + it 'renders correct help text' do + expect(rendered).to have_text('GitLab is unable to verify your identity automatically.') + end + end + + context 'when user has sole ownership of a group' do + let_it_be(:group) { build_stubbed(:group) } + + before do + allow(user).to receive_messages(can_be_removed?: false, solo_owned_groups: [group]) + allow(view) + .to receive(:can?) + .with(user, :destroy_user, user) + .and_return(true) + + render + end + + it 'does not render delete button' do + expect(rendered).not_to have_css("[data-testid='delete-account-button']") + end + + it 'renders correct help text' do + expect(rendered).to have_text('Your account is currently the sole owner in the following:') + end + + it 'renders group as a link in the list' do + expect(rendered).to have_text('Groups') + expect(rendered).to have_link(group.name, href: group.web_url) + end + end + + context 'when user has sole ownership of a organization' do + let_it_be(:organization) { build_stubbed(:organization) } + + before do + allow(user).to receive_messages(can_be_removed?: false, solo_owned_organizations: [organization]) + allow(view) + .to receive(:can?) + .with(user, :destroy_user, user) + .and_return(true) + end + + context 'when feature flag ui_for_organizations is false' do + before do + stub_feature_flags(ui_for_organizations: false) + + render + end + + it 'does not render delete button' do + expect(rendered).not_to have_css("[data-testid='delete-account-button']") + end + + it 'renders correct help text' do + expect(rendered).to have_text('Your account is currently the sole owner in the following:') + end + + it 'does not render organization as a link in the list' do + expect(rendered).not_to have_text('Organizations') + expect(rendered).not_to have_link(organization.name, href: organization.web_url) + end + end + + context 'when feature flag ui_for_organizations is true' do + before do + stub_feature_flags(ui_for_organizations: true) + + render + end + + it 'does not render delete button' do + expect(rendered).not_to have_css("[data-testid='delete-account-button']") + end + + it 'renders correct help text' do + expect(rendered).to have_text('Your account is currently the sole owner in the following:') + end + + it 'renders organization as a link in the list' do + expect(rendered).to have_text('Organizations') + expect(rendered).to have_link(organization.name, href: organization.web_url) + end + end + end + end +end