From 4b7919a60e8cbdd311918b3d0c6a7c35e0c6accd Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Tue, 31 May 2022 15:49:50 +0100 Subject: [PATCH] Cldc 1226 deactivate users (#624) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * CLDC-1195: remove access keys * šŸ¤ content fixes (#539) * Remove ?? * Set hhmemb to totadult and tchild sum if hhmemb is not given (#540) * Set hhmemb to totadult and tchild sum if hhmemb is not given Co-authored-by: Ted Co-authored-by: Dushan * lint * update method * calculate hhmemb from details provided Co-authored-by: Ted Co-authored-by: Dushan * Fix condition affects question (illness type) * map tshortfall value (#543) * Make case log status rely on subsection status so they can't get out of sync * Fix complete case log fixture * Fix Reason preference reason and lettings allocation answer option import * don't route to hbrentshortfall if hb is 7 (#544) * Age known believe import fixes (#545) * amend fixture to make test fail * fix failing test * lint fixes * Enable dynamically dependent answer options * Refactor ecstat age check * Dry depends on evaluation * Derive major repairs for import * Add white other * Set major repair date * Majorrepairs will never match * Rubocop * Cldc 1149 radio buttons filter (#548) * radio button * radio button again * rubocop versioning issue fixed * linux and darwin * Fix Gemfile * rubocop * erb lint Co-authored-by: baarkerlounger * Set tshortfall to 0 if it should exists but is not provided (#550) * Set tshortfall to 0 if it should exists but is not provided * check if tshortfall is overridden * Save correct la if postcode is invalid (#551) * Allow illness type to be refused (#553) * Radio button on log filter is now preset to "All" (#552) * Radio button on log filter is now preset to "All" * lint fixes * removed instance @ variable * CLDC-744-joint-tenancy-validation (#549) * add joint tenancy validation * fix validation and spec * improvements * updates * lint fixes * fix typo * change message displayed on hhmemb page * Add docs dir to dockerignore and cfignore (#555) * Add files via upload (#554) Adding Delta Discovery file * Improve DB seeding (#556) * Improve DB seeding * Don't seed LA Rent Ranges in test * Add navigation items helper * Fix specs and linters * Works but helper is hard to test * add check to prevent error on hhmemb if joint is nil (#560) * add failing spec * add check to prevent error on hhmemb if joint is nil * Refactor for testability * Spec nav bar highlighting from user perspective * CLDC-1218: Fix hbrentshortfall import (#558) * Infer hbrentshortfall not known if tshortfall not provided and overridden * Reorder import * Referral can be internal for homeless (#561) * Set case log ID offset at export (#562) * Make tshortfall optional based on hidden tshortfall_known question (#563) * Make tshortfall optional based on hidden tshortfall_known question * Add test for optional * Add test for JSON derived and dependent on false options * Test routing * Fix optionality * Inactive users (#564) * Allow users to be marked as inactive * Import inactive users * add missing field to seeded user for dev env * Map joint tenancy field (#565) * Use rake task to send onboarding emails (#566) * Use rake task to send onboarding emails * Wrap host lookup so it's easily stubbable * Rake task can be called outside the rails environment so need to pass host in * Not part of the usual app flow so contain to rake task * Including routes helper in a rake task is a rabbit hole * Use ENV var rather than param for host * Sort case log index table by most recent by default (#567) * Add sheltered accom field (#568) * Remove needs type question until we support supported housing logs (#569) * Fix routing for tshortfall (#571) * Fix routing for tshortfall * HB 7 and 8 don't exist in 22/23 anymore * Add visiblly hidden change link text for details known questions * Fix repeated use of Password in error summary (and use smart quotes) * Make logs link less ambiguous * Cldc 1217 retirement soft validation (#570) * Interruption screen refactor * Add test for retirement_age_for_person Co-authored-by: StĆ©phane Meny * Rubocop * update 22-23 form * fix failures * lint fixes * remove whitespace * remove commented code * make spec file use translations instead of content * update content * lint fixes * fix typo * fix question wording * fix failing spec * add full stop Co-authored-by: StĆ©phane Meny Co-authored-by: Dushan Despotovic * Hide inactive users and allow support users to view all users (#576) * Hide inactive users and allow support users to view all users * Enable support users to invite users to any organisation * Add pagination to user views * Update config/locales/devise.en.yml Co-authored-by: Paul Robert Lloyd * remove letting_in_sheltered_accomodation field (#577) * remove letting_in_sheltered_accomodation field This field is now duplicated by the shelteredaccom field * lint fixes * fix all failing specs * Generisize pagination links * CLDC-1101: Case log filter by organisation (#522) * Rename org filter * Ensure filters work when ID is passed * UI init * Lint * Fix filter nesting * Reduce width * Set checked status of filters correctly * Test filter presence * Add filter test * Rubocop * Tweak styles for autocomplete within filter Co-authored-by: Paul Robert Lloyd * CLDC-1224: Tenancy type and tenancy length (#578) * Fix value mapping for tenancy type 22/23 * Update secure helper * Null safe * Spec update * Update mandatory * Use helper * Remove joint tenancy hitn text and add don't know option (#579) * Export improvements (#581) * improved export - set start point for reference * remove completed status from exports * Added exception handling to export * Improved tests, replaced rescue block with Sentry Co-authored-by: Kat * Update logs table column heading to say ā€˜Statusā€™, not ā€˜Completedā€™ * Show ā€˜Data protection officerā€™ and/or ā€˜Key contactā€™ tags in users table * Update heading, label and hint text for user details * Fix organisation filter (#582) * Filter values correctly * Remove filter value if hidden * Return empty string rather than nil for accessible autocomplete * Additional test for accessible automcomplete defaulting * Add status tag helper * Add missing ā€˜orā€™ divider on joint tenancy question * Use dash not hyphen in answer options * CLDC-1118: Implement data export structure (CDS) (#587) Co-authored-by: Dushan Despotovic * CLDC-1217: Retirement soft validation (#586) * Don't trigger soft validation if tenant prefers not to say * Update gender content * Fix spec description * Enable support users to download user details (#589) * Enable support users to download user details * Download all users * Rubocop * Preload organisations to remove n+1 queries * Confirmable (#580) * Confirmable * Remove obsolete rake task * Skip confirmation for inactive users * Send beta onboarding template if migrated from Softwire * Default controller * Use correct link * Redirect confirmation to set password * Confirm account within 3 days * Only redirect to set password if not previously set * Rubocop * Confirm factory bot users * Set password condition * Changing email requires reconfirming * No need to explicitly trigger email, devise does that for us now * Remove flash banner * Mock notify * Mock in the right spec * Test redirect and text * User is confirmable * Rubocop * Redirect to url so we don't bypass authenticity token * Update content * Add test for resend invite flow * Update link to resend confirmation email * Rename password reset resend confirmation partial * Expired link error page * Remove resend confirmation link * Update seed * Expory contact * Time zone Co-authored-by: Paul Robert Lloyd * Attempt at fixing S3 error when saving Zip file (#590) * Enable better other field validation labels * Removing gaps caused by empty exports (#591) * Add Sentry instrumentation (#593) * Avoid NoMethodError when Sentry is not initialised * Instrument collection_start_year * Removes Sentry custom instrumentation * test for user being in dev env not being asked for 2fa * code for user being in dev env not being asked for 2fa * pulled out of support person context, this should work for any user in dev env * rubocop * matching wording * only support user atm need 2fa, so making this explicit * Perf (#592) * Memoize start year * Reset * Recalculate rather than reset * Fix heisenspec (order independent array comparison) (#596) * Redirect confirmed users to sign in * Add support for CSV export (#598) * Cldc 1102 admin organisations page (#557) * Get all organisations in controller * Display organisations data in the table * Route to logs for specific organisation * add tests * update spec * lint fixes * set up failing test for organisation logs page * fix failing test * write test for organisations support user page * Update a organisation page test and lint * added pagination test with next and previous links and total count for support user * test for pagination in organisations title * Added "Organisations" to to organisations page title * add pagination test for organisations page 2, remove second before block * Add the remaining pagination tests * Redirect when accessing organisation logs by non support user * Test for displaying logs for specific organisation * Add test for org name * Add a failing log filter test for specific org * Extract filter methods into a helper * Allow logs filtering for specific org * Fix test, support user was creating an extra org, remove orgs filter for specific org * Remove redundant test, lint * Reuse primary navigation component and add sub navigation for support users * allow support users edit or and add sub navigation to about this org * allow support users to access the edit org page * only allow to edit existing editable fields * display correct values in the organisations table * allow support user to update org * user table component for organisations table * use guard clause for organisation logs page * remove create a new lettings log from organisation logs * Move case logs filter from helpers to modules * lint erb * yarn lint * bring back if statement in logs controller * update modules import * let! * test for links first in the org cotroller spec * interpolate number of orgs * conditionally render sub navigation Co-authored-by: Kat Co-authored-by: Dushan Despotovic Co-authored-by: JG * Bump nokogiri from 1.13.5 to 1.13.6 (#601) Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.13.5 to 1.13.6. - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/main/CHANGELOG.md) - [Commits](https://github.com/sparklemotion/nokogiri/compare/v1.13.5...v1.13.6) --- updated-dependencies: - dependency-name: nokogiri dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * CLDC-1124: User search (#600) * Add search by name for users Co-authored-by: baarkerlounger * Search is now non case sensitive * Made search work for data co-ordinators Co-authored-by: baarkerlounger * Refactored to scope * Added search by email Co-authored-by: baarkerlounger * WIP Commit - added test for if search term matches a name and an email address simultaneously. Also changed search result caption for organisations to display "Matches X of Y users" * Rubocop * Preload org * Linting * Refactor filtered_users into module * Only adjust query param if searched * Add data coordinator tests * Add table caption spec * Dupe attribute * Refactor into Search ViewComponent * Rubocop * Unit test user scopes Co-authored-by: Ted Co-authored-by: baarkerlounger * Use dedicated styles for each navigation component * Use dedicated styles for each navigation component * Use simpler markup for table captions * Use table component for logs list * Correct markup for user and organisation tables * Email allowlist (#603) * Email allowlist * Set rails master key in deploy pipelines * CLDC-1258: Handle invalid void date during import (#604) * Now we raise an exception if a non-existing organisation is referenced by a case log * The void date is not imported if it is after the tenancy start date * make organisations list page default view for support user login (#605) * Use seperate components for primary and sub navigation * Add LA org mapping * Rubocop * User search fixes (#607) * Update query message * Add clear search link * Set input value * Use gem component * Move to list partial pattern * Partial path * Update spec * Rubocop * Unit test filter module * Rubocop * Add search result to page title if searched * Add missing horizontal rule * Use form_group attributes for search input Co-authored-by: Paul Robert Lloyd * CLDC-1225: At import updates relationship to child when a person is under 16 (#609) * Organisation search (#610) * Add search to organisations * Fix title * Spec page title * Don't seed org in test * Handles unicode characters in postcode (#612) * CSV download only includes users in search result * Updates exported fields based on May 25th feedback (#613) * Cldc 1223 pregnancy soft validations (#602) * update hard validation limits for pregnancy age, remove hard validation if there are no females at all * Add soft validations for pregnancy * make the error message consistent * Only check the values for the members with details known in the household * Show interruption screen when resident details are updated * Route back to check answers after an interruption screen and back to previous page if no is selected on the interruption screen Co-authored-by: baarkerlounger * update validation messages * fix a test * fix a typo Co-authored-by: baarkerlounger * Avoid variable number of columns during CSV export (#614) * CLDC-1277: no route matches bug (#615) * don't display the save and go to the next incomplete section button if it errors šŸ¤·ā€ā™€ļø * use 2022/23c fixture for in the test * Allow support users and data coordinators to toggle active users * Add a link to deactivate page * add deactivate page * Show if user is deactivated and fix form * Show deactivated user in the user list * Show reactivate user link for deactivated users * add reactivate user page * Refactor * remove unused method, route and lint * fix routes * Only allow active users to log in * Add flash banner for successful toggle, fix styles * FIx more styling * prevent editing deactivated user * lint * reset confirmed_at, password and sign in count when deactivated. Send reactivation email if user has previously logged in * Use dash not hyphen in confirmation page button and links * Content: Deactivate/reactivate account, not the user * Send confirmation email to both, old and new email addresses * Add possessive gem for names formatting * Use hidden input field for active value * lint * Only send beta onboarding emails if the user hasn't previously logged in * Don't clear the password for deactivated user * refactor sending confirmation emails Co-authored-by: kiddhustle Co-authored-by: Paul Robert Lloyd Co-authored-by: baarkerlounger Co-authored-by: baarkerlounger <5101747+baarkerlounger@users.noreply.github.com> Co-authored-by: Ted Co-authored-by: Dushan Co-authored-by: Dushan <47317567+dushan-madetech@users.noreply.github.com> Co-authored-by: Ted-U <92022120+Ted-U@users.noreply.github.com> Co-authored-by: sona-mhclg <77793209+sona-mhclg@users.noreply.github.com> Co-authored-by: Paul Robert Lloyd Co-authored-by: StĆ©phane Meny Co-authored-by: JG Co-authored-by: J G <7750475+moarpheus@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ted Co-authored-by: baarkerlounger --- Gemfile | 1 + Gemfile.lock | 2 + app/controllers/modules/search_filter.rb | 2 +- app/controllers/users_controller.rb | 32 ++- app/helpers/user_helper.rb | 16 +- app/mailers/devise_notify_mailer.rb | 32 ++- app/models/user.rb | 13 +- app/views/users/_user_list.html.erb | 2 +- app/views/users/show.html.erb | 11 + app/views/users/toggle_active.html.erb | 26 ++ config/locales/devise.en.yml | 6 +- config/routes.rb | 7 +- .../controllers/modules/search_filter_spec.rb | 8 +- spec/features/user_spec.rb | 82 ++++++ .../requests/organisations_controller_spec.rb | 4 +- spec/requests/users_controller_spec.rb | 243 +++++++++++++++++- 16 files changed, 454 insertions(+), 33 deletions(-) create mode 100644 app/views/users/toggle_active.html.erb 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