refactor(jobs): address code review feedback on fp.job
- Sequence: add noupdate="1" to fp_job_sequences.xml. Without it, every module update resets number_next to 1, corrupting the live job-number stream. Matches fp_sequence_data.xml convention. - action_cancel now raises UserError on an already-cancelled job instead of silently rewriting state. Audit-grade traceability expects explicit failures. - Added TODO marker for action_hold / action_resume / action_revert_to_confirmed so future authors don't bypass the state-machine guards. - Tests: added cannot_cancel_done (covers the dead-code UserError branch) and cannot_cancel_already_cancelled. Manifest version bumped 19.0.8.2.0 -> 19.0.8.2.1. Part of: native job model migration (spec 2026-04-25) 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.8.2.0',
|
'version': '19.0.8.2.1',
|
||||||
'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': """
|
||||||
|
|||||||
@@ -1,5 +1,8 @@
|
|||||||
<?xml version="1.0" encoding="UTF-8"?>
|
<?xml version="1.0" encoding="UTF-8"?>
|
||||||
<odoo>
|
<!-- noupdate="1" is REQUIRED — without it, every -u fusion_plating
|
||||||
|
resets number_next back to 1, which corrupts the live sequence
|
||||||
|
on every module update. Matches the convention in fp_sequence_data.xml. -->
|
||||||
|
<odoo noupdate="1">
|
||||||
<!-- Sequence for fp.job. Format: WH/JOB/00001 onwards.
|
<!-- Sequence for fp.job. Format: WH/JOB/00001 onwards.
|
||||||
Migrated mrp.production records keep their WH/MO/... names. -->
|
Migrated mrp.production records keep their WH/MO/... names. -->
|
||||||
<record id="seq_fp_job" model="ir.sequence">
|
<record id="seq_fp_job" model="ir.sequence">
|
||||||
|
|||||||
@@ -89,6 +89,14 @@ class FpJob(models.Model):
|
|||||||
vals['name'] = self.env['ir.sequence'].next_by_code('fp.job') or _('New')
|
vals['name'] = self.env['ir.sequence'].next_by_code('fp.job') or _('New')
|
||||||
return super().create(vals_list)
|
return super().create(vals_list)
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# State machine — actions
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# TODO(fp.job state-machine completeness): action_hold, action_resume,
|
||||||
|
# action_revert_to_confirmed (rework path) — to be added when shopfloor
|
||||||
|
# / rework workflows are wired up. For now, draft → confirmed and the
|
||||||
|
# cancel paths are the only enforced transitions; everything else is
|
||||||
|
# an explicit `state` write by privileged code.
|
||||||
def action_confirm(self):
|
def action_confirm(self):
|
||||||
for job in self:
|
for job in self:
|
||||||
if job.state != 'draft':
|
if job.state != 'draft':
|
||||||
@@ -102,7 +110,11 @@ class FpJob(models.Model):
|
|||||||
for job in self:
|
for job in self:
|
||||||
if job.state == 'done':
|
if job.state == 'done':
|
||||||
raise UserError(_(
|
raise UserError(_(
|
||||||
"Job %s is done - cannot cancel."
|
"Job %s is done — cannot cancel."
|
||||||
|
) % job.name)
|
||||||
|
if job.state == 'cancelled':
|
||||||
|
raise UserError(_(
|
||||||
|
"Job %s is already cancelled."
|
||||||
) % job.name)
|
) % job.name)
|
||||||
job.state = 'cancelled'
|
job.state = 'cancelled'
|
||||||
return True
|
return True
|
||||||
|
|||||||
@@ -44,3 +44,21 @@ class TestFpJobStateMachine(TransactionCase):
|
|||||||
job.action_cancel()
|
job.action_cancel()
|
||||||
with self.assertRaises(UserError):
|
with self.assertRaises(UserError):
|
||||||
job.action_confirm()
|
job.action_confirm()
|
||||||
|
|
||||||
|
def test_cannot_cancel_done(self):
|
||||||
|
# Done jobs cannot be cancelled — covers the UserError branch in
|
||||||
|
# action_cancel.
|
||||||
|
job = self._make_job()
|
||||||
|
job.action_confirm()
|
||||||
|
# Force the state to 'done' for the test (no public action yet —
|
||||||
|
# done is set by step-completion logic landing in Task 1.5+).
|
||||||
|
job.state = 'done'
|
||||||
|
with self.assertRaises(UserError):
|
||||||
|
job.action_cancel()
|
||||||
|
|
||||||
|
def test_cannot_cancel_already_cancelled(self):
|
||||||
|
# Idempotent re-cancel is now an explicit error.
|
||||||
|
job = self._make_job()
|
||||||
|
job.action_cancel()
|
||||||
|
with self.assertRaises(UserError):
|
||||||
|
job.action_cancel()
|
||||||
|
|||||||
Reference in New Issue
Block a user