Merge pull request #6108 from discourse/transaction-sidekiq-fix

Fix notifications for topics moved between categories

app/models/topic.rb

@@ -686,7 +686,9 @@ class Topic < ActiveRecord::Base
 
         if post = self.ordered_posts.first
           notified_user_ids = [post.user_id, post.last_editor_id].uniq
-          Jobs.enqueue(:notify_category_change, post_id: post.id, notified_user_ids: notified_user_ids)
+          DB.after_commit do
+            Jobs.enqueue(:notify_category_change, post_id: post.id, notified_user_ids: notified_user_ids)
+          end
         end
       end
 

lib/mini_sql_multisite_connection.rb

@@ -23,6 +23,27 @@ class MiniSqlMultisiteConnection < MiniSql::Connection
     end
   end
 
+  class AfterCommitWrapper
+    def initialize
+      @callback = Proc.new
+    end
+
+    def committed!(*)
+      @callback.call
+    end
+
+    def before_committed!(*); end
+    def rolledback!(*); end
+  end
+
+  # Allows running arbitrary code after the current transaction has been committed.
+  # Works even with nested transactions. Useful for scheduling sidekiq jobs.
+  def after_commit(&blk)
+    ActiveRecord::Base.connection.add_transaction_record(
+      AfterCommitWrapper.new(&blk)
+    )
+  end
+
   def self.instance
     new(nil, param_encoder: ParamEncoder.new)
   end

spec/lib/mini_sql_multisite_connection_spec.rb

@@ -0,0 +1,52 @@
+require 'rails_helper'
+
+describe MiniSqlMultisiteConnection do
+
+  describe "after_commit" do
+    it "runs callbacks after outermost transaction is committed" do
+      outputString = "1"
+
+      # Main transaction
+      ActiveRecord::Base.transaction do  
+        outputString += "2"
+
+          # Nested transaction
+          ActiveRecord::Base.transaction do
+            outputString += "3"
+
+              DB.after_commit do
+                outputString += "6"
+              end
+              outputString += "4"
+          end
+
+          DB.after_commit do
+            outputString += "7"
+          end
+
+          outputString += "5"
+      end
+
+      expect(outputString).to eq("1234567")
+    end
+
+    it "does not run if the transaction is rolled back" do
+      outputString = "1"
+
+      ActiveRecord::Base.transaction do
+        outputString += "2"
+
+        DB.after_commit do
+          outputString += "4"
+        end
+
+        outputString += "3"
+
+        raise ActiveRecord::Rollback
+      end
+
+      expect(outputString).to eq("123")
+    end
+  end
+
+end