From 034b152beb39762ca117318c307e8cfe2be5185e Mon Sep 17 00:00:00 2001
From: Eugie Limpin <elimpin@gitlab.com>
Date: Mon, 15 Apr 2024 08:49:35 +0800
Subject: [PATCH] Add error to indicate email is linked to an account pending
deletion
Changelog: added
---
app/helpers/form_helper.rb | 4 +--
app/models/user.rb | 17 +++++++--
.../devise/shared/_error_messages.html.haml | 11 ++----
locale/gitlab.pot | 3 ++
spec/helpers/form_helper_spec.rb | 10 +++++-
spec/models/user_spec.rb | 35 +++++++++++++++++++
6 files changed, 66 insertions(+), 14 deletions(-)
diff --git a/app/helpers/form_helper.rb b/app/helpers/form_helper.rb
index bbcf408650d3b..93db454dafc51 100644
--- a/app/helpers/form_helper.rb
+++ b/app/helpers/form_helper.rb
@@ -1,12 +1,12 @@
# frozen_string_literal: true
module FormHelper
- def form_errors(model, type: 'form', truncate: [], custom_message: [])
+ def form_errors(model, type: 'form', truncate: [], custom_message: [], custom_headline: nil)
errors = model.errors
return unless errors.any?
- headline = n_(
+ headline = custom_headline || n_(
'The %{type} contains the following error:',
'The %{type} contains the following errors:',
errors.count
diff --git a/app/models/user.rb b/app/models/user.rb
index 00c338b6f2735..35b0a73049ee7 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1188,10 +1188,23 @@ def will_save_change_to_login?
end
def unique_email
- return if errors.added?(:email, _('has already been taken'))
+ email_taken = errors.added?(:email, _('has already been taken'))
- if !emails.exists?(email: email) && Email.exists?(email: email)
+ if !email_taken && !emails.exists?(email: email) && Email.exists?(email: email)
errors.add(:email, _('has already been taken'))
+ email_taken = true
+ end
+
+ if email_taken &&
+ ::Feature.enabled?(:delay_delete_own_user) &&
+ User.find_by_any_email(email)&.deleted_own_account?
+
+ help_page_url = Rails.application.routes.url_helpers.help_page_url(
+ 'user/profile/account/delete_account',
+ anchor: 'delete-your-own-account'
+ )
+
+ errors.add(:email, _('is linked to an account pending deletion.'), help_page_url: help_page_url)
end
end
diff --git a/app/views/devise/shared/_error_messages.html.haml b/app/views/devise/shared/_error_messages.html.haml
index caebda72b9cee..c1c73a1b753a5 100644
--- a/app/views/devise/shared/_error_messages.html.haml
+++ b/app/views/devise/shared/_error_messages.html.haml
@@ -1,9 +1,2 @@
-- if resource.errors.any?
- = render Pajamas::AlertComponent.new(title: I18n.t("errors.messages.not_saved", count: resource.errors.count, resource: resource.class.model_name.human.downcase),
- variant: :danger,
- dismissible: false,
- alert_options: { id: 'error_explanation', class: 'gl-mb-3'}) do |c|
- - c.with_body do
- %ul.gl-pl-4
- - resource.errors.full_messages.each do |message|
- %li= message
+- headline = I18n.t("errors.messages.not_saved", count: resource.errors.count, resource: resource.class.model_name.human.downcase)
+= form_errors(resource, custom_headline: headline)
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 1e9facaa8a955..f196be6b97c81 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -60769,6 +60769,9 @@ msgstr ""
msgid "is invalid because there is upstream lock"
msgstr ""
+msgid "is linked to an account pending deletion."
+msgstr ""
+
msgid "is not"
msgstr ""
diff --git a/spec/helpers/form_helper_spec.rb b/spec/helpers/form_helper_spec.rb
index 0db48dfc28eee..1048b58d1e495 100644
--- a/spec/helpers/form_helper_spec.rb
+++ b/spec/helpers/form_helper_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe FormHelper do
+RSpec.describe FormHelper, feature_category: :shared do
include Devise::Test::ControllerHelpers
describe '#dropdown_max_select' do
@@ -83,6 +83,14 @@
.to include('The form contains the following errors:')
end
+ it 'uses passed custom headline' do
+ resource = double(errors: errors_stub('A'))
+
+ result = helper.form_errors(resource, custom_headline: 'There were errors:')
+ expect(result).to include('There were errors:')
+ expect(result).not_to include('The form contains the following error:')
+ end
+
it 'renders each message' do
model = double(errors: errors_stub('Error 1', 'Error 2', 'Error 3'))
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 86cfb1c35cfdd..861ab5341b670 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -8501,4 +8501,39 @@ def owner_class_attribute_default; end
end
end
end
+
+ context 'when email is not unique' do
+ let_it_be(:existing_user) { create(:user) }
+
+ subject(:new_user) { build(:user, email: existing_user.email).tap { |user| user.valid? } }
+
+ shared_examples 'it does not add account pending deletion error message' do
+ it 'does not add account pending deletion error message' do
+ expect(new_user.errors.full_messages).to include('Email has already been taken')
+ expect(new_user.errors.full_messages).not_to include('Email is linked to an account pending deletion')
+ end
+ end
+
+ context 'when existing account is pending deletion' do
+ before do
+ UserCustomAttribute.set_deleted_own_account_at(existing_user)
+ end
+
+ it 'adds expected error messages' do
+ expect(new_user.errors.full_messages).to include('Email has already been taken', 'Email is linked to an account pending deletion.')
+ end
+
+ context 'when delay_delete_own_user feature flag is disabled' do
+ before do
+ stub_feature_flags(delay_delete_own_user: false)
+ end
+
+ it_behaves_like 'it does not add account pending deletion error message'
+ end
+ end
+
+ context 'when existing account is not pending deletion' do
+ it_behaves_like 'it does not add account pending deletion error message'
+ end
+ end
end
--
GitLab