fix(contract-review): WO step routes to QA-005 + auto-stage on part create
Two bugs fixed in one drop, both targeting the contract review (QA-005)
enforcement gap reported on entech.
## Bug 1 — WO step routed to wrong wizard
Symptom: clicking Finish & Next or Record on a Contract Review step in
WH/JOB/00339 opened the generic measurement wizard with three fake
prompts (Reviewer Initials / Date Reviewed / QA-005 Approved). No path
to the actual QA-005 form from the work order.
Root cause: action_finish_and_advance + action_open_input_wizard had no
branch for recipe_node.default_kind == 'contract_review'. The step.kind
mapping collapses contract_review -> 'other' so kind-based detection
wouldn't have worked either; gate has to live at the recipe-node layer.
Fix in fusion_plating_jobs/models/fp_job_step.py (v19.0.8.14.6):
- action_finish_and_advance:329 calls _fp_contract_review_redirect
before the input-wizard branch
- action_open_input_wizard:844 same gate, keeps Record button consistent
- _fp_contract_review_redirect:866 (new) returns the part's
action_start_contract_review() unless review.state in
(complete, dismissed) — gate clears so the step can finish after
the operator signs QA-005.
## Bug 2 — Part create did not enforce contract review
Symptom: spec called for a banner-only UX. User wanted true automatic
enforcement on first part creation under an enforced customer.
Fix in fusion_plating_quality/models/fp_part_catalog.py (v19.0.4.10.0):
- @api.model_create_multi def create() override
- _fp_enforce_contract_review_on_create() helper auto-stages the
fp.contract.review record AND surfaces three prominent reminders:
1. Sticky bus.bus warning toast (top-right, doesn't auto-dismiss)
2. mail.activity (To Do) on the part for the current user
3. Smart button on the part form lights up (review now exists)
- Idempotent: skips parts that already carry a review id
- Soft-fails: bus or activity outage doesn't block part creation
- create()-only — write/update flows never re-trigger
Sub 4's existing info banner stays as a fourth surface.
## Tests
- fusion_plating_jobs/tests/test_fp_job_extensions.py:
+TestContractReviewStepRouting (5 tests covering both routing methods,
the complete/dismissed gate-clear, and non-CR step regression)
- fusion_plating_quality/tests/test_part_catalog_contract_review_enforcement.py
(NEW): 9 tests covering auto-create, batch create, idempotency,
activity surface, bus surface, write-must-not-retrigger, soft-fail.
- docs/superpowers/tests/2026-04-22-sub4-smoke.py: flipped the
"no review yet" assertion to "review auto-created" to match new
behavior. Sign-flow assertions unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsinghpal
2
fusion_plating/fusion_plating_quality/tests/__init__.py
Normal file
2
fusion_plating/fusion_plating_quality/tests/__init__.py
Normal file
@@ -0,0 +1,2 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
from . import test_part_catalog_contract_review_enforcement
|
||||
@@ -0,0 +1,240 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Copyright 2026 Nexa Systems Inc.
|
||||
# License OPL-1 (Odoo Proprietary License v1.0)
|
||||
# Part of the Fusion Plating product family.
|
||||
"""Regression coverage for v19.0.4.10.0 contract-review enforcement on
|
||||
fp.part.catalog create.
|
||||
|
||||
The create() override must:
|
||||
1. Auto-create an fp.contract.review record for new parts under a
|
||||
customer that has x_fc_contract_review_required = True.
|
||||
2. Skip parts whose customer has the toggle off.
|
||||
3. Skip parts that already carry a review id (idempotent on copy/import).
|
||||
4. Schedule a mail.activity on the part for the current user.
|
||||
5. Push a sticky bus.bus warning notification.
|
||||
6. NOT fire on plain .write() (i.e. update flows do not re-trigger).
|
||||
"""
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
from odoo.tests.common import TransactionCase
|
||||
|
||||
|
||||
class TestContractReviewEnforcementOnCreate(TransactionCase):
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
# Customer A — enforcement ON
|
||||
self.cust_enforced = self.env['res.partner'].create({
|
||||
'name': 'Enforced Customer',
|
||||
'is_company': True,
|
||||
'customer_rank': 1,
|
||||
'x_fc_contract_review_required': True,
|
||||
})
|
||||
# Customer B — enforcement OFF (control)
|
||||
self.cust_unenforced = self.env['res.partner'].create({
|
||||
'name': 'Unenforced Customer',
|
||||
'is_company': True,
|
||||
'customer_rank': 1,
|
||||
'x_fc_contract_review_required': False,
|
||||
})
|
||||
|
||||
# -- Core auto-creation -----------------------------------------
|
||||
|
||||
def test_review_auto_created_for_enforced_customer(self):
|
||||
part = self.env['fp.part.catalog'].create({
|
||||
'partner_id': self.cust_enforced.id,
|
||||
'part_number': 'AUTO-001',
|
||||
'revision': 'A',
|
||||
})
|
||||
self.assertTrue(
|
||||
part.x_fc_contract_review_id,
|
||||
'Review must be auto-created on .create() when the customer '
|
||||
'has x_fc_contract_review_required=True.',
|
||||
)
|
||||
self.assertEqual(
|
||||
part.x_fc_contract_review_id.state, 'assistant_review',
|
||||
'Auto-created review should start in assistant_review.',
|
||||
)
|
||||
self.assertEqual(
|
||||
part.x_fc_contract_review_id.part_id, part,
|
||||
'Review must be linked back to the part it was created for.',
|
||||
)
|
||||
|
||||
def test_no_review_for_unenforced_customer(self):
|
||||
part = self.env['fp.part.catalog'].create({
|
||||
'partner_id': self.cust_unenforced.id,
|
||||
'part_number': 'NONE-001',
|
||||
'revision': 'A',
|
||||
})
|
||||
self.assertFalse(
|
||||
part.x_fc_contract_review_id,
|
||||
'Review must NOT be created for customers without enforcement.',
|
||||
)
|
||||
|
||||
def test_existing_review_is_not_replaced(self):
|
||||
# Pre-create a review and pass it in vals — create() must not
|
||||
# overwrite it (idempotency on copy/import flows).
|
||||
existing_part = self.env['fp.part.catalog'].create({
|
||||
'partner_id': self.cust_enforced.id,
|
||||
'part_number': 'EXIST-001',
|
||||
'revision': 'A',
|
||||
})
|
||||
existing_review = existing_part.x_fc_contract_review_id
|
||||
self.assertTrue(existing_review)
|
||||
|
||||
# Now create a SECOND part that already references the same
|
||||
# review (simulating a copy with carried-over m2o).
|
||||
part2 = self.env['fp.part.catalog'].create({
|
||||
'partner_id': self.cust_enforced.id,
|
||||
'part_number': 'EXIST-002',
|
||||
'revision': 'A',
|
||||
'x_fc_contract_review_id': existing_review.id,
|
||||
})
|
||||
self.assertEqual(
|
||||
part2.x_fc_contract_review_id, existing_review,
|
||||
'Pre-set review id must be preserved, not replaced.',
|
||||
)
|
||||
|
||||
def test_batch_create_each_part_gets_own_review(self):
|
||||
# Batch create — each enforced part gets its own review.
|
||||
parts = self.env['fp.part.catalog'].create([
|
||||
{'partner_id': self.cust_enforced.id,
|
||||
'part_number': 'BATCH-001', 'revision': 'A'},
|
||||
{'partner_id': self.cust_enforced.id,
|
||||
'part_number': 'BATCH-002', 'revision': 'A'},
|
||||
{'partner_id': self.cust_unenforced.id,
|
||||
'part_number': 'BATCH-003', 'revision': 'A'},
|
||||
])
|
||||
self.assertTrue(parts[0].x_fc_contract_review_id)
|
||||
self.assertTrue(parts[1].x_fc_contract_review_id)
|
||||
self.assertFalse(parts[2].x_fc_contract_review_id)
|
||||
self.assertNotEqual(
|
||||
parts[0].x_fc_contract_review_id,
|
||||
parts[1].x_fc_contract_review_id,
|
||||
'Each enforced part must get its OWN review record.',
|
||||
)
|
||||
|
||||
# -- Activity surface -------------------------------------------
|
||||
|
||||
def test_activity_scheduled_on_part(self):
|
||||
part = self.env['fp.part.catalog'].create({
|
||||
'partner_id': self.cust_enforced.id,
|
||||
'part_number': 'ACT-001',
|
||||
'revision': 'A',
|
||||
})
|
||||
activities = self.env['mail.activity'].search([
|
||||
('res_model', '=', 'fp.part.catalog'),
|
||||
('res_id', '=', part.id),
|
||||
])
|
||||
self.assertTrue(
|
||||
activities,
|
||||
'A mail.activity must be scheduled on the part for the '
|
||||
'current user.',
|
||||
)
|
||||
self.assertIn(
|
||||
'Contract Review',
|
||||
activities[0].summary or '',
|
||||
'Activity summary must mention Contract Review.',
|
||||
)
|
||||
|
||||
def test_no_activity_for_unenforced_customer(self):
|
||||
part = self.env['fp.part.catalog'].create({
|
||||
'partner_id': self.cust_unenforced.id,
|
||||
'part_number': 'NOACT-001',
|
||||
'revision': 'A',
|
||||
})
|
||||
activities = self.env['mail.activity'].search([
|
||||
('res_model', '=', 'fp.part.catalog'),
|
||||
('res_id', '=', part.id),
|
||||
])
|
||||
self.assertFalse(
|
||||
activities,
|
||||
'No activity should be scheduled for unenforced customers.',
|
||||
)
|
||||
|
||||
# -- Bus notification surface -----------------------------------
|
||||
|
||||
def test_bus_notification_pushed_on_create(self):
|
||||
# Spy on bus.bus._sendone to verify a sticky warning was pushed.
|
||||
with patch.object(
|
||||
self.env.registry['bus.bus'], '_sendone', autospec=True,
|
||||
) as mock_send:
|
||||
self.env['fp.part.catalog'].create({
|
||||
'partner_id': self.cust_enforced.id,
|
||||
'part_number': 'BUS-001',
|
||||
'revision': 'A',
|
||||
})
|
||||
self.assertTrue(
|
||||
mock_send.called,
|
||||
'bus.bus._sendone must be called on enforced create.',
|
||||
)
|
||||
# Inspect the call payload — type=warning + sticky=True.
|
||||
# Args: (self, target, type, payload)
|
||||
call_args = mock_send.call_args
|
||||
payload = call_args.args[3] if len(call_args.args) >= 4 else call_args.kwargs.get('notification')
|
||||
self.assertEqual(payload.get('type'), 'warning')
|
||||
self.assertTrue(payload.get('sticky'))
|
||||
self.assertIn('Contract Review', payload.get('title', ''))
|
||||
|
||||
def test_no_bus_notification_for_unenforced_customer(self):
|
||||
with patch.object(
|
||||
self.env.registry['bus.bus'], '_sendone', autospec=True,
|
||||
) as mock_send:
|
||||
self.env['fp.part.catalog'].create({
|
||||
'partner_id': self.cust_unenforced.id,
|
||||
'part_number': 'BUS-002',
|
||||
'revision': 'A',
|
||||
})
|
||||
self.assertFalse(
|
||||
mock_send.called,
|
||||
'bus.bus._sendone must NOT be called for unenforced customers.',
|
||||
)
|
||||
|
||||
# -- Write must NOT re-trigger ----------------------------------
|
||||
|
||||
def test_write_does_not_retrigger_alert(self):
|
||||
# Pre-existing part under unenforced customer — no review yet.
|
||||
part = self.env['fp.part.catalog'].create({
|
||||
'partner_id': self.cust_unenforced.id,
|
||||
'part_number': 'WRITE-001',
|
||||
'revision': 'A',
|
||||
})
|
||||
self.assertFalse(part.x_fc_contract_review_id)
|
||||
|
||||
# Now flip the customer's flag and update the part. The create
|
||||
# gate is .create()-only by design — write/update must NOT
|
||||
# auto-create a review or push a notification.
|
||||
self.cust_unenforced.x_fc_contract_review_required = True
|
||||
with patch.object(
|
||||
self.env.registry['bus.bus'], '_sendone', autospec=True,
|
||||
) as mock_send:
|
||||
part.write({'revision_note': 'updated after enforcement enabled'})
|
||||
self.assertFalse(
|
||||
mock_send.called,
|
||||
'write() must NOT push a contract-review notification — '
|
||||
'enforcement only applies on first creation.',
|
||||
)
|
||||
self.assertFalse(
|
||||
part.x_fc_contract_review_id,
|
||||
'write() must NOT auto-create a review; existing parts '
|
||||
'keep their state.',
|
||||
)
|
||||
|
||||
# -- Soft-fail safety -------------------------------------------
|
||||
|
||||
def test_create_succeeds_even_if_bus_fails(self):
|
||||
# If bus.bus.\_sendone raises, the part create must still succeed.
|
||||
with patch.object(
|
||||
self.env.registry['bus.bus'], '_sendone', autospec=True,
|
||||
side_effect=RuntimeError('bus offline'),
|
||||
):
|
||||
part = self.env['fp.part.catalog'].create({
|
||||
'partner_id': self.cust_enforced.id,
|
||||
'part_number': 'SOFT-001',
|
||||
'revision': 'A',
|
||||
})
|
||||
# Part was created; review was auto-staged BEFORE the bus push
|
||||
# so it survives a bus failure.
|
||||
self.assertTrue(part.id)
|
||||
self.assertTrue(part.x_fc_contract_review_id)
|
||||
Reference in New Issue
Block a user