Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UIPBEX-58: Add error checking for invalid config. #118

Merged
merged 5 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/api/dto/from/dtoToAggregateCriteria.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
});
5 changes: 4 additions & 1 deletion src/api/dto/from/dtoToAggregateCriteria.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@ import { BursarExportFilterAggregate } from '../dto-types';

// inverse of ../to/aggregateCriteriaToFilterDto
export default function dtoToAggregateCriteria(
filter: BursarExportFilterAggregate | undefined,
filter: Partial<BursarExportFilterAggregate> | undefined,
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 {
Expand Down
28 changes: 14 additions & 14 deletions src/api/dto/from/dtoToCriteria.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
} from '../dto-types';

export function dtoToConditionCriteria(
filter: BursarExportFilterCondition,
filter: Partial<BursarExportFilterCondition>,
feeFineTypes: FeeFineTypeDTO[],
locations: LocationDTO[],
stripes: StripesType,
Expand All @@ -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)] : [],
};
}

Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
61 changes: 40 additions & 21 deletions src/hooks/useInitialValues.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,31 +12,50 @@ jest.mock('../api/queries');
(useLocations as any).mockReturnValue({ isSuccess: false });
(useTransferAccounts as any).mockReturnValue({ isSuccess: false });

test('initial values hook', async () => {
const { result, rerender } = renderHook(() => useInitialValues(), {
wrapper: ({ children }: { children: React.ReactNode }) => withIntlConfiguration(<div>{children}</div>),
describe('useInitialValues', () => {
test('initial values hook', async () => {
const { result, rerender } = renderHook(() => useInitialValues(), {
wrapper: ({ children }: { children: React.ReactNode }) => withIntlConfiguration(<div>{children}</div>),
});

await waitFor(() => expect(result.current).toBeNull());

(useCurrentConfig as any).mockReturnValue({ isSuccess: true });
rerender();
expect(result.current).toBeNull();

(useFeeFineTypes as any).mockReturnValue({ isSuccess: true });
rerender();
expect(result.current).toBeNull();

(useLocations as any).mockReturnValue({ isSuccess: true });
rerender();
expect(result.current).toBeNull();

(useTransferAccounts as any).mockReturnValue({ isSuccess: true });
rerender();
await waitFor(() => expect(result.current).toEqual('values'));

// should not change back
(useTransferAccounts as any).mockReturnValue({ isSuccess: false });
rerender();
await waitFor(() => expect(result.current).toEqual('values'));
});

await waitFor(() => expect(result.current).toBeNull());
test('initial values hook with invalid data', async () => {
const { result, rerender } = renderHook(() => useInitialValues(), {
wrapper: ({ children }: { children: React.ReactNode }) => withIntlConfiguration(<div>{children}</div>),
});

(useCurrentConfig as any).mockReturnValue({ isSuccess: true });
rerender();
expect(result.current).toBeNull();
(useTransferAccounts as any).mockReturnValue({ isSuccess: false });

(useFeeFineTypes as any).mockReturnValue({ isSuccess: true });
rerender();
expect(result.current).toBeNull();
// should be null on fail
rerender();
await waitFor(() => expect(result.current).toBeNull());

(useLocations as any).mockReturnValue({ isSuccess: true });
rerender();
expect(result.current).toBeNull();
(useTransferAccounts as any).mockReturnValue({ isSuccess: true });
rerender();

(useTransferAccounts as any).mockReturnValue({ isSuccess: true });
rerender();
await waitFor(() => expect(result.current).toEqual('values'));

// should not change back
(useTransferAccounts as any).mockReturnValue({ isSuccess: false });
rerender();
await waitFor(() => expect(result.current).toEqual('values'));
await waitFor(() => expect(result.current).toEqual('values'));
});
});
11 changes: 7 additions & 4 deletions src/hooks/useInitialValues.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,13 @@
if (!currentConfig.isSuccess || !feeFineTypes.isSuccess || !locations.isSuccess || !transferAccounts.isSuccess) {
return;
}

setInitialValues(
dtoToFormValues(currentConfig.data, localeWeekdays, feeFineTypes.data, locations.data, transferAccounts.data, stripes, intl)
);
try {
setInitialValues(
dtoToFormValues(currentConfig.data, localeWeekdays, feeFineTypes.data, locations.data, transferAccounts.data, stripes, intl)
);
} catch (e) {
console.error('Unable to load initial values', e);

Check warning on line 37 in src/hooks/useInitialValues.ts

View workflow job for this annotation

GitHub Actions / ui / Install and lint / Install and lint

Unexpected console statement
}
}, [
currentConfig.isSuccess,
localeWeekdays,
Expand Down
Loading