-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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
feat: Auto set Party in Bank Transaction (backport #34675) #35808
Merged
deepeshgarg007
merged 16 commits into
frappe:version-14-hotfix
from
marination:bank-party-automatch-hotfix
Jun 24, 2023
Merged
feat: Auto set Party in Bank Transaction (backport #34675) #35808
deepeshgarg007
merged 16 commits into
frappe:version-14-hotfix
from
marination:bank-party-automatch-hotfix
Jun 24, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Created Bank Party Mapper - Created class to auto match by account/iban or party name/description(fuzzy) - Automatch and set in transaction or create mapper - `rapidfuzz` introduced
…ployee/Shareholder)
…tion - Description is volatile and will keep changing - It will lead to multiple Bank Party Mapper docs for the same party that will never be referenced again - Parts of the descripton keep changing which is why it will never match a mapper record - If matched by desc, dont create mapper record.
- On updating bank trans.n party after submit, the corresponding mapper doc will be updated too - The mapper doc in turn will update all linked bank transactions that do not have this updated value - Added Bank Party Mapper hidden link in Bank Transaction - Rename field in BPM to `Party Name` as it does not hold description data - If a BT matches with a BPM record, link that record in the BT
- misc: Clearer naming
- Checkbox in Accounts settings "Enable Automatic Party Matching" - Check before invoking automatching methods - misc: Remove TODO comments
- A BT could have both account and iban, and a Supplier could have only IBAN set - In this case, matching by either (only account) gives no match - Match by Account OR IBAN, use `or_filters` - If matched, set both account no. and IBAN in Bank Party Mapper - Explain AutoMatchParty - Add type hints to return values - Use `set_value` to set values in BT after matching since its an after submit event
- Remove newly added fields in Party doctypes to store bank details - Use Bank Account's fields to match against account no/iban - For employee, if Bank Account does not exist, find in Employee doctype against account no/iban
…ches - Fuzzy matching can be enabled optionally in the settings - If a query gets multiple matches with the same score, do not set a party as it is an extremely close call - misc: Add 'cancelled' status to Bank transaction - Test for skipping matching with extremely close matches
- Matching by Acc No/IBAN can easily happen with Bank Accounts. It's not a tedious query - Historical lookups for Party Name/Desc match are very tricky. The user could have manually set a match and we would not know. Also this leaves the Bank Party Mapper only useful for Party Name/Desc lookups, which feels excessive. - We want to reduce the number of places the same data is stored and reduce confusion - The Party Name/Desc will optionally happen fuzzily, or not at all - There will be no Mapper lookups
marination
requested review from
ankush,
rohitwaghchaure,
s-aga-r,
deepeshgarg007 and
ruthra-kumar
as code owners
June 20, 2023 10:17
marination
changed the title
feat: Auto set Party in Bank Transaction
feat: Auto set Party in Bank Transaction (backport #34675)
Jun 20, 2023
marination
force-pushed
the
bank-party-automatch-hotfix
branch
from
June 20, 2023 10:22
0beb5de
to
4d8c211
Compare
deepeshgarg007
merged commit Jun 24, 2023
d53b197
into
frappe:version-14-hotfix
8 of 9 checks passed
🎉 This PR is included in version 14.28.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #34675