Skip to content
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

Add GoCardless integration for COMMERZBANK_COBADEFF #537

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/app-gocardless/bank-factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import BelfiusGkccbebb from './banks/belfius_gkccbebb.js';
import BerlinerSparkasseBeladebexxx from './banks/berliner_sparkasse_beladebexxx.js';
import BnpBeGebabebb from './banks/bnp_be_gebabebb.js';
import CbcCregbebb from './banks/cbc_cregbebb.js';
import CommerzbankCobadeff from './banks/commerzbank_cobadeff.js';
import DanskebankDabno22 from './banks/danskebank_dabno22.js';
import EasybankBawaatww from './banks/easybank_bawaatww.js';
import EntercardSwednokk from './banks/entercard_swednokk.js';
Expand Down Expand Up @@ -47,6 +48,7 @@ export const banks = [
BerlinerSparkasseBeladebexxx,
BnpBeGebabebb,
CbcCregbebb,
CommerzbankCobadeff,
DanskebankDabno22,
EasybankBawaatww,
EntercardSwednokk,
Expand Down
56 changes: 56 additions & 0 deletions src/app-gocardless/banks/commerzbank_cobadeff.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import Fallback from './integration-bank.js';
import { formatPayeeName } from '../../util/payee-name.js';

/** @type {import('./bank.interface.js').IBank} */
export default {
...Fallback,

institutionIds: ['COMMERZBANK_COBADEFF'],

accessValidForDays: 179,

UnderKoen marked this conversation as resolved.
Show resolved Hide resolved
normalizeTransaction(transaction, _booked) {
// remittanceInformationUnstructured is limited to 140 chars thus ...
// ... missing information form remittanceInformationUnstructuredArray ...
// ... so we recreate it.
transaction.remittanceInformationUnstructured =
transaction.remittanceInformationUnstructuredArray.join(' ');

Comment on lines +10 to +16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add null check for remittanceInformationUnstructuredArray.

The array join operation could fail if the array is undefined. Consider adding a defensive check:

-    transaction.remittanceInformationUnstructured =
-      transaction.remittanceInformationUnstructuredArray.join(' ');
+    transaction.remittanceInformationUnstructured =
+      transaction.remittanceInformationUnstructuredArray?.join(' ') ?? '';
📝 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.

Suggested change
normalizeTransaction(transaction, _booked) {
// remittanceInformationUnstructured is limited to 140 chars thus ...
// ... missing information form remittanceInformationUnstructuredArray ...
// ... so we recreate it.
transaction.remittanceInformationUnstructured =
transaction.remittanceInformationUnstructuredArray.join(' ');
normalizeTransaction(transaction, _booked) {
// remittanceInformationUnstructured is limited to 140 chars thus ...
// ... missing information form remittanceInformationUnstructuredArray ...
// ... so we recreate it.
transaction.remittanceInformationUnstructured =
transaction.remittanceInformationUnstructuredArray?.join(' ') ?? '';

// The limitations of remittanceInformationUnstructuredArray ...
// ... can result in split keywords. We fix these. Other ...
// ... splits will need to be fixed by user with rules.
const keywords = [
'End-to-End-Ref.:',
'Mandatsref:',
'Gläubiger-ID:',
'SEPA-BASISLASTSCHRIFT',
'Kartenzahlung',
'Dauerauftrag',
];
keywords.forEach((keyword) => {
transaction.remittanceInformationUnstructured =
transaction.remittanceInformationUnstructured.replace(
// There can be spaces in keywords
RegExp(keyword.split('').join('\\s*'), 'gi'),
UnderKoen marked this conversation as resolved.
Show resolved Hide resolved
', ' + keyword + ' ',
);
});

// Clean up remittanceInformation, deduplicate payee (removing slashes ...
// ... that are added to the remittanceInformation field), and ...
// ... remove clutter like "End-to-End-Ref.: NOTPROVIDED"
const payee = transaction.creditorName || transaction.debtorName || '';
transaction.remittanceInformationUnstructured =
transaction.remittanceInformationUnstructured
.replace(/\s*(,)?\s+/g, '$1 ')
.replace(RegExp(payee.split(' ').join('(/*| )'), 'gi'), ' ')
.replace(', End-to-End-Ref.: NOTPROVIDED', '')
.trim();

return {
...transaction,
payeeName: formatPayeeName(transaction),
date: transaction.bookingDate,
};
},
};
Comment on lines +37 to +54
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve robustness of remittance information cleanup.

The current implementation has several areas for improvement:

  1. Complex regex patterns could be hard to maintain
  2. No validation of transaction object structure
+    // Validate transaction object structure
+    if (!transaction || typeof transaction !== 'object') {
+      throw new Error('Invalid transaction object');
+    }
+
     const payee = transaction.creditorName || transaction.debtorName || '';
+    
+    // Break down complex regex into more maintainable steps
     transaction.remittanceInformationUnstructured =
       transaction.remittanceInformationUnstructured
-        .replace(/\s*(,)?\s+/g, '$1 ')
-        .replace(RegExp(payee.split(' ').join('(/*| )'), 'gi'), ' ')
+        // Normalize spaces around commas
+        .replace(/\s*,\s*/g, ', ')
+        // Normalize multiple spaces
+        .replace(/\s+/g, ' ')
+        // Remove payee name variations
+        .replace(new RegExp(
+          payee
+            .split(' ')
+            .map(part => `${part}(?:/+|\\s+)?`)
+            .join(''),
+          'gi'
+        ), ' ')
         .replace(', End-to-End-Ref.: NOTPROVIDED', '')
         .trim();
📝 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.

Suggested change
// Clean up remittanceInformation, deduplicate payee (removing slashes ...
// ... that are added to the remittanceInformation field), and ...
// ... remove clutter like "End-to-End-Ref.: NOTPROVIDED"
const payee = transaction.creditorName || transaction.debtorName || '';
transaction.remittanceInformationUnstructured =
transaction.remittanceInformationUnstructured
.replace(/\s*(,)?\s+/g, '$1 ')
.replace(RegExp(payee.split(' ').join('(/*| )'), 'gi'), ' ')
.replace(', End-to-End-Ref.: NOTPROVIDED', '')
.trim();
return {
...transaction,
payeeName: formatPayeeName(transaction),
date: transaction.bookingDate,
};
},
};
// Clean up remittanceInformation, deduplicate payee (removing slashes ...
// ... that are added to the remittanceInformation field), and ...
// ... remove clutter like "End-to-End-Ref.: NOTPROVIDED"
// Validate transaction object structure
if (!transaction || typeof transaction !== 'object') {
throw new Error('Invalid transaction object');
}
const payee = transaction.creditorName || transaction.debtorName || '';
// Break down complex regex into more maintainable steps
transaction.remittanceInformationUnstructured =
transaction.remittanceInformationUnstructured
// Normalize spaces around commas
.replace(/\s*,\s*/g, ', ')
// Normalize multiple spaces
.replace(/\s+/g, ' ')
// Remove payee name variations
.replace(new RegExp(
payee
.split(' ')
.map(part => `${part}(?:/+|\\s+)?`)
.join(''),
'gi'
), ' ')
.replace(', End-to-End-Ref.: NOTPROVIDED', '')
.trim();
return {
...transaction,
payeeName: formatPayeeName(transaction),
date: transaction.bookingDate,
};
},
};

110 changes: 110 additions & 0 deletions src/app-gocardless/banks/tests/commerzbank_cobadeff.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import CommerzbankCobadeff from '../commerzbank_cobadeff.js';

describe('CommerzbankCobadeff', () => {
describe('#normalizeTransaction', () => {
it('correctly formats remittanceInformationUnstructured', () => {
const transaction = {
endToEndId: '1234567890',
mandateId: '321654',
bookingDate: '2024-12-20',
valueDate: '2024-12-20',
transactionAmount: {
amount: '-12.34',
currency: 'EUR',
},
creditorName: 'SHOP NAME CITY DE',
remittanceInformationUnstructured:
'SHOP NAME//CITY/DE\n2024-12-19T15:34:31 KFN 1 AB 1234\nKartenzahlung',
remittanceInformationUnstructuredArray: [
'SHOP NAME//CITY/DE',
'2024-12-19T15:34:31 KFN 1 AB 1234',
'Kartenzahlung',
],
remittanceInformationStructured:
'SHOP NAME//CITY/DE 2024-12-19T15:34:31 KFN 1 AB 1234 Kartenzahlung',
internalTransactionId: '3815213adb654baeadfb231c853',
};
const normalizedTransaction = CommerzbankCobadeff.normalizeTransaction(
transaction,
false,
);
expect(normalizedTransaction.remittanceInformationUnstructured).toEqual(
'2024-12-19T15:34:31 KFN 1 AB 1234, Kartenzahlung',
);
});

it('correctly formats remittanceInformationUnstructured; repair split keyword', () => {
const transaction = {
endToEndId: '901234567890',
mandateId: 'ABC123DEF456',
bookingDate: '2024-10-11',
valueDate: '2024-10-11',
transactionAmount: {
amount: '-56.78',
currency: 'EUR',
},
creditorName: 'Long payee name that is eaxtly 35ch',
remittanceInformationUnstructured:
'Long payee name that is eaxtly 35ch\n901234567890/. Long description tha\nt gets cut and is very long, did I\nmention it is long\nEnd-to-En',
remittanceInformationUnstructuredArray: [
'Long payee name that is eaxtly 35ch',
'901234567890/. Long description tha',
't gets cut and is very long, did I',
'mention it is long',
'End-to-En',
'd-Ref.: 901234567890',
'Mandatsref: ABC123DEF456',
'Gläubiger-ID:',
'AB12CDE0000000000000000012',
'SEPA-BASISLASTSCHRIFT wiederholend',
],
remittanceInformationStructured:
'Long payee name that is eaxtly 35ch 901234567890/. Long description tha t gets cut and is very long, did I mention it is long End-to-En',
internalTransactionId: '812354cfdea36465asdfe',
};
const normalizedTransaction = CommerzbankCobadeff.normalizeTransaction(
transaction,
false,
);
expect(normalizedTransaction.remittanceInformationUnstructured).toEqual(
'901234567890/. Long description tha t gets cut and is very long, did I mention it is long, End-to-End-Ref.: 901234567890, Mandatsref: ABC123DEF456, Gläubiger-ID: AB12CDE0000000000000000012, SEPA-BASISLASTSCHRIFT wiederholend',
);
});

it('correctly formats remittanceInformationUnstructured; removing NOTPROVIDED', () => {
const transaction = {
endToEndId: 'NOTPROVIDED',
bookingDate: '2024-12-02',
valueDate: '2024-12-02',
transactionAmount: {
amount: '-9',
currency: 'EUR',
},
creditorName: 'CREDITOR NAME',
creditorAccount: {
iban: 'CREDITOR000IBAN',
},
remittanceInformationUnstructured:
'CREDITOR NAME\nCREDITOR00BIC\nCREDITOR000IBAN\nDESCRIPTION\nEnd-to-End-Ref.: NOTPROVIDED\nDauerauftrag',
remittanceInformationUnstructuredArray: [
'CREDITOR NAME',
'CREDITOR00BIC',
'CREDITOR000IBAN',
'DESCRIPTION',
'End-to-End-Ref.: NOTPROVIDED',
'Dauerauftrag',
],
remittanceInformationStructured:
'CREDITOR NAME CREDITOR00BIC CREDITOR000IBAN DESCRIPTION End-to-End-Ref.: NOTPROVIDED Dauerauftrag',
internalTransactionId: 'f617dc31ab77622bf13d6c95d6dd8b4a',
};
const normalizedTransaction = CommerzbankCobadeff.normalizeTransaction(
transaction,
false,
);
expect(normalizedTransaction.remittanceInformationUnstructured).toEqual(
'CREDITOR00BIC CREDITOR000IBAN DESCRIPTION, Dauerauftrag',
);
});
});
});
6 changes: 5 additions & 1 deletion src/app-gocardless/gocardless-node.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,11 @@ export type Transaction = {
/**
* Account reference, conditional
*/
creditorAccount?: string;
creditorAccount?:
| string
| {
iban?: string;
};
nsulzer marked this conversation as resolved.
Show resolved Hide resolved

/**
* BICFI
Expand Down
6 changes: 6 additions & 0 deletions upcoming-release-notes/537.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Enhancements
authors: [nsulzer]
---

Add GoCardless integration for COMMERZBANK_COBADEFF
Loading