From 10ddb8a9c4bea55ea4f921b893c42635e79c6b70 Mon Sep 17 00:00:00 2001 From: Kane York Date: Thu, 5 Mar 2020 10:51:51 -0800 Subject: [PATCH] FIX: Use destroy_all instead of delete_all for shared drafts Rails has an odd behavior for calling .delete_all on a has_many relation - the default behavior is to nullify the foreign key fields instead of actually 'DELETE'ing the records. Additionally, publishing a shared draft topic creates a PostRevision that the NotifyPostRevision job picks up which is then promptly deleted. Use destroy_all when cleaning up the revisions and have the NotifyPostRevision job tolerate deleted PostRevision records. This takes a small performance hit (several SQL DELETEs instead of just one) but shouldn't be too much of an issue (high cardinalities range from 30-100). --- app/jobs/regular/notify_post_revision.rb | 2 +- lib/topic_publisher.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/jobs/regular/notify_post_revision.rb b/app/jobs/regular/notify_post_revision.rb index 8d2160699c0..f4b0262e06a 100644 --- a/app/jobs/regular/notify_post_revision.rb +++ b/app/jobs/regular/notify_post_revision.rb @@ -6,7 +6,7 @@ module Jobs raise Discourse::InvalidParameters.new(:user_ids) unless args[:user_ids] post_revision = PostRevision.find_by(id: args[:post_revision_id]) - raise Discourse::InvalidParameters.new(:post_revision_id) unless post_revision + return if post_revision.nil? ActiveRecord::Base.transaction do User.where(id: args[:user_ids]).find_each do |user| diff --git a/lib/topic_publisher.rb b/lib/topic_publisher.rb index b78b58bd11f..63afb8dede5 100644 --- a/lib/topic_publisher.rb +++ b/lib/topic_publisher.rb @@ -36,7 +36,7 @@ class TopicPublisher op = @topic.first_post if op.present? - op.revisions.delete_all + op.revisions.destroy_all op.update_columns( version: 1,