After a long debugging session, trying to work out why a transaction was getting overwritten with earlier values, I came up with the attached patch, which solves some of the problems.
Looking in CVS, it looks like someone had fixed the ec_store_event_transactions_bef_save invocation, but not the ec_store_event_transactions_save invocation. I couldn't see a reason why they should differ.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 0001-697832-ec_store_event_transactions_save-rules-overwr.patch | 917 bytes | gordon |
| ec_store_transaction_save.patch | 1.1 KB | xen |
Comments
Comment #1
gordon commentedsend for testing
Comment #3
gordon commentedRerolled for 6.x-4.x-dev and testing
Comment #4
gordon commentedThis will not work as when you pass by ref the function rules event will not save the changes.
If you could create a test which checked this and I will be able be able to look at this better.
Comment #5
xen commentedProblem is, it's rather hard to make a test case. The problem surfaces when you have an transaction_save rule that triggers an action that might cause something to load and save the transaction again...
Example: I have an an action that has as condition that the transaction is shipped, and only quickpay receipts remain pending, which triggers an action that captures the payments. When the payments is captured, the receipt is saved, which triggers allocation on the transactions, which results in the original transaction being saved again.
As ec_store_transaction_load caches the transaction and ensures that everybody gets a reference to the same object, this works, save for the rule overwriting it afterwards.
"This will not work as when you pass by ref the function rules event will not save the changes."
Well, wouldn't a drupal_write_record('ec_transaction', $txn, 'txnid'); solve that? I find it curious that ec_store_rules_data_type_transaction->save only saves the transaction if pass_by_ref is set.
Comment #6
xen commented> Well, wouldn't a drupal_write_record('ec_transaction', $txn, 'txnid'); solve that?
Another alternative would be to restart ec_store_transaction_save if the transaction object changed. That does risk infinite recursion, but if the transaction changed because of something that happened in the after save rules, shouldn't it really trigger presave rules all over?
Comment #7
xen commentedI've found a way to work around the problem by using scheduled rules.
However, I'm still concerned about race conditions regarding transaction handling.