URL params UXQA items

rilldata · Dec 6, 2024 · e1bb6c0 · e1bb6c0
1 parent d3c4c5f
commit e1bb6c0
Show file tree

Hide file tree

Showing 13 changed files with 36 additions and 72 deletions.
diff --git a/web-admin/src/features/bookmarks/BookmarksDropdownMenuContent.svelte b/web-admin/src/features/bookmarks/BookmarksDropdownMenuContent.svelte
@@ -25,7 +25,6 @@
   import { useMetricsViewTimeRange } from "@rilldata/web-common/features/dashboards/selectors";
   import { useExploreState } from "@rilldata/web-common/features/dashboards/stores/dashboard-stores";
   import { getDefaultExplorePreset } from "@rilldata/web-common/features/dashboards/url-state/getDefaultExplorePreset";
-  import { getLocalUserPreferencesState } from "@rilldata/web-common/features/dashboards/user-preferences";
   import { ResourceKind } from "@rilldata/web-common/features/entity-management/resource-selectors";
   import { useExploreValidSpec } from "@rilldata/web-common/features/explores/selectors";
   import { runtime } from "@rilldata/web-common/runtime-client/runtime-store";
@@ -50,7 +49,6 @@
   );
   $: defaultExplorePreset = getDefaultExplorePreset(
     exploreSpec,
-    getLocalUserPreferencesState(exploreName),
     $metricsViewTimeRange.data,
   );
 

diff --git a/web-common/src/features/dashboards/state-managers/state-managers.ts b/web-common/src/features/dashboards/state-managers/state-managers.ts
@@ -9,10 +9,7 @@ import {
 } from "@rilldata/web-common/features/dashboards/stores/persistent-dashboard-state";
 import { updateExploreSessionStore } from "@rilldata/web-common/features/dashboards/url-state/explore-web-view-store";
 import { getDefaultExplorePreset } from "@rilldata/web-common/features/dashboards/url-state/getDefaultExplorePreset";
-import {
-  getLocalUserPreferencesState,
-  initLocalUserPreferenceStore,
-} from "@rilldata/web-common/features/dashboards/user-preferences";
+import { initLocalUserPreferenceStore } from "@rilldata/web-common/features/dashboards/user-preferences";
 import {
   type ExploreValidSpecResponse,
   useExploreValidSpec,
@@ -153,7 +150,6 @@ export function createStateManagers({
       }
       return getDefaultExplorePreset(
         validSpec.data?.explore ?? {},
-        getLocalUserPreferencesState(exploreName),
         timeRangeSummary.data,
       );
     },

diff --git a/web-common/src/features/dashboards/stores/dashboard-store-defaults.ts b/web-common/src/features/dashboards/stores/dashboard-store-defaults.ts
@@ -6,7 +6,6 @@ import {
 import { getPersistentDashboardState } 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 { getLocalUserPreferencesState } from "@rilldata/web-common/features/dashboards/user-preferences";
 import type {
   V1ExploreSpec,
   V1MetricsViewSpec,
@@ -19,11 +18,7 @@ export function getDefaultExploreState(
   metricsView: V1MetricsViewSpec,
   explore: V1ExploreSpec,
   fullTimeRange: V1MetricsViewTimeRangeResponse | undefined,
-  defaultExplorePreset = getDefaultExplorePreset(
-    explore,
-    getLocalUserPreferencesState(name),
-    fullTimeRange,
-  ),
+  defaultExplorePreset = getDefaultExplorePreset(explore, fullTimeRange),
 ): MetricsExplorerEntity {
   const { partialExploreState } = convertPresetToExploreState(
     metricsView,
@@ -77,5 +72,8 @@ export function restorePersistedDashboardState(
   if (persistedState.sortDirection) {
     metricsExplorer.sortDirection = persistedState.sortDirection;
   }
+
+  // const
+
   return metricsExplorer;
 }
diff --git a/web-common/src/features/dashboards/stores/test-data/data.ts b/web-common/src/features/dashboards/stores/test-data/data.ts
@@ -263,9 +263,6 @@ export const AD_BIDS_PIVOT_PRESET: V1ExplorePreset = {
 
 export const AD_BIDS_BASE_PRESET = getDefaultExplorePreset(
   AD_BIDS_EXPLORE_INIT,
-  {
-    timeZone: "UTC",
-  },
   undefined,
 );
 

diff --git a/web-common/src/features/dashboards/time-controls/time-control-store.spec.ts b/web-common/src/features/dashboards/time-controls/time-control-store.spec.ts
@@ -14,10 +14,7 @@ import {
   type TimeControlStore,
   createTimeControlStore,
 } from "@rilldata/web-common/features/dashboards/time-controls/time-control-store";
-import {
-  getLocalUserPreferences,
-  initLocalUserPreferenceStore,
-} from "@rilldata/web-common/features/dashboards/user-preferences";
+import { initLocalUserPreferenceStore } from "@rilldata/web-common/features/dashboards/user-preferences";
 import {
   TimeComparisonOption,
   TimeRangePreset,
@@ -28,7 +25,9 @@ import { V1TimeGrain } from "@rilldata/web-common/runtime-client";
 import { runtime } from "@rilldata/web-common/runtime-client/runtime-store";
 import { render } from "@testing-library/svelte";
 import { get } from "svelte/store";
-import { beforeAll, beforeEach, describe, expect, it } from "vitest";
+import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
+
+vi.stubEnv("TZ", "UTC");
 
 describe("time-control-store", () => {
   runtime.set({
@@ -43,11 +42,6 @@ describe("time-control-store", () => {
 
   beforeEach(() => {
     metricsExplorerStore.remove(AD_BIDS_EXPLORE_NAME);
-    getLocalUserPreferences().updateTimeZone("UTC");
-    localStorage.setItem(
-      `${AD_BIDS_EXPLORE_NAME}-userPreference`,
-      `{"timezone":"UTC"}`,
-    );
   });
 
   it("Switching from no timestamp column to having one", async () => {

diff --git a/web-common/src/features/dashboards/url-state/DashboardURLStateSyncWrapper.svelte b/web-common/src/features/dashboards/url-state/DashboardURLStateSyncWrapper.svelte
@@ -7,7 +7,6 @@
   import { convertURLToExploreState } 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 { getLocalUserPreferencesState } from "@rilldata/web-common/features/dashboards/user-preferences";
   import { shouldRedirectToViewWithParams } 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";
@@ -33,7 +32,6 @@
   );
   $: defaultExplorePreset = getDefaultExplorePreset(
     exploreSpec,
-    getLocalUserPreferencesState($exploreName),
     $metricsViewTimeRange.data,
   );
 

diff --git a/web-common/src/features/dashboards/url-state/convertExploreStateToURLSearchParams.ts b/web-common/src/features/dashboards/url-state/convertExploreStateToURLSearchParams.ts
@@ -102,7 +102,10 @@ function toTimeRangesUrl(
     );
   }
 
-  if (shouldSetParam(preset.timezone, exploreState.selectedTimezone)) {
+  if (
+    exploreSpec.timeZones?.length &&
+    shouldSetParam(preset.timezone, exploreState.selectedTimezone)
+  ) {
     searchParams.set(
       ExploreStateURLParams.TimeZone,
       exploreState.selectedTimezone,
@@ -141,7 +144,7 @@ function toTimeRangesUrl(
   const mappedTimeGrain =
     ToURLParamTimeGrainMapMap[exploreState.selectedTimeRange?.interval ?? ""] ??
     "";
-  if (shouldSetParam(preset.timeGrain, mappedTimeGrain)) {
+  if (mappedTimeGrain && shouldSetParam(preset.timeGrain, mappedTimeGrain)) {
     searchParams.set(ExploreStateURLParams.TimeGrain, mappedTimeGrain);
   }
 

diff --git a/web-common/src/features/dashboards/url-state/explore-web-view-store.spec.ts b/web-common/src/features/dashboards/url-state/explore-web-view-store.spec.ts
@@ -39,10 +39,7 @@ import {
   applyURLToExploreState,
   getCleanMetricsExploreForAssertion,
 } from "@rilldata/web-common/features/dashboards/url-state/url-state-variations.spec";
-import {
-  getLocalUserPreferences,
-  initLocalUserPreferenceStore,
-} from "@rilldata/web-common/features/dashboards/user-preferences";
+import { initLocalUserPreferenceStore } from "@rilldata/web-common/features/dashboards/user-preferences";
 import { useExploreValidSpec } from "@rilldata/web-common/features/explores/selectors";
 import { waitUntil } from "@rilldata/web-common/lib/waitUtils";
 import {
@@ -70,6 +67,7 @@ vi.mock("$app/stores", () => {
     page: pageMock,
   };
 });
+vi.stubEnv("TZ", "UTC");
 
 type TestView = {
   view: V1ExploreWebView;
@@ -130,7 +128,7 @@ const TestCases: {
         AD_BIDS_SORT_PIVOT_BY_TIME_DAY_ASC,
       ],
       expectedUrl:
-        "http://localhost/explore/AdBids_explore?view=pivot&tr=P7D&compare_tr=rill-PP&grain=day&f=publisher+IN+%28%27Google%27%29&rows=publisher%2Ctime.hour&cols=domain%2Ctime.day%2Cimpressions&sort_by=time.day&sort_dir=ASC",
+        "http://localhost/explore/AdBids_explore?view=pivot&tr=P7D&compare_tr=rill-PP&f=publisher+IN+%28%27Google%27%29&rows=publisher%2Ctime.hour&cols=domain%2Ctime.day%2Cimpressions&sort_by=time.day&sort_dir=ASC",
     },
   },
   {
@@ -148,7 +146,7 @@ const TestCases: {
         AD_BIDS_SORT_PIVOT_BY_TIME_DAY_ASC,
       ],
       expectedUrl:
-        "http://localhost/explore/AdBids_explore?view=pivot&tr=P7D&compare_tr=rill-PP&grain=day&f=publisher+IN+%28%27Google%27%29&rows=publisher%2Ctime.hour&cols=domain%2Ctime.day%2Cimpressions&sort_by=time.day&sort_dir=ASC",
+        "http://localhost/explore/AdBids_explore?view=pivot&tr=P7D&compare_tr=rill-PP&f=publisher+IN+%28%27Google%27%29&rows=publisher%2Ctime.hour&cols=domain%2Ctime.day%2Cimpressions&sort_by=time.day&sort_dir=ASC",
     },
   },
   {
@@ -167,7 +165,7 @@ const TestCases: {
         AD_BIDS_SORT_PIVOT_BY_TIME_DAY_ASC,
       ],
       expectedUrl:
-        "http://localhost/explore/AdBids_explore?view=pivot&tr=P7D&compare_tr=rill-PP&grain=day&f=publisher+IN+%28%27Google%27%29&rows=publisher%2Ctime.hour&cols=domain%2Ctime.day%2Cimpressions&sort_by=time.day&sort_dir=ASC",
+        "http://localhost/explore/AdBids_explore?view=pivot&tr=P7D&compare_tr=rill-PP&f=publisher+IN+%28%27Google%27%29&rows=publisher%2Ctime.hour&cols=domain%2Ctime.day%2Cimpressions&sort_by=time.day&sort_dir=ASC",
     },
   },
 ];
@@ -223,11 +221,6 @@ describe("ExploreWebViewStore", () => {
 
   beforeEach(() => {
     metricsExplorerStore.remove(AD_BIDS_EXPLORE_NAME);
-    getLocalUserPreferences().updateTimeZone("UTC");
-    localStorage.setItem(
-      `${AD_BIDS_EXPLORE_NAME}-userPreference`,
-      `{"timezone":"UTC"}`,
-    );
     clearExploreSessionStore(AD_BIDS_NAME, undefined);
   });
 
@@ -246,9 +239,6 @@ describe("ExploreWebViewStore", () => {
       });
       const defaultExplorePreset = getDefaultExplorePreset(
         AD_BIDS_EXPLORE_INIT,
-        {
-          timeZone: "UTC",
-        },
         AD_BIDS_TIME_RANGE_SUMMARY,
       );
 

diff --git a/web-common/src/features/dashboards/url-state/explore-web-view-store.ts b/web-common/src/features/dashboards/url-state/explore-web-view-store.ts
@@ -17,17 +17,19 @@ const ExploreViewKeys: Record<V1ExploreWebView, (keyof V1ExplorePreset)[]> = {
     "view",
     "measures",
     "dimensions",
+    "timeGrain",
+    "comparisonDimension",
     "exploreExpandedDimension",
     "exploreSortBy",
     "exploreSortAsc",
     "exploreSortType",
-    "comparisonDimension",
   ],
   [V1ExploreWebView.EXPLORE_WEB_VIEW_TIME_DIMENSION]: [
     "view",
     "timeDimensionMeasure",
     "timeDimensionChartType",
     "timeDimensionPin",
+    "timeGrain",
     "comparisonDimension",
   ],
   [V1ExploreWebView.EXPLORE_WEB_VIEW_PIVOT]: [
@@ -126,8 +128,7 @@ export function updateExploreSessionStore(
     if (!sharedKeys?.length) continue;
 
     const otherViewKey = getKeyForSessionStore(exploreName, prefix, otherView);
-    const otherViewRawPreset = sessionStorage.getItem(otherViewKey);
-    if (!otherViewRawPreset) continue;
+    const otherViewRawPreset = sessionStorage.getItem(otherViewKey) ?? "{}";
 
     try {
       const otherViewPreset = JSON.parse(otherViewRawPreset) as V1ExplorePreset;

diff --git a/web-common/src/features/dashboards/url-state/getDefaultExplorePreset.ts b/web-common/src/features/dashboards/url-state/getDefaultExplorePreset.ts
@@ -9,6 +9,7 @@ import type { LocalUserPreferences } from "@rilldata/web-common/features/dashboa
 import { inferCompareTimeRange } from "@rilldata/web-common/lib/time/comparisons";
 import { ISODurationToTimePreset } from "@rilldata/web-common/lib/time/ranges";
 import { isoDurationToFullTimeRange } from "@rilldata/web-common/lib/time/ranges/iso-ranges";
+import { getLocalIANA } from "@rilldata/web-common/lib/time/timezone";
 import {
   V1ExploreComparisonMode,
   V1ExploreSortType,
@@ -20,7 +21,6 @@ import {
 
 export function getDefaultExplorePreset(
   explore: V1ExploreSpec,
-  preferences: LocalUserPreferences,
   fullTimeRange: V1MetricsViewTimeRangeResponse | undefined,
 ) {
   const defaultExplorePreset: V1ExplorePreset = {
@@ -31,7 +31,7 @@ export function getDefaultExplorePreset(
     dimensions: explore.dimensions,
 
     timeRange: fullTimeRange ? "inf" : "",
-    timezone: preferences.timeZone ?? "UTC",
+    timezone: explore.defaultPreset?.timezone ?? getLocalIANA(),
     timeGrain: "",
     comparisonMode: V1ExploreComparisonMode.EXPLORE_COMPARISON_MODE_NONE,
     compareTimeRange: "",

diff --git a/web-common/src/features/dashboards/url-state/invalid-url-state-variations.spec.ts b/web-common/src/features/dashboards/url-state/invalid-url-state-variations.spec.ts
@@ -158,9 +158,6 @@ describe("Invalid Human readable URL State", () => {
       const initState = getCleanMetricsExploreForAssertion();
       const defaultExplorePreset = getDefaultExplorePreset(
         AD_BIDS_EXPLORE_INIT,
-        {
-          timeZone: "UTC",
-        },
         AD_BIDS_TIME_RANGE_SUMMARY,
       );
 

diff --git a/web-common/src/features/dashboards/url-state/url-state-variations.spec.ts b/web-common/src/features/dashboards/url-state/url-state-variations.spec.ts
@@ -44,18 +44,17 @@ import {
 import { convertExploreStateToURLSearchParams } from "@rilldata/web-common/features/dashboards/url-state/convertExploreStateToURLSearchParams";
 import { convertURLToExploreState } from "@rilldata/web-common/features/dashboards/url-state/convertPresetToExploreState";
 import { getDefaultExplorePreset } from "@rilldata/web-common/features/dashboards/url-state/getDefaultExplorePreset";
-import {
-  getLocalUserPreferences,
-  initLocalUserPreferenceStore,
-} from "@rilldata/web-common/features/dashboards/user-preferences";
+import { initLocalUserPreferenceStore } from "@rilldata/web-common/features/dashboards/user-preferences";
 import type { DashboardTimeControls } from "@rilldata/web-common/lib/time/types";
 import {
   type V1ExplorePreset,
   type V1ExploreSpec,
 } from "@rilldata/web-common/runtime-client";
 import { deepClone } from "@vitest/utils";
 import { get } from "svelte/store";
-import { beforeAll, beforeEach, describe, expect, it } from "vitest";
+import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
+
+vi.stubEnv("TZ", "UTC");
 
 const TestCases: {
   title: string;
@@ -331,11 +330,6 @@ describe("Human readable URL state variations", () => {
 
   beforeEach(() => {
     metricsExplorerStore.remove(AD_BIDS_EXPLORE_NAME);
-    getLocalUserPreferences().updateTimeZone("UTC");
-    localStorage.setItem(
-      `${AD_BIDS_EXPLORE_NAME}-userPreference`,
-      `{"timezone":"UTC"}`,
-    );
   });
 
   describe("Should update url state and restore default state on empty params", () => {
@@ -344,6 +338,7 @@ describe("Human readable URL state variations", () => {
         const explore: V1ExploreSpec = {
           ...AD_BIDS_EXPLORE_INIT,
           ...(preset ? { defaultPreset: preset } : {}),
+          timeZones: ["UTC", "Asia/Kathmandu"],
         };
         metricsExplorerStore.init(
           AD_BIDS_EXPLORE_NAME,
@@ -354,9 +349,6 @@ describe("Human readable URL state variations", () => {
         const initState = getCleanMetricsExploreForAssertion();
         const defaultExplorePreset = getDefaultExplorePreset(
           explore,
-          {
-            timeZone: "UTC",
-          },
           AD_BIDS_TIME_RANGE_SUMMARY,
         );
 
@@ -402,9 +394,6 @@ describe("Human readable URL state variations", () => {
         );
         const defaultExplorePreset = getDefaultExplorePreset(
           explore,
-          {
-            timeZone: "UTC",
-          },
           AD_BIDS_TIME_RANGE_SUMMARY,
         );
 

diff --git a/web-common/src/features/explores/selectors.ts b/web-common/src/features/explores/selectors.ts
@@ -9,7 +9,6 @@ import { getExplorePresetForWebView } from "@rilldata/web-common/features/dashbo
 import { getDefaultExplorePreset } from "@rilldata/web-common/features/dashboards/url-state/getDefaultExplorePreset";
 import { FromURLParamViewMap } from "@rilldata/web-common/features/dashboards/url-state/mappers";
 import { ExploreStateURLParams } from "@rilldata/web-common/features/dashboards/url-state/url-params";
-import { getLocalUserPreferencesState } from "@rilldata/web-common/features/dashboards/user-preferences";
 import { queryClient } from "@rilldata/web-common/lib/svelte-query/globalQueryClient";
 import {
   createRuntimeServiceGetExplore,
@@ -133,7 +132,6 @@ export async function fetchExploreSpec(
 
   const defaultExplorePreset = getDefaultExplorePreset(
     exploreResource.explore.state?.validSpec ?? {},
-    getLocalUserPreferencesState(exploreName),
     fullTimeRange,
   );
 
@@ -239,7 +237,12 @@ export function shouldRedirectToViewWithParams(
     defaultExplorePreset,
   );
   // copy over any partial params. this will include the view and measure param
-  url.searchParams.forEach((value, key) => newUrl.searchParams.set(key, value));
+  url.searchParams.forEach((value, key) => {
+    // ignore `view` param since it is already part of partialExploreState
+    // in case `view` was same as default adding it here would mess up the order.
+    if (key === ExploreStateURLParams.WebView) return;
+    newUrl.searchParams.set(key, value);
+  });
   if (newUrl.toString() === url.toString()) {
     // url hasn't changed, avoid redirect loop
     return;