activerecord-bitemporal
activerecord-bitemporal copied to clipboard
Prevent touch from saving unsaved changes
See also https://github.com/kufu/activerecord-bitemporal/pull/140
#140 fixes the issue where touch generates irrelevant changes by fixing _update_row. However, I noticed that the current touch implementation has another problem where unsaved changes are saved.
# non-bitemporalized model
tenant = Tenant.create!(name: "test")
tenant.name = "changed"
tenant.touch
tenant.reload.name # => "test"
# bitemporalized model
employee = Employee.create!(name: "Tom")
employee.name = "Jane"
employee.touch
employee.reload.name # => "Jane"
The problem lies in the difference in the implementation of _update_row:
https://github.com/kufu/activerecord-bitemporal/blob/v4.3.0/lib/activerecord-bitemporal/bitemporal.rb#L315
https://github.com/rails/rails/blob/v7.1.0/activerecord/lib/active_record/persistence.rb#L1210
The original implementation only saves the attribute_names, whereas ActiveRecord::Bitemporal::Persistence saves all unsaved changes.
The touch temporarily evacuate unsaved changes in ActiveRecord::AttributeMethods::Dirty#_touch_row. The order in which you call them is important here. In the original, this is done in the following order:
- Call
ActiveRecord::Persistence#_touch_row - Evacuate unrelated changes to the
changesvariable - Apply changes
- Restore changes from the variable
https://github.com/rails/rails/blob/v7.1.0/activerecord/lib/active_record/attribute_methods/dirty.rb#L203-L222
This works fine when only attribute_names are saved, as mentioned above, but it does not work with bitemporalized models.
This PR overrides _touch_row and changes the order as follows:
- Evacuate unrelated changes to the
changesvariable - Call
ActiveRecord::Bitemporal::Persistence#_update_row - Apply changes
- Restore changes from the variable
This solves the problem of unintended changes being saved and changes caused by bi-temporal operations remaining.