Skip to content

Commit d2dda67

Browse files
prastoinprastoin
authored
Fix upgrade --start-from-workspace-id (#20116)
# Introduction Prevent using both `--start-from-workspace-id` and `--workspace` When any of the two are being passed we prevent passing to the next instance segment, it would require an upgrade re run even if legit When `--start-from-workspace-id` is passed we filter from all the fetched active or suspended workspace ids and apply equivalent filter as before
1 parent 6aec449 commit d2dda67

3 files changed

Lines changed: 235 additions & 9 deletions

File tree

‎packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,15 @@ export class UpgradeCommand extends CommandRunner {
113113
});
114114
}
115115

116+
if (
117+
isDefined(options.workspaceId) &&
118+
isDefined(options.startFromWorkspaceId)
119+
) {
120+
throw new Error(
121+
'Cannot use --start-from-workspace-id together with -w/--workspace-id',
122+
);
123+
}
124+
116125
try {
117126
const sequence = this.upgradeSequenceReaderService.getUpgradeSequence();
118127

‎packages/twenty-server/src/engine/core-modules/upgrade/services/upgrade-sequence-runner.service.ts‎

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,14 @@ export class UpgradeSequenceRunnerService {
6969

7070
if (step.kind === 'fast-instance' || step.kind === 'slow-instance') {
7171
if (
72-
isDefined(options.workspaceIds) &&
73-
options.workspaceIds.length > 0
72+
(isDefined(options.workspaceIds) &&
73+
options.workspaceIds.length > 0) ||
74+
isDefined(options.startFromWorkspaceId) ||
75+
isDefined(options.workspaceCountLimit)
7476
) {
7577
this.logger.log(
7678
`Stopping before instance step "${step.name}": ` +
77-
'upgrade was run with workspace filter (-w). ' +
79+
'upgrade was run with a workspace filter (-w, --start-from-workspace-id, or --workspace-count-limit). ' +
7880
'Instance commands require all workspaces to be aligned.',
7981
);
8082

@@ -303,15 +305,13 @@ export class UpgradeSequenceRunnerService {
303305
allActiveOrSuspendedWorkspaceIds: string[];
304306
options: ParsedUpgradeCommandOptions;
305307
}): Promise<WorkspaceIteratorReport> {
306-
const workspaceIds =
307-
isDefined(options.workspaceIds) && options.workspaceIds.length > 0
308-
? options.workspaceIds
309-
: allActiveOrSuspendedWorkspaceIds;
308+
const workspaceIds = this.deriveWorkspaceIdsToProcess({
309+
allActiveOrSuspendedWorkspaceIds,
310+
options,
311+
});
310312

311313
return this.workspaceIteratorService.iterate({
312314
workspaceIds,
313-
startFromWorkspaceId: options.startFromWorkspaceId,
314-
workspaceCountLimit: options.workspaceCountLimit,
315315
dryRun: options.dryRun,
316316
callback: async (context) => {
317317
const workspaceCursor = workspaceCursors.get(context.workspaceId);
@@ -337,6 +337,32 @@ export class UpgradeSequenceRunnerService {
337337
});
338338
}
339339

340+
private deriveWorkspaceIdsToProcess({
341+
allActiveOrSuspendedWorkspaceIds,
342+
options,
343+
}: {
344+
allActiveOrSuspendedWorkspaceIds: string[];
345+
options: ParsedUpgradeCommandOptions;
346+
}): string[] {
347+
if (isDefined(options.workspaceIds) && options.workspaceIds.length > 0) {
348+
return options.workspaceIds;
349+
}
350+
351+
let workspaceIds = allActiveOrSuspendedWorkspaceIds;
352+
353+
if (isDefined(options.startFromWorkspaceId)) {
354+
workspaceIds = workspaceIds.filter(
355+
(id) => id >= options.startFromWorkspaceId!,
356+
);
357+
}
358+
359+
if (isDefined(options.workspaceCountLimit)) {
360+
workspaceIds = workspaceIds.slice(0, options.workspaceCountLimit);
361+
}
362+
363+
return workspaceIds;
364+
}
365+
340366
private enforceWorkspacesCompletedPreviousWorkspaceSegment({
341367
sequence,
342368
previousWorkspaceStep,

‎packages/twenty-server/test/integration/upgrade/suites/sequence-runner/start-from-workspace-id.integration-spec.ts‎

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
import {
2+
type IntegrationTestContext,
3+
createUpgradeSequenceRunnerIntegrationTestModule,
4+
DEFAULT_OPTIONS,
5+
makeFastInstance,
6+
makeWorkspace,
7+
migrationRecordToKey,
8+
resetSeedSequenceCounter,
9+
seedInstanceMigration,
10+
setMockActiveWorkspaceIds,
11+
testGetExecutedMigrationsInOrder,
12+
WS_1,
13+
WS_2,
14+
WS_3,
15+
WS_4,
16+
} from 'test/integration/upgrade/utils/upgrade-sequence-runner-integration-test.util';
17+
18+
// Sorted ASC: WS_1 < WS_2 < WS_3 < WS_4
19+
20+
describe('UpgradeSequenceRunnerService — startFromWorkspaceId (integration)', () => {
21+
let context: IntegrationTestContext;
22+
23+
beforeAll(async () => {
24+
context = await createUpgradeSequenceRunnerIntegrationTestModule();
25+
}, 30000);
26+
27+
afterAll(async () => {
28+
await context.dataSource.query('DELETE FROM core."upgradeMigration"');
29+
await context.module?.close();
30+
await context.dataSource?.destroy();
31+
}, 15000);
32+
33+
beforeEach(async () => {
34+
await context.dataSource.query('DELETE FROM core."upgradeMigration"');
35+
resetSeedSequenceCounter();
36+
setMockActiveWorkspaceIds([]);
37+
jest.restoreAllMocks();
38+
});
39+
40+
it('should only process workspaces whose id >= startFromWorkspaceId', async () => {
41+
const sequence = [
42+
makeFastInstance('Ic1'),
43+
makeWorkspace('Wc1'),
44+
makeWorkspace('Wc2'),
45+
makeFastInstance('Ic2'),
46+
];
47+
48+
setMockActiveWorkspaceIds([WS_1, WS_2, WS_3]);
49+
50+
await seedInstanceMigration(context.dataSource, {
51+
name: 'Ic1',
52+
status: 'completed',
53+
workspaceIds: [WS_1, WS_2, WS_3],
54+
});
55+
56+
const report = await context.runner.run({
57+
sequence,
58+
options: {
59+
...DEFAULT_OPTIONS,
60+
startFromWorkspaceId: WS_2,
61+
},
62+
});
63+
64+
expect(report.totalFailures).toBe(0);
65+
expect(report.totalSuccesses).toBe(2);
66+
67+
const executed = await testGetExecutedMigrationsInOrder(context.dataSource);
68+
69+
expect(executed.map(migrationRecordToKey)).toStrictEqual([
70+
// Seeds
71+
'Ic1:instance:completed:1',
72+
`Ic1:${WS_1}:completed:1`,
73+
`Ic1:${WS_2}:completed:1`,
74+
`Ic1:${WS_3}:completed:1`,
75+
76+
// Only WS_2 and WS_3 are processed (WS_1 skipped)
77+
`Wc1:${WS_2}:completed:1`,
78+
`Wc2:${WS_2}:completed:1`,
79+
`Wc1:${WS_3}:completed:1`,
80+
`Wc2:${WS_3}:completed:1`,
81+
]);
82+
});
83+
84+
it('should respect workspaceCountLimit together with startFromWorkspaceId', async () => {
85+
const sequence = [makeFastInstance('Ic1'), makeWorkspace('Wc1')];
86+
87+
setMockActiveWorkspaceIds([WS_1, WS_2, WS_3, WS_4]);
88+
89+
await seedInstanceMigration(context.dataSource, {
90+
name: 'Ic1',
91+
status: 'completed',
92+
workspaceIds: [WS_1, WS_2, WS_3, WS_4],
93+
});
94+
95+
const report = await context.runner.run({
96+
sequence,
97+
options: {
98+
...DEFAULT_OPTIONS,
99+
startFromWorkspaceId: WS_2,
100+
workspaceCountLimit: 1,
101+
},
102+
});
103+
104+
expect(report.totalFailures).toBe(0);
105+
expect(report.totalSuccesses).toBe(1);
106+
107+
const executed = await testGetExecutedMigrationsInOrder(context.dataSource);
108+
109+
expect(executed.map(migrationRecordToKey)).toStrictEqual([
110+
// Seeds
111+
'Ic1:instance:completed:1',
112+
`Ic1:${WS_1}:completed:1`,
113+
`Ic1:${WS_2}:completed:1`,
114+
`Ic1:${WS_3}:completed:1`,
115+
`Ic1:${WS_4}:completed:1`,
116+
117+
// Only WS_2 processed (first after startFrom, limited to 1)
118+
`Wc1:${WS_2}:completed:1`,
119+
]);
120+
});
121+
122+
it('should stop before instance step when startFromWorkspaceId is set', async () => {
123+
const sequence = [
124+
makeFastInstance('Ic1'),
125+
makeWorkspace('Wc1'),
126+
makeWorkspace('Wc2'),
127+
makeFastInstance('Ic2'),
128+
makeWorkspace('Wc3'),
129+
];
130+
131+
setMockActiveWorkspaceIds([WS_1, WS_2, WS_3]);
132+
133+
await seedInstanceMigration(context.dataSource, {
134+
name: 'Ic1',
135+
status: 'completed',
136+
workspaceIds: [WS_1, WS_2, WS_3],
137+
});
138+
139+
const report = await context.runner.run({
140+
sequence,
141+
options: {
142+
...DEFAULT_OPTIONS,
143+
startFromWorkspaceId: WS_2,
144+
},
145+
});
146+
147+
expect(report.totalFailures).toBe(0);
148+
expect(report.totalSuccesses).toBe(2);
149+
150+
const executed = await testGetExecutedMigrationsInOrder(context.dataSource);
151+
152+
expect(executed.map(migrationRecordToKey)).toStrictEqual([
153+
// Seeds
154+
'Ic1:instance:completed:1',
155+
`Ic1:${WS_1}:completed:1`,
156+
`Ic1:${WS_2}:completed:1`,
157+
`Ic1:${WS_3}:completed:1`,
158+
159+
// Workspace segment: only WS_2 and WS_3 run
160+
`Wc1:${WS_2}:completed:1`,
161+
`Wc2:${WS_2}:completed:1`,
162+
`Wc1:${WS_3}:completed:1`,
163+
`Wc2:${WS_3}:completed:1`,
164+
165+
// Ic2 and Wc3 never reached — runner stopped at instance step boundary
166+
]);
167+
});
168+
169+
it('should process no workspaces when startFromWorkspaceId is greater than all ids', async () => {
170+
const sequence = [makeFastInstance('Ic1'), makeWorkspace('Wc1')];
171+
172+
setMockActiveWorkspaceIds([WS_1, WS_2]);
173+
174+
await seedInstanceMigration(context.dataSource, {
175+
name: 'Ic1',
176+
status: 'completed',
177+
workspaceIds: [WS_1, WS_2],
178+
});
179+
180+
const report = await context.runner.run({
181+
sequence,
182+
options: {
183+
...DEFAULT_OPTIONS,
184+
startFromWorkspaceId: 'ffffffff-ffff-ffff-ffff-ffffffffffff',
185+
},
186+
});
187+
188+
expect(report.totalFailures).toBe(0);
189+
expect(report.totalSuccesses).toBe(0);
190+
});
191+
});

0 commit comments

Comments
 (0)