From 421dad29009073e94b9b82f79d41250e8eef4628 Mon Sep 17 00:00:00 2001 From: Daniel Uber Date: Fri, 10 Dec 2021 11:24:23 -0600 Subject: [PATCH] Remove reactions to a user when deleting the user (#15724) * Remove reactions to a user when deleting the user Fixes #15245 --- I have no idea why `create(:vomit_reaction, reactable: user)` gave a validation error about category being invalid, but `build().save` worked fine. * Move relation details to reaction scope `for_user` Add a user method to access this by passing self to reaction scope * Create a moderator when flagging user in factory This might be a missing requirement in the reactions factory - but this suppresses an error about invalid category I was getting when setting a privileged reaction on the user via create(:vomit_reaction, reactable: user) Everywhere else it looks like we do this with `user: moderator` (where there's a trusted user in the surrounding context). * Move trusted user requirement into vomit_reaction factory * Add a data update script to remove reactions to deleted users This cleans up the remaining invalid references and should make the change in #15249 obsolete (i.e. we don't need to limit the view to exclude items that no longer exist). --- app/models/reaction.rb | 1 + app/models/user.rb | 4 ++++ app/services/users/delete_activity.rb | 1 + ...09213849_remove_dangling_user_reactions.rb | 11 +++++++++++ spec/factories/reactions.rb | 2 +- .../remove_dangling_user_reactions_spec.rb | 19 +++++++++++++++++++ spec/services/users/delete_spec.rb | 8 ++++++++ 7 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 lib/data_update_scripts/20211209213849_remove_dangling_user_reactions.rb create mode 100644 spec/lib/data_update_scripts/remove_dangling_user_reactions_spec.rb diff --git a/app/models/reaction.rb b/app/models/reaction.rb index 49b3a03ad..66445d6b5 100644 --- a/app/models/reaction.rb +++ b/app/models/reaction.rb @@ -42,6 +42,7 @@ class Reaction < ApplicationRecord .or(user_vomits.where(user_id: user.id)) } scope :privileged_category, -> { where(category: PRIVILEGED_CATEGORIES) } + scope :for_user, ->(user) { where(reactable: user) } validates :category, inclusion: { in: CATEGORIES } validates :reactable_type, inclusion: { in: REACTABLE_TYPES } diff --git a/app/models/user.rb b/app/models/user.rb index e476eb675..f08a99045 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -536,6 +536,10 @@ class User < ApplicationRecord notification_setting.email_follower_notifications end + def reactions_to + Reaction.for_user(self) + end + protected # Send emails asynchronously diff --git a/app/services/users/delete_activity.rb b/app/services/users/delete_activity.rb index 963e45fed..3041df770 100644 --- a/app/services/users/delete_activity.rb +++ b/app/services/users/delete_activity.rb @@ -37,6 +37,7 @@ module Users def delete_profile_info(user) user.notifications.delete_all user.reactions.delete_all + user.reactions_to.delete_all user.follows.delete_all Follow.followable_user(user.id).delete_all user.mentions.delete_all diff --git a/lib/data_update_scripts/20211209213849_remove_dangling_user_reactions.rb b/lib/data_update_scripts/20211209213849_remove_dangling_user_reactions.rb new file mode 100644 index 000000000..cb497f6a4 --- /dev/null +++ b/lib/data_update_scripts/20211209213849_remove_dangling_user_reactions.rb @@ -0,0 +1,11 @@ +module DataUpdateScripts + class RemoveDanglingUserReactions + def run + # reactions to users who have been deleted should be removed: + Reaction + .where(reactable_type: "User") + .where.not(reactable_id: User.all.select(:id)) + .delete_all + end + end +end diff --git a/spec/factories/reactions.rb b/spec/factories/reactions.rb index 3b796186a..955bf07cf 100644 --- a/spec/factories/reactions.rb +++ b/spec/factories/reactions.rb @@ -22,7 +22,7 @@ FactoryBot.define do end factory :vomit_reaction, class: "Reaction" do - user + user { create(:user, :trusted) } association :reactable, factory: :article category { "vomit" } diff --git a/spec/lib/data_update_scripts/remove_dangling_user_reactions_spec.rb b/spec/lib/data_update_scripts/remove_dangling_user_reactions_spec.rb new file mode 100644 index 000000000..d3b20a6e3 --- /dev/null +++ b/spec/lib/data_update_scripts/remove_dangling_user_reactions_spec.rb @@ -0,0 +1,19 @@ +require "rails_helper" +require Rails.root.join( + "lib/data_update_scripts/20211209213849_remove_dangling_user_reactions.rb", +) + +describe DataUpdateScripts::RemoveDanglingUserReactions do + let(:user_to_delete) { create(:user) } + + before do + create(:vomit_reaction, reactable: user_to_delete) + + # there's no dependent callback on reactions_to so the reaction is not removed + user_to_delete.destroy + end + + it "removes reactions to deleted users" do + expect { described_class.new.run }.to change(Reaction, :count).by(-1) + end +end diff --git a/spec/services/users/delete_spec.rb b/spec/services/users/delete_spec.rb index 0b357a11e..7d7e184d6 100644 --- a/spec/services/users/delete_spec.rb +++ b/spec/services/users/delete_spec.rb @@ -68,6 +68,14 @@ RSpec.describe Users::Delete, type: :service do end.to change(FieldTest::Membership, :count).by(-1) end + it "deletes reactions to the user" do + create(:vomit_reaction, reactable: user) + + expect do + described_class.call(user) + end.to change(Reaction, :count).by(-1) + end + # check that all the associated records are being destroyed, # except for those that are kept explicitly (kept_associations) describe "deleting associations" do