feat(spp_programs): deduplication — configure by method, recompute on each run, bulk return to draft - #445
Open
emjay0921 wants to merge 9 commits into
Open
feat(spp_programs): deduplication — configure by method, recompute on each run, bulk return to draft#445emjay0921 wants to merge 9 commits into
emjay0921 wants to merge 9 commits into
Conversation
…raft Deduplication and eligibility checks put a membership into "duplicated" or "not eligible" and nothing took it out again. back_to_draft() existed, but only as a button on the membership form, so resolving a duplicate meant opening each record one at a time (OP#1170). Surfaces it where validators actually work: - a row button on the membership list, which is what a program's Duplicates smart button opens; - a "Back to Draft" server action bound to that list, so a whole selection can be cleared at once - the shape spp_programs already uses for "Reset to Draft" on entitlements, including filtering the selection and complaining only when nothing in it qualifies; - a row button on the Participation list of both individual and group registrants, where the inline list gets no action menu of its own. Validator-level in every place, matching the existing form button rather than the officer-level Pause and Exit buttons beside it. Clearing a duplicate flag from a list should not be easier than doing it on the record. back_to_draft() is also guarded now. It wrote "draft" over any state at all, which was fine while one button behind a state modifier was the only caller and is not fine with four. A mixed selection is refused whole rather than half-applied.
Deduplication only ever added the flag. A membership marked "duplicated" stayed that way even once the clash behind it was fixed, because a membership already in that state was never re-evaluated out of it. Correct the ID or the phone number, press Deduplicate again, and nothing happened - which is what left validators with almost no way back to draft (OP#796). Clear the program's flags before the managers run, so the run recomputes rather than accumulates: whoever still clashes is flagged again a moment later, and whoever no longer does is left in draft. Both entry points go through it, the program's button and the membership's. It is one transaction, so a run that fails part-way leaves the flags as they were. The notification was wrong once a run could also un-flag: it derived "new" by subtracting counts, which goes negative as soon as something is resolved. It now compares the recordsets and reports new, still flagged and no longer duplicate. Two things found and deliberately left alone: last_deduplication is declared on the membership and written nowhere, and the duplicate record's duplicate/not_duplicate state is set by nothing, read by nothing and reachable from no button.
The button acts on the membership state, so it reads as following from Status rather than crowding the registrant at the start of the row. The registrant-side participation lists already placed it there. Refs OP#1170
Duplicate Detection was the last section of the program configuration page still rendered as a bare group under Optional Features, with the wrapper's manager_ref_id edited inline. That is a Reference field: it asks the user to pick a model and then find or create a record of it, which exposes the wrapper/concrete split and lets a manager belonging to another program be wired in by mistake. It is now a card beside Compliance, Payment and the rest, and adding a method goes through a dialog asking for the method and a name — the same shape Compliance and Payment got in #952/#953, with the extra step deduplication needs because it has three methods where those have one. The card body lists every configured method with its own cog rather than a summary line: a program may legitimately check by shared members and by phone. The header Edit is therefore offered only when there is exactly one method — a single button cannot open two, and opening them in a dialog list is worse still, because a dialog list cannot drill into a form and the rows look clickable while doing nothing. Refs OP#1171
… removed methods "Add a line" survived on the card. deduplication_manager_ids is a Many2many, and for those the list renderer reads `"link" in activeActions ? link : create`, so the list's create="0" and the create domain were both dead letters. The row that survived opened the link picker: every deduplication manager in the database, other programs' included, ready to be mis-wired into this one. The domain now covers 'link'. 'unlink' is left alone, so the row's x still removes a method. Removing a method that way drops the relation but keeps the wrapper, whose program_id still names the program. The duplicate check searched on that, so a method the card no longer showed still refused to be added back. It now asks the program's own field, and adding first sweeps wrappers that no program links.
The Add dialog asked for a method and a name, which is not enough for the ID document method. Its check is `id_type_id in supported_id_document_type_ids`, so a manager created with that list empty matches nothing and reports no duplicates at all — the card says Configured, Deduplicate runs, and nothing is ever flagged. QA asked for the list to appear on selecting ID document (OP#1171 finding 5). The dialog now shows the ID document types when that method is selected, with the same widget and options as the manager's own form, and requires at least one. action_create_manager refuses an empty list as well, because the view's required only binds the client and a programmatic caller would otherwise create the silent no-op. The manager's own form — reached from the card's cog — now requires the field too. Without that, the same empty manager could still be saved from there. Existing records keep what they have until someone edits them. Six tests in the wizard's suite created the ID method without types, which is the state now refused; they set them the way a user would, and one test deliberately omits them to assert the refusal.
19.0.2.3.0 — a minor, since it adds the deduplication setup wizard model and its access rows alongside the behaviour fixes. Covers OP#1171, OP#796 and OP#1170; the bump is required for the new model, the ACL rows and the view changes to load at all on an existing database.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 19.0 #445 +/- ##
==========================================
+ Coverage 75.01% 75.39% +0.38%
==========================================
Files 524 542 +18
Lines 35301 35879 +578
==========================================
+ Hits 26480 27051 +571
- Misses 8821 8828 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
emjay0921
marked this pull request as ready for review
August 20, 2026 10:36
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Covers four tickets, all QA-passed: OP#1171, OP#796, OP#1170 and their parent OP#745.
Why is this change needed?
Adding a deduplication method meant editing the wrapper's
manager_ref_id— a Reference field, which asks the user to pick a model and then find or create a record of it. That exposes internal plumbing, and picking an existing record belonging to another program silently mis-wires the manager: it keeps running against the program it was created for while this program's form claims it is configured (OP#1171).Two more faults sat behind it:
duplicatedstayed that way after the clash behind it was fixed, because a membership already in that state was never re-evaluated out of it (OP#796).back_to_draft()existed only as a button on the membership form, so clearing duplicates meant opening records one at a time (OP#1170).How was the change implemented?
id_type_id in supported_id_document_type_ids, so a method created with none set matched nothing and reported no duplicates at all — the card said Configured and Deduplicate ran clean. Required in the dialog and on the method's own form, and refused server-side.link, notcreate— socreate="0"on the list was never read, and the row it left opened a picker listing every program's methods. Both keys are denied;unlinkis untouched, so the ✕ still removes a method.spp_programsalready uses for Reset to Draft on entitlements.New unit tests
spp_programs/tests/test_deduplication_setup_wizard.py(21 tests) covers the dialog end to end: each method can be added, the same one cannot be added twice, a removed one can be added back, the sweep spares a method another program links, a dangling Reference does not crash the Add button, the ID method is refused without types and the chosen types reach the manager. Plus arch assertions that no card offers an add row and that Edit appears only when exactly one method is configured.test_deduplication.pyandtest_program_membership.pycover the recompute and the bulk return.Unit tests executed by the author
Full
spp_programssuite after merging19.0in: 713 tests, 0 failed, 0 errors.How to test manually
Related links
Reviewer notes
spp_programsbumped to 19.0.2.3.0 (minor: adds the setup-wizard model and its ACL rows) with changelog entries, per the in-PR convention.link-vs-createdistinction is the one thing here that arch-level tests cannot prove, since it lives in the web client. QA checked it in the browser; the tests assert the attributes that drive it.