FIX: properly clean out post and user actions on destroy user

This corrects 2 issues:

First is a regression with d7c08e21 for some reason dependent :delete_all
respects default scopes where-as dependent :destroy bypasses it.

Secondly, we were keeping orphan user actions around on user destroy, this
ensures we remove all the user actions not only ones that originated by
the user.

So for example: if I like a post of user A we create a user action saying I
did that, but once user A is deleted we were not removing the action leading
to an orphan action in the database.
This commit is contained in:
Sam 2019-01-17 12:40:30 +11:00
parent d7c08e217a
commit d5ecf8e8c2
2 changed files with 31 additions and 6 deletions

View File

@ -34,12 +34,9 @@ class User < ActiveRecord::Base
has_many :topics
has_many :user_open_ids, dependent: :destroy
# Do Not Change to user_actions/post_actions to dependent: destroy
# users can have lots of actions, bypass tracking of all destroyed objects
# this means there is a much higher likelihood that users with lots of state
# can be destroyed.
has_many :user_actions, dependent: :delete_all
has_many :post_actions, dependent: :delete_all
# dependent deleting handled via before_destroy
has_many :user_actions
has_many :post_actions
has_many :user_badges, -> { where('user_badges.badge_id IN (SELECT id FROM badges WHERE enabled)') }, dependent: :destroy
has_many :badges, through: :user_badges
@ -136,6 +133,11 @@ class User < ActiveRecord::Base
# These tables don't have primary keys, so destroying them with activerecord is tricky:
PostTiming.where(user_id: self.id).delete_all
TopicViewItem.where(user_id: self.id).delete_all
UserAction.where('user_id = :user_id OR target_user_id = :user_id OR acting_user_id = :user_id', user_id: self.id).delete_all
# we need to bypass the default scope here, which appears not bypassed for :delete_all
# however :destroy it is bypassed
PostAction.with_deleted.where(user_id: self.id).delete_all
end
# Skip validating email, for example from a particular auth provider plugin

View File

@ -1966,4 +1966,27 @@ describe User do
end
end
end
context "#destroy!" do
it 'clears up associated data on destroy!' do
user = Fabricate(:user)
post = Fabricate(:post)
PostAction.act(user, post, PostActionType.types[:like])
PostAction.remove_act(user, post, PostActionType.types[:like])
UserAction.create!(user_id: user.id, action_type: UserAction::LIKE)
UserAction.create!(user_id: -1, action_type: UserAction::LIKE, target_user_id: user.id)
UserAction.create!(user_id: -1, action_type: UserAction::LIKE, acting_user_id: user.id)
user.reload
user.destroy!
expect(UserAction.where(user_id: user.id).length).to eq(0)
expect(UserAction.where(target_user_id: user.id).length).to eq(0)
expect(UserAction.where(acting_user_id: user.id).length).to eq(0)
expect(PostAction.with_deleted.where(user_id: user.id).length).to eq(0)
end
end
end