Fix unique standard field #16371

etiennejouan · 2025-12-05T17:40:35Z

update field metadata update logic
uniformize the way index are named
command to migrate v1-named unique index
add integration testing

sentry · 2025-12-05T17:42:13Z

.../metadata-modules/flat-field-metadata/utils/handle-index-changes-during-field-update.util.ts

    return {
-      ...FIELD_METADATA_UPDATE_INDEX_SIDE_EFFECT,
-      flatIndexMetadatasToDelete: uniqueIndexToDelete
-        ? [uniqueIndexToDelete]
-        : [],
+      status: 'success',
+      result: {
+        ...FIELD_METADATA_UPDATE_INDEX_SIDE_EFFECT,
+        flatIndexMetadatasToDelete: uniqueIndexToDelete
+          ? [uniqueIndexToDelete]
+          : [],
+      },
    };
  }
  const updatedIndexes = recomputeIndexOnFlatFieldMetadataNameUpdate({


Bug: Validation for deleting unique indexes fails when applicationId is null for legacy indexes.
_{Severity: CRITICAL | Confidence: High}

🔍 Detailed Analysis

The validation logic at uniqueIndexToDelete.applicationId !== workspaceCustomApplicationId incorrectly evaluates to true when uniqueIndexToDelete.applicationId is null. This occurs for legacy unique indexes because applicationId is nullable and was not populated by the 1-13-rename-unique-index.command.ts migration. This prevents legitimate field updates that attempt to remove uniqueness constraints, causing the operation to fail with the error: "Cannot delete unique index that have not been created by the workspace custom application".

💡 Suggested Fix

Modify the validation to explicitly handle null applicationId values, either by allowing deletion for null values or by checking isDefined(uniqueIndexToDelete.applicationId) before comparison.

🤖 Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: packages/twenty-server/src/engine/metadata-modules/flat-field-metadata/utils/handle-index-changes-during-field-update.util.ts#L152-L182 Potential issue: The validation logic at `uniqueIndexToDelete.applicationId !== workspaceCustomApplicationId` incorrectly evaluates to `true` when `uniqueIndexToDelete.applicationId` is `null`. This occurs for legacy unique indexes because `applicationId` is nullable and was not populated by the `1-13-rename-unique-index.command.ts` migration. This prevents legitimate field updates that attempt to remove uniqueness constraints, causing the operation to fail with the error: "Cannot delete unique index that have not been created by the workspace custom application".

_{Did we get this right? 👍 / 👎 to inform future reviews.}
_{Reference ID: 5856349}

This is true, not all entities have the applicationId set already
We might be checking the isCustom as a fallback assertion

greptile-apps · 2025-12-05T17:48:26Z

Greptile Overview

Greptile Summary

This PR successfully fixes the bug where standard fields in custom objects could not be set as unique. The fix involves three main changes:

Core Logic Updates

Added isUnique to the list of editable properties for standard fields in flat-field-metadata-editable-properties.constant.ts:24
Updated index change handlers to return FieldInputTranspilationResult types for proper error propagation
Added validation to prevent deletion of standard unique indexes (those created by the standard application) when setting isUnique to false

Index Naming Standardization

Introduced generateDeterministicIndexNameV2 that includes a UNIQUE_ prefix in unique index names (format: IDX_UNIQUE_{hash} vs IDX_{hash})
Updated all index generation points (WorkspaceIsUnique decorator, StandardIndexFactory, etc.) to use the new naming convention
Created migration command 1-13-rename-unique-index.command.ts to rename existing unique indexes to the new format

Testing

Added comprehensive integration tests covering both successful scenarios (setting isUnique on standard fields in custom objects) and failure scenarios (attempting to remove standard unique constraints)

The implementation correctly distinguishes between standard indexes (created by the platform) and custom indexes (created by users), preventing users from breaking platform constraints while allowing flexibility for custom objects.

Confidence Score: 4/5

This PR is safe to merge with minor considerations around error message clarity
The implementation is solid with proper validation, comprehensive tests, and a migration path. Score reduced from 5 to 4 due to one grammar issue in an error message that should be fixed for user-facing quality
Pay attention to handle-index-changes-during-field-update.util.ts:165 for the grammar issue in the error message

Important Files Changed

File Analysis

Filename	Score	Overview
packages/twenty-server/src/engine/metadata-modules/flat-field-metadata/constants/flat-field-metadata-editable-properties.constant.ts	5/5	Added `isUnique` to standard field editable properties, enabling unique constraint modification on standard fields in custom objects
packages/twenty-server/src/engine/metadata-modules/flat-field-metadata/utils/handle-index-changes-during-field-update.util.ts	4/5	Changed return type to `FieldInputTranspilationResult` and added validation to prevent deletion of standard unique indexes
packages/twenty-server/src/engine/metadata-modules/flat-field-metadata/utils/handle-flat-field-metadata-update-side-effect.util.ts	5/5	Updated to handle error propagation from index changes, properly wrapping results in `FieldInputTranspilationResult`
packages/twenty-server/src/database/commands/upgrade-version-command/1-13/1-13-rename-unique-index.command.ts	5/5	Migration command to rename existing unique indexes to include UNIQUE in their name for consistency
packages/twenty-server/test/integration/metadata/suites/field-metadata/successful-update-one-standard-field-metadata.integration-spec.ts	5/5	Added comprehensive tests for setting `isUnique` on standard fields in custom objects, including toggling between true and false

Sequence Diagram

sequenceDiagram
    participant User
    participant GraphQL
    participant FieldMetadataService
    participant UpdateUtil as fromUpdateFieldInputToFlatFieldMetadata
    participant SideEffectUtil as handleFlatFieldMetadataUpdateSideEffect
    participant IndexUtil as handleIndexChangesDuringFieldUpdate
    participant MigrationService as WorkspaceMigrationService
    participant Database

    User->>GraphQL: updateOneField(isUnique: true)
    GraphQL->>FieldMetadataService: updateOneField()
    FieldMetadataService->>FieldMetadataService: Fetch existing metadata from cache
    FieldMetadataService->>UpdateUtil: fromUpdateFieldInputToFlatFieldMetadata()
    
    UpdateUtil->>UpdateUtil: Extract and sanitize input
    UpdateUtil->>UpdateUtil: Check editable properties (isUnique now allowed for standard fields)
    UpdateUtil->>UpdateUtil: computeFlatFieldToUpdateAndRelatedFlatFieldToUpdate()
    
    UpdateUtil->>SideEffectUtil: handleFlatFieldMetadataUpdateSideEffect()
    SideEffectUtil->>IndexUtil: handleIndexChangesDuringFieldUpdate()
    
    alt isUnique changed to true
        IndexUtil->>IndexUtil: Check if index exists
        alt No existing index
            IndexUtil->>IndexUtil: generateIndexForFlatFieldMetadata()
            IndexUtil->>IndexUtil: Generate name with UNIQUE prefix (v2)
            IndexUtil-->>SideEffectUtil: Return flatIndexMetadatasToCreate
        end
    else isUnique changed to false
        IndexUtil->>IndexUtil: Find unique index to delete
        IndexUtil->>IndexUtil: Check if index.applicationId == workspaceCustomApplicationId
        alt Index from standard application
            IndexUtil-->>SideEffectUtil: Return error (cannot delete standard index)
        else Index from custom application
            IndexUtil-->>SideEffectUtil: Return flatIndexMetadatasToDelete
        end
    end
    
    SideEffectUtil-->>UpdateUtil: Return side effects with index changes
    UpdateUtil-->>FieldMetadataService: Return flatFieldMetadatasToUpdate + index operations
    
    FieldMetadataService->>MigrationService: validateBuildAndRunWorkspaceMigration()
    MigrationService->>Database: Create/delete indexes
    MigrationService->>Database: Update field metadata
    Database-->>MigrationService: Success
    MigrationService-->>FieldMetadataService: Success
    
    FieldMetadataService->>FieldMetadataService: Recompute cache
    FieldMetadataService-->>GraphQL: Return updated field metadata
    GraphQL-->>User: Field updated with isUnique constraint

greptile-apps

_{21 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2025-12-05T17:48:25Z

.../metadata-modules/flat-field-metadata/utils/handle-index-changes-during-field-update.util.ts

+          {
+            code: FieldMetadataExceptionCode.INVALID_FIELD_INPUT,
+            message:
+              'Cannot delete unique index that have not been created by the workspace custom application',


syntax: Grammar: "have not been created" should be "has not been created"

Suggested change

'Cannot delete unique index that have not been created by the workspace custom application',

'Cannot delete unique index that has not been created by the workspace custom application',

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/twenty-server/src/engine/metadata-modules/flat-field-metadata/utils/handle-index-changes-during-field-update.util.ts Line: 165:165 Comment: **syntax:** Grammar: "have not been created" should be "has not been created" ```suggestion 'Cannot delete unique index that has not been created by the workspace custom application', ``` How can I resolve this? If you propose a fix, please make it concise.

github-actions · 2025-12-05T17:53:05Z

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:16740

This environment will automatically shut down when the PR is closed or after 5 hours.

prastoin

Good job ! We might wanna update the frontend to also be comparing the index applicationId to the workspaceCustomApplicationId in order to disable dyanmically the input instead of checking the isCustom boolean
That's polishing

prastoin · 2025-12-08T12:57:21Z

...erver/src/database/commands/upgrade-version-command/1-13/1-13-rename-unique-index.command.ts

+    const uniqueIndexes = await this.indexMetadataRepository.find({
+      where: {
+        workspaceId,
+        isUnique: true,
+      },
+      relations: ['objectMetadata', 'indexFieldMetadatas.fieldMetadata'],
+    });
+


Suggestion: We could use workspaceCacheService directly here, might be costy to re-construction field relation from maps though TBD

...erver/src/database/commands/upgrade-version-command/1-13/1-13-rename-unique-index.command.ts

prastoin · 2025-12-08T13:01:39Z

...ta-modules/flat-field-metadata/constants/flat-field-metadata-editable-properties.constant.ts

    'label',
    'options',
    'settings',
+    'isUnique', // not editable for standard fields with standard unique constraint


At least from the api metadata

prastoin · 2025-12-08T13:09:05Z

.../metadata-modules/flat-field-metadata/utils/handle-index-changes-during-field-update.util.ts

    return {
-      ...FIELD_METADATA_UPDATE_INDEX_SIDE_EFFECT,
-      flatIndexMetadatasToDelete: uniqueIndexToDelete
-        ? [uniqueIndexToDelete]
-        : [],
+      status: 'success',
+      result: {
+        ...FIELD_METADATA_UPDATE_INDEX_SIDE_EFFECT,
+        flatIndexMetadatasToDelete: uniqueIndexToDelete
+          ? [uniqueIndexToDelete]
+          : [],
+      },
    };
  }
  const updatedIndexes = recomputeIndexOnFlatFieldMetadataNameUpdate({


This is true, not all entities have the applicationId set already
We might be checking the isCustom as a fallback assertion

packages/twenty-server/src/engine/twenty-orm/decorators/workspace-field-index.decorator.ts

prastoin · 2025-12-08T13:10:38Z

...etadata/suites/field-metadata/failing-update-one-standard-field-metadata.integration-spec.ts

  );
 });
+
+describe('Standard field with standard unique index update should fail on isUnique change', () => {


...data/suites/field-metadata/successful-update-one-standard-field-metadata.integration-spec.ts

prastoin · 2025-12-08T13:13:01Z

...data/suites/field-metadata/successful-update-one-standard-field-metadata.integration-spec.ts

+    expect(data.updateOneField.isUnique).toBe(true);
+  });
+
+  it('should set isUnique back to false on standard field', async () => {


Remark: That's a bit of a test leakage, I would rather handle that in an after jest hook
Because if you only run the first test the isUnique value is not updated restored which could have an impact on other tests

...twenty-server/test/integration/metadata/utils/warn-if-error-but-not-expected-to-fail.util.ts

twenty-eng-sync · 2025-12-08T18:05:48Z

Hey @etiennejouan! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

etiennejouan and others added 4 commits December 5, 2025 11:17

wip

7bec427

refactor(server): field input transpilation result

1cd7c05

feat(server): start refactoring side effect error handler

eb2c651

update

09f964f

etiennejouan self-assigned this Dec 5, 2025

etiennejouan requested a review from prastoin December 5, 2025 17:40

etiennejouan added the -PR: awaiting review label Dec 5, 2025

sentry bot reviewed Dec 5, 2025

View reviewed changes

greptile-apps bot reviewed Dec 5, 2025

View reviewed changes

etiennejouan added 2 commits December 8, 2025 09:42

Merge branch 'main' into ej/15925-fix-unique-standard-field

5bb8562

fix

ef89962

prastoin reviewed Dec 8, 2025

View reviewed changes

etiennejouan added 4 commits December 8, 2025 16:09

fix

6f4f223

Merge branch 'main' into ej/15925-fix-unique-standard-field

002240d

fix

a437b69

fix

f5de268

prastoin approved these changes Dec 8, 2025

View reviewed changes

fix

d24c534

etiennejouan enabled auto-merge (squash) December 8, 2025 17:54

etiennejouan merged commit 07cfaa7 into main Dec 8, 2025
53 checks passed

etiennejouan deleted the ej/15925-fix-unique-standard-field branch December 8, 2025 18:05

	'Cannot delete unique index that have not been created by the workspace custom application',
	'Cannot delete unique index that has not been created by the workspace custom application',

Fix unique standard field #16371

Fix unique standard field #16371

Uh oh!

Conversation

etiennejouan commented Dec 5, 2025

Uh oh!

sentry bot Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

prastoin Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Dec 5, 2025

Greptile Overview

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prastoin left a comment

Choose a reason for hiding this comment

Uh oh!

prastoin Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

prastoin Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

prastoin Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

prastoin Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

prastoin Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

twenty-eng-sync bot commented Dec 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Dec 5, 2025 •

edited

Loading