#2901030: Receipt Printing added a new commerce_pos_receipt module which overrides the default behaviour to allow a receipt to be printed as part of the order flow. It uses JavaScript to inject itself in between clicking the complete order button and the regular form submit handler. This behaviour is attached by modules/receipt/js/commerce_pos_receipt.js. The problem is that this prevents the order by placed until the receipt is printed. This is not quite the right behaviour has the order should be placed before the receipt is printed. This way regardless of what happens to the browser at that point the order is in the correct state.
Discussed with @smccabe and decided to explore whether or not we could add a redirect so the order can be completed before printing the receipt and then the order can be printed from that page.
t.b.d.
t.b.d.
None
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 2935465-8.patch | 32.99 KB | alexpott |
| #6 | 2935465-6.patch | 40.39 KB | alexpott |
| #3 | 2935465-3.patch | 25.53 KB | alexpott |
Comments
Comment #2
shabana.navas commentedI would definitely welcome this change as well. The workflow just seemed odd. The order should be completed on submit right away and any other steps like printing receipts should come after. Maybe add a subsequent submit handler to redirect and invoke the printing of the receipt.
Comment #3
alexpottHere's some work that decouples printing the receipt from completing the order. It also separates emailing from printing and generally splits up the code into different units.
I'm going to add a test based on the existing javascript test for the emailing a receipt pathway as that should no longer use the jquery library - or atleast it won't when I'm done.
Comment #5
alexpottGiven this could lead to a user thinking an order has been completed when it hasn't I think this is critical and should block the next alpha / beta release.
Comment #6
alexpottSo here are some tests for the receipt functionality. They revealed that atm we'd be sending two email receipts to the customer because we're not disabling the sendReceipt option for the pos order type! Tests ftw.
The patch attached is built on top of #2935353: Site error during "Show receipt" operation. Given the test changes that issue makes I think it should go in first. No interdiff ... because that'd be a mess.
Comment #8
alexpottThe blocker has landed so now we can have a smaller and simpler patch.
One thought is that should we be overriding commerce's receipt email in this way? Would it be better to decorate the event listener and work with that. It is tricky because the receipt from POS is probably very different from what a receipt generated from an online transaction should look like.
Comment #9
smccabe commentedAfter talking with Alex and Bojan, it looks like we'll likely add a change into Commerce core to facilitate this easier. That still needs to be confirmed, but is currently the most likely option.