Merge pull request #4813 from Tscasady/validations/4773-phone-numbers

app/helpers/phone_number_helper.rb

@@ -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?

app/models/casa_org.rb

@@ -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
@@ -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

app/models/user.rb

@@ -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
 
@@ -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
 #

spec/helpers/phone_number_helper_spec.rb

@@ -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

spec/lib/importers/supervisor_importer_spec.rb

@@ -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
 

spec/lib/importers/volunteer_importer_spec.rb

@@ -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

spec/support/shared_examples/shows_error_for_invalid_phone_numbers.rb

@@ -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

spec/system/devise/passwords/new_spec.rb

@@ -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