fix(spp_scoring,spp_scoring_programs): scoring validation, eligibility enforcement, and access control

spp_programs/models/managers/program_manager.py

@@ -3,7 +3,7 @@
 from datetime import datetime, timedelta
 
 from odoo import _, api, fields, models
-from odoo.exceptions import UserError
+from odoo.exceptions import UserError, ValidationError
 
 from odoo.addons.job_worker.delay import group
 
@@ -219,12 +219,35 @@ def _enroll_eligible_registrants(self, states, offset=0, limit=None, do_count=Fa
         _logger.debug("members filtered: %s", members)
         not_enrolled = members.filtered(lambda m: m.state not in ("enrolled", "duplicated", "exited"))
         _logger.debug("not_enrolled: %s", not_enrolled)
-        not_enrolled.write(
+
+        # Run pre-enrollment hooks (e.g., scoring eligibility checks).
+        # Members that fail the hook are moved to not_eligible.
+        hook_failed = self.env["spp.program.membership"]
+        for member in not_enrolled:
+            try:
+                program._pre_enrollment_hook(member.partner_id)
+            except (ValidationError, UserError) as e:
+                _logger.info(
+                    "Pre-enrollment hook rejected registrant %s: %s",
+                    member.partner_id.id,
+                    str(e),
+                )
+                hook_failed |= member
+
+        enrollable = not_enrolled - hook_failed
+        if hook_failed:
+            hook_failed.write({"state": "not_eligible"})
+
+        enrollable.write(
             {
                 "state": "enrolled",
                 "enrollment_date": fields.Datetime.now(),
             }
         )
+
+        # Run post-enrollment hooks (e.g., auto-score on enrollment)
+        for member in enrollable:
+            program._post_enrollment_hook(member.partner_id)
         # dis-enroll the one not eligible anymore:
         enrolled_members_ids = members.ids
         members_to_remove = member_before.filtered(
@@ -242,4 +265,4 @@ def _enroll_eligible_registrants(self, states, offset=0, limit=None, do_count=Fa
             program._compute_eligible_beneficiary_count()
             program._compute_beneficiary_count()
 
-        return len(not_enrolled)
+        return len(enrollable)

spp_scoring/__manifest__.py

@@ -34,6 +34,7 @@
         "views/scoring_indicator_views.xml",
         "views/scoring_threshold_views.xml",
         "views/scoring_result_views.xml",
+        "views/res_partner_views.xml",
         # Wizards (must be before menus so actions are available)
         "wizard/batch_scoring_wizard_views.xml",
         # Menus (last, references actions from other files)

spp_scoring/models/__init__.py

@@ -8,3 +8,4 @@
 from . import scoring_batch_job
 from . import scoring_indicator_provider
 from . import scoring_data_integration
+from . import res_partner

spp_scoring/models/res_partner.py

@@ -0,0 +1,52 @@
+# Part of OpenSPP. See LICENSE file for full copyright and licensing details.
+"""Extends res.partner with scoring result count and actions."""
+
+from odoo import api, fields, models
+
+
+class ResPartner(models.Model):
+    """Add scoring smart button to registrant form."""
+
+    _inherit = "res.partner"
+
+    scoring_result_ids = fields.One2many(
+        "spp.scoring.result",
+        "registrant_id",
+        string="Scoring Results",
+    )
+    scoring_result_count = fields.Integer(
+        string="# Scores",
+        compute="_compute_scoring_result_count",
+    )
+
+    @api.depends("scoring_result_ids")
+    def _compute_scoring_result_count(self):
+        for partner in self:
+            partner.scoring_result_count = len(partner.scoring_result_ids)
+
+    def action_view_scoring_results(self):
+        """Open scoring results for this registrant."""
+        self.ensure_one()
+        return {
+            "name": self.name,
+            "type": "ir.actions.act_window",
+            "res_model": "spp.scoring.result",
+            "view_mode": "list,form",
+            "domain": [("registrant_id", "=", self.id)],
+            "context": {"default_registrant_id": self.id},
+        }
+
+    def action_score_registrant(self):
+        """Open the batch scoring wizard pre-filled for this registrant."""
+        self.ensure_one()
+        return {
+            "name": "Score Registrant",
+            "type": "ir.actions.act_window",
+            "res_model": "spp.batch.scoring.wizard",
+            "view_mode": "form",
+            "target": "new",
+            "context": {
+                "default_registrant_ids": self.ids,
+                "default_domain": f"[('id', '=', {self.id})]",
+            },
+        }

spp_scoring/models/scoring_engine.py

@@ -115,13 +115,26 @@ def calculate_score(self, registrant, scoring_model, mode="manual"):
         if calculation_method == "cel_formula":
             total_score = self._calculate_cel_total(scoring_model, registrant, inputs_snapshot)
 
+        # In strict mode, required-indicator failures must prevent score creation
+        # (not just mark the result incomplete) — otherwise an invalid score
+        # can still enroll a registrant.
+        if errors and is_strict_mode:
+            raise UserError(
+                _(
+                    "Cannot calculate score for '%(name)s' in strict mode. "
+                    "%(count)d required indicator(s) failed:\n- %(errors)s"
+                )
+                % {
+                    "name": registrant.display_name,
+                    "count": len(errors),
+                    "errors": "\n- ".join(errors),
+                }
+            )
+
         # Determine classification (thresholds already prefetched)
         classification = self._get_classification(total_score, scoring_model)
 
-        # Check for errors in strict mode
         is_complete = True
-        if errors and is_strict_mode:
-            is_complete = False
 
         # Create result record
         Result = self.env["spp.scoring.result"]
@@ -206,10 +219,16 @@ def _calculate_indicator(self, indicator, registrant):
 
             result["field_value"] = field_value
 
-            # Handle missing values
-            if field_value is None:
+            # Handle missing or falsy values
+            # For required indicators, treat False/None/empty as missing
+            if field_value is None or (
+                indicator.is_required and field_value is not True and not field_value and field_value != 0
+            ):
                 if indicator.is_required:
-                    result["error"] = _("Required field '%s' is missing.") % indicator.field_path
+                    result["error"] = _("Required field '%(path)s' has no valid value (got: %(value)s).") % {
+                        "path": indicator.field_path,
+                        "value": repr(field_value),
+                    }
                     return result
                 field_value = indicator._convert_default_value()
 
@@ -590,7 +609,9 @@ def get_or_calculate_score(self, registrant, scoring_model, max_age_days=None):
         """
         Result = self.env["spp.scoring.result"]
 
-        if max_age_days:
+        # max_age_days > 0: reuse cached score if fresh enough
+        # max_age_days = 0 or None: always recalculate
+        if max_age_days and max_age_days > 0:
             existing = Result.get_latest_score(registrant, scoring_model, max_age_days)
             if existing:
                 return existing

spp_scoring/models/scoring_model.py

@@ -116,11 +116,13 @@ class SppScoringModel(models.Model):
         comodel_name="spp.scoring.indicator",
         inverse_name="model_id",
         string="Indicators",
+        copy=True,
     )
     threshold_ids = fields.One2many(
         comodel_name="spp.scoring.threshold",
         inverse_name="model_id",
         string="Thresholds",
+        copy=True,
     )
     result_ids = fields.One2many(
         comodel_name="spp.scoring.result",
@@ -205,7 +207,10 @@ def action_activate(self):
         for record in self:
             errors = record._validate_configuration()
             if errors:
-                raise ValidationError(_("Cannot activate model. Validation errors:\n%s") % "\n".join(errors))
+                raise ValidationError(
+                    _("Cannot activate model '%(name)s'. Validation errors:\n%(errors)s")
+                    % {"name": record.name, "errors": "\n".join(f"• {e}" for e in errors)}
+                )
             record.is_active = True
         return True
 
@@ -221,11 +226,11 @@ def _validate_configuration(self):
 
         # Check indicators exist
         if not self.indicator_ids:
-            errors.append(_("At least one indicator is required."))
+            errors.append(_("No indicators defined. Add at least one indicator in the Indicators tab."))
 
         # Check thresholds exist
         if not self.threshold_ids:
-            errors.append(_("At least one threshold is required."))
+            errors.append(_("No thresholds defined. Add at least one threshold in the Thresholds tab."))
 
         # Check weights sum correctly (if expected)
         if self.expected_total_weight > 0:
@@ -258,21 +263,39 @@ def _validate_configuration(self):
         return errors
 
     def _validate_thresholds(self):
-        """Check that thresholds cover the expected score range without gaps."""
+        """Check that thresholds cover the expected score range without gaps or overlaps."""
         errors = []
         if not self.threshold_ids:
             return errors
 
         sorted_thresholds = self.threshold_ids.sorted(key=lambda t: t.min_score)
 
-        # Check for gaps between thresholds
+        # Check all consecutive threshold boundaries for gaps and overlaps.
+        # Round to 2 decimal places to avoid IEEE-754 false positives
+        # (e.g., 20.00 - 19.99 computing to 0.010000000000000009).
         for i, threshold in enumerate(sorted_thresholds[:-1]):
             next_threshold = sorted_thresholds[i + 1]
-            gap = next_threshold.min_score - threshold.max_score
+            gap = round(next_threshold.min_score - threshold.max_score, 2)
+
             if gap > 0.01:
                 errors.append(
-                    _("Gap detected between thresholds '%(current)s' and '%(next)s'.")
-                    % {"current": threshold.name, "next": next_threshold.name}
+                    _("Gap detected between thresholds '%(current)s' (max %(max)s) and '%(next)s' (min %(min)s).")
+                    % {
+                        "current": threshold.name,
+                        "max": threshold.max_score,
+                        "next": next_threshold.name,
+                        "min": next_threshold.min_score,
+                    }
+                )
+            elif gap < 0:
+                errors.append(
+                    _("Overlap detected between thresholds '%(current)s' (max %(max)s) and '%(next)s' (min %(min)s).")
+                    % {
+                        "current": threshold.name,
+                        "max": threshold.max_score,
+                        "next": next_threshold.name,
+                        "min": next_threshold.min_score,
+                    }
                 )
 
         return errors

spp_scoring/views/res_partner_views.xml

@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<odoo>
+    <!-- Add Scores smart button to the OpenSPP registrant form.
+         spp_registry.view_individuals_form replaces //sheet on res.partner,
+         so inheriting base.view_partner_form does not propagate to the
+         registry form used by Individuals and Groups. -->
+    <record id="view_registrant_form_scoring" model="ir.ui.view">
+        <field name="name">spp_registry.view_registrant_form.scoring</field>
+        <field name="model">res.partner</field>
+        <field name="inherit_id" ref="spp_registry.view_individuals_form" />
+        <field name="arch" type="xml">
+            <xpath expr="//div[@name='button_box']" position="inside">
+                <button
+                    name="action_view_scoring_results"
+                    type="object"
+                    class="oe_stat_button"
+                    icon="fa-bar-chart"
+                >
+                    <field
+                        name="scoring_result_count"
+                        widget="statinfo"
+                        string="Scores"
+                    />
+                </button>
+            </xpath>
+        </field>
+    </record>
+</odoo>

spp_scoring/views/scoring_model_views.xml

@@ -13,12 +13,14 @@
                         type="object"
                         class="btn-primary"
                         invisible="is_active"
+                        groups="spp_scoring.group_scoring_manager"
                     />
                     <button
                         name="action_deactivate"
                         string="Deactivate"
                         type="object"
                         invisible="not is_active"
+                        groups="spp_scoring.group_scoring_manager"
                     />
                     <field name="is_active" widget="boolean_toggle" readonly="1" />
                 </header>

spp_scoring_programs/__manifest__.py

@@ -15,9 +15,7 @@
         "spp_programs",
     ],
     "data": [
-        # Note: No ACL entries needed - this module only extends existing models
-        # (spp.program, spp.program.membership, spp.scoring.model) which have
-        # their own ACLs defined in their respective modules.
+        "security/ir.model.access.csv",
         "views/scoring_model_views.xml",
         "views/program_views.xml",
     ],

spp_scoring_programs/models/program.py

@@ -73,8 +73,11 @@ def check_scoring_eligibility(self, registrant):
             return {"eligible": True, "score": None, "classification": None}
 
         # Get or calculate score
+        # max_age_days=0 means "always recalculate" (no cache);
+        # max_age_days>0 means "reuse if fresher than N days";
+        # max_age_days not set means "always recalculate" (same as 0).
         engine = self.env["spp.scoring.engine"]
-        max_age = self.score_max_age_days if self.score_max_age_days > 0 else None
+        max_age = self.score_max_age_days or 0
 
         score_result = engine.get_or_calculate_score(
             registrant,
@@ -132,31 +135,57 @@ def _pre_enrollment_hook(self, partner):
                 )
 
     def _post_enrollment_hook(self, partner):
-        """Calculate score after enrollment if configured."""
+        """Calculate score after enrollment and record it on membership."""
         super()._post_enrollment_hook(partner)
 
-        if self.is_auto_score_on_enrollment and self.scoring_model_id:
+        if not self.scoring_model_id:
+            return
+
+        score_result = None
+
+        if self.is_auto_score_on_enrollment:
             engine = self.env["spp.scoring.engine"]
             try:
-                engine.calculate_score(
+                score_result = engine.calculate_score(
                     partner,
                     self.scoring_model_id,
                     mode="automatic",
                 )
             except (ValidationError, UserError) as e:
-                # Expected errors from scoring validation - log and continue
                 _logger.warning(
                     "Score calculation failed for registrant_id=%s: %s",
                     partner.id,
                     str(e),
                 )
             except Exception:
-                # Unexpected errors - log type only to avoid PII exposure
                 _logger.exception(
                     "Unexpected error calculating score for registrant_id=%s",
                     partner.id,
                 )
 
+        # If no auto-score, use existing latest score
+        if not score_result:
+            Result = self.env["spp.scoring.result"]
+            score_result = Result.get_latest_score(partner, self.scoring_model_id)
+
+        # Record enrollment score on membership
+        if score_result:
+            membership = self.env["spp.program.membership"].search(
+                [
+                    ("partner_id", "=", partner.id),
+                    ("program_id", "=", self.id),
+                    ("state", "=", "enrolled"),
+                ],
+                limit=1,
+            )
+            if membership:
+                membership.write(
+                    {
+                        "enrollment_score": score_result.score,
+                        "enrollment_classification": score_result.classification_code,
+                    }
+                )
+
 
 class SppProgramMembership(models.Model):
     """Extends membership with scoring information."""

spp_scoring_programs/security/ir.model.access.csv

@@ -0,0 +1,3 @@
+id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
+access_spp_program_scoring_viewer,spp.program scoring viewer,spp_programs.model_spp_program,spp_scoring.group_scoring_viewer,1,0,0,0
+access_spp_program_membership_scoring_viewer,spp.program.membership scoring viewer,spp_programs.model_spp_program_membership,spp_scoring.group_scoring_viewer,1,0,0,0