Add better safety checks for 'dtoTo*' methods

folio-org · Apr 24, 2024 · e53280d · e53280d
1 parent da918ac
commit e53280d
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 15 deletions.
diff --git a/src/api/dto/from/dtoToAggregateCriteria.test.ts b/src/api/dto/from/dtoToAggregateCriteria.test.ts
@@ -61,4 +61,16 @@ describe('dtoToAggregateCriteria', () => {
       },
     ],
   ])('with EUR dtoToAggregateCriteria(%s) === %s', (input, expected) => expect(dtoToAggregateCriteria(input, { currency: 'EUR' } as StripesType, intlEu)).toEqual(expected));
+
+  test.each<[BursarExportFilterAggregate | undefined, CriteriaAggregate | undefined]>([
+    [
+      {
+        type: 'Aggregate',
+        property: 'TOTAL_AMOUNT',
+        condition: 'GREATER_THAN',
+        amount: ('' as unknown) as number,
+      },
+      undefined
+    ],
+  ])('with undefined amount dtoToAggregateCriteria(%s) === %s', (input, expected) => expect(dtoToAggregateCriteria(input, { currency: 'USD' } as StripesType, intlEu)).toEqual(expected));
 });
diff --git a/src/api/dto/from/dtoToAggregateCriteria.ts b/src/api/dto/from/dtoToAggregateCriteria.ts
@@ -10,10 +10,13 @@ import { BursarExportFilterAggregate } from '../dto-types';
 
 // inverse of ../to/aggregateCriteriaToFilterDto
 export default function dtoToAggregateCriteria(
-  filter: BursarExportFilterAggregate | undefined,
+  filter: Partial<BursarExportFilterAggregate>,
   stripes: StripesType,
   intl: IntlShape,
 ): CriteriaAggregate | undefined {
+  if (!filter || typeof filter.property !== 'string' || typeof filter.condition !== 'string' || typeof filter.amount !== 'number') {
+    return undefined;
+  }
   switch (filter?.property) {
     case CriteriaTokenType.NUM_ROWS:
       return {

diff --git a/src/api/dto/from/dtoToCriteria.ts b/src/api/dto/from/dtoToCriteria.ts
@@ -17,7 +17,7 @@ import {
 } from '../dto-types';
 
 export function dtoToConditionCriteria(
-  filter: BursarExportFilterCondition,
+  filter: Partial<BursarExportFilterCondition>,
   feeFineTypes: FeeFineTypeDTO[],
   locations: LocationDTO[],
   stripes: StripesType,
@@ -27,38 +27,38 @@ export function dtoToConditionCriteria(
     return {
       type: CriteriaGroupType.ALL_OF,
       // eslint-disable-next-line @typescript-eslint/no-use-before-define
-      criteria: filter.criteria.map((criteria) => dtoToCriteria(criteria, feeFineTypes, locations, stripes, intl)),
+      criteria: filter.criteria?.map((criteria) => dtoToCriteria(criteria, feeFineTypes, locations, stripes, intl)) ?? [],
     };
   } else {
     return {
       type: CriteriaGroupType.ANY_OF,
       // eslint-disable-next-line @typescript-eslint/no-use-before-define
-      criteria: filter.criteria.map((criteria) => dtoToCriteria(criteria, feeFineTypes, locations, stripes, intl)),
+      criteria: filter.criteria?.map((criteria) => dtoToCriteria(criteria, feeFineTypes, locations, stripes, intl)) ?? [],
     };
   }
 }
 
 export function dtoToNegationCriteria(
-  filter: BursarExportFilterNegation,
+  filter: Partial<BursarExportFilterNegation>,
   feeFineTypes: FeeFineTypeDTO[],
   locations: LocationDTO[],
   stripes: StripesType,
   intl: IntlShape,
 ): CriteriaGroup {
   // NOR gets displayed as "None of" in the UI, so we flatten the inner OR
-  if (filter.criteria.type === 'Condition' && filter.criteria.operation === AndOrOperator.OR) {
+  if (filter.criteria?.type === 'Condition' && filter.criteria?.operation === AndOrOperator.OR) {
     return {
       type: CriteriaGroupType.NONE_OF,
       // eslint-disable-next-line @typescript-eslint/no-use-before-define
-      criteria: filter.criteria.criteria.map((criteria) => dtoToCriteria(criteria, feeFineTypes, locations, stripes, intl)),
+      criteria: filter.criteria.criteria?.map((criteria) => dtoToCriteria(criteria, feeFineTypes, locations, stripes, intl)) ?? [],
     };
   }
 
   // otherwise, just negate the single inner criteria
   return {
     type: CriteriaGroupType.NONE_OF,
     // eslint-disable-next-line @typescript-eslint/no-use-before-define
-    criteria: [dtoToCriteria(filter.criteria, feeFineTypes, locations, stripes, intl)],
+    criteria: filter.criteria ? [dtoToCriteria(filter.criteria, feeFineTypes, locations, stripes, intl)] : [],
   };
 }
 
@@ -91,7 +91,7 @@ export function getLocationProperties(
 
 // inverse of ../to/criteriaToFilterDto
 export default function dtoToCriteria(
-  filter: BursarExportFilterDTO,
+  filter: Partial<BursarExportFilterDTO>,
   feeFineTypes: FeeFineTypeDTO[],
   locations: LocationDTO[],
   stripes: StripesType,
@@ -102,13 +102,13 @@ export default function dtoToCriteria(
       return {
         type: CriteriaTerminalType.AGE,
         operator: filter.condition as ComparisonOperator,
-        numDays: filter.numDays.toString(),
+        numDays: filter.numDays?.toString() ?? '',
       };
     case CriteriaTerminalType.AMOUNT:
       return {
         type: CriteriaTerminalType.AMOUNT,
         operator: filter.condition as ComparisonOperator,
-        amountCurrency: intl.formatNumber(filter.amount / 100, { style: 'currency', currency: stripes.currency }),
+        amountCurrency: intl.formatNumber((filter.amount ?? 0) / 100, { style: 'currency', currency: stripes.currency }),
       };
     case CriteriaTerminalType.FEE_FINE_OWNER:
       return {
@@ -118,14 +118,14 @@ export default function dtoToCriteria(
     case CriteriaTerminalType.FEE_FINE_TYPE:
       return {
         type: CriteriaTerminalType.FEE_FINE_TYPE,
-        feeFineTypeId: filter.feeFineTypeId,
-        feeFineOwnerId: getFeeFineOwnerId(filter.feeFineTypeId, feeFineTypes),
+        feeFineTypeId: filter.feeFineTypeId ?? '',
+        feeFineOwnerId: getFeeFineOwnerId(filter.feeFineTypeId ?? '', feeFineTypes),
       };
     case CriteriaTerminalType.LOCATION:
       return {
         type: CriteriaTerminalType.LOCATION,
-        locationId: filter.locationId,
-        ...getLocationProperties(filter.locationId, locations),
+        locationId: filter.locationId ?? '',
+        ...getLocationProperties(filter.locationId ?? '', locations),
       };
     case CriteriaTerminalType.PATRON_GROUP:
       return {