diff --git a/app/controllers/organizations_controller.rb b/app/controllers/organizations_controller.rb index e08004088..d0e0f561a 100644 --- a/app/controllers/organizations_controller.rb +++ b/app/controllers/organizations_controller.rb @@ -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." diff --git a/app/mailers/notify_mailer.rb b/app/mailers/notify_mailer.rb index d91ee9da4..ac2293b13 100644 --- a/app/mailers/notify_mailer.rb +++ b/app/mailers/notify_mailer.rb @@ -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 diff --git a/app/models/organization.rb b/app/models/organization.rb index 49c059e84..5ab97ddb7 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -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 diff --git a/app/services/organizations/delete.rb b/app/services/organizations/delete.rb new file mode 100644 index 000000000..3999c2e9e --- /dev/null +++ b/app/services/organizations/delete.rb @@ -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 diff --git a/app/views/mailers/notify_mailer/organization_deleted_email.html.erb b/app/views/mailers/notify_mailer/organization_deleted_email.html.erb new file mode 100644 index 000000000..aa406b3df --- /dev/null +++ b/app/views/mailers/notify_mailer/organization_deleted_email.html.erb @@ -0,0 +1,17 @@ +

+ 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 +

diff --git a/app/views/mailers/notify_mailer/organization_deleted_email.text.erb b/app/views/mailers/notify_mailer/organization_deleted_email.text.erb new file mode 100644 index 000000000..03150fcd8 --- /dev/null +++ b/app/views/mailers/notify_mailer/organization_deleted_email.text.erb @@ -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 diff --git a/app/workers/organizations/delete_worker.rb b/app/workers/organizations/delete_worker.rb new file mode 100644 index 000000000..dba4cf6ec --- /dev/null +++ b/app/workers/organizations/delete_worker.rb @@ -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 diff --git a/spec/mailers/notify_mailer_spec.rb b/spec/mailers/notify_mailer_spec.rb index eac31a57d..9ed6b6f47 100644 --- a/spec/mailers/notify_mailer_spec.rb +++ b/spec/mailers/notify_mailer_spec.rb @@ -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 } diff --git a/spec/requests/user/user_organization_spec.rb b/spec/requests/user/user_organization_spec.rb index d843c08de..a88b531ca 100644 --- a/spec/requests/user/user_organization_spec.rb +++ b/spec/requests/user/user_organization_spec.rb @@ -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 diff --git a/spec/services/organizations/delete_spec.rb b/spec/services/organizations/delete_spec.rb new file mode 100644 index 000000000..8f00a510f --- /dev/null +++ b/spec/services/organizations/delete_spec.rb @@ -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 diff --git a/spec/workers/organizations/delete_worker_spec.rb b/spec/workers/organizations/delete_worker_spec.rb new file mode 100644 index 000000000..7ea2ab555 --- /dev/null +++ b/spec/workers/organizations/delete_worker_spec.rb @@ -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