-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Feat(Mollie): Prevent duplicate payments #2691
Feat(Mollie): Prevent duplicate payments #2691
Conversation
❌ Deploy Preview for effervescent-donut-4977b2 failed.
|
@@ -12,6 +12,7 @@ export * from './helpers/order-merger/order-merger'; | |||
export * from './helpers/order-modifier/order-modifier'; | |||
export * from './helpers/order-splitter/order-splitter'; | |||
export * from './helpers/order-state-machine/order-state'; | |||
export * from './helpers/order-state-machine/order-state-machine'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this, this.injector.get(OrderStateMachine);
in the Mollie service throws an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it throws an error if you use a deep import to this?
@vendure/core/dist/service/helpers/order-state-machine/order-state-machine
weird, but ok - it's worth exposing anyway.
CreatePaymentMethod.Mutation, | ||
CreatePaymentMethod.Variables | ||
CreatePaymentMethodMutation, | ||
CreatePaymentMethodMutationVariables |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These types were broken, but there is no type check or linting on test files. (Ideally there are, but that requires quite some time to set up...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah the e2e test suites could use a bit of maintenance re TS errors. Not a high prio though for me since the main function (telling us if we broke something) still works. But yeah I tend to fix errors I see whenever I touch an existing file.
customFields: CustomOrderFields & { | ||
mollieOrderId?: string; | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a custom interface instead of a *-types.d.ts
file, because that would also infer customFields.mollieOrderId
in the Stripe, Paypal and braintree plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, makes sense to encapsulate this implementation detail 👍
return new PaymentIntentError( | ||
'Cannot create payment intent for order with customer that has no firstName set', | ||
); | ||
} | ||
if (!order.customer.lastName.length) { | ||
if (!order.customer?.lastName.length) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other checks like Order has order lines
have been removed, because they are checked by canTransitionTo('ArrangingPayment')
.
Failed tasks:
- @vendure/cli:test |
@martijnvdbrug thank you for this PR - clearly it took quite some time & careful effort to get right! |
Description
Short demo video
After talking to Mollie Support, we do the following to prevent duplicate payments:
order.customFields.mollieOrderId
ℹ️ Whenever updating the existing order fails, we continue the flow as before, which is to just create a new order.
Breaking changes
mollieOrderId
field on an order.Checklist
📌 Always:
👍 Most of the time: