Revert "Revert "Fix :after_commit within nested transaction""

This reverts commit 59ebeca.
aasm · anilmaurya · May 12, 2020 · Mar 5, 2020 · Mar 5, 2020 · Mar 22, 2020
commit 838428c6e459bbeb57fc637e651080115b2b0941
diff --git a/aasm.gemspec b/aasm.gemspec
@@ -17,6 +17,7 @@ Gem::Specification.new do |s|
   s.required_ruby_version = '>= 1.9.3'
 
   s.add_dependency 'concurrent-ruby', '~> 1.0'
+  s.add_dependency 'after_commit_action', '~> 1.0'
 
   s.add_development_dependency 'rake'
   s.add_development_dependency 'sdoc'

diff --git a/lib/aasm/persistence/active_record_persistence.rb b/lib/aasm/persistence/active_record_persistence.rb
@@ -1,3 +1,4 @@
+require 'after_commit_action'
 require 'aasm/persistence/orm'
 module AASM
   module Persistence
@@ -28,6 +29,7 @@ module ActiveRecordPersistence
       #   end
       #
       def self.included(base)
+        base.send(:include, ::AfterCommitAction) unless base.include?(::AfterCommitAction)
         base.send(:include, AASM::Persistence::Base)
         base.send(:include, AASM::Persistence::ORM)
         base.send(:include, AASM::Persistence::ActiveRecordPersistence::InstanceMethods)
@@ -88,6 +90,12 @@ def aasm_transaction(requires_new, requires_lock)
           end
         end
 
+        def aasm_execute_after_commit
+          execute_after_commit do
+            yield
+          end
+        end
+
         def aasm_enum(name=:default)
           case AASM::StateMachineStore.fetch(self.class, true).machine(name).config.enum
           when false then nil

diff --git a/lib/aasm/persistence/orm.rb b/lib/aasm/persistence/orm.rb
@@ -81,6 +81,10 @@ def aasm_supports_transactions?
         true
       end
 
+      def aasm_execute_after_commit
+        yield
+      end
+
       def aasm_write_state_attribute(state, name=:default)
         aasm_write_attribute(self.class.aasm(name).attribute_name, aasm_raw_attribute_value(state, name))
       end
@@ -116,32 +120,32 @@ def requires_lock?(state_machine_name)
 
       # Returns true if event was fired successfully and transaction completed.
       def aasm_fire_event(state_machine_name, name, options, *args, &block)
-        if aasm_supports_transactions? && options[:persist]
-          event = self.class.aasm(state_machine_name).state_machine.events[name]
-          event.fire_callbacks(:before_transaction, self, *args)
-          event.fire_global_callbacks(:before_all_transactions, self, *args)
-
-          begin
-            success = if options[:persist] && use_transactions?(state_machine_name)
-              aasm_transaction(requires_new?(state_machine_name), requires_lock?(state_machine_name)) do
-                super
-              end
-            else
+        return super unless aasm_supports_transactions? && options[:persist]
+
+        event = self.class.aasm(state_machine_name).state_machine.events[name]
+        event.fire_callbacks(:before_transaction, self, *args)
+        event.fire_global_callbacks(:before_all_transactions, self, *args)
+
+        begin
+          success = if options[:persist] && use_transactions?(state_machine_name)
+            aasm_transaction(requires_new?(state_machine_name), requires_lock?(state_machine_name)) do
               super
             end
+          else
+            super
+          end
 
-            if success
+          if success
+            aasm_execute_after_commit do
               event.fire_callbacks(:after_commit, self, *args)
               event.fire_global_callbacks(:after_all_commits, self, *args)
             end
-
-            success
-          ensure
-            event.fire_callbacks(:after_transaction, self, *args)
-            event.fire_global_callbacks(:after_all_transactions, self, *args)
           end
-        else
-          super
+
+          success
+        ensure
+          event.fire_callbacks(:after_transaction, self, *args)
+          event.fire_global_callbacks(:after_all_transactions, self, *args)
         end
       end
 

diff --git a/spec/unit/persistence/active_record_persistence_spec.rb b/spec/unit/persistence/active_record_persistence_spec.rb
@@ -613,6 +613,38 @@
           expect(validator).to be_running
           expect(validator.name).to eq("name")
         end
+
+        context "nested transaction" do
+          it "should fire :after_commit if root transaction was successful" do
+            validator = Validator.create(:name => 'name')
+            expect(validator).to be_sleeping
+
+            validator.transaction do
+              validator.run!
+              expect(validator.name).to eq("name")
+              expect(validator).to be_running
+            end
+
+            expect(validator.name).to eq("name changed")
+            expect(validator.reload).to be_running
+          end
+        end
+
+        it "should not fire :after_commit if root transaction failed" do
+          validator = Validator.create(:name => 'name')
+          expect(validator).to be_sleeping
+
+          validator.transaction do
+            validator.run!
+            expect(validator.name).to eq("name")
+            expect(validator).to be_running
+
+            raise ActiveRecord::Rollback, "failed on purpose"
+          end
+
+          expect(validator.name).to eq("name")
+          expect(validator.reload).to be_sleeping
+        end
       end
 
       describe 'before and after transaction callbacks' do