View Issue Details

IDProjectCategoryView StatusLast Update
0006466Talermerchant backend API (HTTP specification)public2020-10-20 20:22
ReporterChristian Grothoff Assigned ToChristian Grothoff  
PriorityurgentSeverityfeatureReproducibilityN/A
Status resolvedResolutionfixed 
Platformi7OSDebian GNU/LinuxOS Versionsqueeze
Product Versiongit (master) 
Target Version0.8Fixed in Version0.8 
Summary0006466: enable long polling for refund pending in GET /orders/{order_id} page
DescriptionThe 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 ReproduceThis 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.
TagsNo tags attached.

Activities

jonathanbuchanan

2020-08-10 19:48

developer   ~0016584

Spec added in 8a3b86d..3f9f0bb (docs.git).

jonathanbuchanan

2020-08-13 21:52

developer   ~0016598

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?

Christian Grothoff

2020-08-14 09:15

manager   ~0016600

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 ;-)

Christian Grothoff

2020-08-14 09:15

manager   ~0016601

Should have Florian's feedback first before proceeding.

Florian Dold

2020-08-14 09:29

manager   ~0016602

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.

Christian Grothoff

2020-08-14 09:34

manager   ~0016603

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.

jonathanbuchanan

2020-08-15 02:29

developer   ~0016634

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.

jonathanbuchanan

2020-08-15 03:05

developer   ~0016635

Spec added in 8fef03b..b86bb33. Does it look ok?

Christian Grothoff

2020-08-15 10:36

manager   ~0016637

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.

jonathanbuchanan

2020-08-15 23:07

developer   ~0016638

Ok, thanks for the feedback. I'll make those changes and then begin implementing it.

jonathanbuchanan

2020-08-16 08:50

developer   ~0016639

I did some early work on the implementation. Tomorrow I'll fix duplicated/missing code and write tests and the lib implementation.

Christian Grothoff

2020-08-16 17:46

manager   ~0016643

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.

Christian Grothoff

2020-09-05 01:46

manager   ~0016867

jonathan: any progress here?

Christian Grothoff

2020-10-14 23:09

manager   ~0017017

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.

Christian Grothoff

2020-10-20 20:21

manager   ~0017024

Test added and passes with ca347c3..33e80b8

Issue History

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 View Revisions
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