Skip to content

Commit 821c60e

Browse files
committed
Handle GraphQL nulls from "optional" fields as undefined
GraphQL's / Nest's decorators have `nullable` option, which means the value can be null or omitted. Which naturally translates to null or undefined in JS. We actually care about this distinction, though, especially on update mutations. undefined means no change. null means clear the value. We carelessly allowed both meaningly that nulls could mistakenly be stored in lots of spots that are unexpected. Granted, this only would happen in Neo4j, as Gel's schema checks would prevent this. Going forward we have a new `optional` option as a sibling to `nullable`. `optional` lets the field be omitted or undefined. GraphQL can still give us nulls, but be will we convert them to undefined. `nullable` continues to let null & undefined through.
1 parent 290bdce commit 821c60e

File tree

26 files changed

+174
-82
lines changed

26 files changed

+174
-82
lines changed

src/common/id-field.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,20 @@
11
import { applyDecorators } from '@nestjs/common';
2-
import { Field, FieldOptions, ID as IDType } from '@nestjs/graphql';
2+
import { ID as IDType } from '@nestjs/graphql';
33
import { ValidationOptions } from 'class-validator';
44
import { IsAny, IsNever, Tagged } from 'type-fest';
55
import type { ResourceName, ResourceNameLike } from '~/core';
6+
import { OptionalField, OptionalFieldOptions } from './optional-field';
67
import { IsId } from './validators';
78

89
export const IdField = ({
910
validation,
1011
...options
11-
}: FieldOptions & { validation?: ValidationOptions } = {}) =>
12+
}: OptionalFieldOptions & { validation?: ValidationOptions } = {}) =>
1213
applyDecorators(
13-
Field(() => IDType, options),
14+
OptionalField(() => IDType, {
15+
optional: false,
16+
...options,
17+
}),
1418
IsId(validation),
1519
);
1620

src/common/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export * from './lazy-ref';
2727
export * from './lazy-record';
2828
export { DateField, DateTimeField } from './luxon.graphql';
2929
export * from './map-or-else';
30+
export * from './optional-field';
3031
export * from './order.enum';
3132
export * from './pagination.input';
3233
export * from './pagination-list';

src/common/luxon.graphql.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { applyDecorators } from '@nestjs/common';
2-
import { CustomScalar, Field, FieldOptions, Scalar } from '@nestjs/graphql';
2+
import { CustomScalar, Scalar } from '@nestjs/graphql';
33
import { stripIndent } from 'common-tags';
44
import { Kind, ValueNode } from 'graphql';
55
import { DateTime, Settings } from 'luxon';
66
import { InputException } from './exceptions';
7+
import { OptionalField, OptionalFieldOptions } from './optional-field';
78
import { CalendarDate } from './temporal';
89
import { Transform } from './transform.decorator';
910
import { ValidateBy } from './validators/validateBy';
@@ -24,9 +25,12 @@ const IsIsoDate = () =>
2425
},
2526
});
2627

27-
export const DateTimeField = (options?: FieldOptions) =>
28+
export const DateTimeField = (options?: OptionalFieldOptions) =>
2829
applyDecorators(
29-
Field(() => DateTime, options),
30+
OptionalField(() => DateTime, {
31+
optional: false,
32+
...options,
33+
}),
3034
Transform(
3135
({ value }) => {
3236
try {
@@ -43,9 +47,12 @@ export const DateTimeField = (options?: FieldOptions) =>
4347
IsIsoDate(),
4448
);
4549

46-
export const DateField = (options?: FieldOptions) =>
50+
export const DateField = (options?: OptionalFieldOptions) =>
4751
applyDecorators(
48-
Field(() => CalendarDate, options),
52+
OptionalField(() => CalendarDate, {
53+
optional: false,
54+
...options,
55+
}),
4956
Transform(
5057
({ value }) => {
5158
try {

src/common/name-field.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,24 @@ import { MinLength } from 'class-validator';
77
import { DbSort } from './db-sort.decorator';
88
import { Transform } from './transform.decorator';
99

10-
export const NameField = (options: FieldOptions = {}) =>
10+
type NameFieldParams = FieldOptions & {
11+
/**
12+
* If true, values can be omitted/undefined or null.
13+
* This will override `optional` if truthy.
14+
*/
15+
nullable?: true;
16+
/**
17+
* If true, values can be omitted/undefined but not null.
18+
*/
19+
optional?: true;
20+
};
21+
22+
export const NameField = (options: NameFieldParams = {}) =>
1123
applyDecorators(
12-
InferredTypeOrStringField(options),
24+
InferredTypeOrStringField({
25+
...options,
26+
nullable: options.optional ?? options.nullable,
27+
}),
1328
Transform(({ value }) => {
1429
if (value === undefined) {
1530
return undefined;
@@ -18,14 +33,16 @@ export const NameField = (options: FieldOptions = {}) =>
1833
// Treat null & empty strings as null
1934
return value?.trim() || null;
2035
}
36+
if (options.optional && value === null) {
37+
// Treat null as an omitted value
38+
return undefined;
39+
}
2140
// Null & empty string treated as MinLength validation error
2241
return value?.trim() ?? '';
2342
}),
2443
DbSort((value) => `apoc.text.clean(${value})`),
2544
// Using this instead of @IsNotEmpty, as this allows nulls.
26-
MinLength(1, {
27-
message: 'Cannot be empty',
28-
}),
45+
MinLength(1, { message: 'Cannot be empty' }),
2946
);
3047

3148
/**

src/common/optional-field.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { applyDecorators } from '@nestjs/common';
2+
import { Field, FieldOptions } from '@nestjs/graphql';
3+
import { NullableList } from '@nestjs/graphql/dist/interfaces/base-type-options.interface';
4+
import { Transform } from 'class-transformer';
5+
6+
export type OptionalFieldOptions = FieldOptions & {
7+
/**
8+
* If true, values can be omitted/undefined or null.
9+
* This will override `optional` if truthy.
10+
*/
11+
nullable?: boolean | NullableList;
12+
/**
13+
* If true, values can be omitted/undefined but not null.
14+
*/
15+
optional?: boolean;
16+
transform?: (value: any) => unknown;
17+
};
18+
19+
/**
20+
* A field that is optional/omissible/can be undefined.
21+
* Whether it can be explicitly null is based on `nullable`.
22+
*/
23+
export function OptionalField(
24+
typeFn: () => any,
25+
options?: OptionalFieldOptions,
26+
): PropertyDecorator;
27+
export function OptionalField(
28+
options?: OptionalFieldOptions,
29+
): PropertyDecorator;
30+
export function OptionalField(...args: any) {
31+
const typeFn: (() => any) | undefined =
32+
typeof args[0] === 'function' ? (args[0] as () => any) : undefined;
33+
const options: OptionalFieldOptions | undefined =
34+
typeof args[0] === 'function' ? args[1] : args[0];
35+
const opts = {
36+
...options,
37+
nullable: options?.nullable ?? options?.optional ?? true,
38+
};
39+
return applyDecorators(
40+
typeFn ? Field(typeFn, opts) : Field(opts),
41+
Transform(({ value }) => {
42+
if (!options?.nullable && (options?.optional ?? true) && value == null) {
43+
return undefined;
44+
}
45+
return options?.transform ? options.transform(value) : value;
46+
}),
47+
);
48+
}

src/common/rich-text.scalar.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { applyDecorators } from '@nestjs/common';
2-
import { Field, FieldOptions, ObjectType } from '@nestjs/graphql';
2+
import { ObjectType } from '@nestjs/graphql';
33
import { setToStringTag } from '@seedcompany/common';
44
import { markSkipClassTransformation } from '@seedcompany/nest';
55
import { IsObject } from 'class-validator';
@@ -9,7 +9,7 @@ import { GraphQLJSONObject } from 'graphql-scalars';
99
import { isEqual } from 'lodash';
1010
import { JsonObject } from 'type-fest';
1111
import { SecuredProperty } from '~/common/secured-property';
12-
import { Transform } from './transform.decorator';
12+
import { OptionalField, OptionalFieldOptions } from './optional-field';
1313

1414
function hashId(name: string) {
1515
return createHash('shake256', { outputLength: 5 }).update(name).digest('hex');
@@ -76,11 +76,14 @@ export class RichTextDocument {
7676
setToStringTag(RichTextDocument, 'RichText');
7777
markSkipClassTransformation(RichTextDocument);
7878

79-
export const RichTextField = (options?: FieldOptions) =>
79+
export const RichTextField = (options?: OptionalFieldOptions) =>
8080
applyDecorators(
81-
Field(() => RichTextScalar, options),
82-
IsObject(),
83-
Transform(({ value }) => RichTextDocument.fromMaybe(value)),
81+
OptionalField(() => RichTextScalar, {
82+
optional: false,
83+
...options,
84+
transform: RichTextDocument.fromMaybe,
85+
}),
86+
IsObject(), // TODO validate empty blocks becomes null becomes validation error
8487
);
8588

8689
/** @internal */

src/common/sensitivity.enum.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import { applyDecorators } from '@nestjs/common';
2-
import { Field, FieldOptions } from '@nestjs/graphql';
32
import { EnumType, makeEnum } from '@seedcompany/nest';
43
import { Transform } from 'class-transformer';
54
import { uniq } from 'lodash';
65
import { rankSens } from '~/core/database/query';
76
import { DbSort } from './db-sort.decorator';
7+
import { OptionalField, OptionalFieldOptions } from './optional-field';
88

99
export type Sensitivity = EnumType<typeof Sensitivity>;
1010
export const Sensitivity = makeEnum({
@@ -13,9 +13,12 @@ export const Sensitivity = makeEnum({
1313
exposeOrder: true,
1414
});
1515

16-
export const SensitivityField = (options: FieldOptions = {}) =>
16+
export const SensitivityField = (options?: OptionalFieldOptions) =>
1717
applyDecorators(
18-
Field(() => Sensitivity, options),
18+
OptionalField(() => Sensitivity, {
19+
optional: false,
20+
...options,
21+
}),
1922
DbSort(rankSens),
2023
);
2124

src/components/comments/dto/update-comment.dto.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export abstract class UpdateCommentInput {
77
@IdField()
88
readonly id: ID;
99

10-
@RichTextField({ nullable: true })
10+
@RichTextField({ optional: true })
1111
readonly body?: RichTextDocument;
1212
}
1313

src/components/engagement/dto/update-engagement.dto.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
DateField,
88
ID,
99
IdField,
10+
OptionalField,
1011
RichTextDocument,
1112
RichTextField,
1213
} from '~/common';
@@ -40,7 +41,7 @@ export abstract class UpdateEngagement {
4041

4142
readonly initialEndDate?: CalendarDate | null;
4243

43-
@Field(() => EngagementStatus, { nullable: true })
44+
@OptionalField(() => EngagementStatus)
4445
readonly status?: EngagementStatus;
4546

4647
@RichTextField({ nullable: true })
@@ -79,10 +80,10 @@ export abstract class UpdateLanguageEngagement extends UpdateEngagement {
7980
@Field(() => String, { nullable: true })
8081
readonly historicGoal?: string | null;
8182

82-
@Field(() => LanguageMilestone, { nullable: true })
83+
@OptionalField(() => LanguageMilestone)
8384
readonly milestoneReached?: LanguageMilestone;
8485

85-
@Field(() => AIAssistedTranslation, { nullable: true })
86+
@OptionalField(() => AIAssistedTranslation)
8687
readonly usingAIAssistedTranslation?: AIAssistedTranslation;
8788
}
8889

src/components/ethno-art/dto/update-ethno-art.dto.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export abstract class UpdateEthnoArt {
1010
@IdField()
1111
readonly id: ID;
1212

13-
@NameField({ nullable: true })
13+
@NameField({ optional: true })
1414
readonly name?: string;
1515

1616
@ScriptureField({ nullable: true })

0 commit comments

Comments
 (0)