From bb18f78f1b970ed9d89af1ac97a2cb3ce30410c6 Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Mon, 29 Dec 2025 13:55:20 +0100 Subject: [PATCH] [Fix] Fix NaN position in notes resulting in not being able to create notes (#16818) Following [discord thread](https://discord.com/channels/1130383047699738754/1453910755387899996/1453910755387899996), reproductible on twenty-eng https://github.com/user-attachments/assets/ae7f363d-87e1-44fa-8fe2-ee78412d62a7 Records positions are computed at record creation, depending on the position arg from the request, being equal to `last`, `first`, or not present. When being equal to last, as it is done when creating a note from the product, the position is calculated using `.maximum()` function which uses postgres' MAX function. If there is a `NaN` value among the list, the MAX will return `NaN` too. So if for some reason there is a NaN somewhere in the position column, all subsequent records being created with last position argument will be created with NaN value. Until [this PR](https://github.com/twentyhq/twenty/pull/16630), where we introduced a validation on position at record creation which throws when NaN is trying to be introduced as a position, this was going silent. (fyi @etiennejouan , not on you at all but for info) Looking into twenty-eng workspace, I found note records with NaN position dating back to august 2025, making it hard to understand and debug why they were introduced with NaN position. So I did not find the real root cause, but I suggest to - update record-position.service to fix the issue for subsequent records that go through this service (which is what is currently broken) - run a command to fix the existing records with NaN position for Notes, as it is where the issue happened for both the user reporting the issue on discord, and us on twenty-eng. So hopefully the problem was limited to Notes --- ...ix-nan-position-values-in-notes.command.ts | 95 +++++++++++++++++++ .../1-15-upgrade-version-command.module.ts | 13 +++ .../upgrade-version-command.module.ts | 2 + .../upgrade.command.ts | 9 ++ .../services/record-position.service.ts | 70 ++++++++------ .../src/engine/utils/sanitize-number.utli.ts | 8 ++ 6 files changed, 168 insertions(+), 29 deletions(-) create mode 100644 packages/twenty-server/src/database/commands/upgrade-version-command/1-15/1-15-fix-nan-position-values-in-notes.command.ts create mode 100644 packages/twenty-server/src/database/commands/upgrade-version-command/1-15/1-15-upgrade-version-command.module.ts create mode 100644 packages/twenty-server/src/engine/utils/sanitize-number.utli.ts diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/1-15/1-15-fix-nan-position-values-in-notes.command.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/1-15/1-15-fix-nan-position-values-in-notes.command.ts new file mode 100644 index 00000000000..16f8cdc154f --- /dev/null +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/1-15/1-15-fix-nan-position-values-in-notes.command.ts @@ -0,0 +1,95 @@ +import { Logger } from '@nestjs/common'; +import { InjectRepository } from '@nestjs/typeorm'; + +import { Command } from 'nest-commander'; +import { isDefined } from 'twenty-shared/utils'; +import { Repository } from 'typeorm'; + +import { ActiveOrSuspendedWorkspacesMigrationCommandRunner } from 'src/database/commands/command-runners/active-or-suspended-workspaces-migration.command-runner'; +import { RunOnWorkspaceArgs } from 'src/database/commands/command-runners/workspaces-migration.command-runner'; +import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; +import { DataSourceService } from 'src/engine/metadata-modules/data-source/data-source.service'; +import { GlobalWorkspaceOrmManager } from 'src/engine/twenty-orm/global-workspace-datasource/global-workspace-orm.manager'; + +@Command({ + name: 'upgrade:1-15:fix-nan-position-values-in-notes', + description: 'Fix NaN position values in notes by replacing them with 2', +}) +export class FixNanPositionValuesInNotesCommand extends ActiveOrSuspendedWorkspacesMigrationCommandRunner { + protected readonly logger = new Logger( + FixNanPositionValuesInNotesCommand.name, + ); + + constructor( + @InjectRepository(WorkspaceEntity) + protected readonly workspaceRepository: Repository, + protected readonly globalWorkspaceOrmManager: GlobalWorkspaceOrmManager, + protected readonly dataSourceService: DataSourceService, + ) { + super(workspaceRepository, globalWorkspaceOrmManager, dataSourceService); + } + + override async runOnWorkspace({ + workspaceId, + options, + dataSource, + }: RunOnWorkspaceArgs): Promise { + const isDryRun = options.dryRun || false; + + if (!isDefined(dataSource)) { + throw new Error( + `Could not find data source for workspace ${workspaceId}, should never occur`, + ); + } + + if (isDryRun) { + this.logger.log('Dry run mode: No changes will be applied'); + } + + try { + // Count NaN position values in notes + const noteRepository = await this.globalWorkspaceOrmManager.getRepository( + workspaceId, + 'note', + { shouldBypassPermissionChecks: true }, + ); + + const nanCount = await noteRepository.count({ + where: { position: 'NaN' }, + }); + + this.logger.log( + `Found ${nanCount} note(s) with NaN position values in workspace ${workspaceId}`, + ); + + if (nanCount === 0) { + this.logger.log('No NaN position values to fix'); + + return; + } + + if (!isDryRun) { + // Update NaN position values to 2 because 1 is first position + await noteRepository + .createQueryBuilder() + .update() + .set({ position: 2 }) + .where('position = :nanString', { nanString: 'NaN' }) + .execute(); + + this.logger.log( + `Fixed ${nanCount} NaN position value(s) in notes for workspace ${workspaceId}`, + ); + } else { + this.logger.log( + `DRY RUN: Would fix ${nanCount} NaN position value(s) in notes for workspace ${workspaceId}`, + ); + } + } catch (error) { + this.logger.error( + `Could not fix NaN position values in notes for workspace ${workspaceId}`, + ); + this.logger.error(error); + } + } +} diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/1-15/1-15-upgrade-version-command.module.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/1-15/1-15-upgrade-version-command.module.ts new file mode 100644 index 00000000000..d307e34a10f --- /dev/null +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/1-15/1-15-upgrade-version-command.module.ts @@ -0,0 +1,13 @@ +import { Module } from '@nestjs/common'; +import { TypeOrmModule } from '@nestjs/typeorm'; + +import { FixNanPositionValuesInNotesCommand } from 'src/database/commands/upgrade-version-command/1-15/1-15-fix-nan-position-values-in-notes.command'; +import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; +import { DataSourceModule } from 'src/engine/metadata-modules/data-source/data-source.module'; + +@Module({ + imports: [TypeOrmModule.forFeature([WorkspaceEntity]), DataSourceModule], + providers: [FixNanPositionValuesInNotesCommand], + exports: [FixNanPositionValuesInNotesCommand], +}) +export class V1_15_UpgradeVersionCommandModule {} diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade-version-command.module.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade-version-command.module.ts index 34b83c0ce47..cfd84755464 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade-version-command.module.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade-version-command.module.ts @@ -3,6 +3,7 @@ import { TypeOrmModule } from '@nestjs/typeorm'; import { V1_13_UpgradeVersionCommandModule } from 'src/database/commands/upgrade-version-command/1-13/1-13-upgrade-version-command.module'; import { V1_14_UpgradeVersionCommandModule } from 'src/database/commands/upgrade-version-command/1-14/1-14-upgrade-version-command.module'; +import { V1_15_UpgradeVersionCommandModule } from 'src/database/commands/upgrade-version-command/1-15/1-15-upgrade-version-command.module'; import { UpgradeCommand } from 'src/database/commands/upgrade-version-command/upgrade.command'; import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; import { DataSourceModule } from 'src/engine/metadata-modules/data-source/data-source.module'; @@ -12,6 +13,7 @@ import { DataSourceModule } from 'src/engine/metadata-modules/data-source/data-s TypeOrmModule.forFeature([WorkspaceEntity]), V1_13_UpgradeVersionCommandModule, V1_14_UpgradeVersionCommandModule, + V1_15_UpgradeVersionCommandModule, DataSourceModule, ], providers: [UpgradeCommand], diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts index e2e15d3f873..a88fe866f02 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts @@ -18,6 +18,7 @@ import { RenameIndexNameCommand } from 'src/database/commands/upgrade-version-co import { UpdateRoleTargetsUniqueConstraintMigrationCommand } from 'src/database/commands/upgrade-version-command/1-13/1-13-update-role-targets-unique-constraint-migration.command'; import { DeleteRemovedAgentsCommand } from 'src/database/commands/upgrade-version-command/1-14/1-14-delete-removed-agents.command'; import { UpdateCreatedByEnumCommand } from 'src/database/commands/upgrade-version-command/1-14/1-14-update-created-by-enum.command'; +import { FixNanPositionValuesInNotesCommand } from 'src/database/commands/upgrade-version-command/1-15/1-15-fix-nan-position-values-in-notes.command'; import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; import { DataSourceService } from 'src/engine/metadata-modules/data-source/data-source.service'; @@ -49,6 +50,9 @@ export class UpgradeCommand extends UpgradeCommandRunner { // 1.14 Commands protected readonly updateCreatedByEnumCommand: UpdateCreatedByEnumCommand, protected readonly deleteRemovedAgentsCommand: DeleteRemovedAgentsCommand, + + // 1.15 Commands + protected readonly fixNanPositionValuesInNotesCommand: FixNanPositionValuesInNotesCommand, ) { super( workspaceRepository, @@ -75,10 +79,15 @@ export class UpgradeCommand extends UpgradeCommandRunner { this.deleteRemovedAgentsCommand, ]; + const commands_1150: VersionCommands = [ + this.fixNanPositionValuesInNotesCommand, + ]; + this.allCommands = { '1.12.0': commands_1120, '1.13.0': commands_1130, '1.14.0': commands_1140, + '1.15.0': commands_1150, }; } diff --git a/packages/twenty-server/src/engine/core-modules/record-position/services/record-position.service.ts b/packages/twenty-server/src/engine/core-modules/record-position/services/record-position.service.ts index d1b8774088e..cc102cd3c4d 100644 --- a/packages/twenty-server/src/engine/core-modules/record-position/services/record-position.service.ts +++ b/packages/twenty-server/src/engine/core-modules/record-position/services/record-position.service.ts @@ -6,6 +6,7 @@ import { isDefined } from 'twenty-shared/utils'; import { GlobalWorkspaceOrmManager } from 'src/engine/twenty-orm/global-workspace-datasource/global-workspace-orm.manager'; import { buildSystemAuthContext } from 'src/engine/twenty-orm/utils/build-system-auth-context.util'; +import { sanitizeNumber } from 'src/engine/utils/sanitize-number.utli'; export type RecordPositionServiceCreateArgs = { value: number | 'first' | 'last'; @@ -26,7 +27,7 @@ export class RecordPositionService { workspaceId, index = 0, }: RecordPositionServiceCreateArgs): Promise { - if (typeof value === 'number') { + if (isNumber(value) && !Number.isNaN(value)) { return value; } @@ -96,13 +97,18 @@ export class RecordPositionService { const numericPositions = recordsWithExistingNumberPosition .map((record) => record.position) - .filter(isNumber); + .filter((position) => isNumber(position) && !Number.isNaN(position)); const calculatePosition = ( mathOperation: (positions: number[], existingPosition: number) => number, existingPosition: number | null, ): number => { - const fallback = isDefined(existingPosition) ? existingPosition : 1; + const sanitizedExistingPosition = + isDefined(existingPosition) && !Number.isNaN(existingPosition) + ? existingPosition + : null; + + const fallback = sanitizedExistingPosition ?? 1; return numericPositions.length > 0 ? mathOperation(numericPositions, fallback) @@ -208,20 +214,23 @@ export class RecordPositionService { ): Promise { const authContext = buildSystemAuthContext(workspaceId); - return this.globalWorkspaceOrmManager.executeInWorkspaceContext( - authContext, - async () => { - const repository = await this.globalWorkspaceOrmManager.getRepository( - workspaceId, - objectMetadata.nameSingular, - { - shouldBypassPermissionChecks: true, - }, - ); + const result = + await this.globalWorkspaceOrmManager.executeInWorkspaceContext( + authContext, + async () => { + const repository = await this.globalWorkspaceOrmManager.getRepository( + workspaceId, + objectMetadata.nameSingular, + { + shouldBypassPermissionChecks: true, + }, + ); - return await repository.minimum('position'); - }, - ); + return await repository.minimum('position'); + }, + ); + + return sanitizeNumber(result); } private async findMaxPosition( @@ -230,19 +239,22 @@ export class RecordPositionService { ): Promise { const authContext = buildSystemAuthContext(workspaceId); - return this.globalWorkspaceOrmManager.executeInWorkspaceContext( - authContext, - async () => { - const repository = await this.globalWorkspaceOrmManager.getRepository( - workspaceId, - objectMetadata.nameSingular, - { - shouldBypassPermissionChecks: true, - }, - ); + const result = + await this.globalWorkspaceOrmManager.executeInWorkspaceContext( + authContext, + async () => { + const repository = await this.globalWorkspaceOrmManager.getRepository( + workspaceId, + objectMetadata.nameSingular, + { + shouldBypassPermissionChecks: true, + }, + ); - return await repository.maximum('position'); - }, - ); + return await repository.maximum('position'); + }, + ); + + return sanitizeNumber(result); } } diff --git a/packages/twenty-server/src/engine/utils/sanitize-number.utli.ts b/packages/twenty-server/src/engine/utils/sanitize-number.utli.ts new file mode 100644 index 00000000000..cf9da32778e --- /dev/null +++ b/packages/twenty-server/src/engine/utils/sanitize-number.utli.ts @@ -0,0 +1,8 @@ +import { isDefined } from 'twenty-shared/utils'; +export const sanitizeNumber = (value: number | null): number | null => { + if (!isDefined(value) || Number.isNaN(value)) { + return null; + } + + return value; +};