Commit 8aae5866 authored by Luke Duncalfe's avatar Luke Duncalfe

Merge branch 'revert-da560e59' into 'master'

Revert "Merge branch 'remove-custom-getter-methods-for-secondary-emails' into 'master'"

See merge request gitlab-org/gitlab!70473
parents 7fe4ab3c dc08481d
......@@ -231,7 +231,7 @@ class Projects::IssuesController < Projects::ApplicationController
IssuableExportCsvWorker.perform_async(:issue, current_user.id, project.id, finder_options.to_h) # rubocop:disable CodeReuse/Worker
index_path = project_issues_path(project)
message = _('Your CSV export has started. It will be emailed to %{email} when complete.') % { email: current_user.notification_email_or_default }
message = _('Your CSV export has started. It will be emailed to %{email} when complete.') % { email: current_user.notification_email }
redirect_to(index_path, notice: message)
end
......
......@@ -378,7 +378,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
IssuableExportCsvWorker.perform_async(:merge_request, current_user.id, project.id, finder_options.to_h) # rubocop:disable CodeReuse/Worker
index_path = project_merge_requests_path(project)
message = _('Your CSV export has started. It will be emailed to %{email} when complete.') % { email: current_user.notification_email_or_default }
message = _('Your CSV export has started. It will be emailed to %{email} when complete.') % { email: current_user.notification_email }
redirect_to(index_path, notice: message)
end
......
......@@ -221,7 +221,7 @@ module IssuesHelper
can_bulk_update: can?(current_user, :admin_issue, project).to_s,
can_edit: can?(current_user, :admin_project, project).to_s,
can_import_issues: can?(current_user, :import_issues, @project).to_s,
email: current_user&.notification_email_or_default,
email: current_user&.notification_email,
emails_help_page_path: help_page_path('development/emails', anchor: 'email-namespace'),
export_csv_path: export_csv_project_issues_path(project),
has_any_issues: project_issues(project).exists?.to_s,
......
......@@ -435,7 +435,7 @@ module ProjectsHelper
def git_user_email
if current_user
current_user.commit_email_or_default
current_user.commit_email
else
"your@email.com"
end
......
......@@ -4,7 +4,7 @@ module Emails
module AdminNotification
def send_admin_notification(user_id, subject, body)
user = User.find(user_id)
email = user.notification_email_or_default
email = user.notification_email
@unsubscribe_url = unsubscribe_url(email: Base64.urlsafe_encode64(email))
@body = body
mail to: email, subject: subject
......@@ -12,7 +12,7 @@ module Emails
def send_unsubscribed_notification(user_id)
user = User.find(user_id)
email = user.notification_email_or_default
email = user.notification_email
mail to: email, subject: "Unsubscribed from GitLab administrator notifications"
end
end
......
......@@ -6,7 +6,7 @@ module Emails
@current_user = @user = User.find(user_id)
@target_url = user_url(@user)
@token = token
mail(to: @user.notification_email_or_default, subject: subject("Account was created for you"))
mail(to: @user.notification_email, subject: subject("Account was created for you"))
end
def instance_access_request_email(user, recipient)
......@@ -14,7 +14,7 @@ module Emails
@recipient = recipient
profile_email_with_layout(
to: recipient.notification_email_or_default,
to: recipient.notification_email,
subject: subject(_("GitLab Account Request")))
end
......@@ -42,7 +42,7 @@ module Emails
@current_user = @user = @key.user
@target_url = user_url(@user)
mail(to: @user.notification_email_or_default, subject: subject("SSH key was added to your account"))
mail(to: @user.notification_email, subject: subject("SSH key was added to your account"))
end
# rubocop: enable CodeReuse/ActiveRecord
......@@ -54,7 +54,7 @@ module Emails
@current_user = @user = @gpg_key.user
@target_url = user_url(@user)
mail(to: @user.notification_email_or_default, subject: subject("GPG key was added to your account"))
mail(to: @user.notification_email, subject: subject("GPG key was added to your account"))
end
# rubocop: enable CodeReuse/ActiveRecord
......@@ -67,7 +67,7 @@ module Emails
@days_to_expire = PersonalAccessToken::DAYS_TO_EXPIRE
Gitlab::I18n.with_locale(@user.preferred_language) do
mail(to: @user.notification_email_or_default, subject: subject(_("Your personal access tokens will expire in %{days_to_expire} days or less") % { days_to_expire: @days_to_expire }))
mail(to: @user.notification_email, subject: subject(_("Your personal access tokens will expire in %{days_to_expire} days or less") % { days_to_expire: @days_to_expire }))
end
end
......@@ -78,7 +78,7 @@ module Emails
@target_url = profile_personal_access_tokens_url
Gitlab::I18n.with_locale(@user.preferred_language) do
mail(to: @user.notification_email_or_default, subject: subject(_("Your personal access token has expired")))
mail(to: @user.notification_email, subject: subject(_("Your personal access token has expired")))
end
end
......@@ -90,7 +90,7 @@ module Emails
@target_url = profile_keys_url
Gitlab::I18n.with_locale(@user.preferred_language) do
mail(to: @user.notification_email_or_default, subject: subject(_("Your SSH key has expired")))
mail(to: @user.notification_email, subject: subject(_("Your SSH key has expired")))
end
end
......@@ -102,7 +102,7 @@ module Emails
@target_url = profile_keys_url
Gitlab::I18n.with_locale(@user.preferred_language) do
mail(to: @user.notification_email_or_default, subject: subject(_("Your SSH key is expiring soon.")))
mail(to: @user.notification_email, subject: subject(_("Your SSH key is expiring soon.")))
end
end
......@@ -114,7 +114,7 @@ module Emails
Gitlab::I18n.with_locale(@user.preferred_language) do
profile_email_with_layout(
to: @user.notification_email_or_default,
to: @user.notification_email,
subject: subject(_("%{host} sign-in from new location") % { host: Gitlab.config.gitlab.host }))
end
end
......@@ -125,7 +125,7 @@ module Emails
@user = user
Gitlab::I18n.with_locale(@user.preferred_language) do
mail(to: @user.notification_email_or_default, subject: subject(_("Two-factor authentication disabled")))
mail(to: @user.notification_email, subject: subject(_("Two-factor authentication disabled")))
end
end
......
......@@ -229,9 +229,10 @@ class User < ApplicationRecord
validates :first_name, length: { maximum: 127 }
validates :last_name, length: { maximum: 127 }
validates :email, confirmation: true
validates :notification_email, devise_email: true, allow_blank: true, if: ->(user) { user.notification_email != user.email }
validates :notification_email, presence: true
validates :notification_email, devise_email: true, if: ->(user) { user.notification_email != user.email }
validates :public_email, uniqueness: true, devise_email: true, allow_blank: true
validates :commit_email, devise_email: true, allow_blank: true, if: ->(user) { user.commit_email != user.email }
validates :commit_email, devise_email: true, allow_nil: true, if: ->(user) { user.commit_email != user.email }
validates :projects_limit,
presence: true,
numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: Gitlab::Database::MAX_INT_VALUE }
......@@ -383,7 +384,7 @@ class User < ApplicationRecord
after_transition any => :deactivated do |user|
next unless Gitlab::CurrentSettings.user_deactivation_emails_enabled
NotificationService.new.user_deactivated(user.name, user.notification_email_or_default)
NotificationService.new.user_deactivated(user.name, user.notification_email)
end
# rubocop: enable CodeReuse/ServiceClass
......@@ -931,18 +932,33 @@ class User < ApplicationRecord
end
end
def commit_email_or_default
if self.commit_email == Gitlab::PrivateCommitEmail::TOKEN
# Define commit_email-related attribute methods explicitly instead of relying
# on ActiveRecord to provide them. Some of the specs use the current state of
# the model code but an older database schema, so we need to guard against the
# possibility of the commit_email column not existing.
def commit_email
return self.email unless has_attribute?(:commit_email)
if super == Gitlab::PrivateCommitEmail::TOKEN
return private_commit_email
end
# The commit email is the same as the primary email if undefined
self.commit_email.presence || self.email
super.presence || self.email
end
def commit_email=(email)
super if has_attribute?(:commit_email)
end
def commit_email_changed?
has_attribute?(:commit_email) && super
end
def notification_email_or_default
def notification_email
# The notification email is the same as the primary email if undefined
self.notification_email.presence || self.email
super.presence || self.email
end
def private_commit_email
......@@ -1624,7 +1640,7 @@ class User < ApplicationRecord
def notification_email_for(notification_group)
# Return group-specific email address if present, otherwise return global notification email address
notification_group&.notification_email_for(self) || notification_email_or_default
notification_group&.notification_email_for(self) || notification_email
end
def notification_settings_for(source, inherit: false)
......@@ -2003,7 +2019,7 @@ class User < ApplicationRecord
private
def notification_email_verified
return if notification_email.blank? || temp_oauth_email?
return if read_attribute(:notification_email).blank? || temp_oauth_email?
errors.add(:notification_email, _("must be an email you have verified")) unless verified_emails.include?(notification_email)
end
......@@ -2015,7 +2031,7 @@ class User < ApplicationRecord
end
def commit_email_verified
return if commit_email.blank?
return if read_attribute(:commit_email).blank?
errors.add(:commit_email, _("must be an email you have verified")) unless verified_emails.include?(commit_email)
end
......
......@@ -11,6 +11,6 @@
- commit_email_link_url = help_page_path('user/profile/index', anchor: 'change-the-email-displayed-on-your-commits', target: '_blank')
- commit_email_link_start = '<a href="%{url}">'.html_safe % { url: commit_email_link_url }
- commit_email_docs_link = s_('Profiles|This email will be used for web based operations, such as edits and merges. %{commit_email_link_start}Learn more%{commit_email_link_end}').html_safe % { commit_email_link_start: commit_email_link_start, commit_email_link_end: '</a>'.html_safe }
= form.select :commit_email, options_for_select(commit_email_select_options(@user), selected: @user.commit_email),
= form.select :commit_email, options_for_select(commit_email_select_options(@user), selected: @user.read_attribute(:commit_email)),
{ help: commit_email_docs_link },
control_class: 'select2 input-lg', disabled: email_change_disabled
......@@ -38,21 +38,21 @@
= render partial: 'shared/email_with_badge', locals: { email: @primary_email, verified: current_user.confirmed? }
%span.float-right
%span.badge.badge-muted.badge-pill.gl-badge.badge-success= s_('Profiles|Primary email')
- if @primary_email === current_user.commit_email_or_default
- if @primary_email === current_user.commit_email
%span.badge.badge-muted.badge-pill.gl-badge.badge-info= s_('Profiles|Commit email')
- if @primary_email === current_user.public_email
%span.badge.badge-muted.badge-pill.gl-badge.badge-info= s_('Profiles|Public email')
- if @primary_email === current_user.notification_email_or_default
- if @primary_email === current_user.notification_email
%span.badge.badge-muted.badge-pill.gl-badge.badge-info= s_('Profiles|Default notification email')
- @emails.each do |email|
%li{ data: { qa_selector: 'email_row_content' } }
= render partial: 'shared/email_with_badge', locals: { email: email.email, verified: email.confirmed? }
%span.float-right
- if email.email === current_user.commit_email_or_default
- if email.email === current_user.commit_email
%span.badge.badge-muted.badge-pill.gl-badge.badge-info= s_('Profiles|Commit email')
- if email.email === current_user.public_email
%span.badge.badge-muted.badge-pill.gl-badge.badge-info= s_('Profiles|Public email')
- if email.email === current_user.notification_email_or_default
- if email.email === current_user.notification_email
%span.badge.badge-muted.badge-pill.gl-badge.badge-info= s_('Profiles|Notification email')
- unless email.confirmed?
- confirm_title = "#{email.confirmation_sent_at ? _('Resend confirmation email') : _('Send confirmation email')}"
......
- form = local_assigns.fetch(:form)
.form-group
= form.label :notification_email, class: "label-bold"
= form.select :notification_email, @user.public_verified_emails, { include_blank: _('Use primary email (%{email})') % { email: @user.email }, selected: @user.notification_email }, class: "select2", disabled: local_assigns.fetch(:email_change_disabled, nil)
= form.select :notification_email, @user.public_verified_emails, { include_blank: _('Use primary email (%{email})') % { email: @user.email }, selected: @user.read_attribute(:notification_email) }, class: "select2", disabled: local_assigns.fetch(:email_change_disabled, nil)
.help-block
= local_assigns.fetch(:help_text, nil)
.form-group
......
......@@ -3,7 +3,7 @@
- show_export_button = local_assigns.fetch(:show_export_button, true)
- issuable_type = 'issues'
- can_edit = can?(current_user, :admin_project, @project)
- notification_email = @current_user.present? ? @current_user.notification_email_or_default : nil
- notification_email = @current_user.present? ? @current_user.notification_email : nil
.nav-controls.issues-nav-controls
- if show_feed_buttons
......
- issuable_type = 'merge-requests'
- notification_email = @current_user.present? ? @current_user.notification_email_or_default : nil
- notification_email = @current_user.present? ? @current_user.notification_email : nil
= render 'shared/issuable/feed_buttons', show_calendar_button: false
.js-csv-import-export-buttons{ data: { show_export_button: "true", issuable_type: issuable_type, issuable_count: issuables_count_for_state(issuable_type.to_sym, params[:state]), email: notification_email, export_csv_path: export_csv_project_merge_requests_path(@project, request.query_parameters), container_class: 'gl-mr-3' } }
......
......@@ -59,7 +59,7 @@ module CredentialsInventoryActions
if credential.is_a?(Key)
CredentialsInventoryMailer.ssh_key_deleted_email(
params: {
notification_email: credential.user.notification_email_or_default,
notification_email: credential.user.notification_email,
title: credential.title,
last_used_at: credential.last_used_at,
created_at: credential.created_at
......
......@@ -10,7 +10,7 @@ class CredentialsInventoryMailer < ApplicationMailer
@token = token
mail(
to: token.user.notification_email_or_default,
to: token.user.notification_email,
subject: _('Your Personal Access Token was revoked')
)
end
......
......@@ -11,7 +11,7 @@ module EE
@target_url = profile_personal_access_tokens_url
::Gitlab::I18n.with_locale(@user.preferred_language) do
mail(to: user.notification_email_or_default, subject: subject(_("One or more of you personal access tokens were revoked")))
mail(to: user.notification_email, subject: subject(_("One or more of you personal access tokens were revoked")))
end
end
end
......
......@@ -4,7 +4,7 @@ module Emails
module UserCap
def user_cap_reached(user_id)
user = User.find(user_id)
email = user.notification_email_or_default
email = user.notification_email
@url_to_user_cap = 'https://docs.gitlab.com/ee/user/admin_area/settings/sign_up_restrictions.html#user-cap'
@url_to_pending_users = 'https://docs.gitlab.com/ee/user/admin_area/approving_users.html#view-user-sign-ups-pending-approval'
......
......@@ -42,8 +42,8 @@ module EE
"Commit message does not follow the pattern '#{push_rule.commit_message_regex}'"
elsif push_rule.commit_message_blocked?(params[:commit_message])
"Commit message contains the forbidden pattern '#{push_rule.commit_message_negative_regex}'"
elsif !push_rule.author_email_allowed?(current_user.commit_email_or_default)
"Author's commit email '#{current_user.commit_email_or_default}' does not follow the pattern '#{push_rule.author_email_regex}'"
elsif !push_rule.author_email_allowed?(current_user.commit_email)
"Commit author's email '#{current_user.commit_email}' does not follow the pattern '#{push_rule.author_email_regex}'"
end
end
......
......@@ -37,7 +37,7 @@ RSpec.describe 'Merge request > User merges with Push Rules', :js do
it 'displays error message after merge request is clicked' do
click_button 'Merge'
expect(page).to have_content("Author's commit email '#{user.email}' does not follow the pattern '#{push_rule.author_email_regex}'")
expect(page).to have_content("Commit author's email '#{user.email}' does not follow the pattern '#{push_rule.author_email_regex}'")
end
end
......
......@@ -17,7 +17,7 @@ RSpec.describe CredentialsInventoryMailer do
it { is_expected.to have_body_text token.name }
it { is_expected.to have_body_text "Created on #{token.created_at.to_date.to_s(:medium)}" }
it { is_expected.to have_body_text 'Scopes: api, sudo'}
it { is_expected.to be_delivered_to [token.user.notification_email_or_default] }
it { is_expected.to be_delivered_to [token.user.notification_email] }
it { is_expected.to have_body_text 'Last used 21 days ago' }
end
......@@ -26,7 +26,7 @@ RSpec.describe CredentialsInventoryMailer do
let(:params) do
{
notification_email: ssh_key.user.notification_email_or_default,
notification_email: ssh_key.user.notification_email,
title: ssh_key.title,
last_used_at: ssh_key.last_used_at,
created_at: ssh_key.created_at
......@@ -37,7 +37,7 @@ RSpec.describe CredentialsInventoryMailer do
it { is_expected.to have_subject 'Your SSH key was deleted' }
it { is_expected.to have_body_text 'The following SSH key was deleted by an administrator, Revoker' }
it { is_expected.to be_delivered_to [ssh_key.user.notification_email_or_default] }
it { is_expected.to be_delivered_to [ssh_key.user.notification_email] }
it { is_expected.to have_body_text ssh_key.title }
it { is_expected.to have_body_text "Created on #{ssh_key.created_at.to_date.to_s(:medium)}" }
it { is_expected.to have_body_text 'Last used 21 days ago' }
......
......@@ -11,7 +11,7 @@ RSpec.describe Emails::UserCap do
subject { Notify.user_cap_reached(user.id) }
it { is_expected.to have_subject('Important information about usage on your GitLab instance') }
it { is_expected.to be_delivered_to([user.notification_email_or_default]) }
it { is_expected.to be_delivered_to([user.notification_email]) }
it { is_expected.to have_body_text('Your GitLab instance has reached the maximum allowed') }
it { is_expected.to have_body_text('user cap') }
end
......
......@@ -153,7 +153,7 @@ RSpec.describe Users::UpdateService do
end.not_to change { user.reload.commit_email }
end
it 'does not update public email if an user has group managed account' do
it 'does not update public if an user has group managed account' do
allow(user).to receive(:group_managed_account?).and_return(true)
expect do
......@@ -161,7 +161,7 @@ RSpec.describe Users::UpdateService do
end.not_to change { user.reload.public_email }
end
it 'does not update notification email if an user has group managed account' do
it 'does not update public if an user has group managed account' do
allow(user).to receive(:group_managed_account?).and_return(true)
expect do
......
......@@ -4,7 +4,7 @@ module API
module Entities
class GlobalNotificationSetting < Entities::NotificationSetting
expose :notification_email do |notification_setting, options|
notification_setting.user.notification_email_or_default
notification_setting.user.notification_email
end
end
end
......
......@@ -14,7 +14,7 @@ module API
expose :two_factor_enabled?, as: :two_factor_enabled
expose :external
expose :private_profile
expose :commit_email_or_default, as: :commit_email
expose :commit_email
end
end
end
......
......@@ -14,7 +14,7 @@ module Gitlab
mail(
template_path: 'unconfirm_mailer',
template_name: 'unconfirm_notification_email',
to: @user.notification_email_or_default,
to: @user.notification_email,
subject: subject('GitLab email verification request')
)
end
......
......@@ -6,7 +6,7 @@ module Gitlab
attr_reader :username, :name, :email, :gl_id, :timezone
def self.from_gitlab(gitlab_user)
new(gitlab_user.username, gitlab_user.name, gitlab_user.commit_email_or_default, Gitlab::GlId.gl_id(gitlab_user), gitlab_user.timezone)
new(gitlab_user.username, gitlab_user.name, gitlab_user.commit_email, Gitlab::GlId.gl_id(gitlab_user), gitlab_user.timezone)
end
def self.from_gitaly(gitaly_user)
......
......@@ -146,7 +146,7 @@ RSpec.describe Admin::UsersController do
it 'sends the user a rejection email' do
expect_next_instance_of(NotificationService) do |notification|
allow(notification).to receive(:user_admin_rejection).with(user.name, user.notification_email_or_default)
allow(notification).to receive(:user_admin_rejection).with(user.name, user.notification_email)
end
subject
......
......@@ -24,7 +24,7 @@ RSpec.describe 'Groups > Members > Request access' do
it 'user can request access to a group' do
perform_enqueued_jobs { click_link 'Request Access' }
expect(ActionMailer::Base.deliveries.last.to).to eq [owner.notification_email_or_default]
expect(ActionMailer::Base.deliveries.last.to).to eq [owner.notification_email]
expect(ActionMailer::Base.deliveries.last.subject).to match "Request to join the #{group.name} group"
expect(group.requesters.exists?(user_id: user)).to be_truthy
......
......@@ -44,7 +44,7 @@ RSpec.describe 'Issues csv', :js do
request_csv
expect(page).to have_content 'CSV export has started'
expect(page).to have_content "emailed to #{user.notification_email_or_default}"
expect(page).to have_content "emailed to #{user.notification_email}"
end
it 'includes a csv attachment', :sidekiq_might_not_need_inline do
......
......@@ -23,7 +23,7 @@ RSpec.describe 'Projects > Members > User requests access', :js do
it 'user can request access to a project' do
perform_enqueued_jobs { click_link 'Request Access' }
expect(ActionMailer::Base.deliveries.last.to).to eq [maintainer.notification_email_or_default]
expect(ActionMailer::Base.deliveries.last.to).to eq [maintainer.notification_email]
expect(ActionMailer::Base.deliveries.last.subject).to eq "Request to join the #{project.full_name} project"
expect(project.requesters.exists?(user_id: user)).to be_truthy
......
......@@ -310,7 +310,7 @@ RSpec.describe IssuesHelper do
can_bulk_update: 'true',
can_edit: 'true',
can_import_issues: 'true',
email: current_user&.notification_email_or_default,
email: current_user&.notification_email,
emails_help_page_path: help_page_path('development/emails', anchor: 'email-namespace'),
empty_state_svg_path: '#',
export_csv_path: export_csv_project_issues_path(project),
......
......@@ -23,7 +23,7 @@ RSpec.describe Emails::InProductMarketing do
it 'sends to the right user with a link to unsubscribe' do
aggregate_failures do
expect(subject).to deliver_to(user.notification_email_or_default)
expect(subject).to deliver_to(user.notification_email)
expect(subject).to have_body_text(profile_notifications_url)
end
end
......
......@@ -71,7 +71,7 @@ RSpec.describe Notify do
it 'is sent to the assignee as the author' do
aggregate_failures do
expect_sender(current_user)
expect(subject).to deliver_to(recipient.notification_email_or_default)
expect(subject).to deliver_to(recipient.notification_email)
end
end
end
......@@ -710,7 +710,7 @@ RSpec.describe Notify do
it 'contains all the useful information' do
to_emails = subject.header[:to].addrs.map(&:address)
expect(to_emails).to eq([recipient.notification_email_or_default])
expect(to_emails).to eq([recipient.notification_email])
is_expected.to have_subject "Request to join the #{project.full_name} project"
is_expected.to have_body_text project.full_name
......@@ -1047,7 +1047,7 @@ RSpec.describe Notify do
it 'is sent to the given recipient as the author' do
aggregate_failures do
expect_sender(note_author)
expect(subject).to deliver_to(recipient.notification_email_or_default)
expect(subject).to deliver_to(recipient.notification_email)
end
end
......@@ -1204,7 +1204,7 @@ RSpec.describe Notify do
it 'is sent to the given recipient as the author' do
aggregate_failures do
expect_sender(note_author)
expect(subject).to deliver_to(recipient.notification_email_or_default)
expect(subject).to deliver_to(recipient.notification_email)
end
end
......@@ -1341,7 +1341,7 @@ RSpec.describe Notify do
it 'contains all the useful information' do
to_emails = subject.header[:to].addrs.map(&:address)
expect(to_emails).to eq([recipient.notification_email_or_default])
expect(to_emails).to eq([recipient.notification_email])
is_expected.to have_subject "Request to join the #{group.name} group"
is_expected.to have_body_text group.name
......
......@@ -48,7 +48,7 @@ RSpec.describe Integrations::PipelinesEmail, :mailer do
end
it 'sends email' do
emails = receivers.map { |r| double(notification_email_or_default: r) }
emails = receivers.map { |r| double(notification_email: r) }
should_only_email(*emails, kind: :bcc)
end
......
......@@ -1826,7 +1826,7 @@ RSpec.describe Repository do
expect(merge_commit.message).to eq('Custom message')
expect(merge_commit.author_name).to eq(user.name)
expect(merge_commit.author_email).to eq(user.commit_email_or_default)
expect(merge_commit.author_email).to eq(user.commit_email)
expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present
end
end
......
......@@ -435,12 +435,12 @@ RSpec.describe User do
subject { create(:user).tap { |user| user.emails << build(:email, email: email_value, confirmed_at: Time.current) } }
end
describe '#commit_email_or_default' do
describe '#commit_email' do
subject(:user) { create(:user) }
it 'defaults to the primary email' do
expect(user.email).to be_present
expect(user.commit_email_or_default).to eq(user.email)
expect(user.commit_email).to eq(user.email)
end
it 'defaults to the primary email when the column in the database is null' do
......@@ -448,18 +448,14 @@ RSpec.describe User do
found_user = described_class.find_by(id: user.id)
expect(found_user.commit_email_or_default).to eq(user.email)
expect(found_user.commit_email).to eq(user.email)
end
it 'returns the private commit email when commit_email has _private' do
user.update_column(:commit_email, Gitlab::PrivateCommitEmail::TOKEN)
expect(user.commit_email_or_default).to eq(user.private_commit_email)
expect(user.commit_email).to eq(user.private_commit_email)
end
end
describe '#commit_email=' do
subject(:user) { create(:user) }
it 'can be set to a confirmed email' do
confirmed = create(:email, :confirmed, user: user)
......@@ -1250,37 +1246,32 @@ RSpec.describe User do
end
end
describe 'when changing email' do
describe '#update_notification_email' do
# Regression: https://gitlab.com/gitlab-org/gitlab-foss/issues/22846
context 'when changing :email' do
let(:user) { create(:user) }
let(:new_email) { 'new-email@example.com' }
context 'if notification_email was nil' do
it 'sets :unconfirmed_email' do
expect do
user.tap { |u| u.update!(email: new_email) }.reload
end.to change(user, :unconfirmed_email).to(new_email)
end
it 'does not change notification_email or notification_email_or_default before email is confirmed' do
it 'does not change :notification_email' do
expect do
user.tap { |u| u.update!(email: new_email) }.reload
end.not_to change(user, :notification_email_or_default)
expect(user.notification_email).to be_nil
end.not_to change(user, :notification_email)
end
it 'updates notification_email_or_default to the new email once confirmed' do
it 'updates :notification_email to the new email once confirmed' do
user.update!(email: new_email)
expect do
user.tap(&:confirm).reload
end.to change(user, :notification_email_or_default).to eq(new_email)
expect(user.notification_email).to be_nil
end
end.to change(user, :notification_email).to eq(new_email)
end
context 'when notification_email is set to a secondary email' do
context 'and :notification_email is set to a secondary email' do
let!(:email_attrs) { attributes_for(:email, :confirmed, user: user) }
let(:secondary) { create(:email, :confirmed, email: 'secondary@example.com', user: user) }
......@@ -1289,13 +1280,13 @@ RSpec.describe User do
user.tap { |u| u.update!(notification_email: email_attrs[:email]) }.reload
end
it 'does not change notification_email to email before email is confirmed' do
it 'does not change :notification_email to :email' do
expect do
user.tap { |u| u.update!(email: new_email) }.reload
end.not_to change(user, :notification_email)
end
it 'does not change notification_email to email once confirmed' do
it 'does not change :notification_email to :email once confirmed' do
user.update!(email: new_email)
expect do
......@@ -1304,6 +1295,7 @@ RSpec.describe User do
end
end
end
end
describe '#update_invalid_gpg_signatures' do
let(:user) do
......@@ -1886,7 +1878,6 @@ RSpec.describe User do
end
it 'does not send deactivated user an email' do
expect(NotificationService).not_to receive(:new)
user.deactivate
end
end
......@@ -1894,7 +1885,7 @@ RSpec.describe User do
context "when user deactivation emails are enabled" do
it 'sends deactivated user an email' do
expect_next_instance_of(NotificationService) do |notification|
allow(notification).to receive(:user_deactivated).with(user.name, user.notification_email_or_default)
allow(notification).to receive(:user_deactivated).with(user.name, user.notification_email)
end
user.deactivate
......@@ -3093,15 +3084,15 @@ RSpec.describe User do
end
end
describe '#notification_email_or_default' do
describe '#notification_email' do
let(:email) { 'gonzo@muppets.com' }
context 'when the column in the database is null' do
subject { create(:user, email: email, notification_email: nil) }
it 'defaults to the primary email' do
expect(subject.notification_email).to be nil
expect(subject.notification_email_or_default).to eq(email)
expect(subject.read_attribute(:notification_email)).to be nil
expect(subject.notification_email).to eq(email)
end
end
end
......@@ -5344,7 +5335,7 @@ RSpec.describe User do
let(:group) { nil }
it 'returns global notification email' do
is_expected.to eq(user.notification_email_or_default)
is_expected.to eq(user.notification_email)
end
end
......@@ -5352,7 +5343,7 @@ RSpec.describe User do
it 'returns global notification email' do
create(:notification_setting, user: user, source: group, notification_email: '')
is_expected.to eq(user.notification_email_or_default)
is_expected.to eq(user.notification_email)
end
end
......@@ -6141,7 +6132,7 @@ RSpec.describe User do
it 'does nothing' do
expect(subject).not_to receive(:save)
subject.unset_secondary_emails_matching_deleted_email!(deleted_email)
expect(subject.commit_email).to eq commit_email
expect(subject.read_attribute(:commit_email)).to eq commit_email
end
end
......@@ -6151,7 +6142,7 @@ RSpec.describe User do
it 'un-sets the secondary email' do
expect(subject).to receive(:save)
subject.unset_secondary_emails_matching_deleted_email!(deleted_email)
expect(subject.commit_email).to be nil
expect(subject.read_attribute(:commit_email)).to be nil
end
end
end
......
......@@ -13,7 +13,7 @@ RSpec.describe API::NotificationSettings do
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to be_a Hash
expect(json_response['notification_email']).to eq(user.notification_email_or_default)
expect(json_response['notification_email']).to eq(user.notification_email)
expect(json_response['level']).to eq(user.global_notification_setting.level)
end
end
......
......@@ -1202,7 +1202,7 @@ RSpec.describe API::Users do
it 'updates user with a new email' do
old_email = user.email
old_notification_email = user.notification_email_or_default
old_notification_email = user.notification_email
put api("/users/#{user.id}", admin), params: { email: 'new@email.com' }
user.reload
......@@ -1210,7 +1210,7 @@ RSpec.describe API::Users do
expect(response).to have_gitlab_http_status(:ok)
expect(user).to be_confirmed
expect(user.email).to eq(old_email)
expect(user.notification_email_or_default).to eq(old_notification_email)
expect(user.notification_email).to eq(old_notification_email)
expect(user.unconfirmed_email).to eq('new@email.com')
end
......
......@@ -2623,7 +2623,7 @@ RSpec.describe NotificationService, :mailer do
let_it_be(:user) { create(:user) }
it 'sends the user an email' do
notification.user_deactivated(user.name, user.notification_email_or_default)
notification.user_deactivated(user.name, user.notification_email)
should_only_email(user)
end
......
......@@ -70,7 +70,7 @@ RSpec.describe Suggestions::ApplyService do
author = suggestions.first.note.author
expect(user.commit_email).not_to eq(user.email)
expect(commit.author_email).to eq(author.commit_email_or_default)
expect(commit.author_email).to eq(author.commit_email)
expect(commit.committer_email).to eq(user.commit_email)
expect(commit.author_name).to eq(author.name)
expect(commit.committer_name).to eq(user.name)
......@@ -333,9 +333,9 @@ RSpec.describe Suggestions::ApplyService do
end
it 'created commit by same author and committer' do
expect(user.commit_email).to eq(author.commit_email_or_default)
expect(user.commit_email).to eq(author.commit_email)
expect(author).to eq(user)
expect(commit.author_email).to eq(author.commit_email_or_default)
expect(commit.author_email).to eq(author.commit_email)
expect(commit.committer_email).to eq(user.commit_email)
expect(commit.author_name).to eq(author.name)
expect(commit.committer_name).to eq(user.name)
......@@ -350,7 +350,7 @@ RSpec.describe Suggestions::ApplyService do
it 'created commit has authors info and commiters info' do
expect(user.commit_email).not_to eq(user.email)
expect(author).not_to eq(user)
expect(commit.author_email).to eq(author.commit_email_or_default)
expect(commit.author_email).to eq(author.commit_email)
expect(commit.committer_email).to eq(user.commit_email)
expect(commit.author_name).to eq(author.name)
expect(commit.committer_name).to eq(user.name)
......@@ -359,7 +359,7 @@ RSpec.describe Suggestions::ApplyService do
end
context 'multiple suggestions' do
let(:author_emails) { suggestions.map {|s| s.note.author.commit_email_or_default } }
let(:author_emails) { suggestions.map {|s| s.note.author.commit_email } }
let(:first_author) { suggestion.note.author }
let(:commit) { project.repository.commit }
......@@ -369,8 +369,8 @@ RSpec.describe Suggestions::ApplyService do
end
it 'uses first authors information' do
expect(author_emails).to include(first_author.commit_email_or_default).exactly(3)
expect(commit.author_email).to eq(first_author.commit_email_or_default)
expect(author_emails).to include(first_author.commit_email).exactly(3)
expect(commit.author_email).to eq(first_author.commit_email)
end
end
......
......@@ -44,7 +44,7 @@ RSpec.describe Users::RejectService do
it 'emails the user on rejection' do
expect_next_instance_of(NotificationService) do |notification|
allow(notification).to receive(:user_admin_rejection).with(user.name, user.notification_email_or_default)
allow(notification).to receive(:user_admin_rejection).with(user.name, user.notification_email)
end
subject
......
......@@ -2,7 +2,7 @@
module EmailHelpers
def sent_to_user(user, recipients: email_recipients)
recipients.count { |to| to == user.notification_email_or_default }
recipients.count { |to| to == user.notification_email }
end
def reset_delivered_emails!
......@@ -45,7 +45,7 @@ module EmailHelpers
end
def find_email_for(user)
ActionMailer::Base.deliveries.find { |d| d.to.include?(user.notification_email_or_default) }
ActionMailer::Base.deliveries.find { |d| d.to.include?(user.notification_email) }
end
def have_referable_subject(referable, include_project: true, reply: false)
......
......@@ -2,7 +2,7 @@
RSpec.shared_examples 'a multiple recipients email' do
it 'is sent to the given recipient' do
is_expected.to deliver_to recipient.notification_email_or_default
is_expected.to deliver_to recipient.notification_email
end
end
......@@ -21,7 +21,7 @@ end
RSpec.shared_examples 'an email sent to a user' do
it 'is sent to user\'s global notification email address' do
expect(subject).to deliver_to(recipient.notification_email_or_default)
expect(subject).to deliver_to(recipient.notification_email)
end
context 'with group notification email' do
......@@ -227,7 +227,7 @@ RSpec.shared_examples 'a note email' do
aggregate_failures do
expect(sender.display_name).to eq("#{note_author.name} (@#{note_author.username})")
expect(sender.address).to eq(gitlab_sender)
expect(subject).to deliver_to(recipient.notification_email_or_default)
expect(subject).to deliver_to(recipient.notification_email)
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