Merge branch 'djensen-refactor-notification-email-assumption' into 'master'

Make notification_email like commit_email

See merge request gitlab-org/gitlab!68204

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) }