From 9f903a4cc20b29ecc47ccd7fbb7b92db25f3ae43 Mon Sep 17 00:00:00 2001 From: emjay0921 Date: Tue, 14 Apr 2026 12:25:02 +0800 Subject: [PATCH 01/12] fix(spp_scoring): validate all threshold boundaries for gaps and overlaps Previously only positive gaps were detected. Now also checks for overlapping thresholds (negative gaps) across ALL consecutive boundaries, not just the first. Error messages include actual max/min values for easier debugging. --- spp_scoring/models/scoring_model.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/spp_scoring/models/scoring_model.py b/spp_scoring/models/scoring_model.py index 0d141f14..958bd844 100644 --- a/spp_scoring/models/scoring_model.py +++ b/spp_scoring/models/scoring_model.py @@ -258,21 +258,37 @@ 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 for i, threshold in enumerate(sorted_thresholds[:-1]): next_threshold = sorted_thresholds[i + 1] gap = next_threshold.min_score - threshold.max_score + 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.01: + 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 From 2d7db716842306da19286a3d162e6800ea6a77ef Mon Sep 17 00:00:00 2001 From: emjay0921 Date: Tue, 14 Apr 2026 12:35:29 +0800 Subject: [PATCH 02/12] fix(spp_scoring): improve validation error messages for incomplete models Error messages now include model name, bullet points, and actionable text directing users to the Indicators/Thresholds tabs. --- spp_scoring/models/scoring_model.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/spp_scoring/models/scoring_model.py b/spp_scoring/models/scoring_model.py index 958bd844..ff701d6b 100644 --- a/spp_scoring/models/scoring_model.py +++ b/spp_scoring/models/scoring_model.py @@ -205,7 +205,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 +224,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: From 2781ffa6bd657c97394aa361ed58dd8d2465f231 Mon Sep 17 00:00:00 2001 From: emjay0921 Date: Tue, 14 Apr 2026 12:55:15 +0800 Subject: [PATCH 03/12] fix(spp_programs): invoke pre/post enrollment hooks in program manager The _enroll_eligible_registrants method wrote state=enrolled directly without calling pre/post enrollment hooks. This bypassed scoring eligibility checks from spp_scoring_programs. Now the pre-enrollment hook runs per member before enrollment, and members that fail (e.g., NON_POOR classification when only EXTREME_POOR/POOR are allowed) are marked not_eligible instead of enrolled. --- .../models/managers/program_manager.py | 29 +++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/spp_programs/models/managers/program_manager.py b/spp_programs/models/managers/program_manager.py index eccb41a2..f9854f35 100644 --- a/spp_programs/models/managers/program_manager.py +++ b/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) From 4cabe1f00caea90091db3aac77138a73322eda10 Mon Sep 17 00:00:00 2001 From: emjay0921 Date: Tue, 14 Apr 2026 12:59:17 +0800 Subject: [PATCH 04/12] fix(spp_scoring_programs): record enrollment score on membership after enrollment The _post_enrollment_hook calculated a score but did not write it to the membership record. Now writes enrollment_score and enrollment_classification on the membership after enrollment. Falls back to latest existing score if auto-scoring is disabled or fails. --- spp_scoring_programs/models/program.py | 36 ++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/spp_scoring_programs/models/program.py b/spp_scoring_programs/models/program.py index 3f29a815..3a64ee86 100644 --- a/spp_scoring_programs/models/program.py +++ b/spp_scoring_programs/models/program.py @@ -132,31 +132,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.""" From 093b8156e583a69bdc471113a23567f96e293de9 Mon Sep 17 00:00:00 2001 From: emjay0921 Date: Tue, 14 Apr 2026 13:09:49 +0800 Subject: [PATCH 05/12] fix(spp_scoring_programs): show scoring columns by default in membership list Changed enrollment_score, enrollment_classification, latest_score, and latest_classification from optional=hide to optional=show so they are visible by default when spp_scoring_programs is installed. --- spp_scoring_programs/views/program_views.xml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spp_scoring_programs/views/program_views.xml b/spp_scoring_programs/views/program_views.xml index cf62ecbf..9e01af13 100644 --- a/spp_scoring_programs/views/program_views.xml +++ b/spp_scoring_programs/views/program_views.xml @@ -66,10 +66,10 @@ - - - - + + + + From d8096ddb3451a954887dc1c313ef40b6162a8cf7 Mon Sep 17 00:00:00 2001 From: emjay0921 Date: Tue, 14 Apr 2026 13:18:16 +0800 Subject: [PATCH 06/12] fix(spp_scoring,spp_scoring_programs): max score age 0 forces recalculation score_max_age_days=0 was treated as 'no limit' (passed None to engine) instead of 'always recalculate'. Now explicitly passes 0 which the engine treats as 'skip cache, always recalculate'. Also made the engine check explicit: max_age_days > 0 to use cache. --- spp_scoring/models/scoring_engine.py | 4 +++- spp_scoring_programs/models/program.py | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/spp_scoring/models/scoring_engine.py b/spp_scoring/models/scoring_engine.py index 18ae8acc..20012bfa 100644 --- a/spp_scoring/models/scoring_engine.py +++ b/spp_scoring/models/scoring_engine.py @@ -590,7 +590,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 diff --git a/spp_scoring_programs/models/program.py b/spp_scoring_programs/models/program.py index 3a64ee86..af18dbaf 100644 --- a/spp_scoring_programs/models/program.py +++ b/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, From 2f7ebb1ba57656f3fcdd8d9f6451ef6ddee89458 Mon Sep 17 00:00:00 2001 From: emjay0921 Date: Tue, 14 Apr 2026 13:31:02 +0800 Subject: [PATCH 07/12] fix(spp_scoring_programs): grant scoring viewer read access to programs Scoring viewer users got an access error on spp.scoring.model.program_ids because they had no read access to spp.program. Added read-only ACL for spp.program and spp.program.membership to the scoring viewer group. --- spp_scoring_programs/__manifest__.py | 4 +--- spp_scoring_programs/security/ir.model.access.csv | 3 +++ 2 files changed, 4 insertions(+), 3 deletions(-) create mode 100644 spp_scoring_programs/security/ir.model.access.csv diff --git a/spp_scoring_programs/__manifest__.py b/spp_scoring_programs/__manifest__.py index 18824c81..d0dee4c6 100644 --- a/spp_scoring_programs/__manifest__.py +++ b/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", ], diff --git a/spp_scoring_programs/security/ir.model.access.csv b/spp_scoring_programs/security/ir.model.access.csv new file mode 100644 index 00000000..ffd09b64 --- /dev/null +++ b/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 From ac9138ba39a518a33d97c769d80d58a2e6c2eaec Mon Sep 17 00:00:00 2001 From: emjay0921 Date: Tue, 14 Apr 2026 13:48:02 +0800 Subject: [PATCH 08/12] fix(spp_scoring): restrict Activate/Deactivate buttons to scoring managers Officers and viewers could see and click Activate/Deactivate buttons on scoring models even though their ACLs prevented the write from succeeding. Now the buttons are only visible to scoring managers. --- spp_scoring/views/scoring_model_views.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spp_scoring/views/scoring_model_views.xml b/spp_scoring/views/scoring_model_views.xml index f56e1df1..9fa04404 100644 --- a/spp_scoring/views/scoring_model_views.xml +++ b/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" /> + + + + From 2720864750f6095b66d892ae4118684a24c084fe Mon Sep 17 00:00:00 2001 From: emjay0921 Date: Wed, 15 Apr 2026 15:53:21 +0800 Subject: [PATCH 10/12] fix(spp_scoring): strict mode validates false/invalid values for required indicators Previously only checked for None. Now also catches False, empty string, and other falsy values (except 0 and True) when the indicator is required. Error message includes the actual value for debugging. --- spp_scoring/models/scoring_engine.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/spp_scoring/models/scoring_engine.py b/spp_scoring/models/scoring_engine.py index 20012bfa..c3a4de04 100644 --- a/spp_scoring/models/scoring_engine.py +++ b/spp_scoring/models/scoring_engine.py @@ -206,10 +206,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() From 1a8bfdbc537837699eec4739d77a2e23ee9561c4 Mon Sep 17 00:00:00 2001 From: emjay0921 Date: Wed, 15 Apr 2026 15:53:32 +0800 Subject: [PATCH 11/12] fix(spp_scoring): duplicate scoring model copies indicators and thresholds Added copy=True to indicator_ids and threshold_ids One2many fields so that Action > Duplicate copies indicators and thresholds to the new model. The existing copy() method already handles code suffix and deactivation. --- spp_scoring/models/scoring_model.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spp_scoring/models/scoring_model.py b/spp_scoring/models/scoring_model.py index ff701d6b..3b5efccd 100644 --- a/spp_scoring/models/scoring_model.py +++ b/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", From ab87b7948dd378341fd2898c807a7959683ff68f Mon Sep 17 00:00:00 2001 From: emjay0921 Date: Wed, 22 Apr 2026 15:09:26 +0800 Subject: [PATCH 12/12] fix(spp_scoring): address QA findings on thresholds, smart button, strict mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - #835: threshold gap/overlap check now rounds to 2 decimals before comparing. IEEE-754 made 20.00 - 19.99 occasionally compute to 0.010000000000000009, triggering a false "gap" error on valid contiguous boundaries. - #837: Scores smart button was inheriting base.view_partner_form, but spp_registry.view_individuals_form replaces the //sheet, so the button never showed on Individual or Group registrant pages. Inherit the registry form instead. Drop the now-redundant is_registrant guard — the registry form is only opened on registrants. - #838: strict mode detected missing/invalid required indicators but still created the score record with is_complete=False. Now it raises UserError, preventing an incomplete score from being persisted and downstream enrollment logic from acting on it. --- spp_scoring/models/scoring_engine.py | 19 ++++++++++++++++--- spp_scoring/models/scoring_model.py | 8 +++++--- spp_scoring/views/res_partner_views.xml | 12 +++++++----- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/spp_scoring/models/scoring_engine.py b/spp_scoring/models/scoring_engine.py index c3a4de04..e5db8c07 100644 --- a/spp_scoring/models/scoring_engine.py +++ b/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"] diff --git a/spp_scoring/models/scoring_model.py b/spp_scoring/models/scoring_model.py index 3b5efccd..55327698 100644 --- a/spp_scoring/models/scoring_model.py +++ b/spp_scoring/models/scoring_model.py @@ -270,10 +270,12 @@ def _validate_thresholds(self): sorted_thresholds = self.threshold_ids.sorted(key=lambda t: t.min_score) - # Check all consecutive threshold boundaries for gaps and overlaps + # 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( @@ -285,7 +287,7 @@ def _validate_thresholds(self): "min": next_threshold.min_score, } ) - elif gap < -0.01: + elif gap < 0: errors.append( _("Overlap detected between thresholds '%(current)s' (max %(max)s) and '%(next)s' (min %(min)s).") % { diff --git a/spp_scoring/views/res_partner_views.xml b/spp_scoring/views/res_partner_views.xml index c1b8c3cb..2128ab5c 100644 --- a/spp_scoring/views/res_partner_views.xml +++ b/spp_scoring/views/res_partner_views.xml @@ -1,10 +1,13 @@ - - - res.partner.form.scoring + + + spp_registry.view_registrant_form.scoring res.partner - +