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

Make transaction details optional #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

janopae
Copy link

@janopae janopae commented Aug 7, 2022

A few weeks ago, I had problems using this because the key transaction_details was missing. I dirtily hacked a solution by hacking the python scripts in my installation, but I planned to properly commit it back upstream. For doing so, I wrote some unit tests.

I now realised that in the meantime, an issue and a PR have already been created for this (#3). However, I decided to PR this anyway, because I also made unit tests for my fix.

Also, my fix uses the field "purpose" for the memo in case "transaction_details" are missing (which is what my bank provides, and it seems to contain what you'd expect a memo to contain).

Please merge only after #4 has been merged.

Fixes #2.

@janopae janopae force-pushed the make_transaction_details_optional branch from cb912db to 00d0f5f Compare August 7, 2022 14:54
@karyon
Copy link

karyon commented Mar 23, 2023

#3 also protects against transaction.data['customer_reference'] not existing, which was the case for me.

@janopae
Copy link
Author

janopae commented Mar 24, 2023

@karyon Are you sure? I don't see anything protecting from transaction.data['customer_reference'] not existing in the diff of #3 – instead, I see multiple occurences where it still accesses transaction.data['customer_reference'] without checking.

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.

Key Error transaction_details
2 participants