diff --git a/web-admin/tests/reports.spec.ts b/web-admin/tests/reports.spec.ts index 22ee7181262..51c9383b410 100644 --- a/web-admin/tests/reports.spec.ts +++ b/web-admin/tests/reports.spec.ts @@ -231,6 +231,13 @@ test.describe.serial("Reports", () => { await expect(adminPage.getByLabel("Report schedule")).toHaveText( /Repeats\s+At 10:00 PM, on the 1st of each month/m, ); + + // Reopen the report and assert that the filter was saved with it + await adminPage.getByLabel("Report context menu").click(); + await adminPage.getByRole("menuitem", { name: "Edit Report" }).click(); + await expect(filtersForm.getByLabel("Open ad_size filter")).toHaveText( + /Ad Size\s*1024x768\s*\+2 others/, + ); }); test("Should delete report", async ({ adminPage }) => { diff --git a/web-common/src/features/scheduled-reports/FiltersForm.svelte b/web-common/src/features/scheduled-reports/FiltersForm.svelte index e2d4e4ff327..b31049a8c12 100644 --- a/web-common/src/features/scheduled-reports/FiltersForm.svelte +++ b/web-common/src/features/scheduled-reports/FiltersForm.svelte @@ -43,11 +43,15 @@ side?: "top" | "right" | "bottom" | "left"; } = $props(); + // The filters belong to the report or alert in the form, not to the page the form is opened on. + // Syncing with the page URL would replace them with the filters in that URL, which are usually none. // svelte-ignore state_referenced_locally syncStoreWithSource( filters, async (newUrlParams) => filters.setUrlParams(newUrlParams), () => filters.metricsViewsProvider.ready, + undefined, + true, ); let { diff --git a/web-common/src/features/scheduled-reports/ScheduledReportDialog.spec.ts b/web-common/src/features/scheduled-reports/ScheduledReportDialog.spec.ts new file mode 100644 index 00000000000..d0fe775d4a0 --- /dev/null +++ b/web-common/src/features/scheduled-reports/ScheduledReportDialog.spec.ts @@ -0,0 +1,251 @@ +import { + addFilter, + closeFilter, + getFilterChip, + mockPointerEventsForComponentTesting, + mockResizeObserverForComponentTesting, + selectValues, + useDashboardFetchMocksForComponentTests, + waitForBodyScrollCleanup, +} from "@rilldata/web-common/features/dashboards/filters/test/filter-test-utils"; +import { + type HoistedPageForComponentTests, + PageMockForComponentTests, +} from "@rilldata/web-common/features/dashboards/state-managers/loaders/test/PageMockForComponentTests.ts"; +import { + createAndExpression, + createInExpression, +} from "@rilldata/web-common/features/dashboards/stores/filter-utils"; +import { + AD_BIDS_DOMAIN_DIMENSION, + AD_BIDS_EXPLORE_INIT, + AD_BIDS_EXPLORE_NAME, + AD_BIDS_IMPRESSIONS_MEASURE, + AD_BIDS_METRICS_INIT_WITH_TIME, + AD_BIDS_METRICS_NAME, + AD_BIDS_PUBLISHER_DIMENSION, + AD_BIDS_TIME_RANGE_SUMMARY, +} from "@rilldata/web-common/features/dashboards/stores/test-data/data"; +import ScheduledReportDialogTest from "@rilldata/web-common/features/scheduled-reports/test/ScheduledReportDialogTest.svelte"; +import { queryClient } from "@rilldata/web-common/lib/svelte-query/globalQueryClient"; +import { mockAnimationsForComponentTesting } from "@rilldata/web-common/lib/test/mock-animations"; +import { + V1ExportFormat, + type V1Expression, + type V1MetricsViewAggregationRequest, + type V1ReportSpec, +} from "@rilldata/web-common/runtime-client"; +import { + RUNTIME_CONTEXT_KEY, + RuntimeClient, +} from "@rilldata/web-common/runtime-client/v2"; +import type { ActionResult } from "@sveltejs/kit"; +import { act, render, screen, waitFor } from "@testing-library/svelte"; +import { + afterAll, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from "vitest"; + +// The SvelteKit mocks have to be declared in the spec file, since `vi.mock` is hoisted per file. +const hoistedPage: HoistedPageForComponentTests = vi.hoisted(() => ({}) as any); +const editReport = vi.hoisted(() => + vi.fn< + (request: { + data: { options: { queryArgsJson: string } }; + }) => Promise + >(), +); + +vi.stubEnv("TZ", "UTC"); + +vi.mock("$app/navigation", () => { + return { + goto: (url, opts) => hoistedPage.goto(url, opts), + afterNavigate: (cb) => hoistedPage.afterNavigate(cb), + onNavigate: () => {}, + // superforms registers a navigation guard for tainted forms as soon as it is created. + beforeNavigate: () => {}, + }; +}); +vi.mock("$app/forms", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + // The real `applyAction` needs the SvelteKit client runtime, which a component test does not + // boot. superforms reads its validation result back from the page, so hand it to the page mock. + applyAction: (result: ActionResult) => { + hoistedPage.applyAction(result); + return Promise.resolve(); + }, + }; +}); +vi.mock("$app/state", async () => { + return { + page: ( + await import( + "@rilldata/web-common/features/dashboards/state-managers/loaders/test/page-state.mock.svelte" + ) + ).pageStateMock, + }; +}); +vi.mock("$app/stores", () => { + return { + page: hoistedPage, + navigating: { + subscribe: (run: (value: null) => void) => { + run(null); + return () => {}; + }, + }, + }; +}); +// The dialog saves through the admin API, which web-common unit tests cannot reach. +vi.mock("@rilldata/web-admin/client", async () => { + const { readable } = await import("svelte/store"); + return { + getAdminServiceListBookmarksQueryOptions: () => ({}), + createAdminServiceGetCurrentUser: () => + readable({ data: { user: { email: "user@rilldata.com" } } }), + createAdminServiceListProjectMemberUsers: () => + readable({ data: { members: [] } }), + createAdminServiceCreateReport: () => + readable({ mutateAsync: vi.fn(), error: null }), + createAdminServiceEditReport: () => + readable({ mutateAsync: editReport, error: null }), + }; +}); + +const PUBLISHER_FILTER = createAndExpression([ + createInExpression(AD_BIDS_PUBLISHER_DIMENSION, ["Facebook", "Google"]), +]); + +function getReportSpec(where: V1Expression | undefined): V1ReportSpec { + const queryArgs: V1MetricsViewAggregationRequest = { + metricsView: AD_BIDS_METRICS_NAME, + dimensions: [{ name: AD_BIDS_DOMAIN_DIMENSION }], + measures: [{ name: AD_BIDS_IMPRESSIONS_MEASURE }], + timeRange: { isoDuration: "P7D", timeZone: "UTC" }, + where, + }; + return { + displayName: "Weekly report", + refreshSchedule: { cron: "0 9 * * 1", timeZone: "UTC" }, + queryName: "MetricsViewAggregation", + queryArgsJson: JSON.stringify(queryArgs), + exportFormat: V1ExportFormat.EXPORT_FORMAT_CSV, + notifiers: [ + { connector: "email", properties: { recipients: ["user@rilldata.com"] } }, + ], + annotations: { + explore: AD_BIDS_EXPLORE_NAME, + web_open_mode: "creator", + }, + }; +} + +/** Opens the edit dialog for `reportSpec`, the way the report page does. */ +async function openEditDialog(reportSpec: V1ReportSpec) { + const rendered = render(ScheduledReportDialogTest, { + props: { reportSpec, metricsViewName: AD_BIDS_METRICS_NAME }, + context: new Map([ + ["$$_queryClient", queryClient], + [ + RUNTIME_CONTEXT_KEY, + new RuntimeClient({ host: "http://localhost", instanceId: "test" }), + ], + ]), + }); + await waitFor(() => + expect(screen.getByLabelText("Filters form")).toBeVisible(), + ); + return rendered; +} + +/** Saves the report and returns the `where` of the query the dialog sent to the admin API. */ +async function saveAndGetWhere() { + await act(() => screen.getByLabelText("Save report").click()); + await waitFor(() => expect(editReport).toHaveBeenCalledOnce()); + + const { options } = editReport.mock.calls[0][0].data; + const queryArgs = JSON.parse( + options.queryArgsJson, + ) as V1MetricsViewAggregationRequest; + return queryArgs.where; +} + +describe("ScheduledReportDialog", () => { + mockAnimationsForComponentTesting(); + mockPointerEventsForComponentTesting(); + mockResizeObserverForComponentTesting(); + const mocks = useDashboardFetchMocksForComponentTests(); + + // In browsers a `requestSubmit()` from inside a submit handler is a no-op, since the form is + // already firing its submission events. jsdom does not implement that flag, and the report form + // depends on it: its submit handler calls superforms' `submit()`, which requests another submit. + // Every submit in this file starts from the Save button, so nothing else reaches `requestSubmit`. + beforeAll(() => { + vi.spyOn(HTMLFormElement.prototype, "requestSubmit").mockImplementation( + () => {}, + ); + }); + + beforeEach(() => { + new PageMockForComponentTests(hoistedPage); + editReport.mockReset(); + editReport.mockResolvedValue({}); + + mocks.mockMetricsView(AD_BIDS_METRICS_NAME, AD_BIDS_METRICS_INIT_WITH_TIME); + mocks.mockMetricsExplore( + AD_BIDS_EXPLORE_NAME, + AD_BIDS_METRICS_INIT_WITH_TIME, + AD_BIDS_EXPLORE_INIT, + ); + mocks.mockTimeRangeSummary( + AD_BIDS_METRICS_NAME, + AD_BIDS_TIME_RANGE_SUMMARY.timeRangeSummary!, + ); + + localStorage.clear(); + sessionStorage.clear(); + queryClient.clear(); + }); + + afterAll(waitForBodyScrollCleanup); + + it("saves a filter added in the dialog and shows it when the report is edited again", async () => { + const firstDialog = await openEditDialog(getReportSpec(undefined)); + + await addFilter(AD_BIDS_PUBLISHER_DIMENSION); + await selectValues(["Facebook", "Google"]); + // Select mode applies once the dropdown closes. + await closeFilter(AD_BIDS_PUBLISHER_DIMENSION); + + const savedWhere = await saveAndGetWhere(); + expect(savedWhere).toEqual(PUBLISHER_FILTER); + firstDialog.unmount(); + editReport.mockClear(); + + // The report page refetches the report after a save, so the dialog reopens on the saved filter. + await openEditDialog(getReportSpec(savedWhere)); + await waitFor(() => + expect(getFilterChip(AD_BIDS_PUBLISHER_DIMENSION)).toBeVisible(), + ); + + expect(await saveAndGetWhere()).toEqual(PUBLISHER_FILTER); + }); + + it("keeps the saved filters when the report is saved without touching them", async () => { + await openEditDialog(getReportSpec(PUBLISHER_FILTER)); + + await waitFor(() => + expect(getFilterChip(AD_BIDS_PUBLISHER_DIMENSION)).toBeVisible(), + ); + + expect(await saveAndGetWhere()).toEqual(PUBLISHER_FILTER); + }); +}); diff --git a/web-common/src/features/scheduled-reports/ScheduledReportDialog.svelte b/web-common/src/features/scheduled-reports/ScheduledReportDialog.svelte index 019a93b9004..5e817e4f06f 100644 --- a/web-common/src/features/scheduled-reports/ScheduledReportDialog.svelte +++ b/web-common/src/features/scheduled-reports/ScheduledReportDialog.svelte @@ -361,7 +361,8 @@ }), ], ); - updatedAggregationRequest.where = filters?.topLevelJoiner[metricsViewName]; + updatedAggregationRequest.where = + filters?.topLevelJoiner.expr[metricsViewName]; return { ...commonOptions, explore: exploreName, diff --git a/web-common/src/features/scheduled-reports/test/ScheduledReportDialogTest.svelte b/web-common/src/features/scheduled-reports/test/ScheduledReportDialogTest.svelte new file mode 100644 index 00000000000..01010fa59d7 --- /dev/null +++ b/web-common/src/features/scheduled-reports/test/ScheduledReportDialogTest.svelte @@ -0,0 +1,59 @@ + + + + + {#if pageLoaded && open} + + {/if} +