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: narrow generated API types #5735

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
19 changes: 18 additions & 1 deletion proto/gen/rill/runtime/v1/runtime.swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2862,6 +2862,8 @@ definitions:
title: Selector for a dimension
MetricsViewSpecDimensionV2:
type: object
required:
- name
properties:
name:
type: string
Expand Down Expand Up @@ -2889,6 +2891,9 @@ definitions:
title: Type of measure query to generate
MetricsViewSpecMeasureV2:
type: object
required:
- name
- expression
properties:
name:
type: string
Expand Down Expand Up @@ -4715,8 +4720,12 @@ definitions:
title: Response message for QueryService.MetricsViewComparison
v1MetricsViewComparisonRow:
type: object
required:
- dimensionValue
- measureValues
properties:
dimensionValue:
type: string
title: Not optional, not null
measureValues:
type: array
Expand Down Expand Up @@ -4857,6 +4866,9 @@ definitions:
title: Optional, defaults to false
v1MetricsViewSpec:
type: object
required:
- dimensions
- measures
properties:
connector:
type: string
Expand Down Expand Up @@ -4967,6 +4979,8 @@ definitions:
Deprecated: Now defined in the Explore resource.
v1MetricsViewState:
type: object
required:
- validSpec
properties:
validSpec:
$ref: '#/definitions/v1MetricsViewSpec'
Expand Down Expand Up @@ -5144,6 +5158,9 @@ definitions:
title: Not optional, not null
v1MetricsViewV2:
type: object
required:
- spec
- state
properties:
spec:
$ref: '#/definitions/v1MetricsViewSpec'
Expand Down Expand Up @@ -6207,4 +6224,4 @@ definitions:
name:
$ref: '#/definitions/v1ResourceName'
resource:
$ref: '#/definitions/v1Resource'
$ref: '#/definitions/v1Resource'
49 changes: 29 additions & 20 deletions web-admin/src/features/alerts/EditAlert.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -5,30 +5,39 @@
} from "@rilldata/web-common/components/dialog-v2";
import GuardedDialog from "@rilldata/web-common/components/dialog-v2/GuardedDialog.svelte";
import EditAlertForm from "@rilldata/web-common/features/alerts/EditAlertForm.svelte";
import { useMetricsView } from "@rilldata/web-common/features/dashboards/selectors";
import type { V1AlertSpec } from "@rilldata/web-common/runtime-client";
import { runtime } from "@rilldata/web-common/runtime-client/runtime-store";
import Button from "web-common/src/components/button/Button.svelte";

export let alertSpec: V1AlertSpec;
export let metricsViewName: string;

$: ({ instanceId } = $runtime);
$: metricsViewSpecQuery = useMetricsView(instanceId, metricsViewName);
$: validSpec = $metricsViewSpecQuery.data;
</script>

<GuardedDialog
title="Close without saving?"
description="You haven’t saved changes to this alert yet, so closing this window will lose your work."
confirmLabel="Close"
cancelLabel="Keep editing"
let:onCancel
let:onClose
>
<DialogTrigger asChild let:builder>
<Button type="secondary" builders={[builder]}>Edit</Button>
</DialogTrigger>
<DialogContent class="p-0 m-0 w-[802px] max-w-fit" noClose>
<EditAlertForm
{alertSpec}
on:cancel={onCancel}
on:close={onClose}
{metricsViewName}
/>
</DialogContent>
</GuardedDialog>
{#if validSpec}
<GuardedDialog
title="Close without saving?"
description="You haven’t saved changes to this alert yet, so closing this window will lose your work."
confirmLabel="Close"
cancelLabel="Keep editing"
let:onCancel
let:onClose
>
<DialogTrigger asChild let:builder>
<Button type="secondary" builders={[builder]}>Edit</Button>
</DialogTrigger>
<DialogContent class="p-0 m-0 w-[802px] max-w-fit" noClose>
<EditAlertForm
defaultTimeRange={validSpec.defaultTimeRange}
{alertSpec}
on:cancel={onCancel}
on:close={onClose}
{metricsViewName}
/>
</DialogContent>
</GuardedDialog>
{/if}
9 changes: 7 additions & 2 deletions web-common/src/features/alerts/BaseAlertForm.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,16 @@
}
currentTabIndex += 1;

if (isEditForm || currentTabIndex !== 2 || $touched.name) {
if (
isEditForm ||
currentTabIndex !== 2 ||
$touched.name ||
!$metricsView.data
) {
return;
}
// if the user came to the delivery tab and name was not changed then auto generate it
const name = generateAlertName($form, $metricsView.data ?? {});
const name = generateAlertName($form, $metricsView.data);
if (!name) return;
$form.name = name;
}
Expand Down
19 changes: 13 additions & 6 deletions web-common/src/features/alerts/CreateAlertForm.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,30 @@
import { eventBus } from "@rilldata/web-common/lib/event-bus/event-bus";
import { runtime } from "../../runtime-client/runtime-store";
import BaseAlertForm from "./BaseAlertForm.svelte";
import { useMetricsView } from "../dashboards/selectors";

const user = createAdminServiceGetCurrentUser();
const createAlert = createAdminServiceCreateAlert();
$: organization = $page.params.organization;
$: project = $page.params.project;

const queryClient = useQueryClient();
const dispatch = createEventDispatcher();

const {
metricsViewName,
dashboardStore,

selectors: {
timeRangeSelectors: { timeControlsState },
},
} = getStateManagers();
const timeControls = get(timeControlsState);

$: ({ organization, project } = $page.params);

$: metricsViewQuery = useMetricsView($runtime.instanceId, $metricsViewName);

$: metricsView = $metricsViewQuery.data;

// Set defaults depending on UI state
// if in TDD take active measure and comparison dimension
// If expanded leaderboard, take first dimension and active dimensions
Expand All @@ -56,14 +63,14 @@
}

// TODO: get metrics view spec
const timeRange = mapTimeRange(timeControls, {});
const comparisonTimeRange = mapComparisonTimeRange(
$: timeRange = mapTimeRange(timeControls, metricsView?.defaultTimeRange);
$: comparisonTimeRange = mapComparisonTimeRange(
$dashboardStore,
timeControls,
timeRange,
);

const formState = createForm<AlertFormValues>({
$: formState = createForm<AlertFormValues>({
initialValues: {
name: "",
measure:
Expand Down Expand Up @@ -158,7 +165,7 @@
},
});

const { form } = formState;
$: form = formState.form;
$: hasSlackNotifier = getHasSlackConnection($runtime.instanceId);
$: if ($hasSlackNotifier.data) {
$form["enableSlackNotification"] = true;
Expand Down
30 changes: 12 additions & 18 deletions web-common/src/features/alerts/EditAlertForm.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,8 @@
extractAlertFormValues,
extractAlertNotification,
} from "@rilldata/web-common/features/alerts/extract-alert-form-values";
import {
useMetricsView,
useMetricsViewTimeRange,
} from "@rilldata/web-common/features/dashboards/selectors";
import { useQueryClient } from "@tanstack/svelte-query";
import { useMetricsViewTimeRange } from "@rilldata/web-common/features/dashboards/selectors";
import { queryClient } from "@rilldata/web-common/lib/svelte-query/globalQueryClient";
import { createEventDispatcher } from "svelte";
import { createForm } from "svelte-forms-lib";
import { eventBus } from "@rilldata/web-common/lib/event-bus/event-bus";
Expand All @@ -31,25 +28,22 @@

export let alertSpec: V1AlertSpec;
export let metricsViewName: string;
export let defaultTimeRange: string | undefined;

const editAlert = createAdminServiceEditAlert();
const queryClient = useQueryClient();
const dispatch = createEventDispatcher();

$: organization = $page.params.organization;
$: project = $page.params.project;
$: alertName = $page.params.alert;
$: ({ instanceId } = $runtime);

$: ({ project, alert: alertName, organization } = $page.params);
const queryArgsJson = JSON.parse(
(alertSpec.resolverProperties?.query_args_json ??
alertSpec.queryArgsJson) as string,
) as V1MetricsViewAggregationRequest;

$: metricsViewSpec = useMetricsView($runtime?.instanceId, metricsViewName);
$: timeRange = useMetricsViewTimeRange(
$runtime?.instanceId,
metricsViewName,
{ query: { queryClient } },
);
$: timeRange = useMetricsViewTimeRange(instanceId, metricsViewName, {
query: { queryClient },
});

const formState = createForm<AlertFormValues>({
initialValues: {
Expand All @@ -59,7 +53,7 @@
...extractAlertNotification(alertSpec),
...extractAlertFormValues(
queryArgsJson,
$metricsViewSpec?.data ?? {},
defaultTimeRange,
$timeRange?.data ?? {},
),
},
Expand Down Expand Up @@ -112,10 +106,10 @@
},
});
const { form } = formState;
$: if ($metricsViewSpec?.data && $timeRange?.data) {
$: if ($timeRange?.data) {
const formValues = extractAlertFormValues(
queryArgsJson,
$metricsViewSpec.data,
defaultTimeRange,
$timeRange.data,
);
for (const fk in formValues) {
Expand Down
11 changes: 7 additions & 4 deletions web-common/src/features/alerts/alert-preview-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ export function getAlertPreviewData(
): CreateQueryResult<AlertPreviewResponse> {
return derived(
[useMetricsView(get(runtime).instanceId, formValues.metricsViewName)],
([metricsViewResp], set) =>
createQueryServiceMetricsViewAggregation(
([metricsViewResp], set) => {
return createQueryServiceMetricsViewAggregation(
get(runtime).instanceId,
formValues.metricsViewName,
getAlertPreviewQueryRequest(formValues),
Expand All @@ -51,7 +51,8 @@ export function getAlertPreviewData(
metricsViewResp.data,
),
},
).subscribe(set),
).subscribe(set);
},
);
}

Expand Down Expand Up @@ -88,7 +89,9 @@ function getAlertPreviewQueryOptions(
return {
rows: resp.data as V1MetricsViewAggregationResponseDataItem[],
schema: (resp.schema?.fields
?.map((field) => getSchemaEntryForField(metricsViewSpec ?? {}, field))
?.map((field) =>
getSchemaEntryForField(metricsViewSpec as V1MetricsViewSpec, field),
)
.filter(Boolean) ?? []) as VirtualizedTableColumns[],
};
},
Expand Down
6 changes: 3 additions & 3 deletions web-common/src/features/alerts/extract-alert-form-values.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
V1MetricsViewAggregationDimension,
V1MetricsViewAggregationMeasure,
V1MetricsViewAggregationRequest,
type V1MetricsViewSpec,
type V1MetricsViewTimeRangeResponse,
V1Operation,
V1TimeRange,
Expand All @@ -32,17 +31,18 @@ export type AlertFormValuesSubset = Pick<

export function extractAlertFormValues(
queryArgs: V1MetricsViewAggregationRequest,
metricsViewSpec: V1MetricsViewSpec,
defaultTimeRange: string | undefined,
allTimeRange: V1MetricsViewTimeRangeResponse,
): AlertFormValuesSubset {
console.log({ defaultTimeRange });
if (!queryArgs) return {} as AlertFormValuesSubset;

const measures = queryArgs.measures as V1MetricsViewAggregationMeasure[];
const dimensions =
queryArgs.dimensions as V1MetricsViewAggregationDimension[];

const timeRange = (queryArgs.timeRange as V1TimeRange) ?? {
isoDuration: metricsViewSpec.defaultTimeRange ?? TimeRangePreset.ALL_TIME,
isoDuration: defaultTimeRange ?? TimeRangePreset.ALL_TIME,
};
if (!timeRange.end && allTimeRange.timeRangeSummary?.max) {
// alerts only have duration optionally offset, end is added during execution by reconciler
Expand Down
2 changes: 2 additions & 0 deletions web-common/src/features/alerts/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ import { describe, expect, it } from "vitest";
const MetricsView: V1MetricsViewSpec = {
measures: [
{
expression: "",
name: "total_records",
label: "Total records",
},
],
dimensions: [],
};

describe("generateAlertName", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ export function getDimensionTableExportArgs(
([metricViewName, dashboardState, timeControlState, metricsView]) => {
if (!metricsView.data || !timeControlState.ready) return undefined;

const timeRange = mapTimeRange(timeControlState, metricsView.data);
const timeRange = mapTimeRange(
timeControlState,
metricsView.data.defaultTimeRange,
);
if (!timeRange) return undefined;

const comparisonTimeRange = mapComparisonTimeRange(
Expand Down
Loading
Loading