From e4b31cfe2998efcccb7dce807edf7c2afd9ab4e2 Mon Sep 17 00:00:00 2001 From: Jakub Date: Tue, 1 Dec 2015 23:18:23 +0100 Subject: [PATCH] Move automatic official_level assignment to the controller. --- .../users/confirmations_controller.rb | 8 +++++++ app/models/user.rb | 21 +------------------ lib/tasks/users.rake | 11 +++++----- spec/models/user_spec.rb | 14 ++++++------- 4 files changed, 21 insertions(+), 33 deletions(-) diff --git a/app/controllers/users/confirmations_controller.rb b/app/controllers/users/confirmations_controller.rb index 78574cfd9..e4ababb7d 100644 --- a/app/controllers/users/confirmations_controller.rb +++ b/app/controllers/users/confirmations_controller.rb @@ -10,6 +10,7 @@ class Users::ConfirmationsController < Devise::ConfirmationsController if resource.valid? # password is set correctly resource.save + set_official_position if resource.has_official_email? resource.confirm set_flash_message(:notice, :confirmed) if is_flashing_format? sign_in_and_redirect(resource_name, resource) @@ -34,6 +35,7 @@ class Users::ConfirmationsController < Devise::ConfirmationsController if resource.encrypted_password.blank? respond_with_navigational(resource){ render :show } elsif resource.errors.empty? + set_official_position if resource.has_official_email? resource.confirm # Last change: confirm happens here for people with passwords instead of af the top of the show action set_flash_message(:notice, :confirmed) if is_flashing_format? respond_with_navigational(resource){ redirect_to after_confirmation_path_for(resource_name, resource) } @@ -48,4 +50,10 @@ class Users::ConfirmationsController < Devise::ConfirmationsController params.require(resource_name).permit(:password, :password_confirmation, :email) end + private + + def set_official_position + resource.add_official_position! (Setting.value_for 'official_level_1_name'), 1 + end + end diff --git a/app/models/user.rb b/app/models/user.rb index 59167d09b..dd2bc5cf0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -47,8 +47,6 @@ class User < ActiveRecord::Base scope :by_document, -> (document_type, document_number) { where(document_type: document_type, document_number: document_number) } before_validation :clean_document_number - - before_save :check_if_confirmation def self.find_for_oauth(auth, signed_in_resource = nil) # Get the identity and user if they exist @@ -204,24 +202,7 @@ class User < ActiveRecord::Base def has_official_email? domain = Setting.value_for 'email_domain_for_officials' - return false if !email or !domain or domain.length == 0 - (email.end_with? "@#{domain}") or (email.end_with? ".#{domain}") - end - - # Check if the user is confirmed and has an official email address - # In that case, we assign a level 1 official level - def check_if_official_email - if confirmed_at and !official? and has_official_email? - self.official_level = 1 - self.official_position = Setting.value_for 'official_level_1_name' - end - end - - def check_if_confirmation - # If we are confirming the mail address, we check if the user is an official - if confirmed_at and confirmed_at_changed? - check_if_official_email - end + !email.blank? && ( (email.end_with? "@#{domain}") || (email.end_with? ".#{domain}") ) end private diff --git a/lib/tasks/users.rake b/lib/tasks/users.rake index 9e0043272..7f82979f0 100644 --- a/lib/tasks/users.rake +++ b/lib/tasks/users.rake @@ -8,15 +8,16 @@ namespace :users do desc "Assigns official level to users with the officials' email domain" task check_for_official_emails: :environment do domain = Setting.value_for 'email_domain_for_officials' - + # We end the task if there is no email domain configured - if domain.length > 0 + if !domain.blank? # We filter the mail addresses with SQL to speed up the process # The real check will be done by check_if_official_email, however. User.where('official_level = 0 and email like ?', "%#{domain}").find_each do |user| - user.check_if_official_email - puts "#{user.username} (#{user.email}) is now a level-1 official." if user.official? - user.save + if user.has_official_email? + user.add_official_position! (Setting.value_for 'official_level_1_name'), 1 + puts "#{user.username} (#{user.email}) is now a level-1 official." + end end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 6c4b4d199..9b654ddb2 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -240,8 +240,8 @@ describe User do end end - describe "check_if_official_email" do - it "assigns official level to confirmed users with the officials' email domain" do + describe "has_official_email" do + it "checks if the mail address has the officials domain" do # We will use empleados.madrid.es as the officials' domain # Subdomains are also accepted Setting.find_by(key: 'email_domain_for_officials').update(value: 'officials.madrid.es') @@ -251,12 +251,10 @@ describe User do user3 = create(:user, email: "john@unofficials.madrid.es", confirmed_at: Time.now) user4 = create(:user, email: "john@example.org", confirmed_at: Time.now) - expect(user1.official_level).to eq(1) - expect(user2.official_level).to eq(1) - expect(user3.official?).to_not eq(true) - expect(user4.official?).to_not eq(true) - - [user1, user2, user3, user4].each { |user| user.destroy } + expect(user1.has_official_email?).to eq(true) + expect(user2.has_official_email?).to eq(true) + expect(user3.has_official_email?).to eq(false) + expect(user4.has_official_email?).to eq(false) # We reset the officials' domain setting Setting.find_by(key: 'email_domain_for_officials').update(value: '')