From 2d6993aa9f0b34da7736d802333fbe5167a58256 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Thu, 30 Jan 2025 14:15:23 +0000 Subject: [PATCH] feat: feat: refactor updateFilter to separate logic --- .../metabase/dashboard/dashboard.test.ts | 34 +++++------ .../analytics/metabase/dashboard/types.ts | 18 +++++- .../metabase/dashboard/updateFilter.ts | 60 +++++++++++++------ 3 files changed, 75 insertions(+), 37 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/dashboard/dashboard.test.ts b/api.planx.uk/modules/analytics/metabase/dashboard/dashboard.test.ts index 11334c53b0..1355db2c88 100644 --- a/api.planx.uk/modules/analytics/metabase/dashboard/dashboard.test.ts +++ b/api.planx.uk/modules/analytics/metabase/dashboard/dashboard.test.ts @@ -99,7 +99,7 @@ describe("Dashboard Operations", () => { { name: filterName, type: "string/=", - default: ["old_value"], + value: ["old_value"], }, ], }); @@ -110,7 +110,7 @@ describe("Dashboard Operations", () => { { name: filterName, type: "string/=", - default: [filterValue], + value: [filterValue], }, ], }) @@ -119,19 +119,22 @@ describe("Dashboard Operations", () => { { name: filterName, type: "string/=", - default: [filterValue], + value: [filterValue], }, ], param_fields: {}, }); - await expect( - updateFilter({ - dashboardId: dashboardId, - filter: filterName, - value: filterValue, - }), - ).resolves.not.toThrow(); + const result = await updateFilter({ + dashboardId: dashboardId, + filter: filterName, + value: filterValue, + }); + + expect(result).toEqual({ + success: true, + updatedFilter: filterName, + }); }); test("handles non-string filter type appropriately", async () => { @@ -141,18 +144,15 @@ describe("Dashboard Operations", () => { parameters: [ { name: filterName, - slug: "event", - id: "30a24538", type: "number/=", - sectionId: "number", - default: [42], + value: [42], }, ], }); - nock(BASE_URL!).put(`/api/dashboard/${dashboardId}`).reply(400, { - message: "Invalid parameter type. Expected number, got string.", - }); + // nock(BASE_URL!).put(`/api/dashboard/${dashboardId}`).reply(400, { + // message: "Invalid parameter type. Expected number, got string.", + // }); await expect( updateFilter({ diff --git a/api.planx.uk/modules/analytics/metabase/dashboard/types.ts b/api.planx.uk/modules/analytics/metabase/dashboard/types.ts index 9ea42b4d14..15e5fb428b 100644 --- a/api.planx.uk/modules/analytics/metabase/dashboard/types.ts +++ b/api.planx.uk/modules/analytics/metabase/dashboard/types.ts @@ -38,7 +38,6 @@ export type MetabaseCopyDashboardParams = { is_deep_copy?: boolean; }; -// Convert to Metabase API structure export function toMetabaseParams( params: CopyDashboardParams, ): MetabaseCopyDashboardParams { @@ -77,10 +76,11 @@ export type NewDashboardHandler = ValidatedRequestHandler< ApiResponse >; -export interface GetDashboardResponse { +export interface MetabaseDashboardResponse { name: string; id: number; collection_id: number; + parameters: FilterParam[]; } export interface UpdateFilterResponse { @@ -91,4 +91,18 @@ export interface UpdateFilterResponse { export interface FilterParam { name: string; type: string; + // The Metabase API expects filter default values as arrays, even if there's only one (eg for multi-select filters) + value: string[]; +} + +export type GetDashboardResponse = Pick< + MetabaseDashboardResponse, + "name" | "id" | "collection_id" +>; + +export type GetFilterResponse = Pick; + +export interface UpdatedFilterResponse { + parameter: FilterParam; + updatedValue: string | undefined; } diff --git a/api.planx.uk/modules/analytics/metabase/dashboard/updateFilter.ts b/api.planx.uk/modules/analytics/metabase/dashboard/updateFilter.ts index 3d636a12ad..89672ae14a 100644 --- a/api.planx.uk/modules/analytics/metabase/dashboard/updateFilter.ts +++ b/api.planx.uk/modules/analytics/metabase/dashboard/updateFilter.ts @@ -3,33 +3,57 @@ import type { UpdateFilterParams, UpdateFilterResponse, FilterParam, + UpdatedFilterResponse, + GetFilterResponse, } from "./types.js"; +function populateUpdatedFilterResponse( + param: FilterParam, + filterName: string, + filterValue: string, +): UpdatedFilterResponse { + if (param.name === filterName) { + if (!param.type.startsWith("string/")) { + throw new Error( + `Filter type '${param.type}' is not supported. Only string filters are currently supported.`, + ); + } + return { + parameter: { ...param, value: [filterValue] }, + updatedValue: param.name, + }; + } + return { + parameter: param, + updatedValue: undefined, + }; +} + /** Takes the ID of the dashboard to be updated, the name of the filter (a string, must be an exact match), and the new value to be filtered on. * Currently only works for strings. */ export async function updateFilter( params: UpdateFilterParams, ): Promise { // Get existing dashboard data - const response = await $metabase.get(`/api/dashboard/${params.dashboardId}`); - - // Update filter default value parameter - let updatedFilter; - const updatedParameters = response.data.parameters.map( - (param: FilterParam) => { - if (param.name === params.filter) { - // Check if the filter is a string type - if (!param.type.startsWith("string/")) { - throw new Error( - `Filter type '${param.type}' is not supported. Only string filters are currently supported.`, - ); - } - updatedFilter = param.name; - return { ...param, default: [params.value] }; - } - return param; - }, + const response = await $metabase.get( + `/api/dashboard/${params.dashboardId}`, ); + console.log({ response }); + // Update filter default value parameter + let updatedFilter: string | undefined; + const updatedParameters = response.data.parameters.map((param) => { + const result = populateUpdatedFilterResponse( + param, + params.filter, + params.value, + ); + console.log({ result }); + if (result.updatedValue) { + updatedFilter = result.updatedValue; + } + return result.parameter; + }); + console.log({ updatedParameters }); if (!updatedFilter) { throw new Error(