From 6250069f824f3e8ed2640a955207c828eee643c8 Mon Sep 17 00:00:00 2001 From: Aditya Hegde Date: Tue, 10 Dec 2024 12:18:26 +0530 Subject: [PATCH] Fix race condition in reactive statement --- web-admin/src/features/bookmarks/selectors.ts | 1 + .../[project]/-/share/[token]/+page.svelte | 7 +- .../[project]/-/share/[token]/+page.ts | 9 +- .../[project]/explore/[dashboard]/+layout.ts | 72 +++++++++++--- .../explore/[dashboard]/+page.svelte | 22 +++-- .../[project]/explore/[dashboard]/+page.ts | 50 ++++------ .../StateManagersProvider.svelte | 4 +- .../state-managers/state-managers.ts | 24 +++-- .../stores/DashboardStateProvider.svelte | 20 ---- .../stores/dashboard-store-defaults.ts | 54 ++++++----- .../stores/dashboard-stores.spec.ts | 6 -- .../dashboards/stores/dashboard-stores.ts | 8 +- .../stores/persistent-dashboard-state.ts | 30 ++---- .../dashboards/stores/syncDashboardState.ts | 96 ------------------- .../url-state/DashboardURLStateSync.svelte | 70 +++++++------- .../url-state/convertPresetToExploreState.ts | 4 +- .../url-state/convertURLToExplorePreset.ts | 4 +- .../url-state/getUpdatedUrlForExploreState.ts | 39 ++++++++ web-common/src/features/explores/selectors.ts | 35 +++++-- 19 files changed, 269 insertions(+), 286 deletions(-) delete mode 100644 web-common/src/features/dashboards/stores/DashboardStateProvider.svelte delete mode 100644 web-common/src/features/dashboards/stores/syncDashboardState.ts create mode 100644 web-common/src/features/dashboards/url-state/getUpdatedUrlForExploreState.ts diff --git a/web-admin/src/features/bookmarks/selectors.ts b/web-admin/src/features/bookmarks/selectors.ts index 58e764722fb..d870486ad62 100644 --- a/web-admin/src/features/bookmarks/selectors.ts +++ b/web-admin/src/features/bookmarks/selectors.ts @@ -67,6 +67,7 @@ export function categorizeBookmarks( personal: [], shared: [], }; + if (!exploreState) return bookmarks; bookmarkResp?.forEach((bookmarkResource) => { const bookmark = parseBookmark( bookmarkResource, diff --git a/web-admin/src/routes/[organization]/[project]/-/share/[token]/+page.svelte b/web-admin/src/routes/[organization]/[project]/-/share/[token]/+page.svelte index d84646ce3c7..36b60944f6a 100644 --- a/web-admin/src/routes/[organization]/[project]/-/share/[token]/+page.svelte +++ b/web-admin/src/routes/[organization]/[project]/-/share/[token]/+page.svelte @@ -16,6 +16,7 @@ $: ({ defaultExplorePreset, partialExploreState, + loadedOutsideOfURL, token: { resourceName }, } = data); $: ({ organization, project } = $page.params); @@ -55,7 +56,11 @@ metricsViewName={explore.metricsView.meta.name.name} exploreName={resourceName} > - + { +export const load = async ({ url, parent, params }) => { const { explore, metricsView, defaultExplorePreset, token } = await parent(); + const { organization, project } = params; const exploreName = token.resourceName; const metricsViewSpec = metricsView.metricsView?.state?.validSpec; const exploreSpec = explore.explore?.state?.validSpec; @@ -35,12 +36,16 @@ export const load = async ({ url, parent }) => { // Get Explore state from URL params let partialExploreState: Partial = {}; + let loadedOutsideOfURL = false; const errors: Error[] = []; if (metricsViewSpec && exploreSpec) { const { partialExploreState: partialExploreStateFromUrl, + loadedOutsideOfURL: partialLoadedOutsideOfURL, errors: errorsFromConvert, } = convertURLToExploreState( + exploreName, + `__${organization}__${project}`, url.searchParams, metricsViewSpec, exploreSpec, @@ -48,10 +53,12 @@ export const load = async ({ url, parent }) => { ); partialExploreState = partialExploreStateFromUrl; errors.push(...errorsFromConvert); + loadedOutsideOfURL = partialLoadedOutsideOfURL; } return { partialExploreState, + loadedOutsideOfURL, errors, }; }; diff --git a/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+layout.ts b/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+layout.ts index 914f498f0b8..6ef8f266610 100644 --- a/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+layout.ts +++ b/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+layout.ts @@ -2,6 +2,9 @@ import { fetchBookmarks, isHomeBookmark, } from "@rilldata/web-admin/features/bookmarks/selectors"; +import { getDashboardStateFromUrl } from "@rilldata/web-common/features/dashboards/proto-state/fromProto"; +import type { MetricsExplorerEntity } from "@rilldata/web-common/features/dashboards/stores/metrics-explorer-entity"; +import { getUpdatedUrlForExploreState } from "@rilldata/web-common/features/dashboards/url-state/getUpdatedUrlForExploreState"; import { fetchExploreSpec } from "@rilldata/web-common/features/explores/selectors"; import { type V1ExplorePreset, @@ -11,23 +14,26 @@ import { export const load = async ({ params, depends, parent }) => { const { project, runtime } = await parent(); - const exploreName = params.dashboard; + const { organization, project: projectName, dashboard: exploreName } = params; depends(exploreName, "explore"); + let explore: V1Resource | undefined; + let metricsView: V1Resource | undefined; + let defaultExplorePreset: V1ExplorePreset | undefined; + let initExploreState: Partial = {}; + let initLoadedOutsideOfURL = false; try { - const { explore, metricsView, defaultExplorePreset } = - await fetchExploreSpec(runtime?.instanceId, exploreName); - - // used to merge home bookmark to url state - const bookmarks = await fetchBookmarks(project.id, exploreName); - - return { - explore, - metricsView, - defaultExplorePreset, - homeBookmark: bookmarks.find(isHomeBookmark), - }; + const fetchedExploreSpecDetails = await fetchExploreSpec( + runtime?.instanceId, + exploreName, + `__${organization}__${projectName}`, + ); + explore = fetchedExploreSpecDetails.explore; + metricsView = fetchedExploreSpecDetails.metricsView; + defaultExplorePreset = fetchedExploreSpecDetails.defaultExplorePreset; + initExploreState = fetchedExploreSpecDetails.initExploreState; + initLoadedOutsideOfURL = fetchedExploreSpecDetails.initLoadedOutsideOfURL; } catch { // error handled in +page.svelte for now // TODO: move it here @@ -35,7 +41,45 @@ export const load = async ({ params, depends, parent }) => { explore: {}, metricsView: {}, defaultExplorePreset: {}, - homeBookmark: undefined, + initExploreState: {}, + initLoadedOutsideOfURL: false, }; } + + const metricsViewSpec = metricsView.metricsView?.state?.validSpec ?? {}; + const exploreSpec = explore.explore?.state?.validSpec ?? {}; + + try { + const bookmarks = await fetchBookmarks(project.id, exploreName); + const homeBookmark = bookmarks.find(isHomeBookmark); + + if (homeBookmark) { + const exploreStateFromBookmark = getDashboardStateFromUrl( + homeBookmark.data ?? "", + metricsViewSpec, + exploreSpec, + {}, // TODO + ); + Object.assign(initExploreState, exploreStateFromBookmark); + initLoadedOutsideOfURL = true; + } + } catch { + // TODO + } + const initUrlSearch = initLoadedOutsideOfURL + ? getUpdatedUrlForExploreState( + exploreSpec, + defaultExplorePreset, + initExploreState, + new URLSearchParams(), + ) + : ""; + + return { + explore, + metricsView, + defaultExplorePreset, + initExploreState, + initUrlSearch, + }; }; diff --git a/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+page.svelte b/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+page.svelte index 7f5352101bd..30fcf735bdd 100644 --- a/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+page.svelte +++ b/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+page.svelte @@ -19,7 +19,15 @@ // const PollIntervalWhenDashboardOk = 60000; // This triggers a layout shift, so removing for now export let data: PageData; - $: ({ defaultExplorePreset, partialExploreState, loaded, errors } = data); + $: ({ + defaultExplorePreset, + initExploreState, + initUrlSearch, + partialExploreState, + urlSearchForPartial, + errors, + exploreName, + } = data); $: if (errors?.length) { setTimeout(() => { eventBus.emit("notification", { @@ -33,11 +41,7 @@ } $: instanceId = $runtime?.instanceId; - $: ({ - organization: orgName, - project: projectName, - dashboard: exploreName, - } = $page.params); + $: ({ organization: orgName, project: projectName } = $page.params); $: explore = useExplore(instanceId, exploreName, { refetchInterval: () => { @@ -99,9 +103,13 @@ {#key exploreName} diff --git a/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+page.ts b/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+page.ts index f205845d962..905a43429eb 100644 --- a/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+page.ts +++ b/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+page.ts @@ -1,49 +1,35 @@ -import { convertBookmarkToUrlSearchParams } from "@rilldata/web-admin/features/bookmarks/selectors"; -import { metricsExplorerStore } from "@rilldata/web-common/features/dashboards/stores/dashboard-stores"; import { convertURLToExploreState } from "@rilldata/web-common/features/dashboards/url-state/convertPresetToExploreState"; -import { redirect } from "@sveltejs/kit"; -import { get } from "svelte/store"; +import { getUpdatedUrlForExploreState } from "@rilldata/web-common/features/dashboards/url-state/getUpdatedUrlForExploreState"; export const load = async ({ url, parent, params }) => { - const { explore, metricsView, defaultExplorePreset, homeBookmark } = - await parent(); + const { explore, metricsView, defaultExplorePreset } = await parent(); const { organization, project, dashboard: exploreName } = params; const metricsViewSpec = metricsView.metricsView?.state?.validSpec; const exploreSpec = explore.explore?.state?.validSpec; - // On the first dashboard load, if there are no URL params, redirect to the "Home" bookmark. - if ( - homeBookmark && - ![...url.searchParams.keys()].length && - !(exploreName in get(metricsExplorerStore).entities) - ) { - const newUrl = new URL(url); - newUrl.search = convertBookmarkToUrlSearchParams( - homeBookmark, + const { partialExploreState, loadedOutsideOfURL, errors } = + convertURLToExploreState( + exploreName, + `__${organization}__${project}`, + url.searchParams, metricsViewSpec, exploreSpec, - {}, // TODO - undefined, defaultExplorePreset, ); + const urlSearchForPartial = loadedOutsideOfURL + ? getUpdatedUrlForExploreState( + exploreSpec, + defaultExplorePreset, + partialExploreState, + url.searchParams, + ) + : url.searchParams.toString(); - if (newUrl.search !== url.search) { - throw redirect(307, `${newUrl.pathname}${newUrl.search}`); - } - } - - const { partialExploreState, loaded, errors } = convertURLToExploreState( - exploreName, - `__${organization}__${project}`, - url.searchParams, - metricsViewSpec, - exploreSpec, - defaultExplorePreset, - ); - + console.log(exploreName, urlSearchForPartial); return { partialExploreState, - loaded, + urlSearchForPartial, errors, + exploreName, }; }; diff --git a/web-common/src/features/dashboards/state-managers/StateManagersProvider.svelte b/web-common/src/features/dashboards/state-managers/StateManagersProvider.svelte index 2fe6884fe3c..52be5778f78 100644 --- a/web-common/src/features/dashboards/state-managers/StateManagersProvider.svelte +++ b/web-common/src/features/dashboards/state-managers/StateManagersProvider.svelte @@ -1,6 +1,6 @@ - -{#if $dashboardStoreReady.isFetching} -
- -
-{:else} - -{/if} diff --git a/web-common/src/features/dashboards/stores/dashboard-store-defaults.ts b/web-common/src/features/dashboards/stores/dashboard-store-defaults.ts index ce0203be25a..68cc6a95904 100644 --- a/web-common/src/features/dashboards/stores/dashboard-store-defaults.ts +++ b/web-common/src/features/dashboards/stores/dashboard-store-defaults.ts @@ -3,7 +3,7 @@ import { contextColWidthDefaults, type MetricsExplorerEntity, } from "@rilldata/web-common/features/dashboards/stores/metrics-explorer-entity"; -import { getPersistentDashboardState } from "@rilldata/web-common/features/dashboards/stores/persistent-dashboard-state"; +import { getPersistentDashboardStateForKey } from "@rilldata/web-common/features/dashboards/stores/persistent-dashboard-state"; import { convertPresetToExploreState } from "@rilldata/web-common/features/dashboards/url-state/convertPresetToExploreState"; import { getDefaultExplorePreset } from "@rilldata/web-common/features/dashboards/url-state/getDefaultExplorePreset"; import type { @@ -40,38 +40,42 @@ export function getDefaultExploreState( } as MetricsExplorerEntity; } -// TODO: move this to the same place where we load from session store. -// also move the type to V1ExplorePreset similar to session store. export function restorePersistedDashboardState( - metricsExplorer: MetricsExplorerEntity, + exploreSpec: V1ExploreSpec, + key: string, ) { - const persistedState = getPersistentDashboardState(); - if (persistedState.visibleMeasures) { - metricsExplorer.allMeasuresVisible = - persistedState.visibleMeasures.length === - metricsExplorer.visibleMeasureKeys.size; // TODO: check values - metricsExplorer.visibleMeasureKeys = new Set( - persistedState.visibleMeasures, + const stateFromLocalStorage = getPersistentDashboardStateForKey(key); + if (!stateFromLocalStorage) return undefined; + + const partialExploreState: Partial = {}; + + if (stateFromLocalStorage.visibleMeasures) { + partialExploreState.allMeasuresVisible = + stateFromLocalStorage.visibleMeasures.length === + exploreSpec.measures?.length; + partialExploreState.visibleMeasureKeys = new Set( + stateFromLocalStorage.visibleMeasures, ); } - if (persistedState.visibleDimensions) { - metricsExplorer.allDimensionsVisible = - persistedState.visibleDimensions.length === - metricsExplorer.visibleDimensionKeys.size; // TODO: check values - metricsExplorer.visibleDimensionKeys = new Set( - persistedState.visibleDimensions, + if (stateFromLocalStorage.visibleDimensions) { + partialExploreState.allDimensionsVisible = + stateFromLocalStorage.visibleDimensions.length === + exploreSpec.dimensions?.length; + partialExploreState.visibleDimensionKeys = new Set( + stateFromLocalStorage.visibleDimensions, ); } - if (persistedState.leaderboardMeasureName) { - metricsExplorer.leaderboardMeasureName = - persistedState.leaderboardMeasureName; + if (stateFromLocalStorage.leaderboardMeasureName) { + partialExploreState.leaderboardMeasureName = + stateFromLocalStorage.leaderboardMeasureName; } - if (persistedState.dashboardSortType) { - metricsExplorer.dashboardSortType = persistedState.dashboardSortType; + if (stateFromLocalStorage.dashboardSortType) { + partialExploreState.dashboardSortType = + stateFromLocalStorage.dashboardSortType; } - if (persistedState.sortDirection) { - metricsExplorer.sortDirection = persistedState.sortDirection; + if (stateFromLocalStorage.sortDirection) { + partialExploreState.sortDirection = stateFromLocalStorage.sortDirection; } - return metricsExplorer; + return partialExploreState; } diff --git a/web-common/src/features/dashboards/stores/dashboard-stores.spec.ts b/web-common/src/features/dashboards/stores/dashboard-stores.spec.ts index e9cc1126092..f9cd1140612 100644 --- a/web-common/src/features/dashboards/stores/dashboard-stores.spec.ts +++ b/web-common/src/features/dashboards/stores/dashboard-stores.spec.ts @@ -3,10 +3,6 @@ import { createAndExpression, createInExpression, } from "@rilldata/web-common/features/dashboards/stores/filter-utils"; -import { - getPersistentDashboardStore, - initPersistentDashboardStore, -} from "@rilldata/web-common/features/dashboards/stores/persistent-dashboard-state"; import { AD_BIDS_BASE_FILTER, AD_BIDS_BID_PRICE_MEASURE, @@ -46,7 +42,6 @@ import { beforeAll, beforeEach, describe, expect, it } from "vitest"; describe("dashboard-stores", () => { beforeAll(() => { initLocalUserPreferenceStore(AD_BIDS_EXPLORE_NAME); - initPersistentDashboardStore(AD_BIDS_EXPLORE_NAME); runtime.set({ instanceId: "", host: "", @@ -54,7 +49,6 @@ describe("dashboard-stores", () => { }); beforeEach(() => { - getPersistentDashboardStore().reset(); resetDashboardStore(); }); diff --git a/web-common/src/features/dashboards/stores/dashboard-stores.ts b/web-common/src/features/dashboards/stores/dashboard-stores.ts index dfb82049709..a5930de6c45 100644 --- a/web-common/src/features/dashboards/stores/dashboard-stores.ts +++ b/web-common/src/features/dashboards/stores/dashboard-stores.ts @@ -3,10 +3,7 @@ import { getDashboardStateFromUrl } from "@rilldata/web-common/features/dashboar import { getProtoFromDashboardState } from "@rilldata/web-common/features/dashboards/proto-state/toProto"; import { getWhereFilterExpressionIndex } from "@rilldata/web-common/features/dashboards/state-managers/selectors/dimension-filters"; import { AdvancedMeasureCorrector } from "@rilldata/web-common/features/dashboards/stores/AdvancedMeasureCorrector"; -import { - getDefaultExploreState, - restorePersistedDashboardState, -} from "@rilldata/web-common/features/dashboards/stores/dashboard-store-defaults"; +import { getDefaultExploreState } from "@rilldata/web-common/features/dashboards/stores/dashboard-store-defaults"; import { createAndExpression, filterExpressions, @@ -183,9 +180,6 @@ const metricsViewReducers = { ...getDefaultExploreState(name, metricsView, explore, fullTimeRange), ...initState, }; - state.entities[name] = restorePersistedDashboardState( - state.entities[name], - ); updateMetricsExplorerProto(state.entities[name]); diff --git a/web-common/src/features/dashboards/stores/persistent-dashboard-state.ts b/web-common/src/features/dashboards/stores/persistent-dashboard-state.ts index 41c6c01ceb8..66a6096ed82 100644 --- a/web-common/src/features/dashboards/stores/persistent-dashboard-state.ts +++ b/web-common/src/features/dashboards/stores/persistent-dashboard-state.ts @@ -3,7 +3,7 @@ import type { SortType, } from "@rilldata/web-common/features/dashboards/proto-state/derived-types"; import { localStorageStore } from "@rilldata/web-common/lib/store-utils"; -import { get, type Readable, type Updater } from "svelte/store"; +import { type Readable, type Updater } from "svelte/store"; /** * Partial state of the dashboard that is stored in local storage. @@ -54,7 +54,7 @@ export type PersistentDashboardStore = Readable & ReturnType; export function createPersistentDashboardStore(storeKey: string) { const { subscribe, update } = localStorageStore( - `${storeKey}-persistentDashboardStore`, + `${storeKey.toLowerCase()}-persistentDashboardStore`, {}, ); return { @@ -63,22 +63,12 @@ export function createPersistentDashboardStore(storeKey: string) { }; } -// TODO: once we move everything to state-managers we wont need this -let persistentDashboardStore: PersistentDashboardStore; -export function initPersistentDashboardStore(storeKey: string) { - persistentDashboardStore = createPersistentDashboardStore(storeKey); -} - -export function getPersistentDashboardStore() { - return persistentDashboardStore; -} - -export function getPersistentDashboardState(): PersistentDashboardState { - if (!persistentDashboardStore) return {}; - return get(persistentDashboardStore); -} - -export function hasPersistentDashboardData() { - if (!persistentDashboardStore) return false; - return Object.keys(get(persistentDashboardStore)).length > 0; +export function getPersistentDashboardStateForKey( + key: string, +): PersistentDashboardState | undefined { + const dataRaw = localStorage.getItem( + `${key.toLowerCase()}-persistentDashboardStore`, + ); + if (!dataRaw) return undefined; + return JSON.parse(dataRaw) as PersistentDashboardState; } diff --git a/web-common/src/features/dashboards/stores/syncDashboardState.ts b/web-common/src/features/dashboards/stores/syncDashboardState.ts deleted file mode 100644 index b2febc4adc9..00000000000 --- a/web-common/src/features/dashboards/stores/syncDashboardState.ts +++ /dev/null @@ -1,96 +0,0 @@ -import { page } from "$app/stores"; -import type { CompoundQueryResult } from "@rilldata/web-common/features/compound-query-result"; -import { - createMetricsViewSchema, - createTimeRangeSummary, -} from "@rilldata/web-common/features/dashboards/selectors/index"; -import type { StateManagers } from "@rilldata/web-common/features/dashboards/state-managers/state-managers"; -import { metricsExplorerStore } from "@rilldata/web-common/features/dashboards/stores/dashboard-stores"; -import { derived, get } from "svelte/store"; - -/** - * createDashboardStateSync creates a store that keeps the dashboard state in sync with metrics config. - * It derives from metrics view spec, time range summary and metrics view schema. - * - * For the 1st time it is run it will call `metricsExplorerStore.init` to initialise the dashboard store. - * Optionally loads an initial url state. - * - * For successive runs it will call `metricsExplorerStore.sync` to keep the store in sync with metrics config. - * `sync` will make sure any removed measures and dimensions are not selected in anything in the dashboard. - * - * Note that this returns a readable so that the body of the `subscribe` is executed. - * - * @param ctx - * @param initialUrlStateStore Initial url state to load when the dashboard store is initialised for the 1st time. - * @returns A boolean readable that is true once the dashbaord store is created. - */ -export function createDashboardStateSync( - ctx: StateManagers, - initialUrlStateStore?: CompoundQueryResult, -) { - return derived( - [ - ctx.validSpecStore, - createTimeRangeSummary(ctx), - createMetricsViewSchema(ctx), - ...(initialUrlStateStore ? [initialUrlStateStore] : []), - ], - ([ - validSpecRes, - timeRangeRes, - metricsViewSchemaRes, - initialUrlStateRes, - ]) => { - if ( - // still fetching - validSpecRes.isFetching || - timeRangeRes.isFetching || - metricsViewSchemaRes.isFetching || - initialUrlStateRes?.isFetching - ) { - return { isFetching: true, error: false }; - } - - if ( - !validSpecRes.data?.metricsView || - (!!validSpecRes.data.metricsView?.timeDimension && - !timeRangeRes.data) || - !validSpecRes.data?.explore || - !metricsViewSchemaRes.data?.schema - ) { - return { isFetching: false, error: true }; - } - - const { metricsView, explore } = validSpecRes.data; - - const exploreName = get(ctx.exploreName); - if (exploreName in get(metricsExplorerStore).entities) { - // Successive syncs with metrics view spec - // metricsExplorerStore.sync(exploreName, explore); - } else { - // Running for the 1st time. Initialise the dashboard store. - metricsExplorerStore.init( - exploreName, - metricsView, - explore, - timeRangeRes.data, - ); - const initialUrlState = - get(page).url.searchParams.get("state") ?? initialUrlStateRes?.data; - if (initialUrlState) { - // If there is data to be loaded, load it during the init - // metricsExplorerStore.syncFromUrl( - // exploreName, - // initialUrlState, - // metricsView, - // explore, - // metricsViewSchemaRes.data.schema, - // ); - // Call sync to make sure changes in dashboard are honoured - metricsExplorerStore.sync(exploreName, explore); - } - } - return { isFetching: false, error: false }; - }, - ); -} diff --git a/web-common/src/features/dashboards/url-state/DashboardURLStateSync.svelte b/web-common/src/features/dashboards/url-state/DashboardURLStateSync.svelte index 62bd27d79f6..36aa8ca1716 100644 --- a/web-common/src/features/dashboards/url-state/DashboardURLStateSync.svelte +++ b/web-common/src/features/dashboards/url-state/DashboardURLStateSync.svelte @@ -6,8 +6,7 @@ import { metricsExplorerStore } from "@rilldata/web-common/features/dashboards/stores/dashboard-stores"; import type { MetricsExplorerEntity } from "@rilldata/web-common/features/dashboards/stores/metrics-explorer-entity"; import { convertExploreStateToURLSearchParams } from "@rilldata/web-common/features/dashboards/url-state/convertExploreStateToURLSearchParams"; - import { FromURLParamViewMap } from "@rilldata/web-common/features/dashboards/url-state/mappers"; - import { ExploreStateURLParams } from "@rilldata/web-common/features/dashboards/url-state/url-params"; + import { getUpdatedUrlForExploreState } from "@rilldata/web-common/features/dashboards/url-state/getUpdatedUrlForExploreState"; import { createQueryServiceMetricsViewSchema, type V1ExplorePreset, @@ -15,29 +14,30 @@ import type { HTTPError } from "@rilldata/web-common/runtime-client/fetchWrapper"; import { runtime } from "@rilldata/web-common/runtime-client/runtime-store"; + export let metricsViewName: string; + export let exploreName: string; export let defaultExplorePreset: V1ExplorePreset; + export let initExploreState: Partial; + export let initUrlSearch: string; export let partialExploreState: Partial; - export let loaded: boolean; + export let urlSearchForPartial: string; - const { - metricsViewName, - exploreName, - dashboardStore, - validSpecStore, - timeRangeSummaryStore, - } = getStateManagers(); + const { dashboardStore, validSpecStore, timeRangeSummaryStore } = + getStateManagers(); $: exploreSpec = $validSpecStore.data?.explore; $: metricsSpec = $validSpecStore.data?.metricsView; const metricsViewSchema = createQueryServiceMetricsViewSchema( $runtime.instanceId, - $metricsViewName, + metricsViewName, ); $: ({ error: schemaError } = $metricsViewSchema); $: ({ error, data: timeRangeSummary } = $timeRangeSummaryStore); $: timeRangeSummaryError = error as HTTPError; + $: console.log(exploreName, initUrlSearch, urlSearchForPartial); + let prevUrl = ""; function gotoNewState() { if (!exploreSpec) return; @@ -58,40 +58,38 @@ function mergePartialExplorerEntity() { if (!metricsSpec || !exploreSpec) return; - if (!$dashboardStore) { + const isInit = !$dashboardStore; + if (isInit) { // initial page load, create an entry in metricsExplorerStore metricsExplorerStore.init( - $exploreName, + exploreName, metricsSpec, exploreSpec, timeRangeSummary, + initExploreState, ); } - metricsExplorerStore.mergePartialExplorerEntity( - $exploreName, - partialExploreState, - metricsSpec, - ); - if (loaded) { - const curUrl = new URL(location.href); - const redirectUrl = new URL(curUrl); - redirectUrl.search = convertExploreStateToURLSearchParams( - partialExploreState as MetricsExplorerEntity, - exploreSpec, - defaultExplorePreset, + if ( + // if not dashboard init then always merge partial + !isInit || + // else during init only merge if partial updates the url params + urlSearchForPartial !== "" + ) { + metricsExplorerStore.mergePartialExplorerEntity( + exploreName, + partialExploreState, + metricsSpec, ); - curUrl.searchParams.forEach((value, key) => { - if ( - key === ExploreStateURLParams.WebView && - FromURLParamViewMap[value] === defaultExplorePreset.view - ) { - // ignore default view. - // since we do not add params equal to default this will append to the end of the URL breaking the param order. - return; - } - redirectUrl.searchParams.set(key, value); - }); + } + const curSearch = $page.url.searchParams.toString(); + const partialDifferentThanCurrent = curSearch !== urlSearchForPartial; + const initDifferentThanCurrent = curSearch !== initUrlSearch; + if (partialDifferentThanCurrent || (isInit && initDifferentThanCurrent)) { + const redirectUrl = new URL($page.url); + redirectUrl.search = partialDifferentThanCurrent + ? urlSearchForPartial + : initUrlSearch; history.replaceState(history.state, "", redirectUrl); prevUrl = redirectUrl.toString(); } else { diff --git a/web-common/src/features/dashboards/url-state/convertPresetToExploreState.ts b/web-common/src/features/dashboards/url-state/convertPresetToExploreState.ts index 068c53de62e..6cdca48f865 100644 --- a/web-common/src/features/dashboards/url-state/convertPresetToExploreState.ts +++ b/web-common/src/features/dashboards/url-state/convertPresetToExploreState.ts @@ -53,7 +53,7 @@ export function convertURLToExploreState( ) { const errors: Error[] = []; const { - loaded, + loadedOutsideOfURL, preset, errors: errorsFromPreset, } = convertURLToExplorePreset( @@ -68,7 +68,7 @@ export function convertURLToExploreState( const { partialExploreState, errors: errorsFromEntity } = convertPresetToExploreState(metricsView, exploreSpec, preset); errors.push(...errorsFromEntity); - return { partialExploreState, loaded, errors }; + return { partialExploreState, loadedOutsideOfURL, errors }; } /** diff --git a/web-common/src/features/dashboards/url-state/convertURLToExplorePreset.ts b/web-common/src/features/dashboards/url-state/convertURLToExplorePreset.ts index de99d112743..058a8862e9c 100644 --- a/web-common/src/features/dashboards/url-state/convertURLToExplorePreset.ts +++ b/web-common/src/features/dashboards/url-state/convertURLToExplorePreset.ts @@ -154,7 +154,7 @@ export function convertURLToExplorePreset( } } - return { loaded: false, preset, errors }; + return { loadedOutsideOfURL: false, preset, errors }; } function fromLegacyStateUrlParam( @@ -232,7 +232,7 @@ function fromSessionStore( } return { - loaded: true, + loadedOutsideOfURL: true, preset: explorePresetFromSessionStorage, errors: [], }; diff --git a/web-common/src/features/dashboards/url-state/getUpdatedUrlForExploreState.ts b/web-common/src/features/dashboards/url-state/getUpdatedUrlForExploreState.ts new file mode 100644 index 00000000000..69e512e008c --- /dev/null +++ b/web-common/src/features/dashboards/url-state/getUpdatedUrlForExploreState.ts @@ -0,0 +1,39 @@ +import type { MetricsExplorerEntity } from "@rilldata/web-common/features/dashboards/stores/metrics-explorer-entity"; +import { convertExploreStateToURLSearchParams } from "@rilldata/web-common/features/dashboards/url-state/convertExploreStateToURLSearchParams"; +import { FromURLParamViewMap } from "@rilldata/web-common/features/dashboards/url-state/mappers"; +import { ExploreStateURLParams } from "@rilldata/web-common/features/dashboards/url-state/url-params"; +import type { + V1ExplorePreset, + V1ExploreSpec, +} from "@rilldata/web-common/runtime-client"; + +/** + * Sometimes data is loaded from sources other than the url. + * In that case update the URL to make sure the state matches the current url. + */ +export function getUpdatedUrlForExploreState( + exploreSpec: V1ExploreSpec, + defaultExplorePreset: V1ExplorePreset, + partialExploreState: Partial, + curSearchParams: URLSearchParams, +) { + const newUrlSearchParams = new URLSearchParams( + convertExploreStateToURLSearchParams( + partialExploreState as MetricsExplorerEntity, + exploreSpec, + defaultExplorePreset, + ), + ); + curSearchParams.forEach((value, key) => { + if ( + key === ExploreStateURLParams.WebView && + FromURLParamViewMap[value] === defaultExplorePreset.view + ) { + // ignore default view. + // since we do not add params equal to default this will append to the end of the URL breaking the param order. + return; + } + newUrlSearchParams.set(key, value); + }); + return newUrlSearchParams.toString(); +} diff --git a/web-common/src/features/explores/selectors.ts b/web-common/src/features/explores/selectors.ts index dad6f242534..4578f0a56ff 100644 --- a/web-common/src/features/explores/selectors.ts +++ b/web-common/src/features/explores/selectors.ts @@ -1,4 +1,6 @@ import type { CreateQueryOptions, QueryFunction } from "@rilldata/svelte-query"; +import { restorePersistedDashboardState } from "@rilldata/web-common/features/dashboards/stores/dashboard-store-defaults"; +import { convertPresetToExploreState } from "@rilldata/web-common/features/dashboards/url-state/convertPresetToExploreState"; import { getDefaultExplorePreset } from "@rilldata/web-common/features/dashboards/url-state/getDefaultExplorePreset"; import { queryClient } from "@rilldata/web-common/lib/svelte-query/globalQueryClient"; import { @@ -75,6 +77,7 @@ export function useExploreValidSpec( export async function fetchExploreSpec( instanceId: string, exploreName: string, + prefix: string | undefined, ) { const queryParams = { name: exploreName, @@ -100,12 +103,13 @@ export async function fetchExploreSpec( throw error(404, "Metrics view not found"); } + const metricsViewSpec = + metricsViewResource.metricsView.state?.validSpec ?? {}; + const exploreSpec = exploreResource.explore.state?.validSpec ?? {}; + let fullTimeRange: V1MetricsViewTimeRangeResponse | undefined = undefined; - const metricsViewName = exploreResource.explore.state?.validSpec?.metricsView; - if ( - metricsViewResource.metricsView.state?.validSpec?.timeDimension && - metricsViewName - ) { + const metricsViewName = exploreSpec.metricsView; + if (metricsViewSpec.timeDimension && metricsViewName) { fullTimeRange = await queryClient.fetchQuery({ queryFn: () => queryServiceMetricsViewTimeRange(instanceId, metricsViewName, {}), @@ -120,13 +124,32 @@ export async function fetchExploreSpec( } const defaultExplorePreset = getDefaultExplorePreset( - exploreResource.explore.state?.validSpec ?? {}, + exploreSpec, fullTimeRange, ); + const { partialExploreState: initExploreState, errors } = + convertPresetToExploreState( + metricsViewSpec, + exploreSpec, + defaultExplorePreset, + ); + + let initLoadedOutsideOfURL = false; + const stateFromLocalStorage = restorePersistedDashboardState( + exploreSpec, + (prefix ?? "") + exploreName, + ); + if (stateFromLocalStorage) { + initLoadedOutsideOfURL = true; + Object.assign(initExploreState, stateFromLocalStorage); + } return { explore: exploreResource, metricsView: metricsViewResource, defaultExplorePreset, + initExploreState, + initLoadedOutsideOfURL, + errors, }; }