I've not fully debugged this - but thought I would raise early.

It looks like no transactions are being logged against anonymous orders. This then prevents the commerce_payment_order_paid_in_full from firing, with obvious consequences.

I can see from the code that there is an explicit check on the user object, and that the transaction is only saved on the order IF we have a logged in user

      if (!empty($user->uid)) {
        commerce_payment_transaction_save($transaction);
      }

I need to work this through and see if there is a good reason for this - i.e. is there a security issue with adding the transaction when we have no user account (there shouldn't be I don't think) ... but if anyone has any thoughts ...

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mdixoncm created an issue. See original summary.

mdixoncm’s picture

A quick review of some other payment modules, I can't see any examples of transactions not being saved for the anon user.

Looking at the transaction entity - it looks like if no UID is provided then the uid for the transaction will default to 0 - which seems sensible.

mdixoncm’s picture

Also - checking the commerce_payment_example module, specifically commerce_payment_example_transaction - it looks like there is no attempt to check the user is logged in at this point.

mdixoncm’s picture

mdixoncm’s picture

Patch against the 7.x-1.x branch added

visabhishek’s picture

Status: Active » Needs review

Changing status to Needs Review

Falco010’s picture

I ran into the same issue mentioned above. I could not apply this patch to the 7.x-2.x branch which i was working with. So created a reroll of this patch and can confirm this is working now.

However i am wondering why you @mdixoncm removed:

$transaction->uid = $user->uid;

Isn't this going to be a problem if you are an authenticated user and want to store the uid in the transaction?

james.williams’s picture

The entity controller for payment transactions, CommercePaymentTransactionEntityController, populates the 'uid' with the logged-in user's ID, in its 'create' method. So it doesn't really matter at all whether that particular line is removed or kept. The other hunks are the key, to ensure the transaction is saved regardless of the user ID being empty or not.

greggles’s picture

Priority: Normal » Critical

This seems like a pretty critical bug, right?

Steven Jones’s picture

Status: Needs review » Closed (duplicate)

The code being changed by this ticket got removed in #3135890: IPN does not validate data sent.