diff --git a/README.md b/README.md index ba1c279a..28441f9e 100644 --- a/README.md +++ b/README.md @@ -76,22 +76,16 @@ Then, link it to your model: ```ruby class Order < ActiveRecord::Base - include Statesman::Adapters::ActiveRecordQueries - has_many :order_transitions, autosave: false + include Statesman::Adapters::ActiveRecordQueries[ + transition_class: OrderTransition, + initial_state: :pending + ] + def state_machine @state_machine ||= OrderStateMachine.new(self, transition_class: OrderTransition) end - - def self.transition_class - OrderTransition - end - - def self.initial_state - :pending - end - private_class_method :initial_state end ``` @@ -328,43 +322,34 @@ callback code throws an exception, it will not be caught.) A mixin is provided for the ActiveRecord adapter which adds scopes to easily find all models currently in (or not in) a given state. Include it into your -model and define `transition_class` and `initial_state` class methods: +model and passing in `transition_class` and `initial_state` as options. + +In 4.1.1 and below, these two options had to be defined as methods on the model, +but 4.2.0 and above allow this style of configuration as well. The old method +pollutes the model with extra class methods, and is deprecated, to be removed +in 5.0.0. ```ruby class Order < ActiveRecord::Base - include Statesman::Adapters::ActiveRecordQueries - - def self.transition_class - OrderTransition - end - private_class_method :transition_class - - def self.initial_state - OrderStateMachine.initial_state - end - private_class_method :initial_state + has_many :order_transitions, autosave: false + include Statesman::Adapters::ActiveRecordQueries[ + transition_class: OrderTransition, + initial_state: OrderStateMachine.initial_state + ] end ``` If the transition class-name differs from the association name, you will also -need to define a corresponding `transition_name` class method: +need to pass `transition_name` as an option: ```ruby class Order < ActiveRecord::Base has_many :transitions, class_name: "OrderTransition", autosave: false - - def self.transition_name - :transitions - end - - def self.transition_class - OrderTransition - end - - def self.initial_state - OrderStateMachine.initial_state - end - private_class_method :initial_state + include Statesman::Adapters::ActiveRecordQueries[ + transition_class: OrderTransition, + initial_state: OrderStateMachine.initial_state, + transition_name: :transitions + ] end ``` diff --git a/lib/statesman/adapters/active_record_queries.rb b/lib/statesman/adapters/active_record_queries.rb index 3025ef3a..bcd119aa 100644 --- a/lib/statesman/adapters/active_record_queries.rb +++ b/lib/statesman/adapters/active_record_queries.rb @@ -1,51 +1,122 @@ module Statesman module Adapters module ActiveRecordQueries + def self.check_missing_methods!(base) + missing_methods = %i[transition_class initial_state]. + reject { |_method| base.respond_to?(:method) } + return if missing_methods.none? + + raise NotImplementedError, + "#{missing_methods.join(', ')} method(s) should be defined on " \ + "the model. Alternatively, use the new form of `extend " \ + "Statesman::Adapters::ActiveRecordQueries[" \ + "transition_class: MyTransition, " \ + "initial_state: :some_state]`" + end + def self.included(base) - base.extend(ClassMethods) + check_missing_methods!(base) + + base.include( + ClassMethods.new( + transition_class: base.transition_class, + initial_state: base.initial_state, + most_recent_transition_alias: base.try(:most_recent_transition_alias), + transition_name: base.try(:transition_name), + ), + ) end - module ClassMethods - def in_state(*states) - states = states.flatten.map(&:to_s) + def self.[](**args) + ClassMethods.new(**args) + end + + class ClassMethods < Module + def initialize(**args) + @args = args + end + + def included(base) + ensure_inheritance(base) + + query_builder = QueryBuilder.new(base, **@args) + + base.define_singleton_method(:most_recent_transition_join) do + query_builder.most_recent_transition_join + end + + define_in_state(base, query_builder) + define_not_in_state(base, query_builder) + end - joins(most_recent_transition_join). - where(states_where(most_recent_transition_alias, states), states) + private + + def ensure_inheritance(base) + klass = self + existing_inherited = base.method(:inherited) + base.define_singleton_method(:inherited) do |subclass| + existing_inherited.call(subclass) + subclass.send(:include, klass) + end end - def not_in_state(*states) - states = states.flatten.map(&:to_s) + def define_in_state(base, query_builder) + base.define_singleton_method(:in_state) do |*states| + states = states.flatten.map(&:to_s) - joins(most_recent_transition_join). - where("NOT (#{states_where(most_recent_transition_alias, states)})", - states) + joins(most_recent_transition_join). + where(query_builder.states_where(states), states) + end + end + + def define_not_in_state(base, query_builder) + base.define_singleton_method(:not_in_state) do |*states| + states = states.flatten.map(&:to_s) + + joins(most_recent_transition_join). + where("NOT (#{query_builder.states_where(states)})", states) + end + end + end + + class QueryBuilder + def initialize(model, transition_class:, initial_state:, + most_recent_transition_alias: nil, + transition_name: nil) + @model = model + @transition_class = transition_class + @initial_state = initial_state + @most_recent_transition_alias = most_recent_transition_alias + @transition_name = transition_name + end + + def states_where(states) + if initial_state.to_s.in?(states.map(&:to_s)) + "#{most_recent_transition_alias}.to_state IN (?) OR " \ + "#{most_recent_transition_alias}.to_state IS NULL" + else + "#{most_recent_transition_alias}.to_state IN (?) AND " \ + "#{most_recent_transition_alias}.to_state IS NOT NULL" + end end def most_recent_transition_join "LEFT OUTER JOIN #{model_table} AS #{most_recent_transition_alias} - ON #{table_name}.id = + ON #{model.table_name}.id = #{most_recent_transition_alias}.#{model_foreign_key} AND #{most_recent_transition_alias}.most_recent = #{db_true}" end private - def transition_class - raise NotImplementedError, "A transition_class method should be " \ - "defined on the model" - end - - def initial_state - raise NotImplementedError, "An initial_state method should be " \ - "defined on the model" - end + attr_reader :model, :transition_class, :initial_state def transition_name - transition_class.table_name.to_sym + @transition_name || transition_class.table_name.to_sym end def transition_reflection - reflect_on_all_associations(:has_many).each do |value| + model.reflect_on_all_associations(:has_many).each do |value| return value if value.klass == transition_class end @@ -62,18 +133,9 @@ def model_table transition_reflection.table_name end - def states_where(temporary_table_name, states) - if initial_state.to_s.in?(states.map(&:to_s)) - "#{temporary_table_name}.to_state IN (?) OR " \ - "#{temporary_table_name}.to_state IS NULL" - else - "#{temporary_table_name}.to_state IN (?) AND " \ - "#{temporary_table_name}.to_state IS NOT NULL" - end - end - def most_recent_transition_alias - "most_recent_#{transition_name.to_s.singularize}" + @most_recent_transition_alias || + "most_recent_#{transition_name.to_s.singularize}" end def db_true diff --git a/spec/statesman/adapters/active_record_queries_spec.rb b/spec/statesman/adapters/active_record_queries_spec.rb index 9588269e..48ee0674 100644 --- a/spec/statesman/adapters/active_record_queries_spec.rb +++ b/spec/statesman/adapters/active_record_queries_spec.rb @@ -1,6 +1,17 @@ require "spec_helper" describe Statesman::Adapters::ActiveRecordQueries, active_record: true do + def configure_old(klass, transition_class) + klass.define_singleton_method(:transition_class) { transition_class } + klass.define_singleton_method(:initial_state) { :initial } + klass.send(:include, described_class) + end + + def configure_new(klass, transition_class) + klass.send(:include, described_class[transition_class: transition_class, + initial_state: :initial]) + end + before do prepare_model_table prepare_transitions_table @@ -8,32 +19,6 @@ prepare_other_transitions_table Statesman.configure { storage_adapter(Statesman::Adapters::ActiveRecord) } - - MyActiveRecordModel.send(:include, Statesman::Adapters::ActiveRecordQueries) - MyActiveRecordModel.class_eval do - def self.transition_class - MyActiveRecordModelTransition - end - - def self.initial_state - :initial - end - end - - OtherActiveRecordModel.send(:include, - Statesman::Adapters::ActiveRecordQueries) - OtherActiveRecordModel.class_eval do - def self.transition_class - OtherActiveRecordModelTransition - end - - def self.initial_state - :initial - end - end - - MyActiveRecordModel.send(:has_one, :other_active_record_model) - OtherActiveRecordModel.send(:belongs_to, :my_active_record_model) end after { Statesman.configure { storage_adapter(Statesman::Adapters::Memory) } } @@ -59,147 +44,172 @@ def self.initial_state model end - describe ".in_state" do - context "given a single state" do - subject { MyActiveRecordModel.in_state(:succeeded) } + shared_examples "testing methods" do + before do + if config_type == :old + configure_old(MyActiveRecordModel, MyActiveRecordModelTransition) + configure_old(OtherActiveRecordModel, OtherActiveRecordModelTransition) + elsif config_type == :new + configure_new(MyActiveRecordModel, MyActiveRecordModelTransition) + configure_new(OtherActiveRecordModel, OtherActiveRecordModelTransition) + else + raise "Unknown config type #{config_type}" + end - it { is_expected.to include model } - it { is_expected.to_not include other_model } + MyActiveRecordModel.send(:has_one, :other_active_record_model) + OtherActiveRecordModel.send(:belongs_to, :my_active_record_model) end - context "given multiple states" do - subject { MyActiveRecordModel.in_state(:succeeded, :failed) } - - it { is_expected.to include model } - it { is_expected.to include other_model } - end + describe ".in_state" do + context "given a single state" do + subject { MyActiveRecordModel.in_state(:succeeded) } - context "given the initial state" do - subject { MyActiveRecordModel.in_state(:initial) } + it { is_expected.to include model } + it { is_expected.to_not include other_model } + end - it { is_expected.to include initial_state_model } - it { is_expected.to include returned_to_initial_model } - end + context "given multiple states" do + subject { MyActiveRecordModel.in_state(:succeeded, :failed) } - context "given an array of states" do - subject { MyActiveRecordModel.in_state(%i[succeeded failed]) } + it { is_expected.to include model } + it { is_expected.to include other_model } + end - it { is_expected.to include model } - it { is_expected.to include other_model } - end + context "given the initial state" do + subject { MyActiveRecordModel.in_state(:initial) } - context "merging two queries" do - subject do - MyActiveRecordModel.in_state(:succeeded). - joins(:other_active_record_model). - merge(OtherActiveRecordModel.in_state(:initial)) + it { is_expected.to include initial_state_model } + it { is_expected.to include returned_to_initial_model } end - it { is_expected.to be_empty } - end - end - - describe ".not_in_state" do - context "given a single state" do - subject { MyActiveRecordModel.not_in_state(:failed) } + context "given an array of states" do + subject { MyActiveRecordModel.in_state(%i[succeeded failed]) } - it { is_expected.to include model } - it { is_expected.to_not include other_model } - end + it { is_expected.to include model } + it { is_expected.to include other_model } + end - context "given multiple states" do - subject(:not_in_state) { MyActiveRecordModel.not_in_state(:succeeded, :failed) } + context "merging two queries" do + subject do + MyActiveRecordModel.in_state(:succeeded). + joins(:other_active_record_model). + merge(OtherActiveRecordModel.in_state(:initial)) + end - it do - expect(not_in_state).to match_array([initial_state_model, - returned_to_initial_model]) + it { is_expected.to be_empty } end end - context "given an array of states" do - subject(:not_in_state) { MyActiveRecordModel.not_in_state(%i[succeeded failed]) } + describe ".not_in_state" do + context "given a single state" do + subject { MyActiveRecordModel.not_in_state(:failed) } - it do - expect(not_in_state).to match_array([initial_state_model, - returned_to_initial_model]) + it { is_expected.to include model } + it { is_expected.to_not include other_model } end - end - end - context "with a custom name for the transition association" do - before do - # Switch to using OtherActiveRecordModelTransition, so the existing - # relation with MyActiveRecordModelTransition doesn't interfere with - # this spec. - MyActiveRecordModel.send(:has_many, - :custom_name, - class_name: "OtherActiveRecordModelTransition") - - MyActiveRecordModel.class_eval do - def self.transition_class - OtherActiveRecordModelTransition + context "given multiple states" do + subject(:not_in_state) { MyActiveRecordModel.not_in_state(:succeeded, :failed) } + + it do + expect(not_in_state).to match_array([initial_state_model, + returned_to_initial_model]) end end - end - describe ".in_state" do - subject(:query) { MyActiveRecordModel.in_state(:succeeded) } + context "given an array of states" do + subject(:not_in_state) { MyActiveRecordModel.not_in_state(%i[succeeded failed]) } - specify { expect { query }.to_not raise_error } + it do + expect(not_in_state).to match_array([initial_state_model, + returned_to_initial_model]) + end + end end - end - context "with no association with the transition class" do - before do - class UnknownModelTransition < OtherActiveRecordModelTransition; end + context "with a custom name for the transition association" do + before do + # Switch to using OtherActiveRecordModelTransition, so the existing + # relation with MyActiveRecordModelTransition doesn't interfere with + # this spec. + MyActiveRecordModel.send(:has_many, + :custom_name, + class_name: "OtherActiveRecordModelTransition") - MyActiveRecordModel.class_eval do - def self.transition_class - UnknownModelTransition + MyActiveRecordModel.class_eval do + def self.transition_class + OtherActiveRecordModelTransition + end end end - end - describe ".in_state" do - subject(:query) { MyActiveRecordModel.in_state(:succeeded) } + describe ".in_state" do + subject(:query) { MyActiveRecordModel.in_state(:succeeded) } - it "raises a helpful error" do - expect { query }.to raise_error(Statesman::MissingTransitionAssociation) + specify { expect { query }.to_not raise_error } end end - end - context "after_commit transactional integrity" do - before do - MyStateMachine.class_eval do - cattr_accessor(:after_commit_callback_executed) { false } + context "after_commit transactional integrity" do + before do + MyStateMachine.class_eval do + cattr_accessor(:after_commit_callback_executed) { false } - after_transition(from: :initial, to: :succeeded, after_commit: true) do - # This leaks state in a testable way if transactional integrity is broken. - MyStateMachine.after_commit_callback_executed = true + after_transition(from: :initial, to: :succeeded, after_commit: true) do + # This leaks state in a testable way if transactional integrity is broken. + MyStateMachine.after_commit_callback_executed = true + end end end - end - after do - MyStateMachine.class_eval do - callbacks[:after_commit] = [] + after do + MyStateMachine.class_eval do + callbacks[:after_commit] = [] + end + end + + let!(:model) do + MyActiveRecordModel.create + end + + # rubocop:disable RSpec/ExampleLength + it do + expect do + ActiveRecord::Base.transaction do + model.state_machine.transition_to!(:succeeded) + raise ActiveRecord::Rollback + end + end.to_not change(MyStateMachine, :after_commit_callback_executed) end + # rubocop:enable RSpec/ExampleLength end + end + + context "using old configuration method" do + let(:config_type) { :old } + + include_examples "testing methods" + end - let!(:model) do - MyActiveRecordModel.create + context "using new configuration method" do + let(:config_type) { :new } + + include_examples "testing methods" + end + + context "with no association with the transition class" do + before do + class UnknownModelTransition < OtherActiveRecordModelTransition; end + + configure_old(MyActiveRecordModel, UnknownModelTransition) end - # rubocop:disable RSpec/ExampleLength - it do - expect do - ActiveRecord::Base.transaction do - model.state_machine.transition_to!(:succeeded) - raise ActiveRecord::Rollback - end - end.to_not change(MyStateMachine, :after_commit_callback_executed) + describe ".in_state" do + subject(:query) { MyActiveRecordModel.in_state(:succeeded) } + + it "raises a helpful error" do + expect { query }.to raise_error(Statesman::MissingTransitionAssociation) + end end - # rubocop:enable RSpec/ExampleLength end end