This is important for offsite payments, where the payment might arrive after the order has been placed.

Comments

bojanz created an issue. See original summary.

dom.’s picture

Using transition events from state machine, would the event
commerce_order.fulfill.post_transition
be enough to execute code when an order has been properly fully paid (from onsite or offsite payment) ? Specially since fulfill seems to be the transition state from fulfillment to completed according to commerce_order.workflows.yml
Can I use this in a 'Commerce License'-like context ?

mglaman’s picture

I don't think commerce_order.fulfill.post_transition is enough. I wonder if we need something in the Payment storage handler. On saving (or post save) of a transaction, we recalculate the order's balance. If its zero then fire the event. We cannot assume the fulfill state. We only assume draft and some sort of place event.

vasike’s picture

Assigned: Unassigned » vasike

i'll try.

bojanz’s picture

This is blocked on having a payment balance on the order entity, for which we have a PR up.

vasike’s picture

dom.’s picture

Any updates ?

jackbravo’s picture

Status: Active » Needs work
Related issues: +#2862339: No events thrown for Payment CRUD

I'm working on this right now. But it depends on issue [2862339](https://www.drupal.org/node/2862339#comment-12255848). That one should go first. I expect to provide a patch, based on that one in a couple of days.

jackbravo’s picture

There is a PR now for this issue, which includes the changes for #2804227: Add getTotalPaid() and getBalance() methods to orders and #2862339: No events thrown for Payment CRUD. It is here: https://github.com/drupalcommerce/commerce/pull/792

It still needs work, it needs some tests.

jackbravo’s picture

Status: Needs work » Needs review
StatusFileSize
new23.82 KB
new43.72 KB
Paul B’s picture

Can we also have an option to send the confirmation mail when the order is paid, instead of when it is placed?

Status: Needs review » Needs work

The last submitted patch, 10: order-paid-in-full-event-2856586.patch, failed testing. View results

vasike’s picture

Assigned: vasike » Unassigned
dom.’s picture

Can I have an update on this and question at #11 ? Also may I help somehow ?

bojanz’s picture

Assigned: Unassigned » bojanz

#2804227: Add getTotalPaid() and getBalance() methods to orders has landed, this issue can now proceed. We need the event to be an order event, not a payment event.

#11 is out of scope for this issue, feel free to discuss it in a followup.

bojanz’s picture

Title: Add an "order paid in full" event » Add an "order paid" event
Priority: Normal » Major
Status: Needs work » Needs review
StatusFileSize
new5.79 KB

Internal discussions concluded that it's fine to call it the "order paid" event, and have an $order->isPaid() helper, instead of doing "order paid in full" and $order->isPaidInFull(). Others (spree/shopify) already use just "paid", with "partially paid" used for partial payments. So we should be fine with brevity, as long as the docblocks are longer (using "fully paid" there).

Additional requirements:
- The event must only be dispatched once, matching 1.x semantics.
- We must prevent the event from firing on orders completed before Commerce 2.10.
Right now this scenario is possible:
1) Order was completed at some point.
2) Order gets its total_paid field updated (triggered by a payment update or the manual code I put in the change record)
3) The order now has a zero balance for the first time, and is considered paid. Event fires, doing something unexpected (customer gets another email, a deploy is triggered, etc).

We've also discussed whether we should move the order receipt email to the "paid" event, decided to leave it on the order placed event for now, since that matches the 1.x logic. But happy to discuss that in a followup issue.

Attached is a patch written from scratch, no tests yet.

Status: Needs review » Needs work

The last submitted patch, 16: 2856586-16.patch, failed testing. View results

  • bojanz committed 3db1c03 on 8.x-2.x authored by jackbravo
    Issue #2856586 by jackbravo, bojanz: Add an "order paid in full" event
    
bojanz’s picture

Status: Needs work » Fixed

Fixed the test fail, added test coverage.

Gave the commit credit to jackbravo since he spent so much time on this issue.

Thanks, everyone!

bojanz’s picture

Updating credits.

dom.’s picture

@bojanz about #16
Does this means we can't yet rely on this event to conclude an order is paid and finished once and for all ? If there are issues of multi-firing known, do you have a sub-issues / follow-up to send me to so I can follow it please ?

bojanz’s picture

@Dom.
The point of my reroll was to eliminate the multi-firing issues, so it should be fine now.

dom.’s picture

@bojanz : Thanks a lot, for the answer and all of your work !

dom.’s picture

Again some more questions, sorry and thanks for your help !
After testing this in version 2.10: The event seems to be fired before the order is actually made : is that expected ?
- if I have a promotion that is set to "100% free off total" and a coupon called FREE for that promotion
- I have a 100€ product sent to my cart which now worth 100€ and I start proceeding with the order
- in the order process, at the stage of entering information, I have a the coupon redemption pane in the sidebar. If I enter the coupon here, the total balance goes to 0, and the OrderEvents::ORDER_PAID is fired.
But at that stage: the client has not paid He maybe just "tried" his coupon, or will change his mind (and in that case the event is not sent anymore) so it seems a too early stage to fire this event.
Maybe the condition for being paid should be both : "balance get's to 0" and "order is placed" rather than just checking balance in the isPaid() helper.

Is the firing at that stage supposed to happen ? You said at #16, that receipt could be sent on that event, but it can't for the order is not actually made here: customer can change his mind.

I have cases with an offsite-gateway that payment gets refused because 3DS fails (case 3DS is not valid: the user did not had his for with him to get the verification code in time), and yet in that case an email receipt is sent. I hoped OrderEvents::ORDER_PAID would solve my issue but maybe I do not understood properly it's usage. Could you advice on that please ?

bojanz’s picture

- in the order process, at the stage of entering information, I have a the coupon redemption pane in the sidebar. If I enter the coupon here, the total balance goes to 0, and the OrderEvents::ORDER_PAID is fired.

That's definitely not intended. But we can't add a "order is placed" check because the regular payment done via PaymentProcess also happens before the order is placed. Instead, we should expand the isPaid() check to be "balance is 0 AND total_paid is not zero". That will resolve the problem.

Can you open a followup issue for this?

dom.’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.