Fix notification payment lookup (#1049)

* Modify notification handler to get payment by pspReference or merchantReference * Set pspReference as payment key if it is returned after making payment * Refactor the setKey update action * Add setKey action in submit payment details handler * Fix style * Fix style * Fix unit test for make payment handler * Fix unit test for make payment handler with splits * Fix unit test for submit payment details handler * Fix integration test * Fix paypal e2e test * Fix paypal e2e test * Fix integration test for notification module * Fix unit test for notification module * Fix multitenancy test * Update extension/src/paymentHandler/make-payment.handler.js Co-authored-by: Lam Tran <[email protected]> * Revert "Update extension/src/paymentHandler/make-payment.handler.js" This reverts commit f89ebd8. * Remove unnecessary waiting time in Paypal pop-up * Improve the boolean comparison * Fix style * Remove unnecessary merchantReference checking * Use merchant reference and original reference in notification module * Fix paypal e2e test * Remove unnecessary checking for update payment key in notification module * Revert "Use merchant reference and original reference in notification module" This reverts commit 66e60fe. * Remove deprecated test case * Fix loggings * Fix integration test for cancel authorized payment * Fix documentation * Split the ADR * Modify ADR * Update ADR * Modify ADR * Modify ADR * Modify ADR * Modify ADR * Modify ADR * Modify ADR * Modify ADR * Revert "Modify ADR" This reverts commit dcbc579. * Modify ADR * Update docs/adr/0011-matching-adyen-notification.md Co-authored-by: Lam Tran <[email protected]> * Fix affirm payment e2e test Co-authored-by: King-Hin Leung <> Co-authored-by: Lam Tran <[email protected]>
Adyen · Jan 13, 2023 · 7d66a2a · 7d66a2a
1 parent dbd47fb
commit 7d66a2a
Show file tree

Hide file tree

Showing 21 changed files with 374 additions and 152 deletions.
diff --git a/docs/FAQ.md b/docs/FAQ.md
@@ -50,7 +50,7 @@ If you accidentally created a subscription you can edit it and uncheck the **Act
 
 ### How does the notification module find a matching payment?
 
-It first find the payment by `key` where `key=${merchantReference}` and then it finds in this payment the corresponding transaction by `interactionId` where `interactionId=${pspReference}`.
+It first find the payment by `key` where `key in (${merchantReference}, ${pspReference})`. If original reference exists in notification, the payment can be found by `key` where `key in (${merchantReference}, ${originalReference})`. And then it finds in this payment the corresponding transaction by `interactionId` where `interactionId=${pspReference}`.
 
 ### Will we lose a notification if it was not processed for some reason?
 

diff --git a/docs/adr/0002-matching-adyen-notification.md b/docs/adr/0002-matching-adyen-notification.md
@@ -4,7 +4,7 @@ Date: 2020-12-18
 
 ## Status
 
-[Accepted](https://github.com/commercetools/commercetools-adyen-integration/pull/395)
+[Deprecated](https://github.com/commercetools/commercetools-adyen-integration/pull/395)
 
 ## Context
 
@@ -25,3 +25,5 @@ by `interactionId` where `interactionId=${pspReference}`.
 - It is easier to fetch a key rather than using a custom field, also a key is an indexed field, so with a key, it's more performant.
 - The payment key is unique for all payments.
 - It's not possible to set key with my-payments endpoint. This prevents by default changing/removing the key accidentally. It is more secure than custom fields as the custom field might be changed with my-payment endpoint. Check for more details: https://docs.commercetools.com/api/projects/me-payments
+
+Please refer to the [0011-matching-adyen-notification](./0011-matching-adyen-notification.md) for the latest change regarding matching Notification with payment.
diff --git a/docs/adr/0011-matching-adyen-notification.md b/docs/adr/0011-matching-adyen-notification.md
@@ -0,0 +1,33 @@
+# 11. Matching Adyen Notification with commercetools payment.
+
+Date: 2023-01-03
+
+## Status
+
+[Accepted](https://github.com/commercetools/commercetools-adyen-integration/pull/1049)
+
+## Context
+
+The Adyen notification needs to be matched by its commercetools payment equivalent.
+We are using the payment key for the `merchantReference` and fetching the commercetools payment object with query `key="${merchantReference}"`.
+Since [v9.10.0](https://github.com/commercetools/commercetools-adyen-integration/releases/tag/v9.10.0), we have introduced custom reference for refund.
+Payment key could be the custom field in payment transaction defined by user, therefore payment is no longer always able to be obtained by merchant reference as key. 
+
+The alternative for that is to use `pspReference` as payment `key` field.
+
+## Decision
+
+- We will use `originalReference` (or `pspReference` if `originalReference` does not exist) or `merchantReference` as payment key for matching payment for notification.
+- For web component version 4, `pspReference` can be obtained from `makePaymentResponse` in extension module. It is used to update as payment key. Therefore we can match the payment by `pspReference` given in notification.
+- For web component version 5, `pspReference` is first provided in notification with AUTHORIZATION event. It is different from web component version 4, in which `pspReference` has already been provided from Adyen API response in extension module, and used to update as payment key. Therefore we still need `merchantReference` for payment lookup in this scenario.
+Once payment is found, update the payment key with `pspReference` obtained in notification.
+- For events other than AUTHORIZATION, such as REFUND, CAPTURE, CANCEL, we use `originalReference` from notification, which is the original `pspReference` obtained in AUTHORIZATION notification.
+- The notification will use the CTP payment key to fetch payment. It first check if `originalReference` exists. If yes, 
+  it finds the payment by `key` where `key in (${merchantReference}, ${originalReference})`), otherwise it finds the payment by `key` where `key in (${merchantReference}, ${pspReference})`.
+  After that it finds the corresponding transaction in this payment by `interactionId` where `interactionId=${pspReference}`. 
+
+For details, please refer to the [code snippet](https://github.com/commercetools/commercetools-adyen-integration/blob/dcbc5794cd4c470d1cf5a8c23623214671bf1849/notification/src/handler/notification/notification.handler.js#L52)
+
+## Consequences
+- It is possible to lookup payment for different transaction throughout the whole payment process
+- The payment key with `pspReference` (or `originalReference`) is also unique for all payments.
diff --git a/extension/src/paymentHandler/make-payment.handler.js b/extension/src/paymentHandler/make-payment.handler.js
@@ -4,6 +4,7 @@ import {
   createSetMethodInfoMethodAction,
   createSetMethodInfoNameAction,
   createAddTransactionActionByResponse,
+  getPaymentKeyUpdateAction,
 } from './payment-utils.js'
 import c from '../config/constants.js'
 import { makePayment } from '../service/web-component-service.js'
@@ -40,16 +41,12 @@ async function execute(paymentObject) {
     if (action) actions.push(action)
   }
 
-  const paymentKey = paymentObject.key
-  // ensure the key is a string, otherwise the error with "code": "InvalidJsonInput" will return by commercetools API.
-  const reference = requestBodyJson.reference?.toString()
-  // ensure the key and new reference is different, otherwise the error with
-  // "code": "InvalidOperation", "message": "'key' has no changes." will return by commercetools API.
-  if (reference !== paymentKey)
-    actions.push({
-      action: 'setKey',
-      key: reference,
-    })
+  const updatePaymentAction = getPaymentKeyUpdateAction(
+    paymentObject.key,
+    request,
+    response
+  )
+  if (updatePaymentAction) actions.push(updatePaymentAction)
 
   const addTransactionAction = createAddTransactionActionByResponse(
     paymentObject.amountPlanned.centAmount,

diff --git a/extension/src/paymentHandler/payment-utils.js b/extension/src/paymentHandler/payment-utils.js
@@ -197,6 +197,23 @@ function getIdempotencyKey(transaction) {
   return idempotencyKey
 }
 
+function getPaymentKeyUpdateAction(paymentKey, request, response) {
+  const requestBodyJson = JSON.parse(request.body)
+  const reference = requestBodyJson.reference?.toString()
+  const pspReference = response.pspReference?.toString()
+  const newReference = pspReference || reference
+  let paymentKeyUpdateAction
+  // ensure the key and new reference is different, otherwise the error with
+  // "code": "InvalidOperation", "message": "'key' has no changes." will return by commercetools API.
+  if (newReference !== paymentKey) {
+    paymentKeyUpdateAction = {
+      action: 'setKey',
+      key: newReference,
+    }
+  }
+  return paymentKeyUpdateAction
+}
+
 export {
   getChargeTransactionInitial,
   getChargeTransactionPending,
@@ -216,4 +233,5 @@ export {
   isValidJSON,
   isValidMetadata,
   getIdempotencyKey,
+  getPaymentKeyUpdateAction,
 }
diff --git a/extension/src/paymentHandler/submit-payment-details.handler.js b/extension/src/paymentHandler/submit-payment-details.handler.js
@@ -4,6 +4,7 @@ import {
   createAddInterfaceInteractionAction,
   createSetCustomFieldAction,
   createAddTransactionActionByResponse,
+  getPaymentKeyUpdateAction,
 } from './payment-utils.js'
 import c from '../config/constants.js'
 
@@ -53,6 +54,12 @@ async function execute(paymentObject) {
 
       if (addTransactionAction) actions.push(addTransactionAction)
     }
+    const updatePaymentAction = getPaymentKeyUpdateAction(
+      paymentObject.key,
+      request,
+      response
+    )
+    if (updatePaymentAction) actions.push(updatePaymentAction)
   }
   return {
     actions,

diff --git a/extension/test/e2e/pageObjects/AffirmPage.js b/extension/test/e2e/pageObjects/AffirmPage.js
@@ -6,7 +6,7 @@ export default class AffirmPage {
   async finishAffirmPayment() {
     await this.inputPhoneNumberAndClickSubmitButton()
     await this.inputPIN()
-    await this.clickTermCardAndProceed()
+    await this.choosePlan()
     await this.clickAutoPayToggleAndProceed()
   }
 
@@ -22,12 +22,19 @@ export default class AffirmPage {
     await this.page.waitForSelector('[data-test=term-card]')
   }
 
-  async clickTermCardAndProceed() {
-    await this.page.click('[data-test="term-card"]')
-    await this.page.waitForSelector('#autopay-toggle')
+  async choosePlan() {
+    await this.page.waitForTimeout(1_000)
+
+    const paymentRadioButton = await this.page.$('[data-testid="radio_button"]')
+    await this.page.evaluate((cb) => cb.click(), paymentRadioButton)
+    await this.page.waitForSelector('[data-testid="submit-button"]')
+
+    const submitButton = await this.page.$('[data-testid="submit-button"]')
+    await this.page.evaluate((cb) => cb.click(), submitButton)
   }
 
   async clickAutoPayToggleAndProceed() {
+    await this.page.waitForSelector('#autopay-toggle')
     const autoPayToggle = await this.page.$('#autopay-toggle')
     await this.page.evaluate((cb) => cb.click(), autoPayToggle)
     await this.page.waitForTimeout(1_000) // Wait for the page refreshes after toggling the autopay

diff --git a/extension/test/e2e/pageObjects/PaypalPopupPage.js b/extension/test/e2e/pageObjects/PaypalPopupPage.js
@@ -15,6 +15,7 @@ export default class PaypalPopupPage {
     await this.page.click('#btnLogin')
 
     await this.page.waitForSelector('#payment-submit-btn')
+    await this.page.waitForTimeout(1000) // Need to suspend 1 second to avoid page closed before loading data.
     await this.page.click('#payment-submit-btn')
   }
 }
diff --git a/extension/test/integration/make-payment.handler.spec.js b/extension/test/integration/make-payment.handler.spec.js
@@ -85,8 +85,11 @@ describe('::make-payment with multiple adyen accounts use case::', () => {
         ]
       )
 
+      const makePaymentResponse =
+        updatedPayment?.custom?.fields?.makePaymentResponse
+      const pspReference = JSON.parse(makePaymentResponse).pspReference
       expect(statusCode).to.equal(200)
-      expect(updatedPayment.key).to.equal(makePaymentRequestDraft.reference)
+      expect(updatedPayment.key).to.equal(pspReference)
       expect(updatedPayment.paymentMethodInfo.method).to.equal('scheme')
       expect(updatedPayment.paymentMethodInfo.name).to.eql({
         en: 'Credit Card',
@@ -147,8 +150,9 @@ describe('::make-payment with multiple adyen accounts use case::', () => {
     )
 
     expect(statusCode).to.equal(201)
-
-    expect(payment.key).to.equal(reference)
+    const { makePaymentResponse } = payment.custom.fields
+    const pspReference = JSON.parse(makePaymentResponse).pspReference
+    expect(payment.key).to.equal(pspReference)
     expect(payment.paymentMethodInfo.method).to.equal('scheme')
     expect(payment.paymentMethodInfo.name).to.eql({ en: 'Credit Card' })
 
@@ -173,8 +177,6 @@ describe('::make-payment with multiple adyen accounts use case::', () => {
       adyenMerchantAccount
     )
 
-    const { makePaymentResponse } = payment.custom.fields
-
     expect(makePaymentResponse).to.be.deep.equal(
       interfaceInteraction.fields.response
     )

diff --git a/extension/test/unit/make-payment-with-splits.handler.spec.js b/extension/test/unit/make-payment-with-splits.handler.spec.js
@@ -138,7 +138,9 @@ describe('make-payment-with-splits::execute', () => {
       )
 
       const setKeyAction = response.actions.find((a) => a.action === 'setKey')
-      expect(setKeyAction.key).to.equal(makePaymentWithSplitsRequest.reference)
+      expect(setKeyAction.key).to.equal(
+        JSON.parse(paymentSuccessResponse).pspReference
+      )
 
       const addTransaction = response.actions.find(
         (a) => a.action === 'addTransaction'

diff --git a/extension/test/unit/make-payment.handler.spec.js b/extension/test/unit/make-payment.handler.spec.js
@@ -124,7 +124,9 @@ describe('make-payment::execute', () => {
       )
 
       const setKeyAction = response.actions.find((a) => a.action === 'setKey')
-      expect(setKeyAction.key).to.equal(makePaymentRequest.reference)
+      expect(setKeyAction.key).to.equal(
+        JSON.parse(paymentSuccessResponse).pspReference
+      )
 
       const addTransaction = response.actions.find(
         (a) => a.action === 'addTransaction'
@@ -184,7 +186,7 @@ describe('make-payment::execute', () => {
       )
 
       const setKeyAction = response.actions.find((a) => a.action === 'setKey')
-      expect(setKeyAction.key).to.equal(makePaymentRequest.reference)
+      expect(setKeyAction.key).to.equal(makePaymentRequest.reference) // no pspReference until submitting additional details in redirect flow
     }
   )
 
@@ -285,7 +287,9 @@ describe('make-payment::execute', () => {
       )
 
       const setKeyAction = response.actions.find((a) => a.action === 'setKey')
-      expect(setKeyAction.key).to.equal(makePaymentRequest.reference)
+      expect(setKeyAction.key).to.equal(
+        JSON.parse(paymentRefusedResponse).pspReference
+      )
 
       const addTransaction = response.actions.find(
         (a) => a.action === 'addTransaction'
@@ -346,7 +350,9 @@ describe('make-payment::execute', () => {
       )
 
       const setKeyAction = response.actions.find((a) => a.action === 'setKey')
-      expect(setKeyAction.key).to.equal(makePaymentRequest.reference)
+      expect(setKeyAction.key).to.equal(
+        JSON.parse(paymentErrorResponse).pspReference
+      )
 
       const addTransaction = response.actions.find(
         (a) => a.action === 'addTransaction'

diff --git a/extension/test/unit/submit-payment-details.handler.spec.js b/extension/test/unit/submit-payment-details.handler.spec.js
@@ -59,7 +59,7 @@ describe('submit-additional-payment-details::execute', () => {
 
       const response = await execute(ctpPaymentClone)
 
-      expect(response.actions).to.have.lengthOf(3)
+      expect(response.actions).to.have.lengthOf(4)
       const addInterfaceInteraction = response.actions.find(
         (a) => a.action === 'addInterfaceInteraction'
       )
@@ -116,6 +116,10 @@ describe('submit-additional-payment-details::execute', () => {
       expect(addTransaction.transaction.interactionId).to.equal(
         JSON.parse(submitPaymentDetailsSuccessResponse).pspReference
       )
+      const setKeyAction = response.actions.find((a) => a.action === 'setKey')
+      expect(setKeyAction.key).to.equal(
+        JSON.parse(submitPaymentDetailsSuccessResponse).pspReference
+      )
     }
   )
 
@@ -275,7 +279,7 @@ describe('submit-additional-payment-details::execute', () => {
 
       const response = await execute(ctpPaymentClone)
 
-      expect(response.actions).to.have.lengthOf(3)
+      expect(response.actions).to.have.lengthOf(4)
       const addInterfaceInteractionAction = response.actions.find(
         (a) => a.action === 'addInterfaceInteraction'
       )
@@ -286,7 +290,10 @@ describe('submit-additional-payment-details::execute', () => {
         (a) => a.action === 'setCustomField'
       )
       expect(setCustomFieldAction.value).to.include('additionalData')
-
+      const setKeyAction = response.actions.find((a) => a.action === 'setKey')
+      expect(setKeyAction.key).to.equal(
+        JSON.parse(submitPaymentDetailsSuccessResponse).pspReference
+      )
       sandbox.restore()
     }
   )
@@ -321,7 +328,7 @@ describe('submit-additional-payment-details::execute', () => {
 
       const response = await execute(ctpPaymentClone)
 
-      expect(response.actions).to.have.lengthOf(3)
+      expect(response.actions).to.have.lengthOf(4)
       const addInterfaceInteractionAction = response.actions.find(
         (a) => a.action === 'addInterfaceInteraction'
       )
@@ -332,7 +339,10 @@ describe('submit-additional-payment-details::execute', () => {
         (a) => a.action === 'setCustomField'
       )
       expect(setCustomFieldAction.value).to.not.include('additionalData')
-
+      const setKeyAction = response.actions.find((a) => a.action === 'setKey')
+      expect(setKeyAction.key).to.equal(
+        JSON.parse(submitPaymentDetailsSuccessResponse).pspReference
+      )
       sandbox.restore()
     }
   )
@@ -367,11 +377,15 @@ describe('submit-additional-payment-details::execute', () => {
 
       const response = await execute(ctpPaymentClone)
 
-      expect(response.actions).to.have.lengthOf(2)
+      expect(response.actions).to.have.lengthOf(3)
       const addTransaction = response.actions.find(
         (a) => a.action === 'addTransaction'
       )
       expect(addTransaction).to.be.undefined
+      const setKeyAction = response.actions.find((a) => a.action === 'setKey')
+      expect(setKeyAction.key).to.equal(
+        JSON.parse(submitPaymentDetailsSuccessResponse).pspReference
+      )
     }
   )
 })
diff --git a/notification/docs/IntegrationGuide.md b/notification/docs/IntegrationGuide.md
@@ -99,7 +99,7 @@ and [transactionState](https://docs.commercetools.com/http-api-projects-payments
 > All mappings can be found in the [adyen-events.json](./../resources/adyen-events.json) file.
 
 After finding a mapping the notification module will find a matching payment on a commercetools project.
-To find the matching payment, `merchantReference` field from the notification is used to find a payment by key
+To find the matching payment, `merchantReference`, `pspReference` and `originalReference` fields from the notification are used to find a payment by key
 and `pspReference` field from the notification is used to find a transaction by its interactionId.
 
 If there is no transaction on the payment found,