Merge pull request #3517 from SeedCompany/refactor/optional-fields

Author: CarsonF

src/common/list-field.ts

@@ -1,6 +1,10 @@
 import { applyDecorators } from '@nestjs/common';
 import { ArrayNotEmpty } from 'class-validator';
-import { OptionalField, type OptionalFieldOptions } from './optional-field';
+import {
+  OptionalField,
+  type OptionalFieldOptions,
+  withDefaultTransform,
+} from './optional-field';
 
 export type ListFieldOptions = OptionalFieldOptions & {
   /**
@@ -14,11 +18,12 @@ export const ListField = (typeFn: () => any, options: ListFieldOptions) =>
     OptionalField(() => [typeFn()], {
       optional: false,
       ...options,
-      transform: (value) => {
+      transform: withDefaultTransform(options.transform, (prev) => (raw) => {
+        const value = prev(raw);
         let out = value ? [...new Set(value)] : value;
         out = options.empty === 'omit' && out.length === 0 ? undefined : out;
-        return options.transform ? options.transform(out) : out;
-      },
+        return out;
+      }),
     }),
     ...(options.empty === 'deny' ? [ArrayNotEmpty()] : []),
   );

src/common/optional-field.ts

@@ -13,9 +13,19 @@ export type OptionalFieldOptions = FieldOptions & {
    * If true, values can be omitted/undefined but not null.
    */
   optional?: boolean;
-  transform?: (value: any) => unknown;
+  transform?: TransformerLink;
 };
 
+type TransformerLink = (prev: Transformer) => Transformer;
+type Transformer<I = any, O = any> = (value: I) => O;
+export const withDefaultTransform =
+  (
+    input: TransformerLink | undefined,
+    wrapping: TransformerLink,
+  ): TransformerLink =>
+  (base) =>
+    input?.(wrapping(base)) ?? wrapping(base);
+
 /**
  * A field that is optional/omissible/can be undefined.
  * Whether it can be explicitly null is based on `nullable`.
@@ -30,19 +40,27 @@ export function OptionalField(
 export function OptionalField(...args: any) {
   const typeFn: (() => any) | undefined =
     typeof args[0] === 'function' ? (args[0] as () => any) : undefined;
-  const options: OptionalFieldOptions | undefined =
-    typeof args[0] === 'function' ? args[1] : args[0];
-  const opts = {
+  const options: OptionalFieldOptions =
+    (typeof args[0] === 'function' ? args[1] : args[0]) ?? {};
+  const nilIn =
+    options.nullable === 'items' && options.optional
+      ? 'itemsAndList'
+      : options.nullable ?? options.optional ?? true;
+  const nullOut = !!options.nullable && options.nullable !== 'items';
+  const schemaOptions = {
     ...options,
-    nullable: options?.nullable ?? options?.optional ?? true,
+    nullable: nilIn,
+  };
+  const defaultTransformer: Transformer = (value) => {
+    if (value === null && !nullOut) {
+      return undefined;
+    }
+    return value;
   };
+  const finalTransformer =
+    options.transform?.(defaultTransformer) ?? defaultTransformer;
   return applyDecorators(
-    typeFn ? Field(typeFn, opts) : Field(opts),
-    Transform(({ value }) => {
-      if (!options?.nullable && (options?.optional ?? true) && value == null) {
-        return undefined;
-      }
-      return options?.transform ? options.transform(value) : value;
-    }),
+    typeFn ? Field(typeFn, schemaOptions) : Field(schemaOptions),
+    Transform(({ value }) => finalTransformer(value)),
   );
 }

src/common/rich-text.scalar.ts

@@ -1,6 +1,6 @@
 import { applyDecorators } from '@nestjs/common';
 import { ObjectType } from '@nestjs/graphql';
-import { setToStringTag } from '@seedcompany/common';
+import { type Nil, setToStringTag } from '@seedcompany/common';
 import { markSkipClassTransformation } from '@seedcompany/nest';
 import { IsObject } from 'class-validator';
 import { createHash } from 'crypto';
@@ -82,23 +82,19 @@ export const RichTextField = (options?: OptionalFieldOptions) =>
     OptionalField(() => RichTextScalar, {
       optional: false,
       ...options,
-      transform: (value) => {
-        const doc = RichTextDocument.fromMaybe(value);
-        if (doc) {
-          return doc;
+      transform: (prev) => (value) => {
+        const doc: RichTextDocument | Nil = prev(
+          RichTextDocument.fromMaybe(value),
+        );
+        if (doc == null && !options?.nullable && !options?.optional) {
+          // Should never _really_ get here.
+          // UI should understand & send null instead of an empty document.
+          // Would prefer this to be done with validators.
+          // But I believe this needs `null`s to be validated.
+          // skipMissingProperties -> skipUndefinedProperties
+          throw new InputException('RichText must be given');
         }
-        if (options?.nullable) {
-          return null;
-        }
-        if (options?.optional) {
-          return undefined;
-        }
-        // Should never _really_ get here.
-        // UI should understand & send null instead of an empty document.
-        // Would prefer this to be done with validators.
-        // But I believe this needs to `null`s to be validated.
-        // skipMissingProperties -> skipUndefinedProperties
-        throw new InputException('RichText must be given');
+        return doc;
       },
     }),
     IsObject(),

src/common/sensitivity.enum.ts

@@ -27,7 +27,11 @@ export const SensitivitiesFilterField = (options?: ListFieldOptions) =>
     ...options,
     optional: true,
     empty: 'omit',
-    transform: (value) =>
+    transform: (prev) => (raw) => {
+      const value = prev(raw);
       // If given all options, there is no need to filter
-      !value || value.length === Sensitivity.values.size ? undefined : value,
+      return !value || value.length === Sensitivity.values.size
+        ? undefined
+        : value;
+    },
   });

src/components/changeset/dto/changeset.args.ts

@@ -11,8 +11,8 @@ export class ChangesetIds {
   @IdField()
   id: ID;
 
-  @IdField({ nullable: true })
-  changeset: ID;
+  @IdField({ optional: true })
+  changeset?: ID;
 }
 
 export type IdsAndView = ChangesetIds & { view: ObjectView };
@@ -25,10 +25,10 @@ export class ObjectViewPipe implements PipeTransform {
   constructor(private readonly validator: ValidationPipe) {}
 
   async transform(input: ChangesetIds) {
-    const { id, changeset } = await this.validator.transform(input, {
+    const { id, changeset } = (await this.validator.transform(input, {
       metatype: ChangesetIds,
       type: 'body',
-    });
+    })) as ChangesetIds;
     const view: ObjectView = changeset ? { changeset } : { active: true };
     return { id, changeset, view };
   }

src/components/partnership/dto/list-partnership.dto.ts

@@ -20,9 +20,13 @@ export abstract class PartnershipFilters {
   @ListField(() => PartnerType, {
     optional: true,
     empty: 'omit',
-    transform: (value) =>
+    transform: (prev) => (raw) => {
+      const value = prev(raw);
       // If given all options, there is no need to filter
-      !value || value.length === PartnerType.values.size ? undefined : value,
+      return !value || value.length === PartnerType.values.size
+        ? undefined
+        : value;
+    },
   })
   readonly types?: readonly PartnerType[];
 }

src/components/progress-report/variance-explanation/variance-explanation.dto.ts

@@ -3,6 +3,7 @@ import { ArrayMaxSize, IsIn } from 'class-validator';
 import {
   type ID,
   IdField,
+  ListField,
   type RichTextDocument,
   RichTextField,
   SecuredRichTextNullable,
@@ -36,7 +37,10 @@ export abstract class ProgressReportVarianceExplanationInput {
   })
   readonly report: ID;
 
-  @Field(() => [String], { nullable: true })
+  @ListField(() => String, {
+    optional: true,
+    transform: (prev) => (value) => value === null ? [] : prev(value),
+  })
   @IsIn([...ReasonOptions.instance.all], {
     each: true,
     message: 'Reason is not one of our available choices',

src/components/progress-report/variance-explanation/variance-explanation.service.ts

@@ -37,12 +37,7 @@ export class ProgressReportVarianceExplanationService {
     }
 
     const changes = this.repo.getActualChanges(existing, {
-      reasons:
-        input.reasons === undefined
-          ? existing.reasons.value
-          : input.reasons === null
-          ? []
-          : input.reasons,
+      reasons: input.reasons,
       comments: input.comments,
     });
     if (Object.keys(changes).length === 0) {

src/components/progress-summary/dto/progress-summary-filters.dto.ts

@@ -6,14 +6,16 @@ import { ScheduleStatus } from './schedule-status.enum';
 @InputType()
 export abstract class ProgressSummaryFilters {
   @ListField(() => ScheduleStatus, {
-    nullable: 'itemsAndList',
+    optional: true,
+    nullable: 'items',
     description: stripIndent`
       Filter by schedule status.
       - \`[X, Y]\` will allow summaries with either X or Y status.
       - \`[null, X]\` will allow missing summaries or summaries with X status.
       - \`[null]\` will filter to only missing summaries.
       - \`null\` and \`[]\` will be ignored.
     `,
+    empty: 'omit',
   })
   readonly scheduleStatus?: ReadonlyArray<ScheduleStatus | null>;
 }

src/components/progress-summary/progress-summary.repository.ts

@@ -108,9 +108,6 @@ export const progressSummaryFilters = filter.define(
   {
     scheduleStatus: ({ value, query }) => {
       const status = setOf(value);
-      if (status.size === 0) {
-        return undefined;
-      }
       if (status.size === 1 && status.has(null)) {
         return query.where(new WhereExp('node IS NULL'));
       }