Skip to content

Commit ea3df07

Browse files
icecrasher321claude
andcommitted
fix(usage): apply the deep-link guard that was written but never wired
The previous round added `isUsableCustomRange` and left `isResolvedCustom` calling `isCalendarDate` — the edit replacing the guard did not apply, so the function shipped unused and the fix it described never took effect. It is wired now, and a new `organization-usage.test.ts` covers the contract's side of these rules so a dropped edit here fails a test rather than a review. That contract check also had a hole of its own: `if (!datePart) return true` treated a value with no `YYYY-MM-DD` prefix as nothing to verify, so anything `Date.parse` accepted passed. `2026-08` was read as August 1 — a window the caller never asked for, returned as though it had. The prefix is required now, anchored so a trailing suffix cannot slip past, and the client mirrors it. Both charts drop `useRef(generateShortId(7))` for `useId`. The reported hydration mismatch is not real — both return early while `containerWidth === null`, which holds on the server and on the first client render, so the gradient never exists in hydrated markup. The waste is real: a ref initializer runs every render and all but the first result is discarded, which is the repo's own lazy-init rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 9e9719a commit ea3df07

5 files changed

Lines changed: 84 additions & 19 deletions

File tree

apps/sim/components/charts/bar-chart.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
'use client'
22

3-
import { memo, useMemo, useRef, useState } from 'react'
3+
import { memo, useId, useMemo, useState } from 'react'
44
import { cn } from '@sim/emcn'
5-
import { generateShortId } from '@sim/utils/id'
65
import {
76
formatChartCompactNumber,
87
formatChartLatency,
@@ -64,7 +63,12 @@ function BarChartComponent({
6463
height = CHART_DEFAULT_HEIGHT,
6564
highlightIndex,
6665
}: BarChartProps) {
67-
const uniqueId = useRef(`bar-${generateShortId(7)}`).current
66+
/*
67+
`useId`, not `useRef(generateShortId())`: a ref initializer is evaluated on
68+
every render and all but the first result thrown away, and React already has
69+
a hook whose whole job is a stable unique id.
70+
*/
71+
const uniqueId = useId().replace(/:/g, '')
6872
const [containerRef, containerWidth] = useChartWidth()
6973
const width = containerWidth ?? 0
7074
const padding = CHART_PADDING

apps/sim/components/charts/line-chart.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
'use client'
22

3-
import { memo, useMemo, useRef, useState } from 'react'
3+
import { memo, useId, useMemo, useState } from 'react'
44
import { Button, cn } from '@sim/emcn'
5-
import { generateShortId } from '@sim/utils/id'
65
import {
76
formatChartCompactNumber,
87
formatChartLatency,
@@ -62,7 +61,12 @@ function LineChartComponent({
6261
series,
6362
height = CHART_DEFAULT_HEIGHT,
6463
}: LineChartProps) {
65-
const uniqueId = useRef(`chart-${generateShortId(7)}`).current
64+
/*
65+
`useId`, not `useRef(generateShortId())`: a ref initializer is evaluated on
66+
every render and all but the first result thrown away, and React already has
67+
a hook whose whole job is a stable unique id.
68+
*/
69+
const uniqueId = useId().replace(/:/g, '')
6670
const [containerRef, containerWidth] = useChartWidth()
6771
const width = containerWidth ?? 0
6872
const padding = CHART_PADDING

apps/sim/ee/organization-usage/hooks/use-usage-window.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ const DAY_MS = 24 * 60 * 60 * 1000
2020
/** A `YYYY-MM-DD` that survives a calendar round-trip, matching the contract's rule. */
2121
function isCalendarDate(value: string | null): value is string {
2222
if (!value) return false
23-
const datePart = /^(\d{4}-\d{2}-\d{2})/.exec(value)?.[1]
23+
const datePart = /^(\d{4}-\d{2}-\d{2})(?:$|T)/.exec(value)?.[1]
2424
if (!datePart) return false
2525
return new Date(`${datePart}T00:00:00.000Z`).toISOString().slice(0, 10) === datePart
2626
}
@@ -53,16 +53,15 @@ export function useUsageWindow() {
5353
const timezone = getBrowserTimezone()
5454

5555
/**
56-
* Both bounds present *and* real calendar dates.
56+
* Both bounds present, and a range the API will actually accept.
5757
*
58-
* The contract rejects a date that does not exist (`2026-02-30` parses and rolls
59-
* forward, so it has to be refused rather than silently shifted). Without the same
60-
* check here, a deep link carrying one satisfied this guard and every query on the
61-
* page answered 400 — the partial-link fallback exists precisely so a bad link
62-
* degrades to the default window instead.
58+
* Every condition the window resolver refuses with a 400 — an unreal date, an
59+
* inverted pair, a span past the cap — has to be checked here too, or a bookmarked
60+
* link carrying one is marked resolved and fails all four queries on the page. The
61+
* fallback exists precisely so a bad link degrades to the default window instead.
6362
*/
6463
const isResolvedCustom =
65-
state.preset === 'custom' && isCalendarDate(state.startDate) && isCalendarDate(state.endDate)
64+
state.preset === 'custom' && isUsableCustomRange(state.startDate, state.endDate)
6665
const preset: UsageWindowPreset =
6766
state.preset === 'custom' && !isResolvedCustom ? DEFAULT_USAGE_PRESET : state.preset
6867

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { organizationUsageEventsQuerySchema } from '@/lib/api/contracts/organization-usage'
6+
7+
/** The shared window fields every usage contract extends, exercised through one of them. */
8+
function parseWindow(input: Record<string, unknown>) {
9+
return organizationUsageEventsQuerySchema.safeParse({ preset: 'custom', ...input })
10+
}
11+
12+
describe('organization usage window contract', () => {
13+
it('accepts a real calendar date', () => {
14+
expect(parseWindow({ startDate: '2026-08-01', endDate: '2026-08-31' }).success).toBe(true)
15+
})
16+
17+
it('refuses a date that does not exist', () => {
18+
// `Date.parse` accepts this and rolls it forward to March 2, so a request for
19+
// February would otherwise be answered about March without saying so.
20+
expect(parseWindow({ startDate: '2026-02-30' }).success).toBe(false)
21+
})
22+
23+
it('refuses a nonexistent day inside a datetime, not just a bare date', () => {
24+
expect(parseWindow({ startDate: '2026-02-30T00:00:00' }).success).toBe(false)
25+
})
26+
27+
it('refuses a parseable non-date such as a bare month', () => {
28+
// `new Date('2026-08')` is August 1. Accepting it returned a window the caller
29+
// never asked for, with nothing to indicate the value had been reinterpreted.
30+
expect(parseWindow({ startDate: '2026-08' }).success).toBe(false)
31+
})
32+
33+
it('treats an empty limit as omitted rather than as zero', () => {
34+
// `z.coerce.number()` turns `''` into `0`, which then fails `.min(1)` — so a
35+
// client serializing an unset filter got a 400 instead of the declared default.
36+
const parsed = parseWindow({ limit: '' })
37+
expect(parsed.success).toBe(true)
38+
if (parsed.success) expect(parsed.data.limit).toBe(50)
39+
})
40+
41+
it('normalizes a single source to a one-item array', () => {
42+
// One selected filter arrives as a scalar, which a bare `z.array` rejected.
43+
const parsed = parseWindow({ source: 'workflow' })
44+
expect(parsed.success).toBe(true)
45+
if (parsed.success) expect(parsed.data.source).toEqual(['workflow'])
46+
})
47+
48+
it('refuses an unknown source instead of matching nothing', () => {
49+
expect(parseWindow({ source: 'not-a-source' }).success).toBe(false)
50+
})
51+
52+
it('refuses a timezone the runtime does not recognize', () => {
53+
expect(parseWindow({ timezone: 'Mars/Olympus_Mons' }).success).toBe(false)
54+
})
55+
})

apps/sim/lib/api/contracts/organization-usage.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,14 @@ const isoDateSchema = z
5454
.refine(
5555
(value) => {
5656
if (!value) return true
57-
if (Number.isNaN(Date.parse(value))) return false
58-
// The leading `YYYY-MM-DD` of either form, so `2026-02-30T00:00:00` is rejected
59-
// rather than only the bare `2026-02-30`.
60-
const datePart = /^(\d{4}-\d{2}-\d{2})/.exec(value)?.[1]
61-
if (!datePart) return true
57+
// The `YYYY-MM-DD` prefix is required, not merely checked when present. Letting
58+
// anything else through on the grounds that `Date.parse` accepted it meant
59+
// `2026-08` was read as August 1 — a window the caller never asked for, returned
60+
// as though it had.
61+
const datePart = /^(\d{4}-\d{2}-\d{2})(?:$|T)/.exec(value)?.[1]
62+
if (!datePart) return false
63+
// Rolls a nonexistent day forward (`2026-02-30` becomes March 2), so the only
64+
// way to reject one is to check that it survives the round trip.
6265
return new Date(`${datePart}T00:00:00.000Z`).toISOString().slice(0, 10) === datePart
6366
},
6467
{ message: 'Expected a real calendar date such as 2026-08-01' }

0 commit comments

Comments
 (0)