redstreet / beancount_reds_importers

Simple ingesting tools for Beancount (plain text, double entry accounting software). More importantly, a framework to allow you to easily write your own importers.
GNU General Public License v3.0
104 stars 37 forks source link

Values of payee and transaction type fields are switched up in banking.Importer #33

Closed belidzs closed 2 years ago

belidzs commented 2 years ago

It looks like the payee and transaction type fields are switched up in banking.Importer:

https://github.com/redstreet/beancount_reds_importers/blob/fa4d51942ad313ec5d83e679c700b656489c06a4/beancount_reds_importers/libtransactionbuilder/banking.py#L98-L105

I've noticed that this was a conscious choice by reviewing older commits. I think generally the merchant who accepted the card should be recorded as the payee even if in some OFX formatted files follow another convention.

In these problematic cases the cleanest solution would be to correct this issue in the OFX parsing phase instead of doing that in a later stage of the pipeline where other parsers (such as the CSV parser) could also pump data and could reasonably expect that the payee field will be recorded as the payee eventually.

In my case I wrote a CSV parser which puts the payee information in the payee field and the narration information in memo. The generated transactions contained no payee information at all (apparently the transaction type goes there by default) and the narration field contained the value of the payee field. This is very confusing.

I recommend making a change where the payee field ends up as the payee of the transaction and the contents of memo or narration as the narration. In my opinion the transaction type is less important than the contents of a transfer memo field for example.

redstreet commented 2 years ago

Good point, cleaned this up. Does this resolve this issue?

belidzs commented 2 years ago

Looks great, thanks!