Skip to content

Conversation

@Herrie82
Copy link
Contributor

@Herrie82 Herrie82 commented Mar 26, 2021

For a prettier generation of bank letters and other bits used in examples:

  • Added a French template based on implementation guide with French example from ABACUS Research SA.

Expand the bank config to include:

  • "bankFullName" used in the INI letter's which are sent to the bank.

  • "bankShortName" used in the filename for the generated letters and in other places if needed.

  • "languageCode" used for determining which template to use for the bank (currently "en" and "de" are supported).

  • "storageLocation" can be used to specify a local or network path where to store downloaded files.

  • In bankLetter.js: Use the current script folder as output for the bank's letter in HTML format instead of the user/os homedir folder.

Signed-off-by: Herman van Hazendonk github.com@herrie.org

@Herrie82
Copy link
Contributor Author

@nanov I will expand the examples with usage of "storageLocation" after this and other PR's are merged.

@nanov
Copy link
Contributor

nanov commented Mar 29, 2021

The only comment I have here, is why introduce braking change and remove bankName instead of just adding bankFullNme or alternatively bankShortName ( seems and the current one should bet the full one already ), and defaulting the one to the other if not present? ( ie. bankShortName = bankShortName || bankFullName ).

@Herrie82
Copy link
Contributor Author

Herrie82 commented Mar 29, 2021

The only comment I have here, is why introduce braking change and remove bankName instead of just adding bankFullNme or alternatively bankShortName ( seems and the current one should bet the full one already ), and defaulting the one to the other if not present? ( ie. bankShortName = bankShortName || bankFullName ).

@nanov bankShortName would be a name that doesn't contain any whitespace, so either the BIC or an abbreviation that could then be used in generated filenames for example (such as bank letter or downloaded CAMT05x/MT94x files). I will expand the examples a bit once this has been merged.

I thought to change bankName into bankFullName to avoid any confusion and it's more clear. Similar like you changed the config items from "serverAddress" into "url" and "keyStorage" into "keyStoragePath".

@nanov
Copy link
Contributor

nanov commented Mar 30, 2021

I thought to change bankName into bankFullName to avoid any confusion and it's more clear. Similar like you changed the config items from "serverAddress" into "url" and "keyStorage" into "keyStoragePath".

leaving bankName as is and NOT introducing breaking changes seems a cleaner solution.

@Herrie82
Copy link
Contributor Author

Herrie82 commented Mar 30, 2021

I thought to change bankName into bankFullName to avoid any confusion and it's more clear. Similar like you changed the config items from "serverAddress" into "url" and "keyStorage" into "keyStoragePath".

leaving bankName as is and NOT introducing breaking changes seems a cleaner solution.

Yes and no ;) I can update bankName to bankFullName in everywhere it occurs (so the various js files related to BankLetter (which is the only place it's being used anyway for the moment). Or keep it bankName, up to you in the end.

[edit]
Seems I forgot to update lib/BankLetter.js and test/unit/BankLetterTest.js which indeed would break your automated testing
[/edit]

Herrie82 and others added 2 commits March 30, 2021 11:02
For prettier generation of bank letters and other bits used in examples:

Expand the bank config to include:

* "bankFullName" used in the INI letter's which are sent to the bank.
* "bankShortName" used in the filename for the generated letters and in other places if needed.
* "languageCode" used for determining which template to use for the bank (currently "en" and "de" are supported).
* "storageLocation" can be used to specify a local or network path where to store downloaded files.

* In bankLetter.js: Use the current script folder as output for the bank's letter in HTML format instead of the user/os homedir folder.

Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
@Herrie82
Copy link
Contributor Author

@nanov This should be good to go now

@nanov nanov merged commit 3eb997e into node-ebics:master Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants