From fb466f89857459d0344c90e8ef629d064abcd5c9 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 22 Jun 2023 08:08:08 +0100 Subject: [PATCH] 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 | 6 ++- config/locales/en.yml | 1 + spec/features/organisation_spec.rb | 1 + spec/features/user_spec.rb | 24 ------------ spec/requests/users_controller_spec.rb | 53 ++++++++++++++++++++++++++ 5 files changed, 59 insertions(+), 26 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 3f91fa659..31df38da7 100644 --- a/app/controllers/users_controller.rb +++ b/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 diff --git a/config/locales/en.yml b/config/locales/en.yml index 73cf91d0a..df75770f9 100644 --- a/config/locales/en.yml +++ b/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" diff --git a/spec/features/organisation_spec.rb b/spec/features/organisation_spec.rb index 7e25fe933..53b8a75cc 100644 --- a/spec/features/organisation_spec.rb +++ b/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( { diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index 222d2ff04..62e185b2e 100644 --- a/spec/features/user_spec.rb +++ b/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") diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 564167354..92e575cec 100644 --- a/spec/requests/users_controller_spec.rb +++ b/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