From e1bb6c0ee5c4955c2821ba98fedef4c0af2bc980 Mon Sep 17 00:00:00 2001 From: Aditya Hegde Date: Fri, 6 Dec 2024 16:34:37 +0530 Subject: [PATCH] URL params UXQA items --- .../BookmarksDropdownMenuContent.svelte | 2 -- .../state-managers/state-managers.ts | 6 +----- .../stores/dashboard-store-defaults.ts | 10 ++++----- .../dashboards/stores/test-data/data.ts | 3 --- .../time-controls/time-control-store.spec.ts | 14 ++++--------- .../DashboardURLStateSyncWrapper.svelte | 2 -- .../convertExploreStateToURLSearchParams.ts | 7 +++++-- .../url-state/explore-web-view-store.spec.ts | 20 +++++------------- .../url-state/explore-web-view-store.ts | 7 ++++--- .../url-state/getDefaultExplorePreset.ts | 4 ++-- .../invalid-url-state-variations.spec.ts | 3 --- .../url-state/url-state-variations.spec.ts | 21 +++++-------------- web-common/src/features/explores/selectors.ts | 9 +++++--- 13 files changed, 36 insertions(+), 72 deletions(-) diff --git a/web-admin/src/features/bookmarks/BookmarksDropdownMenuContent.svelte b/web-admin/src/features/bookmarks/BookmarksDropdownMenuContent.svelte index 6959b3b0b25..660e7e26ccd 100644 --- 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 index 5b56aed69d6..da823ec7a74 100644 --- 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 index 4fb751bfa60..5f6ef83d17f 100644 --- 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 index 405b3fa2001..18a707952ed 100644 --- 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 index 666ba39113b..3128addf122 100644 --- 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 index cfcd8d35976..a66a528af73 100644 --- 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 index 554ce0e902c..dce27b5f553 100644 --- 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 index 35dc218ba89..c2e7a878aa3 100644 --- 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 index 43bf226a428..68a4ff2d4cf 100644 --- 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 = { "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 index 4957a047910..4c8a8880868 100644 --- 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 index e183992b679..b5045f486bf 100644 --- 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 index a944a1a8095..23db95f7b3f 100644 --- 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,10 +44,7 @@ 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, @@ -55,7 +52,9 @@ import { } 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 index cffd99dd841..9c78bce40a0 100644 --- 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;