Asynchronous organization delete (#13388)

* Asynchronous organization destroy

* Notification after an organization was deleted

* Send data to Datadog and Honeybadger when org deletion failed

* Delete unused code

* Fixed Organization#destroyable? and organization delete specs

* Reorganized organization deletion specs

* Removed redundant specs

* Improved org deleted email text template

* Don't peform the org delete when a user or an org were not found

* Renamed user_id in the org delete worker

* Audit logging when deleten an organization

* Removed specs for tracking pixel and UTM params

* Changed slug for audit log on org delete

* Fixed schema.rb
This commit is contained in:
Anna Buianova 2021-04-19 18:30:35 +03:00 committed by GitHub
parent 3dc8a386d2
commit ba1c13bbb6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 285 additions and 43 deletions

View file

@ -51,16 +51,12 @@ class OrganizationsController < ApplicationController
def destroy
organization = Organization.find_by(id: params[:id])
authorize organization
if organization.destroy
current_user.touch(:organization_info_updated_at)
EdgeCache::BustUser.call(current_user)
flash[:settings_notice] = "Your organization: \"#{organization.name}\" was successfully deleted."
redirect_to user_settings_path(:organization)
else
flash[:settings_notice] = "#{organization.errors.full_messages.to_sentence}.
Please email #{SiteConfig.email_addresses[:contact]} for assistance."
redirect_to user_settings_path(:organization, id: organization.id)
end
Organizations::DeleteWorker.perform_async(organization.id, current_user.id)
flash[:settings_notice] =
"Your organization: \"#{organization.name}\" deletion is scheduled. You'll be notified when it's deleted."
redirect_to user_settings_path(:organization)
rescue Pundit::NotAuthorizedError
flash[:error] = "Your organization was not deleted; you must be an admin, the only member in the organization,
and have no articles connected to the organization."

View file

@ -112,6 +112,14 @@ class NotifyMailer < ApplicationMailer
mail(to: params[:email], subject: subject)
end
def organization_deleted_email
@name = params[:name]
@org_name = params[:org_name]
subject = "#{SiteConfig.community_name} - Organization Deletion Confirmation"
mail(to: params[:email], subject: subject)
end
def account_deletion_requested_email
user = params[:user]
@name = user.name

View file

@ -128,7 +128,7 @@ class Organization < ApplicationRecord
end
def destroyable?
organization_memberships.count == 1 && articles.count.zero?
organization_memberships.count == 1 && articles.count.zero? && credits.count.zero?
end
private

View file

@ -0,0 +1,19 @@
module Organizations
class Delete
def initialize(org)
@org = org
end
def call
org.destroy
end
def self.call(...)
new(...).call
end
private
attr_reader :org
end
end

View file

@ -0,0 +1,17 @@
<p>
Hi <%= @name %>,
</p>
<p>
Your organization <%= @org_name %> on <%= community_name %> has been successfully deleted.
</p>
<p>
Contact us at <%= email_link %> if there is anything more we can help with. 😊
</p>
<p>
Thanks,
<br>
The <%= community_name %> Team
</p>

View file

@ -0,0 +1,8 @@
Hi <%= @name %>.
Your organization <%= @org_name %> on <%= community_name %> has been successfully deleted.
Contact us at <%= email_link %> if there is anything more we can help with.
Thanks,
The <%= community_name %> Team

View file

@ -0,0 +1,44 @@
module Organizations
class DeleteWorker
include Sidekiq::Worker
sidekiq_options queue: :high_priority, retry: 10
# operator_id - the user who deletes the organization
def perform(organization_id, operator_id)
org = Organization.find_by(id: organization_id)
return unless org
user = User.find_by(id: operator_id)
return unless user
Organizations::Delete.call(org)
user.touch(:organization_info_updated_at)
EdgeCache::BustUser.call(user)
# notify user that the org was deleted
NotifyMailer.with(name: user.name, org_name: org.name, email: user.email).organization_deleted_email.deliver_now
audit_log(org, user)
rescue StandardError => e
ForemStatsClient.count("organizations.delete", 1,
tags: ["action:failed", "organization_id:#{org.id}", "user_id:#{user.id}"])
Honeybadger.context({ organization_id: org.id, user_id: user.id })
Honeybadger.notify(e)
end
def audit_log(org, user)
AuditLog.create(
user: user,
category: "user.organization.delete",
roles: user.roles_name,
slug: "organization_delete",
data: {
organization_id: org.id,
organization_slug: org.slug
},
)
end
end
end

View file

@ -4,7 +4,9 @@ RSpec.describe NotifyMailer, type: :mailer do
let(:user) { create(:user) }
let(:user2) { create(:user) }
let(:article) { create(:article, user_id: user.id) }
let(:comment) { create(:comment, user_id: user.id, commentable: article) }
let(:organization) { create(:organization) }
let(:organization_membership) { create(:organization_membership, user: user, organization: organization) }
let(:comment) { create(:comment, user_id: user.id, commentable: article) }
describe "#new_reply_email" do
let(:email) { described_class.with(comment: comment).new_reply_email }
@ -376,6 +378,26 @@ RSpec.describe NotifyMailer, type: :mailer do
end
end
describe "#organization_deleted_email" do
let(:email) do
described_class.with(name: user.name, email: user.email, org_name: organization.name).organization_deleted_email
end
it "renders proper subject" do
expect(email.subject).to eq("#{SiteConfig.community_name} - Organization Deletion Confirmation")
end
it "renders proper sender" do
expect(email.from).to eq([SiteConfig.email_addresses[:default]])
expected_from = "#{SiteConfig.community_name} <#{SiteConfig.email_addresses[:default]}>"
expect(email["from"].value).to eq(expected_from)
end
it "renders proper receiver" do
expect(email.to).to eq([user.email])
end
end
describe "#export_email" do
let(:email) { described_class.with(email: user.email, attachment: "attachment").export_email }

View file

@ -171,49 +171,75 @@ RSpec.describe "UserOrganization", type: :request do
let(:org_member) { create(:user, :org_member) }
let(:user) { create(:user) }
it "deletes the organization" do
org_id = org_admin.organizations.first.id
sign_in org_admin
delete "/organizations/#{org_id}"
expect { Organization.find(org_id) }.to raise_error(ActiveRecord::RecordNotFound)
context "when signed in as org_admin" do
let(:org) { org_admin.organizations.first }
let(:org_id) { org_admin.organizations.first.id }
before do
sign_in org_admin
end
it "deletes the organization" do
sidekiq_assert_enqueued_with(job: Organizations::DeleteWorker, args: [org_id, org_admin.id]) do
delete organization_path(org_id)
end
end
it "does not delete the organization if the organization has an article associated to it" do
create(:article, user: org_admin, organization_id: org_id)
sidekiq_assert_not_enqueued_with(job: Organizations::DeleteWorker) do
delete organization_path(org_id)
end
end
it "does not delete the organization if the organization has more than one member" do
create(:organization_membership, user: user, organization_id: org_id, type_of_user: "member")
sidekiq_assert_not_enqueued_with(job: Organizations::DeleteWorker) do
delete organization_path(org_id)
end
end
it "does not delete the organization if the organization has credits" do
Credit.add_to(org, 1)
sidekiq_assert_not_enqueued_with(job: Organizations::DeleteWorker) do
delete organization_path(org_id)
end
end
it "has the correct flash after deleting an org" do
delete organization_path(org_id)
notice_text = "Your organization: \"#{org.name}\" deletion is scheduled. You'll be notified when it's deleted."
expect(flash[:settings_notice]).to include(notice_text)
end
it "redirects after scheduling deleting an org" do
delete organization_path(org_id)
expect(response).to redirect_to(user_settings_path(:organization))
end
end
it "does not delete the organization if the user is only an org member" do
org_id = org_member.organizations.first.id
sign_in org_member
delete "/organizations/#{org_id}"
expect(Organization.find(org_id).persisted?).to eq true
sidekiq_assert_not_enqueued_with(job: Organizations::DeleteWorker) do
delete organization_path(org_id)
end
end
it "does not delete the organization if the user is not a part of the org" do
org = create(:organization)
sign_in user
delete "/organizations/#{org.id}"
expect(org.persisted?).to eq true
sidekiq_assert_not_enqueued_with(job: Organizations::DeleteWorker) do
delete organization_path(org.id)
end
end
it "does not delete the organization if the organization has an article associated to it" do
org_id = org_admin.organizations.first.id
create(:article, user: org_admin, organization_id: org_id)
sign_in org_admin
delete "/organizations/#{org_id}"
expect(Organization.find(org_id).persisted?).to eq true
end
it "does not delete the organization if the organization has more than one member" do
org_id = org_admin.organizations.first.id
create(:organization_membership, user: user, organization_id: org_id, type_of_user: "member")
sign_in org_admin
delete "/organizations/#{org_id}"
expect(Organization.find(org_id).persisted?).to eq true
end
it "does not delete the organization if the organization has credits" do
org = org_admin.organizations.first
sign_in org_admin
Credit.add_to(org, 1)
delete "/organizations/#{org.id}"
expect(org.persisted?).to eq true
it "redirects correctly when not scheduling" do
org_id = org_member.organizations.first.id
sign_in org_member
delete organization_path(org_id)
expect(flash[:error]).to include("Your organization was not deleted")
expect(response).to redirect_to(user_settings_path(:organization, id: org_id))
end
end
end

View file

@ -0,0 +1,10 @@
require "rails_helper"
RSpec.describe Organizations::Delete, type: :service do
let(:org) { create(:organization) }
it "deletes an organization" do
described_class.call(org)
expect(Organization.find_by(id: org.id)).to be_nil
end
end

View file

@ -0,0 +1,92 @@
require "rails_helper"
RSpec.describe Organizations::DeleteWorker, type: :worker do
let(:worker) { subject }
describe "#perform" do
let!(:org) { create(:organization) }
let!(:user) { create(:user) }
let(:delete) { Organizations::Delete }
let(:mailer_class) { NotifyMailer }
let(:mailer) { double }
let(:message_delivery) { double }
context "when org and user are found" do
it "destroys the org" do
worker.perform(org.id, user.id)
expect(Organization.exists?(id: org.id)).to be(false)
end
it "calls the service" do
allow(delete).to receive(:call)
worker.perform(org.id, user.id)
expect(delete).to have_received(:call).with(org)
end
it "touches the user" do
allow(User).to receive(:find_by) { user }
allow(user).to receive(:touch)
worker.perform(org.id, user.id)
expect(user).to have_received(:touch).with(:organization_info_updated_at)
end
it "busts user's cache" do
bust_cache = EdgeCache::BustUser
allow(bust_cache).to receive(:call)
worker.perform(org.id, user.id)
expect(bust_cache).to have_received(:call).with(user)
end
it "sends the notification" do
expect do
worker.perform(org.id, user.id)
end.to change(ActionMailer::Base.deliveries, :count).by(1)
end
it "sends the correct notification" do
allow(mailer_class).to receive(:with).and_return(mailer)
allow(mailer).to receive(:organization_deleted_email).and_return(message_delivery)
allow(message_delivery).to receive(:deliver_now)
worker.perform(org.id, user.id)
expect(mailer_class).to have_received(:with).with(org_name: org.name, name: user.name, email: user.email)
expect(mailer).to have_received(:organization_deleted_email)
expect(message_delivery).to have_received(:deliver_now)
end
it "creates an audit_log record" do
expect do
worker.perform(org.id, user.id)
end.to change(AuditLog, :count).by(1)
end
it "creates a correct AuditLog record" do
worker.perform(org.id, user.id)
audit_log = AuditLog.find_by(category: "user.organization.delete", slug: "organization_delete",
user_id: user.id)
expect(audit_log).to be_present
expect(audit_log.data["organization_id"]).to eq(org.id)
expect(audit_log.data["organization_slug"]).to eq(org.slug)
end
end
context "when an org or a user is not found" do
it "doesn't fail" do
worker.perform(-1, -1)
end
it "doesn't call org delete when a user was not found" do
allow(delete).to receive(:call)
worker.perform(org.id, -1)
expect(delete).not_to have_received(:call)
end
it "doesn't call org delete when an org was not found" do
allow(delete).to receive(:call)
worker.perform(-1, org.id)
expect(delete).not_to have_received(:call)
end
end
end
end