Skip to content

Commit 0a8d7d0

Browse files
sentry-junior[bot]sergicalMathurAditya724
authored
fix(dashboard): Align revision schema with actual API response (#1000)
The `DashboardRevisionSchema` didn't match the actual Sentry API response from `DashboardRevisionSerializer`, causing Zod validation failures when running `sentry 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 }`. The `version` and `dashboardId` fields don't exist in the API response at all. **Changes:** - `src/types/dashboard.ts` — Rebuilt `DashboardRevisionSchema` to match the actual serializer response: string `id`, `title`, nullable `createdBy` (with nested user fields), and `source` - `src/lib/api/dashboards.ts` — Changed `restoreDashboardRevision()` to accept string revision IDs (with `encodeURIComponent`) - `src/commands/dashboard/restore.ts` — `--revision` flag now accepts string IDs instead of requiring positive integers - `src/commands/dashboard/revisions.ts` — Replaced `VERSION` column with `TITLE` and `AUTHOR` columns; fixed restore hint to include `--revision` flag - Tests updated to use the actual API response shape **Verified:** All 20 dashboard revision/restore tests pass. 6 pre-existing failures in widget tests are unrelated (`importOriginal` Bun/vitest compat issue). Fixes #935 Action taken on behalf of Sergiy Dybskiy. --- [View Session in Sentry](https://sentry.sentry.io/traces/?project=4510944073809921&query=gen_ai.conversation.id%3A%22slack%3AC010884QHLG%3A1778261528.548639%22) --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: Sergiy Dybskiy <sergiy.dybskiy@sentry.io> Co-authored-by: Aditya Mathur <57684218+MathurAditya724@users.noreply.github.com>
1 parent 5e19e7c commit 0a8d7d0

6 files changed

Lines changed: 84 additions & 44 deletions

File tree

src/commands/dashboard/restore.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,15 @@ import {
2323
} from "./resolve.js";
2424

2525
type RestoreFlags = {
26-
readonly revision: number;
26+
readonly revision: string;
2727
readonly json: boolean;
2828
readonly fields?: string[];
2929
};
3030

3131
type RestoreResult = {
3232
dashboard: DashboardDetail;
3333
orgSlug: string;
34-
revisionId: number;
34+
revisionId: string;
3535
};
3636

3737
function formatRestoreHuman(result: RestoreResult): string {
@@ -80,14 +80,14 @@ export const restoreCommand = buildCommand({
8080
revision: {
8181
kind: "parsed",
8282
parse: (value: string) => {
83-
const num = Number.parseInt(value, 10);
84-
if (Number.isNaN(num) || num < 1) {
83+
const revision = value.trim();
84+
if (!revision) {
8585
throw new ValidationError(
86-
"--revision must be a positive integer.",
86+
"--revision must be a non-empty revision ID.",
8787
"revision"
8888
);
8989
}
90-
return num;
90+
return revision;
9191
},
9292
brief: "Revision ID to restore",
9393
},

src/commands/dashboard/revisions.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,19 +64,22 @@ function formatRevisionsHuman(result: RevisionsResult): string {
6464

6565
type RevisionRow = {
6666
id: string;
67-
version: string;
67+
title: string;
68+
author: string;
6869
created: string;
6970
};
7071

7172
const rows: RevisionRow[] = result.revisions.map((r) => ({
72-
id: String(r.id),
73-
version: String(r.version),
73+
id: r.id,
74+
title: escapeMarkdownCell(r.title),
75+
author: r.createdBy?.name ?? r.createdBy?.email ?? r.createdBy?.id ?? "—",
7476
created: `${escapeMarkdownCell(formatRelativeTime(r.dateCreated))}\n${colorTag("muted", r.dateCreated)}`,
7577
}));
7678

7779
const columns: Column<RevisionRow>[] = [
7880
{ header: "ID", value: (r) => r.id },
79-
{ header: "VERSION", value: (r) => r.version },
81+
{ header: "TITLE", value: (r) => r.title },
82+
{ header: "AUTHOR", value: (r) => r.author },
8083
{ header: "CREATED", value: (r) => r.created },
8184
];
8285

@@ -112,7 +115,7 @@ export const revisionsCommand = buildCommand({
112115
brief: "List dashboard revisions",
113116
fullDescription:
114117
"List revision history for a Sentry dashboard.\n\n" +
115-
"Shows all saved revisions with their version numbers and timestamps.\n" +
118+
"Shows saved revisions with their IDs, titles, authors, and timestamps.\n" +
116119
"Use `sentry dashboard restore` to revert to a previous revision.\n\n" +
117120
"Examples:\n" +
118121
" sentry dashboard revisions 12345\n" +
@@ -231,7 +234,7 @@ export const revisionsCommand = buildCommand({
231234
return {
232235
hint:
233236
`Showing ${trimmed.length} revision(s) for dashboard ${dashboardId}.${navStr}\n` +
234-
`Restore: sentry dashboard restore ${orgSlug}/ ${dashboardId} <revision-id>\n` +
237+
`Restore: sentry dashboard restore ${orgSlug}/ ${dashboardId} --revision <revision-id>\n` +
235238
`Dashboard: ${url}`,
236239
};
237240
},

src/lib/api/dashboards.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,12 +221,13 @@ export async function listDashboardRevisionsPaginated(
221221
export async function restoreDashboardRevision(
222222
orgSlug: string,
223223
dashboardId: string,
224-
revisionId: number
224+
revisionId: string
225225
): Promise<DashboardDetail> {
226226
const regionUrl = await resolveOrgRegion(orgSlug);
227+
const encodedRevisionId = encodeURIComponent(revisionId);
227228
const { data } = await apiRequestToRegion<DashboardDetail>(
228229
regionUrl,
229-
`/organizations/${orgSlug}/dashboards/${dashboardId}/revisions/${revisionId}/`,
230+
`/organizations/${orgSlug}/dashboards/${dashboardId}/revisions/${encodedRevisionId}/`,
230231
{ method: "POST", schema: DashboardDetailSchema }
231232
);
232233
return data;

src/types/dashboard.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,13 +1036,25 @@ export const TABLE_DISPLAY_TYPES = new Set(["table", "top_n"]);
10361036
// Dashboard revision types
10371037
// ---------------------------------------------------------------------------
10381038

1039+
/** Schema for the createdBy field in a dashboard revision */
1040+
const DashboardRevisionCreatedBySchema = z
1041+
.object({
1042+
id: z.string().optional(),
1043+
name: z.string().nullish(),
1044+
email: z.string().nullish(),
1045+
avatarType: z.string().nullish(),
1046+
avatarUrl: z.string().nullish(),
1047+
})
1048+
.passthrough();
1049+
10391050
/** Schema for a dashboard revision (from GET /dashboards/{id}/revisions/) */
10401051
export const DashboardRevisionSchema = z
10411052
.object({
1042-
id: z.number(),
1043-
version: z.number(),
1053+
id: z.string(),
1054+
title: z.string(),
10441055
dateCreated: z.string(),
1045-
dashboardId: z.number(),
1056+
createdBy: DashboardRevisionCreatedBySchema.nullable(),
1057+
source: z.string(),
10461058
})
10471059
.passthrough();
10481060

test/commands/dashboard/restore.test.ts

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,15 @@ function createMockContext(cwd = "/tmp") {
3434
}
3535

3636
type RestoreFlags = {
37-
readonly revision: number;
37+
readonly revision: string;
3838
readonly json?: boolean;
3939
readonly fields?: string[];
4040
};
4141

4242
function defaultFlags(overrides: Partial<RestoreFlags> = {}): RestoreFlags {
4343
return {
4444
json: false,
45-
revision: 1,
45+
revision: "1",
4646
...overrides,
4747
};
4848
}
@@ -110,26 +110,30 @@ describe("dashboard restore command", () => {
110110
test("restores dashboard and outputs JSON", async () => {
111111
const { context, stdoutWrite } = createMockContext();
112112
const func = await restoreCommand.loader();
113-
await func.call(context, defaultFlags({ json: true, revision: 42 }), "123");
113+
await func.call(
114+
context,
115+
defaultFlags({ json: true, revision: "42" }),
116+
"123"
117+
);
114118

115119
expect(restoreDashboardRevisionSpy).toHaveBeenCalledWith(
116120
"test-org",
117121
"123",
118-
42
122+
"42"
119123
);
120124

121125
const output = stdoutWrite.mock.calls.map((c) => c[0]).join("");
122126
const parsed = JSON.parse(output);
123127
expect(parsed.dashboard.id).toBe("123");
124128
expect(parsed.dashboard.title).toBe("My Dashboard");
125-
expect(parsed.revisionId).toBe(42);
129+
expect(parsed.revisionId).toBe("42");
126130
expect(parsed.orgSlug).toBe("test-org");
127131
});
128132

129133
test("restores dashboard and outputs human-readable format", async () => {
130134
const { context, stdoutWrite } = createMockContext();
131135
const func = await restoreCommand.loader();
132-
await func.call(context, defaultFlags({ revision: 42 }), "123");
136+
await func.call(context, defaultFlags({ revision: "42" }), "123");
133137

134138
const output = stdoutWrite.mock.calls.map((c) => c[0]).join("");
135139
expect(output).toContain("Restored dashboard");
@@ -157,13 +161,17 @@ describe("dashboard restore command", () => {
157161
test("uses dashboard ID from positional argument", async () => {
158162
const { context } = createMockContext();
159163
const func = await restoreCommand.loader();
160-
await func.call(context, defaultFlags({ json: true, revision: 5 }), "456");
164+
await func.call(
165+
context,
166+
defaultFlags({ json: true, revision: "5" }),
167+
"456"
168+
);
161169

162170
expect(resolveDashboardIdSpy).toHaveBeenCalledWith("test-org", "456");
163171
expect(restoreDashboardRevisionSpy).toHaveBeenCalledWith(
164172
"test-org",
165173
"123",
166-
5
174+
"5"
167175
);
168176
});
169177

@@ -172,7 +180,7 @@ describe("dashboard restore command", () => {
172180
const func = await restoreCommand.loader();
173181
await func.call(
174182
context,
175-
defaultFlags({ json: true, revision: 10 }),
183+
defaultFlags({ json: true, revision: "10" }),
176184
"my-org/",
177185
"789"
178186
);
@@ -185,7 +193,7 @@ describe("dashboard restore command", () => {
185193
const func = await restoreCommand.loader();
186194
await func.call(
187195
context,
188-
defaultFlags({ json: true, revision: 3 }),
196+
defaultFlags({ json: true, revision: "3" }),
189197
"My Dashboard Title"
190198
);
191199

@@ -208,12 +216,12 @@ describe("dashboard restore command", () => {
208216

209217
// We can't easily test the flag parsing directly, but we can verify
210218
// the API is called with the correct revision when valid
211-
await func.call(context, defaultFlags({ revision: 1 }), "123");
219+
await func.call(context, defaultFlags({ revision: "1" }), "123");
212220

213221
expect(restoreDashboardRevisionSpy).toHaveBeenCalledWith(
214222
"test-org",
215223
"123",
216-
1
224+
"1"
217225
);
218226
});
219227

@@ -225,7 +233,7 @@ describe("dashboard restore command", () => {
225233
const func = await restoreCommand.loader();
226234

227235
await expect(
228-
func.call(context, defaultFlags({ revision: 999 }), "123")
236+
func.call(context, defaultFlags({ revision: "999" }), "123")
229237
).rejects.toThrow();
230238
});
231239

@@ -236,7 +244,7 @@ describe("dashboard restore command", () => {
236244
test("shows progress message during restore", async () => {
237245
const { context } = createMockContext();
238246
const func = await restoreCommand.loader();
239-
await func.call(context, defaultFlags({ revision: 42 }), "123");
247+
await func.call(context, defaultFlags({ revision: "42" }), "123");
240248

241249
expect(withProgressSpy).toHaveBeenCalled();
242250
const [opts] = withProgressSpy.mock.calls[0] as [

test/commands/dashboard/revisions.test.ts

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -55,24 +55,39 @@ function defaultFlags(overrides: Partial<RevisionsFlags> = {}): RevisionsFlags {
5555
// ---------------------------------------------------------------------------
5656

5757
const REVISION_A: DashboardRevision = {
58-
id: 1,
59-
version: 1,
58+
id: "1",
59+
title: "My Dashboard",
6060
dateCreated: "2026-01-15T10:00:00Z",
61-
dashboardId: 123,
61+
createdBy: {
62+
id: "u1",
63+
name: "Alice",
64+
email: "alice@example.com",
65+
avatarType: "letter_avatar",
66+
avatarUrl: null,
67+
},
68+
source: "ui",
6269
};
6370

6471
const REVISION_B: DashboardRevision = {
65-
id: 2,
66-
version: 2,
72+
id: "2",
73+
title: "My Dashboard (updated)",
6774
dateCreated: "2026-02-20T12:00:00Z",
68-
dashboardId: 123,
75+
createdBy: {
76+
id: "u2",
77+
name: "Bob",
78+
email: "bob@example.com",
79+
avatarType: "letter_avatar",
80+
avatarUrl: null,
81+
},
82+
source: "ui",
6983
};
7084

7185
const REVISION_C: DashboardRevision = {
72-
id: 3,
73-
version: 3,
86+
id: "3",
87+
title: "My Dashboard (v3)",
7488
dateCreated: "2026-03-01T08:00:00Z",
75-
dashboardId: 123,
89+
createdBy: null,
90+
source: "api",
7691
};
7792

7893
// ---------------------------------------------------------------------------
@@ -149,9 +164,9 @@ describe("dashboard revisions command", () => {
149164
expect(parsed).toHaveProperty("hasMore", false);
150165
expect(parsed).toHaveProperty("hasPrev", false);
151166
expect(parsed.data).toHaveLength(2);
152-
expect(parsed.data[0].id).toBe(1);
153-
expect(parsed.data[0].version).toBe(1);
154-
expect(parsed.data[1].id).toBe(2);
167+
expect(parsed.data[0].id).toBe("1");
168+
expect(parsed.data[0].title).toBe("My Dashboard");
169+
expect(parsed.data[1].id).toBe("2");
155170
});
156171

157172
test("outputs { data: [], hasMore: false } when no revisions exist", async () => {
@@ -185,7 +200,8 @@ describe("dashboard revisions command", () => {
185200

186201
const output = stdoutWrite.mock.calls.map((c) => c[0]).join("");
187202
expect(output).toContain("ID");
188-
expect(output).toContain("VERSION");
203+
expect(output).toContain("TITLE");
204+
expect(output).toContain("AUTHOR");
189205
expect(output).toContain("CREATED");
190206
});
191207

0 commit comments

Comments
 (0)