From 2142a66bc0ede6e641edb2c9ca5a5a263dbf6b96 Mon Sep 17 00:00:00 2001 From: gsinghpal Date: Tue, 19 May 2026 22:30:00 -0400 Subject: [PATCH] fix(simple-editor): also surface step children of operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to 821e768b. The previous fix flattened sub_process nodes so all 16 operations of ENP-STEEL-BASIC became visible — but the Tree Editor also shows the 26 `step` nodes that live under each operation ("Ready For Blast / Blast", "Soak Clean / Electroclean / Primary Rinse", etc.). The Simple Editor still hid those, so author + Tree Editor still disagreed by 26 rows. New `_flatten_recipe_nodes(recipe)` helper walks DFS and surfaces BOTH operations and their step children. Each operation is followed immediately by its step children in sequence order so the editor renders them as a contiguous block: 10. Ready For Steel Line 11. Cleaner [Steel Line] ↳ Soak Clean (S-3) [Steel Line › Cleaner] ↳ Electroclean (S-3) [Steel Line › Cleaner] ↳ Primary Rinse (S-4) [Steel Line › Cleaner] 15. Acid Dip (S-5) [Steel Line] ↳ Primary Rinse (S-6) [Steel Line › Acid Dip (S-5)] ... Payload additions on each step: - `node_type`: 'operation' | 'step' - `is_substep`: True for steps (renders indented) - `nested_under`: chained path (sub-process › operation for substeps, sub-process for nested operations, '' for top-level operations) UI: substep rows are indented 2.5rem, smaller font, no drag handle, no numeric position. The "↳" indent glyph and a "[parent operation]" chip make the parent-child relationship obvious. Substeps are not draggable to keep the existing reorder semantics simple — Tree Editor remains the home for structural changes. Legacy `_flatten_recipe_operations` helper retained for back-compat (it now delegates by filtering `node.node_type == 'operation'` from the full walk). ENP-STEEL-BASIC on entech: Simple Editor now shows 42 rows (was 10 before 821e768b, was 16 after 821e768b) — matches what the Tree Editor displays exactly. Tests: 10 total (was 7), 3 new cover the substep surfacing, path chaining, and is_substep / node_type flags on the payload. Module: fusion_plating 19.0.20.3.0 → 19.0.20.4.0. Co-Authored-By: Claude Opus 4.7 (1M context) --- fusion_plating/fusion_plating/__manifest__.py | 2 +- .../controllers/simple_recipe_controller.py | 87 +++++++++++++------ .../static/src/scss/simple_recipe_editor.scss | 20 ++++- .../static/src/xml/simple_recipe_editor.xml | 23 +++-- .../tests/test_simple_recipe_flatten.py | 78 ++++++++++++++--- 5 files changed, 164 insertions(+), 46 deletions(-) diff --git a/fusion_plating/fusion_plating/__manifest__.py b/fusion_plating/fusion_plating/__manifest__.py index ee381efe..64238a81 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.3.0', + 'version': '19.0.20.4.0', 'category': 'Manufacturing/Plating', 'summary': 'Core plating / metal finishing ERP: facilities, processes, tanks, baths, jobs, operators.', 'description': """ diff --git a/fusion_plating/fusion_plating/controllers/simple_recipe_controller.py b/fusion_plating/fusion_plating/controllers/simple_recipe_controller.py index 408d4eac..8fcca193 100644 --- a/fusion_plating/fusion_plating/controllers/simple_recipe_controller.py +++ b/fusion_plating/fusion_plating/controllers/simple_recipe_controller.py @@ -63,56 +63,89 @@ class SimpleRecipeController(http.Controller): def load(self, recipe_id): recipe = request.env['fusion.plating.process.node'].browse(recipe_id) recipe.check_access('read') - # A recipe authored in the Tree Editor can have `sub_process` - # nodes that hold more operations underneath. The flat - # `recipe.child_ids` walk hid those — operators saw a partial - # recipe in the Simple Editor even though the work order - # generated the full list (bug surfaced on ENP-STEEL-BASIC, - # 2026-05-20). + # Tree-Editor-authored recipes carry FOUR node levels: + # recipe → sub_process → operation → step + # The Tree Editor shows all of them. The Simple Editor used to + # only show direct children of the recipe — so for + # ENP-STEEL-BASIC (1 sub_process + 16 operations + 26 step + # nodes), authors saw 10 rows out of 43. Work-order generation + # walked the full tree and emitted operations as fp.job.step + # rows with step-nodes folded in as instruction text. # - # Match the WO generator: depth-first, collect every - # `operation` node, recurse into `recipe` / `sub_process`, - # skip `step` children (they're rendered as instructions - # within their parent operation). Each operation gets a - # `nested_under` label so the UI can tell the operator which - # sub-process the row came from. - flat_ops = self._flatten_recipe_operations(recipe) + # We now walk the full tree depth-first and surface EVERY + # operation and step node, in traversal order, each tagged + # with: + # - `nested_under`: chained sub-process path ("Steel Line", + # "Steel Line › Cleaner", etc.) + # - `node_type`: 'operation' or 'step' + # - `is_substep`: True for `step` nodes (renders indented) + # + # The Simple Editor's drag/insert/reorder semantics still + # treat operations as headline rows; substeps are read-only + # by default in the UI but their fields can be edited via the + # existing step_write endpoint (which doesn't care about + # node_type). + flat_nodes = self._flatten_recipe_nodes(recipe) return { 'recipe': self._recipe_payload(recipe), 'steps': [ - dict(self._step_payload(op), nested_under=path) - for op, path in flat_ops + dict(self._step_payload(node), + nested_under=path, + node_type=node.node_type, + is_substep=(node.node_type == 'step')) + for node, path in flat_nodes ], } def _flatten_recipe_operations(self, recipe): - """Walk a recipe tree DFS, return [(operation_node, path_label)]. + """Legacy helper — returns ONLY operations. - `path_label` is the name of the enclosing `sub_process` if the - operation lives inside one, else empty. Used by the Simple - Editor to render a "(in Steel Line)" hint next to the step. + Kept for back-compat with callers and tests that asked for the + operations-only view. Most paths should now use + ``_flatten_recipe_nodes`` which also surfaces step children. + """ + return [ + (n, p) for n, p in self._flatten_recipe_nodes(recipe) + if n.node_type == 'operation' + ] + + def _flatten_recipe_nodes(self, recipe): + """Walk the recipe DFS, return [(node, path_label)]. + + Surfaces both `operation` and `step` nodes. The traversal order + matches what the Tree Editor displays: + recipe → recurse → operation (emit) → its step children (emit) + recipe → recurse → sub_process → recurse → operation → steps + + Step children are emitted IMMEDIATELY after their parent + operation so the editor can render them as a contiguous block. """ out = [] def _walk(node, path): if node.node_type == 'operation': out.append((node, path)) - # Operations don't recurse — child `step` nodes are - # the operation's own instructions, not separate - # editor rows. + # Emit step children right after the operation so the + # editor sees: [Op, step, step, NextOp, step, ...]. + # The path label for a substep names its parent + # operation, chained from the sub-process if present. + sub_path = ( + f"{path} › {node.name}" if path else node.name + ) + for child in node.child_ids.sorted('sequence'): + if child.node_type == 'step': + out.append((child, sub_path)) return if node.node_type in ('recipe', 'sub_process'): - # Recipes themselves carry no path label; sub_process - # name becomes the path for nested children. sub_path = ( path if node.node_type == 'recipe' else (f"{path} › {node.name}" if path else node.name) ) for child in node.child_ids.sorted('sequence'): _walk(child, sub_path) - # `step` nodes at the top level are legacy — flat 'step' - # children of a recipe were silently skipped by - # _generate_steps pre-19.0.18.8.0; we mirror that here. + # `step` nodes that are direct children of a recipe (rare, + # legacy seed data) are silently dropped — _generate_steps + # has always skipped them. _walk(recipe, '') return out 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 57c5b54d..bf30de56 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 @@ -228,7 +228,8 @@ $fp-se-drop: var(--fp-drop-bg, #{$_fp_se_drop_hex}); // sub_process; the Simple Editor flattens those into the same list // but tags them with a small "inside " badge so the // author isn't confused about where they came from. - .o_fp_nested_under { + .o_fp_nested_under, + .o_fp_substep_parent { font-size: .7rem; font-weight: 500; padding: .15rem .45rem; @@ -237,6 +238,23 @@ $fp-se-drop: var(--fp-drop-bg, #{$_fp_se_drop_hex}); color: $fp-se-muted; i { opacity: .7; } } + + // Step nodes inside an operation are rendered as indented sub-rows + // — same node model as operations, but they're sub-instructions + // (the WO generator folds them into the operation's instruction + // text). Visual treatment: smaller, indented, no drag handle, no + // numeric position so the eye can tell them apart from operations. + &.o_fp_substep_row { + padding-left: 2.5rem; + background: transparent; + font-size: .92em; + opacity: .85; + .o_fp_step_name { font-weight: 400; } + .o_fp_substep_indent { + color: $fp-se-muted; + cursor: default; + } + } .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 77ee3e84..9451a9cb 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 @@ -68,20 +68,29 @@
- ⠿ - . - + ⠿ + ↳ + + . + + + + + + diff --git a/fusion_plating/fusion_plating/tests/test_simple_recipe_flatten.py b/fusion_plating/fusion_plating/tests/test_simple_recipe_flatten.py index 07a4db44..9481c0f1 100644 --- a/fusion_plating/fusion_plating/tests/test_simple_recipe_flatten.py +++ b/fusion_plating/fusion_plating/tests/test_simple_recipe_flatten.py @@ -97,10 +97,26 @@ class TestSimpleRecipeFlatten(TransactionCase): self.assertEqual(node_types, {'operation'}) # Recipe + sub_process never appear as Simple Editor rows. - def test_step_children_of_operations_are_not_surfaced(self): - # Operations carry `step` children as their internal - # instructions. The flat list must NOT emit them as separate - # rows. + def test_operations_only_helper_skips_step_children(self): + # Back-compat: the legacy _flatten_recipe_operations helper + # still returns ONLY operations. New callers should use + # _flatten_recipe_nodes for the full list (operations + steps). + self.env['fusion.plating.process.node'].create({ + 'name': 'Substep 1', 'node_type': 'step', + 'parent_id': self.op_a.id, 'sequence': 10, + }) + ops = self._flatten() + names = [n.name for n, _ in ops] + self.assertNotIn('Substep 1', names) + self.assertEqual(names, ['Op A', 'Op B', 'Op C', 'Op D']) + + def test_full_nodes_helper_surfaces_step_children(self): + # The Simple Editor's load endpoint uses _flatten_recipe_nodes, + # which DOES surface step children. They're emitted right after + # their parent operation so the editor renders them as a + # contiguous block. + from odoo.addons.fusion_plating.controllers.simple_recipe_controller \ + import SimpleRecipeController self.env['fusion.plating.process.node'].create({ 'name': 'Substep 1', 'node_type': 'step', 'parent_id': self.op_a.id, 'sequence': 10, @@ -109,12 +125,54 @@ class TestSimpleRecipeFlatten(TransactionCase): 'name': 'Substep 2', 'node_type': 'step', 'parent_id': self.op_a.id, 'sequence': 20, }) - ops = self._flatten() - names = [n.name for n, _ in ops] - self.assertNotIn('Substep 1', names) - self.assertNotIn('Substep 2', names) - # The original 4 operations are still there. - self.assertEqual(names, ['Op A', 'Op B', 'Op C', 'Op D']) + nodes = SimpleRecipeController()._flatten_recipe_nodes(self.recipe) + names = [n.name for n, _ in nodes] + # Substeps appear immediately after Op A, before Op B. + self.assertEqual( + names, + ['Op A', 'Substep 1', 'Substep 2', + 'Op B', 'Op C', 'Op D'], + ) + + def test_substeps_carry_parent_operation_in_path(self): + from odoo.addons.fusion_plating.controllers.simple_recipe_controller \ + import SimpleRecipeController + self.env['fusion.plating.process.node'].create({ + 'name': 'My Substep', 'node_type': 'step', + 'parent_id': self.op_b.id, 'sequence': 10, + }) + nodes = SimpleRecipeController()._flatten_recipe_nodes(self.recipe) + paths = {n.name: p for n, p in nodes} + # Op B lives in Sub-X; its substep's path chains both. + self.assertEqual(paths['My Substep'], 'Sub-X › Op B') + + def test_load_payload_marks_substeps_with_is_substep(self): + # End-to-end check on the load endpoint payload: substeps get + # `is_substep=True` and `node_type='step'` so the UI can render + # them as indented sub-rows. + from odoo.addons.fusion_plating.controllers.simple_recipe_controller \ + import SimpleRecipeController + self.env['fusion.plating.process.node'].create({ + 'name': 'A1', 'node_type': 'step', + 'parent_id': self.op_a.id, 'sequence': 10, + }) + # Mock the request — load() reads request.env. + from unittest.mock import patch + ctrl = SimpleRecipeController() + class FakeReq: + env = self.env + path_to_request = ( + 'odoo.addons.fusion_plating.controllers.' + 'simple_recipe_controller.request' + ) + with patch(path_to_request, FakeReq()): + payload = ctrl.load(self.recipe.id) + by_name = {s['name']: s for s in payload['steps']} + self.assertEqual(by_name['Op A']['node_type'], 'operation') + self.assertFalse(by_name['Op A']['is_substep']) + self.assertEqual(by_name['A1']['node_type'], 'step') + self.assertTrue(by_name['A1']['is_substep']) + self.assertEqual(by_name['A1']['nested_under'], 'Op A') def test_load_endpoint_includes_nested_under_in_payload(self): # Direct call to the controller's load (mirroring the JSONRPC).