From b1a40a231bcd91ca2a4e842231ff23b3ae974d82 Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Thu, 9 Jan 2025 14:17:11 -0500 Subject: [PATCH] refactor(core): Update tag endpoints to use DTOs and injectable config (#12380) --- packages/@n8n/api-types/src/dto/index.ts | 3 + .../create-or-update-tag-request.dto.test.ts | 37 +++++++++++ .../__tests__/retrieve-tag-query.dto.test.ts | 64 +++++++++++++++++++ .../tag/create-or-update-tag-request.dto.ts | 6 ++ .../src/dto/tag/retrieve-tag-query.dto.ts | 7 ++ .../@n8n/config/src/configs/tags.config.ts | 10 +++ packages/@n8n/config/src/index.ts | 4 ++ packages/@n8n/config/test/config.test.ts | 3 + packages/cli/src/config/schema.ts | 7 -- .../cli/src/controllers/tags.controller.ts | 58 +++++++++-------- .../repositories/workflow.repository.ts | 3 +- .../handlers/workflows/workflows.handler.ts | 14 ++-- .../handlers/workflows/workflows.service.ts | 7 +- packages/cli/src/requests.ts | 11 ---- packages/cli/src/server.ts | 3 + packages/cli/src/services/frontend.service.ts | 2 +- .../src/workflow-execute-additional-data.ts | 3 +- .../cli/src/workflows/workflow.service.ts | 8 ++- .../cli/src/workflows/workflows.controller.ts | 10 +-- .../integration/public-api/workflows.test.ts | 28 +++----- .../instance-risk-reporter.test.ts | 1 + .../cli/test/integration/tags.api.test.ts | 58 ++++++++++++++++- .../workflows/workflow.service.test.ts | 1 + packages/cli/test/setup-test-folder.ts | 5 ++ packages/editor-ui/src/api/tags.ts | 25 ++++---- packages/editor-ui/src/stores/tags.store.ts | 7 +- 26 files changed, 282 insertions(+), 103 deletions(-) create mode 100644 packages/@n8n/api-types/src/dto/tag/__tests__/create-or-update-tag-request.dto.test.ts create mode 100644 packages/@n8n/api-types/src/dto/tag/__tests__/retrieve-tag-query.dto.test.ts create mode 100644 packages/@n8n/api-types/src/dto/tag/create-or-update-tag-request.dto.ts create mode 100644 packages/@n8n/api-types/src/dto/tag/retrieve-tag-query.dto.ts create mode 100644 packages/@n8n/config/src/configs/tags.config.ts diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index aa888f560d..9be09c02f3 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -44,3 +44,6 @@ export { CredentialsGetOneRequestQuery } from './credentials/credentials-get-one export { CredentialsGetManyRequestQuery } from './credentials/credentials-get-many-request.dto'; export { ImportWorkflowFromUrlDto } from './workflows/import-workflow-from-url.dto'; + +export { CreateOrUpdateTagRequestDto } from './tag/create-or-update-tag-request.dto'; +export { RetrieveTagQueryDto } from './tag/retrieve-tag-query.dto'; diff --git a/packages/@n8n/api-types/src/dto/tag/__tests__/create-or-update-tag-request.dto.test.ts b/packages/@n8n/api-types/src/dto/tag/__tests__/create-or-update-tag-request.dto.test.ts new file mode 100644 index 0000000000..0af3ec6b5c --- /dev/null +++ b/packages/@n8n/api-types/src/dto/tag/__tests__/create-or-update-tag-request.dto.test.ts @@ -0,0 +1,37 @@ +import { CreateOrUpdateTagRequestDto } from '../create-or-update-tag-request.dto'; + +describe('CreateOrUpdateTagRequestDto', () => { + describe('Valid requests', () => { + test.each([ + { + name: 'valid name', + request: { + name: 'tag-name', + }, + }, + ])('should validate $name', ({ request }) => { + const result = CreateOrUpdateTagRequestDto.safeParse(request); + expect(result.success).toBe(true); + }); + }); + + describe('Invalid requests', () => { + test.each([ + { + name: 'empty tag name', + request: { + name: '', + }, + expectedErrorPath: ['name'], + }, + ])('should fail validation for $name', ({ request, expectedErrorPath }) => { + const result = CreateOrUpdateTagRequestDto.safeParse(request); + + expect(result.success).toBe(false); + + if (expectedErrorPath) { + expect(result.error?.issues[0].path).toEqual(expectedErrorPath); + } + }); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/tag/__tests__/retrieve-tag-query.dto.test.ts b/packages/@n8n/api-types/src/dto/tag/__tests__/retrieve-tag-query.dto.test.ts new file mode 100644 index 0000000000..902bc62764 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/tag/__tests__/retrieve-tag-query.dto.test.ts @@ -0,0 +1,64 @@ +import { RetrieveTagQueryDto } from '../retrieve-tag-query.dto'; + +describe('RetrieveTagQueryDto', () => { + describe('Valid requests', () => { + test.each([ + { + name: 'with "true"', + request: { + withUsageCount: 'true', + }, + }, + { + name: 'with "false"', + request: { + withUsageCount: 'false', + }, + }, + ])('should pass validation for withUsageCount $name', ({ request }) => { + const result = RetrieveTagQueryDto.safeParse(request); + expect(result.success).toBe(true); + }); + }); + + describe('Invalid requests', () => { + test.each([ + { + name: 'with number', + request: { + withUsageCount: 1, + }, + expectedErrorPath: ['withUsageCount'], + }, + { + name: 'with boolean (true) ', + request: { + withUsageCount: true, + }, + expectedErrorPath: ['withUsageCount'], + }, + { + name: 'with boolean (false)', + request: { + withUsageCount: false, + }, + expectedErrorPath: ['withUsageCount'], + }, + { + name: 'with invalid string', + request: { + withUsageCount: 'invalid', + }, + expectedErrorPath: ['withUsageCount'], + }, + ])('should fail validation for withUsageCount $name', ({ request, expectedErrorPath }) => { + const result = RetrieveTagQueryDto.safeParse(request); + + expect(result.success).toBe(false); + + if (expectedErrorPath) { + expect(result.error?.issues[0].path).toEqual(expectedErrorPath); + } + }); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/tag/create-or-update-tag-request.dto.ts b/packages/@n8n/api-types/src/dto/tag/create-or-update-tag-request.dto.ts new file mode 100644 index 0000000000..f916e1665f --- /dev/null +++ b/packages/@n8n/api-types/src/dto/tag/create-or-update-tag-request.dto.ts @@ -0,0 +1,6 @@ +import { z } from 'zod'; +import { Z } from 'zod-class'; + +export class CreateOrUpdateTagRequestDto extends Z.class({ + name: z.string().trim().min(1), +}) {} diff --git a/packages/@n8n/api-types/src/dto/tag/retrieve-tag-query.dto.ts b/packages/@n8n/api-types/src/dto/tag/retrieve-tag-query.dto.ts new file mode 100644 index 0000000000..b7b53a0d6f --- /dev/null +++ b/packages/@n8n/api-types/src/dto/tag/retrieve-tag-query.dto.ts @@ -0,0 +1,7 @@ +import { Z } from 'zod-class'; + +import { booleanFromString } from '../../schemas/booleanFromString'; + +export class RetrieveTagQueryDto extends Z.class({ + withUsageCount: booleanFromString.optional().default('false'), +}) {} diff --git a/packages/@n8n/config/src/configs/tags.config.ts b/packages/@n8n/config/src/configs/tags.config.ts new file mode 100644 index 0000000000..1c6db71809 --- /dev/null +++ b/packages/@n8n/config/src/configs/tags.config.ts @@ -0,0 +1,10 @@ +import { Config, Env } from '../decorators'; + +@Config +export class TagsConfig { + /* + Disable workflow tags + */ + @Env('N8N_WORKFLOW_TAGS_DISABLED') + disabled: boolean = false; +} diff --git a/packages/@n8n/config/src/index.ts b/packages/@n8n/config/src/index.ts index 945b5f1237..f462ef9424 100644 --- a/packages/@n8n/config/src/index.ts +++ b/packages/@n8n/config/src/index.ts @@ -18,6 +18,7 @@ import { TaskRunnersConfig } from './configs/runners.config'; import { ScalingModeConfig } from './configs/scaling-mode.config'; import { SecurityConfig } from './configs/security.config'; import { SentryConfig } from './configs/sentry.config'; +import { TagsConfig } from './configs/tags.config'; import { TemplatesConfig } from './configs/templates.config'; import { UserManagementConfig } from './configs/user-management.config'; import { VersionNotificationsConfig } from './configs/version-notifications.config'; @@ -125,4 +126,7 @@ export class GlobalConfig { @Nested aiAssistant: AiAssistantConfig; + + @Nested + tags: TagsConfig; } diff --git a/packages/@n8n/config/test/config.test.ts b/packages/@n8n/config/test/config.test.ts index 2e63b8d10f..d9499d7849 100644 --- a/packages/@n8n/config/test/config.test.ts +++ b/packages/@n8n/config/test/config.test.ts @@ -295,6 +295,9 @@ describe('GlobalConfig', () => { aiAssistant: { baseUrl: '', }, + tags: { + disabled: false, + }, }; it('should use all default values when no env variables are defined', () => { diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index e5bda7d81b..c3a604faa4 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -139,13 +139,6 @@ export const schema = { doc: 'Public URL where the editor is accessible. Also used for emails sent from n8n.', }, - workflowTagsDisabled: { - format: Boolean, - default: false, - env: 'N8N_WORKFLOW_TAGS_DISABLED', - doc: 'Disable workflow tags.', - }, - userManagement: { jwtSecret: { doc: 'Set a specific JWT secret (optional - n8n can generate one)', // Generated @ start.ts diff --git a/packages/cli/src/controllers/tags.controller.ts b/packages/cli/src/controllers/tags.controller.ts index a6551a021d..52432d2d4b 100644 --- a/packages/cli/src/controllers/tags.controller.ts +++ b/packages/cli/src/controllers/tags.controller.ts @@ -1,54 +1,60 @@ -import { Request, Response, NextFunction } from 'express'; +import { CreateOrUpdateTagRequestDto, RetrieveTagQueryDto } from '@n8n/api-types'; +import { Response } from 'express'; -import config from '@/config'; -import { Delete, Get, Middleware, Patch, Post, RestController, GlobalScope } from '@/decorators'; -import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -import { TagsRequest } from '@/requests'; +import { + Delete, + Get, + Patch, + Post, + RestController, + GlobalScope, + Body, + Param, + Query, +} from '@/decorators'; +import { AuthenticatedRequest } from '@/requests'; import { TagService } from '@/services/tag.service'; @RestController('/tags') export class TagsController { - private config = config; - constructor(private readonly tagService: TagService) {} - // TODO: move this into a new decorator `@IfEnabled('workflowTagsDisabled')` - @Middleware() - workflowsEnabledMiddleware(_req: Request, _res: Response, next: NextFunction) { - if (this.config.getEnv('workflowTagsDisabled')) - throw new BadRequestError('Workflow tags are disabled'); - next(); - } - @Get('/') @GlobalScope('tag:list') - async getAll(req: TagsRequest.GetAll) { - return await this.tagService.getAll({ withUsageCount: req.query.withUsageCount === 'true' }); + async getAll(_req: AuthenticatedRequest, _res: Response, @Query query: RetrieveTagQueryDto) { + return await this.tagService.getAll({ withUsageCount: query.withUsageCount }); } @Post('/') @GlobalScope('tag:create') - async createTag(req: TagsRequest.Create) { - const tag = this.tagService.toEntity({ name: req.body.name }); + async createTag( + _req: AuthenticatedRequest, + _res: Response, + @Body payload: CreateOrUpdateTagRequestDto, + ) { + const { name } = payload; + const tag = this.tagService.toEntity({ name }); return await this.tagService.save(tag, 'create'); } @Patch('/:id(\\w+)') @GlobalScope('tag:update') - async updateTag(req: TagsRequest.Update) { - const newTag = this.tagService.toEntity({ id: req.params.id, name: req.body.name.trim() }); + async updateTag( + _req: AuthenticatedRequest, + _res: Response, + @Param('id') tagId: string, + @Body payload: CreateOrUpdateTagRequestDto, + ) { + const newTag = this.tagService.toEntity({ id: tagId, name: payload.name }); return await this.tagService.save(newTag, 'update'); } @Delete('/:id(\\w+)') @GlobalScope('tag:delete') - async deleteTag(req: TagsRequest.Delete) { - const { id } = req.params; - - await this.tagService.delete(id); - + async deleteTag(_req: AuthenticatedRequest, _res: Response, @Param('id') tagId: string) { + await this.tagService.delete(tagId); return true; } } diff --git a/packages/cli/src/databases/repositories/workflow.repository.ts b/packages/cli/src/databases/repositories/workflow.repository.ts index 7edabdde96..0952ae3cdc 100644 --- a/packages/cli/src/databases/repositories/workflow.repository.ts +++ b/packages/cli/src/databases/repositories/workflow.repository.ts @@ -12,7 +12,6 @@ import { type FindOptionsRelations, } from '@n8n/typeorm'; -import config from '@/config'; import type { ListQuery } from '@/requests'; import { isStringArray } from '@/utils'; @@ -132,7 +131,7 @@ export class WorkflowRepository extends Repository { const relations: string[] = []; - const areTagsEnabled = !config.getEnv('workflowTagsDisabled'); + const areTagsEnabled = !this.globalConfig.tags.disabled; const isDefaultSelect = options?.select === undefined; const areTagsRequested = isDefaultSelect || options?.select?.tags === true; const isOwnedByIncluded = isDefaultSelect || options?.select?.ownedBy === true; diff --git a/packages/cli/src/public-api/v1/handlers/workflows/workflows.handler.ts b/packages/cli/src/public-api/v1/handlers/workflows/workflows.handler.ts index b79ea4547b..77a07fea0f 100644 --- a/packages/cli/src/public-api/v1/handlers/workflows/workflows.handler.ts +++ b/packages/cli/src/public-api/v1/handlers/workflows/workflows.handler.ts @@ -1,14 +1,14 @@ +import { GlobalConfig } from '@n8n/config'; import { Container } from '@n8n/di'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import -import type { FindOptionsWhere } from '@n8n/typeorm'; -// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import { In, Like, QueryFailedError } from '@n8n/typeorm'; +// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import +import type { FindOptionsWhere } from '@n8n/typeorm'; import type express from 'express'; import { v4 as uuid } from 'uuid'; import { z } from 'zod'; import { ActiveWorkflowManager } from '@/active-workflow-manager'; -import config from '@/config'; import { WorkflowEntity } from '@/databases/entities/workflow-entity'; import { ProjectRepository } from '@/databases/repositories/project.repository'; import { SharedWorkflowRepository } from '@/databases/repositories/shared-workflow.repository'; @@ -111,7 +111,7 @@ export = { id, req.user, ['workflow:read'], - { includeTags: !config.getEnv('workflowTagsDisabled') }, + { includeTags: !Container.get(GlobalConfig).tags.disabled }, ); if (!workflow) { @@ -209,7 +209,7 @@ export = { skip: offset, take: limit, where, - ...(!config.getEnv('workflowTagsDisabled') && { relations: ['tags'] }), + ...(!Container.get(GlobalConfig).tags.disabled && { relations: ['tags'] }), }); if (excludePinnedData) { @@ -379,7 +379,7 @@ export = { async (req: WorkflowRequest.GetTags, res: express.Response): Promise => { const { id } = req.params; - if (config.getEnv('workflowTagsDisabled')) { + if (Container.get(GlobalConfig).tags.disabled) { return res.status(400).json({ message: 'Workflow Tags Disabled' }); } @@ -406,7 +406,7 @@ export = { const { id } = req.params; const newTags = req.body.map((newTag) => newTag.id); - if (config.getEnv('workflowTagsDisabled')) { + if (Container.get(GlobalConfig).tags.disabled) { return res.status(400).json({ message: 'Workflow Tags Disabled' }); } diff --git a/packages/cli/src/public-api/v1/handlers/workflows/workflows.service.ts b/packages/cli/src/public-api/v1/handlers/workflows/workflows.service.ts index 53e67d8b0d..131919d096 100644 --- a/packages/cli/src/public-api/v1/handlers/workflows/workflows.service.ts +++ b/packages/cli/src/public-api/v1/handlers/workflows/workflows.service.ts @@ -1,7 +1,7 @@ +import { GlobalConfig } from '@n8n/config'; import { Container } from '@n8n/di'; import type { Scope } from '@n8n/permissions'; -import config from '@/config'; import type { Project } from '@/databases/entities/project'; import { SharedWorkflow, type WorkflowSharingRole } from '@/databases/entities/shared-workflow'; import type { User } from '@/databases/entities/user'; @@ -46,7 +46,10 @@ export async function getSharedWorkflow( ...(!['global:owner', 'global:admin'].includes(user.role) && { userId: user.id }), ...(workflowId && { workflowId }), }, - relations: [...insertIf(!config.getEnv('workflowTagsDisabled'), ['workflow.tags']), 'workflow'], + relations: [ + ...insertIf(!Container.get(GlobalConfig).tags.disabled, ['workflow.tags']), + 'workflow', + ], }); } diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index db5aa1365a..b9a5ae97a3 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -264,17 +264,6 @@ export declare namespace OAuthRequest { } } -// ---------------------------------- -// /tags -// ---------------------------------- - -export declare namespace TagsRequest { - type GetAll = AuthenticatedRequest<{}, {}, {}, { withUsageCount: string }>; - type Create = AuthenticatedRequest<{}, {}, { name: string }>; - type Update = AuthenticatedRequest<{ id: string }, {}, { name: string }>; - type Delete = AuthenticatedRequest<{ id: string }>; -} - // ---------------------------------- // /annotation-tags // ---------------------------------- diff --git a/packages/cli/src/server.ts b/packages/cli/src/server.ts index 09e3774aae..17ffd66ad9 100644 --- a/packages/cli/src/server.ts +++ b/packages/cli/src/server.ts @@ -135,6 +135,9 @@ export class Server extends AbstractServer { await import('@/controllers/cta.controller'); } + if (!this.globalConfig.tags.disabled) { + await import('@/controllers/tags.controller'); + } // ---------------------------------------- // SAML // ---------------------------------------- diff --git a/packages/cli/src/services/frontend.service.ts b/packages/cli/src/services/frontend.service.ts index 099ae2c935..33da795f27 100644 --- a/packages/cli/src/services/frontend.service.ts +++ b/packages/cli/src/services/frontend.service.ts @@ -154,7 +154,7 @@ export class FrontendService { enabled: !this.globalConfig.publicApi.swaggerUiDisabled, }, }, - workflowTagsDisabled: config.getEnv('workflowTagsDisabled'), + workflowTagsDisabled: this.globalConfig.tags.disabled, logLevel: this.globalConfig.logging.level, hiringBannerEnabled: config.getEnv('hiringBanner.enabled'), aiAssistant: { diff --git a/packages/cli/src/workflow-execute-additional-data.ts b/packages/cli/src/workflow-execute-additional-data.ts index e350086f9d..f559765ec3 100644 --- a/packages/cli/src/workflow-execute-additional-data.ts +++ b/packages/cli/src/workflow-execute-additional-data.ts @@ -42,7 +42,6 @@ import type { } from 'n8n-workflow'; import { ActiveExecutions } from '@/active-executions'; -import config from '@/config'; import { CredentialsHelper } from '@/credentials-helper'; import { ExecutionRepository } from '@/databases/repositories/execution.repository'; import type { AiEventMap, AiEventPayload } from '@/events/maps/ai.event-map'; @@ -734,7 +733,7 @@ export async function getWorkflowData( let workflowData: IWorkflowBase | null; if (workflowInfo.id !== undefined) { - const relations = config.getEnv('workflowTagsDisabled') ? [] : ['tags']; + const relations = Container.get(GlobalConfig).tags.disabled ? [] : ['tags']; workflowData = await Container.get(WorkflowRepository).get( { id: workflowInfo.id }, diff --git a/packages/cli/src/workflows/workflow.service.ts b/packages/cli/src/workflows/workflow.service.ts index 2141e79ed5..6460348b23 100644 --- a/packages/cli/src/workflows/workflow.service.ts +++ b/packages/cli/src/workflows/workflow.service.ts @@ -1,3 +1,4 @@ +import { GlobalConfig } from '@n8n/config'; import { Service } from '@n8n/di'; import type { Scope } from '@n8n/permissions'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import @@ -54,6 +55,7 @@ export class WorkflowService { private readonly projectService: ProjectService, private readonly executionRepository: ExecutionRepository, private readonly eventService: EventService, + private readonly globalConfig: GlobalConfig, ) {} async getMany(user: User, options?: ListQuery.Options, includeScopes?: boolean) { @@ -202,7 +204,9 @@ export class WorkflowService { ]), ); - if (tagIds && !config.getEnv('workflowTagsDisabled')) { + const tagsDisabled = this.globalConfig.tags.disabled; + + if (tagIds && !tagsDisabled) { await this.workflowTagMappingRepository.overwriteTaggings(workflowId, tagIds); } @@ -210,7 +214,7 @@ export class WorkflowService { await this.workflowHistoryService.saveVersion(user, workflowUpdateData, workflowId); } - const relations = config.getEnv('workflowTagsDisabled') ? [] : ['tags']; + const relations = tagsDisabled ? [] : ['tags']; // We sadly get nothing back from "update". Neither if it updated a record // nor the new value. So query now the hopefully updated entry. diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index 865b38450e..2716b8c4b2 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -89,7 +89,7 @@ export class WorkflowsController { const { tags: tagIds } = req.body; - if (tagIds?.length && !config.getEnv('workflowTagsDisabled')) { + if (tagIds?.length && !this.globalConfig.tags.disabled) { newWorkflow.tags = await this.tagRepository.findMany(tagIds); } @@ -164,7 +164,7 @@ export class WorkflowsController { await this.workflowHistoryService.saveVersion(req.user, savedWorkflow, savedWorkflow.id); - if (tagIds && !config.getEnv('workflowTagsDisabled') && savedWorkflow.tags) { + if (tagIds && !this.globalConfig.tags.disabled && savedWorkflow.tags) { savedWorkflow.tags = this.tagService.sortByRequestOrder(savedWorkflow.tags, { requestOrder: tagIds, }); @@ -260,7 +260,7 @@ export class WorkflowsController { }, }; - if (!config.getEnv('workflowTagsDisabled')) { + if (!this.globalConfig.tags.disabled) { relations.tags = true; } @@ -268,7 +268,7 @@ export class WorkflowsController { workflowId, req.user, ['workflow:read'], - { includeTags: !config.getEnv('workflowTagsDisabled') }, + { includeTags: !this.globalConfig.tags.disabled }, ); if (!workflow) { @@ -296,7 +296,7 @@ export class WorkflowsController { workflowId, req.user, ['workflow:read'], - { includeTags: !config.getEnv('workflowTagsDisabled') }, + { includeTags: !this.globalConfig.tags.disabled }, ); if (!workflow) { diff --git a/packages/cli/test/integration/public-api/workflows.test.ts b/packages/cli/test/integration/public-api/workflows.test.ts index 3a1a9a77bc..7a1fa9591d 100644 --- a/packages/cli/test/integration/public-api/workflows.test.ts +++ b/packages/cli/test/integration/public-api/workflows.test.ts @@ -1,8 +1,8 @@ +import { GlobalConfig } from '@n8n/config'; import { Container } from '@n8n/di'; import type { INode } from 'n8n-workflow'; import { ActiveWorkflowManager } from '@/active-workflow-manager'; -import config from '@/config'; import { STARTING_NODES } from '@/constants'; import type { Project } from '@/databases/entities/project'; import type { TagEntity } from '@/databases/entities/tag-entity'; @@ -36,6 +36,8 @@ let activeWorkflowManager: ActiveWorkflowManager; const testServer = utils.setupTestServer({ endpointGroups: ['publicApi'] }); const license = testServer.license; +const globalConfig = Container.get(GlobalConfig); + mockInstance(ExecutionService); beforeAll(async () => { @@ -69,6 +71,8 @@ beforeEach(async () => { authOwnerAgent = testServer.publicApiAgentFor(owner); authMemberAgent = testServer.publicApiAgentFor(member); + + globalConfig.tags.disabled = false; }); afterEach(async () => { @@ -1287,8 +1291,8 @@ describe('GET /workflows/:id/tags', () => { test('should fail due to invalid API Key', testWithAPIKey('get', '/workflows/2/tags', 'abcXYZ')); - test('should fail if workflowTagsDisabled', async () => { - config.set('workflowTagsDisabled', true); + test('should fail if N8N_WORKFLOW_TAGS_DISABLED', async () => { + globalConfig.tags.disabled = true; const response = await authOwnerAgent.get('/workflows/2/tags'); @@ -1297,16 +1301,12 @@ describe('GET /workflows/:id/tags', () => { }); test('should fail due to non-existing workflow', async () => { - config.set('workflowTagsDisabled', false); - const response = await authOwnerAgent.get('/workflows/2/tags'); expect(response.statusCode).toBe(404); }); test('should return all tags of owned workflow', async () => { - config.set('workflowTagsDisabled', false); - const tags = await Promise.all([await createTag({}), await createTag({})]); const workflow = await createWorkflow({ tags }, member); @@ -1331,8 +1331,6 @@ describe('GET /workflows/:id/tags', () => { }); test('should return empty array if workflow does not have tags', async () => { - config.set('workflowTagsDisabled', false); - const workflow = await createWorkflow({}, member); const response = await authMemberAgent.get(`/workflows/${workflow.id}/tags`); @@ -1347,8 +1345,8 @@ describe('PUT /workflows/:id/tags', () => { test('should fail due to invalid API Key', testWithAPIKey('put', '/workflows/2/tags', 'abcXYZ')); - test('should fail if workflowTagsDisabled', async () => { - config.set('workflowTagsDisabled', true); + test('should fail if N8N_WORKFLOW_TAGS_DISABLED', async () => { + globalConfig.tags.disabled = true; const response = await authOwnerAgent.put('/workflows/2/tags').send([]); @@ -1357,16 +1355,12 @@ describe('PUT /workflows/:id/tags', () => { }); test('should fail due to non-existing workflow', async () => { - config.set('workflowTagsDisabled', false); - const response = await authOwnerAgent.put('/workflows/2/tags').send([]); expect(response.statusCode).toBe(404); }); test('should add the tags, workflow have not got tags previously', async () => { - config.set('workflowTagsDisabled', false); - const workflow = await createWorkflow({}, member); const tags = await Promise.all([await createTag({}), await createTag({})]); @@ -1425,8 +1419,6 @@ describe('PUT /workflows/:id/tags', () => { }); test('should add the tags, workflow have some tags previously', async () => { - config.set('workflowTagsDisabled', false); - const tags = await Promise.all([await createTag({}), await createTag({}), await createTag({})]); const oldTags = [tags[0], tags[1]]; const newTags = [tags[0], tags[2]]; @@ -1513,8 +1505,6 @@ describe('PUT /workflows/:id/tags', () => { }); test('should fail to add the tags as one does not exist, workflow should maintain previous tags', async () => { - config.set('workflowTagsDisabled', false); - const tags = await Promise.all([await createTag({}), await createTag({})]); const oldTags = [tags[0], tags[1]]; const workflow = await createWorkflow({ tags: oldTags }, member); diff --git a/packages/cli/test/integration/security-audit/instance-risk-reporter.test.ts b/packages/cli/test/integration/security-audit/instance-risk-reporter.test.ts index ddf3fce556..05a6b2c31f 100644 --- a/packages/cli/test/integration/security-audit/instance-risk-reporter.test.ts +++ b/packages/cli/test/integration/security-audit/instance-risk-reporter.test.ts @@ -240,6 +240,7 @@ test('should not report outdated instance when up to date', async () => { test('should report security settings', async () => { Container.get(GlobalConfig).diagnostics.enabled = true; + const testAudit = await securityAuditService.run(['instance']); const section = getRiskSection( diff --git a/packages/cli/test/integration/tags.api.test.ts b/packages/cli/test/integration/tags.api.test.ts index 5d9a724f78..90d764f534 100644 --- a/packages/cli/test/integration/tags.api.test.ts +++ b/packages/cli/test/integration/tags.api.test.ts @@ -8,6 +8,7 @@ import type { SuperAgentTest } from './shared/types'; import * as utils from './shared/utils/'; let authOwnerAgent: SuperAgentTest; + const testServer = utils.setupTestServer({ endpointGroups: ['tags'] }); beforeAll(async () => { @@ -22,8 +23,8 @@ beforeEach(async () => { describe('POST /tags', () => { test('should create tag', async () => { const resp = await authOwnerAgent.post('/tags').send({ name: 'test' }); - expect(resp.statusCode).toBe(200); + expect(resp.statusCode).toBe(200); const dbTag = await Container.get(TagRepository).findBy({ name: 'test' }); expect(dbTag.length === 1); }); @@ -38,4 +39,59 @@ describe('POST /tags', () => { const dbTag = await Container.get(TagRepository).findBy({ name: 'test' }); expect(dbTag.length).toBe(1); }); + + test('should delete tag', async () => { + const newTag = Container.get(TagRepository).create({ name: 'test' }); + await Container.get(TagRepository).save(newTag); + + const resp = await authOwnerAgent.delete(`/tags/${newTag.id}`); + expect(resp.status).toBe(200); + + const dbTag = await Container.get(TagRepository).findBy({ name: 'test' }); + expect(dbTag.length).toBe(0); + }); + + test('should update tag name', async () => { + const newTag = Container.get(TagRepository).create({ name: 'test' }); + await Container.get(TagRepository).save(newTag); + + const resp = await authOwnerAgent.patch(`/tags/${newTag.id}`).send({ name: 'updated' }); + expect(resp.status).toBe(200); + + const dbTag = await Container.get(TagRepository).findBy({ name: 'updated' }); + expect(dbTag.length).toBe(1); + }); + + test('should retrieve all tags', async () => { + const newTag = Container.get(TagRepository).create({ name: 'test' }); + const savedTag = await Container.get(TagRepository).save(newTag); + + const resp = await authOwnerAgent.get('/tags'); + expect(resp.status).toBe(200); + + expect(resp.body.data.length).toBe(1); + expect(resp.body.data[0]).toMatchObject({ + id: savedTag.id, + name: savedTag.name, + createdAt: savedTag.createdAt.toISOString(), + updatedAt: savedTag.updatedAt.toISOString(), + }); + }); + + test('should retrieve all tags with with usage count', async () => { + const newTag = Container.get(TagRepository).create({ name: 'test' }); + const savedTag = await Container.get(TagRepository).save(newTag); + + const resp = await authOwnerAgent.get('/tags').query({ withUsageCount: 'true' }); + expect(resp.status).toBe(200); + + expect(resp.body.data.length).toBe(1); + expect(resp.body.data[0]).toMatchObject({ + id: savedTag.id, + name: savedTag.name, + createdAt: savedTag.createdAt.toISOString(), + updatedAt: savedTag.updatedAt.toISOString(), + usageCount: 0, + }); + }); }); diff --git a/packages/cli/test/integration/workflows/workflow.service.test.ts b/packages/cli/test/integration/workflows/workflow.service.test.ts index 8e2c76c981..b7d9c033b4 100644 --- a/packages/cli/test/integration/workflows/workflow.service.test.ts +++ b/packages/cli/test/integration/workflows/workflow.service.test.ts @@ -40,6 +40,7 @@ beforeAll(async () => { mock(), mock(), mock(), + mock(), ); }); diff --git a/packages/cli/test/setup-test-folder.ts b/packages/cli/test/setup-test-folder.ts index 8a58c48f86..80b9953333 100644 --- a/packages/cli/test/setup-test-folder.ts +++ b/packages/cli/test/setup-test-folder.ts @@ -20,3 +20,8 @@ writeFileSync( mode: 0o600, }, ); + +// This is needed to ensure that `process.env` overrides in tests +// are set before any of the config classes are instantiated. +// TODO: delete this after we are done migrating everything to config classes +import '@/config'; diff --git a/packages/editor-ui/src/api/tags.ts b/packages/editor-ui/src/api/tags.ts index 0e429b6433..d85f84cb5d 100644 --- a/packages/editor-ui/src/api/tags.ts +++ b/packages/editor-ui/src/api/tags.ts @@ -1,29 +1,26 @@ import type { IRestApiContext, ITag } from '@/Interface'; import { makeRestApiRequest } from '@/utils/apiUtils'; +import type { CreateOrUpdateTagRequestDto, RetrieveTagQueryDto } from '@n8n/api-types'; type TagsApiEndpoint = '/tags' | '/annotation-tags'; -export interface ITagsApi { - getTags: (context: IRestApiContext, withUsageCount?: boolean) => Promise; - createTag: (context: IRestApiContext, params: { name: string }) => Promise; - updateTag: (context: IRestApiContext, id: string, params: { name: string }) => Promise; - deleteTag: (context: IRestApiContext, id: string) => Promise; -} - -export function createTagsApi(endpoint: TagsApiEndpoint): ITagsApi { +export function createTagsApi(endpoint: TagsApiEndpoint) { return { - getTags: async (context: IRestApiContext, withUsageCount = false): Promise => { - return await makeRestApiRequest(context, 'GET', endpoint, { withUsageCount }); + getTags: async (context: IRestApiContext, data: RetrieveTagQueryDto): Promise => { + return await makeRestApiRequest(context, 'GET', endpoint, data); }, - createTag: async (context: IRestApiContext, params: { name: string }): Promise => { - return await makeRestApiRequest(context, 'POST', endpoint, params); + createTag: async ( + context: IRestApiContext, + data: CreateOrUpdateTagRequestDto, + ): Promise => { + return await makeRestApiRequest(context, 'POST', endpoint, data); }, updateTag: async ( context: IRestApiContext, id: string, - params: { name: string }, + data: CreateOrUpdateTagRequestDto, ): Promise => { - return await makeRestApiRequest(context, 'PATCH', `${endpoint}/${id}`, params); + return await makeRestApiRequest(context, 'PATCH', `${endpoint}/${id}`, data); }, deleteTag: async (context: IRestApiContext, id: string): Promise => { return await makeRestApiRequest(context, 'DELETE', `${endpoint}/${id}`); diff --git a/packages/editor-ui/src/stores/tags.store.ts b/packages/editor-ui/src/stores/tags.store.ts index fade74a39c..00272d96bd 100644 --- a/packages/editor-ui/src/stores/tags.store.ts +++ b/packages/editor-ui/src/stores/tags.store.ts @@ -80,10 +80,9 @@ const createTagsStore = (id: STORES.TAGS | STORES.ANNOTATION_TAGS) => { } loading.value = true; - const retrievedTags = await tagsApi.getTags( - rootStore.restApiContext, - Boolean(withUsageCount), - ); + const retrievedTags = await tagsApi.getTags(rootStore.restApiContext, { + withUsageCount, + }); setAllTags(retrievedTags); loading.value = false; return retrievedTags;