feat: feat: refactor updateFilter to separate logic

theopensystemslab · Jan 30, 2025 · 2d6993a · 2d6993a
1 parent 5257556
commit 2d6993a
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 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
@@ -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
@@ -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<string>
 >;
 
-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<MetabaseDashboardResponse, "parameters">;
+
+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
@@ -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<UpdateFilterResponse> {
   // 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<GetFilterResponse>(
+    `/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(