From 9f469cbdb2f32ba9a1751b5002c4e3c6b766238a Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 18 Nov 2024 10:55:41 +0000 Subject: [PATCH 01/46] feat: create type and validator for Metabase client --- .../analytics/metabase/shared/types.ts | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/api.planx.uk/modules/analytics/metabase/shared/types.ts b/api.planx.uk/modules/analytics/metabase/shared/types.ts index e69de29bb2..e953721b3c 100644 --- a/api.planx.uk/modules/analytics/metabase/shared/types.ts +++ b/api.planx.uk/modules/analytics/metabase/shared/types.ts @@ -0,0 +1,41 @@ +export interface Input { + councilName: string; + originalDashboardId: number; + /** The name of the filter to be updated */ + filter: string; + /** The value of the new filter; updateFilter() only supports strings right now */ + filterValue: string; +} + +export function validateInput(input: unknown): input is Input { + // check that input type is object + if (typeof input !== "object" || input === null) { + return false; + } + + // check that input object is same shape as Input type with same properties + const { councilName, originalDashboardId, filter, filterValue } = + input as Input; + + if (typeof councilName !== "string" || councilName.trim() === "") { + console.error("Invalid councilName: must be a non-empty string"); + return false; + } + + if (!Number.isInteger(originalDashboardId) || originalDashboardId <= 0) { + console.error("Invalid originalDashboardId: must be a positive integer"); + return false; + } + + if (typeof filter !== "string" || filter.trim() === "") { + console.error("Invalid filter: must be a non-empty string"); + return false; + } + + if (typeof filterValue !== "string") { + console.error("Invalid filterValue: must be a string"); + return false; + } + console.log("Input valid"); + return true; +} From 164798fbadf853e8882321e6eb595cb73a628e94 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 18 Nov 2024 11:00:37 +0000 Subject: [PATCH 02/46] chore: change 'council' to 'team' --- api.planx.uk/modules/analytics/metabase/shared/types.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/shared/types.ts b/api.planx.uk/modules/analytics/metabase/shared/types.ts index e953721b3c..e1f491a55f 100644 --- a/api.planx.uk/modules/analytics/metabase/shared/types.ts +++ b/api.planx.uk/modules/analytics/metabase/shared/types.ts @@ -1,5 +1,5 @@ export interface Input { - councilName: string; + teamName: string; originalDashboardId: number; /** The name of the filter to be updated */ filter: string; @@ -14,11 +14,10 @@ export function validateInput(input: unknown): input is Input { } // check that input object is same shape as Input type with same properties - const { councilName, originalDashboardId, filter, filterValue } = - input as Input; + const { teamName, originalDashboardId, filter, filterValue } = input as Input; - if (typeof councilName !== "string" || councilName.trim() === "") { - console.error("Invalid councilName: must be a non-empty string"); + if (typeof teamName !== "string" || teamName.trim() === "") { + console.error("Invalid teamName: must be a non-empty string"); return false; } From dc22f0b116795e648320bb1479c23af1f50b5025 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 18 Nov 2024 12:19:07 +0000 Subject: [PATCH 03/46] feat: setup collection service checks for collection and creates new one --- .../analytics/metabase/collection/service.ts | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index e69de29bb2..7226cc732c 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -0,0 +1,91 @@ +import metabaseClient from "../shared/client.js"; +import type { NewCollectionParams } from "./types.js"; +import axios from "axios"; + +export async function authentication(): Promise { + try { + const response = await metabaseClient.get("/user/current"); + return response.status === 200; + } catch (error) { + console.error("Error testing Metabase connection:", error); + return false; + } +} + +export async function newCollection({ + name, + description, + parent_id, + namespace, + authority_level, +}: NewCollectionParams): Promise { + try { + const requestBody: any = { + name, + description, + parent_id, + namespace, + authority_level, + }; + + // Remove undefined properties + Object.keys(requestBody).forEach( + (key) => requestBody[key] === undefined && delete requestBody[key], + ); + + // console.log('Request body: ', JSON.stringify(requestBody, null, 2)); + + const response = await metabaseClient.post(`/api/collection/`, { + name, + description, + parent_id, + namespace, + authority_level, + }); + console.log( + `New collection: ${response.data.name}, new collection ID: ${response.data.id}`, + ); + return response.data.id; + } catch (error) { + console.error("Error in newCollection:"); + if (axios.isAxiosError(error)) { + if (error.response) { + console.error("Response data:", error.response.data); + console.error("Response status:", error.response.status); + console.error("Response headers:", error.response.headers); + } else if (error.request) { + console.error("No response received. Request:", error.request); + } else { + console.error("Error message:", error.message); + } + console.error("Request config:", error.config); + } else { + console.error("Unexpected error:", error); + } + throw error; + } +} + +/** Checks if a collection exists with name matching `teamName` exists. + * Returns the matching collection ID if exists, otherwise false. */ +export async function checkCollections(teamName: string): Promise { + try { + console.log("Checking for collection: ", teamName); + // Get collections from Metabase + const response = await metabaseClient.get(`/api/collection/`); + + const matchingCollection = response.data.find((collection: any) => + collection.name.toLowerCase().includes(teamName.toLowerCase()), + ); + + if (matchingCollection) { + console.log("Matching collection found with ID: ", matchingCollection.id); + return matchingCollection.id; + } else { + console.log("No matching collection found"); + return false; + } + } catch (error) { + console.error("Error: ", error); + } +} From a957c1af4ec2d0e182189a2f6d477d0d3c8788aa Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 18 Nov 2024 12:19:48 +0000 Subject: [PATCH 04/46] feat: create new collection type --- .../modules/analytics/metabase/collection/types.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/api.planx.uk/modules/analytics/metabase/collection/types.ts b/api.planx.uk/modules/analytics/metabase/collection/types.ts index e69de29bb2..5a2e4badeb 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/types.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts @@ -0,0 +1,11 @@ +export interface NewCollectionParams { + /** The name of the new collection */ + name: string; + description?: string; + /** Optional; if the collection is a child of a parent, specify parent ID here + * For council teams, parent collection should be 58 + */ + parent_id?: string; + namespace?: string; + authority_level?: null; +} From 9739d6b6462de9f740bd8918e1d5aff2630a878d Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 18 Nov 2024 12:22:17 +0000 Subject: [PATCH 05/46] feat: create collection controller --- .../metabase/collection/controller.ts | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/api.planx.uk/modules/analytics/metabase/collection/controller.ts b/api.planx.uk/modules/analytics/metabase/collection/controller.ts index e69de29bb2..0e0056e940 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/controller.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/controller.ts @@ -0,0 +1,62 @@ +import { z } from "zod"; +import type { ValidatedRequestHandler } from "../../../../shared/middleware/validate.js"; +import { checkCollections, newCollection } from "./service.js"; +import type { NewCollectionParams } from "./types.js"; +import type { Request, Response } from "express"; + +// Error response type +interface ErrorResponse { + error: string; +} + +// Response types +type ApiResponse = { + data?: T; + error?: string; +}; + +// Define validation schemas +export const newCollectionSchema = z.object({ + body: z.object({ + name: z.string(), + description: z.string().optional(), + parentId: z.number().optional(), + }), +}); + +// Define types for validated requests +export type NewCollectionRequest = ValidatedRequestHandler< + typeof newCollectionSchema, + ApiResponse +>; + +// Controller functions +export const checkCollectionsController = async ( + _req: Request, + res: Response, +) => { + try { + const name = res.locals.parsedReq.body.name; + const collections = await checkCollections(name); + + res.status(200).json(collections); + } catch (error) { + res.status(500).json({ error: "Failed to fetch collections" }); + } +}; + +export const newCollectionController: NewCollectionRequest = async ( + _req, + res, +) => { + try { + const params = res.locals.parsedReq.body; + const collection = await newCollection(params); + res.status(201).json({ data: collection }); + } catch (error) { + res.status(400).json({ + error: + error instanceof Error ? error.message : "An unexpected error occurred", + }); + } +}; From 16a02b30b5c23f95211e4d21d555e31d8a024d12 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 18 Nov 2024 15:01:35 +0000 Subject: [PATCH 06/46] chore: change id type to number --- api.planx.uk/modules/analytics/metabase/collection/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/types.ts b/api.planx.uk/modules/analytics/metabase/collection/types.ts index 5a2e4badeb..e1b583e13a 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/types.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts @@ -5,7 +5,7 @@ export interface NewCollectionParams { /** Optional; if the collection is a child of a parent, specify parent ID here * For council teams, parent collection should be 58 */ - parent_id?: string; + parent_id?: number; namespace?: string; authority_level?: null; } From 165eca6f41d4cebc82c4b43ad0ed9808e9527665 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 18 Nov 2024 15:03:17 +0000 Subject: [PATCH 07/46] feat: use new MetabaseError type --- .../analytics/metabase/collection/service.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index 7226cc732c..abcf74dc1f 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -1,4 +1,4 @@ -import metabaseClient from "../shared/client.js"; +import { MetabaseError, metabaseClient } from "../shared/client.js"; import type { NewCollectionParams } from "./types.js"; import axios from "axios"; @@ -71,6 +71,7 @@ export async function newCollection({ export async function checkCollections(teamName: string): Promise { try { console.log("Checking for collection: ", teamName); + // Get collections from Metabase const response = await metabaseClient.get(`/api/collection/`); @@ -87,5 +88,20 @@ export async function checkCollections(teamName: string): Promise { } } catch (error) { console.error("Error: ", error); + if (error instanceof MetabaseError) { + console.error("MetabaseError:", { + message: error.message, + statusCode: error.statusCode, + response: error.response, + }); + } else if (error instanceof Error) { + console.error("Unexpected error:", { + message: error.message, + stack: error.stack, + }); + } else { + console.error("Unknown error:", error); + } + throw error; } } From a4d6b5fa5193fc78d2fe1dded28d729fa62fc819 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 18 Nov 2024 15:04:40 +0000 Subject: [PATCH 08/46] chore: tidy error handling in controller --- .../modules/analytics/metabase/collection/controller.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/controller.ts b/api.planx.uk/modules/analytics/metabase/collection/controller.ts index 0e0056e940..c36d2c9365 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/controller.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/controller.ts @@ -36,11 +36,17 @@ export const checkCollectionsController = async ( res: Response, ) => { try { - const name = res.locals.parsedReq.body.name; + const name = _req.query.name as string; + + if (!name) { + return res.status(400).json({ error: "Name parameter is required" }); + } + const collections = await checkCollections(name); res.status(200).json(collections); } catch (error) { + console.error("Controller error:", error); res.status(500).json({ error: "Failed to fetch collections" }); } }; From 62836ba101752813a9e221e071d892a155dc788b Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 18 Nov 2024 16:07:19 +0000 Subject: [PATCH 09/46] chore: move collection types to types.ts --- .../metabase/collection/controller.ts | 28 +----------- .../analytics/metabase/collection/types.ts | 43 +++++++++++++++++++ 2 files changed, 44 insertions(+), 27 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/controller.ts b/api.planx.uk/modules/analytics/metabase/collection/controller.ts index c36d2c9365..039261c7b8 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/controller.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/controller.ts @@ -1,35 +1,9 @@ import { z } from "zod"; import type { ValidatedRequestHandler } from "../../../../shared/middleware/validate.js"; import { checkCollections, newCollection } from "./service.js"; -import type { NewCollectionParams } from "./types.js"; +import type { NewCollectionRequest } from "./types.js"; import type { Request, Response } from "express"; -// Error response type -interface ErrorResponse { - error: string; -} - -// Response types -type ApiResponse = { - data?: T; - error?: string; -}; - -// Define validation schemas -export const newCollectionSchema = z.object({ - body: z.object({ - name: z.string(), - description: z.string().optional(), - parentId: z.number().optional(), - }), -}); - -// Define types for validated requests -export type NewCollectionRequest = ValidatedRequestHandler< - typeof newCollectionSchema, - ApiResponse ->; - // Controller functions export const checkCollectionsController = async ( _req: Request, diff --git a/api.planx.uk/modules/analytics/metabase/collection/types.ts b/api.planx.uk/modules/analytics/metabase/collection/types.ts index e1b583e13a..f35d9cb690 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/types.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts @@ -1,3 +1,26 @@ +import { z } from "zod"; +import type { ValidatedRequestHandler } from "../../../../shared/middleware/validate.js"; + +type ApiResponse = { + data?: T; + error?: string; +}; + +export const checkCollectionsSchema = z.object({ + query: z.object({ + name: z.string().min(1, "Name parameter is required"), + }), +}); + +export type CheckCollectionsRequest = ValidatedRequestHandler< + typeof checkCollectionsSchema, + ApiResponse +>; + +export interface CheckCollectionResponse { + collectionId: number | false; +} + export interface NewCollectionParams { /** The name of the new collection */ name: string; @@ -9,3 +32,23 @@ export interface NewCollectionParams { namespace?: string; authority_level?: null; } + +export const newCollectionSchema = z.object({ + body: z.object({ + name: z.string(), + description: z.string().optional(), + parent_id: z.number().optional(), + namespace: z.string().optional(), + authority_level: z.null().optional(), + }), +}); + +export type NewCollectionRequest = ValidatedRequestHandler< + typeof newCollectionSchema, + ApiResponse +>; + +export interface NewCollectionResponse { + id: number; + name: string; +} From 73ba1e90aef867a0866db3b7988796fbf8f74308 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 2 Dec 2024 13:08:51 +0000 Subject: [PATCH 10/46] feat: remove unnecessary checkCollection controller --- .../metabase/collection/controller.ts | 26 +------------------ .../analytics/metabase/collection/service.ts | 15 ++++++++--- .../analytics/metabase/collection/types.ts | 15 ----------- 3 files changed, 13 insertions(+), 43 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/controller.ts b/api.planx.uk/modules/analytics/metabase/collection/controller.ts index 039261c7b8..db3d1babae 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/controller.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/controller.ts @@ -1,29 +1,5 @@ -import { z } from "zod"; -import type { ValidatedRequestHandler } from "../../../../shared/middleware/validate.js"; -import { checkCollections, newCollection } from "./service.js"; +import { newCollection } from "./service.js"; import type { NewCollectionRequest } from "./types.js"; -import type { Request, Response } from "express"; - -// Controller functions -export const checkCollectionsController = async ( - _req: Request, - res: Response, -) => { - try { - const name = _req.query.name as string; - - if (!name) { - return res.status(400).json({ error: "Name parameter is required" }); - } - - const collections = await checkCollections(name); - - res.status(200).json(collections); - } catch (error) { - console.error("Controller error:", error); - res.status(500).json({ error: "Failed to fetch collections" }); - } -}; export const newCollectionController: NewCollectionRequest = async ( _req, diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index abcf74dc1f..0781e6a7cd 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -20,6 +20,16 @@ export async function newCollection({ authority_level, }: NewCollectionParams): Promise { try { + // Check if collection exists + const existingCollectionId = await checkCollections(name); + if (existingCollectionId) { + console.log( + `Collection "${name}" already exists with ID: ${existingCollectionId}`, + ); + return existingCollectionId; + } + + // If no existing collection, create new one const requestBody: any = { name, description, @@ -33,8 +43,6 @@ export async function newCollection({ (key) => requestBody[key] === undefined && delete requestBody[key], ); - // console.log('Request body: ', JSON.stringify(requestBody, null, 2)); - const response = await metabaseClient.post(`/api/collection/`, { name, description, @@ -66,7 +74,8 @@ export async function newCollection({ } } -/** Checks if a collection exists with name matching `teamName` exists. +/** + * Checks if a collection exists with name matching `teamName` exists. * Returns the matching collection ID if exists, otherwise false. */ export async function checkCollections(teamName: string): Promise { try { diff --git a/api.planx.uk/modules/analytics/metabase/collection/types.ts b/api.planx.uk/modules/analytics/metabase/collection/types.ts index f35d9cb690..664ec72f45 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/types.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts @@ -6,21 +6,6 @@ type ApiResponse = { error?: string; }; -export const checkCollectionsSchema = z.object({ - query: z.object({ - name: z.string().min(1, "Name parameter is required"), - }), -}); - -export type CheckCollectionsRequest = ValidatedRequestHandler< - typeof checkCollectionsSchema, - ApiResponse ->; - -export interface CheckCollectionResponse { - collectionId: number | false; -} - export interface NewCollectionParams { /** The name of the new collection */ name: string; From 22d3ee4b63d4b2b76163e1aa84fb57072f5d07d8 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 2 Dec 2024 13:17:02 +0000 Subject: [PATCH 11/46] chore: simplify error handling to just throw and let errorHandler catch --- .../analytics/metabase/collection/service.ts | 24 +------------------ 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index 0781e6a7cd..98f998ad5e 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -56,20 +56,6 @@ export async function newCollection({ return response.data.id; } catch (error) { console.error("Error in newCollection:"); - if (axios.isAxiosError(error)) { - if (error.response) { - console.error("Response data:", error.response.data); - console.error("Response status:", error.response.status); - console.error("Response headers:", error.response.headers); - } else if (error.request) { - console.error("No response received. Request:", error.request); - } else { - console.error("Error message:", error.message); - } - console.error("Request config:", error.config); - } else { - console.error("Unexpected error:", error); - } throw error; } } @@ -98,18 +84,10 @@ export async function checkCollections(teamName: string): Promise { } catch (error) { console.error("Error: ", error); if (error instanceof MetabaseError) { - console.error("MetabaseError:", { + console.error("Metabase API error:", { message: error.message, statusCode: error.statusCode, - response: error.response, }); - } else if (error instanceof Error) { - console.error("Unexpected error:", { - message: error.message, - stack: error.stack, - }); - } else { - console.error("Unknown error:", error); } throw error; } From e19ce9993041f3d7571d12d0a4ab265505b34a1c Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 2 Dec 2024 13:19:14 +0000 Subject: [PATCH 12/46] nit: correctly rename type as handler --- .../modules/analytics/metabase/collection/controller.ts | 4 ++-- api.planx.uk/modules/analytics/metabase/collection/types.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/controller.ts b/api.planx.uk/modules/analytics/metabase/collection/controller.ts index db3d1babae..7250a281ab 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/controller.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/controller.ts @@ -1,7 +1,7 @@ import { newCollection } from "./service.js"; -import type { NewCollectionRequest } from "./types.js"; +import type { NewCollectionRequestHandler } from "./types.js"; -export const newCollectionController: NewCollectionRequest = async ( +export const newCollectionController: NewCollectionRequestHandler = async ( _req, res, ) => { diff --git a/api.planx.uk/modules/analytics/metabase/collection/types.ts b/api.planx.uk/modules/analytics/metabase/collection/types.ts index 664ec72f45..1f575e7bb9 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/types.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts @@ -28,7 +28,7 @@ export const newCollectionSchema = z.object({ }), }); -export type NewCollectionRequest = ValidatedRequestHandler< +export type NewCollectionRequestHandler = ValidatedRequestHandler< typeof newCollectionSchema, ApiResponse >; From bc133426cd5a92172a63c5f871db49b0b2fa3063 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 2 Dec 2024 13:28:21 +0000 Subject: [PATCH 13/46] chore: tidy data types and undefined values --- .../modules/analytics/metabase/collection/service.ts | 8 +------- .../modules/analytics/metabase/collection/types.ts | 10 ++++------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index 98f998ad5e..b246a1dbfc 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -16,8 +16,6 @@ export async function newCollection({ name, description, parent_id, - namespace, - authority_level, }: NewCollectionParams): Promise { try { // Check if collection exists @@ -34,8 +32,6 @@ export async function newCollection({ name, description, parent_id, - namespace, - authority_level, }; // Remove undefined properties @@ -47,8 +43,6 @@ export async function newCollection({ name, description, parent_id, - namespace, - authority_level, }); console.log( `New collection: ${response.data.name}, new collection ID: ${response.data.id}`, @@ -79,7 +73,7 @@ export async function checkCollections(teamName: string): Promise { return matchingCollection.id; } else { console.log("No matching collection found"); - return false; + return undefined; } } catch (error) { console.error("Error: ", error); diff --git a/api.planx.uk/modules/analytics/metabase/collection/types.ts b/api.planx.uk/modules/analytics/metabase/collection/types.ts index 1f575e7bb9..bec010cd8c 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/types.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts @@ -7,24 +7,22 @@ type ApiResponse = { }; export interface NewCollectionParams { - /** The name of the new collection */ name: string; description?: string; /** Optional; if the collection is a child of a parent, specify parent ID here * For council teams, parent collection should be 58 */ parent_id?: number; - namespace?: string; - authority_level?: null; } +/** Metbase collection ID for the the "Council" collection **/ +const COUNCIL_COLLECTION_ID = 58; + export const newCollectionSchema = z.object({ body: z.object({ name: z.string(), description: z.string().optional(), - parent_id: z.number().optional(), - namespace: z.string().optional(), - authority_level: z.null().optional(), + parent_id: z.number().default(COUNCIL_COLLECTION_ID), }), }); From 97ee946cd857cefc28136794ca61dc9408cd210e Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 2 Dec 2024 13:38:06 +0000 Subject: [PATCH 14/46] chore: add snake_case and camelCase conversions --- .../analytics/metabase/collection/service.ts | 6 ++-- .../analytics/metabase/shared/utils.ts | 34 +++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 api.planx.uk/modules/analytics/metabase/shared/utils.ts diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index b246a1dbfc..49d6301ada 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -1,6 +1,6 @@ import { MetabaseError, metabaseClient } from "../shared/client.js"; import type { NewCollectionParams } from "./types.js"; -import axios from "axios"; +import { toSnakeCase } from "../shared/utils.js"; export async function authentication(): Promise { try { @@ -28,11 +28,11 @@ export async function newCollection({ } // If no existing collection, create new one - const requestBody: any = { + const requestBody = toSnakeCase({ name, description, parent_id, - }; + }); // Remove undefined properties Object.keys(requestBody).forEach( diff --git a/api.planx.uk/modules/analytics/metabase/shared/utils.ts b/api.planx.uk/modules/analytics/metabase/shared/utils.ts new file mode 100644 index 0000000000..084aca384b --- /dev/null +++ b/api.planx.uk/modules/analytics/metabase/shared/utils.ts @@ -0,0 +1,34 @@ +/** + * Converts object keys from camelCase to snake_case + */ +export function toSnakeCase(obj: T): Record { + return Object.entries(obj).reduce( + (acc, [key, value]) => { + // Convert camelCase to snake_case + const snakeKey = key.replace( + /[A-Z]/g, + (letter) => `_${letter.toLowerCase()}`, + ); + acc[snakeKey] = value; + return acc; + }, + {} as Record, + ); +} + +/** + * Converts object keys from snake_case to camelCase + */ +export function toCamelCase(obj: T): Record { + return Object.entries(obj).reduce( + (acc, [key, value]) => { + // Convert snake_case to camelCase + const camelKey = key.replace(/_([a-z])/g, (_, letter) => + letter.toUpperCase(), + ); + acc[camelKey] = value; + return acc; + }, + {} as Record, + ); +} From 1a4ca85d81f89284896db0e16ca757ed823093c5 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 2 Dec 2024 14:31:08 +0000 Subject: [PATCH 15/46] chore: re-import createMetabaseClient after rebase --- .../modules/analytics/metabase/collection/service.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index 49d6301ada..d9bd0856e0 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -1,10 +1,12 @@ -import { MetabaseError, metabaseClient } from "../shared/client.js"; +import { MetabaseError, createMetabaseClient } from "../shared/client.js"; import type { NewCollectionParams } from "./types.js"; import { toSnakeCase } from "../shared/utils.js"; +const client = createMetabaseClient(); + export async function authentication(): Promise { try { - const response = await metabaseClient.get("/user/current"); + const response = await client.get("/user/current"); return response.status === 200; } catch (error) { console.error("Error testing Metabase connection:", error); @@ -39,7 +41,7 @@ export async function newCollection({ (key) => requestBody[key] === undefined && delete requestBody[key], ); - const response = await metabaseClient.post(`/api/collection/`, { + const response = await client.post(`/api/collection/`, { name, description, parent_id, @@ -62,7 +64,7 @@ export async function checkCollections(teamName: string): Promise { console.log("Checking for collection: ", teamName); // Get collections from Metabase - const response = await metabaseClient.get(`/api/collection/`); + const response = await client.get(`/api/collection/`); const matchingCollection = response.data.find((collection: any) => collection.name.toLowerCase().includes(teamName.toLowerCase()), From 6ac50e765b821dd2559b3a1f96918f5a6ba9a78f Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 3 Dec 2024 11:06:33 +0000 Subject: [PATCH 16/46] feat: remove manual toSnakeCase function --- .../analytics/metabase/shared/utils.ts | 34 ------------------- 1 file changed, 34 deletions(-) delete mode 100644 api.planx.uk/modules/analytics/metabase/shared/utils.ts diff --git a/api.planx.uk/modules/analytics/metabase/shared/utils.ts b/api.planx.uk/modules/analytics/metabase/shared/utils.ts deleted file mode 100644 index 084aca384b..0000000000 --- a/api.planx.uk/modules/analytics/metabase/shared/utils.ts +++ /dev/null @@ -1,34 +0,0 @@ -/** - * Converts object keys from camelCase to snake_case - */ -export function toSnakeCase(obj: T): Record { - return Object.entries(obj).reduce( - (acc, [key, value]) => { - // Convert camelCase to snake_case - const snakeKey = key.replace( - /[A-Z]/g, - (letter) => `_${letter.toLowerCase()}`, - ); - acc[snakeKey] = value; - return acc; - }, - {} as Record, - ); -} - -/** - * Converts object keys from snake_case to camelCase - */ -export function toCamelCase(obj: T): Record { - return Object.entries(obj).reduce( - (acc, [key, value]) => { - // Convert snake_case to camelCase - const camelKey = key.replace(/_([a-z])/g, (_, letter) => - letter.toUpperCase(), - ); - acc[camelKey] = value; - return acc; - }, - {} as Record, - ); -} From 1d8dfb419fd6592315da0853ce6ea4cb73b65b6b Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 3 Dec 2024 14:32:57 +0000 Subject: [PATCH 17/46] feat: add metabase collection route --- api.planx.uk/modules/analytics/routes.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/api.planx.uk/modules/analytics/routes.ts b/api.planx.uk/modules/analytics/routes.ts index 9dcaa69010..931ffd0f10 100644 --- a/api.planx.uk/modules/analytics/routes.ts +++ b/api.planx.uk/modules/analytics/routes.ts @@ -5,6 +5,8 @@ import { logUserExitController, logUserResumeController, } from "./analyticsLog/controller.js"; +import { newCollectionController } from "./metabase/collection/controller.js"; +import { newCollectionSchema } from "./metabase/collection/types.js"; const router = Router(); @@ -18,5 +20,10 @@ router.post( validate(logAnalyticsSchema), logUserResumeController, ); +router.post( + "/collection", + validate(newCollectionSchema), + newCollectionController, +); export default router; From ae037aa8e8dabdd8f35263d6215410580ae4c117 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 3 Dec 2024 14:34:16 +0000 Subject: [PATCH 18/46] feat: remove unnecessary authentication and transformation --- .../analytics/metabase/collection/service.ts | 38 ++----------------- 1 file changed, 4 insertions(+), 34 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index d9bd0856e0..d10aaf287d 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -1,51 +1,21 @@ import { MetabaseError, createMetabaseClient } from "../shared/client.js"; import type { NewCollectionParams } from "./types.js"; -import { toSnakeCase } from "../shared/utils.js"; const client = createMetabaseClient(); -export async function authentication(): Promise { - try { - const response = await client.get("/user/current"); - return response.status === 200; - } catch (error) { - console.error("Error testing Metabase connection:", error); - return false; - } -} - -export async function newCollection({ - name, - description, - parent_id, -}: NewCollectionParams): Promise { +export async function newCollection(params: NewCollectionParams): Promise { try { // Check if collection exists - const existingCollectionId = await checkCollections(name); + const existingCollectionId = await checkCollections(params.name); if (existingCollectionId) { console.log( - `Collection "${name}" already exists with ID: ${existingCollectionId}`, + `Collection "${params.name}" already exists with ID: ${existingCollectionId}`, ); return existingCollectionId; } - // If no existing collection, create new one - const requestBody = toSnakeCase({ - name, - description, - parent_id, - }); - - // Remove undefined properties - Object.keys(requestBody).forEach( - (key) => requestBody[key] === undefined && delete requestBody[key], - ); + const response = await client.post(`/api/collection/`, params); - const response = await client.post(`/api/collection/`, { - name, - description, - parent_id, - }); console.log( `New collection: ${response.data.name}, new collection ID: ${response.data.id}`, ); From 9d6878c014c634fe3b0f50829835218c6e35584d Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 3 Dec 2024 14:36:35 +0000 Subject: [PATCH 19/46] feat: transform request in newCollectionSchema from camelCase to snake_case --- .../analytics/metabase/collection/types.ts | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/types.ts b/api.planx.uk/modules/analytics/metabase/collection/types.ts index bec010cd8c..f9293f85ad 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/types.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts @@ -6,24 +6,36 @@ type ApiResponse = { error?: string; }; +/** Interface for incoming request, in camelCase */ export interface NewCollectionParams { name: string; description?: string; - /** Optional; if the collection is a child of a parent, specify parent ID here - * For council teams, parent collection should be 58 - */ + /** Optional; if the collection is a child of a parent, specify parent ID here */ + parentId?: number; +} + +/** Interface for request after transforming to snake case (Metabase takes snake while PlanX API takes camel) */ +export interface MetabaseCollectionParams { + name: string; + description?: string; parent_id?: number; } /** Metbase collection ID for the the "Council" collection **/ -const COUNCIL_COLLECTION_ID = 58; +const COUNCILS_COLLECTION_ID = 58; export const newCollectionSchema = z.object({ - body: z.object({ - name: z.string(), - description: z.string().optional(), - parent_id: z.number().default(COUNCIL_COLLECTION_ID), - }), + body: z + .object({ + name: z.string(), + description: z.string().optional(), + parentId: z.number().default(COUNCILS_COLLECTION_ID), + }) + .transform((data) => ({ + name: data.name, + description: data.description, + parent_id: data.parentId, + })), }); export type NewCollectionRequestHandler = ValidatedRequestHandler< From cea1b05b613072f4815b5c28598a0283efec08ca Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 9 Dec 2024 10:15:06 +0000 Subject: [PATCH 20/46] feat: add metabase_id column to teams table migration --- .../down.sql | 1 + .../up.sql | 2 ++ 2 files changed, 3 insertions(+) create mode 100644 hasura.planx.uk/migrations/1733738650997_alter_table_public_teams_add_column_metabase_id/down.sql create mode 100644 hasura.planx.uk/migrations/1733738650997_alter_table_public_teams_add_column_metabase_id/up.sql diff --git a/hasura.planx.uk/migrations/1733738650997_alter_table_public_teams_add_column_metabase_id/down.sql b/hasura.planx.uk/migrations/1733738650997_alter_table_public_teams_add_column_metabase_id/down.sql new file mode 100644 index 0000000000..c620678461 --- /dev/null +++ b/hasura.planx.uk/migrations/1733738650997_alter_table_public_teams_add_column_metabase_id/down.sql @@ -0,0 +1 @@ +alter table "public"."teams" drop column "metabase_id"; diff --git a/hasura.planx.uk/migrations/1733738650997_alter_table_public_teams_add_column_metabase_id/up.sql b/hasura.planx.uk/migrations/1733738650997_alter_table_public_teams_add_column_metabase_id/up.sql new file mode 100644 index 0000000000..8ab49dd71b --- /dev/null +++ b/hasura.planx.uk/migrations/1733738650997_alter_table_public_teams_add_column_metabase_id/up.sql @@ -0,0 +1,2 @@ +alter table "public"."teams" add column "metabase_id" integer + null; From df3345eac1babf2bf5f30d2f345a06b717829e7a Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 9 Dec 2024 15:17:29 +0000 Subject: [PATCH 21/46] chore: add assert import --- api.planx.uk/modules/analytics/metabase/shared/client.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/api.planx.uk/modules/analytics/metabase/shared/client.ts b/api.planx.uk/modules/analytics/metabase/shared/client.ts index 17c4b7b07c..9a52783d52 100644 --- a/api.planx.uk/modules/analytics/metabase/shared/client.ts +++ b/api.planx.uk/modules/analytics/metabase/shared/client.ts @@ -1,4 +1,5 @@ import axios from "axios"; +import assert from "assert"; import type { AxiosInstance, AxiosError, From 628c151521cb0082b6d905321d142e371a3c9008 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 9 Dec 2024 15:18:10 +0000 Subject: [PATCH 22/46] wip: store metabase id to planx db --- .../metabase/collection/addMetabaseId.ts | 38 +++++++++++++++++++ .../analytics/metabase/collection/service.ts | 5 ++- .../analytics/metabase/collection/types.ts | 4 +- 3 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 api.planx.uk/modules/analytics/metabase/collection/addMetabaseId.ts diff --git a/api.planx.uk/modules/analytics/metabase/collection/addMetabaseId.ts b/api.planx.uk/modules/analytics/metabase/collection/addMetabaseId.ts new file mode 100644 index 0000000000..78a067f480 --- /dev/null +++ b/api.planx.uk/modules/analytics/metabase/collection/addMetabaseId.ts @@ -0,0 +1,38 @@ +import { gql } from "graphql-request"; +import { $public } from "../../../../client/index.js"; + +interface UpdateMetabaseId { + teams: { + id: number; + name: string; + metabaseId: number; + }; +} + +export const updateMetabaseId = async (teamId: number, metabaseId: number) => { + console.log({ teamId, metabaseId }, typeof teamId, typeof metabaseId); + try { + const response = await $public.client.request( + gql` + mutation UpdateTeamMetabaseId($id: Int!, $metabaseId: Int!) { + update_teams(where: {id: {_eq:$id}}, _set:{ metabase_id: $metabaseId }) { + returning { id + name + metabase_id + } + } + `, + { + id: teamId, + metabaseId: metabaseId, + }, + ); + console.log({ response }); + return response; + } catch (e) { + console.error( + "There's been an error while updating the Metabase ID for this team", + (e as Error).stack, + ); + } +}; diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index d10aaf287d..993c8d7ac4 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -1,4 +1,5 @@ import { MetabaseError, createMetabaseClient } from "../shared/client.js"; +import { updateMetabaseId } from "./addMetabaseId.js"; import type { NewCollectionParams } from "./types.js"; const client = createMetabaseClient(); @@ -14,14 +15,16 @@ export async function newCollection(params: NewCollectionParams): Promise { return existingCollectionId; } + console.log({ params }); const response = await client.post(`/api/collection/`, params); console.log( `New collection: ${response.data.name}, new collection ID: ${response.data.id}`, ); + await updateMetabaseId(3, response.data.id); // hard-coded team id return response.data.id; } catch (error) { - console.error("Error in newCollection:"); + console.error("Error in newCollection:", error); throw error; } } diff --git a/api.planx.uk/modules/analytics/metabase/collection/types.ts b/api.planx.uk/modules/analytics/metabase/collection/types.ts index f9293f85ad..78a16fcf4e 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/types.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts @@ -22,14 +22,14 @@ export interface MetabaseCollectionParams { } /** Metbase collection ID for the the "Council" collection **/ -const COUNCILS_COLLECTION_ID = 58; +// const COUNCILS_COLLECTION_ID = 58; export const newCollectionSchema = z.object({ body: z .object({ name: z.string(), description: z.string().optional(), - parentId: z.number().default(COUNCILS_COLLECTION_ID), + parentId: z.number().optional(), //.default(COUNCILS_COLLECTION_ID), }) .transform((data) => ({ name: data.name, From 3974b2cf9a1e054d805982bdb538575715911c4b Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 10 Dec 2024 10:09:15 +0000 Subject: [PATCH 23/46] feat: update Hasura api permissions --- hasura.planx.uk/metadata/tables.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/hasura.planx.uk/metadata/tables.yaml b/hasura.planx.uk/metadata/tables.yaml index 3a24978694..c880711a6d 100644 --- a/hasura.planx.uk/metadata/tables.yaml +++ b/hasura.planx.uk/metadata/tables.yaml @@ -2366,6 +2366,7 @@ - created_at - domain - id + - metabase_id - name - slug - updated_at @@ -2418,6 +2419,13 @@ - updated_at filter: {} update_permissions: + - role: api + permission: + columns: + - metabase_id + filter: {} + check: {} + comment: "" - role: platformAdmin permission: columns: From 1d885e0626e82f7eae45ab872ecae69f468dc486 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 2 Dec 2024 17:07:03 +0000 Subject: [PATCH 24/46] test: write tests for collections service --- .../metabase/collection/collection.test.ts | 203 +++++++++++++++++- 1 file changed, 202 insertions(+), 1 deletion(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts index 1b66e90a88..0ef3b33c29 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts @@ -1 +1,202 @@ -test.todo("should test collection check and creation"); +import { newCollection, getCollection } from "./service.js"; +import nock from "nock"; +import { MetabaseError } from "../shared/client.js"; + +describe("newCollection", () => { + beforeEach(() => { + nock.cleanAll(); + }); + + test("returns a collection ID if collection exists", async () => { + // Mock collection check endpoint + nock(process.env.METABASE_URL_EXT!) + .get("/api/collection/") + .reply(200, [ + { id: 20, name: "Barnet" }, + { id: 21, name: "Another collection" }, + ]); + + const collection = await newCollection({ + name: "Barnet", + }); + console.log("HERE IS THE COLLECTION: ", collection); + expect(collection).toBe(20); + }); + + test("successfully places new collection in parent", async () => { + const testName = "Example council"; + + // Mock collection check endpoint + nock(process.env.METABASE_URL_EXT!).get("/api/collection/").reply(200, []); + + // Mock collection creation endpoint + nock(process.env.METABASE_URL_EXT!) + .post("/api/collection/", { + name: testName, + parent_id: 100, + }) + .reply(200, { + id: 123, + name: testName, + parent_id: 100, + }); + + // Mock GET request for verifying the new collection + nock(process.env.METABASE_URL_EXT!).get("/api/collection/123").reply(200, { + id: 123, + name: testName, + parent_id: 100, + }); + + const collectionId = await newCollection({ + name: testName, + parentId: 100, + }); + + // Check the ID is returned correctly + expect(collectionId).toBe(123); + + // Verify the collection details using the service function + const collection = await getCollection(collectionId); + expect(collection.parent_id).toBe(100); + }); + + test("returns collection correctly no matter collection name case", async () => { + // Mock collection check endpoint + nock(process.env.METABASE_URL_EXT!) + .get("/api/collection/") + .reply(200, [ + { id: 20, name: "Barnet" }, + { id: 21, name: "Another collection" }, + ]); + + const collection = await newCollection({ + name: "BARNET", + }); + expect(collection).toBe(20); + }); + + test("successfully creates a new collection and returns its ID", async () => { + const testName = "Example council"; + + // Mock collection check endpoint + nock(process.env.METABASE_URL_EXT!).get("/api/collection/").reply(200, []); + + // Mock collection creation endpoint + nock(process.env.METABASE_URL_EXT!) + .post("/api/collection/", { + name: testName, + }) + .reply(200, { + id: 123, + name: testName, + }); + + const collection = await newCollection({ + name: testName, + }); + + expect(collection).toBe(123); + }); + + test("throws error if network failure", async () => { + nock(process.env.METABASE_URL_EXT!) + .get("/api/collection/") + .replyWithError("Network error occurred"); + + await expect( + newCollection({ + name: "Test Collection", + }), + ).rejects.toThrow("Network error occurred"); + }); + + test("throws error if API error", async () => { + nock(process.env.METABASE_URL_EXT!).get("/api/collection/").reply(400, { + message: "Bad request", + }); + + await expect( + newCollection({ + name: "Test Collection", + }), + ).rejects.toThrow(MetabaseError); + }); + + test("correctly transforms request to snake case", async () => { + const testData = { + name: "Test Collection", + parentId: 123, // This should become parent_id + }; + + // Mock the check for existing collections + nock(process.env.METABASE_URL_EXT!).get("/api/collection/").reply(200, []); + + // Create a variable to store the request body + let capturedBody: any; + + // Mock and verify the POST request, capturing the body + nock(process.env.METABASE_URL_EXT!) + .post("/api/collection/", (body) => { + capturedBody = body; + return true; // Return true to indicate the request matches + }) + .reply(200, { id: 456 }); + + await newCollection(testData); + + // Verify the transformation + expect(capturedBody).toHaveProperty("parent_id", 123); + expect(capturedBody).not.toHaveProperty("parentId"); + }); +}); + +describe("edge cases", () => { + test("handles missing name", async () => { + await expect( + newCollection({ + name: "", + }), + ).rejects.toThrow(); + }); + + test("handles names with special characters", async () => { + const specialName = "@#$%^&*"; + + nock(process.env.METABASE_URL_EXT!).get("/api/collection/").reply(200, []); + + nock(process.env.METABASE_URL_EXT!) + .post("/api/collection/", { + name: specialName, + }) + .reply(200, { + id: 789, + name: specialName, + }); + + const collection = await newCollection({ + name: specialName, + }); + expect(collection).toBe(789); + }); + + test("handles very long names", async () => { + const longName = "A".repeat(101); + + nock(process.env.METABASE_URL_EXT!).get("/api/collection/").reply(200, []); + + nock(process.env.METABASE_URL_EXT!) + .post("/api/collection/", { + name: longName, + }) + .reply(400, { + message: "Name too long", + }); + + await expect( + newCollection({ + name: longName, + }), + ).rejects.toThrow(); + }); +}); From 62ca29527b4c4ae0d2dd8a6555b72f9b4f72743f Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 10 Dec 2024 10:15:31 +0000 Subject: [PATCH 25/46] chore: update user role to api --- .../metabase/collection/addMetabaseId.ts | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/addMetabaseId.ts b/api.planx.uk/modules/analytics/metabase/collection/addMetabaseId.ts index 78a067f480..44d17e4418 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/addMetabaseId.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/addMetabaseId.ts @@ -1,5 +1,5 @@ import { gql } from "graphql-request"; -import { $public } from "../../../../client/index.js"; +import { $api } from "../../../../client/index.js"; interface UpdateMetabaseId { teams: { @@ -10,18 +10,23 @@ interface UpdateMetabaseId { } export const updateMetabaseId = async (teamId: number, metabaseId: number) => { - console.log({ teamId, metabaseId }, typeof teamId, typeof metabaseId); + console.log({ teamId, metabaseId }); try { - const response = await $public.client.request( + const response = await $api.client.request( gql` - mutation UpdateTeamMetabaseId($id: Int!, $metabaseId: Int!) { - update_teams(where: {id: {_eq:$id}}, _set:{ metabase_id: $metabaseId }) { - returning { id - name - metabase_id - } - } - `, + mutation UpdateTeamMetabaseId($id: Int!, $metabaseId: Int!) { + update_teams( + where: { id: { _eq: $id } } + _set: { metabase_id: $metabaseId } + ) { + returning { + id + name + metabase_id + } + } + } + `, { id: teamId, metabaseId: metabaseId, From 052b0f33e2f08f996f3043fdf30fbeb27ff31ba2 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 10 Dec 2024 10:16:44 +0000 Subject: [PATCH 26/46] feat: change string matching to exact --- .../modules/analytics/metabase/collection/service.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index 993c8d7ac4..a56c45e286 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -12,6 +12,7 @@ export async function newCollection(params: NewCollectionParams): Promise { console.log( `Collection "${params.name}" already exists with ID: ${existingCollectionId}`, ); + await updateMetabaseId(1, existingCollectionId); // TODO: remove hard-coded team id return existingCollectionId; } @@ -21,7 +22,7 @@ export async function newCollection(params: NewCollectionParams): Promise { console.log( `New collection: ${response.data.name}, new collection ID: ${response.data.id}`, ); - await updateMetabaseId(3, response.data.id); // hard-coded team id + await updateMetabaseId(1, response.data.id); // TODO: remove hard-coded team id return response.data.id; } catch (error) { console.error("Error in newCollection:", error); @@ -39,8 +40,9 @@ export async function checkCollections(teamName: string): Promise { // Get collections from Metabase const response = await client.get(`/api/collection/`); - const matchingCollection = response.data.find((collection: any) => - collection.name.toLowerCase().includes(teamName.toLowerCase()), + const matchingCollection = response.data.find( + (collection: any) => + collection.name.toLowerCase === teamName.toLowerCase(), ); if (matchingCollection) { From bb3c642c4d719e3999b7b5cc024e075a75aae683 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Mon, 2 Dec 2024 16:56:23 +0000 Subject: [PATCH 27/46] test: add tsdoc and getCollection function for tests --- .../analytics/metabase/collection/service.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index a56c45e286..67480fb867 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -4,6 +4,12 @@ import type { NewCollectionParams } from "./types.js"; const client = createMetabaseClient(); +/** + * First checks if a collection with a specified name exists. + * If it exists, return an object that includes its id. If not, create the collection. + * @params `name` is required, but `description` and `parent_id` are optional. + * @returns `response.data`, so use dot notation to access `id` or `parent_id`. + */ export async function newCollection(params: NewCollectionParams): Promise { try { // Check if collection exists @@ -63,3 +69,13 @@ export async function checkCollections(teamName: string): Promise { throw error; } } + +/** + * Retrieves info on a collection from Metabase, use to check a parent + * @param id + * @returns + */ +export async function getCollection(id: number): Promise { + const response = await client.get(`/api/collection/${id}`); + return response.data; +} From d88d118fe5874901f7d3faafbeeace0c7249cf10 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 10 Dec 2024 11:09:17 +0000 Subject: [PATCH 28/46] chore: rename updateMetabaseId file --- .../metabase/collection/{addMetabaseId.ts => updateMetabaseId.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename api.planx.uk/modules/analytics/metabase/collection/{addMetabaseId.ts => updateMetabaseId.ts} (100%) diff --git a/api.planx.uk/modules/analytics/metabase/collection/addMetabaseId.ts b/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts similarity index 100% rename from api.planx.uk/modules/analytics/metabase/collection/addMetabaseId.ts rename to api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts From 0b6153ce44903a25e80d646ec93c7cbca5d58242 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 10 Dec 2024 11:31:00 +0000 Subject: [PATCH 29/46] test: add new test for updateMetabaseId --- .../metabase/collection/collection.test.ts | 95 ++++++++++++++++++- 1 file changed, 90 insertions(+), 5 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts index 0ef3b33c29..77c81baae9 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts @@ -1,15 +1,30 @@ import { newCollection, getCollection } from "./service.js"; import nock from "nock"; import { MetabaseError } from "../shared/client.js"; +import { $api } from "../../../../client/index.js"; +import { updateMetabaseId } from "./updateMetabaseId.js"; describe("newCollection", () => { beforeEach(() => { nock.cleanAll(); + + // Mock GraphQL + vi.spyOn($api.client, "request").mockResolvedValue({ + update_teams: { + returning: [ + { + id: 1, + name: "Test Team", + metabase_id: expect.any(Number), + }, + ], + }, + }); }); test("returns a collection ID if collection exists", async () => { // Mock collection check endpoint - nock(process.env.METABASE_URL_EXT!) + const metabaseMock = nock(process.env.METABASE_URL_EXT!) .get("/api/collection/") .reply(200, [ { id: 20, name: "Barnet" }, @@ -19,18 +34,21 @@ describe("newCollection", () => { const collection = await newCollection({ name: "Barnet", }); - console.log("HERE IS THE COLLECTION: ", collection); expect(collection).toBe(20); + expect(metabaseMock.isDone()).toBe(true); }); test("successfully places new collection in parent", async () => { const testName = "Example council"; + const metabaseMock = nock(process.env.METABASE_URL_EXT!); + + console.log("Setting up mocks..."); // Mock collection check endpoint - nock(process.env.METABASE_URL_EXT!).get("/api/collection/").reply(200, []); + metabaseMock.get("/api/collection/").reply(200, []); // Mock collection creation endpoint - nock(process.env.METABASE_URL_EXT!) + metabaseMock .post("/api/collection/", { name: testName, parent_id: 100, @@ -42,7 +60,7 @@ describe("newCollection", () => { }); // Mock GET request for verifying the new collection - nock(process.env.METABASE_URL_EXT!).get("/api/collection/123").reply(200, { + metabaseMock.get("/api/collection/123").reply(200, { id: 123, name: testName, parent_id: 100, @@ -59,6 +77,7 @@ describe("newCollection", () => { // Verify the collection details using the service function const collection = await getCollection(collectionId); expect(collection.parent_id).toBe(100); + expect(metabaseMock.isDone()).toBe(true); }); test("returns collection correctly no matter collection name case", async () => { @@ -151,7 +170,73 @@ describe("newCollection", () => { }); }); +describe("updateMetabaseId", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + test("successfully updates metabase ID", async () => { + // Mock the GraphQL request + vi.spyOn($api.client, "request").mockResolvedValue({ + update_teams: { + returning: [ + { + id: 1, + name: "Test Team", + metabase_id: 123, + }, + ], + }, + }); + + const result = await updateMetabaseId(1, 123); + + expect(result).toEqual({ + update_teams: { + returning: [ + { + id: 1, + name: "Test Team", + metabase_id: 123, + }, + ], + }, + }); + + expect($api.client.request).toHaveBeenCalledWith(expect.any(String), { + id: 1, + metabaseId: 123, + }); + }); + + test("handles GraphQL error", async () => { + // Mock a failed GraphQL request + vi.spyOn($api.client, "request").mockRejectedValue( + new Error("GraphQL error"), + ); + + await expect(updateMetabaseId(1, 123)).rejects.toThrow("GraphQL error"); + }); +}); + describe("edge cases", () => { + beforeEach(() => { + nock.cleanAll(); + + // Mock GraphQL + vi.spyOn($api.client, "request").mockResolvedValue({ + update_teams: { + returning: [ + { + id: 1, + name: "Test Team", + metabase_id: expect.any(Number), + }, + ], + }, + }); + }); + test("handles missing name", async () => { await expect( newCollection({ From b76bde11bc5583d45adcf06ddb2052b4f7e92179 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 10 Dec 2024 11:31:36 +0000 Subject: [PATCH 30/46] chore: throw error in function --- .../modules/analytics/metabase/collection/updateMetabaseId.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts b/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts index 44d17e4418..3cf9c5aca0 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts @@ -39,5 +39,6 @@ export const updateMetabaseId = async (teamId: number, metabaseId: number) => { "There's been an error while updating the Metabase ID for this team", (e as Error).stack, ); + throw e; } }; From 0b33f0b7b8a4cb91eb787a254da2f4f54c7b2c8d Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 10 Dec 2024 11:34:50 +0000 Subject: [PATCH 31/46] chore: update import after updateMetabaseId name change --- .../modules/analytics/metabase/collection/service.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index 67480fb867..8adf22b335 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -1,5 +1,5 @@ import { MetabaseError, createMetabaseClient } from "../shared/client.js"; -import { updateMetabaseId } from "./addMetabaseId.js"; +import { updateMetabaseId } from "./updateMetabaseId.js"; import type { NewCollectionParams } from "./types.js"; const client = createMetabaseClient(); @@ -23,7 +23,13 @@ export async function newCollection(params: NewCollectionParams): Promise { } console.log({ params }); - const response = await client.post(`/api/collection/`, params); + + const transformedParams = { + name: params.name, + parent_id: params.parentId, + }; + + const response = await client.post(`/api/collection/`, transformedParams); console.log( `New collection: ${response.data.name}, new collection ID: ${response.data.id}`, @@ -48,7 +54,7 @@ export async function checkCollections(teamName: string): Promise { const matchingCollection = response.data.find( (collection: any) => - collection.name.toLowerCase === teamName.toLowerCase(), + collection.name.toLowerCase() === teamName.toLowerCase(), ); if (matchingCollection) { From 7ab0398e0626cebec9fd4a2717e05531857db59c Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 10 Dec 2024 14:32:35 +0000 Subject: [PATCH 32/46] chore: remove console.log --- .../modules/analytics/metabase/collection/updateMetabaseId.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts b/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts index 3cf9c5aca0..4b9ed57bfd 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts @@ -10,7 +10,6 @@ interface UpdateMetabaseId { } export const updateMetabaseId = async (teamId: number, metabaseId: number) => { - console.log({ teamId, metabaseId }); try { const response = await $api.client.request( gql` From 110ce9c4d81ac0a57b685cead47fed164e46ad2e Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 10 Dec 2024 14:34:25 +0000 Subject: [PATCH 33/46] chore: remove old checkCollection --- .../analytics/metabase/collection/service.ts | 57 ++++--------------- 1 file changed, 10 insertions(+), 47 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index 8adf22b335..42e5fed9c5 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -1,6 +1,7 @@ import { MetabaseError, createMetabaseClient } from "../shared/client.js"; import { updateMetabaseId } from "./updateMetabaseId.js"; import type { NewCollectionParams } from "./types.js"; +import { getTeamAndMetabaseId } from "./getMetabaseId.js"; const client = createMetabaseClient(); @@ -12,18 +13,13 @@ const client = createMetabaseClient(); */ export async function newCollection(params: NewCollectionParams): Promise { try { - // Check if collection exists - const existingCollectionId = await checkCollections(params.name); - if (existingCollectionId) { - console.log( - `Collection "${params.name}" already exists with ID: ${existingCollectionId}`, - ); - await updateMetabaseId(1, existingCollectionId); // TODO: remove hard-coded team id - return existingCollectionId; + const teamAndMetabaseId = await getTeamAndMetabaseId(params.name); + const { metabaseId, id } = teamAndMetabaseId; + console.log({ metabaseId }); + if (metabaseId) { + await updateMetabaseId(id, metabaseId); + return metabaseId; } - - console.log({ params }); - const transformedParams = { name: params.name, parent_id: params.parentId, @@ -34,7 +30,7 @@ export async function newCollection(params: NewCollectionParams): Promise { console.log( `New collection: ${response.data.name}, new collection ID: ${response.data.id}`, ); - await updateMetabaseId(1, response.data.id); // TODO: remove hard-coded team id + await updateMetabaseId(id, response.data.id); // TODO: remove hard-coded team id return response.data.id; } catch (error) { console.error("Error in newCollection:", error); @@ -43,44 +39,11 @@ export async function newCollection(params: NewCollectionParams): Promise { } /** - * Checks if a collection exists with name matching `teamName` exists. - * Returns the matching collection ID if exists, otherwise false. */ -export async function checkCollections(teamName: string): Promise { - try { - console.log("Checking for collection: ", teamName); - - // Get collections from Metabase - const response = await client.get(`/api/collection/`); - - const matchingCollection = response.data.find( - (collection: any) => - collection.name.toLowerCase() === teamName.toLowerCase(), - ); - - if (matchingCollection) { - console.log("Matching collection found with ID: ", matchingCollection.id); - return matchingCollection.id; - } else { - console.log("No matching collection found"); - return undefined; - } - } catch (error) { - console.error("Error: ", error); - if (error instanceof MetabaseError) { - console.error("Metabase API error:", { - message: error.message, - statusCode: error.statusCode, - }); - } - throw error; - } -} - -/** - * Retrieves info on a collection from Metabase, use to check a parent + * Retrieves info on a collection from Metabase, use to check a parent. Currently only used in tests but could be useful for other Metabase functionality * @param id * @returns */ +// TODO: is this more suited to be part of the collection.test.ts? export async function getCollection(id: number): Promise { const response = await client.get(`/api/collection/${id}`); return response.data; From 5ec8b7a381c0a0dadb94971eb207436cc6f0aab8 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 10 Dec 2024 16:06:30 +0000 Subject: [PATCH 34/46] feat: getTeamAndMetabaseId function --- .../collection/getTeamAndMetabaseId.ts | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 api.planx.uk/modules/analytics/metabase/collection/getTeamAndMetabaseId.ts diff --git a/api.planx.uk/modules/analytics/metabase/collection/getTeamAndMetabaseId.ts b/api.planx.uk/modules/analytics/metabase/collection/getTeamAndMetabaseId.ts new file mode 100644 index 0000000000..8c929a1e10 --- /dev/null +++ b/api.planx.uk/modules/analytics/metabase/collection/getTeamAndMetabaseId.ts @@ -0,0 +1,44 @@ +import { gql } from "graphql-request"; +import { $api } from "../../../../client/index.js"; + +interface GetMetabaseId { + teams: { + id: number; + name: string; + metabaseId: number | null; + }[]; +} + +export const getTeamAndMetabaseId = async (name: string) => { + const lowerName = name.toLowerCase(); + console.log("RUNNING getTeamAndMetabaseId..."); + try { + const response = await $api.client.request( + gql` + query GetTeamAndMetabaseId($name: String!) { + teams(where: { name: { _eq: $name } }) { + id + name + metabaseId: metabase_id + } + } + `, + { + name: lowerName, + }, + ); + console.log("Raw response:"); + console.log(JSON.stringify(response, null, 2)); + + const result = response.teams[0]; + console.log("Processed result:"); + console.log(JSON.stringify(result, null, 2)); + return result; + } catch (e) { + console.error( + "Error fetching team's ID / Metabase ID from PlanX DB:", + (e as Error).stack, + ); + throw e; + } +}; From f2c5ba65a7ad65ade49dea68acd67c7a87069ac9 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 10 Dec 2024 16:07:16 +0000 Subject: [PATCH 35/46] feat: separate newCollection into own function --- .../analytics/metabase/collection/service.ts | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index 42e5fed9c5..eab376d6f5 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -1,10 +1,26 @@ import { MetabaseError, createMetabaseClient } from "../shared/client.js"; import { updateMetabaseId } from "./updateMetabaseId.js"; import type { NewCollectionParams } from "./types.js"; -import { getTeamAndMetabaseId } from "./getMetabaseId.js"; +import { getTeamAndMetabaseId } from "./getTeamAndMetabaseId.js"; const client = createMetabaseClient(); +export async function createCollection( + params: NewCollectionParams, +): Promise { + const transformedParams = { + name: params.name, + parent_id: params.parentId, + }; + + const response = await client.post(`/api/collection/`, transformedParams); + + console.log( + `New collection: ${response.data.name}, new collection ID: ${response.data.id}`, + ); + return response.data.id; +} + /** * First checks if a collection with a specified name exists. * If it exists, return an object that includes its id. If not, create the collection. @@ -15,23 +31,15 @@ export async function newCollection(params: NewCollectionParams): Promise { try { const teamAndMetabaseId = await getTeamAndMetabaseId(params.name); const { metabaseId, id } = teamAndMetabaseId; - console.log({ metabaseId }); if (metabaseId) { + console.log("Updating MetabaseId..."); await updateMetabaseId(id, metabaseId); return metabaseId; } - const transformedParams = { - name: params.name, - parent_id: params.parentId, - }; - - const response = await client.post(`/api/collection/`, transformedParams); - console.log( - `New collection: ${response.data.name}, new collection ID: ${response.data.id}`, - ); - await updateMetabaseId(id, response.data.id); // TODO: remove hard-coded team id - return response.data.id; + const newMetabaseId = await createCollection(params); + await updateMetabaseId(id, newMetabaseId); + return newMetabaseId; } catch (error) { console.error("Error in newCollection:", error); throw error; From a6f005d7b2643b075d3a701e0cd9871fe7b791a6 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 10 Dec 2024 16:07:45 +0000 Subject: [PATCH 36/46] test: test for collection services --- .../metabase/collection/collection.test.ts | 199 +++++++++--------- 1 file changed, 103 insertions(+), 96 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts index 77c81baae9..a155bb4602 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts @@ -1,52 +1,79 @@ -import { newCollection, getCollection } from "./service.js"; +import { newCollection, getCollection, createCollection } from "./service.js"; import nock from "nock"; import { MetabaseError } from "../shared/client.js"; import { $api } from "../../../../client/index.js"; import { updateMetabaseId } from "./updateMetabaseId.js"; +import { getTeamAndMetabaseId } from "./getTeamAndMetabaseId.js"; describe("newCollection", () => { beforeEach(() => { nock.cleanAll(); + }); - // Mock GraphQL - vi.spyOn($api.client, "request").mockResolvedValue({ - update_teams: { - returning: [ - { - id: 1, - name: "Test Team", - metabase_id: expect.any(Number), - }, - ], - }, + test("returns an existing Metabase collection ID if collection exists", async () => { + // Mock getTeamAndMetabaseId response + vi.spyOn($api.client, "request").mockResolvedValueOnce({ + teams: [ + { + id: 26, + name: "Barnet", + metabaseId: 20, + }, + ], }); + + const collectionId = await getTeamAndMetabaseId("Barnet"); + + expect(collectionId.metabaseId).toBe(20); }); - test("returns a collection ID if collection exists", async () => { - // Mock collection check endpoint + test("creates new collection when metabase ID doesn't exist", async () => { + // Mock getTeamAndMetabaseId response with null metabase_id + vi.spyOn($api.client, "request").mockResolvedValueOnce({ + teams: [ + { + id: 26, + name: "Barnet", + metabase_id: null, + }, + ], + }); + + // Mock Metabase API calls const metabaseMock = nock(process.env.METABASE_URL_EXT!) - .get("/api/collection/") - .reply(200, [ - { id: 20, name: "Barnet" }, - { id: 21, name: "Another collection" }, - ]); + .post("/api/collection/", { + name: "Barnet", + }) + .reply(200, { + id: 123, + name: "Barnet", + }); - const collection = await newCollection({ + const collectionId = await createCollection({ name: "Barnet", }); - expect(collection).toBe(20); + + expect(collectionId).toBe(123); expect(metabaseMock.isDone()).toBe(true); }); test("successfully places new collection in parent", async () => { + // Mock updateMetabaseId response + vi.spyOn($api.client, "request").mockResolvedValueOnce({ + update_teams: { + returning: [ + { + id: 26, + name: "Barnet", + metabase_id: 123, + }, + ], + }, + }); + const testName = "Example council"; const metabaseMock = nock(process.env.METABASE_URL_EXT!); - console.log("Setting up mocks..."); - - // Mock collection check endpoint - metabaseMock.get("/api/collection/").reply(200, []); - // Mock collection creation endpoint metabaseMock .post("/api/collection/", { @@ -66,7 +93,7 @@ describe("newCollection", () => { parent_id: 100, }); - const collectionId = await newCollection({ + const collectionId = await createCollection({ name: testName, parentId: 100, }); @@ -81,41 +108,18 @@ describe("newCollection", () => { }); test("returns collection correctly no matter collection name case", async () => { - // Mock collection check endpoint - nock(process.env.METABASE_URL_EXT!) - .get("/api/collection/") - .reply(200, [ - { id: 20, name: "Barnet" }, - { id: 21, name: "Another collection" }, - ]); - - const collection = await newCollection({ - name: "BARNET", + vi.spyOn($api.client, "request").mockResolvedValueOnce({ + teams: [ + { + id: 26, + name: "barnet", + metabaseId: 20, + }, + ], }); - expect(collection).toBe(20); - }); - - test("successfully creates a new collection and returns its ID", async () => { - const testName = "Example council"; - - // Mock collection check endpoint - nock(process.env.METABASE_URL_EXT!).get("/api/collection/").reply(200, []); - // Mock collection creation endpoint - nock(process.env.METABASE_URL_EXT!) - .post("/api/collection/", { - name: testName, - }) - .reply(200, { - id: 123, - name: testName, - }); - - const collection = await newCollection({ - name: testName, - }); - - expect(collection).toBe(123); + const collection = await getTeamAndMetabaseId("BARNET"); + expect(collection.metabaseId).toBe(20); }); test("throws error if network failure", async () => { @@ -124,7 +128,7 @@ describe("newCollection", () => { .replyWithError("Network error occurred"); await expect( - newCollection({ + createCollection({ name: "Test Collection", }), ).rejects.toThrow("Network error occurred"); @@ -136,37 +140,50 @@ describe("newCollection", () => { }); await expect( - newCollection({ + createCollection({ name: "Test Collection", }), ).rejects.toThrow(MetabaseError); }); +}); - test("correctly transforms request to snake case", async () => { - const testData = { - name: "Test Collection", - parentId: 123, // This should become parent_id - }; +describe("getTeamAndMetabaseId", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); - // Mock the check for existing collections - nock(process.env.METABASE_URL_EXT!).get("/api/collection/").reply(200, []); + test("successfully gets team and existing metabase id", async () => { + vi.spyOn($api.client, "request").mockResolvedValue({ + teams: [ + { + id: 26, + name: "Barnet", + metabaseId: 20, + }, + ], + }); - // Create a variable to store the request body - let capturedBody: any; + const teamAndMetabaseId = await getTeamAndMetabaseId("Barnet"); - // Mock and verify the POST request, capturing the body - nock(process.env.METABASE_URL_EXT!) - .post("/api/collection/", (body) => { - capturedBody = body; - return true; // Return true to indicate the request matches - }) - .reply(200, { id: 456 }); + expect(teamAndMetabaseId.id).toEqual(26); + expect(teamAndMetabaseId.metabaseId).toEqual(20); + }); - await newCollection(testData); + test("handles team with null metabase id", async () => { + vi.spyOn($api.client, "request").mockResolvedValue({ + teams: [ + { + id: 26, + name: "Barnet", + metabaseId: null, + }, + ], + }); + + const teamAndMetabaseId = await getTeamAndMetabaseId("Barnet"); - // Verify the transformation - expect(capturedBody).toHaveProperty("parent_id", 123); - expect(capturedBody).not.toHaveProperty("parentId"); + expect(teamAndMetabaseId.id).toEqual(26); + expect(teamAndMetabaseId.metabaseId).toBeNull(); }); }); @@ -222,19 +239,8 @@ describe("updateMetabaseId", () => { describe("edge cases", () => { beforeEach(() => { nock.cleanAll(); - - // Mock GraphQL - vi.spyOn($api.client, "request").mockResolvedValue({ - update_teams: { - returning: [ - { - id: 1, - name: "Test Team", - metabase_id: expect.any(Number), - }, - ], - }, - }); + vi.clearAllMocks(); + vi.resetAllMocks(); }); test("handles missing name", async () => { @@ -246,6 +252,7 @@ describe("edge cases", () => { }); test("handles names with special characters", async () => { + // TODO const specialName = "@#$%^&*"; nock(process.env.METABASE_URL_EXT!).get("/api/collection/").reply(200, []); @@ -259,7 +266,7 @@ describe("edge cases", () => { name: specialName, }); - const collection = await newCollection({ + const collection = await createCollection({ name: specialName, }); expect(collection).toBe(789); From e234edc8d18b6d6f936d8dd95db5f3bab0a743f4 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 10 Dec 2024 16:18:19 +0000 Subject: [PATCH 37/46] chore: tidy function names --- .../analytics/metabase/collection/collection.test.ts | 12 ++++++++---- .../analytics/metabase/collection/controller.ts | 6 +++--- .../modules/analytics/metabase/collection/service.ts | 12 ++++++++---- .../modules/analytics/metabase/collection/types.ts | 4 ++-- api.planx.uk/modules/analytics/routes.ts | 8 ++++---- 5 files changed, 25 insertions(+), 17 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts index a155bb4602..f540c09a33 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts @@ -1,11 +1,15 @@ -import { newCollection, getCollection, createCollection } from "./service.js"; +import { + checkCollections, + getCollection, + createCollection, +} from "./service.js"; import nock from "nock"; import { MetabaseError } from "../shared/client.js"; import { $api } from "../../../../client/index.js"; import { updateMetabaseId } from "./updateMetabaseId.js"; import { getTeamAndMetabaseId } from "./getTeamAndMetabaseId.js"; -describe("newCollection", () => { +describe("checkCollections", () => { beforeEach(() => { nock.cleanAll(); }); @@ -245,7 +249,7 @@ describe("edge cases", () => { test("handles missing name", async () => { await expect( - newCollection({ + checkCollections({ name: "", }), ).rejects.toThrow(); @@ -286,7 +290,7 @@ describe("edge cases", () => { }); await expect( - newCollection({ + checkCollections({ name: longName, }), ).rejects.toThrow(); diff --git a/api.planx.uk/modules/analytics/metabase/collection/controller.ts b/api.planx.uk/modules/analytics/metabase/collection/controller.ts index 7250a281ab..38af98e500 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/controller.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/controller.ts @@ -1,13 +1,13 @@ -import { newCollection } from "./service.js"; +import { checkCollections } from "./service.js"; import type { NewCollectionRequestHandler } from "./types.js"; -export const newCollectionController: NewCollectionRequestHandler = async ( +export const checkCollectionsController: NewCollectionRequestHandler = async ( _req, res, ) => { try { const params = res.locals.parsedReq.body; - const collection = await newCollection(params); + const collection = await checkCollections(params); res.status(201).json({ data: collection }); } catch (error) { res.status(400).json({ diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index eab376d6f5..d4e151634d 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -22,26 +22,30 @@ export async function createCollection( } /** - * First checks if a collection with a specified name exists. - * If it exists, return an object that includes its id. If not, create the collection. + * First uses name to get teams.id and .metabase_id, if present. + * If metabase_id is null, return an object that includes its id. If not, create the collection. * @params `name` is required, but `description` and `parent_id` are optional. * @returns `response.data`, so use dot notation to access `id` or `parent_id`. */ -export async function newCollection(params: NewCollectionParams): Promise { +export async function checkCollections( + params: NewCollectionParams, +): Promise { try { const teamAndMetabaseId = await getTeamAndMetabaseId(params.name); const { metabaseId, id } = teamAndMetabaseId; + if (metabaseId) { console.log("Updating MetabaseId..."); await updateMetabaseId(id, metabaseId); return metabaseId; } + // Create new Metabase collection if !metabaseId const newMetabaseId = await createCollection(params); await updateMetabaseId(id, newMetabaseId); return newMetabaseId; } catch (error) { - console.error("Error in newCollection:", error); + console.error("Error in checkCollections:", error); throw error; } } diff --git a/api.planx.uk/modules/analytics/metabase/collection/types.ts b/api.planx.uk/modules/analytics/metabase/collection/types.ts index 78a16fcf4e..4bc92a0d32 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/types.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts @@ -24,7 +24,7 @@ export interface MetabaseCollectionParams { /** Metbase collection ID for the the "Council" collection **/ // const COUNCILS_COLLECTION_ID = 58; -export const newCollectionSchema = z.object({ +export const checkCollectionsSchema = z.object({ body: z .object({ name: z.string(), @@ -39,7 +39,7 @@ export const newCollectionSchema = z.object({ }); export type NewCollectionRequestHandler = ValidatedRequestHandler< - typeof newCollectionSchema, + typeof checkCollectionsSchema, ApiResponse >; diff --git a/api.planx.uk/modules/analytics/routes.ts b/api.planx.uk/modules/analytics/routes.ts index 931ffd0f10..2065837e27 100644 --- a/api.planx.uk/modules/analytics/routes.ts +++ b/api.planx.uk/modules/analytics/routes.ts @@ -5,8 +5,8 @@ import { logUserExitController, logUserResumeController, } from "./analyticsLog/controller.js"; -import { newCollectionController } from "./metabase/collection/controller.js"; -import { newCollectionSchema } from "./metabase/collection/types.js"; +import { checkCollectionsController } from "./metabase/collection/controller.js"; +import { checkCollectionsSchema } from "./metabase/collection/types.js"; const router = Router(); @@ -22,8 +22,8 @@ router.post( ); router.post( "/collection", - validate(newCollectionSchema), - newCollectionController, + validate(checkCollectionsSchema), + checkCollectionsController, ); export default router; From c384fee827822659d3e5f0761aa7f69583752313 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Tue, 10 Dec 2024 16:33:35 +0000 Subject: [PATCH 38/46] chore: move createCollection to own file --- .../metabase/collection/collection.test.ts | 7 ++----- .../metabase/collection/createCollection.ts | 20 +++++++++++++++++++ .../analytics/metabase/collection/service.ts | 19 ++---------------- 3 files changed, 24 insertions(+), 22 deletions(-) create mode 100644 api.planx.uk/modules/analytics/metabase/collection/createCollection.ts diff --git a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts index f540c09a33..53bf198827 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts @@ -1,13 +1,10 @@ -import { - checkCollections, - getCollection, - createCollection, -} from "./service.js"; +import { checkCollections, getCollection } from "./service.js"; import nock from "nock"; import { MetabaseError } from "../shared/client.js"; import { $api } from "../../../../client/index.js"; import { updateMetabaseId } from "./updateMetabaseId.js"; import { getTeamAndMetabaseId } from "./getTeamAndMetabaseId.js"; +import { createCollection } from "./createCollection.js"; describe("checkCollections", () => { beforeEach(() => { diff --git a/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts b/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts new file mode 100644 index 0000000000..e2f0df0539 --- /dev/null +++ b/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts @@ -0,0 +1,20 @@ +import { createMetabaseClient } from "../shared/client.js"; +import type { NewCollectionParams } from "./types.js"; + +const client = createMetabaseClient(); + +export async function createCollection( + params: NewCollectionParams, +): Promise { + const transformedParams = { + name: params.name, + parent_id: params.parentId, + }; + + const response = await client.post(`/api/collection/`, transformedParams); + + console.log( + `New collection: ${response.data.name}, new collection ID: ${response.data.id}`, + ); + return response.data.id; +} diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index d4e151634d..a0c3fef8af 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -1,26 +1,11 @@ -import { MetabaseError, createMetabaseClient } from "../shared/client.js"; +import { createMetabaseClient } from "../shared/client.js"; import { updateMetabaseId } from "./updateMetabaseId.js"; import type { NewCollectionParams } from "./types.js"; import { getTeamAndMetabaseId } from "./getTeamAndMetabaseId.js"; +import { createCollection } from "./createCollection.js"; const client = createMetabaseClient(); -export async function createCollection( - params: NewCollectionParams, -): Promise { - const transformedParams = { - name: params.name, - parent_id: params.parentId, - }; - - const response = await client.post(`/api/collection/`, transformedParams); - - console.log( - `New collection: ${response.data.name}, new collection ID: ${response.data.id}`, - ); - return response.data.id; -} - /** * First uses name to get teams.id and .metabase_id, if present. * If metabase_id is null, return an object that includes its id. If not, create the collection. From 83786012804f7fa7f82e6790f3d6d7d2532b65c2 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Wed, 11 Dec 2024 16:26:04 +0000 Subject: [PATCH 39/46] chore: tidy comments and console.logs --- .../metabase/collection/getTeamAndMetabaseId.ts | 10 ++-------- .../modules/analytics/metabase/collection/service.ts | 2 -- .../analytics/metabase/collection/updateMetabaseId.ts | 2 +- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/getTeamAndMetabaseId.ts b/api.planx.uk/modules/analytics/metabase/collection/getTeamAndMetabaseId.ts index 8c929a1e10..78aefed97f 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/getTeamAndMetabaseId.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/getTeamAndMetabaseId.ts @@ -10,13 +10,11 @@ interface GetMetabaseId { } export const getTeamAndMetabaseId = async (name: string) => { - const lowerName = name.toLowerCase(); - console.log("RUNNING getTeamAndMetabaseId..."); try { const response = await $api.client.request( gql` query GetTeamAndMetabaseId($name: String!) { - teams(where: { name: { _eq: $name } }) { + teams(where: { name: { _ilike: $name } }) { id name metabaseId: metabase_id @@ -24,15 +22,11 @@ export const getTeamAndMetabaseId = async (name: string) => { } `, { - name: lowerName, + name: name, }, ); - console.log("Raw response:"); - console.log(JSON.stringify(response, null, 2)); const result = response.teams[0]; - console.log("Processed result:"); - console.log(JSON.stringify(result, null, 2)); return result; } catch (e) { console.error( diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index a0c3fef8af..916aedd3cd 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -20,8 +20,6 @@ export async function checkCollections( const { metabaseId, id } = teamAndMetabaseId; if (metabaseId) { - console.log("Updating MetabaseId..."); - await updateMetabaseId(id, metabaseId); return metabaseId; } diff --git a/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts b/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts index 4b9ed57bfd..29de69e692 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts @@ -9,6 +9,7 @@ interface UpdateMetabaseId { }; } +/** Updates column `metabase_id` in the Planx DB `teams` table */ export const updateMetabaseId = async (teamId: number, metabaseId: number) => { try { const response = await $api.client.request( @@ -31,7 +32,6 @@ export const updateMetabaseId = async (teamId: number, metabaseId: number) => { metabaseId: metabaseId, }, ); - console.log({ response }); return response; } catch (e) { console.error( From 1aa5f1c1aa2ba38ba002906842985e3219749f23 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Wed, 11 Dec 2024 16:29:30 +0000 Subject: [PATCH 40/46] feat: move getCollection to own file --- .../analytics/metabase/collection/collection.test.ts | 3 ++- .../analytics/metabase/collection/getCollection.ts | 12 ++++++++++++ .../modules/analytics/metabase/collection/service.ts | 11 ----------- 3 files changed, 14 insertions(+), 12 deletions(-) create mode 100644 api.planx.uk/modules/analytics/metabase/collection/getCollection.ts diff --git a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts index 53bf198827..0b412249fa 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts @@ -1,4 +1,5 @@ -import { checkCollections, getCollection } from "./service.js"; +import { checkCollections } from "./service.js"; +import { getCollection } from "./getCollection.js"; import nock from "nock"; import { MetabaseError } from "../shared/client.js"; import { $api } from "../../../../client/index.js"; diff --git a/api.planx.uk/modules/analytics/metabase/collection/getCollection.ts b/api.planx.uk/modules/analytics/metabase/collection/getCollection.ts new file mode 100644 index 0000000000..be1064a4a7 --- /dev/null +++ b/api.planx.uk/modules/analytics/metabase/collection/getCollection.ts @@ -0,0 +1,12 @@ +import { createMetabaseClient } from "../shared/client.js"; + +const client = createMetabaseClient(); +/** + * Retrieves info on a collection from Metabase, use to check a parent. Currently only used in tests but could be useful for other Metabase functionality + * @param id + * @returns + */ +export async function getCollection(id: number): Promise { + const response = await client.get(`/api/collection/${id}`); + return response.data; +} diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index 916aedd3cd..c4ad6eb388 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -32,14 +32,3 @@ export async function checkCollections( throw error; } } - -/** - * Retrieves info on a collection from Metabase, use to check a parent. Currently only used in tests but could be useful for other Metabase functionality - * @param id - * @returns - */ -// TODO: is this more suited to be part of the collection.test.ts? -export async function getCollection(id: number): Promise { - const response = await client.get(`/api/collection/${id}`); - return response.data; -} From 08f2402400e5e9366c4c5c9bc7eee0a74512659d Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Wed, 11 Dec 2024 16:33:01 +0000 Subject: [PATCH 41/46] feat: type getCollection response --- .../modules/analytics/metabase/collection/getCollection.ts | 5 ++++- api.planx.uk/modules/analytics/metabase/collection/types.ts | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/getCollection.ts b/api.planx.uk/modules/analytics/metabase/collection/getCollection.ts index be1064a4a7..0187f2d8ed 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/getCollection.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/getCollection.ts @@ -1,4 +1,5 @@ import { createMetabaseClient } from "../shared/client.js"; +import type { GetCollectionResponse } from "./types.js"; const client = createMetabaseClient(); /** @@ -6,7 +7,9 @@ const client = createMetabaseClient(); * @param id * @returns */ -export async function getCollection(id: number): Promise { +export async function getCollection( + id: number, +): Promise { const response = await client.get(`/api/collection/${id}`); return response.data; } diff --git a/api.planx.uk/modules/analytics/metabase/collection/types.ts b/api.planx.uk/modules/analytics/metabase/collection/types.ts index 4bc92a0d32..cade18fc8d 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/types.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts @@ -47,3 +47,9 @@ export interface NewCollectionResponse { id: number; name: string; } + +export interface GetCollectionResponse { + id: number; + name: string; + parent_id: number; +} From 26f9ec27ea33e466a0307f185499104e6d4a7285 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Wed, 11 Dec 2024 16:41:34 +0000 Subject: [PATCH 42/46] chore: type checkCollections response --- api.planx.uk/modules/analytics/metabase/collection/service.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index c4ad6eb388..192a6dad8f 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -14,7 +14,7 @@ const client = createMetabaseClient(); */ export async function checkCollections( params: NewCollectionParams, -): Promise { +): Promise { try { const teamAndMetabaseId = await getTeamAndMetabaseId(params.name); const { metabaseId, id } = teamAndMetabaseId; @@ -26,6 +26,7 @@ export async function checkCollections( // Create new Metabase collection if !metabaseId const newMetabaseId = await createCollection(params); await updateMetabaseId(id, newMetabaseId); + console.log({ newMetabaseId }); return newMetabaseId; } catch (error) { console.error("Error in checkCollections:", error); From ee3a730b578cbcb65725d4a331d5a793d58d43f0 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Wed, 11 Dec 2024 16:43:49 +0000 Subject: [PATCH 43/46] chore: rename to getTeamIdAndMetabaseId for clarity --- .../metabase/collection/collection.test.ts | 16 ++++++++-------- ...ndMetabaseId.ts => getTeamIdAndMetabaseId.ts} | 2 +- .../analytics/metabase/collection/service.ts | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) rename api.planx.uk/modules/analytics/metabase/collection/{getTeamAndMetabaseId.ts => getTeamIdAndMetabaseId.ts} (92%) diff --git a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts index 0b412249fa..4498326f78 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts @@ -4,7 +4,7 @@ import nock from "nock"; import { MetabaseError } from "../shared/client.js"; import { $api } from "../../../../client/index.js"; import { updateMetabaseId } from "./updateMetabaseId.js"; -import { getTeamAndMetabaseId } from "./getTeamAndMetabaseId.js"; +import { getTeamIdAndMetabaseId } from "./getTeamIdAndMetabaseId.js"; import { createCollection } from "./createCollection.js"; describe("checkCollections", () => { @@ -13,7 +13,7 @@ describe("checkCollections", () => { }); test("returns an existing Metabase collection ID if collection exists", async () => { - // Mock getTeamAndMetabaseId response + // Mock getTeamIdAndMetabaseId response vi.spyOn($api.client, "request").mockResolvedValueOnce({ teams: [ { @@ -24,13 +24,13 @@ describe("checkCollections", () => { ], }); - const collectionId = await getTeamAndMetabaseId("Barnet"); + const collectionId = await getTeamIdAndMetabaseId("Barnet"); expect(collectionId.metabaseId).toBe(20); }); test("creates new collection when metabase ID doesn't exist", async () => { - // Mock getTeamAndMetabaseId response with null metabase_id + // Mock getTeamIdAndMetabaseId response with null metabase_id vi.spyOn($api.client, "request").mockResolvedValueOnce({ teams: [ { @@ -120,7 +120,7 @@ describe("checkCollections", () => { ], }); - const collection = await getTeamAndMetabaseId("BARNET"); + const collection = await getTeamIdAndMetabaseId("BARNET"); expect(collection.metabaseId).toBe(20); }); @@ -149,7 +149,7 @@ describe("checkCollections", () => { }); }); -describe("getTeamAndMetabaseId", () => { +describe("getTeamIdAndMetabaseId", () => { beforeEach(() => { vi.clearAllMocks(); }); @@ -165,7 +165,7 @@ describe("getTeamAndMetabaseId", () => { ], }); - const teamAndMetabaseId = await getTeamAndMetabaseId("Barnet"); + const teamAndMetabaseId = await getTeamIdAndMetabaseId("Barnet"); expect(teamAndMetabaseId.id).toEqual(26); expect(teamAndMetabaseId.metabaseId).toEqual(20); @@ -182,7 +182,7 @@ describe("getTeamAndMetabaseId", () => { ], }); - const teamAndMetabaseId = await getTeamAndMetabaseId("Barnet"); + const teamAndMetabaseId = await getTeamIdAndMetabaseId("Barnet"); expect(teamAndMetabaseId.id).toEqual(26); expect(teamAndMetabaseId.metabaseId).toBeNull(); diff --git a/api.planx.uk/modules/analytics/metabase/collection/getTeamAndMetabaseId.ts b/api.planx.uk/modules/analytics/metabase/collection/getTeamIdAndMetabaseId.ts similarity index 92% rename from api.planx.uk/modules/analytics/metabase/collection/getTeamAndMetabaseId.ts rename to api.planx.uk/modules/analytics/metabase/collection/getTeamIdAndMetabaseId.ts index 78aefed97f..7763dd533e 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/getTeamAndMetabaseId.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/getTeamIdAndMetabaseId.ts @@ -9,7 +9,7 @@ interface GetMetabaseId { }[]; } -export const getTeamAndMetabaseId = async (name: string) => { +export const getTeamIdAndMetabaseId = async (name: string) => { try { const response = await $api.client.request( gql` diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index 192a6dad8f..2ed2a3bd28 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -1,7 +1,7 @@ import { createMetabaseClient } from "../shared/client.js"; import { updateMetabaseId } from "./updateMetabaseId.js"; import type { NewCollectionParams } from "./types.js"; -import { getTeamAndMetabaseId } from "./getTeamAndMetabaseId.js"; +import { getTeamIdAndMetabaseId } from "./getTeamIdAndMetabaseId.js"; import { createCollection } from "./createCollection.js"; const client = createMetabaseClient(); @@ -16,7 +16,7 @@ export async function checkCollections( params: NewCollectionParams, ): Promise { try { - const teamAndMetabaseId = await getTeamAndMetabaseId(params.name); + const teamAndMetabaseId = await getTeamIdAndMetabaseId(params.name); const { metabaseId, id } = teamAndMetabaseId; if (metabaseId) { From ecca6d658070a72ccb33d797cd5da29c1e43fccc Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Wed, 11 Dec 2024 16:45:26 +0000 Subject: [PATCH 44/46] chore: tidy destructuring --- .../modules/analytics/metabase/collection/service.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index 2ed2a3bd28..51cfb1216d 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -16,8 +16,9 @@ export async function checkCollections( params: NewCollectionParams, ): Promise { try { - const teamAndMetabaseId = await getTeamIdAndMetabaseId(params.name); - const { metabaseId, id } = teamAndMetabaseId; + const { metabaseId, id: teamId } = await getTeamIdAndMetabaseId( + params.name, + ); if (metabaseId) { return metabaseId; @@ -25,7 +26,7 @@ export async function checkCollections( // Create new Metabase collection if !metabaseId const newMetabaseId = await createCollection(params); - await updateMetabaseId(id, newMetabaseId); + await updateMetabaseId(teamId, newMetabaseId); console.log({ newMetabaseId }); return newMetabaseId; } catch (error) { From 7203db013333543966cae30757cdcdc8bf472533 Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Wed, 11 Dec 2024 16:47:42 +0000 Subject: [PATCH 45/46] chore: rename to createCollectionIfDoesNotExist for clarity --- .../metabase/collection/collection.test.ts | 8 ++--- .../metabase/collection/controller.ts | 32 +++++++++---------- .../analytics/metabase/collection/service.ts | 4 +-- .../analytics/metabase/collection/types.ts | 4 +-- api.planx.uk/modules/analytics/routes.ts | 8 ++--- 5 files changed, 28 insertions(+), 28 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts index 4498326f78..a4c064ab62 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts @@ -1,4 +1,4 @@ -import { checkCollections } from "./service.js"; +import { createCollectionIfDoesNotExist } from "./service.js"; import { getCollection } from "./getCollection.js"; import nock from "nock"; import { MetabaseError } from "../shared/client.js"; @@ -7,7 +7,7 @@ import { updateMetabaseId } from "./updateMetabaseId.js"; import { getTeamIdAndMetabaseId } from "./getTeamIdAndMetabaseId.js"; import { createCollection } from "./createCollection.js"; -describe("checkCollections", () => { +describe("createCollectionIfDoesNotExist", () => { beforeEach(() => { nock.cleanAll(); }); @@ -247,7 +247,7 @@ describe("edge cases", () => { test("handles missing name", async () => { await expect( - checkCollections({ + createCollectionIfDoesNotExist({ name: "", }), ).rejects.toThrow(); @@ -288,7 +288,7 @@ describe("edge cases", () => { }); await expect( - checkCollections({ + createCollectionIfDoesNotExist({ name: longName, }), ).rejects.toThrow(); diff --git a/api.planx.uk/modules/analytics/metabase/collection/controller.ts b/api.planx.uk/modules/analytics/metabase/collection/controller.ts index 38af98e500..c621cb15fb 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/controller.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/controller.ts @@ -1,18 +1,18 @@ -import { checkCollections } from "./service.js"; +import { createCollectionIfDoesNotExist } from "./service.js"; import type { NewCollectionRequestHandler } from "./types.js"; -export const checkCollectionsController: NewCollectionRequestHandler = async ( - _req, - res, -) => { - try { - const params = res.locals.parsedReq.body; - const collection = await checkCollections(params); - res.status(201).json({ data: collection }); - } catch (error) { - res.status(400).json({ - error: - error instanceof Error ? error.message : "An unexpected error occurred", - }); - } -}; +export const createCollectionIfDoesNotExistController: NewCollectionRequestHandler = + async (_req, res) => { + try { + const params = res.locals.parsedReq.body; + const collection = await createCollectionIfDoesNotExist(params); + res.status(201).json({ data: collection }); + } catch (error) { + res.status(400).json({ + error: + error instanceof Error + ? error.message + : "An unexpected error occurred", + }); + } + }; diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index 51cfb1216d..e4d7e59ccd 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -12,7 +12,7 @@ const client = createMetabaseClient(); * @params `name` is required, but `description` and `parent_id` are optional. * @returns `response.data`, so use dot notation to access `id` or `parent_id`. */ -export async function checkCollections( +export async function createCollectionIfDoesNotExist( params: NewCollectionParams, ): Promise { try { @@ -30,7 +30,7 @@ export async function checkCollections( console.log({ newMetabaseId }); return newMetabaseId; } catch (error) { - console.error("Error in checkCollections:", error); + console.error("Error in createCollectionIfDoesNotExist:", error); throw error; } } diff --git a/api.planx.uk/modules/analytics/metabase/collection/types.ts b/api.planx.uk/modules/analytics/metabase/collection/types.ts index cade18fc8d..966449cc31 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/types.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts @@ -24,7 +24,7 @@ export interface MetabaseCollectionParams { /** Metbase collection ID for the the "Council" collection **/ // const COUNCILS_COLLECTION_ID = 58; -export const checkCollectionsSchema = z.object({ +export const createCollectionIfDoesNotExistSchema = z.object({ body: z .object({ name: z.string(), @@ -39,7 +39,7 @@ export const checkCollectionsSchema = z.object({ }); export type NewCollectionRequestHandler = ValidatedRequestHandler< - typeof checkCollectionsSchema, + typeof createCollectionIfDoesNotExistSchema, ApiResponse >; diff --git a/api.planx.uk/modules/analytics/routes.ts b/api.planx.uk/modules/analytics/routes.ts index 2065837e27..f88633568d 100644 --- a/api.planx.uk/modules/analytics/routes.ts +++ b/api.planx.uk/modules/analytics/routes.ts @@ -5,8 +5,8 @@ import { logUserExitController, logUserResumeController, } from "./analyticsLog/controller.js"; -import { checkCollectionsController } from "./metabase/collection/controller.js"; -import { checkCollectionsSchema } from "./metabase/collection/types.js"; +import { createCollectionIfDoesNotExistController } from "./metabase/collection/controller.js"; +import { createCollectionIfDoesNotExistSchema } from "./metabase/collection/types.js"; const router = Router(); @@ -22,8 +22,8 @@ router.post( ); router.post( "/collection", - validate(checkCollectionsSchema), - checkCollectionsController, + validate(createCollectionIfDoesNotExistSchema), + createCollectionIfDoesNotExistController, ); export default router; From 5248d512711ade5f026b0ae56cea9437c7bfc35a Mon Sep 17 00:00:00 2001 From: zz-hh-aa Date: Wed, 11 Dec 2024 16:50:03 +0000 Subject: [PATCH 46/46] chore: rename to MetabaseCollectionsController for clarity --- .../modules/analytics/metabase/collection/controller.ts | 2 +- api.planx.uk/modules/analytics/routes.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api.planx.uk/modules/analytics/metabase/collection/controller.ts b/api.planx.uk/modules/analytics/metabase/collection/controller.ts index c621cb15fb..946a35f0e8 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/controller.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/controller.ts @@ -1,7 +1,7 @@ import { createCollectionIfDoesNotExist } from "./service.js"; import type { NewCollectionRequestHandler } from "./types.js"; -export const createCollectionIfDoesNotExistController: NewCollectionRequestHandler = +export const MetabaseCollectionsController: NewCollectionRequestHandler = async (_req, res) => { try { const params = res.locals.parsedReq.body; diff --git a/api.planx.uk/modules/analytics/routes.ts b/api.planx.uk/modules/analytics/routes.ts index f88633568d..3044e33890 100644 --- a/api.planx.uk/modules/analytics/routes.ts +++ b/api.planx.uk/modules/analytics/routes.ts @@ -5,7 +5,7 @@ import { logUserExitController, logUserResumeController, } from "./analyticsLog/controller.js"; -import { createCollectionIfDoesNotExistController } from "./metabase/collection/controller.js"; +import { MetabaseCollectionsController } from "./metabase/collection/controller.js"; import { createCollectionIfDoesNotExistSchema } from "./metabase/collection/types.js"; const router = Router(); @@ -23,7 +23,7 @@ router.post( router.post( "/collection", validate(createCollectionIfDoesNotExistSchema), - createCollectionIfDoesNotExistController, + MetabaseCollectionsController, ); export default router;