From 71fb982eaafa9c3a9cfdd13886d115985c269b9f Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Wed, 11 Dec 2024 17:23:46 +0000
Subject: [PATCH 01/22] feat: new createCollection metabase api call

---
 .../metabase/collection/createCollection.ts   | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 api.planx.uk/modules/analytics/metabase/collection/createCollection.ts

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<any> {
+  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;
+}

From 530f60eff24d39e877221a0ec4468833e92ec744 Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Wed, 11 Dec 2024 17:24:18 +0000
Subject: [PATCH 02/22] feat: new get collection from metabase function

---
 .../metabase/collection/getCollection.ts          | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 api.planx.uk/modules/analytics/metabase/collection/getCollection.ts

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<GetCollectionResponse> {
+  const response = await client.get(`/api/collection/${id}`);
+  return response.data;
+}

From 50f0c1821ffa069612fb624c808ec6998e5036a2 Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Wed, 11 Dec 2024 17:24:55 +0000
Subject: [PATCH 03/22] feat: createCollectionIfDoesNotExist function

---
 .../analytics/metabase/collection/service.ts  | 36 +++++++++++++++++++
 1 file changed, 36 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..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<number> {
+  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;
+  }
+}

From 6c34c7333fbcf5c1df3ab993e8118f66857ab7f6 Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Wed, 11 Dec 2024 17:25:39 +0000
Subject: [PATCH 04/22] feat: new types for collection service

---
 .../analytics/metabase/collection/types.ts    | 55 +++++++++++++++++++
 1 file changed, 55 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..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<T> = {
+  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<NewCollectionResponse>
+>;
+
+export interface NewCollectionResponse {
+  id: number;
+  name: string;
+}
+
+export interface GetCollectionResponse {
+  id: number;
+  name: string;
+  parent_id: number;
+}

From 28b3de66169d3025af0c007c2121b38ee64212f2 Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Wed, 11 Dec 2024 17:26:02 +0000
Subject: [PATCH 05/22] fix: import in metabase client

---
 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 07a969cbc0706ca33cd82fdf96fbb67a06b0f13e Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Wed, 11 Dec 2024 17:27:29 +0000
Subject: [PATCH 06/22] feat: new collection route in api

---
 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..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;

From 521689bcefe65aa470e863a124b28cc0d045ead5 Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Wed, 11 Dec 2024 17:36:22 +0000
Subject: [PATCH 07/22] fix: controller type error

---
 .../metabase/collection/controller.ts          | 18 ++++++++++++++++++
 .../analytics/metabase/collection/types.ts     |  2 +-
 2 files changed, 19 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 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/types.ts b/api.planx.uk/modules/analytics/metabase/collection/types.ts
index 966449cc31..4b4e53169c 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/types.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts
@@ -40,7 +40,7 @@ export const createCollectionIfDoesNotExistSchema = z.object({
 
 export type NewCollectionRequestHandler = ValidatedRequestHandler<
   typeof createCollectionIfDoesNotExistSchema,
-  ApiResponse<NewCollectionResponse>
+  ApiResponse<number>
 >;
 
 export interface NewCollectionResponse {

From b0978612c22d1d1e58b8e2f84fd164e7e0fb46fd Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Wed, 11 Dec 2024 17:38:52 +0000
Subject: [PATCH 08/22] test: test for full metabase collection service

---
 .../metabase/collection/collection.test.ts    | 186 ++++++++++++++++++
 1 file changed, 186 insertions(+)

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 4b9829dd69..e2bfdb625d 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts
@@ -1,6 +1,136 @@
+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("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(() => {
@@ -90,3 +220,59 @@ describe("updateMetabaseId", () => {
     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 () => {
+    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();
+  });
+});

From 30cfbb46116c5f7ddf03edcd8ab1ec1f41a3f2ca Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Mon, 16 Dec 2024 09:41:46 +0000
Subject: [PATCH 09/22] chore: remove unnecessary client import

---
 api.planx.uk/modules/analytics/metabase/collection/service.ts | 3 ---
 1 file changed, 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 e4d7e59ccd..d3445c2064 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/service.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts
@@ -1,11 +1,8 @@
-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.

From 6f251b17b2aad0c856f9e9be11035bfdd3b2595e Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Mon, 16 Dec 2024 15:09:50 +0000
Subject: [PATCH 10/22] test: remove unused axios spy

---
 api.planx.uk/modules/analytics/metabase/shared/client.test.ts | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/api.planx.uk/modules/analytics/metabase/shared/client.test.ts b/api.planx.uk/modules/analytics/metabase/shared/client.test.ts
index 564909ef6a..ab7da6fdba 100644
--- a/api.planx.uk/modules/analytics/metabase/shared/client.test.ts
+++ b/api.planx.uk/modules/analytics/metabase/shared/client.test.ts
@@ -1,4 +1,3 @@
-import axios from "axios";
 import {
   validateConfig,
   createMetabaseClient,
@@ -6,8 +5,6 @@ import {
 } from "./client.js";
 import nock from "nock";
 
-const axiosCreateSpy = vi.spyOn(axios, "create");
-
 describe("Metabase client", () => {
   beforeEach(() => {
     vi.clearAllMocks();

From 398d9b84d67624c0b21d16a94ba06f8f88e84178 Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Thu, 19 Dec 2024 09:28:05 +0000
Subject: [PATCH 11/22] chore: change functions to camelCase

---
 .../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 946a35f0e8..3f2961d98a 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 MetabaseCollectionsController: 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 3044e33890..d7b08a678e 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 { MetabaseCollectionsController } 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),
-  MetabaseCollectionsController,
+  metabaseCollectionsController,
 );
 
 export default router;

From 08d38f23e4f6ca1120bb972d7cdff39d7e37dbd8 Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Thu, 19 Dec 2024 12:19:37 +0000
Subject: [PATCH 12/22] chore: tidy types

---
 .../metabase/collection/createCollection.ts      |  2 +-
 .../analytics/metabase/collection/types.ts       | 16 +++++-----------
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts b/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts
index e2f0df0539..2dcb11b7ca 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts
@@ -5,7 +5,7 @@ const client = createMetabaseClient();
 
 export async function createCollection(
   params: NewCollectionParams,
-): Promise<any> {
+): Promise<number> {
   const transformedParams = {
     name: params.name,
     parent_id: params.parentId,
diff --git a/api.planx.uk/modules/analytics/metabase/collection/types.ts b/api.planx.uk/modules/analytics/metabase/collection/types.ts
index 4b4e53169c..8d241a373c 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/types.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts
@@ -25,17 +25,11 @@ export interface MetabaseCollectionParams {
 // 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,
-    })),
+  body: z.object({
+    name: z.string(),
+    description: z.string().optional(),
+    parentId: z.number().optional(), //.default(COUNCILS_COLLECTION_ID),
+  }),
 });
 
 export type NewCollectionRequestHandler = ValidatedRequestHandler<

From 632834e7bca7fbf3c4ed50947ba7ec5102de2da2 Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Thu, 19 Dec 2024 12:42:40 +0000
Subject: [PATCH 13/22] feat: update function name for clarity

---
 .../analytics/metabase/collection/collection.test.ts      | 8 ++++----
 .../modules/analytics/metabase/collection/controller.ts   | 4 ++--
 .../modules/analytics/metabase/collection/service.ts      | 4 ++--
 .../modules/analytics/metabase/collection/types.ts        | 4 ++--
 api.planx.uk/modules/analytics/routes.ts                  | 4 ++--
 5 files changed, 12 insertions(+), 12 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 e2bfdb625d..0580dbf0c4 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 { createCollectionIfDoesNotExist } from "./service.js";
+import { createTeamCollection } 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("createCollectionIfDoesNotExist", () => {
+describe("createTeamCollection", () => {
   beforeEach(() => {
     nock.cleanAll();
   });
@@ -230,7 +230,7 @@ describe("edge cases", () => {
 
   test("handles missing name", async () => {
     await expect(
-      createCollectionIfDoesNotExist({
+      createTeamCollection({
         name: "",
       }),
     ).rejects.toThrow();
@@ -270,7 +270,7 @@ describe("edge cases", () => {
       });
 
     await expect(
-      createCollectionIfDoesNotExist({
+      createTeamCollection({
         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 3f2961d98a..d66bd2ef70 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/controller.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/controller.ts
@@ -1,11 +1,11 @@
-import { createCollectionIfDoesNotExist } from "./service.js";
+import { createTeamCollection } 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);
+      const collection = await createTeamCollection(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 d3445c2064..debc999f09 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/service.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts
@@ -9,7 +9,7 @@ import { createCollection } from "./createCollection.js";
  * @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(
+export async function createTeamCollection(
   params: NewCollectionParams,
 ): Promise<number> {
   try {
@@ -27,7 +27,7 @@ export async function createCollectionIfDoesNotExist(
     console.log({ newMetabaseId });
     return newMetabaseId;
   } catch (error) {
-    console.error("Error in createCollectionIfDoesNotExist:", error);
+    console.error("Error in createTeamCollection:", 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 8d241a373c..030b434149 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 createCollectionIfDoesNotExistSchema = z.object({
+export const createTeamCollectionSchema = z.object({
   body: z.object({
     name: z.string(),
     description: z.string().optional(),
@@ -33,7 +33,7 @@ export const createCollectionIfDoesNotExistSchema = z.object({
 });
 
 export type NewCollectionRequestHandler = ValidatedRequestHandler<
-  typeof createCollectionIfDoesNotExistSchema,
+  typeof createTeamCollectionSchema,
   ApiResponse<number>
 >;
 
diff --git a/api.planx.uk/modules/analytics/routes.ts b/api.planx.uk/modules/analytics/routes.ts
index d7b08a678e..0b34bce347 100644
--- a/api.planx.uk/modules/analytics/routes.ts
+++ b/api.planx.uk/modules/analytics/routes.ts
@@ -6,7 +6,7 @@ import {
   logUserResumeController,
 } from "./analyticsLog/controller.js";
 import { metabaseCollectionsController } from "./metabase/collection/controller.js";
-import { createCollectionIfDoesNotExistSchema } from "./metabase/collection/types.js";
+import { createTeamCollectionSchema } from "./metabase/collection/types.js";
 
 const router = Router();
 
@@ -22,7 +22,7 @@ router.post(
 );
 router.post(
   "/collection",
-  validate(createCollectionIfDoesNotExistSchema),
+  validate(createTeamCollectionSchema),
   metabaseCollectionsController,
 );
 

From 15ad49c8434f41dda499fd0036acb4a64e695f39 Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Thu, 19 Dec 2024 15:17:53 +0000
Subject: [PATCH 14/22] fix: update name to slug

---
 .../metabase/collection/collection.test.ts    | 52 +++++++++----------
 .../metabase/collection/createCollection.ts   |  6 +--
 .../analytics/metabase/collection/service.ts  | 11 ++--
 .../analytics/metabase/collection/types.ts    |  8 +--
 4 files changed, 38 insertions(+), 39 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 0580dbf0c4..baeb6c6107 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts
@@ -18,7 +18,7 @@ describe("createTeamCollection", () => {
       teams: [
         {
           id: 26,
-          name: "Barnet",
+          slug: "barnet",
           metabase_id: null,
         },
       ],
@@ -27,15 +27,15 @@ describe("createTeamCollection", () => {
     // Mock Metabase API calls
     const metabaseMock = nock(process.env.METABASE_URL_EXT!)
       .post("/api/collection/", {
-        name: "Barnet",
+        slug: "barnet",
       })
       .reply(200, {
         id: 123,
-        name: "Barnet",
+        slug: "barnet",
       });
 
     const collectionId = await createCollection({
-      name: "Barnet",
+      slug: "barnet",
     });
 
     expect(collectionId).toBe(123);
@@ -49,37 +49,37 @@ describe("createTeamCollection", () => {
         returning: [
           {
             id: 26,
-            name: "Barnet",
+            slug: "barnet",
             metabase_id: 123,
           },
         ],
       },
     });
 
-    const testName = "Example council";
+    const testSlug = "example-council";
     const metabaseMock = nock(process.env.METABASE_URL_EXT!);
 
     // Mock collection creation endpoint
     metabaseMock
       .post("/api/collection/", {
-        name: testName,
+        slug: testSlug,
         parent_id: 100,
       })
       .reply(200, {
         id: 123,
-        name: testName,
+        slug: testSlug,
         parent_id: 100,
       });
 
     // Mock GET request for verifying the new collection
     metabaseMock.get("/api/collection/123").reply(200, {
       id: 123,
-      name: testName,
+      slug: testSlug,
       parent_id: 100,
     });
 
     const collectionId = await createCollection({
-      name: testName,
+      slug: testSlug,
       parentId: 100,
     });
 
@@ -92,12 +92,12 @@ describe("createTeamCollection", () => {
     expect(metabaseMock.isDone()).toBe(true);
   });
 
-  test("returns collection correctly no matter collection name case", async () => {
+  test("returns collection correctly no matter collection slug case", async () => {
     vi.spyOn($api.client, "request").mockResolvedValueOnce({
       teams: [
         {
           id: 26,
-          name: "barnet",
+          slug: "barnet",
           metabaseId: 20,
         },
       ],
@@ -114,7 +114,7 @@ describe("createTeamCollection", () => {
 
     await expect(
       createCollection({
-        name: "Test Collection",
+        slug: "test-collection",
       }),
     ).rejects.toThrow("Network error occurred");
   });
@@ -126,7 +126,7 @@ describe("createTeamCollection", () => {
 
     await expect(
       createCollection({
-        name: "Test Collection",
+        slug: "test-collection",
       }),
     ).rejects.toThrow(MetabaseError);
   });
@@ -228,50 +228,50 @@ describe("edge cases", () => {
     vi.resetAllMocks();
   });
 
-  test("handles missing name", async () => {
+  test("handles missing slug", async () => {
     await expect(
       createTeamCollection({
-        name: "",
+        slug: "",
       }),
     ).rejects.toThrow();
   });
 
-  test("handles names with special characters", async () => {
-    const specialName = "@#$%^&*";
+  test("handles slug with special characters", async () => {
+    const specialSlug = "@#$%^&*";
 
     nock(process.env.METABASE_URL_EXT!).get("/api/collection/").reply(200, []);
 
     nock(process.env.METABASE_URL_EXT!)
       .post("/api/collection/", {
-        name: specialName,
+        slug: specialSlug,
       })
       .reply(200, {
         id: 789,
-        name: specialName,
+        slug: specialSlug,
       });
 
     const collection = await createCollection({
-      name: specialName,
+      slug: specialSlug,
     });
     expect(collection).toBe(789);
   });
 
-  test("handles very long names", async () => {
-    const longName = "A".repeat(101);
+  test("handles very long slugs", async () => {
+    const longSlug = "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,
+        slug: longSlug,
       })
       .reply(400, {
-        message: "Name too long",
+        message: "Slug too long",
       });
 
     await expect(
       createTeamCollection({
-        name: longName,
+        slug: longSlug,
       }),
     ).rejects.toThrow();
   });
diff --git a/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts b/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts
index 2dcb11b7ca..79fbe9d7be 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts
@@ -7,14 +7,14 @@ export async function createCollection(
   params: NewCollectionParams,
 ): Promise<number> {
   const transformedParams = {
-    name: params.name,
+    slug: params.slug,
     parent_id: params.parentId,
   };
 
   const response = await client.post(`/api/collection/`, transformedParams);
-
+  const slug = response.data.slug;
   console.log(
-    `New collection: ${response.data.name}, new collection ID: ${response.data.id}`,
+    `New collection: ${response.data.slug}, 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 debc999f09..84718c4833 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/service.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/service.ts
@@ -4,9 +4,10 @@ import { getTeamIdAndMetabaseId } from "./getTeamIdAndMetabaseId.js";
 import { createCollection } from "./createCollection.js";
 
 /**
- * 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.
+ * The `getTeamIdAndMetabaseId()` function is run here to first get teams.id and .metabase_id from PlanX db, if present,
+ * so that the service can figure out if it needs to run `createCollection()` or not.
+ * Instead of running `getTeamIdAndMetabaseId()` first in the controller, it is encapsulated in `createTeamCollection` to keep business logic in `service.ts` instead of `controller.ts`.
+ * @params `slug` 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 createTeamCollection(
@@ -14,17 +15,15 @@ export async function createTeamCollection(
 ): Promise<number> {
   try {
     const { metabaseId, id: teamId } = await getTeamIdAndMetabaseId(
-      params.name,
+      params.slug,
     );
 
     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 createTeamCollection:", error);
diff --git a/api.planx.uk/modules/analytics/metabase/collection/types.ts b/api.planx.uk/modules/analytics/metabase/collection/types.ts
index 030b434149..d7eb1a2070 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/types.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts
@@ -8,7 +8,7 @@ type ApiResponse<T> = {
 
 /** Interface for incoming request, in camelCase */
 export interface NewCollectionParams {
-  name: string;
+  slug: string;
   description?: string;
   /** Optional; if the collection is a child of a parent, specify parent ID here */
   parentId?: number;
@@ -26,7 +26,7 @@ export interface MetabaseCollectionParams {
 
 export const createTeamCollectionSchema = z.object({
   body: z.object({
-    name: z.string(),
+    slug: z.string(),
     description: z.string().optional(),
     parentId: z.number().optional(), //.default(COUNCILS_COLLECTION_ID),
   }),
@@ -39,11 +39,11 @@ export type NewCollectionRequestHandler = ValidatedRequestHandler<
 
 export interface NewCollectionResponse {
   id: number;
-  name: string;
+  slug: string;
 }
 
 export interface GetCollectionResponse {
   id: number;
-  name: string;
+  slug: string;
   parent_id: number;
 }

From 8f12f205708fb03b802f8f9330fce54319a21f93 Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Thu, 19 Dec 2024 15:18:55 +0000
Subject: [PATCH 15/22] chore: replace metabase client w singleton instance

---
 .../metabase/collection/getCollection.ts      |  4 ++--
 .../analytics/metabase/shared/client.test.ts  | 21 +++++++++----------
 .../analytics/metabase/shared/client.ts       |  8 +------
 3 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/api.planx.uk/modules/analytics/metabase/collection/getCollection.ts b/api.planx.uk/modules/analytics/metabase/collection/getCollection.ts
index 0187f2d8ed..f1566f37f1 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/getCollection.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/getCollection.ts
@@ -1,7 +1,7 @@
 import { createMetabaseClient } from "../shared/client.js";
 import type { GetCollectionResponse } from "./types.js";
+import { $metabase } 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
@@ -10,6 +10,6 @@ const client = createMetabaseClient();
 export async function getCollection(
   id: number,
 ): Promise<GetCollectionResponse> {
-  const response = await client.get(`/api/collection/${id}`);
+  const response = await $metabase.get(`/api/collection/${id}`);
   return response.data;
 }
diff --git a/api.planx.uk/modules/analytics/metabase/shared/client.test.ts b/api.planx.uk/modules/analytics/metabase/shared/client.test.ts
index ab7da6fdba..ec570fa3aa 100644
--- a/api.planx.uk/modules/analytics/metabase/shared/client.test.ts
+++ b/api.planx.uk/modules/analytics/metabase/shared/client.test.ts
@@ -4,6 +4,7 @@ import {
   MetabaseError,
 } from "./client.js";
 import nock from "nock";
+import { $metabase } from "./client.js";
 
 describe("Metabase client", () => {
   beforeEach(() => {
@@ -16,13 +17,12 @@ describe("Metabase client", () => {
   });
 
   test("returns configured client", async () => {
-    const client = createMetabaseClient();
-    expect(client.defaults.baseURL).toBe(process.env.METABASE_URL_EXT);
-    expect(client.defaults.headers["X-API-Key"]).toBe(
+    expect($metabase.defaults.baseURL).toBe(process.env.METABASE_URL_EXT);
+    expect($metabase.defaults.headers["X-API-Key"]).toBe(
       process.env.METABASE_API_KEY,
     );
-    expect(client.defaults.headers["Content-Type"]).toBe("application/json");
-    expect(client.defaults.timeout).toBe(30_000);
+    expect($metabase.defaults.headers["Content-Type"]).toBe("application/json");
+    expect($metabase.defaults.timeout).toBe(30_000);
   });
 
   describe("validates configuration", () => {
@@ -61,8 +61,7 @@ describe("Metabase client", () => {
         .get("/test")
         .reply(200, { data: "success" });
 
-      const client = createMetabaseClient();
-      const response = await client.get("/test");
+      const response = await $metabase.get("/test");
 
       expect(response.data).toEqual({ data: "success" });
       expect(metabaseScope.isDone()).toBe(true);
@@ -76,10 +75,10 @@ describe("Metabase client", () => {
         .times(4)
         .reply(500, { message: "Internal Server Error" });
 
-      const client = createMetabaseClient();
+      const $metabase = createMetabaseClient();
 
       try {
-        await client.get("/test");
+        await $metabase.get("/test");
         expect.fail("Should have thrown an error");
       } catch (error) {
         expect(error).toBeInstanceOf(MetabaseError);
@@ -93,8 +92,8 @@ describe("Metabase client", () => {
 
       metabaseScope.get("/test").once().reply(200, { data: "success" });
 
-      const client = createMetabaseClient();
-      const response = await client.get("/test");
+      const $metabase = createMetabaseClient();
+      const response = await $metabase.get("/test");
 
       expect(response.data).toEqual({ data: "success" });
 
diff --git a/api.planx.uk/modules/analytics/metabase/shared/client.ts b/api.planx.uk/modules/analytics/metabase/shared/client.ts
index 9a52783d52..b7f0b66d03 100644
--- a/api.planx.uk/modules/analytics/metabase/shared/client.ts
+++ b/api.planx.uk/modules/analytics/metabase/shared/client.ts
@@ -121,10 +121,4 @@ export const createMetabaseClient = (): AxiosInstance => {
   return client;
 };
 
-// // Export both client and instance with delayed instantiation for test purposes
-// export let metabaseClient: AxiosInstance;
-
-// export const initializeMetabaseClient = () => {
-//   metabaseClient = createMetabaseClient();
-//   return metabaseClient;
-// };
+export const $metabase = createMetabaseClient();

From 85d73f42253e8a84140fc99af113f2a3210c434a Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Thu, 19 Dec 2024 16:12:49 +0000
Subject: [PATCH 16/22] feat: update route and swagger docs

---
 api.planx.uk/modules/analytics/docs.yaml | 54 ++++++++++++++++++++++++
 api.planx.uk/modules/analytics/routes.ts |  2 +-
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/api.planx.uk/modules/analytics/docs.yaml b/api.planx.uk/modules/analytics/docs.yaml
index bfd6f25e0a..1ee84da3a0 100644
--- a/api.planx.uk/modules/analytics/docs.yaml
+++ b/api.planx.uk/modules/analytics/docs.yaml
@@ -2,12 +2,32 @@ openapi: 3.1.0
 info:
   title: Plan✕ API
   version: 0.1.0
+
 tags:
   - name: analytics
+  - name: metabase
+
 components:
   responses:
     AnalyticsResponse:
       description: Successful response with no content. Not awaited from server as endpoint is called via the Beacon API
+
+  schemas:
+    NewCollection:
+      type: object
+      required:
+        - slug
+      properties:
+        slug:
+          type: string
+          description: Name of the collection
+        description:
+          type: string
+          description: Optional description for the collection
+        parentId:
+          type: integer
+          description: Optional ID of the parent collection
+
 paths:
   /analytics/log-user-exit:
     post:
@@ -18,6 +38,7 @@ paths:
       responses:
         "204":
           $ref: "#/components/responses/AnalyticsResponse"
+
   /analytics/log-user-resume:
     post:
       summary: Log user resume
@@ -27,3 +48,36 @@ paths:
       responses:
         "204":
           $ref: "#/components/responses/AnalyticsResponse"
+
+  /metabase/collection:
+    post:
+      summary: Create new Metabase collection
+      description: Creates a new collection in Metabase if it doesn't already exist
+      tags:
+        - metabase
+      requestBody:
+        required: true
+        content:
+          application/json:
+            schema:
+              $ref: "#/components/schemas/NewCollection"
+      responses:
+        "201":
+          description: Collection created successfully
+          content:
+            application/json:
+              schema:
+                type: object
+                properties:
+                  data:
+                    $ref: "#/components/schemas/NewCollection"
+        "400":
+          description: Bad request or collection creation failed
+          content:
+            application/json:
+              schema:
+                type: object
+                properties:
+                  error:
+                    type: string
+                    description: Error message
diff --git a/api.planx.uk/modules/analytics/routes.ts b/api.planx.uk/modules/analytics/routes.ts
index 0b34bce347..e4c9dbdb1f 100644
--- a/api.planx.uk/modules/analytics/routes.ts
+++ b/api.planx.uk/modules/analytics/routes.ts
@@ -21,7 +21,7 @@ router.post(
   logUserResumeController,
 );
 router.post(
-  "/collection",
+  "/metabase/collection",
   validate(createTeamCollectionSchema),
   metabaseCollectionsController,
 );

From 445a576aa268bb6704ae13bc525d6579b5f5a30d Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Thu, 19 Dec 2024 16:13:44 +0000
Subject: [PATCH 17/22] feat: use team name for metabase collection name not
 team slug

---
 .../metabase/collection/collection.test.ts    | 42 ++++++++++---------
 .../metabase/collection/createCollection.ts   | 12 ++----
 .../collection/getTeamIdAndMetabaseId.ts      |  2 +
 .../analytics/metabase/collection/service.ts  | 19 ++++++---
 .../analytics/metabase/collection/types.ts    |  8 ++--
 5 files changed, 45 insertions(+), 38 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 baeb6c6107..b45efae0ed 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts
@@ -27,15 +27,15 @@ describe("createTeamCollection", () => {
     // Mock Metabase API calls
     const metabaseMock = nock(process.env.METABASE_URL_EXT!)
       .post("/api/collection/", {
-        slug: "barnet",
+        name: "Barnet",
       })
       .reply(200, {
         id: 123,
-        slug: "barnet",
+        name: "Barnet",
       });
 
     const collectionId = await createCollection({
-      slug: "barnet",
+      name: "Barnet",
     });
 
     expect(collectionId).toBe(123);
@@ -49,6 +49,7 @@ describe("createTeamCollection", () => {
         returning: [
           {
             id: 26,
+            name: "Barnet",
             slug: "barnet",
             metabase_id: 123,
           },
@@ -56,30 +57,30 @@ describe("createTeamCollection", () => {
       },
     });
 
-    const testSlug = "example-council";
+    const testName = "Example Council";
     const metabaseMock = nock(process.env.METABASE_URL_EXT!);
 
     // Mock collection creation endpoint
     metabaseMock
       .post("/api/collection/", {
-        slug: testSlug,
-        parent_id: 100,
+        name: testName,
+        parentId: 100,
       })
       .reply(200, {
         id: 123,
-        slug: testSlug,
-        parent_id: 100,
+        name: testName,
+        parentId: 100,
       });
 
     // Mock GET request for verifying the new collection
     metabaseMock.get("/api/collection/123").reply(200, {
       id: 123,
-      slug: testSlug,
-      parent_id: 100,
+      name: testName,
+      parentId: 100,
     });
 
     const collectionId = await createCollection({
-      slug: testSlug,
+      name: testName,
       parentId: 100,
     });
 
@@ -88,7 +89,8 @@ describe("createTeamCollection", () => {
 
     // Verify the collection details using the service function
     const collection = await getCollection(collectionId);
-    expect(collection.parent_id).toBe(100);
+    console.log({ collection });
+    expect(collection.parentId).toBe(100);
     expect(metabaseMock.isDone()).toBe(true);
   });
 
@@ -97,6 +99,7 @@ describe("createTeamCollection", () => {
       teams: [
         {
           id: 26,
+          name: "Barnet",
           slug: "barnet",
           metabaseId: 20,
         },
@@ -114,7 +117,7 @@ describe("createTeamCollection", () => {
 
     await expect(
       createCollection({
-        slug: "test-collection",
+        name: "Test Collection",
       }),
     ).rejects.toThrow("Network error occurred");
   });
@@ -126,7 +129,7 @@ describe("createTeamCollection", () => {
 
     await expect(
       createCollection({
-        slug: "test-collection",
+        name: "Test Collection",
       }),
     ).rejects.toThrow(MetabaseError);
   });
@@ -142,6 +145,7 @@ describe("getTeamIdAndMetabaseId", () => {
       teams: [
         {
           id: 26,
+          name: "Barnet",
           slug: "barnet",
           metabaseId: 20,
         },
@@ -236,22 +240,22 @@ describe("edge cases", () => {
     ).rejects.toThrow();
   });
 
-  test("handles slug with special characters", async () => {
-    const specialSlug = "@#$%^&*";
+  test("handles name 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/", {
-        slug: specialSlug,
+        name: specialName,
       })
       .reply(200, {
         id: 789,
-        slug: specialSlug,
+        name: specialName,
       });
 
     const collection = await createCollection({
-      slug: specialSlug,
+      name: specialName,
     });
     expect(collection).toBe(789);
   });
diff --git a/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts b/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts
index 79fbe9d7be..24ae4bd0e0 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts
@@ -1,18 +1,12 @@
 import { createMetabaseClient } from "../shared/client.js";
-import type { NewCollectionParams } from "./types.js";
+import type { MetabaseCollectionParams, NewCollectionParams } from "./types.js";
 
 const client = createMetabaseClient();
 
 export async function createCollection(
-  params: NewCollectionParams,
+  params: MetabaseCollectionParams,
 ): Promise<number> {
-  const transformedParams = {
-    slug: params.slug,
-    parent_id: params.parentId,
-  };
-
-  const response = await client.post(`/api/collection/`, transformedParams);
-  const slug = response.data.slug;
+  const response = await client.post(`/api/collection/`, params);
   console.log(
     `New collection: ${response.data.slug}, new collection ID: ${response.data.id}`,
   );
diff --git a/api.planx.uk/modules/analytics/metabase/collection/getTeamIdAndMetabaseId.ts b/api.planx.uk/modules/analytics/metabase/collection/getTeamIdAndMetabaseId.ts
index 4007bfef64..5d711158cc 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/getTeamIdAndMetabaseId.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/getTeamIdAndMetabaseId.ts
@@ -4,6 +4,7 @@ import { $api } from "../../../../client/index.js";
 interface GetMetabaseId {
   teams: {
     id: number;
+    name: string;
     slug: string;
     metabaseId: number | null;
   }[];
@@ -16,6 +17,7 @@ export const getTeamIdAndMetabaseId = async (slug: string) => {
         query GetTeamAndMetabaseId($slug: String!) {
           teams(where: { slug: { _eq: $slug } }) {
             id
+            name
             slug
             metabaseId: metabase_id
           }
diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts
index 84718c4833..80cde51b03 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 { updateMetabaseId } from "./updateMetabaseId.js";
-import type { NewCollectionParams } from "./types.js";
+import type { NewCollectionParams, MetabaseCollectionParams } from "./types.js";
 import { getTeamIdAndMetabaseId } from "./getTeamIdAndMetabaseId.js";
 import { createCollection } from "./createCollection.js";
 
@@ -14,15 +14,24 @@ export async function createTeamCollection(
   params: NewCollectionParams,
 ): Promise<number> {
   try {
-    const { metabaseId, id: teamId } = await getTeamIdAndMetabaseId(
-      params.slug,
-    );
+    const {
+      metabaseId,
+      name,
+      id: teamId,
+    } = await getTeamIdAndMetabaseId(params.slug);
 
     if (metabaseId) {
       return metabaseId;
     }
 
-    const newMetabaseId = await createCollection(params);
+    const { slug, ...rest } = params;
+    const metabaseParams = {
+      name,
+      ...rest,
+    } as const;
+
+    const newMetabaseId = await createCollection(metabaseParams);
+
     await updateMetabaseId(teamId, newMetabaseId);
     return newMetabaseId;
   } catch (error) {
diff --git a/api.planx.uk/modules/analytics/metabase/collection/types.ts b/api.planx.uk/modules/analytics/metabase/collection/types.ts
index d7eb1a2070..651c96b468 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/types.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts
@@ -15,11 +15,9 @@ export interface NewCollectionParams {
 }
 
 /** Interface for request after transforming to snake case (Metabase takes snake while PlanX API takes camel) */
-export interface MetabaseCollectionParams {
+export type MetabaseCollectionParams = Omit<NewCollectionParams, "slug"> & {
   name: string;
-  description?: string;
-  parent_id?: number;
-}
+};
 
 /** Metbase collection ID for the the "Council" collection **/
 // const COUNCILS_COLLECTION_ID = 58;
@@ -45,5 +43,5 @@ export interface NewCollectionResponse {
 export interface GetCollectionResponse {
   id: number;
   slug: string;
-  parent_id: number;
+  parentId: number;
 }

From f4dea96c0a7e3578784d1e46cfb4fed8bf9bedff Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Thu, 19 Dec 2024 16:41:38 +0000
Subject: [PATCH 18/22] feat: change route to include team slug

---
 api.planx.uk/modules/analytics/docs.yaml      | 23 +++++++++++--------
 .../metabase/collection/controller.ts         |  5 +++-
 .../analytics/metabase/collection/types.ts    |  4 +++-
 api.planx.uk/modules/analytics/routes.ts      |  2 +-
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/api.planx.uk/modules/analytics/docs.yaml b/api.planx.uk/modules/analytics/docs.yaml
index 1ee84da3a0..3d26438b36 100644
--- a/api.planx.uk/modules/analytics/docs.yaml
+++ b/api.planx.uk/modules/analytics/docs.yaml
@@ -15,13 +15,8 @@ components:
   schemas:
     NewCollection:
       type: object
-      required:
-        - slug
-      properties:
-        slug:
-          type: string
-          description: Name of the collection
-        description:
+      properties:  
+        description:  
           type: string
           description: Optional description for the collection
         parentId:
@@ -49,12 +44,19 @@ paths:
         "204":
           $ref: "#/components/responses/AnalyticsResponse"
 
-  /metabase/collection:
+  /metabase/collection/{slug}:  
     post:
       summary: Create new Metabase collection
       description: Creates a new collection in Metabase if it doesn't already exist
       tags:
         - metabase
+      parameters:  
+        - name: slug  
+          in: path  
+          required: true  
+          schema:  
+            type: string  
+          description: PlanX team slug  
       requestBody:
         required: true
         content:
@@ -70,7 +72,8 @@ paths:
                 type: object
                 properties:
                   data:
-                    $ref: "#/components/schemas/NewCollection"
+                    type: integer  
+                    description: Metabase collection ID  
         "400":
           description: Bad request or collection creation failed
           content:
@@ -80,4 +83,4 @@ paths:
                 properties:
                   error:
                     type: string
-                    description: Error message
+                    description: Error message
\ No newline at end of file
diff --git a/api.planx.uk/modules/analytics/metabase/collection/controller.ts b/api.planx.uk/modules/analytics/metabase/collection/controller.ts
index d66bd2ef70..b88c3de553 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/controller.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/controller.ts
@@ -4,7 +4,10 @@ import type { NewCollectionRequestHandler } from "./types.js";
 export const metabaseCollectionsController: NewCollectionRequestHandler =
   async (_req, res) => {
     try {
-      const params = res.locals.parsedReq.body;
+      const params = {
+        ...res.locals.parsedReq.params,
+        ...res.locals.parsedReq.body,
+      };
       const collection = await createTeamCollection(params);
       res.status(201).json({ data: collection });
     } catch (error) {
diff --git a/api.planx.uk/modules/analytics/metabase/collection/types.ts b/api.planx.uk/modules/analytics/metabase/collection/types.ts
index 651c96b468..4f97b247f4 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/types.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/types.ts
@@ -23,8 +23,10 @@ export type MetabaseCollectionParams = Omit<NewCollectionParams, "slug"> & {
 // const COUNCILS_COLLECTION_ID = 58;
 
 export const createTeamCollectionSchema = z.object({
-  body: z.object({
+  params: z.object({
     slug: z.string(),
+  }),
+  body: z.object({
     description: z.string().optional(),
     parentId: z.number().optional(), //.default(COUNCILS_COLLECTION_ID),
   }),
diff --git a/api.planx.uk/modules/analytics/routes.ts b/api.planx.uk/modules/analytics/routes.ts
index e4c9dbdb1f..3de94d5c39 100644
--- a/api.planx.uk/modules/analytics/routes.ts
+++ b/api.planx.uk/modules/analytics/routes.ts
@@ -21,7 +21,7 @@ router.post(
   logUserResumeController,
 );
 router.post(
-  "/metabase/collection",
+  "/metabase/collection/:slug",
   validate(createTeamCollectionSchema),
   metabaseCollectionsController,
 );

From 5d8818c7a2fd4ef1c9a4c6cd9ed10d8f1f365664 Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Thu, 19 Dec 2024 16:49:19 +0000
Subject: [PATCH 19/22] chore: remove console.logs

---
 .../modules/analytics/metabase/collection/collection.test.ts   | 1 -
 .../modules/analytics/metabase/collection/createCollection.ts  | 3 ---
 2 files changed, 4 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 b45efae0ed..ec1dddc8f5 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts
@@ -89,7 +89,6 @@ describe("createTeamCollection", () => {
 
     // Verify the collection details using the service function
     const collection = await getCollection(collectionId);
-    console.log({ collection });
     expect(collection.parentId).toBe(100);
     expect(metabaseMock.isDone()).toBe(true);
   });
diff --git a/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts b/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts
index 24ae4bd0e0..43f8604413 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts
@@ -7,8 +7,5 @@ export async function createCollection(
   params: MetabaseCollectionParams,
 ): Promise<number> {
   const response = await client.post(`/api/collection/`, params);
-  console.log(
-    `New collection: ${response.data.slug}, new collection ID: ${response.data.id}`,
-  );
   return response.data.id;
 }

From 55d089bb20d9dedf94118f11179347fffa6887e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= <DafyddLlyr@gmail.com>
Date: Fri, 20 Dec 2024 15:22:06 +0000
Subject: [PATCH 20/22] test(api): Set up mock for metabase client

---
 api.planx.uk/modules/send/s3/index.test.ts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/api.planx.uk/modules/send/s3/index.test.ts b/api.planx.uk/modules/send/s3/index.test.ts
index 4215e17a84..b1c3d958c9 100644
--- a/api.planx.uk/modules/send/s3/index.test.ts
+++ b/api.planx.uk/modules/send/s3/index.test.ts
@@ -23,6 +23,11 @@ vi.mock("../../file/service/uploadFile.js", () => ({
 
 vi.mock("../../client/index.js");
 
+// Mock the Metabase client which also relies on Axios which these tests need to mock
+vi.mock("../../analytics/metabase/shared/client.js", () => ({
+  createMetabaseClient: vi.fn(),
+}));
+
 vi.mock("axios", () => ({
   default: vi.fn(),
 }));

From c9e9740d0e46a5209ef639872c0988eb33f1470e Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Mon, 23 Dec 2024 10:35:49 +0000
Subject: [PATCH 21/22] chore: remove metabase tag from swagger docs

---
 api.planx.uk/modules/analytics/docs.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/api.planx.uk/modules/analytics/docs.yaml b/api.planx.uk/modules/analytics/docs.yaml
index 3d26438b36..49e9489e42 100644
--- a/api.planx.uk/modules/analytics/docs.yaml
+++ b/api.planx.uk/modules/analytics/docs.yaml
@@ -5,7 +5,6 @@ info:
 
 tags:
   - name: analytics
-  - name: metabase
 
 components:
   responses:

From 6d8cd9cde2925a834678d32653056d1e81d2067b Mon Sep 17 00:00:00 2001
From: zz-hh-aa <zzhhaa@protonmail.com>
Date: Mon, 23 Dec 2024 10:55:37 +0000
Subject: [PATCH 22/22] chore: tidy imports and variables

---
 .../metabase/collection/createCollection.ts   |  8 +++---
 .../metabase/collection/getCollection.ts      |  1 -
 .../analytics/metabase/collection/service.ts  | 26 ++++++++-----------
 3 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts b/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts
index 43f8604413..56fe09a0de 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/createCollection.ts
@@ -1,11 +1,9 @@
-import { createMetabaseClient } from "../shared/client.js";
-import type { MetabaseCollectionParams, NewCollectionParams } from "./types.js";
-
-const client = createMetabaseClient();
+import type { MetabaseCollectionParams } from "./types.js";
+import { $metabase } from "../shared/client.js";
 
 export async function createCollection(
   params: MetabaseCollectionParams,
 ): Promise<number> {
-  const response = await client.post(`/api/collection/`, params);
+  const response = await $metabase.post(`/api/collection/`, params);
   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
index f1566f37f1..587fb8bf0f 100644
--- a/api.planx.uk/modules/analytics/metabase/collection/getCollection.ts
+++ b/api.planx.uk/modules/analytics/metabase/collection/getCollection.ts
@@ -1,4 +1,3 @@
-import { createMetabaseClient } from "../shared/client.js";
 import type { GetCollectionResponse } from "./types.js";
 import { $metabase } from "../shared/client.js";
 
diff --git a/api.planx.uk/modules/analytics/metabase/collection/service.ts b/api.planx.uk/modules/analytics/metabase/collection/service.ts
index 80cde51b03..a52044c858 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 { updateMetabaseId } from "./updateMetabaseId.js";
-import type { NewCollectionParams, MetabaseCollectionParams } from "./types.js";
+import type { NewCollectionParams } from "./types.js";
 import { getTeamIdAndMetabaseId } from "./getTeamIdAndMetabaseId.js";
 import { createCollection } from "./createCollection.js";
 
@@ -10,27 +10,23 @@ import { createCollection } from "./createCollection.js";
  * @params `slug` 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 createTeamCollection(
-  params: NewCollectionParams,
-): Promise<number> {
+export async function createTeamCollection({
+  slug,
+  parentId,
+  description,
+}: NewCollectionParams): Promise<number> {
   try {
-    const {
-      metabaseId,
-      name,
-      id: teamId,
-    } = await getTeamIdAndMetabaseId(params.slug);
+    const { metabaseId, name, id: teamId } = await getTeamIdAndMetabaseId(slug);
 
     if (metabaseId) {
       return metabaseId;
     }
 
-    const { slug, ...rest } = params;
-    const metabaseParams = {
+    const newMetabaseId = await createCollection({
       name,
-      ...rest,
-    } as const;
-
-    const newMetabaseId = await createCollection(metabaseParams);
+      parentId,
+      description,
+    });
 
     await updateMetabaseId(teamId, newMetabaseId);
     return newMetabaseId;