Handle field isNullable update (#22362)

## Context Setting isNullable on a field via the app SDK manifest was silently ignored when re-syncing an existing field. The first sync that creates a field honored isNullable correctly, but any later manifest change to isNullable had no effect, neither on the field metadata nor on the underlying Postgres column. Two compounding gaps caused this: The diff never detected the change. isNullable was configured with toCompare: false, so compareTwoFlatEntity excluded it from the diff and no update action was ever generated. There was no DDL to apply it. Even if detected, the update field action handler only altered name, options, defaultValue, and settings. The column manager had no way to alter a column's NOT NULL constraint. ## Fix - Set isNullable.toCompare: true so manifest changes are detected and persisted to the field metadata (via the existing executeForMetadata path). - Add WorkspaceSchemaColumnManagerService.alterColumnNullable(): emits SET NOT NULL / DROP NOT NULL, with an optional pre-serialized backfill (UPDATE … WHERE col IS NULL) applied only on the nullable → non-nullable transition. - Add handleFieldNullableUpdate() to the update field action handler, dispatched after the defaultValue block so the default is in place before NOT NULL is enforced. It is composite-aware (mirrors the per-sub-column parentIsNullable || !property.isRequired rule used at column creation) and skips relation/morph join columns and TS_VECTOR, which are always nullable by design.  <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22362?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
2026-07-01 13:31:22 +02:00
parent 49a80c72d2
commit fab0358df5
9 changed files with 323 additions and 2 deletions
@@ -97,6 +97,27 @@ Unquoted strings are reserved for computed defaults, evaluated when a record is

 The same convention applies to string sub-fields of composite defaults (e.g. `{ source: "'MANUAL'" }` on an `ACTOR` field) and to `SELECT`/`MULTI_SELECT` values. A literal string default left unquoted raises a warning when your app is built.

+## Nullability
+
+`isNullable` controls whether a field accepts `NULL`. It defaults to `true` — omit it for optional fields. Set `isNullable: false` to make a field required at the database level.
+
+Changes to `isNullable` are applied on every sync, including syncs that update an existing field — so you can flip a field's nullability by editing the manifest and re-syncing.
+
+<Note>
+**Making an existing field non-nullable requires a default value.** When you change a field to `isNullable: false`, you must also provide a non-null `defaultValue`. The default backfills any existing `NULL` rows before the `NOT NULL` constraint is applied; without it the sync fails with `Default value cannot be null for non-nullable fields`. Relation fields and `TS_VECTOR` fields are always nullable, so `isNullable` has no effect on them.
+</Note>
+
+```ts
+{
+  universalIdentifier: 'b1a7c0de-1234-4f00-9abc-000000000000',
+  name: 'reference',
+  type: FieldType.TEXT,
+  label: 'Reference',
+  isNullable: false,
+  defaultValue: "'N/A'",
+}
+```
+
 ## What's next

 - **Connect this object to others** — see [Relations](/developers/extend/apps/data/relations) for the bidirectional relation pattern.
@@ -75,6 +75,7 @@ exports[`ALL_UNIVERSAL_FLAT_ENTITY_PROPERTIES_TO_COMPARE_AND_STRINGIFY should ma
      "standardOverrides",
      "universalSettings",
      "isUIEditable",
+      "isNullable",
    ],
    "propertiesToStringify": [
      "defaultValue",
@@ -124,7 +124,7 @@ export const ALL_ENTITY_PROPERTIES_CONFIGURATION_BY_METADATA_NAME = {
      universalProperty: undefined,
    },
    isNullable: {
-      toCompare: false,
+      toCompare: true,
      toStringify: false,
      universalProperty: undefined,
    },
@@ -23,6 +23,7 @@ type Assertions = [
      | 'isUnique'
      | 'isLabelSyncedWithName'
      | 'isUIEditable'
+      | 'isNullable'
      | 'universalSettings'
    >
  >,
@@ -104,4 +104,37 @@ export class WorkspaceSchemaColumnManagerService {

    await queryRunner.query(sql);
  }
+
+  async alterColumnNullable({
+    queryRunner,
+    schemaName,
+    tableName,
+    columnName,
+    isNullable,
+    backfillValue,
+  }: {
+    queryRunner: QueryRunner;
+    schemaName: string;
+    tableName: string;
+    columnName: string;
+    isNullable: boolean;
+    backfillValue?: string;
+  }): Promise<void> {
+    const tableRef = `${escapeIdentifier(schemaName)}.${escapeIdentifier(tableName)}`;
+    const columnRef = escapeIdentifier(columnName);
+
+    if (
+      !isNullable &&
+      backfillValue !== undefined &&
+      backfillValue !== 'NULL'
+    ) {
+      await queryRunner.query(
+        `UPDATE ${tableRef} SET ${columnRef} = ${backfillValue} WHERE ${columnRef} IS NULL`,
+      );
+    }
+
+    await queryRunner.query(
+      `ALTER TABLE ${tableRef} ALTER COLUMN ${columnRef} ${isNullable ? 'DROP NOT NULL' : 'SET NOT NULL'}`,
+    );
+  }
 }
@@ -18,6 +18,7 @@ type Assertions = [
      | 'isUnique'
      | 'isLabelSyncedWithName'
      | 'isUIEditable'
+      | 'isNullable'
      | 'universalSettings'
    >
  >,
@@ -41,6 +41,7 @@ type Assertions = [
      | 'isUnique'
      | 'isLabelSyncedWithName'
      | 'isUIEditable'
+      | 'isNullable'
      | 'universalSettings'
    >
  >,
@@ -76,6 +76,13 @@ type DefaultValueUpdateHandlerArgs<
  toDefaultValue: FlatFieldMetadata['defaultValue'];
 };

+type NullableUpdateHandlerArgs = Omit<
+  UpdateFieldPropertyHandlerArgs,
+  'update'
+> & {
+  toIsNullable: boolean;
+};
+
 type OptionsUpdateHandlerArgs<T extends FieldMetadataType = FieldMetadataType> =
  UpdateFieldPropertyHandlerArgs<T> & {
    toOptions: FlatFieldMetadata['options'];
@@ -248,6 +255,19 @@ export class UpdateFieldActionHandlerService extends WorkspaceMigrationRunnerAct
      }
    }

+    if (update.isNullable !== undefined) {
+      const toIsNullable = update.isNullable ?? true;
+
+      await this.handleFieldNullableUpdate({
+        queryRunner,
+        schemaName,
+        tableName,
+        flatFieldMetadata: optimisticFlatFieldMetadata,
+        toIsNullable,
+      });
+      optimisticFlatFieldMetadata.isNullable = toIsNullable;
+    }
+
    if (isDefined(update.settings)) {
      // Handle onDelete change (for morph/relation fields) order matters
      if (isMorphOrRelationFlatFieldMetadata(optimisticFlatFieldMetadata)) {
@@ -547,6 +567,86 @@ export class UpdateFieldActionHandlerService extends WorkspaceMigrationRunnerAct
    );
  }

+  private async handleFieldNullableUpdate({
+    flatFieldMetadata,
+    queryRunner,
+    schemaName,
+    tableName,
+    toIsNullable,
+  }: NullableUpdateHandlerArgs) {
+    if (
+      isMorphOrRelationFlatFieldMetadata(flatFieldMetadata) ||
+      isFlatFieldMetadataOfType(flatFieldMetadata, FieldMetadataType.TS_VECTOR)
+    ) {
+      return;
+    }
+
+    if (isCompositeFlatFieldMetadata(flatFieldMetadata)) {
+      const compositeType = getCompositeTypeOrThrow(flatFieldMetadata.type);
+
+      for (const property of compositeType.properties) {
+        if (isMorphOrRelationFieldMetadataType(property.type)) {
+          throw new WorkspaceMigrationActionExecutionException({
+            message:
+              'Relation field metadata in composite type is not supported yet',
+            code: WorkspaceMigrationActionExecutionExceptionCode.NOT_SUPPORTED,
+          });
+        }
+
+        const compositeColumnName = computeCompositeColumnName(
+          flatFieldMetadata.name,
+          property,
+        );
+        const propertyIsNullable = toIsNullable || !property.isRequired;
+        const fieldDefaultValue = flatFieldMetadata.defaultValue;
+        // @ts-expect-error - composite default value is keyed by property name
+        const compositeDefaultValue = fieldDefaultValue?.[property.name];
+
+        await this.workspaceSchemaManagerService.columnManager.alterColumnNullable(
+          {
+            queryRunner,
+            schemaName,
+            tableName,
+            columnName: compositeColumnName,
+            isNullable: propertyIsNullable,
+            backfillValue: propertyIsNullable
+              ? undefined
+              : serializeDefaultValue({
+                  columnName: compositeColumnName,
+                  schemaName,
+                  tableName,
+                  columnType: fieldMetadataTypeToColumnType(
+                    property.type,
+                  ) as ColumnType,
+                  defaultValue: compositeDefaultValue,
+                }),
+          },
+        );
+      }
+
+      return;
+    }
+
+    await this.workspaceSchemaManagerService.columnManager.alterColumnNullable({
+      queryRunner,
+      schemaName,
+      tableName,
+      columnName: flatFieldMetadata.name,
+      isNullable: toIsNullable,
+      backfillValue: toIsNullable
+        ? undefined
+        : serializeDefaultValue({
+            columnName: flatFieldMetadata.name,
+            schemaName,
+            tableName,
+            columnType: fieldMetadataTypeToColumnType(
+              flatFieldMetadata.type,
+            ) as ColumnType,
+            defaultValue: flatFieldMetadata.defaultValue,
+          }),
+    });
+  }
+
  private async handleFieldOptionsUpdate({
    flatFieldMetadata,
    queryRunner,
@@ -3,15 +3,21 @@ import { buildDefaultObjectManifest } from 'test/integration/metadata/suites/app
 import { cleanupApplicationAndAppRegistration } from 'test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util';
 import { setupApplicationForSync } from 'test/integration/metadata/suites/application/utils/setup-application-for-sync.util';
 import { syncApplication } from 'test/integration/metadata/suites/application/utils/sync-application.util';
+import { createOneOperationFactory } from 'test/integration/graphql/utils/create-one-operation-factory.util';
+import { findOneOperationFactory } from 'test/integration/graphql/utils/find-one-operation-factory.util';
+import { makeGraphqlAPIRequest } from 'test/integration/graphql/utils/make-graphql-api-request.util';
+import { findManyObjectMetadata } from 'test/integration/metadata/suites/object-metadata/utils/find-many-object-metadata.util';
 import { findManyObjectMetadataWithIndexes } from 'test/integration/metadata/suites/object-metadata/utils/find-many-object-metadata-with-indexes.util';
-import { type Manifest } from 'twenty-shared/application';
+import { type FieldManifest, type Manifest } from 'twenty-shared/application';
 import { FieldMetadataType } from 'twenty-shared/types';
+import { capitalize, isDefined } from 'twenty-shared/utils';
 import { v4 as uuidv4 } from 'uuid';

 const TEST_APP_ID = uuidv4();
 const TEST_ROLE_ID = uuidv4();
 const TEST_FIELD_ID = uuidv4();
 const TEST_SECOND_FIELD_ID = uuidv4();
+const TEST_NUMBER_FIELD_ID = uuidv4();

 const TEST_OBJECT = buildDefaultObjectManifest({
  nameSingular: 'ticket',
@@ -41,6 +47,94 @@ const findObjectFields = async () => {
  return object?.fieldsList ?? [];
 };

+const findFieldWithNullable = async (fieldName: string) => {
+  const { objects } = await findManyObjectMetadata({
+    expectToFail: false,
+    input: {
+      filter: {},
+      paging: { first: 100 },
+    },
+    gqlFields: `
+        id
+        universalIdentifier
+        fieldsList {
+          name
+          isNullable
+        }
+      `,
+  });
+
+  const object = objects.find(
+    (o) => o.universalIdentifier === TEST_OBJECT.universalIdentifier,
+  );
+
+  return object?.fieldsList?.find((field) => field.name === fieldName);
+};
+
+const buildReferenceFieldManifest = (isNullable: boolean): FieldManifest => ({
+  universalIdentifier: TEST_FIELD_ID,
+  type: FieldMetadataType.TEXT,
+  name: 'reference',
+  label: 'Reference',
+  description: 'Ticket reference',
+  icon: 'IconFileDescription',
+  isNullable,
+  defaultValue: "'N/A'",
+  objectUniversalIdentifier: TEST_OBJECT.universalIdentifier,
+});
+
+// NUMBER is used here (rather than a TEXT field) because the data API
+// coerces null/omitted TEXT values to '' via the null-equivalent processor,
+// so a TEXT column can never actually hold NULL. NUMBER preserves NULL, which
+// is what the nullable -> non-nullable backfill needs to act on.
+const buildEstimateFieldManifest = ({
+  isNullable,
+  defaultValue,
+}: {
+  isNullable: boolean;
+  defaultValue?: number;
+}): FieldManifest => ({
+  universalIdentifier: TEST_NUMBER_FIELD_ID,
+  type: FieldMetadataType.NUMBER,
+  name: 'estimate',
+  label: 'Estimate',
+  description: 'Ticket estimate',
+  icon: 'IconNumber',
+  isNullable,
+  ...(isDefined(defaultValue) ? { defaultValue } : {}),
+  objectUniversalIdentifier: TEST_OBJECT.universalIdentifier,
+});
+
+const createTicketRecord = async (data: Record<string, unknown>) => {
+  const response = await makeGraphqlAPIRequest(
+    createOneOperationFactory({
+      objectMetadataSingularName: TEST_OBJECT.nameSingular,
+      gqlFields: `
+        id
+        estimate
+      `,
+      data,
+    }),
+  );
+
+  return response.body.data?.[`create${capitalize(TEST_OBJECT.nameSingular)}`];
+};
+
+const findTicketRecordById = async (recordId: string) => {
+  const response = await makeGraphqlAPIRequest(
+    findOneOperationFactory({
+      objectMetadataSingularName: TEST_OBJECT.nameSingular,
+      gqlFields: `
+        id
+        estimate
+      `,
+      filter: { id: { eq: recordId } },
+    }),
+  );
+
+  return response.body.data?.[TEST_OBJECT.nameSingular];
+};
+
 describe('Manifest update - fields', () => {
  beforeEach(async () => {
    await setupApplicationForSync({
@@ -240,6 +334,75 @@ describe('Manifest update - fields', () => {
    ).toBeUndefined();
  }, 60000);

+  it('should update isNullable when changed in manifest on second sync', async () => {
+    await syncApplication({
+      manifest: buildManifest({ fields: [buildReferenceFieldManifest(true)] }),
+      expectToFail: false,
+    });
+
+    const fieldAfterFirstSync = await findFieldWithNullable('reference');
+
+    expect(fieldAfterFirstSync).toBeDefined();
+    expect(fieldAfterFirstSync?.isNullable).toBe(true);
+
+    await syncApplication({
+      manifest: buildManifest({ fields: [buildReferenceFieldManifest(false)] }),
+      expectToFail: false,
+    });
+
+    const fieldAfterSecondSync = await findFieldWithNullable('reference');
+
+    expect(fieldAfterSecondSync?.isNullable).toBe(false);
+
+    await syncApplication({
+      manifest: buildManifest({ fields: [buildReferenceFieldManifest(true)] }),
+      expectToFail: false,
+    });
+
+    const fieldAfterThirdSync = await findFieldWithNullable('reference');
+
+    expect(fieldAfterThirdSync?.isNullable).toBe(true);
+  }, 60000);
+
+  it('should backfill existing null rows when a field becomes non-nullable on second sync', async () => {
+    // First sync creates a nullable field with no default.
+    await syncApplication({
+      manifest: buildManifest({
+        fields: [buildEstimateFieldManifest({ isNullable: true })],
+      }),
+      expectToFail: false,
+    });
+
+    // Persist a record whose estimate is NULL on the underlying column.
+    const recordId = uuidv4();
+    const createdRecord = await createTicketRecord({
+      id: recordId,
+      estimate: null,
+    });
+
+    expect(createdRecord?.id).toBe(recordId);
+    expect(createdRecord?.estimate).toBeNull();
+
+    // Second sync makes the field non-nullable with a default value, which
+    // must backfill the existing NULL row before SET NOT NULL is enforced.
+    await syncApplication({
+      manifest: buildManifest({
+        fields: [
+          buildEstimateFieldManifest({ isNullable: false, defaultValue: 42 }),
+        ],
+      }),
+      expectToFail: false,
+    });
+
+    const fieldAfterSecondSync = await findFieldWithNullable('estimate');
+
+    expect(fieldAfterSecondSync?.isNullable).toBe(false);
+
+    const backfilledRecord = await findTicketRecordById(recordId);
+
+    expect(backfilledRecord?.estimate).toBe(42);
+  }, 60000);
+
  it('should create a unique index when field has isUnique set to true', async () => {
    await syncApplication({
      manifest: buildManifest({