diff --git a/frontend/src/interfaces/uiConfig.ts b/frontend/src/interfaces/uiConfig.ts index 5f7ded0b20..a622aed5e2 100644 --- a/frontend/src/interfaces/uiConfig.ts +++ b/frontend/src/interfaces/uiConfig.ts @@ -91,6 +91,7 @@ export type UiFlags = { healthToTechDebt?: boolean; improvedJsonDiff?: boolean; impactMetrics?: boolean; + crDiffView?: boolean; changeRequestApproverEmails?: boolean; }; diff --git a/frontend/yarn.lock b/frontend/yarn.lock index 496fc31597..ed16b80d16 100644 --- a/frontend/yarn.lock +++ b/frontend/yarn.lock @@ -3209,20 +3209,20 @@ __metadata: linkType: hard "@types/node@npm:*": - version: 24.0.3 - resolution: "@types/node@npm:24.0.3" + version: 24.0.4 + resolution: "@types/node@npm:24.0.4" dependencies: undici-types: "npm:~7.8.0" - checksum: 10c0/9c3c4e87600d1cf11e291c2fd4bfd806a615455463c30a0ef6dc9c801b3423344d9b82b8084e3ccabce485a7421ebb61a66e9676181bd7d9aea4759998a120d5 + checksum: 10c0/590e8cb0ec59fb9cd566402120e690d87ecbdf57f1ee2b8493266121ed33aa4b25949a0c6156b84a6ffb9250baaf1f80e9af142da542ed603e6ee73fc4d1115f languageName: node linkType: hard "@types/node@npm:^22.0.0": - version: 22.15.32 - resolution: "@types/node@npm:22.15.32" + version: 22.15.33 + resolution: "@types/node@npm:22.15.33" dependencies: undici-types: "npm:~6.21.0" - checksum: 10c0/63a2fa52adf1134d1b3bee8b1862d4b8e4550fffc190551068d3d41a41d9e5c0c8f1cb81faa18767b260637360f662115c26c5e4e7718868ead40c4a57cbc0e3 + checksum: 10c0/ee040c29c891aa37fffc27d04a8529318c391356346933646b7692eaf62236831ad532f6ebaf43ebd6a2ef1f0f091860d8a0a83a4e3c5a4f66d37aa1b2c99f31 languageName: node linkType: hard diff --git a/package.json b/package.json index 198d041e49..bf8242be07 100644 --- a/package.json +++ b/package.json @@ -130,7 +130,7 @@ "type-is": "^2.0.0", "ulidx": "^2.4.1", "unleash-client": "^6.7.0-beta.0", - "uuid": "^9.0.0" + "uuid": "^11.0.0" }, "devDependencies": { "@apidevtools/swagger-parser": "10.1.1", diff --git a/src/lib/db/transaction.test.ts b/src/lib/db/transaction.test.ts new file mode 100644 index 0000000000..88172b449e --- /dev/null +++ b/src/lib/db/transaction.test.ts @@ -0,0 +1,273 @@ +import { + withTransactional, + withRollbackTransaction, + withFakeTransactional, + inTransaction, + type TransactionUserParams, +} from './transaction.js'; +import { type Mock, vi } from 'vitest'; + +interface MockService { + getData: () => string; + saveData: (data: string) => Promise; +} + +describe('transaction utilities', () => { + let mockDb: any; + let mockTransaction: any; + let mockServiceFactory: Mock; + let mockService: MockService; + + beforeEach(() => { + mockTransaction = { + commit: vi.fn(), + rollback: vi.fn(), + isTransaction: true, + select: vi.fn(), + insert: vi.fn(), + update: vi.fn(), + userParams: undefined, + }; + + mockDb = { + transaction: vi + .fn() + .mockImplementation((callback) => callback(mockTransaction)), + isTransaction: false, + }; + + mockService = { + getData: vi.fn().mockReturnValue('test-data'), + saveData: vi.fn().mockResolvedValue(undefined), + }; + + mockServiceFactory = vi.fn().mockReturnValue(mockService); + }); + + describe('withTransactional', () => { + it('should add transactional method to service', () => { + const serviceWithTransactional = withTransactional( + mockServiceFactory, + mockDb, + ); + + expect(typeof serviceWithTransactional.transactional).toBe( + 'function', + ); + expect(serviceWithTransactional.getData).toBe(mockService.getData); + expect(serviceWithTransactional.saveData).toBe( + mockService.saveData, + ); + }); + + it('should execute callback within database transaction', async () => { + const serviceWithTransactional = withTransactional( + mockServiceFactory, + mockDb, + ); + + const result = await serviceWithTransactional.transactional( + (service) => { + return service.getData(); + }, + ); + + expect(mockDb.transaction).toHaveBeenCalledTimes(1); + expect(result).toBe('test-data'); + }); + + it('should set default userParams when no transactionContext provided', async () => { + const serviceWithTransactional = withTransactional( + mockServiceFactory, + mockDb, + ); + + await serviceWithTransactional.transactional((service) => { + return service.getData(); + }); + + expect(mockTransaction.userParams).toBeDefined(); + expect(mockTransaction.userParams.type).toBe('transaction'); + expect(mockTransaction.userParams.id).toBeDefined(); + expect(typeof mockTransaction.userParams.id).toBe('string'); + }); + + it('should use provided transactionContext when given', async () => { + const serviceWithTransactional = withTransactional( + mockServiceFactory, + mockDb, + ); + const customContext: TransactionUserParams = { + type: 'change-request', + id: '01HQVX5K8P9EXAMPLE123456', + }; + + await serviceWithTransactional.transactional((service) => { + return service.getData(); + }, customContext); + + expect(mockTransaction.userParams).toEqual(customContext); + expect(mockTransaction.userParams.type).toBe('change-request'); + expect(mockTransaction.userParams.id).toBe( + '01HQVX5K8P9EXAMPLE123456', + ); + }); + + it('should generate unique ULID strings for default context', async () => { + const serviceWithTransactional = withTransactional( + mockServiceFactory, + mockDb, + ); + const userParamsIds: string[] = []; + + for (let i = 0; i < 3; i++) { + await serviceWithTransactional.transactional((service) => { + userParamsIds.push(mockTransaction.userParams.id); + return service.getData(); + }); + } + + expect(userParamsIds).toHaveLength(3); + expect(userParamsIds.every((id) => typeof id === 'string')).toBe( + true, + ); + expect(new Set(userParamsIds).size).toBe(3); + userParamsIds.forEach((id) => { + expect(id).toMatch(/^[0-9A-HJKMNP-TV-Z]{26}$/); + }); + }); + + it('should create transactional service with transaction instance', async () => { + const serviceWithTransactional = withTransactional( + mockServiceFactory, + mockDb, + ); + + await serviceWithTransactional.transactional((service) => { + return service.getData(); + }); + + expect(mockServiceFactory).toHaveBeenCalledWith(mockTransaction); + }); + + it('should handle promise-based callbacks', async () => { + const serviceWithTransactional = withTransactional( + mockServiceFactory, + mockDb, + ); + + const result = await serviceWithTransactional.transactional( + async (service) => { + await service.saveData('new-data'); + return 'success'; + }, + ); + + expect(result).toBe('success'); + expect(mockService.saveData).toHaveBeenCalledWith('new-data'); + }); + + it('should propagate errors from callback', async () => { + const serviceWithTransactional = withTransactional( + mockServiceFactory, + mockDb, + ); + const error = new Error('Test error'); + + await expect( + serviceWithTransactional.transactional(() => { + throw error; + }), + ).rejects.toThrow('Test error'); + }); + }); + + describe('withRollbackTransaction', () => { + beforeEach(() => { + mockDb.transaction = vi.fn().mockResolvedValue(mockTransaction); + }); + + it('should add rollbackTransaction method to service', () => { + const serviceWithRollback = withRollbackTransaction( + mockServiceFactory, + mockDb, + ); + + expect(typeof serviceWithRollback.rollbackTransaction).toBe( + 'function', + ); + expect(serviceWithRollback.getData).toBe(mockService.getData); + expect(serviceWithRollback.saveData).toBe(mockService.saveData); + }); + + it('should execute callback and rollback transaction', async () => { + const serviceWithRollback = withRollbackTransaction( + mockServiceFactory, + mockDb, + ); + + const result = await serviceWithRollback.rollbackTransaction( + (service) => { + return service.getData(); + }, + ); + + expect(mockDb.transaction).toHaveBeenCalledTimes(1); + expect(mockTransaction.rollback).toHaveBeenCalledTimes(1); + expect(result).toBe('test-data'); + }); + }); + + describe('withFakeTransactional', () => { + it('should add transactional method to service', () => { + const serviceWithFakeTransactional = + withFakeTransactional(mockService); + + expect(typeof serviceWithFakeTransactional.transactional).toBe( + 'function', + ); + expect(serviceWithFakeTransactional.getData).toBe( + mockService.getData, + ); + expect(serviceWithFakeTransactional.saveData).toBe( + mockService.saveData, + ); + }); + + it('should execute callback directly without transaction', async () => { + const serviceWithFakeTransactional = + withFakeTransactional(mockService); + + const result = await serviceWithFakeTransactional.transactional( + (service) => { + return service.getData(); + }, + ); + + expect(result).toBe('test-data'); + }); + }); + + describe('inTransaction', () => { + it('should execute callback directly when db is already a transaction', async () => { + const transactionDb = { ...mockDb, isTransaction: true }; + const callback = vi.fn().mockReturnValue('result'); + + const result = await inTransaction(transactionDb, callback); + + expect(result).toBe('result'); + expect(callback).toHaveBeenCalledWith(transactionDb); + expect(transactionDb.transaction).not.toHaveBeenCalled(); + }); + + it('should create new transaction when db is not a transaction', async () => { + const callback = vi.fn().mockReturnValue('result'); + + const result = await inTransaction(mockDb, callback); + + expect(result).toBe('result'); + expect(mockDb.transaction).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith(mockTransaction); + }); + }); +}); diff --git a/src/lib/db/transaction.ts b/src/lib/db/transaction.ts index ac59b18b49..f98cd182fa 100644 --- a/src/lib/db/transaction.ts +++ b/src/lib/db/transaction.ts @@ -1,5 +1,15 @@ import type { Knex } from 'knex'; import type { IUnleashConfig } from '../types/index.ts'; +import { ulid } from 'ulidx'; + +export interface TransactionUserParams { + type: 'change-request' | 'transaction'; + id: string; +} + +function generateTransactionId(): string { + return ulid(); +} export type KnexTransaction = Knex.Transaction; @@ -38,7 +48,10 @@ export type ServiceFactory = ( ) => DeferredServiceFactory; export type WithTransactional = S & { - transactional: (fn: (service: S) => R) => Promise; + transactional: ( + fn: (service: S) => R, + transactionContext?: TransactionUserParams, + ) => Promise; }; export type WithRollbackTransaction = S & { @@ -75,10 +88,17 @@ export function withTransactional( ): WithTransactional { const service = serviceFactory(db) as WithTransactional; - service.transactional = async (fn: (service: S) => R) => - // Maybe: inTransaction(db, async (trx: Knex.Transaction) => fn(serviceFactory(trx))); - // this assumes that the caller didn't start a transaction already and opens a new one. + service.transactional = async ( + fn: (service: S) => R, + transactionContext?: TransactionUserParams, + ) => db.transaction(async (trx: Knex.Transaction) => { + const defaultContext: TransactionUserParams = { + type: 'transaction', + id: generateTransactionId(), + }; + + trx.userParams = transactionContext || defaultContext; const transactionalService = serviceFactory(trx); return fn(transactionalService); }); diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index aefdcb3d72..e155c81844 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -61,6 +61,7 @@ export type IFlagKey = | 'impactMetrics' | 'createFlagDialogCache' | 'improvedJsonDiff' + | 'crDiffView' | 'changeRequestApproverEmails'; export type IFlags = Partial<{ [key in IFlagKey]: boolean | Variant }>; @@ -282,6 +283,10 @@ const flags: IFlags = { process.env.UNLEASH_EXPERIMENTAL_IMPROVED_JSON_DIFF, false, ), + crDiffView: parseEnvVarBoolean( + process.env.UNLEASH_EXPERIMENTAL_CR_DIFF_VIEW, + false, + ), impactMetrics: parseEnvVarBoolean( process.env.UNLEASH_EXPERIMENTAL_IMPACT_METRICS, false, diff --git a/src/server-dev.ts b/src/server-dev.ts index b49244861d..ad51d0a923 100644 --- a/src/server-dev.ts +++ b/src/server-dev.ts @@ -57,6 +57,7 @@ process.nextTick(async () => { lifecycleMetrics: true, improvedJsonDiff: true, impactMetrics: true, + crDiffView: true, }, }, authentication: { diff --git a/website/package.json b/website/package.json index 1ee40e1eec..e69ccbfb16 100644 --- a/website/package.json +++ b/website/package.json @@ -64,7 +64,7 @@ }, "resolutions": { "http-proxy-middleware": "3.0.5", - "express/path-to-regexp": "0.1.12" + "express/path-to-regexp": "1.9.0" }, "packageManager": "yarn@4.9.2" } diff --git a/website/yarn.lock b/website/yarn.lock index 4c5df5c34a..ffafa4d226 100644 --- a/website/yarn.lock +++ b/website/yarn.lock @@ -12839,10 +12839,12 @@ __metadata: languageName: node linkType: hard -"path-to-regexp@npm:0.1.12": - version: 0.1.12 - resolution: "path-to-regexp@npm:0.1.12" - checksum: 10c0/1c6ff10ca169b773f3bba943bbc6a07182e332464704572962d277b900aeee81ac6aa5d060ff9e01149636c30b1f63af6e69dd7786ba6e0ddb39d4dee1f0645b +"path-to-regexp@npm:1.9.0, path-to-regexp@npm:^1.7.0": + version: 1.9.0 + resolution: "path-to-regexp@npm:1.9.0" + dependencies: + isarray: "npm:0.0.1" + checksum: 10c0/de9ddb01b84d9c2c8e2bed18630d8d039e2d6f60a6538595750fa08c7a6482512257464c8da50616f266ab2cdd2428387e85f3b089e4c3f25d0c537e898a0751 languageName: node linkType: hard @@ -12853,15 +12855,6 @@ __metadata: languageName: node linkType: hard -"path-to-regexp@npm:^1.7.0": - version: 1.9.0 - resolution: "path-to-regexp@npm:1.9.0" - dependencies: - isarray: "npm:0.0.1" - checksum: 10c0/de9ddb01b84d9c2c8e2bed18630d8d039e2d6f60a6538595750fa08c7a6482512257464c8da50616f266ab2cdd2428387e85f3b089e4c3f25d0c537e898a0751 - languageName: node - linkType: hard - "path-type@npm:^4.0.0": version: 4.0.0 resolution: "path-type@npm:4.0.0" diff --git a/yarn.lock b/yarn.lock index 805a565976..0ec137937a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7875,7 +7875,7 @@ __metadata: typescript: "npm:5.8.3" ulidx: "npm:^2.4.1" unleash-client: "npm:^6.7.0-beta.0" - uuid: "npm:^9.0.0" + uuid: "npm:^11.0.0" vite-node: "npm:^3.1.3" vitest: "npm:^3.1.3" wait-on: "npm:^8.0.0" @@ -7950,6 +7950,15 @@ __metadata: languageName: node linkType: hard +"uuid@npm:^11.0.0": + version: 11.1.0 + resolution: "uuid@npm:11.1.0" + bin: + uuid: dist/esm/bin/uuid + checksum: 10c0/34aa51b9874ae398c2b799c88a127701408cd581ee89ec3baa53509dd8728cbb25826f2a038f9465f8b7be446f0fbf11558862965b18d21c993684297628d4d3 + languageName: node + linkType: hard + "uuid@npm:^3.3.2": version: 3.4.0 resolution: "uuid@npm:3.4.0" @@ -7959,15 +7968,6 @@ __metadata: languageName: node linkType: hard -"uuid@npm:^9.0.0": - version: 9.0.1 - resolution: "uuid@npm:9.0.1" - bin: - uuid: dist/bin/uuid - checksum: 10c0/1607dd32ac7fc22f2d8f77051e6a64845c9bce5cd3dd8aa0070c074ec73e666a1f63c7b4e0f4bf2bc8b9d59dc85a15e17807446d9d2b17c8485fbc2147b27f9b - languageName: node - linkType: hard - "v8-compile-cache-lib@npm:^3.0.1": version: 3.0.1 resolution: "v8-compile-cache-lib@npm:3.0.1"