Skip to content

Conversation

alexanderadam
Copy link

Hi @huerlisi,

this is basically a feature wish or question for support of mt940 with curly brackets / heade-/body-structure.
This PR includes an additional spec (on top of the cleanup PR).

I just saw that some banks (ie PostFinance) sometimes encapsulate their mt940 files within some curly brackets.
There is some documentation about it (ie here on chapter two or here in the examples).

If I understand this correctly it would be totally fine for bookyt to throw away the {1:, {2: and {3: blocks and just use the fourth one as it contains the message. Is that correct so far?
If you agree I can also implement it by myself but as you probably know the format and the needs of bookyt in particular I just wanted to ask before. 😉

PS: the example is from this pdf

EDIT: I implemented it how I understood it now.

@huerlisi
Copy link
Member

Looks good! I think we need support for this, as it seems this is the whole MT940 messages vs. only the main content blocks. The files I've seen so far only contained the message blocks, but this might be because I always got them from some E-Banking of specific banks.

We probably could go a little bit further and expose the 4 sections in the API. If no sections (braces) are available we only set the 'Text' block and let the other be nil. I don't have a current need for that though;-)

I'm happy to take these changes so we quickly get support for PostFinance in Bookyt!

But I'd prefer to include them without the cleanup changes to reduce noise for now.

@alexanderadam
Copy link
Author

I totally understand. But I had problems bundling development dependencies with current ruby (2.3) anyway so it wasn't feasible for me to implement the "curly bracket issue" for me without moving the gem a bit more into the year 2016.

So I hope you can understand a bit why I added it on top of the cleanup branch. 😉

@huerlisi
Copy link
Member

Okay, fair enough. I think updating the dependencies is fine, should be fine to merge later on.

But maybe you can leave out the rubocop fixes;-)

@alexanderadam
Copy link
Author

Is it because you want a particular branch to merge where you would see major merge conflicts?
Because ie upgrading rspec results of course in using the expect syntax instead of the should syntax (except we enable the deprecated synax again of course) so "many" changed lines in rspec files will be the consequence and some quote or indention changes in Gemfile or gemspec shouldn't be critical neither, right?
So I guess the only, probably "cluttering" changes would be the ones in lib/mt940.rb.
Would it be therefore acceptable you that I revert the rubocop changes in this file?

@huerlisi
Copy link
Member

Sounds like a plan.

I guess direct merge is not possible, but I'd like to port our changes back to upstream. So reducing noise in lib/mt940.rb should be enough.

I'll merge you changes if you unclutter lib/mt940.rb so we can go forward.

Thanks again for your nice contribution!

alexanderadam added a commit to alexanderadam/mt940_parser that referenced this pull request Feb 28, 2016
@alexanderadam
Copy link
Author

Closed in favor of PR 3.

@alexanderadam alexanderadam deleted the feature/support_for_curly_brackets branch February 28, 2016 20:35
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