From b46892f22aa2fe73857a6798c630123259e41a5c Mon Sep 17 00:00:00 2001 From: Harsohail Brar Date: Sat, 2 Apr 2022 15:38:54 -0600 Subject: [PATCH 01/11] add migration for phone_number attribute in user model --- app/models/casa_admin.rb | 1 + app/models/supervisor.rb | 1 + app/models/user.rb | 1 + app/models/volunteer.rb | 1 + db/migrate/20220402201247_add_phone_number_to_users.rb | 5 +++++ db/schema.rb | 3 ++- db/seeds.rb | 1 + db/seeds/db_populator.rb | 1 + 8 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20220402201247_add_phone_number_to_users.rb diff --git a/app/models/casa_admin.rb b/app/models/casa_admin.rb index 11b13237d9..5a127b159f 100644 --- a/app/models/casa_admin.rb +++ b/app/models/casa_admin.rb @@ -36,6 +36,7 @@ def change_to_supervisor! # invited_by_type :string # last_sign_in_at :datetime # last_sign_in_ip :string +# phone_number :string default("") # reset_password_sent_at :datetime # reset_password_token :string # sign_in_count :integer default(0), not null diff --git a/app/models/supervisor.rb b/app/models/supervisor.rb index f004339403..343f5161c5 100644 --- a/app/models/supervisor.rb +++ b/app/models/supervisor.rb @@ -63,6 +63,7 @@ def recently_unassigned_volunteers # invited_by_type :string # last_sign_in_at :datetime # last_sign_in_ip :string +# phone_number :string default("") # reset_password_sent_at :datetime # reset_password_token :string # sign_in_count :integer default(0), not null diff --git a/app/models/user.rb b/app/models/user.rb index e9e855df3a..3510acc9c6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -175,6 +175,7 @@ def last_deactivated_by # invited_by_type :string # last_sign_in_at :datetime # last_sign_in_ip :string +# phone_number :string default("") # reset_password_sent_at :datetime # reset_password_token :string # sign_in_count :integer default(0), not null diff --git a/app/models/volunteer.rb b/app/models/volunteer.rb index 3ae6022064..5dd3bc5689 100644 --- a/app/models/volunteer.rb +++ b/app/models/volunteer.rb @@ -143,6 +143,7 @@ def cases_where_contact_made_in_days(num_days = CONTACT_MADE_IN_DAYS_NUM) # invited_by_type :string # last_sign_in_at :datetime # last_sign_in_ip :string +# phone_number :string default("") # reset_password_sent_at :datetime # reset_password_token :string # sign_in_count :integer default(0), not null diff --git a/db/migrate/20220402201247_add_phone_number_to_users.rb b/db/migrate/20220402201247_add_phone_number_to_users.rb new file mode 100644 index 0000000000..82f76ebc93 --- /dev/null +++ b/db/migrate/20220402201247_add_phone_number_to_users.rb @@ -0,0 +1,5 @@ +class AddPhoneNumberToUsers < ActiveRecord::Migration[7.0] + def change + add_column :users, :phone_number, :string, :default => "" + end +end diff --git a/db/schema.rb b/db/schema.rb index a124f30657..5412e667d9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2022_03_23_145733) do +ActiveRecord::Schema[7.0].define(version: 2022_04_02_201247) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -400,6 +400,7 @@ t.datetime "last_sign_in_at", precision: nil t.string "current_sign_in_ip" t.string "last_sign_in_ip" + t.string "phone_number", default: "" t.index ["casa_org_id"], name: "index_users_on_casa_org_id" t.index ["email"], name: "index_users_on_email", unique: true t.index ["invitation_token"], name: "index_users_on_invitation_token", unique: true diff --git a/db/seeds.rb b/db/seeds.rb index e3e6dc09b5..eedc81ffa2 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -20,6 +20,7 @@ def initialize @rng = Random.new(random_seed) # rng = random number generator @db_populator = DbPopulator.new(rng) Faker::Config.random = rng + Faker::Config.locale = 'en-US' # only allow US phone numbers end def seed diff --git a/db/seeds/db_populator.rb b/db/seeds/db_populator.rb index 61eaa090d7..d237b50772 100644 --- a/db/seeds/db_populator.rb +++ b/db/seeds/db_populator.rb @@ -80,6 +80,7 @@ def create_users(casa_org, options) password: SEED_PASSWORD, password_confirmation: SEED_PASSWORD, display_name: Faker::Name.name, + phone_number: Faker::PhoneNumber.cell_phone_in_e164, active: true } # Approximately 1 out of 30 volunteers should be set to inactive. From c23c92cc704078e1847679c23dd29eb3abe399fa Mon Sep 17 00:00:00 2001 From: Harsohail Brar Date: Sat, 2 Apr 2022 16:45:47 -0600 Subject: [PATCH 02/11] update supervisor import to accept phone number as column --- app/lib/importers/supervisor_importer.rb | 15 +++++++++++---- public/supervisors.csv | 6 +++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/app/lib/importers/supervisor_importer.rb b/app/lib/importers/supervisor_importer.rb index 4607fe44a6..a12d630975 100644 --- a/app/lib/importers/supervisor_importer.rb +++ b/app/lib/importers/supervisor_importer.rb @@ -1,21 +1,28 @@ class SupervisorImporter < FileImporter - IMPORT_HEADER = ["email", "display_name", "supervisor_volunteers"] + IMPORT_HEADER = ["email", "display_name", "supervisor_volunteers", "phone_number"] + VALID_PHONE_NUMBER_LENGTH = 11 def self.import_supervisors(csv_filespec, org_id) new(csv_filespec, org_id).import_supervisors end def initialize(csv_filespec, org_id) - super(csv_filespec, org_id, "supervisors", ["email", "display_name", "supervisor_volunteers"]) + super(csv_filespec, org_id, "supervisors", ["email", "display_name", "supervisor_volunteers", "phone_number"]) end def import_supervisors import do |row| - supervisor_params = row.to_hash.slice(:display_name, :email).compact - + supervisor_params = row.to_hash.slice(:display_name, :email, :phone_number).compact + unless supervisor_params.key?(:email) raise "Row does not contain e-mail address." end + + if supervisor_params.key?(:phone_number) && supervisor_params[:phone_number].length != VALID_PHONE_NUMBER_LENGTH + raise "Phone number is not in correct format" + else + supervisor_params[:phone_number] = "+#{supervisor_params[:phone_number]}" + end supervisor = Supervisor.find_by(email: supervisor_params[:email]) volunteer_assignment_list = email_addresses_to_users(Volunteer, String(row[:supervisor_volunteers])) diff --git a/public/supervisors.csv b/public/supervisors.csv index 84b9002d08..26f3fcd184 100644 --- a/public/supervisors.csv +++ b/public/supervisors.csv @@ -1,3 +1,3 @@ -email,display_name,supervisor_volunteers -supervisor1@example.net,Supervisor One,volunteer1@example.net -supervisor2@example.net,Supervisor Two,"volunteer2@example.net, volunteer3@example.net" \ No newline at end of file +email,display_name,supervisor_volunteers,phone_number +supervisor1@example.net,Supervisor One,volunteer1@example.net,11111111111 +supervisor2@example.net,Supervisor Two,"volunteer2@example.net, volunteer3@example.net",11111111111 \ No newline at end of file From 27231b57e1e644181981c38fae788d43b7cc7fec Mon Sep 17 00:00:00 2001 From: Harsohail Brar Date: Sat, 2 Apr 2022 17:06:58 -0600 Subject: [PATCH 03/11] change existing test csv files to include phone numbers --- spec/fixtures/supervisor_volunteers.csv | 6 +++--- spec/fixtures/supervisors.csv | 8 ++++---- spec/fixtures/supervisors_without_email.csv | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/fixtures/supervisor_volunteers.csv b/spec/fixtures/supervisor_volunteers.csv index 70c1e8b56b..a20abbd7cb 100644 --- a/spec/fixtures/supervisor_volunteers.csv +++ b/spec/fixtures/supervisor_volunteers.csv @@ -1,3 +1,3 @@ -email,display_name,supervisor_volunteers -s5@example.com,s5,volunteer1@example.net -s6@example.com,s6,volunteer1@example.net +email,display_name,supervisor_volunteers,phone_number +s5@example.com,s5,volunteer1@example.net,11111111111 +s6@example.com,s6,volunteer1@example.net,11111111111 diff --git a/spec/fixtures/supervisors.csv b/spec/fixtures/supervisors.csv index 2ca92854fe..e6fcb859e1 100644 --- a/spec/fixtures/supervisors.csv +++ b/spec/fixtures/supervisors.csv @@ -1,4 +1,4 @@ -email,display_name,supervisor_volunteers -supervisor1@example.net,Supervisor One,volunteer1@example.net -supervisor2@example.net,Supervisor Two,"volunteer2@example.net, volunteer3@example.net" -supervisor3@example.net,Supervisor Three, +email,display_name,supervisor_volunteers,phone_number +supervisor1@example.net,Supervisor One,volunteer1@example.net,11111111111 +supervisor2@example.net,Supervisor Two,"volunteer2@example.net, volunteer3@example.net",11111111111 +supervisor3@example.net,Supervisor Three,,11111111111 diff --git a/spec/fixtures/supervisors_without_email.csv b/spec/fixtures/supervisors_without_email.csv index 614c99c8b5..037ba6840e 100644 --- a/spec/fixtures/supervisors_without_email.csv +++ b/spec/fixtures/supervisors_without_email.csv @@ -1,3 +1,3 @@ -email,display_name,supervisor_volunteers -supervisor1@example.net,Supervisor One,volunteer1@example.net -,Supervisor Two,"volunteer2@example.net, volunteer3@example.net" +email,display_name,supervisor_volunteers,phone_number +supervisor1@example.net,Supervisor One,volunteer1@example.net,11111111111 +,Supervisor Two,"volunteer2@example.net, volunteer3@example.net",11111111111 From 683088d8a4ce41d172c13051e8f3f3fac580dfe3 Mon Sep 17 00:00:00 2001 From: Harsohail Brar Date: Sat, 2 Apr 2022 19:05:58 -0600 Subject: [PATCH 04/11] add tests for importing supervisors with phone number column --- app/lib/importers/supervisor_importer.rb | 10 ++++-- .../supervisors_invalid_phone_numbers.csv | 5 +++ .../lib/importers/supervisor_importer_spec.rb | 32 +++++++++++++++++++ 3 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 spec/fixtures/supervisors_invalid_phone_numbers.csv diff --git a/app/lib/importers/supervisor_importer.rb b/app/lib/importers/supervisor_importer.rb index a12d630975..b9ba993bb3 100644 --- a/app/lib/importers/supervisor_importer.rb +++ b/app/lib/importers/supervisor_importer.rb @@ -18,10 +18,14 @@ def import_supervisors raise "Row does not contain e-mail address." end - if supervisor_params.key?(:phone_number) && supervisor_params[:phone_number].length != VALID_PHONE_NUMBER_LENGTH - raise "Phone number is not in correct format" + if supervisor_params.key?(:phone_number) + if supervisor_params[:phone_number].length != VALID_PHONE_NUMBER_LENGTH + raise "Phone number is not in correct format" + else + supervisor_params[:phone_number] = "+#{supervisor_params[:phone_number]}" + end else - supervisor_params[:phone_number] = "+#{supervisor_params[:phone_number]}" + supervisor_params[:phone_number] = "" end supervisor = Supervisor.find_by(email: supervisor_params[:email]) diff --git a/spec/fixtures/supervisors_invalid_phone_numbers.csv b/spec/fixtures/supervisors_invalid_phone_numbers.csv new file mode 100644 index 0000000000..e9b8266ce7 --- /dev/null +++ b/spec/fixtures/supervisors_invalid_phone_numbers.csv @@ -0,0 +1,5 @@ +email,display_name,supervisor_volunteers,phone_number +supervisor1@example.net,Supervisor One,volunteer1@example.net,12345678 +supervisor2@example.net,Supervisor Two,"volunteer2@example.net, volunteer3@example.net",+++++++ +supervisor3@example.net,Supervisor Three,,111111111111111111 +supervisor4@example.net,Supervisor Four,,1.111.111.1111 \ No newline at end of file diff --git a/spec/lib/importers/supervisor_importer_spec.rb b/spec/lib/importers/supervisor_importer_spec.rb index 630d3c9866..4e2a250ea0 100644 --- a/spec/lib/importers/supervisor_importer_spec.rb +++ b/spec/lib/importers/supervisor_importer_spec.rb @@ -77,6 +77,13 @@ existing_supervisor.reload }.to change(existing_supervisor, :display_name).to("Supervisor Two") end + + it "updates phone number to valid number" do + expect { + supervisor_importer.import_supervisors + existing_supervisor.reload + }.to change(existing_supervisor, :phone_number).to("+11111111111") + end end context "when row doesn't have e-mail address" do @@ -91,6 +98,31 @@ end end + context "when row doesn't have phone number" do + let(:supervisor_import_data_path) { Rails.root.join("spec", "fixtures", "supervisors_without_phone_numbers.csv") } + + let!(:existing_supervisor_with_number) { create(:supervisor, display_name: "#", email: "supervisor1@example.net", phone_number: "+11111111111") } + + it "updates phone number to be deleted" do + expect { + supervisor_importer.import_supervisors + existing_supervisor_with_number.reload + }.to change(existing_supervisor_with_number, :phone_number).to("") + end + end + + context "when phone number in row is invalid" do + let(:supervisor_import_data_path) { Rails.root.join("spec", "fixtures", "supervisors_invalid_phone_numbers.csv") } + + it "returns an error message" do + alert = supervisor_importer.import_supervisors + + expect(alert[:type]).to eq(:error) + expect(alert[:message]).to eq("Not all rows were imported.") + expect(alert[:exported_rows]).to include("Phone number is not in correct format") + end + end + specify "static and instance methods have identical results" do SupervisorImporter.new(supervisor_import_data_path, casa_org_id).import_supervisors data_using_instance = Supervisor.pluck(:email).sort From a663676ad054c8ddcce28484fd6ae867328c28c0 Mon Sep 17 00:00:00 2001 From: Harsohail Brar Date: Sat, 2 Apr 2022 19:34:18 -0600 Subject: [PATCH 05/11] add check for correct phone number length but invalid characters --- app/lib/importers/supervisor_importer.rb | 2 +- spec/fixtures/supervisors_invalid_phone_numbers.csv | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/lib/importers/supervisor_importer.rb b/app/lib/importers/supervisor_importer.rb index b9ba993bb3..149b0e7b51 100644 --- a/app/lib/importers/supervisor_importer.rb +++ b/app/lib/importers/supervisor_importer.rb @@ -19,7 +19,7 @@ def import_supervisors end if supervisor_params.key?(:phone_number) - if supervisor_params[:phone_number].length != VALID_PHONE_NUMBER_LENGTH + if supervisor_params[:phone_number].length != VALID_PHONE_NUMBER_LENGTH || !supervisor_params[:phone_number].scan(/\D/).empty? raise "Phone number is not in correct format" else supervisor_params[:phone_number] = "+#{supervisor_params[:phone_number]}" diff --git a/spec/fixtures/supervisors_invalid_phone_numbers.csv b/spec/fixtures/supervisors_invalid_phone_numbers.csv index e9b8266ce7..ed7eeb8451 100644 --- a/spec/fixtures/supervisors_invalid_phone_numbers.csv +++ b/spec/fixtures/supervisors_invalid_phone_numbers.csv @@ -1,5 +1,5 @@ email,display_name,supervisor_volunteers,phone_number supervisor1@example.net,Supervisor One,volunteer1@example.net,12345678 -supervisor2@example.net,Supervisor Two,"volunteer2@example.net, volunteer3@example.net",+++++++ +supervisor2@example.net,Supervisor Two,"volunteer2@example.net, volunteer3@example.net",+++++++++++ supervisor3@example.net,Supervisor Three,,111111111111111111 supervisor4@example.net,Supervisor Four,,1.111.111.1111 \ No newline at end of file From 8b49fb7a3cbe37192e004020264956b91ae3ec61 Mon Sep 17 00:00:00 2001 From: Harsohail Brar Date: Sat, 2 Apr 2022 21:20:16 -0600 Subject: [PATCH 06/11] fix linting errors --- app/lib/importers/supervisor_importer.rb | 4 ++-- db/migrate/20220402201247_add_phone_number_to_users.rb | 2 +- db/seeds.rb | 2 +- spec/lib/importers/supervisor_importer_spec.rb | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/lib/importers/supervisor_importer.rb b/app/lib/importers/supervisor_importer.rb index 149b0e7b51..d9c458291e 100644 --- a/app/lib/importers/supervisor_importer.rb +++ b/app/lib/importers/supervisor_importer.rb @@ -13,11 +13,11 @@ def initialize(csv_filespec, org_id) def import_supervisors import do |row| supervisor_params = row.to_hash.slice(:display_name, :email, :phone_number).compact - + unless supervisor_params.key?(:email) raise "Row does not contain e-mail address." end - + if supervisor_params.key?(:phone_number) if supervisor_params[:phone_number].length != VALID_PHONE_NUMBER_LENGTH || !supervisor_params[:phone_number].scan(/\D/).empty? raise "Phone number is not in correct format" diff --git a/db/migrate/20220402201247_add_phone_number_to_users.rb b/db/migrate/20220402201247_add_phone_number_to_users.rb index 82f76ebc93..9d9b7a0949 100644 --- a/db/migrate/20220402201247_add_phone_number_to_users.rb +++ b/db/migrate/20220402201247_add_phone_number_to_users.rb @@ -1,5 +1,5 @@ class AddPhoneNumberToUsers < ActiveRecord::Migration[7.0] def change - add_column :users, :phone_number, :string, :default => "" + add_column :users, :phone_number, :string, default: "" end end diff --git a/db/seeds.rb b/db/seeds.rb index eedc81ffa2..c9fce17b3d 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -20,7 +20,7 @@ def initialize @rng = Random.new(random_seed) # rng = random number generator @db_populator = DbPopulator.new(rng) Faker::Config.random = rng - Faker::Config.locale = 'en-US' # only allow US phone numbers + Faker::Config.locale = "en-US" # only allow US phone numbers end def seed diff --git a/spec/lib/importers/supervisor_importer_spec.rb b/spec/lib/importers/supervisor_importer_spec.rb index 4e2a250ea0..d6b61d56c2 100644 --- a/spec/lib/importers/supervisor_importer_spec.rb +++ b/spec/lib/importers/supervisor_importer_spec.rb @@ -98,11 +98,11 @@ end end - context "when row doesn't have phone number" do + context "when row doesn't have phone number" do let(:supervisor_import_data_path) { Rails.root.join("spec", "fixtures", "supervisors_without_phone_numbers.csv") } let!(:existing_supervisor_with_number) { create(:supervisor, display_name: "#", email: "supervisor1@example.net", phone_number: "+11111111111") } - + it "updates phone number to be deleted" do expect { supervisor_importer.import_supervisors From 342ed9a3613493becec4d50889e99a2fa0566e1b Mon Sep 17 00:00:00 2001 From: Harsohail Brar Date: Sun, 3 Apr 2022 00:30:11 -0600 Subject: [PATCH 07/11] add supervisors without phone numbers csv file for testing --- spec/fixtures/supervisors_without_phone_numbers.csv | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 spec/fixtures/supervisors_without_phone_numbers.csv diff --git a/spec/fixtures/supervisors_without_phone_numbers.csv b/spec/fixtures/supervisors_without_phone_numbers.csv new file mode 100644 index 0000000000..26ee2a4424 --- /dev/null +++ b/spec/fixtures/supervisors_without_phone_numbers.csv @@ -0,0 +1,4 @@ +email,display_name,supervisor_volunteers,phone_number +supervisor1@example.net,Supervisor One,volunteer1@example.net, +supervisor2@example.net,Supervisor Two,"volunteer2@example.net, volunteer3@example.net", +supervisor3@example.net,Supervisor Three,, \ No newline at end of file From e32bbc10d866e59383c5c154550dd513b3e7ebb5 Mon Sep 17 00:00:00 2001 From: Harsohail Brar Date: Sun, 3 Apr 2022 10:50:43 -0600 Subject: [PATCH 08/11] add user validator to validate phone number contents --- app/lib/importers/supervisor_importer.rb | 12 ++------ app/models/user.rb | 2 ++ app/validators/user_validator.rb | 30 +++++++++++++++++++ .../lib/importers/supervisor_importer_spec.rb | 2 +- 4 files changed, 35 insertions(+), 11 deletions(-) create mode 100644 app/validators/user_validator.rb diff --git a/app/lib/importers/supervisor_importer.rb b/app/lib/importers/supervisor_importer.rb index d9c458291e..d2e37985bd 100644 --- a/app/lib/importers/supervisor_importer.rb +++ b/app/lib/importers/supervisor_importer.rb @@ -1,6 +1,5 @@ class SupervisorImporter < FileImporter IMPORT_HEADER = ["email", "display_name", "supervisor_volunteers", "phone_number"] - VALID_PHONE_NUMBER_LENGTH = 11 def self.import_supervisors(csv_filespec, org_id) new(csv_filespec, org_id).import_supervisors @@ -18,15 +17,8 @@ def import_supervisors raise "Row does not contain e-mail address." end - if supervisor_params.key?(:phone_number) - if supervisor_params[:phone_number].length != VALID_PHONE_NUMBER_LENGTH || !supervisor_params[:phone_number].scan(/\D/).empty? - raise "Phone number is not in correct format" - else - supervisor_params[:phone_number] = "+#{supervisor_params[:phone_number]}" - end - else - supervisor_params[:phone_number] = "" - end + has_phone_number = supervisor_params.key?(:phone_number) + supervisor_params[:phone_number] = has_phone_number ? "+#{supervisor_params[:phone_number]}" : "" supervisor = Supervisor.find_by(email: supervisor_params[:email]) volunteer_assignment_list = email_addresses_to_users(Volunteer, String(row[:supervisor_volunteers])) diff --git a/app/models/user.rb b/app/models/user.rb index 3510acc9c6..8896bbbbe8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -5,6 +5,8 @@ class User < ApplicationRecord include Roles include ByOrganizationScope + validates_with UserValidator + has_paper_trail devise :database_authenticatable, :invitable, :recoverable, :validatable, :timeoutable, :trackable diff --git a/app/validators/user_validator.rb b/app/validators/user_validator.rb new file mode 100644 index 0000000000..da2d67f8e8 --- /dev/null +++ b/app/validators/user_validator.rb @@ -0,0 +1,30 @@ +class UserValidator < ActiveModel::Validator + VALID_PHONE_NUMBER_LENGTH = 12 + VALID_COUNTRY_CODE = "+1" + PHONE_NUMBER_ERROR = "Phone number is not in correct format: 1XXXXXXXXXX" + + def validate(record) + if !valid_phone_number_contents(record.phone_number) + record.errors.add(:phone_number, PHONE_NUMBER_ERROR) + end + end + + private + + def valid_phone_number_contents(number) + unless number.empty? + if number.length != VALID_PHONE_NUMBER_LENGTH + return false + end + + country_code = number[0..1] + phone_number = number[2..number.length] + + if country_code != VALID_COUNTRY_CODE || !phone_number.scan(/\D/).empty? + return false + end + end + + true + end +end diff --git a/spec/lib/importers/supervisor_importer_spec.rb b/spec/lib/importers/supervisor_importer_spec.rb index d6b61d56c2..d4d46d25f0 100644 --- a/spec/lib/importers/supervisor_importer_spec.rb +++ b/spec/lib/importers/supervisor_importer_spec.rb @@ -119,7 +119,7 @@ expect(alert[:type]).to eq(:error) expect(alert[:message]).to eq("Not all rows were imported.") - expect(alert[:exported_rows]).to include("Phone number is not in correct format") + expect(alert[:exported_rows]).to include("Phone number is not in correct format: 1XXXXXXXXXX") end end From e5128e17c6b499417ec1093e0614faa843dd06c6 Mon Sep 17 00:00:00 2001 From: Harsohail Brar Date: Sun, 3 Apr 2022 10:55:54 -0600 Subject: [PATCH 09/11] reduce cognitive complexity of validation function --- app/validators/user_validator.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/app/validators/user_validator.rb b/app/validators/user_validator.rb index da2d67f8e8..5d4f37f76d 100644 --- a/app/validators/user_validator.rb +++ b/app/validators/user_validator.rb @@ -12,17 +12,19 @@ def validate(record) private def valid_phone_number_contents(number) - unless number.empty? - if number.length != VALID_PHONE_NUMBER_LENGTH + if number.empty? + return true + end + + if number.length != VALID_PHONE_NUMBER_LENGTH return false - end + end - country_code = number[0..1] - phone_number = number[2..number.length] + country_code = number[0..1] + phone_number = number[2..number.length] - if country_code != VALID_COUNTRY_CODE || !phone_number.scan(/\D/).empty? + if country_code != VALID_COUNTRY_CODE || !phone_number.scan(/\D/).empty? return false - end end true From 6a3fb0dcb6525ed85be42dda8b104d2f0a55bfd1 Mon Sep 17 00:00:00 2001 From: Harsohail Brar Date: Sun, 3 Apr 2022 10:59:52 -0600 Subject: [PATCH 10/11] fix linting issues --- app/lib/importers/supervisor_importer.rb | 3 +-- app/validators/user_validator.rb | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/app/lib/importers/supervisor_importer.rb b/app/lib/importers/supervisor_importer.rb index d2e37985bd..9d269e84e0 100644 --- a/app/lib/importers/supervisor_importer.rb +++ b/app/lib/importers/supervisor_importer.rb @@ -17,8 +17,7 @@ def import_supervisors raise "Row does not contain e-mail address." end - has_phone_number = supervisor_params.key?(:phone_number) - supervisor_params[:phone_number] = has_phone_number ? "+#{supervisor_params[:phone_number]}" : "" + supervisor_params[:phone_number] = supervisor_params.key?(:phone_number) ? "+#{supervisor_params[:phone_number]}" : "" supervisor = Supervisor.find_by(email: supervisor_params[:email]) volunteer_assignment_list = email_addresses_to_users(Volunteer, String(row[:supervisor_volunteers])) diff --git a/app/validators/user_validator.rb b/app/validators/user_validator.rb index 5d4f37f76d..89aee34a4e 100644 --- a/app/validators/user_validator.rb +++ b/app/validators/user_validator.rb @@ -13,18 +13,18 @@ def validate(record) def valid_phone_number_contents(number) if number.empty? - return true + return true end if number.length != VALID_PHONE_NUMBER_LENGTH - return false + return false end country_code = number[0..1] phone_number = number[2..number.length] if country_code != VALID_COUNTRY_CODE || !phone_number.scan(/\D/).empty? - return false + return false end true From 2842c057387c90382db31f5b9831ce0f09857124 Mon Sep 17 00:00:00 2001 From: Harsohail Brar Date: Sun, 3 Apr 2022 11:17:54 -0600 Subject: [PATCH 11/11] add user validator to skipped from testing like other validators --- .allow_skipping_tests | 1 + 1 file changed, 1 insertion(+) diff --git a/.allow_skipping_tests b/.allow_skipping_tests index 9a869609e2..00ac8036ca 100644 --- a/.allow_skipping_tests +++ b/.allow_skipping_tests @@ -61,6 +61,7 @@ services/create_all_casa_admin_service.rb services/create_casa_admin_service.rb services/mileage_export_csv_service.rb validators/court_report_validator.rb +validators/user_validator.rb values/all_casa_admin_parameters.rb values/casa_admin_parameters.rb values/supervisor_parameters.rb