Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wip: feat: collection service & controller #3971

Draft
wants to merge 46 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
9f469cb
feat: create type and validator
zz-hh-aa Nov 18, 2024
164798f
chore: change 'council' to 'team'
zz-hh-aa Nov 18, 2024
dc22f0b
feat: setup collection service
zz-hh-aa Nov 18, 2024
a957c1a
feat: create new collection type
zz-hh-aa Nov 18, 2024
9739d6b
feat: create collection controller
zz-hh-aa Nov 18, 2024
16a02b3
chore: change id type to number
zz-hh-aa Nov 18, 2024
165eca6
feat: use new MetabaseError type
zz-hh-aa Nov 18, 2024
a4d6b5f
chore: tidy error handling in controller
zz-hh-aa Nov 18, 2024
62836ba
chore: move collection types to types.ts
zz-hh-aa Nov 18, 2024
73ba1e9
feat: remove unnecessary checkCollection controller
zz-hh-aa Dec 2, 2024
22d3ee4
chore: simplify error handling to just throw
zz-hh-aa Dec 2, 2024
e19ce99
nit: correctly rename type as handler
zz-hh-aa Dec 2, 2024
bc13342
chore: tidy data types and undefined values
zz-hh-aa Dec 2, 2024
97ee946
chore: add snake_case and camelCase conversions
zz-hh-aa Dec 2, 2024
1a4ca85
chore: re-import createMetabaseClient after rebase
zz-hh-aa Dec 2, 2024
6ac50e7
feat: remove manual toSnakeCase function
zz-hh-aa Dec 3, 2024
1d8dfb4
feat: add metabase collection route
zz-hh-aa Dec 3, 2024
ae037aa
feat: remove unnecessary authentication and transformation
zz-hh-aa Dec 3, 2024
9d6878c
feat: transform request in newCollectionSchema
zz-hh-aa Dec 3, 2024
cea1b05
feat: add metabase_id column to teams table migration
zz-hh-aa Dec 9, 2024
df3345e
chore: add assert import
zz-hh-aa Dec 9, 2024
628c151
wip: store metabase id to planx db
zz-hh-aa Dec 9, 2024
3974b2c
feat: update Hasura api permissions
zz-hh-aa Dec 10, 2024
1d885e0
test: write tests for collections service
zz-hh-aa Dec 2, 2024
62ca295
chore: update user role to api
zz-hh-aa Dec 10, 2024
052b0f3
feat: change string matching to exact
zz-hh-aa Dec 10, 2024
bb3c642
test: add tsdoc and getCollection function for tests
zz-hh-aa Dec 2, 2024
d88d118
chore: rename updateMetabaseId file
zz-hh-aa Dec 10, 2024
0b6153c
test: add new test for updateMetabaseId
zz-hh-aa Dec 10, 2024
b76bde1
chore: throw error in function
zz-hh-aa Dec 10, 2024
0b33f0b
chore: update import after updateMetabaseId name change
zz-hh-aa Dec 10, 2024
7ab0398
chore: remove console.log
zz-hh-aa Dec 10, 2024
110ce9c
chore: remove old checkCollection
zz-hh-aa Dec 10, 2024
5ec8b7a
feat: getTeamAndMetabaseId function
zz-hh-aa Dec 10, 2024
f2c5ba6
feat: separate newCollection into own function
zz-hh-aa Dec 10, 2024
a6f005d
test: test for collection services
zz-hh-aa Dec 10, 2024
e234edc
chore: tidy function names
zz-hh-aa Dec 10, 2024
c384fee
chore: move createCollection to own file
zz-hh-aa Dec 10, 2024
8378601
chore: tidy comments and console.logs
zz-hh-aa Dec 11, 2024
1aa5f1c
feat: move getCollection to own file
zz-hh-aa Dec 11, 2024
08f2402
feat: type getCollection response
zz-hh-aa Dec 11, 2024
26f9ec2
chore: type checkCollections response
zz-hh-aa Dec 11, 2024
ee3a730
chore: rename to getTeamIdAndMetabaseId for clarity
zz-hh-aa Dec 11, 2024
ecca6d6
chore: tidy destructuring
zz-hh-aa Dec 11, 2024
7203db0
chore: rename to createCollectionIfDoesNotExist for clarity
zz-hh-aa Dec 11, 2024
5248d51
chore: rename to MetabaseCollectionsController for clarity
zz-hh-aa Dec 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
297 changes: 296 additions & 1 deletion api.planx.uk/modules/analytics/metabase/collection/collection.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
18 changes: 18 additions & 0 deletions api.planx.uk/modules/analytics/metabase/collection/controller.ts
Original file line number Diff line number Diff line change
@@ -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",
});
}
};
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
@@ -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<GetMetabaseId>(
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;
}
};
Loading
Loading