diff --git a/web-common/src/features/dashboards/dashboard-fetch-mocks.ts b/web-common/src/features/dashboards/dashboard-fetch-mocks.ts index 59481ec2c06b..3d94e1b13c46 100644 --- a/web-common/src/features/dashboards/dashboard-fetch-mocks.ts +++ b/web-common/src/features/dashboards/dashboard-fetch-mocks.ts @@ -61,13 +61,15 @@ export class DashboardFetchMocks { name: string, metricsView: V1MetricsViewSpec, explore: V1ExploreSpec, + // The resource name of the metrics view, which can differ in casing from `explore.metricsView`. + metricsViewName = explore.metricsView ?? name, ) { this.responses.set(`resources__explore__${name}`, { metricsView: { meta: { name: { kind: ResourceKind.MetricsView, - name, + name: metricsViewName, }, }, metricsView: { diff --git a/web-common/src/features/dashboards/providers/DashboardConfigProvider.spec.ts b/web-common/src/features/dashboards/providers/DashboardConfigProvider.spec.ts new file mode 100644 index 000000000000..718534a34067 --- /dev/null +++ b/web-common/src/features/dashboards/providers/DashboardConfigProvider.spec.ts @@ -0,0 +1,52 @@ +import { + AD_BIDS_EXPLORE_INIT, + AD_BIDS_EXPLORE_NAME, + AD_BIDS_METRICS_INIT, +} from "@rilldata/web-common/features/dashboards/stores/test-data/data"; +import { DashboardFetchMocks } from "@rilldata/web-common/features/dashboards/dashboard-fetch-mocks"; +import { ExploreDashboardConfigProvider } from "@rilldata/web-common/features/dashboards/providers/DashboardConfigProvider.svelte.ts"; +import { renderInRuntimeContext } from "@rilldata/web-common/features/metrics-views/providers/test/metrics-views-test-utils.svelte.ts"; +import { waitUntil } from "@rilldata/web-common/lib/waitUtils"; +import { describe, expect, it } from "vitest"; + +// Mixed-case resource name, as produced by a file like `AdBids_Mixed_Metrics.yaml`. +const MIXED_CASE_METRICS_NAME = "AdBids_Mixed_Metrics"; + +const mocks = DashboardFetchMocks.useDashboardFetchMocks(); +mocks.mockMetricsView(MIXED_CASE_METRICS_NAME, AD_BIDS_METRICS_INIT); +mocks.mockMetricsExplore( + AD_BIDS_EXPLORE_NAME, + AD_BIDS_METRICS_INIT, + { + ...AD_BIDS_EXPLORE_INIT, + // The explore references the metrics view in lowercase. Resource names are case-insensitive + // in the runtime, so the explore still reconciles and GetExplore returns the mixed-case resource. + metricsView: MIXED_CASE_METRICS_NAME.toLowerCase(), + }, + MIXED_CASE_METRICS_NAME, +); + +describe("ExploreDashboardConfigProvider", () => { + it("names the metrics view by its resource name rather than the explore's reference", async () => { + const { value: provider, destroy } = renderInRuntimeContext( + ({ runtimeClient }) => + new ExploreDashboardConfigProvider(runtimeClient, AD_BIDS_EXPLORE_NAME), + ); + + await waitUntil( + () => provider.metricsViewsProvider.metricsViewNames.length > 0, + 5000, + ); + expect(provider.metricsViewsProvider.metricsViewNames).toEqual([ + MIXED_CASE_METRICS_NAME, + ]); + + await waitUntil(() => provider.metricsViewsProvider.ready, 5000); + expect( + provider.metricsViewsProvider.specs[MIXED_CASE_METRICS_NAME], + ).toMatchObject(AD_BIDS_METRICS_INIT); + + destroy(); + provider.cleanup?.(); + }); +}); diff --git a/web-common/src/features/dashboards/providers/DashboardConfigProvider.svelte.ts b/web-common/src/features/dashboards/providers/DashboardConfigProvider.svelte.ts index af3aac622770..58be592c25d1 100644 --- a/web-common/src/features/dashboards/providers/DashboardConfigProvider.svelte.ts +++ b/web-common/src/features/dashboards/providers/DashboardConfigProvider.svelte.ts @@ -32,8 +32,14 @@ export class ExploreDashboardConfigProvider extends DashboardConfigProvider { const exploreSpec = getExploreResp.data?.explore?.explore?.state?.validSpec ?? {}; + // Prefer the resource's own name over the explore's reference to it. Resource names are + // case-insensitive in the runtime, so the two can differ in casing, and the rest of the + // explore identifies the metrics view by the resource name. + const metricsViewName = + getExploreResp.data?.metricsView?.meta?.name?.name ?? + exploreSpec.metricsView; this.metricsViewsProvider.setMetricsViewNames( - exploreSpec.metricsView ? [exploreSpec.metricsView] : [], + metricsViewName ? [metricsViewName] : [], ); // this.yamlConfigProvider.update() // TODO: once we have this support for explore diff --git a/web-common/src/features/metrics-views/providers/MetricsViewsProvider.spec.ts b/web-common/src/features/metrics-views/providers/MetricsViewsProvider.spec.ts index 2578d60d17af..81555881ea76 100644 --- a/web-common/src/features/metrics-views/providers/MetricsViewsProvider.spec.ts +++ b/web-common/src/features/metrics-views/providers/MetricsViewsProvider.spec.ts @@ -13,9 +13,12 @@ import { import { describe, expect, it } from "vitest"; const AD_BIDS_MIRROR_METRICS_NAME = "AdBids_mirror_metrics"; +// Mixed-case resource name, as produced by a file like `AdBids_Mixed_Metrics.yaml`. +const AD_BIDS_MIXED_CASE_METRICS_NAME = "AdBids_Mixed_Metrics"; useMetricsViewMocks({ [AD_BIDS_METRICS_NAME]: AD_BIDS_METRICS_INIT, + [AD_BIDS_MIXED_CASE_METRICS_NAME]: AD_BIDS_METRICS_INIT, // Shares the publisher dimension and the impressions measure with AdBids. [AD_BIDS_MIRROR_METRICS_NAME]: { ...AD_BIDS_METRICS_INIT, @@ -48,6 +51,29 @@ describe("MetricsViewsProvider", () => { destroy(); }); + it("resolves a lowercase reference to a mixed-case metrics view", async () => { + // Resource names are case-insensitive in the runtime, so an explore whose + // `metrics_view` is written in lowercase still reconciles against a mixed-case + // metrics view. The provider has to resolve it the same way. + const requestedName = AD_BIDS_MIXED_CASE_METRICS_NAME.toLowerCase(); + const { value: provider, destroy } = await createTestMetricsViewsProvider([ + requestedName, + ]); + + // Keyed by the requested name, which is what callers use for lookups. + expect(provider.specs[requestedName]).toMatchObject(AD_BIDS_METRICS_INIT); + expect( + Object.keys(provider.dimensionSpecs[AD_BIDS_PUBLISHER_DIMENSION]), + ).toEqual([requestedName]); + expect(provider.dimensions.map((d) => d.name)).toEqual([ + AD_BIDS_PUBLISHER_DIMENSION, + AD_BIDS_DOMAIN_DIMENSION, + ]); + expect(provider.ready).toBe(true); + + destroy(); + }); + it("maps a shared measure or dimension to every metrics view defining it", async () => { const { value: provider, destroy } = await createTestMetricsViewsProvider([ AD_BIDS_METRICS_NAME, diff --git a/web-common/src/features/metrics-views/providers/MetricsViewsProvider.svelte.ts b/web-common/src/features/metrics-views/providers/MetricsViewsProvider.svelte.ts index 3dab6a80127f..08301a3f119c 100644 --- a/web-common/src/features/metrics-views/providers/MetricsViewsProvider.svelte.ts +++ b/web-common/src/features/metrics-views/providers/MetricsViewsProvider.svelte.ts @@ -187,9 +187,13 @@ export class MetricsViewsProvider { const dimensions: MetricsViewSpecDimension[] = []; for (const metricsViewName of this.metricsViewNames) { + // Resource names are case-insensitive in the runtime, so an explore or canvas can reference + // a metrics view with different casing than the resource itself. Keep the maps keyed by the + // requested name, since that is what the callers use to look them up. + const lowerName = metricsViewName.toLowerCase(); const res = this.resources.find( (resource) => - resource.meta?.name?.name === metricsViewName && + resource.meta?.name?.name?.toLowerCase() === lowerName && resource.meta?.name?.kind === ResourceKind.MetricsView, ); const spec = res?.metricsView?.state?.validSpec;