Skip to content
This repository has been archived by the owner on Sep 22, 2023. It is now read-only.

Statement information parsed as transaction description #12

Closed
ndrsn opened this issue Feb 25, 2020 · 4 comments
Closed

Statement information parsed as transaction description #12

ndrsn opened this issue Feb 25, 2020 · 4 comments

Comments

@ndrsn
Copy link
Contributor

ndrsn commented Feb 25, 2020

First of all, thank you for all the work on this! It's already saved me a bunch of time.

That said, I believe there may be a bug in the way statements are parsed, with regards to field type 86.

This definition describes how field 86 is used for extra information pertaining a transaction, when it follows field 61.

However, this definition, following field 65 describes it as a field containing extra information on the statement as a whole.

I've encountered an issue with mt940-js when attempting to read an MT940 similar to the one below (simplified):

{1:000001INGBNL2AXXXX00001}
{2:I940INGBNL2AXXXN}
{4:
:20:INGEB
:25:0111111111
:28C:100
:60F:C100722EUR44,89
:61:2001010101C9035,09N078NONREF//20002424669822
/TRCD/00112/
:86:0111111111 V. DE JONG KERKSTRAAT 1154 1234 BW
ENSCHEDE BET.KENM. 1004510036716378 3305330802
AFLOSSINGSTERMIJN 188616 / 1E TERMIJN
:62F:C200101EUR550000,00
:64:C200101EUR550000,00
:65:C200103EUR550000,00
:65:C200101EUR550000,00
:86:/SUM/10/3/308435,72/14435,72/
-}

I would expect the transaction description to match

"0111111111 V. DE JONG KERKSTRAAT 1154 1234 BW ENSCHEDE BET.KENM. 1004510036716378 3305330802 AFLOSSINGSTERMIJN 188616 / 1E TERMIJN"

whereas now it sets it to

"/SUM/10/3/308435,72/14435,72/-}"

...which is the information related to the statement, not the transaction.

I believe this is because right now no accounting is made for the relative position of field 86 (see this line, which indiscriminately sets/overwrites the transaction description.

Can you confirm this is a bug in the implementation?

@webschik
Copy link
Owner

Hi @ndrsn !
Thanks for kind words and your report.
Indeed, it looks like a bug, I will try to check it when I have time. Feel free to open PR ;)

ndrsn added a commit to ndrsn/mt940-js that referenced this issue Feb 26, 2020
Fixes issue raised in webschik#12, where an `:86:` field relating to the statement as a whole was being parsed as relating to a specific transaction.
@ndrsn
Copy link
Contributor Author

ndrsn commented Feb 26, 2020

How about this?

@ndrsn
Copy link
Contributor Author

ndrsn commented Feb 28, 2020

I've encountered another issue related to this field: when no transactions at all are present in the file, and the parser encounters field :86: (like the /SUM/... field above), it will throw an error:

TypeError: Cannot read property 'description' of undefined

  at Object.close (src/tags/information-for-account-owner.ts:36:139)
  at closeCurrentTag (src/parser.ts:29:19)
  at Object.read (src/parser.ts:73:5)
  at Object.read (src/index.ts:17:19)

This is because state.transactionIndex will still be set to -1 once it encounters field :86:. We need to take the possibility of no transactions into account.

I've fixed this in the latest commit on master.

@webschik
Copy link
Owner

Thank you for contributing ;)
Fixes are released in version 1.0.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants