From 468f888b64cd9111bb556dd146c84af8e2a07f71 Mon Sep 17 00:00:00 2001 From: emjay0921 Date: Tue, 18 Aug 2026 13:34:36 +0800 Subject: [PATCH] feat(spp_programs): one Add dialog for program configuration, and keep it per program MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every card on the Configuration tab was filled through an inline list with a manager_ref_id Reference field. That control asks for a model and then a record of it, and both halves offered other programs' managers — the Reference picker directly, and on the Many2many cards the link dialog behind "Add a line". A manager picked that way keeps running against the program it was created for, so the second program's form showed configuration that was never going to apply to it. Adding now goes through one dialog for every card: which method, what to call it, then the method's own form to configure it. The methods on offer come from the wrapper's _selection_manager_ref_id(), so a module that registers one is included without editing the wizard, and a category with none says so instead of opening an empty list. Eligibility, Entitlement, Cycle, Compliance, Payment and Notifications now share one shape: a badge, an Add button, a row per method with its own cog, an empty state, and an Edit button only when there is exactly one method — it used to open the first of several silently. Notifications was the last section still rendered as a bare group and is now a card like the rest. The rows deny both 'create' and 'link'; for a Many2many the renderer reads the second, which is why create="0" alone never suppressed the row. 'unlink' is untouched, so the x still removes a method. Isolation no longer depends on the form: create and write refuse a manager owned by another program, and duplicating a program copies its methods instead of linking the original's. Only what a write adds is checked, so a database that already holds a cross-program link stays editable and the link can be removed. Duplicate Detection is deliberately untouched here — its card is being converted under OP#1171 on another branch. The isolation rules still cover it. --- spp_programs/__manifest__.py | 1 + spp_programs/models/constants.py | 53 +++ spp_programs/models/program_manager_ui.py | 124 +++--- spp_programs/models/programs.py | 96 ++++- spp_programs/security/ir.model.access.csv | 3 + spp_programs/tests/__init__.py | 1 + .../tests/test_manager_setup_wizard.py | 271 +++++++++++++ .../views/program_config_cards_view.xml | 359 ++++++++++++------ spp_programs/wizard/__init__.py | 1 + spp_programs/wizard/manager_setup_wizard.py | 196 ++++++++++ spp_programs/wizard/manager_setup_wizard.xml | 59 +++ 11 files changed, 992 insertions(+), 172 deletions(-) create mode 100644 spp_programs/tests/test_manager_setup_wizard.py create mode 100644 spp_programs/wizard/manager_setup_wizard.py create mode 100644 spp_programs/wizard/manager_setup_wizard.xml diff --git a/spp_programs/__manifest__.py b/spp_programs/__manifest__.py index 8dac1cba2..c3029d903 100644 --- a/spp_programs/__manifest__.py +++ b/spp_programs/__manifest__.py @@ -114,6 +114,7 @@ "wizard/enrollment_wizard_views.xml", "wizard/exit_membership_wizard.xml", "wizard/prepare_entitlement_confirm_wizard.xml", + "wizard/manager_setup_wizard.xml", ], "assets": { "web.assets_backend": [ diff --git a/spp_programs/models/constants.py b/spp_programs/models/constants.py index efb0e14d3..f8c0bf23e 100644 --- a/spp_programs/models/constants.py +++ b/spp_programs/models/constants.py @@ -40,3 +40,56 @@ "spp.compliance.manager": "spp.compliance.manager.default", }, } + +# The cards on a program's Configuration tab (OP#1172). Each names the field on +# spp.program, the wrapper model behind it, and the wording the Add dialog uses. +# The keys match MANAGER_TYPE_INFO's "category" so the two can be read together: +# this map says where a category lives, MANAGER_TYPE_INFO describes the methods +# inside it. +# +# The concrete methods themselves are deliberately absent. They come from the +# wrapper's `_selection_manager_ref_id()`, which is what other modules extend +# when they add one — spp_program_geofence adds an eligibility method that way, +# and a hard-coded list here would never see it. +MANAGER_CATEGORIES = { + "eligibility": { + "field": "eligibility_manager_ids", + "wrapper": "spp.eligibility.manager", + "label": "Eligibility Method", + }, + "entitlement": { + "field": "entitlement_manager_ids", + "wrapper": "spp.program.entitlement.manager", + "label": "Entitlement Type", + }, + "cycle": { + "field": "cycle_manager_ids", + "wrapper": "spp.cycle.manager", + "label": "Cycle Schedule", + }, + "compliance": { + "field": "compliance_manager_ids", + "wrapper": "spp.compliance.manager", + "label": "Compliance Criteria", + }, + "payment": { + "field": "payment_manager_ids", + "wrapper": "spp.program.payment.manager", + "label": "Payment Method", + }, + "deduplication": { + "field": "deduplication_manager_ids", + "wrapper": "spp.deduplication.manager", + "label": "Deduplication Method", + }, + "notification": { + "field": "notification_manager_ids", + "wrapper": "spp.program.notification.manager", + "label": "Notification Channel", + }, + "program": { + "field": "program_manager_ids", + "wrapper": "spp.program.manager", + "label": "Program Manager", + }, +} diff --git a/spp_programs/models/program_manager_ui.py b/spp_programs/models/program_manager_ui.py index b44621553..f05c82936 100644 --- a/spp_programs/models/program_manager_ui.py +++ b/spp_programs/models/program_manager_ui.py @@ -7,6 +7,9 @@ """ from odoo import _, api, fields, models +from odoo.exceptions import UserError + +from .constants import MANAGER_CATEGORIES def _format_recurrence(duration, rrule_type): @@ -241,6 +244,12 @@ class ProgramManagerUI(models.Model): payment_manager_display = fields.Char(compute="_compute_banner_layout_helpers") payment_manager_detail = fields.Text(compute="_compute_banner_layout_helpers") + # OP#1172: Notifications became a card like the rest, so it needs the same + # single-vs-multi helpers the other cards use. + notification_manager_count = fields.Integer(compute="_compute_banner_layout_helpers") + notification_manager_display = fields.Char(compute="_compute_banner_layout_helpers") + notification_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: @@ -403,6 +412,8 @@ def _compute_compliance_summary(self): "compliance_manager_ids.manager_ref_id", "payment_manager_ids", "payment_manager_ids.manager_ref_id", + "notification_manager_ids", + "notification_manager_ids.manager_ref_id", ) def _compute_banner_layout_helpers(self): """Populate the `_manager_count / _display / _detail` fields @@ -413,6 +424,7 @@ def _compute_banner_layout_helpers(self): ("cycle_manager_ids", "cycle"), ("compliance_manager_ids", "compliance"), ("payment_manager_ids", "payment"), + ("notification_manager_ids", "notification"), ) for rec in self: for field_name, prefix in banners: @@ -624,86 +636,68 @@ def action_configure_compliance(self): return self._open_manager_setup_wizard("compliance") return False - def action_add_compliance_manager(self): - """Open the default compliance manager form in create mode. - - The program form's compliance banner shows a `+ Add` zero-state - button when no compliance manager is configured. We open the - concrete model (`spp.compliance.manager.default`) in create mode - with `default_program_id` and `_spp_wrapper_model` in context. - Saving the dialog runs the source-mixin's `create()` override, - which auto-creates the wrapper (see source_mixin.py). Dismissing - the dialog with `X` leaves nothing in the DB — that's the whole - point of #953. + def action_add_manager(self): + """Open the Add dialog for one Configuration card (OP#1172). + + One action serves every card: the button passes its category in the + context, so adding an eligibility method and adding a payment method + are the same gesture instead of one bespoke action per section. + + The methods on offer come from the wrapper, so a category whose module + is not installed says so rather than opening a dialog with an empty + list — notifications have no channel at all until a bridge module such + as SMS is installed. """ self.ensure_one() - if not self.can_edit_configuration: + if not self.can_edit_configuration or self.state == "ended": return False - if self.compliance_manager_ids: - return self.action_configure_compliance() - Concrete = self.env["spp.compliance.manager.default"] + category = self.env.context.get("manager_category") + info = MANAGER_CATEGORIES.get(category) + if not info: + raise UserError(_("Unknown configuration category %s.") % category) + wizard = self.env["spp.manager.setup.wizard"] + methods = wizard._methods_for_category(category) + if not methods: + raise UserError( + _("No %s is available. Install a module that provides one, then add it here.") % info["label"].lower() + ) return { "type": "ir.actions.act_window", - "name": _("Compliance Criteria"), - "res_model": Concrete._name, + "name": _("Add a %s") % info["label"], + "res_model": wizard._name, "view_mode": "form", - "views": [(Concrete.get_manager_view_id(), "form")], + "views": [(False, "form")], "target": "new", "context": { "default_program_id": self.id, - # The mixin's create() will create the wrapper and rely - # on its `program_id` inverse to populate the program's - # One2many `compliance_manager_ids` automatically — no - # m2m write needed. - "_spp_wrapper_model": "spp.compliance.manager", + "default_category": category, + "default_method": methods[0][0], + "default_name": methods[0][1], }, } - def action_add_payment_manager(self): - """Open the default payment manager form in create mode. - - Mirrors `action_add_compliance_manager`. The concrete model's - `create()` override auto-creates the default batch tag if the - form was saved with `create_batch=True` and no tag selected — - so we don't have to pre-create it here (which would orphan the - tag if the user dismisses the dialog). The source-mixin's - `create()` override creates the wrapper, then writes it into - the program's `payment_manager_ids` Many2many because that - field doesn't auto-resolve via the wrapper's `program_id` - inverse. See #953. + def action_add_compliance_manager(self): + """Compliance's Add button, kept for callers that predate OP#1172. + + Compliance opened its concrete form directly (#952) and payment did the + same (#953), while the other cards had no Add at all. Every card now + goes through one dialog, so all this does is name the category. """ - self.ensure_one() - if not self.can_edit_configuration: - return False - if self.payment_manager_ids: - return self.action_configure_payment() - Concrete = self.env["spp.program.payment.manager.default"] - return { - "type": "ir.actions.act_window", - "name": _("Payment Processing"), - "res_model": Concrete._name, - "view_mode": "form", - "views": [(Concrete.get_manager_view_id(), "form")], - "target": "new", - "context": { - "default_program_id": self.id, - "_spp_wrapper_model": "spp.program.payment.manager", - "_spp_program_m2m_field": "payment_manager_ids", - }, - } + return self.with_context(manager_category="compliance").action_add_manager() + + def action_add_payment_manager(self): + """Payment's Add button, kept for callers that predate OP#1172.""" + return self.with_context(manager_category="payment").action_add_manager() def _open_manager_setup_wizard(self, manager_type): - """Open wizard to set up a new manager of the specified type.""" - return { - "type": "ir.actions.client", - "tag": "display_notification", - "params": { - "title": _("Setup Required"), - "message": _("Please add a %s manager first using the list below.") % manager_type, - "sticky": False, - "type": "warning", - }, - } + """Point a caller at the Add dialog for this category (OP#1172). + + This used to pop a warning telling the user to "add a manager using the + list below" — the inline list with the Reference field, which is the + control this ticket removes. The categories it is called with are the + MANAGER_CATEGORIES keys, so it can now open the real thing. + """ + return self.with_context(manager_category=manager_type).action_add_manager() def get_manager_type_options(self, category): """Get available manager type options for a category.""" diff --git a/spp_programs/models/programs.py b/spp_programs/models/programs.py index 5841fb332..e3e44de92 100644 --- a/spp_programs/models/programs.py +++ b/spp_programs/models/programs.py @@ -2,7 +2,7 @@ import logging from odoo import _, api, fields, models -from odoo.exceptions import UserError +from odoo.exceptions import UserError, ValidationError from . import constants @@ -203,6 +203,77 @@ def _check_unique_program_name(self): if existing: raise UserError(_("A program with this name already exists. Program names must be unique.")) + # ------------------------------------------------------------------ + # configuration isolation (OP#1172) + # ------------------------------------------------------------------ + + @staticmethod + def _configuration_fields(): + """The Configuration tab's fields, in one place for the rules below.""" + return [info["field"] for info in constants.MANAGER_CATEGORIES.values()] + + def _check_configuration_is_own(self, field, wrappers): + """Refuse configuration that belongs to another program. + + Every manager names the program it was created for and runs against + that program, so linking one into a second program does not configure + the second — it only makes the form lie about what will happen. The + Configuration tab no longer offers a picker that can do this; this + covers the API, data imports and duplicated programs. + + Only what is being linked now is checked. A database that already holds + a cross-program link from the old picker stays loadable, and the row's + ✕ can still take it off. + """ + self.ensure_one() + for wrapper in wrappers: + concrete = wrapper.manager_ref_id + owner = wrapper.program_id or ( + concrete.program_id if concrete and "program_id" in concrete._fields else False + ) + if owner and owner != self: + raise ValidationError( + _( + "%(method)s belongs to the program %(owner)s, so it cannot be used by " + "%(program)s as well. Each program's configuration is its own — add a " + "method to this program instead." + ) + % { + "method": wrapper.display_name or self.env[wrapper._name]._description, + "owner": owner.display_name, + "program": self.display_name, + } + ) + + def copy(self, default=None): + """Duplicate the program with its own copy of the configuration. + + These fields are mostly Many2many, so a plain copy would link the + source's managers into the duplicate — the sharing this ticket removes. + Each method is copied instead: the duplicate starts configured the same + way and owns what it runs. + """ + default = dict(default or {}) + fields_to_copy = [field for field in self._configuration_fields() if field in self._fields] + for field in fields_to_copy: + default.setdefault(field, False) + new_programs = super().copy(default) + for source, new_program in zip(self, new_programs, strict=False): + for field in fields_to_copy: + context = { + "_spp_wrapper_model": source._fields[field].comodel_name, + "default_program_id": new_program.id, + } + if source._fields[field].type == "many2many": + # A Many2many does not resolve from the wrapper's program_id, + # so the copy has to be linked explicitly — see the source mixin. + context["_spp_program_m2m_field"] = field + for wrapper in source[field]: + concrete = wrapper.manager_ref_id + if concrete and concrete.exists(): + concrete.with_context(**context).copy({"program_id": new_program.id}) + return new_programs + @api.depends("program_membership_ids") def _compute_has_members(self): if self.env.context.get("skip_program_statistics"): @@ -276,6 +347,13 @@ def _compute_can_edit_configuration(self): @api.model def create(self, vals): res = super().create(vals) + # Everything linked at creation is being linked now, so all of it is + # checked. Reading `vals` instead would miss it: base create() is + # model_create_multi, so what arrives here is a list of dicts. + for record in res: + for field in record._configuration_fields(): + if record[field]: + record._check_configuration_is_own(field, record[field]) if self.env.context.get("skip_default_managers"): return res if self.env.context.get("create_default_managers"): @@ -285,6 +363,22 @@ def create(self, vals): res.update({man: [(4, man_ids[man])]}) return res + def write(self, vals): + """Refuse configuration linked in from another program (OP#1172). + + Only the links this write adds are checked, so a database that already + holds a cross-program link stays editable and the link can be removed. + """ + touched = [field for field in self._configuration_fields() if field in vals] + before = {(rec.id, field): set(rec[field].ids) for rec in self for field in touched} + result = super().write(vals) + for rec in self: + for field in touched: + added = set(rec[field].ids) - before[(rec.id, field)] + if added: + rec._check_configuration_is_own(field, rec[field].browse(sorted(added))) + return result + @api.model def create_default_managers(self, program_id): ret_vals = {} diff --git a/spp_programs/security/ir.model.access.csv b/spp_programs/security/ir.model.access.csv index baced8b9a..5c6c93a5d 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_manager_setup_wizard_manager,Manager Setup Wizard Manager Access,spp_programs.model_spp_manager_setup_wizard,group_programs_manager,1,1,1,1 +access_spp_manager_setup_wizard_validator,Manager Setup Wizard Validator Access,spp_programs.model_spp_manager_setup_wizard,group_programs_validator,1,1,1,0 +access_spp_manager_setup_wizard_admin,Manager Setup Wizard Admin Access,spp_programs.model_spp_manager_setup_wizard,spp_security.group_spp_admin,1,1,1,1 diff --git a/spp_programs/tests/__init__.py b/spp_programs/tests/__init__.py index 15dc1cbe6..a08b13f2a 100644 --- a/spp_programs/tests/__init__.py +++ b/spp_programs/tests/__init__.py @@ -43,3 +43,4 @@ from . import test_cycle_null_entitlement_approval from . import test_approve_entitlements_program_isolation from . import test_payment_batch_payment_ids +from . import test_manager_setup_wizard diff --git a/spp_programs/tests/test_manager_setup_wizard.py b/spp_programs/tests/test_manager_setup_wizard.py new file mode 100644 index 000000000..45b4205cc --- /dev/null +++ b/spp_programs/tests/test_manager_setup_wizard.py @@ -0,0 +1,271 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""OP#1172: one way to configure a program, and it stays that program's own. + +Every card on the Configuration tab used to be filled through an inline list +with a `manager_ref_id` Reference field. Both halves of that control offered +other programs' managers, so one program could be configured with another's +while the manager went on running against the program it was created for. + +These tests cover the replacement — one Add dialog for every card — and the +isolation rules that hold whether the configuration arrives from the form, the +API, or a duplicated program. +""" + +from lxml import etree + +from odoo.exceptions import UserError, ValidationError +from odoo.tests import TransactionCase, tagged + +from ..models.constants import MANAGER_CATEGORIES + +# The cards this branch converted. Deduplication is deliberately absent: its +# card is being converted under OP#1171 and lands separately. +CONVERTED = ["eligibility", "entitlement", "cycle", "compliance", "payment", "notification"] + + +@tagged("post_install", "-at_install") +class TestManagerSetupWizard(TransactionCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.program = cls.env["spp.program"].create({"name": "Manager Setup Wizard [TEST]"}) + cls.wizard_model = cls.env["spp.manager.setup.wizard"] + + def _add(self, category, method=None, name="A method", program=None): + """Add a method the way the dialog does.""" + methods = self.wizard_model._methods_for_category(category) + wizard = self.wizard_model.with_context(default_category=category).create( + { + "program_id": (program or self.program).id, + "category": category, + "method": method or methods[0][0], + "name": name, + } + ) + wizard.action_create_manager() + return wizard + + def _configured(self, category, program=None): + return (program or self.program)[MANAGER_CATEGORIES[category]["field"]] + + # ------------------------------------------------------------------ + # the dialog + # ------------------------------------------------------------------ + + def test_every_card_can_be_configured_through_the_dialog(self): + """One dialog, every category — including the One2many one. + + Compliance resolves from the wrapper's program_id while the rest are + Many2many and need an explicit link, which is the step that used to be + forgotten: the manager gets created and the program never picks it up. + """ + for category in CONVERTED: + methods = self.wizard_model._methods_for_category(category) + if not methods: + continue + with self.subTest(category=category): + program = self.env["spp.program"].create({"name": f"Dialog {category} [TEST]"}) + self._add(category, name=f"My {category}", program=program) + + configured = self._configured(category, program) + self.assertEqual(len(configured), 1, f"{category} should be wired to the program") + self.assertEqual(configured.manager_ref_id.name, f"My {category}") + self.assertEqual(configured.manager_ref_id.program_id, program) + + def test_the_methods_come_from_the_wrapper_not_a_list_here(self): + """So a module that adds a method is offered without editing the wizard.""" + wrapper = self.env[MANAGER_CATEGORIES["eligibility"]["wrapper"]] + offered = dict(self.wizard_model._methods_for_category("eligibility")) + + self.assertTrue(offered, "eligibility should offer at least one method") + for model, _label in wrapper._selection_manager_ref_id(): + if model in self.env: + self.assertIn(model, offered, f"{model} is registered on the wrapper but not offered") + + def test_an_unknown_category_offers_nothing(self): + self.assertEqual(self.wizard_model._methods_for_category("not-a-category"), []) + + def test_the_name_is_suggested_from_the_method(self): + methods = self.wizard_model._methods_for_category("eligibility") + if len(methods) < 2: + self.skipTest("needs a category with more than one method") + wizard = self.wizard_model.with_context(default_category="eligibility").new( + {"program_id": self.program.id, "category": "eligibility", "method": methods[0][0]} + ) + wizard._onchange_method_suggests_a_name() + self.assertEqual(wizard.name, methods[0][1]) + + # A name the user typed is left alone. + wizard.name = "Our own wording" + wizard.method = methods[1][0] + wizard._onchange_method_suggests_a_name() + self.assertEqual(wizard.name, "Our own wording") + + def test_the_same_method_cannot_be_added_twice(self): + program = self.env["spp.program"].create({"name": "Twice [TEST]"}) + self._add("eligibility", name="First", program=program) + + with self.assertRaises(UserError): + self._add("eligibility", name="Second", program=program) + + def test_a_method_can_be_added_again_after_it_is_removed(self): + """The ✕ on a Many2many row removes the relation, not the record. + + The leftover kept its program_id, so the duplicate check used to refuse + a method the card no longer showed (OP#1171). + """ + program = self.env["spp.program"].create({"name": "Re-add [TEST]"}) + self._add("eligibility", name="First", program=program) + removed = program.eligibility_manager_ids + program.write({"eligibility_manager_ids": [(3, removed.id)]}) + + self._add("eligibility", name="Second", program=program) + + self.assertEqual(len(program.eligibility_manager_ids), 1, "the method should be back") + self.assertFalse(removed.exists(), "the removed method should not linger") + + def test_a_category_with_no_method_says_so(self): + """Notifications have no channel until a bridge module is installed.""" + empty = [c for c in CONVERTED if not self.wizard_model._methods_for_category(c)] + if not empty: + self.skipTest("every category has a method installed") + with self.assertRaises(UserError): + self.program.with_context(manager_category=empty[0]).action_add_manager() + + def test_add_is_refused_on_an_ended_program(self): + program = self.env["spp.program"].create({"name": "Ended [TEST]", "state": "ended"}) + self.assertFalse(program.with_context(manager_category="eligibility").action_add_manager()) + + def test_the_dead_end_helper_now_opens_the_dialog(self): + """It used to pop "add a manager using the list below" — that list is gone.""" + action = self.program._open_manager_setup_wizard("eligibility") + + self.assertEqual(action.get("type"), "ir.actions.act_window") + self.assertEqual(action.get("res_model"), "spp.manager.setup.wizard") + + # ------------------------------------------------------------------ + # isolation + # ------------------------------------------------------------------ + + def test_another_programs_method_cannot_be_linked_in(self): + owner = self.env["spp.program"].create({"name": "Owner [TEST]"}) + self._add("eligibility", name="Owner's rule", program=owner) + borrower = self.env["spp.program"].create({"name": "Borrower [TEST]"}) + + with self.assertRaises(ValidationError): + borrower.write({"eligibility_manager_ids": [(4, owner.eligibility_manager_ids.id)]}) + + def test_another_programs_method_cannot_be_linked_at_creation(self): + owner = self.env["spp.program"].create({"name": "Owner At Create [TEST]"}) + self._add("eligibility", name="Owner's rule", program=owner) + + with self.assertRaises(ValidationError): + self.env["spp.program"].create( + { + "name": "Borrower At Create [TEST]", + "eligibility_manager_ids": [(4, owner.eligibility_manager_ids.id)], + } + ) + + def test_a_database_that_already_shares_one_stays_editable(self): + """Only what a write adds is checked. + + Rejecting everything already linked would trap a database polluted by + the old picker: the ✕ is itself a write, and with two foreign methods + linked, removing one would be refused because of the other. + """ + owner = self.env["spp.program"].create({"name": "Legacy Owner [TEST]"}) + self._add("eligibility", name="First", program=owner) + self._add("cycle", name="Second", program=owner) + polluted = self.env["spp.program"].create({"name": "Legacy Borrower [TEST]"}) + for field_name, wrapper in ( + ("eligibility_manager_ids", owner.eligibility_manager_ids), + ("cycle_manager_ids", owner.cycle_manager_ids), + ): + field = self.env["spp.program"]._fields[field_name] + self.env.cr.execute( + f"INSERT INTO {field.relation} ({field.column1}, {field.column2}) VALUES (%s, %s)", + (polluted.id, wrapper.id), + ) + polluted.invalidate_recordset() + + # Taking one off is a write on a field that still holds the other. + polluted.write({"eligibility_manager_ids": [(3, owner.eligibility_manager_ids.id)]}) + + self.assertFalse(polluted.eligibility_manager_ids) + self.assertEqual(polluted.cycle_manager_ids, owner.cycle_manager_ids) + + def test_duplicating_a_program_copies_its_configuration(self): + """A plain copy would link the source's methods into the duplicate.""" + source = self.env["spp.program"].create({"name": "Source [TEST]"}) + self._add("eligibility", name="Source rule", program=source) + + duplicate = source.copy({"name": "Duplicate [TEST]"}) + + self.assertTrue(duplicate.eligibility_manager_ids, "the duplicate should be configured too") + self.assertNotEqual( + duplicate.eligibility_manager_ids, + source.eligibility_manager_ids, + "the duplicate must not share the source's method", + ) + self.assertEqual(duplicate.eligibility_manager_ids.manager_ref_id.program_id, duplicate) + self.assertEqual(source.eligibility_manager_ids.manager_ref_id.name, "Source rule") + + # ------------------------------------------------------------------ + # the cards + # ------------------------------------------------------------------ + + def _arch(self): + return etree.fromstring(self.env.ref("spp_programs.view_program_form_config_cards").arch) + + def test_every_card_offers_add(self): + arch = self._arch() + for category in CONVERTED: + with self.subTest(category=category): + buttons = arch.xpath(f"//button[@name='action_add_manager'][contains(@context, \"'{category}'\")]") + self.assertTrue(buttons, f"the {category} card needs an Add button") + + def test_no_card_edits_the_reference_field_inline(self): + """The Reference field is what offered other programs' managers.""" + arch = self._arch() + for category in CONVERTED: + field = MANAGER_CATEGORIES[category]["field"] + with self.subTest(category=category): + self.assertFalse( + arch.xpath(f"//field[@name='{field}']//field[@name='manager_ref_id']"), + f"{category} should list methods, not edit their Reference", + ) + + def test_no_card_offers_add_a_line(self): + """Denied through 'link' as well as 'create'. + + These fields are Many2many, and for those the list renderer reads + `"link" in activeActions ? link : create`, so create="0" on the list + was never consulted and the row it left opened a picker listing every + program's managers. + """ + arch = self._arch() + for category in CONVERTED: + field_name = MANAGER_CATEGORIES[category]["field"] + with self.subTest(category=category): + field = arch.xpath(f"//field[@name='{field_name}']")[0] + options = field.get("options") or "" + self.assertIn("'link'", options, "the link row is what a Many2many shows") + 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") + + def test_edit_is_only_offered_when_there_is_one_method(self): + """One button cannot sensibly open two, and it used to open the first.""" + arch = self._arch() + for category in CONVERTED: + count = f"{category}_manager_count" + with self.subTest(category=category): + edit = arch.xpath(f"//button[@name='action_configure_{category}'][contains(@class,'btn-primary')]")[0] + self.assertIn(count, edit.get("invisible") or "") + + def test_notifications_is_a_card_like_the_rest(self): + """It was the last section still rendered as a bare group.""" + headings = [h.strip() for h in self._arch().xpath("//div[contains(@class, 'card-header')]//h5/text()")] + + self.assertIn("Notifications", headings, f"found {headings}") diff --git a/spp_programs/views/program_config_cards_view.xml b/spp_programs/views/program_config_cards_view.xml index c11f3bfd3..fe05b97fd 100644 --- a/spp_programs/views/program_config_cards_view.xml +++ b/spp_programs/views/program_config_cards_view.xml @@ -20,6 +20,25 @@ Replaces the technical manager configuration with intuitive sections. string="Configuration" groups="spp_security.group_spp_admin,spp_programs.group_programs_manager,spp_programs.group_programs_validator" > + @@ -47,24 +66,46 @@ Replaces the technical manager configuration with intuitive sections. > Configured + +
+
+ + No eligibility method configured — click Add above to choose who qualifies for this program. +
- + - +
+
+ + No entitlement type configured — click Add above to choose what beneficiaries receive. +
- + - +
+
+ + No schedule configured — click Add above to choose how often this program runs. +
- + - - + +
@@ -405,24 +491,23 @@ Replaces the technical manager configuration with intuitive sections. placeholder="No compliance rule configured yet — click Edit above." /> - + - + @@ -516,24 +608,23 @@ Replaces the technical manager configuration with intuitive sections. placeholder="No payment processing configured yet — click Edit above." /> - + - + + + + + +
+ + + + + +
+ + No outgoing mail server is configured. Ask your administrator to set one up under + Settings → Technical → Email → Outgoing Mail Servers before enabling email + notifications. +
+
+ + No notification channel configured. It is optional — click Add above to message beneficiaries. +
+ + - - + +
+ - - + + + + + +
- Send SMS or other notifications to beneficiaries. -
- - -
- - No outgoing mail server is configured. Ask your administrator to set one up under - Settings → Technical → Email → Outgoing Mail Servers before enabling email - notifications. + Check for duplicate beneficiaries by phone, ID, etc.
diff --git a/spp_programs/wizard/__init__.py b/spp_programs/wizard/__init__.py index 333f0b72a..d84c205ef 100644 --- a/spp_programs/wizard/__init__.py +++ b/spp_programs/wizard/__init__.py @@ -15,3 +15,4 @@ from . import enrollment_wizard from . import exit_membership_wizard from . import prepare_entitlement_confirm_wizard +from . import manager_setup_wizard diff --git a/spp_programs/wizard/manager_setup_wizard.py b/spp_programs/wizard/manager_setup_wizard.py new file mode 100644 index 000000000..193696c51 --- /dev/null +++ b/spp_programs/wizard/manager_setup_wizard.py @@ -0,0 +1,196 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""One dialog for adding any program configuration method (OP#1172). + +Every card on a program's Configuration tab used to be filled in the same way: +an inline list with a `manager_ref_id` Reference field and an "Add a line" row. +That control asks the user to pick a *model* and then find or create a record +of it, and both halves of it leak — the Reference picker and, on the Many2many +cards, the link dialog behind "Add a line" both list managers belonging to +other programs, which silently wires another program's configuration into this +one. + +This wizard asks the two questions that actually matter — which method, and +what to call it — and creates a record that belongs to this program only. The +methods on offer come from the wrapper's ``_selection_manager_ref_id()``, so a +module that adds a method (spp_program_geofence adds an eligibility one) shows +up here without touching this file. +""" + +from odoo import _, api, fields, models +from odoo.exceptions import UserError + +from ..models.constants import MANAGER_CATEGORIES +from ..models.program_manager_ui import MANAGER_TYPE_INFO + + +class ManagerSetupWizard(models.TransientModel): + _name = "spp.manager.setup.wizard" + _description = "Add a Configuration Method" + + program_id = fields.Many2one( + "spp.program", + required=True, + readonly=True, + ) + category = fields.Selection( + selection=[(key, info["label"]) for key, info in MANAGER_CATEGORIES.items()], + required=True, + readonly=True, + help="Which card on the Configuration tab this method belongs to.", + ) + method = fields.Selection( + selection="_selection_method", + string="Method", + required=True, + help="How this part of the program is handled. Each method can be added once per program.", + ) + method_description = fields.Char(compute="_compute_method_description") + # Drives whether the Method question is asked at all: a category with one + # method has nothing to choose, and a radio list of one is just noise. + method_count = fields.Integer(compute="_compute_method_count") + name = fields.Char( + string="Name", + required=True, + help="Shown on the program's configuration page.", + ) + + # ------------------------------------------------------------------ + # the methods on offer + # ------------------------------------------------------------------ + + @api.model + def _methods_for_category(self, category): + """The concrete manager models a category can offer, as selection pairs. + + Read from the wrapper rather than from a list here so that methods + added by other modules are included, and so that a method whose module + has been uninstalled drops out instead of raising when it is picked. + MANAGER_TYPE_INFO only supplies nicer wording where it has some. + """ + info = MANAGER_CATEGORIES.get(category) + if not info or info["wrapper"] not in self.env: + return [] + methods = [] + for model, label in self.env[info["wrapper"]]._selection_manager_ref_id(): + if model in self.env: + methods.append((model, MANAGER_TYPE_INFO.get(model, {}).get("name") or label)) + return methods + + @api.model + def _selection_method(self): + """Selection values for the Method field. + + A Selection cannot depend on another field's value, so the category + comes from the context the Add button opens this dialog with. + """ + return self._methods_for_category(self.env.context.get("default_category")) + + @api.depends("method") + def _compute_method_description(self): + for wizard in self: + wizard.method_description = MANAGER_TYPE_INFO.get(wizard.method, {}).get("description", "") + + @api.depends("category") + def _compute_method_count(self): + for wizard in self: + wizard.method_count = len(self._methods_for_category(wizard.category)) + + @api.onchange("method") + def _onchange_method_suggests_a_name(self): + """Pre-fill the name from the method, so naming is one keystroke. + + Only while the user has not typed their own, and only replacing a + suggestion we made ourselves. + """ + labels = dict(self._methods_for_category(self.category)) + if not self.name or self.name in set(labels.values()): + self.name = labels.get(self.method, "") + + # ------------------------------------------------------------------ + # creating the method + # ------------------------------------------------------------------ + + def _sweep_removed_methods(self): + """Delete the methods the card no longer shows. + + Most of these program fields are Many2many, so the ✕ on a row removes + the *relation* and leaves the manager behind with its ``program_id`` + still pointing here. Those leftovers never run — a program is + configured through its own field, not through the managers' + ``program_id`` — but they used to make the duplicate check below refuse + a method the card no longer showed (OP#1171). + + Only managers that no program links are swept: on a Many2many, one this + program created but another program links is that program's method now, + not garbage. + """ + self.ensure_one() + field = MANAGER_CATEGORIES[self.category]["field"] + wrapper = MANAGER_CATEGORIES[self.category]["wrapper"] + removed = self.env[wrapper].search([("program_id", "=", self.program_id.id)]) - self.program_id[field] + if not removed: + return + linked = self.env["spp.program"].search([(field, "in", removed.ids)]) + for leftover in removed - linked[field]: + # The concrete record owns the wrapper: spp.manager.source.mixin's + # unlink() takes the wrapper with it. manager_ref_id is a Reference, + # so it carries no foreign key and can outlive what it points at — + # unlinking that blind would raise MissingError on the Add button. + concrete = leftover.manager_ref_id + ((concrete and concrete.exists()) or leftover).unlink() + + def action_create_manager(self): + """Create the concrete manager; the wrapper follows automatically. + + ``spp.manager.source.mixin.create`` builds the wrapper when it sees + ``_spp_wrapper_model`` in the context, so this creates one record and + gets both — and dismissing the dialog leaves nothing behind (#953). + + ``_spp_program_m2m_field`` matters as much as the wrapper model on the + Many2many cards: unlike a One2many they do not resolve from the + wrapper's ``program_id``, so without it the manager is created and the + program never picks it up — the card keeps saying nothing is configured + and the method never runs. + """ + self.ensure_one() + self._sweep_removed_methods() + + field = MANAGER_CATEGORIES[self.category]["field"] + configured = self.program_id[field].filtered( + lambda wrapper: wrapper.manager_ref_id and wrapper.manager_ref_id._name == self.method + ) + if configured: + raise UserError( + _("This program already has a %(method)s %(category)s.") + % { + "method": dict(self._methods_for_category(self.category)).get(self.method, self.method), + "category": MANAGER_CATEGORIES[self.category]["label"].lower(), + } + ) + + context = { + "default_program_id": self.program_id.id, + "_spp_wrapper_model": MANAGER_CATEGORIES[self.category]["wrapper"], + } + if self.env["spp.program"]._fields[field].type == "many2many": + context["_spp_program_m2m_field"] = field + concrete = ( + self.env[self.method] + .with_context(**context) + .create( + { + "name": self.name, + "program_id": self.program_id.id, + } + ) + ) + wrapper = self.env[MANAGER_CATEGORIES[self.category]["wrapper"]].search( + [("manager_ref_id", "=", f"{concrete._name},{concrete.id}")], + limit=1, + ) + if wrapper: + # Land on the method's own form rather than back on the card with + # something unconfigured and a cog to discover. Eligibility filters, + # entitlement amounts and compliance criteria all live there. + return wrapper.open_manager_form(title=MANAGER_CATEGORIES[self.category]["label"]) + return {"type": "ir.actions.act_window_close"} diff --git a/spp_programs/wizard/manager_setup_wizard.xml b/spp_programs/wizard/manager_setup_wizard.xml new file mode 100644 index 000000000..8acf617a7 --- /dev/null +++ b/spp_programs/wizard/manager_setup_wizard.xml @@ -0,0 +1,59 @@ + + + + + spp.manager.setup.wizard.form + spp.manager.setup.wizard + +
+ + + + + + +
+ + +
+ + +
+
+ + +
+