Skip to content

MPDX-8678 - Improve AddDonation #1369

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.

Sign up for GitHub

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

Open
zweatshirt wants to merge 6 commits into
base: main
Choose a base branch
from
Open

MPDX-8678 - Improve AddDonation #1369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2f208bf
Standardizes AddDonation form errors behavior
zweatshirt Jul 15, 2025
819e6b1
Change CustomDateField, CurrencyAutocomplete, DonorAccountAutocomplet…
zweatshirt Jul 15, 2025
b4544f8
fixup: Import FocusEventHandler in CurrencyAutocomplete
zweatshirt Jul 15, 2025
84967f1
Change onBlur handler to adhere to Formik expectations, for fields wh…
zweatshirt Jul 16, 2025
3a34c77
Removes onBlur from CustomDateField, it doesn't work as intended and …
zweatshirt Jul 16, 2025
a3dad43
Changes i18n import statement
zweatshirt Jul 16, 2025
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
89 changes: 64 additions & 25 deletions src/components/Layouts/Primary/TopBar/Items/AddMenu/Items/AddDonation/AddDonation.tsx
View file
Open in desktop
Copy link
Copy Markdown
Contributor

@wjames111 wjames111 Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making these changes!

Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
SubmitButton,
} from 'src/components/common/Modal/ActionButtons/ActionButtons';
import { requiredDateTime } from 'src/lib/formikHelpers';
import i18n from 'src/lib/i18n';
import {
useAddDonationMutation,
useGetDonationModalQuery,
Expand All @@ -41,38 +42,40 @@
const donationSchema = yup.object({
amount: yup
.number()
.typeError('Amount must be a valid number')
.required()
.typeError(i18n.t('Amount must be a valid number'))
.required(i18n.t('Amount is required'))
.test(
'Is amount in valid currency format?',
'Amount must be in valid currency format',
i18n.t('Amount must be in valid currency format'),
(amount) => /\$?[0-9][0-9.,]*/.test(amount as unknown as string),
)
.test(
'Is positive?',
'Must use a positive number for amount',
i18n.t('Must use a positive number for amount'),
(value) => parseFloat(value as unknown as string) > 0,
),
appealAmount: yup
.number()
.typeError('Appeal amount must be a valid number')
.typeError(i18n.t('Appeal amount must be a valid number'))
.nullable()
.test(
'Is appeal amount in valid currency format?',
'Appeal amount must be in valid currency format',
i18n.t('Appeal amount must be in valid currency format'),
(amount) =>
!amount || /\$?[0-9][0-9.,]*/.test(amount as unknown as string),
)
.test(
'Is positive?',
'Must use a positive number for appeal amount',
i18n.t('Must use a positive number for appeal amount'),
(value) => !value || parseFloat(value as unknown as string) > 0,
),
appealId: yup.string().nullable(),
currency: yup.string().required(),
designationAccountId: yup.string().required(),
donationDate: requiredDateTime(),
donorAccountId: yup.string().required(),
currency: yup.string().required(i18n.t('Currency is required')),
designationAccountId: yup
.string()
.required(i18n.t('Designation account is required')),
donationDate: requiredDateTime(i18n.t('Date is required')),
donorAccountId: yup.string().required(i18n.t('Partner Account is required')),
Comment on lines +59 to +78
Copy link
Copy Markdown
Contributor

@wjames111 wjames111 Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will be a lot more helpful to the user!

memo: yup.string().nullable(),
motivation: yup.string().nullable(),
paymentMethod: yup.string().nullable(),
Expand Down Expand Up @@ -175,255 +178,291 @@
validateOnMount
>
{({
values: { appealId, currency },
handleBlur,
setFieldValue,
setFieldTouched,
isSubmitting,
isValid,
errors,
touched,
}): ReactElement => (
<Form>
<DialogContent dividers>
<Grid container spacing={1}>
{/* Amount and Currency Row */}
<Grid container item xs={12} spacing={1}>
<Grid item xs={isMobile ? 12 : 8}>
<FormControl
size="small"
fullWidth
error={!!errors.amount && touched.amount}
>
<LogFormLabel
htmlFor="amount-input"
id="amount-input-label"
required
>
{t('Amount')}
</LogFormLabel>
<FastField name="amount">
{({ field, meta }: FieldProps) => (
<Box width="100%">
<FormTextField
{...field}
size="small"
variant="outlined"
onChange={(e) => {
setFieldValue('amount', e.target.value);
setFieldTouched('amount', true, false);
}}
onBlur={() => setFieldTouched('amount', true)}
Comment on lines +214 to +218
Copy link
Copy Markdown
Contributor

@wjames111 wjames111 Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for using the setFieldTouched on these!

fullWidth
type="text"
inputProps={{
'aria-labelledby': 'amount-input-label',
}}
id="amount-input"
error={!!errors.amount && touched.amount}
/>
<FormHelperText>
{meta.touched && meta.error}
</FormHelperText>
</Box>
)}
</FastField>
</FormControl>
</Grid>
<Grid item xs={isMobile ? 12 : 4}>
<FormControl
size="small"
fullWidth
error={!!errors.currency && touched.currency}
>
<LogFormLabel
htmlFor="currency-select"
required
id="currency-select-label"
>
{t('Currency')}
</LogFormLabel>

<Box width="100%">
<CurrencyAutocomplete
disabled={isSubmitting}
id="currency-select"
disableClearable
value={currency}
onChange={(_, currencyCode) => {
setFieldValue('currency', currencyCode);
setFieldTouched('currency', true, false);
}}
onBlur={() => setFieldTouched('currency', true)}
textFieldProps={{
error: !!errors.currency,
error: !!errors.currency && touched.currency,
helperText: touched.currency && errors.currency,
}}
size="small"
/>
<FormHelperText
error={true}
data-testid="pledgeCurrencyError"
>
{touched.currency && errors.currency}
</FormHelperText>
</Box>
</FormControl>
</Grid>
</Grid>
{/* Donation Date and Motivation Row */}
<Grid container item xs={12} spacing={1}>
<Grid item xs={isMobile ? 12 : 6}>
<FormControl
size="small"
fullWidth
error={Boolean(errors.donationDate && touched.donationDate)}
>
<LogFormLabel htmlFor="date-input" id="date-label" required>
{t('Date')}
</LogFormLabel>
<FastField name="donationDate">
{({ field }: FieldProps) => (
<CustomDateField
id="date-input"
size="small"
inputProps={{
'aria-labelledby': 'date-label',
}}
{...field}
onChange={(date) =>
setFieldValue('donationDate', date)
}
onChange={(date) => {
setFieldValue('donationDate', date);
}}
error={!!errors.donationDate}
helperText={errors.donationDate as string}
/>
)}
</FastField>
</FormControl>
</Grid>
<Grid item xs={isMobile ? 12 : 6}>
<FormControl
size="small"
fullWidth
disabled
error={!!errors.motivation && touched.motivation}
>
<LogFormLabel
htmlFor="motivation-input"
id="motivation-label"
>
{t('Motivation')}
</LogFormLabel>
<Field name="motivation">
{({ field }: FieldProps) => (
<Box width="100%">
<FormTextField
{...field}
size="small"
id="motivation-input"
variant="outlined"
fullWidth
type="text"
disabled
inputProps={{
'aria-labelledby': 'motivation-label',
}}
/>
</Box>
)}
</Field>
</FormControl>
</Grid>
</Grid>
{/* Partner and Designation Account Row */}
<Grid container item xs={12} spacing={1}>
<Grid item xs={isMobile ? 12 : 6}>
<FormControl
size="small"
fullWidth
error={!!errors.donorAccountId && touched.donorAccountId}
>
<LogFormLabel
htmlFor="partner-account-input"
id="partner-account-label"
required
>
{t('Partner Account')}
</LogFormLabel>
<Field name="donorAccountId">
{({ field }: FieldProps) => (
<Box width="100%">
<DonorAccountAutocomplete
accountListId={accountListId}
onChange={(donorAccountId) =>
setFieldValue('donorAccountId', donorAccountId)
onChange={(donorAccountId) => {
setFieldValue('donorAccountId', donorAccountId);
setFieldTouched('donorAccountId', true, false);
}}
onBlur={() =>
setFieldTouched('donorAccountId', true)
}
onBlur={handleBlur('donorAccountId')}
textFieldProps={{
error:
!!errors.donorAccountId &&
touched.donorAccountId,
helperText:
touched.donorAccountId && errors.donorAccountId,
}}
value={field.value}
autocompleteId="partner-account-input"
labelId="partner-account-label"
size="small"
/>
</Box>
)}
</Field>
</FormControl>
</Grid>
<Grid item xs={isMobile ? 12 : 6}>
<FormControl
size="small"
fullWidth
error={
!!errors.designationAccountId &&
touched.designationAccountId
}
>
<LogFormLabel
htmlFor="designation-account-input"
id="designation-account-label"
required
>
{t('Designation Account')}
</LogFormLabel>
<FastField name="designationAccountId">
{({ field }: FieldProps) => (
<Box width="100%">
<Autocomplete
{...field}
id="designation-account-input"
loading={designationAccountsLoading}
autoSelect
autoHighlight
onChange={(_, designationAccountId) => {
setFieldValue(
'designationAccountId',
designationAccountId,
);
setFieldTouched(
'designationAccountId',
true,
false,
);
}}
onBlur={() =>
setFieldTouched('designationAccountId', true)
}
options={
designationAccounts?.map(({ id }) => id) ?? []
}
getOptionLabel={(accountId): string => {
const account = designationAccounts?.find(
({ id }) => id === accountId,
);
return account
? `${account?.name} (${account.id})`
: '';
}}
renderInput={(params): ReactElement => (
<TextField
{...params}
size="small"
variant="outlined"
error={
!!errors.designationAccountId &&
touched.designationAccountId
}
helperText={
touched.designationAccountId &&
errors.designationAccountId
}
InputProps={{
...params.InputProps,
'aria-labelledby':
'designation-account-label',
endAdornment: (
<>
{designationAccountsLoading && (
<CircularProgress
color="primary"
size={20}
/>
)}
{params.InputProps.endAdornment}
</>
),
}}
/>
)}
value={field.value}

Check warning on line 465 in src/components/Layouts/Primary/TopBar/Items/AddMenu/Items/AddDonation/AddDonation.tsx

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ Getting worse: Complex Method 

AddDonation increases in cyclomatic complexity from 41 to 47, threshold = 10. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
onChange={(_, designationAccountId) =>
setFieldValue(
'designationAccountId',
designationAccountId,
)
}
/>
</Box>
)}
Expand Down
4 changes: 4 additions & 0 deletions src/components/common/Autocomplete/CurrencyAutocomplete/CurrencyAutocomplete.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { FocusEventHandler } from 'react';
import {
Autocomplete,
AutocompleteProps,
Expand All @@ -15,11 +16,13 @@ interface CurrencyAutocompleteProps
extends Partial<AutocompleteProps<string, boolean, boolean, boolean>> {
textFieldProps?: Partial<TextFieldProps>;
format?: PledgeCurrencyOptionFormatEnum;
onBlur?: FocusEventHandler<HTMLDivElement>;
}

export const CurrencyAutocomplete = ({
textFieldProps,
format = PledgeCurrencyOptionFormatEnum.Long,
onBlur,
...props
}: CurrencyAutocompleteProps) => {
const constants = useApiConstants();
Expand Down Expand Up @@ -49,6 +52,7 @@ export const CurrencyAutocomplete = ({
return selectedCurrency.codeSymbolString ?? '';
}}
renderInput={(params) => <TextField {...params} {...textFieldProps} />}
onBlur={onBlur}
/>
);
};
4 changes: 4 additions & 0 deletions src/components/common/Autocomplete/DonorAccountAutocomplete/DonorAccountAutocomplete.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
BaseTextFieldProps,
CircularProgress,
TextField,
TextFieldProps,
} from '@mui/material';
import { map, unionBy } from 'lodash';
import { useDebouncedCallback } from 'src/hooks/useDebounce';
Expand All @@ -19,6 +20,7 @@ export interface DonorAccountAutocompleteProps {
labelId?: string;
label?: string;
size?: BaseTextFieldProps['size'];
textFieldProps?: Partial<TextFieldProps>;
}

export const DonorAccountAutocomplete: React.FC<
Expand All @@ -33,6 +35,7 @@ export const DonorAccountAutocomplete: React.FC<
labelId,
label,
size,
textFieldProps,
}) => {
const [searchForDonorAccounts, { loading, data: donorAccountData }] =
useGetDonorAccountsLazyQuery();
Expand Down Expand Up @@ -66,6 +69,7 @@ export const DonorAccountAutocomplete: React.FC<
renderInput={(params) => (
<TextField
{...params}
{...textFieldProps}
size={size}
variant="outlined"
label={label}
Expand Down
Loading