Merge branch 'djensen-refactor-unsetting-of-user-secondary-emails-after-deletion' into 'master'

Refactor unset of User secondary emails on email destroy

See merge request gitlab-org/gitlab!68203

app/models/user.rb

@@ -39,6 +39,12 @@ class User < ApplicationRecord
   MAX_USERNAME_LENGTH = 255
   MIN_USERNAME_LENGTH = 2
 
+  SECONDARY_EMAIL_ATTRIBUTES = [
+    :commit_email,
+    :notification_email,
+    :public_email
+  ].freeze
+
   add_authentication_token_field :incoming_email_token, token_generator: -> { SecureRandom.hex.to_i(16).to_s(36) }
   add_authentication_token_field :feed_token
   add_authentication_token_field :static_object_token
@@ -1310,11 +1316,15 @@ class User < ApplicationRecord
     end
   end
 
-  def update_secondary_emails!
-    set_notification_email
-    set_public_email
-    set_commit_email
-    save if notification_email_changed? || public_email_changed? || commit_email_changed?
+  def unset_secondary_emails_matching_deleted_email!(deleted_email)
+    secondary_email_attribute_changed = false
+    SECONDARY_EMAIL_ATTRIBUTES.each do |attribute|
+      if read_attribute(attribute) == deleted_email
+        self.write_attribute(attribute, nil)
+        secondary_email_attribute_changed = true
+      end
+    end
+    save if secondary_email_attribute_changed
   end
 
   def admin_unsubscribe!

app/services/emails/destroy_service.rb

@@ -3,14 +3,14 @@
 module Emails
   class DestroyService < ::Emails::BaseService
     def execute(email)
-      email.destroy && update_secondary_emails!
+      email.destroy && update_secondary_emails!(email.email)
     end
 
     private
 
-    def update_secondary_emails!
+    def update_secondary_emails!(deleted_email)
       result = ::Users::UpdateService.new(@current_user, user: @user).execute do |user|
-        user.update_secondary_emails!
+        user.unset_secondary_emails_matching_deleted_email!(deleted_email)
       end
 
       result[:status] == :success

spec/models/user_spec.rb

@@ -6024,4 +6024,30 @@ RSpec.describe User do
       expect(described_class.by_provider_and_extern_uid(:github, 'my_github_id')).to match_array([expected_user])
     end
   end
+
+  describe '#unset_secondary_emails_matching_deleted_email!' do
+    let(:deleted_email) { 'kermit@muppets.com' }
+
+    subject { build(:user, commit_email: commit_email) }
+
+    context 'when no secondary email matches the deleted email' do
+      let(:commit_email) { 'fozzie@muppets.com' }
+
+      it 'does nothing' do
+        expect(subject).not_to receive(:save)
+        subject.unset_secondary_emails_matching_deleted_email!(deleted_email)
+        expect(subject.read_attribute(:commit_email)).to eq commit_email
+      end
+    end
+
+    context 'when a secondary email matches the deleted_email' do
+      let(:commit_email) { deleted_email }
+
+      it 'un-sets the secondary email' do
+        expect(subject).to receive(:save)
+        subject.unset_secondary_emails_matching_deleted_email!(deleted_email)
+        expect(subject.read_attribute(:commit_email)).to be nil
+      end
+    end
+  end
 end