Align campaign view column positions with the standard layout (#23496)

Follow-up to #23493 (merged). Now based on `main`. ## Problem `AddMessageCampaignNameFieldCommand` places the new `name` column below the lowest existing position when it is the label identifier (`min - 1`). That satisfies `isViewFieldInLowestPosition` once, but does not hold: `viewField.position` is compared by the standard-application sync (`position: { toCompare: true }`), so the stored position is pulled back to the standard one and re-fails `validateLabelIdentifierFieldMetadataIdFlatViewField`, throwing `WorkspaceMigrationBuilderException` on every sync. That sync runs during `WorkspaceManagerService.init`, which is what `ActivateWorkspace` calls. cubic flagged this on the original PR: [discussion_r3653452416](https://github.com/twentyhq/twenty/pull/23188#discussion_r3653452416). ## Confirmed against prod Queried the workspace failing in Sentry ([TWENTY-SERVER-JJ2](https://twenty-v7.sentry.io/issues/7639768673/)): the `allMessageCampaigns` view has no `name` column at all, `subject` at position `0` (still the label identifier), and every column at the old layout. The activation sync tries to create `name` at standard position `0` while repointing the label identifier in the same batch, ties with `subject` at `0`, and throws. Exactly the mechanism this PR fixes. ## Change A `2-25` workspace command (`upgrade:2-25:align-message-campaign-view-field-positions`, timestamp `1785332560000`) that aligns the `allMessageCampaigns` columns to the standard layout, so the sync has nothing left to change: - Standard columns take their standard positions (`subject 0→1`, `status 1→2`, …), freeing slot `0` for `name`. - Columns the standard application does not know about keep their relative order and move above the standard ones. - Updates run in two migration passes: everything else first, then the lowest-target column alone. The migration builder validates updates one at a time against optimistic maps it mutates as it goes, so a single batch is order-dependent (caught by Greptile below); two passes keep every intermediate state valid. Placed in `2-25/` rather than `2-26/` so it can ship in a 2.25.x patch — the failures are on `v2.25.0` instances. This is why `server-previous-version-upgrade-mutation-guard` is red: it needs the `ci:allow-previous-version-upgrade-mutation` label (same as #23493). The position arithmetic and the pass-splitting are pure utils with 12 tests. ## Traced against the confirmed prod state Pass 1 moves `status`…`createdAt` up while `subject` (label identifier) stays lowest at `0`; pass 2 moves `subject` to `1`, still strictly lowest. The subsequent activation sync then creates `name` at `0` below `subject` at `1` and repoints the label identifier. Converges. Run order matters only relative to the sibling command from #23493: removal (`…550000`) sorts before alignment (`…560000`), which is correct.
2026-07-29 16:16:52 +02:00
parent e79424ddb0
commit 33970dd45d
6 changed files with 462 additions and 0 deletions
@@ -7,6 +7,7 @@ import { AddMessageCampaignComposerTabCommand } from 'src/database/commands/upgr
 import { ConfigureMessageCampaignCommandMenuCommand } from 'src/database/commands/upgrade-version-command/2-25/2-25-workspace-command-1785229960000-configure-message-campaign-command-menu.command';
 import { AddMessageCampaignNameFieldCommand } from 'src/database/commands/upgrade-version-command/2-25/2-25-workspace-command-1785229970000-add-message-campaign-name-field.command';
 import { RemoveMessageCampaignNavigationMenuItemCommand } from 'src/database/commands/upgrade-version-command/2-25/2-25-workspace-command-1785332550000-remove-message-campaign-navigation-menu-item.command';
+import { AlignMessageCampaignViewFieldPositionsCommand } from 'src/database/commands/upgrade-version-command/2-25/2-25-workspace-command-1785332560000-align-message-campaign-view-field-positions.command';
 import { ApplicationModule } from 'src/engine/core-modules/application/application.module';
 import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
 import { WorkspaceCacheModule } from 'src/engine/workspace-cache/workspace-cache.module';
@@ -28,6 +29,7 @@ import { WorkspaceMigrationModule } from 'src/engine/workspace-manager/workspace
    ConfigureMessageCampaignCommandMenuCommand,
    AddMessageCampaignNameFieldCommand,
    RemoveMessageCampaignNavigationMenuItemCommand,
+    AlignMessageCampaignViewFieldPositionsCommand,
  ],
 })
 export class V2_25_UpgradeVersionCommandModule {}
@@ -0,0 +1,190 @@
+import { Command } from 'nest-commander';
+
+import { STANDARD_OBJECTS } from 'twenty-shared/metadata';
+import { isDefined } from 'twenty-shared/utils';
+
+import { ProvisionedWorkspaceCommandRunner } from 'src/database/commands/command-runners/provisioned-workspace.command-runner';
+import { WorkspaceIteratorService } from 'src/database/commands/command-runners/workspace-iterator.service';
+import { type RunOnWorkspaceArgs } from 'src/database/commands/command-runners/workspace.command-runner';
+import { computeViewFieldPositionsAlignedToStandard } from 'src/database/commands/upgrade-version-command/2-25/utils/compute-view-field-positions-aligned-to-standard.util';
+import { splitViewFieldPositionUpdates } from 'src/database/commands/upgrade-version-command/2-25/utils/split-view-field-position-updates.util';
+import { ApplicationService } from 'src/engine/core-modules/application/application.service';
+import { RegisteredWorkspaceCommand } from 'src/engine/core-modules/upgrade/decorators/registered-workspace-command.decorator';
+import { type FlatViewField } from 'src/engine/metadata-modules/flat-view-field/types/flat-view-field.type';
+import { WorkspaceCacheService } from 'src/engine/workspace-cache/services/workspace-cache.service';
+import { computeTwentyStandardApplicationAllFlatEntityMaps } from 'src/engine/workspace-manager/twenty-standard-application/utils/twenty-standard-application-all-flat-entity-maps.constant';
+import { WorkspaceMigrationValidateBuildAndRunService } from 'src/engine/workspace-manager/workspace-migration/services/workspace-migration-validate-build-and-run-service';
+
+const ALL_MESSAGE_CAMPAIGNS_VIEW_UNIVERSAL_IDENTIFIER =
+  STANDARD_OBJECTS.messageCampaign.views.allMessageCampaigns
+    .universalIdentifier;
+
+@RegisteredWorkspaceCommand('2.25.0', 1785332560000)
+@Command({
+  name: 'upgrade:2-25:align-message-campaign-view-field-positions',
+  description:
+    'Align the all campaigns view columns with the standard layout so the name label identifier sits strictly first and the standard-application sync stops trying to move it back',
+})
+export class AlignMessageCampaignViewFieldPositionsCommand extends ProvisionedWorkspaceCommandRunner {
+  constructor(
+    protected readonly workspaceIteratorService: WorkspaceIteratorService,
+    private readonly applicationService: ApplicationService,
+    private readonly workspaceCacheService: WorkspaceCacheService,
+    private readonly workspaceMigrationValidateBuildAndRunService: WorkspaceMigrationValidateBuildAndRunService,
+  ) {
+    super(workspaceIteratorService);
+  }
+
+  override async runOnWorkspace({
+    workspaceId,
+    options,
+  }: RunOnWorkspaceArgs): Promise<void> {
+    const isDryRun = options.dryRun ?? false;
+
+    const { flatViewMaps, flatViewFieldMaps } =
+      await this.workspaceCacheService.getOrRecompute(workspaceId, [
+        'flatViewMaps',
+        'flatViewFieldMaps',
+      ]);
+
+    const existingView =
+      flatViewMaps.byUniversalIdentifier[
+        ALL_MESSAGE_CAMPAIGNS_VIEW_UNIVERSAL_IDENTIFIER
+      ];
+
+    if (!isDefined(existingView)) {
+      this.logger.log(
+        `All campaigns view does not exist for workspace ${workspaceId}, skipping`,
+      );
+
+      return;
+    }
+
+    const { twentyStandardFlatApplication } =
+      await this.applicationService.findWorkspaceTwentyStandardAndCustomApplicationOrThrow(
+        { workspaceId },
+      );
+
+    const { allFlatEntityMaps: standardAllFlatEntityMaps } =
+      computeTwentyStandardApplicationAllFlatEntityMaps({
+        now: new Date().toISOString(),
+        workspaceId,
+        twentyStandardApplicationId: twentyStandardFlatApplication.id,
+      });
+
+    const standardPositionByUniversalIdentifier = Object.fromEntries(
+      Object.values(
+        standardAllFlatEntityMaps.flatViewFieldMaps.byUniversalIdentifier,
+      )
+        .filter(isDefined)
+        .filter(
+          (viewField) =>
+            viewField.viewUniversalIdentifier ===
+            ALL_MESSAGE_CAMPAIGNS_VIEW_UNIVERSAL_IDENTIFIER,
+        )
+        .map(({ universalIdentifier, position }) => [
+          universalIdentifier,
+          position,
+        ]),
+    );
+
+    const existingViewFields = existingView.viewFieldUniversalIdentifiers
+      .map(
+        (viewFieldUniversalIdentifier) =>
+          flatViewFieldMaps.byUniversalIdentifier[viewFieldUniversalIdentifier],
+      )
+      .filter(isDefined);
+
+    const positionUpdates = computeViewFieldPositionsAlignedToStandard({
+      existingViewFields: existingViewFields.map(
+        ({ universalIdentifier, position }) => ({
+          universalIdentifier,
+          position,
+        }),
+      ),
+      standardPositionByUniversalIdentifier,
+    });
+
+    const viewFieldsToUpdate = positionUpdates
+      .map(({ universalIdentifier, position }) => {
+        const existingViewField =
+          flatViewFieldMaps.byUniversalIdentifier[universalIdentifier];
+
+        return isDefined(existingViewField)
+          ? { ...existingViewField, position }
+          : null;
+      })
+      .filter((viewField): viewField is FlatViewField => isDefined(viewField));
+
+    if (viewFieldsToUpdate.length === 0) {
+      this.logger.log(
+        `All campaigns view columns already match the standard layout for workspace ${workspaceId}, skipping`,
+      );
+
+      return;
+    }
+
+    this.logger.log(
+      `${isDryRun ? '[DRY RUN] ' : ''}Aligning ${viewFieldsToUpdate.length} all campaigns view column(s) for workspace ${workspaceId}`,
+    );
+
+    if (isDryRun) {
+      return;
+    }
+
+    const { others, lowest } = splitViewFieldPositionUpdates(viewFieldsToUpdate);
+
+    for (const viewFieldBatch of [others, lowest]) {
+      if (viewFieldBatch.length === 0) {
+        continue;
+      }
+
+      await this.runViewFieldUpdates({
+        workspaceId,
+        applicationUniversalIdentifier:
+          twentyStandardFlatApplication.universalIdentifier,
+        viewFieldsToUpdate: viewFieldBatch,
+      });
+    }
+
+    this.logger.log(
+      `Aligned the all campaigns view columns for workspace ${workspaceId}`,
+    );
+  }
+
+  private async runViewFieldUpdates({
+    workspaceId,
+    applicationUniversalIdentifier,
+    viewFieldsToUpdate,
+  }: {
+    workspaceId: string;
+    applicationUniversalIdentifier: string;
+    viewFieldsToUpdate: FlatViewField[];
+  }): Promise<void> {
+    const result =
+      await this.workspaceMigrationValidateBuildAndRunService.validateBuildAndRunLegacyWorkspaceMigration(
+        {
+          isSystemBuild: true,
+          workspaceId,
+          applicationUniversalIdentifier,
+          allFlatEntityOperationByMetadataName: {
+            viewField: {
+              flatEntityToCreate: [],
+              flatEntityToDelete: [],
+              flatEntityToUpdate: viewFieldsToUpdate,
+            },
+          },
+        },
+      );
+
+    if (result.status === 'fail') {
+      this.logger.error(
+        `Failed to align the all campaigns view columns:\n${JSON.stringify(result, null, 2)}`,
+      );
+
+      throw new Error(
+        `Failed to align the all campaigns view columns for workspace ${workspaceId}`,
+      );
+    }
+  }
+}
@@ -0,0 +1,118 @@
+import { computeViewFieldPositionsAlignedToStandard } from 'src/database/commands/upgrade-version-command/2-25/utils/compute-view-field-positions-aligned-to-standard.util';
+
+const STANDARD = {
+  name: 0,
+  subject: 1,
+  status: 2,
+};
+
+describe('computeViewFieldPositionsAlignedToStandard', () => {
+  it('should return nothing when the view already matches the standard layout', () => {
+    expect(
+      computeViewFieldPositionsAlignedToStandard({
+        existingViewFields: [
+          { universalIdentifier: 'name', position: 0 },
+          { universalIdentifier: 'subject', position: 1 },
+          { universalIdentifier: 'status', position: 2 },
+        ],
+        standardPositionByUniversalIdentifier: STANDARD,
+      }),
+    ).toEqual([]);
+  });
+
+  it('should shift the pre-name layout onto the standard positions', () => {
+    expect(
+      computeViewFieldPositionsAlignedToStandard({
+        existingViewFields: [
+          { universalIdentifier: 'subject', position: 0 },
+          { universalIdentifier: 'status', position: 1 },
+        ],
+        standardPositionByUniversalIdentifier: STANDARD,
+      }),
+    ).toEqual([
+      { universalIdentifier: 'subject', position: 1 },
+      { universalIdentifier: 'status', position: 2 },
+    ]);
+  });
+
+  it('should pull a column left below the standard position back up', () => {
+    expect(
+      computeViewFieldPositionsAlignedToStandard({
+        existingViewFields: [
+          { universalIdentifier: 'name', position: -1 },
+          { universalIdentifier: 'subject', position: 0 },
+        ],
+        standardPositionByUniversalIdentifier: STANDARD,
+      }),
+    ).toEqual([
+      { universalIdentifier: 'name', position: 0 },
+      { universalIdentifier: 'subject', position: 1 },
+    ]);
+  });
+
+  it('should move unknown columns above every standard one, keeping their order', () => {
+    expect(
+      computeViewFieldPositionsAlignedToStandard({
+        existingViewFields: [
+          { universalIdentifier: 'customB', position: 5 },
+          { universalIdentifier: 'subject', position: 0 },
+          { universalIdentifier: 'customA', position: 1 },
+        ],
+        standardPositionByUniversalIdentifier: STANDARD,
+      }),
+    ).toEqual([
+      { universalIdentifier: 'customB', position: 4 },
+      { universalIdentifier: 'subject', position: 1 },
+      { universalIdentifier: 'customA', position: 3 },
+    ]);
+  });
+
+  it('should leave the label identifier strictly lowest in every case', () => {
+    const cases = [
+      [
+        { universalIdentifier: 'subject', position: 0 },
+        { universalIdentifier: 'status', position: 1 },
+      ],
+      [
+        { universalIdentifier: 'name', position: -1 },
+        { universalIdentifier: 'subject', position: 0 },
+      ],
+      [
+        { universalIdentifier: 'custom', position: 0 },
+        { universalIdentifier: 'subject', position: 1 },
+      ],
+    ];
+
+    cases.forEach((existingViewFields) => {
+      const updates = computeViewFieldPositionsAlignedToStandard({
+        existingViewFields,
+        standardPositionByUniversalIdentifier: STANDARD,
+      });
+
+      const finalPositions = existingViewFields.map(
+        ({ universalIdentifier, position }) =>
+          updates.find(
+            (update) => update.universalIdentifier === universalIdentifier,
+          )?.position ?? position,
+      );
+      const namePosition = STANDARD.name;
+
+      finalPositions.forEach((position, index) => {
+        if (existingViewFields[index].universalIdentifier === 'name') {
+          return;
+        }
+
+        expect(position).toBeGreaterThan(namePosition);
+      });
+    });
+  });
+
+  it('should return nothing when the standard layout is unknown', () => {
+    expect(
+      computeViewFieldPositionsAlignedToStandard({
+        existingViewFields: [{ universalIdentifier: 'subject', position: 0 }],
+        standardPositionByUniversalIdentifier: {},
+      }),
+    ).toEqual([]);
+  });
+});
@@ -0,0 +1,67 @@
+import { splitViewFieldPositionUpdates } from 'src/database/commands/upgrade-version-command/2-25/utils/split-view-field-position-updates.util';
+
+describe('splitViewFieldPositionUpdates', () => {
+  it('should hold back the column that ends up lowest', () => {
+    expect(
+      splitViewFieldPositionUpdates([
+        { universalIdentifier: 'name', position: 0 },
+        { universalIdentifier: 'subject', position: 1 },
+        { universalIdentifier: 'status', position: 2 },
+      ]),
+    ).toEqual({
+      others: [
+        { universalIdentifier: 'subject', position: 1 },
+        { universalIdentifier: 'status', position: 2 },
+      ],
+      lowest: [{ universalIdentifier: 'name', position: 0 }],
+    });
+  });
+
+  it('should hold it back regardless of where it sits in the input', () => {
+    expect(
+      splitViewFieldPositionUpdates([
+        { universalIdentifier: 'status', position: 2 },
+        { universalIdentifier: 'name', position: 0 },
+      ]).lowest,
+    ).toEqual([{ universalIdentifier: 'name', position: 0 }]);
+  });
+
+  it('should handle negative positions', () => {
+    expect(
+      splitViewFieldPositionUpdates([
+        { universalIdentifier: 'subject', position: 1 },
+        { universalIdentifier: 'name', position: -1 },
+      ]).lowest,
+    ).toEqual([{ universalIdentifier: 'name', position: -1 }]);
+  });
+
+  it('should pass a single update straight through', () => {
+    expect(
+      splitViewFieldPositionUpdates([
+        { universalIdentifier: 'subject', position: 1 },
+      ]),
+    ).toEqual({
+      others: [],
+      lowest: [{ universalIdentifier: 'subject', position: 1 }],
+    });
+  });
+
+  it('should return empty batches for no updates', () => {
+    expect(splitViewFieldPositionUpdates([])).toEqual({
+      others: [],
+      lowest: [],
+    });
+  });
+
+  it('should never leave a column below the held-back one in the first batch', () => {
+    const { others, lowest } = splitViewFieldPositionUpdates([
+      { universalIdentifier: 'a', position: 3 },
+      { universalIdentifier: 'b', position: -2 },
+      { universalIdentifier: 'c', position: 0 },
+    ]);
+
+    others.forEach(({ position }) => {
+      expect(position).toBeGreaterThan(lowest[0].position);
+    });
+  });
+});
@@ -0,0 +1,53 @@
+type ViewFieldPosition = {
+  universalIdentifier: string;
+  position: number;
+};
+
+// The label identifier's column has to sit strictly below every other column of
+// its view. Placing the incoming column below the current lowest position
+// satisfies that once but does not survive: viewField.position is compared by
+// the standard-application sync, so a position that differs from the standard
+// one gets pulled back and trips the same validation again.
+//
+// Aligning the view to the standard layout leaves the sync with nothing to do.
+// Columns the standard application does not know about keep their relative
+// order and move above the standard ones, so they cannot tie with the label
+// identifier either.
+export const computeViewFieldPositionsAlignedToStandard = ({
+  existingViewFields,
+  standardPositionByUniversalIdentifier,
+}: {
+  existingViewFields: ViewFieldPosition[];
+  standardPositionByUniversalIdentifier: Record<string, number>;
+}): ViewFieldPosition[] => {
+  const standardPositions = Object.values(standardPositionByUniversalIdentifier);
+
+  if (standardPositions.length === 0) {
+    return [];
+  }
+
+  const highestStandardPosition = Math.max(...standardPositions);
+
+  const customUniversalIdentifiers = existingViewFields
+    .filter(
+      ({ universalIdentifier }) =>
+        standardPositionByUniversalIdentifier[universalIdentifier] === undefined,
+    )
+    .sort((a, b) => a.position - b.position)
+    .map(({ universalIdentifier }) => universalIdentifier);
+
+  return existingViewFields.flatMap(({ universalIdentifier, position }) => {
+    const standardPosition =
+      standardPositionByUniversalIdentifier[universalIdentifier];
+
+    const targetPosition =
+      standardPosition ??
+      highestStandardPosition +
+        1 +
+        customUniversalIdentifiers.indexOf(universalIdentifier);
+
+    return targetPosition === position
+      ? []
+      : [{ universalIdentifier, position: targetPosition }];
+  });
+};
@@ -0,0 +1,32 @@
+type ViewFieldPosition = {
+  universalIdentifier: string;
+  position: number;
+};
+
+// The migration builder validates updates one at a time against optimistic maps
+// it mutates as it goes, so a batch is only valid if every intermediate state is
+// valid too. Moving the column that ends up lowest at the same time as the ones
+// it has to sit below can therefore be rejected: whether it passes depends on
+// which update the builder happens to reach first.
+//
+// Applying the lowest column on its own, after the others have moved up, keeps
+// every intermediate state valid whatever order the builder picks.
+export const splitViewFieldPositionUpdates = <T extends ViewFieldPosition>(
+  positionUpdates: T[],
+): { others: T[]; lowest: T[] } => {
+  if (positionUpdates.length <= 1) {
+    return { others: [], lowest: positionUpdates };
+  }
+
+  const lowestPosition = Math.min(
+    ...positionUpdates.map(({ position }) => position),
+  );
+  const lowestIndex = positionUpdates.findIndex(
+    ({ position }) => position === lowestPosition,
+  );
+
+  return {
+    others: positionUpdates.filter((_, index) => index !== lowestIndex),
+    lowest: [positionUpdates[lowestIndex]],
+  };
+};