From ce6e45eb18c0313789ea10fac73bb1d9a1a193ec Mon Sep 17 00:00:00 2001 From: Nat Dean-Lewis Date: Thu, 23 Apr 2026 09:30:40 +0100 Subject: [PATCH 1/9] CLDC-4071: initial UI changes to display extension numbers --- app/helpers/user_helper.rb | 4 +++- app/models/user.rb | 1 + app/policies/user_policy.rb | 1 + app/views/users/show.html.erb | 15 +++++++++++++++ 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb index 13eab0d14..931f18680 100644 --- a/app/helpers/user_helper.rb +++ b/app/helpers/user_helper.rb @@ -85,13 +85,15 @@ module UserHelper current_user.data_coordinator? || current_user.support? ? govuk_link_to("Select role", aliased_user_edit(user, current_user), class: "govuk-link govuk-link--no-visited-state") : "No role assigned" when "phone" govuk_link_to("Enter telephone number", aliased_user_edit(user, current_user), class: "govuk-link govuk-link--no-visited-state") + when "phone_extension" + current_user.data_coordinator? || current_user.support? ? govuk_link_to("Enter extension number", aliased_user_edit(user, current_user), class: "govuk-link govuk-link--no-visited-state") : "No answer provided" else "No answer provided" end end def user_action_text(user, attribute) - return "Change" if %w[role phone].include?(attribute) && user.send(attribute).present? + return "Change" if %w[role phone phone_extension].include?(attribute) && user.send(attribute).present? "" end diff --git a/app/models/user.rb b/app/models/user.rb index ea8289e53..4e80df2b8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -335,6 +335,7 @@ class User < ApplicationRecord end def phone_with_extension + #use this return phone if phone_extension.blank? "#{phone}, Ext. #{phone_extension}" diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index dc8aaf28e..48beb1216 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -26,6 +26,7 @@ class UserPolicy %w[ edit_emails? edit_telephone_numbers? + edit_extension_numbers? edit_names? ].each do |method_name| define_method method_name do diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index a1f104b10..06fda18a3 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -65,6 +65,21 @@ end end %> + <%= summary_list.with_row do |row| + row.with_key { "Extension number" } + row.with_value { user_details_html(@user, current_user, "phone_extension") } + if UserPolicy.new(current_user, @user).edit_extension_numbers? && @user.phone_extension.present? + row.with_action( + text: user_action_text(@user, "phone_extension"), + visually_hidden_text: "extension number", + href: aliased_user_edit(@user, current_user), + html_attributes: { "data-qa": "change-extension-number" }, + ) + else + row.with_action + end + end %> + <%= summary_list.with_row do |row| row.with_key { "Password" } row.with_value { "••••••••" } From 2016dfdaa0968dbd06bd616e1125f76093390252 Mon Sep 17 00:00:00 2001 From: Nat Dean-Lewis Date: Thu, 23 Apr 2026 10:03:40 +0100 Subject: [PATCH 2/9] CLDC-4071: wip change link behaviour --- app/helpers/user_helper.rb | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb index 931f18680..b1ecdc7ad 100644 --- a/app/helpers/user_helper.rb +++ b/app/helpers/user_helper.rb @@ -82,13 +82,21 @@ module UserHelper case attribute when "role" - current_user.data_coordinator? || current_user.support? ? govuk_link_to("Select role", aliased_user_edit(user, current_user), class: "govuk-link govuk-link--no-visited-state") : "No role assigned" + if current_user.data_coordinator? || current_user.support? + edit_link("Select role", user, current_user) + else + "No role assigned" + end when "phone" - govuk_link_to("Enter telephone number", aliased_user_edit(user, current_user), class: "govuk-link govuk-link--no-visited-state") + edit_link("Enter telephone number", user, current_user) when "phone_extension" - current_user.data_coordinator? || current_user.support? ? govuk_link_to("Enter extension number", aliased_user_edit(user, current_user), class: "govuk-link govuk-link--no-visited-state") : "No answer provided" + if user == current_user || current_user.data_coordinator? || current_user.support? + edit_link("Enter extension number", user, current_user) + else + no_answer_provided_text + end else - "No answer provided" + no_answer_provided_text end end @@ -97,4 +105,18 @@ module UserHelper "" end + +private + + def edit_link(text, user, current_user) + govuk_link_to( + text, + aliased_user_edit(user, current_user), + class: "govuk-link govuk-link--no-visited-state", + ) + end + + def no_answer_provided_text + "No answer provided" + end end From 4f4e2e0c41cedfcd9acc4240b0ac42a4d967b4e9 Mon Sep 17 00:00:00 2001 From: Nat Dean-Lewis Date: Thu, 23 Apr 2026 10:05:33 +0100 Subject: [PATCH 3/9] CLDC-4071: show change link even when extension number not provided --- app/helpers/user_helper.rb | 8 +------- app/views/users/show.html.erb | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb index b1ecdc7ad..f6168e108 100644 --- a/app/helpers/user_helper.rb +++ b/app/helpers/user_helper.rb @@ -89,19 +89,13 @@ module UserHelper end when "phone" edit_link("Enter telephone number", user, current_user) - when "phone_extension" - if user == current_user || current_user.data_coordinator? || current_user.support? - edit_link("Enter extension number", user, current_user) - else - no_answer_provided_text - end else no_answer_provided_text end end def user_action_text(user, attribute) - return "Change" if %w[role phone phone_extension].include?(attribute) && user.send(attribute).present? + return "Change" if (%w[role phone].include?(attribute) && user.send(attribute).present?) || attribute == "phone_extension" "" end diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 06fda18a3..ff8a3647e 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -68,7 +68,7 @@ <%= summary_list.with_row do |row| row.with_key { "Extension number" } row.with_value { user_details_html(@user, current_user, "phone_extension") } - if UserPolicy.new(current_user, @user).edit_extension_numbers? && @user.phone_extension.present? + if UserPolicy.new(current_user, @user).edit_extension_numbers? row.with_action( text: user_action_text(@user, "phone_extension"), visually_hidden_text: "extension number", From 8701b2a0dd4aff9d23901883aa2cb6fec5bf28d7 Mon Sep 17 00:00:00 2001 From: Nat Dean-Lewis Date: Thu, 23 Apr 2026 10:07:24 +0100 Subject: [PATCH 4/9] CLDC-4071: revert user_details_html update --- app/helpers/user_helper.rb | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb index f6168e108..24a1edcae 100644 --- a/app/helpers/user_helper.rb +++ b/app/helpers/user_helper.rb @@ -82,15 +82,11 @@ module UserHelper case attribute when "role" - if current_user.data_coordinator? || current_user.support? - edit_link("Select role", user, current_user) - else - "No role assigned" - end + current_user.data_coordinator? || current_user.support? ? edit_link("Select role", user, current_user) : "No role assigned" when "phone" edit_link("Enter telephone number", user, current_user) else - no_answer_provided_text + "No answer provided" end end @@ -109,8 +105,4 @@ private class: "govuk-link govuk-link--no-visited-state", ) end - - def no_answer_provided_text - "No answer provided" - end end From 6e7d157cbccda31e5b181c128cc233beac16f68a Mon Sep 17 00:00:00 2001 From: Nat Dean-Lewis Date: Thu, 23 Apr 2026 10:10:22 +0100 Subject: [PATCH 5/9] CLDC-4071: revert DRY change --- app/helpers/user_helper.rb | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb index 24a1edcae..397fbed7e 100644 --- a/app/helpers/user_helper.rb +++ b/app/helpers/user_helper.rb @@ -82,27 +82,17 @@ module UserHelper case attribute when "role" - current_user.data_coordinator? || current_user.support? ? edit_link("Select role", user, current_user) : "No role assigned" + current_user.data_coordinator? || current_user.support? ? govuk_link_to("Select role", aliased_user_edit(user, current_user), class: "govuk-link govuk-link--no-visited-state") : "No role assigned" when "phone" - edit_link("Enter telephone number", user, current_user) + govuk_link_to("Enter telephone number", aliased_user_edit(user, current_user), class: "govuk-link govuk-link--no-visited-state") else "No answer provided" end end def user_action_text(user, attribute) - return "Change" if (%w[role phone].include?(attribute) && user.send(attribute).present?) || attribute == "phone_extension" + return "Change" if attribute == "phone_extension" || (%w[role phone].include?(attribute) && user.send(attribute).present?) "" end - -private - - def edit_link(text, user, current_user) - govuk_link_to( - text, - aliased_user_edit(user, current_user), - class: "govuk-link govuk-link--no-visited-state", - ) - end end From 11c605367af9ce63477c0bae819fb8e02239a052 Mon Sep 17 00:00:00 2001 From: Nat Dean-Lewis Date: Thu, 23 Apr 2026 10:16:38 +0100 Subject: [PATCH 6/9] CLDC-4071: update export service --- app/models/user.rb | 7 ------- app/services/exports/user_export_constants.rb | 1 + app/services/exports/user_export_service.rb | 3 ++- spec/fixtures/exports/user.xml | 3 ++- spec/services/exports/user_export_service_spec.rb | 2 +- 5 files changed, 6 insertions(+), 10 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 4e80df2b8..c81d17426 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -334,13 +334,6 @@ class User < ApplicationRecord save!(validate: false) end - def phone_with_extension - #use this - return phone if phone_extension.blank? - - "#{phone}, Ext. #{phone_extension}" - end - def assigned_to_lettings_logs lettings_logs.where(assigned_to: self) end diff --git a/app/services/exports/user_export_constants.rb b/app/services/exports/user_export_constants.rb index 9ce5840d9..15fa10ea0 100644 --- a/app/services/exports/user_export_constants.rb +++ b/app/services/exports/user_export_constants.rb @@ -6,6 +6,7 @@ module Exports::UserExportConstants "email", "name", "phone", + "extension_number", "organisation_id", "organisation_name", "role", diff --git a/app/services/exports/user_export_service.rb b/app/services/exports/user_export_service.rb index 0a8ebe34e..3a9f37417 100644 --- a/app/services/exports/user_export_service.rb +++ b/app/services/exports/user_export_service.rb @@ -64,7 +64,8 @@ module Exports attribute_hash["role"] = user.role attribute_hash["organisation_name"] = user.organisation.name attribute_hash["active"] = user.active? - attribute_hash["phone"] = [user.phone, user.phone_extension].compact.join(" ") + attribute_hash["phone"] = user.phone + attribute_hash["extension_number"] = user.phone_extension attribute_hash["last_sign_in_at"] = user.last_sign_in_at&.iso8601 attribute_hash end diff --git a/spec/fixtures/exports/user.xml b/spec/fixtures/exports/user.xml index 4c5286c68..bf3f2fb4c 100644 --- a/spec/fixtures/exports/user.xml +++ b/spec/fixtures/exports/user.xml @@ -8,7 +8,8 @@ 5 2022-03-03T00:00:00+00:00 data_provider - 1234512345123 123 + 1234512345123 + ext. 123 false false true diff --git a/spec/services/exports/user_export_service_spec.rb b/spec/services/exports/user_export_service_spec.rb index f62664945..a2f00f565 100644 --- a/spec/services/exports/user_export_service_spec.rb +++ b/spec/services/exports/user_export_service_spec.rb @@ -45,7 +45,7 @@ RSpec.describe Exports::UserExportService do end context "and one user is available for export" do - let!(:user) { create(:user, organisation:, name: "Danny Rojas", phone_extension: "123", last_sign_in_at: Time.zone.local(2022, 3, 3)) } + let!(:user) { create(:user, organisation:, name: "Danny Rojas", phone_extension: "ext. 123", last_sign_in_at: Time.zone.local(2022, 3, 3)) } it "generates a ZIP export file with the expected filename" do expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) From 97003cb19d25157269763ca58b71a7b019e16d30 Mon Sep 17 00:00:00 2001 From: Nat Dean-Lewis Date: Thu, 23 Apr 2026 10:17:16 +0100 Subject: [PATCH 7/9] CLDC-4071: update export service --- spec/fixtures/exports/user.xml | 2 +- spec/services/exports/user_export_service_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/fixtures/exports/user.xml b/spec/fixtures/exports/user.xml index bf3f2fb4c..1972e2a9d 100644 --- a/spec/fixtures/exports/user.xml +++ b/spec/fixtures/exports/user.xml @@ -9,7 +9,7 @@ 2022-03-03T00:00:00+00:00 data_provider 1234512345123 - ext. 123 + 123 false false true diff --git a/spec/services/exports/user_export_service_spec.rb b/spec/services/exports/user_export_service_spec.rb index a2f00f565..f62664945 100644 --- a/spec/services/exports/user_export_service_spec.rb +++ b/spec/services/exports/user_export_service_spec.rb @@ -45,7 +45,7 @@ RSpec.describe Exports::UserExportService do end context "and one user is available for export" do - let!(:user) { create(:user, organisation:, name: "Danny Rojas", phone_extension: "ext. 123", last_sign_in_at: Time.zone.local(2022, 3, 3)) } + let!(:user) { create(:user, organisation:, name: "Danny Rojas", phone_extension: "123", last_sign_in_at: Time.zone.local(2022, 3, 3)) } it "generates a ZIP export file with the expected filename" do expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) From 2d8154c6ff0a7522d2e0b570dc721e8b1d20bfd7 Mon Sep 17 00:00:00 2001 From: Nat Dean-Lewis Date: Thu, 23 Apr 2026 10:28:37 +0100 Subject: [PATCH 8/9] CLDC-4071: linting --- app/views/users/show.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index ff8a3647e..330222bbb 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -74,7 +74,7 @@ visually_hidden_text: "extension number", href: aliased_user_edit(@user, current_user), html_attributes: { "data-qa": "change-extension-number" }, - ) + ) else row.with_action end From ffe5551db8d4ce3d1ced6e7acb99b4f1bd4f1405 Mon Sep 17 00:00:00 2001 From: Nat Dean-Lewis Date: Thu, 23 Apr 2026 10:29:54 +0100 Subject: [PATCH 9/9] CLDC-4071: linting --- app/views/users/show.html.erb | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 330222bbb..d2ad8b182 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -66,19 +66,19 @@ end %> <%= summary_list.with_row do |row| - row.with_key { "Extension number" } - row.with_value { user_details_html(@user, current_user, "phone_extension") } - if UserPolicy.new(current_user, @user).edit_extension_numbers? - row.with_action( - text: user_action_text(@user, "phone_extension"), - visually_hidden_text: "extension number", - href: aliased_user_edit(@user, current_user), - html_attributes: { "data-qa": "change-extension-number" }, - ) - else - row.with_action - end - end %> + row.with_key { "Extension number" } + row.with_value { user_details_html(@user, current_user, "phone_extension") } + if UserPolicy.new(current_user, @user).edit_extension_numbers? + row.with_action( + text: user_action_text(@user, "phone_extension"), + visually_hidden_text: "extension number", + href: aliased_user_edit(@user, current_user), + html_attributes: { "data-qa": "change-extension-number" }, + ) + else + row.with_action + end + end %> <%= summary_list.with_row do |row| row.with_key { "Password" }