Skip to content

feat(releases): deep-link release validation errors to the offending field#12978

Open
EoinFalconer wants to merge 2 commits into
mainfrom
sapp-3892
Open

feat(releases): deep-link release validation errors to the offending field#12978
EoinFalconer wants to merge 2 commits into
mainfrom
sapp-3892

Conversation

@EoinFalconer

Copy link
Copy Markdown
Contributor

Description

Editors are told a document in a content release has validation errors, but the release detail view's validation indicator was a static icon (with only an error-count tooltip) — there was no way to find which field. This makes the indicator a link that opens the document in the release perspective and focuses the first field-level validation error. Fixes SAPP-3892.

It reuses the existing params.path field-focus mechanism (already used by the in-document validation inspector and the incoming-references deep links) — the release validation data already carries the marker paths, so this is mostly UI wiring.

What to review

  • getReleaseDocumentIntent.ts (new) — extracts ReleaseDocumentPreview's intent/perspective derivation (active / published / archived / cardinality-one) into a shared helper, with an optional focus path. ReleaseDocumentPreview now uses it (single source of truth → the validation link resolves to the exact same document + perspective as the title link).
  • DocumentTableColumnDefs.tsx — the validation column icon is now an IntentLink carrying path = the first field-level error's path (falls back to the first error). Existing count tooltip retained; aria-label added.
  • getIntentState.ts — preserve the path param in the edit fast-path (the already-open-pane case), matching resolveIntent's cold path which already forwards it.

Testing

  • Unit: getReleaseDocumentIntent (6 cases — params + perspective search params for active/published/archived/cardinality-one, plus path inclusion/omission); getIntentState (path preserved through the open-pane fast-path — fails without the fix).
  • pnpm --filter sanity check:types clean; releases tool tests pass (the ReleaseTypePicker scheduled-time test is a pre-existing flake — passed on isolated re-run).
  • Manual QA: in a release containing a document with a validation error, hover the release-table error icon (tooltip shows the count), then click it → the document opens in the release perspective with the first erroring field focused/scrolled into view.

The end-to-end UX is covered by the unit tests (each link in the chain: helper → intent params → getIntentState → existing field-focus) plus the manual-QA steps above; I wasn't able to run an automated browser check this session (the local browser tooling dropped its handle).

Notes for release

Clicking the validation error icon for a document in a content release now opens the document and jumps to the offending field, instead of only showing an error count.

…field

The validation error indicator in the release detail document table was a
static icon (with only an error-count tooltip), so editors were told a document
had errors but had no easy way to find which field. It is now a link that opens
the document in the release perspective and focuses the first field-level
validation error, reusing the existing params.path field-focus mechanism.

- Extract ReleaseDocumentPreview's intent/perspective derivation into a shared
  getReleaseDocumentIntent() helper so the validation link resolves to the same
  document/perspective, with an optional focus path.
- Make the validation column icon an IntentLink carrying path = first error path.
- Preserve the path param in getIntentState's edit fast-path so the deep-link
  works even when the target document pane is already open.

Fixes SAPP-3892.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 8, 2026 21:25
@vercel

vercel Bot commented Jun 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
auth-test-studio Ready Ready Preview, Comment Jun 9, 2026 8:39am
page-building-studio Ready Ready Preview, Comment Jun 9, 2026 8:39am
test-studio Ready Ready Preview, Comment Jun 9, 2026 8:39am

Request Review

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

📦 Bundle Stats — sanity

Compared against main (604e337e) · v5.30.0 (npm)

sanity

Metric Value vs main (604e337) vs v5.30.0
Internal (raw) 4.55 MB +309 B, +0.0% +1.8 KB, +0.0%
Internal (gzip) 1.05 MB +90 B, +0.0% +768 B, +0.1%
Bundled (raw) 12.33 MB +313 B, +0.0% -4.7 KB, -0.0%
Bundled (gzip) 2.77 MB +85 B, +0.0% -761 B, -0.0%
Import time 1.55s -20ms, -1.3% +56ms, +3.8%

bin:sanity

Metric Value vs main (604e337) vs v5.30.0
Internal (raw) 7.1 KB - -
Internal (gzip) 2.9 KB - -
Bundled (raw) 7.1 KB - -
Bundled (gzip) 2.8 KB - -
Import time 5ms -0ms, -0.9% +0ms, +7.2%

🗺️ View treemap · Artifacts

Details
  • Import time regressions over 10% are flagged with ⚠️
  • Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

📚 TypeDoc Generation Result

TypeDoc generated successfully!

  • File size: 8.2M
  • Total exports: 1067
  • Artifact: sanity-typedoc-9e8906c2df6eb53028b2044a8682dd5327b48e69

The TypeDoc JSON file has been generated and validated. All documentation scripts completed successfully.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

⚡️ Editor Performance Report

Updated Tue, 09 Jun 2026 08:44:01 GMT

Benchmark reference
latency of sanity@latest
experiment
latency of this branch
Δ (%)
latency difference
arrayI18n (simple-en) 50.0 efps (20ms) 51.3 efps (20ms) -1ms (-2.5%)
article (title) 35.7 efps (28ms) 34.5 efps (29ms) +1ms (+3.6%)
article (body) 42.1 efps (24ms) 42.4 efps (24ms) -0ms (-0.6%)
article (string inside object) 37.7 efps (27ms) 38.5 efps (26ms) -1ms (-1.9%)
article (string inside array) 41.7 efps (24ms) 40.0 efps (25ms) +1ms (+4.2%)
recipe (name) 83.3 efps (12ms) 83.3 efps (12ms) +0ms (-/-%)
recipe (description) 47.6 efps (21ms) 47.6 efps (21ms) +0ms (-/-%)
recipe (instructions) 99.9+ efps (9ms) 99.9+ efps (8ms) -1ms (-/-%)
singleString (stringField) 99.9+ efps (8ms) 99.9+ efps (9ms) +1ms (-/-%)
synthetic (title) 62.5 efps (16ms) 62.5 efps (16ms) +0ms (-/-%)
synthetic (string inside object) 62.5 efps (16ms) 62.5 efps (16ms) +0ms (-/-%)

efps — editor "frames per second". The number of updates assumed to be possible within a second.

Derived from input latency. efps = 1000 / input_latency

Detailed information

🏠 Reference result

The performance result of sanity@latest

Benchmark latency p75 p90 p99 blocking time test duration
arrayI18n (simple-en) 20ms 25ms 39ms 67ms 27ms 7.0s
article (title) 28ms 34ms 51ms 83ms 47ms 8.3s
article (body) 24ms 31ms 47ms 126ms 230ms 6.2s
article (string inside object) 27ms 32ms 38ms 59ms 33ms 7.0s
article (string inside array) 24ms 27ms 32ms 67ms 20ms 7.2s
recipe (name) 12ms 13ms 17ms 32ms 0ms 5.7s
recipe (description) 21ms 24ms 25ms 40ms 0ms 4.6s
recipe (instructions) 9ms 11ms 13ms 42ms 0ms 3.2s
singleString (stringField) 8ms 11ms 13ms 26ms 0ms 4.9s
synthetic (title) 16ms 17ms 20ms 93ms 1373ms 9.2s
synthetic (string inside object) 16ms 20ms 31ms 83ms 1106ms 9.0s

🧪 Experiment result

The performance result of this branch

Benchmark latency p75 p90 p99 blocking time test duration
arrayI18n (simple-en) 20ms 24ms 26ms 58ms 3ms 6.7s
article (title) 29ms 36ms 47ms 88ms 64ms 8.1s
article (body) 24ms 31ms 66ms 131ms 211ms 5.8s
article (string inside object) 26ms 31ms 38ms 61ms 46ms 7.0s
article (string inside array) 25ms 27ms 34ms 70ms 19ms 7.2s
recipe (name) 12ms 16ms 21ms 57ms 0ms 5.6s
recipe (description) 21ms 24ms 26ms 37ms 0ms 4.6s
recipe (instructions) 8ms 11ms 11ms 24ms 0ms 3.1s
singleString (stringField) 9ms 11ms 13ms 22ms 0ms 4.8s
synthetic (title) 16ms 19ms 21ms 82ms 1371ms 9.1s
synthetic (string inside object) 16ms 17ms 20ms 83ms 1032ms 8.8s

📚 Glossary

column definitions

  • benchmark — the name of the test, e.g. "article", followed by the label of the field being measured, e.g. "(title)".
  • latency — the time between when a key was pressed and when it was rendered. derived from a set of samples. the median (p50) is shown to show the most common latency.
  • p75 — the 75th percentile of the input latency in the test run. 75% of the sampled inputs in this benchmark were processed faster than this value. this provides insight into the upper range of typical performance.
  • p90 — the 90th percentile of the input latency in the test run. 90% of the sampled inputs were faster than this. this metric helps identify slower interactions that occurred less frequently during the benchmark.
  • p99 — the 99th percentile of the input latency in the test run. only 1% of sampled inputs were slower than this. this represents the worst-case scenarios encountered during the benchmark, useful for identifying potential performance outliers.
  • blocking time — the total time during which the main thread was blocked, preventing user input and UI updates. this metric helps identify performance bottlenecks that may cause the interface to feel unresponsive.
  • test duration — how long the test run took to complete.

Copilot AI 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.

Pull request overview

This PR improves the Releases tool UX by turning the release validation error indicator into a deep-link that opens the affected document (in the correct release/perspective context) and focuses the first field-level validation error via the existing params.path mechanism.

Changes:

  • Add shared getReleaseDocumentIntent() helper to centralize release-aware intent params/searchParams (optionally including a focus path).
  • Make the validation error icon in the release document table an IntentLink that passes path to focus the first field-level error.
  • Update Structure tool intent fast-path (getIntentState) to preserve path, with unit tests covering the behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/sanity/src/structure/getIntentState.ts Preserves path in the open-pane edit intent fast-path.
packages/sanity/src/structure/getIntentState.test.ts Adds unit tests verifying path is preserved through the fast-path.
packages/sanity/src/core/releases/tool/detail/documentTable/DocumentTableColumnDefs.tsx Turns validation icon into a deep-link IntentLink with focused error path + accessible label.
packages/sanity/src/core/releases/tool/components/ReleaseDocumentPreview.tsx Refactors preview link intent construction to use the shared helper.
packages/sanity/src/core/releases/tool/components/getReleaseDocumentIntent.ts New helper for consistent release-aware intent params/searchParams (and optional focus path).
packages/sanity/src/core/releases/tool/components/getReleaseDocumentIntent.test.ts Adds unit tests covering helper behavior across release states and optional path.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 76 to 80
case 'edit':
return {inspect, comment, task, scheduledDraft}
// `path` deep-links to (and focuses) a specific field when the document opens.
return {inspect, comment, task, scheduledDraft, path}
default:
return EMPTY_PARAMS

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — fixed in d7696e2. The edit fast-path now forwards rev/since/historyEvent/historyVersion/archivedRelease (plus path) to match the cold-path resolver, so opening a release/history version works the same whether or not the pane is already open.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 43.01% 29498 / 68584
🔵 Statements 35.8% 39684 / 110837
🔵 Functions 33.28% 6272 / 18846
🔵 Branches 26.82% 23864 / 88959
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/sanity/src/core/releases/tool/components/ReleaseDocumentPreview.tsx 59.57% 39.28% 28.57% 66.66% 48, 71, 76, 81, 83, 84, 89, 93, 94, 37-94
packages/sanity/src/core/releases/tool/components/getReleaseDocumentIntent.ts 100% 100% 100% 100%
packages/sanity/src/core/releases/tool/detail/documentTable/DocumentTableColumnDefs.tsx 81.25% 64.04% 76% 94.54% 39, 49, 58, 59, 76, 79, 179, 182, 189, 277, 39-49, 58, 104-277
packages/sanity/src/structure/getIntentState.ts 83.33% 58.82% 100% 87.5% 34, 93, 112
Generated in workflow #56994 for commit d7696e2 by the Vitest Coverage Report Action

…path

Address review feedback: getIntentState's open-pane fast-path forwarded only a
hardcoded subset of edit-intent params, dropping rev/since/historyEvent/
historyVersion/archivedRelease that the cold-path resolver (resolveIntent)
forwards. This could open a release/history version at the wrong view when the
target document pane was already open. Forward them too.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
comment?: string
task?: string
scheduledDraft?: string
path?: string

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There are two intent resolvers that must agree: this getIntentState fast-path (used when a pane is already open, to avoid resetting panes) and the resolveIntent cold path (resolveIntent.ts:144). The cold path forwards all params via rest-spread (otherParams = {...params} minus id/type); this fast-path forwards a hardcoded allowlist. That mismatch is the root cause of the bug — path existed in the params but the allowlist didn't list it, so the already-open-pane case dropped it.

Widening the allowlist from 4 to 11 entries fixes path today, but the next edit-intent param added anywhere will silently break the same way (version, for instance, is forwarded for create but not edit — whether that's intentional or not, the allowlist gives you no signal). Closing the gap structurally avoids the recurrence:

case 'edit': {
  const {id, type, template, ...rest} = params
  return rest
}

@jordanl17 jordanl17 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice - just one comment to maintain consistency between getIntentState with the existing resolveIntent

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.

3 participants