feat(plugin-kanban): a drop that makes fields required collects them instead of dead-ending (#4254) - #4555
Merged
Conversation
…instead of dead-ending (#4254) Dragging a card into a column whose value flips a field's `requiredWhen` predicate to TRUE PATCHed the column value alone. The engine refused the whole update — correctly; that is what the predicate declares — and the board had no way to finish the move, so closing a won deal meant leaving the board for the record form. The drop now pre-evaluates the target column's predicates against the record WITH the target value applied. Fields the move makes required while still empty are collected in a small dialog, and the column value plus everything collected go out as ONE PATCH: two writes would leave the record in the refused state if the second failed. A drop that triggers no predicate keeps its exact path, PATCH body included. One evaluator, not two: the verdict is `@object-ui/core`'s `resolveFieldRuleState`, the same function the record form, the wizard and the line-item grid already use, delegating to `@objectstack/formula`'s CEL engine — so the board's prompt and the server's enforcement cannot drift. Emptiness is core's `isMissingForRequired`, the presence contract the form and the server share. Every control is `@object-ui/fields`' `FieldEditWidget`, the widget the record form renders for that type, so no second set of field-rendering decisions is introduced. Four kinds of field are deliberately not collected and fall through to the unchanged PATCH, where the refusal is legible since objectstack#7525: one that already has a value, one `visibleWhen` hides, a readonly one, and a type with no edit widget. Cancel writes nothing and leaves the card in its source column; a combined PATCH refused for some other reason surfaces the refusal and rolls back exactly as a plain rejected move does (#4138), rather than looping the dialog on an arbitrary server error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
PM step-7 复核 — ACCEPT ON IMPLEMENTATION; flip+arm deferred to CI convergence (session_017Qqyix2QcnpUC9XeYVDzx3) Implementation review is complete and positive; the only outstanding item is the repo-wide Actions stall (evidence in the report is conclusive: main's own merge-queue run and a sibling PR stalled at the same ~08:04 boundary with zero step movement, while every pre-boundary job completed in minutes — infrastructure, not this change). The PM will re-read this PR's per-job conclusions when the runner pool recovers and then flip+arm. What is accepted:
Generated by Claude Code Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4254
Dragging a card into a column whose value flips a field's
requiredWhenpredicate to TRUE used to PATCH the column value alone. The engine refused the whole update — correctly, that is what the predicate declares — and the board had no way to finish the move: closing a won deal meant abandoning the board for the record form. HotCRM's opportunity pipeline is the reported case (win_reasonrequired when stage isclosed_won), but the dead end belonged to every board whose target column carries a conditional requirement.Implements the maintainer's approved shape (2026-08-12): pre-evaluate, then collect, then one PATCH.
What changed
On drop, the board evaluates the target column's predicates against the record with the target value applied, before writing anything. If that makes fields required while they are still empty, a small dialog collects exactly those fields and the column value plus everything collected go out as one PATCH — two writes would leave the record in the refused state if the second failed. A drop that triggers no predicate is untouched, PATCH body included.
Against the dispatch ruling
1. One evaluator, measured. The record form resolves
requiredWhenthrough@object-ui/core'sresolveFieldRuleState(packages/core/src/evaluator/fieldRules.ts), exported from the package barrel viaevaluator/index.ts. plugin-kanban already declares@object-ui/core, so it is reachable with no seam edit and no new dependency — the same barrelcomponents/renderers/form/form.tsx:9imports from. Nothing was hand-rolled.The CEL subset was measured against the card's own predicate rather than assumed —
@objectstack/formula17.0.0-rc.6,has(record.stage) && record.stage == "closed_won":All four
ok:true—has(),==and&&are genuinely evaluated, not fail-open verdicts wearing the right answer.2. Dialog reuses the record form's renderers. Every control is
@object-ui/fields'FieldEditWidget— by its own header "the SAME dedicated widgets the form renders", already the basis of the grid's inline editor and the detail page's inline edit. A select edits as a select, a date as a date picker. No parallel mini-form. Local required validation before submit uses core'sisMissingForRequired, the presence contract the form and the server share, sofalseand0count as answers and are not re-asked.3. Flow. Empty required-set leaves today's PATCH untouched and pinned byte-identical; non-empty opens the dialog and submits
{stage, ...collected}as one PATCH. Cancel writes nothing and leaves the card in its source column. A combined PATCH still refused for some other reason surfaces the legible refusal and rolls back exactly as a plain rejected move does — the #4138 contract, unchanged — and does not loop the dialog.4. Fallback clause not used. Pre-evaluation is feasible; the prompt-on-reject fallback was not needed and was not silently substituted.
5. i18n. Two new
kanban.*keys through this package's existinguseSafeTranslatechannel, added to all ten packs in each pack's own quote convention and key order.common.cancel,common.requiredandkanban.moveCardare reused rather than minted twice. NoI18nProvideris mounted in the new tests, per the #4514 trap.Deliberate non-collection
Four kinds of required-and-empty field are not collected, each falling through to the unchanged PATCH where the refusal is legible (objectstack#7525 closed): one that already has a value, one
visibleWhenhides, a readonly one, and a type with no edit widget. A dialog row with no control would be a worse dead end than the one being fixed.Red-first, predicted before running
Predicted 5 red / 3 green; measured exactly that against unfixed code. The sharpest red names the defect itself rather than a missing dialog:
That lone
{stage}PATCH is the issue.Reverse verification — direction predicted first: removing only the pre-evaluation gate limb (leaving the module and dialog in place) should turn the 5 feature tests red and leave the 3 pins green. Measured: exactly that,
5 failed | 3 passed. Restored via patch-file plus backup copy, nevergit stash, and the restore was sha256-verified on all six touched files.Verification
vitest run packages/plugin-kanban/ packages/i18n/type-check(both tsc passes, kanban + i18n)eslintcheck:i18n-keysenvaluecheck:i18n-driftenvalues changedcheck:control-bytes/check:phantom-deps/ changeset gates.d.tsdiff, both packages, measured both waysChangeset:
@object-ui/plugin-kanbanminor (new user-facing capability),@object-ui/i18npatch (graded on its measured-identical.d.ts). Never major.Surface
packages/plugin-kanban/**, the ten locale packs, one changeset. No edits topackages/fields,data-objectstack,metadata-admin, studio inspectors,DashboardRenderer/ListView, orcontent/docs/releases/. Consumption of@object-ui/fieldsand the form machinery is read-only.Generated by Claude Code