diff --git a/Gemfile b/Gemfile index 10ce38f07..e1cb5e1a9 100644 --- a/Gemfile +++ b/Gemfile @@ -59,6 +59,7 @@ gem "sentry-rails" gem "sentry-ruby" # Pagination gem "pagy" +gem "possessive" group :development, :test do # Check gems for known vulnerabilities diff --git a/Gemfile.lock b/Gemfile.lock index 71c6bd92f..1859e66a6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -253,6 +253,7 @@ GEM parser (3.1.2.0) ast (~> 2.4.1) pg (1.3.5) + possessive (1.0.1) postcodes_io (0.4.0) excon (~> 0.39) propshaft (0.6.4) @@ -452,6 +453,7 @@ DEPENDENCIES paper_trail paper_trail-globalid pg (~> 1.1) + possessive postcodes_io propshaft pry-byebug diff --git a/app/controllers/modules/search_filter.rb b/app/controllers/modules/search_filter.rb index 09c387ee4..c32298987 100644 --- a/app/controllers/modules/search_filter.rb +++ b/app/controllers/modules/search_filter.rb @@ -8,6 +8,6 @@ module Modules::SearchFilter end def filtered_users(base_collection, search_term = nil) - filtered_collection(base_collection, search_term).filter_by_active.includes(:organisation) + filtered_collection(base_collection, search_term).includes(:organisation) end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index f506e75cb..7c52cdc0d 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -30,6 +30,10 @@ class UsersController < ApplicationController def show; end + def edit + redirect_to user_path(@user) unless @user.active? + end + def update if @user.update(user_params) if @user == current_user @@ -37,6 +41,14 @@ class UsersController < ApplicationController flash[:notice] = I18n.t("devise.passwords.updated") if user_params.key?("password") redirect_to account_path else + case user_params[:active] + when "false" + @user.update!(confirmed_at: nil, sign_in_count: 0) + flash[:notice] = I18n.t("devise.activation.deactivated", user_name: @user.name.possessive) + when "true" + @user.send_confirmation_instructions + flash[:notice] = I18n.t("devise.activation.reactivated", user_name: @user.name.possessive) + end redirect_to user_path(@user) end elsif user_params.key?("password") @@ -80,6 +92,22 @@ class UsersController < ApplicationController render "devise/passwords/edit", locals: { resource: @user, resource_name: "user" } end + def deactivate + if current_user.can_toggle_active?(@user) + render "toggle_active", locals: { action: "deactivate" } + else + redirect_to user_path(@user) + end + end + + def reactivate + if current_user.can_toggle_active?(@user) + render "toggle_active", locals: { action: "reactivate" } + else + redirect_to user_path(@user) + end + end + private def format_error_messages @@ -116,9 +144,9 @@ private params.require(:user).permit(:email, :name, :password, :password_confirmation) end elsif current_user.data_coordinator? - params.require(:user).permit(:email, :name, :role, :is_dpo, :is_key_contact) + params.require(:user).permit(:email, :name, :role, :is_dpo, :is_key_contact, :active) elsif current_user.support? - params.require(:user).permit(:email, :name, :role, :is_dpo, :is_key_contact, :organisation_id) + params.require(:user).permit(:email, :name, :role, :is_dpo, :is_key_contact, :organisation_id, :active) end end diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb index fc82b26cb..6f6853375 100644 --- a/app/helpers/user_helper.rb +++ b/app/helpers/user_helper.rb @@ -8,27 +8,27 @@ module UserHelper end def can_edit_names?(user, current_user) - current_user == user || current_user.data_coordinator? || current_user.support? + (current_user == user || current_user.data_coordinator? || current_user.support?) && user.active? end def can_edit_emails?(user, current_user) - current_user == user || current_user.data_coordinator? || current_user.support? + (current_user == user || current_user.data_coordinator? || current_user.support?) && user.active? end def can_edit_password?(user, current_user) current_user == user end - def can_edit_roles?(_user, current_user) - current_user.data_coordinator? || current_user.support? + def can_edit_roles?(user, current_user) + (current_user.data_coordinator? || current_user.support?) && user.active? end - def can_edit_dpo?(_user, current_user) - current_user.data_coordinator? || current_user.support? + def can_edit_dpo?(user, current_user) + (current_user.data_coordinator? || current_user.support?) && user.active? end - def can_edit_key_contact?(_user, current_user) - current_user.data_coordinator? || current_user.support? + def can_edit_key_contact?(user, current_user) + (current_user.data_coordinator? || current_user.support?) && user.active? end def can_edit_org?(current_user) diff --git a/app/mailers/devise_notify_mailer.rb b/app/mailers/devise_notify_mailer.rb index 7fd2e256f..2e77c8b17 100644 --- a/app/mailers/devise_notify_mailer.rb +++ b/app/mailers/devise_notify_mailer.rb @@ -15,10 +15,10 @@ class DeviseNotifyMailer < Devise::Mailer ) end - def personalisation(record, token, url) + def personalisation(record, token, url, username: false) { name: record.name || record.email, - email: record.email, + email: username || record.email, organisation: record.respond_to?(:organisation) ? record.organisation.name : "", link: "#{url}#{token}", } @@ -35,12 +35,12 @@ class DeviseNotifyMailer < Devise::Mailer end def confirmation_instructions(record, token, _opts = {}) - url = "#{user_confirmation_url}?confirmation_token=" - send_email( - record.email, - record.confirmable_template, - personalisation(record, token, url), - ) + username = record.email + if email_changed(record) + username = record.unconfirmed_email + send_confirmation_email(record.unconfirmed_email, record, token, username) + end + send_confirmation_email(record.email, record, token, username) end def intercept_send?(email) @@ -52,6 +52,22 @@ class DeviseNotifyMailer < Devise::Mailer Rails.application.credentials[:email_allowlist] end +private + + def email_changed(record) + record.confirmable_template == User::CONFIRMABLE_TEMPLATE_ID && (record.unconfirmed_email.present? && record.unconfirmed_email != record.email) + end + + def send_confirmation_email(email, record, token, username) + url = "#{user_confirmation_url}?confirmation_token=" + + send_email( + email, + record.confirmable_template, + personalisation(record, token, url, username:), + ) + end + # def unlock_instructions(record, token, opts = {}) # super # end diff --git a/app/models/user.rb b/app/models/user.rb index f8e1dfd10..e85d74e67 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -74,13 +74,16 @@ class User < ApplicationRecord RESET_PASSWORD_TEMPLATE_ID = "2c410c19-80a7-481c-a531-2bcb3264f8e6".freeze CONFIRMABLE_TEMPLATE_ID = "257460a6-6616-4640-a3f9-17c3d73d9e91".freeze BETA_ONBOARDING_TEMPLATE_ID = "b48bc2cd-5887-4611-8296-d0ab3ed0e7fd".freeze + USER_REACTIVATED_TEMPLATE_ID = "ac45a899-490e-4f59-ae8d-1256fc0001f9".freeze def reset_password_notify_template RESET_PASSWORD_TEMPLATE_ID end def confirmable_template - if was_migrated_from_softwire? + if last_sign_in_at.present? && (unconfirmed_email.blank? || unconfirmed_email == email) + USER_REACTIVATED_TEMPLATE_ID + elsif was_migrated_from_softwire? && last_sign_in_at.blank? BETA_ONBOARDING_TEMPLATE_ID else CONFIRMABLE_TEMPLATE_ID @@ -139,4 +142,12 @@ class User < ApplicationRecord end end end + + def can_toggle_active?(user) + self != user && (support? || data_coordinator?) + end + + def valid_for_authentication? + super && active? + end end diff --git a/app/views/users/_user_list.html.erb b/app/views/users/_user_list.html.erb index 171e21220..780d481f0 100644 --- a/app/views/users/_user_list.html.erb +++ b/app/views/users/_user_list.html.erb @@ -48,7 +48,7 @@ ) : "" %> <% end %> <% row.cell(text: simple_format(org_cell(user), {}, wrapper_tag: "div")) %> - <% row.cell(text: user.last_sign_in_at&.to_formatted_s(:govuk_date)) %> + <% row.cell(text: user.active? ? user.last_sign_in_at&.to_formatted_s(:govuk_date) : "Deactivated") %> <% end %> <% end %> <% end %> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 94446f9d0..3c274e1a7 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -5,6 +5,17 @@

<%= content_for(:title) %>

+

+ <% if current_user.can_toggle_active?(@user) %> + <% if @user.active? %> + <%= govuk_link_to "Deactivate user", "/users/#{@user.id}/deactivate" %> + <% else %> + + This user has been deactivated. <%= govuk_link_to "Reactivate user", "/users/#{@user.id}/reactivate" %> + + <% end %> + <% end %> +

Personal details diff --git a/app/views/users/toggle_active.html.erb b/app/views/users/toggle_active.html.erb new file mode 100644 index 000000000..40eaca551 --- /dev/null +++ b/app/views/users/toggle_active.html.erb @@ -0,0 +1,26 @@ +<% content_for :title, "#{action.capitalize} #{@user.name.presence || @user.email}’s account" %> + +
+ <%= form_for(@user, as: :user, html: { method: :patch }) do |f| %> +
+

+ <%= @user.name %> + Are you sure you want to <%= action %> this user? +

+ <% if action == "deactivate" %> +

Deactivating this user will mean they can no longer access this service to submit CORE data.

+

Any logs this user has already submitted will not be affected.

+ <% end %> + <% active_value = action != "deactivate" %> + + <%= f.hidden_field :active, value: active_value %> + + <%= f.govuk_submit "I’m sure – #{action} this user", warning: action == "deactivate" %> + +

+ <%= govuk_link_to("No – I’ve changed my mind", user_path(@user)) %> +

+
+
+ <% end %> + diff --git a/config/locales/devise.en.yml b/config/locales/devise.en.yml index 707e8cffa..e0babe522 100644 --- a/config/locales/devise.en.yml +++ b/config/locales/devise.en.yml @@ -28,7 +28,7 @@ en: password_change: subject: "Password successfully changed" omniauth_callbacks: - failure: "We could not authenticate you from %{kind} because \"%{reason}\"" + failure: 'We could not authenticate you from %{kind} because "%{reason}"' success: "Successfully authenticated from %{kind} account" passwords: no_token: "You can’t access this page unless you’re trying to reset your password. Check you’re using the correct URL in the email we sent you." @@ -54,6 +54,10 @@ en: send_instructions: "You will receive an email in a few minutes with instructions for how to unlock your account" send_paranoid_instructions: "If your account exists, you will receive an email in a few minutes with instructions for how to unlock it" unlocked: "Your account has been successfully unlocked. Sign in to continue." + activation: + deactivated: "%{user_name} account has been deactivated." + reactivated: "%{user_name} account has been reactivated." + errors: messages: already_confirmed: "Email has already been confirmed. Sign in." diff --git a/config/routes.rb b/config/routes.rb index c70cb2525..4f7bc91e5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -35,7 +35,12 @@ Rails.application.routes.draw do get "edit/password", to: "users#edit_password" end - resources :users + resources :users do + member do + get "deactivate", to: "users#deactivate" + get "reactivate", to: "users#reactivate" + end + end resources :organisations do member do diff --git a/spec/controllers/modules/search_filter_spec.rb b/spec/controllers/modules/search_filter_spec.rb index 4db149e69..b46878d82 100644 --- a/spec/controllers/modules/search_filter_spec.rb +++ b/spec/controllers/modules/search_filter_spec.rb @@ -40,16 +40,16 @@ RSpec.describe Modules::SearchFilter do context "when given a search term" do let(:search_term) { "Blogg" } - it "filters the collection on search term and active users" do - expect(instance.filtered_users(user_list, search_term).count).to eq(1) + it "filters the collection on search term" do + expect(instance.filtered_users(user_list, search_term).count).to eq(2) end end context "when not given a search term" do let(:search_term) { nil } - it "filters the collection on active users" do - expect(instance.filtered_users(user_list, search_term).count).to eq(6) + it "returns all the users" do + expect(instance.filtered_users(user_list, search_term).count).to eq(7) end end end diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index a1cfb93e4..63bddc885 100644 --- a/spec/features/user_spec.rb +++ b/spec/features/user_spec.rb @@ -172,6 +172,22 @@ RSpec.describe "User Features" do end end + context "when the user is trying to log in with deactivated user" do + before do + user.update!(active: false) + end + + it "shows a gov uk error summary and no flash message" do + visit("/logs") + fill_in("user[email]", with: user.email) + fill_in("user[password]", with: "pAssword1") + click_button("Sign in") + expect(page).to have_selector("#error-summary-title") + expect(page).to have_no_css(".govuk-notification-banner.govuk-notification-banner--success") + expect(page).to have_title("Error") + end + end + context "when signed in as a data provider" do context "when viewing your account" do before do @@ -360,6 +376,72 @@ RSpec.describe "User Features" do )).to be_a(User) end end + + context "when deactivating a user" do + let!(:user) { FactoryBot.create(:user, :data_coordinator, last_sign_in_at: Time.zone.now) } + let!(:other_user) { FactoryBot.create(:user, name: "Other name", organisation: user.organisation) } + + before do + visit("/logs") + fill_in("user[email]", with: user.email) + fill_in("user[password]", with: "pAssword1") + click_button("Sign in") + visit("/users/#{other_user.id}") + click_link("Deactivate user") + end + + it "allows to cancel user deactivation" do + click_link("No – I’ve changed my mind") + expect(page).to have_current_path("/users/#{other_user.id}") + expect(page).to have_no_content("This user has been deactivated.") + expect(page).to have_no_css(".govuk-notification-banner.govuk-notification-banner--success") + end + + it "allows to deactivate the user" do + click_button("I’m sure – deactivate this user") + expect(page).to have_current_path("/users/#{other_user.id}") + expect(page).to have_content("This user has been deactivated.") + expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success") + end + end + + context "when reactivating a user" do + let!(:user) { FactoryBot.create(:user, :data_coordinator, last_sign_in_at: Time.zone.now) } + let!(:other_user) { FactoryBot.create(:user, name: "Other name", active: false, organisation: user.organisation, last_sign_in_at: Time.zone.now) } + let(:personalisation) do + { + name: other_user.name, + email: other_user.email, + organisation: other_user.organisation.name, + link: include("/account/confirmation?confirmation_token=#{other_user.confirmation_token}"), + } + end + + before do + other_user.update!(confirmation_token: "abc") + visit("/logs") + fill_in("user[email]", with: user.email) + fill_in("user[password]", with: "pAssword1") + click_button("Sign in") + visit("/users/#{other_user.id}") + click_link("Reactivate user") + end + + it "allows to cancel user reactivation" do + click_link("No – I’ve changed my mind") + expect(page).to have_current_path("/users/#{other_user.id}") + expect(page).to have_content("This user has been deactivated.") + expect(page).to have_no_css(".govuk-notification-banner.govuk-notification-banner--success") + end + + it "allows to reactivate the user" do + expect(notify_client).to receive(:send_email).with(email_address: other_user.email, template_id: User::USER_REACTIVATED_TEMPLATE_ID, personalisation:).once + click_button("I’m sure – reactivate this user") + expect(page).to have_current_path("/users/#{other_user.id}") + expect(page).to have_no_content("This user has been deactivated.") + expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success") + end + end end context "when the user is a customer support person" do diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index deb809831..d49e6a61a 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -139,12 +139,12 @@ RSpec.describe OrganisationsController, type: :request do it "shows only active users in the current user's organisation" do expect(page).to have_content(user.name) expect(page).to have_content(other_user.name) - expect(page).not_to have_content(inactive_user.name) + expect(page).to have_content(inactive_user.name) expect(page).not_to have_content(other_org_user.name) end it "shows the pagination count" do - expect(page).to have_content("2 total users") + expect(page).to have_content("3 total users") end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 51e63286f..c0c667906 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -111,6 +111,20 @@ RSpec.describe UsersController, type: :request do expect(CGI.unescape_html(response.body)).to include(expected_link) end end + + describe "#deactivate" do + it "does not let you see deactivate page" do + get "/users/#{user.id}/deactivate", headers: headers, params: {} + expect(response).to redirect_to("/account/sign-in") + end + end + + describe "#reactivate" do + it "does not let you see reactivate page" do + get "/users/#{user.id}/reactivate", headers: headers, params: {} + expect(response).to redirect_to("/account/sign-in") + end + end end context "when user is signed in as a data provider" do @@ -133,6 +147,21 @@ RSpec.describe UsersController, type: :request do expect(page).not_to have_link("Change", text: "are you a data protection officer?") expect(page).not_to have_link("Change", text: "are you a key contact?") end + + it "does not allow deactivating the user" do + expect(page).not_to have_link("Deactivate user", href: "/users/#{user.id}/deactivate") + end + + context "when user is deactivated" do + before do + user.update!(active: false) + get "/users/#{user.id}", headers:, params: {} + end + + it "does not allow reactivating the user" do + expect(page).not_to have_link("Reactivate user", href: "/users/#{user.id}/reactivate") + end + end end context "when the current user does not match the user ID" do @@ -157,6 +186,21 @@ RSpec.describe UsersController, type: :request do expect(page).not_to have_link("Change", text: "are you a data protection officer?") expect(page).not_to have_link("Change", text: "are you a key contact?") end + + it "does not allow deactivating the user" do + expect(page).not_to have_link("Deactivate user", href: "/users/#{other_user.id}/deactivate") + end + + context "when user is deactivated" do + before do + other_user.update!(active: false) + get "/users/#{other_user.id}", headers:, params: {} + end + + it "does not allow reactivating the user" do + expect(page).not_to have_link("Reactivate user", href: "/users/#{other_user.id}/reactivate") + end + end end context "when the user is not part of the same organisation" do @@ -457,6 +501,21 @@ RSpec.describe UsersController, type: :request do expect(page).to have_link("Change", text: "are you a data protection officer?") expect(page).to have_link("Change", text: "are you a key contact?") end + + it "does not allow deactivating the user" do + expect(page).not_to have_link("Deactivate user", href: "/users/#{user.id}/deactivate") + end + + context "when user is deactivated" do + before do + user.update!(active: false) + get "/users/#{user.id}", headers:, params: {} + end + + it "does not allow reactivating the user" do + expect(page).not_to have_link("Reactivate user", href: "/users/#{user.id}/reactivate") + end + end end context "when the current user does not match the user ID" do @@ -482,6 +541,25 @@ RSpec.describe UsersController, type: :request do expect(page).to have_link("Change", text: "are they a data protection officer?") expect(page).to have_link("Change", text: "are they a key contact?") end + + it "allows deactivating the user" do + expect(page).to have_link("Deactivate user", href: "/users/#{other_user.id}/deactivate") + end + + context "when user is deactivated" do + before do + other_user.update!(active: false) + get "/users/#{other_user.id}", headers:, params: {} + end + + it "shows if user is not active" do + expect(page).to have_content("This user has been deactivated.") + end + + it "allows reactivating the user" do + expect(page).to have_link("Reactivate user", href: "/users/#{other_user.id}/reactivate") + end + end end context "when the user is not part of the same organisation as the current user" do @@ -686,6 +764,36 @@ RSpec.describe UsersController, type: :request do .to change { other_user.reload.name }.from("filter name").to("new name") end end + + context "when the data coordinator edits the user" do + let(:params) do + { + id: other_user.id, user: { active: value } + } + end + + context "and tries to deactivate the user" do + let(:value) { false } + + it "marks user as deactivated" do + expect { patch "/users/#{other_user.id}", headers:, params: } + .to change { other_user.reload.active }.from(true).to(false) + end + end + + context "and tries to activate deactivated user" do + let(:value) { true } + + before do + other_user.update!(active: false) + end + + it "marks user as active" do + expect { patch "/users/#{other_user.id}", headers:, params: } + .to change { other_user.reload.active }.from(false).to(true) + end + end + end end context "when the current user does not match the user ID" do @@ -730,6 +838,15 @@ RSpec.describe UsersController, type: :request do }, } end + + let(:personalisation) do + { + name: params[:user][:name], + email: params[:user][:email], + organisation: user.organisation.name, + link: include("/account/confirmation?confirmation_token="), + } + end let(:request) { post "/users/", headers:, params: } before do @@ -740,6 +857,11 @@ RSpec.describe UsersController, type: :request do expect { request }.to change(User, :count).by(1) end + it "sends an invitation email" do + expect(notify_client).to receive(:send_email).with(email_address: params[:user][:email], template_id: User::CONFIRMABLE_TEMPLATE_ID, personalisation:).once + request + end + it "redirects back to organisation users page" do request expect(response).to redirect_to("/organisations/#{user.organisation.id}/users") @@ -786,6 +908,57 @@ RSpec.describe UsersController, type: :request do expect(page).not_to have_field("user-role-support-field") end end + + describe "#deactivate" do + before do + sign_in user + end + + context "when the current user matches the user ID" do + before do + get "/users/#{user.id}/deactivate", headers: headers, params: {} + end + + it "redirects user to user page" do + expect(response).to redirect_to("/users/#{user.id}") + end + end + + context "when the current user does not match the user ID" do + before do + get "/users/#{other_user.id}/deactivate", headers: headers, params: {} + end + + it "shows deactivation page with deactivate and cancel buttons for the user" do + expect(path).to include("/users/#{other_user.id}/deactivate") + expect(page).to have_content(other_user.name) + expect(page).to have_content("Are you sure you want to deactivate this user?") + expect(page).to have_button("I’m sure – deactivate this user") + expect(page).to have_link("No – I’ve changed my mind", href: "/users/#{other_user.id}") + end + end + end + + describe "#reactivate" do + before do + sign_in user + end + + context "when the current user does not match the user ID" do + before do + other_user.update!(active: false) + get "/users/#{other_user.id}/reactivate", headers: headers, params: {} + end + + it "shows reactivation page with reactivate and cancel buttons for the user" do + expect(path).to include("/users/#{other_user.id}/reactivate") + expect(page).to have_content(other_user.name) + expect(page).to have_content("Are you sure you want to reactivate this user?") + expect(page).to have_button("I’m sure – reactivate this user") + expect(page).to have_link("No – I’ve changed my mind", href: "/users/#{other_user.id}") + end + end + end end context "when user is signed in as a support user" do @@ -806,15 +979,19 @@ RSpec.describe UsersController, type: :request do get "/users", headers:, params: {} end - it "shows all active users" do + it "shows all users" do expect(page).to have_content(user.name) expect(page).to have_content(other_user.name) - expect(page).not_to have_content(inactive_user.name) + expect(page).to have_content(inactive_user.name) expect(page).to have_content(other_org_user.name) end + it "shows last logged in as deactivated for inactive users" do + expect(page).to have_content("Deactivated") + end + it "shows the pagination count" do - expect(page).to have_content("3 total users") + expect(page).to have_content("4 total users") end it "shows the download csv link" do @@ -955,6 +1132,10 @@ RSpec.describe UsersController, type: :request do expect(page).to have_link("Change", text: "are you a data protection officer?") expect(page).to have_link("Change", text: "are you a key contact?") end + + it "does not allow deactivating the user" do + expect(page).not_to have_link("Deactivate user", href: "/users/#{user.id}/deactivate") + end end context "when the current user does not match the user ID" do @@ -980,6 +1161,25 @@ RSpec.describe UsersController, type: :request do expect(page).to have_link("Change", text: "are they a data protection officer?") expect(page).to have_link("Change", text: "are they a key contact?") end + + it "allows deactivating the user" do + expect(page).to have_link("Deactivate user", href: "/users/#{other_user.id}/deactivate") + end + + context "when user is deactivated" do + before do + other_user.update!(active: false) + get "/users/#{other_user.id}", headers:, params: {} + end + + it "shows if user is not active" do + expect(page).to have_content("This user has been deactivated.") + end + + it "allows reactivating the user" do + expect(page).to have_link("Reactivate user", href: "/users/#{other_user.id}/reactivate") + end + end end context "when the user is not part of the same organisation as the current user" do @@ -1072,6 +1272,19 @@ RSpec.describe UsersController, type: :request do expect(page).to have_field("user[is_key_contact]") end end + + context "when trying to edit deactivated user" do + before do + other_user.update!(active: false) + get "/users/#{other_user.id}/edit", headers:, params: {} + end + + it "redirects to user details page" do + expect(response).to redirect_to("/users/#{other_user.id}") + follow_redirect! + expect(page).not_to have_link("Change") + end + end end end @@ -1106,17 +1319,20 @@ RSpec.describe UsersController, type: :request do describe "#update" do context "when the current user matches the user ID" do + let(:request) { patch "/users/#{user.id}", headers:, params: } + before do sign_in user - patch "/users/#{user.id}", headers:, params: end it "updates the user" do + request user.reload expect(user.name).to eq(new_name) end it "tracks who updated the record" do + request user.reload whodunnit_actor = user.versions.last.actor expect(whodunnit_actor).to be_a(User) @@ -1125,13 +1341,32 @@ RSpec.describe UsersController, type: :request do context "when user changes email, dpo and key contact" do let(:params) { { id: user.id, user: { name: new_name, email: new_email, is_dpo: "true", is_key_contact: "true" } } } + let(:personalisation) do + { + name: params[:user][:name], + email: new_email, + organisation: user.organisation.name, + link: include("/account/confirmation?confirmation_token="), + } + end + + before do + user.update(old_user_id: nil) + end it "allows changing email and dpo" do + request user.reload expect(user.unconfirmed_email).to eq(new_email) expect(user.is_data_protection_officer?).to be true expect(user.is_key_contact?).to be true end + + it "sends a confirmation email to both emails" do + expect(notify_client).to receive(:send_email).with(email_address: new_email, template_id: User::CONFIRMABLE_TEMPLATE_ID, personalisation:).once + expect(notify_client).to receive(:send_email).with(email_address: user.email, template_id: User::CONFIRMABLE_TEMPLATE_ID, personalisation:).once + request + end end context "when we update the user password" do