Repository: directus/directus
File: api/src/services/items.test.ts (TypeScript)

Code before:

		beforeEach(() => {
			service = new ItemsService('test', {
				knex: db,
				schema,
			});
		});

		afterEach(() => {
			vi.clearAllMocks();
		});

		describe('createOne', () => {
			it('should validate user count if requested', async () => {
				await service.createOne({}, { userIntegrityCheckFlags: UserIntegrityCheckFlag.All });

				expect(validateUserCountIntegrity).toHaveBeenCalled();
			});

			it('should detect MS SQL client and prepare correct returning options', () => {
				// Mock MS SQL client
				vi.mocked(getDatabaseClient).mockReturnValueOnce('mssql');

				// Test the logic that determines returning options
				const client = getDatabaseClient();
				const returningOptions = client === 'mssql' ? { includeTriggerModifications: true } : undefined;

				expect(client).toBe('mssql');
				expect(returningOptions).toEqual({ includeTriggerModifications: true });
			});

			it('should detect non-MS SQL client and prepare no returning options', () => {
				// Mock PostgreSQL client
				vi.mocked(getDatabaseClient).mockReturnValueOnce('postgres');

				// Test the logic that determines returning options
				const client = getDatabaseClient();
				const returningOptions = client === 'mssql' ? { includeTriggerModifications: true } : undefined;

				expect(client).toBe('postgres');
				expect(returningOptions).toBeUndefined();
			});
		});

		describe('createMany', () => {
			it('should validate user count if requested', async () => {
				await service.createMany([{}], { userIntegrityCheckFlags: UserIntegrityCheckFlag.All });

				expect(validateUserCountIntegrity).toHaveBeenCalled();
			});
		});

Review comment (suggestion):
Lets revise this so we don't repeat the actual business logic in the test. We don't need the `client === 'mssql'` branching here as we've already mocked `getDatabaseClient` to output `mssql`

Code after:

		beforeEach(() => {
			service = new ItemsService('test', {
				knex: db,
				schema,
			});
		});

		afterEach(() => {
			vi.clearAllMocks();
		});

		describe('createOne', () => {
			it('should validate user count if requested', async () => {
				await service.createOne({}, { userIntegrityCheckFlags: UserIntegrityCheckFlag.All });

				expect(validateUserCountIntegrity).toHaveBeenCalled();
			});

			it('should use includeTriggerModifications for MS SQL', async () => {
				vi.mocked(getDatabaseClient).mockReturnValue('mssql');

				const mockReturning = vi.fn().mockResolvedValue([{ id: 1 }]);

				const mockQuery = {
					insert: vi.fn().mockReturnThis(),
					into: vi.fn().mockReturnThis(),
					returning: mockReturning,
				};

				const transactionSpy = vi.spyOn(db, 'transaction').mockImplementation(async (callback) => {
					const trx = { ...db, ...mockQuery };
					return await callback(trx as any);
				});

				await service.createOne({ name: 'Test' });

				expect(mockReturning).toHaveBeenCalledWith('id', { includeTriggerModifications: true });

				transactionSpy.mockRestore();
			});

			it('should not use includeTriggerModifications for non-MS SQL', async () => {
				vi.mocked(getDatabaseClient).mockReturnValue('postgres');

				const mockReturning = vi.fn().mockResolvedValue([{ id: 1 }]);

				const mockQuery = {
					insert: vi.fn().mockReturnThis(),
					into: vi.fn().mockReturnThis(),
					returning: mockReturning,