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..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 +1,296 @@ -test.todo("should test collection check and creation"); +import { createCollectionIfDoesNotExist } from "./service.js"; +import { getCollection } from "./getCollection.js"; +import nock from "nock"; +import { MetabaseError } from "../shared/client.js"; +import { $api } from "../../../../client/index.js"; +import { updateMetabaseId } from "./updateMetabaseId.js"; +import { getTeamIdAndMetabaseId } from "./getTeamIdAndMetabaseId.js"; +import { createCollection } from "./createCollection.js"; + +describe("createCollectionIfDoesNotExist", () => { + beforeEach(() => { + nock.cleanAll(); + }); + + test("returns an existing Metabase collection ID if collection exists", async () => { + // Mock getTeamIdAndMetabaseId response + vi.spyOn($api.client, "request").mockResolvedValueOnce({ + teams: [ + { + id: 26, + name: "Barnet", + metabaseId: 20, + }, + ], + }); + + const collectionId = await getTeamIdAndMetabaseId("Barnet"); + + expect(collectionId.metabaseId).toBe(20); + }); + + test("creates new collection when metabase ID doesn't exist", async () => { + // Mock getTeamIdAndMetabaseId 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!) + .post("/api/collection/", { + name: "Barnet", + }) + .reply(200, { + id: 123, + name: "Barnet", + }); + + const collectionId = await createCollection({ + name: "Barnet", + }); + + 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!); + + // Mock collection creation endpoint + metabaseMock + .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 + metabaseMock.get("/api/collection/123").reply(200, { + id: 123, + name: testName, + parent_id: 100, + }); + + const collectionId = await createCollection({ + 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); + expect(metabaseMock.isDone()).toBe(true); + }); + + test("returns collection correctly no matter collection name case", async () => { + vi.spyOn($api.client, "request").mockResolvedValueOnce({ + teams: [ + { + id: 26, + name: "barnet", + metabaseId: 20, + }, + ], + }); + + const collection = await getTeamIdAndMetabaseId("BARNET"); + expect(collection.metabaseId).toBe(20); + }); + + test("throws error if network failure", async () => { + nock(process.env.METABASE_URL_EXT!) + .get("/api/collection/") + .replyWithError("Network error occurred"); + + await expect( + createCollection({ + 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( + createCollection({ + name: "Test Collection", + }), + ).rejects.toThrow(MetabaseError); + }); +}); + +describe("getTeamIdAndMetabaseId", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + test("successfully gets team and existing metabase id", async () => { + vi.spyOn($api.client, "request").mockResolvedValue({ + teams: [ + { + id: 26, + name: "Barnet", + metabaseId: 20, + }, + ], + }); + + const teamAndMetabaseId = await getTeamIdAndMetabaseId("Barnet"); + + expect(teamAndMetabaseId.id).toEqual(26); + expect(teamAndMetabaseId.metabaseId).toEqual(20); + }); + + test("handles team with null metabase id", async () => { + vi.spyOn($api.client, "request").mockResolvedValue({ + teams: [ + { + id: 26, + name: "Barnet", + metabaseId: null, + }, + ], + }); + + const teamAndMetabaseId = await getTeamIdAndMetabaseId("Barnet"); + + expect(teamAndMetabaseId.id).toEqual(26); + expect(teamAndMetabaseId.metabaseId).toBeNull(); + }); +}); + +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(); + vi.clearAllMocks(); + vi.resetAllMocks(); + }); + + test("handles missing name", async () => { + await expect( + createCollectionIfDoesNotExist({ + name: "", + }), + ).rejects.toThrow(); + }); + + test("handles names with special characters", async () => { + // TODO + 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 createCollection({ + 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( + 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 e69de29bb2..946a35f0e8 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/controller.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/controller.ts @@ -0,0 +1,18 @@ +import { createCollectionIfDoesNotExist } from "./service.js"; +import type { NewCollectionRequestHandler } from "./types.js"; + +export const MetabaseCollectionsController: 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/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/getCollection.ts b/api.planx.uk/modules/analytics/metabase/collection/getCollection.ts new file mode 100644 index 0000000000..0187f2d8ed --- /dev/null +++ b/api.planx.uk/modules/analytics/metabase/collection/getCollection.ts @@ -0,0 +1,15 @@ +import { createMetabaseClient } from "../shared/client.js"; +import type { GetCollectionResponse } from "./types.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/getTeamIdAndMetabaseId.ts b/api.planx.uk/modules/analytics/metabase/collection/getTeamIdAndMetabaseId.ts new file mode 100644 index 0000000000..7763dd533e --- /dev/null +++ b/api.planx.uk/modules/analytics/metabase/collection/getTeamIdAndMetabaseId.ts @@ -0,0 +1,38 @@ +import { gql } from "graphql-request"; +import { $api } from "../../../../client/index.js"; + +interface GetMetabaseId { + teams: { + id: number; + name: string; + metabaseId: number | null; + }[]; +} + +export const getTeamIdAndMetabaseId = async (name: string) => { + try { + const response = await $api.client.request( + gql` + query GetTeamAndMetabaseId($name: String!) { + teams(where: { name: { _ilike: $name } }) { + id + name + metabaseId: metabase_id + } + } + `, + { + name: name, + }, + ); + + const result = response.teams[0]; + return result; + } catch (e) { + console.error( + "Error fetching team's ID / Metabase ID from PlanX DB:", + (e as Error).stack, + ); + throw e; + } +}; diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts index e69de29bb2..e4d7e59ccd 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/service.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts @@ -0,0 +1,36 @@ +import { createMetabaseClient } from "../shared/client.js"; +import { updateMetabaseId } from "./updateMetabaseId.js"; +import type { NewCollectionParams } from "./types.js"; +import { getTeamIdAndMetabaseId } from "./getTeamIdAndMetabaseId.js"; +import { createCollection } from "./createCollection.js"; + +const client = createMetabaseClient(); + +/** + * 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 createCollectionIfDoesNotExist( + params: NewCollectionParams, +): Promise { + try { + const { metabaseId, id: teamId } = await getTeamIdAndMetabaseId( + params.name, + ); + + if (metabaseId) { + return metabaseId; + } + + // Create new Metabase collection if !metabaseId + const newMetabaseId = await createCollection(params); + await updateMetabaseId(teamId, newMetabaseId); + console.log({ newMetabaseId }); + return newMetabaseId; + } catch (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 e69de29bb2..966449cc31 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/types.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts @@ -0,0 +1,55 @@ +import { z } from "zod"; +import type { ValidatedRequestHandler } from "../../../../shared/middleware/validate.js"; + +type ApiResponse = { + data?: T; + 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 */ + 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 COUNCILS_COLLECTION_ID = 58; + +export const createCollectionIfDoesNotExistSchema = z.object({ + body: z + .object({ + name: z.string(), + description: z.string().optional(), + parentId: z.number().optional(), //.default(COUNCILS_COLLECTION_ID), + }) + .transform((data) => ({ + name: data.name, + description: data.description, + parent_id: data.parentId, + })), +}); + +export type NewCollectionRequestHandler = ValidatedRequestHandler< + typeof createCollectionIfDoesNotExistSchema, + ApiResponse +>; + +export interface NewCollectionResponse { + id: number; + name: string; +} + +export interface GetCollectionResponse { + id: number; + name: string; + parent_id: number; +} diff --git a/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts b/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts new file mode 100644 index 0000000000..29de69e692 --- /dev/null +++ b/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts @@ -0,0 +1,43 @@ +import { gql } from "graphql-request"; +import { $api } from "../../../../client/index.js"; + +interface UpdateMetabaseId { + teams: { + id: number; + name: string; + metabaseId: number; + }; +} + +/** 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( + 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, + }, + ); + return response; + } catch (e) { + console.error( + "There's been an error while updating the Metabase ID for this team", + (e as Error).stack, + ); + throw e; + } +}; 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, diff --git a/api.planx.uk/modules/analytics/metabase/shared/types.ts b/api.planx.uk/modules/analytics/metabase/shared/types.ts index e69de29bb2..e1f491a55f 100644 --- a/api.planx.uk/modules/analytics/metabase/shared/types.ts +++ b/api.planx.uk/modules/analytics/metabase/shared/types.ts @@ -0,0 +1,40 @@ +export interface Input { + teamName: 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 { teamName, originalDashboardId, filter, filterValue } = input as Input; + + if (typeof teamName !== "string" || teamName.trim() === "") { + console.error("Invalid teamName: 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; +} diff --git a/api.planx.uk/modules/analytics/routes.ts b/api.planx.uk/modules/analytics/routes.ts index 9dcaa69010..3044e33890 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 { MetabaseCollectionsController } from "./metabase/collection/controller.js"; +import { createCollectionIfDoesNotExistSchema } from "./metabase/collection/types.js"; const router = Router(); @@ -18,5 +20,10 @@ router.post( validate(logAnalyticsSchema), logUserResumeController, ); +router.post( + "/collection", + validate(createCollectionIfDoesNotExistSchema), + MetabaseCollectionsController, +); export default router; 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: 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;