From 0b1eaf9b490057a1b5bf1c8b643a41dc1b34fdf9 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 14 Jan 2025 16:53:39 +0000 Subject: [PATCH] fix: dashboard service following tests --- .../metabase/dashboard/getDashboard.ts | 5 ++--- .../analytics/metabase/dashboard/service.ts | 5 ++--- .../analytics/metabase/dashboard/types.ts | 19 ++++++------------- .../metabase/dashboard/updateFilter.ts | 15 ++++++++++----- 4 files changed, 20 insertions(+), 24 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/dashboard/getDashboard.ts b/api.planx.uk/modules/analytics/metabase/dashboard/getDashboard.ts index 53ef2961a8..1aa2ccf9c0 100644 --- a/api.planx.uk/modules/analytics/metabase/dashboard/getDashboard.ts +++ b/api.planx.uk/modules/analytics/metabase/dashboard/getDashboard.ts @@ -1,11 +1,10 @@ import { $metabase } from "../shared/client.js"; -/** Takes dashboard ID and returns dashboard name. */ +/** Takes dashboard ID and returns dashboard with name as param; it's useful to return `response.data` to access other properties in testing. */ export async function getDashboard(dashboardId: number): Promise { try { const response = await $metabase.get(`/api/dashboard/${dashboardId}`); - console.log("Original dashboard name: ", response.data.name); - return response.data.name; + return response.data; } catch (error) { console.error("Error in getDashboard:", error); throw error; diff --git a/api.planx.uk/modules/analytics/metabase/dashboard/service.ts b/api.planx.uk/modules/analytics/metabase/dashboard/service.ts index c94a363fb1..f9b2f38b39 100644 --- a/api.planx.uk/modules/analytics/metabase/dashboard/service.ts +++ b/api.planx.uk/modules/analytics/metabase/dashboard/service.ts @@ -11,15 +11,14 @@ export async function createNewDashboard( params: CreateNewDashboardParams, ): Promise { try { - const templateName = await getDashboard(params.templateId); - const newName = templateName.replace("Template", params.teamName); + const template = await getDashboard(params.templateId); + const newName = template.name.replace("Template", params.teamName); const copiedDashboardId = await copyDashboard({ name: newName, templateId: params.templateId, description: params.description, collectionId: params.collectionId, collectionPosition: params.collectionPosition, - isDeepCopy: false, }); // updateFilter() does not need to be saved to a variable because we don't need to access its output anywhere else diff --git a/api.planx.uk/modules/analytics/metabase/dashboard/types.ts b/api.planx.uk/modules/analytics/metabase/dashboard/types.ts index 92938c20f8..4e5de03817 100644 --- a/api.planx.uk/modules/analytics/metabase/dashboard/types.ts +++ b/api.planx.uk/modules/analytics/metabase/dashboard/types.ts @@ -5,7 +5,7 @@ export interface CreateNewDashboardParams { teamName: string; /** Original / template Metabase Dashboard ID, it is the number that follows /dashboard/ in the URL */ templateId: number; - /** What the copied dashboard should be named; + /** What the copied dashboard should be named; * should be the original dashboard name * but with 'Template' replaced with council name */ // name?: string; @@ -15,25 +15,18 @@ export interface CreateNewDashboardParams { collectionId: number; /** Optional number for the copied dashboard's placement within the collection */ collectionPosition?: number | null; - /** Toggle whether or not the questions are copied as well; - * Metabase deep-copies by default, but we want shallow - * shallow = "Only duplicate the dashboard" */ - isDeepCopy: boolean; /** A filter that should be automatically set, eg `Team slug` */ filter: string; /** Default filter value, eg `council-name` */ value: string; } +/* We don't want users to be able to deep copy templates / dashboards because it will wreak Metabase havoc. This is why there is no isDeepCopy option here */ export type CopyDashboardParams = { name: string; } & Pick< CreateNewDashboardParams, - | "templateId" - | "description" - | "collectionId" - | "collectionPosition" - | "isDeepCopy" + "templateId" | "description" | "collectionId" | "collectionPosition" >; // Version of CopyDashboardParams suitable for Metabase API @@ -42,7 +35,7 @@ export type MetabaseCopyDashboardParams = { description?: string; collection_id?: number; collection_position?: number | null; - is_deep_copy: boolean; + is_deep_copy?: boolean; }; // Convert to Metabase API structure @@ -54,7 +47,8 @@ export function toMetabaseParams( description: params.description, collection_id: params.collectionId, collection_position: params.collectionPosition, - is_deep_copy: params.isDeepCopy, + /* Hard-coded false because deep copies will be messy in Metabase */ + is_deep_copy: false, }; } @@ -78,7 +72,6 @@ export const createNewDashboardSchema = z.object({ description: z.string().optional(), collectionId: z.coerce.number(), collectionPosition: z.coerce.number().nullable().optional(), - isDeepCopy: z.coerce.boolean().default(false), filter: z.string(), value: z.string(), }), diff --git a/api.planx.uk/modules/analytics/metabase/dashboard/updateFilter.ts b/api.planx.uk/modules/analytics/metabase/dashboard/updateFilter.ts index d70a674d0d..693e714c5a 100644 --- a/api.planx.uk/modules/analytics/metabase/dashboard/updateFilter.ts +++ b/api.planx.uk/modules/analytics/metabase/dashboard/updateFilter.ts @@ -1,3 +1,4 @@ +import { error } from "console"; import { $metabase } from "../shared/client.js"; import type { UpdateFilterParams } from "./types.js"; @@ -11,6 +12,12 @@ export async function updateFilter(params: UpdateFilterParams): Promise { let updatedFilter; const updatedParameters = response.data.parameters.map((param: any) => { 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] }; } @@ -18,7 +25,9 @@ export async function updateFilter(params: UpdateFilterParams): Promise { }); if (!updatedFilter) { - console.warn(`Filter "${params.filter}" not found in dashboard parameters`); + throw new Error( + `Filter "${params.filter}" not found in dashboard parameters`, + ); } // Prepare the update payload @@ -32,10 +41,6 @@ export async function updateFilter(params: UpdateFilterParams): Promise { updatePayload, ); - const updatedResponseDataParamFields = updatedResponse.data.param_fields; - console.log({ updatedResponseDataParamFields }); - const updatePayloadParams = updatePayload.parameters; - console.log({ updatePayloadParams }); console.log( `Updated dashboard ${params.dashboardId} filter "${updatedFilter}" with: ${params.value}`, );