add primary email as a secondary email whenever the primary is changed

3 files changed, 12 insertions, 14 deletions

diff --git a/app/models/user.rb b/app/models/user.rb
index 57f619a083f..0f6144f9250 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -535,15 +535,13 @@ class User < ActiveRecord::Base
   # By using an `after_commit` instead of `after_update`, we avoid the recursive callback
   # scenario, though it then requires us to use the `previous_changes` hash
   def update_emails_with_primary_email
+    previous_email = previous_changes[:email][0]  # grab this before the DestroyService is called
     primary_email_record = emails.find_by(email: email)
-    if primary_email_record
-      previous_email = previous_changes[:email][0]
-      Emails::DestroyService.new(self).execute(primary_email_record)
+    Emails::DestroyService.new(self).execute(primary_email_record) if primary_email_record
 
-      # the original primary email was confirmed, and we want that to carry over.  We don't
-      # have access to the original confirmation values at this point, so just set confirmed_at
-      Emails::CreateService.new(self, email: previous_email).execute(confirmed_at: confirmed_at)
-    end
+    # the original primary email was confirmed, and we want that to carry over.  We don't
+    # have access to the original confirmation values at this point, so just set confirmed_at
+    Emails::CreateService.new(self, email: previous_email).execute(confirmed_at: confirmed_at)
   end
 
   def update_invalid_gpg_signatures
diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb
index 8e8ef411f23..7c88b544d3c 100644
--- a/spec/models/email_spec.rb
+++ b/spec/models/email_spec.rb
@@ -32,8 +32,8 @@ describe Email do
 
     it 'scopes confirmed emails' do
       create(:email, :confirmed, user: user)
-      expect(user.emails.count).to eq 1
-      expect(user.emails.unconfirmed.count).to eq 0
+      create(:email, user: user)
+      expect(user.emails.count).to eq 2
       expect(user.emails.confirmed.count).to eq 1
     end
   end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 83addbb809e..839b1f5da79 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -374,7 +374,7 @@ describe User do
     end
   end
 
-  describe 'after update hook' do
+  describe 'after commit hook' do
     describe '.update_invalid_gpg_signatures' do
       let(:user) do
         create(:user, email: 'tula.torphy@abshire.ca').tap do |user|
@@ -388,7 +388,7 @@ describe User do
       end
 
       it 'synchronizes the gpg keys when the email is updated' do
-        expect(user).to receive(:update_invalid_gpg_signatures)
+        expect(user).to receive(:update_invalid_gpg_signatures).at_most(:twice)
         user.update_attributes!(email: 'shawnee.ritchie@denesik.com')
       end
     end
@@ -407,11 +407,11 @@ describe User do
         @user.update_attributes!(email: 'new_primary@example.com')
       end
 
-      it 'does not add old primary to secondary emails' do
+      it 'adds old primary to secondary emails when secondary is a new email ' do
         @user.update_attributes!(email: 'new_primary@example.com')
         @user.reload
-        expect(@user.emails.count).to eq 1
-        expect(@user.emails.first.email).to eq @secondary.email
+        expect(@user.emails.count).to eq 2
+        expect(@user.emails.pluck(:email)).to match_array([@secondary.email, 'primary@example.com'])
       end
 
       it 'adds old primary to secondary emails if secondary is becoming a primary' do