From 9b0d9f4a6d1702ef0d64ad15f63edfefd181ed73 Mon Sep 17 00:00:00 2001 From: Chris Mytton Date: Tue, 18 Jul 2023 07:56:39 +0100 Subject: [PATCH] Allow users to request account closure --- app/controllers/admin_user_controller.rb | 9 ++++ .../admin_users_account_closing_controller.rb | 2 +- app/controllers/application_controller.rb | 3 ++ .../users/close_account_controller.rb | 34 ++++++++++++++ app/mailers/user_mailer.rb | 7 +++ app/models/account_closure_request.rb | 13 ++++++ app/models/outgoing_message.rb | 2 +- app/models/user.rb | 4 ++ .../admin_general/_admin_navbar.html.erb | 1 + .../admin_user/_close_account_form.html.erb | 9 ++++ app/views/admin_user/_scopes.html.erb | 4 ++ .../account_closure_requests.html.erb | 35 ++++++++++++++ app/views/admin_user/edit.html.erb | 10 +--- app/views/user/_show_user_info.html.erb | 3 +- .../account_closure_requested.text.erb | 7 +++ app/views/users/close_account/new.html.erb | 26 +++++++++++ config/routes.rb | 4 ++ ...8062820_create_account_closure_requests.rb | 8 ++++ .../controllers/admin_user_controller_spec.rb | 13 ++++++ .../users/close_account_controller_spec.rb | 46 +++++++++++++++++++ spec/factories/account_closure_requests.rb | 14 ++++++ .../admin_user_account_closing_spec.rb | 40 ++++++++++++++++ 22 files changed, 282 insertions(+), 12 deletions(-) create mode 100644 app/controllers/users/close_account_controller.rb create mode 100644 app/models/account_closure_request.rb create mode 100644 app/views/admin_user/_close_account_form.html.erb create mode 100644 app/views/admin_user/account_closure_requests.html.erb create mode 100644 app/views/user_mailer/account_closure_requested.text.erb create mode 100644 app/views/users/close_account/new.html.erb create mode 100644 db/migrate/20230718062820_create_account_closure_requests.rb create mode 100644 spec/controllers/users/close_account_controller_spec.rb create mode 100644 spec/factories/account_closure_requests.rb create mode 100644 spec/integration/admin_user_account_closing_spec.rb diff --git a/app/controllers/admin_user_controller.rb b/app/controllers/admin_user_controller.rb index 4c43fbd704c..3a7d3d98827 100644 --- a/app/controllers/admin_user_controller.rb +++ b/app/controllers/admin_user_controller.rb @@ -131,6 +131,15 @@ def modify_comment_visibility redirect_back(fallback_location: admin_users_url) end + def account_closure_requests + @title = 'Account closure requests' + + # Find requests where the account associated with the request is not closed + @account_closure_requests = AccountClosureRequest. + joins(:user). + where(users: { closed_at: nil }) + end + private def user_params diff --git a/app/controllers/admin_users_account_closing_controller.rb b/app/controllers/admin_users_account_closing_controller.rb index 75e26fef7cd..9ae2b5afb85 100644 --- a/app/controllers/admin_users_account_closing_controller.rb +++ b/app/controllers/admin_users_account_closing_controller.rb @@ -12,7 +12,7 @@ def create 'Something went wrong. The user account could not be closed.' end - redirect_to admin_user_path(@closed_user) + redirect_back_or_to admin_user_path(@closed_user) end private diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 260d379e1f9..93270dcb78c 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -29,6 +29,9 @@ class RouteNotFound < StandardError # Standard headers, footers and navigation for whole site layout "default" + # Defaults are :alert and :notice, but Alaveteli uses :error as well + add_flash_types :error + include FastGettext::Translation # make functions like _, n_, N_ etc available) include AlaveteliPro::PostRedirectHandler diff --git a/app/controllers/users/close_account_controller.rb b/app/controllers/users/close_account_controller.rb new file mode 100644 index 00000000000..702c8558a21 --- /dev/null +++ b/app/controllers/users/close_account_controller.rb @@ -0,0 +1,34 @@ +class Users::CloseAccountController < ApplicationController + before_action :authenticate_user! + + def new + # Display a form that explains the process to the users + end + + def create + # If they haven't checked the "confirm" checkbox, then redirect them back to the form + return redirect_to users_close_account_path, error: "You must confirm that you want to close your account" if params[:confirm] == "0" + + # Otherwise, create a record of the user's request to close their account + current_user.create_account_closure_request! + + # Send the user an acknowledgement email + UserMailer.account_closure_requested(current_user).deliver_now + + # TODO: Should the user be logged out here? + + redirect_to root_path, notice: "Your account closure request has been received. We will be in touch." + end + + private + + def authenticate_user! + return if authenticated? + + ask_to_login( + web: _('To close your account on {{site_name}}', site_name: site_name), + email: _('Then you can close your account on {{site_name}}', site_name: site_name), + email_subject: _('Close your account on {{site_name}}', site_name: site_name) + ) + end +end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 56aeb3fef83..13b127d8beb 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -45,4 +45,11 @@ def changeemail_already_used(old_email, new_email) to: new_email, subject: _("Unable to change email address on {{site_name}}", site_name: site_name)) end + + def account_closure_requested(user) + @name = user.name + + set_reply_to_headers(user) + mail_user(user, _("Your account closure request on {{site_name}}", site_name: site_name)) + end end diff --git a/app/models/account_closure_request.rb b/app/models/account_closure_request.rb new file mode 100644 index 00000000000..fda806d3588 --- /dev/null +++ b/app/models/account_closure_request.rb @@ -0,0 +1,13 @@ +# == Schema Information +# Schema version: 20230718062820 +# +# Table name: account_closure_requests +# +# id :bigint not null, primary key +# user_id :bigint not null +# created_at :datetime not null +# updated_at :datetime not null +# +class AccountClosureRequest < ApplicationRecord + belongs_to :user +end diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb index 80a489226f3..f904706a5c9 100644 --- a/app/models/outgoing_message.rb +++ b/app/models/outgoing_message.rb @@ -1,5 +1,5 @@ # == Schema Information -# Schema version: 20230412084830 +# Schema version: 20230718062820 # # Table name: outgoing_messages # diff --git a/app/models/user.rb b/app/models/user.rb index c33aae63d4b..ca7a5eb09fe 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -159,6 +159,10 @@ class User < ApplicationRecord inverse_of: :user, dependent: :destroy + has_one :account_closure_request, + inverse_of: :user, + dependent: :destroy + scope :active, -> { not_banned.not_closed } scope :banned, -> { where.not(ban_text: '') } scope :not_banned, -> { where(ban_text: '') } diff --git a/app/views/admin_general/_admin_navbar.html.erb b/app/views/admin_general/_admin_navbar.html.erb index 8838e892bdc..f8e06c44b17 100644 --- a/app/views/admin_general/_admin_navbar.html.erb +++ b/app/views/admin_general/_admin_navbar.html.erb @@ -46,6 +46,7 @@ diff --git a/app/views/admin_user/_close_account_form.html.erb b/app/views/admin_user/_close_account_form.html.erb new file mode 100644 index 00000000000..ae3b4afd4ce --- /dev/null +++ b/app/views/admin_user/_close_account_form.html.erb @@ -0,0 +1,9 @@ +<%= form_tag admin_users_account_closing_index_path(user_id: user.id), class: 'span3 form form-inline' do %> + <% disabled = user.closed? %> + <% submit_class = %w(btn btn-danger) %> + <% submit_class << 'disabled' if disabled %> + <%= submit_tag 'Close', + class: submit_class, + disabled: disabled, + data: { confirm: 'Are you sure? This is irreversible.' } %> +<% end %> diff --git a/app/views/admin_user/_scopes.html.erb b/app/views/admin_user/_scopes.html.erb index 1fe55bd3165..2a2f68249dc 100644 --- a/app/views/admin_user/_scopes.html.erb +++ b/app/views/admin_user/_scopes.html.erb @@ -22,6 +22,10 @@ <%= nav_li(admin_sign_ins_path) do %> <%= link_to 'Sign Ins', admin_sign_ins_path %> <% end %> + + <%= nav_li(account_closure_requests_admin_users_path) do %> + <%= link_to 'Account closure requests', account_closure_requests_admin_users_path %> + <% end %> diff --git a/app/views/admin_user/account_closure_requests.html.erb b/app/views/admin_user/account_closure_requests.html.erb new file mode 100644 index 00000000000..75c0c48758a --- /dev/null +++ b/app/views/admin_user/account_closure_requests.html.erb @@ -0,0 +1,35 @@ +<%= render 'scopes' %> + +
+
+ <% if @account_closure_requests.any? %> + + + + + + + + + + + <% @account_closure_requests.each do |request| %> + + + + + + + <% end %> + +
Request IDUserCreated atAction
<%= request.id %> + <%= link_to request.user.name, admin_user_path(request.user) %> + <%= request.created_at.to_fs(:long) %> + <%= render 'close_account_form', { user: request.user } %> +
+ + <% else %> +

No users have requested to close their accounts.

+ <% end %> +
+
diff --git a/app/views/admin_user/edit.html.erb b/app/views/admin_user/edit.html.erb index a793b693c89..d8d000ec720 100644 --- a/app/views/admin_user/edit.html.erb +++ b/app/views/admin_user/edit.html.erb @@ -39,15 +39,7 @@

- <%= form_tag admin_users_account_closing_index_path(user_id: @admin_user.id), class: 'span3 form form-inline' do %> - <% disabled = @admin_user.closed? %> - <% submit_class = %w(btn btn-danger) %> - <% submit_class << 'disabled' if disabled %> - <%= submit_tag 'Close', - class: submit_class, - disabled: disabled, - data: { confirm: 'Are you sure? This is irreversible.' } %> - <% end %> + <%= render 'close_account_form', { user: @admin_user } %>
diff --git a/app/views/user/_show_user_info.html.erb b/app/views/user/_show_user_info.html.erb index 5d1dc28a755..ea3b05206d4 100644 --- a/app/views/user/_show_user_info.html.erb +++ b/app/views/user/_show_user_info.html.erb @@ -14,7 +14,8 @@ <%= link_to _('Change profile photo'), set_profile_photo_path %> | <% end %> <%= link_to _('Change your password'), new_password_change_path %> | - <%= link_to _('Change your email'), signchangeemail_path %> + <%= link_to _('Change your email'), signchangeemail_path %> | + <%= link_to _('Close your account'), users_close_account_path %> <% if AlaveteliConfiguration.enable_two_factor_auth %> | <% if @display_user.otp_enabled? %> diff --git a/app/views/user_mailer/account_closure_requested.text.erb b/app/views/user_mailer/account_closure_requested.text.erb new file mode 100644 index 00000000000..ba9efc8909a --- /dev/null +++ b/app/views/user_mailer/account_closure_requested.text.erb @@ -0,0 +1,7 @@ +<%= raw @name %>, + +<%= _("You've requested to close your account on {{site_name}}." \ + 'We will process your request and will be in touch once it has been actioned.', + :site_name => site_name.html_safe) %> + +-- <%= _('the {{site_name}} team', :site_name => site_name.html_safe) %> diff --git a/app/views/users/close_account/new.html.erb b/app/views/users/close_account/new.html.erb new file mode 100644 index 00000000000..6afda5721e4 --- /dev/null +++ b/app/views/users/close_account/new.html.erb @@ -0,0 +1,26 @@ +<% @title = "Close your account" %> +

<%= @title %>

+ +

If you no longer wish to use your WhatDoTheyKnow account, you can ask us to close your account.

+ +

Closing your account will:

+ + + +

Closing your account will prevent you from logging in. If you have any requests that are ongoing, you will not be able to send any follow up messages to public authorities.

+ +<%= form_with url: users_close_account_path, method: :post do |form| %> +

+ <%= form.check_box :confirm, class: "checkbox" %> + I understand that closing my account will mean that I will not be able to login or follow up on my requests, and that this cannot be undone. +

+ + <%= form.submit "Close my account", class: "button alert", data: { confirm: "Are you sure you want to close your account?" } %> +<% end %> diff --git a/config/routes.rb b/config/routes.rb index 333c517ea9a..68f4fa9fd05 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -309,6 +309,9 @@ def matches?(request) get 'email_alerts/disable/:token', to: 'email_alerts#destroy', as: :disable_email_alerts + + get 'close_account', to: 'close_account#new', as: :close_account + post 'close_account', to: 'close_account#create' end namespace :users, path: 'profile' do @@ -704,6 +707,7 @@ def matches?(request) get 'active', :on => :collection get 'banned', :on => :collection get 'closed', :on => :collection + get 'account_closure_requests', :on => :collection get 'show_bounce_message', :on => :member post 'clear_bounce', :on => :member post 'clear_profile_photo', :on => :member diff --git a/db/migrate/20230718062820_create_account_closure_requests.rb b/db/migrate/20230718062820_create_account_closure_requests.rb new file mode 100644 index 00000000000..32dc89e10a4 --- /dev/null +++ b/db/migrate/20230718062820_create_account_closure_requests.rb @@ -0,0 +1,8 @@ +class CreateAccountClosureRequests < ActiveRecord::Migration[7.0] + def change + create_table :account_closure_requests do |t| + t.references :user, null: false, foreign_key: true + t.timestamps + end + end +end diff --git a/spec/controllers/admin_user_controller_spec.rb b/spec/controllers/admin_user_controller_spec.rb index ed2e47d4780..69bd82dc2f4 100644 --- a/spec/controllers/admin_user_controller_spec.rb +++ b/spec/controllers/admin_user_controller_spec.rb @@ -449,4 +449,17 @@ }.to change(ActsAsXapian::ActsAsXapianJob, :count).by(1) end end + + describe "GET #account_closure_requests" do + it "assigns @account_closure_requests with requests where accounts are not closed" do + user_with_closed_account = FactoryBot.create(:user, closed_at: Time.now) + user_with_open_account = FactoryBot.create(:user) + request_for_closed_account = FactoryBot.create(:account_closure_request, user: user_with_closed_account) + request_for_open_account = FactoryBot.create(:account_closure_request, user: user_with_open_account) + + get :account_closure_requests + + expect(assigns(:account_closure_requests)).to eq([request_for_open_account]) + end + end end diff --git a/spec/controllers/users/close_account_controller_spec.rb b/spec/controllers/users/close_account_controller_spec.rb new file mode 100644 index 00000000000..7e58f0db04b --- /dev/null +++ b/spec/controllers/users/close_account_controller_spec.rb @@ -0,0 +1,46 @@ +# spec/controllers/users/close_account_controller_spec.rb +require 'spec_helper' + +RSpec.describe Users::CloseAccountController, type: :controller do + describe "POST #create" do + let(:user) { FactoryBot.create(:user) } + + before do + sign_in user + end + + after do + user.account_closure_request&.destroy + end + + it "shows the user a confirmation page" do + get :new + assert_response :success + expect(response).to render_template(:new) + end + + it "asks the user to check the confirmation checkbox" do + post :create, params: { confirm: "0" } + assert_response :redirect + expect(response).to redirect_to(users_close_account_path) + expect(flash[:error]).to eq("You must confirm that you want to close your account") + end + + it "creates a record of the user's request to close their account" do + post :create, params: { confirm: "1" } + + user.reload + expect(user.account_closure_request).to be_present + + # Check email has been sent + expect(ActionMailer::Base.deliveries.count).to eq(1) + email = ActionMailer::Base.deliveries.last + expect(email.to).to eq([user.email]) + expect(email.subject).to eq("Your account closure request on #{site_name}") + + assert_response :redirect + expect(response).to redirect_to(root_path) + expect(flash[:notice]).to eq("Your account closure request has been received. We will be in touch.") + end + end +end diff --git a/spec/factories/account_closure_requests.rb b/spec/factories/account_closure_requests.rb new file mode 100644 index 00000000000..c2dbc6da5ec --- /dev/null +++ b/spec/factories/account_closure_requests.rb @@ -0,0 +1,14 @@ +# == Schema Information +# Schema version: 20230718062820 +# +# Table name: account_closure_requests +# +# id :bigint not null, primary key +# user_id :bigint not null +# created_at :datetime not null +# updated_at :datetime not null +# +FactoryBot.define do + factory :account_closure_request do + end +end diff --git a/spec/integration/admin_user_account_closing_spec.rb b/spec/integration/admin_user_account_closing_spec.rb new file mode 100644 index 00000000000..f5b2c3769f6 --- /dev/null +++ b/spec/integration/admin_user_account_closing_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' +require 'integration/alaveteli_dsl' + +RSpec.describe 'Admin Account Closure Requests' do + before do + allow(AlaveteliConfiguration).to receive(:skip_admin_auth).and_return(false) + + confirm(:admin_user) + @admin = login(:admin_user) + @user = FactoryBot.create(:user) + @account_closure_request = FactoryBot.create(:account_closure_request, user: @user) + end + + context 'viewing account closure requests' do + it 'displays link to "Account closure requests" on admin homepage' do + using_session(@admin) do + visit admin_general_index_path + expect(page).to have_link('Account closure requests') + end + end + + it 'can close an account from the "Account closure requests" page' do + using_session(@admin) do + expect(@user).to_not be_closed + + visit account_closure_requests_admin_users_path + within("tr#account-closure-request-#{@account_closure_request.id}") do + click_button 'Close' + end + expect(page).to have_text('The user account was closed.') + + @user.reload + expect(@user).to be_closed + + visit account_closure_requests_admin_users_path + expect(page).to_not have_text(@user.name) + end + end + end +end