From 763ab0c7eb5430235ca8a354d11e00de1d8ba6dd Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 8 Aug 2021 15:29:57 +0200 Subject: [PATCH] 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 --- app/models/concerns/account_interactions.rb | 3 +++ app/services/delete_account_service.rb | 2 ++ ...0808071221_clear_orphaned_account_notes.rb | 21 +++++++++++++++++++ db/schema.rb | 2 +- spec/services/delete_account_service_spec.rb | 5 ++++- 5 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 db/post_migrate/20210808071221_clear_orphaned_account_notes.rb diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index 958f6c78e2..4bf62539ce 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -81,6 +81,9 @@ module AccountInteractions 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 + # Account notes + has_many :account_notes, dependent: :destroy + # Block relationships 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 diff --git a/app/services/delete_account_service.rb b/app/services/delete_account_service.rb index 182f0e1277..d8270498a9 100644 --- a/app/services/delete_account_service.rb +++ b/app/services/delete_account_service.rb @@ -4,6 +4,7 @@ class DeleteAccountService < BaseService include Payloadable ASSOCIATIONS_ON_SUSPEND = %w( + account_notes account_pins active_relationships aliases @@ -34,6 +35,7 @@ class DeleteAccountService < BaseService # by foreign keys, making them safe to delete without loading # into memory ASSOCIATIONS_WITHOUT_SIDE_EFFECTS = %w( + account_notes account_pins aliases conversation_mutes diff --git a/db/post_migrate/20210808071221_clear_orphaned_account_notes.rb b/db/post_migrate/20210808071221_clear_orphaned_account_notes.rb new file mode 100644 index 0000000000..71171658af --- /dev/null +++ b/db/post_migrate/20210808071221_clear_orphaned_account_notes.rb @@ -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 diff --git a/db/schema.rb b/db/schema.rb index b2929f693c..a0a98eb03e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # 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 enable_extension "plpgsql" diff --git a/spec/services/delete_account_service_spec.rb b/spec/services/delete_account_service_spec.rb index cd7d32d592..b1da970366 100644 --- a/spec/services/delete_account_service_spec.rb +++ b/spec/services/delete_account_service_spec.rb @@ -21,6 +21,8 @@ RSpec.describe DeleteAccountService, type: :service do 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!(:account_note) { Fabricate(:account_note, account: account) } + subject do -> { described_class.new.call(account) } end @@ -35,8 +37,9 @@ RSpec.describe DeleteAccountService, type: :service do account.active_relationships, account.passive_relationships, account.polls, + account.account_notes, ].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 it 'deletes associated target records' do