feat(core): Add internal API to manage metrics of a test definition (no-changelog) (#11851)

packages/cli/src/evaluation/metric.schema.ts

@@ -0,0 +1,13 @@
+import { z } from 'zod';
+
+export const testMetricCreateRequestBodySchema = z
+	.object({
+		name: z.string().min(1).max(255),
+	})
+	.strict();
+
+export const testMetricPatchRequestBodySchema = z
+	.object({
+		name: z.string().min(1).max(255),
+	})
+	.strict();

packages/cli/src/evaluation/metrics.controller.ts

@@ -0,0 +1,130 @@
+import express from 'express';
+
+import { TestMetricRepository } from '@/databases/repositories/test-metric.repository.ee';
+import { Delete, Get, Patch, Post, RestController } from '@/decorators';
+import { NotFoundError } from '@/errors/response-errors/not-found.error';
+import {
+	testMetricCreateRequestBodySchema,
+	testMetricPatchRequestBodySchema,
+} from '@/evaluation/metric.schema';
+import { getSharedWorkflowIds } from '@/public-api/v1/handlers/workflows/workflows.service';
+
+import { TestDefinitionService } from './test-definition.service.ee';
+import { TestMetricsRequest } from './test-definitions.types.ee';
+
+@RestController('/evaluation/test-definitions')
+export class TestMetricsController {
+	constructor(
+		private readonly testDefinitionService: TestDefinitionService,
+		private readonly testMetricRepository: TestMetricRepository,
+	) {}
+
+	// This method is used in multiple places in the controller to get the test definition
+	// (or just check that it exists and the user has access to it).
+	private async getTestDefinition(
+		req:
+			| TestMetricsRequest.GetOne
+			| TestMetricsRequest.GetMany
+			| TestMetricsRequest.Patch
+			| TestMetricsRequest.Delete
+			| TestMetricsRequest.Create,
+	) {
+		const { testDefinitionId } = req.params;
+
+		const userAccessibleWorkflowIds = await getSharedWorkflowIds(req.user, ['workflow:read']);
+
+		const testDefinition = await this.testDefinitionService.findOne(
+			testDefinitionId,
+			userAccessibleWorkflowIds,
+		);
+
+		if (!testDefinition) throw new NotFoundError('Test definition not found');
+
+		return testDefinition;
+	}
+
+	@Get('/:testDefinitionId/metrics')
+	async getMany(req: TestMetricsRequest.GetMany) {
+		const { testDefinitionId } = req.params;
+
+		await this.getTestDefinition(req);
+
+		return await this.testMetricRepository.find({
+			where: { testDefinition: { id: testDefinitionId } },
+		});
+	}
+
+	@Get('/:testDefinitionId/metrics/:id')
+	async getOne(req: TestMetricsRequest.GetOne) {
+		const { id: metricId, testDefinitionId } = req.params;
+
+		await this.getTestDefinition(req);
+
+		const metric = await this.testMetricRepository.findOne({
+			where: { id: metricId, testDefinition: { id: testDefinitionId } },
+		});
+
+		if (!metric) throw new NotFoundError('Metric not found');
+
+		return metric;
+	}
+
+	@Post('/:testDefinitionId/metrics')
+	async create(req: TestMetricsRequest.Create, res: express.Response) {
+		const bodyParseResult = testMetricCreateRequestBodySchema.safeParse(req.body);
+		if (!bodyParseResult.success) {
+			res.status(400).json({ errors: bodyParseResult.error.errors });
+			return;
+		}
+
+		const testDefinition = await this.getTestDefinition(req);
+
+		const metric = this.testMetricRepository.create({
+			...req.body,
+			testDefinition,
+		});
+
+		return await this.testMetricRepository.save(metric);
+	}
+
+	@Patch('/:testDefinitionId/metrics/:id')
+	async patch(req: TestMetricsRequest.Patch, res: express.Response) {
+		const { id: metricId, testDefinitionId } = req.params;
+
+		const bodyParseResult = testMetricPatchRequestBodySchema.safeParse(req.body);
+		if (!bodyParseResult.success) {
+			res.status(400).json({ errors: bodyParseResult.error.errors });
+			return;
+		}
+
+		await this.getTestDefinition(req);
+
+		const metric = await this.testMetricRepository.findOne({
+			where: { id: metricId, testDefinition: { id: testDefinitionId } },
+		});
+
+		if (!metric) throw new NotFoundError('Metric not found');
+
+		await this.testMetricRepository.update(metricId, bodyParseResult.data);
+
+		// Respond with the updated metric
+		return await this.testMetricRepository.findOneBy({ id: metricId });
+	}
+
+	@Delete('/:testDefinitionId/metrics/:id')
+	async delete(req: TestMetricsRequest.GetOne) {
+		const { id: metricId, testDefinitionId } = req.params;
+
+		await this.getTestDefinition(req);
+
+		const metric = await this.testMetricRepository.findOne({
+			where: { id: metricId, testDefinition: { id: testDefinitionId } },
+		});
+
+		if (!metric) throw new NotFoundError('Metric not found');
+
+		await this.testMetricRepository.delete(metricId);
+
+		return { success: true };
+	}
+}

packages/cli/src/evaluation/test-definitions.types.ee.ts

@@ -33,3 +33,33 @@ export declare namespace TestDefinitionsRequest {
 
 	type Run = AuthenticatedRequest<RouteParams.TestId>;
 }
+
+// ----------------------------------
+//             /test-definitions/:testDefinitionId/metrics
+// ----------------------------------
+
+export declare namespace TestMetricsRequest {
+	namespace RouteParams {
+		type TestDefinitionId = {
+			testDefinitionId: string;
+		};
+
+		type TestMetricId = {
+			id: string;
+		};
+	}
+
+	type GetOne = AuthenticatedRequest<RouteParams.TestDefinitionId & RouteParams.TestMetricId>;
+
+	type GetMany = AuthenticatedRequest<RouteParams.TestDefinitionId>;
+
+	type Create = AuthenticatedRequest<RouteParams.TestDefinitionId, {}, { name: string }>;
+
+	type Patch = AuthenticatedRequest<
+		RouteParams.TestDefinitionId & RouteParams.TestMetricId,
+		{},
+		{ name: string }
+	>;
+
+	type Delete = AuthenticatedRequest<RouteParams.TestDefinitionId & RouteParams.TestMetricId>;
+}

packages/cli/src/server.ts

@@ -64,6 +64,7 @@ import '@/executions/executions.controller';
 import '@/external-secrets/external-secrets.controller.ee';
 import '@/license/license.controller';
 import '@/evaluation/test-definitions.controller.ee';
+import '@/evaluation/metrics.controller';
 import '@/workflows/workflow-history/workflow-history.controller.ee';
 import '@/workflows/workflows.controller';
 

packages/cli/test/integration/evaluation/metrics.api.test.ts

@@ -0,0 +1,381 @@
+import { Container } from 'typedi';
+
+import type { TestDefinition } from '@/databases/entities/test-definition.ee';
+import type { User } from '@/databases/entities/user';
+import type { WorkflowEntity } from '@/databases/entities/workflow-entity';
+import { TestDefinitionRepository } from '@/databases/repositories/test-definition.repository.ee';
+import { TestMetricRepository } from '@/databases/repositories/test-metric.repository.ee';
+import { createUserShell } from '@test-integration/db/users';
+import { createWorkflow } from '@test-integration/db/workflows';
+import * as testDb from '@test-integration/test-db';
+import type { SuperAgentTest } from '@test-integration/types';
+import * as utils from '@test-integration/utils';
+
+let authOwnerAgent: SuperAgentTest;
+let workflowUnderTest: WorkflowEntity;
+let otherWorkflow: WorkflowEntity;
+let testDefinition: TestDefinition;
+let otherTestDefinition: TestDefinition;
+let ownerShell: User;
+
+const testServer = utils.setupTestServer({ endpointGroups: ['evaluation'] });
+
+beforeAll(async () => {
+	ownerShell = await createUserShell('global:owner');
+	authOwnerAgent = testServer.authAgentFor(ownerShell);
+});
+
+beforeEach(async () => {
+	await testDb.truncate(['TestDefinition', 'TestMetric']);
+
+	workflowUnderTest = await createWorkflow({ name: 'workflow-under-test' }, ownerShell);
+
+	testDefinition = Container.get(TestDefinitionRepository).create({
+		name: 'test',
+		workflow: { id: workflowUnderTest.id },
+	});
+	await Container.get(TestDefinitionRepository).save(testDefinition);
+
+	otherWorkflow = await createWorkflow({ name: 'other-workflow' });
+
+	otherTestDefinition = Container.get(TestDefinitionRepository).create({
+		name: 'other-test',
+		workflow: { id: otherWorkflow.id },
+	});
+	await Container.get(TestDefinitionRepository).save(otherTestDefinition);
+});
+
+describe('GET /evaluation/test-definitions/:testDefinitionId/metrics', () => {
+	test('should retrieve empty list of metrics for a test definition', async () => {
+		const resp = await authOwnerAgent.get(
+			`/evaluation/test-definitions/${testDefinition.id}/metrics`,
+		);
+
+		expect(resp.statusCode).toBe(200);
+		expect(resp.body.data.length).toBe(0);
+	});
+
+	test('should retrieve metrics for a test definition', async () => {
+		const newMetric = Container.get(TestMetricRepository).create({
+			testDefinition: { id: testDefinition.id },
+			name: 'metric-1',
+		});
+		await Container.get(TestMetricRepository).save(newMetric);
+
+		const newMetric2 = Container.get(TestMetricRepository).create({
+			testDefinition: { id: testDefinition.id },
+			name: 'metric-2',
+		});
+		await Container.get(TestMetricRepository).save(newMetric2);
+
+		const resp = await authOwnerAgent.get(
+			`/evaluation/test-definitions/${testDefinition.id}/metrics`,
+		);
+
+		expect(resp.statusCode).toBe(200);
+		expect(resp.body.data.length).toBe(2);
+		expect(resp.body.data).toEqual(
+			expect.arrayContaining([
+				expect.objectContaining({
+					id: expect.any(String),
+					name: 'metric-1',
+				}),
+				expect.objectContaining({
+					id: expect.any(String),
+					name: 'metric-2',
+				}),
+			]),
+		);
+	});
+
+	test('should return 404 if test definition does not exist', async () => {
+		const resp = await authOwnerAgent.get('/evaluation/test-definitions/999/metrics');
+
+		expect(resp.statusCode).toBe(404);
+	});
+
+	test('should return 404 if test definition is not accessible to the user', async () => {
+		const resp = await authOwnerAgent.get(
+			`/evaluation/test-definitions/${otherTestDefinition.id}/metrics`,
+		);
+
+		expect(resp.statusCode).toBe(404);
+	});
+});
+
+describe('GET /evaluation/test-definitions/:testDefinitionId/metrics/:id', () => {
+	test('should retrieve a metric for a test definition', async () => {
+		const newMetric = Container.get(TestMetricRepository).create({
+			testDefinition: { id: testDefinition.id },
+			name: 'metric-1',
+		});
+		await Container.get(TestMetricRepository).save(newMetric);
+
+		const resp = await authOwnerAgent.get(
+			`/evaluation/test-definitions/${testDefinition.id}/metrics/${newMetric.id}`,
+		);
+
+		expect(resp.statusCode).toBe(200);
+		expect(resp.body.data).toEqual(
+			expect.objectContaining({
+				id: newMetric.id,
+				name: 'metric-1',
+			}),
+		);
+	});
+
+	test('should return 404 if metric does not exist', async () => {
+		const resp = await authOwnerAgent.get(
+			`/evaluation/test-definitions/${testDefinition.id}/metrics/999`,
+		);
+
+		expect(resp.statusCode).toBe(404);
+	});
+
+	test('should return 404 if metric is not accessible to the user', async () => {
+		const newMetric = Container.get(TestMetricRepository).create({
+			testDefinition: { id: otherTestDefinition.id },
+			name: 'metric-1',
+		});
+		await Container.get(TestMetricRepository).save(newMetric);
+
+		const resp = await authOwnerAgent.get(
+			`/evaluation/test-definitions/${otherTestDefinition.id}/metrics/${newMetric.id}`,
+		);
+
+		expect(resp.statusCode).toBe(404);
+	});
+});
+
+describe('POST /evaluation/test-definitions/:testDefinitionId/metrics', () => {
+	test('should create a metric for a test definition', async () => {
+		const resp = await authOwnerAgent
+			.post(`/evaluation/test-definitions/${testDefinition.id}/metrics`)
+			.send({
+				name: 'metric-1',
+			});
+
+		expect(resp.statusCode).toBe(200);
+		expect(resp.body.data).toEqual(
+			expect.objectContaining({
+				id: expect.any(String),
+				name: 'metric-1',
+			}),
+		);
+
+		const metrics = await Container.get(TestMetricRepository).find({
+			where: { testDefinition: { id: testDefinition.id } },
+		});
+		expect(metrics.length).toBe(1);
+		expect(metrics[0].name).toBe('metric-1');
+	});
+
+	test('should return 400 if name is missing', async () => {
+		const resp = await authOwnerAgent
+			.post(`/evaluation/test-definitions/${testDefinition.id}/metrics`)
+			.send({});
+
+		expect(resp.statusCode).toBe(400);
+		expect(resp.body.errors).toEqual(
+			expect.arrayContaining([
+				expect.objectContaining({
+					code: 'invalid_type',
+					message: 'Required',
+					path: ['name'],
+				}),
+			]),
+		);
+	});
+
+	test('should return 400 if name is not a string', async () => {
+		const resp = await authOwnerAgent
+			.post(`/evaluation/test-definitions/${testDefinition.id}/metrics`)
+			.send({
+				name: 123,
+			});
+
+		expect(resp.statusCode).toBe(400);
+		expect(resp.body.errors).toEqual(
+			expect.arrayContaining([
+				expect.objectContaining({
+					code: 'invalid_type',
+					message: 'Expected string, received number',
+					path: ['name'],
+				}),
+			]),
+		);
+	});
+
+	test('should return 404 if test definition does not exist', async () => {
+		const resp = await authOwnerAgent.post('/evaluation/test-definitions/999/metrics').send({
+			name: 'metric-1',
+		});
+
+		expect(resp.statusCode).toBe(404);
+	});
+
+	test('should return 404 if test definition is not accessible to the user', async () => {
+		const resp = await authOwnerAgent
+			.post(`/evaluation/test-definitions/${otherTestDefinition.id}/metrics`)
+			.send({
+				name: 'metric-1',
+			});
+
+		expect(resp.statusCode).toBe(404);
+	});
+});
+
+describe('PATCH /evaluation/test-definitions/:testDefinitionId/metrics/:id', () => {
+	test('should update a metric for a test definition', async () => {
+		const newMetric = Container.get(TestMetricRepository).create({
+			testDefinition: { id: testDefinition.id },
+			name: 'metric-1',
+		});
+		await Container.get(TestMetricRepository).save(newMetric);
+
+		const resp = await authOwnerAgent
+			.patch(`/evaluation/test-definitions/${testDefinition.id}/metrics/${newMetric.id}`)
+			.send({
+				name: 'metric-2',
+			});
+
+		expect(resp.statusCode).toBe(200);
+		expect(resp.body.data).toEqual(
+			expect.objectContaining({
+				id: newMetric.id,
+				name: 'metric-2',
+			}),
+		);
+
+		const metrics = await Container.get(TestMetricRepository).find({
+			where: { testDefinition: { id: testDefinition.id } },
+		});
+		expect(metrics.length).toBe(1);
+		expect(metrics[0].name).toBe('metric-2');
+	});
+
+	test('should return 400 if name is missing', async () => {
+		const newMetric = Container.get(TestMetricRepository).create({
+			testDefinition: { id: testDefinition.id },
+			name: 'metric-1',
+		});
+		await Container.get(TestMetricRepository).save(newMetric);
+
+		const resp = await authOwnerAgent
+			.patch(`/evaluation/test-definitions/${testDefinition.id}/metrics/${newMetric.id}`)
+			.send({});
+
+		expect(resp.statusCode).toBe(400);
+		expect(resp.body.errors).toEqual(
+			expect.arrayContaining([
+				expect.objectContaining({
+					code: 'invalid_type',
+					message: 'Required',
+					path: ['name'],
+				}),
+			]),
+		);
+	});
+
+	test('should return 400 if name is not a string', async () => {
+		const newMetric = Container.get(TestMetricRepository).create({
+			testDefinition: { id: testDefinition.id },
+			name: 'metric-1',
+		});
+		await Container.get(TestMetricRepository).save(newMetric);
+
+		const resp = await authOwnerAgent
+			.patch(`/evaluation/test-definitions/${testDefinition.id}/metrics/${newMetric.id}`)
+			.send({
+				name: 123,
+			});
+
+		expect(resp.statusCode).toBe(400);
+		expect(resp.body.errors).toEqual(
+			expect.arrayContaining([
+				expect.objectContaining({
+					code: 'invalid_type',
+					message: 'Expected string, received number',
+				}),
+			]),
+		);
+	});
+
+	test('should return 404 if metric does not exist', async () => {
+		const resp = await authOwnerAgent
+			.patch(`/evaluation/test-definitions/${testDefinition.id}/metrics/999`)
+			.send({
+				name: 'metric-1',
+			});
+
+		expect(resp.statusCode).toBe(404);
+	});
+
+	test('should return 404 if test definition does not exist', async () => {
+		const resp = await authOwnerAgent.patch('/evaluation/test-definitions/999/metrics/999').send({
+			name: 'metric-1',
+		});
+
+		expect(resp.statusCode).toBe(404);
+	});
+
+	test('should return 404 if metric is not accessible to the user', async () => {
+		const newMetric = Container.get(TestMetricRepository).create({
+			testDefinition: { id: otherTestDefinition.id },
+			name: 'metric-1',
+		});
+		await Container.get(TestMetricRepository).save(newMetric);
+
+		const resp = await authOwnerAgent
+			.patch(`/evaluation/test-definitions/${otherTestDefinition.id}/metrics/${newMetric.id}`)
+			.send({
+				name: 'metric-2',
+			});
+
+		expect(resp.statusCode).toBe(404);
+	});
+});
+
+describe('DELETE /evaluation/test-definitions/:testDefinitionId/metrics/:id', () => {
+	test('should delete a metric for a test definition', async () => {
+		const newMetric = Container.get(TestMetricRepository).create({
+			testDefinition: { id: testDefinition.id },
+			name: 'metric-1',
+		});
+		await Container.get(TestMetricRepository).save(newMetric);
+
+		const resp = await authOwnerAgent.delete(
+			`/evaluation/test-definitions/${testDefinition.id}/metrics/${newMetric.id}`,
+		);
+
+		expect(resp.statusCode).toBe(200);
+		expect(resp.body.data).toEqual({ success: true });
+
+		const metrics = await Container.get(TestMetricRepository).find({
+			where: { testDefinition: { id: testDefinition.id } },
+		});
+		expect(metrics.length).toBe(0);
+	});
+
+	test('should return 404 if metric does not exist', async () => {
+		const resp = await authOwnerAgent.delete(
+			`/evaluation/test-definitions/${testDefinition.id}/metrics/999`,
+		);
+
+		expect(resp.statusCode).toBe(404);
+	});
+
+	test('should return 404 if metric is not accessible to the user', async () => {
+		const newMetric = Container.get(TestMetricRepository).create({
+			testDefinition: { id: otherTestDefinition.id },
+			name: 'metric-1',
+		});
+		await Container.get(TestMetricRepository).save(newMetric);
+
+		const resp = await authOwnerAgent.delete(
+			`/evaluation/test-definitions/${otherTestDefinition.id}/metrics/${newMetric.id}`,
+		);
+
+		expect(resp.statusCode).toBe(404);
+	});
+});

packages/cli/test/integration/shared/test-db.ts

@@ -75,6 +75,7 @@ const repositories = [
 	'SharedWorkflow',
 	'Tag',
 	'TestDefinition',
+	'TestMetric',
 	'User',
 	'Variables',
 	'Webhook',

packages/cli/test/integration/shared/utils/test-server.ts

@@ -279,6 +279,7 @@ export const setupTestServer = ({
 						break;
 
 					case 'evaluation':
+						await import('@/evaluation/metrics.controller');
 						await import('@/evaluation/test-definitions.controller.ee');
 						break;
 				}