From dd3cd0b4501793336e4f4373d5ad403fdd9f8d73 Mon Sep 17 00:00:00 2001 From: Mike Patrick Date: Tue, 26 Sep 2023 14:03:30 +0100 Subject: [PATCH] Introduce Personal details page under /account This provides the functionality that was lost in the switch from Users#edit to the new Account page but wasn't already covered by one of the existing/repurposed "sub-pages". We've deliberately left out the ability to change your name because we're not convinced that it's ever used. Since this is another mixed bag of a page and another Account sub-page, for consistency I've mostly followed the approaches taken by the Change your email password page. --- .../account/personal_details_controller.rb | 39 ++++++++++++ .../account/personal_details_policy.rb | 7 +++ .../account/personal_details/show.html.erb | 52 ++++++++++++++++ app/views/accounts/show.html.erb | 9 +++ config/routes.rb | 4 ++ .../account_personal_details_test.rb | 62 +++++++++++++++++++ test/integration/account_test.rb | 4 +- 7 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 app/controllers/account/personal_details_controller.rb create mode 100644 app/policies/account/personal_details_policy.rb create mode 100644 app/views/account/personal_details/show.html.erb create mode 100644 test/integration/account_personal_details_test.rb diff --git a/app/controllers/account/personal_details_controller.rb b/app/controllers/account/personal_details_controller.rb new file mode 100644 index 000000000..5664f4fdc --- /dev/null +++ b/app/controllers/account/personal_details_controller.rb @@ -0,0 +1,39 @@ +class Account::PersonalDetailsController < ApplicationController + layout "admin_layout" + + before_action :authenticate_user! + before_action :authorise_user + + def show; end + + def update_organisation + new_organisation_id = params[:user][:organisation_id] + new_organisation = Organisation.find(new_organisation_id) + + if current_user.update(organisation_id: new_organisation_id) + redirect_to account_path, notice: "Your organisation is now #{new_organisation.name}" + else + flash[:alert] = "There was a problem changing your organisation." + render :show + end + end + + def update_role + previous_role = current_user.role + new_role = params[:user][:role] + + if current_user.update(role: new_role) + EventLog.record_role_change(current_user, previous_role, new_role, current_user) + redirect_to account_path, notice: "Your role is now #{new_role.humanize}" + else + flash[:alert] = "There was a problem changing your role." + render :show + end + end + +private + + def authorise_user + authorize %i[account personal_details] + end +end diff --git a/app/policies/account/personal_details_policy.rb b/app/policies/account/personal_details_policy.rb new file mode 100644 index 000000000..0abc46f34 --- /dev/null +++ b/app/policies/account/personal_details_policy.rb @@ -0,0 +1,7 @@ +class Account::PersonalDetailsPolicy < BasePolicy + def show? + current_user.govuk_admin? + end + alias_method :update_organisation?, :show? + alias_method :update_role?, :show? +end diff --git a/app/views/account/personal_details/show.html.erb b/app/views/account/personal_details/show.html.erb new file mode 100644 index 000000000..68b3c8386 --- /dev/null +++ b/app/views/account/personal_details/show.html.erb @@ -0,0 +1,52 @@ +<% content_for :title, "Personal details" %> + +<% content_for :breadcrumbs, + render("govuk_publishing_components/components/breadcrumbs", { + collapse_on_mobile: true, + breadcrumbs: [ + { + title: "Dashboard", + url: root_path, + }, + { + title: "Settings", + url: account_path, + }, + { + title: "Personal details", + } + ] + }) +%> + +
+
+

Change your role

+ + <%= form_for current_user, url: update_role_account_personal_details_path do |f| %> + <%= render "govuk_publishing_components/components/select", { + id: "user_role", + name: "user[role]", + label: "Role", + options: current_user.manageable_roles.map { |role| { text: role.humanize, value: role, selected: current_user.role == role } } + } %> + <%= render "govuk_publishing_components/components/button", { + text: "Change role" + } %> + <% end %> + +
+ + <%= form_for current_user, url: update_organisation_account_personal_details_path do |f| %> + <%= render "govuk_publishing_components/components/select", { + id: "user_organisation_id", + name: "user[organisation_id]", + label: "Organisation", + options: policy_scope(Organisation).map { |organisation| { text: organisation.name_with_abbreviation, value: organisation.id, selected: current_user.organisation == organisation } } + } %> + <%= render "govuk_publishing_components/components/button", { + text: "Change organisation" + } %> + <% end %> +
+
diff --git a/app/views/accounts/show.html.erb b/app/views/accounts/show.html.erb index 9cc35f6a7..1b815e1b1 100644 --- a/app/views/accounts/show.html.erb +++ b/app/views/accounts/show.html.erb @@ -40,5 +40,14 @@ }, description: "Change your 2-step verification configuration." }, + ( + { + link: { + text: "Personal details", + path: account_personal_details_path, + }, + description: "Make changes to your personal details such as your role.", + } if current_user.govuk_admin? + ), ].compact } %> diff --git a/config/routes.rb b/config/routes.rb index 2f253be47..7d8d3740e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -57,6 +57,10 @@ put :resend_email_change delete :cancel_email_change end + resource :personal_details, only: [:show] do + patch :update_organisation + patch :update_role + end end resources :batch_invitations, only: %i[new create show] do diff --git a/test/integration/account_personal_details_test.rb b/test/integration/account_personal_details_test.rb new file mode 100644 index 000000000..421344a31 --- /dev/null +++ b/test/integration/account_personal_details_test.rb @@ -0,0 +1,62 @@ +require "test_helper" + +class AccountPersonalDetailsTest < ActionDispatch::IntegrationTest + context "#show" do + should "only allow GOV.UK Admins access" do + [ + FactoryBot.create(:user), + FactoryBot.create(:organisation_admin_user), + FactoryBot.create(:super_organisation_admin_user), + ].each do |non_govuk_admin_user| + visit new_user_session_path + signin_with non_govuk_admin_user + + visit account_personal_details_path + assert page.has_text? "You do not have permission to perform this action." + + signout + end + end + + should "allow user to change their role" do + user = FactoryBot.create(:superadmin_user) + + visit new_user_session_path + signin_with user + + visit account_personal_details_path + + select "Admin", from: "Role" + click_button "Change role" + + assert_current_url account_path + assert page.has_text? "Your role is now Admin" + + visit account_personal_details_path + + assert page.has_select? "Role", selected: "Admin" + end + + should "allow user to change their organisation" do + current_organisation = create(:organisation, name: "Judiciary") + user = FactoryBot.create(:admin_user, organisation: current_organisation) + + create(:organisation, name: "Postage") + + visit new_user_session_path + signin_with user + + visit account_personal_details_path + + select "Postage", from: "Organisation" + click_button "Change organisation" + + assert_current_url account_path + assert page.has_text? "Your organisation is now Postage" + + visit account_personal_details_path + + assert page.has_select? "Organisation", selected: "Postage" + end + end +end diff --git a/test/integration/account_test.rb b/test/integration/account_test.rb index f8a14f36b..506fc7ed4 100644 --- a/test/integration/account_test.rb +++ b/test/integration/account_test.rb @@ -8,7 +8,7 @@ class AccountTest < ActionDispatch::IntegrationTest assert_current_url new_user_session_path end - should "link to Change email/password, Manage permissions and 2SV setup for admin users" do + should "link to Change email/password, Manage permissions, 2SV setup and Personal details for admin users" do user = FactoryBot.create(:admin_user) visit new_user_session_path @@ -21,6 +21,7 @@ class AccountTest < ActionDispatch::IntegrationTest assert page.has_link?("Change your email or password", href: account_email_password_path) assert page.has_link?("Manage permissions", href: edit_user_path(user)) assert page.has_link?("Reset 2-step verification", href: two_step_verification_path) + assert page.has_link?("Personal details", href: account_personal_details_path) end should "link to Change email/password and 2SV setup for normal users" do @@ -37,6 +38,7 @@ class AccountTest < ActionDispatch::IntegrationTest assert page.has_link?("Reset 2-step verification", href: two_step_verification_path) assert_not page.has_link?("Manage permissions", href: edit_user_path(user)) + assert_not page.has_link?("Personal details", href: account_personal_details_path) end end end