diff --git a/.claude/plane-propagation-queue.md b/.claude/plane-propagation-queue.md index d6586fb81..3123a89ed 100644 --- a/.claude/plane-propagation-queue.md +++ b/.claude/plane-propagation-queue.md @@ -39,3 +39,47 @@ 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. +- 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 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..50f7c364b --- /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") + _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") + _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) 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(() => { 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/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[]; +} 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 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.