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

Add JavaScript-style "links" #232

Merged
merged 3 commits into from
Nov 4, 2021
Merged

Add JavaScript-style "links" #232

merged 3 commits into from
Nov 4, 2021

Conversation

Pimm
Copy link
Collaborator

@Pimm Pimm commented Nov 4, 2021

API

To get the refunds for a payment, you can do:

const payment = await mollieClient.payments.get('tr_bT5VxTvBtB');
const refunds = await mollieClient.paymentRefunds.list({ paymentId: payment.id });

Or*:

const payment = await mollieClient.payments.get('tr_bT5VxTvBtB', { embed: PaymentEmbed.refunds });
const refunds = payment._embedded?.refunds;

Or (booo! don't do this! ❌):

const payment = await mollieClient.payments.get('tr_bT5VxTvBtB');
const refunds = await fetch(payment._links.refunds.href, );

Now, you can also do it in a way more native to JavaScript:

const payment = await mollieClient.payments.get('tr_bT5VxTvBtB');
const refunds = await payment.getRefunds();

This makes _links and _embedded redundant entirely ‒ and candidates for removal in 4.0.0.

[*] the version with _embedded is not great, as it can produce undefined; whereas the first and last versions will always produce a (potentially empty) array.

Internal

getRefunds internally uses the embedded refunds if present, or fetches them if not.

Caveats

  • Because getRefunds "sometimes" uses refunds which were embedded when the payment was fetched, refunds which were created in the timeframe between the payments.get and getRefunds calls might or might not appear in the array. This might be confusing. However, this library doesn't guarantee to keep any data "fresh". I feel consumers of this library should be aware that data they keep in memory for longer periods of time becomes stale, in general.
  • If getRefunds ends up calling the endpoint (because no refunds were embedded), that endpoint will return a limited number of refunds as it applies pagination. This might be unexpected. I am currently unsure how this compares to embedding refunds. Does the API return all refunds when embedded? I'll look into this before releasing.
  • Because there is no endpoint which returns all payments for an order (of which I know), order.getPayments() has an odd implementation.

To get the refunds for payments, you can do:
  const payment = await mollieClient.payments.get('tr_bT5VxTvBtB');
  const refunds = await mollieClient.paymentRefunds.list({ paymentId: payment.id });
Or (booo! don't do this!):
  const payment = await mollieClient.payments.get('tr_bT5VxTvBtB');
  const refunds = await fetch(payment._links.refunds.href, …);
Now, you can also do it in a way more native to JavaScript:
  const payment = await mollieClient.payments.get('tr_bT5VxTvBtB');
  const refunds = await payment.getRefunds();

This makes _links and _embedded redundant completely, except order._embedded.payments ‒ which is a story in and of itself.
This implementation is odd, not to mention wasteful. This is because the Mollie API currently has no endpoint which returns the payments for an order, at least not one of which I know.
Copy link
Collaborator

@edorivai edorivai left a comment

Choose a reason for hiding this comment

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

LGTM

@Pimm Pimm merged commit 894c54e into master Nov 4, 2021
@Pimm Pimm deleted the pimm/v3.6.0 branch November 4, 2021 12:35
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.

2 participants