Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validations/4773 phone numbers #4813

Merged
merged 4 commits into from
May 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions app/helpers/phone_number_helper.rb
Original file line number Diff line number Diff line change
@@ -1,23 +1,27 @@
module PhoneNumberHelper
VALID_PHONE_NUMBER_LENGTH = 12
VALID_PHONE_NUMBER_LENGTHS = [10, 12]
VALID_COUNTRY_CODE = "+1"

def valid_phone_number(number)
message = "must be 12 digits including country code (+1)"
message = "must be 10 digits or 12 digits including country code (+1)"

if number.nil? || number.empty?
return true, nil
end

if number.length != VALID_PHONE_NUMBER_LENGTH
if !VALID_PHONE_NUMBER_LENGTHS.include?(number.length)
return false, message
end

country_code = number[0..1]
phone_number = number[2..number.length]
if number.length == 12
country_code = number[0..1]
phone_number = number[2..number.length]

if country_code != VALID_COUNTRY_CODE
return false, message
if country_code != VALID_COUNTRY_CODE
return false, message
end
else
phone_number = number
end

if !phone_number.scan(/\D/).empty?
Expand Down
7 changes: 7 additions & 0 deletions app/models/casa_org.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ class CasaOrg < ApplicationRecord

before_create :set_slug
before_update :sanitize_svg
before_save :normalize_phone_number

validates :name, presence: true, uniqueness: true
validates_with CasaOrgValidator
Expand Down Expand Up @@ -103,6 +104,12 @@ def validate_twilio_credentials
errors.add(:base, "Your Twilio credentials are incorrect, kindly check and try again.")
end
end

def normalize_phone_number
if self.twilio_phone_number&.length == 10
self.twilio_phone_number = "+1#{self.twilio_phone_number}"
end
end
end

# == Schema Information
Expand Down
9 changes: 9 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class User < ApplicationRecord

before_update :record_previous_email
after_create :skip_email_confirmation_upon_creation
before_save :normalize_phone_number

validates_with UserValidator

Expand Down Expand Up @@ -171,6 +172,14 @@ def send_email_changed_notification?
def after_confirmation
send_email_changed_notification
end

private

def normalize_phone_number
if self.phone_number&.length == 10
self.phone_number = "+1#{self.phone_number}"
end
end
end
# == Schema Information
#
Expand Down
18 changes: 15 additions & 3 deletions spec/helpers/phone_number_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,30 +5,42 @@
include PhoneNumberHelper

context "valid phone number" do
it 'with empty string' do
valid, error = valid_phone_number("")
expect(valid).to be(true)
expect(error).to be_nil
end

it "with correct country code and 12 digits" do
valid, error = valid_phone_number("+12223334444")
expect(valid).to be(true)
expect(error).to be_nil
end

it "with 10 digits" do
valid, error = valid_phone_number("2223334444")
expect(valid).to be(true)
expect(error).to be_nil
end
end

context "invalid phone number" do
it "with incorrect country code" do
valid, error = valid_phone_number("+22223334444")
expect(valid).to be(false)
expect(error).to have_text("must be 12 digits including country code (+1)")
expect(error).to have_text("must be 10 digits or 12 digits including country code (+1)")
end

it "with short phone number" do
valid, error = valid_phone_number("+122")
expect(valid).to be(false)
expect(error).to have_text("must be 12 digits including country code (+1)")
expect(error).to have_text("must be 10 digits or 12 digits including country code (+1)")
end

it "with long phone number" do
valid, error = valid_phone_number("+12223334444555")
expect(valid).to be(false)
expect(error).to have_text("must be 12 digits including country code (+1)")
expect(error).to have_text("must be 10 digits or 12 digits including country code (+1)")
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/importers/supervisor_importer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,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 must be 12 digits including country code (+1)")
expect(alert[:exported_rows]).to include("Phone number must be 10 digits or 12 digits including country code (+1)")
end
end

Expand Down
2 changes: 1 addition & 1 deletion spec/lib/importers/volunteer_importer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,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 must be 12 digits including country code (+1)")
expect(alert[:exported_rows]).to include("Phone number must be 10 digits or 12 digits including country code (+1)")
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,24 @@
it "shows error message for phone number < 12 digits" do
role == "admin" || role == "user" ? fill_in("Phone number", with: "+141632489") : fill_in("#{role}_phone_number", with: "+141632489")
role == "user" ? click_on("Update Profile") : click_on("Submit")
expect(page).to have_text "Phone number must be 12 digits including country code (+1)"
expect(page).to have_text "Phone number must be 10 digits or 12 digits including country code (+1)"
end

it "shows error message for phone number > 12 digits" do
role == "admin" || role == "user" ? fill_in("Phone number", with: "+141632180923") : fill_in("#{role}_phone_number", with: "+141632180923")
role == "user" ? click_on("Update Profile") : click_on("Submit")
expect(page).to have_text "Phone number must be 12 digits including country code (+1)"
expect(page).to have_text "Phone number must be 10 digits or 12 digits including country code (+1)"
end

it "shows error message for bad phone number" do
role == "admin" || role == "user" ? fill_in("Phone number", with: "+141632u809o") : fill_in("#{role}_phone_number", with: "+141632u809o")
role == "user" ? click_on("Update Profile") : click_on("Submit")
expect(page).to have_text "Phone number must be 12 digits including country code (+1)"
expect(page).to have_text "Phone number must be 10 digits or 12 digits including country code (+1)"
end

it "shows error message for phone number without country code" do
role == "admin" || role == "user" ? fill_in("Phone number", with: "+24163218092") : fill_in("#{role}_phone_number", with: "+24163218092")
role == "user" ? click_on("Update Profile") : click_on("Submit")
expect(page).to have_text "Phone number must be 12 digits including country code (+1)"
expect(page).to have_text "Phone number must be 10 digits or 12 digits including country code (+1)"
end
end
2 changes: 1 addition & 1 deletion spec/system/devise/passwords/new_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

click_on "Send me reset password instructions"
expect(page).to have_content "1 error prohibited this User from being saved:"
expect(page).to have_text("Phone number must be 12 digits including country code (+1)")
expect(page).to have_text("Phone number must be 10 digits or 12 digits including country code (+1)")
end

it "displays error if user tries to submit empty form" do
Expand Down