Skip to content

Conversation

dbaneman
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 24.137% when pulling 058f98b on dbaneman:payment-details-request-options into 9b8afeb on Adyen:develop.

@martinsrenato
Copy link
Contributor

Hi @dbaneman ,

Thanks for your PR.
Can you explain a little bit what's the use case behind this change?
Adding RequestOptions shouldn't break anything, but we just want to understand what's the reasoning behind that.

Thanks!
Renato
Adyen

@dbaneman
Copy link
Contributor Author

Hi @martinsrenato ,

The use case is that we'd like to use the Idempotency-Key header on /payment/details requests when completing the 3DS flow. Specifically, once 3DS authentication has succeeded, we need to send a /payment/details request to actually authorize the payment, and we want that /payment/details request to be idempotent so it can be safely retried if there's a connection error or request timeout. I tested this on the test environment and it seemed to work as expected; when I sent the request the second time, I got back the response from the original request.

Let me know if I'm misunderstanding anything here. Thanks!

Cheers,
Dan

@martinsrenato martinsrenato self-requested a review January 20, 2020 13:29
@AlexandrosMor AlexandrosMor self-requested a review January 22, 2020 12:43
@AlexandrosMor
Copy link
Contributor

Hello @dbaneman,

Thank you for the PR, Is it possible to add some unit tests related to your changes?

kind regards,
Alexandros

@dbaneman
Copy link
Contributor Author

Hi @AlexandrosMor,

I don't see any existing unit tests around RequestOptions. (I do see a few tests that mock out RequestOptions, but nothing that actually asserts that its values are used as expected.) So that might be a somewhat larger change, which might be more effort than it's worth, given that the proposed change is quite small? But if you're able to point me in the right direction on an existing test to modify or emulate, I can give it a shot.

Thanks,
Dan

@martinsrenato
Copy link
Contributor

Hi @dbaneman ,

Indeed, your user case is valid. Without the idempotency key, you will get an error ("request already processed or in progress") when retrying the /payment/details request with data already sent before. With the key you can retrieve the response from the original request.

Just be sure to use distinct keys for every distinct request, since the idempotency keys do not differentiate between the endpoints being called. For example, if a /payments request is done with idempotency key KEY-1 and a /payment/details request is done with the same key KEY-1, you will get on the second request the same response you had for the first request.
Also be sure to check Adyen docs for other general instructions on idempotency on our APIs.

We will merge your PR and soon include it on our next release.

Thanks for the contribution :)

Renato
Adyen

@AlexandrosMor AlexandrosMor merged commit 4714633 into Adyen:develop Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants