DEV: Delete upload references on draft cleanup (#26877)

In #22851 we added a dependent strategy for deleting upload references when a draft is destroyed. This, however, didn't catch all cases, because we still have some code that issues DELETE drafts queries directly to the database. Specifically in the weekly cleanup job handled by Draft#cleanup!.

This PR fixes that by turning the raw query into an ActiveRecord #destroy_all, which will invoke the dependent strategy that ultimately deletes the upload references. It also includes a post migration to clear orphaned upload references that are already in the database.
This commit is contained in:
Ted Johansson 2024-05-06 14:08:10 +08:00 committed by GitHub
parent 74f1a79d36
commit 9655bf3e24
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 32 additions and 6 deletions

View File

@ -237,19 +237,18 @@ class Draft < ActiveRecord::Base
end
def self.cleanup!
DB.exec(<<~SQL)
DELETE FROM drafts
WHERE sequence < (
Draft.where(<<~SQL).in_batches(of: 100).destroy_all
sequence < (
SELECT MAX(s.sequence)
FROM draft_sequences s
WHERE s.draft_key = drafts.draft_key
AND s.user_id = drafts.user_id
WHERE s.draft_key = drafts.draft_key
AND s.user_id = drafts.user_id
)
SQL
# remove old drafts
delete_drafts_older_than_n_days = SiteSetting.delete_drafts_older_than_n_days.days.ago
Draft.where("updated_at < ?", delete_drafts_older_than_n_days).destroy_all
Draft.where("updated_at < ?", delete_drafts_older_than_n_days).in_batches(of: 100).destroy_all
UserStat.update_draft_count
end

View File

@ -0,0 +1,20 @@
# frozen_string_literal: true
class ClearOrphanedDraftUploadReferences < ActiveRecord::Migration[7.0]
def up
execute <<~SQL
DELETE
FROM
"upload_references"
WHERE
"upload_references"."target_type" = 'Draft' AND
"upload_references"."target_id" NOT IN (
SELECT "drafts"."id" FROM "drafts"
)
SQL
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -133,6 +133,7 @@ RSpec.describe Draft do
key = Draft::NEW_TOPIC
Draft.set(user, key, 0, "draft")
Draft.cleanup!
expect(Draft.count).to eq 1
expect(user.user_stat.draft_count).to eq(1)
@ -142,10 +143,16 @@ RSpec.describe Draft do
Draft.set(user, key, seq, "draft")
DraftSequence.update_all("sequence = sequence + 1")
draft = Draft.first
draft.upload_references << UploadReference.create!(target: draft, upload: Fabricate(:upload))
expect(UploadReference.count).to eq(1)
Draft.cleanup!
expect(Draft.count).to eq 0
expect(user.reload.user_stat.draft_count).to eq(0)
expect(UploadReference.count).to eq(0)
Draft.set(Fabricate(:user), Draft::NEW_TOPIC, 0, "draft")