Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update README.md #26

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

BjornvdLaan
Copy link

Overview of changes:

  • Change handling of amount.
  • Change step about idempotency to just a best practices tip.
  • Restructured text on 3DS2, making Native option less prominent.

Copy link
Contributor

@Kwok-he-Chu Kwok-he-Chu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice suggestions!

.currency("EUR")
.value(9998L);
paymentRequest.setAmount(amount);
paymentRequest.setAmount(body.getAmount());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best practice: the amount should not be retrieved from the frontend

In a 'real'-world integration, the amount should be fetched from a storage (db), and then shown on the frontend. The dependency is flipped

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats a fair point. Let's stick to your way.

```

</details>
**Best practices:** The Adyen API supports idempotent requests, allowing you to retry a request multiple times while only performing the action once. This helps avoid unwanted duplication in case of failures and retries (e.g., you don't want to charge a shopper twice because they've hit the pay button two times, right?). To learn more about idempotency for payment requests, see [documentation](https://docs.adyen.com/development-resources/api-idempotency/).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've added this as folks mentioned that it's unclear from a SDK-perspective how to pass this key. I'm okay with removing it and referencing the documentation instead

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think we should remove the idempotency paragraph. We should keep the text and refer to the docs.

I just think it shouldn't be a step, especially as we do a suboptimal implementation. It would teach them a bad practice, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense, should we then keep the code snippet?

onAdditionalDetails: async (state, component, actions)=> {
const response = await sendPostRequest("/api/payments/details", state.data);
handleResponse(response, component);
// Uncomment this if you want to add Native 3DS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: I need to make some changes to this function -> We need to accept/reject as follows: https://github.com/adyen-examples/adyen-java-spring-online-payments/blob/main/checkout-example-advanced/src/main/resources/static/dropin.js#L66-L87

We no longer need to check the response.action part

@Kwok-he-Chu
Copy link
Contributor

@BjornvdLaan I've added some of the README changes here: #24, could you have a look? I'll merge it into main afterwards

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.

3 participants