diff --git a/spp_programs/__manifest__.py b/spp_programs/__manifest__.py index c7eefaa1..f84a56ae 100644 --- a/spp_programs/__manifest__.py +++ b/spp_programs/__manifest__.py @@ -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.6", + "version": "19.0.2.0.7", "sequence": 1, "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_programs/models/cycle_membership.py b/spp_programs/models/cycle_membership.py index 1e10faed..4fdb0bf6 100644 --- a/spp_programs/models/cycle_membership.py +++ b/spp_programs/models/cycle_membership.py @@ -1,7 +1,11 @@ # Part of OpenSPP. See LICENSE file for full copyright and licensing details. -from odoo import _, fields, models +import logging + +from odoo import _, api, fields, models from odoo.exceptions import ValidationError +_logger = logging.getLogger(__name__) + class SPPCycleMembership(models.Model): _name = "spp.cycle.membership" @@ -87,6 +91,73 @@ def open_registrant_form(self): }, } + @api.model + def bulk_create_memberships(self, vals_list, chunk_size=1000, skip_duplicates=False): + """Create cycle memberships in bulk with optional duplicate skipping. + + :param vals_list: List of dicts with membership values + :param chunk_size: Number of records per batch (default 1000) + :param skip_duplicates: When True, use INSERT ... ON CONFLICT DO NOTHING + to silently skip duplicate (partner_id, cycle_id) pairs. + Returns the count of inserted rows. + :return: Recordset (skip_duplicates=False) or int count (skip_duplicates=True) + """ + if not vals_list: + return 0 if skip_duplicates else self.env["spp.cycle.membership"] + + if skip_duplicates: + return self._bulk_insert_on_conflict(vals_list, chunk_size) + + return self.create(vals_list) + + def _bulk_insert_on_conflict(self, vals_list, chunk_size=1000): + """Insert cycle memberships using raw SQL with ON CONFLICT DO NOTHING. + + :param vals_list: List of dicts with at least partner_id, cycle_id, state + :param chunk_size: Number of records per SQL INSERT batch + :return: Total number of rows actually inserted + """ + cr = self.env.cr + uid = self.env.uid + total_inserted = 0 + today = fields.Date.today() + + for i in range(0, len(vals_list), chunk_size): + batch = vals_list[i : i + chunk_size] + values = [] + params = [] + for v in batch: + values.append("(%s, %s, %s, %s, %s, %s, now(), now())") + params.extend( + [ + v["partner_id"], + v["cycle_id"], + v.get("state", "draft"), + v.get("enrollment_date", today), + uid, + uid, + ] + ) + + sql = """ + INSERT INTO spp_cycle_membership + (partner_id, cycle_id, state, enrollment_date, + create_uid, write_uid, create_date, write_date) + VALUES {} + ON CONFLICT (partner_id, cycle_id) DO NOTHING + """.format( # noqa: S608 # nosec B608 + ", ".join(values) + ) + cr.execute(sql, params) + total_inserted += cr.rowcount + + _logger.info( + "Bulk inserted %d cycle memberships (%d skipped as duplicates)", + total_inserted, + len(vals_list) - total_inserted, + ) + return total_inserted + def unlink(self): if not self: return diff --git a/spp_programs/models/managers/cycle_manager_base.py b/spp_programs/models/managers/cycle_manager_base.py index 1a09533b..10716d47 100644 --- a/spp_programs/models/managers/cycle_manager_base.py +++ b/spp_programs/models/managers/cycle_manager_base.py @@ -835,25 +835,26 @@ def _add_beneficiaries(self, cycle, beneficiaries, state="draft", do_count=False """Add Beneficiaries :param cycle: Recordset of cycle - :param beneficiaries: Recordset of beneficiaries + :param beneficiaries: List of partner IDs :param state: String state to be set to beneficiary :param do_count: Boolean - set to False to not run compute functions - :return: Integer - count of not enrolled members - """ - new_beneficiaries = [] - for r in beneficiaries: - new_beneficiaries.append( - [ - 0, - 0, - { - "partner_id": r, - "enrollment_date": fields.Date.today(), - "state": state, - }, - ] - ) - cycle.update({"cycle_membership_ids": new_beneficiaries}) + :return: Integer - count of inserted members + """ + today = fields.Date.today() + vals_list = [ + { + "partner_id": partner_id, + "cycle_id": cycle.id, + "enrollment_date": today, + "state": state, + } + for partner_id in beneficiaries + ] + self.env["spp.cycle.membership"].bulk_create_memberships(vals_list, skip_duplicates=True) + + # Raw SQL bypasses the ORM cache — invalidate so subsequent reads + # (e.g. cycle.cycle_membership_ids) reflect the new rows. + cycle.invalidate_recordset(["cycle_membership_ids"]) if do_count: # Update Statistics diff --git a/spp_programs/models/managers/eligibility_manager.py b/spp_programs/models/managers/eligibility_manager.py index d54f7945..5392af4a 100644 --- a/spp_programs/models/managers/eligibility_manager.py +++ b/spp_programs/models/managers/eligibility_manager.py @@ -1,7 +1,7 @@ # Part of OpenSPP. See LICENSE file for full copyright and licensing details. import logging -from odoo import Command, _, api, fields, models +from odoo import _, api, fields, models from odoo.addons.job_worker.delay import group @@ -174,11 +174,13 @@ def mark_import_as_done(self): def _import_registrants(self, new_beneficiaries, state="draft", do_count=False): _logger.info("Importing %s beneficiaries", len(new_beneficiaries)) - _logger.info("updated") - beneficiaries_val = [] - for beneficiary in new_beneficiaries: - beneficiaries_val.append(Command.create({"partner_id": beneficiary.id, "state": state})) - self.program_id.update({"program_membership_ids": beneficiaries_val}) + vals_list = [{"partner_id": b.id, "program_id": self.program_id.id, "state": state} for b in new_beneficiaries] + count = self.env["spp.program.membership"].bulk_create_memberships(vals_list, skip_duplicates=True) + _logger.info("Imported %d new memberships (%d duplicates skipped)", count, len(vals_list) - count) + + # Raw SQL bypasses the ORM cache — invalidate so subsequent reads + # (e.g. program.program_membership_ids) reflect the new rows. + self.program_id.invalidate_recordset(["program_membership_ids"]) if do_count: # Compute Statistics diff --git a/spp_programs/models/program_membership.py b/spp_programs/models/program_membership.py index c22f5a0d..48634fa8 100644 --- a/spp_programs/models/program_membership.py +++ b/spp_programs/models/program_membership.py @@ -1,4 +1,5 @@ # Part of OpenSPP. See LICENSE file for full copyright and licensing details. +import logging from lxml import etree @@ -7,6 +8,8 @@ from . import constants +_logger = logging.getLogger(__name__) + class SPPProgramMembership(models.Model): _inherit = [ @@ -345,26 +348,26 @@ def action_exit(self): } ) - @api.model_create_multi - def bulk_create_memberships(self, vals_list, chunk_size=1000): + @api.model + def bulk_create_memberships(self, vals_list, chunk_size=1000, skip_duplicates=False): """Create program memberships in bulk with optional chunking. This helper is intended for large enrollment jobs (e.g. CEL-driven bulk enrollment) where thousands of memberships need to be created in a single operation. - It preserves the normal create() semantics, including: - - standard ORM validations and constraints - - audit logging (via spp_audit rules) - - source tracking mixins - - The only optimisation is to: - - accept already-prepared value dicts - - optionally split very large batches into smaller chunks to keep - memory use and per-transaction work bounded. + :param vals_list: List of dicts with membership values + :param chunk_size: Number of records per batch (default 1000) + :param skip_duplicates: When True, use INSERT ... ON CONFLICT DO NOTHING + to silently skip duplicate (partner_id, program_id) pairs instead of + raising IntegrityError. Returns the count of inserted rows. + :return: Recordset (skip_duplicates=False) or int count (skip_duplicates=True) """ if not vals_list: - return self.env["spp.program.membership"] + return 0 if skip_duplicates else self.env["spp.program.membership"] + + if skip_duplicates: + return self._bulk_insert_on_conflict(vals_list, chunk_size) if chunk_size and chunk_size > 0: all_memberships = self.env["spp.program.membership"] @@ -386,3 +389,58 @@ def bulk_create_memberships(self, vals_list, chunk_size=1000): SPPProgramMembership, self.sudo(), # nosemgrep: odoo-sudo-without-context ).create(vals_list) + + def _bulk_insert_on_conflict(self, vals_list, chunk_size=1000): + """Insert memberships using raw SQL with ON CONFLICT DO NOTHING. + + Bypasses ORM for maximum throughput during bulk enrollment. Duplicates + (matching the UNIQUE constraint on partner_id, program_id) are silently + skipped. + + :param vals_list: List of dicts with at least partner_id, program_id, state + :param chunk_size: Number of records per SQL INSERT batch + :return: Total number of rows actually inserted + """ + cr = self.env.cr + uid = self.env.uid + total_inserted = 0 + + now = fields.Datetime.now() + + for i in range(0, len(vals_list), chunk_size): + batch = vals_list[i : i + chunk_size] + values = [] + params = [] + for v in batch: + state = v.get("state", "draft") + enrollment_date = now if state == "enrolled" else None + values.append("(%s, %s, %s, %s, %s, %s, now(), now())") + params.extend( + [ + v["partner_id"], + v["program_id"], + state, + enrollment_date, + uid, + uid, + ] + ) + + sql = """ + INSERT INTO spp_program_membership + (partner_id, program_id, state, enrollment_date, + create_uid, write_uid, create_date, write_date) + VALUES {} + ON CONFLICT (partner_id, program_id) DO NOTHING + """.format( # noqa: S608 # nosec B608 + ", ".join(values) + ) + cr.execute(sql, params) + total_inserted += cr.rowcount + + _logger.info( + "Bulk inserted %d program memberships (%d skipped as duplicates)", + total_inserted, + len(vals_list) - total_inserted, + ) + return total_inserted diff --git a/spp_programs/tests/__init__.py b/spp_programs/tests/__init__.py index a3ae74d3..344574d9 100644 --- a/spp_programs/tests/__init__.py +++ b/spp_programs/tests/__init__.py @@ -32,3 +32,4 @@ from . import test_payment_and_accounting from . import test_managers from . import test_cycle_auto_approve_fund_check +from . import test_bulk_membership diff --git a/spp_programs/tests/test_bulk_membership.py b/spp_programs/tests/test_bulk_membership.py new file mode 100644 index 00000000..e494c4d0 --- /dev/null +++ b/spp_programs/tests/test_bulk_membership.py @@ -0,0 +1,178 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Tests for Phase 7: Bulk membership creation with INSERT ON CONFLICT. + +These tests verify that bulk_create_memberships() with skip_duplicates=True +uses raw SQL INSERT ... ON CONFLICT DO NOTHING to silently skip duplicates +instead of raising IntegrityError or doing per-record search() checks. +""" + +import uuid + +from odoo import fields +from odoo.tests import TransactionCase + + +class TestBulkProgramMembership(TransactionCase): + """Test bulk_create_memberships on spp.program.membership.""" + + def setUp(self): + super().setUp() + self.program = self.env["spp.program"].create({"name": f"Test Program {uuid.uuid4().hex[:8]}"}) + self.partners = self.env["res.partner"].create( + [{"name": f"Registrant {i}", "is_registrant": True} for i in range(10)] + ) + + def test_bulk_create_inserts_all(self): + """bulk_create_memberships with skip_duplicates inserts all new records.""" + vals_list = [{"partner_id": p.id, "program_id": self.program.id, "state": "draft"} for p in self.partners] + count = self.env["spp.program.membership"].bulk_create_memberships(vals_list, skip_duplicates=True) + self.assertEqual(count, 10) + self.assertEqual( + self.env["spp.program.membership"].search_count([("program_id", "=", self.program.id)]), + 10, + ) + + def test_bulk_create_skips_duplicates(self): + """Duplicate (partner_id, program_id) pairs must be silently skipped.""" + # Create first batch + vals_list = [{"partner_id": p.id, "program_id": self.program.id, "state": "draft"} for p in self.partners[:5]] + self.env["spp.program.membership"].bulk_create_memberships(vals_list, skip_duplicates=True) + + # Create second batch with overlap + vals_list_overlap = [ + {"partner_id": p.id, "program_id": self.program.id, "state": "draft"} + for p in self.partners # includes first 5 again + ] + count = self.env["spp.program.membership"].bulk_create_memberships(vals_list_overlap, skip_duplicates=True) + # Only 5 new records should be inserted + self.assertEqual(count, 5) + self.assertEqual( + self.env["spp.program.membership"].search_count([("program_id", "=", self.program.id)]), + 10, + ) + + def test_bulk_create_all_duplicates_returns_zero(self): + """If all records already exist, return 0.""" + vals_list = [{"partner_id": p.id, "program_id": self.program.id, "state": "draft"} for p in self.partners[:3]] + self.env["spp.program.membership"].bulk_create_memberships(vals_list, skip_duplicates=True) + count = self.env["spp.program.membership"].bulk_create_memberships(vals_list, skip_duplicates=True) + self.assertEqual(count, 0) + + def test_bulk_create_empty_list(self): + """Empty vals_list should return 0.""" + count = self.env["spp.program.membership"].bulk_create_memberships([], skip_duplicates=True) + self.assertEqual(count, 0) + + def test_bulk_create_without_skip_duplicates_uses_orm(self): + """Without skip_duplicates, bulk_create_memberships should use the ORM path.""" + vals_list = [{"partner_id": p.id, "program_id": self.program.id, "state": "draft"} for p in self.partners[:3]] + result = self.env["spp.program.membership"].bulk_create_memberships(vals_list) + # ORM path returns a recordset + self.assertEqual(len(result), 3) + + def test_bulk_create_respects_chunk_size(self): + """With skip_duplicates and chunk_size, should process in chunks.""" + vals_list = [{"partner_id": p.id, "program_id": self.program.id, "state": "draft"} for p in self.partners] + count = self.env["spp.program.membership"].bulk_create_memberships( + vals_list, skip_duplicates=True, chunk_size=3 + ) + self.assertEqual(count, 10) + + +class TestBulkCycleMembership(TransactionCase): + """Test bulk_create_memberships on spp.cycle.membership.""" + + 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(10)] + ) + + def test_bulk_create_inserts_all(self): + """bulk_create_memberships with skip_duplicates inserts all new records.""" + vals_list = [{"partner_id": p.id, "cycle_id": self.cycle.id, "state": "draft"} for p in self.partners] + count = self.env["spp.cycle.membership"].bulk_create_memberships(vals_list, skip_duplicates=True) + self.assertEqual(count, 10) + + def test_bulk_create_skips_duplicates(self): + """Duplicate (partner_id, cycle_id) pairs must be silently skipped.""" + vals_first = [{"partner_id": p.id, "cycle_id": self.cycle.id, "state": "draft"} for p in self.partners[:5]] + self.env["spp.cycle.membership"].bulk_create_memberships(vals_first, skip_duplicates=True) + + vals_overlap = [{"partner_id": p.id, "cycle_id": self.cycle.id, "state": "draft"} for p in self.partners] + count = self.env["spp.cycle.membership"].bulk_create_memberships(vals_overlap, skip_duplicates=True) + self.assertEqual(count, 5) + + def test_bulk_create_empty_list(self): + """Empty vals_list should return 0.""" + count = self.env["spp.cycle.membership"].bulk_create_memberships([], skip_duplicates=True) + self.assertEqual(count, 0) + + def test_bulk_create_without_skip_duplicates_uses_orm(self): + """Without skip_duplicates, bulk_create_memberships should use the ORM path.""" + vals_list = [{"partner_id": p.id, "cycle_id": self.cycle.id, "state": "draft"} for p in self.partners[:3]] + result = self.env["spp.cycle.membership"].bulk_create_memberships(vals_list) + self.assertEqual(len(result), 3) + + +class TestCallerIntegration(TransactionCase): + """Test that _import_registrants and _add_beneficiaries use bulk_create_memberships.""" + + 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_add_beneficiaries_skips_duplicates(self): + """_add_beneficiaries should not raise on duplicate partner IDs.""" + cycle_manager = self.env["spp.cycle.manager.default"].create( + { + "name": "Test Cycle Manager", + "program_id": self.program.id, + } + ) + + partner_ids = self.partners.ids + # Add beneficiaries twice — second call should not raise + cycle_manager._add_beneficiaries(self.cycle, partner_ids, "draft") + cycle_manager._add_beneficiaries(self.cycle, partner_ids, "draft") + + # Should still only have 5 memberships + count = self.env["spp.cycle.membership"].search_count([("cycle_id", "=", self.cycle.id)]) + self.assertEqual(count, 5) + + def test_import_registrants_skips_duplicates(self): + """_import_registrants should not raise on duplicate registrants.""" + elig_manager = self.env["spp.program.membership.manager.default"].create( + { + "name": "Test Elig Manager", + "program_id": self.program.id, + } + ) + + # Import registrants twice + elig_manager._import_registrants(self.partners, "draft") + elig_manager._import_registrants(self.partners, "draft") + + # Should still only have 5 memberships + count = self.env["spp.program.membership"].search_count([("program_id", "=", self.program.id)]) + self.assertEqual(count, 5)