summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBrett Walker <brett@digitalmoksha.com>2017-09-13 15:02:27 +0200
committerBrett Walker <brett@digitalmoksha.com>2017-09-23 15:24:53 +0200
commit85d2bf778a72f82b10f4bb6ebddc3cedf8ce4e4e (patch)
treea9de7ecc0d7580f447f91cf325ae9887dd43e374
parentb2d53791614da093a32fd7040371cf2054aa9817 (diff)
downloadgitlab-ce-85d2bf778a72f82b10f4bb6ebddc3cedf8ce4e4e.tar.gz
when a primary email is replaced and added to the secondary emails list,
make sure it stays confirmed
-rw-r--r--app/models/user.rb7
-rw-r--r--app/services/emails/create_service.rb4
-rw-r--r--spec/models/user_spec.rb38
-rw-r--r--spec/services/emails/create_service_spec.rb5
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