diff --git a/CHANGELOG.rdoc b/CHANGELOG.rdoc index 73644359..02fb377f 100644 --- a/CHANGELOG.rdoc +++ b/CHANGELOG.rdoc @@ -1,5 +1,7 @@ == master +* Add a validation error when failing to transition for ActiveRecord / DataMapper / Sequel integrations + == 0.6.0 / 2009-03-03 * Allow multiple conditions for callbacks / class behaviors diff --git a/README.rdoc b/README.rdoc index ab462abb..bb9fffe7 100644 --- a/README.rdoc +++ b/README.rdoc @@ -224,7 +224,7 @@ A brief overview of these integrations is described below. === ActiveRecord The ActiveRecord integration adds support for database transactions, automatically -saving the record, named scopes, and observers. For example, +saving the record, named scopes, validation errors, and observers. For example, class Vehicle < ActiveRecord::Base state_machine :initial => :parked do @@ -266,7 +266,7 @@ machines, see StateMachine::Integrations::ActiveRecord. Like the ActiveRecord integration, the DataMapper integration adds support for database transactions, automatically saving the record, named scopes, Extlib-like -callbacks, and observers. For example, +callbacks, validation errors, and observers. For example, class Vehicle include DataMapper::Resource @@ -319,8 +319,8 @@ machines, see StateMachine::Integrations::DataMapper. === Sequel Like the ActiveRecord integration, the Sequel integration adds support for -database transactions, automatically saving the record, named scopes, and -callbacks. For example, +database transactions, automatically saving the record, named scopes, validation +errors and callbacks. For example, class Vehicle < Sequel::Model state_machine :initial => :parked do diff --git a/lib/state_machine/event.rb b/lib/state_machine/event.rb index fd252fb0..8bbfae35 100644 --- a/lib/state_machine/event.rb +++ b/lib/state_machine/event.rb @@ -180,6 +180,7 @@ def fire(object, *args) if transition = next_transition(object) transition.perform(*args) else + machine.invalidate(object, self) false end end diff --git a/lib/state_machine/integrations.rb b/lib/state_machine/integrations.rb index 8386b55d..f9a94e44 100644 --- a/lib/state_machine/integrations.rb +++ b/lib/state_machine/integrations.rb @@ -13,6 +13,7 @@ module StateMachine # * Observers # * Scopes # * Callbacks + # * Validation errors # # This type of integration allows the user to work with state machines in a # fashion similar to other object models in their application. diff --git a/lib/state_machine/integrations/active_record.rb b/lib/state_machine/integrations/active_record.rb index d5899522..bee1b519 100644 --- a/lib/state_machine/integrations/active_record.rb +++ b/lib/state_machine/integrations/active_record.rb @@ -60,6 +60,23 @@ module Integrations #:nodoc: # rolled back. If an after callback halts the chain, the previous result # still applies and the transaction is *not* rolled back. # + # == Validation errors + # + # If an event fails to successfully fire because there are no matching + # transitions for the current record, a validation error is added to the + # record's state attribute to help in determining why it failed and for + # reporting via the UI. + # + # For example, + # + # vehicle = Vehicle.create(:state => 'idling') # => # + # vehicle.ignite # => false + # vehicle.errors.full_messages # => ["State cannot be transitioned via :ignite from :idling"] + # + # If an event fails to fire because of a validation error on the record and + # *not* because a matching transition was not available, no error messages + # will be added to the state attribute. + # # == Scopes # # To assist in filtering models with specific states, a series of named @@ -172,6 +189,12 @@ def self.extended(base) #:nodoc: require 'state_machine/integrations/active_record/observer' end + # Adds a validation error to the given object after failing to fire a + # specific event + def invalidate(object, event) + object.errors.add(attribute, invalid_message(object, event)) + end + # Runs a new database transaction, rolling back any changes by raising # an ActiveRecord::Rollback exception if the yielded block fails # (i.e. returns false). diff --git a/lib/state_machine/integrations/data_mapper.rb b/lib/state_machine/integrations/data_mapper.rb index 1ed835ee..b8812038 100644 --- a/lib/state_machine/integrations/data_mapper.rb +++ b/lib/state_machine/integrations/data_mapper.rb @@ -70,6 +70,23 @@ module Integrations #:nodoc: # rolled back. If an after callback halts the chain, the previous result # still applies and the transaction is *not* rolled back. # + # == Validation errors + # + # If an event fails to successfully fire because there are no matching + # transitions for the current record, a validation error is added to the + # record's state attribute to help in determining why it failed and for + # reporting via the UI. + # + # For example, + # + # vehicle = Vehicle.create(:state => 'idling') # => # + # vehicle.ignite # => false + # vehicle.errors.full_messages # => ["cannot be transitioned via :ignite from :idling"] + # + # If an event fails to fire because of a validation error on the record and + # *not* because a matching transition was not available, no error messages + # will be added to the state attribute. + # # == Scopes # # To assist in filtering models with specific states, a series of class @@ -159,6 +176,12 @@ def self.extended(base) #:nodoc: require 'state_machine/integrations/data_mapper/observer' if ::DataMapper.const_defined?('Observer') end + # Adds a validation error to the given object after failing to fire a + # specific event + def invalidate(object, event) + object.errors.add(attribute, invalid_message(object, event)) if object.respond_to?(:errors) + end + # Runs a new database transaction, rolling back any changes if the # yielded block fails (i.e. returns false). def within_transaction(object) diff --git a/lib/state_machine/integrations/sequel.rb b/lib/state_machine/integrations/sequel.rb index 9a7d3103..99babd1e 100644 --- a/lib/state_machine/integrations/sequel.rb +++ b/lib/state_machine/integrations/sequel.rb @@ -60,6 +60,23 @@ module Integrations #:nodoc: # rolled back. If an after callback halts the chain, the previous result # still applies and the transaction is *not* rolled back. # + # == Validation errors + # + # If an event fails to successfully fire because there are no matching + # transitions for the current record, a validation error is added to the + # record's state attribute to help in determining why it failed and for + # reporting via the UI. + # + # For example, + # + # vehicle = Vehicle.create(:state => 'idling') # => # + # vehicle.ignite # => false + # vehicle.errors.full_messages # => ["state cannot be transitioned via :ignite from :idling"] + # + # If an event fails to fire because of a validation error on the record and + # *not* because a matching transition was not available, no error messages + # will be added to the state attribute. + # # == Scopes # # To assist in filtering models with specific states, a series of class @@ -129,6 +146,12 @@ def self.matches?(klass) defined?(::Sequel::Model) && klass <= ::Sequel::Model end + # Adds a validation error to the given object after failing to fire a + # specific event + def invalidate(object, event) + object.errors.add(attribute, invalid_message(object, event)) + end + # Runs a new database transaction, rolling back any changes if the # yielded block fails (i.e. returns false). def within_transaction(object) diff --git a/lib/state_machine/machine.rb b/lib/state_machine/machine.rb index 2f9dec22..1e5b8cbc 100644 --- a/lib/state_machine/machine.rb +++ b/lib/state_machine/machine.rb @@ -142,6 +142,10 @@ class Machine include MatcherHelpers class << self + # The default message to use when invalidating objects that fail to + # transition when triggering an event + attr_accessor :default_invalid_message + # Attempts to find or create a state machine for the given class. For # example, # @@ -208,6 +212,9 @@ def draw(class_names, options = {}) end end + # Set defaults + self.default_invalid_message = 'cannot be transitioned via :%s from :%s' + # The class that the machine is defined in attr_accessor :owner_class @@ -245,7 +252,7 @@ def draw(class_names, options = {}) # Creates a new state machine for the given attribute def initialize(owner_class, *args, &block) options = args.last.is_a?(Hash) ? args.pop : {} - assert_valid_keys(options, :initial, :action, :plural, :namespace, :integration) + assert_valid_keys(options, :initial, :action, :plural, :namespace, :integration, :invalid_message) # Set machine configuration @attribute = args.first || :state @@ -253,6 +260,7 @@ def initialize(owner_class, *args, &block) @states = StateCollection.new @callbacks = {:before => [], :after => []} @namespace = options[:namespace] + @invalid_message = options[:invalid_message] || self.class.default_invalid_message self.owner_class = owner_class self.initial_state = options[:initial] @@ -904,6 +912,13 @@ def after_transition(options = {}, &block) add_callback(:after, options.is_a?(Hash) ? options : {:do => options}, &block) end + # Marks the given object as invalid after failing to transition via the + # given event. + # + # By default, this is a no-op. + def invalidate(object, event) + end + # Runs a transaction, rolling back any changes if the yielded block fails. # # This is only applicable to integrations that involve databases. By @@ -1077,5 +1092,11 @@ def add_states(new_states) state end end + + # Generates the message to use when invalidating the given object after + # failing to transition on a specific event + def invalid_message(object, event) + @invalid_message % [event.name, state_for(object).name] + end end end diff --git a/lib/state_machine/transition.rb b/lib/state_machine/transition.rb index 802f3b6f..230b8ed2 100644 --- a/lib/state_machine/transition.rb +++ b/lib/state_machine/transition.rb @@ -85,7 +85,7 @@ def perform(run_action = true) # Updates the object's attribute to the ending state object.send("#{attribute}=", to) - result = run_action && machine.action ? object.send(machine.action) : true + result = run_action && machine.action ? object.send(machine.action) != false : true # Always run after callbacks regardless of whether the action failed. # Result is included in case the callback depends on this value diff --git a/test/unit/event_test.rb b/test/unit/event_test.rb index 410c2f9a..8c952939 100644 --- a/test/unit/event_test.rb +++ b/test/unit/event_test.rb @@ -264,8 +264,17 @@ def test_should_not_change_the_current_state class EventWithMatchingDisabledTransitionsTest < Test::Unit::TestCase def setup - @klass = Class.new - @machine = StateMachine::Machine.new(@klass) + StateMachine::Integrations.const_set('Custom', Module.new do + def invalidate(object, event) + object.error = invalid_message(object, event) + end + end) + + @klass = Class.new do + attr_accessor :error + end + + @machine = StateMachine::Machine.new(@klass, :integration => :custom) @machine.state :parked, :idling @event = StateMachine::Event.new(@machine, :ignite) @@ -291,12 +300,30 @@ def test_should_not_change_the_current_state @event.fire(@object) assert_equal 'parked', @object.state end + + def test_should_invalidate_the_state + @event.fire(@object) + assert_equal 'cannot be transitioned via :ignite from :parked', @object.error + end + + def teardown + StateMachine::Integrations.send(:remove_const, 'Custom') + end end class EventWithMatchingEnabledTransitionsTest < Test::Unit::TestCase def setup - @klass = Class.new - @machine = StateMachine::Machine.new(@klass) + StateMachine::Integrations.const_set('Custom', Module.new do + def invalidate(object, event) + object.error = invalid_message(object, event) + end + end) + + @klass = Class.new do + attr_accessor :error + end + + @machine = StateMachine::Machine.new(@klass, :integration => :custom) @machine.state :parked, :idling @event = StateMachine::Event.new(@machine, :ignite) @@ -326,6 +353,15 @@ def test_should_change_the_current_state @event.fire(@object) assert_equal 'idling', @object.state end + + def test_should_not_invalidate_the_state + @event.fire(@object) + assert_nil @object.error + end + + def teardown + StateMachine::Integrations.send(:remove_const, 'Custom') + end end class EventWithTransitionWithoutToStateTest < Test::Unit::TestCase diff --git a/test/unit/integrations/active_record_test.rb b/test/unit/integrations/active_record_test.rb index e719ce49..bafbfb9f 100644 --- a/test/unit/integrations/active_record_test.rb +++ b/test/unit/integrations/active_record_test.rb @@ -160,6 +160,16 @@ def test_should_not_rollback_transaction_if_true assert_equal 1, @model.count end + def test_should_invalidate_using_errors + record = @model.new + record.state = 'parked' + + @machine.invalidate(record, StateMachine::Event.new(@machine, :park)) + + assert record.errors.invalid?(:state) + assert_equal 'cannot be transitioned via :park from :parked', record.errors.on(:state) + end + def test_should_not_override_the_column_reader record = @model.new record[:state] = 'parked' diff --git a/test/unit/integrations/data_mapper_test.rb b/test/unit/integrations/data_mapper_test.rb index 834b7f59..d9de5c82 100644 --- a/test/unit/integrations/data_mapper_test.rb +++ b/test/unit/integrations/data_mapper_test.rb @@ -141,6 +141,15 @@ def test_should_not_rollback_transaction_if_true assert_equal 1, @resource.all.size end + def test_should_invalidate_using_errors + record = @resource.new + record.state = 'parked' + + @machine.invalidate(record, StateMachine::Event.new(@machine, :park)) + + assert_equal ['cannot be transitioned via :park from :parked'], record.errors.on(:state) + end + def test_should_not_override_the_column_reader record = @resource.new record.attribute_set(:state, 'parked') diff --git a/test/unit/integrations/sequel_test.rb b/test/unit/integrations/sequel_test.rb index 229eaabe..80fc7e2c 100644 --- a/test/unit/integrations/sequel_test.rb +++ b/test/unit/integrations/sequel_test.rb @@ -22,6 +22,8 @@ def new_model(auto_migrate = true, &block) column :state, :string end if auto_migrate model = Class.new(Sequel::Model(:foo)) do + self.raise_on_save_failure = false + def self.name; 'SequelTest::Foo'; end end model.class_eval(&block) if block_given? @@ -127,6 +129,15 @@ def test_should_not_rollback_transaction_if_true assert_equal 1, @model.count end + def test_should_invalidate_using_errors + record = @model.new + record.state = 'parked' + + @machine.invalidate(record, StateMachine::Event.new(@machine, :park)) + + assert_equal ['cannot be transitioned via :park from :parked'], record.errors.on(:state) + end + def test_should_not_override_the_column_reader record = @model.new record[:state] = 'parked' @@ -271,7 +282,7 @@ def test_should_pass_transition_and_result_into_after_callbacks_with_multiple_ar @machine.after_transition(lambda {|*args| callback_args = args}) @transition.perform - assert_equal [@transition, @record], callback_args + assert_equal [@transition, true], callback_args end def test_should_run_after_callbacks_with_the_context_of_the_record diff --git a/test/unit/machine_test.rb b/test/unit/machine_test.rb index 775d95aa..5fd747fa 100644 --- a/test/unit/machine_test.rb +++ b/test/unit/machine_test.rb @@ -250,6 +250,10 @@ def test_transaction_should_yield assert @yielded end + + def test_invalidation_should_do_nothing + assert_nil @machine.invalidate(@object, StateMachine::Event.new(@machine, :park)) + end end class MachineWithCustomIntegrationTest < Test::Unit::TestCase @@ -328,7 +332,6 @@ class MachineWithCustomPluralTest < Test::Unit::TestCase def setup @integration = Module.new do class << self; attr_accessor :with_scopes, :without_scopes; end - @initialized = false @with_scopes = [] @without_scopes = [] @@ -360,11 +363,45 @@ def teardown end end +class MachineWithCustomInvalidationTest < Test::Unit::TestCase + def setup + @integration = Module.new do + def invalidate(object, event) + object.error = invalid_message(object, event) + end + end + StateMachine::Integrations.const_set('Custom', @integration) + + @klass = Class.new do + attr_accessor :error + end + + @machine = StateMachine::Machine.new(@klass, :integration => :custom, :invalid_message => 'cannot %s when %s') + @machine.state :parked + + @object = @klass.new + @object.state = 'parked' + end + + def test_use_custom_message + @machine.invalidate(@object, StateMachine::Event.new(@machine, :park)) + assert_equal 'cannot park when parked', @object.error + end + + def teardown + StateMachine::Integrations.send(:remove_const, 'Custom') + end +end + class MachineTest < Test::Unit::TestCase def test_should_raise_exception_if_invalid_option_specified assert_raise(ArgumentError) {StateMachine::Machine.new(Class.new, :invalid => true)} end + def test_should_not_raise_exception_if_custom_invalid_message_specified + assert_nothing_raised {StateMachine::Machine.new(Class.new, :invalid_message => 'custom')} + end + def test_should_evaluate_a_block_during_initialization called = true StateMachine::Machine.new(Class.new) do @@ -969,8 +1006,8 @@ def setup @klass = Class.new @machine = StateMachine::Machine.new(@klass, :initial => :parked) @event = @machine.event(:ignite) do - transition :to => :idling, :from => :parked - transition :to => :idling, :from => :stalled + transition :parked => :idling + transition :stalled => :idling end end @@ -984,7 +1021,7 @@ def test_should_track_states_defined_in_event_transitions def test_should_not_duplicate_states_defined_in_multiple_event_transitions @machine.event :park do - transition :to => :parked, :from => :idling + transition :idling => :parked end assert_equal [:parked, :idling, :stalled], @machine.states.map {|state| state.name} @@ -992,7 +1029,7 @@ def test_should_not_duplicate_states_defined_in_multiple_event_transitions def test_should_track_state_from_new_events @machine.event :shift_up do - transition :to => :first_gear, :from => :idling + transition :idling => :first_gear end assert_equal [:parked, :idling, :stalled, :first_gear], @machine.states.map {|state| state.name} @@ -1004,7 +1041,7 @@ def setup @klass = Class.new @machine = StateMachine::Machine.new(@klass, :initial => :parked) @park, @shift_down = @machine.event(:park, :shift_down) do - transition :to => :parked, :from => :first_gear + transition :first_gear => :parked end end @@ -1037,7 +1074,7 @@ def setup @machine = StateMachine::Machine.new(@klass, :initial => :parked) @event = @machine.event :ignite do - transition :to => :idling, :from => :parked + transition :parked => :idling end @object = @klass.new @@ -1118,16 +1155,16 @@ def test_should_support_implicit_requirement end def test_should_track_states_defined_in_transition_callbacks - @machine.before_transition :from => :parked, :to => :idling, :do => lambda {} - @machine.after_transition :from => :first_gear, :to => :second_gear, :do => lambda {} + @machine.before_transition :parked => :idling, :do => lambda {} + @machine.after_transition :first_gear => :second_gear, :do => lambda {} assert_equal [:parked, :idling, :first_gear, :second_gear], @machine.states.map {|state| state.name} end def test_should_not_duplicate_states_defined_in_multiple_event_transitions - @machine.before_transition :from => :parked, :to => :idling, :do => lambda {} - @machine.after_transition :from => :first_gear, :to => :second_gear, :do => lambda {} - @machine.after_transition :from => :parked, :to => :idling, :do => lambda {} + @machine.before_transition :parked => :idling, :do => lambda {} + @machine.after_transition :first_gear => :second_gear, :do => lambda {} + @machine.after_transition :parked => :idling, :do => lambda {} assert_equal [:parked, :idling, :first_gear, :second_gear], @machine.states.map {|state| state.name} end @@ -1177,11 +1214,11 @@ def setup @klass = Class.new @machine = StateMachine::Machine.new(@klass, :namespace => 'car', :initial => :parked) do event :ignite do - transition :to => :idling, :from => :parked + transition :parked => :idling end event :park do - transition :to => :parked, :from => :idling + transition :idling => :parked end end @object = @klass.new @@ -1354,7 +1391,7 @@ def self.name; 'Vehicle'; end end @machine = StateMachine::Machine.new(@klass, :initial => :parked) @machine.event :ignite do - transition :to => :idling, :from => :parked + transition :parked => :idling end end @@ -1398,7 +1435,7 @@ def self.name; 'Vehicle'; end end @machine = StateMachine::Machine.new(@klass, :state_id, :initial => :parked) @machine.event :ignite do - transition :to => :idling, :from => :parked + transition :parked => :idling end @machine.state :parked, :value => 1 @machine.state :idling, :value => 2 @@ -1419,7 +1456,7 @@ def self.name; 'Vehicle'; end end @machine = StateMachine::Machine.new(@klass, :initial => :parked) @machine.event :ignite do - transition :to => :idling, :from => :parked + transition :parked => :idling end @machine.state :parked, :value => nil @machine.draw @@ -1439,7 +1476,7 @@ def self.name; 'Vehicle'; end end @machine = StateMachine::Machine.new(@klass, :initial => :parked) @machine.event :activate do - transition :to => :idling, :from => :parked + transition :parked => :idling end @machine.state :idling, :value => lambda {Time.now} @machine.draw @@ -1459,7 +1496,7 @@ def self.name; 'Vehicle'; end end @machine = StateMachine::Machine.new(@klass) @machine.event :ignite do - transition :to => :idling, :from => :parked + transition :parked => :idling end end