Decide on transaction date during GoCardless transaction normalization (

#243)

package.json

@@ -21,6 +21,7 @@
     "better-sqlite3": "^8.2.0",
     "body-parser": "^1.20.1",
     "cors": "^2.8.5",
+    "date-fns": "^2.30.0",
     "debug": "^4.3.4",
     "express": "4.18.2",
     "express-actuator": "1.8.4",

src/app-gocardless/banks/american-express-aesudef1.js

@@ -21,27 +21,23 @@ export default {
   },
 
   normalizeTransaction(transaction, _booked) {
-    /**
-     * The American Express Europe integration sends the actual date of
-     * purchase as `bookingDate`, and `valueDate` appears to contain a date
-     * related to the actual booking date, though sometimes offset by a day
-     * compared to the American Express website.
-     */
-    delete transaction.valueDate;
-    return transaction;
+    return {
+      ...transaction,
+      date: transaction.bookingDate,
+    };
   },
 
   sortTransactions(transactions = []) {
     return sortByBookingDateOrValueDate(transactions);
   },
 
   /**
-   *  For SANDBOXFINANCE_SFIN0000 we don't know what balance was
+   *  For AMERICAN_EXPRESS_AESUDEF1 we don't know what balance was
    *  after each transaction so we have to calculate it by getting
    *  current balance from the account and subtract all the transactions
    *
-   *  As a current balance we use `interimBooked` balance type because
-   *  it includes transaction placed during current day
+   *  As a current balance we use the non-standard `information` balance type
+   *  which is the only one provided for American Express.
    */
   calculateStartingBalance(sortedTransactions = [], balances = []) {
     const currentBalance = balances.find(

src/app-gocardless/banks/bank.interface.ts

@@ -15,11 +15,16 @@ export interface IBank {
 
   /**
    * Returns a normalized transaction object
+   *
+   * The GoCardless integrations with different banks are very inconsistent in
+   * what each of the different date fields actually mean, so this function is
+   * expected to set a `date` field which corresponds to the expected
+   * transaction date.
    */
   normalizeTransaction: (
     transaction: Transaction,
     booked: boolean,
-  ) => Transaction | null;
+  ) => (Transaction & { date?: string }) | null;
 
   /**
    * Function sorts an array of transactions from newest to oldest

src/app-gocardless/banks/fintro-be-gebabebb.js

@@ -19,11 +19,6 @@ export default {
   institutionIds: ['FINTRO_BE_GEBABEBB'],
 
   normalizeAccount(account) {
-    console.log(
-      'Available account properties for new institution integration',
-      { account: JSON.stringify(account) },
-    );
-
     return {
       account_id: account.id,
       institution: account.institution,
@@ -69,28 +64,18 @@ export default {
         ].filter(Boolean);
       }
     }
-    return transaction;
+
+    return {
+      ...transaction,
+      date: transaction.valueDate,
+    };
   },
 
   sortTransactions(transactions = []) {
-    console.log(
-      'Available (first 10) transactions properties for new integration of institution in sortTransactions function',
-      { top10Transactions: JSON.stringify(transactions.slice(0, 10)) },
-    );
     return sortByBookingDateOrValueDate(transactions);
   },
 
   calculateStartingBalance(sortedTransactions = [], balances = []) {
-    console.log(
-      'Available (first 10) transactions properties for new integration of institution in calculateStartingBalance function',
-      {
-        balances: JSON.stringify(balances),
-        top10SortedTransactions: JSON.stringify(
-          sortedTransactions.slice(0, 10),
-        ),
-      },
-    );
-
     const currentBalance = balances
       .filter((item) => SORTED_BALANCE_TYPE_LIST.includes(item.balanceType))
       .sort(

src/app-gocardless/banks/ing-pl-ingbplpw.js

@@ -17,7 +17,10 @@ export default {
   },
 
   normalizeTransaction(transaction, _booked) {
-    return transaction;
+    return {
+      ...transaction,
+      date: transaction.bookingDate || transaction.valueDate,
+    };
   },
 
   sortTransactions(transactions = []) {

src/app-gocardless/banks/integration-bank.js

@@ -1,3 +1,4 @@
+import * as d from 'date-fns';
 import {
   sortByBookingDateOrValueDate,
   amountToInteger,
@@ -37,7 +38,21 @@ export default {
   },
 
   normalizeTransaction(transaction, _booked) {
-    return transaction;
+    const date =
+      transaction.bookingDate ||
+      transaction.bookingDateTime ||
+      transaction.valueDate ||
+      transaction.valueDateTime;
+    // If we couldn't find a valid date field we filter out this transaction
+    // and hope that we will import it again once the bank has processed the
+    // transaction further.
+    if (!date) {
+      return null;
+    }
+    return {
+      ...transaction,
+      date: d.format(d.parseISO(date), 'yyyy-MM-dd'),
+    };
   },
 
   sortTransactions(transactions = []) {

src/app-gocardless/banks/mbank-retail-brexplpw.js

@@ -17,7 +17,10 @@ export default {
   },
 
   normalizeTransaction(transaction, _booked) {
-    return transaction;
+    return {
+      ...transaction,
+      date: transaction.bookingDate || transaction.valueDate,
+    };
   },
 
   sortTransactions(transactions = []) {

src/app-gocardless/banks/norwegian-xx-norwnok1.js

@@ -28,50 +28,45 @@ export default {
   },
 
   normalizeTransaction(transaction, booked) {
-    /**
-     * The way Bank Norwegian handles the date fields is rather strange and
-     * countrary to GoCardless's documentation.
-     *
-     * For booked transactions Bank Norwegian sends a `valueDate` field that
-     * doesn't match the NextGenPSD2 definition of `valueDate` which is what we
-     * expect to receive from GoCardless.  Therefore we remove the incorrect
-     * field so that transactions are correctly imported.
-     */
     if (booked) {
-      delete transaction.valueDate;
-      return transaction;
+      return {
+        ...transaction,
+        date: transaction.bookingDate,
+      };
     }
 
     /**
-     * For pending transactions there are two possibilities.  Either the
-     * transaction has a `valueDate`, in which case the `valueDate` we receive
-     * corresponds to when the transaction actually occurred, and so we simply
-     * return the transaction as-is.
+     * For pending transactions there are two possibilities:
+     *
+     * - Either a `valueDate` was set, in which case it corresponds to when the
+     *   transaction actually occurred, or
+     * - There is no date field, in which case we try to parse the correct date
+     *   out of the `remittanceInformationStructured` field.
+     *
+     * If neither case succeeds then we return `null` causing this transaction
+     * to be filtered out for now, and hopefully we'll be able to import it
+     * once the bank has processed it further.
      */
     if (transaction.valueDate !== undefined) {
-      return transaction;
+      return {
+        ...transaction,
+        date: transaction.valueDate,
+      };
     }
 
-    /**
-     * If the pending transaction didn't have a `valueDate` field then it
-     * should have a `remittanceInformationStructured` field which contains the
-     * date we expect to receive as the `valueDate`.  In this case we extract
-     * the date from that field and set it as `valueDate`.
-     */
     if (transaction.remittanceInformationStructured) {
       const remittanceInfoRegex = / (\d{4}-\d{2}-\d{2}) /;
       const matches =
         transaction.remittanceInformationStructured.match(remittanceInfoRegex);
       if (matches) {
         transaction.valueDate = matches[1];
-        return transaction;
+        return {
+          ...transaction,
+          date: matches[1],
+        };
       }
     }
 
-    /**
-     * If neither pending case is true we return `null` and ignore the
-     * transaction until it's been further processed by the bank.
-     */
     return null;
   },
 

src/app-gocardless/banks/sandboxfinance-sfin0000.js

@@ -1,4 +1,8 @@
-import { printIban, amountToInteger } from '../utils.js';
+import {
+  printIban,
+  amountToInteger,
+  sortByBookingDateOrValueDate,
+} from '../utils.js';
 
 /** @type {import('./bank.interface.js').IBank} */
 export default {
@@ -16,17 +20,23 @@ export default {
     };
   },
 
+  /**
+   * Following the GoCardless documentation[0] we should prefer `bookingDate`
+   * here, though some of their bank integrations uses the date field
+   * differently from what's describen in their documentation and so it's
+   * sometimes necessary to use `valueDate` instead.
+   *
+   *   [0]: https://nordigen.zendesk.com/hc/en-gb/articles/7899367372829-valueDate-and-bookingDate-for-transactions
+   */
   normalizeTransaction(transaction, _booked) {
-    return transaction;
+    return {
+      ...transaction,
+      date: transaction.bookingDate || transaction.valueDate,
+    };
   },
 
   sortTransactions(transactions = []) {
-    return transactions.sort((a, b) => {
-      const [aTime, aSeq] = a.transactionId.split('-');
-      const [bTime, bSeq] = b.transactionId.split('-');
-
-      return Number(bTime) - Number(aTime) || Number(bSeq) - Number(aSeq);
-    });
+    return sortByBookingDateOrValueDate(transactions);
   },
 
   /**

src/app-gocardless/banks/tests/sandboxfinance-sfin0000.spec.js

@@ -1,5 +1,4 @@
 import SandboxfinanceSfin0000 from '../sandboxfinance-sfin0000.js';
-import { mockTransactionAmount } from '../../services/tests/fixtures.js';
 
 describe('SandboxfinanceSfin0000', () => {
   describe('#normalizeAccount', () => {
@@ -58,55 +57,6 @@ describe('SandboxfinanceSfin0000', () => {
   });
 
   describe('#sortTransactions', () => {
-    it('sorts transactions by time and sequence from newest to oldest', () => {
-      const transactions = [
-        {
-          transactionId: '2023012301927902-2',
-          transactionAmount: mockTransactionAmount,
-        },
-        {
-          transactionId: '2023012301927902-1',
-          transactionAmount: mockTransactionAmount,
-        },
-        {
-          transactionId: '2023012301927900-2',
-          transactionAmount: mockTransactionAmount,
-        },
-        {
-          transactionId: '2023012301927900-1',
-          transactionAmount: mockTransactionAmount,
-        },
-        {
-          transactionId: '2023012301927900-3',
-          transactionAmount: mockTransactionAmount,
-        },
-      ];
-      const sortedTransactions =
-        SandboxfinanceSfin0000.sortTransactions(transactions);
-      expect(sortedTransactions).toEqual([
-        {
-          transactionId: '2023012301927902-2',
-          transactionAmount: mockTransactionAmount,
-        },
-        {
-          transactionId: '2023012301927902-1',
-          transactionAmount: mockTransactionAmount,
-        },
-        {
-          transactionId: '2023012301927900-3',
-          transactionAmount: mockTransactionAmount,
-        },
-        {
-          transactionId: '2023012301927900-2',
-          transactionAmount: mockTransactionAmount,
-        },
-        {
-          transactionId: '2023012301927900-1',
-          transactionAmount: mockTransactionAmount,
-        },
-      ]);
-    });
-
     it('handles empty arrays', () => {
       const transactions = [];
       const sortedTransactions =

src/app-gocardless/services/gocardless-service.js

@@ -439,14 +439,12 @@ export const goCardlessService = {
 
     handleGoCardlessError(response);
 
-    const bankAccount = BankFactory(institutionId);
+    const bank = BankFactory(institutionId);
     response.transactions.booked = response.transactions.booked
-      .map((transaction) => bankAccount.normalizeTransaction(transaction, true))
+      .map((transaction) => bank.normalizeTransaction(transaction, true))
       .filter((transaction) => transaction);
     response.transactions.pending = response.transactions.pending
-      .map((transaction) =>
-        bankAccount.normalizeTransaction(transaction, false),
-      )
+      .map((transaction) => bank.normalizeTransaction(transaction, false))
       .filter((transaction) => transaction);
 
     return response;

src/app-gocardless/services/tests/gocardless-service.spec.js

@@ -489,6 +489,7 @@ describe('goCardlessService', () => {
               {
                 "bankTransactionCode": "string",
                 "bookingDate": "date",
+                "date": "date",
                 "debtorAccount": {
                   "iban": "string",
                 },
@@ -503,6 +504,7 @@ describe('goCardlessService', () => {
               {
                 "bankTransactionCode": "string",
                 "bookingDate": "date",
+                "date": "date",
                 "transactionAmount": {
                   "amount": "947.26",
                   "currency": "EUR",
@@ -513,6 +515,7 @@ describe('goCardlessService', () => {
             ],
             "pending": [
               {
+                "date": "date",
                 "transactionAmount": {
                   "amount": "947.26",
                   "currency": "EUR",

upcoming-release-notes/243.md

@@ -0,0 +1,6 @@
+---
+category: Enhancements
+authors: [kyrias]
+---
+
+Decide on transaction date during GoCardless transaction normalization.