refactor(core): Remove legacy middleware from insights module (#14819)

packages/@n8n/api-types/src/dto/index.ts

@@ -64,3 +64,4 @@ export { DeleteFolderDto } from './folders/delete-folder.dto';
 export { ListFolderQueryDto } from './folders/list-folder-query.dto';
 
 export { ListInsightsWorkflowQueryDto } from './insights/list-workflow-query.dto';
+export { PaginationDto } from './pagination/pagination.dto';

packages/@n8n/api-types/src/dto/insights/list-workflow-query.dto.ts

@@ -1,6 +1,8 @@
 import { z } from 'zod';
 import { Z } from 'zod-class';
 
+import { paginationSchema } from '../pagination/pagination.dto';
+
 const VALID_SORT_OPTIONS = [
 	'total:asc',
 	'total:desc',
@@ -22,31 +24,11 @@ const VALID_SORT_OPTIONS = [
 // Parameter Validators
 // ---------------------
 
-// Skip parameter validation
-const skipValidator = z
-	.string()
-	.optional()
-	.transform((val) => (val ? parseInt(val, 10) : 0))
-	.refine((val) => !isNaN(val), {
-		message: 'Skip must be a valid number',
-	});
-
-// Take parameter validation
-const takeValidator = z
-	.string()
-	.optional()
-	.transform((val) => (val ? parseInt(val, 10) : 10))
-	.refine((val) => !isNaN(val), {
-		message: 'Take must be a valid number',
-	});
-
-// SortBy parameter validation
 const sortByValidator = z
 	.enum(VALID_SORT_OPTIONS, { message: `sortBy must be one of: ${VALID_SORT_OPTIONS.join(', ')}` })
 	.optional();
 
 export class ListInsightsWorkflowQueryDto extends Z.class({
-	skip: skipValidator,
+	...paginationSchema,
-	take: takeValidator,
 	sortBy: sortByValidator,
 }) {}

packages/@n8n/api-types/src/dto/pagination/__tests__/pagination.dto.test.ts

@@ -0,0 +1,136 @@
+import { PaginationDto, MAX_ITEMS_PER_PAGE } from '../pagination.dto';
+
+describe('PaginationDto', () => {
+	describe('valid inputs', () => {
+		test('should validate with both take and skip', () => {
+			const result = PaginationDto.safeParse({ take: '10', skip: '5' });
+
+			expect(result.success).toBe(true);
+			if (result.success) {
+				expect(result.data).toEqual({ take: 10, skip: 5 });
+			}
+		});
+
+		test('should validate with only take', () => {
+			const result = PaginationDto.safeParse({ take: '10' });
+
+			expect(result.success).toBe(true);
+			if (result.success) {
+				expect(result.data).toEqual({ take: 10, skip: 0 });
+			}
+		});
+
+		test('should cap take at MAX_ITEMS_PER_PAGE', () => {
+			const result = PaginationDto.safeParse({ take: `${MAX_ITEMS_PER_PAGE + 10}` });
+
+			expect(result.success).toBe(true);
+			if (result.success) {
+				expect(result.data).toEqual({ take: MAX_ITEMS_PER_PAGE, skip: 0 });
+			}
+		});
+
+		test('should handle zero values', () => {
+			const result = PaginationDto.safeParse({ take: '0', skip: '0' });
+
+			expect(result.success).toBe(true);
+			if (result.success) {
+				expect(result.data).toEqual({ take: 0, skip: 0 });
+			}
+		});
+	});
+
+	describe('invalid inputs', () => {
+		test('should reject non-integer take', () => {
+			const result = PaginationDto.safeParse({ take: 'hello' });
+
+			expect(result.success).toBe(false);
+			if (!result.success) {
+				expect(result.error.issues[0].path).toContain('take');
+			}
+		});
+
+		test('should reject non-integer skip', () => {
+			const result = PaginationDto.safeParse({ take: '10', skip: 'hello' });
+
+			expect(result.success).toBe(false);
+			if (!result.success) {
+				expect(result.error.issues[0].path).toContain('skip');
+			}
+		});
+
+		test('should reject non-numeric take', () => {
+			const result = PaginationDto.safeParse({ take: 'abc' });
+
+			expect(result.success).toBe(false);
+			if (!result.success) {
+				expect(result.error.issues[0].path).toContain('take');
+			}
+		});
+
+		test('should reject non-numeric skip', () => {
+			const result = PaginationDto.safeParse({ take: '10', skip: 'abc' });
+
+			expect(result.success).toBe(false);
+			if (!result.success) {
+				expect(result.error.issues[0].path).toContain('skip');
+			}
+		});
+
+		test('should reject object as take', () => {
+			const result = PaginationDto.safeParse({ take: {} });
+
+			expect(result.success).toBe(false);
+			if (!result.success) {
+				expect(result.error.issues[0].path).toContain('take');
+			}
+		});
+
+		test('should reject array as skip', () => {
+			const result = PaginationDto.safeParse({ take: '10', skip: [] });
+
+			expect(result.success).toBe(false);
+			if (!result.success) {
+				expect(result.error.issues[0].path).toContain('skip');
+			}
+		});
+
+		test('should reject negative take', () => {
+			const result = PaginationDto.safeParse({ take: '-5' });
+
+			expect(result.success).toBe(false);
+			if (!result.success) {
+				expect(result.error.issues[0].path).toContain('take');
+			}
+		});
+
+		test('should reject negative skip', () => {
+			const result = PaginationDto.safeParse({ take: '10', skip: '-5' });
+
+			expect(result.success).toBe(false);
+			if (!result.success) {
+				expect(result.error.issues[0].path).toContain('skip');
+			}
+		});
+	});
+
+	describe('edge cases', () => {
+		test('should apply cap on extremely large numbers', () => {
+			const veryLargeNumber = '9'.repeat(20);
+			const result = PaginationDto.safeParse({ take: veryLargeNumber });
+
+			expect(result.success).toBe(true);
+			if (result.success) {
+				expect(result.data.take).toBe(MAX_ITEMS_PER_PAGE);
+			}
+		});
+
+		test('should fall back to default on empty string value', () => {
+			const result = PaginationDto.safeParse({ take: '' });
+
+			expect(result.success).toBe(true);
+			if (result.success) {
+				expect(result.data.take).toBe(10);
+			}
+		});
+	});
+});

packages/@n8n/api-types/src/dto/pagination/pagination.dto.ts

@@ -0,0 +1,34 @@
+import { z } from 'zod';
+import { Z } from 'zod-class';
+
+export const MAX_ITEMS_PER_PAGE = 50;
+
+const skipValidator = z
+	.string()
+	.optional()
+	.transform((val) => (val ? parseInt(val, 10) : 0))
+	.refine((val) => !isNaN(val) && Number.isInteger(val), {
+		message: 'Param `skip` must be a valid integer',
+	})
+	.refine((val) => val >= 0, {
+		message: 'Param `skip` must be a non-negative integer',
+	});
+
+const takeValidator = z
+	.string()
+	.optional()
+	.transform((val) => (val ? parseInt(val, 10) : 10))
+	.refine((val) => !isNaN(val) && Number.isInteger(val), {
+		message: 'Param `take` must be a valid integer',
+	})
+	.refine((val) => val >= 0, {
+		message: 'Param `take` must be a non-negative integer',
+	})
+	.transform((val) => Math.min(val, MAX_ITEMS_PER_PAGE));
+
+export const paginationSchema = {
+	skip: skipValidator,
+	take: takeValidator,
+};
+
+export class PaginationDto extends Z.class(paginationSchema) {}

packages/cli/src/middlewares/list-query/index.ts

@@ -13,6 +13,9 @@ export type ListQueryMiddleware = (
 	next: NextFunction,
 ) => void;
 
+/**
+ * @deprecated Please create Zod validators in `@n8n/api-types` instead.
+ */
 export const listQueryMiddleware: ListQueryMiddleware[] = [
 	filterListQueryMiddleware,
 	selectListQueryMiddleware,

packages/cli/src/modules/insights/insights.controller.ts

@@ -2,8 +2,6 @@ import { ListInsightsWorkflowQueryDto } from '@n8n/api-types';
 import type { InsightsSummary, InsightsByTime, InsightsByWorkflow } from '@n8n/api-types';
 
 import { Get, GlobalScope, Licensed, Query, RestController } from '@/decorators';
-import { paginationListQueryMiddleware } from '@/middlewares/list-query/pagination';
-import { sortByQueryMiddleware } from '@/middlewares/list-query/sort-by';
 import { AuthenticatedRequest } from '@/requests';
 
 import { InsightsService } from './insights.service';
@@ -22,7 +20,7 @@ export class InsightsController {
 		});
 	}
 
-	@Get('/by-workflow', { middlewares: [paginationListQueryMiddleware, sortByQueryMiddleware] })
+	@Get('/by-workflow')
 	@GlobalScope('insights:list')
 	@Licensed('feat:insights:viewDashboard')
 	async getInsightsByWorkflow(

packages/cli/src/workflows/workflow-history.ee/workflow-history.controller.ee.ts

@@ -1,10 +1,10 @@
+import { PaginationDto } from '@n8n/api-types';
 import { Request, Response, NextFunction } from 'express';
 
-import { RestController, Get, Middleware } from '@/decorators';
+import { RestController, Get, Middleware, Query } from '@/decorators';
 import { NotFoundError } from '@/errors/response-errors/not-found.error';
 import { SharedWorkflowNotFoundError } from '@/errors/shared-workflow-not-found.error';
 import { WorkflowHistoryVersionNotFoundError } from '@/errors/workflow-history-version-not-found.error';
-import { paginationListQueryMiddleware } from '@/middlewares/list-query/pagination';
 import { WorkflowHistoryRequest } from '@/requests';
 
 import { isWorkflowHistoryEnabled, isWorkflowHistoryLicensed } from './workflow-history-helper.ee';
@@ -36,14 +36,14 @@ export class WorkflowHistoryController {
 		next();
 	}
 
-	@Get('/workflow/:workflowId', { middlewares: [paginationListQueryMiddleware] })
+	@Get('/workflow/:workflowId')
-	async getList(req: WorkflowHistoryRequest.GetList) {
+	async getList(req: WorkflowHistoryRequest.GetList, _res: Response, @Query query: PaginationDto) {
 		try {
 			return await this.historyService.getList(
 				req.user,
 				req.params.workflowId,
-				req.query.take ?? DEFAULT_TAKE,
+				query.take ?? DEFAULT_TAKE,
-				req.query.skip ?? 0,
+				query.skip ?? 0,
 			);
 		} catch (e) {
 			if (e instanceof SharedWorkflowNotFoundError) {

packages/cli/test/integration/insights/insights.api.test.ts

@@ -72,9 +72,9 @@ describe('GET /insights/by-workflow', () => {
 			take: 'not_a_number',
 		},
 	])(
-		'Call should return internal server error with invalid pagination query parameters',
+		'Call should return bad request with invalid pagination query parameters',
 		async (queryParams) => {
-			await agents.owner.get('/insights/by-workflow').query(queryParams).expect(500);
+			await agents.owner.get('/insights/by-workflow').query(queryParams).expect(400);
 		},
 	);