From 0ed923e8acc34b864cef534950494b572a047d8d Mon Sep 17 00:00:00 2001 From: Brian Holmes <120223836+briangregoryholmes@users.noreply.github.com> Date: Tue, 15 Oct 2024 23:21:14 -0400 Subject: [PATCH] fix: alternate connector support for visual metrics (#5909) * wip * cleanup * wip * cleanup * feedback --- web-common/src/components/forms/Input.svelte | 35 +- .../src/components/forms/InputLabel.svelte | 40 ++ web-common/src/components/tag/Tag.svelte | 6 +- .../features/connectors/ConnectorEntry.svelte | 34 +- .../connectors/ConnectorExplorer.svelte | 41 +- .../connectors/connector-explorer-store.ts | 290 +++++++----- .../connectors/olap/DatabaseEntry.svelte | 17 +- .../connectors/olap/DatabaseExplorer.svelte | 4 +- .../olap/DatabaseSchemaEntry.svelte | 25 +- .../connectors/olap/TableEntry.svelte | 100 ++-- .../connectors/olap/TableMenuItems.svelte | 3 +- .../AlertConfirmation.svelte | 81 ++++ .../features/visual-metrics-editing/lib.ts | 3 + .../features/workspaces/VisualMetrics.svelte | 446 +++++++++++------- .../src/layout/navigation/Navigation.svelte | 8 +- 15 files changed, 735 insertions(+), 398 deletions(-) create mode 100644 web-common/src/components/forms/InputLabel.svelte create mode 100644 web-common/src/features/visual-metrics-editing/AlertConfirmation.svelte diff --git a/web-common/src/components/forms/Input.svelte b/web-common/src/components/forms/Input.svelte index 3893273e293..1035c8ad7a3 100644 --- a/web-common/src/components/forms/Input.svelte +++ b/web-common/src/components/forms/Input.svelte @@ -2,10 +2,8 @@ import { EyeIcon, EyeOffIcon } from "lucide-svelte"; import { onMount } from "svelte"; import { slide } from "svelte/transition"; - import InfoCircle from "../icons/InfoCircle.svelte"; - import Tooltip from "../tooltip/Tooltip.svelte"; - import TooltipContent from "../tooltip/TooltipContent.svelte"; import Select from "./Select.svelte"; + import InputLabel from "./InputLabel.svelte"; const voidFunction = () => {}; @@ -93,24 +91,9 @@
{#if label} -
- - {#if hint} - -
- -
- - {@html hint} - - - {/if} -
+ + + {/if} {#if fields && fields?.length > 1} @@ -203,7 +186,7 @@ {id} bind:selectElement bind:value - options={options ?? []} + {options} {onChange} fontSize={14} {truncate} @@ -235,14 +218,6 @@ @apply flex flex-col gap-y-1 h-fit justify-center; } - .label-wrapper { - @apply flex items-center gap-x-1; - } - - label { - @apply text-sm font-medium text-gray-800; - } - .option-wrapper { @apply flex h-6 text-sm w-fit mb-1 rounded-[2px]; } diff --git a/web-common/src/components/forms/InputLabel.svelte b/web-common/src/components/forms/InputLabel.svelte new file mode 100644 index 00000000000..bca493296ae --- /dev/null +++ b/web-common/src/components/forms/InputLabel.svelte @@ -0,0 +1,40 @@ + + +
+ + {#if hint} + +
+ +
+ + {@html hint} + + + {/if} + +
+ + diff --git a/web-common/src/components/tag/Tag.svelte b/web-common/src/components/tag/Tag.svelte index 616a52a1e43..7aa86d69a38 100644 --- a/web-common/src/components/tag/Tag.svelte +++ b/web-common/src/components/tag/Tag.svelte @@ -16,6 +16,10 @@ export let height = 21; export let text: string = ""; + let className: string | undefined = undefined; + + export { className as class }; + function getColorClass(color: string) { switch (color) { case "gray": @@ -46,7 +50,7 @@ style:height="{height}px" class="px-2 border rounded-[20px] items-center justify-center inline-flex shrink-0 {getColorClass( color, - )}" + )} {className}" > {#if text !== ""} diff --git a/web-common/src/features/connectors/ConnectorEntry.svelte b/web-common/src/features/connectors/ConnectorEntry.svelte index 2551151578a..63cd96fe893 100644 --- a/web-common/src/features/connectors/ConnectorEntry.svelte +++ b/web-common/src/features/connectors/ConnectorEntry.svelte @@ -6,14 +6,16 @@ createRuntimeServiceGetInstance, } from "../../runtime-client"; import { runtime } from "../../runtime-client/runtime-store"; - import { connectorExplorerStore } from "./connector-explorer-store"; + import type { ConnectorExplorerStore } from "./connector-explorer-store"; import { connectorIconMapping } from "./connector-icon-mapping"; import DatabaseExplorer from "./olap/DatabaseExplorer.svelte"; export let connector: V1AnalyzedConnector; + export let store: ConnectorExplorerStore; $: connectorName = connector?.name as string; - $: expanded = connectorExplorerStore.getItem(connectorName); + $: expandedStore = store.getItem(connectorName); + $: expanded = $expandedStore; $: ({ instanceId } = $runtime); $: instance = createRuntimeServiceGetInstance(instanceId, { sensitive: true, @@ -29,30 +31,34 @@
  • - {#if $expanded} - + + {#if expanded} + {/if}
    • {/if} @@ -60,13 +66,13 @@ diff --git a/web-common/src/features/connectors/connector-explorer-store.ts b/web-common/src/features/connectors/connector-explorer-store.ts index e2e626a40f4..aa5e97f22bd 100644 --- a/web-common/src/features/connectors/connector-explorer-store.ts +++ b/web-common/src/features/connectors/connector-explorer-store.ts @@ -1,47 +1,67 @@ import { localStorageStore } from "@rilldata/web-common/lib/store-utils"; -import { derived, type Readable } from "svelte/store"; +import { derived, get, writable, type Writable } from "svelte/store"; -interface ConnectorExplorerState { +type ConnectorExplorerState = { showConnectors: boolean; expandedItems: Record; -} - -const initialState: ConnectorExplorerState = { - showConnectors: true, - expandedItems: {}, }; -function createConnectorExplorerStore() { - const { subscribe, update } = localStorageStore( - "connector-explorer-state", - initialState, - ); - - function getItemKey( - connector: string, - database?: string, - schema?: string, - ): string { - return [connector, database, schema].filter(Boolean).join("|"); - } - - function getDefaultState( - connector: string, // Included for API consistency, but not used in this function - database?: string, - schema?: string, - ): boolean { - if (schema) return false; // Database Schema - if (database) return true; // Database - return true; // Connector +export class ConnectorExplorerStore { + allowNavigateToTable: boolean; + allowContextMenu: boolean; + allowSelectTable: boolean; + allowShowSchema: boolean; + store: Writable; + onToggleItem: + | undefined + | (( + connector: string, + database?: string, + schema?: string, + table?: string, + ) => void) = undefined; + + constructor( + { + allowNavigateToTable = true, + allowContextMenu = true, + allowShowSchema = true, + allowSelectTable = false, + + showConnectors = true, + expandedItems = {}, + localStorage = true, + } = {}, + onToggleItem?: ( + connector: string, + database?: string, + schema?: string, + table?: string, + ) => void, + ) { + this.allowNavigateToTable = allowNavigateToTable; + this.allowContextMenu = allowContextMenu; + this.allowShowSchema = allowShowSchema; + this.allowSelectTable = allowSelectTable; + + if (onToggleItem) this.onToggleItem = onToggleItem; + + this.store = localStorage + ? localStorageStore("connector-explorer-state", { + showConnectors, + expandedItems, + }) + : writable({ showConnectors, expandedItems }); } - function createItemIfNotExists( + createItemIfNotExists( connector: string, database?: string, schema?: string, + table?: string, ) { - update((state) => { - const key = getItemKey(connector, database, schema); + this.store.update((state) => { + const key = getItemKey(connector, database, schema, table); if (key in state.expandedItems) return state; // Item already exists @@ -49,89 +69,145 @@ function createConnectorExplorerStore() { ...state, expandedItems: { ...state.expandedItems, - [key]: getDefaultState(connector, database, schema), + [key]: getDefaultState(connector, database, schema, table), }, }; }); } - return { - subscribe, - toggleExplorer: () => - update((state) => ({ ...state, showConnectors: !state.showConnectors })), - - getItem: ( + duplicateStore( + onToggleItem?: ( connector: string, database?: string, schema?: string, - ): Readable => { - createItemIfNotExists(connector, database, schema); - - const key = getItemKey(connector, database, schema); - - return derived({ subscribe }, ($state) => { - return $state.expandedItems[key]; - }); - }, - - toggleItem: (connector: string, database?: string, schema?: string) => - update((state) => { - const key = getItemKey(connector, database, schema); - const currentState = - state.expandedItems[key] ?? - getDefaultState(connector, database, schema); - return { - ...state, - expandedItems: { - ...state.expandedItems, - [key]: !currentState, - }, - }; - }), - - // Not used yet. Currently, the reconciler does not track connector renames. - renameItem: ( - oldConnector: string, - newConnector: string, - oldDatabase?: string, - newDatabase?: string, - oldSchema?: string, - newSchema?: string, - ) => - update((state) => { - const oldKeyPrefix = getItemKey(oldConnector, oldDatabase, oldSchema); - const newKeyPrefix = getItemKey(newConnector, newDatabase, newSchema); - - const updatedExpandedItems = Object.fromEntries( - Object.entries(state.expandedItems).map(([key, value]) => { - if (key.startsWith(oldKeyPrefix)) { - const newKey = key.replace(oldKeyPrefix, newKeyPrefix); - return [newKey, value]; - } - return [key, value]; - }), - ); - - return { - ...state, - expandedItems: updatedExpandedItems, - }; - }), - - deleteItem: (connector: string, database?: string, schema?: string) => - update((state) => { - const keyPrefix = getItemKey(connector, database, schema); - const updatedExpandedItems = Object.fromEntries( - Object.entries(state.expandedItems).filter( - ([key]) => !key.startsWith(keyPrefix), - ), - ); - return { - ...state, - expandedItems: updatedExpandedItems, - }; - }), + table?: string, + ) => void | Promise, + ) { + const state = get(this.store); + return new ConnectorExplorerStore( + { + allowNavigateToTable: false, + allowContextMenu: false, + allowShowSchema: false, + allowSelectTable: true, + localStorage: false, + showConnectors: state.showConnectors, + expandedItems: {}, + }, + onToggleItem ?? this.onToggleItem, + ); + } + + toggleExplorer = () => + this.store.update((state) => ({ + ...state, + showConnectors: !state.showConnectors, + })); + + getItem = ( + connector: string, + database?: string, + schema?: string, + table?: string, + ) => { + this.createItemIfNotExists(connector, database, schema, table); + + const key = getItemKey(connector, database, schema, table); + + return derived(this.store, ($state) => { + return $state.expandedItems[key]; + }); + }; + + toggleItem = ( + connector: string, + database?: string, + schema?: string, + table?: string, + ) => { + if (this.onToggleItem) + this.onToggleItem(connector, database, schema, table); + + if (table && !this.allowShowSchema) return; + + this.store.update((state) => { + const key = getItemKey(connector, database, schema, table); + const currentState = + state.expandedItems[key] ?? + getDefaultState(connector, database, schema, table); + return { + ...state, + expandedItems: { + ...state.expandedItems, + [key]: !currentState, + }, + }; + }); }; + + // Not used yet. Currently, the reconciler does not track connector renames. + renameItem = ( + oldConnector: string, + newConnector: string, + oldDatabase?: string, + newDatabase?: string, + oldSchema?: string, + newSchema?: string, + ) => + this.store.update((state) => { + const oldKeyPrefix = getItemKey(oldConnector, oldDatabase, oldSchema); + const newKeyPrefix = getItemKey(newConnector, newDatabase, newSchema); + + const updatedExpandedItems = Object.fromEntries( + Object.entries(state.expandedItems).map(([key, value]) => { + if (key.startsWith(oldKeyPrefix)) { + const newKey = key.replace(oldKeyPrefix, newKeyPrefix); + return [newKey, value]; + } + return [key, value]; + }), + ); + + return { + ...state, + expandedItems: updatedExpandedItems, + }; + }); + + deleteItem = (connector: string, database?: string, schema?: string) => + this.store.update((state) => { + const keyPrefix = getItemKey(connector, database, schema); + const updatedExpandedItems = Object.fromEntries( + Object.entries(state.expandedItems).filter( + ([key]) => !key.startsWith(keyPrefix), + ), + ); + return { + ...state, + expandedItems: updatedExpandedItems, + }; + }); +} + +export const connectorExplorerStore = new ConnectorExplorerStore(); + +// Helpers +function getItemKey( + connector: string, + database?: string, + schema?: string, + table?: string, +): string { + return [connector, database, schema, table].filter(Boolean).join("|"); } -export const connectorExplorerStore = createConnectorExplorerStore(); +function getDefaultState( + connector: string, // Included for API consistency, but not used in this function + database?: string, + schema?: string, + table?: string, +): boolean { + if (schema || table) return false; // Database Schema or Table + if (database) return true; // Database + return true; // Connector +} diff --git a/web-common/src/features/connectors/olap/DatabaseEntry.svelte b/web-common/src/features/connectors/olap/DatabaseEntry.svelte index eb9089eff7c..85962b8f36e 100644 --- a/web-common/src/features/connectors/olap/DatabaseEntry.svelte +++ b/web-common/src/features/connectors/olap/DatabaseEntry.svelte @@ -4,21 +4,24 @@ import CaretDownIcon from "../../../components/icons/CaretDownIcon.svelte"; import { LIST_SLIDE_DURATION as duration } from "../../../layout/config"; import type { V1AnalyzedConnector } from "../../../runtime-client"; - import { connectorExplorerStore } from "../connector-explorer-store"; import DatabaseSchemaEntry from "./DatabaseSchemaEntry.svelte"; import { useDatabaseSchemas } from "./selectors"; + import type { ConnectorExplorerStore } from "../connector-explorer-store"; export let instanceId: string; export let connector: V1AnalyzedConnector; export let database: string; + export let store: ConnectorExplorerStore; $: connectorName = connector?.name as string; - $: expanded = connectorExplorerStore.getItem(connectorName, database); + $: expandedStore = store.getItem(connectorName, database); + $: expanded = $expandedStore; $: databaseSchemasQuery = useDatabaseSchemas( instanceId, connector?.name as string, database, ); + $: ({ data, error, isLoading } = $databaseSchemasQuery); @@ -26,12 +29,11 @@ {#if database} - {#if $expanded} + {#if expanded} {#if connector?.errorMessage}
    {connector.errorMessage}
    {:else if !connector.driver || !connector.driver.name} @@ -83,6 +77,7 @@ {databaseSchema} table={tableInfo.name} hasUnsupportedDataTypes={tableInfo.hasUnsupportedDataTypes} + {store} /> {/each} diff --git a/web-common/src/features/connectors/olap/TableEntry.svelte b/web-common/src/features/connectors/olap/TableEntry.svelte index 214cf46ce43..a977b4817cf 100644 --- a/web-common/src/features/connectors/olap/TableEntry.svelte +++ b/web-common/src/features/connectors/olap/TableEntry.svelte @@ -12,6 +12,7 @@ makeFullyQualifiedTableName, makeTablePreviewHref, } from "./olap-config"; + import type { ConnectorExplorerStore } from "../connector-explorer-store"; export let instanceId: string; export let driver: string; @@ -20,9 +21,14 @@ export let databaseSchema: string; // The backend interprets an empty string as the default schema export let table: string; export let hasUnsupportedDataTypes: boolean; + export let store: ConnectorExplorerStore; let contextMenuOpen = false; - let showSchema = false; + + $: expandedStore = store.getItem(connector, database, databaseSchema, table); + $: showSchema = $expandedStore; + + const { allowContextMenu, allowNavigateToTable, allowShowSchema } = store; $: fullyQualifiedTableName = makeFullyQualifiedTableName( driver, @@ -38,26 +44,47 @@ databaseSchema, table, ); + $: open = $page.url.pathname === href; + + $: element = allowNavigateToTable ? "a" : "button";
  • - + {/if} - + { + store.toggleItem(connector, database, databaseSchema, table); + }} + > {table} - + + {#if hasUnsupportedDataTypes} {/if} - - - + + + + + + - - - - - - - - + + + + {/if} +
    • - {#if showSchema} + {#if allowShowSchema && showSchema} {/if} @@ -118,4 +148,8 @@ @apply flex grow items-center gap-x-1; @apply text-gray-900 truncate; } + + .selected:hover { + @apply bg-slate-200; + } diff --git a/web-common/src/features/connectors/olap/TableMenuItems.svelte b/web-common/src/features/connectors/olap/TableMenuItems.svelte index 94fcd217a85..5fbe2d585bf 100644 --- a/web-common/src/features/connectors/olap/TableMenuItems.svelte +++ b/web-common/src/features/connectors/olap/TableMenuItems.svelte @@ -9,7 +9,6 @@ MetricsEventScreenName, MetricsEventSpace, } from "@rilldata/web-common/metrics/service/MetricsTypes"; - import { useQueryClient } from "@tanstack/svelte-query"; import { WandIcon } from "lucide-svelte"; import ExploreIcon from "../../../components/icons/ExploreIcon.svelte"; import MetricsViewIcon from "../../../components/icons/MetricsViewIcon.svelte"; @@ -18,9 +17,9 @@ import { useCreateMetricsViewFromTableUIAction } from "../../metrics-views/ai-generation/generateMetricsView"; import { createModelFromTable } from "./createModel"; import { useIsModelingSupportedForOlapDriver } from "./selectors"; + import { queryClient } from "@rilldata/web-common/lib/svelte-query/globalQueryClient"; const { ai } = featureFlags; - const queryClient = useQueryClient(); export let connector: string; export let database: string = ""; diff --git a/web-common/src/features/visual-metrics-editing/AlertConfirmation.svelte b/web-common/src/features/visual-metrics-editing/AlertConfirmation.svelte new file mode 100644 index 00000000000..ce2168226f1 --- /dev/null +++ b/web-common/src/features/visual-metrics-editing/AlertConfirmation.svelte @@ -0,0 +1,81 @@ + + + + + + + {#if confirmation.action === "delete"} +

    + Delete {confirmation.index === undefined + ? "selected items" + : "this " + confirmation.type?.slice(0, -1)}? +

    + {:else if confirmation.action === "cancel"} +

    Cancel changes to {confirmation.type?.slice(0, -1)}?

    + {:else if confirmation.action === "switch"} +

    Switch reference model?

    + {/if} +     + + {#if confirmation.action === "cancel"} + You haven't saved changes to this {confirmation.type?.slice(0, -1)} yet, + so closing this window will lose your work. + {:else if confirmation.action === "delete"} + You will permanently remove {confirmation.index === undefined + ? "the selected items" + : "this " + confirmation.type?.slice(0, -1)} from all associated dashboards. + {:else if confirmation.action === "switch"} + Switching to a different model may break your measures and dimensions + unless the new model has similar data. + {/if} + +     + + + + + + + + + +     +     + + diff --git a/web-common/src/features/visual-metrics-editing/lib.ts b/web-common/src/features/visual-metrics-editing/lib.ts index 9bc0d352f77..7c5aa8844ec 100644 --- a/web-common/src/features/visual-metrics-editing/lib.ts +++ b/web-common/src/features/visual-metrics-editing/lib.ts @@ -20,6 +20,9 @@ export type Confirmation = { action: "cancel" | "delete" | "switch"; type?: ItemType; model?: string; + database?: string; + connector?: string; + schema?: string; index?: number; field?: string; }; diff --git a/web-common/src/features/workspaces/VisualMetrics.svelte b/web-common/src/features/workspaces/VisualMetrics.svelte index ff785ed6304..96ef92057b5 100644 --- a/web-common/src/features/workspaces/VisualMetrics.svelte +++ b/web-common/src/features/workspaces/VisualMetrics.svelte @@ -1,6 +1,9 @@
    - {#key confirmation} - { - if (!modelOrSourceName) { - await updateProperty("model", newModelOrSourceName, "table"); - } else { - confirmation = { - action: "switch", - model: newModelOrSourceName, - }; - } - }} - /> - {/key} + {#if tableMode || modelingDisabled} +
    + + + {#if !modelingDisabled} + + {/if} + + + + + + + +
    + +
    +     +     +
    + {:else} + {#key confirmation} + { + if (!modelOrSourceOrTableName) { + await updateProperties({ model: newModelOrSourceName }, [ + "table", + "database", + "connector", + "database_schema", + ]); + } else { + confirmation = { + action: "switch", + model: newModelOrSourceName, + }; + } + }} + > + + {#if hasNonDuckDBOLAPConnector} + + {/if} + + + {/key} + {/if} { - await updateProperty("timeseries", value); + await updateProperties({ timeseries: value }); }} /> @@ -457,7 +638,7 @@ label="Smallest time grain" hint="The smallest time unit by which your charts and tables can be bucketed" onChange={async (value) => { - await updateProperty("smallest_time_grain", value); + await updateProperties({ smallest_time_grain: value }); }} />
    @@ -611,7 +792,14 @@ {#key $editingItem} { triggerDelete(index, type); }} @@ -626,117 +814,53 @@ resetEditing(); } }} - {index} - {type} - {resetEditing} - editing={index !== -1} - {fileArtifact} - {switchView} - bind:unsavedChanges /> {/key} {/if}
    {#if confirmation} - - - - - {#if confirmation.action === "delete"} -

    - Delete {confirmation.index === undefined - ? "selected items" - : "this " + confirmation.type?.slice(0, -1)}? -

    - {:else if confirmation.action === "cancel"} -

    Cancel changes to {confirmation.type?.slice(0, -1)}?

    - {:else if confirmation.action === "switch"} -

    Switch reference model?

    - {/if} -     - - {#if confirmation.action === "cancel"} - You haven't saved changes to this {confirmation.type?.slice(0, -1)} yet, - so closing this window will lose your work. - {:else if confirmation.action === "delete"} - You will permanently remove {confirmation.index === undefined - ? "the selected items" - : "this " + confirmation.type?.slice(0, -1)} from all associated dashboards. - {:else if confirmation.action === "switch"} - Switching to a different model may break your measures and - dimensions unless the new model has similar data. - {/if} - -     - - - - - - - - - -     -     + : selected, + ); + } else if (confirmation?.action === "switch") { + await updateProperties( + { + model: confirmation.model, + database: confirmation.database, + connector: confirmation.connector, + database_schema: confirmation.schema, + }, + ["table"], + ); + } else if (confirmation?.action === "cancel") { + if ( + confirmation?.field && + confirmation?.index !== undefined && + confirmation?.type + ) { + unsavedChanges = false; + await setEditing( + confirmation.index, + confirmation.type, + confirmation.field, + ); + } + } + + resetEditing(); + confirmation = null; + }} + /> {/if} diff --git a/web-common/src/layout/navigation/Navigation.svelte b/web-common/src/layout/navigation/Navigation.svelte index 6f47f6bcc03..c67151db2a3 100644 --- a/web-common/src/layout/navigation/Navigation.svelte +++ b/web-common/src/layout/navigation/Navigation.svelte @@ -34,7 +34,7 @@ $: navWrapperHeight = contentRect.height; - $: showConnectors = $connectorExplorerStore.showConnectors; + let showConnectors = true; $: connectorSectionHeight = navWrapperHeight * connectorHeightPercentage; @@ -112,7 +112,7 @@ on:click={() => { const open = showConnectors; - if (!open) connectorExplorerStore.toggleExplorer(); + if (!open) showConnectors = true; connectorWrapper.animate( [ @@ -128,7 +128,7 @@ easing: "ease-out", }, ).onfinish = () => { - if (open) connectorExplorerStore.toggleExplorer(); + if (open) showConnectors = false; }; }} > @@ -146,7 +146,7 @@ style:height="{showConnectors ? connectorSectionHeight : 0}px" > {#if showConnectors} - + {/if}