diff --git a/app/controllers/internal/users_controller.rb b/app/controllers/internal/users_controller.rb index 948ca2dda..3ebc752de 100644 --- a/app/controllers/internal/users_controller.rb +++ b/app/controllers/internal/users_controller.rb @@ -58,7 +58,7 @@ class Internal::UsersController < Internal::ApplicationController @user = User.find(params[:id]) begin Moderator::DeleteUser.call(admin: current_user, user: @user, user_params: user_params) - flash[:success] = "@" + @user.username + " (email: " + @user.email + ", user_id: " + @user.id.to_s + ") has been fully deleted. If requested, old content may have been ghostified. If this is a GDPR delete, delete them from Mailchimp & Google Analytics." + flash[:success] = "@#{@user.username} (email: #{@user.email.presence || 'no email'}, user_id: #{@user.id}) has been fully deleted. If requested, old content may have been ghostified. If this is a GDPR delete, delete them from Mailchimp & Google Analytics." rescue StandardError => e flash[:danger] = e.message end diff --git a/app/models/user.rb b/app/models/user.rb index 1a119ce73..02252a586 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -463,6 +463,8 @@ class User < ApplicationRecord end def unsubscribe_from_newsletters + return if email.blank? + MailchimpBot.new(self).unsubscribe_all_newsletters end diff --git a/app/workers/users/delete_worker.rb b/app/workers/users/delete_worker.rb index 20ab4da67..f36d1b79a 100644 --- a/app/workers/users/delete_worker.rb +++ b/app/workers/users/delete_worker.rb @@ -9,7 +9,9 @@ module Users return unless user Users::Delete.call(user) - NotifyMailer.account_deleted_email(user).deliver unless admin_delete + return if admin_delete || user.email.blank? + + NotifyMailer.account_deleted_email(user).deliver rescue StandardError => e DatadogStatsClient.count("users.delete", 1, tags: ["action:failed", "user_id:#{user.id}"]) Rails.logger.error("Error while deleting user: #{e}") diff --git a/spec/requests/internal/users_spec.rb b/spec/requests/internal/users_spec.rb index 2928dc4ed..5d55733f8 100644 --- a/spec/requests/internal/users_spec.rb +++ b/spec/requests/internal/users_spec.rb @@ -22,7 +22,7 @@ RSpec.describe "internal/users", type: :request do end end - describe "GET internal/users/:id/edit" do + describe "GET /internal/users/:id/edit" do it "redirects from /username/moderate" do get "/#{user.username}/moderate" expect(response).to redirect_to("/internal/users/#{user.id}") @@ -39,14 +39,14 @@ RSpec.describe "internal/users", type: :request do end end - describe "POST internal/users/:id/banish" do + describe "POST /internal/users/:id/banish" do it "bans user for spam" do post "/internal/users/#{user.id}/banish" expect(user.reload.username).to include("spam") end end - describe "DELETE internal/users/:id/remove_identity" do + describe "DELETE /internal/users/:id/remove_identity" do it "removes the given identity" do identity = user.identities.first delete "/internal/users/#{user.id}/remove_identity", params: { user: { identity_id: identity.id } } diff --git a/spec/system/internal/admin_deletes_user_spec.rb b/spec/system/internal/admin_deletes_user_spec.rb new file mode 100644 index 000000000..4facc001c --- /dev/null +++ b/spec/system/internal/admin_deletes_user_spec.rb @@ -0,0 +1,33 @@ +require "rails_helper" + +RSpec.describe "Admin deletes user", type: :system do + let(:admin) { create(:user, :super_admin) } + let(:user) { create(:user) } + + before do + sign_in admin + visit "/internal/users/#{user.id}/edit" + end + + it "enqueues a job for deleting the user" do + sidekiq_assert_enqueued_jobs(1, only: Users::DeleteWorker) do + click_button "☠️ Fully Delete User & All Activity ☠️" + end + + message = "@#{user.username} (email: #{user.email}, user_id: #{user.id}) has been fully deleted." + expect(page).to have_content(message) + end + + # See: https://github.com/thepracticaldev/tech-private/issues/404 + it "deletes users when they have no email address" do + user.update(email: nil) + + sidekiq_perform_enqueued_jobs do + click_button "☠️ Fully Delete User & All Activity ☠️" + end + + message = "@#{user.username} (email: no email, user_id: #{user.id}) has been fully deleted." + expect(page).to have_content(message) + expect(User.find_by(id: user.id)).to be_nil + end +end