-
-
Notifications
You must be signed in to change notification settings - Fork 665
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,87 @@ | ||||||||||||||||||||||||||||||||||||||||||||
import Fallback from './integration-bank.js'; | ||||||||||||||||||||||||||||||||||||||||||||
import { amountToInteger, printIban } from '../utils.js'; | ||||||||||||||||||||||||||||||||||||||||||||
import { formatPayeeName } from '../../util/payee-name.js'; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/** @type {import('./bank.interface.js').IBank} */ | ||||||||||||||||||||||||||||||||||||||||||||
export default { | ||||||||||||||||||||||||||||||||||||||||||||
...Fallback, | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
institutionIds: ['COMMERZBANK_COBADEFF'], | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
accessValidForDays: 179, | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
normalizeAccount(account) { | ||||||||||||||||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||||||||||||||||
account_id: account.id, | ||||||||||||||||||||||||||||||||||||||||||||
institution: account.institution, | ||||||||||||||||||||||||||||||||||||||||||||
mask: account.iban.slice(-4), | ||||||||||||||||||||||||||||||||||||||||||||
iban: account.iban, | ||||||||||||||||||||||||||||||||||||||||||||
name: [account.name, printIban(account)].join(' '), | ||||||||||||||||||||||||||||||||||||||||||||
official_name: account.product, | ||||||||||||||||||||||||||||||||||||||||||||
type: 'checking', | ||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+13
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As this is the same as in the fallback you could remove this and rely on the fallback
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(' '); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// 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( | ||||||||||||||||||||||||||||||||||||||||||||
RegExp(keyword.split('').join('\\s*'), 'gi'), | ||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
', ' + 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, | ||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||
* For COMMERZBANK_COBADEFF 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 `expected` balance type because it | ||||||||||||||||||||||||||||||||||||||||||||
* corresponds to the current running balance, whereas `interimAvailable` | ||||||||||||||||||||||||||||||||||||||||||||
* holds the remaining credit limit. | ||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||
calculateStartingBalance(sortedTransactions = [], balances = []) { | ||||||||||||||||||||||||||||||||||||||||||||
const currentBalance = balances.find( | ||||||||||||||||||||||||||||||||||||||||||||
(balance) => 'expected' === balance.balanceType, | ||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
return sortedTransactions.reduce((total, trans) => { | ||||||||||||||||||||||||||||||||||||||||||||
return total - amountToInteger(trans.transactionAmount.amount); | ||||||||||||||||||||||||||||||||||||||||||||
}, amountToInteger(currentBalance.balanceAmount.amount)); | ||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+78
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check for empty calculateStartingBalance(sortedTransactions = [], balances = []) {
const currentBalance = balances.find(
(balance) => 'expected' === balance.balanceType,
);
+ if (!currentBalance) {
+ // Decide on fallback or throw error
+ return 0;
+ }
return sortedTransactions.reduce((total, trans) => {
return total - amountToInteger(trans.transactionAmount.amount);
}, amountToInteger(currentBalance.balanceAmount.amount));
}, 📝 Committable suggestion
Suggested change
Comment on lines
+69
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this could also just use the fallback? |
||||||||||||||||||||||||||||||||||||||||||||
}; |
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', | ||
); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -356,7 +356,11 @@ export type Transaction = { | |||||||||||||||||
/** | ||||||||||||||||||
* Account reference, conditional | ||||||||||||||||||
*/ | ||||||||||||||||||
creditorAccount?: string; | ||||||||||||||||||
creditorAccount?: | ||||||||||||||||||
| string | ||||||||||||||||||
| { | ||||||||||||||||||
iban?: string; | ||||||||||||||||||
}; | ||||||||||||||||||
Comment on lines
+359
to
+363
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As it happens, this can just be an object. It was wrongly typed before.
Suggested change
|
||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* BICFI | ||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
--- | ||
category: Enhancements | ||
authors: [nsulzer] | ||
--- | ||
|
||
Add GoCardless integration for COMMERZBANK_COBADEFF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Based on the evidence gathered, I can now provide the final response:
Add safeguard for missing IBAN property in
normalizeAccount
The codebase shows that:
printIban
utility already handles missing IBAN gracefully by returning an empty stringintegration-bank.spec.js
that explicitly tests withiban: undefined
The current implementation needs safeguarding in two places:
mask: account.iban?.slice(-4) ?? ''
iban: account.iban ?? ''
🔗 Analysis chain
Ensure robustness for missing
account.iban
The
normalizeAccount
function assumes aniban
property is present on theaccount
. If the bank feed or fallback code potentially returns anaccount
object without aniban
, this could cause errors. Consider safe-guarding it, for example:Let me verify if there are any cases where
account
objects might be missing theiban
property by analyzing the codebase.Let me search for how the account objects are constructed and any test cases that might reveal the behavior.
Let me check the specific test file for Commerzbank and the utility function that handles IBAN printing.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
Length of output: 9005
Script:
Length of output: 1977
Script:
Length of output: 5599