fix(dashboard): Align revision schema with actual API response#1000
Conversation
The DashboardRevisionSchema expected numeric id, version, and dashboardId fields, but the Sentry API returns string id, title, createdBy, and source. This caused Zod validation failures when running `sentry dashboard revisions`. - Change id from z.number() to z.string() - Remove non-existent version and dashboardId fields - Add title, createdBy (nullable), and source fields - Update restore command to accept string revision IDs - Update revisions formatter to show title and author columns Fixes #935 Co-authored-by: Sergiy Dybskiy <sergiy.dybskiy@sentry.io>
|
MathurAditya724
left a comment
MathurAditya724
left a comment
There was a problem hiding this comment.
verified the new DashboardRevisionSchema against the actual DashboardRevisionSerializer in getsentry/sentry — fields match exactly (id: str, title, dateCreated, createdBy: {id, name, email, avatarType, avatarUrl} | null, source).
all 20 dashboard revision/restore tests pass. the 3 typecheck errors are pre-existing (missing generated files), unrelated to this change.
the author fallback chain (name → email → id → "—") and encodeURIComponent on the revision ID in the restore URL are both good defensive choices. .passthrough() on the Zod schemas gives forward-compat if the API adds fields later.
lgtm — ready to take out of draft once CI is green.
Codecov Results 📊❌ Patch coverage is 42.86%. Project has 4289 uncovered lines. Files with missing lines (2)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 81.64% 81.63% -0.01%
==========================================
Files 328 328 —
Lines 23341 23344 +3
Branches 15109 15111 +2
==========================================
+ Hits 19056 19055 -1
- Misses 4285 4289 +4
- Partials 1613 1615 +2Generated by Codecov Action |
|
fix-ci: attempt 1 — biome formatter wants different line wrapping in revisions.ts and restore.test.ts, running lint:fix |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| version: String(r.version), | ||
| id: r.id, | ||
| title: escapeMarkdownCell(r.title), | ||
| author: r.createdBy?.name ?? r.createdBy?.email ?? r.createdBy?.id ?? "—", |
There was a problem hiding this comment.
Bug: The author field is not escaped with escapeMarkdownCell, which can break markdown table rendering if a user's name or email contains special characters like |.
Severity: LOW
Suggested Fix
Wrap the author value assignment with the escapeMarkdownCell() helper function to ensure any special characters are properly escaped before being rendered in the table.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/commands/dashboard/revisions.ts#L75
Potential issue: The `author` field, which is populated using `r.createdBy?.name`,
`r.createdBy?.email`, or `r.createdBy?.id`, is not being escaped before it is rendered
in a markdown table. If any of these values contain special markdown characters like a
pipe (`|`), it will be interpreted as a table column separator. This will break the
table's structure and corrupt the command's output for dashboards where an author's
details contain such characters. Other parts of the codebase, like
`src/commands/team/list.ts`, correctly use the `escapeMarkdownCell` helper for similar
user-provided data.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
good catch — wrapped the author fallback chain with escapeMarkdownCell() in 56d4f35.
## Summary - wraps the `author` column in `dashboard revisions` with `escapeMarkdownCell()` to prevent markdown table breakage when a user's name or email contains `|` - missed in #1000 — caught by sentry[bot]'s automated review One-line change, no new dependencies.
1 participant
The
DashboardRevisionSchemadidn't match the actual Sentry API response fromDashboardRevisionSerializer, causing Zod validation failures when runningsentry dashboard revisions.Root cause: The schema expected
{ id: number, version: number, dashboardId: number }but the API returns{ id: string, title: string, createdBy: object | null, source: string }. TheversionanddashboardIdfields don't exist in the API response at all.Changes:
src/types/dashboard.ts— RebuiltDashboardRevisionSchemato match the actual serializer response: stringid,title, nullablecreatedBy(with nested user fields), andsourcesrc/lib/api/dashboards.ts— ChangedrestoreDashboardRevision()to accept string revision IDs (withencodeURIComponent)src/commands/dashboard/restore.ts—--revisionflag now accepts string IDs instead of requiring positive integerssrc/commands/dashboard/revisions.ts— ReplacedVERSIONcolumn withTITLEandAUTHORcolumns; fixed restore hint to include--revisionflagVerified: All 20 dashboard revision/restore tests pass. 6 pre-existing failures in widget tests are unrelated (
importOriginalBun/vitest compat issue).Fixes #935
Action taken on behalf of Sergiy Dybskiy.
View Session in Sentry