From 7c3126969178f9aa27e7d9adcd4103e4ac581a31 Mon Sep 17 00:00:00 2001 From: gsinghpal Date: Tue, 19 May 2026 22:53:09 -0400 Subject: [PATCH] fix(simple-editor): stop seed resurrection + add promote/demote + drag substeps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bugs reported on 2026-05-20: 1. RESURRECTION. User deletes a substep in the Simple Editor (e.g. Soak Clean (S-3) under Cleaner), then on the next -u fusion_plating the substep comes back. Root cause: the recipe XML lived in the manifest's `data` list with `noupdate="1"`. Odoo's noupdate=1 only blocks UPDATE of existing records — when a record's ir.model.data row is missing, the loader treats it as "not yet created" and re-creates from XML. Every upgrade resurrected every user-deleted seed node. Fix: pull the recipe XML files out of `data` and load them once via post_init_hook → _seed_starter_recipes_once. Sentinel checks ir.model.data for each recipe's root xmlid; if present, skip loading entirely. Result: deletions are permanent across all future upgrades. Existing entech recipes untouched. Files affected: fp_recipe_enp_alum_basic, fp_recipe_enp_steel_basic, fp_recipe_enp_sp, fp_recipe_general_processing, fp_recipe_anodize, fp_recipe_chem_conversion. 2. PROMOTE / DEMOTE. Simple Editor had no way to turn a substep into a top-level operation, or to tuck an operation under another as a substep. Authors had to delete + re-create. New endpoints: * /fp/simple_recipe/step/promote → flips node_type 'step' → 'operation', re-parents to the recipe (or sub-process) root, places right after the old parent operation. * /fp/simple_recipe/step/demote → flips 'operation' → 'step', re-parents under the preceding operation (or a caller-supplied target_op_id). Blocks demoting an operation that has its own children, with a helpful message. UI: each row in the editor now carries an up-arrow (promote, only shown on substeps) and a down-arrow (demote, only shown on operations). Confirmation dialog explains what's about to happen. 3. DRAG SUBSTEPS. Last commit (2142a66b) disabled drag on substep rows. Operators couldn't reorder substeps within an operation. Re-enabled drag on substeps. The step_reorder endpoint now groups incoming node_ids by parent_id and renumbers within each parent (10, 20, 30…). Cross-parent drag still no-ops on parent change — Promote/Demote buttons are the way to move between parents. Drive-by: - Added `from odoo import _` to the controller (missing import the new endpoints surfaced). - Edit-panel field wiring audited: all fields visible in the screen (Step name, Default instructions, Step Type, Triggers Workflow, Parallel Start, QA Sign-off, Collect measurements, Instruction Images, custom prompts) persist correctly through step_write or dedicated endpoints. No broken wires. Tests: 15 total in TestSimpleRecipeFlatten (was 10). 5 new cover promote happy-path, promote reject (non-substep), demote happy-path, demote block on has_children, and reorder parent-scoping. Module: fusion_plating 19.0.20.4.0 → 19.0.20.5.0. Co-Authored-By: Claude Opus 4.7 (1M context) --- fusion_plating/fusion_plating/__init__.py | 73 ++++++++++ fusion_plating/fusion_plating/__manifest__.py | 21 ++- .../controllers/simple_recipe_controller.py | 132 +++++++++++++++++- .../static/src/js/simple_recipe_editor.js | 55 ++++++++ .../static/src/scss/simple_recipe_editor.scss | 15 ++ .../static/src/xml/simple_recipe_editor.xml | 18 ++- .../tests/test_simple_recipe_flatten.py | 97 +++++++++++++ 7 files changed, 398 insertions(+), 13 deletions(-) diff --git a/fusion_plating/fusion_plating/__init__.py b/fusion_plating/fusion_plating/__init__.py index 3982819e..aaaee1d7 100644 --- a/fusion_plating/fusion_plating/__init__.py +++ b/fusion_plating/fusion_plating/__init__.py @@ -30,6 +30,79 @@ def post_init_hook(env): _backfill_contract_review_template(env) _seed_rack_tags_if_empty(env) _migrate_legacy_uom_columns(env) + _seed_starter_recipes_once(env) + + +def _seed_starter_recipes_once(env): + """Load starter recipe XML files on FIRST install only. + + Before 19.0.20.5.0 the recipe XML files (ENP-STEEL-BASIC, ENP-SP, + ENP-ALUM-BASIC, etc.) lived in the manifest's ``data`` list. With + ``noupdate="1"`` we expected user edits / deletions to survive + module upgrades — but Odoo only treats noupdate=1 as "don't update + existing records". If a record's ir.model.data row is deleted via + unlink, Odoo on the next ``-u`` sees the xmlid as missing and + RE-CREATES the record from XML. Bug reported 2026-05-20: every + time the user deleted a substep from a starter recipe, the next + upgrade brought it back. + + Fix: pull those files out of the manifest's data list, load them + here via convert_file ONCE per xmlid. Each file gets a sentinel + check (does the root recipe's xmlid exist in ir.model.data?); if + yes, skip. The hook is itself idempotent so it's safe to run on + every upgrade as well — but the sentinel ensures recipe content + is only seeded the very first time. + """ + from odoo.tools import convert + Module = env['ir.module.module'] + mod = Module.search([('name', '=', 'fusion_plating')], limit=1) + if not mod: + return + + # (xmlid_to_check, data_file_path) pairs. + # If the xmlid already exists in ir.model.data, the file is skipped. + sentinels = [ + ('fusion_plating.recipe_enp_alum_basic', + 'data/fp_recipe_enp_alum_basic.xml'), + ('fusion_plating.recipe_enp_steel_basic', + 'data/fp_recipe_enp_steel_basic.xml'), + ('fusion_plating.recipe_enp_sp', + 'data/fp_recipe_enp_sp.xml'), + ('fusion_plating.recipe_general_processing', + 'data/fp_recipe_general_processing.xml'), + ('fusion_plating.recipe_anodize', + 'data/fp_recipe_anodize.xml'), + ('fusion_plating.recipe_chem_conversion', + 'data/fp_recipe_chem_conversion.xml'), + ] + IMD = env['ir.model.data'] + for xmlid, filepath in sentinels: + module_name, name = xmlid.split('.', 1) + if IMD.search_count([('module', '=', module_name), ('name', '=', name)]): + # Recipe already in DB (either from a previous install, or + # already loaded by an earlier hook run). Don't touch — user + # may have made edits. + continue + # File not yet loaded for this DB. Run it once. + try: + with open_module_data_file(filepath) as fh: + convert.convert_file( + env, module_name, filepath, idref={}, mode='init', + noupdate=True, + ) + _logger.info('Seeded starter recipe %s', xmlid) + except FileNotFoundError: + _logger.warning('Starter recipe file %s not found, skipping', + filepath) + except Exception as exc: + _logger.warning('Could not seed %s: %s', xmlid, exc) + + +def open_module_data_file(relpath): + """Open a file relative to the fusion_plating module root.""" + import os + here = os.path.dirname(__file__) + return open(os.path.join(here, relpath), 'rb') def _resolve_kind_id(env, code): diff --git a/fusion_plating/fusion_plating/__manifest__.py b/fusion_plating/fusion_plating/__manifest__.py index 64238a81..7c8edac0 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.20.4.0', + 'version': '19.0.20.5.0', 'category': 'Manufacturing/Plating', 'summary': 'Core plating / metal finishing ERP: facilities, processes, tanks, baths, jobs, operators.', 'description': """ @@ -120,12 +120,19 @@ Copyright (c) 2026 Nexa Systems Inc. All rights reserved. 'views/fp_jobs_menu.xml', 'data/fp_work_role_data.xml', 'views/fp_work_role_views.xml', - 'data/fp_recipe_enp_alum_basic.xml', - 'data/fp_recipe_enp_steel_basic.xml', - 'data/fp_recipe_enp_sp.xml', - 'data/fp_recipe_general_processing.xml', - 'data/fp_recipe_anodize.xml', - 'data/fp_recipe_chem_conversion.xml', + # Starter recipes are NOT in 'data' on purpose. They get + # loaded once via post_init_hook → _seed_starter_recipes_once + # so user edits / deletions survive every -u upgrade. Putting + # them back here would re-create deleted nodes on every + # module upgrade (the noupdate="1" flag only blocks UPDATE, + # not CREATE-when-missing — Odoo treats a missing ir.model.data + # record as "needs creating"). + # 'data/fp_recipe_enp_alum_basic.xml', + # 'data/fp_recipe_enp_steel_basic.xml', + # 'data/fp_recipe_enp_sp.xml', + # 'data/fp_recipe_general_processing.xml', + # 'data/fp_recipe_anodize.xml', + # 'data/fp_recipe_chem_conversion.xml', 'data/fp_step_template_data.xml', ], 'post_init_hook': 'post_init_hook', diff --git a/fusion_plating/fusion_plating/controllers/simple_recipe_controller.py b/fusion_plating/fusion_plating/controllers/simple_recipe_controller.py index 8fcca193..8819bfb4 100644 --- a/fusion_plating/fusion_plating/controllers/simple_recipe_controller.py +++ b/fusion_plating/fusion_plating/controllers/simple_recipe_controller.py @@ -9,7 +9,7 @@ enforced by the underlying ACL on fp.step.template + process.node: operators get read; supervisors+ get write. """ -from odoo import http +from odoo import _, http from odoo.http import request @@ -667,11 +667,137 @@ class SimpleRecipeController(http.Controller): @http.route('/fp/simple_recipe/step/reorder', type='jsonrpc', auth='user') def step_reorder(self, node_ids): + """Renumber sequence within each parent group. + + Naive version (pre-19.0.20.5.0): renumber the entire flat list + 1..N regardless of parent. Broke when the flat list mixed + operations and substeps — siblings got out-of-order numbers + because the list interleaved them. + + New version: group node ids by their parent_id, then renumber + within each parent. Substeps stay sequenced under their + operation; operations stay sequenced under the recipe / sub- + process. Drop-across-parent shows up as a same-position no-op + — the UI's Promote/Demote buttons are the way to change + parents. + """ Node = request.env['fusion.plating.process.node'] - for i, nid in enumerate(node_ids, start=1): - Node.browse(nid).write({'sequence': i * 10}) + nodes = Node.browse([int(n) for n in node_ids]) + # Group by parent_id (preserve client-provided order within each). + from collections import OrderedDict + by_parent = OrderedDict() + for n in nodes: + by_parent.setdefault(n.parent_id.id, []).append(n) + for parent_id, siblings in by_parent.items(): + for i, n in enumerate(siblings, start=1): + target = i * 10 + if n.sequence != target: + n.sequence = target return {'ok': True} + @http.route('/fp/simple_recipe/step/promote', type='jsonrpc', auth='user') + def step_promote(self, node_id): + """Promote a substep (`step` node) to an operation under the + recipe root. + + Use case: author added a sub-step under an operation in the + Tree Editor, but actually wants it as a standalone operation + that the operator clocks separately. This call: + 1. Flips node_type 'step' → 'operation' + 2. Re-parents to the recipe root (or sub-process root if + the parent operation lives inside a sub_process) + 3. Places the new operation immediately after its old + parent (so it shows up in a sensible position in the + editor list) + """ + Node = request.env['fusion.plating.process.node'] + node = Node.browse(int(node_id)) + if not node.exists(): + return {'ok': False, 'error': 'not_found'} + node.check_access('write') + if node.node_type != 'step': + return {'ok': False, 'error': 'not_a_substep', + 'message': 'Only substeps can be promoted.'} + parent_op = node.parent_id + if not parent_op or parent_op.node_type != 'operation': + return {'ok': False, 'error': 'no_parent_op', + 'message': 'Substep has no operation parent to promote out of.'} + new_parent = parent_op.parent_id + if not new_parent or new_parent.node_type not in ('recipe', 'sub_process'): + return {'ok': False, 'error': 'no_grandparent', + 'message': 'Cannot find a recipe / sub-process to promote into.'} + # Place the new operation right after parent_op. + new_seq = parent_op.sequence + 1 + # Bump later siblings to make room (so we don't collide). + for sibling in new_parent.child_ids.filtered( + lambda s: s.sequence > parent_op.sequence and s.id != node.id + ): + sibling.sequence = sibling.sequence + 10 + node.write({ + 'node_type': 'operation', + 'parent_id': new_parent.id, + 'sequence': new_seq, + }) + return {'ok': True, 'new_parent_id': new_parent.id, + 'new_sequence': new_seq} + + @http.route('/fp/simple_recipe/step/demote', type='jsonrpc', auth='user') + def step_demote(self, node_id, target_op_id=False): + """Demote an operation to a substep under another operation. + + If ``target_op_id`` is provided, the node becomes a substep of + that operation. Otherwise it falls under the operation + immediately preceding it in the editor list (most common case + — author drops a header into the preceding section). + """ + Node = request.env['fusion.plating.process.node'] + node = Node.browse(int(node_id)) + if not node.exists(): + return {'ok': False, 'error': 'not_found'} + node.check_access('write') + if node.node_type != 'operation': + return {'ok': False, 'error': 'not_an_operation', + 'message': 'Only operations can be demoted to substeps.'} + # Substeps of operations don't recurse further — bail if this + # operation has its own step children (would lose them on demote). + if node.child_ids: + return {'ok': False, 'error': 'has_children', + 'message': ( + 'Operation "%s" has %d child step(s). Remove ' + 'or promote them first before demoting this ' + 'operation.' + ) % (node.name, len(node.child_ids))} + # Resolve target operation. + if target_op_id: + target = Node.browse(int(target_op_id)) + if not target.exists() or target.node_type != 'operation': + return {'ok': False, 'error': 'invalid_target', + 'message': 'Target must be an operation.'} + else: + # Find the preceding operation in the same parent. + parent = node.parent_id + if not parent: + return {'ok': False, 'error': 'no_parent'} + siblings = parent.child_ids.sorted('sequence') + before = [s for s in siblings if s.sequence < node.sequence + and s.node_type == 'operation'] + if not before: + return {'ok': False, 'error': 'no_preceding_op', + 'message': ( + 'There is no preceding operation to demote ' + 'into. Add one above this step first, or ' + 'pick an operation manually.' + )} + target = before[-1] + # Place the substep at the end of the target operation's children. + last_seq = max(target.child_ids.mapped('sequence') or [0]) + node.write({ + 'node_type': 'step', + 'parent_id': target.id, + 'sequence': last_seq + 10, + }) + return {'ok': True, 'new_parent_id': target.id} + # -------------------------------------------------------------- template @http.route('/fp/simple_recipe/template/list', type='jsonrpc', auth='user') def template_list(self): diff --git a/fusion_plating/fusion_plating/static/src/js/simple_recipe_editor.js b/fusion_plating/fusion_plating/static/src/js/simple_recipe_editor.js index 1e290f66..f8fd9b40 100644 --- a/fusion_plating/fusion_plating/static/src/js/simple_recipe_editor.js +++ b/fusion_plating/fusion_plating/static/src/js/simple_recipe_editor.js @@ -152,6 +152,61 @@ export class FpSimpleRecipeEditor extends Component { await this.loadAll(); } + // ---- Promote / demote ------------------------------------------------- + // + // Substep → operation: turn a child step into a top-level operation + // under the recipe root (or sub-process root if applicable). + // Operation → substep: tuck a top-level operation under the + // preceding operation as one of its substeps. Handy when the author + // realises a "header" should actually live as part of another + // operation's workflow. + + async onPromoteStep(stepId) { + const proceed = await this._confirm( + _t( + "Promote this substep to a top-level operation? It will be " + + "moved out of its parent operation and placed directly under " + + "the recipe." + ) + ); + if (!proceed) return; + const res = await rpc("/fp/simple_recipe/step/promote", { + node_id: stepId, + }); + if (!res.ok) { + this.notification.add( + res.message || _t("Could not promote step."), + { type: "warning" } + ); + return; + } + await this.loadAll(); + this.notification.add(_t("Step promoted to operation."), { type: "success" }); + } + + async onDemoteStep(stepId) { + const proceed = await this._confirm( + _t( + "Demote this operation to a substep under the previous " + + "operation? It will be tucked underneath the operation " + + "immediately above it in the list." + ) + ); + if (!proceed) return; + const res = await rpc("/fp/simple_recipe/step/demote", { + node_id: stepId, + }); + if (!res.ok) { + this.notification.add( + res.message || _t("Could not demote step."), + { type: "warning" } + ); + return; + } + await this.loadAll(); + this.notification.add(_t("Operation demoted to substep."), { type: "success" }); + } + async onAddInlineStep() { await rpc("/fp/simple_recipe/step/insert", { recipe_id: this._recipeId, diff --git a/fusion_plating/fusion_plating/static/src/scss/simple_recipe_editor.scss b/fusion_plating/fusion_plating/static/src/scss/simple_recipe_editor.scss index bf30de56..a2703a8d 100644 --- a/fusion_plating/fusion_plating/static/src/scss/simple_recipe_editor.scss +++ b/fusion_plating/fusion_plating/static/src/scss/simple_recipe_editor.scss @@ -255,6 +255,21 @@ $fp-se-drop: var(--fp-drop-bg, #{$_fp_se_drop_hex}); cursor: default; } } + .o_fp_step_promote, + .o_fp_step_demote { + background: none; + border: none; + color: $fp-se-muted; + padding: .2rem .4rem; + cursor: pointer; + font-size: .85rem; + border-radius: 4px; + transition: background .12s ease, color .12s ease; + &:hover { + background: $fp-se-page; + color: $fp-se-accent; + } + } .o_fp_step_edit, .o_fp_step_remove { background: none; diff --git a/fusion_plating/fusion_plating/static/src/xml/simple_recipe_editor.xml b/fusion_plating/fusion_plating/static/src/xml/simple_recipe_editor.xml index 9451a9cb..7657730c 100644 --- a/fusion_plating/fusion_plating/static/src/xml/simple_recipe_editor.xml +++ b/fusion_plating/fusion_plating/static/src/xml/simple_recipe_editor.xml @@ -69,11 +69,11 @@
⠿ - ↳ + ⠿ . @@ -107,6 +107,18 @@ + +