Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions apps/api/src/views/views.service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,36 @@ describe('cleanViewConfig — dashboard metric tiles (MN-225 / #168)', () => {
expect(result).toHaveLength(1);
});

// #304: a tile carries its OWN filter so each tile can measure a different slice.
// It must survive the read path (the zod config would strip an unknown key) and get
// the same dead-field pruning the view's own filter gets.
it('keeps a tile filter, and prunes only its dead conditions (#304)', () => {
const result = tiles([
{
id: '66666666-6666-6666-6666-666666666666',
label: '',
op: 'count',
filter: { and: [{ field: 'amount', op: 'gt', value: 5 }] },
} as never,
]);
expect(result).toHaveLength(1);
expect(result![0]!.filter).toEqual({ and: [{ field: 'amount', op: 'gt', value: 5 }] });
});

it('drops a tile-filter condition on a deleted field but keeps the tile (#304)', () => {
const result = tiles([
{
id: '77777777-7777-7777-7777-777777777777',
label: '',
op: 'count',
filter: { and: [{ field: 'ghost', op: 'eq', value: 1 }] },
} as never,
]);
// The tile is still configured — only the dead condition goes (#305's rule).
expect(result).toHaveLength(1);
expect(result![0]!.filter).toBeUndefined();
});

it('defaults to an empty array when tiles are absent', () => {
expect(tiles(undefined as unknown as ViewConfig['dashboard_tiles'])).toEqual([]);
});
Expand Down
14 changes: 11 additions & 3 deletions apps/api/src/views/views.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,17 @@ export function cleanViewConfig(
// field) produced `{op:'sum'}` with no field, and the tile was silently
// garbage-collected on the very next read — the card "deleted itself".
// "Unconfigured" and "dangling" are different states; only the latter is junk.
dashboard_tiles: (config.dashboard_tiles ?? []).filter(
(t) => t.field_api_name == null || liveApiNames.has(t.field_api_name),
),
dashboard_tiles: (config.dashboard_tiles ?? [])
.filter((t) => t.field_api_name == null || liveApiNames.has(t.field_api_name))
// #304: a tile's own filter gets the same pruning the view's filter gets —
// a condition on a deleted field is dropped rather than left to fail at query
// time. The TILE itself survives (it is still configured); only the dead
// condition goes, exactly as cleanFilterNode does for the view.
.map((t) =>
t.filter
? { ...t, filter: cleanFilterNode(t.filter, liveApiNames) as typeof t.filter }
: t,
),
// Dashboard chart/table widgets (MN-225 / #168, Phase 2): same rule.
// #305: requiring a live `group_by_field_api_name` meant a freshly added
// chart (created with no group-by, by design — you pick it afterwards) was
Expand Down
76 changes: 71 additions & 5 deletions apps/web/src/components/views/dashboard-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

import { useEffect, useMemo } from 'react';
import { Plus, Trash2 } from 'lucide-react';
import { useDatabase, useRecordsInfinite } from '../table-view/use-table-data';
import type { FilterNode, ViewConfig } from './use-view-state';
import { queryBodyFromConfig } from './use-view-state';
import { useDatabase, useMembers, useRecordsInfinite } from '../table-view/use-table-data';
import type { FilterNode, ViewConfig, FilterGroup } from './use-view-state';
import { andFilterNodes, queryBodyFromConfig } from './use-view-state';
import { FiltersSection } from './view-toolbar';
import {
TILE_OPS,
computeTileValue,
Expand All @@ -23,6 +24,8 @@ export interface DashboardTile {
label: string;
op: TileOp;
field_api_name?: string;
/** #304 — this tile's own scope, ANDed with the view's filter. */
filter?: FilterNode;
}

const SELECT_CLASS =
Expand Down Expand Up @@ -57,6 +60,12 @@ export function DashboardView({
onPatch: (updates: Partial<ViewConfig>) => void;
}) {
const database = useDatabase(ws, db);
// #304 — the tile filter builder needs the roster for person-field conditions.
const members = useMembers(ws, !readOnly);
const memberList = useMemo(
() => (members.data ?? []).map((m) => ({ id: m.user.id, name: m.user.name })),
[members.data],
);
const queryBody = useMemo(() => queryBodyFromConfig(config, personalFilter), [config, personalFilter]);
const records = useRecordsInfinite(ws, db, queryBody);

Expand Down Expand Up @@ -125,7 +134,6 @@ export function DashboardView({

<div className="grid gap-3" style={{ gridTemplateColumns: 'repeat(auto-fill, minmax(220px, 1fr))' }}>
{tiles.map((tile) => {
const value = loading ? null : computeTileValue(tile.op, tile.field_api_name, rows);
const heading = tile.label.trim() || defaultTileLabel(tile.op, fieldName.get(tile.field_api_name ?? ''));
return (
<div
Expand All @@ -146,7 +154,7 @@ export function DashboardView({
)}
</div>
<span className="text-3xl font-semibold tabular-nums text-ink">
{loading ? <span className="text-muted">…</span> : formatTileValue(value)}
<TileValue ws={ws} db={db} config={config} personalFilter={personalFilter} tile={tile} />
</span>

{!readOnly && (
Expand All @@ -158,6 +166,18 @@ export function DashboardView({
onChange={(e) => updateTile(tile.id, { label: e.target.value })}
className="h-8 rounded-[var(--radius-control)] border border-border-default bg-card px-2 text-[13px] text-ink placeholder:text-faint"
/>
{/* #304 — this tile's own scope. The SAME builder the view toolbar
uses (one filter spec, one UI), so a tile can measure a slice
instead of every tile repeating the view's total. No viewId is
passed: Personal scope is a per-VIEW override, not a per-tile one. */}
<FiltersSection
ws={ws}
db={db}
fields={fields}
members={memberList}
filters={tile.filter as FilterGroup | undefined}
onChange={(filter) => updateTile(tile.id, { filter: filter as FilterNode | undefined })}
/>
<div className="flex gap-1.5">
<select
aria-label="Aggregation"
Expand Down Expand Up @@ -245,3 +265,49 @@ export function DashboardView({
</div>
);
}

/**
* #304 — one tile's number, fetched with THAT tile's own scope.
*
* A tile is its own component precisely so it can own a query: the tile filter is
* ANDed onto the view's (and the viewer's personal) filter and sent to the SAME
* grant-scoped /records/query path every other view uses. That reuses the server's
* filter semantics instead of re-implementing operator behaviour client-side, and
* it keeps a tile from ever reading past the viewer's access.
*
* Two tiles with identical scope share one request — react-query dedupes on the
* query key, so N tiles do not mean N round trips.
*/
function TileValue({
ws,
db,
config,
personalFilter,
tile,
}: {
ws: string;
db: string;
config: ViewConfig;
personalFilter?: FilterNode;
tile: DashboardTile;
}) {
const scoped = useMemo(
() => andFilterNodes(personalFilter, tile.filter),
[personalFilter, tile.filter],
);
const queryBody = useMemo(
() => queryBodyFromConfig(config, scoped as FilterNode | undefined),
[config, scoped],
);
const records = useRecordsInfinite(ws, db, queryBody);
const { hasNextPage, isFetchingNextPage, fetchNextPage } = records;
// Aggregate over the whole matching set, not just page 1.
useEffect(() => {
if (hasNextPage && !isFetchingNextPage) void fetchNextPage();
}, [hasNextPage, isFetchingNextPage, fetchNextPage]);

const rows = useMemo(() => (records.data?.pages ?? []).flatMap((p) => p.data), [records.data]);
const loading = records.isLoading || hasNextPage || isFetchingNextPage;
if (loading) return <span className="text-muted">…</span>;
return <>{formatTileValue(computeTileValue(tile.op, tile.field_api_name, rows))}</>;
}
6 changes: 6 additions & 0 deletions docs/api/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -9252,6 +9252,9 @@
"type": "string",
"minLength": 1,
"maxLength": 100
},
"filter": {
"$ref": "#/components/schemas/CreateViewDto__schema0"
}
},
"required": [
Expand Down Expand Up @@ -9612,6 +9615,9 @@
"type": "string",
"minLength": 1,
"maxLength": 100
},
"filter": {
"$ref": "#/components/schemas/UpdateViewDto__schema0"
}
},
"required": [
Expand Down
7 changes: 7 additions & 0 deletions packages/schemas/src/views.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ export const dashboardTileSchema = z.object({
op: z.enum(['count', 'sum', 'avg', 'min', 'max']),
/** Required for sum/avg/min/max; omitted (ignored) for count. */
field_api_name: z.string().trim().min(1).max(100).optional(),
/**
* #304 — this tile's OWN scope, ANDed with the view's filter. Without it every
* tile on a dashboard necessarily shows the same number, which is what made the
* feature useless ("I can't select what to show. Except for how to aggregate").
* Same filter AST as views / /records/query / rollups — never a second language.
*/
filter: filterSchema.optional(),
});
export type DashboardTile = z.infer<typeof dashboardTileSchema>;

Expand Down
2 changes: 2 additions & 0 deletions packages/sdk/src/generated/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3839,6 +3839,7 @@ export interface components {
/** @enum {string} */
op: "count" | "sum" | "avg" | "min" | "max";
field_api_name?: string;
filter?: components["schemas"]["CreateViewDto__schema0"];
}[];
/** @default [] */
dashboard_widgets: {
Expand Down Expand Up @@ -3941,6 +3942,7 @@ export interface components {
/** @enum {string} */
op: "count" | "sum" | "avg" | "min" | "max";
field_api_name?: string;
filter?: components["schemas"]["UpdateViewDto__schema0"];
}[];
/** @default [] */
dashboard_widgets: {
Expand Down
Loading