From b4be4259b783e803da6b72c4a4984efc8bbd4fcf Mon Sep 17 00:00:00 2001 From: Jakub Date: Sun, 29 Nov 2015 00:55:15 +0100 Subject: [PATCH 1/5] Level 1 officials are now auto-assigned. Rake user:check_email_domains created. --- app/models/user.rb | 14 ++++++++++++++ config/locales/settings.en.yml | 1 + config/locales/settings.es.yml | 1 + db/seeds.rb | 3 +++ lib/tasks/users.rake | 8 ++++++++ 5 files changed, 27 insertions(+) diff --git a/app/models/user.rb b/app/models/user.rb index b6ee626d6..d936d044e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -47,6 +47,8 @@ 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_create :check_email_domain def self.find_for_oauth(auth, signed_in_resource = nil) # Get the identity and user if they exist @@ -199,6 +201,18 @@ class User < ActiveRecord::Base def email_required? !erased? end + + def has_officials_email_domain? + domain = Setting.value_for 'email_domain_for_officials' + email.end_with? "@#{domain}" + end + + def check_email_domain + if !official? and has_officials_email_domain? + self.official_level = 1 + self.official_position = Setting.value_for 'official_level_1_name' + end + end private def clean_document_number diff --git a/config/locales/settings.en.yml b/config/locales/settings.en.yml index 5527736b8..b322d5e6a 100755 --- a/config/locales/settings.en.yml +++ b/config/locales/settings.en.yml @@ -10,3 +10,4 @@ en: max_votes_for_debate_edit: "Number of votes from which a Debate can no longer be edited" proposal_code_prefix: "Prefix for Proposal codes" votes_for_proposal_success: "Number of votes necessary for approval of a Proposal" + email_domain_for_officials: "Email domain for public officials" diff --git a/config/locales/settings.es.yml b/config/locales/settings.es.yml index 1a0bf67f2..da5d38e24 100644 --- a/config/locales/settings.es.yml +++ b/config/locales/settings.es.yml @@ -10,3 +10,4 @@ es: max_votes_for_debate_edit: "Número de votos en que un Debate deja de poderse editar" proposal_code_prefix: "Prefijo para los códigos de Propuestas" votes_for_proposal_success: "Número de votos necesarios para aprobar una Propuesta" + email_domain_for_officials: "Dominio de email para cargos públicos" \ No newline at end of file diff --git a/db/seeds.rb b/db/seeds.rb index 8e65d2925..939f484f2 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -26,3 +26,6 @@ Setting.create(key: 'proposal_code_prefix', value: 'MAD') # Number of votes needed for proposal success Setting.create(key: 'votes_for_proposal_success', value: '53726') + +# Users with this email domain will automatically be marked as level 1 officials +Setting.create(key: 'email_domain_for_officials', value: 'madrid.es') diff --git a/lib/tasks/users.rake b/lib/tasks/users.rake index 0e51d1c19..278cd0481 100644 --- a/lib/tasks/users.rake +++ b/lib/tasks/users.rake @@ -4,5 +4,13 @@ namespace :users do task count_failed_census_calls: :environment do User.find_each{ |user| User.reset_counters(user.id, :failed_census_calls)} end + + desc "Assigns official level to users with the officials' email domain" + task check_email_domains: :environment do + User.find_each do |user| + user.check_email_domain + user.save + end + end end From a1e4c1526b5612e5baf58f404aa954a4a82bb18c Mon Sep 17 00:00:00 2001 From: Jakub Date: Sun, 29 Nov 2015 14:23:50 +0100 Subject: [PATCH 2/5] Spec: test email_domain_for_officials --- spec/models/user_spec.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9b045a7ed..6ffe3901f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -239,6 +239,15 @@ describe User do end end end + + describe "check_email_domain" do + it "assigns official level to users with the officials' email domain" do + user1 = create(:user, email: "john@madrid.es") + user2 = create(:user, email: "john@example.org") + expect(user1.official_level).to eq(1) + expect(user2.official?).to_not eq(true) + end + end describe "self.search" do it "find users by email" do From da490a46616fa7c3f22d119d26aafd44d1b30485 Mon Sep 17 00:00:00 2001 From: Jakub Date: Mon, 30 Nov 2015 20:48:12 +0100 Subject: [PATCH 3/5] Officials level 1 is auto-assigned on email confirmation. Settings option is now optional. Also working on subdomains --- app/models/user.rb | 18 ++++++++++++++---- db/seeds.rb | 3 ++- lib/tasks/users.rake | 16 ++++++++++++---- spec/models/user_spec.rb | 24 +++++++++++++++++++----- 4 files changed, 47 insertions(+), 14 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index d936d044e..8d470f2e5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -48,7 +48,7 @@ class User < ActiveRecord::Base before_validation :clean_document_number - before_create :check_email_domain + before_save :check_if_confirmation def self.find_for_oauth(auth, signed_in_resource = nil) # Get the identity and user if they exist @@ -204,15 +204,25 @@ class User < ActiveRecord::Base def has_officials_email_domain? domain = Setting.value_for 'email_domain_for_officials' - email.end_with? "@#{domain}" + return false if !email or !domain or domain.length == 0 + (email.end_with? "@#{domain}") or (email.end_with? ".#{domain}") end - def check_email_domain - if !official? and has_officials_email_domain? + # 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_officials_email_domain + if confirmed_at and !official? and has_officials_email_domain? 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_officials_email_domain + end + end private def clean_document_number diff --git a/db/seeds.rb b/db/seeds.rb index 939f484f2..6870497d2 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -28,4 +28,5 @@ Setting.create(key: 'proposal_code_prefix', value: 'MAD') Setting.create(key: 'votes_for_proposal_success', value: '53726') # Users with this email domain will automatically be marked as level 1 officials -Setting.create(key: 'email_domain_for_officials', value: 'madrid.es') +# Emails under the domain's subdomains will also be included +Setting.create(key: 'email_domain_for_officials', value: '') diff --git a/lib/tasks/users.rake b/lib/tasks/users.rake index 278cd0481..f93d6c5be 100644 --- a/lib/tasks/users.rake +++ b/lib/tasks/users.rake @@ -6,10 +6,18 @@ namespace :users do end desc "Assigns official level to users with the officials' email domain" - task check_email_domains: :environment do - User.find_each do |user| - user.check_email_domain - user.save + task check_if_officials_email_domains: :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 + # We filter the mail addresses with SQL to speed up the process + # The real check will be done by check_if_officials_email_domains, however. + User.where('official_level = 0 and email like ?', "%#{domain}").find_each do |user| + user.check_if_officials_email_domain + puts "#{user.username} (#{user.email}) is now a level-1 official." if user.official? + user.save + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 6ffe3901f..82f9f9a58 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -240,12 +240,26 @@ describe User do end end - describe "check_email_domain" do - it "assigns official level to users with the officials' email domain" do - user1 = create(:user, email: "john@madrid.es") - user2 = create(:user, email: "john@example.org") + describe "check_if_officials_email_domain" do + it "assigns official level to confirmed users with the officials' email 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') + + user1 = create(:user, email: "john@officials.madrid.es", confirmed_at: Time.now) + user2 = create(:user, email: "john@yes.officials.madrid.es", confirmed_at: Time.now) + 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?).to_not eq(true) + 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 } + + # We reset the officials' domain setting + Setting.find_by(key: 'email_domain_for_officials').update(value: '') end end From ea9a2b3be6c0203c9377d0d900468e47c91db7b1 Mon Sep 17 00:00:00 2001 From: Jakub Date: Mon, 30 Nov 2015 21:52:48 +0100 Subject: [PATCH 4/5] Better names for the official email methods --- app/models/user.rb | 8 ++++---- config/locales/settings.es.yml | 2 +- lib/tasks/users.rake | 6 +++--- spec/models/user_spec.rb | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 8d470f2e5..59167d09b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -202,7 +202,7 @@ class User < ActiveRecord::Base !erased? end - def has_officials_email_domain? + 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}") @@ -210,8 +210,8 @@ class User < ActiveRecord::Base # 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_officials_email_domain - if confirmed_at and !official? and has_officials_email_domain? + 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 @@ -220,7 +220,7 @@ class User < ActiveRecord::Base 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_officials_email_domain + check_if_official_email end end diff --git a/config/locales/settings.es.yml b/config/locales/settings.es.yml index da5d38e24..681d45c41 100644 --- a/config/locales/settings.es.yml +++ b/config/locales/settings.es.yml @@ -10,4 +10,4 @@ es: max_votes_for_debate_edit: "Número de votos en que un Debate deja de poderse editar" proposal_code_prefix: "Prefijo para los códigos de Propuestas" votes_for_proposal_success: "Número de votos necesarios para aprobar una Propuesta" - email_domain_for_officials: "Dominio de email para cargos públicos" \ No newline at end of file + email_domain_for_officials: "Dominio de email para cargos públicos" diff --git a/lib/tasks/users.rake b/lib/tasks/users.rake index f93d6c5be..9e0043272 100644 --- a/lib/tasks/users.rake +++ b/lib/tasks/users.rake @@ -6,15 +6,15 @@ namespace :users do end desc "Assigns official level to users with the officials' email domain" - task check_if_officials_email_domains: :environment do + 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 # We filter the mail addresses with SQL to speed up the process - # The real check will be done by check_if_officials_email_domains, however. + # 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_officials_email_domain + user.check_if_official_email puts "#{user.username} (#{user.email}) is now a level-1 official." if user.official? user.save end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 82f9f9a58..6c4b4d199 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -240,7 +240,7 @@ describe User do end end - describe "check_if_officials_email_domain" do + describe "check_if_official_email" do it "assigns official level to confirmed users with the officials' email domain" do # We will use empleados.madrid.es as the officials' domain # Subdomains are also accepted From e4b31cfe2998efcccb7dce807edf7c2afd9ab4e2 Mon Sep 17 00:00:00 2001 From: Jakub Date: Tue, 1 Dec 2015 23:18:23 +0100 Subject: [PATCH 5/5] 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: '')