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

fill in creditor name for BNP bank transactions #349

Merged

Conversation

vojeroen
Copy link
Contributor

@vojeroen vojeroen commented May 1, 2024

Ensure payee names don't contain transactional information when pulling in transactions from BNP bank with GoCardless.

@vojeroen
Copy link
Contributor Author

vojeroen commented May 1, 2024

Hi, this is a PR related to issue #347. Based on the transactions I have, this seems to work. If a case occurs that isn't handled by the rules I added, it should fall back to the existing processing.

I'm not very used to writing JavaScript so please excuse any less-than-ideal syntax.

@vojeroen
Copy link
Contributor Author

vojeroen commented May 1, 2024

Also, it looks like this change poses no problems when re-processing transactions that have already been synced before, but now with a different creditorName. I suppose the GoCardless sync code is already intelligent enough to filter that out.

@MatissJanis
Copy link
Member

@CharlieMK @feliciaan
If either of you are around - would you be open to check this PR? Both of you worked on this bank adapter and I wouldn't want things to break for you.

@trafico-bot trafico-bot bot added the ✅ Approved Pull Request has been approved and can be merged label May 16, 2024
@MatissJanis MatissJanis merged commit abd049e into actualbudget:master May 16, 2024
6 checks passed
@trafico-bot trafico-bot bot added ✨ Merged Pull Request has been merged successfully and removed 🔍 Ready for Review ✅ Approved Pull Request has been approved and can be merged ✨ Merged Pull Request has been merged successfully labels May 16, 2024
MMichotte pushed a commit to MMichotte/actual-server that referenced this pull request Sep 9, 2024
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