Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 25 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
234b304
Use history API to redirect URL
AdityaHegde Dec 9, 2024
ddb1aa1
Fix issues with cloud
AdityaHegde Dec 9, 2024
010a89f
Merge branch 'main' into fix/url-history-issues
AdityaHegde Dec 9, 2024
66d545a
Fix tests
AdityaHegde Dec 9, 2024
6250069
Fix race condition in reactive statement
AdityaHegde Dec 10, 2024
97ac340
Disable TTD for no timeseries dashboards
AdityaHegde Dec 10, 2024
1a46aca
Update rill-dev
AdityaHegde Dec 10, 2024
fbe200d
Update token and emebd
AdityaHegde Dec 10, 2024
fc5c5f8
Merge branch 'main' into fix/url-history-issues
AdityaHegde Dec 10, 2024
30b0075
Remove console.log
AdityaHegde Dec 10, 2024
d88e4ba
Reset expore state when metrics view is changed
AdityaHegde Dec 10, 2024
6b526ae
Move all state loading decision to DashboardURLStateSync
AdityaHegde Dec 12, 2024
7c3ce36
Fix race condition on cloud
AdityaHegde Dec 12, 2024
3fc4354
Update tests
AdityaHegde Dec 12, 2024
fa2f85e
Intercept url in beforeNavigate
AdityaHegde Dec 12, 2024
c9b0fe9
Use goto with replaceState instead of beforeNavigate
AdityaHegde Dec 13, 2024
9cafbd1
Fix missing home bookmark while siwtching using breadcrumbs
AdityaHegde Dec 13, 2024
14d76a9
Fix CI
AdityaHegde Dec 13, 2024
a902ded
Rename initState before Sync component
AdityaHegde Dec 16, 2024
273a675
Add schema requests
AdityaHegde Dec 16, 2024
df6b483
Fix misc issues
AdityaHegde Dec 17, 2024
8f3b759
Fix explore preview to not retain state from visual editor
AdityaHegde Dec 17, 2024
6e127c9
Merge branch 'main' into fix/url-history-issues
AdityaHegde Dec 17, 2024
33a0ac9
Fix timeranges with invalid comparison
AdityaHegde Dec 17, 2024
dfd93a8
Use token id as key
AdityaHegde Dec 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -51,6 +52,10 @@
exploreSpec,
$metricsViewTimeRange.data,
);
$: schemaResp = createQueryServiceMetricsViewSchema(
$runtime.instanceId,
metricsViewName,
);

$: projectIdResp = useProjectId(organization, project);
const userResp = createAdminServiceGetCurrentUser();
Expand All @@ -72,7 +77,7 @@
$bookamrksResp.data?.bookmarks ?? [],
metricsViewSpec,
exploreSpec,
{},
$schemaResp.data?.schema,
$exploreState,
defaultExplorePreset,
);
Expand Down
1 change: 1 addition & 0 deletions web-admin/src/features/bookmarks/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export function categorizeBookmarks(
personal: [],
shared: [],
};
if (!exploreState) return bookmarks;
bookmarkResp?.forEach((bookmarkResource) => {
const bookmark = parseBookmark(
bookmarkResource,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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 }) => {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@

$: ({
defaultExplorePreset,
partialExploreState,
token: { resourceName },
tokenExploreState,
exploreStateFromYAMLConfig,
partialExploreStateFromUrl,
exploreStateFromSessionStorage,
token: { resourceName, id: tokenId },
} = data);
$: ({ organization, project } = $page.params);

Expand Down Expand Up @@ -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}
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
);
};
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand All @@ -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: () => {
Expand Down Expand Up @@ -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>
Expand Down
Loading
Loading