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

re #30: Add a parser for transaction details. #43

Merged
merged 5 commits into from
Jul 24, 2017
Merged

re #30: Add a parser for transaction details. #43

merged 5 commits into from
Jul 24, 2017

Conversation

sweh
Copy link
Contributor

@sweh sweh commented Jun 13, 2017

As announced in #30 here is a suggestion how to implement the parser.

@wolph
Copy link
Owner

wolph commented Jun 14, 2017

Very nice work, thanks! :)
The only thing that I'm not yet sure about is enabling it as a default post processor... not sure if that's a good idea if it's only relevant for a few banks (and might break with the files from other banks).

Before I can merge it I need to make sure everything is properly tested so that can take a little bit of time but I'll get on it.

@benkonrath
Copy link
Contributor

This implementation looks like it's for German banks. Other countries format the description differently.

See "Codes used in field 86" on page 10 for various country specific codes:
https://www.handelsbanken.fi/shb/inet/icentsv.nsf/vlookuppics/a_filmformatbeskrivningar_fil_mt940_account_statement_20081212/$file/mt940_account_statement.pdf

See "Structured Field 86" on page 7 and examples in Appendix C on page 13 for how it's formatted in the Netherlands:
https://www.staalbankiers.nl/media/124389/mt940%20description%20v1.1.pdf

It could be included specifically as the German variant but not added by default. Other variants could be added and maybe some common code could be reused. There might even be an 'auto-detect' version that could be added at some point.

Note: I'm not the maintainer of this project, I'm just giving an opinion.

@wolph
Copy link
Owner

wolph commented Jun 14, 2017 via email

@sweh
Copy link
Contributor Author

sweh commented Jun 15, 2017

Hi there,

I changed the parsed to introspect transaction_details and leave result untouched, if it can not handle the detail string. Maybe this is a good compromise to leave it as a default processor which does not change anything if its not going to be successful in parsing.

@benkonrath
Copy link
Contributor

Since this code will mostly likely be used for other countries as well, it would be nice if you could use English instead of German in the DETAIL_KEYS and the GVC_KEYS.

@sweh
Copy link
Contributor Author

sweh commented Jun 16, 2017

Done. Since English is a foreign language to me, feel free to change some of the translations if you find them not accurate.

@wolph
Copy link
Owner

wolph commented Jul 3, 2017

just a little update, I'm still working on merging the code in and getting everything tested :)

@wolph wolph merged commit a8d63ab into wolph:develop Jul 24, 2017
@wolph
Copy link
Owner

wolph commented Jul 26, 2017

For some reason the tests are working for me locally but not within tox or travis... still working on getting everything working

@raphaelm
Copy link
Contributor

raphaelm commented Jul 27, 2017

FWIW: The parser we developed internally is now available publicly at https://github.com/pretix/pretix-banktool/blob/master/pretix_banktool/parse.py

Feel free to take it as inspiration or ignore it. We find it especially convenient as it is able to join references that have been split into multiple lines back into one useful string value.

@wolph
Copy link
Owner

wolph commented Jul 27, 2017

That code looks quite nice, I'll see if I can modify it to fit :)

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

Successfully merging this pull request may close these issues.

4 participants