CLDC-3577: Add optional phone extension to users (#2597)

* CLDC-3577: Add optional phone extension to users

* CLDC-3577: Validate that phone extension is a number

app/controllers/users_controller.rb

@@ -192,14 +192,14 @@ private
   def user_params
     if @user == current_user
       if current_user.data_coordinator? || current_user.support?
-        params.require(:user).permit(:email, :phone, :name, :password, :password_confirmation, :role, :is_dpo, :is_key_contact, :initial_confirmation_sent)
+        params.require(:user).permit(:email, :phone, :phone_extension, :name, :password, :password_confirmation, :role, :is_dpo, :is_key_contact, :initial_confirmation_sent)
       else
-        params.require(:user).permit(:email, :phone, :name, :password, :password_confirmation, :initial_confirmation_sent)
+        params.require(:user).permit(:email, :phone, :phone_extension, :name, :password, :password_confirmation, :initial_confirmation_sent)
       end
     elsif current_user.data_coordinator?
-      params.require(:user).permit(:email, :phone, :name, :role, :is_dpo, :is_key_contact, :active, :initial_confirmation_sent)
+      params.require(:user).permit(:email, :phone, :phone_extension, :name, :role, :is_dpo, :is_key_contact, :active, :initial_confirmation_sent)
     elsif current_user.support?
-      params.require(:user).permit(:email, :phone, :name, :role, :is_dpo, :is_key_contact, :organisation_id, :active, :initial_confirmation_sent)
+      params.require(:user).permit(:email, :phone, :phone_extension, :name, :role, :is_dpo, :is_key_contact, :organisation_id, :active, :initial_confirmation_sent)
     end
   end
 

app/models/user.rb

@@ -19,6 +19,7 @@ class User < ApplicationRecord
   validates :password, presence: { if: :password_required? }
   validates :password, length: { within: Devise.password_length, allow_blank: true }
   validates :password, confirmation: { if: :password_required? }
+  validates :phone_extension, format: { with: /\A\d+\z/, allow_blank: true, message: I18n.t("validations.numeric.format", field: "") }
 
   after_validation :send_data_protection_confirmation_reminder, if: :is_dpo_changed?
 
@@ -263,6 +264,12 @@ class User < ApplicationRecord
     save!(validate: false)
   end
 
+  def phone_with_extension
+    return phone if phone_extension.blank?
+
+    "#{phone}, Ext. #{phone_extension}"
+  end
+
 protected
 
   # Checks whether a password is needed or not. For validations only.

app/views/users/edit.html.erb

@@ -24,7 +24,12 @@
 
       <%= f.govuk_phone_field :phone,
         label: { text: "Telephone number", size: "m" },
-        autocomplete: "phone",
+        autocomplete: "tel-national",
+        spellcheck: "false" %>
+
+      <%= f.govuk_phone_field :phone_extension,
+        label: { text: "Extension number (optional)", size: "m" },
+        autocomplete: "tel-extension",
         spellcheck: "false" %>
 
       <% if current_user.data_coordinator? || current_user.support? %>

app/views/users/new.html.erb

@@ -25,10 +25,16 @@
 
       <%= f.govuk_phone_field :phone,
         label: { text: "Telephone number", size: "m" },
-        autocomplete: "phone",
+        autocomplete: "tel-national",
         spellcheck: "false",
         value: @user.phone %>
 
+      <%= f.govuk_phone_field :phone_extension,
+        label: { text: "Extension number (optional)", size: "m" },
+        autocomplete: "tel-extension",
+        spellcheck: "false",
+        value: @user.phone_extension %>
+
       <% if current_user.support? %>
         <% null_option = [OpenStruct.new(id: "", name: "Select an option")] %>
         <% organisations = Organisation.filter_by_active.map { |org| OpenStruct.new(id: org.id, name: org.name) } %>

app/views/users/show.html.erb

@@ -43,7 +43,7 @@
 
       <%= summary_list.with_row do |row|
             row.with_key { "Telephone number" }
-            row.with_value { @user.phone }
+            row.with_value { @user.phone_with_extension }
             if UserPolicy.new(current_user, @user).edit_telephone_numbers?
               row.with_action(visually_hidden_text: "telephone number", href: aliased_user_edit(@user, current_user), html_attributes: { "data-qa": "change-telephone-number" })
             else

db/migrate/20240819143150_add_phone_extension_to_users.rb

@@ -0,0 +1,5 @@
+class AddPhoneExtensionToUsers < ActiveRecord::Migration[7.0]
+  def change
+    add_column :users, :phone_extension, :string
+  end
+end

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: 2024_07_15_082338) do
+ActiveRecord::Schema[7.0].define(version: 2024_08_19_143150) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -788,6 +788,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_07_15_082338) do
     t.boolean "initial_confirmation_sent"
     t.boolean "reactivate_with_organisation"
     t.datetime "discarded_at"
+    t.string "phone_extension"
     t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true
     t.index ["email"], name: "index_users_on_email", unique: true
     t.index ["encrypted_otp_secret_key"], name: "index_users_on_encrypted_otp_secret_key", unique: true

spec/requests/users_controller_spec.rb

@@ -1002,6 +1002,7 @@ RSpec.describe UsersController, type: :request do
             email: "new_user@example.com",
             role: "data_coordinator",
             phone: "12345678910",
+            phone_extension: "1234",
           },
         }
       end
@@ -1025,12 +1026,14 @@ RSpec.describe UsersController, type: :request do
         request
       end
 
-      it "creates a new scheme for user organisation with valid params" do
+      it "creates a new user for user organisation with valid params" do
         request
 
         expect(User.last.name).to eq("new user")
         expect(User.last.email).to eq("new_user@example.com")
         expect(User.last.role).to eq("data_coordinator")
+        expect(User.last.phone).to eq("12345678910")
+        expect(User.last.phone_extension).to eq("1234")
       end
 
       it "redirects back to organisation users page" do
@@ -1617,11 +1620,12 @@ RSpec.describe UsersController, type: :request do
           expect(page).to have_content("Change your personal details")
         end
 
-        it "has fields for name, email, role, dpo and key contact" do
+        it "has fields for name, email, role, phone number and phone extension" 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]")
+          expect(page).to have_field("user[phone_extension]")
         end
 
         it "allows setting the role to `support`" do
@@ -1643,10 +1647,12 @@ RSpec.describe UsersController, type: :request do
             expect(page).to have_content("Change #{other_user.name}’s personal details")
           end
 
-          it "has fields for name, email, role, dpo and key contact" do
+          it "has fields for name, email, role, phone number and phone extension" 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]")
+            expect(page).to have_field("user[phone_extension]")
           end
         end
 
@@ -1661,10 +1667,12 @@ RSpec.describe UsersController, type: :request do
             expect(page).to have_content("Change #{other_user.name}’s personal details")
           end
 
-          it "has fields for name, email, role, dpo and key contact" do
+          it "has fields for name, email, role, phone number and phone extension" 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]")
+            expect(page).to have_field("user[phone_extension]")
           end
         end
 
@@ -1994,6 +2002,7 @@ RSpec.describe UsersController, type: :request do
             email:,
             role: "data_coordinator",
             phone: "12345612456",
+            phone_extension: "1234",
             organisation_id: organisation.id,
           },
         }
@@ -2009,6 +2018,14 @@ RSpec.describe UsersController, type: :request do
         expect(User.find_by(email:).organisation).to eq(organisation)
       end
 
+      it "sets expected values on the user" do
+        request
+        user = User.find_by(email:)
+        expect(user.name).to eq("new user")
+        expect(user.phone).to eq("12345612456")
+        expect(user.phone_extension).to eq("1234")
+      end
+
       it "redirects back to users page" do
         request
         expect(response).to redirect_to("/users")