Make notification_email like commit_email

When there is no database value for the User#commit_email attribute,
the User model returns the primary email instead. This modifies the
User#notification_email attribute to behave the same way, instead of
setting the database value as if there had been user input. The intent
of this change is to help standardize the behavior of User secondary
email attributes, as part of a broader refactoring.

app/models/user.rb

@@ -248,7 +248,6 @@ class User < ApplicationRecord
     message: _("%{placeholder} is not a valid color scheme") % { placeholder: '%{value}' } }
 
   before_validation :sanitize_attrs
-  before_validation :set_notification_email, if: :new_record?
   before_validation :set_public_email, if: :public_email_changed?
   before_validation :set_commit_email, if: :commit_email_changed?
   before_save :default_private_profile_to_false
@@ -273,11 +272,6 @@ class User < ApplicationRecord
 
       update_emails_with_primary_email(previous_confirmed_at, previous_email)
       update_invalid_gpg_signatures
-
-      if previous_email == notification_email
-        self.notification_email = email
-        save
-      end
     end
   end
 
@@ -929,7 +923,7 @@ class User < ApplicationRecord
   end
 
   def notification_email_verified
-    return if new_record? || 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
@@ -970,6 +964,11 @@ class User < ApplicationRecord
     has_attribute?(:commit_email) && super
   end
 
+  def notification_email
+    # The notification email is the same as the primary email if undefined
+    super.presence || self.email
+  end
+
   def private_commit_email
     Gitlab::PrivateCommitEmail.for_user(self)
   end

spec/controllers/profiles/notifications_controller_spec.rb

@@ -162,7 +162,7 @@ RSpec.describe Profiles::NotificationsController do
     it 'shows an error message if the params are invalid' do
       sign_in(user)
 
-      put :update, params: { user: { notification_email: '' } }
+      put :update, params: { user: { notification_email: 'unverified@example.com' } }
 
       expect(user.reload.notification_email).to eq('original@example.com')
       expect(controller).to set_flash[:alert].to('Failed to save new settings')

spec/models/user_spec.rb

@@ -3125,6 +3125,19 @@ RSpec.describe User do
     end
   end
 
+  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.read_attribute(:notification_email)).to be nil
+        expect(subject.notification_email).to eq(email)
+      end
+    end
+  end
+
   describe '.find_by_private_commit_email' do
     context 'with email' do
       let_it_be(:user) { create(:user) }