diff --git a/spp_programs/README.rst b/spp_programs/README.rst index b79966144..fd188f6e9 100644 --- a/spp_programs/README.rst +++ b/spp_programs/README.rst @@ -254,6 +254,30 @@ Dependencies Changelog ========= +19.0.2.3.0 +~~~~~~~~~~ + +- feat(spp_programs): **Duplicate Detection is a card with an Add + dialog.** Adding a method asks which method and what to call it, + instead of editing a Reference field that exposed the model/record + plumbing — and could quietly wire another program's method into this + one. Selecting **ID document** now also asks which ID types to + compare, without which the method matched nothing and reported no + duplicates at all (#1171) +- fix(spp_programs): **Deduplicate now clears flags it no longer + finds.** A membership marked as duplicated stayed that way after the + clash behind it was fixed, because a membership already in that state + was never re-evaluated. Each run recomputes rather than accumulates + (#796) +- feat(spp_programs): **validators can return duplicated memberships to + draft in bulk** — a row button on the membership list and a Back to + Draft server action bound to it, so a whole selection can be cleared + instead of opening records one at a time (#1170) +- fix(spp_programs): a removed deduplication method can be added again. + Removing a row unlinked it without deleting it, and the duplicate + check counted the leftover, so the method the card no longer showed + still blocked its own re-adding (#1171) + 19.0.2.2.1 ~~~~~~~~~~ diff --git a/spp_programs/__manifest__.py b/spp_programs/__manifest__.py index 48590c716..7e550a47e 100644 --- a/spp_programs/__manifest__.py +++ b/spp_programs/__manifest__.py @@ -4,7 +4,7 @@ "name": "OpenSPP Programs", "summary": "Manage programs, cycles, beneficiary enrollment, entitlements (cash and in-kind), payments, and fund tracking for social protection.", "category": "OpenSPP/Core", - "version": "19.0.2.2.1", + "version": "19.0.2.3.0", "sequence": 1, "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", @@ -109,6 +109,7 @@ "wizard/reject_entitlement_wizard.xml", "wizard/reject_inkind_entitlement_wizard.xml", "wizard/reset_to_pending_wizard.xml", + "wizard/deduplication_setup_wizard.xml", "wizard/create_program_wizard_compliance_views.xml", "wizard/create_program_wizard_cel_views.xml", "wizard/enrollment_wizard_views.xml", diff --git a/spp_programs/models/program_manager_ui.py b/spp_programs/models/program_manager_ui.py index b44621553..d99fac292 100644 --- a/spp_programs/models/program_manager_ui.py +++ b/spp_programs/models/program_manager_ui.py @@ -241,6 +241,10 @@ class ProgramManagerUI(models.Model): payment_manager_display = fields.Char(compute="_compute_banner_layout_helpers") payment_manager_detail = fields.Text(compute="_compute_banner_layout_helpers") + deduplication_manager_count = fields.Integer(compute="_compute_banner_layout_helpers") + deduplication_manager_display = fields.Char(compute="_compute_banner_layout_helpers") + deduplication_manager_detail = fields.Text(compute="_compute_banner_layout_helpers") + @api.depends("eligibility_manager_ids", "eligibility_manager_ids.manager_ref_id") def _compute_eligibility_summary(self): for rec in self: @@ -413,6 +417,7 @@ def _compute_banner_layout_helpers(self): ("cycle_manager_ids", "cycle"), ("compliance_manager_ids", "compliance"), ("payment_manager_ids", "payment"), + ("deduplication_manager_ids", "deduplication"), ) for rec in self: for field_name, prefix in banners: @@ -595,13 +600,35 @@ def action_configure_payment(self): return False def action_configure_deduplication(self): - """Open deduplication manager configuration.""" + """Open deduplication configuration. + + Unlike compliance or payment, a program may have several deduplication + methods — by ID and by phone, say. Opening ``[0]`` would silently edit + the first and leave the rest unreachable, so the card only offers this + button when there is exactly one; with several, each method has its own + cog in the card body (OP#1171). + """ self.ensure_one() readonly = not self.can_edit_configuration - if self.deduplication_manager_ids and self.deduplication_manager_ids[0].manager_ref_id: - return self.deduplication_manager_ids[0].open_manager_form(readonly=readonly, title=_("Deduplication")) + configured = self.deduplication_manager_ids.filtered(lambda wrapper: wrapper.manager_ref_id) + if len(configured) == 1: + return configured.open_manager_form(readonly=readonly, title=_("Deduplication")) + if len(configured) > 1: + # Not target="new": a list in a dialog cannot drill into a form, so + # it renders as a dead end — rows look clickable and do nothing. + # Opening it in the breadcrumb keeps the rows navigable. The card + # itself lists the methods with their own buttons, so this is a + # fallback for programmatic callers rather than the normal route. + return { + "type": "ir.actions.act_window", + "name": _("Duplicate Detection"), + "res_model": "spp.deduplication.manager", + "view_mode": "list,form", + "domain": [("id", "in", configured.ids)], + "context": {"create": False, "default_program_id": self.id}, + } if not readonly: - return self._open_manager_setup_wizard("deduplication") + return self.action_add_deduplication_manager() return False def action_configure_notification(self): @@ -692,6 +719,30 @@ def action_add_payment_manager(self): }, } + def action_add_deduplication_manager(self): + """Open the two-step dialog for adding a deduplication method (OP#1171). + + Compliance and Payment open their single concrete model directly + (#952, #953). Deduplication has three methods rather than one, so the + dialog has to ask which before it can ask for a name — the wizard does + both, then creates the concrete record with the context that makes + `source_mixin.create()` build the wrapper alongside it. + + Adding is offered even when a method already exists, because a program + may legitimately check by ID *and* by phone. + """ + self.ensure_one() + if not self.can_edit_configuration: + return False + return { + "type": "ir.actions.act_window", + "name": _("Add a Deduplication Method"), + "res_model": "spp.deduplication.setup.wizard", + "view_mode": "form", + "target": "new", + "context": {"default_program_id": self.id}, + } + def _open_manager_setup_wizard(self, manager_type): """Open wizard to set up a new manager of the specified type.""" return { diff --git a/spp_programs/models/program_membership.py b/spp_programs/models/program_membership.py index 0b3c5a532..9ebea36a7 100644 --- a/spp_programs/models/program_membership.py +++ b/spp_programs/models/program_membership.py @@ -10,6 +10,11 @@ _logger = logging.getLogger(__name__) +# States a membership may be returned to draft from. Both are dead ends +# otherwise: deduplication and eligibility checks put a membership here and +# nothing ever took it out again (OP#1170). +RESETTABLE_TO_DRAFT = ("duplicated", "not_eligible") + class SPPProgramMembership(models.Model): _inherit = [ @@ -354,6 +359,12 @@ def deduplicate_beneficiaries(self): message = None kind = "success" if len(deduplication_managers): + # The managers work across the whole program, not just this + # membership, so clear the program's flags first and let the run + # re-apply them. Without this a membership stays "duplicated" long + # after the clash behind it was fixed (OP#796). + self.program_id._reset_duplicate_flags() + states = ["draft", "enrolled", "eligible", "paused", "duplicated"] duplicates = 0 for el in deduplication_managers: @@ -384,13 +395,26 @@ def deduplicate_beneficiaries(self): } def back_to_draft(self): - """Reset membership to draft state.""" - self.write( - { - "state": "draft", - } - ) - return + """Return duplicated or not-eligible memberships to draft. + + Deliberately recordset-safe: the "Back to Draft" server action hands + this a whole selection, and it is offered from lists as well as from + the membership form. + + Guarded because this is now reachable from four places rather than one. + It used to write ``draft`` over any state at all, so a stray call could + quietly undo an enrolment or reopen an exit (OP#1170). + """ + blocked = self.filtered(lambda membership: membership.state not in RESETTABLE_TO_DRAFT) + if blocked: + raise UserError( + _( + "Only duplicated or not-eligible memberships can be returned to draft. " + "%s of the selected memberships are in another state.", + len(blocked), + ) + ) + self.write({"state": "draft"}) def action_pause(self): """Pause the membership.""" diff --git a/spp_programs/models/programs.py b/spp_programs/models/programs.py index 5841fb332..b73d43704 100644 --- a/spp_programs/models/programs.py +++ b/spp_programs/models/programs.py @@ -448,36 +448,65 @@ def verify_eligibility(self): else: raise UserError(_("No Program Manager defined.")) + def _duplicated_memberships(self): + """The memberships this program currently has flagged as duplicates.""" + self.ensure_one() + return self.env["spp.program.membership"].search([("program_id", "=", self.id), ("state", "=", "duplicated")]) + + def _reset_duplicate_flags(self): + """Clear every duplicate flag so a run can re-apply it (OP#796). + + Deduplication only ever added the flag. A membership marked duplicated + stayed that way even once the clash behind it was fixed — correct the + ID or the phone number, run Deduplicate again, and nothing happened, + because a membership already in ``duplicated`` is never re-evaluated + out of it. + + Clearing first turns the run into a recompute: whoever still clashes is + flagged again by the managers a moment later, and whoever no longer does + is simply left in draft. The whole thing is one transaction, so a run + that fails part-way leaves the flags as they were. + + Returns the memberships that were flagged going in, so the caller can + report what the run actually resolved. + """ + self.ensure_one() + flagged = self._duplicated_memberships() + if flagged: + flagged.back_to_draft() + return flagged + def deduplicate_beneficiaries(self): for rec in self: deduplication_managers = rec.get_managers(self.MANAGER_DEDUPLICATION) message = None kind = "success" if len(deduplication_managers): - # Count already-flagged duplicates before running - already_duplicated = self.env["spp.program.membership"].search_count( - [("program_id", "=", rec.id), ("state", "=", "duplicated")] - ) + # Flagged going in — cleared now, and re-applied below to + # whoever is still a duplicate. + previously_flagged = rec._reset_duplicate_flags() states = ["draft", "enrolled", "eligible", "paused", "duplicated"] duplicates = 0 for el in deduplication_managers: duplicates += el.deduplicate_beneficiaries(states) - # Count total duplicates after running - total_duplicated = self.env["spp.program.membership"].search_count( - [("program_id", "=", rec.id), ("state", "=", "duplicated")] - ) - new_duplicates = total_duplicated - already_duplicated + currently_flagged = rec._duplicated_memberships() + total_duplicated = len(currently_flagged) + still_flagged = len(previously_flagged & currently_flagged) + new_duplicates = len(currently_flagged - previously_flagged) + resolved = len(previously_flagged - currently_flagged) - if total_duplicated > 0: + if total_duplicated > 0 or resolved: parts = [] if new_duplicates > 0: parts.append(_("%(new)s new duplicate(s) found", new=new_duplicates)) - if already_duplicated > 0: - parts.append(_("%(existing)s already flagged", existing=already_duplicated)) + if still_flagged > 0: + parts.append(_("%(existing)s still flagged", existing=still_flagged)) + if resolved > 0: + parts.append(_("%(resolved)s no longer duplicate(s)", resolved=resolved)) message = ", ".join(parts) + "." - kind = "warning" + kind = "warning" if total_duplicated else "success" elif duplicates > 0: message = _( "Found %(count)s duplicate beneficiaries.", diff --git a/spp_programs/readme/HISTORY.md b/spp_programs/readme/HISTORY.md index 37046e6ba..e06e722c8 100644 --- a/spp_programs/readme/HISTORY.md +++ b/spp_programs/readme/HISTORY.md @@ -1,3 +1,10 @@ +### 19.0.2.3.0 + +- feat(spp_programs): **Duplicate Detection is a card with an Add dialog.** Adding a method asks which method and what to call it, instead of editing a Reference field that exposed the model/record plumbing — and could quietly wire another program's method into this one. Selecting **ID document** now also asks which ID types to compare, without which the method matched nothing and reported no duplicates at all (#1171) +- fix(spp_programs): **Deduplicate now clears flags it no longer finds.** A membership marked as duplicated stayed that way after the clash behind it was fixed, because a membership already in that state was never re-evaluated. Each run recomputes rather than accumulates (#796) +- feat(spp_programs): **validators can return duplicated memberships to draft in bulk** — a row button on the membership list and a Back to Draft server action bound to it, so a whole selection can be cleared instead of opening records one at a time (#1170) +- fix(spp_programs): a removed deduplication method can be added again. Removing a row unlinked it without deleting it, and the duplicate check counted the leftover, so the method the card no longer showed still blocked its own re-adding (#1171) + ### 19.0.2.2.1 - fix(spp_programs): stop Enroll Eligible undoing a deliberate pause. A paused membership is now left alone wherever eligibility is re-run — the enrol pass, the disenrol sweep that would otherwise have moved it to Not Eligible, and the per-membership methods reachable over RPC. Pausing is a decision that only Resume reverses (#1117) diff --git a/spp_programs/security/ir.model.access.csv b/spp_programs/security/ir.model.access.csv index baced8b9a..d628bf21b 100644 --- a/spp_programs/security/ir.model.access.csv +++ b/spp_programs/security/ir.model.access.csv @@ -404,3 +404,6 @@ access_spp_prepare_entitlement_confirm_wizard_validator,Prepare Entitlement Conf access_spp_program_membership_exit_wizard_officer,Program Membership Exit Wizard Officer Access,spp_programs.model_spp_program_membership_exit_wizard,spp_programs.group_programs_officer,1,1,1,0 access_spp_program_membership_exit_wizard_manager,Program Membership Exit Wizard Manager Access,spp_programs.model_spp_program_membership_exit_wizard,spp_programs.group_programs_manager,1,1,1,1 access_spp_program_membership_exit_wizard_admin,Program Membership Exit Wizard Admin Access,spp_programs.model_spp_program_membership_exit_wizard,spp_security.group_spp_admin,1,1,1,1 +access_spp_deduplication_setup_wizard_manager,Deduplication Setup Wizard Manager Access,spp_programs.model_spp_deduplication_setup_wizard,group_programs_manager,1,1,1,1 +access_spp_deduplication_setup_wizard_validator,Deduplication Setup Wizard Validator Access,spp_programs.model_spp_deduplication_setup_wizard,group_programs_validator,1,1,1,0 +access_spp_deduplication_setup_wizard_admin,Deduplication Setup Wizard Admin Access,spp_programs.model_spp_deduplication_setup_wizard,spp_security.group_spp_admin,1,1,1,1 diff --git a/spp_programs/static/description/index.html b/spp_programs/static/description/index.html index 71163ce80..d7dbcaed6 100644 --- a/spp_programs/static/description/index.html +++ b/spp_programs/static/description/index.html @@ -658,6 +658,31 @@

Changelog

+

19.0.2.3.0

+ +
+

19.0.2.2.1

-
+

19.0.2.1.3

-
+

19.0.2.1.2

  • fix(security): add global ir.rule records on @@ -700,7 +725,7 @@

    19.0.2.1.2

    no-op for users with no center areas (global roles).
-
+

19.0.2.1.1

  • fix(views): apply spp_registry.x2many_no_padding widget to the @@ -709,7 +734,7 @@

    19.0.2.1.1

    19 inserts on inline list-in-form views (#943).
-
+

19.0.2.0.11

  • Fix TypeError: 'NoneType' object is not iterable when clicking @@ -720,7 +745,7 @@

    19.0.2.0.11

    omit the state filter instead of crashing on tuple(None)
-
+

19.0.2.0.10

  • Increase parallel-safe channel limits (cycle, eligibility_manager, @@ -733,7 +758,7 @@

    19.0.2.0.10

    submission on double-click
-
+

19.0.2.0.9

  • Add context flags (skip_registrant_statistics, @@ -746,7 +771,7 @@

    19.0.2.0.9

    _compute_has_members
-
+

19.0.2.0.8

  • Replace OFFSET pagination with NTILE-based ID-range batching in all @@ -757,7 +782,7 @@

    19.0.2.0.8

    program and cycle
-
+

19.0.2.0.7

  • Bulk membership creation using raw SQL INSERT ON CONFLICT DO NOTHING @@ -766,7 +791,7 @@

    19.0.2.0.7

    _add_beneficiaries with bulk SQL path
-
+

19.0.2.0.6

  • Remove unused entitlement_base_model.py (dead code, never imported)
  • @@ -775,34 +800,34 @@

    19.0.2.0.6

    payment, and fund tests (172 → 492 tests)
-
+

19.0.2.0.5

  • Batch create entitlements and payments instead of one-by-one ORM creates
-
+

19.0.2.0.4

  • Fetch fund balance once per approval batch instead of per entitlement
-
+

19.0.2.0.3

  • Replace cycle computed fields (total_amount, entitlements_count, approval flags) with SQL aggregation queries
-
+

19.0.2.0.2

  • Add composite indexes for frequent query patterns on entitlements and program memberships
-
+

19.0.2.0.1

  • Replace Python-level uniqueness checks with SQL UNIQUE constraints for @@ -811,7 +836,7 @@

    19.0.2.0.1

    constraint creation
-
+

19.0.2.0.0

  • Initial migration to OpenSPP2
  • diff --git a/spp_programs/tests/__init__.py b/spp_programs/tests/__init__.py index 15dc1cbe6..294dd9c82 100644 --- a/spp_programs/tests/__init__.py +++ b/spp_programs/tests/__init__.py @@ -5,6 +5,7 @@ from . import test_create_program_wizard_cel from . import test_cycle from . import test_deduplication +from . import test_deduplication_setup_wizard from . import test_eligibility_cel from . import test_eligibility_cel_integration from . import test_enrollment_wizard diff --git a/spp_programs/tests/test_deduplication.py b/spp_programs/tests/test_deduplication.py index f7242ebe2..a152ae511 100644 --- a/spp_programs/tests/test_deduplication.py +++ b/spp_programs/tests/test_deduplication.py @@ -1,7 +1,7 @@ # Part of OpenSPP. See LICENSE file for full copyright and licensing details. import uuid -from odoo.tests import TransactionCase +from odoo.tests import TransactionCase, tagged class TestDeduplicationCommon(TransactionCase): @@ -475,3 +475,80 @@ def test_eligibility_selection_includes_id_and_phone(self): names = [s[0] for s in selection] self.assertIn("spp.program.membership.manager.id_dedup", names) self.assertIn("spp.program.membership.manager.phone_number", names) + + +@tagged("post_install", "-at_install") +class TestDeduplicationIsResolvable(TestDeduplicationCommon): + """OP#796: running Deduplicate again must clear a resolved duplicate. + + The run 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 ``duplicated`` was never re-evaluated out of it. Fixing the data + and running again did nothing, which is what left validators with "almost + no way for duplicated records to reset back to draft". + """ + + def _shared_member_setup(self): + """Two groups sharing one individual — the classic duplicate.""" + shared = self._create_individual("Shared Member") + group_a = self._create_group("Group A", [shared]) + group_b = self._create_group("Group B", [shared]) + membership_a = self._enroll_in_program(group_a, "draft") + membership_b = self._enroll_in_program(group_b, "draft") + return shared, membership_a, membership_b + + def test_resolving_the_clash_clears_the_flag_on_the_next_run(self): + shared, membership_a, membership_b = self._shared_member_setup() + + self.program.deduplicate_beneficiaries() + self.assertEqual(membership_a.state, "duplicated") + self.assertEqual(membership_b.state, "duplicated") + + # The clash is resolved: the shared individual leaves one of the groups. + self.env["spp.group.membership"].search( + [("group", "=", membership_b.partner_id.id), ("individual", "=", shared.id)] + ).unlink() + + self.program.deduplicate_beneficiaries() + + self.assertEqual(membership_a.state, "draft", "the flag should have been lifted") + self.assertEqual(membership_b.state, "draft", "the flag should have been lifted") + + def test_an_unresolved_duplicate_stays_flagged(self): + """The recompute must not simply clear everything.""" + _shared, membership_a, membership_b = self._shared_member_setup() + + self.program.deduplicate_beneficiaries() + self.program.deduplicate_beneficiaries() + + self.assertEqual(membership_a.state, "duplicated") + self.assertEqual(membership_b.state, "duplicated") + + def test_running_twice_is_stable(self): + """Two runs in a row leave the same set flagged, not a growing one.""" + _shared, membership_a, membership_b = self._shared_member_setup() + unrelated = self._enroll_in_program(self._create_group("Group C"), "enrolled") + + self.program.deduplicate_beneficiaries() + first = {membership_a.state, membership_b.state} + + self.program.deduplicate_beneficiaries() + + self.assertEqual({membership_a.state, membership_b.state}, first) + self.assertEqual(unrelated.state, "enrolled", "a clean membership must be left alone") + + def test_membership_level_run_also_recomputes(self): + """The Deduplicate button on a membership goes through the same path.""" + shared, membership_a, membership_b = self._shared_member_setup() + + membership_a.deduplicate_beneficiaries() + self.assertEqual(membership_a.state, "duplicated") + + self.env["spp.group.membership"].search( + [("group", "=", membership_b.partner_id.id), ("individual", "=", shared.id)] + ).unlink() + + membership_a.deduplicate_beneficiaries() + + self.assertEqual(membership_a.state, "draft") + self.assertEqual(membership_b.state, "draft") diff --git a/spp_programs/tests/test_deduplication_setup_wizard.py b/spp_programs/tests/test_deduplication_setup_wizard.py new file mode 100644 index 000000000..2539934ea --- /dev/null +++ b/spp_programs/tests/test_deduplication_setup_wizard.py @@ -0,0 +1,349 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""OP#1171: adding a deduplication method should not expose the plumbing. + +Adding one used to mean editing the wrapper's ``manager_ref_id`` inline — a +Reference field asking for a model and then a record of it. These tests cover +the replacement: a dialog that asks for the method and a name, and a card that +matches the other configuration sections. +""" + +from lxml import etree + +from odoo.exceptions import UserError +from odoo.tests import TransactionCase, tagged + + +@tagged("post_install", "-at_install") +class TestDeduplicationSetupWizard(TransactionCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.program = cls.env["spp.program"].create({"name": "Dedup Setup Wizard [TEST]"}) + + def _wizard(self, method, name, with_id_types=True): + wizard = self.env["spp.deduplication.setup.wizard"].create( + {"program_id": self.program.id, "method": method, "name": name} + ) + if with_id_types and method == "spp.deduplication.manager.id_dedup": + # The dialog requires these for the ID method, so the helper fills + # them in the way a user would. An ID manager with none set compares + # nothing (OP#1171 round 2). + wizard.supported_id_document_type_ids = self._id_types() + return wizard + + def _id_types(self, limit=2): + return self.env["spp.vocabulary.code"].search( + [("vocabulary_id.namespace_uri", "=", "urn:openspp:vocab:id-type")], + limit=limit, + ) + + def _wrappers(self): + return self.env["spp.deduplication.manager"].search([("program_id", "=", self.program.id)]) + + def test_it_creates_the_concrete_manager_and_its_wrapper(self): + """One dialog, both records — the wrapper is what the program reads.""" + self._wizard("spp.deduplication.manager.id_dedup", "By ID").action_create_manager() + + wrappers = self._wrappers() + self.assertEqual(len(wrappers), 1, "the program should have one deduplication method") + concrete = wrappers.manager_ref_id + self.assertEqual(concrete._name, "spp.deduplication.manager.id_dedup") + self.assertEqual(concrete.name, "By ID") + self.assertEqual(concrete.program_id, self.program) + + def test_each_method_can_be_added(self): + for method, name in ( + ("spp.deduplication.manager.default", "Shared members"), + ("spp.deduplication.manager.id_dedup", "By ID"), + ("spp.deduplication.manager.phone_number", "By phone"), + ): + with self.subTest(method=method): + self._wizard(method, name).action_create_manager() + + self.assertEqual(len(self._wrappers()), 3, "a program may check by more than one method") + + def test_the_same_method_cannot_be_added_twice(self): + """Two identical methods would just run the same check twice.""" + self._wizard("spp.deduplication.manager.phone_number", "By phone").action_create_manager() + + with self.assertRaises(UserError): + self._wizard("spp.deduplication.manager.phone_number", "By phone again").action_create_manager() + + self.assertEqual(len(self._wrappers()), 1) + + def test_the_name_is_suggested_from_the_method(self): + """The second step should be one keystroke, not a blank field.""" + wizard = self.env["spp.deduplication.setup.wizard"].new( + {"program_id": self.program.id, "method": "spp.deduplication.manager.phone_number"} + ) + wizard._onchange_method_suggests_a_name() + self.assertEqual(wizard.name, "Phone number") + + # A name the user typed is left alone. + wizard.name = "Our own wording" + wizard.method = "spp.deduplication.manager.id_dedup" + wizard._onchange_method_suggests_a_name() + self.assertEqual(wizard.name, "Our own wording") + + def test_the_method_description_follows_the_selection(self): + wizard = self._wizard("spp.deduplication.manager.default", "Shared members") + self.assertIn("member in common", wizard.method_description) + + # ------------------------------------------------------------------ + # the program card + # ------------------------------------------------------------------ + + def test_the_count_drives_the_card_zero_state(self): + self.assertEqual(self.program.deduplication_manager_count, 0) + + self._wizard("spp.deduplication.manager.id_dedup", "By ID").action_create_manager() + self.program.invalidate_recordset() + + self.assertEqual(self.program.deduplication_manager_count, 1) + + def test_duplicate_detection_is_a_card_like_the_others(self): + """It was the last section still rendered as a bare group with an + inline list, which is what put the Reference field in front of users.""" + arch = etree.fromstring(self.env.ref("spp_programs.view_program_form_config_cards").arch) + + headings = arch.xpath("//div[contains(@class, 'card-header')]//h5/text()") + self.assertIn( + "Duplicate Detection", + [h.strip() for h in headings], + f"Duplicate Detection should be a card beside the others, found {headings}", + ) + + add_buttons = arch.xpath("//button[@name='action_add_deduplication_manager']") + self.assertTrue(add_buttons, "the card needs an Add button") + + def test_the_reference_field_is_gone_from_the_card(self): + """No inline manager_ref_id list for deduplication any more.""" + arch = etree.fromstring(self.env.ref("spp_programs.view_program_form_config_cards").arch) + inline = arch.xpath("//field[@name='deduplication_manager_ids']//field[@name='manager_ref_id']") + self.assertFalse(inline, "the wrapper's Reference field should no longer be edited inline") + + # ------------------------------------------------------------------ + # access + # ------------------------------------------------------------------ + + def test_a_programs_manager_can_actually_use_it(self): + """Guards the ACL, which the rest of this file cannot. + + Tests run as superuser, and superuser bypasses access rules entirely. + The first version of this wizard shipped with no ir.model.access row at + all: every test here passed, and the first real user to click Add got + "You are not allowed to access 'Add a Deduplication Method' records — + no group currently allows this operation". + """ + manager = self.env["res.users"].create( + { + "name": "Dedup Wizard Manager [TEST]", + "login": "dedup_wizard_manager_test", + "email": "dedup_wizard_manager@example.test", + "group_ids": [ + ( + 6, + 0, + [ + self.env.ref("base.group_user").id, + self.env.ref("spp_programs.group_programs_manager").id, + ], + ) + ], + } + ) + + wizard = ( + self.env["spp.deduplication.setup.wizard"] + .with_user(manager) + .create( + { + "program_id": self.program.id, + "method": "spp.deduplication.manager.phone_number", + "name": "By phone", + } + ) + ) + wizard.action_create_manager() + + self.assertEqual(len(self._wrappers()), 1, "a programs manager should be able to add a method") + + # ------------------------------------------------------------------ + # editing when there is more than one method + # ------------------------------------------------------------------ + + def test_edit_opens_the_method_when_there_is_only_one(self): + self._wizard("spp.deduplication.manager.id_dedup", "By ID").action_create_manager() + self.program.invalidate_recordset() + + action = self.program.action_configure_deduplication() + + self.assertEqual(action.get("res_model"), "spp.deduplication.manager.id_dedup") + + def test_edit_is_not_offered_when_there_are_several(self): + """One button cannot sensibly open two methods. + + It used to open deduplication_manager_ids[0] — silently editing the + first and leaving the second unreachable. Opening a list in a dialog + was no better: a dialog list cannot drill into a form, so the rows + looked clickable and did nothing. With several methods the header + offers no Edit at all, and each row in the card body carries its own. + """ + arch = etree.fromstring(self.env.ref("spp_programs.view_program_form_config_cards").arch) + edit = arch.xpath("//button[@name='action_configure_deduplication'][contains(@class,'btn-primary')]")[0] + self.assertIn( + "deduplication_manager_count != 1", + edit.get("invisible") or "", + "Edit should only appear when there is exactly one method", + ) + + def test_the_multi_method_fallback_is_navigable(self): + """If anything does call it with several, it must not be a dead end.""" + self._wizard("spp.deduplication.manager.id_dedup", "By ID").action_create_manager() + self._wizard("spp.deduplication.manager.phone_number", "By phone").action_create_manager() + self.program.invalidate_recordset() + + action = self.program.action_configure_deduplication() + + self.assertEqual(action.get("res_model"), "spp.deduplication.manager") + self.assertNotEqual(action.get("target"), "new", "a dialog list cannot open a form") + _field, _operator, ids = action["domain"][0] + self.assertEqual(len(ids), 2, "both methods should be reachable") + + def test_the_card_lists_every_method_not_a_summary_line(self): + """Each configured method gets its own row and its own cog.""" + arch = etree.fromstring(self.env.ref("spp_programs.view_program_form_config_cards").arch) + rows = arch.xpath("//field[@name='deduplication_manager_ids']//field[@name='display_name']") + cogs = arch.xpath("//field[@name='deduplication_manager_ids']//button[@name='open_manager_form']") + + self.assertTrue(rows, "the card should list the methods") + self.assertTrue(cogs, "each method needs its own way in") + + def test_the_card_does_not_offer_add_a_line(self): + """Adding goes through the Add button, which asks for the method. + + The row has to be denied through ``link``. The field is a Many2many, + and for those the list renderer reads + ``"link" in activeActions ? link : create`` — so the list's create="0" + and a create domain were both ignored, the row stayed, and it opened + the link picker: every deduplication manager in the database, other + programs' included (OP#1171 round 1). + + ``unlink`` is deliberately left out: removing a method stays available. + """ + arch = etree.fromstring(self.env.ref("spp_programs.view_program_form_config_cards").arch) + field = arch.xpath("//field[@name='deduplication_manager_ids']")[0] + options = field.get("options") or "" + + self.assertIn("'link'", options, "the link row is what the renderer shows for a Many2many") + self.assertIn("'create'", options, "create must be denied too") + self.assertNotIn("'unlink'", options, "removing a method must stay possible") + self.assertEqual(field.xpath("./list")[0].get("create"), "0") + + # ------------------------------------------------------------------ + # the ID document method + # ------------------------------------------------------------------ + + def test_the_dialog_asks_which_id_types_to_compare(self): + """The field belongs to this method, so the dialog has to carry it. + + `deduplicate_beneficiaries` keeps only documents whose type is in + supported_id_document_type_ids, so a manager created without any finds + no duplicates at all and says nothing about why (OP#1171 round 2). + """ + arch = etree.fromstring(self.env.ref("spp_programs.view_deduplication_setup_wizard_form").arch) + field = arch.xpath("//field[@name='supported_id_document_type_ids']")[0] + + self.assertIn("id_dedup", field.get("invisible") or "", "only the ID method compares documents") + self.assertIn("id_dedup", field.get("required") or "", "an empty list would match nothing") + + def test_the_chosen_id_types_reach_the_manager(self): + id_types = self._id_types() + self.assertTrue(id_types, "spp_vocabulary seeds ID types; the dialog needs them") + wizard = self._wizard("spp.deduplication.manager.id_dedup", "By ID") + wizard.supported_id_document_type_ids = id_types + + wizard.action_create_manager() + + concrete = self._wrappers().manager_ref_id + self.assertEqual(concrete.supported_id_document_type_ids, id_types) + + def test_the_id_method_is_refused_without_a_type(self): + """Required in the view covers the dialog, not a programmatic caller.""" + with self.assertRaises(UserError): + self._wizard("spp.deduplication.manager.id_dedup", "By ID", with_id_types=False).action_create_manager() + + self.assertFalse(self._wrappers(), "nothing should be created") + + def test_the_other_methods_do_not_ask_for_id_types(self): + for method in ("spp.deduplication.manager.default", "spp.deduplication.manager.phone_number"): + with self.subTest(method=method): + program = self.env["spp.program"].create({"name": f"No ID types {method} [TEST]"}) + wizard = self.env["spp.deduplication.setup.wizard"].create( + {"program_id": program.id, "method": method, "name": "No types"} + ) + + wizard.action_create_manager() + + self.assertEqual(len(program.deduplication_manager_ids), 1) + + # ------------------------------------------------------------------ + # removing a method + # ------------------------------------------------------------------ + + def test_a_method_can_be_added_again_after_it_is_removed(self): + """The ✕ removes the relation, not the record. + + QA removed a method and could not add it back: the duplicate check + searched wrappers by ``program_id`` and found the one the card no + longer showed (OP#1171 round 1). + """ + self._wizard("spp.deduplication.manager.default", "Shared members").action_create_manager() + self.program.invalidate_recordset() + removed = self.program.deduplication_manager_ids + concrete = removed.manager_ref_id + self.program.write({"deduplication_manager_ids": [(3, removed.id)]}) + + self._wizard("spp.deduplication.manager.default", "Shared members").action_create_manager() + self.program.invalidate_recordset() + + self.assertEqual(len(self.program.deduplication_manager_ids), 1, "the method should be back") + self.assertFalse(removed.exists(), "the removed wrapper should not linger") + self.assertFalse(concrete.exists(), "nor the method behind it") + + def test_the_sweep_survives_a_reference_pointing_nowhere(self): + """manager_ref_id is a Reference: no foreign key, so it can dangle. + + Unlinking it blind would raise MissingError, and it would raise it on + the Add button — the one place this sweep runs. + """ + self._wizard("spp.deduplication.manager.default", "Shared members").action_create_manager() + self.program.invalidate_recordset() + dangling = self.program.deduplication_manager_ids + dangling.manager_ref_id.unlink() # takes the wrapper with it + dangling = self.env["spp.deduplication.manager"].create( + { + "program_id": self.program.id, + "manager_ref_id": "spp.deduplication.manager.default,999999999", + } + ) + + self._wizard("spp.deduplication.manager.default", "Shared members").action_create_manager() + self.program.invalidate_recordset() + + self.assertFalse(dangling.exists(), "the stale wrapper should be swept") + self.assertEqual(len(self.program.deduplication_manager_ids), 1) + + def test_the_sweep_spares_a_method_another_program_uses(self): + """The relation is a Many2many; a linked wrapper is not garbage.""" + self._wizard("spp.deduplication.manager.id_dedup", "By ID").action_create_manager() + self.program.invalidate_recordset() + shared = self.program.deduplication_manager_ids + other = self.env["spp.program"].create({"name": "Dedup Sweep Bystander [TEST]"}) + self.program.write({"deduplication_manager_ids": [(3, shared.id)]}) + other.write({"deduplication_manager_ids": [(4, shared.id)]}) + + self._wizard("spp.deduplication.manager.phone_number", "By phone").action_create_manager() + + self.assertTrue(shared.exists(), "another program still uses this method") + self.assertIn(shared, other.deduplication_manager_ids) diff --git a/spp_programs/tests/test_program_membership.py b/spp_programs/tests/test_program_membership.py index 421bf6f46..58198bf91 100644 --- a/spp_programs/tests/test_program_membership.py +++ b/spp_programs/tests/test_program_membership.py @@ -60,6 +60,21 @@ def _create_membership(self, partner=None, program=None, state="draft"): } ) + def _membership_for_a_new_registrant(self, state): + """A membership on its own registrant. + + spp.program.membership is unique per (partner, program), so tests that + need several memberships at once cannot share the class fixture. + """ + partner = self.env["res.partner"].create( + { + "name": f"Back To Draft {state} [TEST]", + "is_registrant": True, + "is_group": False, + } + ) + return self._create_membership(partner=partner, state=state) + # ------------------------------------------------------------------ # Creation # ------------------------------------------------------------------ @@ -200,13 +215,57 @@ def test_13_action_exit_from_invalid_state_raises(self): with self.assertRaisesRegex(UserError, "Only enrolled or paused memberships can be exited"): membership.action_exit() - def test_14_back_to_draft_resets_state(self): - """back_to_draft() resets any membership to 'draft'.""" - membership = self._create_membership(state="enrolled") + def test_14_back_to_draft_resets_a_duplicated_membership(self): + """The dead end this exists to undo (OP#1170).""" + membership = self._membership_for_a_new_registrant("duplicated") + membership.back_to_draft() + + self.assertEqual(membership.state, "draft") + + def test_14a_back_to_draft_resets_a_not_eligible_membership(self): + membership = self._membership_for_a_new_registrant("not_eligible") membership.back_to_draft() self.assertEqual(membership.state, "draft") + def test_14b_back_to_draft_refuses_other_states(self): + """It used to overwrite any state, which could undo an enrolment. + + Now reachable from a list row, a bulk action and the form, so a stray + call is much easier to make than when it lived on the form alone. + """ + for state in ("draft", "enrolled", "paused", "exited"): + with self.subTest(state=state): + membership = self._membership_for_a_new_registrant(state) + with self.assertRaises(UserError): + membership.back_to_draft() + self.assertEqual(membership.state, state, "the state should be untouched") + + def test_14c_back_to_draft_handles_a_recordset(self): + """The bulk action hands it a selection, not one record.""" + duplicated = self._membership_for_a_new_registrant("duplicated") + not_eligible = self._membership_for_a_new_registrant("not_eligible") + + (duplicated | not_eligible).back_to_draft() + + self.assertEqual(duplicated.state, "draft") + self.assertEqual(not_eligible.state, "draft") + + def test_14d_back_to_draft_rejects_a_mixed_selection_whole(self): + """A selection containing an ineligible record is refused outright. + + The server action filters before calling, so the UI never sends a mixed + selection; this is the guard behind it, and it must not half-apply. + """ + duplicated = self._membership_for_a_new_registrant("duplicated") + enrolled = self._membership_for_a_new_registrant("enrolled") + + with self.assertRaises(UserError): + (duplicated | enrolled).back_to_draft() + + self.assertEqual(duplicated.state, "duplicated", "nothing should have been written") + self.assertEqual(enrolled.state, "enrolled") + # ------------------------------------------------------------------ # Full lifecycle # ------------------------------------------------------------------ diff --git a/spp_programs/views/managers/deduplication_manager_view.xml b/spp_programs/views/managers/deduplication_manager_view.xml index d1220a776..3847590be 100644 --- a/spp_programs/views/managers/deduplication_manager_view.xml +++ b/spp_programs/views/managers/deduplication_manager_view.xml @@ -169,9 +169,18 @@ Part of OpenSPP. See LICENSE file for full copyright and licensing details.
+
- - - - - - -
- Check for duplicate beneficiaries by phone, ID, etc. + + +
+
+
+ +
+
Duplicate Detection
+ Check for duplicate beneficiaries by phone, ID, etc. +
- +
+ + Configured + + + + + + +
+
+
+ +
+ + No duplicate detection configured. It is optional — click Add above to check for duplicate beneficiaries. +
+ - - + + +
+
+ + + +
diff --git a/spp_programs/views/program_membership_view.xml b/spp_programs/views/program_membership_view.xml index f65a67afd..64ae08102 100644 --- a/spp_programs/views/program_membership_view.xml +++ b/spp_programs/views/program_membership_view.xml @@ -31,6 +31,22 @@ Part of OpenSPP. See LICENSE file for full copyright and licensing details. widget="badge" string="Status" /> + +