From 85d2bf778a72f82b10f4bb6ebddc3cedf8ce4e4e Mon Sep 17 00:00:00 2001
From: Brett Walker <brett@digitalmoksha.com>
Date: Wed, 13 Sep 2017 15:02:27 +0200
Subject: [PATCH] when a primary email is replaced and added to the secondary
 emails list, make sure it stays confirmed

---
 app/models/user.rb                          |  7 ++--
 app/services/emails/create_service.rb       |  4 +--
 spec/models/user_spec.rb                    | 38 +++++++++++++++++++++
 spec/services/emails/create_service_spec.rb |  5 +++
 4 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 5e1355662b6..88dc5565a3a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -526,8 +526,11 @@ class User < ActiveRecord::Base
   def update_emails_with_primary_email
     primary_email_record = emails.find_by(email: email)
     if primary_email_record
-      Emails::DestroyService.new(self, email: email).execute
-      Emails::CreateService.new(self, email: email_was).execute
+      Emails::DestroyService.new(self).execute(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: email_was).execute(confirmed_at: confirmed_at_was)
     end
   end
 
diff --git a/app/services/emails/create_service.rb b/app/services/emails/create_service.rb
index b6491ee9804..051efd2b2c0 100644
--- a/app/services/emails/create_service.rb
+++ b/app/services/emails/create_service.rb
@@ -1,7 +1,7 @@
 module Emails
   class CreateService < ::Emails::BaseService
-    def execute
-      @user.emails.create(email: @email)
+    def execute(options = {})
+      @user.emails.create({email: @email}.merge(options))
     end
   end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 08f58e6d069..45f0144f68e 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -379,6 +379,44 @@ describe User do
         user.update_attributes!(email: 'shawnee.ritchie@denesik.com')
       end
     end
+
+    describe '#update_emails_with_primary_email' do
+      before do
+        @user = create(:user, email: 'primary@example.com').tap do |user|
+          user.skip_reconfirmation!
+        end
+        @secondary = create :email, email: 'secondary@example.com', user: @user
+        @user.reload
+      end
+
+      it 'gets called when email updated' do
+        expect(@user).to receive(:update_emails_with_primary_email)
+        @user.update_attributes!(email: 'new_primary@example.com')
+      end
+
+      it 'does not add old primary to secondary emails' 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
+      end
+
+      it 'adds old primary to secondary emails if secondary is becoming a primary' do
+        @user.update_attributes!(email: @secondary.email)
+        @user.reload
+        expect(@user.emails.count).to eq 1
+        expect(@user.emails.first.email).to eq 'primary@example.com'
+      end
+
+      it 'transfers old confirmation values into new secondary' do
+        org_user = @user
+        @user.update_attributes!(email: @secondary.email)
+        @user.reload
+        expect(@user.emails.count).to eq 1
+        expect(@user.emails.first.confirmed_at).not_to eq nil
+      end
+    end
+
   end
 
   describe '#update_tracked_fields!', :clean_gitlab_redis_shared_state do
diff --git a/spec/services/emails/create_service_spec.rb b/spec/services/emails/create_service_spec.rb
index 641d5538de8..d028ee27a16 100644
--- a/spec/services/emails/create_service_spec.rb
+++ b/spec/services/emails/create_service_spec.rb
@@ -12,6 +12,11 @@ describe Emails::CreateService do
       expect(Email.where(opts)).not_to be_empty
     end
 
+    it 'creates an email with additional attributes' do
+      expect { service.execute(confirmation_token: 'abc') }.to change { Email.count }.by(1)
+      expect(Email.where(opts).first.confirmation_token).to eq 'abc'
+    end
+
     it 'has the right user association' do
       service.execute