ar_transaction_changes icon indicating copy to clipboard operation
ar_transaction_changes copied to clipboard

Changes to record in after_commit change transaction_changed_attributes

Open dylanahsmith opened this issue 8 years ago • 2 comments

The main use case for the gem is to get all the committed changes in the after_commit callback, however, changes to the record in the after_commit callbacks affect transaction_changed_attributes.

For instance, unsaved changes will be added to transaction_changed_attributes. If the record were saved in the after_commit callback, then transaction_changed_attributes would be cleared. If the changes happen in one after_commit callback then they will affect the following after_commit callbacks.

I actually noticed this problem while looking into a similar issue in active record with saved_changes where it isn't safe to assume that saved_changes in an after_save callback is giving the changes for the save that caused the callback to be called, since the record could have been saved in a preceding after save callback.

I would recommend avoiding changing the record in after commit callbacks as a workaround if anyone encounters this issue.

dylanahsmith avatar Aug 30 '17 21:08 dylanahsmith

Once we fix this behaviour, we should expose it under the name committed_changes ~~and raise if the method is called outside of an after_commit callback~~. We could deprecated transaction_changed_attributes and remove it in the next major release.

dylanahsmith avatar Apr 02 '20 20:04 dylanahsmith

raise if the method is called outside of an after_commit callback

Actually, we may want to be able to get the transaction changes for another record during an after_commit hook, where we may not know if there are any committed changes.

For example, it looks like the identity_cache gem does this to expire parent records when an embedded one changes. If the parent was committed in the same transaction, then it shouldn't need to expire that record from the child's after_commit callback, since the parent's after_commit callbacks will also be called to expire its cache entry.

Although, I'm not sure if that even works properly right now. We clear the transaction changes after the after_commit callbacks are called for a given record, which may be before the after_commit callbacks are called for another record. So whether or not we would see the transaction changes for another record depends on the order the after_commit callbacks are called in.

dylanahsmith avatar Apr 03 '20 14:04 dylanahsmith