From 7af061fafa67bd332b4063a70ec34703df51431c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Thu, 19 Nov 2020 14:09:56 +0100
Subject: [PATCH] DEV: raise an exception when trying to create a post within a
 transaction (#11287)

Hopefully this will prevent fellow developers from spending a morning
trying to figure out why the jobs that are run after a post is created
are not finding the post in the database.

That's because if you're inside a transaction when creating a post, the jobs
will likely run before the transaction is being commited to the database.

There was a "warning" in the comments of the PostCreator class but raising an
exception should hopefully help catch those issue much faster :fingers_crossed:.

This is hard to test since all the tests are run in a transaction...
---
 lib/post_creator.rb | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/lib/post_creator.rb b/lib/post_creator.rb
index eab06f329c6..ab3ad240f21 100644
--- a/lib/post_creator.rb
+++ b/lib/post_creator.rb
@@ -59,21 +59,20 @@ class PostCreator
     # TODO: we should reload user in case it is tainted, should take in a user_id as opposed to user
     # If we don't do this we introduce a rather risky dependency
     @user = user
-    @opts = opts || {}
-    opts[:title] = pg_clean_up(opts[:title]) if opts[:title] && opts[:title].include?("\u0000")
-    opts[:raw] = pg_clean_up(opts[:raw]) if opts[:raw] && opts[:raw].include?("\u0000")
-    opts.delete(:reply_to_post_number) unless opts[:topic_id]
-    opts[:visible] = false if opts[:visible].nil? && opts[:hidden_reason_id].present?
-    @guardian = opts[:guardian] if opts[:guardian]
-
     @spam = false
+    @opts = opts || {}
+
+    opts[:title] = pg_clean_up(opts[:title]) if opts[:title]&.include?("\u0000")
+    opts[:raw] = pg_clean_up(opts[:raw]) if opts[:raw]&.include?("\u0000")
+    opts[:visible] = false if opts[:visible].nil? && opts[:hidden_reason_id].present?
+
+    opts.delete(:reply_to_post_number) unless opts[:topic_id]
   end
 
   def pg_clean_up(str)
     str.gsub("\u0000", "")
   end
 
-  # True if the post was considered spam
   def spam?
     @spam
   end
@@ -83,7 +82,7 @@ class PostCreator
   end
 
   def guardian
-    @guardian ||= Guardian.new(@user)
+    @guardian ||= @opts[:guardian] || Guardian.new(@user)
   end
 
   def valid?
@@ -200,6 +199,10 @@ class PostCreator
   end
 
   def create
+    if !Rails.env.test? && ActiveRecord::Base.connection.open_transactions > 0 && !@opts[:skip_jobs]
+      raise "You must use 'skip_jobs = true' when creating a post inside a transaction, otherwise jobs won't run properly."
+    end
+
     if valid?
       transaction do
         build_post_stats