From 74a8c050257af839771982540acdb8954fa1957b Mon Sep 17 00:00:00 2001 From: Martijn Date: Tue, 25 Jun 2024 21:15:04 +0200 Subject: [PATCH] fix(payments-plugin): Prevent duplicate Mollie order lines (#2922) --- .../payments-plugin/e2e/mollie-dev-server.ts | 12 ++-- .../e2e/mollie-payment.e2e-spec.ts | 58 ++++++++++++++----- .../src/mollie/mollie.helpers.ts | 12 ---- .../src/mollie/mollie.service.ts | 57 ++++++------------ 4 files changed, 70 insertions(+), 69 deletions(-) diff --git a/packages/payments-plugin/e2e/mollie-dev-server.ts b/packages/payments-plugin/e2e/mollie-dev-server.ts index 3d3835d941..442685bd96 100644 --- a/packages/payments-plugin/e2e/mollie-dev-server.ts +++ b/packages/payments-plugin/e2e/mollie-dev-server.ts @@ -102,16 +102,20 @@ async function runMollieDevServer() { quantity: 1, }); await setShipping(shopClient); - // Comment out these lines if you want to test the payment flow via Mollie - await createFixedDiscountCoupon(adminClient, 156880, 'DISCOUNT_ORDER'); - await createFreeShippingCoupon(adminClient, 'FREE_SHIPPING'); - await shopClient.query(APPLY_COUPON_CODE, { couponCode: 'DISCOUNT_ORDER' }); await shopClient.query(APPLY_COUPON_CODE, { couponCode: 'FREE_SHIPPING' }); // Create Payment Intent const result = await shopClient.query(CREATE_MOLLIE_PAYMENT_INTENT, { input: {} }); // eslint-disable-next-line no-console console.log('Payment intent result', result); + + // Change order amount and create new intent + await createFixedDiscountCoupon(adminClient, 20000, 'DISCOUNT_ORDER'); + await shopClient.query(APPLY_COUPON_CODE, { couponCode: 'DISCOUNT_ORDER' }); + await new Promise(resolve => setTimeout(resolve, 3000)); + const result2 = await shopClient.query(CREATE_MOLLIE_PAYMENT_INTENT, { input: {} }); + // eslint-disable-next-line no-console + console.log('Payment intent result', result2); } (async () => { diff --git a/packages/payments-plugin/e2e/mollie-payment.e2e-spec.ts b/packages/payments-plugin/e2e/mollie-payment.e2e-spec.ts index bb930983eb..2ca30f3d8b 100644 --- a/packages/payments-plugin/e2e/mollie-payment.e2e-spec.ts +++ b/packages/payments-plugin/e2e/mollie-payment.e2e-spec.ts @@ -74,7 +74,36 @@ const mockData = { href: 'https://www.mollie.com/payscreen/select-method/mock-payment', }, }, - lines: [], + lines: [ + { + resource: 'orderline', + id: 'odl_3.c0qfy7', + orderId: 'ord_1.6i4fed', + name: 'Pinelab stickers', + status: 'created', + isCancelable: false, + quantity: 10, + createdAt: '2024-06-25T11:41:56+00:00', + }, + { + resource: 'orderline', + id: 'odl_3.nj3d5u', + orderId: 'ord_1.6i4fed', + name: 'Express Shipping', + isCancelable: false, + quantity: 1, + createdAt: '2024-06-25T11:41:57+00:00', + }, + { + resource: 'orderline', + id: 'odl_3.nklsl4', + orderId: 'ord_1.6i4fed', + name: 'Negative test surcharge', + isCancelable: false, + quantity: 1, + createdAt: '2024-06-25T11:41:57+00:00', + }, + ], _embedded: { payments: [ { @@ -360,7 +389,7 @@ describe('Mollie payments', () => { }); }); - it('Should update existing Mollie order', async () => { + it('Should recreate all order lines in Mollie', async () => { // Should fetch the existing order from Mollie nock('https://api.mollie.com/') .get('/v2/orders/ord_mockId') @@ -382,18 +411,21 @@ describe('Mollie payments', () => { paymentMethodCode: mockData.methodCode, }, }); - // We expect the patch request to add 3 order lines, because the mock response has 0 lines expect(createMolliePaymentIntent.url).toBeDefined(); - expect(molliePatchRequest.operations).toBeDefined(); - expect(molliePatchRequest.operations[0].operation).toBe('add'); - expect(molliePatchRequest.operations[0].data).toHaveProperty('name'); - expect(molliePatchRequest.operations[0].data).toHaveProperty('quantity'); - expect(molliePatchRequest.operations[0].data).toHaveProperty('unitPrice'); - expect(molliePatchRequest.operations[0].data).toHaveProperty('totalAmount'); - expect(molliePatchRequest.operations[0].data).toHaveProperty('vatRate'); - expect(molliePatchRequest.operations[0].data).toHaveProperty('vatAmount'); - expect(molliePatchRequest.operations[1].operation).toBe('add'); - expect(molliePatchRequest.operations[2].operation).toBe('add'); + // Should have removed all 3 previous order lines + const cancelledLines = molliePatchRequest.operations.filter((o: any) => o.operation === 'cancel'); + expect(cancelledLines.length).toBe(3); + // Should have added all 3 new order lines + const addedLines = molliePatchRequest.operations.filter((o: any) => o.operation === 'add'); + expect(addedLines.length).toBe(3); + addedLines.forEach((line: any) => { + expect(line.data).toHaveProperty('name'); + expect(line.data).toHaveProperty('quantity'); + expect(line.data).toHaveProperty('unitPrice'); + expect(line.data).toHaveProperty('totalAmount'); + expect(line.data).toHaveProperty('vatRate'); + expect(line.data).toHaveProperty('vatAmount'); + }); }); it('Should get payment url with deducted amount if a payment is already made', async () => { diff --git a/packages/payments-plugin/src/mollie/mollie.helpers.ts b/packages/payments-plugin/src/mollie/mollie.helpers.ts index 321bda73ef..9a17cc2710 100644 --- a/packages/payments-plugin/src/mollie/mollie.helpers.ts +++ b/packages/payments-plugin/src/mollie/mollie.helpers.ts @@ -149,15 +149,3 @@ export function getLocale(countryCode: string, channelLanguage: string): string // If no order locale and no channel locale, return a default, otherwise order creation will fail return allowedLocales[0]; } - -export function areOrderLinesEqual(line1: CreateParameters['lines'][0], line2: CreateParameters['lines'][0]): boolean { - return ( - line1.name === line2.name && - line1.quantity === line2.quantity && - line1.unitPrice.value === line2.unitPrice.value && - line1.unitPrice.currency === line2.unitPrice.currency && - line1.totalAmount.value === line2.totalAmount.value && - line1.vatRate === line2.vatRate && - line1.vatAmount.value === line2.vatAmount.value - ); -} diff --git a/packages/payments-plugin/src/mollie/mollie.service.ts b/packages/payments-plugin/src/mollie/mollie.service.ts index e880554b4a..6874bf3f36 100644 --- a/packages/payments-plugin/src/mollie/mollie.service.ts +++ b/packages/payments-plugin/src/mollie/mollie.service.ts @@ -39,14 +39,7 @@ import { MolliePaymentMethod, } from './graphql/generated-shop-types'; import { molliePaymentHandler } from './mollie.handler'; -import { - amountToCents, - areOrderLinesEqual, - getLocale, - toAmount, - toMollieAddress, - toMollieOrderLines, -} from './mollie.helpers'; +import { amountToCents, getLocale, toAmount, toMollieAddress, toMollieOrderLines } from './mollie.helpers'; import { MolliePluginOptions } from './mollie.plugin'; interface OrderStatusInput { @@ -483,48 +476,32 @@ export class MollieService { } /** - * Compare existing order lines with the new input, - * and update, add or cancel the order lines accordingly. - * - * We compare and update order lines based on their index, because there is no unique identifier + * Delete all order lines of current Mollie order, and create new ones based on the new Vendure order lines */ private async updateMollieOrderLines( mollieClient: ExtendedMollieClient, existingMollieOrder: MollieOrder, + /** + * These are the new order lines based on the Vendure order + */ newMollieOrderLines: CreateParameters['lines'], ): Promise { const manageOrderLinesInput: ManageOrderLineInput = { operations: [], }; - // Update or add new order lines - newMollieOrderLines.forEach((newLine, index) => { - const existingLine = existingMollieOrder.lines[index]; - if (existingLine && !areOrderLinesEqual(existingLine, newLine)) { - // Update if exists but not equal - manageOrderLinesInput.operations.push({ - operation: 'update', - data: { - ...newLine, - id: existingLine.id, - }, - }); - } else { - // Add new line if it doesn't exist - manageOrderLinesInput.operations.push({ - operation: 'add', - data: newLine, - }); - } + // Cancel all previous order lines and create new ones + existingMollieOrder.lines.forEach(existingLine => { + manageOrderLinesInput.operations.push({ + operation: 'cancel', + data: { id: existingLine.id }, + }); }); - // Cancel any order lines that are in the existing Mollie order, but not in the new input - existingMollieOrder.lines.forEach((existingLine, index) => { - const newLine = newMollieOrderLines[index]; - if (!newLine) { - manageOrderLinesInput.operations.push({ - operation: 'cancel', - data: { id: existingLine.id }, - }); - } + // Add new order lines + newMollieOrderLines.forEach(newLine => { + manageOrderLinesInput.operations.push({ + operation: 'add', + data: newLine, + }); }); return await mollieClient.manageOrderLines(existingMollieOrder.id, manageOrderLinesInput); }