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 + +
+ + + + + + +
+ + +
+ + +
+
+ + +
+