diff --git a/docs/feature-telemetry.md b/docs/feature-telemetry.md index f5ce2357ca..469de3ffbc 100644 --- a/docs/feature-telemetry.md +++ b/docs/feature-telemetry.md @@ -4,10 +4,10 @@ anonymous usage-stats payload. It answers "does anyone use this, and which variant do they pick" without any identifying data leaving the instance. -It currently carries 42 registered actions across seventeen features (`ai_session`, `ai_chat`, -`ai_fix`, `ai_agent`, `ai_agent_eval`, `app_sandbox`, `datatable`, `flow_editor`, `flow_run`, -`flow_step`, `run_form`, `debugger`, `trigger`, `command_script`, `hub_script`, `usage_meter`, -`sso_groups_claim`). Nearly all of the +It currently carries 43 registered actions across eighteen features (`ai_session`, `ai_chat`, +`ai_fix`, `ai_agent`, `ai_agent_eval`, `app_sandbox`, `datatable`, `db_manager`, `flow_editor`, +`flow_run`, `flow_step`, `run_form`, `debugger`, `trigger`, `command_script`, `hub_script`, +`usage_meter`, `sso_groups_claim`). Nearly all of the product is uninstrumented, so new user-facing work is the opportunity to change that. ## When to instrument diff --git a/frontend/src/lib/components/DBManager.svelte b/frontend/src/lib/components/DBManager.svelte index 987b19963b..5b4e6ed42e 100644 --- a/frontend/src/lib/components/DBManager.svelte +++ b/frontend/src/lib/components/DBManager.svelte @@ -100,13 +100,18 @@ onImport }: Props = $props() - let viewMode = $state('data') + let requestedViewMode = $state('data') // PostgreSQL is the only database whose foreign keys can be read for the whole // database in one query; the others would need one job per table. A caller // already using the sidebar checkboxes to collect tables keeps them. let supportsDiagram = $derived(dbType === 'postgresql' && !multiSelectMode) + // The manager is not remounted when the drawer switches database, so a switch + // away from PostgreSQL would otherwise leave the diagram on screen with no + // toggle left to leave it by. + let viewMode = $derived(supportsDiagram ? requestedViewMode : 'data') + // The tables drawn on the diagram. Kept apart from `selectedTables` so that // checking a table to see it in the diagram can never add it to whatever the // caller's multi-select is collecting. @@ -266,6 +271,13 @@ selected = { schemaKey, tableKey: table } } + /** Selecting a table fetches its foreign keys for the data view. The diagram + * shows no preview and reads every relation from one query of its own, so a + * checkbox there must not queue that per-table job. */ + function previewTableFromSidebar(schemaKey: string, table: string) { + if (viewMode !== 'diagram') selectTable(schemaKey, table) + } + /** Where a foreign key's `schema.table` target lives in the sidebar, or * undefined when it cannot be opened from here. */ function resolveForeignKeyTarget( @@ -369,15 +381,19 @@ // Fetched once for the whole database rather than per table: the diagram needs // every relation at once, and the per-table query would be one job each. let relationsError = $state(undefined) + // The keys are only re-read when the schema itself was reloaded, which is what + // a new `colDefs` identity means. Toggling back to the diagram must not queue + // the query again. + let relationsFetchedFor: Record | undefined let relations = resource( [() => viewMode, () => colDefs], - async ([mode]): Promise => { - // Keeps what was fetched when leaving the diagram, so coming back to it - // doesn't queue the query again. - if (mode !== 'diagram') return relations.current ?? [] + async ([mode, defs], _prev, { data }): Promise => { + if (mode !== 'diagram' || (data && relationsFetchedFor === defs)) return data ?? [] relationsError = undefined try { - return await dbSchemaOps.onFetchAllForeignKeys() + const fetched = await dbSchemaOps.onFetchAllForeignKeys() + relationsFetchedFor = defs + return fetched } catch (e) { relationsError = (e as any)?.body ?? (e as Error)?.message ?? String(e) return [] @@ -387,15 +403,24 @@ // Opening the diagram on an empty canvas would make it look broken, so the // current schema is drawn to start with — unless it is big enough that drawing - // all of it is a choice the user should make. + // all of it is a choice the user should make. Once per entry into the mode: + // re-running it would refill a selection the user has just emptied. const DIAGRAM_AUTOSELECT_LIMIT = 40 + let autoSelected = false $effect(() => { - if (viewMode !== 'diagram' || diagramTables.length) return - const schemaKey = untrack(() => selected.schemaKey) - if (!schemaKey) return - const tables = Object.keys(dbSchema.schema[schemaKey] ?? {}) - if (tables.length > DIAGRAM_AUTOSELECT_LIMIT) return - diagramTables = tables.map((table) => ({ schema: schemaKey, table })) + if (viewMode !== 'diagram') { + autoSelected = false + return + } + if (autoSelected) return + autoSelected = true + untrack(() => { + const schemaKey = selected.schemaKey + if (!schemaKey || diagramTables.length) return + const tables = Object.keys(dbSchema.schema[schemaKey] ?? {}) + if (!tables.length || tables.length > DIAGRAM_AUTOSELECT_LIMIT) return + diagramTables = tables.map((table) => ({ schema: schemaKey, table })) + }) }) let _dbTable: DBTable | undefined = $state() @@ -410,7 +435,7 @@ {/if} {#if supportsDiagram} logFeatureUsage('db_manager', 'view_mode', { key: v })} > @@ -553,12 +578,12 @@ role="button" tabindex="0" onclick={() => { - selectTable(schemaKey, tableKey) + previewTableFromSidebar(schemaKey, tableKey) toggleTableSelection(schemaKey, tableKey) }} onkeydown={(e) => { if (e.key === 'Enter' || e.key === ' ') { - selectTable(schemaKey, tableKey) + previewTableFromSidebar(schemaKey, tableKey) toggleTableSelection(schemaKey, tableKey) } }} @@ -705,7 +730,7 @@ loading={relations.loading} error={relationsError} onOpenTable={({ schema, table }) => { - viewMode = 'data' + requestedViewMode = 'data' selectTable(schema, table) }} /> diff --git a/frontend/src/lib/components/dbdiagram/dbDiagramModel.test.ts b/frontend/src/lib/components/dbdiagram/dbDiagramModel.test.ts new file mode 100644 index 0000000000..8b6b584860 --- /dev/null +++ b/frontend/src/lib/components/dbdiagram/dbDiagramModel.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from 'vitest' +import { CARD_WIDTH, cardHeight, layoutTables, type DiagramTable } from './dbDiagramModel' + +function table(name: string, columnCount: number): DiagramTable { + return { + key: `shop.${name}`, + schema: 'shop', + table: name, + columns: Array.from({ length: columnCount }, (_, i) => ({ + name: `c${i}`, + datatype: 'text', + isPrimaryKey: i === 0 + })) + } +} + +describe('layoutTables', () => { + // Runs again on every checkbox click, so cards must not land on top of each + // other for any mix of card heights. + it('never overlaps two cards', () => { + const tables = Array.from({ length: 40 }, (_, i) => table(`t${i}`, 1 + (i % 17))) + const expanded = new Set(['shop.t3', 'shop.t20']) + const positions = layoutTables(tables, expanded) + + const boxes = tables.map((t) => ({ + x: positions[t.key].x, + y: positions[t.key].y, + h: cardHeight(t, expanded.has(t.key)) + })) + for (let i = 0; i < boxes.length; i++) { + for (let j = i + 1; j < boxes.length; j++) { + const a = boxes[i] + const b = boxes[j] + const apart = + a.x + CARD_WIDTH <= b.x || b.x + CARD_WIDTH <= a.x || a.y + a.h <= b.y || b.y + b.h <= a.y + expect(apart, `${tables[i].key} overlaps ${tables[j].key}`).toBe(true) + } + } + }) + + it('places the same tables in the same spots every time', () => { + const tables = Array.from({ length: 12 }, (_, i) => table(`t${i}`, 3 + i)) + expect(layoutTables(tables, new Set())).toEqual(layoutTables(tables, new Set())) + }) +})