diff --git a/spp_programs/__manifest__.py b/spp_programs/__manifest__.py index b8bf4c1d..d86a98a5 100644 --- a/spp_programs/__manifest__.py +++ b/spp_programs/__manifest__.py @@ -4,7 +4,7 @@ "name": "OpenSPP Programs", "summary": "Manage cash and in-kind entitlements, integrate with inventory, and enhance program management features for comprehensive social protection and agricultural support.", "category": "OpenSPP/Core", - "version": "19.0.2.0.0", + "version": "19.0.2.1.0", "sequence": 1, "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_programs/migrations/19.0.2.1.0/pre-migration.py b/spp_programs/migrations/19.0.2.1.0/pre-migration.py new file mode 100644 index 00000000..0536116a --- /dev/null +++ b/spp_programs/migrations/19.0.2.1.0/pre-migration.py @@ -0,0 +1,90 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +import logging + +_logger = logging.getLogger(__name__) + + +def migrate(cr, version): + """Deduplicate data before adding SQL UNIQUE constraints. + + Removes duplicate rows using ROW_NUMBER() OVER (PARTITION BY ...), + keeping the earliest record (lowest id) for each unique combination. + """ + if not version: + return + + _deduplicate_program_memberships(cr) + _deduplicate_cycle_memberships(cr) + _deduplicate_entitlement_codes(cr) + + +def _deduplicate_program_memberships(cr): + """Remove duplicate (partner_id, program_id) rows from spp_program_membership.""" + cr.execute( + """ + DELETE FROM spp_program_membership + WHERE id IN ( + SELECT id FROM ( + SELECT id, + ROW_NUMBER() OVER ( + PARTITION BY partner_id, program_id + ORDER BY id + ) AS rn + FROM spp_program_membership + ) sub + WHERE rn > 1 + ) + """ + ) + if cr.rowcount: + _logger.info("Deduplicated %d duplicate program membership rows", cr.rowcount) + + +def _deduplicate_cycle_memberships(cr): + """Remove duplicate (partner_id, cycle_id) rows from spp_cycle_membership.""" + cr.execute( + """ + DELETE FROM spp_cycle_membership + WHERE id IN ( + SELECT id FROM ( + SELECT id, + ROW_NUMBER() OVER ( + PARTITION BY partner_id, cycle_id + ORDER BY id + ) AS rn + FROM spp_cycle_membership + ) sub + WHERE rn > 1 + ) + """ + ) + if cr.rowcount: + _logger.info("Deduplicated %d duplicate cycle membership rows", cr.rowcount) + + +def _deduplicate_entitlement_codes(cr): + """Remove duplicate code values from spp_entitlement. + + For duplicate codes, regenerates codes for the newer records rather + than deleting them, since entitlements may have financial significance. + """ + cr.execute( + """ + UPDATE spp_entitlement + SET code = code || '-' || id::text + WHERE id IN ( + SELECT id FROM ( + SELECT id, + ROW_NUMBER() OVER ( + PARTITION BY code + ORDER BY id + ) AS rn + FROM spp_entitlement + WHERE code IS NOT NULL + ) sub + WHERE rn > 1 + ) + """ + ) + if cr.rowcount: + _logger.info("Deduplicated %d entitlement rows with duplicate codes", cr.rowcount) diff --git a/spp_programs/models/cycle_membership.py b/spp_programs/models/cycle_membership.py index 8d2c1eaa..922f65b6 100644 --- a/spp_programs/models/cycle_membership.py +++ b/spp_programs/models/cycle_membership.py @@ -1,5 +1,5 @@ # Part of OpenSPP. See LICENSE file for full copyright and licensing details. -from odoo import _, api, fields, models +from odoo import _, fields, models from odoo.exceptions import ValidationError @@ -8,6 +8,14 @@ class SPPCycleMembership(models.Model): _description = "Cycle Membership" _order = "partner_id asc,id desc" + _sql_constraints = [ + ( + "unique_partner_cycle", + "UNIQUE(partner_id, cycle_id)", + "Beneficiary must be unique per cycle.", + ), + ] + partner_id = fields.Many2one("res.partner", "Registrant", help="A beneficiary", required=True, index=True) cycle_id = fields.Many2one("spp.cycle", "Cycle", help="A cycle", required=True, index=True) enrollment_date = fields.Date(default=lambda self: fields.Datetime.now()) @@ -24,25 +32,6 @@ class SPPCycleMembership(models.Model): copy=False, ) - @api.constrains("partner_id", "cycle_id") - def _check_unique_partner_per_cycle(self): - # Prefetch partner_id and cycle_id to avoid N+1 queries in loop - self.mapped("partner_id") - self.mapped("cycle_id") - - for record in self: - if record.partner_id and record.cycle_id: - existing = self.search( - [ - ("partner_id", "=", record.partner_id.id), - ("cycle_id", "=", record.cycle_id.id), - ("id", "!=", record.id), - ], - limit=1, - ) - if existing: - raise ValidationError(_("Beneficiary must be unique per cycle.")) - def _compute_display_name(self): res = super()._compute_display_name() # Prefetch cycle_id and partner_id to avoid N+1 queries in loop diff --git a/spp_programs/models/entitlement.py b/spp_programs/models/entitlement.py index ad2c1ce8..25f0987b 100644 --- a/spp_programs/models/entitlement.py +++ b/spp_programs/models/entitlement.py @@ -139,20 +139,6 @@ def _generate_code(self): payment_status = fields.Selection([("paid", "Paid"), ("notpaid", "Not Paid")], compute="_compute_payment_status") payment_date = fields.Date(compute="_compute_payment_status") - @api.constrains("code") - def _check_unique_code(self): - for record in self: - if record.code: - existing = self.search( - [ - ("code", "=", record.code), - ("id", "!=", record.id), - ], - limit=1, - ) - if existing: - raise ValidationError(_("The entitlement code must be unique.")) - @api.constrains("valid_from", "valid_until") def _check_valid_dates(self): for record in self: diff --git a/spp_programs/models/entitlement_base_model.py b/spp_programs/models/entitlement_base_model.py index 53353a86..94b272a6 100644 --- a/spp_programs/models/entitlement_base_model.py +++ b/spp_programs/models/entitlement_base_model.py @@ -17,6 +17,14 @@ class SPPEntitlement(models.Model): _order = "partner_id asc,id desc" _check_company_auto = True + _sql_constraints = [ + ( + "unique_code", + "UNIQUE(code)", + "Entitlement code must be unique.", + ), + ] + @api.model def _generate_code(self): return str(uuid4())[4:-8][3:] @@ -87,20 +95,6 @@ def _generate_code(self): payment_status = fields.Selection([("paid", "Paid"), ("notpaid", "Not Paid")], compute="_compute_payment_status") payment_date = fields.Date(compute="_compute_payment_status") - @api.constrains("code") - def _check_unique_code(self): - for record in self: - if record.code: - existing = self.search( - [ - ("code", "=", record.code), - ("id", "!=", record.id), - ], - limit=1, - ) - if existing: - raise ValidationError(_("The entitlement code must be unique.")) - @api.constrains("valid_from", "valid_until") def _check_valid_dates(self): for record in self: diff --git a/spp_programs/models/program_membership.py b/spp_programs/models/program_membership.py index ee4132ae..9af27df2 100644 --- a/spp_programs/models/program_membership.py +++ b/spp_programs/models/program_membership.py @@ -3,7 +3,7 @@ from lxml import etree from odoo import _, api, fields, models -from odoo.exceptions import UserError, ValidationError +from odoo.exceptions import UserError from . import constants @@ -19,6 +19,14 @@ class SPPProgramMembership(models.Model): _inherits = {"res.partner": "partner_id"} _order = "id desc" + _sql_constraints = [ + ( + "unique_partner_program", + "UNIQUE(partner_id, program_id)", + "Beneficiary must be unique per program.", + ), + ] + partner_id = fields.Many2one( "res.partner", "Registrant", @@ -52,25 +60,6 @@ class SPPProgramMembership(models.Model): registrant_id = fields.Integer(string="Registrant ID", related="partner_id.id") - @api.constrains("partner_id", "program_id") - def _check_unique_partner_per_program(self): - # Prefetch partner_id and program_id to avoid N+1 queries in loop - self.mapped("partner_id") - self.mapped("program_id") - - for record in self: - if record.partner_id and record.program_id: - existing = self.search( - [ - ("partner_id", "=", record.partner_id.id), - ("program_id", "=", record.program_id.id), - ("id", "!=", record.id), - ], - limit=1, - ) - if existing: - raise ValidationError(_("Beneficiary must be unique per program.")) - # TODO: Implement exit reasons # exit_reason_id = fields.Many2one("Exit Reason") Default: Completed, Opt-Out, Other diff --git a/spp_programs/tests/__init__.py b/spp_programs/tests/__init__.py index c56ebc92..a382bdd6 100644 --- a/spp_programs/tests/__init__.py +++ b/spp_programs/tests/__init__.py @@ -15,3 +15,4 @@ from . import test_eligibility_cel_integration from . import test_compliance_cel from . import test_create_program_wizard_cel +from . import test_sql_constraints diff --git a/spp_programs/tests/test_sql_constraints.py b/spp_programs/tests/test_sql_constraints.py new file mode 100644 index 00000000..cd311f12 --- /dev/null +++ b/spp_programs/tests/test_sql_constraints.py @@ -0,0 +1,255 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +import uuid + +from psycopg2 import IntegrityError + +from odoo import fields +from odoo.tests import TransactionCase +from odoo.tools import mute_logger + + +class TestSQLConstraints(TransactionCase): + """Test that SQL UNIQUE constraints enforce uniqueness at the database level. + + These constraints replace the old Python @api.constrains checks that + performed per-record search() calls, causing O(N^2) during bulk inserts. + """ + + def setUp(self): + super().setUp() + self.program = self.env["spp.program"].create({"name": f"Test Program {uuid.uuid4().hex[:8]}"}) + self.registrant = self.env["res.partner"].create( + { + "name": "Test Registrant", + "is_registrant": True, + } + ) + self.registrant2 = self.env["res.partner"].create( + { + "name": "Test Registrant 2", + "is_registrant": True, + } + ) + 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(), + } + ) + + # -- Program Membership uniqueness -- + + def test_program_membership_unique_constraint_exists(self): + """SQL UNIQUE constraint on (partner_id, program_id) must exist.""" + self.env.cr.execute( + """ + SELECT 1 FROM pg_constraint + WHERE conrelid = 'spp_program_membership'::regclass + AND contype = 'u' + AND conkey @> ARRAY[ + (SELECT attnum FROM pg_attribute + WHERE attrelid = 'spp_program_membership'::regclass + AND attname = 'partner_id'), + (SELECT attnum FROM pg_attribute + WHERE attrelid = 'spp_program_membership'::regclass + AND attname = 'program_id') + ] + """ + ) + self.assertTrue( + self.env.cr.fetchone(), + "UNIQUE constraint on (partner_id, program_id) must exist on spp_program_membership", + ) + + def test_program_membership_duplicate_blocked(self): + """Inserting a duplicate (partner_id, program_id) must raise IntegrityError.""" + self.env["spp.program.membership"].create( + { + "partner_id": self.registrant.id, + "program_id": self.program.id, + } + ) + with self.assertRaises(IntegrityError), mute_logger("odoo.sql_db"): + with self.env.cr.savepoint(): + self.env.cr.execute( + """ + INSERT INTO spp_program_membership + (partner_id, program_id, state, + create_uid, write_uid, create_date, write_date) + VALUES (%s, %s, 'draft', %s, %s, now(), now()) + """, + ( + self.registrant.id, + self.program.id, + self.env.uid, + self.env.uid, + ), + ) + + def test_program_membership_different_partners_allowed(self): + """Different partners in the same program must be allowed.""" + self.env["spp.program.membership"].create( + { + "partner_id": self.registrant.id, + "program_id": self.program.id, + } + ) + membership2 = self.env["spp.program.membership"].create( + { + "partner_id": self.registrant2.id, + "program_id": self.program.id, + } + ) + self.assertTrue(membership2.id) + + def test_program_membership_same_partner_different_programs(self): + """Same partner in different programs must be allowed.""" + program2 = self.env["spp.program"].create({"name": f"Test Program 2 {uuid.uuid4().hex[:8]}"}) + self.env["spp.program.membership"].create( + { + "partner_id": self.registrant.id, + "program_id": self.program.id, + } + ) + membership2 = self.env["spp.program.membership"].create( + { + "partner_id": self.registrant.id, + "program_id": program2.id, + } + ) + self.assertTrue(membership2.id) + + # -- Cycle Membership uniqueness -- + + def test_cycle_membership_unique_constraint_exists(self): + """SQL UNIQUE constraint on (partner_id, cycle_id) must exist.""" + self.env.cr.execute( + """ + SELECT 1 FROM pg_constraint + WHERE conrelid = 'spp_cycle_membership'::regclass + AND contype = 'u' + AND conkey @> ARRAY[ + (SELECT attnum FROM pg_attribute + WHERE attrelid = 'spp_cycle_membership'::regclass + AND attname = 'partner_id'), + (SELECT attnum FROM pg_attribute + WHERE attrelid = 'spp_cycle_membership'::regclass + AND attname = 'cycle_id') + ] + """ + ) + self.assertTrue( + self.env.cr.fetchone(), + "UNIQUE constraint on (partner_id, cycle_id) must exist on spp_cycle_membership", + ) + + def test_cycle_membership_duplicate_blocked(self): + """Inserting a duplicate (partner_id, cycle_id) must raise IntegrityError.""" + self.env["spp.cycle.membership"].create( + { + "partner_id": self.registrant.id, + "cycle_id": self.cycle.id, + } + ) + with self.assertRaises(IntegrityError), mute_logger("odoo.sql_db"): + with self.env.cr.savepoint(): + self.env.cr.execute( + """ + INSERT INTO spp_cycle_membership + (partner_id, cycle_id, state, + create_uid, write_uid, create_date, write_date) + VALUES (%s, %s, 'draft', %s, %s, now(), now()) + """, + ( + self.registrant.id, + self.cycle.id, + self.env.uid, + self.env.uid, + ), + ) + + def test_cycle_membership_different_partners_allowed(self): + """Different partners in the same cycle must be allowed.""" + self.env["spp.cycle.membership"].create( + { + "partner_id": self.registrant.id, + "cycle_id": self.cycle.id, + } + ) + membership2 = self.env["spp.cycle.membership"].create( + { + "partner_id": self.registrant2.id, + "cycle_id": self.cycle.id, + } + ) + self.assertTrue(membership2.id) + + # -- Entitlement code uniqueness -- + + def test_entitlement_code_unique_constraint_exists(self): + """SQL UNIQUE constraint on (code) must exist on spp_entitlement.""" + self.env.cr.execute( + """ + SELECT 1 FROM pg_constraint + WHERE conrelid = 'spp_entitlement'::regclass + AND contype = 'u' + AND conkey @> ARRAY[ + (SELECT attnum FROM pg_attribute + WHERE attrelid = 'spp_entitlement'::regclass + AND attname = 'code') + ] + """ + ) + self.assertTrue( + self.env.cr.fetchone(), + "UNIQUE constraint on (code) must exist on spp_entitlement", + ) + + def test_entitlement_duplicate_code_blocked(self): + """Inserting a duplicate entitlement code must raise IntegrityError.""" + entitlement = self.env["spp.entitlement"].create( + { + "partner_id": self.registrant.id, + "cycle_id": self.cycle.id, + "initial_amount": 100.0, + } + ) + with self.assertRaises(IntegrityError), mute_logger("odoo.sql_db"): + with self.env.cr.savepoint(): + self.env.cr.execute( + """ + INSERT INTO spp_entitlement + (code, partner_id, cycle_id, initial_amount, + state, create_uid, write_uid, + create_date, write_date) + VALUES (%s, %s, %s, 100.0, 'draft', %s, %s, now(), now()) + """, + ( + entitlement.code, + self.registrant2.id, + self.cycle.id, + self.env.uid, + self.env.uid, + ), + ) + + def test_entitlement_different_codes_allowed(self): + """Different entitlement codes must be allowed.""" + ent1 = self.env["spp.entitlement"].create( + { + "partner_id": self.registrant.id, + "cycle_id": self.cycle.id, + "initial_amount": 100.0, + } + ) + ent2 = self.env["spp.entitlement"].create( + { + "partner_id": self.registrant2.id, + "cycle_id": self.cycle.id, + "initial_amount": 200.0, + } + ) + self.assertNotEqual(ent1.code, ent2.code) + self.assertTrue(ent2.id)