diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 2f7bb2bd6..c5c5241f9 100644 --- a/app/controllers/users_controller.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index 23e787268..31956e54d 100644 --- a/app/models/user.rb +++ b/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. diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index 3f7259bc3..6fdfdb5aa 100644 --- a/app/views/users/edit.html.erb +++ b/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? %> diff --git a/app/views/users/new.html.erb b/app/views/users/new.html.erb index 925f1ada7..f9f9b1e48 100644 --- a/app/views/users/new.html.erb +++ b/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) } %> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index acb53d51d..df8c0e915 100644 --- a/app/views/users/show.html.erb +++ b/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 diff --git a/db/migrate/20240819143150_add_phone_extension_to_users.rb b/db/migrate/20240819143150_add_phone_extension_to_users.rb new file mode 100644 index 000000000..0dcd8d0c5 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 50073d2d6..a76cf9eea 100644 --- a/db/schema.rb +++ b/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 diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index c2240a77d..dac2806a5 100644 --- a/spec/requests/users_controller_spec.rb +++ b/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")