feat(shared): require defaultValue on non-nullable field manifests (#22419)
## Context Follow-up to #22362, which made `isNullable` manifest changes actually apply (including a nullable → non-nullable backfill). This models the `isNullable` / `defaultValue` relationship directly in the `FieldManifest` type. ## Rule - A **non-nullable** field (`isNullable: false`) must declare a `defaultValue`, so the column always has a value to fall back on (e.g. for the backfill on the nullable → non-nullable transition). - A **nullable** or **unspecified** field may omit `defaultValue`. ## Changes - Split `RegularFieldManifest` into a base shape plus a discriminated nullability union. The union keeps `isNullable` free once a `defaultValue` is supplied, so helpers that always provide one can still pass a dynamic `boolean` `isNullable`. - `defaultValue` keeps its rich per-type `FieldMetadataDefaultValue<T>` (POSITION → number, ACTOR → composite) rather than a bare `string`. - `RelationFieldManifest` is rebased on the shared base and keeps `isNullable` / `defaultValue` optional, since relation join columns are always nullable by design. - Narrowed `buildEstimateFieldManifest` in the manifest-update integration test to satisfy the stricter type. ## Verification Environment couldn't install the monorepo deps (registry connections aborting), so `nx typecheck` wasn't run here. Validated the union structure with standalone `tsc` synthetic tests mirroring every construction pattern in the codebase: - ✅ nullable/no-default, no-`isNullable`, non-nullable with string/number/composite defaults, dynamic-boolean-with-default, and the `DistributiveOmit` path into `ObjectFieldManifest` - ✅ non-nullable **without** a default is correctly rejected with a clear "defaultValue is missing but required" error Recommend a full `nx typecheck twenty-shared twenty-sdk twenty-server` in CI to confirm against full project resolution. https://claude.ai/code/session_01VnbrgBB3kNGP876qaKPYDL --- _Generated by [Claude Code](https://claude.ai/code/session_01VnbrgBB3kNGP876qaKPYDL)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22419?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> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
+32
-18
@@ -87,23 +87,37 @@ const buildReferenceFieldManifest = (isNullable: boolean): FieldManifest => ({
|
||||
// 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 buildEstimateFieldManifest = (
|
||||
params:
|
||||
| { isNullable: true; defaultValue?: number }
|
||||
| { isNullable: false; defaultValue: number },
|
||||
): FieldManifest => {
|
||||
const commonEstimateFields = {
|
||||
universalIdentifier: TEST_NUMBER_FIELD_ID,
|
||||
type: FieldMetadataType.NUMBER as const,
|
||||
name: 'estimate',
|
||||
label: 'Estimate',
|
||||
description: 'Ticket estimate',
|
||||
icon: 'IconNumber',
|
||||
objectUniversalIdentifier: TEST_OBJECT.universalIdentifier,
|
||||
};
|
||||
|
||||
if (params.isNullable) {
|
||||
return {
|
||||
...commonEstimateFields,
|
||||
isNullable: true,
|
||||
...(isDefined(params.defaultValue)
|
||||
? { defaultValue: params.defaultValue }
|
||||
: {}),
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
...commonEstimateFields,
|
||||
isNullable: false,
|
||||
defaultValue: params.defaultValue,
|
||||
};
|
||||
};
|
||||
|
||||
const createTicketRecord = async (data: Record<string, unknown>) => {
|
||||
const response = await makeGraphqlAPIRequest(
|
||||
@@ -415,7 +429,7 @@ describe('Manifest update - fields', () => {
|
||||
description: 'Unique external identifier',
|
||||
icon: 'IconId',
|
||||
isUnique: true,
|
||||
isNullable: false,
|
||||
isNullable: true,
|
||||
objectUniversalIdentifier: TEST_OBJECT.universalIdentifier,
|
||||
},
|
||||
],
|
||||
|
||||
@@ -7,7 +7,7 @@ import {
|
||||
type RelationAndMorphRelationFieldMetadataType,
|
||||
} from '@/types';
|
||||
|
||||
export type RegularFieldManifest<
|
||||
type BaseRegularFieldManifest<
|
||||
T extends FieldMetadataType = Exclude<
|
||||
FieldMetadataType,
|
||||
RelationAndMorphRelationFieldMetadataType
|
||||
@@ -18,36 +18,37 @@ export type RegularFieldManifest<
|
||||
label: string;
|
||||
description?: string;
|
||||
icon?: string;
|
||||
/**
|
||||
* Default value in the canonical metadata format.
|
||||
*
|
||||
* Literal string defaults must be wrapped in single quotes inside the
|
||||
* string (e.g. `"'Draft'"`), including string sub-fields of composite
|
||||
* defaults (e.g. `{ source: "'MANUAL'" }`) and SELECT/MULTI_SELECT values.
|
||||
* Unquoted strings are reserved for computed defaults such as `'uuid'`
|
||||
* and `'now'`; any other unquoted string raises a validation warning.
|
||||
*/
|
||||
defaultValue?: FieldMetadataDefaultValue<T>;
|
||||
options?: FieldMetadataOptions<T>;
|
||||
universalSettings?: FieldMetadataUniversalSettings<T>;
|
||||
isNullable?: boolean;
|
||||
// When false, this field is not editable through the generic UI
|
||||
isUIEditable?: boolean;
|
||||
/**
|
||||
* @deprecated Use defineIndex({ isUnique: true, fields: [...] }) instead.
|
||||
* Indexes are the SDK primitive for uniqueness — they support both single-
|
||||
* and multi-column unique constraints with a single, consistent API. This
|
||||
* field still works but will be removed in a future release.
|
||||
*/
|
||||
isUnique?: boolean;
|
||||
objectUniversalIdentifier: string;
|
||||
};
|
||||
|
||||
type RegularFieldManifestNullability<T extends FieldMetadataType> =
|
||||
| {
|
||||
defaultValue: FieldMetadataDefaultValue<T>;
|
||||
isNullable?: boolean;
|
||||
}
|
||||
| {
|
||||
defaultValue?: FieldMetadataDefaultValue<T>;
|
||||
isNullable?: true;
|
||||
};
|
||||
|
||||
export type RegularFieldManifest<
|
||||
T extends FieldMetadataType = Exclude<
|
||||
FieldMetadataType,
|
||||
RelationAndMorphRelationFieldMetadataType
|
||||
>,
|
||||
> = BaseRegularFieldManifest<T> & RegularFieldManifestNullability<T>;
|
||||
|
||||
export type RelationFieldManifest<
|
||||
T extends RelationAndMorphRelationFieldMetadataType =
|
||||
RelationAndMorphRelationFieldMetadataType,
|
||||
> = Omit<RegularFieldManifest<T>, 'universalSettings' | 'type'> & {
|
||||
> = Omit<BaseRegularFieldManifest<T>, 'universalSettings' | 'type'> & {
|
||||
type: T;
|
||||
isNullable?: boolean;
|
||||
defaultValue?: FieldMetadataDefaultValue<T>;
|
||||
relationTargetFieldMetadataUniversalIdentifier: string;
|
||||
relationTargetObjectMetadataUniversalIdentifier: string;
|
||||
universalSettings: FieldMetadataUniversalSettings<T>;
|
||||
|
||||
@@ -40,7 +40,6 @@ const getSliderProgress = ({
|
||||
return Math.min(1, Math.max(0, (value - min) / (max - min)));
|
||||
};
|
||||
|
||||
|
||||
const getSliderFill = ({
|
||||
max,
|
||||
min,
|
||||
|
||||
Reference in New Issue
Block a user