Commit ed3c25e4 authored by Alexandra's avatar Alexandra

spacing and small optimisations

parent 39fbac86
...@@ -10,7 +10,7 @@ class ConfirmationsController < Devise::ConfirmationsController ...@@ -10,7 +10,7 @@ class ConfirmationsController < Devise::ConfirmationsController
users_almost_there_path users_almost_there_path
end end
def after_confirmation_path_for(resource_name, resource) def after_confirmation_path_for(_resource_name, resource)
# incoming resource can either be a :user or an :email # incoming resource can either be a :user or an :email
if signed_in?(:user) if signed_in?(:user)
after_sign_in_path_for(resource) after_sign_in_path_for(resource)
......
...@@ -2,7 +2,7 @@ class Profiles::EmailsController < Profiles::ApplicationController ...@@ -2,7 +2,7 @@ class Profiles::EmailsController < Profiles::ApplicationController
before_action :find_email, only: [:destroy, :resend_confirmation_instructions] before_action :find_email, only: [:destroy, :resend_confirmation_instructions]
def index def index
@primary = current_user.email @primary_email = current_user.email
@emails = current_user.emails.order_id_desc @emails = current_user.emails.order_id_desc
end end
...@@ -30,6 +30,7 @@ class Profiles::EmailsController < Profiles::ApplicationController ...@@ -30,6 +30,7 @@ class Profiles::EmailsController < Profiles::ApplicationController
else else
flash[:alert] = "There was a problem sending the confirmation email" flash[:alert] = "There was a problem sending the confirmation email"
end end
redirect_to profile_emails_url redirect_to profile_emails_url
end end
......
...@@ -164,7 +164,7 @@ class User < ActiveRecord::Base ...@@ -164,7 +164,7 @@ class User < ActiveRecord::Base
before_save :ensure_authentication_token, :ensure_incoming_email_token before_save :ensure_authentication_token, :ensure_incoming_email_token
before_save :ensure_user_rights_and_limits, if: :external_changed? before_save :ensure_user_rights_and_limits, if: :external_changed?
before_save :skip_reconfirmation!, if: ->(user) { user.email_changed? && user.read_only_attribute?(:email) } before_save :skip_reconfirmation!, if: ->(user) { user.email_changed? && user.read_only_attribute?(:email) }
before_save :check_for_verified_email, if: ->(user) { user.email_changed? && !user.new_record?} before_save :check_for_verified_email, if: ->(user) { user.email_changed? && !user.new_record? }
after_save :ensure_namespace_correct after_save :ensure_namespace_correct
after_destroy :post_destroy_hook after_destroy :post_destroy_hook
after_commit :update_emails_with_primary_email, on: :update, if: -> { previous_changes.key?('email') } after_commit :update_emails_with_primary_email, on: :update, if: -> { previous_changes.key?('email') }
...@@ -223,11 +223,6 @@ class User < ActiveRecord::Base ...@@ -223,11 +223,6 @@ class User < ActiveRecord::Base
end end
end end
# see if the new email is already a verified secondary email
def check_for_verified_email
skip_reconfirmation! if emails.find_by(email: self.email).try(:confirmed?)
end
mount_uploader :avatar, AvatarUploader mount_uploader :avatar, AvatarUploader
has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
...@@ -529,6 +524,11 @@ class User < ActiveRecord::Base ...@@ -529,6 +524,11 @@ class User < ActiveRecord::Base
errors.add(:public_email, "is not an email you own") unless all_emails.include?(public_email) errors.add(:public_email, "is not an email you own") unless all_emails.include?(public_email)
end end
# see if the new email is already a verified secondary email
def check_for_verified_email
skip_reconfirmation! if emails.confirmed.where(email: self.email).any?
end
# Note: the use of the Emails services will cause `saves` on the user object, running # Note: the use of the Emails services will cause `saves` on the user object, running
# through the callbacks again and can have side effects, such as the `previous_changes` # through the callbacks again and can have side effects, such as the `previous_changes`
# hash and `_was` variables getting munged. # hash and `_was` variables getting munged.
...@@ -843,7 +843,7 @@ class User < ActiveRecord::Base ...@@ -843,7 +843,7 @@ class User < ActiveRecord::Base
def verified_email?(check_email) def verified_email?(check_email)
downcased = check_email.downcase downcased = check_email.downcase
(email == downcased && primary_email_verified?) || emails.confirmed.where(email: downcased).exists? email == downcased ? primary_email_verified? : emails.confirmed.where(email: downcased).exists?
end end
def hook_attrs def hook_attrs
......
module Emails module Emails
class BaseService class BaseService
def initialize(user, opts = {}) def initialize(user, params = {})
@user = user @user, @params = user, params.dup
@email = opts[:email]
end end
end end
end end
module Emails module Emails
class ConfirmService < ::Emails::BaseService class ConfirmService < ::Emails::BaseService
def execute(email_record) def execute(email)
email_record.resend_confirmation_instructions email.resend_confirmation_instructions
end end
end end
end end
module Emails module Emails
class CreateService < ::Emails::BaseService class CreateService < ::Emails::BaseService
def execute(options = {}) def execute(extra_params = {})
@user.emails.create({ email: @email }.merge(options)) @user.emails.create(@params.merge(extra_params))
end end
end end
end end
module Emails module Emails
class DestroyService < ::Emails::BaseService class DestroyService < ::Emails::BaseService
def execute(email_record) def execute(email)
email_record.destroy && update_secondary_emails! email.destroy && update_secondary_emails!
end end
private private
......
...@@ -32,12 +32,12 @@ ...@@ -32,12 +32,12 @@
All email addresses will be used to identify your commits. All email addresses will be used to identify your commits.
%ul.well-list %ul.well-list
%li %li
= render partial: 'shared/email_with_badge', locals: { email: @primary, verified: current_user.confirmed? } = render partial: 'shared/email_with_badge', locals: { email: @primary_email, verified: current_user.confirmed? }
%span.pull-right %span.pull-right
%span.label.label-success Primary email %span.label.label-success Primary email
- if @primary === current_user.public_email - if @primary_email === current_user.public_email
%span.label.label-info Public email %span.label.label-info Public email
- if @primary === current_user.notification_email - if @primary_email === current_user.notification_email
%span.label.label-info Notification email %span.label.label-info Notification email
- @emails.each do |email| - @emails.each do |email|
%li %li
......
...@@ -11,7 +11,7 @@ describe Profiles::EmailsController do ...@@ -11,7 +11,7 @@ describe Profiles::EmailsController do
let(:email_params) { { email: "add_email@example.com" } } let(:email_params) { { email: "add_email@example.com" } }
it 'sends an email confirmation' do it 'sends an email confirmation' do
expect {post(:create, { email: email_params })}.to change { ActionMailer::Base.deliveries.size } expect { post(:create, { email: email_params }) }.to change { ActionMailer::Base.deliveries.size }
expect(ActionMailer::Base.deliveries.last.to).to eq [email_params[:email]] expect(ActionMailer::Base.deliveries.last.to).to eq [email_params[:email]]
expect(ActionMailer::Base.deliveries.last.subject).to match "Confirmation instructions" expect(ActionMailer::Base.deliveries.last.subject).to match "Confirmation instructions"
end end
...@@ -22,13 +22,14 @@ describe Profiles::EmailsController do ...@@ -22,13 +22,14 @@ describe Profiles::EmailsController do
it 'resends an email confirmation' do it 'resends an email confirmation' do
email = user.emails.create(email: 'add_email@example.com') email = user.emails.create(email: 'add_email@example.com')
expect {put(:resend_confirmation_instructions, { id: email })}.to change { ActionMailer::Base.deliveries.size }
expect { put(:resend_confirmation_instructions, { id: email }) }.to change { ActionMailer::Base.deliveries.size }
expect(ActionMailer::Base.deliveries.last.to).to eq [email_params[:email]] expect(ActionMailer::Base.deliveries.last.to).to eq [email_params[:email]]
expect(ActionMailer::Base.deliveries.last.subject).to match "Confirmation instructions" expect(ActionMailer::Base.deliveries.last.subject).to match "Confirmation instructions"
end end
it 'unable to resend an email confirmation' do it 'unable to resend an email confirmation' do
expect {put(:resend_confirmation_instructions, { id: 1 })}.not_to change { ActionMailer::Base.deliveries.size } expect { put(:resend_confirmation_instructions, { id: 1 }) }.not_to change { ActionMailer::Base.deliveries.size }
end end
end end
end end
...@@ -22,7 +22,9 @@ describe Email do ...@@ -22,7 +22,9 @@ describe Email do
it 'synchronizes the gpg keys when the email is updated' do it 'synchronizes the gpg keys when the email is updated' do
email = user.emails.create(email: 'new@email.com') email = user.emails.create(email: 'new@email.com')
expect(user).to receive(:update_invalid_gpg_signatures) expect(user).to receive(:update_invalid_gpg_signatures)
email.confirm email.confirm
end end
end end
...@@ -33,6 +35,7 @@ describe Email do ...@@ -33,6 +35,7 @@ describe Email do
it 'scopes confirmed emails' do it 'scopes confirmed emails' do
create(:email, :confirmed, user: user) create(:email, :confirmed, user: user)
create(:email, user: user) create(:email, user: user)
expect(user.emails.count).to eq 2 expect(user.emails.count).to eq 2
expect(user.emails.confirmed.count).to eq 1 expect(user.emails.confirmed.count).to eq 1
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment