Create state changes in background job

In many state machines (order, payment and shipment)
we record state changes in the `spree_state_changes` table
as well as in the `OrderUpdater`.

This happens in the same transaction as the model change
itself. In the latest `state_machines` gem version (`> 0.30`)
these record creations cause validation errors and an
infinite validation loop.

Since the state changes are for debugging / analyse purposes
only and are not critical to the actual process of buying
we can easily background them.

In order to keep the correct timeline of events we make sure
that the time is stored on the event time and not background
job runtime.

Author: tvdeyen

core/app/jobs/spree/base_job.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+module Spree
+  # Base class for all Solidus background jobs
+  class BaseJob < ActiveJob::Base # rubocop:disable Rails/ApplicationJob
+    # Automatically retry jobs that encountered a deadlock
+    retry_on ActiveRecord::Deadlocked
+
+    # Most jobs are safe to ignore if the underlying records are no longer available
+    discard_on ActiveJob::DeserializationError
+  end
+end

core/app/jobs/spree/state_change_tracking_job.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module Spree
+  # Background job to track state changes asynchronously
+  # This avoids performance impact during checkout and prevents
+  # callback-related issues with recent versions of the state_machines gem.
+  class StateChangeTrackingJob < BaseJob
+    # @param stateful [GlobalId] The stateful object to track changes for
+    # @param previous_state [String] The previous state of the order
+    # @param current_state [String] The current state of the order
+    # @param transition_timestamp [Time] When the state transition occurred
+    def perform(stateful, previous_state, current_state, transition_timestamp)
+      Spree::StateChange.create!(
+        name: stateful.class.model_name.element,
+        stateful: stateful,
+        previous_state: previous_state,
+        next_state: current_state,
+        created_at: transition_timestamp,
+        updated_at: transition_timestamp,
+        user_id: stateful.try(:user_id) || stateful.try(:order)&.user_id
+      )
+    end
+  end
+end

core/app/models/concerns/spree/state_change_tracking.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+module Spree
+  module StateChangeTracking
+    extend ActiveSupport::Concern
+
+    included do
+      after_update :enqueue_state_change_tracking, if: :saved_change_to_state?
+    end
+
+    private
+
+    # Enqueue background job to track state changes asynchronously
+    def enqueue_state_change_tracking
+      previous_state, current_state = saved_changes['state']
+
+      # Enqueue the job to track this state change
+      StateChangeTrackingJob.perform_later(
+        self,
+        previous_state,
+        current_state,
+        Time.current
+      )
+    end
+  end
+end

core/app/models/spree/core/state_machines/order.rb

@@ -6,6 +6,7 @@ class StateMachines
       module Order
         def self.included(klass)
           klass.extend ClassMethods
+          klass.include StateChangeTracking
         end
 
         def checkout_steps

core/app/models/spree/core/state_machines/order/class_methods.rb

@@ -38,22 +38,6 @@ def define_state_machine!
             state_machine :state, initial: :cart, use_transactions: false do
               klass.next_event_transitions.each { |state| transition(state.merge(on: :next)) }
 
-              # Persist the state on the order
-              after_transition do |order, transition|
-                # Hard to say if this is really necessary, it was introduced in this commit:
-                # https://github.com/mamhoff/solidus/commit/fa1d66c42e4c04ee7cd1c20d87e4cdb74a226d3d
-                # But it seems to be harmless, so we'll keep it for now.
-                order.state = order.state # rubocop:disable Lint/SelfAssignment
-
-                order.state_changes.create(
-                  previous_state: transition.from,
-                  next_state:     transition.to,
-                  name:           'order',
-                  user_id:        order.user_id
-                )
-                order.save
-              end
-
               event :cancel do
                 transition to: :canceled, if: :allow_cancel?, from: :complete
               end

core/app/models/spree/core/state_machines/payment.rb

@@ -15,6 +15,7 @@ class StateMachines
       #
       module Payment
         extend ActiveSupport::Concern
+        include StateChangeTracking
 
         included do
           state_machine initial: :checkout do
@@ -45,14 +46,6 @@ module Payment
             event :invalidate do
               transition from: [:checkout], to: :invalid
             end
-
-            after_transition do |payment, transition|
-              payment.state_changes.create!(
-                previous_state: transition.from,
-                next_state:     transition.to,
-                name:           'payment'
-              )
-            end
           end
         end
       end

core/app/models/spree/core/state_machines/shipment.rb

@@ -15,6 +15,7 @@ class StateMachines
       #
       module Shipment
         extend ActiveSupport::Concern
+        include StateChangeTracking
 
         included do
           state_machine initial: :pending, use_transactions: false do
@@ -42,14 +43,6 @@ module Shipment
               transition from: :canceled, to: :pending
             end
             after_transition from: :canceled, to: [:pending, :ready, :shipped], do: :after_resume
-
-            after_transition do |shipment, transition|
-              shipment.state_changes.create!(
-                previous_state: transition.from,
-                next_state:     transition.to,
-                name:           'shipment'
-              )
-            end
           end
         end
       end

core/app/models/spree/order_updater.rb

@@ -185,11 +185,11 @@ def log_state_change(name)
       yield
       new_state = order.public_send(state)
       if old_state != new_state
-        order.state_changes.new(
-          previous_state: old_state,
-          next_state:     new_state,
-          name:,
-          user_id:        order.user_id
+        StateChangeTrackingJob.perform_later(
+          order,
+          old_state,
+          new_state,
+          Time.current
         )
       end
     end

core/lib/spree/testing_support/dummy_app.rb

@@ -23,6 +23,10 @@ class ApplicationRecord < ActiveRecord::Base
   self.abstract_class = true
 end
 
+# @private
+class ApplicationJob < ActiveJob::Base
+end
+
 # @private
 class ApplicationMailer < ActionMailer::Base
 end
@@ -94,6 +98,9 @@ class Application < ::Rails::Application
     # We don't want to send email in the test environment.
     config.action_mailer.delivery_method = :test
 
+    # Do not actually run background jobs
+    config.active_job.queue_adapter = :test
+
     # No need to use credentials file in a test environment.
     config.secret_key_base = 'SECRET_TOKEN'
 

core/spec/jobs/spree/state_change_tracking_job_spec.rb

@@ -0,0 +1,121 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Spree::StateChangeTrackingJob, type: :job do
+  let(:order) { create(:order, user: user) }
+  let(:user) { create(:user) }
+  let(:transition_timestamp) { Time.current }
+
+  describe '#perform' do
+    it 'creates a state change record with correct attributes' do
+      expect {
+        described_class.perform_now(
+          order,
+          'cart',
+          'address',
+          transition_timestamp
+        )
+      }.to change(Spree::StateChange, :count).by(1)
+
+      state_change = Spree::StateChange.last
+      expect(state_change.previous_state).to eq('cart')
+      expect(state_change.next_state).to eq('address')
+      expect(state_change.name).to eq('order')
+      expect(state_change.user_id).to eq(user.id)
+      expect(state_change.stateful_id).to eq(order.id)
+      expect(state_change.stateful_type).to eq('Spree::Order')
+      expect(state_change.created_at).to be_within(1.second).of(transition_timestamp)
+      expect(state_change.updated_at).to be_within(1.second).of(transition_timestamp)
+    end
+
+    it 'stores all state transitions in correct order' do
+      transitions = [
+        ['cart', 'address'],
+        ['address', 'delivery'],
+        ['delivery', 'payment'],
+        ['payment', 'confirm'],
+        ['confirm', 'complete'],
+        ['complete', 'canceled'],
+        ['canceled', 'resumed']
+      ]
+
+      transitions.each do |from_state, to_state|
+        described_class.perform_now(
+          order,
+          from_state,
+          to_state,
+          transition_timestamp
+        )
+
+        state_change = Spree::StateChange.last
+        expect(state_change.previous_state).to eq(from_state)
+        expect(state_change.next_state).to eq(to_state)
+        expect(state_change.stateful_id).to eq(order.id)
+        expect(state_change.stateful_type).to eq('Spree::Order')
+      end
+
+      expect(Spree::StateChange.count).to eq(transitions.length)
+      expect(Spree::StateChange.order(:created_at).pluck(:previous_state, :next_state)).to eq(transitions)
+    end
+
+    it 'preserves the exact transition timestamp' do
+      specific_time = Time.zone.parse('2023-12-25 10:30:45')
+
+      described_class.perform_now(
+        order,
+        'cart',
+        'address',
+        specific_time
+      )
+
+      state_change = Spree::StateChange.last
+      expect(state_change.created_at).to eq(specific_time)
+      expect(state_change.updated_at).to eq(specific_time)
+    end
+
+    context 'when the order has no user' do
+      let(:order) { create(:order, user: nil) }
+
+      it 'sets user_id to nil in the state change record' do
+        described_class.perform_now(
+          order,
+          'cart',
+          'address',
+          transition_timestamp
+        )
+
+        state_change = Spree::StateChange.last
+        expect(state_change.user_id).to be_nil
+      end
+    end
+
+    context 'when the object has a order association' do
+      let(:payment) { create(:payment, order: order) }
+
+      it 'uses the order user_id if available' do
+        described_class.perform_now(
+          payment,
+          'checkout',
+          'completed',
+          transition_timestamp
+        )
+
+        state_change = Spree::StateChange.last
+        expect(state_change.user_id).to eq(order.user_id)
+      end
+    end
+
+    it 'stores stateful name' do
+      described_class.perform_now(
+        order,
+        'cart',
+        'address',
+        transition_timestamp
+      )
+
+      state_change = Spree::StateChange.last
+      expect(state_change.name).to eq('order')
+    end
+  end
+end