diff --git a/apps/sim/app/api/audit-logs/export/route.test.ts b/apps/sim/app/api/audit-logs/export/route.test.ts index 6f177ab7b9f..7c0667a9de7 100644 --- a/apps/sim/app/api/audit-logs/export/route.test.ts +++ b/apps/sim/app/api/audit-logs/export/route.test.ts @@ -164,4 +164,29 @@ describe('GET /api/audit-logs/export', () => { expect(response.status).toBe(400) expect(mockQueryAuditLogs).not.toHaveBeenCalled() }) + + /** + * The export has to filter by everything the on-screen feed does. It did not + * forward `workspaceId`, so an admin exporting from a workspace-scoped feed + * downloaded the whole organization — silently, because every field of + * `AuditLogFilterParams` is optional and dropping one still type-checks. + */ + it('forwards the workspace filter the on-screen feed applies', async () => { + mockGetOrgWorkspaceIds.mockResolvedValue(['workspace-1']) + + await GET(makeRequest('?workspaceId=workspace-1')) + + expect(mockBuildFilterConditions).toHaveBeenCalledWith( + expect.objectContaining({ workspaceId: 'workspace-1' }) + ) + }) + + it('rejects a workspaceId outside the organization, as the list route does', async () => { + mockGetOrgWorkspaceIds.mockResolvedValue(['workspace-1']) + + const response = await GET(makeRequest('?workspaceId=workspace-elsewhere')) + + expect(response.status).toBe(400) + expect(mockQueryAuditLogs).not.toHaveBeenCalled() + }) }) diff --git a/apps/sim/app/api/audit-logs/export/route.ts b/apps/sim/app/api/audit-logs/export/route.ts index c4860a85092..bcf02a37d72 100644 --- a/apps/sim/app/api/audit-logs/export/route.ts +++ b/apps/sim/app/api/audit-logs/export/route.ts @@ -66,8 +66,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => { } const { organizationId, orgMemberIds } = authResult.context - const { search, action, resourceType, actorId, startDate, endDate, includeDeparted } = - parsed.data.query + const { actorId, workspaceId, includeDeparted } = parsed.data.query if (actorId && !orgMemberIds.includes(actorId)) { return NextResponse.json( @@ -77,20 +76,34 @@ export const GET = withRouteHandler(async (request: NextRequest) => { } const orgWorkspaceIds = await getOrgWorkspaceIds(organizationId) + /** + * The same refusal `listAuditLogs` gives. The scope predicate already makes an + * out-of-organization id return nothing, but an empty CSV and a 400 that names the + * problem are very different answers to the same bad request, and the two paths + * disagreeing about which one you get is what an audit trail cannot afford. + */ + if (workspaceId && !orgWorkspaceIds.includes(workspaceId)) { + return NextResponse.json( + { error: 'workspaceId does not belong to your organization' }, + { status: 400 } + ) + } const scopeCondition = buildOrgScopeCondition({ organizationId, orgWorkspaceIds, orgMemberIds, includeDeparted, }) - const filterConditions = buildFilterConditions({ - action, - resourceType, - actorId, - search, - startDate, - endDate, - }) + /** + * The whole parsed query, not a hand-listed subset. + * + * Every field of `AuditLogFilterParams` is optional, so dropping one type-checks + * silently — which is how `workspaceId` came to be accepted by the contract, + * honoured by the list route, and ignored here: an admin looking at one + * workspace's feed downloaded the entire organization's, under a truncation + * warning that blamed the date range. + */ + const filterConditions = buildFilterConditions(parsed.data.query) const conditions = [scopeCondition, ...filterConditions] const rows: ReturnType[] = [] diff --git a/apps/sim/app/api/audit-logs/route.ts b/apps/sim/app/api/audit-logs/route.ts index f2cc96a2b3f..f63e99575c2 100644 --- a/apps/sim/app/api/audit-logs/route.ts +++ b/apps/sim/app/api/audit-logs/route.ts @@ -27,6 +27,7 @@ export const GET = defineInternalJsonRoute({ action: query.action, resourceType: query.resourceType, actorId: query.actorId, + workspaceId: query.workspaceId, startDate: query.startDate, endDate: query.endDate, }, diff --git a/apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx b/apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx index b565424b422..8295fa0b230 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx @@ -176,7 +176,11 @@ export function SettingsPage({ section }: SettingsPageProps) { )} {effectiveSection === 'usage' && organizationId && ( - + )} {effectiveSection === 'apikeys' && } {isBillingEnabled && effectiveSection === 'billing' && ( diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/billing/components/usage-limit-field/usage-limit-field.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/billing/components/usage-limit-field/usage-limit-field.tsx index 48b198dc5b0..8d7d9dc5072 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/billing/components/usage-limit-field/usage-limit-field.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/billing/components/usage-limit-field/usage-limit-field.tsx @@ -121,10 +121,14 @@ export function UsageLimitField({ return ( + {/* + Text with a numeric input mode rather than `type='number'`: the native stepper + is all that type buys and it does not fit the chip chrome. The minimum is + enforced on commit below, where it can explain itself, rather than by a `min` + attribute the browser enforces silently. + */} setDraft(e.target.value)} placeholder={ @@ -135,7 +139,6 @@ export function UsageLimitField({ : String(dollarsToCredits(currentLimit)) } disabled={!canEdit} - inputClassName='[appearance:textfield] [&::-webkit-inner-spin-button]:appearance-none [&::-webkit-outer-spin-button]:appearance-none' /> ) diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/manage-credits-modal/manage-credits-modal.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/manage-credits-modal/manage-credits-modal.tsx index bbe033f0575..0654a34d997 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/manage-credits-modal/manage-credits-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/manage-credits-modal/manage-credits-modal.tsx @@ -109,9 +109,17 @@ export function ManageCreditsModal({ value={isLoading ? 'Loading…' : creditsUsed} copyLabel='Copy credits used' /> + {/* + Text with a numeric input mode, not `inputType='number'` — the same choice + the retry settings field documents. The native stepper is all the number + type buys, and it paints browser chrome inside a flat chip surface. It also + reports `''` for anything the browser considers invalid, so a typo arrived + here indistinguishable from a cleared field and saved as "no limit"; as text + it reaches the `Number.isInteger` check below and is refused. + */} Credit limit diff --git a/apps/sim/app/workspace/[workspaceId]/settings/usage/events/loading.tsx b/apps/sim/app/workspace/[workspaceId]/settings/usage/events/loading.tsx index aa745a1024e..1e9fd7ecb72 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/usage/events/loading.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/usage/events/loading.tsx @@ -16,7 +16,7 @@ export default function UsageEventsLoading() { onSelect: () => router.push(`/workspace/${workspaceId}/settings/usage`), }} title='Usage events' - description='Every credit-consuming event behind your usage.' + description="Every credit-consuming event across your organization's workspaces." /> ) } diff --git a/apps/sim/components/charts/bar-chart.tsx b/apps/sim/components/charts/bar-chart.tsx index 8a40747c18b..17acfbe4cb9 100644 --- a/apps/sim/components/charts/bar-chart.tsx +++ b/apps/sim/components/charts/bar-chart.tsx @@ -8,19 +8,21 @@ import { formatChartTimestamp, } from '@/components/charts/chart-format' import { + CHART_AXIS_LABEL_GAP, CHART_DEFAULT_HEIGHT, CHART_GRID_FRACTIONS, - CHART_PADDING, CHART_TICK_FILL, CHART_TICK_FONT_SIZE, chartPlotBand, formatTimeTick, + resolveChartPadding, resolveSpanMs, resolveTimeTickIndices, } from '@/components/charts/chart-geometry' import { ChartTooltip, ChartTooltipRow, + estimateTooltipHeight, estimateTooltipWidth, positionChartTooltip, } from '@/components/charts/chart-tooltip' @@ -47,6 +49,17 @@ interface BarChartProps { highlightIndex?: number } +/** Tick and tooltip text for a bucket's value, in the caller's unit. */ +function formatBarValue(value: number | undefined, unit: string | undefined): string { + if (typeof value !== 'number' || !Number.isFinite(value)) return '—' + const suffix = (unit ?? '').toLowerCase() + if (suffix.includes('%')) return `${value.toFixed(1)}%` + if (suffix === 'latency') return formatChartLatency(value) + if (suffix.includes('ms')) return `${Math.round(value)}ms` + if (suffix === 'credits') return formatChartCompactNumber(value) + return `${Math.round(value)}${unit ?? ''}` +} + /** * Discrete time buckets as bars. * @@ -71,16 +84,11 @@ function BarChartComponent({ const uniqueId = useId().replace(/:/g, '') const [containerRef, containerWidth] = useChartWidth() const width = containerWidth ?? 0 - const padding = CHART_PADDING - const chartWidth = width - padding.left - padding.right - const chartHeight = height - padding.top - padding.bottom const { yMin, yMax } = chartPlotBand(height) const isDark = useIsDarkTheme() - const [hoverIndex, setHoverIndex] = useState(null) const [hoverPos, setHoverPos] = useState<{ x: number; y: number } | null>(null) - const colorTokens = useMemo(() => ({ base: color }), [color]) - const resolvedColors = useResolvedChartColors(colorTokens) + const resolvedColors = useResolvedChartColors({ base: color }) const resolvedColor = resolvedColors.base || color const hasExternalWrapper = !label @@ -100,10 +108,24 @@ function BarChartComponent({ return peak <= 0 ? 1 : peak * 1.1 }, [data]) + const padding = resolveChartPadding([formatBarValue(maxValue, unit), '0']) + const chartWidth = width - padding.left - padding.right + const chartHeight = height - padding.top - padding.bottom + /** Slot geometry: every bucket owns an equal slice, with the bar centred in it. */ const slot = data.length > 0 ? Math.max(1, chartWidth) / data.length : 0 const barWidth = Math.max(1, Math.min(24, slot * 0.7)) + /** + * Bars own a slot, so the hovered bucket is which slot the cursor is in — not the + * nearest sample, which is how a line chart resolves it. Derived, so a resize + * mid-hover cannot leave an index disagreeing with the slot geometry. + */ + const hoverIndex = + hoverPos === null || data.length === 0 || slot <= 0 + ? null + : Math.max(0, Math.min(data.length - 1, Math.floor((hoverPos.x - padding.left) / slot))) + const bars = useMemo( () => data.map((point, index) => { @@ -127,21 +149,14 @@ function BarChartComponent({ [data, slot, barWidth, maxValue, chartHeight, height, padding.left, padding.top, yMin, yMax] ) - const formatValue = (value?: number) => { - if (typeof value !== 'number' || !Number.isFinite(value)) return '—' - const suffix = (unit ?? '').toLowerCase() - if (suffix.includes('%')) return `${value.toFixed(1)}%` - if (suffix === 'latency') return formatChartLatency(value) - if (suffix.includes('ms')) return `${Math.round(value)}ms` - if (suffix === 'credits') return formatChartCompactNumber(value) - return `${Math.round(value)}${unit ?? ''}` - } - if (containerWidth === null) { return (
) @@ -156,7 +171,7 @@ function BarChartComponent({ ref={containerRef} className={cn( 'flex w-full items-center justify-center', - !hasExternalWrapper && 'rounded-lg border bg-card p-4' + !hasExternalWrapper && 'rounded-lg border bg-[var(--surface-1)] p-4' )} /* Height only. `width` is floored at CHART_MIN_WIDTH for the plot geometry, @@ -185,8 +200,8 @@ function BarChartComponent({ contradicted the constant's own note that the chart "scrolls rather than compresses". At or above the floor there is no overflow and nothing changes. */ - 'w-full overflow-x-auto', - !hasExternalWrapper && 'rounded-[11px] border bg-card p-4 shadow-sm' + 'w-full overflow-x-auto overflow-y-hidden', + !hasExternalWrapper && 'rounded-lg border bg-[var(--surface-1)] p-4 shadow-card' )} > {!hasExternalWrapper && ( @@ -202,20 +217,9 @@ function BarChartComponent({ onMouseMove={(e) => { if (bars.length === 0 || slot <= 0) return const rect = (e.currentTarget as SVGSVGElement).getBoundingClientRect() - const x = e.clientX - rect.left - // Bars own a slot, so the hovered bucket is which slot the cursor is in — - // not the nearest sample, which is how a line chart resolves it. - const index = Math.max( - 0, - Math.min(data.length - 1, Math.floor((x - padding.left) / slot)) - ) - setHoverIndex(index) - setHoverPos({ x, y: e.clientY - rect.top }) - }} - onMouseLeave={() => { - setHoverIndex(null) - setHoverPos(null) + setHoverPos({ x: e.clientX - rect.left, y: e.clientY - rect.top }) }} + onMouseLeave={() => setHoverPos(null)} > @@ -229,7 +233,7 @@ function BarChartComponent({ y1={padding.top} x2={padding.left} y2={height - padding.bottom} - stroke='hsl(var(--border))' + stroke='var(--border)' strokeWidth='1' /> @@ -240,7 +244,7 @@ function BarChartComponent({ y1={padding.top + chartHeight * fraction} x2={width - padding.right} y2={padding.top + chartHeight * fraction} - stroke='hsl(var(--muted))' + stroke='var(--border)' strokeOpacity='0.35' strokeWidth='1' /> @@ -313,7 +317,7 @@ function BarChartComponent({ })} {/* Same formatter the tooltip uses, or the axis and the hover disagree about what the numbers mean on any non-`credits` unit. */} - {formatValue(maxValue)} + {formatBarValue(maxValue, unit)} @@ -347,20 +351,19 @@ function BarChartComponent({ bars[hoverIndex] && (() => { const bar = bars[hoverIndex] - const value = formatValue(bar.point.value) + const value = formatBarValue(bar.point.value, unit) + const date = formatChartTimestamp(bar.point.timestamp) const { left, top } = positionChartTooltip({ anchorX: hoverPos?.x ?? bar.x, anchorY: hoverPos?.y ?? bar.y, width, height, tooltipMaxWidth: estimateTooltipWidth(value.length), + tooltipHeight: estimateTooltipHeight(1, Boolean(date)), + padding, }) return ( - + ) diff --git a/apps/sim/components/charts/chart-geometry.test.ts b/apps/sim/components/charts/chart-geometry.test.ts index 82c2694abbe..6108dc3386b 100644 --- a/apps/sim/components/charts/chart-geometry.test.ts +++ b/apps/sim/components/charts/chart-geometry.test.ts @@ -3,9 +3,12 @@ */ import { describe, expect, it } from 'vitest' import { + CHART_AXIS_LABEL_GAP, CHART_PADDING, chartPlotBand, + estimateAxisLabelWidth, formatTimeTick, + resolveChartPadding, resolveSpanMs, resolveTimeTickIndices, } from '@/components/charts/chart-geometry' @@ -78,3 +81,40 @@ describe('chartPlotBand', () => { expect(chartPlotBand(240).yMax).toBeGreaterThan(chartPlotBand(166).yMax) }) }) + +describe('resolveChartPadding', () => { + it('widens the gutter until the longest label fits beside the axis', () => { + const { left } = resolveChartPadding(['7.3k', '0']) + expect(left).toBeGreaterThanOrEqual(estimateAxisLabelWidth('7.3k') + CHART_AXIS_LABEL_GAP) + }) + + it('never narrows below the shared padding', () => { + expect(resolveChartPadding(['0', '0']).left).toBeGreaterThanOrEqual(CHART_PADDING.left) + expect(resolveChartPadding([]).left).toBeGreaterThanOrEqual(CHART_PADDING.left) + }) + + /** + * Three charts sit side by side on the logs dashboard. A gutter derived exactly from + * each one's own labels put their plot origins at 26, 27 and 32 — visibly ragged + * across a row that used to share one origin. + */ + it('resolves labels of similar width to the same gutter', () => { + const gutters = [['5'], ['1.2s'], ['12.3k'], ['0'], ['7.3k']].map( + (labels) => resolveChartPadding(labels).left + ) + expect(new Set(gutters).size).toBe(1) + }) + + it('still grows for a genuinely wider label', () => { + expect(resolveChartPadding(['123456.7m']).left).toBeGreaterThan( + resolveChartPadding(['7.3k']).left + ) + }) + + it('leaves the other three sides on the shared constant', () => { + const padding = resolveChartPadding(['123.4m']) + expect(padding.top).toBe(CHART_PADDING.top) + expect(padding.right).toBe(CHART_PADDING.right) + expect(padding.bottom).toBe(CHART_PADDING.bottom) + }) +}) diff --git a/apps/sim/components/charts/chart-geometry.ts b/apps/sim/components/charts/chart-geometry.ts index e85a010d251..3494543c3c2 100644 --- a/apps/sim/components/charts/chart-geometry.ts +++ b/apps/sim/components/charts/chart-geometry.ts @@ -10,16 +10,79 @@ export const CHART_PADDING = { top: 16, right: 28, bottom: 26, left: 26 } as const +export type ChartPadding = { top: number; right: number; bottom: number; left: number } + /** Matches the loader placeholders callers size themselves against. */ export const CHART_DEFAULT_HEIGHT = 166 -/** Below this the axis labels collide, so the chart scrolls rather than compresses. */ +/** + * Below this the axis labels collide, so the chart scrolls rather than compresses. + * + * Consumers pair `overflow-x-auto` with `overflow-y-hidden`: a computed `overflow-x` + * other than `visible` promotes `overflow-y: visible` to `auto`, so the tooltip's + * shadow reaching the foot of the box raised a vertical scrollbar over the chart + * whenever the cursor neared the axis. + */ export const CHART_MIN_WIDTH = 280 export const CHART_TICK_FILL = 'var(--text-tertiary)' -export const CHART_TICK_FONT_SIZE = '9' +export const CHART_TICK_FONT_SIZE = 9 export const CHART_GRID_FRACTIONS = [0.25, 0.5, 0.75] as const +/** Punctuation and whitespace, which sit near half the width of a digit or letter. */ +const NARROW_GLYPH = /[.,:\s]/ + +/** Gap between a y-axis tick label's right edge and the axis rule. */ +export const CHART_AXIS_LABEL_GAP = 8 + +/** + * The gutter is rounded up to a multiple of this. + * + * Charts are read side by side — the logs dashboard puts three in one row — and a + * gutter derived exactly from each chart's own labels made `5`, `1.2s` and `12.3k` + * resolve to 26, 27 and 32, so three plots that used to share an origin no longer + * did. Quantizing collapses differences this small to one value while still growing + * for a genuinely wider label, and it turns the sub-pixel slack that `Math.ceil` + * alone left into several pixels. + */ +const CHART_AXIS_GUTTER_STEP = 8 + +/** + * Rendered width of a right-anchored y-axis tick label. + * + * SVG `` cannot be measured before layout, so the gutter that has to hold it + * is estimated from the glyphs instead. The ratios are for the UI sans at + * {@link CHART_TICK_FONT_SIZE}: digits and letters sit near 0.58em, punctuation and + * spaces near 0.3em. Deliberately generous — an over-wide gutter costs a couple of + * plot pixels, an under-wide one clips the label against the container's edge. + */ +export function estimateAxisLabelWidth(text: string): number { + let width = 0 + for (const character of text) { + width += NARROW_GLYPH.test(character) ? 0.3 : 0.58 + } + return width * CHART_TICK_FONT_SIZE +} + +/** + * {@link CHART_PADDING} with a left gutter wide enough for the chart's own y-axis + * labels. + * + * The fixed 26px gutter left 18px of drawable width once the label gap is taken out, + * which fits four narrow glyphs — so any tick past `7.3k` was cut off at the left edge + * of the container. Both charts resolve their gutter through this one function from + * the labels they are about to draw, so a bar and a line chart showing comparable + * magnitudes still line up when stacked in one card, and neither can clip. + */ +export function resolveChartPadding(yAxisLabels: readonly string[]): ChartPadding { + const widest = yAxisLabels.reduce((max, label) => Math.max(max, estimateAxisLabelWidth(label)), 0) + const required = Math.max(CHART_PADDING.left, widest + CHART_AXIS_LABEL_GAP) + return { + ...CHART_PADDING, + left: Math.ceil(required / CHART_AXIS_GUTTER_STEP) * CHART_AXIS_GUTTER_STEP, + } +} + /** Vertical clamp for plotted geometry, keeping strokes off the axis rules. */ export function chartPlotBand(height: number): { yMin: number; yMax: number } { const chartHeight = height - CHART_PADDING.top - CHART_PADDING.bottom diff --git a/apps/sim/components/charts/chart-layout.test.tsx b/apps/sim/components/charts/chart-layout.test.tsx new file mode 100644 index 00000000000..3ab1c4e4bc8 --- /dev/null +++ b/apps/sim/components/charts/chart-layout.test.tsx @@ -0,0 +1,239 @@ +/** + * @vitest-environment jsdom + */ +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { BarChart } from '@/components/charts/bar-chart' +import { CHART_PADDING } from '@/components/charts/chart-geometry' +import { RadarChart } from '@/components/charts/radar-chart' + +/** + * Rendered-geometry guards for the chart family. + * + * These assert against the real SVG the components emit rather than against the + * geometry helpers in isolation: the two clipping bugs this file exists for — a + * y-axis label cut off at the container's left edge, and a radar caption painting + * over the section beside it — were both invisible to a unit test of the maths, + * because each came from a *callsite* combining correct helpers wrongly. + */ + +let container: HTMLDivElement +let root: Root + +/** jsdom lays nothing out, so the width the chart measures has to be supplied. */ +function mountAtWidth(width: number, element: React.ReactElement): SVGSVGElement { + container = document.createElement('div') + document.body.appendChild(container) + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockReturnValue({ + width, + height: 0, + top: 0, + left: 0, + right: width, + bottom: 0, + x: 0, + y: 0, + toJSON: () => ({}), + } as DOMRect) + root = createRoot(container) + act(() => root.render(element)) + const svg = container.querySelector('svg') + if (!svg) throw new Error('chart did not render an svg') + return svg +} + +/** Right-anchored SVG text at 9px, measured the way the chart's own estimator does. */ +function textExtent(text: string): number { + let width = 0 + for (const character of text) width += /[.,:\s]/.test(character) ? 0.3 : 0.58 + return width * 9 +} + +beforeEach(() => { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + globalThis.ResizeObserver = class { + observe() {} + unobserve() {} + disconnect() {} + } as unknown as typeof ResizeObserver +}) + +afterEach(() => { + act(() => root?.unmount()) + container?.remove() + vi.restoreAllMocks() +}) + +function dailySeries(count: number, peak: number) { + return Array.from({ length: count }, (_, index) => ({ + timestamp: new Date(Date.UTC(2026, 0, 1 + index)).toISOString(), + value: index === 0 ? peak : peak / 10, + })) +} + +describe('BarChart rendered geometry', () => { + const widths = [280, 420, 680, 1024] + const peaks = [7300, 173_000, 1_234_567] + + it.each(widths.flatMap((width) => peaks.map((peak) => [width, peak] as const)))( + 'keeps the y-axis labels inside the box at width %i, peak %i', + (width, peak) => { + const svg = mountAtWidth( + width, + + ) + const labels = [...svg.querySelectorAll('text')].filter( + (node) => node.getAttribute('text-anchor') === 'end' + ) + expect(labels.length).toBe(2) + for (const label of labels) { + const anchorX = Number(label.getAttribute('x')) + // Right-anchored: the glyphs run leftward from the anchor. + expect(anchorX - textExtent(label.textContent ?? '')).toBeGreaterThanOrEqual(0) + } + } + ) + + it.each(widths)('keeps every bar inside the plot area at width %i', (width) => { + const svg = mountAtWidth( + width, + + ) + const bars = [...svg.querySelectorAll('rect')] + expect(bars.length).toBeGreaterThan(0) + const svgWidth = Number(svg.getAttribute('width')) + for (const bar of bars) { + const x = Number(bar.getAttribute('x')) + const right = x + Number(bar.getAttribute('width')) + expect(x).toBeGreaterThanOrEqual(CHART_PADDING.left) + expect(right).toBeLessThanOrEqual(svgWidth - CHART_PADDING.right + 0.01) + } + }) + + it('keeps the first and last x-axis tick label inside the box', () => { + const width = 680 + const svg = mountAtWidth( + width, + + ) + const ticks = [...svg.querySelectorAll('text')].filter( + (node) => node.getAttribute('text-anchor') === 'middle' + ) + expect(ticks.length).toBeGreaterThan(1) + for (const tick of ticks) { + const centre = Number(tick.getAttribute('x')) + const half = textExtent(tick.textContent ?? '') / 2 + expect(centre - half).toBeGreaterThanOrEqual(0) + expect(centre + half).toBeLessThanOrEqual(width) + } + }) +}) + +describe('RadarChart rendered geometry', () => { + const LONG = 'Knowledge Base Sync' + + /** + * Every caption long, not just the first. + * + * The first axis sits at twelve o'clock, where a caption is centred and has the + * whole half-width to spend — the one position that cannot overflow horizontally. + * A fixture that only made that one long proved nothing about the axes that + * actually run out of room. + */ + function axesOf(count: number) { + return Array.from({ length: count }, (_, index) => ({ + label: `${LONG} ${index}`, + value: 100 * (index + 1), + display: String(100 * (index + 1)), + })) + } + + it.each([ + [280, 3], + [280, 6], + [320, 4], + [420, 5], + [420, 6], + [520, 7], + [680, 6], + ])('keeps every axis caption inside the box at width %i with %i axes', (width, axisCount) => { + const svg = mountAtWidth(width, ) + const height = Number(svg.getAttribute('height')) + const captions = [...svg.querySelectorAll('text')] + expect(captions.length).toBe(axisCount) + + for (const caption of captions) { + const x = Number(caption.getAttribute('x')) + const y = Number(caption.getAttribute('y')) + const anchor = caption.getAttribute('text-anchor') + const extent = textExtent(caption.textContent ?? '') + const left = anchor === 'start' ? x : anchor === 'end' ? x - extent : x - extent / 2 + const right = left + extent + expect(left).toBeGreaterThanOrEqual(0) + expect(right).toBeLessThanOrEqual(width) + + // An 'auto' baseline sits the glyphs above y; 'middle' centres them on it. + const capHeight = 9 + const top = + caption.getAttribute('dominant-baseline') === 'middle' ? y - capHeight / 2 : y - capHeight + const bottom = top + capHeight + expect(top).toBeGreaterThanOrEqual(0) + expect(bottom).toBeLessThanOrEqual(height) + } + }) + + it('draws a positive-radius web rather than collapsing at the narrow floor', () => { + const svg = mountAtWidth(280, ) + const rings = [...svg.querySelectorAll('polygon')].filter( + (node) => node.getAttribute('fill') === 'none' + ) + expect(rings.length).toBeGreaterThan(0) + const outer = rings[rings.length - 1] + const points = (outer.getAttribute('points') ?? '') + .split(' ') + .map((pair) => pair.split(',').map(Number)) + const xs = points.map(([x]) => x) + const ys = points.map(([, y]) => y) + expect(Math.max(...xs) - Math.min(...xs)).toBeGreaterThan(40) + expect(Math.max(...ys) - Math.min(...ys)).toBeGreaterThan(40) + }) + + it('renders the empty state rather than a degenerate polygon below three axes', () => { + container = document.createElement('div') + document.body.appendChild(container) + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockReturnValue({ + width: 420, + height: 0, + top: 0, + left: 0, + right: 420, + bottom: 0, + x: 0, + y: 0, + toJSON: () => ({}), + } as DOMRect) + root = createRoot(container) + act(() => root.render()) + expect(container.querySelector('svg')).toBeNull() + expect(container.textContent).toContain('No data') + }) +}) diff --git a/apps/sim/components/charts/chart-tooltip.test.ts b/apps/sim/components/charts/chart-tooltip.test.ts new file mode 100644 index 00000000000..0f414b14a5f --- /dev/null +++ b/apps/sim/components/charts/chart-tooltip.test.ts @@ -0,0 +1,114 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { CHART_PADDING, resolveChartPadding } from '@/components/charts/chart-geometry' +import { + estimateTooltipHeight, + estimateTooltipWidth, + positionChartTooltip, +} from '@/components/charts/chart-tooltip' + +const WIDTH = 800 +const HEIGHT = 166 + +function place(anchorY: number, rows = 1, hasDate = true) { + const tooltipHeight = estimateTooltipHeight(rows, hasDate) + const position = positionChartTooltip({ + anchorX: 400, + anchorY, + width: WIDTH, + height: HEIGHT, + tooltipMaxWidth: estimateTooltipWidth(12), + tooltipHeight, + }) + return { ...position, tooltipHeight } +} + +describe('positionChartTooltip', () => { + /** Guards the height-aware vertical clamp — see `positionChartTooltip`. */ + it('keeps the whole box inside the chart when the cursor is at the very bottom', () => { + const { top, tooltipHeight } = place(HEIGHT) + expect(top + tooltipHeight).toBeLessThanOrEqual(HEIGHT) + }) + + it('holds for a taller multi-row tooltip, which overflows soonest', () => { + const { top, tooltipHeight } = place(HEIGHT, 5) + expect(top + tooltipHeight).toBeLessThanOrEqual(HEIGHT) + expect(top).toBeGreaterThanOrEqual(0) + }) + + it('never places the box above the chart when the cursor is at the top', () => { + expect(place(0).top).toBeGreaterThanOrEqual(0) + }) + + it('prefers the right of the cursor and flips left near the right edge', () => { + const boxWidth = estimateTooltipWidth(12) + const right = positionChartTooltip({ + anchorX: 100, + anchorY: 80, + width: WIDTH, + height: HEIGHT, + tooltipMaxWidth: boxWidth, + tooltipHeight: estimateTooltipHeight(1, true), + }) + expect(right.left).toBeGreaterThan(100) + + const flipped = positionChartTooltip({ + anchorX: WIDTH - CHART_PADDING.right, + anchorY: 80, + width: WIDTH, + height: HEIGHT, + tooltipMaxWidth: boxWidth, + tooltipHeight: estimateTooltipHeight(1, true), + }) + expect(flipped.left + boxWidth).toBeLessThanOrEqual(WIDTH - CHART_PADDING.right) + }) + + /** A chart with wide axis labels has a wider gutter, and the clamp must follow it. */ + it('clamps the left edge to the resolved gutter, not the shared constant', () => { + const padding = resolveChartPadding(['123456.7m']) + const { left } = positionChartTooltip({ + anchorX: 0, + anchorY: 80, + width: WIDTH, + height: HEIGHT, + tooltipMaxWidth: estimateTooltipWidth(12), + tooltipHeight: estimateTooltipHeight(1, true), + padding, + }) + expect(left).toBeGreaterThanOrEqual(padding.left) + expect(padding.left).toBeGreaterThan(CHART_PADDING.left) + }) +}) + +describe('estimateTooltipHeight', () => { + it('grows with each row and with the date header', () => { + expect(estimateTooltipHeight(2, true)).toBeGreaterThan(estimateTooltipHeight(1, true)) + expect(estimateTooltipHeight(1, true)).toBeGreaterThan(estimateTooltipHeight(1, false)) + }) + + it('reserves a row even when told there are none', () => { + expect(estimateTooltipHeight(0, false)).toBe(estimateTooltipHeight(1, false)) + }) + + /** + * The estimate is what the clamp measures against, and the chart clips its overflow, + * so it must never come in under the real box — an underestimate cuts the bottom off + * rather than moving the box up. Measured here against the box model the tooltip's + * own class string implies: `border` + `py-1.5`, a `text-micro` date with `mb-1`, + * and one `text-xs` row per value, every line at the ambient 1.5 line-height. + */ + it('never comes in under the box the tooltip actually renders', () => { + const chrome = 2 + 6 + 6 + const dateLine = 10 * 1.5 + 4 + const rowLine = 11 * 1.5 + + for (const rows of [1, 2, 5]) { + expect(estimateTooltipHeight(rows, true)).toBeGreaterThanOrEqual( + chrome + dateLine + rows * rowLine + ) + expect(estimateTooltipHeight(rows, false)).toBeGreaterThanOrEqual(chrome + rows * rowLine) + } + }) +}) diff --git a/apps/sim/components/charts/chart-tooltip.tsx b/apps/sim/components/charts/chart-tooltip.tsx index 6a49032394a..518a470479a 100644 --- a/apps/sim/components/charts/chart-tooltip.tsx +++ b/apps/sim/components/charts/chart-tooltip.tsx @@ -1,7 +1,7 @@ 'use client' import type { ReactNode } from 'react' -import { CHART_PADDING } from '@/components/charts/chart-geometry' +import { CHART_PADDING, type ChartPadding } from '@/components/charts/chart-geometry' /** * The chart family's hover surface. Defined once so a sibling chart cannot ship a @@ -9,7 +9,7 @@ import { CHART_PADDING } from '@/components/charts/chart-geometry' * between the line chart and the status bar. */ export const CHART_TOOLTIP_CLASSES = - 'pointer-events-none absolute rounded-lg border border-[var(--border-1)] bg-[var(--surface-1)] px-2 py-1.5 text-xs shadow-lg' + 'pointer-events-none absolute rounded-lg border border-[var(--border)] bg-[var(--surface-1)] px-2 py-1.5 text-xs shadow-overlay' interface PositionChartTooltipArgs { anchorX: number @@ -17,11 +17,19 @@ interface PositionChartTooltipArgs { width: number height: number tooltipMaxWidth: number + tooltipHeight: number + /** The chart's resolved padding, whose left gutter varies with its axis labels. */ + padding?: ChartPadding } /** * Places the tooltip beside the cursor, preferring the right and flipping left when - * it would overflow, then clamping into the plot band so it never escapes the card. + * it would overflow, then clamping it wholly inside the chart box. + * + * The vertical clamp is against the tooltip's own height rather than a fixed inset. + * A fixed one let the box hang a pixel or two past the bottom near the foot of the + * plot, and because the scroll container's `overflow-x` forces `overflow-y` to `auto`, + * those pixels raised a vertical scrollbar the moment the cursor approached the axis. */ export function positionChartTooltip({ anchorX, @@ -29,20 +37,19 @@ export function positionChartTooltip({ width, height, tooltipMaxWidth, + tooltipHeight, + padding = CHART_PADDING, }: PositionChartTooltipArgs): { left: number; top: number } { const margin = 10 - const rightEdge = width - CHART_PADDING.right + const rightEdge = width - padding.right const preferRight = anchorX + margin + tooltipMaxWidth <= rightEdge const left = preferRight - ? Math.max(CHART_PADDING.left, Math.min(anchorX + margin, rightEdge - tooltipMaxWidth)) + ? Math.max(padding.left, Math.min(anchorX + margin, rightEdge - tooltipMaxWidth)) : Math.max( - CHART_PADDING.left, + padding.left, Math.min(anchorX - margin - tooltipMaxWidth, rightEdge - tooltipMaxWidth) ) - const top = Math.min( - Math.max(anchorY - 26, CHART_PADDING.top), - height - CHART_PADDING.bottom - 18 - ) + const top = Math.max(0, Math.min(anchorY - 26, height - tooltipHeight)) return { left, top } } @@ -51,6 +58,37 @@ export function estimateTooltipWidth(longestRowLength: number): number { return Math.min(220, Math.max(80, 7 * longestRowLength + 24)) } +/** Border plus the `py-1.5` the tooltip's own class string sets. */ +const TOOLTIP_CHROME_HEIGHT = 2 + 12 + +/** + * The `text-micro` date's line box plus its `mb-1`. + * + * The type scale pairs no line-height with a font size, so a line occupies the + * ambient 1.5 rather than the font size itself — 15px for 10px `text-micro`, not 10. + */ +const TOOLTIP_DATE_HEIGHT = 15 + 4 + +/** One `text-xs` row's line box: 11px at the ambient 1.5, rounded up from 16.5. */ +const TOOLTIP_ROW_HEIGHT = 17 + +/** + * Height of the box {@link ChartTooltip} renders, from its own box model. + * + * Estimated rather than measured because the position is computed in the same render + * that mounts the tooltip — reading a real height would need a second paint, which + * shows up as the tooltip visibly jumping under the cursor. Every part rounds up: + * this is what {@link positionChartTooltip} clamps against and the chart clips its + * overflow, so an underestimate cuts the bottom off the box rather than moving it. + */ +export function estimateTooltipHeight(rowCount: number, hasDate: boolean): number { + return ( + TOOLTIP_CHROME_HEIGHT + + (hasDate ? TOOLTIP_DATE_HEIGHT : 0) + + Math.max(1, rowCount) * TOOLTIP_ROW_HEIGHT + ) +} + interface ChartTooltipProps { left: number top: number @@ -83,7 +121,7 @@ export function ChartTooltipRow({ color, label, value }: ChartTooltipRowProps) { style={{ backgroundColor: color }} /> {label && {label}} - {value} + {value}
) } diff --git a/apps/sim/components/charts/index.ts b/apps/sim/components/charts/index.ts index 138241f0436..1be948503bf 100644 --- a/apps/sim/components/charts/index.ts +++ b/apps/sim/components/charts/index.ts @@ -10,3 +10,4 @@ export { type LineChartMultiSeries, type LineChartPoint, } from '@/components/charts/line-chart' +export { RadarChart, type RadarChartAxis } from '@/components/charts/radar-chart' diff --git a/apps/sim/components/charts/line-chart.tsx b/apps/sim/components/charts/line-chart.tsx index 6cfd8d43979..0896f1df4ee 100644 --- a/apps/sim/components/charts/line-chart.tsx +++ b/apps/sim/components/charts/line-chart.tsx @@ -8,19 +8,21 @@ import { formatChartTimestamp, } from '@/components/charts/chart-format' import { + CHART_AXIS_LABEL_GAP, CHART_DEFAULT_HEIGHT, CHART_GRID_FRACTIONS, - CHART_PADDING, CHART_TICK_FILL, CHART_TICK_FONT_SIZE, chartPlotBand, formatTimeTick, + resolveChartPadding, resolveSpanMs, resolveTimeTickIndices, } from '@/components/charts/chart-geometry' import { ChartTooltip, ChartTooltipRow, + estimateTooltipHeight, estimateTooltipWidth, positionChartTooltip, } from '@/components/charts/chart-tooltip' @@ -53,6 +55,38 @@ interface LineChartProps { height?: number } +/** + * Smoothed path through `points`, with every control point clamped into the plot + * band so a curve between two near-axis samples cannot bow over an axis rule. + * + * At module scope because the base line and each extra series need the identical + * curve: the two copies had drifted apart before, and a clamp fixed in one drew a + * different shape from the other. + */ +function buildSmoothPath( + points: ReadonlyArray<{ x: number; y: number }>, + yMin: number, + yMax: number +): string { + if (points.length <= 1) return '' + const tension = 0.2 + let d = `M ${points[0].x} ${points[0].y}` + for (let i = 0; i < points.length - 1; i++) { + const p0 = points[i - 1] || points[i] + const p1 = points[i] + const p2 = points[i + 1] + const p3 = points[i + 2] || points[i + 1] + const cp1x = p1.x + ((p2.x - p0.x) / 6) * tension + let cp1y = p1.y + ((p2.y - p0.y) / 6) * tension + const cp2x = p2.x - ((p3.x - p1.x) / 6) * tension + let cp2y = p2.y - ((p3.y - p1.y) / 6) * tension + cp1y = Math.max(yMin, Math.min(yMax, cp1y)) + cp2y = Math.max(yMin, Math.min(yMax, cp2y)) + d += ` C ${cp1x} ${cp1y}, ${cp2x} ${cp2y}, ${p2.x} ${p2.y}` + } + return d +} + function LineChartComponent({ data, label, @@ -69,23 +103,16 @@ function LineChartComponent({ const uniqueId = useId().replace(/:/g, '') const [containerRef, containerWidth] = useChartWidth() const width = containerWidth ?? 0 - const padding = CHART_PADDING - const chartWidth = width - padding.left - padding.right - const chartHeight = height - padding.top - padding.bottom - const [hoverIndex, setHoverIndex] = useState(null) const isDark = useIsDarkTheme() const [hoverSeriesId, setHoverSeriesId] = useState(null) const [activeSeriesId, setActiveSeriesId] = useState(null) const [hoverPos, setHoverPos] = useState<{ x: number; y: number } | null>(null) - const colorTokens = useMemo(() => { - const tokens: Record = { base: color } - for (const s of series ?? []) { - const id = s.id || s.label || '' - if (id) tokens[id] = s.color - } - return tokens - }, [color, series]) + const colorTokens: Record = { base: color } + for (const s of series ?? []) { + const id = s.id || s.label || '' + if (id) colorTokens[id] = s.color + } const resolvedColors = useResolvedChartColors(colorTokens) const hasExternalWrapper = !label || label === '' @@ -129,6 +156,25 @@ function LineChartComponent({ } }, [allSeries, unit]) + /** + * The two y-axis tick labels, resolved once so the gutter that has to hold them is + * measured from the same strings the axis draws. + */ + const yAxisLabels = useMemo(() => { + const unitSuffix = (unit || '').trim() + const isLatency = unitSuffix.toLowerCase() === 'latency' + const suffix = unitSuffix === '%' && !isLatency ? unitSuffix : '' + const compact = (value: number) => { + if (isLatency) return value === 0 ? '0' : formatChartLatency(value) + return `${formatChartCompactNumber(value)}${suffix}` + } + return [compact(maxValue), compact(minValue)] as const + }, [maxValue, minValue, unit]) + + const padding = resolveChartPadding(yAxisLabels) + const chartWidth = width - padding.left - padding.right + const chartHeight = height - padding.top - padding.bottom + const { yMin, yMax } = chartPlotBand(height) const scaledPoints = useMemo( @@ -143,6 +189,28 @@ function LineChartComponent({ [data, chartWidth, chartHeight, minValue, valueRange, yMin, yMax, padding.left, padding.top] ) + /** + * The hovered sample, derived from the stored cursor rather than stored beside it. + * + * Clamped here rather than relying on the stored x having been clamped at mousemove + * time: `padding.left` follows the axis labels and `chartWidth` follows the + * container, so either can move with no pointer event at all — a sidebar collapse + * mid-hover otherwise pushed the ratio past 1 and indexed off the end, and the dot, + * the rule and the tooltip all vanished until the cursor moved again. + */ + const hoverIndex = + hoverPos === null || scaledPoints.length === 0 + ? null + : Math.max( + 0, + Math.min( + scaledPoints.length - 1, + Math.round( + ((hoverPos.x - padding.left) / (chartWidth || 1)) * (scaledPoints.length - 1) + ) + ) + ) + const scaledSeries = useMemo( () => allSeries.map((s) => { @@ -169,31 +237,11 @@ function LineChartComponent({ ) const getSeriesById = (id?: string | null) => scaledSeries.find((s) => s.id === id) - const visibleSeries = useMemo( - () => (activeSeriesId ? scaledSeries.filter((s) => s.id === activeSeriesId) : scaledSeries), - [activeSeriesId, scaledSeries] - ) + const visibleSeries = activeSeriesId + ? scaledSeries.filter((s) => s.id === activeSeriesId) + : scaledSeries - const pathD = useMemo(() => { - if (scaledPoints.length <= 1) return '' - const p = scaledPoints - const tension = 0.2 - let d = `M ${p[0].x} ${p[0].y}` - for (let i = 0; i < p.length - 1; i++) { - const p0 = p[i - 1] || p[i] - const p1 = p[i] - const p2 = p[i + 1] - const p3 = p[i + 2] || p[i + 1] - const cp1x = p1.x + ((p2.x - p0.x) / 6) * tension - let cp1y = p1.y + ((p2.y - p0.y) / 6) * tension - const cp2x = p2.x - ((p3.x - p1.x) / 6) * tension - let cp2y = p2.y - ((p3.y - p1.y) / 6) * tension - cp1y = Math.max(yMin, Math.min(yMax, cp1y)) - cp2y = Math.max(yMin, Math.min(yMax, cp2y)) - d += ` C ${cp1x} ${cp1y}, ${cp2x} ${cp2y}, ${p2.x} ${p2.y}` - } - return d - }, [scaledPoints, yMin, yMax]) + const pathD = useMemo(() => buildSmoothPath(scaledPoints, yMin, yMax), [scaledPoints, yMin, yMax]) const currentHoverDate = hoverIndex !== null && data[hoverIndex] ? formatChartTimestamp(data[hoverIndex].timestamp) : '' @@ -202,7 +250,10 @@ function LineChartComponent({ return (
) @@ -213,7 +264,7 @@ function LineChartComponent({
{!hasExternalWrapper && ( @@ -259,11 +310,11 @@ function LineChartComponent({ variant='ghost' aria-pressed={activeSeriesId === s.id} aria-label={`Toggle ${s.label}`} - className='inline-flex items-center gap-1 rounded-md border border-[var(--border)] bg-transparent px-1.5 py-0.5 text-micro' - style={{ - color: resolvedColors[s.id || ''] || s.color, - opacity: dimmed ? 0.4 : isHovered ? 1 : 0.9, - }} + className={cn( + 'inline-flex items-center gap-1 rounded-md border border-[var(--border)] bg-transparent px-1.5 py-0.5 text-micro', + dimmed ? 'opacity-40' : isHovered ? 'opacity-100' : 'opacity-90' + )} + style={{ color: resolvedColors[s.id || ''] || s.color }} onMouseEnter={() => setHoverSeriesId(s.id || null)} onMouseLeave={() => setHoverSeriesId((prev) => (prev === s.id ? null : prev))} onKeyDown={(e) => { @@ -301,7 +352,6 @@ function LineChartComponent({ const clamped = Math.max(padding.left, Math.min(width - padding.right, x)) const ratio = (clamped - padding.left) / (chartWidth || 1) const i = Math.round(ratio * (scaledPoints.length - 1)) - setHoverIndex(i) setHoverPos({ x: clamped, y: e.clientY - rect.top }) const cursorY = e.clientY - rect.top if (activeSeriesId) { @@ -321,7 +371,6 @@ function LineChartComponent({ } }} onMouseLeave={() => { - setHoverIndex(null) setHoverPos(null) setHoverSeriesId(null) }} @@ -355,7 +404,7 @@ function LineChartComponent({ y1={padding.top} x2={padding.left} y2={height - padding.bottom} - stroke='hsl(var(--border))' + stroke='var(--border)' strokeWidth='1' /> @@ -366,7 +415,7 @@ function LineChartComponent({ y1={padding.top + chartHeight * p} x2={width - padding.right} y2={padding.top + chartHeight * p} - stroke='hsl(var(--muted))' + stroke='var(--border)' strokeOpacity='0.35' strokeWidth='1' /> @@ -433,25 +482,7 @@ function LineChartComponent({ /> ) } - const p = (() => { - const p = s.pts - const tension = 0.2 - let d = `M ${p[0].x} ${p[0].y}` - for (let i = 0; i < p.length - 1; i++) { - const p0 = p[i - 1] || p[i] - const p1 = p[i] - const p2 = p[i + 1] - const p3 = p[i + 2] || p[i + 1] - const cp1x = p1.x + ((p2.x - p0.x) / 6) * tension - let cp1y = p1.y + ((p2.y - p0.y) / 6) * tension - const cp2x = p2.x - ((p3.x - p1.x) / 6) * tension - let cp2y = p2.y - ((p3.y - p1.y) / 6) * tension - cp1y = Math.max(yMin, Math.min(yMax, cp1y)) - cp2y = Math.max(yMin, Math.min(yMax, cp2y)) - d += ` C ${cp1x} ${cp1y}, ${cp2x} ${cp2y}, ${p2.x} ${p2.y}` - } - return d - })() + const p = buildSmoothPath(s.pts, yMin, yMax) return ( { - const unitSuffix = (unit || '').trim() - const showInTicks = unitSuffix === '%' - const isLatency = unitSuffix.toLowerCase() === 'latency' - const fmtCompact = (v: number) => { - if (isLatency) return v === 0 ? '0' : formatChartLatency(v) - return formatChartCompactNumber(v) - } - return ( - <> - - {fmtCompact(maxValue)} - {showInTicks && !isLatency ? unit : ''} - - - {fmtCompact(minValue)} - {showInTicks && !isLatency ? unit : ''} - - - ) - })()} + + {yAxisLabels[0]} + + + {yAxisLabels[1]} + @@ -613,6 +629,8 @@ function LineChartComponent({ width, height, tooltipMaxWidth: estimateTooltipWidth(longest), + tooltipHeight: estimateTooltipHeight(toDisplay.length, Boolean(currentHoverDate)), + padding, }) return ( @@ -639,7 +657,4 @@ function LineChartComponent({ ) } -/** - * Memoized LineChart component to prevent re-renders when parent updates. - */ export const LineChart = memo(LineChartComponent) diff --git a/apps/sim/components/charts/radar-chart.tsx b/apps/sim/components/charts/radar-chart.tsx new file mode 100644 index 00000000000..0f33a02b783 --- /dev/null +++ b/apps/sim/components/charts/radar-chart.tsx @@ -0,0 +1,327 @@ +'use client' + +import { memo, useId, useMemo, useState } from 'react' +import { truncate } from '@sim/utils/string' +import { + CHART_GRID_FRACTIONS, + CHART_TICK_FILL, + CHART_TICK_FONT_SIZE, + estimateAxisLabelWidth, +} from '@/components/charts/chart-geometry' +import { + ChartTooltip, + ChartTooltipRow, + estimateTooltipHeight, + estimateTooltipWidth, + positionChartTooltip, +} from '@/components/charts/chart-tooltip' +import { + useChartWidth, + useIsDarkTheme, + useResolvedChartColors, +} from '@/components/charts/use-chart-theme' + +export interface RadarChartAxis { + label: string + value: number + /** Text shown for `value` in the hover row. Defaults to the raw number. */ + display?: string +} + +interface RadarChartProps { + axes: RadarChartAxis[] + color: string + height?: number +} + +/** Room above and below the web for the captions on the vertical centreline. */ +const LABEL_GUTTER = 52 + +/** Gap between the outer ring and a caption anchored beyond it. */ +const LABEL_GAP = 12 + +/** + * The web's rings: the family's gridline fractions plus the outer ring, which is this + * chart's axis rule. Read from the constant rather than divided into `RING_COUNT` + * even steps — the arithmetic agreed with the siblings only while the fractions + * happened to be uniform, which is exactly the drift `chart-geometry` exists to stop. + */ +const RING_FRACTIONS = [...CHART_GRID_FRACTIONS, 1] as const + +/** + * Caption budget. A long source name would otherwise run past the container, and the + * svg paints outside its box so it would not even clip — it would overlap the section + * beside it. The hover row carries the full name. + */ +const MAX_LABEL_LENGTH = 16 + +/** + * Polar coordinates for an axis. `-90°` puts the first axis at twelve o'clock, so a + * list read top-down and the web read clockwise start in the same place. + */ +function axisPoint(index: number, count: number, radius: number, cx: number, cy: number) { + const angle = (index / count) * Math.PI * 2 - Math.PI / 2 + return { x: cx + Math.cos(angle) * radius, y: cy + Math.sin(angle) * radius } +} + +function polygon(points: ReadonlyArray<{ x: number; y: number }>): string { + return points.map((p) => `${p.x.toFixed(2)},${p.y.toFixed(2)}`).join(' ') +} + +/** + * Shape of a distribution across a handful of named categories. + * + * The third member of the chart family, and built from the same tokens, tooltip, and + * theme hooks as {@link BarChart} and {@link LineChart}. It answers a question the + * other two cannot: a bar list ranks categories but says nothing about balance, and + * "one source dominates" versus "spend is spread evenly" is legible here at a glance + * and nowhere else on the panel. + * + * Every axis is scaled against the largest value rather than against its own range, + * so the polygon's area is proportional to the real distribution — normalising each + * axis independently would draw a balanced pentagon for any input at all. + */ +function RadarChartComponent({ axes, color, height = 200 }: RadarChartProps) { + const uniqueId = useId().replace(/:/g, '') + const [containerRef, containerWidth] = useChartWidth() + const isDark = useIsDarkTheme() + const [hoverIndex, setHoverIndex] = useState(null) + + const resolvedColors = useResolvedChartColors({ base: color }) + const resolvedColor = resolvedColors.base || color + + const width = containerWidth ?? 0 + const cx = width / 2 + const cy = height / 2 + /* + One memo over the whole web: hovering re-renders this component on every wedge + enter and leave, and none of this geometry can move under a hover. Guarding only + the point projection left the costlier half — a per-glyph estimate of every + caption — running on each of those renders. + + The horizontal budget is the caption's own estimated width, the same + `estimateAxisLabelWidth` the sibling charts use to size a gutter around SVG text + they cannot measure. A 16-glyph caption runs to ~84px, so a fixed inset let every + side caption run past the plot; budgeting the radius against the real caption + width is what keeps them inside the box the svg clips to. + */ + const { maxValue, radius, points } = useMemo(() => { + const labelWidth = axes.reduce( + (max, axis) => Math.max(max, estimateAxisLabelWidth(truncate(axis.label, MAX_LABEL_LENGTH))), + 0 + ) + const webRadius = Math.max( + 0, + Math.min(width / 2 - labelWidth - LABEL_GAP, height / 2 - LABEL_GUTTER / 2) + ) + const peak = Math.max(...axes.map((axis) => axis.value), 0) + return { + maxValue: peak, + radius: webRadius, + points: axes.map((axis, index) => { + const fraction = peak > 0 ? axis.value / peak : 0 + return { + axis, + outer: axisPoint(index, axes.length, webRadius, cx, cy), + value: axisPoint(index, axes.length, webRadius * fraction, cx, cy), + label: axisPoint(index, axes.length, webRadius + LABEL_GAP, cx, cy), + } + }), + } + }, [axes, width, height, cx, cy]) + + if (containerWidth === null) { + return
+ } + + /* + Three axes are the fewest that enclose an area; below that the "polygon" is a + line or a point and reads as a rendering fault rather than as a distribution. + */ + if (axes.length < 3 || maxValue <= 0) { + return ( +
+

No data

+
+ ) + } + + const hovered = hoverIndex !== null ? points[hoverIndex] : null + + return ( + /* + Two boxes, like the siblings: the outer one scrolls, the inner one is the + positioning context. `relative` on the scroll container itself left the + absolutely-positioned tooltip anchored to the viewport of the scroll rather than + to the plot — below CHART_MIN_WIDTH it stayed nailed while the web slid under it. + + Captions are inside the plot by construction, since `radius` is budgeted against + `labelWidth`, so the horizontal scroll never cuts one off. + */ +
+
+ + + {/* + Radial rather than the siblings' vertical linear gradient — a shape with + radial symmetry lit from the top reads as a rendering error. The stop + opacities stay in the family's range, and light is the more opaque theme + because dark composites through `screen` below. + */} + + + + + + + {RING_FRACTIONS.map((fraction) => ( + axisPoint(index, axes.length, radius * fraction, cx, cy)) + )} + fill='none' + stroke='var(--border)' + strokeOpacity={fraction === 1 ? 1 : 0.35} + strokeWidth='1' + /> + ))} + {points.map((point, index) => ( + + ))} + + + point.value))} + fill={`url(#radar-${uniqueId})`} + stroke={resolvedColor} + strokeWidth={isDark ? 1.7 : 2} + strokeLinejoin='round' + /> + {points.map((point, index) => ( + + ))} + + + {points.map((point, index) => ( + cx ? 'start' : 'end' + } + dominantBaseline={ + Math.abs(point.label.x - cx) >= 1 + ? 'middle' + : point.label.y > cy + ? 'hanging' + : 'auto' + } + fontSize={CHART_TICK_FONT_SIZE} + fill={CHART_TICK_FILL} + > + {truncate(point.axis.label, MAX_LABEL_LENGTH)} + + ))} + + {/* + Hit targets last so they sit above the painted web, and wedge-sized — a + vertex-sized target is far too small to hover on a 200px chart. + + An arc sector, not a triangle. A triangle's far edge is the chord, which + along its own spoke reaches only `reach·cos(π/n)` — at three axes that is + 50px against a 74px radius, so the largest value's vertex, the one a reader + aims at, sat outside its own target and outside every other. Sectors tile + identically and reach `reach` in every direction. The sweep flag is 1 + because SVG's y grows downward, and the arc is never a major one: 2π/n ≤ + 2π/3 < π for the three-or-more axes this chart requires. + */} + {points.map((point, index) => { + const half = Math.PI / axes.length + const angle = (index / axes.length) * Math.PI * 2 - Math.PI / 2 + const reach = radius + LABEL_GUTTER / 2 + const a = { + x: cx + Math.cos(angle - half) * reach, + y: cy + Math.sin(angle - half) * reach, + } + const b = { + x: cx + Math.cos(angle + half) * reach, + y: cy + Math.sin(angle + half) * reach, + } + return ( + setHoverIndex(index)} + onMouseLeave={() => setHoverIndex(null)} + /> + ) + })} + + + {hovered && + (() => { + const value = hovered.axis.display ?? String(hovered.axis.value) + /* + Beside the hovered vertex, through the same placer the siblings use, so + the box flips and clamps identically. Centring it on the web instead put + a filled panel over the densest part of the gradient — the concentration + this chart exists to show. The padding passed is the caption gap rather + than the axis-bearing charts' gutters: a radar has no axis rules to keep + clear of. + */ + const { left, top } = positionChartTooltip({ + anchorX: hovered.value.x, + anchorY: hovered.value.y, + width, + height, + tooltipMaxWidth: estimateTooltipWidth( + Math.max(hovered.axis.label.length, value.length) + ), + tooltipHeight: estimateTooltipHeight(1, true), + padding: { top: 0, right: LABEL_GAP, bottom: 0, left: LABEL_GAP }, + }) + return ( + + + + ) + })()} +
+
+ ) +} + +export const RadarChart = memo(RadarChartComponent) diff --git a/apps/sim/components/charts/use-chart-theme.ts b/apps/sim/components/charts/use-chart-theme.ts index 30bcd9fd588..b6b864b0644 100644 --- a/apps/sim/components/charts/use-chart-theme.ts +++ b/apps/sim/components/charts/use-chart-theme.ts @@ -1,27 +1,49 @@ 'use client' -import { type RefObject, useEffect, useRef, useState } from 'react' +import { type RefObject, useEffect, useRef, useState, useSyncExternalStore } from 'react' import { CHART_MIN_WIDTH } from '@/components/charts/chart-geometry' +function subscribeToDarkTheme(onStoreChange: () => void): () => void { + const observer = new MutationObserver(onStoreChange) + observer.observe(document.documentElement, { attributes: true, attributeFilter: ['class'] }) + return () => observer.disconnect() +} + +function getDarkThemeSnapshot(): boolean { + return document.documentElement.classList.contains('dark') +} + +/** Dark is the assumed default before the class is readable, matching first paint. */ +function getServerDarkThemeSnapshot(): boolean { + return true +} + /** - * Whether the document is in dark mode, tracked by observing the class the theme - * toggle writes. Charts need this as a *value* rather than a CSS class because SVG - * stroke opacity and blend mode are set per element, not by a selector. + * Whether the document is in dark mode, read from the class the theme toggle writes. + * Charts need this as a *value* rather than a CSS class because SVG stroke opacity + * and blend mode are set per element, not by a selector. + * + * The class is an external store, so it is read through `useSyncExternalStore`: the + * first client render already sees the real value instead of painting the default and + * correcting it in an effect. */ export function useIsDarkTheme(): boolean { - const [isDark, setIsDark] = useState(true) - - useEffect(() => { - if (typeof window === 'undefined') return - const element = document.documentElement - const update = () => setIsDark(element.classList.contains('dark')) - update() - const observer = new MutationObserver(update) - observer.observe(element, { attributes: true, attributeFilter: ['class'] }) - return () => observer.disconnect() - }, []) + return useSyncExternalStore( + subscribeToDarkTheme, + getDarkThemeSnapshot, + getServerDarkThemeSnapshot + ) +} - return isDark +/** Materializes one `var(--token)` into a concrete `rgb()` via a throwaway probe node. */ +function resolveColor(value: string): string { + if (!value.startsWith('var(')) return value + const probe = document.createElement('div') + probe.style.color = value + document.body.appendChild(probe) + const computed = window.getComputedStyle(probe).color + probe.remove() + return computed } /** @@ -34,26 +56,22 @@ export function useIsDarkTheme(): boolean { export function useResolvedChartColors(colors: Record): Record { const [resolved, setResolved] = useState>({}) const serialized = JSON.stringify(colors) + /* + A token resolves to a different `rgb()` per theme, and the probe runs once per + token set — so without this the colours resolved on the theme the chart mounted + under survived a toggle, and the series kept its dark-mode fill on a light page. + */ + const isDark = useIsDarkTheme() useEffect(() => { if (typeof window === 'undefined') return - const resolveColor = (value: string): string => { - if (!value.startsWith('var(')) return value - const probe = document.createElement('div') - probe.style.color = value - document.body.appendChild(probe) - const computed = window.getComputedStyle(probe).color - probe.remove() - return computed - } - const next: Record = {} for (const [key, value] of Object.entries(JSON.parse(serialized) as Record)) { next[key] = resolveColor(value) } setResolved(next) - }, [serialized]) + }, [serialized, isDark]) return resolved } diff --git a/apps/sim/ee/audit-logs/components/audit-logs.test.ts b/apps/sim/ee/audit-logs/components/audit-logs.test.ts new file mode 100644 index 00000000000..bef651a2835 --- /dev/null +++ b/apps/sim/ee/audit-logs/components/audit-logs.test.ts @@ -0,0 +1,51 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import type { AuditLogPage } from '@/lib/api/contracts/audit-logs' +import { presentableAuditEntries } from '@/ee/audit-logs/components/audit-logs' + +function page(...ids: string[]): AuditLogPage { + return { + success: true, + data: ids.map((id) => ({ + id, + workspaceId: null, + actorId: null, + actorName: null, + actorEmail: null, + action: 'organization.updated', + resourceType: 'organization', + resourceId: null, + resourceName: null, + description: null, + metadata: null, + createdAt: '2026-01-01T00:00:00.000Z', + })), + } +} + +describe('presentableAuditEntries', () => { + it('flattens every loaded page while the scope is answerable', () => { + expect(presentableAuditEntries([page('a', 'b'), page('c')], true).map((e) => e.id)).toEqual([ + 'a', + 'b', + 'c', + ]) + }) + + /** + * The case this exists for: an unresolved workspace scope drops the filter, so its + * query key equals the unscoped feed's. Disabling the query does not clear that + * cache entry, so an admin who had just been reading the organization-wide feed + * would have kept its rows on screen under a scoped URL — and Export, which gates + * on this list being non-empty, stayed armed against them. + */ + it('presents nothing when the scope cannot be answered, even with pages cached', () => { + expect(presentableAuditEntries([page('a', 'b')], false)).toEqual([]) + }) + + it('presents nothing before any page has loaded', () => { + expect(presentableAuditEntries(undefined, true)).toEqual([]) + }) +}) diff --git a/apps/sim/ee/audit-logs/components/audit-logs.tsx b/apps/sim/ee/audit-logs/components/audit-logs.tsx index 62f41b4c16d..2c8f1c8993b 100644 --- a/apps/sim/ee/audit-logs/components/audit-logs.tsx +++ b/apps/sim/ee/audit-logs/components/audit-logs.tsx @@ -1,27 +1,27 @@ 'use client' -import { type ReactNode, useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { type ReactNode, useEffect, useMemo, useRef, useState } from 'react' import { Badge, Button, Calendar, + Chip, ChipCombobox, ChipInput, ChipSelect, type ComboboxOption, - Download, OverflowText, Popover, PopoverAnchor, PopoverContent, - RefreshCw, - Search, toast, } from '@sim/emcn' +import { Download, RefreshCw, Search, X } from '@sim/emcn/icons' import { createLogger } from '@sim/logger' import { formatDateTime } from '@sim/utils/formatting' import { isRecordLike } from '@sim/utils/object' import { useQueryStates } from 'nuqs' +import type { AuditLogPage } from '@/lib/api/contracts/audit-logs' import { formatDateShort } from '@/lib/core/utils/date-display' import { getEndDateFromTimeRange, getStartDateFromTimeRange } from '@/lib/logs/filters' import { SEARCH_DEBOUNCE_MS } from '@/lib/url-state' @@ -33,6 +33,7 @@ import { import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state' import { SettingsPanel } from '@/app/workspace/[workspaceId]/settings/components/settings-panel' import { useSettingsSearch } from '@/app/workspace/[workspaceId]/settings/components/use-settings-search' +import { useOrganizationWorkspaces } from '@/ee/access-control/hooks/permission-groups' import { RESOURCE_TYPE_OPTIONS } from '@/ee/audit-logs/constants' import { type AuditLogFilters, useAuditLogs } from '@/ee/audit-logs/hooks/audit-logs' import { @@ -150,12 +151,15 @@ function renderMetadataValue(value: unknown) { ) } +/** Already rendered as their own labelled rows, so the metadata block would repeat them. */ +const HIDDEN_METADATA_KEYS = new Set(['name', 'description']) + function getMetadataEntries(metadata: unknown) { if (!isRecordLike(metadata)) return [] return Object.entries(metadata).filter(([key, value]) => { if (value === undefined) return false - return !['name', 'description'].includes(key) + return !HIDDEN_METADATA_KEYS.has(key) }) } @@ -237,6 +241,24 @@ interface AuditLogsProps { organizationId: string } +/** + * Entries the feed is allowed to present. + * + * A disabled query still serves whatever is cached under its key, and an unresolved + * workspace scope resolves to the same key as the unscoped feed — so an admin looking + * at the organization-wide feed who then followed a stale scoped link kept those rows + * on screen, with Export still armed against them. The scope a link asks for is a + * ceiling, so when it cannot be honoured the feed presents nothing rather than + * whatever it happens to be holding. + */ +export function presentableAuditEntries( + pages: AuditLogPage[] | undefined, + isScopeAnswerable: boolean +): EnterpriseAuditLogEntry[] { + if (!isScopeAnswerable || !pages) return [] + return pages.flatMap((page) => page.data) +} + export function AuditLogs({ organizationId }: AuditLogsProps) { const [urlFilters, setUrlFilters] = useQueryStates(auditLogFilterParsers, auditLogFilterUrlKeys) const { types: selectedTypes } = urlFilters @@ -251,30 +273,83 @@ export function AuditLogs({ organizationId }: AuditLogsProps) { urlFilters.timeRange === 'Custom range' && (!customStartDate || !customEndDate) ? DEFAULT_AUDIT_TIME_RANGE : urlFilters.timeRange + /** + * Resolved, not merely present. Only the id lives in the URL, and the filter is + * applied once it matches a workspace the organization actually owns — a stale id + * from an old link would otherwise be shown under a chip labelled with a bare uuid. + */ + const workspaceScope = urlFilters.workspace + const orgWorkspaces = useOrganizationWorkspaces(organizationId, Boolean(workspaceScope)) + const scopedWorkspace = workspaceScope + ? orgWorkspaces.data?.find((entry) => entry.id === workspaceScope) + : undefined + const [datePickerOpen, setDatePickerOpen] = useState(false) const dateRangeAppliedRef = useRef(false) const [searchTerm, setSearchTerm] = useSettingsSearch() const debouncedSearch = useDebounce(searchTerm, SEARCH_DEBOUNCE_MS).trim() const [isVisuallyRefreshing, setIsVisuallyRefreshing] = useState(false) - const refreshTimersRef = useRef(new Set()) + const refreshTimersRef = useRef | null>(null) + refreshTimersRef.current ??= new Set() + const refreshTimers = refreshTimersRef.current const [isExporting, setIsExporting] = useState(false) useEffect(() => { - const timers = refreshTimersRef.current return () => { - for (const timerId of timers) window.clearTimeout(timerId) + for (const timerId of refreshTimers) window.clearTimeout(timerId) } - }, []) - - const filters = useMemo(() => { - return { - search: debouncedSearch || undefined, - resourceType: selectedTypes.length > 0 ? selectedTypes.join(',') : undefined, - startDate: getStartDateFromTimeRange(timeRange, customStartDate)?.toISOString(), - endDate: getEndDateFromTimeRange(timeRange, customEndDate)?.toISOString(), - } - }, [debouncedSearch, selectedTypes, timeRange, customStartDate, customEndDate]) + }, [refreshTimers]) + + /* + Not memoized: this object is only ever hashed, never compared by identity — React + Query hashes a query key structurally, and the export handler reads its fields + directly. The same rule `useUsageWindow` applies to its window object. + */ + const filters: AuditLogFilters = { + search: debouncedSearch || undefined, + resourceType: selectedTypes.length > 0 ? selectedTypes.join(',') : undefined, + workspaceId: scopedWorkspace?.id, + startDate: getStartDateFromTimeRange(timeRange, customStartDate)?.toISOString(), + endDate: getEndDateFromTimeRange(timeRange, customEndDate)?.toISOString(), + } + + /** + * A deep-linked workspace scope is only resolvable once the organization's workspace + * list has loaded. Querying before then fetches the whole organization's feed and + * immediately refetches it narrowed — two requests, with a flash of rows the link + * did not ask for in between. + */ + const isWorkspaceScopePending = Boolean(workspaceScope) && orgWorkspaces.isPending + /** + * The lookup itself failed, so whether the workspace exists is simply unknown. + * + * Kept apart from {@link isWorkspaceScopeUnresolved}: telling an admin their + * workspace is not part of the organization because a request timed out is a wrong + * answer, not a cautious one, and it offers nothing to do about it. Refresh retries + * this lookup alongside the feed. + */ + const isWorkspaceScopeUnavailable = Boolean(workspaceScope) && orgWorkspaces.isError + + /** + * The link named a workspace this organization does not have — deleted since, or + * never one of ours. + * + * The feed stays closed rather than falling back to the organization. Every other + * deep-linked id in the app degrades to the unfiltered view, but an audit feed is + * the one place where widening is the dangerous direction: dropping the filter + * would answer a request for one workspace's history with everybody's, under a URL + * that still claims to be scoped, and the CSV export would follow. + */ + const isWorkspaceScopeUnresolved = + Boolean(workspaceScope) && + !isWorkspaceScopePending && + !isWorkspaceScopeUnavailable && + !scopedWorkspace + + /** The feed can answer the scope the URL asks for — the gate on reading or exporting. */ + const isScopeAnswerable = + !isWorkspaceScopePending && !isWorkspaceScopeUnresolved && !isWorkspaceScopeUnavailable const { data, isLoading, @@ -283,12 +358,12 @@ export function AuditLogs({ organizationId }: AuditLogsProps) { hasNextPage, fetchNextPage, refetch, - } = useAuditLogs(organizationId, filters) + } = useAuditLogs(organizationId, filters, !isWorkspaceScopePending && !isWorkspaceScopeUnresolved) - const allEntries = useMemo(() => { - if (!data?.pages) return [] - return data.pages.flatMap((page) => page.data) - }, [data]) + const allEntries = useMemo( + () => presentableAuditEntries(data?.pages, isScopeAnswerable), + [data, isScopeAnswerable] + ) const typeDisplayLabel = selectedTypes.length === 0 @@ -324,25 +399,38 @@ export function AuditLogs({ organizationId }: AuditLogsProps) { setDatePickerOpen(false) } - const handleRefresh = useCallback(() => { + const handleRefresh = () => { setIsVisuallyRefreshing(true) const timerId = window.setTimeout(() => { setIsVisuallyRefreshing(false) - refreshTimersRef.current.delete(timerId) + refreshTimers.delete(timerId) }, REFRESH_SPINNER_DURATION_MS) - refreshTimersRef.current.add(timerId) - refetch().catch((error: unknown) => { + refreshTimers.add(timerId) + const pending: Promise[] = [] + /* + `refetch` ignores `enabled`, so this has to repeat the gate. While the scope is + unanswerable the feed's filter carries no workspace, and refreshing it would + issue exactly the organization-wide read the gate exists to prevent. + */ + if (isScopeAnswerable) pending.push(refetch()) + /* + The lookup is what has to succeed for a closed feed to reopen, so it is retried + whenever a scope asked for it — and skipped entirely when none did, where it is + a disabled query with nothing to say. + */ + if (workspaceScope) pending.push(orgWorkspaces.refetch()) + Promise.all(pending).catch((error: unknown) => { logger.error('Failed to refresh audit logs', { error }) }) - }, [refetch]) + } - const handleLoadMore = useCallback(() => { + const handleLoadMore = () => { if (hasNextPage && !isFetchingNextPage) { fetchNextPage().catch((error: unknown) => { logger.error('Failed to load more audit logs', { error }) }) } - }, [hasNextPage, isFetchingNextPage, fetchNextPage]) + } const handleExportCsv = async () => { setIsExporting(true) @@ -351,6 +439,7 @@ export function AuditLogs({ organizationId }: AuditLogsProps) { params.set('organizationId', organizationId) if (filters.search) params.set('search', filters.search) if (filters.resourceType) params.set('resourceType', filters.resourceType) + if (filters.workspaceId) params.set('workspaceId', filters.workspaceId) if (filters.startDate) params.set('startDate', filters.startDate) if (filters.endDate) params.set('endDate', filters.endDate) @@ -385,7 +474,13 @@ export function AuditLogs({ organizationId }: AuditLogsProps) { text: 'Export', icon: Download, onSelect: () => void handleExportCsv(), - disabled: allEntries.length === 0 || isExporting || isPlaceholderData, + /* + `isScopeAnswerable` explicitly, not just via the empty `allEntries` it + implies: the export is the action that leaves the building, so the + condition that makes it safe belongs where it is read. + */ + disabled: + !isScopeAnswerable || allEntries.length === 0 || isExporting || isPlaceholderData, }, ]} > @@ -410,6 +505,28 @@ export function AuditLogs({ organizationId }: AuditLogsProps) { allOptionLabel='All types' align='start' /> + {workspaceScope && ( + /* + A deep-linked scope, not a picker: the organization can hold hundreds of + workspaces, so this narrows the feed only when a link asks it to and + offers exactly one action — take it back off. Trailing `X` and a bounded + width, matching the app's other removable filter chips; the label names + the dimension because a bare workspace name gives no clue what it scopes. + */ + void setUrlFilters({ workspace: null })} + aria-label='Clear the workspace filter' + className='max-w-[280px] shrink-0' + > + {/* Rendered for an unresolved scope too, or a bad link would leave the + feed closed with no control to reopen it. */} + + + )}
{/* ChipCombobox (Radix Popover, non-modal), not ChipSelect (Radix DropdownMenu, modal by default) — a modal trigger closing in the @@ -469,7 +586,15 @@ export function AuditLogs({ organizationId }: AuditLogsProps) { + Couldn't check that workspace. Refresh to try again. + + ) : isWorkspaceScopeUnresolved ? ( + + That workspace is not part of this organization. + + ) : debouncedSearch ? ( No results for "{debouncedSearch}" diff --git a/apps/sim/ee/audit-logs/hooks/audit-logs.test.tsx b/apps/sim/ee/audit-logs/hooks/audit-logs.test.tsx index 9fd71da3ada..70d1a2fea8f 100644 --- a/apps/sim/ee/audit-logs/hooks/audit-logs.test.tsx +++ b/apps/sim/ee/audit-logs/hooks/audit-logs.test.tsx @@ -50,8 +50,16 @@ let container: HTMLDivElement let root: Root let queryClient: QueryClient -function AuditProbe({ organizationId }: { organizationId: string }) { - const auditLogs = useAuditLogs(organizationId, {}) +function AuditProbe({ + organizationId, + workspaceId, + search, +}: { + organizationId: string + workspaceId?: string + search?: string +}) { + const auditLogs = useAuditLogs(organizationId, { workspaceId, search }) const entries = auditLogs.data?.pages.flatMap((page) => page.data) ?? [] return ( @@ -62,11 +70,20 @@ function AuditProbe({ organizationId }: { organizationId: string }) { ) } -function renderAuditLogs(organizationId: string) { +interface RenderOptions { + workspaceId?: string + search?: string +} + +function renderAuditLogs(organizationId: string, options: RenderOptions = {}) { act(() => { root.render( - + ) }) @@ -130,4 +147,50 @@ describe('useAuditLogs identity transitions', () => { }) ) }) + + /** Blanking the feed on each keystroke is what the placeholder exists to stop. */ + it('holds the current entries while a filter change loads, within one scope', async () => { + const filteredPage = createDeferred() + mockRequestJson.mockImplementation( + (contract: unknown, input: { query?: { search?: string } }) => { + if (contract !== listAuditLogsContract) throw new Error('Unexpected contract') + return input.query?.search ? filteredPage.promise : Promise.resolve(AUDIT_PAGE_A) + } + ) + + renderAuditLogs('org-a') + await flushQueries() + expect(container).toHaveTextContent('Updated Organization A') + + renderAuditLogs('org-a', { search: 'canary' }) + await flushQueries() + + expect(container).toHaveTextContent('Updated Organization A') + }) + + /** + * The other side of that rule. A workspace is a scope, not a filter: holding the + * organization-wide rows while the scoped page loads would show, under a + * workspace-scoped URL, entries that scope does not cover — with Export armed + * against them, since it gates on this list being non-empty. + */ + it('clears the entries when the workspace scope changes, within one organization', async () => { + const scopedPage = createDeferred() + mockRequestJson.mockImplementation( + (contract: unknown, input: { query?: { workspaceId?: string } }) => { + if (contract !== listAuditLogsContract) throw new Error('Unexpected contract') + return input.query?.workspaceId ? scopedPage.promise : Promise.resolve(AUDIT_PAGE_A) + } + ) + + renderAuditLogs('org-a') + await flushQueries() + expect(container).toHaveTextContent('Updated Organization A') + + renderAuditLogs('org-a', { workspaceId: 'workspace-a' }) + await flushQueries() + + expect(container).not.toHaveTextContent('Updated Organization A') + expect(container.querySelector('button')).toBeNull() + }) }) diff --git a/apps/sim/ee/audit-logs/hooks/audit-logs.ts b/apps/sim/ee/audit-logs/hooks/audit-logs.ts index 7685f83b3ff..0880b13c064 100644 --- a/apps/sim/ee/audit-logs/hooks/audit-logs.ts +++ b/apps/sim/ee/audit-logs/hooks/audit-logs.ts @@ -1,4 +1,4 @@ -import { useInfiniteQuery } from '@tanstack/react-query' +import { hashKey, useInfiniteQuery } from '@tanstack/react-query' import { requestJson } from '@/lib/api/client/request' import { type AuditLogPage, listAuditLogsContract } from '@/lib/api/contracts/audit-logs' @@ -7,8 +7,22 @@ export const AUDIT_LOG_LIST_STALE_TIME = 30 * 1000 export const auditLogKeys = { all: ['audit-logs'] as const, lists: () => [...auditLogKeys.all, 'list'] as const, + /** + * What a key is allowed to see: the organization, and the workspace within it. + * + * It leads the key, ahead of the filters, because previous data may be held across + * a filter change but never across a scope change — and a leading scope makes that + * a prefix comparison rather than a reach inside the filter object. + */ + scope: (organizationId: string, workspaceId?: string) => + [...auditLogKeys.lists(), organizationId, workspaceId ?? ''] as const, list: (organizationId: string, filters: AuditLogFilters) => - [...auditLogKeys.lists(), organizationId, filters] as const, + [...auditLogKeys.scope(organizationId, filters.workspaceId), filters] as const, +} + +/** The scope a key reads from, which is everything but its trailing filter object. */ +function auditListScopeIdentity(key: readonly unknown[]): string { + return hashKey(key.slice(0, -1)) } export interface AuditLogFilters { @@ -16,6 +30,8 @@ export interface AuditLogFilters { action?: string resourceType?: string actorId?: string + /** Narrows the feed to one workspace in the organization. */ + workspaceId?: string startDate?: string endDate?: string } @@ -34,6 +50,7 @@ async function fetchAuditLogs( action: filters.action, resourceType: filters.resourceType, actorId: filters.actorId, + workspaceId: filters.workspaceId, startDate: filters.startDate, endDate: filters.endDate, cursor, @@ -43,12 +60,29 @@ async function fetchAuditLogs( } export function useAuditLogs(organizationId: string, filters: AuditLogFilters, enabled = true) { + const queryKey = auditLogKeys.list(organizationId, filters) return useInfiniteQuery({ - queryKey: auditLogKeys.list(organizationId, filters), + queryKey, queryFn: ({ pageParam, signal }) => fetchAuditLogs(organizationId, filters, pageParam, signal), initialPageParam: undefined as string | undefined, getNextPageParam: (lastPage) => lastPage.nextCursor, enabled: Boolean(organizationId) && enabled, staleTime: AUDIT_LOG_LIST_STALE_TIME, + /** + * Held across a filter change, never across a scope change. + * + * Search, types and the window are all part of the key, so without a placeholder + * the feed blanks to its empty state on each keystroke and the Export action's + * `isPlaceholderData` guard is dead. But the organization and the workspace are in + * the key too, and holding across either shows rows the current scope does not + * cover — one tenant's entries under another's heading, or the organization's + * under a workspace-scoped URL — with Export armed against them. + */ + placeholderData: (previous, previousQuery) => + previous && + previousQuery && + auditListScopeIdentity(previousQuery.queryKey) === auditListScopeIdentity(queryKey) + ? previous + : undefined, }) } diff --git a/apps/sim/ee/audit-logs/search-params.ts b/apps/sim/ee/audit-logs/search-params.ts index 1c4a5284a9e..b28f253b942 100644 --- a/apps/sim/ee/audit-logs/search-params.ts +++ b/apps/sim/ee/audit-logs/search-params.ts @@ -1,4 +1,4 @@ -import { parseAsArrayOf, parseAsString } from 'nuqs/server' +import { createSerializer, parseAsArrayOf, parseAsString } from 'nuqs/server' import { parseAsDateString, parseAsTimeRange, @@ -20,6 +20,13 @@ export const DEFAULT_AUDIT_TIME_RANGE: TimeRange = 'Past 30 days' */ export const auditLogFilterParsers = { types: parseAsArrayOf(parseAsString).withDefault([]), + /** + * Nullable by design: the feed is organization-wide unless a link narrows it, and + * the usage panel's workspace drill-down is what does. Only the id is stored — the + * name is resolved from the loaded workspace list, so a stale id from an old link + * clears the filter rather than labelling it with nothing. + */ + workspace: parseAsString, timeRange: parseAsTimeRange.withDefault(DEFAULT_AUDIT_TIME_RANGE), startDate: parseAsDateString, endDate: parseAsDateString, @@ -36,3 +43,13 @@ export const auditLogFilterUrlKeys = { endDate: 'end-date', }, } as const + +/** + * Outbound links into the audit feed — the usage panel's workspace drill-down builds + * one — serialized from the map the feed itself parses rather than by concatenation, + * which emitted a bare `?workspace=` for a null id and left the value unencoded. + */ +export const serializeAuditLogFilters = createSerializer(auditLogFilterParsers, { + clearOnDefault: true, + urlKeys: auditLogFilterUrlKeys.urlKeys, +}) diff --git a/apps/sim/ee/organization-usage/components/usage-consumers.test.ts b/apps/sim/ee/organization-usage/components/usage-consumers.test.ts new file mode 100644 index 00000000000..510d065a2a2 --- /dev/null +++ b/apps/sim/ee/organization-usage/components/usage-consumers.test.ts @@ -0,0 +1,15 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { USAGE_PROVIDER_ICON_IDS } from '@/ee/organization-usage/components/usage-consumers' +import { PROVIDER_DEFINITIONS } from '@/providers/models' + +describe('PROVIDER_ICONS', () => { + /** A gap is silent: the row simply renders with no mark. */ + it('covers every provider the model registry defines', () => { + const covered = new Set(USAGE_PROVIDER_ICON_IDS) + const missing = Object.keys(PROVIDER_DEFINITIONS).filter((id) => !covered.has(id)) + expect(missing).toEqual([]) + }) +}) diff --git a/apps/sim/ee/organization-usage/components/usage-consumers.tsx b/apps/sim/ee/organization-usage/components/usage-consumers.tsx index e20de0359ac..610056f46cd 100644 --- a/apps/sim/ee/organization-usage/components/usage-consumers.tsx +++ b/apps/sim/ee/organization-usage/components/usage-consumers.tsx @@ -1,21 +1,33 @@ 'use client' import type { ComponentType } from 'react' -import { cn } from '@sim/emcn' -import { ChevronRight } from '@sim/emcn/icons' +import { cn, disclosureChevronClass } from '@sim/emcn' +import { ArrowRight, ChevronDown } from '@sim/emcn/icons' import { formatChartCompactNumber } from '@/components/charts' import { AnthropicIcon, AzureIcon, + BasetenIcon, + BedrockIcon, CerebrasIcon, DeepseekIcon, - GoogleIcon, + FireworksIcon, + GeminiIcon, GroqIcon, + KimiIcon, + LitellmIcon, + MetaIcon, MistralIcon, + NvidiaIcon, OllamaIcon, OpenAIIcon, OpenRouterIcon, + SakanaIcon, + TogetherIcon, + VertexIcon, + VllmIcon, xAIIcon, + ZaiIcon, } from '@/components/icons' import type { OrganizationUsageBreakdown, @@ -27,34 +39,65 @@ import { RowActionsMenu, } from '@/app/workspace/[workspaceId]/settings/components/row-actions-menu' import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state' +import { RESOURCE_ROW_ARROW_CLASSES } from '@/app/workspace/[workspaceId]/settings/components/settings-resource-row' import { USAGE_TAB_EMPTY_COPY } from '@/ee/organization-usage/constants' /** * Provider brand marks, keyed by the `providerId` the server resolves. * - * Kept here rather than read from `providers/models.ts`: that module carries the + * Kept here rather than read from `PROVIDER_DEFINITIONS`: that module carries the * whole model registry and would land in this settings chunk for two dozen glyphs. + * The icons themselves come from the same `@/components/icons` module the registry + * imports, so this is a re-keying, never a second set of artwork. + * + * It must list every provider the registry defines, or a model resolving to a + * missing one renders an unexplained blank where every neighbouring row has a mark + * — which is how `zai` (GLM) shipped iconless. `usage-consumers.test.ts` fails when + * the two drift, so the coverage is checked rather than remembered. */ const PROVIDER_ICONS: Readonly>> = { - openai: OpenAIIcon, anthropic: AnthropicIcon, - google: GoogleIcon, - 'azure-openai': AzureIcon, + baseten: BasetenIcon, + bedrock: BedrockIcon, + cerebras: CerebrasIcon, deepseek: DeepseekIcon, - xai: xAIIcon, + fireworks: FireworksIcon, + google: GeminiIcon, groq: GroqIcon, - cerebras: CerebrasIcon, + kimi: KimiIcon, + litellm: LitellmIcon, + meta: MetaIcon, + mistral: MistralIcon, + nvidia: NvidiaIcon, ollama: OllamaIcon, + 'ollama-cloud': OllamaIcon, + openai: OpenAIIcon, openrouter: OpenRouterIcon, - mistral: MistralIcon, + sakana: SakanaIcon, + together: TogetherIcon, + vertex: VertexIcon, + vllm: VllmIcon, + xai: xAIIcon, + zai: ZaiIcon, + 'azure-anthropic': AzureIcon, + /** Not a registry provider — a BYOK credential kind the breakdown can also emit. */ + 'azure-openai': AzureIcon, } +export const USAGE_PROVIDER_ICON_IDS = Object.keys(PROVIDER_ICONS) + interface UsageConsumerRowProps { row: OrganizationUsageBreakdownRow /** BYOK rows carry no cost, so tokens are the only usage they can show. */ showTokensOnly: boolean onSelect?: (row: OrganizationUsageBreakdownRow) => void actions?: RowAction[] + /** + * Width of the affordance some other row in this list carries, reserved here so + * every figure stays in one column — including when the only row that carries one + * is `Other`. + */ + reservedTrailing?: string } /** @@ -62,17 +105,31 @@ interface UsageConsumerRowProps { * same slot on its `Other` row and keep every figure in one column. */ const TRAILING_SLOT_CLASSES = { - /** `ChevronRight` at the platform icon size. */ - chevron: 'size-[14px]', + arrow: 'size-4', /** `RowActionsMenu`'s trigger: a 14px glyph in a `chipVariants()` pill. */ menu: 'size-[30px]', + /** The disclosure chevron on an expandable `Other` row, at the default icon size. */ + disclosure: 'size-[14px]', } as const +/** + * Geometry of the bespoke tabular usage row — the sanctioned exception to + * `SettingsResourceRow` in `sim-settings-pages.md`. One definition, so the breakdown + * rows, the `Other` row, and the events ledger cannot drift apart. + */ +export const USAGE_ROW_CLASSES = 'flex w-full items-center gap-2.5 rounded-lg p-2 text-left' + /** * A tabular row, not `SettingsResourceRow` — tabular columns are the sanctioned * exception in `sim-settings-pages.md`, alongside billing invoices and credit usage. */ -function UsageConsumerRow({ row, showTokensOnly, onSelect, actions }: UsageConsumerRowProps) { +function UsageConsumerRow({ + row, + showTokensOnly, + onSelect, + actions, + reservedTrailing, +}: UsageConsumerRowProps) { const ProviderIcon = row.providerId ? PROVIDER_ICONS[row.providerId] : undefined const Row = onSelect ? 'button' : 'div' @@ -86,8 +143,8 @@ function UsageConsumerRow({ row, showTokensOnly, onSelect, actions }: UsageConsu } : {})} className={cn( - 'flex w-full items-center gap-2.5 rounded-lg p-2 text-left', - onSelect && 'transition-colors hover:bg-[var(--surface-active)]' + USAGE_ROW_CLASSES, + onSelect && 'transition-colors hover-hover:bg-[var(--surface-active)]' )} > {ProviderIcon && ( @@ -106,13 +163,13 @@ function UsageConsumerRow({ row, showTokensOnly, onSelect, actions }: UsageConsu {showTokensOnly ? formatChartCompactNumber(row.tokens ?? 0) : row.credits.toLocaleString()} - {/* A chevron or a menu, never both — `sim-settings-pages.md`. */} + {/* An arrow or a menu, never both — `sim-settings-pages.md`. */} {onSelect ? ( - + ) : actions?.length ? ( + ) : reservedTrailing ? ( +