Skip to content

Commit

Permalink
fix: dashboard service following tests
Browse files Browse the repository at this point in the history
  • Loading branch information
zz-hh-aa committed Jan 14, 2025
1 parent 6f58495 commit 0b1eaf9
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 24 deletions.
Original file line number Diff line number Diff line change
@@ -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<any> {
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;
Expand Down
5 changes: 2 additions & 3 deletions api.planx.uk/modules/analytics/metabase/dashboard/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,14 @@ export async function createNewDashboard(
params: CreateNewDashboardParams,
): Promise<string> {
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
Expand Down
19 changes: 6 additions & 13 deletions api.planx.uk/modules/analytics/metabase/dashboard/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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,
};
}

Expand All @@ -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(),
}),
Expand Down
15 changes: 10 additions & 5 deletions api.planx.uk/modules/analytics/metabase/dashboard/updateFilter.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { error } from "console";
import { $metabase } from "../shared/client.js";
import type { UpdateFilterParams } from "./types.js";

Expand All @@ -11,14 +12,22 @@ export async function updateFilter(params: UpdateFilterParams): Promise<any> {
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] };
}
return param;
});

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
Expand All @@ -32,10 +41,6 @@ export async function updateFilter(params: UpdateFilterParams): Promise<any> {
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}`,
);
Expand Down

0 comments on commit 0b1eaf9

Please sign in to comment.