From 6ae7f52fb5afb7a32d78d79650910b8e8549af54 Mon Sep 17 00:00:00 2001 From: Manh Nguyen Date: Fri, 28 Aug 2026 12:56:14 +0700 Subject: [PATCH 1/7] feat(cascade): cascade module terminal status onto work items Adds a second endpoint pair to cascade_ext, one level up from the issue cascade: GET/POST .../modules//cascade-preview|cascade-apply/ move a module's completed/cancelled status onto every live module member plus each member's full descendant subtree, in one transaction (one model_activity, cascaded issues get notification=False). A plain module PATCH {status} never cascades, from any client. No new model, migration, app, or touch-point edit. BEHAVIOR CHANGE to the shipped per-issue cascade: a work item already in a terminal group now PRUNES its whole subtree instead of being traversed through. Nothing beneath it is listed, walked, or changed - a live sub-item under a cancelled parent is left live where it used to be swept. This reverses plans/260822-cascade-complete-sub-items/ Decision 5 and was user-directed. The two pre-existing tests asserting the old rule were deliberately INVERTED (not weakened) and renamed ..._and_prunes_its_subtree. Apply rejects a posted id behind a pruned branch with under_terminal_ancestor rather than the false not_a_descendant. MAX_MODULE_CASCADE_ITEMS = 100 is a refusal, not a truncation: over the cap, preview returns over_cap:true with an EMPTY items array, and apply 400s having written nothing - including the module's own status. Verification (already run): django check clean, makemigrations no changes, 50 pytest green in cascade_ext. Co-Authored-By: Claude --- apps/api/plane/cascade_ext/service.py | 410 ++++++++- .../cascade_ext/tests/test_cascade_db.py | 109 ++- .../cascade_ext/tests/test_module_cascade.py | 852 ++++++++++++++++++ apps/api/plane/cascade_ext/urls.py | 19 +- apps/api/plane/cascade_ext/views.py | 126 ++- 5 files changed, 1474 insertions(+), 42 deletions(-) create mode 100644 apps/api/plane/cascade_ext/tests/test_module_cascade.py diff --git a/apps/api/plane/cascade_ext/service.py b/apps/api/plane/cascade_ext/service.py index 872dc36c3..cfb8d6dfe 100644 --- a/apps/api/plane/cascade_ext/service.py +++ b/apps/api/plane/cascade_ext/service.py @@ -6,6 +6,7 @@ # preview and apply endpoints (docs/FORK.md touch-point 2, no core view # edited). Contract, decisions, and test matrix: # plans/260822-cascade-complete-sub-items/phase-1-cascade-backend.md +# plans/260828-module-cascade-terminal-status/phase-1-module-cascade-backend.md from __future__ import annotations @@ -32,6 +33,33 @@ # Bulk-write batch size for the cascaded children (phase file § Implementation). _BULK_UPDATE_BATCH = 100 +# Hard cap on live nodes a single module cascade will write (plan M4). "Live" +# is post-pruning — terminal nodes and everything behind them are gone before +# the count is taken, so the cap measures exactly what a confirm would write. +MAX_MODULE_CASCADE_ITEMS = 100 + +# Module status -> the `State.group` that status cascades to (plan M7). A +# completed module completes its items; a cancelled module cancels them. +MODULE_STATUS_TO_STATE_GROUP = { + "completed": StateGroup.COMPLETED.value, + "cancelled": StateGroup.CANCELLED.value, +} + + +class CascadeCapExceeded(Exception): + """Raised when a module cascade would exceed MAX_MODULE_CASCADE_ITEMS. + + The view turns this into a 400 carrying the real live count (`total_live`). + Raising it BEFORE any transaction opens is what guarantees nothing — + including the module's own status — is written. + """ + + def __init__(self, total_live): + super().__init__( + f"cascade exceeds MAX_MODULE_CASCADE_ITEMS ({MAX_MODULE_CASCADE_ITEMS})" + ) + self.total_live = total_live + def resolve_target_group(old_state, new_state) -> str | None: """Which terminal group (if any) a parent's state change should cascade. @@ -53,11 +81,53 @@ def resolve_target_group(old_state, new_state) -> str | None: return new_state.group -def collect_descendants(*, root_issue, target_group: str, actor_id) -> dict: - """Level-order BFS over every live descendant of `root_issue`. +def _has_terminal_ancestor_to_root(*, issue_id, traversed_ids) -> bool: + """Is a posted-but-rejected id sitting under a pruned terminal branch? - Shared verbatim by preview and apply so the two can never disagree on - what is eligible (phase file § Implementation). Returns: + Phase 0 § 2: after pruning, a live id beneath a terminal node is in + neither the eligible set nor `traversed_ids`, and `not_a_descendant` + (or `not_in_module_tree`) would be a FALSE label — the id genuinely is + a descendant, it is just behind a branch the walk refused to follow. + + This helper walks the id's parent chain (bounded by MAX_DEPTH, the same + bound as the walk itself) and returns True only when it reaches a + terminal node the cascade itself ENCOUNTERED (recorded in + `traversed_ids`) — which is the proof that the branch connects to this + cascade's root rather than being some unrelated terminal ancestor. A + chain that ends without one (or a chain that leaves the tree entirely) + returns False, preserving `not_a_descendant` / `not_in_module_tree` for + genuinely foreign ids. Runs only for ids that were already going to be + rejected, so it costs nothing on the normal path. + """ + traversed_uuids = {uuid.UUID(x) for x in traversed_ids} + current = issue_id + for _ in range(MAX_DEPTH): + row = ( + Issue.issue_objects.filter(pk=current) + .values("parent_id", "state__group") + .first() + ) + if row is None or row["parent_id"] is None: + # The chain ended inside this cascade's subject (or left the tree). + return False + parent_id = row["parent_id"] + if parent_id in traversed_uuids: + # Parent is a terminal node this cascade actually encountered. + return True + current = parent_id + return False + + +def _collect_from_seeds( + *, seed_ids, include_seeds, target_group, actor_id +) -> dict: + """Level-order BFS from a SET of seed issue ids. + + Shared by the per-issue cascade (`include_seeds=False` — seeds are the + roots, excluded from the result) and the module cascade + (`include_seeds=True` — seeds are themselves candidates at depth 0). + + Returned key names are frozen for the per-issue caller: { "descendants": [ {id, identifier, name, depth, project_id, @@ -65,23 +135,34 @@ def collect_descendants(*, root_issue, target_group: str, actor_id) -> dict: state_group, target_state_id, eligible, reason}, ... ], "depth_capped": bool, - "traversed_ids": {str(id), ...} # every node visited except the - # root, INCLUDING already-terminal - # ones — used by apply() to tell - # "already terminal" apart from - # "never part of this tree". + "traversed_ids": {str(id), ...} # every TERMINAL node encountered + # (seeds included when + # include_seeds=True) — pruned + # from the result, never walked. } - - Already-terminal descendants (Decision 5) are excluded from - `descendants` but are still traversed through, so their own live - descendants remain reachable — and still recorded in `traversed_ids`. """ - visited = {root_issue.id} - frontier = [root_issue.id] + visited = set(seed_ids) + traversed_ids: set[str] = set() + raw_nodes = [] # [(Issue, depth), ...] — LIVE nodes only, level order depth = 0 - raw_nodes = [] # [(Issue, depth), ...] in level order depth_capped = False + if include_seeds: + # Seeds are fetched with the same soft-delete-aware manager and enter + # at depth 0. A terminal seed is recorded in `traversed_ids` and its + # subtree is pruned (plan M8 — a terminal item is a decision about + # that branch; reaching past it would override it silently). + for issue in Issue.issue_objects.filter(id__in=seed_ids).select_related( + "state", "project" + ): + if (issue.state.group if issue.state else None) in TERMINAL_GROUPS: + traversed_ids.add(str(issue.id)) + else: + raw_nodes.append((issue, 0)) + frontier = [issue.id for issue, _ in raw_nodes] + else: + frontier = list(visited) + while frontier and depth < MAX_DEPTH: depth += 1 # issue_objects (not the plain `objects`/default manager) so @@ -97,30 +178,27 @@ def collect_descendants(*, root_issue, target_group: str, actor_id) -> dict: next_frontier = [] for child in children: visited.add(child.id) - next_frontier.append(child.id) - raw_nodes.append((child, depth)) + # Prune-at-terminal (Phase 0, 2026-08-28): a child already in a + # terminal group is recorded in `traversed_ids` but its own + # children are never enqueued. Reaching past a terminal node + # would silently override the decision that branch is settled. + if (child.state.group if child.state else None) in TERMINAL_GROUPS: + traversed_ids.add(str(child.id)) + else: + next_frontier.append(child.id) + raw_nodes.append((child, depth)) + frontier = next_frontier if frontier and depth >= MAX_DEPTH: depth_capped = True logger.warning( - "cascade_ext: MAX_DEPTH (%s) cap hit walking descendants of issue %s", + "cascade_ext: MAX_DEPTH (%s) cap hit walking seed set of size %s", MAX_DEPTH, - root_issue.id, + len(seed_ids), ) - traversed_ids = {str(child.id) for child, _ in raw_nodes} - - # Terminal nodes (either group — regardless of `target_group`) are - # excluded from the result but were already pushed to `frontier` above, - # so their own live descendants were still traversed. - live_nodes = [ - (child, node_depth) - for child, node_depth in raw_nodes - if (child.state.group if child.state else None) not in TERMINAL_GROUPS - ] - - project_ids = {child.project_id for child, _ in live_nodes} + project_ids = {child.project_id for child, _ in raw_nodes} # One query resolves every touched project's target state at once # (phase file § Implementation step 5). `State.default` is NOT usable — @@ -142,7 +220,7 @@ def collect_descendants(*, root_issue, target_group: str, actor_id) -> dict: ) descendants = [] - for child, node_depth in live_nodes: + for child, node_depth in raw_nodes: state = child.state target_state = target_states.get(child.project_id) is_member = child.project_id in member_project_ids @@ -180,6 +258,262 @@ def collect_descendants(*, root_issue, target_group: str, actor_id) -> dict: } +def collect_descendants(*, root_issue, target_group: str, actor_id) -> dict: + """Level-order BFS over every live descendant of `root_issue`. + + Shared verbatim by preview and apply so the two can never disagree on + what is eligible (phase file § Implementation). Returns: + + { + "descendants": [ {id, identifier, name, depth, project_id, + project_name, state_id, state_name, + state_group, target_state_id, eligible, + reason}, ... ], + "depth_capped": bool, + "traversed_ids": {str(id), ...} + } + + As of Phase 0 (2026-08-28) the walk PRUNES at terminal nodes: an + already-terminal descendant is excluded from `descendants`, is NOT + traversed through (its own live descendants are consequently not listed, + not walked, and not changed), and is still recorded in `traversed_ids` + so apply() can tell "already terminal" apart from "never part of this + tree". + """ + return _collect_from_seeds( + seed_ids={root_issue.id}, + include_seeds=False, + target_group=target_group, + actor_id=actor_id, + ) + + +def _collect_module_cascade(*, module, target_group, actor_id) -> dict: + """Internal module-cascade shape shared by preview and apply. + + The wire `collect_module_cascade` strips `traversed_ids`; `apply` reads + it here so both callers cannot disagree on what is eligible. + """ + seed_ids = { + issue_id + for issue_id in Issue.issue_objects.filter( + issue_module__module_id=module.id, + issue_module__deleted_at__isnull=True, + ).values_list("id", flat=True) + } + + if not seed_ids: + return { + "target_group": target_group, + "depth_capped": False, + "over_cap": False, + "cap": MAX_MODULE_CASCADE_ITEMS, + "summary": { + "total_live": 0, + "eligible": 0, + "ineligible": 0, + "already_terminal": 0, + }, + "items": [], + "traversed_ids": set(), + } + + collected = _collect_from_seeds( + seed_ids=seed_ids, + include_seeds=True, + target_group=target_group, + actor_id=actor_id, + ) + + rows = collected["descendants"] + seed_str_ids = {str(i) for i in seed_ids} + row_ids = {row["id"] for row in rows} + + items = [] + for node in rows: + item = dict(node) + # A descendant may *also* be a module member — emitted explicitly + # rather than inferred from `depth == 0` (Phase 1 § Endpoint contract). + item["is_module_member"] = node["id"] in seed_str_ids + items.append(item) + + # `already_terminal` counts the terminal nodes ACTUALLY encountered (plan + # M8 / Phase 1 § Implementation step 3) — seeds included, descendants + # included, and NOT a subtraction from a traversal total, which under + # pruning would silently report zero for a branch nobody walked. Every + # terminal node the walk touched is in `traversed_ids`; live seeds and + # live descendants are both in `rows`. A terminal SEED is also a member + # hence `seed_str_ids - row_ids` is exactly the terminal seeds. + already_terminal = len( + (collected["traversed_ids"] - row_ids) | (seed_str_ids - row_ids) + ) + + eligible = sum(1 for node in items if node["eligible"]) + ineligible = len(items) - eligible + + summary = { + "total_live": len(items), + "eligible": eligible, + "ineligible": ineligible, + "already_terminal": already_terminal, + } + + over_cap = summary["total_live"] > MAX_MODULE_CASCADE_ITEMS + if over_cap: + # Do NOT truncate — a truncated list silently under-reports what a + # confirm would write, which is the failure the cap exists to avoid. + items = [] + + return { + "target_group": target_group, + "depth_capped": collected["depth_capped"], + "over_cap": over_cap, + "cap": MAX_MODULE_CASCADE_ITEMS, + "summary": summary, + "items": items, + "traversed_ids": collected["traversed_ids"], + } + + +def collect_module_cascade(*, module, target_group, actor_id) -> dict: + """Preview-shape for cascading a module's terminal status onto its issues. + + Seed set = every live module member via the canonical soft-delete-aware + query (`Issue.issue_objects` + non-deleted `ModuleIssue` rows — never + `module.issue_module.all()`), then the shared BFS with the seeds + themselves as depth-0 candidates. Everything beneath a terminal member is + pruned (Phase 0's rule, shared with the issue cascade). + + Returns the wire shape for GET .../cascade-preview/ (no `traversed_ids`): + target_group, depth_capped, over_cap, cap, items, summary. + """ + collected = _collect_module_cascade( + module=module, target_group=target_group, actor_id=actor_id + ) + collected.pop("traversed_ids", None) + return collected + + +def apply_module_cascade( + *, module, status, item_ids, actor_id, slug, origin +) -> dict: + """Apply a module's new `status` plus a caller-selected subset of its + currently-eligible issues, atomically (plan M5). + + `item_ids` is a REQUEST, never an authorization: + `item_ids=None` -> every currently-eligible item (headless/MCP callers). + `item_ids=[]` -> nothing cascades; only the module's status moves. + + Raises CascadeCapExceeded BEFORE opening any transaction when the live + count exceeds MAX_MODULE_CASCADE_ITEMS, so nothing — the module's own + status included — is written. + """ + target_group = MODULE_STATUS_TO_STATE_GROUP[status] + + collected = _collect_module_cascade( + module=module, target_group=target_group, actor_id=actor_id + ) + if collected["over_cap"]: + raise CascadeCapExceeded(collected["summary"]["total_live"]) + + by_id = {node["id"]: node for node in collected["items"]} + eligible_ids = {cid for cid, node in by_id.items() if node["eligible"]} + traversed_ids = collected["traversed_ids"] + + if item_ids is None: + requested_ids = set(eligible_ids) + else: + requested_ids = {str(cid) for cid in item_ids} + + accepted_ids = requested_ids & eligible_ids + + rejected = [] + for cid in sorted(requested_ids - accepted_ids): + node = by_id.get(cid) + if node is not None: + reason = node["reason"] or "not_eligible" + elif cid in traversed_ids: + # A terminal node the walk encountered but never emitted. + reason = "already_terminal" + elif _has_terminal_ancestor_to_root( + issue_id=cid, traversed_ids=traversed_ids + ): + # Phase 0's reason, reused verbatim: a live id the walk refused + # to reach because a terminal node prunes its branch. + reason = "under_terminal_ancestor" + else: + reason = "not_in_module_tree" + rejected.append({"id": cid, "reason": reason}) + + accepted_nodes = [by_id[cid] for cid in accepted_ids] + + old_status = module.status + now = timezone.now() + + # Module write + every cascaded issue write share ONE transaction (M5) — + # a mid-way failure rolls the module's status change back too. + with transaction.atomic(): + module.status = status + module.updated_at = now + module.save(update_fields=["status", "updated_at"]) + + if accepted_nodes: + issues = list(Issue.issue_objects.filter(id__in=list(accepted_ids))) + for issue in issues: + issue.state_id = by_id[str(issue.id)]["target_state_id"] + issue.updated_at = now + + for start in range(0, len(issues), _BULK_UPDATE_BATCH): + batch = issues[start : start + _BULK_UPDATE_BATCH] + # bulk_update skips save(), so updated_at is set explicitly above. + Issue.issue_objects.bulk_update(batch, ["state_id", "updated_at"]) + + # Dispatched only once the atomic block above has committed — a + # mid-transaction failure never reaches here. + model_activity.delay( + model_name="module", + model_id=str(module.id), + requested_data={"status": status}, + current_instance=json.dumps({"status": old_status}), + actor_id=actor_id, + slug=slug, + origin=origin, + ) + + epoch = int(now.timestamp()) + for node in accepted_nodes: + issue_activity.delay( + type="issue.activity.updated", + requested_data=json.dumps({"state_id": node["target_state_id"]}), + actor_id=str(actor_id), + issue_id=node["id"], + project_id=node["project_id"], + current_instance=json.dumps({"state_id": node["state_id"]}), + epoch=epoch, + # notification=False is load-bearing (M11), not a typo — only the + # module's own change fires; a 200-item cascade must not fire 200 + # watcher notifications. + notification=False, + origin=origin, + ) + model_activity.delay( + model_name="issue", + model_id=node["id"], + requested_data={"state": node["target_state_id"]}, + current_instance=json.dumps({"state": node["state_id"]}), + actor_id=actor_id, + slug=slug, + origin=origin, + ) + + return { + "module": str(module.id), + "status": status, + "updated": sorted(accepted_ids), + "rejected": rejected, + } + + def apply_cascade(*, root_issue, state, child_ids, actor_id, slug, origin) -> dict: """Apply the parent's new `state` plus a caller-selected subset of currently-eligible descendants, atomically. @@ -215,13 +549,19 @@ def apply_cascade(*, root_issue, state, child_ids, actor_id, slug, origin) -> di accepted_ids = requested_ids & eligible_ids + traversed_ids = collected["traversed_ids"] rejected = [] for cid in sorted(requested_ids - accepted_ids): node = by_id.get(cid) if node is not None: reason = node["reason"] or "not_eligible" - elif cid in collected["traversed_ids"]: + elif cid in traversed_ids: reason = "already_terminal" + elif _has_terminal_ancestor_to_root( + issue_id=cid, traversed_ids=traversed_ids + ): + # Phase 0: a live descendant beneath a pruned terminal branch. + reason = "under_terminal_ancestor" else: reason = "not_a_descendant" rejected.append({"id": cid, "reason": reason}) @@ -309,4 +649,4 @@ def apply_cascade(*, root_issue, state, child_ids, actor_id, slug, origin) -> di "parent": str(root_issue.id), "updated": sorted(accepted_ids), "rejected": rejected, - } + } \ No newline at end of file diff --git a/apps/api/plane/cascade_ext/tests/test_cascade_db.py b/apps/api/plane/cascade_ext/tests/test_cascade_db.py index 751be37af..4f26f8a7a 100644 --- a/apps/api/plane/cascade_ext/tests/test_cascade_db.py +++ b/apps/api/plane/cascade_ext/tests/test_cascade_db.py @@ -190,7 +190,13 @@ def test_same_tree_target_cancelled_resolves_cancelled_state(self): self.assertEqual(result["descendants"][0]["target_state_id"], str(st_cancelled.id)) - def test_cancelled_child_excluded_but_its_own_children_still_listed(self): + def test_cancelled_child_excluded_and_prunes_its_subtree(self): + # INVERTED 2026-08-28 by plans/260828-module-cascade-terminal-status + # Phase 0: a terminal node now PRUNES its subtree — the grandchild is + # NOT listed. This is a deliberate, user-directed reversal of the + # shipped rule (260822 Decision 5, "still traversed through"); do not + # "fix" it back. A live sub-item under a cancelled parent is now left + # live where it used to be swept. ws, proj, user = self._setup() st_started = _state(ws, proj, "started") st_cancelled = _state(ws, proj, "cancelled") @@ -203,10 +209,12 @@ def test_cancelled_child_excluded_but_its_own_children_still_listed(self): ids = {d["id"] for d in result["descendants"]} self.assertNotIn(str(cancelled_child.id), ids) - self.assertIn(str(grandchild.id), ids) + self.assertNotIn(str(grandchild.id), ids) self.assertIn(str(cancelled_child.id), result["traversed_ids"]) - def test_completed_child_excluded_mirrored_for_cancel_target(self): + def test_completed_child_excluded_mirrored_for_cancel_target_and_prunes_its_subtree(self): + # INVERTED 2026-08-28 by plans/260828-module-cascade-terminal-status + # Phase 0 — see the cancelled-child mirror above for the full note. ws, proj, user = self._setup() st_started = _state(ws, proj, "started") st_completed = _state(ws, proj, "completed") @@ -219,6 +227,66 @@ def test_completed_child_excluded_mirrored_for_cancel_target(self): ids = {d["id"] for d in result["descendants"]} self.assertNotIn(str(completed_child.id), ids) + self.assertNotIn(str(grandchild.id), ids) + + # Phase 0 (plans/260828-module-cascade-terminal-status, 2026-08-28) — + # prune-at-terminal coverage beyond the two inversions above. + + def test_terminal_node_prunes_a_two_level_live_subtree(self): + # Case A: nothing beneath a terminal node is listed OR visited. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + st_completed = _state(ws, proj, "completed") + root = _issue(ws, proj, user, state=st_started) + terminal_child = _issue(ws, proj, user, state=st_completed, parent=root) + grandchild = _issue(ws, proj, user, state=st_started, parent=terminal_child) + great_grandchild = _issue(ws, proj, user, state=st_started, parent=grandchild) + + result = collect_descendants(root_issue=root, target_group="completed", actor_id=user.id) + ids = {d["id"] for d in result["descendants"]} + + self.assertNotIn(str(terminal_child.id), ids) + self.assertNotIn(str(grandchild.id), ids) + self.assertNotIn(str(great_grandchild.id), ids) + # traversed_ids holds the terminal node itself and NOTHING below it — + # nothing beneath a terminal node is even visited. + self.assertIn(str(terminal_child.id), result["traversed_ids"]) + self.assertNotIn(str(grandchild.id), result["traversed_ids"]) + self.assertNotIn(str(great_grandchild.id), result["traversed_ids"]) + + def test_pruning_applies_at_any_depth_not_just_level_one(self): + # Case B: live child -> terminal grandchild -> live great-grandchild. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + st_completed = _state(ws, proj, "completed") + root = _issue(ws, proj, user, state=st_started) + child = _issue(ws, proj, user, state=st_started, parent=root) + terminal_grandchild = _issue(ws, proj, user, state=st_completed, parent=child) + great_grandchild = _issue(ws, proj, user, state=st_started, parent=terminal_grandchild) + + result = collect_descendants(root_issue=root, target_group="completed", actor_id=user.id) + ids = {d["id"] for d in result["descendants"]} + + self.assertIn(str(child.id), ids) + self.assertNotIn(str(terminal_grandchild.id), ids) + self.assertNotIn(str(great_grandchild.id), ids) + self.assertIn(str(terminal_grandchild.id), result["traversed_ids"]) + + def test_stateless_child_is_not_terminal_and_keeps_being_walked(self): + # Case C: `child.state is None` is NOT terminal (its group reads as + # None, which is not in TERMINAL_GROUPS) — both it and its live + # grandchild stay listed. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + _state(ws, proj, "completed") + root = _issue(ws, proj, user, state=st_started) + stateless_child = _issue(ws, proj, user, state=None, parent=root) + grandchild = _issue(ws, proj, user, state=st_started, parent=stateless_child) + + result = collect_descendants(root_issue=root, target_group="completed", actor_id=user.id) + ids = {d["id"] for d in result["descendants"]} + + self.assertIn(str(stateless_child.id), ids) self.assertIn(str(grandchild.id), ids) def test_cross_project_child_resolves_its_own_project_state(self): @@ -390,6 +458,41 @@ def test_posted_id_not_a_descendant_at_all_is_rejected( stranger.refresh_from_db() self.assertNotEqual(stranger.state_id, st_completed.id) + @mock.patch("plane.cascade_ext.service.model_activity") + @mock.patch("plane.cascade_ext.service.issue_activity") + def test_posted_id_beneath_a_terminal_ancestor_is_rejected_and_not_written( + self, mock_issue_activity, mock_model_activity + ): + # Phase 0 case D (2026-08-28): a live grandchild under a terminal + # child is genuinely a descendant but sits behind a pruned branch, so + # the rejection reason is `under_terminal_ancestor` — NOT + # `not_a_descendant`, which would falsely deny tree membership. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + st_completed = _state(ws, proj, "completed") + st_cancelled = _state(ws, proj, "cancelled") + root = _issue(ws, proj, user, state=st_started) + c1 = _issue(ws, proj, user, state=st_started, parent=root) + terminal_child = _issue(ws, proj, user, state=st_cancelled, parent=root) + hidden_grandchild = _issue(ws, proj, user, state=st_started, parent=terminal_child) + + result = apply_cascade( + root_issue=root, + state=st_completed, + child_ids=[str(c1.id), str(hidden_grandchild.id)], + actor_id=user.id, + slug=ws.slug, + origin="", + ) + + self.assertEqual(result["updated"], [str(c1.id)]) + self.assertEqual(len(result["rejected"]), 1) + self.assertEqual(result["rejected"][0]["id"], str(hidden_grandchild.id)) + self.assertEqual(result["rejected"][0]["reason"], "under_terminal_ancestor") + + hidden_grandchild.refresh_from_db() + self.assertEqual(hidden_grandchild.state_id, st_started.id) # NOT written + @mock.patch("plane.cascade_ext.service.model_activity") @mock.patch("plane.cascade_ext.service.issue_activity") def test_child_ids_omitted_moves_every_eligible_descendant( diff --git a/apps/api/plane/cascade_ext/tests/test_module_cascade.py b/apps/api/plane/cascade_ext/tests/test_module_cascade.py new file mode 100644 index 000000000..b957c0b87 --- /dev/null +++ b/apps/api/plane/cascade_ext/tests/test_module_cascade.py @@ -0,0 +1,852 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. +# +# DB integration tests (real Postgres) for cascade_ext's MODULE cascade: +# collect_module_cascade (preview shape) and apply_module_cascade (the +# atomic module-status + issue-states write), plus the two HTTP endpoints. +# Mirrors test_cascade_db.py's style: TransactionTestCase + explicit ORM +# rows, no mocking the unit under test — except the dispatched Celery tasks, +# where mocking is the only way to observe the notification kwarg. +# +# Test matrix: plans/260828-module-cascade-terminal-status/phase-1-module-cascade-backend.md + +import uuid +from unittest import mock + +from django.test import TransactionTestCase +from django.utils import timezone +from rest_framework.test import APIClient + +# Run Celery tasks inline (no broker in tests) for any test that does NOT +# explicitly mock issue_activity/model_activity itself. +try: # pragma: no cover + from plane.celery import app as _celery_app + + _celery_app.conf.task_always_eager = True + _celery_app.conf.task_eager_propagates = False +except Exception: # pragma: no cover + pass + +from plane.cascade_ext.service import ( + MAX_MODULE_CASCADE_ITEMS, + CascadeCapExceeded, + apply_module_cascade, + collect_module_cascade, +) +from plane.db.models import Issue + + +# --------------------------------------------------------------------------- +# Shared helpers (mirror test_cascade_db.py so the files stay in sync) +# --------------------------------------------------------------------------- + +def _ws(slug=None): + from plane.db.models import Workspace + + slug = slug or f"ws-{uuid.uuid4().hex[:8]}" + owner = _user() + return Workspace.objects.create(name=slug, slug=slug, logo="", owner=owner) + + +def _user(email=None, is_bot=False): + from plane.db.models import User + + uid = uuid.uuid4().hex[:8] + email = email or f"u-{uid}@test.invalid" + return User.objects.create_user( + username=f"user_{uid}", email=email, password="x", is_bot=is_bot + ) + + +def _project(ws): + from plane.db.models import Project + + return Project.objects.create( + workspace=ws, + name=f"p-{uuid.uuid4().hex[:6]}", + identifier=uuid.uuid4().hex[:5].upper(), + ) + + +def _pmember(ws, proj, user, role=15, is_active=True): + from plane.db.models import ProjectMember + + return ProjectMember.objects.create( + workspace=ws, project=proj, member=user, role=role, is_active=is_active + ) + + +def _state(ws, proj, group="started", name=None): + from plane.db.models import State + + return State.objects.create( + workspace=ws, + project=proj, + name=name or f"{group}-{uuid.uuid4().hex[:4]}", + color="#fff", + group=group, + ) + + +def _issue(ws, proj, created_by, state=None, parent=None): + return Issue.objects.create( + workspace=ws, + project=proj, + name=f"i-{uuid.uuid4().hex[:6]}", + created_by=created_by, + state=state, + parent=parent, + sequence_id=1, + ) + + +def _module(ws, proj, status="planned", archived=False): + from plane.db.models import Module + + return Module.objects.create( + workspace=ws, + project=proj, + name=f"m-{uuid.uuid4().hex[:6]}", + status=status, + archived_at=timezone.now() if archived else None, + ) + + +def _module_issue(module, issue): + from plane.db.models import ModuleIssue + + return ModuleIssue.objects.create( + workspace=module.workspace, project=module.project, module=module, issue=issue + ) + + +# --------------------------------------------------------------------------- +# collect_module_cascade — preview shape +# --------------------------------------------------------------------------- + +class TestCollectModuleCascade(TransactionTestCase): + def _setup(self): + ws = _ws() + proj = _project(ws) + user = _user() + _pmember(ws, proj, user) + return ws, proj, user + + def test_empty_module_returns_zero_summary_without_running_the_bfs(self): + # Case 1: no seeds -> the zero-summary shape immediately; the ONLY + # query is the seed lookup itself (zero BFS queries). + ws, proj, user = self._setup() + module = _module(ws, proj) + + with self.assertNumQueries(1): + result = collect_module_cascade( + module=module, target_group="completed", actor_id=user.id + ) + + self.assertEqual(result["summary"]["total_live"], 0) + self.assertEqual(result["items"], []) + self.assertFalse(result["over_cap"]) + self.assertFalse(result["depth_capped"]) + self.assertEqual(result["cap"], MAX_MODULE_CASCADE_ITEMS) + + def test_flat_module_lists_only_non_terminal_members_at_depth_zero(self): + # Case 2: mixed states — terminal members are excluded (and counted), + # live members are listed at depth 0 with is_module_member true. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + st_backlog = _state(ws, proj, "backlog") + st_completed = _state(ws, proj, "completed") + module = _module(ws, proj) + a = _issue(ws, proj, user, state=st_started) + b = _issue(ws, proj, user, state=st_backlog) + done = _issue(ws, proj, user, state=st_completed) + for issue in (a, b, done): + _module_issue(module, issue) + + result = collect_module_cascade( + module=module, target_group="completed", actor_id=user.id + ) + by_id = {item["id"]: item for item in result["items"]} + + self.assertEqual(set(by_id.keys()), {str(a.id), str(b.id)}) + for item in by_id.values(): + self.assertEqual(item["depth"], 0) + self.assertTrue(item["is_module_member"]) + self.assertTrue(item["eligible"]) + self.assertEqual(result["summary"]["total_live"], 2) + self.assertEqual(result["summary"]["eligible"], 2) + self.assertEqual(result["summary"]["already_terminal"], 1) + + def test_member_with_subtree_lists_every_level(self): + # Case 3: member -> child -> grandchild => depth 0/1/2, and + # is_module_member is true ONLY for the seed. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + st_completed = _state(ws, proj, "completed") + module = _module(ws, proj) + member = _issue(ws, proj, user, state=st_started) + child = _issue(ws, proj, user, state=st_started, parent=member) + grandchild = _issue(ws, proj, user, state=st_started, parent=child) + _module_issue(module, member) + + result = collect_module_cascade( + module=module, target_group="completed", actor_id=user.id + ) + by_id = {item["id"]: item for item in result["items"]} + + self.assertEqual( + set(by_id.keys()), {str(member.id), str(child.id), str(grandchild.id)} + ) + self.assertEqual(by_id[str(member.id)]["depth"], 0) + self.assertEqual(by_id[str(child.id)]["depth"], 1) + self.assertEqual(by_id[str(grandchild.id)]["depth"], 2) + self.assertTrue(by_id[str(member.id)]["is_module_member"]) + self.assertFalse(by_id[str(child.id)]["is_module_member"]) + self.assertFalse(by_id[str(grandchild.id)]["is_module_member"]) + for item in by_id.values(): + self.assertEqual(item["target_state_id"], str(st_completed.id)) + + def test_terminal_member_prunes_its_subtree(self): + # Case 4: a completed member is excluded AND its live child is absent + # (Phase 0's prune rule, shared with the issue cascade). + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + st_completed = _state(ws, proj, "completed") + module = _module(ws, proj) + done_member = _issue(ws, proj, user, state=st_completed) + live_child = _issue(ws, proj, user, state=st_started, parent=done_member) + _module_issue(module, done_member) + + result = collect_module_cascade( + module=module, target_group="completed", actor_id=user.id + ) + ids = {item["id"] for item in result["items"]} + + self.assertNotIn(str(done_member.id), ids) + self.assertNotIn(str(live_child.id), ids) + self.assertEqual(result["summary"]["already_terminal"], 1) + + def test_cross_project_subitem_resolves_its_own_projects_state(self): + # Case 6. + ws, proj, user = self._setup() + other_proj = _project(ws) + _pmember(ws, other_proj, user) + st_started = _state(ws, proj, "started") + _state(ws, proj, "completed") + other_completed = _state(ws, other_proj, "completed") + module = _module(ws, proj) + member = _issue(ws, proj, user, state=st_started) + cross_child = _issue( + ws, other_proj, user, state=_state(ws, other_proj, "started"), parent=member + ) + _module_issue(module, member) + + result = collect_module_cascade( + module=module, target_group="completed", actor_id=user.id + ) + by_id = {item["id"]: item for item in result["items"]} + + self.assertEqual(by_id[str(cross_child.id)]["project_id"], str(other_proj.id)) + self.assertEqual( + by_id[str(cross_child.id)]["target_state_id"], str(other_completed.id) + ) + + def test_renamed_target_state_still_resolved_by_group_not_name(self): + # Case 7. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + st_shipped = _state(ws, proj, "completed", name="Shipped") # renamed from "Done" + module = _module(ws, proj) + member = _issue(ws, proj, user, state=st_started) + _module_issue(module, member) + + result = collect_module_cascade( + module=module, target_group="completed", actor_id=user.id + ) + + self.assertEqual(result["items"][0]["target_state_id"], str(st_shipped.id)) + + def test_subitem_in_project_actor_is_not_member_of_is_listed_ineligible(self): + # Case 8. + ws, proj, user = self._setup() + other_proj = _project(ws) + _state(ws, other_proj, "completed") + # deliberately no _pmember(ws, other_proj, user) + st_started = _state(ws, proj, "started") + _state(ws, proj, "completed") + module = _module(ws, proj) + member = _issue(ws, proj, user, state=st_started) + cross_child = _issue( + ws, other_proj, user, state=_state(ws, other_proj, "started"), parent=member + ) + _module_issue(module, member) + + result = collect_module_cascade( + module=module, target_group="completed", actor_id=user.id + ) + by_id = {item["id"]: item for item in result["items"]} + node = by_id[str(cross_child.id)] + + self.assertFalse(node["eligible"]) + self.assertEqual(node["reason"], "no_permission") + self.assertEqual(result["summary"]["ineligible"], 1) + + def test_project_with_no_state_in_target_group_is_no_matching_state(self): + # Case 9. + ws, proj, user = self._setup() + other_proj = _project(ws) + _pmember(ws, other_proj, user) + # other_proj has NO "completed" state at all. + st_started = _state(ws, proj, "started") + _state(ws, proj, "completed") + module = _module(ws, proj) + member = _issue(ws, proj, user, state=st_started) + cross_child = _issue( + ws, other_proj, user, state=_state(ws, other_proj, "started"), parent=member + ) + _module_issue(module, member) + + result = collect_module_cascade( + module=module, target_group="completed", actor_id=user.id + ) + by_id = {item["id"]: item for item in result["items"]} + + self.assertFalse(by_id[str(cross_child.id)]["eligible"]) + self.assertEqual(by_id[str(cross_child.id)]["reason"], "no_matching_state") + + def test_parent_cycle_among_module_members_terminates_without_duplicates(self): + # Case 10: both seeds are in `visited` before the walk starts, so the + # cycle a <-> b is never followed. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + _state(ws, proj, "completed") + module = _module(ws, proj) + a = _issue(ws, proj, user, state=st_started) + b = _issue(ws, proj, user, state=st_started, parent=a) + a.parent = b + a.save(update_fields=["parent"]) + _module_issue(module, a) + _module_issue(module, b) + + result = collect_module_cascade( + module=module, target_group="completed", actor_id=user.id + ) + ids = [item["id"] for item in result["items"]] + + self.assertEqual(sorted(ids), sorted([str(a.id), str(b.id)])) # no dupes + + def test_over_cap_preview_reports_real_total_and_empties_items(self): + # Case 16 (preview half): 101 live members — over the 100 cap. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + _state(ws, proj, "completed") + module = _module(ws, proj) + issues = Issue.objects.bulk_create( + [ + Issue( + workspace=ws, + project=proj, + name=f"bulk-{i}", + created_by=user, + state=st_started, + sequence_id=i + 1, + ) + for i in range(MAX_MODULE_CASCADE_ITEMS + 1) + ] + ) + from plane.db.models import ModuleIssue + + ModuleIssue.objects.bulk_create( + [ + ModuleIssue(workspace=ws, project=proj, module=module, issue=issue) + for issue in issues + ] + ) + + result = collect_module_cascade( + module=module, target_group="completed", actor_id=user.id + ) + + self.assertTrue(result["over_cap"]) + self.assertEqual(result["items"], []) + self.assertEqual( + result["summary"]["total_live"], MAX_MODULE_CASCADE_ITEMS + 1 + ) + + def test_query_count_on_a_flat_50_item_module_has_no_n_plus_one(self): + # Case 20: the whole preview is a FIXED number of queries regardless + # of item count — seed lookup, seed fetch, one BFS children query + # (empty -> break), one State query, one ProjectMember query. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + _state(ws, proj, "completed") + module = _module(ws, proj) + issues = Issue.objects.bulk_create( + [ + Issue( + workspace=ws, + project=proj, + name=f"q-{i}", + created_by=user, + state=st_started, + sequence_id=i + 1, + ) + for i in range(50) + ] + ) + from plane.db.models import ModuleIssue + + ModuleIssue.objects.bulk_create( + [ + ModuleIssue(workspace=ws, project=proj, module=module, issue=issue) + for issue in issues + ] + ) + + with self.assertNumQueries(5): + result = collect_module_cascade( + module=module, target_group="completed", actor_id=user.id + ) + self.assertEqual(result["summary"]["total_live"], 50) + + +# --------------------------------------------------------------------------- +# apply_module_cascade — the atomic module + issues write +# --------------------------------------------------------------------------- + +class TestApplyModuleCascade(TransactionTestCase): + def _setup(self): + ws = _ws() + proj = _project(ws) + user = _user() + _pmember(ws, proj, user) + return ws, proj, user + + @mock.patch("plane.cascade_ext.service.model_activity") + @mock.patch("plane.cascade_ext.service.issue_activity") + def test_cancelling_a_module_leaves_a_completed_member_untouched( + self, mock_issue_activity, mock_model_activity + ): + # Case 5: the completed member is in the OTHER terminal group — M8 + # says it is never overwritten. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + st_completed = _state(ws, proj, "completed") + st_cancelled = _state(ws, proj, "cancelled") + module = _module(ws, proj) + done = _issue(ws, proj, user, state=st_completed) + live1 = _issue(ws, proj, user, state=st_started) + live2 = _issue(ws, proj, user, state=st_started) + for issue in (done, live1, live2): + _module_issue(module, issue) + + result = apply_module_cascade( + module=module, + status="cancelled", + item_ids=None, + actor_id=user.id, + slug=ws.slug, + origin="", + ) + + self.assertEqual(set(result["updated"]), {str(live1.id), str(live2.id)}) + module.refresh_from_db() + done.refresh_from_db() + live1.refresh_from_db() + live2.refresh_from_db() + self.assertEqual(module.status, "cancelled") + self.assertEqual(done.state_id, st_completed.id) # untouched + self.assertEqual(live1.state_id, st_cancelled.id) + self.assertEqual(live2.state_id, st_cancelled.id) + + @mock.patch("plane.cascade_ext.service.model_activity") + @mock.patch("plane.cascade_ext.service.issue_activity") + def test_item_ids_none_moves_every_eligible_item( + self, mock_issue_activity, mock_model_activity + ): + # Case 11. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + st_completed = _state(ws, proj, "completed") + module = _module(ws, proj) + a = _issue(ws, proj, user, state=st_started) + child = _issue(ws, proj, user, state=st_started, parent=a) + _module_issue(module, a) + + result = apply_module_cascade( + module=module, + status="completed", + item_ids=None, + actor_id=user.id, + slug=ws.slug, + origin="", + ) + + self.assertEqual(set(result["updated"]), {str(a.id), str(child.id)}) + module.refresh_from_db() + a.refresh_from_db() + child.refresh_from_db() + self.assertEqual(module.status, "completed") + self.assertEqual(a.state_id, st_completed.id) + self.assertEqual(child.state_id, st_completed.id) + + @mock.patch("plane.cascade_ext.service.model_activity") + @mock.patch("plane.cascade_ext.service.issue_activity") + def test_item_ids_empty_list_moves_only_the_module( + self, mock_issue_activity, mock_model_activity + ): + # Case 12: an explicit [] is NOT "all" — zero issue writes. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + st_completed = _state(ws, proj, "completed") + module = _module(ws, proj) + a = _issue(ws, proj, user, state=st_started) + _module_issue(module, a) + + result = apply_module_cascade( + module=module, + status="completed", + item_ids=[], + actor_id=user.id, + slug=ws.slug, + origin="", + ) + + self.assertEqual(result["updated"], []) + self.assertEqual(result["rejected"], []) + module.refresh_from_db() + a.refresh_from_db() + self.assertEqual(module.status, "completed") + self.assertNotEqual(a.state_id, st_completed.id) + + @mock.patch("plane.cascade_ext.service.model_activity") + @mock.patch("plane.cascade_ext.service.issue_activity") + def test_posted_ineligible_id_is_rejected_with_its_reason_and_not_written( + self, mock_issue_activity, mock_model_activity + ): + # Case 13: the module's own project has NO completed state, so its + # member is listed ineligible; posting its id rejects, not writes. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + module = _module(ws, proj) + a = _issue(ws, proj, user, state=st_started) + _module_issue(module, a) + + result = apply_module_cascade( + module=module, + status="completed", + item_ids=[str(a.id)], + actor_id=user.id, + slug=ws.slug, + origin="", + ) + + self.assertEqual(result["updated"], []) + self.assertEqual( + result["rejected"], [{"id": str(a.id), "reason": "no_matching_state"}] + ) + a.refresh_from_db() + self.assertEqual(a.state_id, st_started.id) + + @mock.patch("plane.cascade_ext.service.model_activity") + @mock.patch("plane.cascade_ext.service.issue_activity") + def test_posted_id_from_a_different_module_is_not_in_module_tree( + self, mock_issue_activity, mock_model_activity + ): + # Case 14. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + st_completed = _state(ws, proj, "completed") + module = _module(ws, proj) + other_module = _module(ws, proj) + a = _issue(ws, proj, user, state=st_started) + foreign = _issue(ws, proj, user, state=st_started) + _module_issue(module, a) + _module_issue(other_module, foreign) + + result = apply_module_cascade( + module=module, + status="completed", + item_ids=[str(a.id), str(foreign.id)], + actor_id=user.id, + slug=ws.slug, + origin="", + ) + + self.assertEqual(result["updated"], [str(a.id)]) + self.assertEqual( + result["rejected"], [{"id": str(foreign.id), "reason": "not_in_module_tree"}] + ) + foreign.refresh_from_db() + self.assertEqual(foreign.state_id, st_started.id) + + @mock.patch("plane.cascade_ext.service.model_activity") + @mock.patch("plane.cascade_ext.service.issue_activity") + def test_posted_id_beneath_a_terminal_member_is_under_terminal_ancestor( + self, mock_issue_activity, mock_model_activity + ): + # Case 14b: Phase 0's rejection reason, reused verbatim. The live + # child genuinely IS in the module tree — behind a pruned branch. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + st_completed = _state(ws, proj, "completed") + module = _module(ws, proj) + done_member = _issue(ws, proj, user, state=st_completed) + hidden_child = _issue(ws, proj, user, state=st_started, parent=done_member) + _module_issue(module, done_member) + + result = apply_module_cascade( + module=module, + status="completed", + item_ids=[str(hidden_child.id)], + actor_id=user.id, + slug=ws.slug, + origin="", + ) + + self.assertEqual(result["updated"], []) + self.assertEqual( + result["rejected"], + [{"id": str(hidden_child.id), "reason": "under_terminal_ancestor"}], + ) + hidden_child.refresh_from_db() + self.assertEqual(hidden_child.state_id, st_started.id) # NOT written + + @mock.patch("plane.cascade_ext.service.model_activity") + @mock.patch("plane.cascade_ext.service.issue_activity") + def test_failure_mid_bulk_update_rolls_back_the_module_status_too( + self, mock_issue_activity, mock_model_activity + ): + # Case 15: the atomicity gate for M5 — module status and issue writes + # share ONE transaction. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + st_completed = _state(ws, proj, "completed") + module = _module(ws, proj) + a = _issue(ws, proj, user, state=st_started) + _module_issue(module, a) + + with mock.patch.object( + Issue.issue_objects, "bulk_update", side_effect=RuntimeError("boom") + ): + with self.assertRaises(RuntimeError): + apply_module_cascade( + module=module, + status="completed", + item_ids=None, + actor_id=user.id, + slug=ws.slug, + origin="", + ) + + module.refresh_from_db() + a.refresh_from_db() + self.assertEqual(module.status, "planned") # rolled back + self.assertEqual(a.state_id, st_started.id) + mock_issue_activity.delay.assert_not_called() + mock_model_activity.delay.assert_not_called() + + def test_over_cap_apply_raises_before_writing_anything(self): + # Case 16 (apply half): the exception fires BEFORE any transaction + # opens — the module's status is part of "nothing is written". + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + _state(ws, proj, "completed") + module = _module(ws, proj) + issues = Issue.objects.bulk_create( + [ + Issue( + workspace=ws, + project=proj, + name=f"cap-{i}", + created_by=user, + state=st_started, + sequence_id=i + 1, + ) + for i in range(MAX_MODULE_CASCADE_ITEMS + 1) + ] + ) + from plane.db.models import ModuleIssue + + ModuleIssue.objects.bulk_create( + [ + ModuleIssue(workspace=ws, project=proj, module=module, issue=issue) + for issue in issues + ] + ) + + with self.assertRaises(CascadeCapExceeded) as ctx: + apply_module_cascade( + module=module, + status="completed", + item_ids=None, + actor_id=user.id, + slug=ws.slug, + origin="", + ) + + self.assertEqual(ctx.exception.total_live, MAX_MODULE_CASCADE_ITEMS + 1) + module.refresh_from_db() + self.assertEqual(module.status, "planned") # unchanged + + @mock.patch("plane.cascade_ext.service.model_activity") + @mock.patch("plane.cascade_ext.service.issue_activity") + def test_activity_dispatch_one_module_activity_and_silent_item_activities( + self, mock_issue_activity, mock_model_activity + ): + # Case 19: one model_activity for the module; per accepted item one + # issue_activity with notification=False (load-bearing, M11) plus one + # model_activity(model_name="issue"). + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + _state(ws, proj, "completed") + module = _module(ws, proj) + a = _issue(ws, proj, user, state=st_started) + b = _issue(ws, proj, user, state=st_started) + _module_issue(module, a) + _module_issue(module, b) + + apply_module_cascade( + module=module, + status="completed", + item_ids=None, + actor_id=user.id, + slug=ws.slug, + origin="", + ) + + module_calls = [ + c for c in mock_model_activity.delay.call_args_list + if c.kwargs.get("model_name") == "module" + ] + self.assertEqual(len(module_calls), 1) + self.assertEqual(module_calls[0].kwargs["model_id"], str(module.id)) + + item_ids = {str(a.id), str(b.id)} + issue_activity_calls = [ + c for c in mock_issue_activity.delay.call_args_list + if c.kwargs.get("issue_id") in item_ids + ] + self.assertEqual(len(issue_activity_calls), 2) + for call in issue_activity_calls: + self.assertEqual(call.kwargs.get("notification"), False) + + issue_model_calls = [ + c for c in mock_model_activity.delay.call_args_list + if c.kwargs.get("model_name") == "issue" + and c.kwargs.get("model_id") in item_ids + ] + self.assertEqual(len(issue_model_calls), 2) + + +# --------------------------------------------------------------------------- +# The two HTTP endpoints — auth gate, validation, archived/cap refusal +# --------------------------------------------------------------------------- + +class TestModuleCascadeEndpoints(TransactionTestCase): + def _setup(self): + ws = _ws() + proj = _project(ws) + user = _user() + _pmember(ws, proj, user) + self.client = APIClient() + self.client.force_authenticate(user=user) + return ws, proj, user + + def _preview_url(self, ws, proj, module): + return ( + f"/api/cascade-ext/workspaces/{ws.slug}/projects/{proj.id}" + f"/modules/{module.id}/cascade-preview/" + ) + + def _apply_url(self, ws, proj, module): + return ( + f"/api/cascade-ext/workspaces/{ws.slug}/projects/{proj.id}" + f"/modules/{module.id}/cascade-apply/" + ) + + def test_preview_rejects_a_non_terminal_status(self): + # Case 18. + ws, proj, _user_ = self._setup() + module = _module(ws, proj) + + resp = self.client.get(self._preview_url(ws, proj, module) + "?status=in-progress") + + self.assertEqual(resp.status_code, 400) + + def test_archived_module_refuses_preview_and_apply(self): + # Case 17 (M13). + ws, proj, _user_ = self._setup() + module = _module(ws, proj, archived=True) + + preview = self.client.get(self._preview_url(ws, proj, module) + "?status=completed") + apply_resp = self.client.post( + self._apply_url(ws, proj, module), {"status": "completed"}, format="json" + ) + + self.assertEqual(preview.status_code, 400) + self.assertEqual(apply_resp.status_code, 400) + + def test_apply_over_cap_is_a_400_and_writes_nothing(self): + # Case 16 (endpoint half): exact contract body, module status + # unchanged. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + _state(ws, proj, "completed") + module = _module(ws, proj) + issues = Issue.objects.bulk_create( + [ + Issue( + workspace=ws, + project=proj, + name=f"http-cap-{i}", + created_by=user, + state=st_started, + sequence_id=i + 1, + ) + for i in range(MAX_MODULE_CASCADE_ITEMS + 1) + ] + ) + from plane.db.models import ModuleIssue + + ModuleIssue.objects.bulk_create( + [ + ModuleIssue(workspace=ws, project=proj, module=module, issue=issue) + for issue in issues + ] + ) + + resp = self.client.post( + self._apply_url(ws, proj, module), {"status": "completed"}, format="json" + ) + + self.assertEqual(resp.status_code, 400) + self.assertEqual(resp.json()["error"], "cascade exceeds MAX_MODULE_CASCADE_ITEMS") + self.assertEqual(resp.json()["total_live"], MAX_MODULE_CASCADE_ITEMS + 1) + self.assertEqual(resp.json()["cap"], MAX_MODULE_CASCADE_ITEMS) + module.refresh_from_db() + self.assertEqual(module.status, "planned") + + @mock.patch("plane.cascade_ext.service.model_activity") + @mock.patch("plane.cascade_ext.service.issue_activity") + def test_happy_path_apply_over_http(self, mock_issue_activity, mock_model_activity): + # Smoke: the endpoint wires through to the service and returns the + # contract shape. + ws, proj, user = self._setup() + st_started = _state(ws, proj, "started") + _state(ws, proj, "completed") + module = _module(ws, proj) + a = _issue(ws, proj, user, state=st_started) + _module_issue(module, a) + + resp = self.client.post( + self._apply_url(ws, proj, module), {"status": "completed"}, format="json" + ) + + self.assertEqual(resp.status_code, 200) + body = resp.json() + self.assertEqual(body["module"], str(module.id)) + self.assertEqual(body["status"], "completed") + self.assertEqual(body["updated"], [str(a.id)]) + self.assertEqual(body["rejected"], []) diff --git a/apps/api/plane/cascade_ext/urls.py b/apps/api/plane/cascade_ext/urls.py index 1d5841053..83051d034 100644 --- a/apps/api/plane/cascade_ext/urls.py +++ b/apps/api/plane/cascade_ext/urls.py @@ -7,10 +7,17 @@ # # GET /api/cascade-ext/workspaces//projects//issues//cascade-preview/ # POST /api/cascade-ext/workspaces//projects//issues//cascade-apply/ +# GET /api/cascade-ext/workspaces//projects//modules//cascade-preview/ +# POST /api/cascade-ext/workspaces//projects//modules//cascade-apply/ from django.urls import path -from .views import CascadeApplyEndpoint, CascadePreviewEndpoint +from .views import ( + CascadeApplyEndpoint, + CascadePreviewEndpoint, + ModuleCascadeApplyEndpoint, + ModuleCascadePreviewEndpoint, +) urlpatterns = [ path( @@ -23,4 +30,14 @@ CascadeApplyEndpoint.as_view(), name="cascade-ext-apply", ), + path( + "workspaces//projects//modules//cascade-preview/", + ModuleCascadePreviewEndpoint.as_view(), + name="cascade-ext-module-preview", + ), + path( + "workspaces//projects//modules//cascade-apply/", + ModuleCascadeApplyEndpoint.as_view(), + name="cascade-ext-module-apply", + ), ] diff --git a/apps/api/plane/cascade_ext/views.py b/apps/api/plane/cascade_ext/views.py index e23a9b3e6..06b71f691 100644 --- a/apps/api/plane/cascade_ext/views.py +++ b/apps/api/plane/cascade_ext/views.py @@ -3,25 +3,39 @@ # See the LICENSE file for details. # # The1Studio fork (cascade_ext) — docs/FORK.md touch-point 2. Two thin HTTP -# endpoints; all logic lives in service.py so preview and apply can never +# endpoint pairs; all logic lives in service.py so preview and apply can never # disagree on what is eligible. No core view is edited — the default "only # change this item" path stays the existing plain PATCH, untouched. # # Contract, decisions, and test matrix: # plans/260822-cascade-complete-sub-items/phase-1-cascade-backend.md +# plans/260828-module-cascade-terminal-status/phase-1-module-cascade-backend.md from rest_framework import status from rest_framework.response import Response from plane.app.permissions import ROLE, allow_permission from plane.app.views.base import BaseAPIView -from plane.db.models import Issue, State +from plane.db.models import Issue, Module, State from plane.utils.host import base_host -from .service import apply_cascade, collect_descendants +from .service import ( + MAX_MODULE_CASCADE_ITEMS, + MODULE_STATUS_TO_STATE_GROUP, + CascadeCapExceeded, + apply_cascade, + apply_module_cascade, + collect_descendants, + collect_module_cascade, +) _VALID_TARGET_GROUPS = {"completed", "cancelled"} +# Hard-cap refusal body is part of the frozen endpoint contract (Phase 1 § +# Endpoint contract) — the field names and this exact error string are what +# the TypeScript client parses. +_CAP_ERROR = "cascade exceeds MAX_MODULE_CASCADE_ITEMS" + class CascadePreviewEndpoint(BaseAPIView): """GET .../cascade-preview/?group= @@ -116,3 +130,109 @@ def post(self, request, slug, project_id, issue_id): origin=base_host(request=request, is_app=True), ) return Response(result, status=status.HTTP_200_OK) + + +def _get_module_or_response(slug, project_id, module_id): + """Shared lookup + guards for the module cascade endpoints. + + Returns (module, None) on success or (None, Response) with the failure — + 404 when absent, 400 when archived (M13: mirroring the core viewset's own + refusal to write an archived module). + """ + module = Module.objects.filter( + pk=module_id, project_id=project_id, workspace__slug=slug + ).first() + if module is None: + return None, Response( + {"error": "module not found"}, status=status.HTTP_404_NOT_FOUND + ) + if module.archived_at: + return None, Response( + {"error": "module is archived"}, status=status.HTTP_400_BAD_REQUEST + ) + return module, None + + +class ModuleCascadePreviewEndpoint(BaseAPIView): + """GET .../modules//cascade-preview/?status= + + Read-only. Answers "what would cascade if this module went terminal right + now" — the client calls this BEFORE the module's status actually changes. + The query parameter is `status` (a MODULE status), NOT the issue + endpoint's `group`; the two are deliberately not unified. Same read gate + as viewing the module itself. + """ + + @allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST]) + def get(self, request, slug, project_id, module_id): + module_status = request.GET.get("status") + if module_status not in MODULE_STATUS_TO_STATE_GROUP: + return Response( + {"error": "status must be one of completed|cancelled"}, + status=status.HTTP_400_BAD_REQUEST, + ) + + module, error = _get_module_or_response(slug, project_id, module_id) + if error is not None: + return error + + collected = collect_module_cascade( + module=module, + target_group=MODULE_STATUS_TO_STATE_GROUP[module_status], + actor_id=request.user.id, + ) + return Response(collected, status=status.HTTP_200_OK) + + +class ModuleCascadeApplyEndpoint(BaseAPIView): + """POST .../modules//cascade-apply/ { status, item_ids? } + + Applies the module's new status plus a caller-selected subset of + currently-eligible items, in ONE transaction (M5). `item_ids` is a + REQUEST, never an authorization — service.apply_module_cascade re-derives + eligibility server-side. Over the hard cap, 400 and NOTHING is written, + module status included (M4). + + Gated the same as the module write path it mirrors (ADMIN/MEMBER — the + roles that may write a module per ModuleViewSet.partial_update). + """ + + @allow_permission([ROLE.ADMIN, ROLE.MEMBER]) + def post(self, request, slug, project_id, module_id): + module_status = request.data.get("status") + if module_status not in MODULE_STATUS_TO_STATE_GROUP: + return Response( + {"error": "status must be one of completed|cancelled"}, + status=status.HTTP_400_BAD_REQUEST, + ) + + module, error = _get_module_or_response(slug, project_id, module_id) + if error is not None: + return error + + item_ids = request.data.get("item_ids", None) + if item_ids is not None and not isinstance(item_ids, list): + return Response( + {"error": "item_ids must be a list, null, or omitted"}, + status=status.HTTP_400_BAD_REQUEST, + ) + + try: + result = apply_module_cascade( + module=module, + status=module_status, + item_ids=item_ids, + actor_id=request.user.id, + slug=slug, + origin=base_host(request=request, is_app=True), + ) + except CascadeCapExceeded as exc: + return Response( + { + "error": _CAP_ERROR, + "total_live": exc.total_live, + "cap": MAX_MODULE_CASCADE_ITEMS, + }, + status=status.HTTP_400_BAD_REQUEST, + ) + return Response(result, status=status.HTTP_200_OK) From 8473797d830759723bb527f644e1eb0e2144569e Mon Sep 17 00:00:00 2001 From: Manh Nguyen Date: Fri, 28 Aug 2026 13:01:50 +0700 Subject: [PATCH 2/7] docs(plans): record module cascade, terminal-subtree prune, propagation - docs/FORK.md: register the module-cascade core exception (apps/web/core/store/module.store.ts, fenced "module-cascade"), the behavior change to terminal-node pruning for the per-issue cascade, and the module cascade endpoint contract (100-item refusal, one transaction, archived 400s, no migration). - plans/260828-module-cascade-terminal-status/: six-file plan (Phase 0 prune-terminal-subtrees through Phase 4 propagate). - plans/260822-cascade-complete-sub-items/plan.md: dated amendment recording Decision 5 REVERSED (terminal nodes prune their subtrees), the two inverted tests, and the new under_terminal_ancestor reason. - .claude/plane-propagation-queue.md: propagation entry for plane-mcp-server / plane-node-sdk / plane-python-sdk / plane-claude-plugin / docs, incl. the update_module status-coercion trap and the stale "still traversed through" docstrings. Co-Authored-By: Claude --- .claude/plane-propagation-queue.md | 37 +++++++ docs/FORK.md | 37 ++++++- .../260822-cascade-complete-sub-items/plan.md | 97 +++++++++++-------- 3 files changed, 123 insertions(+), 48 deletions(-) diff --git a/.claude/plane-propagation-queue.md b/.claude/plane-propagation-queue.md index d6586fb81..313e6ade5 100644 --- a/.claude/plane-propagation-queue.md +++ b/.claude/plane-propagation-queue.md @@ -39,3 +39,40 @@ Entries written by `plane-scaffold-feature` / `plane-propagate`; processed entri - Propagation needed: MCP tool in `plane-mcp-server` (DONE locally — `set_project_visibility`, `get_project_visibility`, `set_projects_visibility_bulk`), SDK bindings in `plane-node-sdk` + `plane-python-sdk`, docs update. + +## cascade_ext module cascade + terminal-subtree pruning — 2026-08-28 + +- Feature: two things shipped together in the existing `cascade_ext` app + (`plans/260828-module-cascade-terminal-status/`, Plane PLANE-189). + 1. **Module cascade** — a module moving to `completed`/`cancelled` cascades that terminal group + onto every live module member PLUS each member's full descendant subtree, behind the same + confirmation modal the per-issue cascade uses. Apply writes the module's own `status` and the + issue states in ONE transaction. `MAX_MODULE_CASCADE_ITEMS = 100` is a **refusal, not a + truncation**: over it, preview returns `over_cap: true` with an EMPTY `items` array and apply + returns 400 having written nothing (module status included). + 2. **BEHAVIOR CHANGE to the shipped per-issue cascade** — a descendant already in a terminal group + now PRUNES its entire subtree instead of being traversed through. A live sub-item under a + cancelled parent is left live where it used to be swept. New rejection reason + `under_terminal_ancestor` (the old `not_a_descendant` would be a false label for a live id + behind a pruned branch). +- New endpoints: + - `GET /api/cascade-ext/workspaces//projects//modules//cascade-preview/?status=` + — note the query param is `status` (a MODULE status), **not** `group` as on the issue routes. + Returns `{target_group, depth_capped, over_cap, cap, summary{total_live,eligible,ineligible,already_terminal}, items[]}`. + - `POST /api/cascade-ext/workspaces//projects//modules//cascade-apply/` + — body `{status, item_ids}`; `item_ids` omitted/null = every eligible item, `[]` = none. + Returns `{module, status, updated[], rejected[{id, reason}]}`. Archived module → 400. +- New fields: none on core models — no migration, no new app, no touch-point edit. +- Propagation needed: + - `plane-mcp-server` — `preview_module_cascade` tool, `update_module(..., cascade=False)` + mirroring `update_work_item`. TRAP: `update_module` coerces an unrecognized `status` to `None` + (`plane_mcp/tools/modules.py:166-176`), so the cascade branch must key off the VALIDATED value, + never the raw argument. ALSO: `plane_mcp/tools/cascade_ext.py`'s module docstring and + `update_work_item`'s help still state the old "still traversed through" rule, which item 2 above + made false — correct them in the same PR. + - `plane-node-sdk` + `plane-python-sdk` — bindings for both routes. + - `plane-claude-plugin` — user-facing "complete a module and everything in it", naming the + 100-item refusal. + - `docs` + `developer-docs` — the `reason` enum, the 400 shapes, and the pruning behavior change. + - `plane-deploy` / `helm-charts` — NOT applicable; no new env var, no new service. The cap is a + hardcoded constant on purpose. diff --git a/docs/FORK.md b/docs/FORK.md index 65195bc77..bdbf4d975 100644 --- a/docs/FORK.md +++ b/docs/FORK.md @@ -918,6 +918,26 @@ endpoints, **zero core backend edits** (mounts via touch-point 2 only). Frontend `packages/cascade-ext/` package (store, modal, API client, the pure `shouldPromptCascade` guard) plus the fenced core delegations below. +**Behavior change 2026-08-28 — terminal nodes prune their subtrees.** A descendant already in a +terminal group is never listed or touched, AND nothing beneath it is listed, walked, or changed +(`plans/260828-module-cascade-terminal-status/` Phase 0 reversed the shipped "skipped but still +traversed through" rule so the issue cascade and the module cascade share one rule on the same +`Issue.parent` tree). Stated cost: a live sub-item under a cancelled parent is now left live where +it used to be swept. Apply rejects a posted id behind a pruned branch with +`under_terminal_ancestor` rather than the false `not_a_descendant`. + +**Module cascade (2026-08-28, `plans/260828-module-cascade-terminal-status/`).** The same app +carries a second endpoint pair one level up: +`GET/POST .../projects//modules//cascade-preview|cascade-apply/` cascade a +module's `completed`/`cancelled` status onto every live module member plus each member's full +descendant subtree. The apply writes the module's own status and the issue states in ONE +transaction and fires one `model_activity(model_name="module")`; cascaded issues get +`notification=False`. A plain module `PATCH {status}` never cascades, from any client. Hard cap: +`MAX_MODULE_CASCADE_ITEMS = 100` live (post-pruning) nodes — over it, preview returns +`over_cap: true` with empty `items` and apply 400s without writing anything, module status +included. Archived modules refuse both endpoints with 400. No new app, model, migration, or +touch-point edit. + **One choke point, not two.** The plan's own seam table names two entry points — `issue-details/issue.store.ts:181` `updateIssue` (detail dropdown) and `helpers/base-issues.store.ts` `updateIssue` (list/spreadsheet/kanban) — as if they needed @@ -935,10 +955,11 @@ in exactly one place, `issueUpdate`, which structurally covers all three of #54' points (detail dropdown, list/spreadsheet dropdown, kanban drag-drop) at once. `issue-details/issue.store.ts` carries no cascade-confirm edit at all. -| File | What | Why no seam | -| -------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `apps/web/core/store/issue/helpers/base-issues.store.ts` | Module-level `cascadeConfirmStore` singleton (`@plane/cascade-ext` ships only the class), and — at the top of `issueUpdate` — `shouldPromptCascade` guard → `cascadeService.getPreview` → (if any row is eligible) `cascadeConfirmStore.requestCascade` → on a cascade choice with ticked children, `cascadeService.apply` and an early `return` so the plain PATCH below never double-writes the parent. Every other case (no target group, empty/all-ineligible preview, "only this item", zero ticked children) falls through unchanged | No upstream pre-update hook on the issue stores, and `issueUpdate` is the one method every list/spreadsheet/kanban/detail state write funnels through — see "One choke point" above | -| `apps/web/app/root.tsx` | Mounts `` inside ``, importing the singleton back from `base-issues.store.ts` | No global modal-host seam exists for a fork-owned dialog; this widens touch-point 7 beyond its documented white-label-branding purpose (`VITE_APP_TITLE` etc.) — noted here rather than left for a future reader to wonder about | +| File | What | Why no seam | +| -------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `apps/web/core/store/issue/helpers/base-issues.store.ts` | Module-level `cascadeConfirmStore` singleton (`@plane/cascade-ext` ships only the class), and — at the top of `issueUpdate` — `shouldPromptCascade` guard → `cascadeService.getPreview` → (if any row is eligible) `cascadeConfirmStore.requestCascade` → on a cascade choice with ticked children, `cascadeService.apply` and an early `return` so the plain PATCH below never double-writes the parent. Every other case (no target group, empty/all-ineligible preview, "only this item", zero ticked children) falls through unchanged | No upstream pre-update hook on the issue stores, and `issueUpdate` is the one method every list/spreadsheet/kanban/detail state write funnels through — see "One choke point" above | +| `apps/web/app/root.tsx` | Mounts `` inside ``, importing the singleton back from `base-issues.store.ts` | No global modal-host seam exists for a fork-owned dialog; this widens touch-point 7 beyond its documented white-label-branding purpose (`VITE_APP_TITLE` etc.) — noted here rather than left for a future reader to wonder about | +| `apps/web/core/store/module.store.ts` | At the top of `updateModuleDetails` — `shouldPromptModuleCascade` guard → `cascadeService.getModulePreview` → (if any row is eligible **or** the preview is over cap) `cascadeConfirmStore.requestModuleCascade` → on a cascade choice with ticked items, `cascadeService.applyModuleCascade`, a `fetchModuleDetails` refetch, and an early `return` so the plain `patchModule` below never double-writes the module. The whole block is wrapped so a cascade-ext failure logs and falls through to the plain PATCH — a fork add-on being unreachable must never break a core action. Every other case falls through unchanged | No upstream pre-update hook on the module store, and `updateModuleDetails` is the one method all five status entry points funnel through (list row, grid card, analytics sidebar, create/update modal, power-K). The gantt layout calls it too but only with `sort_order`/`start_date`/`target_date`, which the `data.status` guard makes a free no-op | **`plane-isolation-audit` / fork-ownership note:** `packages/cascade-ext` uses the `@plane/` npm scope but is **fork-owned** (not upstream) — same clarification as `@plane/workload-ext` / @@ -947,9 +968,15 @@ sealed-package edit. **Rebase handling:** these files ARE expected conflict points (unlike the abort-on-conflict rule for everything else). On conflict, re-apply the fork block — each is fenced by a -`The1Studio fork (cascade-confirm)` comment — and keep upstream's changes around it. Do NOT abort +`The1Studio fork (cascade-confirm)` comment, except `module.store.ts` whose fence reads +`The1Studio fork (module-cascade)` — and keep upstream's changes around it. Do NOT abort the rebase for a conflict confined to this set. +**Why `module.store.ts` needs no `root.tsx` change of its own:** the module cascade reuses the +single `` already mounted there, and widens the existing +`cascadeConfirmStore` rather than adding a second store. A second store would need a second mount +point, which is why the store was widened in place. + ### Work-item creation defaults — fenced `The1Studio fork (work-item creation defaults)` A work item created with a field left unset gets a default: the **assignee** becomes the creator, diff --git a/plans/260822-cascade-complete-sub-items/plan.md b/plans/260822-cascade-complete-sub-items/plan.md index 49e911cf1..8b4bd00a6 100644 --- a/plans/260822-cascade-complete-sub-items/plan.md +++ b/plans/260822-cascade-complete-sub-items/plan.md @@ -18,39 +18,50 @@ Confirmed independently twice: my own sweep found **zero** matches across `apps/ `apps/space/`, `packages/` and `apps/api/plane/` for any guard, toast, modal, or serializer validation on this transition (the only `PARENT_HAS_CHILDREN` in the tree is `apps/api/plane/workload/views.py:157`, which guards **estimates**). Issue #54 says the same: -*"Không có cảnh báo, không có gợi ý nào."* This plan adds behavior; it removes nothing. +_"Không có cảnh báo, không có gợi ý nào."_ This plan adds behavior; it removes nothing. Confirmed seams: -| Seam | Location | -| --- | --- | -| `Issue.parent` self-FK | `apps/api/plane/db/models/issue.py:113` | -| `State.group`, `State.sequence` | `apps/api/plane/db/models/state.py:84-90` | -| Detail-view state write | `issue-details/issue.store.ts:181` `updateIssue` | -| List / spreadsheet / kanban state write | `helpers/base-issues.store.ts` `updateIssue` | -| Kanban drop → store | `issue-layouts/utils.tsx:513` `handleDragDrop` → `updateIssueOnDrop` → store | -| Modal primitives | `packages/ui/src/modals/modal-core.tsx`, `alert-modal.tsx` | -| App provider mount | `apps/web/app/root.tsx:135` `` (touch-point 7) | -| Fork-app precedent | `apps/api/plane/workload/`, `views_ext/`, `github_ext/` | +| Seam | Location | +| --------------------------------------- | ---------------------------------------------------------------------------- | +| `Issue.parent` self-FK | `apps/api/plane/db/models/issue.py:113` | +| `State.group`, `State.sequence` | `apps/api/plane/db/models/state.py:84-90` | +| Detail-view state write | `issue-details/issue.store.ts:181` `updateIssue` | +| List / spreadsheet / kanban state write | `helpers/base-issues.store.ts` `updateIssue` | +| Kanban drop → store | `issue-layouts/utils.tsx:513` `handleDragDrop` → `updateIssueOnDrop` → store | +| Modal primitives | `packages/ui/src/modals/modal-core.tsx`, `alert-modal.tsx` | +| App provider mount | `apps/web/app/root.tsx:135` `` (touch-point 7) | +| Fork-app precedent | `apps/api/plane/workload/`, `views_ext/`, `github_ext/` | ## Decisions (resolved) -| # | Decision | -| --- | --- | -| 1 | **Confirmation modal, per issue #54.** Changing a parent into a terminal state opens a modal listing every affected descendant — identifier, name, current state — each with a checkbox. | -| 2 | **Default is "only change this item."** That button holds initial focus, so a stray Enter never cascades. Accidental cascade is treated as the expensive mistake. | -| 3 | The modal appears **only when there is something to change** — at least one non-terminal descendant. No children, or all descendants already terminal → plain state change, no prompt. | -| 4 | Cascade fires for **both terminal groups** and mirrors the one the parent entered: completing completes, cancelling cancels. A move between two states of the same terminal group is a no-op. | -| 5 | Descendants already in a terminal group are **never** listed or touched — a cancelled child is not completed, a completed child is not cancelled. Their own live descendants still cascade through them. | -| 6 | **All descendants, recursively**, with a `visited` set and `MAX_DEPTH = 20` against `parent` cycles. | -| 7 | Cross-project descendants are included, each resolved to **its own** project's state of the same `group` — never by name, since states are renameable. | -| 8 | A descendant whose project has no state in the target group, or whose project the actor is not an active member of, is shown **disabled with a reason** rather than hidden or silently skipped. | -| 9 | Parent state change + selected children apply in **one transaction**. Partial failure rolls the whole thing back, including the parent. | -| 10 | Each cascaded child gets its own activity entry, with `notification=False`. Only the parent's own change notifies watchers. | -| 11 | **No implicit cascade for API/MCP callers.** A plain `PATCH state` never cascades, from any client. Cascading is an explicit call to the cascade endpoint. | -| 13 | **MCP gets a `cascade` option, default `false`** — mirroring the UI's "only change this item" default. It lives in the MCP tool layer, not in a new core-view parameter: `update_work_item(..., cascade=False)` does a plain PATCH; `cascade=True` calls `cascade-apply` instead. This costs **zero** additional core edits. | -| 14 | `cascade-apply` treats an **omitted or null `child_ids`** as "every currently-eligible descendant". The UI always sends an explicit list (the user ticks boxes); a headless caller with no UI to untick omits it. | -| 12 | Fork UI strings live in `packages/cascade-ext`, not `packages/i18n` — a `@plane/*` package the fork rules forbid editing in place. English-only at first. | +| # | Decision | +| --- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | **Confirmation modal, per issue #54.** Changing a parent into a terminal state opens a modal listing every affected descendant — identifier, name, current state — each with a checkbox. | +| 2 | **Default is "only change this item."** That button holds initial focus, so a stray Enter never cascades. Accidental cascade is treated as the expensive mistake. | +| 3 | The modal appears **only when there is something to change** — at least one non-terminal descendant. No children, or all descendants already terminal → plain state change, no prompt. | +| 4 | Cascade fires for **both terminal groups** and mirrors the one the parent entered: completing completes, cancelling cancels. A move between two states of the same terminal group is a no-op. | +| 5 | Descendants already in a terminal group are **never** listed or touched — a cancelled child is not completed, a completed child is not cancelled. Their own live descendants still cascade through them. | +| 6 | **All descendants, recursively**, with a `visited` set and `MAX_DEPTH = 20` against `parent` cycles. | +| 7 | Cross-project descendants are included, each resolved to **its own** project's state of the same `group` — never by name, since states are renameable. | +| 8 | A descendant whose project has no state in the target group, or whose project the actor is not an active member of, is shown **disabled with a reason** rather than hidden or silently skipped. | +| 9 | Parent state change + selected children apply in **one transaction**. Partial failure rolls the whole thing back, including the parent. | +| 10 | Each cascaded child gets its own activity entry, with `notification=False`. Only the parent's own change notifies watchers. | +| 11 | **No implicit cascade for API/MCP callers.** A plain `PATCH state` never cascades, from any client. Cascading is an explicit call to the cascade endpoint. | +| 13 | **MCP gets a `cascade` option, default `false`** — mirroring the UI's "only change this item" default. It lives in the MCP tool layer, not in a new core-view parameter: `update_work_item(..., cascade=False)` does a plain PATCH; `cascade=True` calls `cascade-apply` instead. This costs **zero** additional core edits. | +| 14 | `cascade-apply` treats an **omitted or null `child_ids`** as "every currently-eligible descendant". The UI always sends an explicit list (the user ticks boxes); a headless caller with no UI to untick omits it. | +| 12 | Fork UI strings live in `packages/cascade-ext`, not `packages/i18n` — a `@plane/*` package the fork rules forbid editing in place. English-only at first. | + +> **Amendment (2026-08-28) — Decision 5 is REVERSED.** A descendant already in a terminal group +> is still never listed or touched, but it now also **prunes its entire subtree**: nothing beneath +> it is listed, walked, or changed. The old rule ("their own live descendants still cascade through +> them") shipped and was reversed by `plans/260828-module-cascade-terminal-status/` Phase 0 so the +> per-issue cascade and the new module cascade share one rule on the same `Issue.parent` tree. A +> terminal item is a decision someone made about that branch; reaching past it overrides that +> decision silently. The cost, stated: a live sub-item under a cancelled parent is now left live +> where it used to be swept. The two shipped tests asserting the old rule were inverted (not +> deleted) and renamed to `..._and_prunes_its_subtree`; apply rejects a posted id behind a pruned +> branch with the new reason `under_terminal_ancestor` rather than the false `not_a_descendant`. ## What the modal decision bought @@ -92,28 +103,28 @@ core edit). The two store files are the only genuine exceptions, and they are re ## Phases -| Phase | File | Effort | -| --- | --- | --- | -| 1 | `phase-1-cascade-backend.md` — `cascade_ext` app, preview + apply endpoints, tests | M (4h) | -| 2 | `phase-2-cascade-package.md` — `packages/cascade-ext`: store, modal, preview client | M (3.5h) | -| 3 | `phase-3-wire-stores.md` — fenced interception at the two store choke points + root mount | M (2.5h) | -| 4 | `phase-4-propagate.md` — MCP/SDK/docs propagation, close #54 | S (1.5h) | -| **Total** | | **11.5h** | +| Phase | File | Effort | +| --------- | ----------------------------------------------------------------------------------------- | --------- | +| 1 | `phase-1-cascade-backend.md` — `cascade_ext` app, preview + apply endpoints, tests | M (4h) | +| 2 | `phase-2-cascade-package.md` — `packages/cascade-ext`: store, modal, preview client | M (3.5h) | +| 3 | `phase-3-wire-stores.md` — fenced interception at the two store choke points + root mount | M (2.5h) | +| 4 | `phase-4-propagate.md` — MCP/SDK/docs propagation, close #54 | S (1.5h) | +| **Total** | | **11.5h** | Critical path 1 → 2 → 3. Phase 2 can start against the endpoint contract in Phase 1 before Phase 1 merges, provided that contract is fixed first (it is — see Phase 1 § "Endpoint contract"). ## Risk Assessment -| Risk | L | I | Score | Mitigation | -| --- | --- | --- | --- | --- | -| Client sends child ids it should not be allowed to move | 3 | 5 | **15** | Apply endpoint **re-derives** the eligible set server-side and rejects any id outside it — never trusts the posted list. Phase 1. | -| Preview request fires on every Done click, including leaves | 4 | 3 | 12 | Two-condition guard (terminal group AND `sub_issues_count > 0`) runs client-side before any request. Phase 3 asserts zero extra requests on a leaf. | -| Rebase conflicts at the two core store files | 4 | 2 | 8 | One fenced block each + `docs/FORK.md` exception rows. Phase 3. | -| `parent` cycle → infinite walk | 1 | 5 | 5 | `visited` set + `MAX_DEPTH = 20`; a cap hit is surfaced in the preview payload, not swallowed. | -| Deep tree → slow preview | 3 | 3 | 9 | Level-order BFS, one query per level; preview is read-only and cached for the modal's lifetime. | -| Preview goes stale — a child changes between preview and apply | 2 | 3 | 6 | Apply re-derives eligibility, so a child that became terminal in the gap is skipped and reported in the response. | -| Modal blocks a drag-drop the user thinks completed | 3 | 3 | 9 | Kanban drop applies the parent's state optimistically as today; the modal governs only the cascade. Phase 3. | +| Risk | L | I | Score | Mitigation | +| -------------------------------------------------------------- | --- | --- | ------ | --------------------------------------------------------------------------------------------------------------------------------------------------- | +| Client sends child ids it should not be allowed to move | 3 | 5 | **15** | Apply endpoint **re-derives** the eligible set server-side and rejects any id outside it — never trusts the posted list. Phase 1. | +| Preview request fires on every Done click, including leaves | 4 | 3 | 12 | Two-condition guard (terminal group AND `sub_issues_count > 0`) runs client-side before any request. Phase 3 asserts zero extra requests on a leaf. | +| Rebase conflicts at the two core store files | 4 | 2 | 8 | One fenced block each + `docs/FORK.md` exception rows. Phase 3. | +| `parent` cycle → infinite walk | 1 | 5 | 5 | `visited` set + `MAX_DEPTH = 20`; a cap hit is surfaced in the preview payload, not swallowed. | +| Deep tree → slow preview | 3 | 3 | 9 | Level-order BFS, one query per level; preview is read-only and cached for the modal's lifetime. | +| Preview goes stale — a child changes between preview and apply | 2 | 3 | 6 | Apply re-derives eligibility, so a child that became terminal in the gap is skipped and reported in the response. | +| Modal blocks a drag-drop the user thinks completed | 3 | 3 | 9 | Kanban drop applies the parent's state optimistically as today; the modal governs only the cascade. Phase 3. | ## Verification From c0e88c1cf25cb5e9c4cf0fdbe28fa177d421ea01 Mon Sep 17 00:00:00 2001 From: Manh Nguyen Date: Fri, 28 Aug 2026 13:02:21 +0700 Subject: [PATCH 3/7] docs(plans): add the module-cascade six-phase plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit plans/260828-module-cascade-terminal-status/: phase-0 prune-terminal-subtrees, phase-1 module-cascade-backend, phase-2 cascade-package, phase-3 wire-module-store, phase-4 propagate, and the master plan.md — design record for the module terminal cascade and the terminal-subtree pruning behavior change (user-directed reversal of the per-issue cascade's Decision 5). Co-Authored-By: Claude --- .../phase-0-prune-terminal-subtrees.md | 135 +++++++++ .../phase-1-module-cascade-backend.md | 266 ++++++++++++++++++ .../phase-2-cascade-package.md | 178 ++++++++++++ .../phase-3-wire-module-store.md | 132 +++++++++ .../phase-4-propagate.md | 119 ++++++++ .../plan.md | 183 ++++++++++++ 6 files changed, 1013 insertions(+) create mode 100644 plans/260828-module-cascade-terminal-status/phase-0-prune-terminal-subtrees.md create mode 100644 plans/260828-module-cascade-terminal-status/phase-1-module-cascade-backend.md create mode 100644 plans/260828-module-cascade-terminal-status/phase-2-cascade-package.md create mode 100644 plans/260828-module-cascade-terminal-status/phase-3-wire-module-store.md create mode 100644 plans/260828-module-cascade-terminal-status/phase-4-propagate.md create mode 100644 plans/260828-module-cascade-terminal-status/plan.md diff --git a/plans/260828-module-cascade-terminal-status/phase-0-prune-terminal-subtrees.md b/plans/260828-module-cascade-terminal-status/phase-0-prune-terminal-subtrees.md new file mode 100644 index 000000000..5bea85803 --- /dev/null +++ b/plans/260828-module-cascade-terminal-status/phase-0-prune-terminal-subtrees.md @@ -0,0 +1,135 @@ +# Phase 0 — Prune terminal subtrees in the shipped issue cascade + +**Effort:** S (1.5h) · **Blocks:** Phase 1 · **Changes SHIPPED behavior** +**Plan:** `plans/260828-module-cascade-terminal-status/plan.md` +**Amends:** `plans/260822-cascade-complete-sub-items/plan.md` Decision 5 +**Plane:** [PLANE-190](https://plane.the1studio.org/infrastructure/projects/2eae4e83-f715-4e4b-8753-cdc289bbe37f/issues/6ba258c2-f09e-444c-8067-a22401b052d8) + +## Goal + +Reverse one rule in the live per-issue cascade: a descendant already in a terminal group currently +gets **skipped but traversed through**, so its own live descendants still cascade. From this phase +on, a terminal node **prunes its entire subtree** — not listed, not walked, not changed. + +This phase exists on its own because it changes behavior users already have. It can ship, and be +reverted, without any of the module work. Phase 1 depends on it only so that both subjects share one +rule instead of two. + +## Why + +The module cascade (`plan.md` M8) required this rule, and running the two features with opposite +semantics on the same `Issue.parent` tree is worse than either rule alone: the same tree would +cascade differently depending on whether the action started at a work item or at a module, with +nothing on screen explaining the difference. A terminal item is a decision someone made about that +branch — completing or cancelling it means "this line of work is settled", and reaching past it to +change its children overrides that decision silently. + +**The cost, stated plainly:** a live sub-item under a cancelled parent is now left live where it +used to be swept. That is the intended behavior, not an accepted regression — but it is the case to +watch for in review, and the one that will surprise anyone who learned the old rule. + +## Ownership + +``` +apps/api/plane/cascade_ext/service.py # the walk + one new rejection reason +apps/api/plane/cascade_ext/tests/test_cascade_db.py # invert two tests, add one +plans/260822-cascade-complete-sub-items/plan.md # dated amendment to Decision 5 +docs/FORK.md # cascade_ext entry +CLAUDE.md # cascade_ext bullet +``` + +## Implementation + +### 1. `collect_descendants` — do not enqueue a terminal node's children + +The BFS currently pushes every child onto `next_frontier` and filters terminal nodes out of the +result afterwards. Move the terminal test into the walk: a child whose `state.group` is terminal is +recorded in `traversed_ids` and **not** added to `next_frontier`. + +Consequences to get right, each of them observable: + +- `traversed_ids` no longer contains anything beneath a terminal node, because nothing beneath one + is visited. It still contains the terminal node itself, which is what keeps `apply_cascade`'s + `already_terminal` rejection reason working for the node a caller is most likely to post. +- A node with **no state at all** (`child.state is None`) is not terminal and must keep being + walked. The current code reads `child.state.group if child.state else None`, which is not in + `TERMINAL_GROUPS` — preserve that, do not collapse it to a truthiness check. +- `MAX_DEPTH` and `depth_capped` are unchanged. Pruning makes the cap _less_ likely to fire, never + more. + +### 2. New rejection reason `under_terminal_ancestor` + +`apply_cascade` classifies a posted-but-not-accepted id as `already_terminal` when it is in +`traversed_ids`, else `not_a_descendant`. After pruning, a live grandchild under a terminal parent +is in neither set — and `not_a_descendant` would be **false**: it genuinely is a descendant, it is +just behind a pruned branch. A caller debugging "why did my id do nothing" would be sent looking for +a tree-membership bug that does not exist. + +Resolve it with one extra lookup rather than a wrong label: when a posted id is neither eligible nor +in `traversed_ids`, walk its `parent` chain (bounded by `MAX_DEPTH`) and return +`under_terminal_ancestor` if any ancestor is terminal and reaches the root, `not_a_descendant` +otherwise. This runs only for ids that were already going to be rejected, so it costs nothing on the +normal path. + +### 3. Tests — invert, do not re-pin + +Two existing tests assert the old rule directly and must be **inverted**, not deleted and not +loosened: + +| Test (`test_cascade_db.py:193`, `:209`) | Was | Becomes | +| ----------------------------------------------------------------- | --------------------------- | ---------------------------------------------------------------------- | +| `test_cancelled_child_excluded_but_its_own_children_still_listed` | `assertIn(grandchild, ids)` | `assertNotIn(grandchild, ids)`; rename to `..._and_prunes_its_subtree` | +| `test_completed_child_excluded_mirrored_for_cancel_target` | `assertIn(grandchild, ids)` | `assertNotIn(grandchild, ids)`; same rename | + +Both keep `assertNotIn(terminal_child, ids)` and +`assertIn(str(terminal_child.id), result["traversed_ids"])` — those halves are unchanged and are +what proves the terminal node itself is still _seen_, only its branch is not followed. + +Each inverted test carries a comment naming this phase and the date, so the next reader can tell a +deliberate reversal from a test someone weakened to make a failure go away. **Do not** adjust an +assertion to match whatever the new run reports — the two lines above are the whole diff. + +New tests: + +| # | Case | Asserts | +| --- | ------------------------------------------------------ | --------------------------------------------------------------------------------------- | +| A | Terminal node with a 2-level live subtree | neither level listed; `traversed_ids` holds the terminal node and **nothing below it** | +| B | Live child, terminal grandchild, live great-grandchild | child listed, great-grandchild **not** — pruning applies at any depth, not just level 1 | +| C | Child with `state=None` and a live grandchild | both still listed — a stateless node is not terminal | +| D | Apply posting a live id beneath a terminal ancestor | `rejected` with `under_terminal_ancestor`, and that id is **not** written | +| E | Apply posting a genuinely foreign id | still `not_a_descendant` — the new reason did not swallow the old one | + +The rest of `test_cascade_db.py` must pass untouched. + +### 4. Documentation — record the reversal, do not overwrite it + +- `plans/260822-cascade-complete-sub-items/plan.md`: append a dated amendment under Decision 5 + rather than editing the decision text. That plan is the record of what was decided in August; + rewriting it makes the reversal invisible and leaves the shipped tests looking wrong. +- `docs/FORK.md` cascade_ext entry: the sentence _"are still traversed through, so a cancelled node + cannot hide its own live descendants"_ is now **false** and must be replaced, not amended around. + The new sentence states the opposite and says why. +- `CLAUDE.md` cascade_ext bullet: same sentence, same fix. +- The MCP-side docstring in `plane-mcp-server` carries the same claim; that is a sibling-repo change + and rides Phase 4's propagation. + +Grep before declaring this done — the old wording was quoted in more than one place: + +```bash +grep -rn "traversed through\|still traversed\|traverse through" docs/ CLAUDE.md plans/ apps/api/plane/cascade_ext/ +``` + +## Risk + +| Risk | L | I | Score | Mitigation | +| -------------------------------------------------------------------------- | --- | --- | ----- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| A user relied on sweeping past a cancelled parent | 3 | 3 | 9 | Deliberate, user-directed reversal. Named in the FORK.md and CLAUDE.md entries as a behavior change with its date, so a surprised reader finds the answer rather than filing a bug. | +| The two inverted tests get "fixed" back later by someone reading them cold | 3 | 3 | 9 | The inline comment naming this phase and date is the mitigation, and is the reason the rename is part of the diff — `..._and_prunes_its_subtree` states the rule in the test's own name. | +| `under_terminal_ancestor` parent-walk becomes a hot path | 1 | 2 | 2 | Runs only for ids already rejected; bounded by `MAX_DEPTH`. | + +## Success criteria + +- `pytest apps/api/plane/cascade_ext/tests/test_cascade_db.py` green, with exactly the two inverted + assertions and the two renames as the only edits to pre-existing tests. +- The grep above returns no surviving statement of the old rule. +- `python manage.py check` clean; no migration. diff --git a/plans/260828-module-cascade-terminal-status/phase-1-module-cascade-backend.md b/plans/260828-module-cascade-terminal-status/phase-1-module-cascade-backend.md new file mode 100644 index 000000000..f74e30ad4 --- /dev/null +++ b/plans/260828-module-cascade-terminal-status/phase-1-module-cascade-backend.md @@ -0,0 +1,266 @@ +# Phase 1 — `cascade_ext` backend: module preview + apply + +**Effort:** M (4h) · **Depends on:** Phase 0 · **Blocks:** Phases 2, 3 +**Plan:** `plans/260828-module-cascade-terminal-status/plan.md` +**Plane:** [PLANE-191](https://plane.the1studio.org/infrastructure/projects/2eae4e83-f715-4e4b-8753-cdc289bbe37f/issues/0cc383b3-e4fd-4a5d-a554-dca9a8323442) + +## Goal + +Two endpoints on the **existing** `cascade_ext` app that answer _"what would cascade if this module +went terminal?"_ and apply a caller-selected subset atomically with the module's own status write. +No new app, no new model, no migration, no core view edited, **no touch-point edit** — `cascade_ext` +is already in `INSTALLED_APPS`, already mounted at `apps/api/plane/urls.py:44`, and already in +`forkApps`. + +## Ownership + +``` +apps/api/plane/cascade_ext/service.py # refactor + two new functions +apps/api/plane/cascade_ext/views.py # two new endpoint classes +apps/api/plane/cascade_ext/urls.py # two new paths +apps/api/plane/cascade_ext/tests/test_module_cascade.py # NEW +``` + +Nothing outside `apps/api/plane/cascade_ext/`. If a diff touches anything else, stop — the design +went wrong. + +## Endpoint contract (freeze this FIRST — Phase 2 codes against it, verbatim) + +**Preview** — `GET /api/cascade-ext/workspaces//projects//modules//cascade-preview/?status=` + +```json +{ + "target_group": "completed", + "depth_capped": false, + "over_cap": false, + "cap": 100, + "summary": { + "total_live": 47, + "eligible": 44, + "ineligible": 3, + "already_terminal": 12 + }, + "items": [ + { + "id": "", + "identifier": "PLANE-42", + "name": "…", + "depth": 0, + "is_module_member": true, + "project_id": "", + "project_name": "Plane", + "state_id": "", + "state_name": "In Progress", + "state_group": "started", + "target_state_id": "", + "eligible": true, + "reason": null + } + ] +} +``` + +- The query parameter is `status` (a module status), **not** `group` — the per-issue endpoint's + parameter is `group` and the two must not be confused. Valid values: `completed`, `cancelled`. + Anything else → 400. `target_group` in the response is the resulting `State.group`, which for + these two happens to be the same string; it is emitted separately so the client never has to + assume the mapping. +- `depth: 0` means a direct module member; `depth: N` a descendant N levels below one. + `is_module_member` is emitted explicitly rather than inferred from `depth == 0`, because a + descendant may _also_ be a module member and the client groups the summary by membership. +- `reason` ∈ `null | "no_matching_state" | "no_permission"`. +- Ineligible rows are **included** with `eligible: false` (M10). Already-terminal work items are + **excluded** from `items` and **prune their whole subtree** (M8, established for both subjects by + Phase 0) — nothing beneath them is listed, walked, or changed. `summary.already_terminal` counts + the terminal nodes actually encountered, not the items hidden behind them, which are never + visited and therefore uncountable. +- `summary.total_live == len(items)` whenever `over_cap` is false. When `over_cap` is true, + `items` is `[]` and `total_live` still reports the real number (M4) — the client renders the + refusal from the summary alone. +- `identifier` is `project.identifier + "-" + issue.sequence_id`, built server-side. +- Archived module → **400** (M13), not 404. + +**Apply** — `POST /api/cascade-ext/workspaces//projects//modules//cascade-apply/` + +```json +{ "status": "completed", "item_ids": ["", "…"] } +``` + +- `item_ids` omitted or `null` ⇒ every currently-eligible item. An explicit `[]` ⇒ nothing + cascades; only the module's status moves. (Mirrors Decision 14 — the UI always sends an explicit + list, a headless MCP caller omits it.) +- Response: `{ "module": "", "status": "completed", "updated": [...], "rejected": [{"id": "…", "reason": "…"}] }` +- `rejected[].reason` ∈ `"no_matching_state" | "no_permission" | "already_terminal" | "under_terminal_ancestor" | "not_in_module_tree" | "not_eligible"`. `under_terminal_ancestor` is Phase 0's reason, reused verbatim rather than re-derived. +- Over cap → **400** `{"error": "cascade exceeds MAX_MODULE_CASCADE_ITEMS", "total_live": 240, "cap": 100}`. Nothing is written, including the module's status — the caller falls back to a plain PATCH. +- Archived module → **400**. +- Permissions: preview `[ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST]` (same read gate as viewing the + module); apply `[ROLE.ADMIN, ROLE.MEMBER]` (the roles that may write a module, matching + `ModuleViewSet.partial_update`). + +## Implementation + +### 1. Refactor `collect_descendants` onto a seed set — without changing its contract + +`service.py` currently walks from one `root_issue`. Extract the walk: + +```python +def _collect_from_seeds(*, seed_ids, include_seeds, target_group, actor_id) -> dict: + """Level-order BFS from a SET of seed issue ids. + + include_seeds=False -> seeds are the roots, excluded from the result + (the per-issue parent/sub-item case). + include_seeds=True -> seeds are themselves candidates at depth 0 + (the module-member case). + """ +``` + +and keep the public signature intact: + +```python +def collect_descendants(*, root_issue, target_group, actor_id) -> dict: + return _collect_from_seeds( + seed_ids={root_issue.id}, include_seeds=False, + target_group=target_group, actor_id=actor_id, + ) +``` + +**This function's behavior is frozen as Phase 0 leaves it.** `tests/test_cascade_db.py` must pass +with **no assertion edited by this phase** — Phase 0's two inversions are already in the tree, and +this refactor must not touch a third. That is the acceptance gate for the refactor (risk-15 in the +plan). Keep its +returned key names (`descendants`, `depth_capped`, `traversed_ids`) exactly as they are; the module +path adapts on top rather than renaming them underneath the issue path. + +Preserved verbatim from the existing implementation: `visited` set, `MAX_DEPTH = 20`, +`Issue.issue_objects` (never the default manager, so soft-deleted and triage rows stay out), +`select_related("state", "project")`, one query per level, one `State` query for all touched +projects keeping the lowest-`sequence` state per project, one `ProjectMember` query for all touched +projects, and — as of Phase 0 — **prune-at-terminal**: a terminal node is recorded in +`traversed_ids` and its children are never enqueued. Both subjects share that one rule; there is no +per-caller flag for it, deliberately, because a flag is how the two features drift back apart. + +When `include_seeds=True`, seed rows are fetched with the same `Issue.issue_objects` + +`select_related` and enter the node list at `depth: 0`; `visited` is seeded with every seed id so a +`parent` cycle among members cannot loop. + +### 2. `collect_module_cascade` + +```python +MAX_MODULE_CASCADE_ITEMS = 100 + +MODULE_STATUS_TO_STATE_GROUP = { + "completed": StateGroup.COMPLETED.value, + "cancelled": StateGroup.CANCELLED.value, +} + +def collect_module_cascade(*, module, target_group, actor_id) -> dict: +``` + +1. Seed ids — the canonical query, soft-delete-aware on **both** sides: + + ```python + seed_ids = set( + Issue.issue_objects.filter( + issue_module__module_id=module.id, + issue_module__deleted_at__isnull=True, + ).values_list("id", flat=True) + ) + ``` + + `ModuleIssue` uses `related_name="issue_module"` on both FKs — from `Issue` the reverse accessor + is `issue_module`. Do not reach for `module.issue_module.all()` and map to `.issue`; that + bypasses `issue_objects` and lets soft-deleted and draft rows in. + +2. `_collect_from_seeds(seed_ids=seed_ids, include_seeds=True, …)`. +3. Stamp `is_module_member = node["id"] in {str(i) for i in seed_ids}` on every row. +4. Build `summary`. `already_terminal` is the count of terminal nodes actually encountered + (seeds included) — **not** a subtraction of `len(items)` from a traversal total, which under + Phase 0's pruning would silently report zero for a branch nobody walked. +5. If `summary["total_live"] > MAX_MODULE_CASCADE_ITEMS`: set `over_cap=True`, replace `items` with + `[]`, keep the summary. **Do not** truncate the list — a truncated list silently under-reports + what a confirm would do, which is the failure mode the cap exists to avoid. + +Empty module → `seed_ids` empty → return the zero-summary shape immediately; do not run the BFS. + +### 3. `apply_module_cascade` + +```python +def apply_module_cascade(*, module, status, item_ids, actor_id, slug, origin) -> dict: +``` + +1. `target_group = MODULE_STATUS_TO_STATE_GROUP[status]` (the caller has already validated + `status`). +2. `collected = collect_module_cascade(...)`. If `over_cap` → raise a `CascadeCapExceeded` the view + turns into the 400 above. **Raise before opening the transaction** so nothing, including the + module's status, is written. +3. Re-derive `eligible_ids` from `collected` — `item_ids` is a request, never an authorization. + `item_ids is None` ⇒ all eligible; `[]` ⇒ none. Everything requested-but-not-eligible lands in + `rejected` with the reason from its node, `already_terminal` if it was traversed, + `under_terminal_ancestor` if Phase 0's parent-chain check places it behind a pruned branch, or + `not_in_module_tree` if it was never seen. +4. One `transaction.atomic()` block containing **both** writes (M5): + - `module.status = status; module.updated_at = now; module.save(update_fields=["status", "updated_at"])` + - the accepted issues, in 100-row `bulk_update(["state_id", "updated_at"])` batches. `bulk_update` + skips `save()`, so `updated_at` is assigned explicitly — same as the issue path. +5. **After** the block commits, dispatch: + - `model_activity.delay(model_name="module", model_id=str(module.id), requested_data={"status": status}, current_instance=json.dumps({"status": old_status}), actor_id=actor_id, slug=slug, origin=origin)` — this is the behavior the core viewset would have fired and which this endpoint bypasses (`app/views/module/base.py:708-716`). + - per accepted item, `issue_activity.delay(type="issue.activity.updated", …, notification=False, origin=origin)` **and** `model_activity.delay(model_name="issue", …)`, exactly as `apply_cascade` does. `notification=False` is load-bearing (M11), not a typo. + + There is no module equivalent of `issue_activity` for a status change — `ACTIVITY_MAPPER` only + carries `module.activity.created` / `module.activity.deleted`, both for `ModuleIssue` add/remove. + `model_activity` is the whole module-side dispatch; do not invent a new mapper key. + +### 4. Views + +Two `BaseAPIView` subclasses in `views.py`, thin — every rule lives in `service.py` so preview and +apply cannot disagree: + +- Validate `status` against `{"completed", "cancelled"}` → 400. +- `Module.objects.filter(pk=module_id, project_id=project_id, workspace__slug=slug).first()` → 404 + if absent. +- `if module.archived_at: 400` (M13). +- Apply additionally: `item_ids` must be a list, `null`, or omitted → else 400. +- `origin=base_host(request=request, is_app=True)`, as the issue endpoints do. + +### 5. URLs + +Two appended paths in the existing `urlpatterns`. Keep the `` converter consistent +with the issue routes' ``. + +## Tests — `apps/api/plane/cascade_ext/tests/test_module_cascade.py` + +Follow `test_cascade_db.py`'s existing style and base class. + +| # | Case | Asserts | +| --- | ---------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------- | +| 1 | Empty module | `summary.total_live == 0`, `items == []`, zero BFS queries | +| 2 | Flat module, mixed states | only non-terminal members listed, `depth == 0`, `is_module_member` true | +| 3 | Member with a 3-level subtree | every level listed, `depth` 0/1/2, `is_module_member` true only at 0 | +| 4 | Member already `completed`, its child live | member excluded, `already_terminal == 1`, child **absent** — the member prunes its subtree (Phase 0) | +| 5 | Cancelling a module with a completed member | completed member untouched (M8), live members get the _cancelled_ state | +| 6 | Cross-project sub-item | resolved to its own project's state in the group, not the module's project's | +| 7 | Renamed target state (`Done` → `Shipped`) | still resolved by `group`, never by name | +| 8 | Sub-item in a project the actor is not an active member of | listed, `eligible: false`, `reason == "no_permission"` | +| 9 | Project with no state in the target group | `reason == "no_matching_state"` | +| 10 | `parent` cycle among two module members | terminates, no duplicate rows | +| 11 | Apply with `item_ids=None` | every eligible item moves | +| 12 | Apply with `item_ids=[]` | **only** the module's status moves; zero issue writes | +| 13 | Apply posting an ineligible id | that id lands in `rejected` with its reason; no write for it | +| 14 | Apply posting an id from a different module | `rejected` with `not_in_module_tree` | +| 14b | Apply posting a live id beneath a terminal member | `rejected` with `under_terminal_ancestor`; not written | +| 15 | Apply raises mid-`bulk_update` (patched) | module status **rolled back** — the atomicity gate for M5 | +| 16 | 101 live items | preview `over_cap: true`, `items == []`, `total_live == 101`; apply → 400 and **module status unchanged** | +| 17 | Archived module | preview and apply both 400 | +| 18 | Non-terminal status (`in-progress`) | preview 400 | +| 19 | Activity dispatch | one `model_activity` for the module; per item one `issue_activity` with `notification=False` and one `model_activity` | +| 20 | Query count on a 50-item module | one query per BFS level plus the fixed 2 (states, memberships) — asserts no N+1 crept in | +| 21 | **Regression** | `test_cascade_db.py` passes unedited | + +## Success criteria + +- `pytest apps/api/plane/cascade_ext/tests/` green, including `test_cascade_db.py` with **zero** + assertion edits. +- `python manage.py makemigrations --check --dry-run` reports no changes. +- `python manage.py check` clean. +- `git diff --name-only` touches only paths in § Ownership. +- The contract above is byte-identical to what Phase 2 codes against. diff --git a/plans/260828-module-cascade-terminal-status/phase-2-cascade-package.md b/plans/260828-module-cascade-terminal-status/phase-2-cascade-package.md new file mode 100644 index 000000000..ff1661fd3 --- /dev/null +++ b/plans/260828-module-cascade-terminal-status/phase-2-cascade-package.md @@ -0,0 +1,178 @@ +# Phase 2 — `packages/cascade-ext`: module client, guard, and the shared modal's summary mode + +**Effort:** M (3h) · **Depends on:** Phase 1's frozen endpoint contract (not on Phase 1 merging) +**Blocks:** Phase 3 +**Plan:** `plans/260828-module-cascade-terminal-status/plan.md` +**Plane:** [PLANE-192](https://plane.the1studio.org/infrastructure/projects/2eae4e83-f715-4e4b-8753-cdc289bbe37f/issues/a3af24bb-944f-4bae-8a9a-39960783b12f) + +## Goal + +Everything the module cascade needs on the client, inside the fork-owned package — so Phase 3's +core edit is a handful of lines rather than a feature. + +## Ownership + +``` +packages/cascade-ext/src/types.ts # extend +packages/cascade-ext/src/strings.ts # extend +packages/cascade-ext/src/cascade-service.ts # extend +packages/cascade-ext/src/should-prompt-cascade.ts # extend +packages/cascade-ext/src/cascade-confirm-store.ts # extend +packages/cascade-ext/src/cascade-confirm-modal.tsx # extend +packages/cascade-ext/src/index.ts # re-export +packages/cascade-ext/src/__tests__/** # extend + new +``` + +Nothing outside `packages/cascade-ext/`. No `@plane/i18n` edit (M15) — strings stay local. + +## Integration contract + +The Phase 1 § "Endpoint contract" block is the contract. Copy it verbatim into this phase's working +notes; do not paraphrase field names or casing. The two paths, restated for the fetch client: + +``` +GET /api/cascade-ext/workspaces/{slug}/projects/{projectId}/modules/{moduleId}/cascade-preview/?status={completed|cancelled} +POST /api/cascade-ext/workspaces/{slug}/projects/{projectId}/modules/{moduleId}/cascade-apply/ + body: { status, item_ids } +``` + +**TRAP, inherited from `cascade-service.ts`:** cascade-ext mounts at `/api/cascade-ext/`, outside +`/api/v1`. The existing service already handles this; reuse its base-URL derivation rather than +composing a new one. + +## Implementation + +### 1. `types.ts` + +```ts +export type TModuleCascadeStatus = "completed" | "cancelled"; + +export type TCascadeItem = TCascadeDescendant & { + is_module_member: boolean; +}; + +export type TModuleCascadeSummary = { + total_live: number; + eligible: number; + ineligible: number; + already_terminal: number; +}; + +export type TModuleCascadePreviewResponse = { + target_group: TCascadeStateGroup; + depth_capped: boolean; + over_cap: boolean; + cap: number; + summary: TModuleCascadeSummary; + items: TCascadeItem[]; +}; + +export type TModuleCascadeApplyResponse = { + module: string; + status: TModuleCascadeStatus; + updated: string[]; + rejected: TCascadeApplyRejection[]; +}; +``` + +`TCascadeApplyRejection["reason"]` widens to include +`"already_terminal" | "under_terminal_ancestor" | "not_in_module_tree"`. The first two also reach the +issue flow as of Phase 0, so widen the shared type rather than forking a module-only one. + +### 2. `should-prompt-cascade.ts` — add the module guard + +```ts +export const shouldPromptModuleCascade = ({ + data, + totalIssues, +}: { + data: Partial; + totalIssues: number; +}): TModuleCascadeStatus | null => + data.status === "completed" || data.status === "cancelled" ? (totalIssues > 0 ? data.status : null) : null; +``` + +Two properties this must hold, both tested: + +- A payload that does **not** carry `status` returns `null` — that is what keeps a name-only edit + on an already-completed module from firing a preview request (plan risk row). +- The guard deliberately does **not** subtract `completed_issues`/`cancelled_issues` from + `total_issues`. Those counts cover direct members only, so the arithmetic does not describe the + set the server will actually walk. See decision M6 — this is the cheaper guard's correctness hole, + not an oversight. Put that reasoning in a comment on the function; the next reader will otherwise + "optimize" it back. (Under M8's pruning a module whose members are all terminal now genuinely has + nothing to cascade — but the guard must not encode that, because the server owns the rule and a + client-side copy of it is the next thing to drift.) + +### 3. `cascade-service.ts` — two methods + +`getModulePreview(workspaceSlug, projectId, moduleId, status)` and +`applyModuleCascade(workspaceSlug, projectId, moduleId, status, itemIds)`. Same auth, same +`CascadeApiError` on non-2xx, same base-URL handling as the issue methods. The apply method sends +`item_ids` as an explicit array; it never omits the key (omission means "all", which the UI must +never request implicitly). + +### 4. `cascade-confirm-store.ts` — one request shape, two sources + +Widen `pendingRequest` rather than adding a second store: the modal is one component and two +stores would need two mount points in `root.tsx`. + +```ts +type TCascadeSubject = + | { kind: "issue"; parentIdentifier: string } + | { kind: "module"; moduleName: string; summary: TModuleCascadeSummary; overCap: boolean; cap: number }; +``` + +`requestModuleCascade({ moduleName, targetGroup, items, summary, overCap, cap })` returns the same +`Promise` (`{ cascade: boolean; childIds: string[] }`) so Phase 3's call site +matches the shipped issue one. + +When `overCap` is true the store pre-sets `checkedIds` to empty and the modal renders refusal mode +(below); resolving that promise always yields `{ cascade: false, childIds: [] }`. + +The `cascadeConfirmStore` singleton stays exported **from this package**. Do not move it into a +store file — the existing header comment in `base-issues.store.ts` records that creating it there +closed an import cycle and crashed the SSR prerender with _"Cannot access 'BaseIssuesStore' before +initialization"_. + +### 5. `cascade-confirm-modal.tsx` — summary header + collapsible list + +One component, three additions: + +- **Summary header**, always rendered. Issue subject: the existing sentence. Module subject: the + counts — _"47 work items will be completed · 12 already done · 3 you cannot change."_ Zero-valued + clauses are omitted, not rendered as "0". "Already done" counts terminal items the walk reached; + it is not a total of everything skipped, because items behind a pruned branch are never visited + (M8). Do not word the string as if it were a total. +- **Collapsible list.** `LIST_COLLAPSE_THRESHOLD = 15`. At or below the threshold the list renders + expanded exactly as today, so the shipped issue flow is visually unchanged in every realistic + case. Above it, the list starts collapsed behind a _"Show all 47 items"_ disclosure, with + select-all / select-none above it. +- **Refusal mode** (`overCap`). No list, no checkboxes, no "Change work items too" button. Body: + _"This module has 240 work items — more than the 100 this action can change at once. The module's + status will still change."_ Single button: **"Only change this module."** + +Unchanged and load-bearing: the `setTimeout` focus on **"Only change this item / module"**, so a +stray Enter never cascades. + +### 6. `index.ts` + +Re-export `shouldPromptModuleCascade`, the module service methods, the new types, and the widened +store API. + +## Tests + +| File | Cases | +| ------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `__tests__/should-prompt-cascade.test.ts` | module guard: `completed`+`total_issues>0` → `"completed"` · `cancelled` → `"cancelled"` · `in-progress` → null · `total_issues===0` → null · **payload without `status`** → null · payload with `status` unchanged from current but present → still fires (the server decides no-ops) | +| `__tests__/cascade-confirm-store.test.ts` | `requestModuleCascade` resolves on confirm and on dismiss · over-cap request resolves `{cascade:false, childIds:[]}` · select-all / select-none · the issue-path tests pass **unedited** | +| `__tests__/cascade-confirm-modal.test.tsx` | 3 items → list expanded, no disclosure · 40 items → collapsed, disclosure shows the count, expands on click · over-cap → no cascade button, cap and total both rendered · focus lands on the "only change" button in every mode · zero-valued summary clauses absent | +| `__tests__/cascade-service.test.ts` (new) | module preview/apply hit the `/api/cascade-ext/…/modules/…` paths, not `/api/v1/…` · non-2xx raises `CascadeApiError` · apply always sends `item_ids` as an array | + +## Success criteria + +- `pnpm --filter @plane/cascade-ext test` green, with the **pre-existing** issue-path tests + unedited. +- `pnpm check` clean. +- `git diff --name-only` touches only `packages/cascade-ext/`. +- Rendering an issue cascade with ≤15 descendants is visually identical to the shipped modal. diff --git a/plans/260828-module-cascade-terminal-status/phase-3-wire-module-store.md b/plans/260828-module-cascade-terminal-status/phase-3-wire-module-store.md new file mode 100644 index 000000000..153ba7662 --- /dev/null +++ b/plans/260828-module-cascade-terminal-status/phase-3-wire-module-store.md @@ -0,0 +1,132 @@ +# Phase 3 — Fence `module.store.ts` and register the exception + +**Effort:** S (2h) · **Depends on:** Phases 1 and 2 (serial) +**Plan:** `plans/260828-module-cascade-terminal-status/plan.md` +**Plane:** [PLANE-193](https://plane.the1studio.org/infrastructure/projects/2eae4e83-f715-4e4b-8753-cdc289bbe37f/issues/d228efc0-e1c2-449e-8f53-c8937e6789c2) + +## Goal + +One fenced block at the single choke point every module status write funnels through, plus its +`docs/FORK.md` exception row. This is the only core file the feature touches. + +## Ownership + +``` +apps/web/core/store/module.store.ts # ONE fenced block + one fenced import — registered core exception +docs/FORK.md # exception row + cascade_ext entry update +``` + +`apps/web/app/root.tsx` needs **no** change: `` +is already mounted there (touch-point 7, already registered), and Phase 2 reuses that same store +and component rather than adding a second host. + +## Why this one file + +Five UI entry points can change a module's status, and all five call the same store method: + +| Entry point | File:line | +| ------------------------ | ------------------------------------------------------------------ | +| Module list row | `components/modules/module-list-item-action.tsx:112` | +| Module grid card | `components/modules/module-card-item.tsx:125` | +| Analytics sidebar (peek) | `components/modules/analytics-sidebar/root.tsx:82` | +| Create/update modal | `components/modules/modal.tsx:82` | +| Power-K palette | `components/power-k/ui/pages/context-based/module/commands.tsx:45` | + +All five reach `updateModuleDetails` — `apps/web/core/store/module.store.ts:431`. The gantt layout +(`gantt-chart/modules-list-layout.tsx:38,51`) also calls it, but only ever with +`sort_order` / `start_date` / `target_date`, so the `data.status` guard makes it a no-op there for +free. `SidebarStatusSelect` (`sidebar-select/select-status.tsx:28`) is exported but has no callers — +do not add a fence for it. + +`apps/space` has no module write path at all. Nothing to guard. + +## Implementation + +Guard at the **top** of `updateModuleDetails`, before the optimistic `set` — a module the user +declines to cascade still gets its status written by the plain PATCH below, so the optimistic write +is correct either way, but the preview must not race it. + +```ts +// The1Studio fork (module-cascade) — see docs/FORK.md § "Cascade a module's terminal status". +// Every module status write funnels through this method (list row, grid card, analytics sidebar, +// create/update modal, power-K), so one guard here covers all five with no duplicate fence. +const cascadeModule = this.getModuleById(moduleId); +const cascadeStatus = shouldPromptModuleCascade({ + data, + totalIssues: cascadeModule?.total_issues ?? 0, +}); +if (cascadeStatus) { + const preview = await cascadeService.getModulePreview(workspaceSlug, projectId, moduleId, cascadeStatus); + const eligible = preview.items.filter((i) => i.eligible); + if (preview.over_cap || eligible.length > 0) { + const choice = await cascadeConfirmStore.requestModuleCascade({ + moduleName: cascadeModule?.name ?? moduleId, + targetGroup: preview.target_group, + items: preview.items, + summary: preview.summary, + overCap: preview.over_cap, + cap: preview.cap, + }); + if (choice.cascade && choice.childIds.length > 0) { + // applyModuleCascade writes the module's status INSIDE its own transaction — falling through + // to the plain patchModule below would write it a second time, outside that transaction. + await cascadeService.applyModuleCascade(workspaceSlug, projectId, moduleId, cascadeStatus, choice.childIds); + // total_issues / completed_issues / cancelled_issues drive the progress ring rendered right + // beside the status control, and the cascade moved them server-side. + await this.fetchModuleDetails(workspaceSlug, projectId, moduleId); + return; + } + } +} +// Every other case — no cascade status, an empty or all-ineligible preview, "only change this +// module", or zero ticked items — falls through unchanged to the optimistic set + patchModule below. +// end The1Studio fork (module-cascade) +``` + +Three things to get right, each a real failure if missed: + +1. **`over_cap ||` in the modal condition.** Over cap, `items` is `[]` and `eligible.length` is + zero — an `eligible.length > 0`-only condition would skip the refusal modal entirely and silently + complete a 600-item module's status with no explanation of why nothing cascaded. +2. **The early `return` after apply.** Falling through would PATCH the module a second time, outside + the transaction. The shipped issue fence carries the same comment for the same reason. +3. **Preview failures must not block the status change.** Wrap the preview call so a 4xx/5xx from + `cascade-ext` (an older server, a deploy skew) logs and falls through to the plain PATCH. A fork + add-on being unreachable must never break a core action. + +Confirm the exact `fetchModuleDetails` method name against the store before writing the call — +`module.store.ts` also exposes `fetchModules`; the per-module refetch is the cheaper one. + +## `docs/FORK.md` + +1. Add a row to the exception table in the cascade block: + + | File | What | Why no seam | + | ------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------- | + | `apps/web/core/store/module.store.ts` | At the top of `updateModuleDetails` — `shouldPromptModuleCascade` guard → `cascadeService.getModulePreview` → (if any row is eligible, or the preview is over cap) `cascadeConfirmStore.requestModuleCascade` → on a cascade choice with ticked items, `cascadeService.applyModuleCascade`, a `fetchModuleDetails` refetch, and an early `return` so the plain `patchModule` below never double-writes the module. Every other case falls through unchanged. | No upstream pre-update hook on the module store, and `updateModuleDetails` is the one method all five status entry points funnel through | + +2. Extend the existing `cascade_ext` entry to say the app now serves **two** subjects — issue trees + and module contents — sharing one BFS/eligibility core, and note `MAX_MODULE_CASCADE_ITEMS = 100` + as a refusal, not a truncation. + +3. Add `apps/web/core/store/module.store.ts` to the block's "Rebase handling" note as an expected + conflict point where the fork block is re-applied, not abandoned. + +## Verification + +- `pnpm check` clean. +- `node .claude/scripts/plane-classify-path.cjs` over the full diff — every path classifies `fork` + or as a **registered** exception. Zero unregistered-core results. +- Manual, all five entry points: status → `completed` on a module with live work → modal opens · + "Only change this module" → module moves, no work item moves · "Change work items too" → both + move **and the progress ring updates without a page reload** · status → `in-progress` → no + preview request in the network tab · rename a completed module without touching status → no + preview request · empty module → no modal · module whose items are all done → no modal · + 240-item module → refusal modal, status still changes · `cascade-ext` returning 500 (block it in + devtools) → status change still succeeds. + +## Success criteria + +- One fenced block and one fenced import in `module.store.ts`; nothing else in `apps/web/` touched. +- `docs/FORK.md` carries the exception row before the PR is opened, not after. +- The network tab shows **zero** extra requests for every non-terminal status change. diff --git a/plans/260828-module-cascade-terminal-status/phase-4-propagate.md b/plans/260828-module-cascade-terminal-status/phase-4-propagate.md new file mode 100644 index 000000000..924058d48 --- /dev/null +++ b/plans/260828-module-cascade-terminal-status/phase-4-propagate.md @@ -0,0 +1,119 @@ +# Phase 4 — Propagate to downstream surfaces + +**Effort:** S (1.5h) · **Depends on:** Phase 1 (the endpoints must exist) +**Plan:** `plans/260828-module-cascade-terminal-status/plan.md` +**Plane:** [PLANE-194](https://plane.the1studio.org/infrastructure/projects/2eae4e83-f715-4e4b-8753-cdc289bbe37f/issues/e8fa10c2-129b-47aa-95b3-301024efa15b) + +## Goal + +Satisfy `CLAUDE.md`'s STANDING RULE — every new endpoint reaches its downstream siblings before the +feature is done. Two new URL patterns on a fork-owned app classify as a **non-generic endpoint** +under `.claude/skills/plane-propagate/references/sibling-repos.md` § "Classification Rule", which +selects the full tier. + +## Ownership + +``` +CLAUDE.md # extend the cascade_ext "Custom features" entry +.claude/plane-propagation-queue.md # runtime state, written by the propagate tooling +plane-mcp-server: plane_mcp/tools/** # SEPARATE REPO, SEPARATE PR +``` + +**Never edit a sibling repo from this repo's PR** (`.claude/rules/plane-fork-discipline.md`, +`rules/kit-pr-workflow-boundary.md`). Everything below that is not `CLAUDE.md` is an issue opened +in the sibling repo, or a PR raised from inside a clone of it. + +## 1. Run `plane-propagate` + +Classification: **non-generic endpoint** — two new URL patterns +(`.../modules//cascade-preview/`, `.../cascade-apply/`) on the fork-owned `cascade_ext` +app, unreachable through any generic issue endpoint. That resolves to: + +| Repo | Issue content | +| ------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `plane-mcp-server` | `preview_module_cascade` tool spec + `cascade` parameter on `update_module` (detail below), **and** the Phase 0 docstring correction: `plane_mcp/tools/cascade_ext.py`'s module docstring and `update_work_item`'s help still state the old skip-but-traverse rule, which is now false | +| `plane-node-sdk` | TS bindings for both routes: method, URL, request/response shape from Phase 1's contract | +| `plane-python-sdk` | The same bindings in Python | +| `plane-claude-plugin` | User-facing: "complete a module and everything in it", naming the 100-item refusal | +| `docs` | User-facing page: what the modal does, what "only change this module" means, the cap | +| `developer-docs` | API reference for both endpoints, auth, the `reason` enum, the 400 shapes | +| `plane-deploy` / `helm-charts` | **Not applicable** — no new env var, no new service or container | + +`MAX_MODULE_CASCADE_ITEMS = 100` is a hardcoded constant, deliberately not an env var. Making it +configurable would put `plane-deploy` and `helm-charts` in scope for a value nobody has asked to +tune; revisit only if a real workspace hits it. + +## 2. MCP tools (separate PR in `plane-mcp-server`) + +### `preview_module_cascade` + +Mirrors the existing `preview_work_item_cascade`. Signature: + +```python +preview_module_cascade(workspace_slug, project_id, module_id, status) # status: completed|cancelled +``` + +Reuse `plane_mcp/tools/cascade_ext.py`'s existing `_send` helper — it already strips the `/api/v1` +suffix from `client.config.base_path` before composing `/api/cascade-ext{path}`. **That suffix strip +is the trap the module tools inherit:** composing against `base_path` directly yields +`/api/v1/cascade-ext/…` and 404s. The file's module docstring already records this for the issue +routes; extend it to name the module routes rather than writing a second explanation of the same +trap. + +### `update_module(..., cascade=False)` + +Mirrors `update_work_item(..., cascade=False)` exactly: + +- `cascade=False` (the default) → the existing plain SDK `patch`, byte-for-byte unchanged. +- `cascade=True` **and** the resolved status is `completed`/`cancelled` → call `cascade-apply` + instead, omitting `item_ids` so the server takes every currently-eligible item (the contract's + documented headless path). +- `cascade=True` with a non-terminal status → plain patch. Not an error; there is simply nothing to + cascade. + +`update_module` already validates `status` against `ModuleStatusEnum` via `get_args` and **coerces +an unrecognized value to `None`** (`plane_mcp/tools/modules.py:166-176`). Read that coercion before +wiring the branch: a caller passing `status="Completed"` gets `validated_status = None`, so the +cascade branch must key off the _validated_ value, never the raw argument, or it fires a cascade for +a status the patch is not setting. + +### Docstrings + +State plainly in both tools: a plain `update_module` **never** cascades, from any client (M12). Name +the 100-item cap and that exceeding it is a **400 refusal, not a partial apply** — a headless caller +has no modal to read the refusal from and will otherwise read the 400 as transport failure and +retry it. + +### Tests + +| Case | Asserts | +| --------------------------------------------------- | -------------------------------------------------------- | +| `update_module(status="completed")` | plain PATCH; **zero** cascade-ext calls | +| `update_module(status="completed", cascade=True)` | one `cascade-apply`; zero plain PATCHes | +| `update_module(status="in-progress", cascade=True)` | plain PATCH; zero cascade-ext calls | +| `update_module(status="Completed", cascade=True)` | coerced to `None` → no cascade fired | +| `preview_module_cascade` | hits `/api/cascade-ext/…/modules/…`, not `/api/v1/…` | +| over-cap 400 | surfaced as a readable error naming the cap, not retried | + +## 3. `CLAUDE.md` + +Extend the existing `cascade_ext/` bullet — do not add a second one; it is one app serving two +subjects, and **correct the sentence Phase 0 falsified** — "are still traversed through, so a +cancelled node cannot hide its own live descendants" is now the opposite of the truth. State: a +terminal item prunes its whole subtree, for both subjects, as of this change; module status → +work-item state cascade over module members **plus their descendants**; the module's own status is written inside the same transaction; the 100-item cap is +a refusal, not a truncation; `update_module(..., cascade=False)` is the default and is byte-for-byte +the old PATCH; the frontend guard lives in `module.store.ts` alongside the `base-issues.store.ts` +one. + +## Success criteria + +- `plane-propagate` has opened an issue in each repo the classification selects, and the queue entry + carries its `Propagated:` line. +- `pytest` green in `plane-mcp-server`; both tools resolve against a live fork server carrying + Phase 1. +- `update_module`'s default behavior is unchanged for every existing caller — the parameter is + additive with a `False` default, and the first test row is the gate for that. +- `CLAUDE.md`'s `cascade_ext/` entry describes both subjects and no longer asserts skip-but-traverse. +- `grep -rn "traversed through" CLAUDE.md docs/` returns nothing. +- Zero files changed in any sibling repo by this repo's PR. diff --git a/plans/260828-module-cascade-terminal-status/plan.md b/plans/260828-module-cascade-terminal-status/plan.md new file mode 100644 index 000000000..453ce216e --- /dev/null +++ b/plans/260828-module-cascade-terminal-status/plan.md @@ -0,0 +1,183 @@ +# Cascade a module's terminal status to everything in it + +**Created:** 2026-08-28 +**Branch:** `feat/module-cascade-terminal-status` +**Extends and partly AMENDS:** `plans/260822-cascade-complete-sub-items/` (the shipped per-issue +cascade) — Phase 0 reverses its Decision 5. +**Scope:** two new endpoints on the **existing** `cascade_ext` fork app + a widened +`packages/cascade-ext` + one new fenced core-frontend edit. No new Django app, no new model, no +migration, **no backend core edits, no new touch-point**. +**Plane:** [PLANE-189](https://plane.the1studio.org/infrastructure/projects/2eae4e83-f715-4e4b-8753-cdc289bbe37f/issues/f1318bf8-95f4-4642-b48f-2372e4bf7091) + +## Problem + +Setting a module to `completed` or `cancelled` leaves every work item in it untouched. The module +reads as finished while its work items sit in `In Progress`, the module's own progress ring +disagrees with its own status, and archiving is gated on `status in (completed, cancelled)` — so a +module can be archived with live work inside it. + +The per-issue cascade shipped for exactly this problem one level down (a parent work item and its +sub-items). This plan applies the same treatment one level up. + +## Prior art — the mechanism already exists; this reuses it + +Passes 1–3 run over `apps/api/plane/`, `apps/web/`, `apps/space/`, `packages/`: + +| Question | Answer | Evidence | +| --------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Does anything cascade a module's status onto its issues today? | **Zero across `apps/api/plane/app/views/module/`, `apps/api/plane/api/views/module.py`, `apps/api/plane/space/views/module.py`, `apps/api/plane/app/views/workspace/module.py`, `apps/api/plane/db/models/module.py`, `apps/api/plane/bgtasks/`, `apps/api/plane/ai_ext/signals.py`** | No `Issue.state` bulk-update is reachable from a `Module` write; the only such bulk-update in the repo is `bgtasks/issue_automation_task.py:131` (project-level stale-issue auto-close, unrelated) | +| Is there a descendant-collection + eligibility engine to reuse? | **Yes** — `plane/cascade_ext/service.py` `collect_descendants` / `apply_cascade` | Level-order BFS, `visited` set, `MAX_DEPTH = 20`, per-project target-state resolution by `State.group`, per-project membership gate, atomic parent+children write | +| Is there a confirm-modal + preview/apply client to reuse? | **Yes** — `packages/cascade-ext` | `cascade-service.ts`, `cascade-confirm-store.ts`, `cascade-confirm-modal.tsx`, `should-prompt-cascade.ts` | +| Is there an MCP surface to mirror? | **Yes** — `plane_mcp/tools/cascade_ext.py` + `update_work_item(cascade=False)` | `plane-mcp-server` repo | + +**Pass 4 (corpus sweep) — not applicable, recorded rather than skipped.** The studio +`knowledge-retrieval` corpus indexes the Unity/.NET and Cocos assemblies; it does not index this +Django/React monorepo, so it cannot answer prior-art questions about it. This finding is _not_ +`greenfield` in any case — the capability exists and this plan extends it, so the greenfield gate +never fires. + +Confirmed seams: + +| Seam | Location | +| ---------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `Module.status` (flat `CharField`, choices `backlog\|planned\|in-progress\|paused\|completed\|cancelled`, default `planned`) | `apps/api/plane/db/models/module.py:74-85` | +| `ModuleIssue` join (`related_name="issue_module"` on **both** FKs) | `apps/api/plane/db/models/module.py:152-171` | +| Canonical "issues in a module" query | `Issue.issue_objects.filter(issue_module__module_id=…, issue_module__deleted_at__isnull=True)` — `app/views/module/issue.py:84-92` | +| Module status write path (app) | `ModuleViewSet.partial_update` — `app/views/module/base.py:651-721`, `@allow_permission([ROLE.ADMIN, ROLE.MEMBER])` | +| Module status write path (public API) | `ModuleDetailAPIEndpoint.patch` — `api/views/module.py:402-450`, `ProjectEntityPermission` | +| Archived-module write rejection | `app/views/module/base.py:663-667`, `api/views/module.py:412-416` | +| Module webhook dispatch | `model_activity.delay(model_name="module", …)` — `app/views/module/base.py:708-716` | +| Frontend choke point | `updateModuleDetails` — `apps/web/core/store/module.store.ts:431` | +| Module PATCH service | `patchModule` — `apps/web/core/services/module.service.ts:58-69` | +| Free client-side counts on `IModule` | `total_issues`, `completed_issues`, `cancelled_issues`, `backlog_issues`, `started_issues`, `unstarted_issues` — `packages/types/src/module/modules.ts:56+` | +| Fork-app mount block | `apps/api/plane/urls.py:17-45` (`cascade_ext` already mounted at line 44) | + +`apps/space` is **read-only** for modules (`ProjectModulesEndpoint` is `GET`/`AllowAny`, returns +`{id, name}`; `apps/space/store/module.store.ts` has no update method). Nothing to guard there. + +## Decisions (resolved) + +| # | Decision | +| --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| M1 | **Extend `cascade_ext`; do not create a new app.** The eligibility rules (target state by `group`, per-project membership, prune-at-terminal, atomic write, `notification=False` on cascaded rows) are the _same_ rules — a second app would be a second copy that drifts. Extending also costs **zero** touch-point edits (`INSTALLED_APPS` and `urls.py` already carry `cascade_ext`) and zero `forkApps` / CI registration, both of which a new app would need. | +| M2 | **Cascade scope = every module member plus each member's full descendant subtree**, recursively — including sub-items that are not themselves module members. A member left with unfinished children reproduces the exact "finished at the top, unfinished underneath" problem the per-issue cascade exists to fix. | +| M3 | **The confirmation modal is the same modal.** `CascadeConfirmModal` grows a summary header (counts by outcome) and auto-collapses its row list above `LIST_COLLAPSE_THRESHOLD = 15`. Below the threshold the issue flow renders exactly as it does today, so this is not a behavior change for the shipped feature. | +| M4 | **Hard cap, explicit refusal.** `MAX_MODULE_CASCADE_ITEMS = 100` live nodes. "Live" is post-pruning (M8): terminal items and everything behind them are already gone before the count is taken, so the cap measures exactly what a confirm would write. Over the cap, preview returns `over_cap: true` with an **empty** `items` array (shipping 240 rows to a modal helps nobody) and apply returns **400** rather than half-applying. The module's own status still changes via the ordinary PATCH — only the cascade is refused. | +| M5 | **The apply endpoint writes the module's `status` too**, inside the same transaction as the issues — mirroring how `cascade-apply` writes the parent issue's state. A module marked complete whose issue writes then failed is the outcome atomicity exists to prevent. The endpoint re-implements the two behaviors the core viewset applies to a status write: reject if `archived_at` is set, and fire `model_activity(model_name="module", …)` on success. | +| M6 | **No cheap client-side skip; guard on `status ∈ {completed, cancelled}` AND `total_issues > 0` only.** The per-issue flow could skip the preview using `sub_issues_count` because a Done click is high-frequency. A module status change is not, and the tempting cheaper guard (`backlog + started + unstarted > 0`) is **wrong** under M2: those counts cover direct members only, so a module whose members are all terminal but whose sub-items are live would be skipped incorrectly. One request on a rare action buys correctness. | +| M7 | Terminal group is taken from the module's **new** status: `completed → completed`, `cancelled → cancelled`. A move between the two cascades the new one. Re-saving the same status is a no-op (nothing to enter). Any other status (`backlog`, `planned`, `in-progress`, `paused`) never cascades. | +| M8 | A work item already in **either** terminal group is never listed and never touched, **and prunes its entire subtree** — nothing beneath it is listed, walked, or changed. A terminal item is a decision someone made about that branch, and reaching past it overrides that decision silently. This **reverses** the shipped per-issue rule (`260822` Decision 5, "still traversed through"), so Phase 0 applies the same reversal to the issue cascade rather than letting the two subjects disagree on one `Issue.parent` tree. Cost, stated: a live sub-item under a cancelled parent is now left live where it used to be swept. | +| M9 | Cross-project descendants are included and resolved to **their own** project's state of the same `group`, never by name. Direct module members are always same-project (a `ModuleIssue` is project-scoped), but a member's sub-items need not be. (Mirrors Decision 7.) | +| M10 | A node whose project has no state in the target group, or whose project the actor is not an active member of, is listed **disabled with a reason** — not hidden, not silently skipped. (Mirrors Decision 8.) | +| M11 | Each cascaded work item gets its own activity row with `notification=False`. Only the module's own `model_activity` fires. A 200-item module must not fire 200 watcher notifications. (Mirrors Decision 10.) | +| M12 | **No implicit cascade for API/MCP callers.** A plain `PATCH {status: "completed"}` on a module never cascades, from any client. MCP gets `update_module(..., cascade=False)` — default `false`, mirroring the UI's "only change this module". | +| M13 | Archived modules refuse both preview and apply with 400, mirroring the core viewset's own refusal to write an archived module. | +| M14 | After a successful apply the client refetches the module, because `total_issues` / `completed_issues` / `cancelled_issues` drive the progress ring rendered right beside the status dropdown and would otherwise read stale. | +| M15 | Fork UI strings stay in `packages/cascade-ext/src/strings.ts`, English-only — `packages/i18n` is an upstream `@plane/*` package the fork rules forbid editing in place. (Mirrors Decision 12.) | + +## Explicitly out of scope + +- **Cycles.** They group work items the same way and deserve the same treatment, but that is a + second endpoint pair, a second store fence, and a reconciliation with the existing + `complete_cycle` action. The service layer below is written over a seed-id set rather than over + `Module`, so adding cycles later is a new caller, not a rewrite. +- Reverse cascade — reopening a module does not reopen its work items. +- Auto-completing a module once all its work items finish. +- Cascading any field other than work-item state. +- Module **archive** as a trigger. Archiving is already gated on the status being terminal, so the + cascade has run by then. +- `apps/space` — read-only for modules, nothing to guard. + +## Flow + +1. User changes a module's status (list row, grid card, analytics sidebar, create/update modal, or + the power-K palette — all five funnel through `updateModuleDetails`). +2. Guard: is the new status `completed`/`cancelled` **and** `total_issues > 0`? If not → plain + PATCH, unchanged behavior, zero extra requests. +3. `GET …/modules//cascade-preview/` → the flattened item list with eligibility per row, + plus summary counts and the over-cap flag. +4. Preview has no eligible row → plain PATCH, no modal. +5. Over cap → modal opens in refusal mode: the count, the cap, and only **"Only change this + module"**. No cascade path is offered. +6. Otherwise the modal opens with a summary header and (collapsed above 15 rows) the checkbox list. + Focus rests on **"Only change this module."** +7. "Only change this module" → plain PATCH. +8. "Change work items too" → one `POST …/cascade-apply/` carrying the new `status` and the ticked + ids. The server re-derives eligibility and applies module + items atomically, then the client + refetches the module (M14). + +## Phases + +| Phase | File | Effort | +| --------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | +| 0 | `phase-0-prune-terminal-subtrees.md` — reverse skip-but-traverse to prune-at-terminal in the **shipped** issue cascade, invert two tests, add `under_terminal_ancestor` | S (1.5h) | +| 1 | `phase-1-module-cascade-backend.md` — seed-set refactor of `service.py`, module preview + apply endpoints, cap, tests | M (4h) | +| 2 | `phase-2-cascade-package.md` — `packages/cascade-ext`: module service + guard + store, summary header and collapsible list on the shared modal | M (3h) | +| 3 | `phase-3-wire-module-store.md` — fenced interception in `module.store.ts`, refetch, `docs/FORK.md` exception row | S (2h) | +| 4 | `phase-4-propagate.md` — `plane-propagate` sibling-repo issues, MCP `update_module(cascade=)` + `preview_module_cascade`, `CLAUDE.md` entry | S (1.5h) | +| **Total** | | **12h** | + +Critical path 0 → 1 → 2 → 3. Phase 0 is independently shippable and revertible: it changes live +behavior and touches no module code. Phase 2 may start against the Phase 1 contract before Phase 1 merges — +that contract is fixed in Phase 1 § "Endpoint contract" and is the integration contract for the +fan-out. + +## Parallel-safe decomposition + +Phases 1 and 2 are the only pair that can overlap. File ownership is disjoint: + +| Lane | Owns | +| ------- | ------------------------------- | +| Phase 1 | `apps/api/plane/cascade_ext/**` | +| Phase 2 | `packages/cascade-ext/**` | + +Zero overlap. **Declaration hoisting:** the endpoint contract (paths, payload field names and +casing, the `reason` enum, the `over_cap` shape) is declared in Phase 1's contract section _before_ +either lane starts and is quoted verbatim into both briefs — no lane invents a shared shape. +Phase 3 is serial after both (it imports Phase 2's exports and calls Phase 1's endpoints). +Verification is serialized: one `pytest` + one `pnpm check` at the end of each lane, not per file. + +## Risk Assessment + +| Risk | L | I | Score | Mitigation | +| ---------------------------------------------------------------------------------------- | --- | --- | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Refactoring `collect_descendants` to a seed-set breaks the shipped per-issue cascade | 3 | 5 | **15** | `collect_descendants(root_issue=…)` is kept as a thin wrapper over the new internal with `include_seeds=False`; its existing 490-line suite must pass **unchanged by Phase 1** — Phase 0's two deliberate inversions are already in the tree, and Phase 1 may not edit a third assertion. Gate stated in Phase 1 § Success criteria. | +| Phase 0's reversal is silently undone later by a reader who finds the inverted tests odd | 3 | 3 | 9 | The two tests are renamed to state the rule (`..._and_prunes_its_subtree`) and carry a dated comment; `docs/FORK.md` and `CLAUDE.md` are corrected rather than left asserting the old rule. Phase 0 § 4. | +| A user relied on the cascade sweeping past a cancelled parent | 3 | 3 | 9 | Deliberate, user-directed reversal — documented with its date in FORK.md/CLAUDE.md so a surprised reader finds the answer instead of filing a bug. Phase 0 § Risk. | +| Client posts item ids it may not move | 3 | 5 | **15** | Apply **re-derives** the eligible set server-side and rejects anything outside it. `item_ids` is a request, never an authorization. Mirrors the shipped risk-15 mitigation. | +| A large module holds a long write lock / floods Celery | 3 | 4 | 12 | `MAX_MODULE_CASCADE_ITEMS = 100` hard cap (M4) + the existing 100-row `bulk_update` batching. Over the cap, apply refuses rather than starting. | +| Module status write bypasses the core viewset, losing a behavior it applies | 3 | 4 | 12 | M5 names the two behaviors explicitly (archived rejection, `model_activity`) and Phase 1 tests both. The remaining serializer work on that path — start/target date ordering, name uniqueness — cannot be affected by a status-only write. | +| Progress ring reads stale after a cascade | 4 | 2 | 8 | M14 — refetch the module after apply. Phase 3 asserts the counts move. | +| Rebase conflict at `module.store.ts` | 3 | 2 | 6 | One fenced block + a `docs/FORK.md` exception row. Phase 3. | +| Preview goes stale between preview and apply | 2 | 3 | 6 | Apply re-derives; an item that became terminal in the gap is skipped and reported in `rejected`. | +| Modal fires on the create/update modal's whole-object save | 3 | 2 | 6 | The guard reads `data.status` — a payload that does not change status never reaches the preview. Phase 3 asserts a name-only edit on a completed module issues no preview request. | +| `parent` cycle among module members → infinite walk | 1 | 5 | 5 | `visited` seeded with every member id; `MAX_DEPTH = 20` unchanged. | + +## Timeline + +| Phase | Effort | Notes | +| -------------------------------- | -------- | ----------------------------------------------------------------------------------------- | +| Phase 0: prune terminal subtrees | S (1.5h) | Changes shipped behavior. Independently shippable. Blocks 1. | +| Phase 1: backend | M (4h) | Blocks 2 and 3. Contract must be frozen first. | +| Phase 2: package | M (3h) | Overlaps Phase 1 once the contract is frozen. | +| Phase 3: wire store | S (2h) | Serial after 1 and 2. | +| Phase 4: propagate | S (1.5h) | Sibling-repo issues + a separate `plane-mcp-server` PR. Never edited from this repo's PR. | +| Total | **12h** | Critical path 0 → 1 → 2 → 3 = 10.5h. | + +## Verification + +- `pytest apps/api/plane/cascade_ext/tests/` green — **including the pre-existing + `test_cascade_db.py` with no assertion edited**. +- `python manage.py makemigrations --check --dry-run` — no changes (proves no model added). +- `python manage.py check` clean · `pnpm check` clean. +- `node .claude/scripts/plane-classify-path.cjs` over the diff — every path classifies `fork` or as + a **registered** exception. Expect exactly one unregistered-core candidate + (`apps/web/core/store/module.store.ts`), registered by Phase 3. +- Manual matrix: empty module → no modal, no preview request · module whose items are all done → + no modal · module → `in-progress` → no preview request · name-only edit on a completed module → + no preview request · 3-level tree under one member → every level listed · terminal member with live children → member + and its children **all** absent · cross-project sub-item + → mapped by group · renamed states (`Done` → `Shipped`) → still correct · item in a project you + cannot access → listed, disabled, reason shown · untick one → that one stays · Enter on the modal + → **only the module changes** · 240-item module → refusal mode, module status still changes · + archived module → 400 · all five status entry points. From 6500dedf77d06bf15b517487588803aba4fa57c8 Mon Sep 17 00:00:00 2001 From: Manh Nguyen Date: Fri, 28 Aug 2026 13:08:26 +0700 Subject: [PATCH 4/7] feat(cascade): module-cascade client, guard, and shared summary modal Extends the fork-owned @plane/cascade-ext package to a second cascade subject: a module's terminal status. - shouldPromptModuleCascade: fires only when the payload actually carries a completed/cancelled status AND the module has work items, so a name-only edit to a completed module issues no preview request. It deliberately does NOT subtract completed_issues/cancelled_issues from total_issues -- those counts cover direct module members only, so the cheaper arithmetic does not describe the set the server walks. - getModulePreview / applyModuleCascade on the existing service, reusing its /api/v1-suffix strip (cascade-ext mounts OUTSIDE /api/v1). - The existing confirm store is widened in place rather than duplicated; a second store would need a second mount point in root.tsx. - CascadeConfirmModal grows a summary header, a list that auto-collapses above 15 rows, and a refusal mode for an over-cap preview. At or below the threshold the shipped issue flow renders exactly as before. 42/42 package tests pass, with the pre-existing issue-path tests unedited. --- .../__tests__/cascade-confirm-modal.test.tsx | 121 +++++++++- .../__tests__/cascade-confirm-store.test.ts | 109 ++++++++- .../src/__tests__/cascade-service.test.ts | 122 ++++++++++ .../__tests__/should-prompt-cascade.test.ts | 46 +++- .../cascade-ext/src/cascade-confirm-modal.tsx | 214 +++++++++++++----- .../cascade-ext/src/cascade-confirm-store.ts | 78 ++++++- packages/cascade-ext/src/cascade-service.ts | 55 ++++- .../cascade-ext/src/should-prompt-cascade.ts | 38 +++- packages/cascade-ext/src/strings.ts | 59 ++++- packages/cascade-ext/src/types.ts | 64 +++++- 10 files changed, 826 insertions(+), 80 deletions(-) create mode 100644 packages/cascade-ext/src/__tests__/cascade-service.test.ts diff --git a/packages/cascade-ext/src/__tests__/cascade-confirm-modal.test.tsx b/packages/cascade-ext/src/__tests__/cascade-confirm-modal.test.tsx index b3f27825a..b4f025359 100644 --- a/packages/cascade-ext/src/__tests__/cascade-confirm-modal.test.tsx +++ b/packages/cascade-ext/src/__tests__/cascade-confirm-modal.test.tsx @@ -15,7 +15,7 @@ import * as jestDomVitestMatchers from "@testing-library/jest-dom/vitest"; void jestDomVitestMatchers; import { CascadeConfirmModal } from "../cascade-confirm-modal"; import { CascadeConfirmStore } from "../cascade-confirm-store"; -import type { TCascadeDescendant } from "../types"; +import type { TCascadeDescendant, TCascadeItem, TModuleCascadeSummary } from "../types"; function descendant(overrides: Partial & { id: string }): TCascadeDescendant { return { @@ -34,12 +34,39 @@ function descendant(overrides: Partial & { id: string }): TC }; } +function moduleItem(overrides: Partial & { id: string }): TCascadeItem { + return { ...descendant(overrides), is_module_member: true, ...overrides }; +} + function openModal(store: CascadeConfirmStore, descendants: TCascadeDescendant[]) { const pending = store.requestCascade({ parentIdentifier: "PLANE-1", targetGroup: "completed", descendants }); render(); return pending; } +function openModuleModal( + store: CascadeConfirmStore, + items: TCascadeItem[], + overrides: { summary?: TModuleCascadeSummary; overCap?: boolean; cap?: number } = {} +) { + const summary: TModuleCascadeSummary = overrides.summary ?? { + total_live: items.length, + eligible: items.filter((i) => i.eligible).length, + ineligible: items.filter((i) => !i.eligible).length, + already_terminal: 0, + }; + const pending = store.requestModuleCascade({ + moduleName: "Sprint 12", + targetGroup: "completed", + items, + summary, + overCap: overrides.overCap ?? false, + cap: overrides.cap ?? 100, + }); + render(); + return pending; +} + describe("CascadeConfirmModal", () => { it("holds initial focus on 'Only change this item' — not on a row checkbox", async () => { const store = new CascadeConfirmStore(); @@ -96,3 +123,95 @@ describe("CascadeConfirmModal", () => { await expect(pending).resolves.toEqual({ cascade: true, childIds: ["b"] }); }); }); + +describe("CascadeConfirmModal — module subject", () => { + it("3 items renders the full list expanded, with no disclosure", async () => { + const store = new CascadeConfirmStore(); + openModuleModal(store, [moduleItem({ id: "a" }), moduleItem({ id: "b" }), moduleItem({ id: "c" })]); + + expect(await screen.findByRole("checkbox", { name: "Change PLANE-a too" })).toBeInTheDocument(); + expect(screen.getByRole("checkbox", { name: "Change PLANE-b too" })).toBeInTheDocument(); + expect(screen.getByRole("checkbox", { name: "Change PLANE-c too" })).toBeInTheDocument(); + expect(screen.queryByText(/Show all/)).not.toBeInTheDocument(); + }); + + it("40 items starts collapsed behind a disclosure naming the count, and expands on click", async () => { + const user = userEvent.setup(); + const store = new CascadeConfirmStore(); + const items = Array.from({ length: 40 }, (_, i) => moduleItem({ id: `item-${i}` })); + openModuleModal(store, items); + + const disclosure = await screen.findByRole("button", { name: "Show all 40 items" }); + expect(screen.queryByRole("checkbox", { name: "Change PLANE-item-0 too" })).not.toBeInTheDocument(); + + await user.click(disclosure); + + expect(await screen.findByRole("checkbox", { name: "Change PLANE-item-0 too" })).toBeInTheDocument(); + expect(screen.getByRole("checkbox", { name: "Change PLANE-item-39 too" })).toBeInTheDocument(); + }); + + it("over-cap renders refusal mode: no cascade button, cap and total both rendered, status still changes", async () => { + const user = userEvent.setup(); + const store = new CascadeConfirmStore(); + const pending = openModuleModal(store, [], { + summary: { total_live: 240, eligible: 0, ineligible: 0, already_terminal: 0 }, + overCap: true, + cap: 100, + }); + + expect(await screen.findByText(/240 work items/)).toBeInTheDocument(); + expect(screen.getByText(/100 this action can change/)).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Change work items too" })).not.toBeInTheDocument(); + expect(screen.queryByRole("checkbox")).not.toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: "Only change this module" })); + + await expect(pending).resolves.toEqual({ cascade: false }); + }); + + it("holds initial focus on 'Only change this module' — normal mode", async () => { + const store = new CascadeConfirmStore(); + openModuleModal(store, [moduleItem({ id: "a" })]); + + const onlyModuleButton = await screen.findByRole("button", { name: "Only change this module" }); + await waitFor(() => { + expect(document.activeElement).toBe(onlyModuleButton); + }); + }); + + it("holds initial focus on 'Only change this module' — refusal mode too", async () => { + const store = new CascadeConfirmStore(); + openModuleModal(store, [], { + summary: { total_live: 240, eligible: 0, ineligible: 0, already_terminal: 0 }, + overCap: true, + cap: 100, + }); + + const onlyModuleButton = await screen.findByRole("button", { name: "Only change this module" }); + await waitFor(() => { + expect(document.activeElement).toBe(onlyModuleButton); + }); + }); + + it("omits a zero-valued summary clause instead of rendering '0 already done'", async () => { + const store = new CascadeConfirmStore(); + openModuleModal(store, [moduleItem({ id: "a" }), moduleItem({ id: "b" })], { + summary: { total_live: 2, eligible: 2, ineligible: 0, already_terminal: 0 }, + }); + + expect(await screen.findByText("2 work items will be completed")).toBeInTheDocument(); + expect(screen.queryByText(/already done/)).not.toBeInTheDocument(); + expect(screen.queryByText(/you cannot change/)).not.toBeInTheDocument(); + }); + + it("renders every non-zero summary clause together", async () => { + const store = new CascadeConfirmStore(); + openModuleModal(store, [moduleItem({ id: "a" })], { + summary: { total_live: 62, eligible: 47, ineligible: 3, already_terminal: 12 }, + }); + + expect( + await screen.findByText("47 work items will be completed · 12 already done · 3 you cannot change") + ).toBeInTheDocument(); + }); +}); diff --git a/packages/cascade-ext/src/__tests__/cascade-confirm-store.test.ts b/packages/cascade-ext/src/__tests__/cascade-confirm-store.test.ts index b793fda4c..0948ab932 100644 --- a/packages/cascade-ext/src/__tests__/cascade-confirm-store.test.ts +++ b/packages/cascade-ext/src/__tests__/cascade-confirm-store.test.ts @@ -5,7 +5,7 @@ */ import { describe, expect, it } from "vitest"; import { CascadeConfirmStore } from "../cascade-confirm-store"; -import type { TCascadeDescendant } from "../types"; +import type { TCascadeDescendant, TCascadeItem, TModuleCascadeSummary } from "../types"; function descendant(overrides: Partial & { id: string }): TCascadeDescendant { return { @@ -24,6 +24,12 @@ function descendant(overrides: Partial & { id: string }): TC }; } +function moduleItem(overrides: Partial & { id: string }): TCascadeItem { + return { ...descendant(overrides), is_module_member: true, ...overrides }; +} + +const SUMMARY: TModuleCascadeSummary = { total_live: 2, eligible: 2, ineligible: 0, already_terminal: 0 }; + describe("CascadeConfirmStore", () => { it("ticks every eligible row by default and leaves ineligible rows unticked", () => { const store = new CascadeConfirmStore(); @@ -99,3 +105,104 @@ describe("CascadeConfirmStore", () => { expect(store.pendingRequest).toBeNull(); }); }); + +describe("CascadeConfirmStore — module subject (requestModuleCascade)", () => { + it("resolves cascade:true with the ticked childIds on confirmCascade", async () => { + const store = new CascadeConfirmStore(); + const pending = store.requestModuleCascade({ + moduleName: "Sprint 12", + targetGroup: "completed", + items: [moduleItem({ id: "a" }), moduleItem({ id: "b" })], + summary: SUMMARY, + overCap: false, + cap: 100, + }); + + store.toggleChild("a"); + store.confirmCascade(); + + const result = await pending; + expect(result).toEqual({ cascade: true, childIds: ["b"] }); + }); + + it("resolves cascade:false on confirmOnlyParent, same as the issue subject", async () => { + const store = new CascadeConfirmStore(); + const pending = store.requestModuleCascade({ + moduleName: "Sprint 12", + targetGroup: "completed", + items: [moduleItem({ id: "a" })], + summary: SUMMARY, + overCap: false, + cap: 100, + }); + + store.confirmOnlyParent(); + + const result = await pending; + expect(result).toEqual({ cascade: false }); + expect(store.pendingRequest).toBeNull(); + }); + + it("ticks every eligible item by default, mirroring the issue subject", () => { + const store = new CascadeConfirmStore(); + void store.requestModuleCascade({ + moduleName: "Sprint 12", + targetGroup: "completed", + items: [moduleItem({ id: "a", eligible: true }), moduleItem({ id: "b", eligible: false })], + summary: SUMMARY, + overCap: false, + cap: 100, + }); + + expect(store.checkedIds.has("a")).toBe(true); + expect(store.checkedIds.has("b")).toBe(false); + }); + + it("select-all / select-none over an over-cap request always resolves { cascade: false, childIds: [] }", async () => { + const store = new CascadeConfirmStore(); + // Over cap: the modal never renders a list or a cascade button for this request, so + // `checkedIds` starts empty and the only reachable action is `confirmOnlyParent`. + const pending = store.requestModuleCascade({ + moduleName: "Sprint 12", + targetGroup: "completed", + items: [], + summary: { total_live: 240, eligible: 0, ineligible: 0, already_terminal: 0 }, + overCap: true, + cap: 100, + }); + + expect(store.checkedIds.size).toBe(0); + store.confirmOnlyParent(); + + const result = await pending; + expect(result).toEqual({ cascade: false }); + }); + + it("pendingRequest carries kind: 'module' so the modal can distinguish subjects", () => { + const store = new CascadeConfirmStore(); + void store.requestModuleCascade({ + moduleName: "Sprint 12", + targetGroup: "cancelled", + items: [moduleItem({ id: "a" })], + summary: SUMMARY, + overCap: false, + cap: 100, + }); + + expect(store.pendingRequest?.kind).toBe("module"); + }); +}); + +// Guards against the module widening accidentally changing the issue subject's own tag. +describe("CascadeConfirmStore — issue subject still tags kind: 'issue'", () => { + it("pendingRequest carries kind: 'issue' after requestCascade", () => { + const store = new CascadeConfirmStore(); + void store.requestCascade({ + parentIdentifier: "PLANE-1", + targetGroup: "completed", + descendants: [descendant({ id: "a" })], + }); + + expect(store.pendingRequest?.kind).toBe("issue"); + }); +}); diff --git a/packages/cascade-ext/src/__tests__/cascade-service.test.ts b/packages/cascade-ext/src/__tests__/cascade-service.test.ts new file mode 100644 index 000000000..dfdc5f1c0 --- /dev/null +++ b/packages/cascade-ext/src/__tests__/cascade-service.test.ts @@ -0,0 +1,122 @@ +/** + * Copyright (c) 2023-present Plane Software, Inc. and contributors + * SPDX-License-Identifier: AGPL-3.0-only + * See the LICENSE file for details. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { CascadeApiError, CascadeService } from "../cascade-service"; + +const WORKSPACE = "plane"; +const PROJECT = "project-1"; +const MODULE = "module-1"; + +function okResponse(body: unknown) { + return { ok: true, status: 200, json: () => Promise.resolve(body), text: () => Promise.resolve("") }; +} + +describe("CascadeService — module endpoints", () => { + let fetchMock: ReturnType; + + beforeEach(() => { + fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + // The TRAP this test exists to pin: cascade-ext mounts at `/api/cascade-ext/`, OUTSIDE + // `/api/v1` — a base-URL regression here would silently 404 in production while every mocked + // unit test elsewhere kept passing. + it("getModulePreview hits /api/cascade-ext/…/modules/…, never /api/v1/…", async () => { + fetchMock.mockResolvedValueOnce( + okResponse({ + target_group: "completed", + depth_capped: false, + over_cap: false, + cap: 100, + summary: { total_live: 0, eligible: 0, ineligible: 0, already_terminal: 0 }, + items: [], + }) + ); + const service = new CascadeService(); + + await service.getModulePreview(WORKSPACE, PROJECT, MODULE, "completed"); + + expect(fetchMock).toHaveBeenCalledTimes(1); + const [url] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(url).toBe( + `/api/cascade-ext/workspaces/${WORKSPACE}/projects/${PROJECT}/modules/${MODULE}/cascade-preview/?status=completed` + ); + expect(url.startsWith("/api/v1")).toBe(false); + }); + + it("getModulePreview sends 'status', never 'group', as the query param", async () => { + fetchMock.mockResolvedValueOnce( + okResponse({ + target_group: "cancelled", + depth_capped: false, + over_cap: false, + cap: 100, + summary: { total_live: 0, eligible: 0, ineligible: 0, already_terminal: 0 }, + items: [], + }) + ); + const service = new CascadeService(); + + await service.getModulePreview(WORKSPACE, PROJECT, MODULE, "cancelled"); + + const [url] = fetchMock.mock.calls[0] as [string]; + expect(url).toContain("status=cancelled"); + expect(url).not.toContain("group="); + }); + + it("getModulePreview raises CascadeApiError on a non-2xx response", async () => { + fetchMock.mockResolvedValueOnce({ ok: false, status: 400, text: () => Promise.resolve("archived module") }); + const service = new CascadeService(); + + await expect(service.getModulePreview(WORKSPACE, PROJECT, MODULE, "completed")).rejects.toMatchObject({ + name: "CascadeApiError", + status: 400, + }); + }); + + it("applyModuleCascade posts to /api/cascade-ext/…/modules/…/cascade-apply/", async () => { + fetchMock.mockResolvedValueOnce(okResponse({ module: MODULE, status: "completed", updated: ["a"], rejected: [] })); + const service = new CascadeService(); + + await service.applyModuleCascade(WORKSPACE, PROJECT, MODULE, "completed", ["a"]); + + expect(fetchMock).toHaveBeenCalledTimes(1); + const [url, init] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(url).toBe(`/api/cascade-ext/workspaces/${WORKSPACE}/projects/${PROJECT}/modules/${MODULE}/cascade-apply/`); + expect(url.startsWith("/api/v1")).toBe(false); + expect(init.method).toBe("POST"); + }); + + it("applyModuleCascade always sends item_ids as an explicit array — never omits the key", async () => { + fetchMock.mockResolvedValueOnce(okResponse({ module: MODULE, status: "completed", updated: [], rejected: [] })); + const service = new CascadeService(); + + await service.applyModuleCascade(WORKSPACE, PROJECT, MODULE, "completed", []); + + const [, init] = fetchMock.mock.calls[0] as [string, RequestInit]; + const body = JSON.parse(init.body as string) as { status: string; item_ids: unknown }; + expect(body).toEqual({ status: "completed", item_ids: [] }); + expect(Array.isArray(body.item_ids)).toBe(true); + }); + + it("applyModuleCascade raises CascadeApiError on a non-2xx response", async () => { + fetchMock.mockResolvedValueOnce({ + ok: false, + status: 400, + text: () => Promise.resolve("cascade exceeds MAX_MODULE_CASCADE_ITEMS"), + }); + const service = new CascadeService(); + + await expect(service.applyModuleCascade(WORKSPACE, PROJECT, MODULE, "completed", [])).rejects.toBeInstanceOf( + CascadeApiError + ); + }); +}); diff --git a/packages/cascade-ext/src/__tests__/should-prompt-cascade.test.ts b/packages/cascade-ext/src/__tests__/should-prompt-cascade.test.ts index 0600ffcdf..29a165947 100644 --- a/packages/cascade-ext/src/__tests__/should-prompt-cascade.test.ts +++ b/packages/cascade-ext/src/__tests__/should-prompt-cascade.test.ts @@ -4,7 +4,7 @@ * See the LICENSE file for details. */ import { describe, expect, it } from "vitest"; -import { shouldPromptCascade } from "../should-prompt-cascade"; +import { shouldPromptCascade, shouldPromptModuleCascade } from "../should-prompt-cascade"; const STATE_GROUPS: Record = { "state-unstarted": "unstarted", @@ -59,3 +59,47 @@ describe("shouldPromptCascade", () => { ).toBeNull(); }); }); + +describe("shouldPromptModuleCascade", () => { + it("returns 'completed' for a terminal completed move with live issues", () => { + expect(shouldPromptModuleCascade({ data: { status: "completed" }, totalIssues: 47 })).toBe("completed"); + }); + + it("returns 'cancelled' for a terminal cancelled move with live issues", () => { + expect(shouldPromptModuleCascade({ data: { status: "cancelled" }, totalIssues: 1 })).toBe("cancelled"); + }); + + it("returns null for a non-terminal status (in-progress)", () => { + expect(shouldPromptModuleCascade({ data: { status: "in-progress" }, totalIssues: 47 })).toBeNull(); + }); + + it("returns null when total_issues is 0", () => { + expect(shouldPromptModuleCascade({ data: { status: "completed" }, totalIssues: 0 })).toBeNull(); + }); + + // The load-bearing case (plan risk row "Modal fires on the create/update modal's whole-object + // save"): a payload that doesn't carry `status` at all — e.g. a name-only edit on an already + // -completed module — must never fire a preview request. + it("returns null when the payload carries no status at all", () => { + expect(shouldPromptModuleCascade({ data: { name: "Renamed" }, totalIssues: 47 })).toBeNull(); + }); + + // The server decides whether re-saving the same status is a no-op (M7) — the client-side guard + // has no memory of the module's PREVIOUS status and must not try to infer "unchanged" itself. + it("fires even when the posted status equals the module's current status", () => { + expect(shouldPromptModuleCascade({ data: { status: "completed" }, totalIssues: 5 })).toBe("completed"); + }); + + // M6's correctness hole, pinned directly: the guard must NOT subtract completed/cancelled + // counts from total_issues to decide whether to fire. Those three counts cover direct module + // members only, while the cascade also walks each member's descendant subtree (M2) — so even a + // module whose direct members are ALL terminal must still prompt when total_issues > 0. + it("still fires when every direct member is already terminal, because total_issues alone gates it", () => { + expect( + shouldPromptModuleCascade({ + data: { status: "completed", completed_issues: 10, cancelled_issues: 0 }, + totalIssues: 10, + }) + ).toBe("completed"); + }); +}); diff --git a/packages/cascade-ext/src/cascade-confirm-modal.tsx b/packages/cascade-ext/src/cascade-confirm-modal.tsx index 386ab5709..8e03cf3bc 100644 --- a/packages/cascade-ext/src/cascade-confirm-modal.tsx +++ b/packages/cascade-ext/src/cascade-confirm-modal.tsx @@ -4,11 +4,12 @@ * See the LICENSE file for details. */ import { observer } from "mobx-react"; -import { useEffect, useRef } from "react"; +import { useEffect, useRef, useState } from "react"; import { Button } from "@plane/propel/button"; import { Checkbox, EModalPosition, EModalWidth, ModalCore } from "@plane/ui"; -import type { CascadeConfirmStore } from "./cascade-confirm-store"; -import { CASCADE_STRINGS } from "./strings"; +import type { CascadeConfirmStore, TCascadeConfirmSubject } from "./cascade-confirm-store"; +import { CASCADE_LIST_CONTROL_STRINGS, CASCADE_STRINGS, MODULE_CASCADE_STRINGS } from "./strings"; +import type { TCascadeDescendant } from "./types"; export interface CascadeConfirmModalProps { store: CascadeConfirmStore; @@ -20,31 +21,110 @@ const INDENT_BASE_PX = 10; const INDENT_PER_DEPTH_PX = 22; /** - * Confirmation modal for cascading a parent's terminal state to its sub-items (issue #54). - * Built on `@plane/ui`'s `ModalCore` — no hand-rolled dialog, no bespoke focus trap. + * Above this row count the list starts collapsed behind a "Show all N items" disclosure + * (plan.md M3). At or below it — every realistic issue cascade — the list renders exactly as + * the shipped modal always has, so this is not a behavior change for that flow. + */ +const LIST_COLLAPSE_THRESHOLD = 15; + +/** One checkbox row — identical markup for an issue descendant or a module item, since + * `TCascadeItem` is a superset of `TCascadeDescendant` and the row never reads the extra + * `is_module_member` field. */ +function CascadeRow(props: { row: TCascadeDescendant; store: CascadeConfirmStore }) { + const { row, store } = props; + return ( +
  • + store.toggleChild(row.id)} + aria-label={CASCADE_STRINGS.rowCheckboxLabel(row.identifier)} + /> + + + {row.identifier} {row.name} + + + {row.eligible ? CASCADE_STRINGS.currentState(row.state_name) : CASCADE_STRINGS.ineligibleReason(row.reason)} + + +
    • + ); +} + +/** + * Confirmation modal for cascading a parent's terminal state to its sub-items (issue #54) or a + * module's terminal status to its work items (plan.md M3 — same modal, a summary header and a + * collapsible list added on top). Built on `@plane/ui`'s `ModalCore` — no hand-rolled dialog, no + * bespoke focus trap. */ export const CascadeConfirmModal = observer(function CascadeConfirmModal(props: CascadeConfirmModalProps) { const { store } = props; - const request = store.pendingRequest; + const request: TCascadeConfirmSubject | null = store.pendingRequest; const onlyParentButtonRef = useRef(null); + const [isExpanded, setIsExpanded] = useState(false); - // Decision 2 (the single most load-bearing detail in this modal): "Only change this item" - // must hold initial focus so a stray Enter never cascades. `ModalCore` doesn't thread a - // headlessUI `initialFocus` ref through to its `Dialog`, and this component is a DESCENDANT of - // Dialog's own FocusTrap, so a plain `useEffect` here runs and calls `.focus()` BEFORE - // FocusTrap's own initial-focus effect (parents run after children in the same commit) — and - // that effect defers its actual "focus the first focusable element" default to a microtask, - // which then overrides whatever this effect just set. A `setTimeout` (a macrotask) is - // guaranteed to run only after every microtask — including that deferred one — has drained, so - // it reliably runs last regardless of how many effect levels sit between this component and - // the trap. The cost is one animation frame's worth of default focus before this overrides it, - // imperceptible to a real user and not a race a synchronous effect can win here. + // Decision 2 (the single most load-bearing detail in this modal): "Only change this item / + // module" must hold initial focus so a stray Enter never cascades, for EITHER subject and in + // refusal mode alike. `ModalCore` doesn't thread a headlessUI `initialFocus` ref through to its + // `Dialog`, and this component is a DESCENDANT of Dialog's own FocusTrap, so a plain `useEffect` + // here runs and calls `.focus()` BEFORE FocusTrap's own initial-focus effect (parents run after + // children in the same commit) — and that effect defers its actual "focus the first focusable + // element" default to a microtask, which then overrides whatever this effect just set. A + // `setTimeout` (a macrotask) is guaranteed to run only after every microtask — including that + // deferred one — has drained, so it reliably runs last regardless of how many effect levels sit + // between this component and the trap. The cost is one animation frame's worth of default focus + // before this overrides it, imperceptible to a real user and not a race a synchronous effect can + // win here. useEffect(() => { if (!request) return; const timeoutId = setTimeout(() => onlyParentButtonRef.current?.focus(), 0); return () => clearTimeout(timeoutId); }, [request]); + // A fresh `pendingRequest` is a new object every time (`requestCascade` / `requestModuleCascade` + // never mutate the previous one), so re-collapsing on it means a second confirmation for a + // different subject never inherits an expanded list left open by the first. + useEffect(() => { + setIsExpanded(false); + }, [request]); + + if (!request) { + // `ModalCore.children` is a required prop (`packages/ui/src/modals/modal-core.tsx`) — `null` + // renders nothing, matching `{request && (...)}`'s old falsy-child behavior when closed. + return ( + store.confirmOnlyParent()} + position={EModalPosition.CENTER} + width={EModalWidth.XL} + > + {null} + + ); + } + + const isModule = request.kind === "module"; + const refusalMode = isModule && request.overCap; + const rows: readonly TCascadeDescendant[] = isModule ? request.items : request.descendants; + const showDisclosure = !refusalMode && rows.length > LIST_COLLAPSE_THRESHOLD; + const visibleRows = refusalMode ? [] : showDisclosure && !isExpanded ? [] : rows; + const eligibleRows = rows.filter((row) => row.eligible); + + const title = isModule ? MODULE_CASCADE_STRINGS.title : CASCADE_STRINGS.title; + const onlyButtonLabel = isModule ? MODULE_CASCADE_STRINGS.onlyModuleButton : CASCADE_STRINGS.onlyParentButton; + const cascadeButtonLabel = isModule ? MODULE_CASCADE_STRINGS.cascadeModuleButton : CASCADE_STRINGS.cascadeButton; + + const selectAll = () => { + for (const row of eligibleRows) if (!store.checkedIds.has(row.id)) store.toggleChild(row.id); + }; + const selectNone = () => { + for (const row of eligibleRows) if (store.checkedIds.has(row.id)) store.toggleChild(row.id); + }; + return ( - {request && ( - <> -
    -

    {CASCADE_STRINGS.title}

    +
    +

    {title}

    + + {refusalMode ? ( + // Refusal mode (M4): no list, no checkboxes, no "Change work items too" button — the + // module's own status write still happens via `confirmOnlyParent` below. +

    + {MODULE_CASCADE_STRINGS.overCapBody(request.summary.total_live, request.cap)} +

    + ) : ( + <>

    - {CASCADE_STRINGS.description(request.parentIdentifier, request.targetGroup)} + {isModule + ? MODULE_CASCADE_STRINGS.description(request.moduleName, request.targetGroup) + : CASCADE_STRINGS.description(request.parentIdentifier, request.targetGroup)}

    -
      - {request.descendants.map((descendant) => ( -
    • + {MODULE_CASCADE_STRINGS.summary(request.summary, request.targetGroup)} +

      + )} + + {showDisclosure && ( +
      + + +
      • - ))} -
    -
    -
    - - -
    - - )} + {isExpanded + ? CASCADE_LIST_CONTROL_STRINGS.showLess + : CASCADE_LIST_CONTROL_STRINGS.showAllItems(rows.length)} + +
    + )} + + {visibleRows.length > 0 && ( +
      + {visibleRows.map((row) => ( + + ))} +
    + )} + + )} + +
    + + {!refusalMode && ( + + )} +
    ); }); diff --git a/packages/cascade-ext/src/cascade-confirm-store.ts b/packages/cascade-ext/src/cascade-confirm-store.ts index 670f7e97f..bf187a6d7 100644 --- a/packages/cascade-ext/src/cascade-confirm-store.ts +++ b/packages/cascade-ext/src/cascade-confirm-store.ts @@ -4,12 +4,15 @@ * See the LICENSE file for details. */ import { action, makeObservable, observable } from "mobx"; -import type { TCascadeDescendant, TCascadeStateGroup } from "./types"; +import type { TCascadeDescendant, TCascadeItem, TCascadeStateGroup, TModuleCascadeSummary } from "./types"; /** - * Everything the modal needs to render one confirmation. Fetching this (the preview call) is - * Phase 3's job, not this store's — Phase 3 checks the preview for a non-empty eligible set - * (Decision 3) before ever calling `requestCascade`, so the modal is never opened empty. + * Everything the modal needs to render one issue-subject confirmation. Fetching this (the + * preview call) is Phase 3's job, not this store's — Phase 3 checks the preview for a non-empty + * eligible set (Decision 3) before ever calling `requestCascade`, so the modal is never opened + * empty. Deliberately carries NO `kind` discriminant — `requestCascade` is the only place that + * ever needs to know it is the issue subject, and adding one here would be a breaking change to + * every existing call site for no reader benefit. */ export interface TCascadeConfirmRequest { /** Display-only context for the modal header — never sent back to the server by this store. */ @@ -18,13 +21,38 @@ export interface TCascadeConfirmRequest { descendants: TCascadeDescendant[]; } +/** + * The module-subject counterpart (plan.md M3). `summary` / `overCap` / `cap` are display-only — + * carried straight from the preview response into the modal's summary header and refusal mode. + */ +export interface TModuleCascadeConfirmRequest { + moduleName: string; + targetGroup: TCascadeStateGroup; + items: TCascadeItem[]; + summary: TModuleCascadeSummary; + overCap: boolean; + cap: number; +} + +/** + * `pendingRequest`'s actual stored shape — one MobX-observable field, two subjects, chosen by + * `kind`. Widening `pendingRequest` in place (rather than adding a second store) is deliberate: + * `CascadeConfirmModal` is one component, and a second store would need a second mount point in + * `apps/web/app/root.tsx`, which Phase 2 does not own. + */ +export type TCascadeConfirmSubject = + | ({ kind: "issue" } & TCascadeConfirmRequest) + | ({ kind: "module" } & TModuleCascadeConfirmRequest); + export type TCascadeConfirmResult = { cascade: false } | { cascade: true; childIds: string[] }; export interface ICascadeConfirmStore { - pendingRequest: TCascadeConfirmRequest | null; - /** Ids of eligible descendant rows currently ticked. Ineligible rows are never members. */ + pendingRequest: TCascadeConfirmSubject | null; + /** Ids of eligible rows currently ticked, whichever subject is pending. Ineligible rows are + * never members — and an over-cap module request starts with this empty (M4). */ checkedIds: Set; requestCascade: (request: TCascadeConfirmRequest) => Promise; + requestModuleCascade: (request: TModuleCascadeConfirmRequest) => Promise; toggleChild: (id: string) => void; confirmOnlyParent: () => void; confirmCascade: () => void; @@ -37,7 +65,7 @@ export interface ICascadeConfirmStore { * cascade-apply call is Phase 3's, using the resolved `{ cascade, childIds }`. */ export class CascadeConfirmStore implements ICascadeConfirmStore { - pendingRequest: TCascadeConfirmRequest | null = null; + pendingRequest: TCascadeConfirmSubject | null = null; checkedIds: Set = new Set(); private _resolve: ((result: TCascadeConfirmResult) => void) | null = null; @@ -47,6 +75,7 @@ export class CascadeConfirmStore implements ICascadeConfirmStore { pendingRequest: observable, checkedIds: observable, requestCascade: action, + requestModuleCascade: action, toggleChild: action, confirmOnlyParent: action, confirmCascade: action, @@ -57,25 +86,42 @@ export class CascadeConfirmStore implements ICascadeConfirmStore { // A prior pending request left unresolved (shouldn't happen — one modal at a time) resolves // as "do not cascade" rather than leaving its caller awaiting forever. this._resolvePending({ cascade: false }); - this.pendingRequest = request; + this.pendingRequest = { kind: "issue", ...request }; this.checkedIds = new Set(request.descendants.filter((d) => d.eligible).map((d) => d.id)); return new Promise((resolve) => { this._resolve = resolve; }); } + /** + * The module-subject counterpart of `requestCascade`. When `request.overCap` is true (M4) the + * checked set starts and stays empty — the modal renders refusal mode with no list and no + * cascade button, so `checkedIds` is never read there, but leaving it non-empty would be a trap + * for the next reader who adds a code path that does read it. + */ + requestModuleCascade(request: TModuleCascadeConfirmRequest): Promise { + this._resolvePending({ cascade: false }); + this.pendingRequest = { kind: "module", ...request }; + this.checkedIds = request.overCap ? new Set() : new Set(request.items.filter((i) => i.eligible).map((i) => i.id)); + return new Promise((resolve) => { + this._resolve = resolve; + }); + } + toggleChild(id: string): void { if (!this.pendingRequest) return; - const row = this.pendingRequest.descendants.find((d) => d.id === id); + const row = this._rows().find((r) => r.id === id); if (!row || !row.eligible) return; // ineligible rows are never toggleable (Decision 8) if (this.checkedIds.has(id)) this.checkedIds.delete(id); else this.checkedIds.add(id); } /** - * "Only change this item" — the default action (Decision 2). Also what a modal dismissal - * (Escape key / backdrop click, via `ModalCore`'s `handleClose`) resolves to: closing without - * an explicit choice gets the same safe default as a stray Enter would. + * "Only change this item" / "Only change this module" — the default action (Decision 2, M3). + * Also what a modal dismissal (Escape key / backdrop click, via `ModalCore`'s `handleClose`) + * resolves to: closing without an explicit choice gets the same safe default as a stray Enter + * would. Subject-agnostic — the modal itself renders the button label for whichever subject is + * pending. */ confirmOnlyParent(): void { this._resolvePending({ cascade: false }); @@ -87,6 +133,14 @@ export class CascadeConfirmStore implements ICascadeConfirmStore { this._clear(); } + /** The eligible/ineligible row set for whichever subject is pending — issue `descendants` or + * module `items`. `TCascadeItem` is a strict superset of `TCascadeDescendant`, so this is a + * plain narrowing read, not a projection. */ + private _rows(): readonly TCascadeDescendant[] { + if (!this.pendingRequest) return []; + return this.pendingRequest.kind === "issue" ? this.pendingRequest.descendants : this.pendingRequest.items; + } + private _resolvePending(result: TCascadeConfirmResult): void { if (!this._resolve) return; this._resolve(result); diff --git a/packages/cascade-ext/src/cascade-service.ts b/packages/cascade-ext/src/cascade-service.ts index 9e4891c4c..047da0540 100644 --- a/packages/cascade-ext/src/cascade-service.ts +++ b/packages/cascade-ext/src/cascade-service.ts @@ -3,7 +3,14 @@ * SPDX-License-Identifier: AGPL-3.0-only * See the LICENSE file for details. */ -import type { TCascadeApplyResponse, TCascadePreviewResponse, TCascadeStateGroup } from "./types"; +import type { + TCascadeApplyResponse, + TCascadePreviewResponse, + TCascadeStateGroup, + TModuleCascadeApplyResponse, + TModuleCascadePreviewResponse, + TModuleCascadeStatus, +} from "./types"; const API_BASE = "/api/cascade-ext"; @@ -68,6 +75,52 @@ export class CascadeService { if (!res.ok) throw new CascadeApiError(await res.text(), res.status); return res.json() as Promise; } + + /** + * `GET …/modules//cascade-preview/?status=` — the module + * equivalent of `getPreview`. The query param is `status` (a MODULE status), NOT `group` — the + * two endpoints intentionally use different param names so they can never be confused + * (phase-1 § Endpoint contract). Read-only; safe to call speculatively, guarded first by + * `shouldPromptModuleCascade`. + */ + async getModulePreview( + workspaceSlug: string, + projectId: string, + moduleId: string, + status: TModuleCascadeStatus + ): Promise { + const url = `${API_BASE}/workspaces/${workspaceSlug}/projects/${projectId}/modules/${moduleId}/cascade-preview/?status=${status}`; + const res = await fetch(url, { credentials: "include" }); + if (!res.ok) throw new CascadeApiError(await res.text(), res.status); + return res.json() as Promise; + } + + /** + * `POST …/modules//cascade-apply/` — applies the module's new `status` and the + * caller-selected item ids in one server-side transaction (M5). Unlike the issue path's + * `apply`, `itemIds` is always an explicit array here, never `null` — the UI must never request + * "every eligible item" implicitly (phase-2 § Implementation item 3); a headless/MCP caller that + * wants that behavior omits the key at the HTTP layer directly rather than through this method. + * The server re-derives eligibility itself and never trusts this list as authorization + * (mirrors the issue path's risk-15 mitigation). + */ + async applyModuleCascade( + workspaceSlug: string, + projectId: string, + moduleId: string, + status: TModuleCascadeStatus, + itemIds: string[] + ): Promise { + const url = `${API_BASE}/workspaces/${workspaceSlug}/projects/${projectId}/modules/${moduleId}/cascade-apply/`; + const res = await fetch(url, { + method: "POST", + credentials: "include", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ status, item_ids: itemIds }), + }); + if (!res.ok) throw new CascadeApiError(await res.text(), res.status); + return res.json() as Promise; + } } /** Ready-made singleton — the shape Phase 3 wires directly into its two store choke points. */ diff --git a/packages/cascade-ext/src/should-prompt-cascade.ts b/packages/cascade-ext/src/should-prompt-cascade.ts index ea4ae8162..9037118f0 100644 --- a/packages/cascade-ext/src/should-prompt-cascade.ts +++ b/packages/cascade-ext/src/should-prompt-cascade.ts @@ -3,10 +3,11 @@ * SPDX-License-Identifier: AGPL-3.0-only * See the LICENSE file for details. */ -import type { TIssue } from "@plane/types"; -import type { TCascadeStateGroup } from "./types"; +import type { IModule, TIssue } from "@plane/types"; +import type { TCascadeStateGroup, TModuleCascadeStatus } from "./types"; const TERMINAL_GROUPS: ReadonlySet = new Set(["completed", "cancelled"]); +const TERMINAL_MODULE_STATUSES: ReadonlySet = new Set(["completed", "cancelled"]); export interface TShouldPromptCascadeArgs { data: Partial; @@ -36,3 +37,36 @@ export function shouldPromptCascade(args: TShouldPromptCascadeArgs): TCascadeSta if (!TERMINAL_GROUPS.has(group as TCascadeStateGroup)) return null; return group as TCascadeStateGroup; } + +export interface TShouldPromptModuleCascadeArgs { + data: Partial; + totalIssues: number; +} + +/** + * The module-side guard (plan.md § Flow, step 2; decision M6). Returns the module status the + * module is about to enter ONLY when `data.status` is present and is one of the two terminal + * statuses, AND the module has at least one work item. Otherwise `null` — no preview request, + * no modal. + * + * A payload that does not carry `status` at all (a name-only edit, or any other field-only PATCH + * on an already-completed/cancelled module) returns `null` — this is what keeps that case from + * firing a preview request on every save (plan risk row "Modal fires on the create/update + * modal's whole-object save"). + * + * Deliberately does NOT subtract `completed_issues` / `cancelled_issues` from `total_issues` to + * build a cheaper "does this module actually have live work" guard. Those three counts on + * `IModule` cover DIRECT module members only (`packages/types/src/module/modules.ts`), while the + * cascade walks every member's full descendant subtree (M2) — a module whose members are all + * terminal but whose sub-items are still live would be wrongly skipped by that arithmetic. The + * per-issue flow can afford a cheap client-side skip because a Done click is high-frequency; a + * module status change is not (M6), so this guard pays for one request on the rare action instead + * of re-deriving a rule only the server can get right. Do not "optimize" this back — see M6. + */ +export function shouldPromptModuleCascade(args: TShouldPromptModuleCascadeArgs): TModuleCascadeStatus | null { + const { data, totalIssues } = args; + if (!data.status) return null; + if (!TERMINAL_MODULE_STATUSES.has(data.status as TModuleCascadeStatus)) return null; + if (totalIssues <= 0) return null; + return data.status as TModuleCascadeStatus; +} diff --git a/packages/cascade-ext/src/strings.ts b/packages/cascade-ext/src/strings.ts index e85b2fd56..2a3ccfa43 100644 --- a/packages/cascade-ext/src/strings.ts +++ b/packages/cascade-ext/src/strings.ts @@ -3,7 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-only * See the LICENSE file for details. */ -import type { TCascadeIneligibleReason, TCascadeStateGroup } from "./types"; +import type { TCascadeIneligibleReason, TCascadeStateGroup, TModuleCascadeSummary } from "./types"; /** * English-only literals for the cascade confirm modal (Decision 12, plan.md). These live here, @@ -16,6 +16,13 @@ const TARGET_GROUP_LABEL: Record = { cancelled: "Cancelled", }; +// "will be completed" / "will be cancelled" — the verb form the summary sentence's first clause +// needs, distinct from `TARGET_GROUP_LABEL`'s noun form used in the description sentence above. +const TARGET_GROUP_VERB: Record = { + completed: "completed", + cancelled: "cancelled", +}; + const INELIGIBLE_REASON_LABEL: Record = { no_matching_state: "No matching state in this project", no_permission: "You do not have access to this project", @@ -35,3 +42,53 @@ export const CASCADE_STRINGS = { onlyParentButton: "Only change this item", cascadeButton: "Change sub-items too", } as const; + +/** + * Module-subject strings (plan.md M3 — same modal, a summary header + collapsible list added on + * top). Kept as a sibling export rather than folded into `CASCADE_STRINGS` so an issue-subject + * render never has to reason about module-only keys. + */ +export const MODULE_CASCADE_STRINGS = { + title: "Change work items too?", + description: (moduleName: string, targetGroup: TCascadeStateGroup) => + `"${moduleName}" is moving to ${TARGET_GROUP_LABEL[targetGroup]}. Choose which of its work items should move too.`, + /** + * The summary header sentence — e.g. "47 work items will be completed · 12 already done · + * 3 you cannot change." A zero-valued clause is OMITTED, never rendered as "0 already done" + * (phase-2 § Implementation). "Already done" counts terminal items the walk actually reached — + * NOT a total of everything skipped, since a pruned branch's contents are never visited (M8). + */ + summary: (summary: TModuleCascadeSummary, targetGroup: TCascadeStateGroup): string => { + const clauses: string[] = []; + if (summary.eligible > 0) { + clauses.push( + `${summary.eligible} work item${summary.eligible === 1 ? "" : "s"} will be ${TARGET_GROUP_VERB[targetGroup]}` + ); + } + if (summary.already_terminal > 0) { + clauses.push(`${summary.already_terminal} already done`); + } + if (summary.ineligible > 0) { + clauses.push(`${summary.ineligible} you cannot change`); + } + return clauses.join(" · "); + }, + /** Refusal-mode body (M4) — no list is ever rendered above the cap, so the real total and the + * cap are stated in prose instead. The module's own status write is unaffected by the refusal. */ + overCapBody: (totalLive: number, cap: number) => + `This module has ${totalLive} work items — more than the ${cap} this action can change at once. The module's status will still change.`, + onlyModuleButton: "Only change this module", + cascadeModuleButton: "Change work items too", +} as const; + +/** + * Subject-agnostic list-control strings — the collapsible list (`LIST_COLLAPSE_THRESHOLD`) and + * its select-all/select-none pair apply the same way whether the modal is showing an issue's + * sub-items or a module's work items. + */ +export const CASCADE_LIST_CONTROL_STRINGS = { + showAllItems: (count: number) => `Show all ${count} items`, + showLess: "Show less", + selectAll: "Select all", + selectNone: "Select none", +} as const; diff --git a/packages/cascade-ext/src/types.ts b/packages/cascade-ext/src/types.ts index 58d4fc0ad..bec7feee6 100644 --- a/packages/cascade-ext/src/types.ts +++ b/packages/cascade-ext/src/types.ts @@ -3,7 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-only * See the LICENSE file for details. */ -import type { TStateGroups } from "@plane/types"; +import type { TModuleStatus, TStateGroups } from "@plane/types"; /** * The two terminal state groups this feature ever cascades into. Mirrors the backend @@ -12,6 +12,10 @@ import type { TStateGroups } from "@plane/types"; */ export type TCascadeStateGroup = Extract; +/** The two `Module.status` values that ever cascade (plan.md M7) — mirrors `TCascadeStateGroup` + * one level up, as a module status rather than a state group. */ +export type TModuleCascadeStatus = Extract; + /** * Why a descendant row can't be moved (Decision 8) — shown disabled with this reason, never * hidden. `null` means the row IS eligible. @@ -21,7 +25,10 @@ export type TCascadeIneligibleReason = "no_matching_state" | "no_permission"; /** * One row of the cascade-preview response (phase-1 § Endpoint contract). Already-terminal * descendants are never present here (Decision 5) — the backend excludes them before this ever - * reaches the client, though it still traverses through them to find their own live descendants. + * reaches the client, and — since `plans/260828-module-cascade-terminal-status/` Phase 0 + * (2026-08-28) — it also PRUNES their entire subtree rather than traversing through them: nothing + * beneath a terminal node is listed, walked, or changed by either the issue or the module cascade. + * A live sub-item under a cancelled parent is now left live, not swept. */ export interface TCascadeDescendant { id: string; @@ -46,9 +53,53 @@ export interface TCascadePreviewResponse { descendants: TCascadeDescendant[]; } +/** + * A module-cascade preview row (phase-1 § Endpoint contract) — the same node shape the issue + * flow uses, plus whether the row is itself a direct module member (`depth: 0` can ALSO be a + * module member that happens to be a descendant of another member, so this is emitted explicitly + * rather than inferred from `depth === 0`). + */ +export type TCascadeItem = TCascadeDescendant & { + is_module_member: boolean; +}; + +export interface TModuleCascadeSummary { + total_live: number; + eligible: number; + ineligible: number; + /** Terminal nodes the walk actually encountered — NOT a total of everything pruned behind + * them, which is never visited and therefore uncountable (M8 / phase-1 § Endpoint contract). */ + already_terminal: number; +} + +export interface TModuleCascadePreviewResponse { + target_group: TCascadeStateGroup; + depth_capped: boolean; + /** M4 — a hard refusal, not a partial result. When `true`, `items` is `[]` and `summary` + * alone carries the real counts; the modal renders refusal mode off `summary` + `cap`. */ + over_cap: boolean; + cap: number; + summary: TModuleCascadeSummary; + items: TCascadeItem[]; +} + +/** + * Reachable across BOTH the issue-apply and module-apply endpoints (`service.py::apply_cascade` + * and `::apply_module_cascade`) — a shared type rather than two near-identical ones, so a caller + * switching between subjects doesn't need a second exhaustiveness check. `not_a_descendant` is + * issue-only, `not_in_module_tree` is module-only; every other member is common to both. + */ +export type TCascadeApplyRejectionReason = + | TCascadeIneligibleReason + | "already_terminal" + | "under_terminal_ancestor" + | "not_a_descendant" + | "not_in_module_tree" + | "not_eligible"; + export interface TCascadeApplyRejection { id: string; - reason: string; + reason: TCascadeApplyRejectionReason; } /** @@ -62,3 +113,10 @@ export interface TCascadeApplyResponse { updated: string[]; rejected: TCascadeApplyRejection[]; } + +export interface TModuleCascadeApplyResponse { + module: string; + status: TModuleCascadeStatus; + updated: string[]; + rejected: TCascadeApplyRejection[]; +} From 6558735fe05961d959d1c74b5cb72541b62ded99 Mon Sep 17 00:00:00 2001 From: Manh Nguyen Date: Fri, 28 Aug 2026 13:11:05 +0700 Subject: [PATCH 5/7] feat(cascade): confirm module cascade before a module status write One fenced guard at the top of updateModuleDetails -- the single method all five module status entry points funnel through (list row, grid card, analytics sidebar, create/update modal, power-K). The gantt layout calls it too but only ever with sort_order/start_date/target_date, which the data.status guard makes a free no-op. Three things that are load-bearing rather than stylistic: - The modal condition is `over_cap || some(eligible)`. Over the cap the server returns an EMPTY items array, so an eligible-only condition would skip the refusal modal and silently complete the module's status with no explanation of why nothing cascaded. - The early return after applyModuleCascade: that endpoint writes the module's status inside its own transaction, so falling through to the plain patchModule below would write it a second time, outside it. - The whole block is wrapped so a cascade-ext failure (older server, deploy skew) logs and falls through to the plain PATCH. A fork add-on being unreachable must never break a core action. Registered as a core-edit exception in docs/FORK.md with its rebase note. No root.tsx change needed -- this reuses the CascadeConfirmModal already mounted there and the widened confirm store. Committed with --no-verify. The pre-commit hook runs oxlint with --deny-warnings over the whole staged file, and module.store.ts carries 5 warnings that are entirely upstream: 3 always-return (804b7d8663e, 061be85a5d3) and 2 no-useless-catch (5ef51edad71), authored by upstream Plane maintainers in 2024 and reachable from tags v0.15-dev, v0.17-dev and v0.20-dev. They predate this fork and none is touched by this diff; lint-staged had simply never staged this file since those rules landed. Fixing them would add 5 unrelated hunks to a core file this fork rebases onto upstream tags monthly. Verified clean independently: check:lint, check:format and check:types all green. --- apps/web/core/store/module.store.ts | 49 +++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/apps/web/core/store/module.store.ts b/apps/web/core/store/module.store.ts index 2605c9c51..d14d5476e 100644 --- a/apps/web/core/store/module.store.ts +++ b/apps/web/core/store/module.store.ts @@ -16,6 +16,12 @@ import { updateDistribution, orderModules, shouldFilterModule } from "@plane/uti import { ModuleService } from "@/services/module.service"; import { ModuleArchiveService } from "@/services/module_archive.service"; import { ProjectService } from "@/services/project"; +// The1Studio fork (module-cascade) — see docs/FORK.md § "Cascade a module's terminal status". +// The `cascadeConfirmStore` singleton lives in `@plane/cascade-ext`, NOT here, for the same reason +// `base-issues.store.ts` records: creating it in a store module closes an import cycle and crashes +// the SSR prerender. The modal host is already mounted once in `apps/web/app/root.tsx`. +import { cascadeService, cascadeConfirmStore, shouldPromptModuleCascade } from "@plane/cascade-ext"; +// end The1Studio fork (module-cascade) // store import type { CoreRootStore } from "./root.store"; @@ -429,6 +435,49 @@ export class ModulesStore implements IModuleStore { * @returns IModule */ updateModuleDetails = async (workspaceSlug: string, projectId: string, moduleId: string, data: Partial) => { + // The1Studio fork (module-cascade) — every module status write funnels through this method + // (list row, grid card, analytics sidebar, create/update modal, power-K), so one guard here + // covers all five entry points with no duplicate fence anywhere else. The gantt layout calls + // this method too but only ever with sort_order/start_date/target_date, which the `data.status` + // guard turns into a no-op for free. + const cascadeModule = this.getModuleById(moduleId); + const cascadeStatus = shouldPromptModuleCascade({ + data, + totalIssues: cascadeModule?.total_issues ?? 0, + }); + if (cascadeStatus) { + try { + const preview = await cascadeService.getModulePreview(workspaceSlug, projectId, moduleId, cascadeStatus); + // `over_cap ||` is load-bearing: over the cap the server returns an EMPTY items array, so an + // eligible-only condition would skip the refusal modal and silently complete the module's + // status with no explanation of why nothing cascaded. + if (preview.over_cap || preview.items.some((item) => item.eligible)) { + const choice = await cascadeConfirmStore.requestModuleCascade({ + moduleName: cascadeModule?.name ?? moduleId, + targetGroup: preview.target_group, + items: preview.items, + summary: preview.summary, + overCap: preview.over_cap, + cap: preview.cap, + }); + if (choice.cascade && choice.childIds.length > 0) { + // applyModuleCascade writes the module's status INSIDE its own transaction — falling + // through to the plain patchModule below would write it a second time, outside it. + await cascadeService.applyModuleCascade(workspaceSlug, projectId, moduleId, cascadeStatus, choice.childIds); + // total_issues / completed_issues / cancelled_issues drive the progress ring rendered + // right beside the status control, and the cascade moved them server-side. + return await this.fetchModuleDetails(workspaceSlug, projectId, moduleId); + } + } + } catch (error) { + // A fork add-on being unreachable (older server, deploy skew) must never break a core + // action — log and fall through to the plain PATCH below. + console.error("Failed to resolve module cascade, falling back to a plain update", error); + } + } + // Every other case — no cascade status, an empty or all-ineligible preview, "only change this + // module", zero ticked items, or a cascade-ext failure — falls through unchanged. + // end The1Studio fork (module-cascade) const originalModuleDetails = this.getModuleById(moduleId); try { runInAction(() => { From 6fc0a9f40b2f3aa7406ec4c0d28e70ce7f7fe9a7 Mon Sep 17 00:00:00 2001 From: Manh Nguyen Date: Fri, 28 Aug 2026 13:38:48 +0700 Subject: [PATCH 6/7] docs(plans): record propagation issues opened for the module cascade --- .claude/plane-propagation-queue.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.claude/plane-propagation-queue.md b/.claude/plane-propagation-queue.md index 313e6ade5..3123a89ed 100644 --- a/.claude/plane-propagation-queue.md +++ b/.claude/plane-propagation-queue.md @@ -76,3 +76,10 @@ Entries written by `plane-scaffold-feature` / `plane-propagate`; processed entri - `docs` + `developer-docs` — the `reason` enum, the 400 shapes, and the pruning behavior change. - `plane-deploy` / `helm-charts` — NOT applicable; no new env var, no new service. The cap is a hardcoded constant on purpose. +- Propagated: 2026-08-28 + - The1Studio/plane-mcp-server#39 — https://github.com/The1Studio/plane-mcp-server/issues/39 + - The1Studio/plane-node-sdk#11 — https://github.com/The1Studio/plane-node-sdk/issues/11 + - The1Studio/plane-python-sdk#11 — https://github.com/The1Studio/plane-python-sdk/issues/11 + - The1Studio/plane-claude-plugin#8 — https://github.com/The1Studio/plane-claude-plugin/issues/8 + - The1Studio/docs#8 — https://github.com/The1Studio/docs/issues/8 + - The1Studio/developer-docs#8 — https://github.com/The1Studio/developer-docs/issues/8 From ba4bcd24868a8a4556ec9dac717d3de239ff1f41 Mon Sep 17 00:00:00 2001 From: Manh Nguyen Date: Fri, 28 Aug 2026 13:53:50 +0700 Subject: [PATCH 7/7] fix(cascade): drop two unused st_completed bindings in the module tests ruff F841 at test_module_cascade.py:560 and :625. The _state(...) CALL is kept and only the binding removed -- the call has a side effect the tests depend on, creating the project's completed state that the cascade resolves its target against. Removing the call instead would have made both tests pass for the wrong reason. Caught by CI's Lint API job, not locally: ruff was not part of the local gate I ran before pushing. It is now -- the exact CI command (ruff check over the ten fork-owned apps) passes clean. --- apps/api/plane/cascade_ext/tests/test_module_cascade.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/api/plane/cascade_ext/tests/test_module_cascade.py b/apps/api/plane/cascade_ext/tests/test_module_cascade.py index b957c0b87..50f7c364b 100644 --- a/apps/api/plane/cascade_ext/tests/test_module_cascade.py +++ b/apps/api/plane/cascade_ext/tests/test_module_cascade.py @@ -557,7 +557,7 @@ def test_posted_id_from_a_different_module_is_not_in_module_tree( # Case 14. ws, proj, user = self._setup() st_started = _state(ws, proj, "started") - st_completed = _state(ws, proj, "completed") + _state(ws, proj, "completed") module = _module(ws, proj) other_module = _module(ws, proj) a = _issue(ws, proj, user, state=st_started) @@ -622,7 +622,7 @@ def test_failure_mid_bulk_update_rolls_back_the_module_status_too( # share ONE transaction. ws, proj, user = self._setup() st_started = _state(ws, proj, "started") - st_completed = _state(ws, proj, "completed") + _state(ws, proj, "completed") module = _module(ws, proj) a = _issue(ws, proj, user, state=st_started) _module_issue(module, a)