CLDC-2475 Update telephone validation (#1714)

* Change number validation to allow 0 at the beginning

* Update valid_phone_number?

* Move tests to request

* Make phone required for new users

* Allow + in the numbers

* Feature test

app/controllers/users_controller.rb

@@ -121,13 +121,15 @@ private
       @resource.errors.add :role, I18n.t("validations.role.invalid")
     end
 
-    if user_params[:phone].present? && !valid_phone_number?(user_params[:phone])
+    if user_params[:phone].blank?
+      @resource.errors.add :phone, :blank
+    elsif !valid_phone_number?(user_params[:phone])
       @resource.errors.add :phone
     end
   end
 
   def valid_phone_number?(number)
-    number.to_i.to_s == number && number.length >= 11
+    /^[+\d]{11,}$/.match?(number)
   end
 
   def format_error_messages

config/locales/en.yml

@@ -156,6 +156,7 @@ en:
               taken: "Enter an email address that hasn’t already been used to sign up"
             phone:
               invalid: "Enter a telephone number in the correct format"
+              blank: "Enter a telephone number"
             role:
               invalid: "Role must be data accessor, data provider or data coordinator"
               blank: "Select role"

spec/features/organisation_spec.rb

@@ -72,6 +72,7 @@ RSpec.describe "User Features" do
         expect(page).to have_content("Invite somebody to submit CORE data")
         fill_in("user[name]", with: "New User")
         fill_in("user[email]", with: "new_user@example.com")
+        fill_in("user[phone]", with: "+88877677777")
         choose("user-role-data-provider-field")
         expect(notify_client).to receive(:send_email).with(
           {

spec/features/user_spec.rb

@@ -322,30 +322,6 @@ RSpec.describe "User Features" do
         expect(page).to have_title("Error")
       end
 
-      it "validates telephone number is numeric" do
-        visit("users/new")
-        fill_in("user[name]", with: "New User")
-        fill_in("user[email]", with: "newuser@example.com")
-        fill_in("user[phone]", with: "randomstring")
-        click_button("Continue")
-        expect(page).to have_selector("#error-summary-title")
-        expect(page).to have_selector("#user-phone-field-error")
-        expect(page).to have_content(/Enter a telephone number in the correct format/)
-        expect(page).to have_title("Error")
-      end
-
-      it "validates telephone number is longer than 11 digits" do
-        visit("users/new")
-        fill_in("user[name]", with: "New User")
-        fill_in("user[email]", with: "newuser@example.com")
-        fill_in("user[phone]", with: "123")
-        click_button("Continue")
-        expect(page).to have_selector("#error-summary-title")
-        expect(page).to have_selector("#user-phone-field-error")
-        expect(page).to have_content(/Enter a telephone number in the correct format/)
-        expect(page).to have_title("Error")
-      end
-
       it "sets name, email, role, is_dpo and is_key_contact fields" do
         visit("users/new")
         fill_in("user[name]", with: "New User")

spec/requests/users_controller_spec.rb

@@ -850,6 +850,7 @@ RSpec.describe UsersController, type: :request do
             name: "new user ",
             email: "new_user@example.com",
             role: "data_coordinator",
+            phone: "12345678910",
           },
         }
       end
@@ -938,6 +939,54 @@ RSpec.describe UsersController, type: :request do
           expect(page).to have_content(I18n.t("activerecord.errors.models.user.attributes.email.blank"))
         end
       end
+
+      context "when validating telephone numbers" do
+        let(:params) do
+          {
+            "user": {
+              phone:,
+            },
+          }
+        end
+
+        context "when telephone number is not numeric" do
+          let(:phone) { "randomstring" }
+
+          it "validates telephone number" do
+            request
+            expect(response).to have_http_status(:unprocessable_entity)
+            expect(page).to have_content(I18n.t("activerecord.errors.models.user.attributes.phone.invalid"))
+          end
+        end
+
+        context "when telephone number is shorter than 11 digits" do
+          let(:phone) { "123" }
+
+          it "validates telephone number" do
+            request
+            expect(response).to have_http_status(:unprocessable_entity)
+            expect(page).to have_content(I18n.t("activerecord.errors.models.user.attributes.phone.invalid"))
+          end
+        end
+
+        context "when telephone number is in correct format" do
+          let(:phone) { "012345678919" }
+
+          it "validates telephone number" do
+            request
+            expect(page).not_to have_content(I18n.t("activerecord.errors.models.user.attributes.phone.invalid"))
+          end
+        end
+
+        context "when telephone number is in correct format and includes +" do
+          let(:phone) { "+12345678919" }
+
+          it "validates telephone number" do
+            request
+            expect(page).not_to have_content(I18n.t("activerecord.errors.models.user.attributes.phone.invalid"))
+          end
+        end
+      end
     end
 
     describe "#new" do
@@ -1273,6 +1322,7 @@ RSpec.describe UsersController, type: :request do
           expect(page).to have_field("user[name]")
           expect(page).to have_field("user[email]")
           expect(page).to have_field("user[role]")
+          expect(page).to have_field("user[phone]")
         end
 
         it "allows setting the role to `support`" do
@@ -1571,6 +1621,7 @@ RSpec.describe UsersController, type: :request do
             name: "new user",
             email:,
             role: "data_coordinator",
+            phone: "12345612456",
             organisation_id: organisation.id,
           },
         }
@@ -1602,6 +1653,7 @@ RSpec.describe UsersController, type: :request do
               name: "",
               email: "",
               role: "",
+              phone: "",
               organisation_id: nil,
             },
           }
@@ -1617,6 +1669,7 @@ RSpec.describe UsersController, type: :request do
           expect(page).to have_content(I18n.t("activerecord.errors.models.user.attributes.name.blank"))
           expect(page).to have_content(I18n.t("activerecord.errors.models.user.attributes.email.blank"))
           expect(page).to have_content(I18n.t("activerecord.errors.models.user.attributes.organisation_id.blank"))
+          expect(page).to have_content(I18n.t("activerecord.errors.models.user.attributes.phone.blank"))
         end
       end