[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
This commit is contained in:
Marie
2025-12-29 13:55:20 +01:00
committed by GitHub
parent ee0e2ed854
commit bb18f78f1b
6 changed files with 168 additions and 29 deletions
@@ -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<WorkspaceEntity>,
protected readonly globalWorkspaceOrmManager: GlobalWorkspaceOrmManager,
protected readonly dataSourceService: DataSourceService,
) {
super(workspaceRepository, globalWorkspaceOrmManager, dataSourceService);
}
override async runOnWorkspace({
workspaceId,
options,
dataSource,
}: RunOnWorkspaceArgs): Promise<void> {
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);
}
}
}
@@ -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 {}
@@ -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],
@@ -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,
};
}
@@ -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<number> {
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<number | null> {
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<number | null> {
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);
}
}
@@ -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;
};