Add latest changes from gitlab-org/gitlab@master

13 files changed, 51 insertions, 36 deletions

diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index f885ff9b45b..d5b771886f6 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -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
 
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index cb68aaf4583..1129f3ee7e1 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -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
 
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index 40e86b4623c..8897d73613c 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -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,
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index d7f1cd505e9..f30223f6f1e 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -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
diff --git a/app/mailers/emails/admin_notification.rb b/app/mailers/emails/admin_notification.rb
index e11f06d8fc9..f4540ef81a5 100644
--- a/app/mailers/emails/admin_notification.rb
+++ b/app/mailers/emails/admin_notification.rb
@@ -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
diff --git a/app/mailers/emails/profile.rb b/app/mailers/emails/profile.rb
index 592c394bb48..a8affb34f62 100644
--- a/app/mailers/emails/profile.rb
+++ b/app/mailers/emails/profile.rb
@@ -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
 
diff --git a/app/models/deploy_keys_project.rb b/app/models/deploy_keys_project.rb
index 40c66d5bc4c..363ef0b1c9a 100644
--- a/app/models/deploy_keys_project.rb
+++ b/app/models/deploy_keys_project.rb
@@ -3,7 +3,6 @@
 class DeployKeysProject < ApplicationRecord
   belongs_to :project, inverse_of: :deploy_keys_projects
   belongs_to :deploy_key, inverse_of: :deploy_keys_projects
-  scope :without_project_deleted,  -> { joins(:project).where(projects: { pending_delete: false }) }
   scope :in_project, ->(project) { where(project: project) }
   scope :with_write_access, -> { where(can_push: true) }
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 18b33ab031f..b0c79ffd0f3 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -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
diff --git a/app/views/profiles/_email_settings.html.haml b/app/views/profiles/_email_settings.html.haml
index bc678c2c429..95306633556 100644
--- a/app/views/profiles/_email_settings.html.haml
+++ b/app/views/profiles/_email_settings.html.haml
@@ -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
diff --git a/app/views/profiles/emails/index.html.haml b/app/views/profiles/emails/index.html.haml
index 35bdfbb1c29..c14efa99555 100644
--- a/app/views/profiles/emails/index.html.haml
+++ b/app/views/profiles/emails/index.html.haml
@@ -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')}"
diff --git a/app/views/profiles/notifications/_email_settings.html.haml b/app/views/profiles/notifications/_email_settings.html.haml
index f2121199412..e1e4b2bf2d6 100644
--- a/app/views/profiles/notifications/_email_settings.html.haml
+++ b/app/views/profiles/notifications/_email_settings.html.haml
@@ -1,7 +1,7 @@
 - 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
diff --git a/app/views/projects/issues/_nav_btns.html.haml b/app/views/projects/issues/_nav_btns.html.haml
index 0d69f6f69aa..1289f7aa0c4 100644
--- a/app/views/projects/issues/_nav_btns.html.haml
+++ b/app/views/projects/issues/_nav_btns.html.haml
@@ -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
diff --git a/app/views/projects/merge_requests/_nav_btns.html.haml b/app/views/projects/merge_requests/_nav_btns.html.haml
index b34cf23634c..84edf62b300 100644
--- a/app/views/projects/merge_requests/_nav_btns.html.haml
+++ b/app/views/projects/merge_requests/_nav_btns.html.haml
@@ -1,5 +1,5 @@
 - 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' } }