fix(spp_programs): fix deduplication and enrollment interaction bugs#113
fix(spp_programs): fix deduplication and enrollment interaction bugs#113
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses several critical bugs within the deduplication and enrollment workflows, aiming to improve data integrity, user clarity, and process robustness. The changes provide more granular insights into why beneficiaries are flagged as duplicates, prevent unintended state changes during enrollment, and modernize the underlying data models for ID types. The overall impact is a more reliable and transparent system for managing program memberships and identifying duplicate entries. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 19.0 #113 +/- ##
==========================================
+ Coverage 70.14% 70.85% +0.70%
==========================================
Files 739 783 +44
Lines 43997 46652 +2655
==========================================
+ Hits 30863 33055 +2192
- Misses 13134 13597 +463
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request provides significant improvements to the deduplication and enrollment workflows. The changes address several bugs, including providing more detailed reasons for duplicates, preventing state overwrites during enrollment, and migrating to the correct model for ID types. The refactoring of the individual deduplication logic for both IDs and phone numbers is a major improvement in correctness and clarity. The UI changes also enhance user visibility into duplicate records. I have one suggestion to improve performance by avoiding a potential N+1 query issue.
| individual_id_docs = {} | ||
| # Check ID Documents of each individual | ||
| reg_id_to_partner = {} | ||
| for i in individuals: | ||
| for x in i.reg_ids: | ||
| if x.id_type_id in self.supported_id_document_type_ids and ( | ||
| (not x.expiry_date) or x.expiry_date > date.today() | ||
| ): | ||
| id_doc_id_with_id_type_and_value = {x.id: x.id_type_id.name + "-" + x.value} | ||
| individual_id_docs.update(id_doc_id_with_id_type_and_value) | ||
| doc_key = x.id_type_id.display + "-" + x.value | ||
| individual_id_docs[x.id] = doc_key | ||
| reg_id_to_partner[x.id] = i.id |
There was a problem hiding this comment.
This nested loop over individuals and their reg_ids can cause N+1 query performance issues, especially with a large number of beneficiaries. It's more efficient to fetch all relevant spp.registry.id records in a single search and then process them.
| individual_id_docs = {} | |
| # Check ID Documents of each individual | |
| reg_id_to_partner = {} | |
| for i in individuals: | |
| for x in i.reg_ids: | |
| if x.id_type_id in self.supported_id_document_type_ids and ( | |
| (not x.expiry_date) or x.expiry_date > date.today() | |
| ): | |
| id_doc_id_with_id_type_and_value = {x.id: x.id_type_id.name + "-" + x.value} | |
| individual_id_docs.update(id_doc_id_with_id_type_and_value) | |
| doc_key = x.id_type_id.display + "-" + x.value | |
| individual_id_docs[x.id] = doc_key | |
| reg_id_to_partner[x.id] = i.id | |
| individual_id_docs = {} | |
| reg_id_to_partner = {} | |
| all_reg_ids = self.env["spp.registry.id"].search([ | |
| ("partner_id", "in", individuals.ids), | |
| ("id_type_id", "in", self.supported_id_document_type_ids.ids), | |
| ]) | |
| for x in all_reg_ids: | |
| if (not x.expiry_date) or x.expiry_date > date.today(): | |
| doc_key = f"{x.id_type_id.display}-{x.value}" | |
| individual_id_docs[x.id] = doc_key | |
| reg_id_to_partner[x.id] = x.partner_id.id |
…ment, and membership
1 participant
Why is this change needed?
Several bugs in the deduplication and enrollment workflows cause incorrect behavior:
duplicatedandexitedstates, undoing deduplication resultsspp.id.typemodel instead ofspp.vocabulary.codeHow was the change implemented?
Bug fixes
duplicatedandexitedstates are excluded from bulk enrollment and individual enrollment, preventing accidental state overwritessupported_id_document_type_idsnow usesspp.vocabulary.codewith domain filter for ID types, and uses.displayinstead of.nameduplicate_reasonfield onspp.program.membershipshown in the form view when state isduplicated_check_duplicate_by_individualfor both ID and phone deduplication now properly groups duplicates by shared value and applies keep-one-enrolled logicTest coverage
test_deduplication.py— 30+ tests covering default, ID document, and phone number deduplication (shared members, detailed reasons, keep-one-enrolled, expired IDs, unsupported types, record duplicate)test_program_membership.py— 18+ tests covering membership lifecycle (enrollment protection for duplicated/exited, pause/resume/exit, duplicate reason, eligibility verification, deduplication from membership, form view context)test_program_enrollment.py— 20+ tests covering program-level enrollment and deduplication workflows (enrollment skips duplicated/exited, dedup summary messages, program model methods)__init__.pyto import previously missing test files (test_create_program_wizard_inkind,test_cycle_auto_approve_fund_check)New unit tests
spp_programs/tests/test_deduplication.pyspp_programs/tests/test_program_membership.pyspp_programs/tests/test_program_enrollment.pyUnit tests executed by the author
All new tests pass. Pre-existing test failures in
test_entitlement_amount_celandtest_cycle_auto_approve_fund_checkare unrelated.How to test manually
Related links