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 ...
Comment | File | Size | Author |
---|---|---|---|
#7 | allow-anon-checkout-2597187-7.patch | 2.14 KB | Falco010 |
#5 | allow-anon-checkout-2597187-5.patch | 2.2 KB | mdixoncm |
Comments
Comment #2
mdixoncm CreditAttribution: mdixoncm at ComputerMinds commentedA 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.
Comment #3
mdixoncm CreditAttribution: mdixoncm at ComputerMinds commentedAlso - 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.
Comment #4
mdixoncm CreditAttribution: mdixoncm at ComputerMinds commentedComment #5
mdixoncm CreditAttribution: mdixoncm at ComputerMinds commentedPatch against the 7.x-1.x branch added
Comment #6
visabhishek CreditAttribution: visabhishek as a volunteer and at Azri Solutions commentedChanging status to Needs Review
Comment #7
Falco010I 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:
Isn't this going to be a problem if you are an authenticated user and want to store the uid in the transaction?
Comment #8
james.williams CreditAttribution: james.williams at ComputerMinds commentedThe 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.
Comment #9
gregglesThis seems like a pretty critical bug, right?
Comment #10
Steven Jones CreditAttribution: Steven Jones at ComputerMinds commentedThe code being changed by this ticket got removed in #3135890: IPN does not validate data sent.