Fix misc issues

rilldata · Dec 17, 2024 · df6b483 · df6b483
1 parent 273a675
commit df6b483
Show file tree

Hide file tree

Showing 14 changed files with 72 additions and 44 deletions.
diff --git a/web-admin/src/features/dashboards/query-mappers/mapQueryToDashboard.ts b/web-admin/src/features/dashboards/query-mappers/mapQueryToDashboard.ts
@@ -5,7 +5,7 @@ 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";
@@ -128,7 +128,7 @@ export function mapQueryToDashboard(
         validSpecResp.data.explore,
         defaultExplorePreset,
       );
-      const defaultExploreState = getDefaultExploreState(
+      const defaultExploreState = getFullInitExploreState(
         metricsViewName,
         partialExploreState,
       );

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,4 +1,4 @@
-import { getExploreStores } from "@rilldata/web-common/features/explores/selectors";
+import { getExploreStates } from "@rilldata/web-common/features/explores/selectors";
 
 export const load = async ({ url, parent, params }) => {
   const { explore, metricsView, defaultExplorePreset, token } = await parent();
@@ -7,7 +7,7 @@ export const load = async ({ url, parent, params }) => {
   const metricsViewSpec = metricsView.metricsView?.state?.validSpec;
   const exploreSpec = explore.explore?.state?.validSpec;
 
-  return getExploreStores(
+  return getExploreStates(
     exploreName,
     `${organization}__${project}__`,
     url.searchParams,

diff --git a/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+page.ts b/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+page.ts
@@ -1,4 +1,4 @@
-import { getExploreStores } from "@rilldata/web-common/features/explores/selectors";
+import { getExploreStates } from "@rilldata/web-common/features/explores/selectors";
 
 export const load = async ({ url, parent, params }) => {
   const { explore, metricsView, defaultExplorePreset } = await parent();
@@ -8,7 +8,7 @@ export const load = async ({ url, parent, params }) => {
 
   return {
     exploreName,
-    ...getExploreStores(
+    ...getExploreStates(
       exploreName,
       `${organization}__${project}__`,
       url.searchParams,

diff --git a/web-common/src/features/dashboards/proto-state/proto.spec.ts b/web-common/src/features/dashboards/proto-state/proto.spec.ts
@@ -1,7 +1,7 @@
 import { protoBase64, Value } from "@bufbuild/protobuf";
 import { getDashboardStateFromUrl } from "@rilldata/web-common/features/dashboards/proto-state/fromProto";
 import { getProtoFromDashboardState } from "@rilldata/web-common/features/dashboards/proto-state/toProto";
-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 {
   createAndExpression,
   createInExpression,
@@ -40,7 +40,7 @@ describe("toProto/fromProto", () => {
   });
 
   it("backwards compatibility for time controls", () => {
-    const metricsExplorer = getDefaultExploreState(
+    const metricsExplorer = getFullInitExploreState(
       AD_BIDS_NAME,
       getInitExploreStateForTest(
         AD_BIDS_METRICS_INIT_WITH_TIME,

diff --git a/web-common/src/features/dashboards/proto-state/sparse-proto.spec.ts b/web-common/src/features/dashboards/proto-state/sparse-proto.spec.ts
@@ -1,5 +1,5 @@
 import { getProtoFromDashboardState } from "@rilldata/web-common/features/dashboards/proto-state/toProto";
-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 { metricsExplorerStore } from "@rilldata/web-common/features/dashboards/stores/dashboard-stores";
 import type { MetricsExplorerEntity } from "@rilldata/web-common/features/dashboards/stores/metrics-explorer-entity";
 import {
@@ -95,7 +95,7 @@ describe("sparse proto", () => {
   describe("should reset dashboard store", () => {
     for (const { title, mutations } of TestCases) {
       it(`from ${title}`, () => {
-        const dashboard = getDefaultExploreState(
+        const dashboard = getFullInitExploreState(
           AD_BIDS_EXPLORE_NAME,
           getInitExploreStateForTest(
             AD_BIDS_METRICS_INIT,

diff --git a/web-common/src/features/dashboards/stores/AdvancedMeasureCorrector.spec.ts b/web-common/src/features/dashboards/stores/AdvancedMeasureCorrector.spec.ts
@@ -3,7 +3,7 @@ import {
   MeasureFilterType,
 } from "@rilldata/web-common/features/dashboards/filters/measure-filters/measure-filter-options";
 import { AdvancedMeasureCorrector } from "@rilldata/web-common/features/dashboards/stores/AdvancedMeasureCorrector";
-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 { getInitExploreStateForTest } from "@rilldata/web-common/features/dashboards/stores/test-data/helpers";
 import type { DashboardTimeControls } from "@rilldata/web-common/lib/time/types";
 import {
@@ -35,7 +35,7 @@ describe("AdvancedMeasureCorrector", () => {
   } as V1ExploreSpec;
 
   it("changing grain while in TDD for measure based on timestamp", () => {
-    const dashboard = getDefaultExploreState(
+    const dashboard = getFullInitExploreState(
       "AdBids",
       getInitExploreStateForTest(MetricsView, Explore),
     );
@@ -70,7 +70,7 @@ describe("AdvancedMeasureCorrector", () => {
   });
 
   it("metrics view spec changed converting a measure to an advanced measure", () => {
-    const dashboard = getDefaultExploreState(
+    const dashboard = getFullInitExploreState(
       "AdBids",
       getInitExploreStateForTest(MetricsView, Explore),
     );

diff --git a/web-common/src/features/dashboards/stores/dashboard-store-defaults.ts b/web-common/src/features/dashboards/stores/dashboard-store-defaults.ts
@@ -5,9 +5,9 @@ import {
 } from "@rilldata/web-common/features/dashboards/stores/metrics-explorer-entity";
 
 // TODO: Remove this in favour of just `getBasePreset`
-export function getDefaultExploreState(
+export function getFullInitExploreState(
   name: string,
-  initState: Partial<MetricsExplorerEntity>,
+  partialInitState: Partial<MetricsExplorerEntity>,
 ): MetricsExplorerEntity {
   return {
     // fields filled here are the ones that are not stored and loaded to/from URL
@@ -20,6 +20,6 @@ export function getDefaultExploreState(
 
     lastDefinedScrubRange: undefined,
 
-    ...initState,
+    ...partialInitState,
   } as MetricsExplorerEntity;
 }
diff --git a/web-common/src/features/dashboards/stores/dashboard-stores.ts b/web-common/src/features/dashboards/stores/dashboard-stores.ts
@@ -3,7 +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 } from "@rilldata/web-common/features/dashboards/stores/dashboard-store-defaults";
+import { getFullInitExploreState } from "@rilldata/web-common/features/dashboards/stores/dashboard-store-defaults";
 import {
   createAndExpression,
   filterExpressions,
@@ -169,7 +169,7 @@ const metricsViewReducers = {
     update((state) => {
       if (state.entities[name]) return state;
 
-      state.entities[name] = getDefaultExploreState(name, initState);
+      state.entities[name] = getFullInitExploreState(name, initState);
 
       updateMetricsExplorerProto(state.entities[name]);
 

diff --git a/web-common/src/features/dashboards/url-state/DashboardURLStateSync.svelte b/web-common/src/features/dashboards/url-state/DashboardURLStateSync.svelte
@@ -15,6 +15,7 @@
   import type { HTTPError } from "@rilldata/web-common/runtime-client/fetchWrapper";
   import { runtime } from "@rilldata/web-common/runtime-client/runtime-store";
   import { onMount } from "svelte";
+  import { get } from "svelte/store";
 
   export let metricsViewName: string;
   export let exploreName: string;
@@ -62,9 +63,11 @@
       return;
     }
 
+    // Pressing back button and going to empty urls state should not restore from session store
+    const backButtonUsed = type === "popstate";
     let partialExplore = partialExploreStateFromUrl;
     let shouldUpdateUrl = false;
-    if (exploreStateFromSessionStorage) {
+    if (exploreStateFromSessionStorage && !backButtonUsed) {
       partialExplore = exploreStateFromSessionStorage;
       shouldUpdateUrl = true;
     }
@@ -124,19 +127,20 @@
       initState,
       $page.url.searchParams,
     );
+    // update session store to make sure updated to url or the initial state is propagated to the session store
+    updateExploreSessionStore(
+      exploreName,
+      extraKeyPrefix,
+      get(metricsExplorerStore).entities[exploreName],
+      exploreSpec!,
+    );
     prevUrl = redirectUrl.toString();
 
     if (!shouldUpdateUrl || redirectUrl.search === $page.url.search) {
       return;
     }
 
     replaceState(redirectUrl, $page.state);
-    updateExploreSessionStore(
-      exploreName,
-      extraKeyPrefix,
-      $dashboardStore,
-      exploreSpec!,
-    );
   }
 
   function gotoNewState() {
@@ -153,7 +157,9 @@
     const newUrl = u.toString();
     if (!prevUrl || prevUrl === newUrl) return;
 
+    // dashboard changed so we should update the url
     void goto(newUrl);
+    // also update the session store
     updateExploreSessionStore(
       exploreName,
       extraKeyPrefix,

diff --git a/web-common/src/features/dashboards/url-state/DashboardURLStateSyncWrapper.svelte b/web-common/src/features/dashboards/url-state/DashboardURLStateSyncWrapper.svelte
@@ -6,7 +6,7 @@
   import { convertPresetToExploreState } from "@rilldata/web-common/features/dashboards/url-state/convertPresetToExploreState";
   import DashboardURLStateSync from "@rilldata/web-common/features/dashboards/url-state/DashboardURLStateSync.svelte";
   import { getDefaultExplorePreset } from "@rilldata/web-common/features/dashboards/url-state/getDefaultExplorePreset";
-  import { getExploreStores } from "@rilldata/web-common/features/explores/selectors";
+  import { getExploreStates } from "@rilldata/web-common/features/explores/selectors";
   import type { V1ExplorePreset } from "@rilldata/web-common/runtime-client";
   import { runtime } from "@rilldata/web-common/runtime-client/runtime-store";
 
@@ -46,7 +46,7 @@
     | undefined = undefined;
   function parseUrl(url: URL, defaultExplorePreset: V1ExplorePreset) {
     ({ partialExploreStateFromUrl, exploreStateFromSessionStorage } =
-      getExploreStores(
+      getExploreStates(
         $exploreName,
         storeKeyPrefix,
         url.searchParams,

diff --git a/web-common/src/features/dashboards/url-state/convertExploreStateToPreset.ts b/web-common/src/features/dashboards/url-state/convertExploreStateToPreset.ts
@@ -13,6 +13,7 @@ import {
   ToURLParamTimeDimensionMap,
   ToURLParamTimeGrainMapMap,
 } from "@rilldata/web-common/features/dashboards/url-state/mappers";
+import { inferCompareTimeRange } from "@rilldata/web-common/lib/time/comparisons";
 import { DashboardState_LeaderboardSortDirection } from "@rilldata/web-common/proto/gen/rill/ui/v1/dashboard_pb";
 import {
   V1ExploreComparisonMode,
@@ -22,7 +23,7 @@ import {
 
 export function convertExploreStateToPreset(
   exploreState: Partial<MetricsExplorerEntity>,
-  explore: V1ExploreSpec,
+  exploreSpec: V1ExploreSpec,
 ) {
   const preset: V1ExplorePreset = {};
 
@@ -37,9 +38,9 @@ export function convertExploreStateToPreset(
     );
   }
 
-  Object.assign(preset, getTimeRangeFields(exploreState));
+  Object.assign(preset, getTimeRangeFields(exploreState, exploreSpec));
 
-  Object.assign(preset, getExploreFields(exploreState, explore));
+  Object.assign(preset, getExploreFields(exploreState, exploreSpec));
 
   Object.assign(preset, getTimeDimensionFields(exploreState));
 
@@ -48,7 +49,10 @@ export function convertExploreStateToPreset(
   return preset;
 }
 
-function getTimeRangeFields(exploreState: Partial<MetricsExplorerEntity>) {
+function getTimeRangeFields(
+  exploreState: Partial<MetricsExplorerEntity>,
+  exploreSpec: V1ExploreSpec,
+) {
   const preset: V1ExplorePreset = {};
 
   if (exploreState.selectedTimeRange?.name) {
@@ -59,15 +63,28 @@ function getTimeRangeFields(exploreState: Partial<MetricsExplorerEntity>) {
       ToURLParamTimeGrainMapMap[exploreState.selectedTimeRange.interval];
   }
 
-  if (
-    exploreState.showTimeComparison &&
-    exploreState.selectedComparisonTimeRange?.name
-  ) {
-    preset.compareTimeRange = toTimeRangeParam(
-      exploreState.selectedComparisonTimeRange,
-    );
-    preset.comparisonMode =
-      V1ExploreComparisonMode.EXPLORE_COMPARISON_MODE_TIME;
+  if (exploreState.showTimeComparison) {
+    if (exploreState.selectedComparisonTimeRange?.name) {
+      preset.compareTimeRange = toTimeRangeParam(
+        exploreState.selectedComparisonTimeRange,
+      );
+      preset.comparisonMode =
+        V1ExploreComparisonMode.EXPLORE_COMPARISON_MODE_TIME;
+    } else if (
+      !exploreState.selectedComparisonTimeRange &&
+      exploreState.selectedTimeRange?.name
+    ) {
+      // we infer compare time range if the user has not explicitly selected one but has enabled comparison
+      const inferredCompareTimeRange = inferCompareTimeRange(
+        exploreSpec.timeRanges,
+        exploreState.selectedTimeRange.name,
+      );
+      if (inferredCompareTimeRange) {
+        preset.compareTimeRange = inferredCompareTimeRange;
+        preset.comparisonMode =
+          V1ExploreComparisonMode.EXPLORE_COMPARISON_MODE_TIME;
+      }
+    }
   }
 
   if (exploreState.selectedComparisonDimension !== undefined) {
@@ -82,7 +99,10 @@ function getTimeRangeFields(exploreState: Partial<MetricsExplorerEntity>) {
     preset.timezone = exploreState.selectedTimezone;
   }
 
-  if (exploreState.selectedScrubRange) {
+  if (
+    exploreState.selectedScrubRange &&
+    !exploreState.selectedScrubRange?.isScrubbing
+  ) {
     preset.selectTimeRange = toTimeRangeParam(exploreState.selectedScrubRange);
   }
 

diff --git a/web-common/src/features/dashboards/url-state/convertPresetToExploreState.ts b/web-common/src/features/dashboards/url-state/convertPresetToExploreState.ts
@@ -174,6 +174,8 @@ function fromTimeRangesParams(
       ...fromTimeRangeUrlParam(preset.selectTimeRange),
       isScrubbing: false,
     };
+  } else {
+    partialExploreState.selectedScrubRange = undefined;
   }
 
   if (

diff --git a/web-common/src/features/explores/selectors.ts b/web-common/src/features/explores/selectors.ts
@@ -163,7 +163,7 @@ export async function fetchMetricsViewSchema(
   return schemaResp.schema ?? {};
 }
 
-export function getExploreStores(
+export function getExploreStates(
   exploreName: string,
   prefix: string | undefined,
   searchParams: URLSearchParams,

diff --git a/web-local/src/routes/(viz)/explore/[name]/+page.ts b/web-local/src/routes/(viz)/explore/[name]/+page.ts
@@ -1,4 +1,4 @@
-import { getExploreStores } from "@rilldata/web-common/features/explores/selectors";
+import { getExploreStates } from "@rilldata/web-common/features/explores/selectors";
 
 export const load = async ({ url, parent, params }) => {
   const { explore, metricsView, defaultExplorePreset } = await parent();
@@ -8,7 +8,7 @@ export const load = async ({ url, parent, params }) => {
 
   return {
     exploreName,
-    ...getExploreStores(
+    ...getExploreStates(
       exploreName,
       undefined,
       url.searchParams,