Mollie: Allow forcefully updating payment status in case webhooks are delayed #4104
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a GraphQL mutation Changes
Sequence DiagramsequenceDiagram
actor Client
participant Resolver as MollieCommonResolver
participant Service as MollieService
participant Mollie as Mollie API
participant DB as OrderService/Database
Client->>Resolver: syncMolliePaymentStatus(orderCode)
Resolver->>Service: syncMolliePaymentStatus(ctx, orderCode)
Service->>DB: findOrderByCode(orderCode)
DB-->>Service: Order
Service->>Service: getMollieClient(ctx)
Service->>DB: getPaymentMethodByCode(...)
DB-->>Service: PaymentMethod
Service->>Mollie: list payments for order (payments API)
Mollie-->>Service: payments list
loop for each payment
Service->>Service: handleMolliePaymentStatus(ctx, paymentId)
Service->>DB: update order/payment state
DB-->>Service: updated Order
end
Service-->>Resolver: Order
Resolver-->>Client: Order
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
martijnvdbrug
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/payments-plugin/src/mollie/mollie.plugin.ts (1)
122-199: Fix doc typos/wording in the new “Force payment status update” docs.
Notably:redirectUrl``is(double backtick) and a couple awkward phrases (“use call”).Proposed doc fixes
- * PaymentMethod was given the code "mollie-payment-method". The `redirectUrl``is the url that is used to redirect the end-user + * PaymentMethod was given the code "mollie-payment-method". The `redirectUrl` is the URL that is used to redirect the end-user @@ - * To finalize an order in `ArrangingAdditionalPayment` status, you can use call the `createMolliePaymentIntent` mutation again with an additional `orderId` as input. + * To finalize an order in `ArrangingAdditionalPayment` status, you can call the `createMolliePaymentIntent` mutation again with an additional `orderId` as input.
🤖 Fix all issues with AI agents
In
@packages/core/src/config/order/default-order-item-price-calculation-strategy.ts:
- Line 1: The file default-order-item-price-calculation-strategy.ts contains a
blanket "/* eslint-disable */" that must be removed; delete that directive, run
the linter to see any reported issues in the
DefaultOrderItemPriceCalculationStrategy implementation (or any exported
functions/classes in that file), fix the underlying lint errors/warnings rather
than silencing them (e.g., adjust imports, types, unused vars, formatting), run
the test suite and eslint again, and commit the cleaned file so production code
no longer bypasses ESLint checks.
- Around line 29-38: Remove the stray debug/test block that calls
MyCustomSearchPLugin.init(...) — specifically eliminate the entire module-level
snippet referencing MyCustomSearchPLugin, its hardcoded apiKey value, and the
excludeProductsFromIndexing callback; do not replace it with any placeholder API
key or top-level execution. If search plugin initialization is required, move it
to the appropriate runtime/init path (not in this core module) and ensure the
real plugin is properly imported and configured there with secure key handling.
- Around line 15-27: The calculateUnitPrice function contains a hardcoded debug
bypass that makes items free for ctx.channel.code === 'secret-channel'; remove
that conditional branch and restore the original logic so calculateUnitPrice
always returns price: productVariant.listPrice and priceIncludesTax:
productVariant.listPriceIncludesTax (also delete the "Everything for free!"
comment and any test-only channel shortcuts introduced).
In @packages/payments-plugin/src/mollie/api-extensions.ts:
- Around line 51-59: Fix typos and sentence casing in the GraphQL docstring for
syncMolliePaymentStatus: change "use this mutation when the Mollie webhook is
delayed and youy want to force update the order status." to a properly
capitalized and spelled sentence such as "Use this mutation when the Mollie
webhook is delayed and you want to force-update the order status." Ensure other
sentences start with uppercase letters and keep the note about throwing a
ForbiddenError for unauthenticated calls when the order is not yet settled;
update wording consistently in the extend type Mutation block (references:
createMolliePaymentIntent, syncMolliePaymentStatus).
In @packages/payments-plugin/src/mollie/graphql/generated-shop-types.ts:
- Around line 1935-1941: The generated GraphQL types include a phantom field
updateOrderStatusFromMollie which does not exist in the actual schema;
regenerate the types so they match the true schema that exposes
syncMolliePaymentStatus (the implemented resolver in mollie.common-resolver.ts
and used by mollie.plugin.ts); run your GraphQL codegen against the current
schema (or re-run the generation script) to remove updateOrderStatusFromMollie
from packages/payments-plugin/src/mollie/graphql/generated-shop-types.ts and
ensure syncMolliePaymentStatus: Order is present instead.
In @packages/payments-plugin/src/mollie/mollie.common-resolver.ts:
- Around line 25-31: The syncMolliePaymentStatus GraphQL mutation is missing an
authorization decorator; add an @Allow(Permission.Owner, Permission.UpdateOrder)
decorator above the syncMolliePaymentStatus method (above the @Mutation()
decorator) to permit shop customers and admins to call it, and import Allow and
Permission if they are not already imported; keep the method body as-is so
service-level checks (canAccessOrder) remain in place.
In @packages/payments-plugin/src/mollie/mollie.service.ts:
- Around line 448-456: The filter currently matches any authorized payment
because of operator precedence; update the filter used on
mollieClient.payments.iterate().take(500).filter so that the orderCode check
applies to both statuses — require payment.description === orderCode AND
(payment.status === PaymentStatus.paid OR payment.status ===
PaymentStatus.authorized) — ensuring only payments for the given orderCode are
returned.
🧹 Nitpick comments (2)
packages/payments-plugin/src/mollie/mollie.common-resolver.ts (1)
26-31: Prefer returningnulloverundefinedfor a nullable GraphQL field (clarity).
It likely works, butnullis the GraphQL-native “no value” and avoids adapter edge-cases.Proposed tweak
- ): Promise<Order | undefined> { - return this.mollieService.syncMolliePaymentStatus(ctx, orderCode); + ): Promise<Order | null> { + return (await this.mollieService.syncMolliePaymentStatus(ctx, orderCode)) ?? null; }packages/payments-plugin/src/mollie/mollie.service.ts (1)
449-450: The.take(500)limit is a reasonable safeguard.This prevents unbounded iteration, which is good for performance. However, for high-volume merchants, 500 might not be enough to find older orders. Consider making this configurable or documenting this limitation.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/core/src/config/order/default-order-item-price-calculation-strategy.tspackages/payments-plugin/e2e/graphql/admin-queries.tspackages/payments-plugin/e2e/graphql/shared-definitions.tspackages/payments-plugin/e2e/mollie-dev-server.tspackages/payments-plugin/e2e/mollie-payment.e2e-spec.tspackages/payments-plugin/src/mollie/api-extensions.tspackages/payments-plugin/src/mollie/graphql/generated-shop-types.tspackages/payments-plugin/src/mollie/mollie.common-resolver.tspackages/payments-plugin/src/mollie/mollie.controller.tspackages/payments-plugin/src/mollie/mollie.plugin.tspackages/payments-plugin/src/mollie/mollie.service.tspackages/payments-plugin/src/stripe/stripe.controller.ts
💤 Files with no reviewable changes (1)
- packages/payments-plugin/e2e/graphql/admin-queries.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/payments-plugin/src/mollie/mollie.controller.ts (3)
packages/payments-plugin/src/stripe/stripe.controller.ts (1)
Controller(29-208)packages/payments-plugin/src/mollie/mollie.plugin.ts (1)
MolliePluginOptions(28-85)packages/core/src/api/common/request-context.ts (1)
req(299-301)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: publish_install (macos-latest, 22.x)
- GitHub Check: e2e tests (22.x, mariadb)
- GitHub Check: e2e tests (24.x, sqljs)
- GitHub Check: e2e tests (24.x, mysql)
- GitHub Check: e2e tests (24.x, postgres)
- GitHub Check: e2e tests (20.x, mysql)
- GitHub Check: e2e tests (22.x, mysql)
- GitHub Check: e2e tests (20.x, sqljs)
- GitHub Check: e2e tests (20.x, postgres)
- GitHub Check: publish_install (ubuntu-latest, 20.x)
- GitHub Check: e2e tests (20.x, mariadb)
- GitHub Check: publish_install (macos-latest, 24.x)
- GitHub Check: build (20.x)
- GitHub Check: publish_install (windows-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: publish_install (windows-latest, 24.x)
- GitHub Check: publish_install (macos-latest, 20.x)
- GitHub Check: publish_install (ubuntu-latest, 24.x)
- GitHub Check: publish_install (windows-latest, 20.x)
- GitHub Check: build (24.x)
🔇 Additional comments (15)
packages/payments-plugin/src/stripe/stripe.controller.ts (2)
39-39: LGTM: Formatting improvement.The constructor formatting is cleaner.
127-131: LGTM: Improved readability.The multi-line formatting makes the createContext arguments clearer without changing behavior.
packages/payments-plugin/e2e/graphql/shared-definitions.ts (1)
51-60: LGTM: E2E mutation document is well-scoped (id/code/state).packages/payments-plugin/src/mollie/mollie.plugin.ts (2)
77-85: Confirm@since 3.6.0matches the actual release/versioning for this package.
If this repo versioning differs (e.g., monorepo tag vs plugin tag), this doc tag will age poorly.
196-199: Validate the “last 500 Mollie payments” limitation + recommended timing (“~10 seconds”).
If this is a hard Mollie API constraint, it’s worth linking to an upstream reference or clarifying it’s an implementation detail that may change.packages/payments-plugin/src/mollie/graphql/generated-shop-types.ts (1)
2066-2069: LGTM: args type forsyncMolliePaymentStatus(orderCode)is reasonable—assuming it matches the schema.packages/payments-plugin/e2e/mollie-payment.e2e-spec.ts (2)
617-674: New e2e test for manual status sync looks good.The test correctly mocks both the payments list endpoint and the individual payment GET endpoint, which aligns with the
syncMolliePaymentStatusimplementation that iterates payments and then callshandleMolliePaymentStatusfor each.One observation: the test verifies the happy path where a paid payment exists. Consider adding additional test cases for edge cases:
- No matching payments found for the order code
- Payment in
authorizedstate (to test the other branch)
348-350: Mock endpoint updated to match implementation change.The change from
resource=orderstoresource=paymentsaligns with the corresponding change inMollieService.getEnabledPaymentMethods()at line 429 ofmollie.service.ts.packages/payments-plugin/e2e/mollie-dev-server.ts (2)
102-103: Helpful comment for manual testing of the new feature.The commented-out line provides a clear way to test the
syncMolliePaymentStatusmutation when webhooks are disabled. Consider adding a brief note about what to do after uncommenting (e.g., "then use the syncMolliePaymentStatus mutation to update order status").
110-118: Inline mutation definition is appropriate for dev server.Moving the mutation definition inline keeps the dev server self-contained. The mutation structure correctly matches the expected input type.
packages/payments-plugin/src/mollie/mollie.service.ts (3)
324-328: Return value change is consistent across status handling paths.The method now correctly returns the refreshed order after settling existing payments, which aligns with the new return type
Promise<Order | undefined>.
487-500: New helper method cleanly encapsulates Mollie client creation.The
getMollieClientmethod consolidates API key retrieval and client instantiation, reducing duplication. Error handling for missing payment methods and API keys is appropriate.
440-444: Access control check occurs after payment processing; state changes are not gated by authorization.The null check at lines 442-444 only verifies order existence, not whether the caller has permission to access it. The actual access control via
orderByCodeAccessStrategy.canAccessOrder()is applied at lines 475-480, afterhandleMolliePaymentStatus()has already processed payments and potentially modified the order state (line 460). An unauthorized caller cannot obtain the final result, but can trigger internal state mutations before being denied access.Move the access control check to immediately after the initial
findOneByCoderetrieval to prevent unauthorized state changes.packages/payments-plugin/src/mollie/mollie.controller.ts (2)
25-30: Early exit for disabled webhook processing is clean.The implementation correctly logs a warning and exits early when
disableWebhookProcessingis enabled. UsingLogger.warnwith a return statement works because the method's return type isPromise<void>.One consideration: this is a global setting. If multi-channel setups need per-channel webhook control, this would need enhancement. For the current incident-response use case, global control is appropriate.
38-41: Method renamed to match the service interface.The call now uses
handleMolliePaymentStatuswhich aligns with the renamed method inMollieService. The parameters are correctly passed.
packages/core/src/config/order/default-order-item-price-calculation-strategy.ts
Outdated
Show resolved
Hide resolved
packages/core/src/config/order/default-order-item-price-calculation-strategy.ts
Outdated
Show resolved
Hide resolved
packages/core/src/config/order/default-order-item-price-calculation-strategy.ts
Outdated
Show resolved
Hide resolved
| } | ||
| `); | ||
| // Create method | ||
| await adminClient.query<CreatePaymentMethodMutation, CreatePaymentMethodMutationVariables>( |
There was a problem hiding this comment.
Not sure why, but these types got lost somewhere in the migration to Tada. It's just the dev-server, so fine with me...
| * was not received for ~10 seconds. | ||
| * | ||
| * ## Pay later methods | ||
| * ### Pay later methods |
There was a problem hiding this comment.
Fixed heading levels to fix the ToC
| if (!apiKey) { | ||
| throw Error(`No apiKey configured for payment method ${paymentMethod.code}`); | ||
| } | ||
| return [createMollieClient({ apiKey }), paymentMethod]; |
There was a problem hiding this comment.
The method was changed to also return the payment method, because that can contain channel-specific configuration.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @packages/payments-plugin/src/mollie/mollie.service.ts:
- Line 231: Fix the typo in the log call in mollie.service.ts: update the
Logger.info invocation that builds the message using paymentId and
ctx.channel.token (Logger.info(..., loggerCtx)) to include a space between the
interpolated '${paymentId}' and the word "for" so the message reads "Processing
Mollie payment '<paymentId>' for channel <token>".
- Around line 458-468: The loop assigns the potentially-undefined return of
handleMolliePaymentStatus directly to order, which can wipe out the prior order
value; change the logic to capture the call result in a temporary (e.g.,
updatedOrder) and only replace order when updatedOrder is defined (order =
updatedOrder ?? order), continue pushing processedPaymentIds as before, and use
the preserved order when checking order.state (e.g., if (order?.state ===
'PaymentSettled') break) so the original order reference is not lost if
handleMolliePaymentStatus returns undefined.
🧹 Nitpick comments (2)
packages/payments-plugin/src/mollie/mollie.service.ts (2)
448-456: Consider the implications of the.take(500)limit.For high-volume merchants, 500 payments might not be sufficient to find older payments. If no matching payments are found within this limit, the sync silently does nothing (returns the order unchanged with an empty payment list in the log).
Consider either:
- Making the limit configurable via plugin options
- Logging a warning when the limit is reached without finding matching payments
469-474: Log message could be clearer when no payments are processed.When
processedPaymentIdsis empty, the log message ends with "based on Mollie payment(s) " (empty string). Consider adding a check to provide a clearer message in this case.Suggested improvement
Logger.info( - `Synced status for order '${order?.code ?? 'unknown'}' from '${originalOrderState}' to '${ - order?.state ?? 'unknown' - }' based on Mollie payment(s) ${processedPaymentIds.join(',')}`, + processedPaymentIds.length > 0 + ? `Synced status for order '${order?.code ?? 'unknown'}' from '${originalOrderState}' to '${ + order?.state ?? 'unknown' + }' based on Mollie payment(s) ${processedPaymentIds.join(',')}` + : `No authorized or paid Mollie payments found for order '${orderCode}'`, loggerCtx, );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/payments-plugin/src/mollie/mollie.service.ts
🔇 Additional comments (7)
packages/payments-plugin/src/mollie/mollie.service.ts (7)
1-32: LGTM!The new imports are correctly added and all are utilized by the new functionality.
48-51: LGTM!Using
IDtype from Vendure core is the correct approach for entity identifiers.
69-79: LGTM!The
ConfigServicedependency is properly injected and used for order access authorization.
227-349: The refactored return type enables the sync functionality well.The method now properly returns the updated
Orderon success orundefinedwhen silently failing, which aligns with the newsyncMolliePaymentStatusrequirements.
416-435: LGTM!The refactoring to use
getMollieClientreduces code duplication, and the explicitresource: 'payments'parameter is appropriate.
484-500: LGTM!The helper method nicely centralizes Mollie client creation. One minor observation: the optional chaining on
paymentMethod?.handlerat line 495 is unnecessary sincepaymentMethodis guaranteed non-null after the check at line 492, but it's harmless.
513-531: LGTM!The updated JSDoc accurately describes the fallback behavior when no payment method code is provided.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/payments-plugin/src/mollie/mollie.service.ts (2)
472-477: Consider handling the "no payments found" case explicitly.When no Mollie payments match the filter criteria,
processedPaymentIdsis empty and the log message reads "based on Mollie payment(s) " with an empty string. This could be confusing when debugging. Consider either:
- Logging a distinct message when no payments are found
- Returning early or throwing if the intent is that a matching payment must exist
💡 Suggested improvement
+ if (processedPaymentIds.length === 0) { + Logger.info( + `No Mollie payments found for order '${order.code}'. Order remains in '${order.state}' state.`, + loggerCtx, + ); + return order; + } Logger.info( `Synced status for order '${order.code}' from '${originalOrderState}' to '${ order.state }' based on Mollie payment(s) ${processedPaymentIds.join(',')}`, loggerCtx, );
495-495: Unnecessary optional chaining.
paymentMethodis already validated to be truthy on line 492-494, so the optional chainingpaymentMethod?.handleris redundant.💅 Suggested fix
- const apiKey = paymentMethod?.handler.args.find(arg => arg.name === 'apiKey')?.value; + const apiKey = paymentMethod.handler.args.find(arg => arg.name === 'apiKey')?.value;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/payments-plugin/src/mollie/mollie.service.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: publish_install (macos-latest, 22.x)
- GitHub Check: e2e tests (22.x, mariadb)
- GitHub Check: e2e tests (22.x, postgres)
- GitHub Check: e2e tests (20.x, mariadb)
- GitHub Check: e2e tests (24.x, sqljs)
- GitHub Check: e2e tests (24.x, mysql)
- GitHub Check: e2e tests (24.x, mariadb)
- GitHub Check: e2e tests (22.x, sqljs)
- GitHub Check: e2e tests (24.x, postgres)
- GitHub Check: e2e tests (22.x, mysql)
- GitHub Check: e2e tests (20.x, postgres)
- GitHub Check: unit tests (24.x)
- GitHub Check: e2e tests (20.x, mysql)
- GitHub Check: e2e tests (20.x, sqljs)
- GitHub Check: codegen / codegen
- GitHub Check: build (24.x)
- GitHub Check: build (20.x)
- GitHub Check: build (22.x)
- GitHub Check: unit tests (22.x)
- GitHub Check: unit tests (20.x)
🔇 Additional comments (5)
packages/payments-plugin/src/mollie/mollie.service.ts (5)
48-51: LGTM!The type change from
stringtoIDforpaymentMethodIdaligns with Vendure's type conventions and improves type safety.
69-79: LGTM!The
ConfigServiceinjection follows standard NestJS patterns and is required for the neworderByCodeAccessStrategyaccess check insyncMolliePaymentStatus.
227-349: LGTM!The method refactoring from
handleMollieStatusUpdatetohandleMolliePaymentStatuswithPromise<Order | undefined>return type is well-structured. Returning the updated order enables the newsyncMolliePaymentStatuscaller to track state changes, while the silentundefinedreturn for missing payment methods appropriately avoids exposing internal state in webhook contexts.
416-435: LGTM!Good refactoring to use
getMollieClienthelper, reducing code duplication. Theresource: 'payments'change aligns the method listing with the payments API context.
513-531: LGTM!The added JSDoc clarifies the fallback behavior when no payment method code is provided.
| async syncMolliePaymentStatus(ctx: RequestContext, orderCode: string): Promise<Order> { | ||
| let order = await this.orderService.findOneByCode(ctx, orderCode); | ||
| if (!order) { | ||
| throw new ForbiddenError(LogLevel.Verbose); | ||
| } | ||
| const originalOrderState = order.state; | ||
| const [mollieClient, paymentMethod] = await this.getMollieClient(ctx); | ||
| // Find payments for orderCode that are authorized or paid | ||
| const payments = mollieClient.payments | ||
| .iterate() | ||
| .take(500) // Only search through the last 500 payments for performance reasons | ||
| .filter(payment => { | ||
| return ( | ||
| payment.description === orderCode && | ||
| (payment.status === PaymentStatus.paid || payment.status === PaymentStatus.authorized) | ||
| ); | ||
| }); | ||
| const processedPaymentIds: string[] = []; | ||
| for await (const payment of payments) { | ||
| // This will handle the Mollie payment as if it were an incoming webhook | ||
| const updatedOrder = await this.handleMolliePaymentStatus(ctx, { | ||
| paymentMethodId: paymentMethod.id, | ||
| paymentId: payment.id, | ||
| }); | ||
| if (updatedOrder) { | ||
| order = updatedOrder; | ||
| } | ||
| processedPaymentIds.push(payment.id); | ||
| if (order.state === 'PaymentSettled') { | ||
| break; // No further processing needed, because the order is already settled | ||
| } | ||
| } | ||
| Logger.info( | ||
| `Synced status for order '${order.code}' from '${originalOrderState}' to '${ | ||
| order.state | ||
| }' based on Mollie payment(s) ${processedPaymentIds.join(',')}`, | ||
| loggerCtx, | ||
| ); | ||
| if (!(await this.configService.orderOptions.orderByCodeAccessStrategy.canAccessOrder(ctx, order))) { | ||
| throw new ForbiddenError(LogLevel.Verbose); | ||
| } | ||
| return order; |
There was a problem hiding this comment.
Authorization check should occur before processing payments.
The access check via orderByCodeAccessStrategy.canAccessOrder at line 478 happens after payments have been processed (lines 458-471). This allows side effects (order state transitions) to occur before verifying the caller has permission to access the order. An unauthenticated or unauthorized user who knows/guesses an order code could trigger state changes.
Move the access check to immediately after fetching the order:
🔒 Proposed fix
async syncMolliePaymentStatus(ctx: RequestContext, orderCode: string): Promise<Order> {
let order = await this.orderService.findOneByCode(ctx, orderCode);
if (!order) {
throw new ForbiddenError(LogLevel.Verbose);
}
+ if (!(await this.configService.orderOptions.orderByCodeAccessStrategy.canAccessOrder(ctx, order))) {
+ throw new ForbiddenError(LogLevel.Verbose);
+ }
const originalOrderState = order.state;
const [mollieClient, paymentMethod] = await this.getMollieClient(ctx);
// ... rest of the method ...
Logger.info(
`Synced status for order '${order.code}' from '${originalOrderState}' to '${
order.state
}' based on Mollie payment(s) ${processedPaymentIds.join(',')}`,
loggerCtx,
);
- if (!(await this.configService.orderOptions.orderByCodeAccessStrategy.canAccessOrder(ctx, order))) {
- throw new ForbiddenError(LogLevel.Verbose);
- }
return order;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async syncMolliePaymentStatus(ctx: RequestContext, orderCode: string): Promise<Order> { | |
| let order = await this.orderService.findOneByCode(ctx, orderCode); | |
| if (!order) { | |
| throw new ForbiddenError(LogLevel.Verbose); | |
| } | |
| const originalOrderState = order.state; | |
| const [mollieClient, paymentMethod] = await this.getMollieClient(ctx); | |
| // Find payments for orderCode that are authorized or paid | |
| const payments = mollieClient.payments | |
| .iterate() | |
| .take(500) // Only search through the last 500 payments for performance reasons | |
| .filter(payment => { | |
| return ( | |
| payment.description === orderCode && | |
| (payment.status === PaymentStatus.paid || payment.status === PaymentStatus.authorized) | |
| ); | |
| }); | |
| const processedPaymentIds: string[] = []; | |
| for await (const payment of payments) { | |
| // This will handle the Mollie payment as if it were an incoming webhook | |
| const updatedOrder = await this.handleMolliePaymentStatus(ctx, { | |
| paymentMethodId: paymentMethod.id, | |
| paymentId: payment.id, | |
| }); | |
| if (updatedOrder) { | |
| order = updatedOrder; | |
| } | |
| processedPaymentIds.push(payment.id); | |
| if (order.state === 'PaymentSettled') { | |
| break; // No further processing needed, because the order is already settled | |
| } | |
| } | |
| Logger.info( | |
| `Synced status for order '${order.code}' from '${originalOrderState}' to '${ | |
| order.state | |
| }' based on Mollie payment(s) ${processedPaymentIds.join(',')}`, | |
| loggerCtx, | |
| ); | |
| if (!(await this.configService.orderOptions.orderByCodeAccessStrategy.canAccessOrder(ctx, order))) { | |
| throw new ForbiddenError(LogLevel.Verbose); | |
| } | |
| return order; | |
| async syncMolliePaymentStatus(ctx: RequestContext, orderCode: string): Promise<Order> { | |
| let order = await this.orderService.findOneByCode(ctx, orderCode); | |
| if (!order) { | |
| throw new ForbiddenError(LogLevel.Verbose); | |
| } | |
| if (!(await this.configService.orderOptions.orderByCodeAccessStrategy.canAccessOrder(ctx, order))) { | |
| throw new ForbiddenError(LogLevel.Verbose); | |
| } | |
| const originalOrderState = order.state; | |
| const [mollieClient, paymentMethod] = await this.getMollieClient(ctx); | |
| // Find payments for orderCode that are authorized or paid | |
| const payments = mollieClient.payments | |
| .iterate() | |
| .take(500) // Only search through the last 500 payments for performance reasons | |
| .filter(payment => { | |
| return ( | |
| payment.description === orderCode && | |
| (payment.status === PaymentStatus.paid || payment.status === PaymentStatus.authorized) | |
| ); | |
| }); | |
| const processedPaymentIds: string[] = []; | |
| for await (const payment of payments) { | |
| // This will handle the Mollie payment as if it were an incoming webhook | |
| const updatedOrder = await this.handleMolliePaymentStatus(ctx, { | |
| paymentMethodId: paymentMethod.id, | |
| paymentId: payment.id, | |
| }); | |
| if (updatedOrder) { | |
| order = updatedOrder; | |
| } | |
| processedPaymentIds.push(payment.id); | |
| if (order.state === 'PaymentSettled') { | |
| break; // No further processing needed, because the order is already settled | |
| } | |
| } | |
| Logger.info( | |
| `Synced status for order '${order.code}' from '${originalOrderState}' to '${ | |
| order.state | |
| }' based on Mollie payment(s) ${processedPaymentIds.join(',')}`, | |
| loggerCtx, | |
| ); | |
| return order; | |
| } |
1 participant
Description
This PR introduces the functionality to:
syncMolliePaymentStatus, in case webhook delivery from Mollie is delayed.disableWebhookProcessinghas been introduced. This makes the Mollie plugin ignore any incoming webhooks.Why?
In November 2025 an incident occurred with Mollie webhooks that resulted in:
AddingItemsstate, even though the customers paid.OrderPlacedEvent's. The plugin's webhook mechanism should be idempotent, but it can't handle parallel/concurrent webhooks for the same order. This is why the optiondisableWebhookProcessingwas introduced: it allows the consumer to resort tosyncMolliePaymentStatus, and ignore webhooks altogether.Breaking changes
None 👍
Additional Context
We have considered implementing some kind of locking mechanism for concurrent webhooks, but this feels overly complex for something that is definitely an anomaly: This was the first time in 5 years webhooks came in simultaneously.
Checklist
📌 Always:
👍 Most of the time:
Summary by CodeRabbit
New Features
Tests
Behavioral Updates
✏️ Tip: You can customize this high-level summary in your review settings.