Skip to content

Commit 54ea1d4

Browse files
betegonclaude
andcommitted
fix(dashboard): persist widgets on PUT and invalidate response cache
Two bugs prevented dashboard widget commands from working correctly: 1. stripWidgetServerFields used a denylist that missed dozens of extra fields returned by the GET API via Zod .passthrough() (description, thresholds, interval, axisRange, datasetSource, etc.). Switch to an allowlist that only includes fields the PUT API accepts. 2. updateDashboard PUT succeeded but subsequent dashboard view returned stale cached GET data. Add invalidateCachedResponse() to response-cache and call it after every dashboard PUT with the correct URL format. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 3b9fa51 commit 54ea1d4

File tree

3 files changed

+71
-48
lines changed

3 files changed

+71
-48
lines changed

src/lib/api/dashboards.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import type {
1111
} from "../../types/dashboard.js";
1212

1313
import { resolveOrgRegion } from "../region.js";
14+
import { invalidateCachedResponse } from "../response-cache.js";
1415

1516
import { apiRequestToRegion } from "./infrastructure.js";
1617

@@ -88,10 +89,18 @@ export async function updateDashboard(
8889
body: { title: string; widgets: DashboardWidget[]; projects?: number[] }
8990
): Promise<DashboardDetail> {
9091
const regionUrl = await resolveOrgRegion(orgSlug);
91-
const { data } = await apiRequestToRegion<DashboardDetail>(
92-
regionUrl,
93-
`/organizations/${orgSlug}/dashboards/${dashboardId}/`,
94-
{ method: "PUT", body }
95-
);
92+
const path = `/organizations/${orgSlug}/dashboards/${dashboardId}/`;
93+
const { data } = await apiRequestToRegion<DashboardDetail>(regionUrl, path, {
94+
method: "PUT",
95+
body,
96+
});
97+
98+
// Invalidate cached GET for this dashboard so subsequent view commands
99+
// return fresh data instead of the pre-mutation cached response.
100+
const normalizedBase = regionUrl.endsWith("/")
101+
? regionUrl.slice(0, -1)
102+
: regionUrl;
103+
await invalidateCachedResponse(`${normalizedBase}/api/0${path}`);
104+
96105
return data;
97106
}

src/lib/response-cache.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,23 @@ async function writeResponseToCache(
555555
return serialized.length;
556556
}
557557

558+
/**
559+
* Invalidate the cached GET response for a specific URL.
560+
*
561+
* Used by mutation commands (PUT/POST/DELETE) that change server state,
562+
* so subsequent GET commands don't serve stale data.
563+
*
564+
* @param url - Full URL of the GET endpoint to invalidate
565+
*/
566+
export async function invalidateCachedResponse(url: string): Promise<void> {
567+
try {
568+
const key = buildCacheKey("GET", url);
569+
await rm(cacheFilePath(key), { force: true });
570+
} catch {
571+
// Best-effort — ignore if file doesn't exist or can't be deleted
572+
}
573+
}
574+
558575
/**
559576
* Remove all cached responses.
560577
* Called on `auth logout` and `auth login` since cached data is tied to the user.

src/types/dashboard.ts

Lines changed: 40 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -507,60 +507,57 @@ export function assignDefaultLayout(
507507

508508
// ---------------------------------------------------------------------------
509509
// Server field stripping utilities
510+
//
511+
// The Sentry dashboard API returns many extra fields via GET that should NOT
512+
// be sent back in PUT requests. Using an allowlist approach ensures only
513+
// fields the API accepts are included, avoiding silent rejection of widgets.
510514
// ---------------------------------------------------------------------------
511515

512-
/**
513-
* Server-generated fields on widget queries that must be stripped before PUT.
514-
* NEVER strip user-controlled fields like conditions, columns, aggregates.
515-
*/
516-
const QUERY_SERVER_FIELDS = ["id", "widgetId", "dateCreated"] as const;
517-
518-
/**
519-
* Server-generated fields on widgets that must be stripped before PUT.
520-
* CRITICAL: Never strip widgetType, displayType, or layout — these are
521-
* user-controlled and stripping them causes widgets to reset to defaults.
522-
*/
523-
const WIDGET_SERVER_FIELDS = ["id", "dashboardId", "dateCreated"] as const;
524-
525-
/**
526-
* Server-generated fields on widget layout that must be stripped before PUT.
527-
*/
528-
const LAYOUT_SERVER_FIELDS = ["isResizable"] as const;
516+
/** Extract only the query fields the PUT API accepts */
517+
function cleanQuery(q: DashboardWidgetQuery): DashboardWidgetQuery {
518+
return {
519+
name: q.name ?? "",
520+
conditions: q.conditions ?? "",
521+
columns: q.columns ?? [],
522+
aggregates: q.aggregates ?? [],
523+
fields: q.fields ?? [],
524+
...(q.fieldAliases && { fieldAliases: q.fieldAliases }),
525+
...(q.orderby && { orderby: q.orderby }),
526+
};
527+
}
529528

530529
/**
531-
* Strip server-generated fields from a single widget for PUT requests.
530+
* Strip server-generated and passthrough fields from a widget for PUT requests.
531+
*
532+
* Uses an allowlist approach: only includes fields the dashboard PUT API
533+
* accepts. The GET response includes many extra fields (description, thresholds,
534+
* interval, axisRange, datasetSource, etc.) that cause silent failures if
535+
* sent back in PUT.
532536
*
533537
* @param widget - Widget object from GET response
534-
* @returns Widget safe for PUT (widgetType, displayType, layout preserved)
538+
* @returns Widget safe for PUT (only API-accepted fields)
535539
*/
536540
export function stripWidgetServerFields(
537541
widget: DashboardWidget
538542
): DashboardWidget {
539-
const cleaned = { ...widget };
540-
541-
// Strip widget-level server fields
542-
for (const field of WIDGET_SERVER_FIELDS) {
543-
delete (cleaned as Record<string, unknown>)[field];
544-
}
545-
546-
// Strip query-level server fields
547-
if (cleaned.queries) {
548-
cleaned.queries = cleaned.queries.map((q) => {
549-
const cleanedQuery = { ...q };
550-
for (const field of QUERY_SERVER_FIELDS) {
551-
delete (cleanedQuery as Record<string, unknown>)[field];
552-
}
553-
return cleanedQuery;
554-
});
555-
}
543+
const cleaned: DashboardWidget = {
544+
title: widget.title,
545+
displayType: widget.displayType,
546+
...(widget.widgetType && { widgetType: widget.widgetType }),
547+
...(widget.queries && { queries: widget.queries.map(cleanQuery) }),
548+
...(widget.limit !== undefined &&
549+
widget.limit !== null && { limit: widget.limit }),
550+
};
556551

557-
// Strip layout server fields
558-
if (cleaned.layout) {
559-
const cleanedLayout = { ...cleaned.layout };
560-
for (const field of LAYOUT_SERVER_FIELDS) {
561-
delete (cleanedLayout as Record<string, unknown>)[field];
562-
}
563-
cleaned.layout = cleanedLayout;
552+
// Preserve layout (x, y, w, h, minH only — not isResizable)
553+
if (widget.layout) {
554+
cleaned.layout = {
555+
x: widget.layout.x,
556+
y: widget.layout.y,
557+
w: widget.layout.w,
558+
h: widget.layout.h,
559+
...(widget.layout.minH !== undefined && { minH: widget.layout.minH }),
560+
};
564561
}
565562

566563
return cleaned;

0 commit comments

Comments
 (0)