From 688fe8317c1597ce9e1c2b36cc989425ec54599d Mon Sep 17 00:00:00 2001 From: gsinghpal Date: Fri, 24 Apr 2026 22:09:16 -0400 Subject: [PATCH] refactor(jobs): address code review feedback on fp.job.step (Task 1.5) - I2: Add TODO comment block + stub button_pause/button_skip/ button_cancel that raise NotImplementedError. Makes the missing state-machine paths explicit instead of invisible gaps. Future Task 1.6 wires the real implementations; shop-floor buttons in Task 1.8 can already point to the right method names. - I3: button_finish now preserves first-finish audit timestamp via 'if not step.date_finished:' guard, mirroring button_start. Future rework flow that re-opens a step won't lose the original finish data. The duration_actual rollup landing in Task 1.7 will use timelog rows for the most-recent interval if needed. - I4: step_count and step_done_count are now store=True so list views and stat buttons (Task 1.8) don't recompute across all job rows on every render. step_progress_pct and current_step_id stay non-stored - they're cheap derivatives. Split compute methods so stored + non-stored fields don't share one method (Odoo flags the mix as inconsistent and recomputes stored fields whenever the non-stored one is read, defeating the perf gain). Manifest 19.0.8.4.0 -> 19.0.8.4.1. Part of: native job model migration (spec 2026-04-25) Co-Authored-By: Claude Opus 4.7 (1M context) --- fusion_plating/fusion_plating/__manifest__.py | 2 +- .../fusion_plating/models/fp_job.py | 15 +++++-- .../fusion_plating/models/fp_job_step.py | 42 ++++++++++++++++++- 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/fusion_plating/fusion_plating/__manifest__.py b/fusion_plating/fusion_plating/__manifest__.py index b5ccd317..af07c27e 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.8.4.0', + 'version': '19.0.8.4.1', '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_job.py b/fusion_plating/fusion_plating/models/fp_job.py index 511aca90..8172c50d 100644 --- a/fusion_plating/fusion_plating/models/fp_job.py +++ b/fusion_plating/fusion_plating/models/fp_job.py @@ -186,9 +186,14 @@ class FpJob(models.Model): 'job_id', string='Steps', ) - step_count = fields.Integer(compute='_compute_step_counts') - step_done_count = fields.Integer(compute='_compute_step_counts') - step_progress_pct = fields.Float(compute='_compute_step_counts') + # step_count + step_done_count are stored (drive list views / stat + # buttons in Task 1.8). step_progress_pct stays non-stored — it's a + # cheap derivative. Odoo flags as inconsistent when stored and + # non-stored fields share a compute method, so they get distinct + # methods below. + step_count = fields.Integer(compute='_compute_step_counts', store=True) + step_done_count = fields.Integer(compute='_compute_step_counts', store=True) + step_progress_pct = fields.Float(compute='_compute_step_progress_pct') current_step_id = fields.Many2one( 'fp.job.step', compute='_compute_current_step', @@ -199,6 +204,10 @@ class FpJob(models.Model): for job in self: job.step_count = len(job.step_ids) job.step_done_count = len(job.step_ids.filtered(lambda s: s.state == 'done')) + + @api.depends('step_count', 'step_done_count') + def _compute_step_progress_pct(self): + for job in self: job.step_progress_pct = ( (job.step_done_count / job.step_count * 100.0) if job.step_count else 0.0 diff --git a/fusion_plating/fusion_plating/models/fp_job_step.py b/fusion_plating/fusion_plating/models/fp_job_step.py index 363b3d0d..6cca637f 100644 --- a/fusion_plating/fusion_plating/models/fp_job_step.py +++ b/fusion_plating/fusion_plating/models/fp_job_step.py @@ -76,6 +76,39 @@ class FpJobStep(models.Model): duration_actual = fields.Float(string='Actual Minutes', readonly=True) instructions = fields.Html(string='Step Instructions') + # ------------------------------------------------------------------ + # State machine — actions + # ------------------------------------------------------------------ + # Implemented: button_start (ready/paused → in_progress), + # button_finish (in_progress → done). + # Stubs (raise NotImplementedError for Task 1.6): + # button_pause (in_progress → paused) + # button_resume (covered by button_start when state='paused') + # button_skip (pending/ready → skipped) + # button_cancel (any non-done → cancelled) + # Predecessor-driven transition pending → ready will land in + # Task 1.6 along with first-step / dependency wiring. + # ------------------------------------------------------------------ + + def button_pause(self): + raise NotImplementedError(_( + "button_pause lands in Task 1.6 (operator pause / break / " + "end-of-shift). Use button_finish to complete a step or set " + "state directly via privileged code." + )) + + def button_skip(self): + raise NotImplementedError(_( + "button_skip lands in Task 1.6 (skip an opt-in step that " + "wasn't activated for this job)." + )) + + def button_cancel(self): + raise NotImplementedError(_( + "button_cancel lands in Task 1.6 (cancelling a single step; " + "cancelling the whole job runs through fp.job.action_cancel)." + )) + def button_start(self): for step in self: if step.state not in ('ready', 'paused'): @@ -95,6 +128,11 @@ class FpJobStep(models.Model): "Step '%s' is in state '%s' — only in-progress steps can finish." ) % (step.name, step.state)) step.state = 'done' - step.date_finished = fields.Datetime.now() - step.finished_by_user_id = self.env.user + # First-finish audit (mirrors button_start's first-start guard). + # If a future rework flow re-opens then re-finishes, the original + # finish timestamp/user is preserved. duration_actual rollups + # in Task 1.7 will use timelog rows for the latest interval. + if not step.date_finished: + step.date_finished = fields.Datetime.now() + step.finished_by_user_id = self.env.user return True