Skip to content

Commit f802294

Browse files
Improve upgrade command and prepare 1.5 release (#14325)
In this PR: - refactor the upgrade command / upgrade command runner to keep upgrade command as light as possible (all wrapping logic should go to upgrade command runner) - prevent any upgrade if there is at least one workspace.version < previsousVersion ==> this leads to corrupted state where only core migrations are run if the self-hoster is skipping a version
1 parent d5ef4cb commit f802294

11 files changed

+265
-247
lines changed

packages/twenty-server/src/database/commands/command-runners/__tests__/__snapshots__/upgrade.command-runner.spec.ts.snap

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,14 @@ exports[`UpgradeCommandRunner Workspace upgrade should fail when current version
88

99
exports[`UpgradeCommandRunner Workspace upgrade should fail when previous version is not found 1`] = `[Error: No previous version found for version 1.0.0. Please review the "allCommands" record. Available versions are: 1.0.0, 2.0.0]`;
1010

11-
exports[`UpgradeCommandRunner Workspace upgrade should fail when workspace version is not defined 1`] = `[Error: WORKSPACE_VERSION_NOT_DEFINED workspace=workspace_0]`;
12-
13-
exports[`UpgradeCommandRunner Workspace upgrade should fail when workspace version is not equal to fromVersion 1`] = `[Error: WORKSPACE_VERSION_MISSMATCH Upgrade for workspace workspace_0 failed as its version is beneath fromWorkspaceVersion=1.0.0]`;
14-
15-
exports[`UpgradeCommandRunner should run upgrade command with failing and successful workspaces 1`] = `[Error: WORKSPACE_VERSION_MISSMATCH Upgrade for workspace outated_version_workspace failed as its version is beneath fromWorkspaceVersion=1.0.0]`;
16-
17-
exports[`UpgradeCommandRunner should run upgrade command with failing and successful workspaces 2`] = `[Error: Received invalid version: invalid 1.0.0]`;
18-
19-
exports[`UpgradeCommandRunner should run upgrade command with failing and successful workspaces 3`] = `[Error: WORKSPACE_VERSION_NOT_DEFINED workspace=null_version_workspace]`;
11+
exports[`UpgradeCommandRunner Workspace upgrade should fail when workspace version is not defined 1`] = `
12+
[Error: Unable to run the upgrade command. Aborting the upgrade process.
13+
Please ensure that all workspaces are on at least the previous minor version (1.0.0).
14+
If any workspaces are not on the previous minor version, roll back to that version and run the upgrade command again.]
15+
`;
16+
17+
exports[`UpgradeCommandRunner Workspace upgrade should fail when workspace version is not equal to fromVersion 1`] = `
18+
[Error: Unable to run the upgrade command. Aborting the upgrade process.
19+
Please ensure that all workspaces are on at least the previous minor version (1.0.0).
20+
If any workspaces are not on the previous minor version, roll back to that version and run the upgrade command again.]
21+
`;

packages/twenty-server/src/database/commands/command-runners/__tests__/upgrade.command-runner.spec.ts

Lines changed: 43 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import { Test, type TestingModule } from '@nestjs/testing';
22
import { getRepositoryToken } from '@nestjs/typeorm';
33

4-
import { type EachTestingContext } from 'twenty-shared/testing';
4+
import {
5+
eachTestingContextFilter,
6+
type EachTestingContext,
7+
} from 'twenty-shared/testing';
58
import { type Repository } from 'typeorm';
69

710
import { UpgradeCommandRunner } from 'src/database/commands/command-runners/upgrade.command-runner';
@@ -125,6 +128,7 @@ describe('UpgradeCommandRunner', () => {
125128
let syncWorkspaceMetadataCommand: jest.Mocked<SyncWorkspaceMetadataCommand>;
126129
let runAfterSyncMetadataSpy: jest.SpyInstance;
127130
let runBeforeSyncMetadataSpy: jest.SpyInstance;
131+
let runCoreMigrationsSpy: jest.SpyInstance;
128132
let twentyORMGlobalManagerSpy: TwentyORMGlobalManager;
129133

130134
type BuildModuleAndSetupSpiesArgs = {
@@ -165,6 +169,9 @@ describe('UpgradeCommandRunner', () => {
165169
'runAfterSyncMetadata',
166170
);
167171
jest.spyOn(upgradeCommandRunner, 'runOnWorkspace');
172+
runCoreMigrationsSpy = jest
173+
.spyOn(upgradeCommandRunner, 'runCoreMigrations')
174+
.mockImplementation(() => Promise.resolve());
168175

169176
workspaceRepository = module.get<Repository<Workspace>>(
170177
getRepositoryToken(Workspace),
@@ -213,76 +220,6 @@ describe('UpgradeCommandRunner', () => {
213220
].forEach((fn) => expect(fn).not.toHaveBeenCalled());
214221
});
215222

216-
it('should run upgrade command with failing and successful workspaces', async () => {
217-
const outdatedVersionWorkspaces = generateMockWorkspace({
218-
id: 'outated_version_workspace',
219-
version: '0.42.42',
220-
});
221-
const invalidVersionWorkspace = generateMockWorkspace({
222-
id: 'invalid_version_workspace',
223-
version: 'invalid',
224-
});
225-
const nullVersionWorkspace = generateMockWorkspace({
226-
id: 'null_version_workspace',
227-
version: null,
228-
});
229-
const numberOfValidWorkspace = 4;
230-
const failingWorkspaces = [
231-
outdatedVersionWorkspaces,
232-
invalidVersionWorkspace,
233-
nullVersionWorkspace,
234-
];
235-
const totalWorkspace = numberOfValidWorkspace + failingWorkspaces.length;
236-
const appVersion = 'v2.0.0';
237-
const expectedToVersion = '2.0.0';
238-
239-
await buildModuleAndSetupSpies({
240-
numberOfWorkspace: numberOfValidWorkspace,
241-
workspaces: failingWorkspaces,
242-
appVersion,
243-
});
244-
// @ts-expect-error legacy noImplicitAny
245-
const passedParams = [];
246-
const options = {};
247-
248-
// @ts-expect-error legacy noImplicitAny
249-
await upgradeCommandRunner.run(passedParams, options);
250-
251-
// Common assertions
252-
const { fail: failReport, success: successReport } =
253-
upgradeCommandRunner.migrationReport;
254-
255-
[
256-
twentyORMGlobalManagerSpy.destroyDataSourceForWorkspace,
257-
upgradeCommandRunner.runOnWorkspace,
258-
].forEach((fn) => expect(fn).toHaveBeenCalledTimes(totalWorkspace));
259-
expect(failReport.length + successReport.length).toBe(totalWorkspace);
260-
261-
// Success assertions
262-
[
263-
upgradeCommandRunner.runBeforeSyncMetadata,
264-
syncWorkspaceMetadataCommand.runOnWorkspace,
265-
upgradeCommandRunner.runAfterSyncMetadata,
266-
].forEach((fn) => expect(fn).toHaveBeenCalledTimes(numberOfValidWorkspace));
267-
expect(successReport.length).toBe(numberOfValidWorkspace);
268-
expect(workspaceRepository.update).toHaveBeenNthCalledWith(
269-
numberOfValidWorkspace,
270-
{ id: expect.any(String) },
271-
{ version: expectedToVersion },
272-
);
273-
274-
// Failing assertions
275-
expect(failReport.length).toBe(failingWorkspaces.length);
276-
failReport.forEach((report) => {
277-
expect(
278-
failingWorkspaces.some(
279-
(workspace) => workspace.id === report.workspaceId,
280-
),
281-
).toBe(true);
282-
expect(report.error).toMatchSnapshot();
283-
});
284-
});
285-
286223
it('should run upgrade over several workspaces', async () => {
287224
const numberOfWorkspace = 42;
288225
const appVersion = '2.0.0';
@@ -383,7 +320,7 @@ describe('UpgradeCommandRunner', () => {
383320
},
384321
];
385322

386-
it.each(successfulTestUseCases)(
323+
it.each(eachTestingContextFilter(successfulTestUseCases))(
387324
'$title',
388325
async ({ context: { input } }) => {
389326
await buildModuleAndSetupSpies(input);
@@ -400,8 +337,9 @@ describe('UpgradeCommandRunner', () => {
400337

401338
expect(failReport.length).toBe(0);
402339
expect(successReport.length).toBe(1);
403-
expect(runAfterSyncMetadataSpy).toBeCalledTimes(1);
404-
expect(runBeforeSyncMetadataSpy).toBeCalledTimes(1);
340+
expect(runCoreMigrationsSpy).toHaveBeenCalledTimes(1);
341+
expect(runAfterSyncMetadataSpy).toHaveBeenCalledTimes(1);
342+
expect(runBeforeSyncMetadataSpy).toHaveBeenCalledTimes(1);
405343
const { workspaceId } = successReport[0];
406344

407345
expect(workspaceId).toBe('workspace_0');
@@ -412,6 +350,9 @@ describe('UpgradeCommandRunner', () => {
412350
describe('Workspace upgrade should fail', () => {
413351
const failingTestUseCases: EachTestingContext<{
414352
input: Omit<BuildModuleAndSetupSpiesArgs, 'numberOfWorkspace'>;
353+
output?: {
354+
failReportWorkspaceId: string;
355+
};
415356
}>[] = [
416357
{
417358
title: 'when workspace version is not equal to fromVersion',
@@ -422,6 +363,9 @@ describe('UpgradeCommandRunner', () => {
422363
version: '0.1.0',
423364
},
424365
},
366+
output: {
367+
failReportWorkspaceId: 'workspace_0',
368+
},
425369
},
426370
},
427371
{
@@ -432,6 +376,9 @@ describe('UpgradeCommandRunner', () => {
432376
version: null,
433377
},
434378
},
379+
output: {
380+
failReportWorkspaceId: 'workspace_0',
381+
},
435382
},
436383
},
437384
{
@@ -440,6 +387,9 @@ describe('UpgradeCommandRunner', () => {
440387
input: {
441388
appVersion: null,
442389
},
390+
output: {
391+
failReportWorkspaceId: 'global',
392+
},
443393
},
444394
},
445395
{
@@ -448,6 +398,9 @@ describe('UpgradeCommandRunner', () => {
448398
input: {
449399
appVersion: '42.0.0',
450400
},
401+
output: {
402+
failReportWorkspaceId: 'global',
403+
},
451404
},
452405
},
453406
{
@@ -468,25 +421,26 @@ describe('UpgradeCommandRunner', () => {
468421
},
469422
];
470423

471-
it.each(failingTestUseCases)('$title', async ({ context: { input } }) => {
472-
await buildModuleAndSetupSpies(input);
424+
it.each(eachTestingContextFilter(failingTestUseCases))(
425+
'$title',
426+
async ({ context: { input, output } }) => {
427+
await buildModuleAndSetupSpies(input);
473428

474-
// @ts-expect-error legacy noImplicitAny
475-
const passedParams = [];
476-
const options = {};
429+
const passedParams: string[] = [];
430+
const options = {};
477431

478-
// @ts-expect-error legacy noImplicitAny
479-
await upgradeCommandRunner.run(passedParams, options);
432+
await upgradeCommandRunner.run(passedParams, options);
480433

481-
const { fail: failReport, success: successReport } =
482-
upgradeCommandRunner.migrationReport;
434+
const { fail: failReport, success: successReport } =
435+
upgradeCommandRunner.migrationReport;
483436

484-
expect(successReport.length).toBe(0);
485-
expect(failReport.length).toBe(1);
486-
const { workspaceId, error } = failReport[0];
437+
expect(successReport.length).toBe(0);
438+
expect(failReport.length).toBe(1);
439+
const { workspaceId, error } = failReport[0];
487440

488-
expect(workspaceId).toBe('workspace_0');
489-
expect(error).toMatchSnapshot();
490-
});
441+
expect(workspaceId).toBe(output?.failReportWorkspaceId ?? 'global');
442+
expect(error).toMatchSnapshot();
443+
},
444+
);
491445
});
492446
});

0 commit comments

Comments
 (0)