fix: session storage retrieval for url messes with the url history (#…

…6238) * Use history API to redirect URL * Fix issues with cloud * Fix tests * Fix race condition in reactive statement * Disable TTD for no timeseries dashboards * Update rill-dev * Update token and emebd * Remove console.log * Reset expore state when metrics view is changed * Move all state loading decision to DashboardURLStateSync * Fix race condition on cloud * Update tests * Intercept url in beforeNavigate * Use goto with replaceState instead of beforeNavigate * Fix missing home bookmark while siwtching using breadcrumbs * Fix CI * Rename initState before Sync component * Add schema requests * Fix misc issues * Fix explore preview to not retain state from visual editor * Fix timeranges with invalid comparison * Use token id as key
rilldata · Dec 17, 2024 · e5ffc5d · e5ffc5d · github-actions · Dec 17, 2024
1 parent 8e7bca1
commit e5ffc5d
Show file tree

Hide file tree

Showing 45 changed files with 808 additions and 764 deletions.
diff --git a/web-admin/src/features/bookmarks/BookmarksDropdownMenuContent.svelte b/web-admin/src/features/bookmarks/BookmarksDropdownMenuContent.svelte
@@ -27,6 +27,7 @@
   import { getDefaultExplorePreset } from "@rilldata/web-common/features/dashboards/url-state/getDefaultExplorePreset";
   import { ResourceKind } from "@rilldata/web-common/features/entity-management/resource-selectors";
   import { useExploreValidSpec } from "@rilldata/web-common/features/explores/selectors";
+  import { createQueryServiceMetricsViewSchema } from "@rilldata/web-common/runtime-client";
   import { runtime } from "@rilldata/web-common/runtime-client/runtime-store";
   import { BookmarkPlusIcon } from "lucide-svelte";
   import { createEventDispatcher } from "svelte";
@@ -51,6 +52,10 @@
     exploreSpec,
     $metricsViewTimeRange.data,
   );
+  $: schemaResp = createQueryServiceMetricsViewSchema(
+    $runtime.instanceId,
+    metricsViewName,
+  );
 
   $: projectIdResp = useProjectId(organization, project);
   const userResp = createAdminServiceGetCurrentUser();
@@ -72,7 +77,7 @@
     $bookamrksResp.data?.bookmarks ?? [],
     metricsViewSpec,
     exploreSpec,
-    {},
+    $schemaResp.data?.schema,
     $exploreState,
     defaultExplorePreset,
   );

diff --git 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/features/dashboards/query-mappers/mapQueryToDashboard.ts b/web-admin/src/features/dashboards/query-mappers/mapQueryToDashboard.ts
@@ -5,8 +5,10 @@ import type {
   QueryRequests,
 } from "@rilldata/web-admin/features/dashboards/query-mappers/types";
 import type { CompoundQueryResult } from "@rilldata/web-common/features/compound-query-result";
-import { getDefaultExploreState } from "@rilldata/web-common/features/dashboards/stores/dashboard-store-defaults";
+import { getFullInitExploreState } from "@rilldata/web-common/features/dashboards/stores/dashboard-store-defaults";
 import type { MetricsExplorerEntity } from "@rilldata/web-common/features/dashboards/stores/metrics-explorer-entity";
+import { convertPresetToExploreState } from "@rilldata/web-common/features/dashboards/url-state/convertPresetToExploreState";
+import { getDefaultExplorePreset } from "@rilldata/web-common/features/dashboards/url-state/getDefaultExplorePreset";
 import { initLocalUserPreferenceStore } from "@rilldata/web-common/features/dashboards/user-preferences";
 import { useExploreValidSpec } from "@rilldata/web-common/features/explores/selectors";
 import { queryClient } from "@rilldata/web-common/lib/svelte-query/globalQueryClient";
@@ -117,12 +119,19 @@ export function mapQueryToDashboard(
       const { metricsView, explore } = validSpecResp.data;
 
       initLocalUserPreferenceStore(metricsViewName);
-      const defaultExploreState = getDefaultExploreState(
-        metricsViewName,
-        metricsView,
-        explore,
+      const defaultExplorePreset = getDefaultExplorePreset(
+        validSpecResp.data.explore,
         timeRangeSummary.data,
       );
+      const { partialExploreState } = convertPresetToExploreState(
+        validSpecResp.data.metricsView,
+        validSpecResp.data.explore,
+        defaultExplorePreset,
+      );
+      const defaultExploreState = getFullInitExploreState(
+        metricsViewName,
+        partialExploreState,
+      );
       getDashboardState({
         queryClient,
         instanceId,

diff --git a/web-admin/src/routes/[organization]/[project]/-/share/[token]/+layout.ts b/web-admin/src/routes/[organization]/[project]/-/share/[token]/+layout.ts
@@ -1,5 +1,10 @@
 import { fetchMagicAuthToken } from "@rilldata/web-admin/features/projects/selectors";
-import { fetchExploreSpec } from "@rilldata/web-common/features/explores/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 {
+  fetchExploreSpec,
+  fetchMetricsViewSchema,
+} from "@rilldata/web-common/features/explores/selectors";
 import { error } from "@sveltejs/kit";
 
 export const load = async ({ params: { token }, parent }) => {
@@ -12,16 +17,38 @@ export const load = async ({ params: { token }, parent }) => {
       throw new Error("Token does not have an associated resource name");
     }
 
-    const { explore, metricsView, defaultExplorePreset } =
-      await fetchExploreSpec(
-        runtime.instanceId as string,
-        tokenData.token.resourceName,
+    const exploreName = tokenData.token?.resourceName;
+
+    const {
+      explore,
+      metricsView,
+      defaultExplorePreset,
+      exploreStateFromYAMLConfig,
+    } = await fetchExploreSpec(runtime?.instanceId, exploreName);
+    const metricsViewSpec = metricsView.metricsView?.state?.validSpec ?? {};
+    const exploreSpec = explore.explore?.state?.validSpec ?? {};
+
+    let tokenExploreState: Partial<MetricsExplorerEntity> | undefined =
+      undefined;
+    if (tokenData.token?.state) {
+      const schema = await fetchMetricsViewSchema(
+        runtime?.instanceId,
+        exploreSpec.metricsView ?? "",
+      );
+      tokenExploreState = getDashboardStateFromUrl(
+        tokenData.token?.state,
+        metricsViewSpec,
+        exploreSpec,
+        schema,
       );
+    }
 
     return {
       explore,
       metricsView,
       defaultExplorePreset,
+      exploreStateFromYAMLConfig,
+      tokenExploreState,
       token: tokenData?.token,
     };
   } catch (e) {

diff --git a/web-admin/src/routes/[organization]/[project]/-/share/[token]/+page.svelte b/web-admin/src/routes/[organization]/[project]/-/share/[token]/+page.svelte
@@ -15,8 +15,11 @@
 
   $: ({
     defaultExplorePreset,
-    partialExploreState,
-    token: { resourceName },
+    tokenExploreState,
+    exploreStateFromYAMLConfig,
+    partialExploreStateFromUrl,
+    exploreStateFromSessionStorage,
+    token: { resourceName, id: tokenId },
   } = data);
   $: ({ organization, project } = $page.params);
 
@@ -55,7 +58,16 @@
       metricsViewName={explore.metricsView.meta.name.name}
       exploreName={resourceName}
     >
-      <DashboardURLStateSync {defaultExplorePreset} {partialExploreState}>
+      <DashboardURLStateSync
+        metricsViewName={explore.metricsView.meta.name.name}
+        exploreName={resourceName}
+        extraKeyPrefix={`${tokenId}__`}
+        {defaultExplorePreset}
+        initExploreState={tokenExploreState}
+        {exploreStateFromYAMLConfig}
+        {partialExploreStateFromUrl}
+        {exploreStateFromSessionStorage}
+      >
         <DashboardThemeProvider>
           <Dashboard
             exploreName={resourceName}

diff --git a/web-admin/src/routes/[organization]/[project]/-/share/[token]/+page.ts b/web-admin/src/routes/[organization]/[project]/-/share/[token]/+page.ts
@@ -1,57 +1,17 @@
-import { getDashboardStateFromUrl } from "@rilldata/web-common/features/dashboards/proto-state/fromProto";
-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 { convertURLToExploreState } from "@rilldata/web-common/features/dashboards/url-state/convertPresetToExploreState";
-import { redirect } from "@sveltejs/kit";
-import { get } from "svelte/store";
+import { getExploreStates } from "@rilldata/web-common/features/explores/selectors";
 
 export const load = async ({ url, parent }) => {
   const { explore, metricsView, defaultExplorePreset, token } = await parent();
-  const exploreName = token.resourceName;
+  const exploreName = token?.resourceName;
   const metricsViewSpec = metricsView.metricsView?.state?.validSpec;
   const exploreSpec = explore.explore?.state?.validSpec;
 
-  // On the first dashboard load, if there are no URL params, append the token's state (in human-readable format) to the URL.
-  if (
-    token.state &&
-    ![...url.searchParams.keys()].length &&
-    !(exploreName in get(metricsExplorerStore).entities)
-  ) {
-    const exploreState = getDashboardStateFromUrl(
-      token.state,
-      metricsViewSpec,
-      exploreSpec,
-      {}, // TODO
-    );
-    const newUrl = new URL(url);
-    newUrl.search = convertExploreStateToURLSearchParams(
-      exploreState as MetricsExplorerEntity,
-      exploreSpec,
-      defaultExplorePreset,
-    );
-    throw redirect(307, `${newUrl.pathname}${newUrl.search}`);
-  }
-
-  // Get Explore state from URL params
-  let partialExploreState: Partial<MetricsExplorerEntity> = {};
-  const errors: Error[] = [];
-  if (metricsViewSpec && exploreSpec) {
-    const {
-      partialExploreState: partialExploreStateFromUrl,
-      errors: errorsFromConvert,
-    } = convertURLToExploreState(
-      url.searchParams,
-      metricsViewSpec,
-      exploreSpec,
-      defaultExplorePreset,
-    );
-    partialExploreState = partialExploreStateFromUrl;
-    errors.push(...errorsFromConvert);
-  }
-
-  return {
-    partialExploreState,
-    errors,
-  };
+  return getExploreStates(
+    exploreName,
+    `${token.id}__`,
+    url.searchParams,
+    metricsViewSpec,
+    exploreSpec,
+    defaultExplorePreset,
+  );
 };
diff --git a/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+layout.ts b/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+layout.ts
@@ -1,8 +1,14 @@
+import type { V1Bookmark } from "@rilldata/web-admin/client";
 import {
   fetchBookmarks,
   isHomeBookmark,
 } from "@rilldata/web-admin/features/bookmarks/selectors";
-import { fetchExploreSpec } from "@rilldata/web-common/features/explores/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 {
+  fetchExploreSpec,
+  fetchMetricsViewSchema,
+} from "@rilldata/web-common/features/explores/selectors";
 import {
   type V1ExplorePreset,
   type V1Resource,
@@ -11,31 +17,69 @@ import {
 export const load = async ({ params, depends, parent }) => {
   const { project, runtime } = await parent();
 
-  const exploreName = params.dashboard;
+  const { dashboard: exploreName } = params;
 
   depends(exploreName, "explore");
 
-  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);
+  let explore: V1Resource | undefined;
+  let metricsView: V1Resource | undefined;
+  let defaultExplorePreset: V1ExplorePreset | undefined;
+  let exploreStateFromYAMLConfig: Partial<MetricsExplorerEntity> = {};
+  let bookmarks: V1Bookmark[] | undefined;
 
-    return {
-      explore,
-      metricsView,
-      defaultExplorePreset,
-      homeBookmark: bookmarks.find(isHomeBookmark),
-    };
+  try {
+    [
+      {
+        explore,
+        metricsView,
+        defaultExplorePreset,
+        exploreStateFromYAMLConfig,
+      },
+      bookmarks,
+    ] = await Promise.all([
+      fetchExploreSpec(runtime?.instanceId, exploreName),
+      fetchBookmarks(project.id, exploreName),
+    ]);
   } catch {
     // error handled in +page.svelte for now
     // TODO: move it here
     return {
       explore: <V1Resource>{},
       metricsView: <V1Resource>{},
       defaultExplorePreset: <V1ExplorePreset>{},
-      homeBookmark: undefined,
+      exploreStateFromYAMLConfig,
     };
   }
+
+  const metricsViewSpec = metricsView.metricsView?.state?.validSpec ?? {};
+  const exploreSpec = explore.explore?.state?.validSpec ?? {};
+
+  let homeBookmarkExploreState: Partial<MetricsExplorerEntity> | undefined =
+    undefined;
+  try {
+    const homeBookmark = bookmarks.find(isHomeBookmark);
+    const schema = await fetchMetricsViewSchema(
+      runtime?.instanceId,
+      exploreSpec.metricsView ?? "",
+    );
+
+    if (homeBookmark) {
+      homeBookmarkExploreState = getDashboardStateFromUrl(
+        homeBookmark.data ?? "",
+        metricsViewSpec,
+        exploreSpec,
+        schema,
+      );
+    }
+  } catch {
+    // TODO
+  }
+
+  return {
+    explore,
+    metricsView,
+    defaultExplorePreset,
+    exploreStateFromYAMLConfig,
+    homeBookmarkExploreState,
+  };
 };
diff --git a/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+page.svelte b/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+page.svelte
@@ -19,12 +19,21 @@
   // const PollIntervalWhenDashboardOk = 60000; // This triggers a layout shift, so removing for now
 
   export let data: PageData;
-  $: ({ defaultExplorePreset, partialExploreState, errors } = data);
+  $: ({
+    defaultExplorePreset,
+    homeBookmarkExploreState,
+    exploreStateFromYAMLConfig,
+    partialExploreStateFromUrl,
+    exploreStateFromSessionStorage,
+    errors,
+    exploreName,
+  } = data);
   $: if (errors?.length) {
+    const _errs = errors;
     setTimeout(() => {
       eventBus.emit("notification", {
         type: "error",
-        message: errors[0].message,
+        message: _errs[0].message,
         options: {
           persisted: true,
         },
@@ -33,11 +42,7 @@
   }
   $: instanceId = $runtime?.instanceId;
 
-  $: ({
-    organization: orgName,
-    project: projectName,
-    dashboard: exploreName,
-  } = $page.params);
+  $: ({ organization: orgName, project: projectName } = $page.params);
 
   $: explore = useExplore(instanceId, exploreName, {
     refetchInterval: () => {
@@ -98,7 +103,16 @@
   {:else if metricsViewName}
     {#key exploreName}
       <StateManagersProvider {metricsViewName} {exploreName}>
-        <DashboardURLStateSync {defaultExplorePreset} {partialExploreState}>
+        <DashboardURLStateSync
+          {metricsViewName}
+          {exploreName}
+          extraKeyPrefix={`${orgName}__${projectName}__`}
+          {defaultExplorePreset}
+          initExploreState={homeBookmarkExploreState}
+          {exploreStateFromYAMLConfig}
+          {partialExploreStateFromUrl}
+          {exploreStateFromSessionStorage}
+        >
           <DashboardThemeProvider>
             <Dashboard {metricsViewName} {exploreName} />
           </DashboardThemeProvider>