Skip to content

feat(workflow-rules): project pairing UI (depends on #238) - #244

Open
ahmadoqsrawi wants to merge 11 commits into
Platform-Collective:developfrom
ahmadoqsrawi:feature/workflow-rules-ui
Open

ahmadoqsrawi wants to merge 11 commits into
Platform-Collective:developfrom
ahmadoqsrawi:feature/workflow-rules-ui

Conversation

@ahmadoqsrawi

Copy link
Copy Markdown

Depends on #238. This branch is stacked on top of feature/cross-project-workflow-rules. The UI commits are the only ones in this PR; the server trigger + data model shown in the diff come from #238 and will drop out once that merges.

What this adds

A Workflow rules tab on every tracker project (same navigator slot style as Templates and Milestones). It lets a project admin pair the project with another one and declare how statuses mirror between them, without touching the DB.

Two surfaces:

  1. Pairing wizard (PairProjectEditor.svelte) - the default path. Pick a linked project, give the pair a name, toggle bidirectional, review the status map.

    • Initial map is auto-filled by status category first (UnStarted/ToDo/Active/Won/Lost) then by name. A new pair is usable with zero clicks when the two projects share a TaskType.
    • allowedLoops are derived automatically. Non-terminal to non-terminal is allowed in both directions, Won/Lost stay one-way so closed work does not resurrect.
    • Forward + reverse rules are saved as a single atomic sequence.
  2. Advanced editor (EditWorkflowRule.svelte) - exposed via an "Advanced" button on a pairing row. Full per-rule control over forwardMap and allowedLoops for the cases the wizard's defaults do not cover.

New package

plugins/workflow-rules-resources holds the three Svelte components and wires them into the plugin descriptor as AnyComponent slots. dev/prod registers the location so the bundle loads.

Model change

WorkflowRule gains an optional name: string so pairings can be labelled in the list. Everything else from #238 is unchanged.

Screens

The panel sits inside the project's existing header + breadcrumbs chrome, uses IssueStatusIcon from tracker-resources for status rows, and relies on existing design tokens (--theme-list-row-color, --theme-progress-color, etc). No new CSS variables, no new global styles.

Testing done

Built the whole stack against this branch, flipped prod workspace at workspace.pixeliquedigital.com onto it, exercised the pairing wizard against two live tracker projects (Content Calendar <-> Design Requests), confirmed:

  • forward + reverse rules written atomically
  • status matrix auto-filled by category then by name
  • backward move from "In Progress" to a lower-ranked target blocked by the server guard from feat: cross-project workflow rules (closes discussion #235) #238
  • Won/Lost stays one-way automatically
  • Advanced editor still writes custom allowedLoops entries correctly
  • Rename and disable toggle round-trip

Review order

Please review #238 first; this one builds directly on it. Will rebase onto main after #238 merges.

Companion doc for Platform-Collective/platform discussion Platform-Collective#235. Describes:
- WorkflowRule data model (per-project rule attached to a linked project)
- forwardMap + allowedLoops + autoCreate shape
- Server-side Tx trigger pattern reusing TxApplyIf.scope for echo-prevention
- UI tab under Project Settings with a matrix editor
- Explicitly deferred to a later feature: fan-out, field mirror, delete
  propagation, JSON import/export
…rules

Adds @hcengineering/workflow-rules — the type-declaration plugin for the
feature in discussion Platform-Collective#235. Compiles cleanly against the existing monorepo
with no changes to tracker, task or core.

Shape:
- WorkflowRule (space-attached Doc, one direction per rule)
- forwardMap (Ref<IssueStatus> -> Ref<IssueStatus>)
- allowedLoops (explicit backward transitions on the target)
- autoCreate (triggerStatuses + componentMap + inherit whitelist)
- linkKind ('subIssue' | 'parentIssue' for the MVP, extension point)
- enabled soft-off switch

See docs/workflow-rules/DESIGN.md for the full rationale. Model registration,
server trigger, and UI follow in subsequent commits.
Adds the four packages that let the Transactor run the cross-project status
mirror and wires them into the existing bootstrap:

- models/workflow-rules — TWorkflowRule class, DOMAIN_WORKFLOW_RULES
- server-plugins/workflow-rules — OnWorkflowRuleMirror trigger descriptor
- server-plugins/workflow-rules-resources — trigger implementation:
    * TxUpdateDoc<Issue> listener with status-change short-circuit
    * resolveCounterpart() for subIssue + parentIssue link kinds
    * checkRegressionGuard() consulting TaskType.statuses as rank
    * echo-prevention via "no change needed" counterpart.status === target
- models/server-workflow-rules — createDoc(serverCore.class.Trigger) binding
- models/all + server/server-pipeline — new workflow-rules modules in the
  model aggregator and the resource location registry

Jest unit tests on the pure logic helpers:
  Test Suites: 1 passed, 1 total
  Tests:       9 passed, 9 total

Auto-create path is a logged no-op for this PR; the sequence allocation
needed to emit TxCreateDoc<Issue> wants its own review and is tracked for
the follow-up. UI tab and i18n assets also follow in later commits.
Replaces em-dash and en-dash characters with plain "-" across the files
authored for this feature. Matches the house style used elsewhere in the
tree and avoids rendering surprises in editors with non-UTF8 defaults.
No logic changes.
Adds @hcengineering/workflow-rules-assets with:
- lang/en.json: English strings for the matrix editor UI
- assets/icons.svg: single workflow-rule icon
- src/index.ts: loadMetadata(workflow-rules.icon)
- src/lang.ts: dynamic Loader

Wires the loader into:
- dev/prod/src/platform.ts (webpack client bundle)
- server/server-pipeline/src/internationalization.ts (server i18n)

Follows the shape used by every other *-assets package in the tree, so
rush-build-cache and locale-expansion by translators work the same way.
Three fixes to make the whole monorepo pass "rush validate":

- plugins/workflow-rules: WorkflowRule class ref typed as
  Ref<Class<WorkflowRule>> so @model() accepts it. The earlier Ref<Doc>
  was too wide and tripped TS2345 on the decorator call site.
- server-plugins/workflow-rules-resources: narrow the findAll result via
  `as unknown as WorkflowRule[]` so the strict overlap check in TS2352
  is satisfied without widening the public call signature.
- plugins/workflow-rules-assets: extend profiles/assets instead of
  profiles/default (unlocks `require` from @types/node) and switch the
  eslint config to the matching assets profile. Matches tracker-assets.
The earlier commit wired model-server-workflow-rules into the server plugin
list but missed the matching client-side @hcengineering/model-workflow-rules
entry. Without it the TWorkflowRule class is never built into the workspace
model, so client findAll on tracker.class.WorkflowRule throws
"domain not found: workflow-rules:class:WorkflowRule" and the trigger has
nothing to look up.

Caught by running the stack locally end-to-end and querying the loaded
hierarchy.
…, strict guards

Addresses the inline review on PR Platform-Collective#238:

- Skip derived writes (TxUpdateDoc.space === core.space.DerivedTx) so a
  mirror update cannot itself trigger another mirror. One hop semantics,
  independent of status equality. The equality short-circuit alone did
  not catch non-inverse mapping pairs across two linked projects.
- Validate a persisted WorkflowRule doc on read (shape, linkKind,
  forwardMap entries, allowedLoops entries, autoCreate.triggerStatuses).
  A malformed rule is skipped with a warn log; the batch keeps going for
  the other rules on the same source.
- Resolve forwardMap[newStatus] via Object.prototype.hasOwnProperty.call
  and require a non-empty string value. Avoids prototype chain hits and
  non-string stored values resolving to junk targets.
- Make checkRegressionGuard fail closed: a missing TaskType, a status
  that is not a member of the target TaskType's statuses, or malformed
  allowedLoops now blocks the move. An allowedLoops entry can never
  authorize a status that does not exist in the target.
- Match isRegression to the new fail-closed contract. Unknown statuses
  now return true (block). The jsdoc explains the trigger validates
  TaskType membership upstream, so the helper is defense in depth.
- Require a unique child in the subIssue counterpart resolver. If a
  parent has two or more children in the linked project, return
  undefined (treat as absence) rather than letting the database pick
  one at its own ordering.

Tests updated for the fail-closed isRegression behavior. All 9 tests
pass locally; rush validate for the resources package passes.

Cross-project authorization and the batch pending-status map are being
tracked as a follow-up PR because they need new server policy and
multi-rule conflict ordering, which deserve their own review surface.
Adds a Workflow rules tab under each tracker project (Project Settings
style navigator entry, same pattern as Templates and Milestones) so
admins can manage cross-project pairings without touching the DB.

New plugin: @hcengineering/workflow-rules-resources
  - WorkflowRulesPanel.svelte: card list of pairings for the current
    project. Groups the forward + reverse rules into one visual row,
    shows direction (Two-way / One-way), the mapping count, enabled
    state (dot indicator + Toggle), and inline Advanced + Remove
    actions. Breadcrumbs header matches other project tabs.
  - PairProjectEditor.svelte: pairing wizard. Pick a linked project,
    give the pair a name, toggle bidirectional on/off, then review
    the status mapping. The initial map is auto-filled by matching
    categories (UnStarted/ToDo/Active/Won/Lost) and then by status
    name, so a new pair is usable with zero clicks when the two
    projects share a TaskType. The regression-guard allowedLoops are
    derived automatically: non-terminal to non-terminal is always
    allowed, Won/Lost stay one-way to avoid resurrecting closed work.
    Saves forward + reverse in one atomic sequence.
  - EditWorkflowRule.svelte: advanced per-rule matrix editor retained
    for power users who need full control over forwardMap and
    allowedLoops without the auto-pair shaping.

Plugin wiring
  - WorkflowRule gains an optional name field (plugins + model) so
    pairings can be labelled.
  - Three AnyComponent slots in the workflow-rules plugin descriptor.
  - en.json strings for the panel, wizard, and advanced editor.

Navigator
  - models/tracker: WorkflowRulesPanel registered as a Project special
    between Milestones and Templates. Uses the plugin icon so the tab
    is self-explanatory.
  - dev/prod: addLocation for the new resources plugin so the UI
    bundle actually loads at runtime.
  - rush.json + pnpm-lock.yaml: new project entry.

The server trigger and data model from Platform-Collective#238 are the dependency; this
is the admin surface for them.
@ahmadoqsrawi
ahmadoqsrawi changed the base branch from main to develop October 27, 2026 07:07
…ig, tests

Addresses the remaining blockers and polish from the second review pass.

Authorization at execution (comment 3)
  The derived mirror update bypasses the ordinary space permission
  middleware, so the trigger now stands in for it. Before emitting a
  write it resolves the acting account from
  control.ctx.contextData.socialStringsToUsers.get(tx.modifiedBy) and
  requires one of:
    - system account,
    - AccountRole.Admin,
    - member or owner of the target Space.
  Source-project membership alone is not sufficient. Results are
  memoized per target space for one invocation. An unresolvable acting
  account fails closed.

Batch pending-status map (comment 8)
  Transactions in one invocation are now sorted by modifiedOn, matching
  the derived-tx sort in triggers.ts. A pendingStatuses map retains the
  queued target status per counterpart across the whole batch, and
  equality + regression checks run against the effective pending status
  instead of the stale persisted one. Two rules mapping the same source
  event to different targets, or two source transactions on children of
  one parent, can no longer slip a forbidden regression past the guard.
  checkRegressionGuard now takes an explicit effectiveFrom.

Configure toggle actually stops mirroring
  Settings > Configure writes a PluginConfiguration row for the plugin.
  The trigger now reads it via control.modelDb.findAllSync on every
  invocation and returns [] when the row says enabled: false. Missing
  or unreadable config is treated as enabled, matching the default
  registration in models/all.

Configure card label/description/icon
  models/all now binds workflowRules.string.ConfigLabel +
  ConfigDescription IntlStrings and workflowRules.icon.WorkflowRule,
  replacing the raw strings and 'icon: undefined as any' from the
  previous pass. en.json gains the two new strings.

Tests + helper cleanup
  - Dropped applyRuleToUpdate. The trigger does the hardened lookup
    inline (own-property guard, non-empty string check). The tests that
    exercised it are removed; the file no longer re-exports it.
  - New trigger.spec.ts with 17 cases covering: mirror emission,
    DerivedTx skip, isValidRule shape enforcement, prototype pollution
    guard on forwardMap, missing TaskType as absence, ambiguous
    subIssue counterpart as absence, allowedLoops entry allowing a
    specific backward move, batch pending-status map blocking a
    second-rule regression against a queued target, target-side
    authorization refusal for a non-member, admin bypass, and the
    plugin kill switch.
  - logic.spec.ts trimmed to the isRegression cases.

Docs
  docs/workflow-rules/DESIGN.md: dropped the TxApplyIf cascade-token
  description, replaced with the DerivedTx-space check we actually
  ship. Added authorization and batch pending-status steps so the file
  matches the code.

@MichaelUray MichaelUray left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed only the delta over #238 (pc/pr-238...HEAD, 21 files). The pairing wizard is a nice idea, but a few things need to change first.

Blocking

  1. No authorization for rule mutations, in the UI or on the server. Pair, Edit, Toggle, Advanced and Delete are offered to everyone who can open the panel, and the project pickers list every readable project, including public projects the user is not a member of (PairProjectEditor.svelte:38, EditWorkflowRule.svelte:29, WorkflowRulesPanel.svelte:58). The wizard writes the reverse rule into the linked project's space (PairProjectEditor.svelte:200) and toggle/remove also change the other project's rule (WorkflowRulesPanel.svelte:71-83). Nothing on the server stops this either, since there is no permission for WorkflowRule (see my #238 review). So a member of A alone can configure, pause or delete rules in B. Please gate these actions (and the server side in #238) on the user's rights in both projects.
  2. Saving a pair from the other project breaks it. linkKind is hardcoded on save (PairProjectEditor.svelte:174-191): create A → B (A's rule subIssue, B's reverse parentIssue), then open the pairing from B and press Save without changes: both rules get the swapped link kind and mirroring silently stops in both directions.
  3. Changing the linked project while editing corrupts the pair (PairProjectEditor.svelte:106-123,197-200,231). The picker stays enabled in edit mode; switching B to C keeps B's status mappings, retargets the forward rule to C, but still updates the reverse rule in B's space and never creates C → A. The Advanced editor (EditWorkflowRule.svelte:69) can likewise change one rule and leave its partner active.

Should fix

  1. The wizard overwrites settings: every save regenerates allowedLoops and forces enabled: true (PairProjectEditor.svelte:170-180,194-195), so renaming a paused pair resumes it and custom loops from Advanced are lost.
  2. Not atomic: forward and reverse writes are separate calls without client.apply() or error handling (PairProjectEditor.svelte:182-203, WorkflowRulesPanel.svelte:71-81), although the description says they are atomic; a failure halfway leaves a half-written pair.
  3. Stale loads / phantom selection: status loads can complete out of order when switching projects quickly, Save stays enabled while loading, and the project DropdownLabels (autoSelect on) shows a project as chosen while the editor still has none. Only the first task type's statuses are offered.
  4. Duplicates and visibility: duplicate pairings are allowed and the panel matches reverse rules with a plain find, so deleting one duplicate can remove the other's reverse rule; one-way rules that write into a project don't show up in that project's panel.
  5. Tab and registration: the tab is wired into the tracker model, so it shows even when the plugin is disabled; the desktop app registers neither the plugin's resources nor its strings.
  6. i18n: around 30 visible strings are hardcoded English (e.g. WorkflowRulesPanel.svelte:132-135, PairProjectEditor.svelte:127, EditWorkflowRule.svelte:61-65), "Pairing with …" is stored as the rule name, and only en.json exists while other packages in the fork ship all locales.
  7. Keyboard: the row has role="button" and tabindex="0" but a bare on:keydown (WorkflowRulesPanel.svelte:140), so Enter/Space does not open the editor.
  8. Tests: the new package has a Jest config but no spec; the critical paths (editing from either endpoint, retargeting, preserving settings, authorization) deserve regression tests.

Nits

  • In this fork .svelte files use the formatter style function name(; the three new components use function name (.
  • Unused Resource import in plugins/workflow-rules/src/index.ts:10 (lint).
  • Delete has no confirmation; the matching order (name, then category) is the reverse of what the description says; loops are not validated before save.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants