search

benzelano / test_lh_import

1 stars 0 forks source link

Loopback transitions don't appear to update Rails' record #128

Closed benzelano closed 13 years ago

benzelano commented 13 years ago

onitony opened this issue

It seems that if a State Machine instance performs a transition that loops back into it’s previous state, the record is not saved (because no fields have been modified?). The desired behaviour though, is for updated_at to update, to note the time the transition took place (same as it happens in non-loopback transitions).

original LH ticket

This ticket has 1 attachment(s).

benzelano commented 13 years ago

Thanks for the report and nice catch. This would definitely be a problem if the ORM only saves a record when the field has been really been changed. I’ll work on getting a fix applied over the next day.

benzelano commented 13 years ago

(from [bcaafa8eb3308c78c64a404e9c3ca0ccf2c4bb5f]) Fix loopbacks not causing records to save in ORM integrations if no other fields were changed [#28 state:resolved] http://github.com/pluginaweek/state_machine/commit/bcaafa8eb3308c78c64a404e9c3ca0ccf2c4bb5f

benzelano commented 13 years ago

Kenneth Kalmer commented

Aaron

The patch also creates another issue with ActiveRecord. I’ve attached a fix to this ticket. Unfortunately I didn’t have the time to dissect your test cases and update the tests for ActiveRecord, but I did write a test file that you can drop into the root of the state_machine project and run.

The test file is gisted here: http://gist.github.com/204224

The long and the short of it is that when combined with the acts_as_audited plugin (http://github.com/kennethkalmer/acts_as_audited) the state changes are impossible to track. Inside write(), the call to state_will_change!() forces the changes hash to have the new state as the before and after values, which is not desirable at all. The audits, which work by way of sweeper or callback doesn’t gain access to the original state before the transition.

Best

benzelano commented 13 years ago

Kenneth,

This has now been fixed: http://github.com/pluginaweek/state_machine/commit/2929f0edc2b3167c0f14d498e04c41eba0b86be2

My apologies for taking so long to get this fix into the main line.