fix(numbering): post-review fixes — credit notes, SO unlink, multi-part grouping, SQL whitelist
- 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-<parent>). - 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) <noreply@anthropic.com>
This commit is contained in:
@@ -5,7 +5,7 @@
|
|||||||
|
|
||||||
{
|
{
|
||||||
'name': 'Fusion Plating',
|
'name': 'Fusion Plating',
|
||||||
'version': '19.0.18.15.14',
|
'version': '19.0.18.15.15',
|
||||||
'category': 'Manufacturing/Plating',
|
'category': 'Manufacturing/Plating',
|
||||||
'summary': 'Core plating / metal finishing ERP: facilities, processes, tanks, baths, jobs, operators.',
|
'summary': 'Core plating / metal finishing ERP: facilities, processes, tanks, baths, jobs, operators.',
|
||||||
'description': """
|
'description': """
|
||||||
|
|||||||
@@ -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
|
See docs/superpowers/specs/2026-05-12-parent-number-hierarchy-design.md
|
||||||
for the design rationale.
|
for the design rationale.
|
||||||
"""
|
"""
|
||||||
|
import re
|
||||||
|
|
||||||
from odoo import fields, models
|
from odoo import fields, models
|
||||||
from odoo.exceptions import UserError
|
from odoo.exceptions import UserError
|
||||||
from odoo.tools.translate import _
|
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):
|
class FpParentNumberedMixin(models.AbstractModel):
|
||||||
_name = 'fp.parent.numbered.mixin'
|
_name = 'fp.parent.numbered.mixin'
|
||||||
@@ -73,6 +82,16 @@ class FpParentNumberedMixin(models.AbstractModel):
|
|||||||
if not so or not so.x_fc_parent_number:
|
if not so or not so.x_fc_parent_number:
|
||||||
return False
|
return False
|
||||||
counter_field = self._fp_parent_counter_field()
|
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
|
# SELECT FOR UPDATE - locks the SO row until commit, so a
|
||||||
# concurrent create on the same SO blocks here and reads the
|
# concurrent create on the same SO blocks here and reads the
|
||||||
# updated counter after we release. No race, no drift.
|
# updated counter after we release. No race, no drift.
|
||||||
|
|||||||
@@ -3,7 +3,7 @@
|
|||||||
# License OPL-1 (Odoo Proprietary License v1.0)
|
# License OPL-1 (Odoo Proprietary License v1.0)
|
||||||
{
|
{
|
||||||
'name': 'Fusion Plating — Native Jobs',
|
'name': 'Fusion Plating — Native Jobs',
|
||||||
'version': '19.0.8.22.7',
|
'version': '19.0.8.22.8',
|
||||||
'category': 'Manufacturing/Plating',
|
'category': 'Manufacturing/Plating',
|
||||||
'summary': 'Native plating job model — replaces mrp.production / mrp.workorder bridge.',
|
'summary': 'Native plating job model — replaces mrp.production / mrp.workorder bridge.',
|
||||||
'author': 'Nexa Systems Inc.',
|
'author': 'Nexa Systems Inc.',
|
||||||
|
|||||||
@@ -24,7 +24,7 @@ from odoo.tools.translate import _
|
|||||||
|
|
||||||
_logger = logging.getLogger(__name__)
|
_logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
CUSTOMER_TYPES = ('out_invoice', 'out_refund')
|
CUSTOMER_TYPES = ('out_invoice', 'out_refund', 'out_receipt')
|
||||||
|
|
||||||
|
|
||||||
class AccountMove(models.Model):
|
class AccountMove(models.Model):
|
||||||
@@ -35,16 +35,27 @@ class AccountMove(models.Model):
|
|||||||
# =================================================================
|
# =================================================================
|
||||||
def _fp_parent_sale_order(self):
|
def _fp_parent_sale_order(self):
|
||||||
"""Find linked SO via SO context flag (set by _create_invoices),
|
"""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')
|
so_id = self.env.context.get('fp_invoice_source_so_id')
|
||||||
if so_id:
|
if so_id:
|
||||||
so = self.env['sale.order'].browse(so_id).exists()
|
so = self.env['sale.order'].browse(so_id).exists()
|
||||||
if so:
|
if so:
|
||||||
return so
|
return so
|
||||||
if self.invoice_origin:
|
if self.invoice_origin:
|
||||||
return self.env['sale.order'].search(
|
so = self.env['sale.order'].search(
|
||||||
[('name', '=', self.invoice_origin)], limit=1,
|
[('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']
|
return self.env['sale.order']
|
||||||
|
|
||||||
def _fp_name_prefix(self):
|
def _fp_name_prefix(self):
|
||||||
@@ -68,8 +79,9 @@ class AccountMove(models.Model):
|
|||||||
|
|
||||||
@api.model
|
@api.model
|
||||||
def _fp_validate_customer_invoice(self, vals):
|
def _fp_validate_customer_invoice(self, vals):
|
||||||
"""Refuse out_invoice / out_refund creation that didn't come
|
"""Refuse out_invoice / out_refund / out_receipt creation that
|
||||||
through the SO workflow. Applies to ALL users including admins."""
|
didn't come through the SO workflow. Applies to ALL users
|
||||||
|
including admins."""
|
||||||
mtype = vals.get('move_type', 'entry')
|
mtype = vals.get('move_type', 'entry')
|
||||||
if mtype not in CUSTOMER_TYPES:
|
if mtype not in CUSTOMER_TYPES:
|
||||||
return
|
return
|
||||||
@@ -80,10 +92,22 @@ class AccountMove(models.Model):
|
|||||||
[('name', '=', origin)]
|
[('name', '=', origin)]
|
||||||
):
|
):
|
||||||
return
|
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(_(
|
raise UserError(_(
|
||||||
'Customer invoices and credit notes must be created from a '
|
'Customer invoices, credit notes, and receipts must be '
|
||||||
'Sale Order. Open the originating SO and use the Create '
|
'created from a Sale Order. Open the originating SO and '
|
||||||
'Invoice / Add Credit Note action.\n\n'
|
'use the Create Invoice / Add Credit Note action.\n\n'
|
||||||
'This rule applies to all users including administrators. '
|
'This rule applies to all users including administrators. '
|
||||||
'It is enforced to keep the parent-number audit trail '
|
'It is enforced to keep the parent-number audit trail '
|
||||||
'intact (see fusion_plating numbering policy).'
|
'intact (see fusion_plating numbering policy).'
|
||||||
|
|||||||
@@ -314,6 +314,23 @@ class SaleOrder(models.Model):
|
|||||||
fp_invoice_source_so_id=self.id if len(self) == 1 else False,
|
fp_invoice_source_so_id=self.id if len(self) == 1 else False,
|
||||||
))._create_invoices(grouped=grouped, final=final, date=date)
|
))._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):
|
def _fp_resolve_recipe_for_line(self, line):
|
||||||
"""4-tier recipe resolution. Used BOTH for grouping (Task 6
|
"""4-tier recipe resolution. Used BOTH for grouping (Task 6
|
||||||
recipe-driven WO splits) AND for the per-job vals construction.
|
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)
|
_logger.info('SO %s: no plating lines, skipping job creation.', self.name)
|
||||||
return
|
return
|
||||||
|
|
||||||
# Group by resolved recipe id (lines sharing a recipe → one WO).
|
# Group by (recipe, part, coating). Lines that share ALL THREE
|
||||||
# No-recipe lines get their own group each (preserves the legacy
|
# collapse into one WO. Sharing only the recipe is not enough —
|
||||||
# "one job per line" behaviour for unrecipe'd SOs).
|
# 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 = {}
|
groups = {}
|
||||||
unrecipe_idx = 0
|
unrecipe_idx = 0
|
||||||
for line in plating_lines:
|
for line in plating_lines:
|
||||||
recipe = self._fp_resolve_recipe_for_line(line)
|
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:
|
if recipe:
|
||||||
key = recipe.id
|
key = (recipe.id, part_id, coating_id)
|
||||||
else:
|
else:
|
||||||
unrecipe_idx += 1
|
unrecipe_idx += 1
|
||||||
key = ('no_recipe', unrecipe_idx)
|
key = ('no_recipe', unrecipe_idx)
|
||||||
|
|||||||
Reference in New Issue
Block a user