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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gordon’s picture

Status: Active » Needs review

send for testing

Status: Needs review » Needs work

The last submitted patch, ec_store_transaction_save.patch, failed testing.

gordon’s picture

Version: 6.x-4.0-rc15 » 6.x-4.x-dev
Status: Needs work » Needs review
FileSize
917 bytes

Rerolled for 6.x-4.x-dev and testing

gordon’s picture

Status: Needs review » Needs work

This 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.

Xen’s picture

Problem 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.

Xen’s picture

> 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?

Xen’s picture

Status: Needs work » Postponed

I've found a way to work around the problem by using scheduled rules.

However, I'm still concerned about race conditions regarding transaction handling.