Commit 4d5dac91 authored by Dan Jensen's avatar Dan Jensen

Clarify secondary email validation methods

Previously the validation methods for secondary emails used the term
"own" when the established term is "verified". This updates both the
method names and the language line to use the term "verified", as
suggested in previous code review.
parent d5ae78b0
...@@ -16,7 +16,7 @@ class NotificationSetting < ApplicationRecord ...@@ -16,7 +16,7 @@ class NotificationSetting < ApplicationRecord
validates :user_id, uniqueness: { scope: [:source_type, :source_id], validates :user_id, uniqueness: { scope: [:source_type, :source_id],
message: "already exists in source", message: "already exists in source",
allow_nil: true } allow_nil: true }
validate :owns_notification_email, if: :notification_email_changed? validate :notification_email_verified, if: :notification_email_changed?
scope :for_groups, -> { where(source_type: 'Namespace') } scope :for_groups, -> { where(source_type: 'Namespace') }
...@@ -110,11 +110,11 @@ class NotificationSetting < ApplicationRecord ...@@ -110,11 +110,11 @@ class NotificationSetting < ApplicationRecord
has_attribute?(event) && !!read_attribute(event) has_attribute?(event) && !!read_attribute(event)
end end
def owns_notification_email def notification_email_verified
return if user.temp_oauth_email? return if user.temp_oauth_email?
return if notification_email.empty? return if notification_email.empty?
errors.add(:notification_email, _("is not an email you own")) unless user.verified_emails.include?(notification_email) errors.add(:notification_email, _("must be an email you have verified")) unless user.verified_emails.include?(notification_email)
end end
end end
......
...@@ -235,9 +235,9 @@ class User < ApplicationRecord ...@@ -235,9 +235,9 @@ class User < ApplicationRecord
validate :namespace_move_dir_allowed, if: :username_changed? validate :namespace_move_dir_allowed, if: :username_changed?
validate :unique_email, if: :email_changed? validate :unique_email, if: :email_changed?
validate :owns_notification_email, if: :notification_email_changed? validate :notification_email_verified, if: :notification_email_changed?
validate :owns_public_email, if: :public_email_changed? validate :public_email_verified, if: :public_email_changed?
validate :owns_commit_email, if: :commit_email_changed? validate :commit_email_verified, if: :commit_email_changed?
validate :signup_email_valid?, on: :create, if: ->(user) { !user.created_by_id } validate :signup_email_valid?, on: :create, if: ->(user) { !user.created_by_id }
validate :check_username_format, if: :username_changed? validate :check_username_format, if: :username_changed?
...@@ -928,22 +928,22 @@ class User < ApplicationRecord ...@@ -928,22 +928,22 @@ class User < ApplicationRecord
end end
end end
def owns_notification_email def notification_email_verified
return if new_record? || temp_oauth_email? return if new_record? || temp_oauth_email?
errors.add(:notification_email, _("is not an email you own")) unless verified_emails.include?(notification_email) errors.add(:notification_email, _("must be an email you have verified")) unless verified_emails.include?(notification_email)
end end
def owns_public_email def public_email_verified
return if public_email.blank? return if public_email.blank?
errors.add(:public_email, _("is not an email you own")) unless verified_emails.include?(public_email) errors.add(:public_email, _("must be an email you have verified")) unless verified_emails.include?(public_email)
end end
def owns_commit_email def commit_email_verified
return if read_attribute(:commit_email).blank? return if read_attribute(:commit_email).blank?
errors.add(:commit_email, _("is not an email you own")) unless verified_emails.include?(commit_email) errors.add(:commit_email, _("must be an email you have verified")) unless verified_emails.include?(commit_email)
end end
# Define commit_email-related attribute methods explicitly instead of relying # Define commit_email-related attribute methods explicitly instead of relying
......
...@@ -39449,9 +39449,6 @@ msgstr "" ...@@ -39449,9 +39449,6 @@ msgstr ""
msgid "is not allowed. We do not currently support project-level iterations" msgid "is not allowed. We do not currently support project-level iterations"
msgstr "" msgstr ""
msgid "is not an email you own"
msgstr ""
msgid "is not from an allowed domain." msgid "is not from an allowed domain."
msgstr "" msgstr ""
...@@ -39886,6 +39883,9 @@ msgstr "" ...@@ -39886,6 +39883,9 @@ msgstr ""
msgid "must be after start" msgid "must be after start"
msgstr "" msgstr ""
msgid "must be an email you have verified"
msgstr ""
msgid "must be greater than start date" msgid "must be greater than start date"
msgstr "" msgstr ""
......
...@@ -704,7 +704,7 @@ RSpec.describe User do ...@@ -704,7 +704,7 @@ RSpec.describe User do
user.notification_email = email.email user.notification_email = email.email
expect(user).to be_invalid expect(user).to be_invalid
expect(user.errors[:notification_email]).to include('is not an email you own') expect(user.errors[:notification_email]).to include(_('must be an email you have verified'))
end end
end end
...@@ -723,7 +723,7 @@ RSpec.describe User do ...@@ -723,7 +723,7 @@ RSpec.describe User do
user.public_email = email.email user.public_email = email.email
expect(user).to be_invalid expect(user).to be_invalid
expect(user.errors[:public_email]).to include('is not an email you own') expect(user.errors[:public_email]).to include(_('must be an email you have verified'))
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment