feat(API): Add project and projectId fields to get and update a variable project (#20544)

This commit is contained in:
Guillaume Jacquart 2025-10-09 15:33:26 +02:00 committed by GitHub
parent 4faf3f3e04
commit 5bddbedf2e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 195 additions and 24 deletions

View File

@ -261,7 +261,12 @@ export class VariablesService {
await this.variablesRepository.update(id, {
key: variable.key,
value: variable.value,
project: variable.projectId ? { id: variable.projectId } : null,
// Only update the project if it was explicitly set in the update
// If project id is undefined, keep the existing
// If project id is null, move to global (no project)
...(typeof variable.projectId !== 'undefined'
? { project: variable.projectId ? { id: variable.projectId } : null }
: {}),
});
await this.updateCache();
return (await this.getCached(id))!;

View File

@ -29,7 +29,7 @@ put:
content:
application/json:
schema:
$ref: '../schemas/variable.yml'
$ref: '../schemas/variable.create.yml'
required: true
responses:
'204':

View File

@ -10,7 +10,7 @@ post:
content:
application/json:
schema:
$ref: '../schemas/variable.yml'
$ref: '../schemas/variable.create.yml'
required: true
responses:
'201':
@ -29,6 +29,20 @@ get:
parameters:
- $ref: '../../../../shared/spec/parameters/limit.yml'
- $ref: '../../../../shared/spec/parameters/cursor.yml'
- name: projectId
in: query
required: false
explode: false
allowReserved: true
schema:
type: string
example: VmwOO9HeTEj20kxM
- name: state
in: query
required: false
schema:
type: string
enum: [empty]
responses:
'200':
description: Operation successful.

View File

@ -0,0 +1,21 @@
type: object
additionalProperties: false
required:
- key
- value
properties:
id:
type: string
readOnly: true
key:
type: string
value:
type: string
example: test
type:
type: string
readOnly: true
projectId:
type: string
example: VmwOO9HeTEj20kxM
nullable: true

View File

@ -15,3 +15,5 @@ properties:
type:
type: string
readOnly: true
project:
$ref: '../../../projects/spec/schemas/project.yml'

View File

@ -2,6 +2,8 @@ import { CreateVariableRequestDto } from '@n8n/api-types';
import type { AuthenticatedRequest } from '@n8n/db';
import { VariablesRepository } from '@n8n/db';
import { Container } from '@n8n/di';
// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
import { IsNull } from '@n8n/typeorm';
import type { Response } from 'express';
import {
@ -12,12 +14,8 @@ import {
import { encodeNextCursor } from '../../shared/services/pagination.service';
import { VariablesController } from '@/environments.ee/variables/variables.controller.ee';
import type { PaginatedRequest } from '@/public-api/types';
import type { VariablesRequest } from '@/requests';
type Delete = VariablesRequest.Delete;
type GetAll = PaginatedRequest;
export = {
createVariable: [
isLicensed('feat:variables'),
@ -48,7 +46,7 @@ export = {
deleteVariable: [
isLicensed('feat:variables'),
apiKeyHasScopeWithGlobalScopeFallback({ scope: 'variable:delete' }),
async (req: Delete, res: Response) => {
async (req: AuthenticatedRequest<{ id: string }>, res: Response) => {
await Container.get(VariablesController).deleteVariable(req);
return res.status(204).send();
@ -58,12 +56,17 @@ export = {
isLicensed('feat:variables'),
apiKeyHasScopeWithGlobalScopeFallback({ scope: 'variable:list' }),
validCursor,
async (req: GetAll, res: Response) => {
const { offset = 0, limit = 100 } = req.query;
async (req: VariablesRequest.GetAll, res: Response) => {
const { offset = 0, limit = 100, projectId, state } = req.query;
const [variables, count] = await Container.get(VariablesRepository).findAndCount({
skip: offset,
take: limit,
where: {
project: projectId === 'null' ? IsNull() : { id: projectId },
value: state === 'empty' ? '' : undefined,
},
relations: ['project'],
});
return res.json({

View File

@ -235,7 +235,19 @@ export declare namespace LicenseRequest {
export declare namespace VariablesRequest {
type CreateUpdatePayload = Omit<Variables, 'id'> & { id?: unknown };
type GetAll = AuthenticatedRequest;
type GetAll = AuthenticatedRequest<
{},
{},
{},
{
limit?: number;
cursor?: string;
offset?: number;
lastId?: string;
projectId?: string;
state?: 'empty';
}
>;
type Get = AuthenticatedRequest<{ id: string }, {}, {}, {}>;
type Create = AuthenticatedRequest<{}, {}, CreateUpdatePayload, {}>;
type Update = AuthenticatedRequest<{ id: string }, {}, CreateUpdatePayload, {}>;

View File

@ -1,13 +1,18 @@
import { testDb } from '@n8n/backend-test-utils';
import type { User, Variables } from '@n8n/db';
import { createTeamProject, testDb } from '@n8n/backend-test-utils';
import type { Project, User, Variables } from '@n8n/db';
import { createOwnerWithApiKey } from '@test-integration/db/users';
import {
createProjectVariable,
createVariable,
getVariableByIdOrFail,
} from '@test-integration/db/variables';
import { setupTestServer } from '@test-integration/utils';
import { FeatureNotLicensedError } from '@/errors/feature-not-licensed.error';
import { createOwnerWithApiKey } from '@test-integration/db/users';
import { createVariable, getVariableByIdOrFail } from '@test-integration/db/variables';
import { setupTestServer } from '@test-integration/utils';
describe('Variables in Public API', () => {
let owner: User;
let project: Project;
const testServer = setupTestServer({ endpointGroups: ['publicApi'] });
const licenseErrorMessage = new FeatureNotLicensedError('feat:variables').message;
@ -19,6 +24,7 @@ describe('Variables in Public API', () => {
await testDb.truncate(['Variables', 'User']);
owner = await createOwnerWithApiKey();
project = await createTeamProject();
});
describe('GET /variables', () => {
@ -27,7 +33,12 @@ describe('Variables in Public API', () => {
* Arrange
*/
testServer.license.enable('feat:variables');
const variables = await Promise.all([createVariable(), createVariable(), createVariable()]);
const variables = await Promise.all([
createVariable(),
createVariable(),
createVariable(),
createProjectVariable('projectKey', 'projectValue', project),
]);
/**
* Act
@ -43,11 +54,55 @@ describe('Variables in Public API', () => {
expect(Array.isArray(response.body.data)).toBe(true);
expect(response.body.data.length).toBe(variables.length);
variables.forEach(({ id, key, value }) => {
variables.forEach(({ id, key, value, project }) => {
expect(response.body.data).toContainEqual(expect.objectContaining({ id, key, value }));
if (project) {
const projectResponse = response.body.data.find((v: Variables) => v.id === id).project;
expect(projectResponse).toBeDefined();
expect(projectResponse).toEqual(
expect.objectContaining({ id: project.id, name: project.name }),
);
}
});
});
it('if licensed, should be able to filter variables by projectId and state', async () => {
/**
* Arrange
*/
testServer.license.enable('feat:variables');
await Promise.all([
createVariable(),
createProjectVariable('projectKey', 'projectValue', project),
createProjectVariable('emptyVar', '', project),
createVariable('emptyVar', ''),
]);
/**
* Act
*/
const response = await testServer
.publicApiAgentFor(owner)
.get('/variables')
.query({ projectId: project.id, state: 'empty' });
/**
* Assert
*/
expect(response.status).toBe(200);
expect(response.body).toHaveProperty('data');
expect(response.body).toHaveProperty('nextCursor');
expect(Array.isArray(response.body.data)).toBe(true);
expect(response.body.data.length).toBe(1);
expect(response.body.data[0]).toEqual(
expect.objectContaining({
key: 'emptyVar',
value: '',
project: expect.objectContaining({ id: project.id }),
}),
);
});
it('if not licensed, should reject', async () => {
/**
* Act
@ -87,6 +142,34 @@ describe('Variables in Public API', () => {
);
});
it('if licensed, should create a variable linked to a project', async () => {
/**
* Arrange
*/
testServer.license.enable('feat:variables');
const variablePayload = { key: 'key', value: 'value', projectId: project.id };
/**
* Act
*/
const response = await testServer
.publicApiAgentFor(owner)
.post('/variables')
.send(variablePayload);
/**
* Assert
*/
expect(response.status).toBe(201);
await expect(getVariableByIdOrFail(response.body.id)).resolves.toEqual(
expect.objectContaining({
key: 'key',
value: 'value',
project: expect.objectContaining({ id: project.id }),
}),
);
});
it('if not licensed, should reject', async () => {
/**
* Arrange
@ -129,6 +212,24 @@ describe('Variables in Public API', () => {
expect(updatedVariable).toEqual(expect.objectContaining(variablePayload));
});
it('if licensed, should update a variable to link it to a project', async () => {
testServer.license.enable('feat:variables');
const response = await testServer
.publicApiAgentFor(owner)
.put(`/variables/${variable.id}`)
.send({ ...variablePayload, projectId: project.id });
expect(response.status).toBe(204);
const updatedVariable = await getVariableByIdOrFail(variable.id);
expect(updatedVariable).toEqual(
expect.objectContaining({
...variablePayload,
project: expect.objectContaining({ id: project.id }),
}),
);
});
it('if not licensed, should reject', async () => {
const response = await testServer
.publicApiAgentFor(owner)

View File

@ -32,7 +32,10 @@ export async function createProjectVariable(
}
export async function getVariableByIdOrFail(id: string) {
return await Container.get(VariablesRepository).findOneOrFail({ where: { id } });
return await Container.get(VariablesRepository).findOneOrFail({
where: { id },
relations: ['project'],
});
}
export async function getVariableByKey(key: string) {

View File

@ -1,9 +1,14 @@
import { testDb } from '@n8n/backend-test-utils';
import type { Variables } from '@n8n/db';
import { createTeamProject, linkUserToProject, testDb } from '@n8n/backend-test-utils';
import type { Project, Variables } from '@n8n/db';
import { Container } from '@n8n/di';
import { CacheService } from '@/services/cache/cache.service';
import { createVariable, getVariableById, getVariableByKey } from '@test-integration/db/variables';
import {
createProjectVariable,
createVariable,
getVariableById,
getVariableByKey,
} from '@test-integration/db/variables';
import { createOwner, createUser } from './shared/db/users';
import type { SuperAgentTest } from './shared/types';
@ -11,6 +16,7 @@ import * as utils from './shared/utils/';
let authOwnerAgent: SuperAgentTest;
let authMemberAgent: SuperAgentTest;
let project: Project;
const testServer = utils.setupTestServer({ endpointGroups: ['variables'] });
const license = testServer.license;
@ -21,6 +27,9 @@ beforeAll(async () => {
const member = await createUser();
authMemberAgent = testServer.authAgentFor(member);
project = await createTeamProject();
await linkUserToProject(member, project, 'project:editor');
license.setDefaults({
features: ['feat:variables'],
// quota: {
@ -42,6 +51,7 @@ describe('GET /variables', () => {
createVariable('test1', 'value1'),
createVariable('test2', 'value2'),
createVariable('empty', ''),
createProjectVariable('testProject1', 'projectValue1', project),
]);
});
@ -57,13 +67,13 @@ describe('GET /variables', () => {
test('should return all variables for an owner', async () => {
const response = await authOwnerAgent.get('/variables');
expect(response.statusCode).toBe(200);
expect(response.body.data.length).toBe(3);
expect(response.body.data.length).toBe(4);
});
test('should return all variables for a member', async () => {
const response = await authMemberAgent.get('/variables');
expect(response.statusCode).toBe(200);
expect(response.body.data.length).toBe(3);
expect(response.body.data.length).toBe(4);
});
describe('state:empty', () => {