Fix owned account notes not being deleted when an account is deleted (#16579)

* Add account_notes relationship

* Add tests

* Fix owned account notes not being deleted when an account is deleted

* Add post-migration to clean up orphaned account notes
This commit is contained in:
Claire 2021-08-08 15:29:57 +02:00 committed by GitHub
parent 818e0b314f
commit 763ab0c7eb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 31 additions and 2 deletions

View file

@ -81,6 +81,9 @@ module AccountInteractions
has_many :following, -> { order('follows.id desc') }, through: :active_relationships, source: :target_account has_many :following, -> { order('follows.id desc') }, through: :active_relationships, source: :target_account
has_many :followers, -> { order('follows.id desc') }, through: :passive_relationships, source: :account has_many :followers, -> { order('follows.id desc') }, through: :passive_relationships, source: :account
# Account notes
has_many :account_notes, dependent: :destroy
# Block relationships # Block relationships
has_many :block_relationships, class_name: 'Block', foreign_key: 'account_id', dependent: :destroy has_many :block_relationships, class_name: 'Block', foreign_key: 'account_id', dependent: :destroy
has_many :blocking, -> { order('blocks.id desc') }, through: :block_relationships, source: :target_account has_many :blocking, -> { order('blocks.id desc') }, through: :block_relationships, source: :target_account

View file

@ -4,6 +4,7 @@ class DeleteAccountService < BaseService
include Payloadable include Payloadable
ASSOCIATIONS_ON_SUSPEND = %w( ASSOCIATIONS_ON_SUSPEND = %w(
account_notes
account_pins account_pins
active_relationships active_relationships
aliases aliases
@ -34,6 +35,7 @@ class DeleteAccountService < BaseService
# by foreign keys, making them safe to delete without loading # by foreign keys, making them safe to delete without loading
# into memory # into memory
ASSOCIATIONS_WITHOUT_SIDE_EFFECTS = %w( ASSOCIATIONS_WITHOUT_SIDE_EFFECTS = %w(
account_notes
account_pins account_pins
aliases aliases
conversation_mutes conversation_mutes

View file

@ -0,0 +1,21 @@
# frozen_string_literal: true
class ClearOrphanedAccountNotes < ActiveRecord::Migration[5.2]
class Account < ApplicationRecord
# Dummy class, to make migration possible across version changes
end
class AccountNote < ApplicationRecord
# Dummy class, to make migration possible across version changes
belongs_to :account
belongs_to :target_account, class_name: 'Account'
end
def up
AccountNote.where('NOT EXISTS (SELECT * FROM users u WHERE u.account_id = account_notes.account_id)').in_batches.delete_all
end
def down
# nothing to do
end
end

View file

@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2021_06_30_000137) do ActiveRecord::Schema.define(version: 2021_08_08_071221) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"

View file

@ -21,6 +21,8 @@ RSpec.describe DeleteAccountService, type: :service do
let!(:favourite_notification) { Fabricate(:notification, account: local_follower, activity: favourite, type: :favourite) } let!(:favourite_notification) { Fabricate(:notification, account: local_follower, activity: favourite, type: :favourite) }
let!(:follow_notification) { Fabricate(:notification, account: local_follower, activity: active_relationship, type: :follow) } let!(:follow_notification) { Fabricate(:notification, account: local_follower, activity: active_relationship, type: :follow) }
let!(:account_note) { Fabricate(:account_note, account: account) }
subject do subject do
-> { described_class.new.call(account) } -> { described_class.new.call(account) }
end end
@ -35,8 +37,9 @@ RSpec.describe DeleteAccountService, type: :service do
account.active_relationships, account.active_relationships,
account.passive_relationships, account.passive_relationships,
account.polls, account.polls,
account.account_notes,
].map(&:count) ].map(&:count)
}.from([2, 1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0, 0]) }.from([2, 1, 1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0, 0, 0])
end end
it 'deletes associated target records' do it 'deletes associated target records' do