Ensuring we delete podcasts when we banish a user (#18063)

Prior to this commit, we may have left lingering podcast entries that
required manual cleanup.  As a bonus, I also added deleting podcasts
when we delete users.

Closes forem/forem#18041

In addition I adjusted rubocop to not complain about the require method call

```
spec/services/users/delete_spec.rb:1:1: C: \
  Style/TopLevelMethodDefinition: \
  Do not define methods at the top-level. \
  (https://rubystyle.guide#top-level-methods)
require "rails_helper"
^^^^^^^^^^^^^^^^^^^^^^
```
This commit is contained in:
Jeremy Friesen 2022-07-12 09:18:06 -04:00 committed by GitHub
parent 9e9c3e5f39
commit 0ef5bc6351
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 95 additions and 0 deletions

View file

@ -676,6 +676,7 @@ Style/TopLevelMethodDefinition:
Exclude:
- scripts/release
- scripts/stage_release
- 'spec/**/*_spec.rb'
Style/TrailingCommaInArguments:
Description: 'Checks for trailing comma in argument lists.'

View file

@ -21,6 +21,7 @@ module Moderator
delete_user_activity
delete_comments
delete_articles
delete_user_podcasts
reassign_and_bust_username
delete_vomit_reactions
end

View file

@ -24,6 +24,10 @@ module Moderator
Users::DeleteActivity.call(user)
end
def delete_user_podcasts
Users::DeletePodcasts.call(user)
end
def remove_privileges
@user.remove_role(:workshop_pass)
remove_mod_roles

View file

@ -7,6 +7,7 @@ module Users
def call
delete_comments
delete_articles
delete_podcasts
delete_user_activity
user.remove_from_mailchimp_newsletters
EdgeCache::Bust.call("/#{user.username}")
@ -34,5 +35,9 @@ module Users
def delete_articles
DeleteArticles.call(user)
end
def delete_podcasts
DeletePodcasts.call(user)
end
end
end

View file

@ -0,0 +1,31 @@
module Users
module DeletePodcasts
def self.call(user)
return unless user
# We'll want to bust the podcast paths that match our criteria.
paths = []
user.podcast_ownerships.includes(:podcast).find_each do |ownership|
ownership.destroy
podcast = ownership.podcast
# Guard against ownership without podcast
next if podcast.blank?
# We have another owner, don't delete the podcast.
next if PodcastOwnership.where(podcast: podcast).where.not(user_id: user.id).exists?
# No sense keeping the roles around.
Role.where(resource: podcast).destroy_all
paths << podcast.path if podcast.path.present?
podcast.destroy
end
paths.each do |path|
EdgeCache::BustPodcast.call(path)
end
end
end
end

View file

@ -46,6 +46,16 @@ RSpec.describe Moderator::BanishUser, type: :service do
expect(user.comments.count).to eq 0
end
it "removes all their podcasts" do
podcast = create(:podcast, creator: user)
create(:podcast_ownership, owner: user, podcast: podcast)
expect do
sidekiq_perform_enqueued_jobs do
described_class.call(user: user, admin: admin)
end
end.to change(Podcast, :count).by(-1)
end
it "creates a BanishedUser record with their original username" do
original_username = user.username
sidekiq_perform_enqueued_jobs do

View file

@ -0,0 +1,35 @@
require "rails_helper"
RSpec.describe Users::DeletePodcasts do
let!(:user) { create(:user) }
let!(:podcast) { create(:podcast, creator: user) }
before do
create(:podcast_ownership, owner: user, podcast: podcast)
end
context "when podcast is owned by multiple users" do
before do
other_user = create(:user)
create(:podcast_ownership, owner: other_user, podcast: podcast)
end
it "only removes ownership from the given user", :aggregate_failures do
expect do
expect do
described_class.call(user)
end.not_to change(Podcast, :count)
end.to change(PodcastOwnership, :count).by(-1)
end
end
context "when podcast is owned by one user" do
it "removes ownership from the given user and deletes the podcast", :aggregate_failures do
expect do
expect do
described_class.call(user)
end.to change(Podcast, :count).by(-1)
end.to change(PodcastOwnership, :count).by(-1)
end
end
end

View file

@ -44,6 +44,14 @@ RSpec.describe Users::Delete, type: :service do
expect(Article.find_by(id: article.id)).to be_nil
end
it "deletes user's owned podcasts" do
podcast = create(:podcast, creator: user)
create(:podcast_ownership, owner: user, podcast: podcast)
expect do
described_class.call(user)
end.to change(Podcast, :count).by(-1)
end
it "deletes the destroy token" do
allow(Rails.cache).to receive(:delete).and_call_original
described_class.call(user)