From 457d9b7dbff20c3f90aafa6f17b91789b84db046 Mon Sep 17 00:00:00 2001 From: gsinghpal Date: Tue, 12 May 2026 18:19:08 -0400 Subject: [PATCH] =?UTF-8?q?fix(numbering):=20post-review=20fixes=20?= =?UTF-8?q?=E2=80=94=20credit=20notes,=20SO=20unlink,=20multi-part=20group?= =?UTF-8?q?ing,=20SQL=20whitelist?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - B1: Add Credit Note wizard path was blocked because invoice_origin has copy=False and the wizard doesn't set fp_from_so_invoice. Now the validator allows reversals when reversed_entry_id points at a customer-facing move that itself went through the validator at original creation time. account.move._fp_parent_sale_order also walks self.reversed_entry_id._fp_parent_sale_order so the credit note inherits the parent number (CN-). - Bug 1: sale.order.unlink() now blocks deletion when x_fc_parent_number is set (matches spec §6.2). Draft quotes remain freely deletable per Odoo standard. Applies to all users including admins. - Bug 2: out_receipt added to CUSTOMER_TYPES so POS-style receipts hit the same SO-flow gate as out_invoice / out_refund. - C1: WO grouping key changed from recipe.id to (recipe.id, part.id, coating.id). Bundling lines with different parts under one WO put first_line's part_number on the CoC header — silent compliance mis-attestation. Now distinct parts always get distinct WOs even when they share a recipe. - C3: SQL whitelist (_FP_COUNTER_FIELD_RE) on _fp_assign_parent_name's interpolated counter field name. No user input today; defence in depth for future subclasses that might read the name from context. Verified on entech: parent=30017, credit note = CN-30017, multi-part SO produces 2 WOs (one per part), confirmed-SO unlink blocked, out_receipt blocked, whitelist regex enforced. Co-Authored-By: Claude Opus 4.7 (1M context) --- fusion_plating/fusion_plating/__manifest__.py | 2 +- .../models/fp_parent_numbered_mixin.py | 19 +++++++++ .../fusion_plating_jobs/__manifest__.py | 2 +- .../models/account_move.py | 40 +++++++++++++++---- .../fusion_plating_jobs/models/sale_order.py | 39 ++++++++++++++++-- 5 files changed, 88 insertions(+), 14 deletions(-) diff --git a/fusion_plating/fusion_plating/__manifest__.py b/fusion_plating/fusion_plating/__manifest__.py index 68143908..a015b56a 100644 --- a/fusion_plating/fusion_plating/__manifest__.py +++ b/fusion_plating/fusion_plating/__manifest__.py @@ -5,7 +5,7 @@ { 'name': 'Fusion Plating', - 'version': '19.0.18.15.14', + 'version': '19.0.18.15.15', 'category': 'Manufacturing/Plating', 'summary': 'Core plating / metal finishing ERP: facilities, processes, tanks, baths, jobs, operators.', 'description': """ diff --git a/fusion_plating/fusion_plating/models/fp_parent_numbered_mixin.py b/fusion_plating/fusion_plating/models/fp_parent_numbered_mixin.py index ab6d859f..c5c0ccdb 100644 --- a/fusion_plating/fusion_plating/models/fp_parent_numbered_mixin.py +++ b/fusion_plating/fusion_plating/models/fp_parent_numbered_mixin.py @@ -11,10 +11,19 @@ are impossible. Subclasses implement three small hooks and call See docs/superpowers/specs/2026-05-12-parent-number-hierarchy-design.md for the design rationale. """ +import re + from odoo import fields, models from odoo.exceptions import UserError from odoo.tools.translate import _ +# Whitelist regex for counter-field names. The mixin interpolates the +# returned name into raw SQL, so a future subclass that read this from +# a context value or Selection field would otherwise open a SQL-injection +# surface. Enforce: must look like one of our x_fc_pn_*_count counters +# (lowercase letters / underscores only). +_FP_COUNTER_FIELD_RE = re.compile(r'^x_fc_pn_[a-z_]+_count$') + class FpParentNumberedMixin(models.AbstractModel): _name = 'fp.parent.numbered.mixin' @@ -73,6 +82,16 @@ class FpParentNumberedMixin(models.AbstractModel): if not so or not so.x_fc_parent_number: return False counter_field = self._fp_parent_counter_field() + # Whitelist check — the field name is interpolated directly into + # SQL below, so we never trust an arbitrary string. All current + # subclasses return a literal; this guard exists so a future + # subclass that reads the field name from context / Selection / + # user input can't smuggle a SQL fragment in. + if not _FP_COUNTER_FIELD_RE.match(counter_field or ''): + raise UserError(_( + 'Invalid parent-counter field name %r — must match ' + 'pattern x_fc_pn_*_count.' + ) % counter_field) # SELECT FOR UPDATE - locks the SO row until commit, so a # concurrent create on the same SO blocks here and reads the # updated counter after we release. No race, no drift. diff --git a/fusion_plating/fusion_plating_jobs/__manifest__.py b/fusion_plating/fusion_plating_jobs/__manifest__.py index d542e670..822024bd 100644 --- a/fusion_plating/fusion_plating_jobs/__manifest__.py +++ b/fusion_plating/fusion_plating_jobs/__manifest__.py @@ -3,7 +3,7 @@ # License OPL-1 (Odoo Proprietary License v1.0) { 'name': 'Fusion Plating — Native Jobs', - 'version': '19.0.8.22.7', + 'version': '19.0.8.22.8', 'category': 'Manufacturing/Plating', 'summary': 'Native plating job model — replaces mrp.production / mrp.workorder bridge.', 'author': 'Nexa Systems Inc.', diff --git a/fusion_plating/fusion_plating_jobs/models/account_move.py b/fusion_plating/fusion_plating_jobs/models/account_move.py index 829ac535..cd333808 100644 --- a/fusion_plating/fusion_plating_jobs/models/account_move.py +++ b/fusion_plating/fusion_plating_jobs/models/account_move.py @@ -24,7 +24,7 @@ from odoo.tools.translate import _ _logger = logging.getLogger(__name__) -CUSTOMER_TYPES = ('out_invoice', 'out_refund') +CUSTOMER_TYPES = ('out_invoice', 'out_refund', 'out_receipt') class AccountMove(models.Model): @@ -35,16 +35,27 @@ class AccountMove(models.Model): # ================================================================= def _fp_parent_sale_order(self): """Find linked SO via SO context flag (set by _create_invoices), - or fall back to invoice_origin name match.""" + or fall back to invoice_origin name match, then to the reversed + entry's SO (for the Add Credit Note path where invoice_origin + has copy=False and doesn't survive the move.copy()).""" so_id = self.env.context.get('fp_invoice_source_so_id') if so_id: so = self.env['sale.order'].browse(so_id).exists() if so: return so if self.invoice_origin: - return self.env['sale.order'].search( + so = self.env['sale.order'].search( [('name', '=', self.invoice_origin)], limit=1, ) + if so: + return so + # Reversal path: read the parent move's SO link so the credit + # note's name flows from the same parent number as the invoice + # it's reversing. + if self.reversed_entry_id: + parent_so = self.reversed_entry_id._fp_parent_sale_order() + if parent_so: + return parent_so return self.env['sale.order'] def _fp_name_prefix(self): @@ -68,8 +79,9 @@ class AccountMove(models.Model): @api.model def _fp_validate_customer_invoice(self, vals): - """Refuse out_invoice / out_refund creation that didn't come - through the SO workflow. Applies to ALL users including admins.""" + """Refuse out_invoice / out_refund / out_receipt creation that + didn't come through the SO workflow. Applies to ALL users + including admins.""" mtype = vals.get('move_type', 'entry') if mtype not in CUSTOMER_TYPES: return @@ -80,10 +92,22 @@ class AccountMove(models.Model): [('name', '=', origin)] ): return + # Credit-note / reversal path: Odoo's "Add Credit Note" wizard + # calls move.copy() with reversed_entry_id set in the defaults, + # but invoice_origin has copy=False on the standard field so + # it doesn't survive the copy. Allow reversals through as long + # as the reversed entry is itself a customer-facing move (which + # means it already went through this validator at original + # creation time — the audit trail is intact). + reversed_id = vals.get('reversed_entry_id') + if reversed_id: + parent = self.env['account.move'].sudo().browse(reversed_id) + if parent.exists() and parent.move_type in CUSTOMER_TYPES: + return raise UserError(_( - 'Customer invoices and credit notes must be created from a ' - 'Sale Order. Open the originating SO and use the Create ' - 'Invoice / Add Credit Note action.\n\n' + 'Customer invoices, credit notes, and receipts must be ' + 'created from a Sale Order. Open the originating SO and ' + 'use the Create Invoice / Add Credit Note action.\n\n' 'This rule applies to all users including administrators. ' 'It is enforced to keep the parent-number audit trail ' 'intact (see fusion_plating numbering policy).' diff --git a/fusion_plating/fusion_plating_jobs/models/sale_order.py b/fusion_plating/fusion_plating_jobs/models/sale_order.py index d09537d7..e1ff3f2a 100644 --- a/fusion_plating/fusion_plating_jobs/models/sale_order.py +++ b/fusion_plating/fusion_plating_jobs/models/sale_order.py @@ -314,6 +314,23 @@ class SaleOrder(models.Model): fp_invoice_source_so_id=self.id if len(self) == 1 else False, ))._create_invoices(grouped=grouped, final=final, date=date) + def unlink(self): + """Spec §6.2 — confirmed SOs are part of the compliance audit + trail and cannot be deleted. Cancellation must go through the + state machine instead. Draft SOs (no parent_number assigned + yet) remain freely deletable per Odoo standard. Applies to + all users including administrators.""" + for so in self: + if so.x_fc_parent_number: + raise UserError(_( + 'Sale Order "%(name)s" cannot be deleted — it has ' + 'been confirmed (parent number %(parent)s issued) ' + 'and is part of the compliance audit trail. Cancel ' + 'it instead. This rule applies to all users ' + 'including administrators.' + ) % {'name': so.display_name, 'parent': so.x_fc_parent_number}) + return super().unlink() + def _fp_resolve_recipe_for_line(self, line): """4-tier recipe resolution. Used BOTH for grouping (Task 6 recipe-driven WO splits) AND for the per-job vals construction. @@ -395,15 +412,29 @@ class SaleOrder(models.Model): _logger.info('SO %s: no plating lines, skipping job creation.', self.name) return - # Group by resolved recipe id (lines sharing a recipe → one WO). - # No-recipe lines get their own group each (preserves the legacy - # "one job per line" behaviour for unrecipe'd SOs). + # Group by (recipe, part, coating). Lines that share ALL THREE + # collapse into one WO. Sharing only the recipe is not enough — + # the WO header captures part_id and coating_config_id from + # first_line, and downstream the CoC prints the WO header's + # part_number on the customer-facing cert. Bundling Part A + + # Part B under one WO because they happen to share a recipe + # would put Part A's number on a cert covering both, which is + # a compliance bug (silent mis-attestation). + # No-recipe lines get their own group each. groups = {} unrecipe_idx = 0 for line in plating_lines: recipe = self._fp_resolve_recipe_for_line(line) + part_id = ( + 'x_fc_part_catalog_id' in line._fields + and line.x_fc_part_catalog_id.id + ) or False + coating_id = ( + 'x_fc_coating_config_id' in line._fields + and line.x_fc_coating_config_id.id + ) or False if recipe: - key = recipe.id + key = (recipe.id, part_id, coating_id) else: unrecipe_idx += 1 key = ('no_recipe', unrecipe_idx)