Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions spp_programs/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,18 @@ Dependencies
Changelog
=========

19.0.2.0.9
~~~~~~~~~~

- Add context flags (``skip_registrant_statistics``,
``skip_program_statistics``) to suppress expensive computed field
recomputation during bulk operations
- Add ``refresh_beneficiary_counts()`` on program and
``refresh_statistics()`` on cycle for one-shot recomputation after
bulk operations
- Replace ``bool(rec.program_membership_ids)`` with SQL query in
``_compute_has_members``

19.0.2.0.8
~~~~~~~~~~

Expand Down
2 changes: 1 addition & 1 deletion spp_programs/__manifest__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"name": "OpenSPP Programs",
"summary": "Manage programs, cycles, beneficiary enrollment, entitlements (cash and in-kind), payments, and fund tracking for social protection.",
"category": "OpenSPP/Core",
"version": "19.0.2.0.8",
"version": "19.0.2.0.9",
"sequence": 1,
"author": "OpenSPP.org",
"website": "https://github.com/OpenSPP/OpenSPP2",
Expand Down
10 changes: 10 additions & 0 deletions spp_programs/models/cycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,16 @@ def _compute_entitlements_count(self):
entitlements_count = self.env["spp.entitlement"].search_count([("cycle_id", "=", rec.id)])
rec.entitlements_count = entitlements_count

def refresh_statistics(self):
"""Refresh all cycle statistics after bulk operations.

Call this after raw SQL inserts that bypass ORM dependency tracking
(e.g. bulk_create_memberships with skip_duplicates=True).
"""
self._compute_members_count()
self._compute_entitlements_count()
self._compute_total_entitlements_count()
Comment on lines +278 to +286
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The refresh_statistics method appears to be redundant in its current implementation. The fields it attempts to refresh (members_count, entitlements_count, and total_entitlements_count) are all store=False and do not implement the "canary" skip logic found in other models.

Since these fields are computed on-demand and the underlying relation caches are correctly invalidated in the managers (e.g., via cycle.invalidate_recordset(['cycle_membership_ids'])), they will naturally reflect the correct values upon the next access without an explicit refresh call. If the intention was to optimize these fields for bulk operations, they should be made store=True and implement the skip logic, similar to has_members in spp.program.


@api.depends("entitlement_ids", "inkind_entitlement_ids")
def _compute_total_entitlements_count(self):
if not self.ids:
Expand Down
4 changes: 2 additions & 2 deletions spp_programs/models/managers/cycle_manager_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,8 @@ def mark_import_as_done(self, cycle, msg):
cycle.locked_reason = None
cycle.message_post(body=msg)

# Update Statistics
cycle._compute_members_count()
# Refresh statistics after bulk operations
cycle.refresh_statistics()

def mark_prepare_entitlement_as_done(self, cycle, msg):
"""Complete the preparation of entitlements.
Expand Down
3 changes: 1 addition & 2 deletions spp_programs/models/managers/eligibility_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,7 @@ def _import_registrants_async(self, new_beneficiaries, state="draft"):

def mark_import_as_done(self):
self.ensure_one()
self.program_id._compute_eligible_beneficiary_count()
self.program_id._compute_beneficiary_count()
self.program_id.refresh_beneficiary_counts()

self.program_id.is_locked = False
self.program_id.locked_reason = None
Expand Down
27 changes: 26 additions & 1 deletion spp_programs/models/programs.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,23 @@ def _check_unique_program_name(self):

@api.depends("program_membership_ids")
def _compute_has_members(self):
if self.env.context.get("skip_program_statistics"):
return
if not self.ids:
for rec in self:
rec.has_members = False
return
self.env.cr.execute(
"""
SELECT program_id FROM spp_program_membership
WHERE program_id IN %s
GROUP BY program_id
""",
(tuple(self.ids),),
)
programs_with_members = {row[0] for row in self.env.cr.fetchall()}
for rec in self:
rec.has_members = bool(rec.program_membership_ids)
rec.has_members = rec.id in programs_with_members

@api.depends("compliance_manager_ids", "compliance_manager_ids.manager_ref_id")
def _compute_has_compliance_criteria(self):
Expand Down Expand Up @@ -273,6 +288,16 @@ def _compute_beneficiary_count(self):
count = rec.count_beneficiaries(None)["value"]
rec.update({"beneficiaries_count": count})

def refresh_beneficiary_counts(self):
"""Refresh all beneficiary statistics after bulk operations.

Call this after raw SQL inserts that bypass ORM dependency tracking
(e.g. bulk_create_memberships with skip_duplicates=True).
"""
self._compute_beneficiary_count()
self._compute_eligible_beneficiary_count()
self._compute_has_members()

@api.depends("cycle_ids")
def _compute_cycle_count(self):
for rec in self:
Expand Down
8 changes: 8 additions & 0 deletions spp_programs/models/registrant.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ def _compute_total_entitlements_count(self):
@api.depends("program_membership_ids")
def _compute_program_membership_count(self):
"""Batch-efficient program membership count using read_group."""
if self.env.context.get("skip_registrant_statistics"):
return
Comment on lines +46 to +47
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The skip_registrant_statistics context flag allows bypassing the recomputation of store=True fields such as program_membership_count. However, unlike the spp.program and spp.cycle models, no corresponding refresh method is provided for res.partner to recompute these statistics after bulk operations. This will result in stale data remaining in the database if the flag is used or if raw SQL is employed.

Please consider adding a refresh_registrant_statistics() method to the res.partner inheritance and calling it in the appropriate completion handlers (e.g., in mark_import_as_done within eligibility_manager.py).

if not self:
return

Expand All @@ -66,6 +68,8 @@ def _compute_program_membership_count(self):
@api.depends("entitlement_ids")
def _compute_entitlements_count(self):
"""Batch-efficient entitlements count using _read_group."""
if self.env.context.get("skip_registrant_statistics"):
return
if not self:
return

Expand All @@ -89,6 +93,8 @@ def _compute_entitlements_count(self):
@api.depends("cycle_ids")
def _compute_cycle_count(self):
"""Batch-efficient cycle membership count using _read_group."""
if self.env.context.get("skip_registrant_statistics"):
return
if not self:
return

Expand All @@ -112,6 +118,8 @@ def _compute_cycle_count(self):
@api.depends("inkind_entitlement_ids")
def _compute_inkind_entitlements_count(self):
"""Batch-efficient in-kind entitlements count using _read_group."""
if self.env.context.get("skip_registrant_statistics"):
return
if not self:
return

Expand Down
6 changes: 6 additions & 0 deletions spp_programs/readme/HISTORY.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
### 19.0.2.0.9

- Add context flags (`skip_registrant_statistics`, `skip_program_statistics`) to suppress expensive computed field recomputation during bulk operations
- Add `refresh_beneficiary_counts()` on program and `refresh_statistics()` on cycle for one-shot recomputation after bulk operations
- Replace `bool(rec.program_membership_ids)` with SQL query in `_compute_has_members`

### 19.0.2.0.8

- Replace OFFSET pagination with NTILE-based ID-range batching in all async job dispatchers
Expand Down
29 changes: 21 additions & 8 deletions spp_programs/static/description/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,19 @@ <h2><a class="toc-backref" href="#toc-entry-1">Changelog</a></h2>
</div>
</div>
<div class="section" id="section-1">
<h1>19.0.2.0.9</h1>
<ul class="simple">
<li>Add context flags (<tt class="docutils literal">skip_registrant_statistics</tt>,
<tt class="docutils literal">skip_program_statistics</tt>) to suppress expensive computed field
recomputation during bulk operations</li>
<li>Add <tt class="docutils literal">refresh_beneficiary_counts()</tt> on program and
<tt class="docutils literal">refresh_statistics()</tt> on cycle for one-shot recomputation after
bulk operations</li>
<li>Replace <tt class="docutils literal">bool(rec.program_membership_ids)</tt> with SQL query in
<tt class="docutils literal">_compute_has_members</tt></li>
</ul>
</div>
<div class="section" id="section-2">
<h1>19.0.2.0.8</h1>
<ul class="simple">
<li>Replace OFFSET pagination with NTILE-based ID-range batching in all
Expand All @@ -668,7 +681,7 @@ <h1>19.0.2.0.8</h1>
program and cycle</li>
</ul>
</div>
<div class="section" id="section-2">
<div class="section" id="section-3">
<h1>19.0.2.0.7</h1>
<ul class="simple">
<li>Bulk membership creation using raw SQL INSERT ON CONFLICT DO NOTHING
Expand All @@ -677,7 +690,7 @@ <h1>19.0.2.0.7</h1>
<tt class="docutils literal">_add_beneficiaries</tt> with bulk SQL path</li>
</ul>
</div>
<div class="section" id="section-3">
<div class="section" id="section-4">
<h1>19.0.2.0.6</h1>
<ul class="simple">
<li>Remove unused entitlement_base_model.py (dead code, never imported)</li>
Expand All @@ -686,34 +699,34 @@ <h1>19.0.2.0.6</h1>
payment, and fund tests (172 → 492 tests)</li>
</ul>
</div>
<div class="section" id="section-4">
<div class="section" id="section-5">
<h1>19.0.2.0.5</h1>
<ul class="simple">
<li>Batch create entitlements and payments instead of one-by-one ORM
creates</li>
</ul>
</div>
<div class="section" id="section-5">
<div class="section" id="section-6">
<h1>19.0.2.0.4</h1>
<ul class="simple">
<li>Fetch fund balance once per approval batch instead of per entitlement</li>
</ul>
</div>
<div class="section" id="section-6">
<div class="section" id="section-7">
<h1>19.0.2.0.3</h1>
<ul class="simple">
<li>Replace cycle computed fields (total_amount, entitlements_count,
approval flags) with SQL aggregation queries</li>
</ul>
</div>
<div class="section" id="section-7">
<div class="section" id="section-8">
<h1>19.0.2.0.2</h1>
<ul class="simple">
<li>Add composite indexes for frequent query patterns on entitlements and
program memberships</li>
</ul>
</div>
<div class="section" id="section-8">
<div class="section" id="section-9">
<h1>19.0.2.0.1</h1>
<ul class="simple">
<li>Replace Python-level uniqueness checks with SQL UNIQUE constraints for
Expand All @@ -722,7 +735,7 @@ <h1>19.0.2.0.1</h1>
constraint creation</li>
</ul>
</div>
<div class="section" id="section-9">
<div class="section" id="section-10">
<h1>19.0.2.0.0</h1>
<ul class="simple">
<li>Initial migration to OpenSPP2</li>
Expand Down
1 change: 1 addition & 0 deletions spp_programs/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,4 @@
from . import test_cycle_auto_approve_fund_check
from . import test_bulk_membership
from . import test_keyset_pagination
from . import test_canary_patterns
125 changes: 125 additions & 0 deletions spp_programs/tests/test_canary_patterns.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
# Part of OpenSPP. See LICENSE file for full copyright and licensing details.
"""Tests for Phase 8: Canary patterns for bulk operations.

During bulk operations, expensive computed field recomputation should be
skipped via context flags and refreshed once at completion.
"""

import uuid

from odoo import fields
from odoo.tests import TransactionCase


class TestRegistrantCanaryFlags(TransactionCase):
"""Test that registrant statistics skip recomputation with context flags."""

def setUp(self):
super().setUp()
self.program = self.env["spp.program"].create({"name": f"Test Program {uuid.uuid4().hex[:8]}"})
self.partner = self.env["res.partner"].create({"name": "Test Registrant", "is_registrant": True})

def test_skip_registrant_statistics_skips_program_membership_count(self):
"""With skip_registrant_statistics, _compute_program_membership_count should be a no-op."""
self.partner.with_context(skip_registrant_statistics=True)._compute_program_membership_count()
# Value should remain at default (0) since compute was skipped
self.assertEqual(self.partner.program_membership_count, 0)

def test_skip_registrant_statistics_skips_entitlements_count(self):
"""With skip_registrant_statistics, _compute_entitlements_count should be a no-op."""
self.partner.with_context(skip_registrant_statistics=True)._compute_entitlements_count()
self.assertEqual(self.partner.entitlements_count, 0)

def test_skip_registrant_statistics_skips_cycle_count(self):
"""With skip_registrant_statistics, _compute_cycle_count should be a no-op."""
self.partner.with_context(skip_registrant_statistics=True)._compute_cycle_count()
self.assertEqual(self.partner.cycles_count, 0)

def test_skip_registrant_statistics_skips_inkind_count(self):
"""With skip_registrant_statistics, _compute_inkind_entitlements_count should be a no-op."""
self.partner.with_context(skip_registrant_statistics=True)._compute_inkind_entitlements_count()
self.assertEqual(self.partner.inkind_entitlements_count, 0)

def test_without_flag_computes_normally(self):
"""Without the flag, compute methods should work normally."""
self.env["spp.program.membership"].create(
{
"partner_id": self.partner.id,
"program_id": self.program.id,
"state": "draft",
}
)
self.partner._compute_program_membership_count()
self.assertEqual(self.partner.program_membership_count, 1)


class TestProgramCanaryFlags(TransactionCase):
"""Test that program statistics skip recomputation with context flags."""

def setUp(self):
super().setUp()
self.program = self.env["spp.program"].create({"name": f"Test Program {uuid.uuid4().hex[:8]}"})

def test_skip_program_statistics_skips_has_members(self):
"""With skip_program_statistics, _compute_has_members should be a no-op."""
self.program.with_context(skip_program_statistics=True)._compute_has_members()
self.assertFalse(self.program.has_members)

def test_has_members_uses_sql_exists(self):
"""_compute_has_members should detect members without loading the recordset."""
partner = self.env["res.partner"].create({"name": "Registrant", "is_registrant": True})
self.env["spp.program.membership"].create(
{
"partner_id": partner.id,
"program_id": self.program.id,
"state": "draft",
}
)
self.program._compute_has_members()
self.assertTrue(self.program.has_members)


class TestRefreshMethods(TransactionCase):
"""Test refresh_beneficiary_counts and refresh_statistics methods."""

def setUp(self):
super().setUp()
self.program = self.env["spp.program"].create({"name": f"Test Program {uuid.uuid4().hex[:8]}"})
self.cycle = self.env["spp.cycle"].create(
{
"name": "Test Cycle",
"program_id": self.program.id,
"start_date": fields.Date.today(),
"end_date": fields.Date.today(),
}
)
self.partners = self.env["res.partner"].create(
[{"name": f"Registrant {i}", "is_registrant": True} for i in range(5)]
)

def test_program_refresh_beneficiary_counts(self):
"""refresh_beneficiary_counts should update all program statistics."""
# Create memberships via SQL (bypassing ORM triggers)
for p in self.partners:
self.env["spp.program.membership"].bulk_create_memberships(
[{"partner_id": p.id, "program_id": self.program.id, "state": "enrolled"}],
skip_duplicates=True,
)

# Counts are stale (SQL insert bypassed ORM)
self.program.refresh_beneficiary_counts()

self.assertEqual(self.program.beneficiaries_count, 5)
self.assertEqual(self.program.eligible_beneficiaries_count, 5)
self.assertTrue(self.program.has_members)

def test_cycle_refresh_statistics(self):
"""refresh_statistics should update cycle member and entitlement counts."""
for p in self.partners:
self.env["spp.cycle.membership"].bulk_create_memberships(
[{"partner_id": p.id, "cycle_id": self.cycle.id, "state": "enrolled"}],
skip_duplicates=True,
)

self.cycle.refresh_statistics()
self.assertEqual(self.cycle.members_count, 5)
Loading