View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0006466 | Taler | specification | public | 2020-08-09 14:15 | 2021-08-24 16:23 |
| Reporter | Christian Grothoff | Assigned To | Christian Grothoff | ||
| Priority | urgent | Severity | feature | Reproducibility | N/A |
| Status | closed | Resolution | fixed | ||
| Platform | i7 | OS | Debian GNU/Linux | OS Version | squeeze |
| Product Version | git (master) | ||||
| Target Version | 0.8 | Fixed in Version | 0.8 | ||
| Summary | 0006466: enable long polling for refund pending in GET /orders/{order_id} page | ||||
| Description | The page currently returns us a 'refund_pending' flag. We should have a wait to long-poll for this flag to change to 'false'. Basically: extra query parameter "await_refund_obtained" that, if 'true' means we wait until timeout_ms _or_ the wallet obtained the refund. | ||||
| Steps To Reproduce | This is for the design document 007 specification, where the HTML page showing the refund URI should long-poll and stop showing the QR code once the wallet obtained the refund. | ||||
| Tags | No tags attached. | ||||
|
|
Spec added in 8a3b86d..3f9f0bb (docs.git). |
|
|
If I understand correctly, a refund is no longer pending once the merchant sends a refund request to the exchange and it runs successfully. So if 'await_refund_obtained' is 'yes', then we should make these requests as usual, but suspend and wait for the exchange's replies, and if all are successful, then reply to the client, right? If not, should we reply with the error we got from the exchange, or keep retrying? |
|
|
Oh, this is an interesting mess. Basically, we want to NOT trigger the exchange interaction _unless_ it is really a Taler wallet requesting the GET /orders/{order_id} page. So _neither_ for the HTML version _nor_ for the long-polling request by (with await_refund_obtained=true) should we actually initiate the exchange interaction. Only once the "real" wallet makes the request (and it won't use await_refund_obtained=true) should we interact with the exchange. And then, I would say we _could_ return from the long polling immediately (instead of waiting for those requests to the exchange to succeed or fail). I'm calling it a mess because the logic of what the GET does diverges, and because we do have the 'odd' situation that a GET (from the wallet) does cause a significant state change. Florian: maybe we should have the wallet do a POST /orders/{order_id}/refund to obtain the refund (for the first time, yielding the refunds[] array just like StatusPaid does) --- and until such a request happens, have the GET only return 'refund_pending' but with an empty refunds[] array? That would simplify the already complex 'GET' logic, avoid GET changing the state like this, and would also make for a reasonable symmetriy with the /private/orders/{order_id} situation. And you can relatively easily detect that you should do a POST because you are given a taler://refund/-URI ;-) |
|
|
Should have Florian's feedback first before proceeding. |
|
|
Yes, I actually like separating out the refunds like this. I would even prefer *always* having to do "POST /orders/{order_id}/refund". The "/orders/{order_id}" can just indicate that there *is* a refund, but not return the whole list. Makes the API a bit cleaner by separating concerns. |
|
|
Ok, so then it is agreed. Jonathan: feel free to start on this (please put the new specification entry point under 'refunds', not under 'payments'). If you can't finish on Friday (today!), please update the bug on the status, I may take over on Saturday/Sunday (where I understand you are 'off') as we want this addressed ASAP. |
|
|
Actually this weekend isn't busy at all for me, so I should be fine to work on this over the weekend. I'll have the spec added soon. |
|
|
Spec added in 8fef03b..b86bb33. Does it look ok? |
|
|
Let's move the 'h_contract' of the POST /orders/$ORDER_ID/refund into the body of the upload (with POST we should have an upload / JSON request body, and as this is mandatory, it fits much better there). I think we should also avoid the 'refunded' boolean in the WalletRefundResponse and instead have an (empty) "204 No content" reply to indicate that there are no refunds. The 'merchant_pub' can now be removed from the 'StatusPaid' of the GET request, it was only there to allow verification of the refund signatures. |
|
|
Ok, thanks for the feedback. I'll make those changes and then begin implementing it. |
|
|
I did some early work on the implementation. Tomorrow I'll fix duplicated/missing code and write tests and the lib implementation. |
|
|
I've fixed some major general (not refund-related) issue in the long polling logic today: we didn't re-do the DB transaction upon resuming, and would instantly go back to sleep. This is fixed in git master. Alas: I did this while testing the payment case, did not touch refunds. |
|
|
jonathan: any progress here? |
|
|
Looking at the code, it seems the main logic is there. What was missing is a test. Added testing commands in Git bf9641e..e98ca0d. But NOT yet integrated with test run. |
|
|
Test added and passes with ca347c3..33e80b8 |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2020-08-09 14:15 | Christian Grothoff | New Issue | |
| 2020-08-09 14:15 | Christian Grothoff | Status | new => assigned |
| 2020-08-09 14:15 | Christian Grothoff | Assigned To | => jonathanbuchanan |
| 2020-08-09 14:16 | Christian Grothoff | Steps to Reproduce Updated | |
| 2020-08-10 19:48 | jonathanbuchanan | Note Added: 0016584 | |
| 2020-08-13 21:52 | jonathanbuchanan | Note Added: 0016598 | |
| 2020-08-14 09:15 | Christian Grothoff | Note Added: 0016600 | |
| 2020-08-14 09:15 | Christian Grothoff | Assigned To | jonathanbuchanan => Florian Dold |
| 2020-08-14 09:15 | Christian Grothoff | Status | assigned => feedback |
| 2020-08-14 09:15 | Christian Grothoff | Note Added: 0016601 | |
| 2020-08-14 09:29 | Florian Dold | Note Added: 0016602 | |
| 2020-08-14 09:31 | Christian Grothoff | Assigned To | Florian Dold => jonathanbuchanan |
| 2020-08-14 09:34 | Christian Grothoff | Status | feedback => assigned |
| 2020-08-14 09:34 | Christian Grothoff | Note Added: 0016603 | |
| 2020-08-15 02:29 | jonathanbuchanan | Note Added: 0016634 | |
| 2020-08-15 03:05 | jonathanbuchanan | Note Added: 0016635 | |
| 2020-08-15 10:36 | Christian Grothoff | Note Added: 0016637 | |
| 2020-08-15 23:07 | jonathanbuchanan | Note Added: 0016638 | |
| 2020-08-16 08:50 | jonathanbuchanan | Note Added: 0016639 | |
| 2020-08-16 17:46 | Christian Grothoff | Note Added: 0016643 | |
| 2020-09-05 01:46 | Christian Grothoff | Note Added: 0016867 | |
| 2020-09-05 13:45 | Christian Grothoff | Priority | high => urgent |
| 2020-09-10 18:46 | Christian Grothoff | Target Version | 0.8 => 0.8.1 |
| 2020-10-11 20:58 | Christian Grothoff | Assigned To | jonathanbuchanan => Christian Grothoff |
| 2020-10-14 23:09 | Christian Grothoff | Note Added: 0017017 | |
| 2020-10-20 20:21 | Christian Grothoff | Note Added: 0017024 | |
| 2020-10-20 20:22 | Christian Grothoff | Status | assigned => resolved |
| 2020-10-20 20:22 | Christian Grothoff | Resolution | open => fixed |
| 2020-10-20 20:22 | Christian Grothoff | Fixed in Version | => 0.8 |
| 2020-10-20 20:22 | Christian Grothoff | Target Version | 0.8.1 => 0.8 |
| 2021-08-24 16:23 | Christian Grothoff | Status | resolved => closed |
| 2024-01-12 14:02 | Christian Grothoff | Category | merchant backend API (HTTP specification) => specification |
