spacing and small optimisations

10 files changed, 28 insertions, 24 deletions

diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb
index 51c26b9c17e..62ee9470704 100644
--- a/app/controllers/confirmations_controller.rb
+++ b/app/controllers/confirmations_controller.rb
@@ -10,7 +10,7 @@ class ConfirmationsController < Devise::ConfirmationsController
     users_almost_there_path
   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
     if signed_in?(:user)
       after_sign_in_path_for(resource)
diff --git a/app/controllers/profiles/emails_controller.rb b/app/controllers/profiles/emails_controller.rb
index 2c85606c271..3ffe5729c2c 100644
--- a/app/controllers/profiles/emails_controller.rb
+++ b/app/controllers/profiles/emails_controller.rb
@@ -2,7 +2,7 @@ class Profiles::EmailsController < Profiles::ApplicationController
   before_action :find_email, only: [:destroy, :resend_confirmation_instructions]
 
   def index
-    @primary = current_user.email
+    @primary_email = current_user.email
     @emails = current_user.emails.order_id_desc
   end
 
@@ -30,6 +30,7 @@ class Profiles::EmailsController < Profiles::ApplicationController
     else
       flash[:alert] = "There was a problem sending the confirmation email"
     end
+
     redirect_to profile_emails_url
   end
 
diff --git a/app/models/user.rb b/app/models/user.rb
index c6b2baea8e8..7a9561a04ff 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -164,7 +164,7 @@ class User < ActiveRecord::Base
   before_save :ensure_authentication_token, :ensure_incoming_email_token
   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 :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_destroy :post_destroy_hook
   after_commit :update_emails_with_primary_email, on: :update, if: -> { previous_changes.key?('email') }
@@ -223,11 +223,6 @@ class User < ActiveRecord::Base
     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
   has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
 
@@ -529,6 +524,11 @@ class User < ActiveRecord::Base
     errors.add(:public_email, "is not an email you own") unless all_emails.include?(public_email)
   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
   # through the callbacks again and can have side effects, such as the `previous_changes`
   # hash and `_was` variables getting munged.
@@ -843,7 +843,7 @@ class User < ActiveRecord::Base
 
   def verified_email?(check_email)
     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
 
   def hook_attrs
diff --git a/app/services/emails/base_service.rb b/app/services/emails/base_service.rb
index 227c87602fc..caddbf505fe 100644
--- a/app/services/emails/base_service.rb
+++ b/app/services/emails/base_service.rb
@@ -1,8 +1,7 @@
 module Emails
   class BaseService
-    def initialize(user, opts = {})
-      @user = user
-      @email = opts[:email]
+    def initialize(user, params = {})
+      @user, @params = user, params.dup
     end
   end
 end
diff --git a/app/services/emails/confirm_service.rb b/app/services/emails/confirm_service.rb
index e764f18ddd0..b5301bf2b82 100644
--- a/app/services/emails/confirm_service.rb
+++ b/app/services/emails/confirm_service.rb
@@ -1,7 +1,7 @@
 module Emails
   class ConfirmService < ::Emails::BaseService
-    def execute(email_record)
-      email_record.resend_confirmation_instructions
+    def execute(email)
+      email.resend_confirmation_instructions
     end
   end
 end
diff --git a/app/services/emails/create_service.rb b/app/services/emails/create_service.rb
index 0f6a1e24f02..94a841af7c3 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(options = {})
-      @user.emails.create({ email: @email }.merge(options))
+    def execute(extra_params = {})
+      @user.emails.create(@params.merge(extra_params))
     end
   end
 end
diff --git a/app/services/emails/destroy_service.rb b/app/services/emails/destroy_service.rb
index d29d7e69bde..aef863bd9d1 100644
--- a/app/services/emails/destroy_service.rb
+++ b/app/services/emails/destroy_service.rb
@@ -1,7 +1,7 @@
 module Emails
   class DestroyService < ::Emails::BaseService
-    def execute(email_record)
-      email_record.destroy && update_secondary_emails!
+    def execute(email)
+      email.destroy && update_secondary_emails!
     end
 
     private
diff --git a/app/views/profiles/emails/index.html.haml b/app/views/profiles/emails/index.html.haml
index b0dc8c526e1..df1df4f5d72 100644
--- a/app/views/profiles/emails/index.html.haml
+++ b/app/views/profiles/emails/index.html.haml
@@ -32,12 +32,12 @@
           All email addresses will be used to identify your commits.
     %ul.well-list
       %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.label.label-success Primary email
-          - if @primary === current_user.public_email
+          - if @primary_email === current_user.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
       - @emails.each do |email|
         %li
diff --git a/spec/controllers/profiles/emails_controller_spec.rb b/spec/controllers/profiles/emails_controller_spec.rb
index 69b34db0c2e..ecf14aad54f 100644
--- a/spec/controllers/profiles/emails_controller_spec.rb
+++ b/spec/controllers/profiles/emails_controller_spec.rb
@@ -11,7 +11,7 @@ describe Profiles::EmailsController do
     let(:email_params) { { email: "add_email@example.com" } }
 
     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.subject).to match "Confirmation instructions"
     end
@@ -22,13 +22,14 @@ describe Profiles::EmailsController do
 
     it 'resends an email confirmation' do
       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.subject).to match "Confirmation instructions"
     end
 
     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
diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb
index 3fd3fe03be5..b32dd31ae6d 100644
--- a/spec/models/email_spec.rb
+++ b/spec/models/email_spec.rb
@@ -22,7 +22,9 @@ describe Email do
 
     it 'synchronizes the gpg keys when the email is updated' do
       email = user.emails.create(email: 'new@email.com')
+
       expect(user).to receive(:update_invalid_gpg_signatures)
+
       email.confirm
     end
   end
@@ -33,6 +35,7 @@ describe Email do
     it 'scopes confirmed emails' do
       create(:email, :confirmed, user: user)
       create(:email, user: user)
+
       expect(user.emails.count).to eq 2
       expect(user.emails.confirmed.count).to eq 1
     end