From 6d3d10f9fc614babb6c01edb34e90f0755919cb1 Mon Sep 17 00:00:00 2001 From: Kat Date: Tue, 5 Mar 2024 10:34:12 +0000 Subject: [PATCH] Allow deleting user --- app/controllers/organisations_controller.rb | 4 +- app/controllers/users_controller.rb | 4 +- app/models/user.rb | 7 ++- config/locales/en.yml | 1 + spec/requests/users_controller_spec.rb | 60 +++++++++++++++++++++ 5 files changed, 72 insertions(+), 4 deletions(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index dc5a36c9b..4982c4287 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -46,14 +46,14 @@ class OrganisationsController < ApplicationController end def users - organisation_users = @organisation.users.sorted_by_organisation_and_role + organisation_users = @organisation.users.visible.sorted_by_organisation_and_role unpaginated_filtered_users = filter_manager.filtered_users(organisation_users, search_term, session_filters) respond_to do |format| format.html do @pagy, @users = pagy(unpaginated_filtered_users) @searched = search_term.presence - @total_count = @organisation.users.size + @total_count = @organisation.users.visible.size @filter_type = "users" if current_user.support? diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index b2c74433e..d783ef843 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -13,7 +13,7 @@ class UsersController < ApplicationController def index redirect_to users_organisation_path(current_user.organisation) unless current_user.support? - all_users = User.sorted_by_organisation_and_role + all_users = User.visible.sorted_by_organisation_and_role filtered_users = filter_manager.filtered_users(all_users, search_term, session_filters) @pagy, @users = pagy(filtered_users) @searched = search_term.presence @@ -128,6 +128,8 @@ class UsersController < ApplicationController def delete authorize @user + @user.discard! + redirect_to users_organisation_path(@user.organisation), notice: I18n.t("notification.user_deleted", name: @user.name) end private diff --git a/app/models/user.rb b/app/models/user.rb index 33fc07482..6d13e8cd1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -76,6 +76,7 @@ class User < ApplicationRecord scope :not_signed_in, -> { where(last_sign_in_at: nil, active: true) } scope :deactivated, -> { where(active: false) } scope :active_status, -> { where(active: true).where.not(last_sign_in_at: nil) } + scope :visible, -> { where(discarded_at: nil) } def lettings_logs if support? @@ -240,11 +241,15 @@ class User < ApplicationRecord def status return :deleted if discarded_at.present? return :deactivated unless active - return :unconfirmed if !confirmed? + return :unconfirmed unless confirmed? :active end + def discard! + update!(discarded_at: Time.zone.now) + end + protected # Checks whether a password is needed or not. For validations only. diff --git a/config/locales/en.yml b/config/locales/en.yml index d95187a49..079967095 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -198,6 +198,7 @@ en: other: "There are %{count} sets of duplicate logs" location_deleted: "%{postcode} has been deleted." scheme_deleted: "%{service_name} has been deleted." + user_deleted: "%{name} has been deleted." validations: organisation: diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 3f8b59d98..3534d9665 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -110,6 +110,13 @@ RSpec.describe UsersController, type: :request do expect(response).to redirect_to("/account/sign-in") end end + + describe "#delete" do + it "redirects to the sign in page" do + delete "/users/#{user.id}/delete" + expect(response).to redirect_to("/account/sign-in") + end + end end context "when user is signed in as a data provider" do @@ -400,6 +407,18 @@ RSpec.describe UsersController, type: :request do expect(response).to have_http_status(:unauthorized) end end + + describe "#delete" do + before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + delete "/users/#{user.id}/delete" + end + + it "returns 401 unauthorized" do + expect(response).to have_http_status(:unauthorized) + end + end end context "when user is signed in as a data coordinator" do @@ -1193,6 +1212,18 @@ RSpec.describe UsersController, type: :request do expect(response).to have_http_status(:unauthorized) end end + + describe "#delete" do + before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + delete "/users/#{user.id}/delete" + end + + it "returns 401 unauthorized" do + expect(response).to have_http_status(:unauthorized) + end + end end context "when user is signed in as a support user" do @@ -2084,6 +2115,35 @@ RSpec.describe UsersController, type: :request do expect(page).to have_link(text: "Cancel", href: user_path(other_user)) end end + + describe "#delete" do + let(:other_user) { create(:user, name: "User to be deleted") } + + before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + delete "/users/#{other_user.id}/delete" + end + + it "deletes the user" do + other_user.reload + expect(other_user.status).to eq(:deleted) + expect(other_user.discarded_at).not_to be nil + end + + it "redirects to the users list and displays a notice that the user has been deleted" do + expect(response).to redirect_to users_organisation_path(other_user.organisation) + follow_redirect! + expect(page).to have_selector(".govuk-notification-banner--success") + expect(page).to have_selector(".govuk-notification-banner--success", text: "User to be deleted has been deleted.") + end + + it "does not display the deleted user" do + expect(response).to redirect_to users_organisation_path(other_user.organisation) + follow_redirect! + expect(page).not_to have_link("User to be deleted") + end + end end describe "title link" do