From e32b48636ee39a102895fe9813528f5c1f8ab41f Mon Sep 17 00:00:00 2001 From: Jonny Wheeler Date: Tue, 3 Oct 2017 08:36:19 +0100 Subject: [PATCH] Revert changes made to machine find_or_create method in #32 PR #32 introduced functionality to always return a machine if one is present and no key is supplied to the `state_machine` call. This broke having multiple state machines on the same class unless both have explicit names eg.using a slightly modified example from the readme. ``` class Vehicle state_machine initial: :parked do event :crash do transition all => :parked end end state_machine :alarm_state, initial: :active, namespace: :'alarm' do event :enable do transition all => :active end end end ``` The second machine would not be created and `enable` would be declared on state, without the alarm namespace. Tests were lacking to detect this behaviour so have added them. An alternative fix would be to insist that classes with multiple state machines always have a state key on each machine including the default `:state` but I would argue that if you have a state machine with a non default key then you would always supply the key when interacting with the machine and have updated test/functional/driver_default_nonstandard_test.rb to reflect this. Happy to make further amendments as you see fit. --- Contributors.md | 3 ++- lib/state_machines/machine.rb | 6 ++--- .../driver_default_nonstandard_test.rb | 2 +- test/functional/vehicle_insurance_test.rb | 25 +++++++++++++++++++ ...other_named_existing_on_same_class_test.rb | 14 +++++++++++ 5 files changed, 44 insertions(+), 6 deletions(-) create mode 100644 test/functional/vehicle_insurance_test.rb create mode 100644 test/unit/machine/machine_finder_with_other_named_existing_on_same_class_test.rb diff --git a/Contributors.md b/Contributors.md index b84d93e..c028031 100644 --- a/Contributors.md +++ b/Contributors.md @@ -36,4 +36,5 @@ - @gmitrev - @nblumoe - @reiner -- @sanemat \ No newline at end of file +- @sanemat +- @jonathan-wheeler \ No newline at end of file diff --git a/lib/state_machines/machine.rb b/lib/state_machines/machine.rb index b99806a..ddb91f6 100644 --- a/lib/state_machines/machine.rb +++ b/lib/state_machines/machine.rb @@ -419,10 +419,8 @@ def find_or_create(owner_class, *args, &block) name = args.first || :state # Find an existing machine - machine = owner_class.respond_to?(:state_machines) && - (args.first && owner_class.state_machines[name] || !args.first && - owner_class.state_machines.values.first) || nil - + machine = owner_class.respond_to?(:state_machines) && owner_class.state_machines[name] + if machine # Only create a new copy if changes are being made to the machine in # a subclass diff --git a/test/functional/driver_default_nonstandard_test.rb b/test/functional/driver_default_nonstandard_test.rb index 771ce1b..f5af94f 100644 --- a/test/functional/driver_default_nonstandard_test.rb +++ b/test/functional/driver_default_nonstandard_test.rb @@ -4,7 +4,7 @@ class DriverNonstandardTest < MiniTest::Test def setup @driver = Driver.new - @events = Driver.state_machine.events + @events = Driver.state_machine(:status).events end def test_should_have diff --git a/test/functional/vehicle_insurance_test.rb b/test/functional/vehicle_insurance_test.rb new file mode 100644 index 0000000..058c0db --- /dev/null +++ b/test/functional/vehicle_insurance_test.rb @@ -0,0 +1,25 @@ +require_relative '../test_helper' +require_relative '../files/models/vehicle' + +class VehicleInsuranceTest < MiniTest::Test + def setup + @vehicle = Vehicle.new + end + + def test_insurance_state_should_be_in_inactive_state + assert_equal 'inactive', @vehicle.insurance_state + end + + def test_insurance_should_be_inactive + assert @vehicle.insurance_inactive? + end + + def test_should_allow_buy_insurance + assert @vehicle.buy_insurance + end + + def test_insurance_should_be_active_after_buy_event + @vehicle.buy_insurance + assert @vehicle.insurance_active? + end +end diff --git a/test/unit/machine/machine_finder_with_other_named_existing_on_same_class_test.rb b/test/unit/machine/machine_finder_with_other_named_existing_on_same_class_test.rb new file mode 100644 index 0000000..9aab690 --- /dev/null +++ b/test/unit/machine/machine_finder_with_other_named_existing_on_same_class_test.rb @@ -0,0 +1,14 @@ +require_relative '../../test_helper' + +class MachineFinderWithOtherNamedExistingOnSameClassTest < StateMachinesTest + def setup + @klass = Class.new + @existing_machine = StateMachines::Machine.new(@klass, :status) + @machine = StateMachines::Machine.find_or_create(@klass) + end + + def test_should_create_a_new_machine + refute_same @machine, @existing_machine + end +end +