Skip to content

MPDX-9653 - Simplify From Account -> To Account behavior in Savings Fund Transfer #1812

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

Merged
zweatshirt merged 1 commit into from
Jun 2, 2026
Merged

MPDX-9653 - Simplify From Account -> To Account behavior in Savings Fund Transfer #1812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 0 additions & 16 deletions src/components/HrTools/SavingsFundTransfer/BalanceCard/BalanceCard.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';

Check notice on line 1 in src/components/HrTools/SavingsFundTransfer/BalanceCard/BalanceCard.test.tsx

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ Getting better: Code Duplication 

reduced similar code in: 'should call handleOpenTransferModal with correct parameters when Transfer From is clicked','should call handleOpenTransferModal with correct parameters when Transfer To is clicked'. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.
import { ThemeProvider } from '@mui/material/styles';
import { render } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
Expand Down Expand Up @@ -62,7 +62,6 @@
expect(getByText('Current Balance')).toBeInTheDocument();
expect(getByText('$15,000.00')).toBeInTheDocument();
expect(getByRole('button', { name: /transfer from/i })).toBeInTheDocument();
expect(getByRole('button', { name: /transfer to/i })).toBeInTheDocument();
});

it('should display title correctly', () => {
Expand Down Expand Up @@ -190,21 +189,6 @@
});
});

it('should call handleOpenTransferModal with correct parameters when Transfer To is clicked', async () => {
const { findByRole } = render(<Components />);

const transferToButton = await findByRole('button', {
name: /transfer to/i,
});
userEvent.click(transferToButton);

expect(mockHandleOpenTransferModal).toHaveBeenCalledWith({
transfer: {
transferTo: expect.any(String),
},
});
});

it('should disable transfer from button when current balance goes beyond deficit limit', async () => {
const { findByRole } = render(
<Components
Expand Down
31 changes: 9 additions & 22 deletions src/components/HrTools/SavingsFundTransfer/BalanceCard/BalanceCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
import React from 'react';
import {
Groups,
MoveToInbox,
Outbox,
Savings,
Wallet,
} from '@mui/icons-material';
import { Groups, Outbox, Savings, Wallet } from '@mui/icons-material';
import { Box, Button, Card, Typography } from '@mui/material';
import { useTranslation } from 'react-i18next';
import { SimpleScreenOnly } from 'src/components/Reports/styledComponents';
Expand Down Expand Up @@ -50,106 +44,99 @@
},
});
};

const handleTransferTo = () => {
handleOpenTransferModal({
transfer: {
transferTo: fund.fundType,
},
});
};

return (
<Card
variant="outlined"
sx={{
p: 2,
flex: 1,
minWidth: 0,
maxWidth: 'none',
fontSize: '1.25rem',
boxShadow: isSelected ? 3 : 1,
transition: 'box-shadow 0.3s ease-in-out',
display: 'flex',
flexDirection: 'column',
}}
>
<Box display={'flex'} flexDirection="row" alignItems="center" gap={1}>
<Box
sx={{
backgroundColor: iconBgColor || 'primary.main',
color: 'primary.contrastText',
borderRadius: 1,
p: 1,
display: 'flex',
alignItems: 'center',
justifyContent: 'center',
}}
className="StyledIconBox-root"
>
<Icon
sx={{
color: 'inherit',
}}
/>
</Box>
<Box>
<Typography
variant="body1"
mb={0}
sx={{
'@media print': { fontSize: '12pt' },
fontWeight: 500,
fontSize: '13pt',
minHeight: '3em',
display: 'flex',
alignItems: 'center',
}}
>
{title}
</Typography>
</Box>
</Box>
<Box
sx={{
mt: 5,
mt: 3,
'@media print': { mt: 2 },
}}
>
<Typography
variant="body1"
mb={0}
sx={{ '@media print': { fontSize: '12pt' } }}
>
{t('Current Balance')}
</Typography>

<Typography
variant="h5"
color={fund.endBalance < 0 ? 'error.main' : 'text.primary'}
sx={{ fontSize: 'inherit' }}
>
{fund.endBalance < 0 ? '(' : ''}
{currencyFormat(Math.abs(fund.endBalance), 'USD', locale, {
showTrailingZeros: true,
})}
{fund.endBalance < 0 ? ')' : ''}
</Typography>
</Box>

<SimpleScreenOnly
sx={{
alignItems: 'left',
mt: 2,
ml: 0,
mt: 'auto',
}}
>
<Button
onClick={handleTransferFrom}
disabled={fund.endBalance <= fund.deficitLimit}
fullWidth
>
<Outbox fontSize="small" sx={{ mr: 0.5 }} />
{t('TRANSFER FROM')}
</Button>

Check notice on line 139 in src/components/HrTools/SavingsFundTransfer/BalanceCard/BalanceCard.tsx

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ Getting better: Large Method 

BalanceCard:React.FC<BalanceCardProps> decreases from 127 to 121 lines of code, threshold = 100. Large functions with many lines of code are generally harder to understand and lower the code health. Avoid adding more lines to this function.
<Button onClick={handleTransferTo}>
<MoveToInbox fontSize="small" sx={{ mr: 0.5 }} />
{t('TRANSFER TO')}
</Button>
</SimpleScreenOnly>
</Card>
);
Expand Down
73 changes: 11 additions & 62 deletions src/components/HrTools/SavingsFundTransfer/TransferModal/TransferModal.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';

Check notice on line 1 in src/components/HrTools/SavingsFundTransfer/TransferModal/TransferModal.test.tsx

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ Getting better: Lines of Code in a Single File 

The lines of code decreases from 417 to 380, improve code health by reducing it to 300. The number of Lines of Code in a single file. More Lines of Code lowers the code health.
import { ThemeProvider } from '@mui/material/styles';
import { AdapterLuxon } from '@mui/x-date-pickers/AdapterLuxon';
import { LocalizationProvider } from '@mui/x-date-pickers/LocalizationProvider';
Expand Down Expand Up @@ -53,7 +53,7 @@
});

const transferDefaultData: TransferModalData['transfer'] = {
transferFrom: 'transferFrom',
transferFrom: 'Staff Account',
transferTo: '',
amount: 0,
schedule: ScheduleEnum.OneTime,
Expand Down Expand Up @@ -152,11 +152,8 @@
userEvent.type(amountField, '-100');
userEvent.tab();

const fromAccount = getByRole('combobox', { name: /from account/i });
const toAccount = getByRole('combobox', { name: /to account/i });

userEvent.click(fromAccount);
userEvent.click(getByRole('option', { name: /staff account/i }));
userEvent.click(toAccount);
userEvent.click(getByRole('option', { name: /staff savings/i }));

Expand Down Expand Up @@ -199,13 +196,9 @@
it('should submit form with valid data', async () => {
const { getByRole } = render(<Components />);

const fromAccount = getByRole('combobox', { name: /from account/i });
const toAccount = getByRole('combobox', { name: /to account/i });
const amountField = getByRole('spinbutton', { name: /amount/i });

userEvent.click(fromAccount);
userEvent.click(getByRole('option', { name: /staff account/i }));

userEvent.click(toAccount);
userEvent.click(getByRole('option', { name: /staff savings/i }));

Expand All @@ -227,14 +220,10 @@
it('should handle form submission successfully', async () => {
const { getByRole } = render(<Components />);

const fromAccount = getByRole('combobox', { name: /from account/i });
const toAccount = getByRole('combobox', { name: /to account/i });
const amountField = getByRole('spinbutton', { name: /amount/i });
const submitButton = getByRole('button', { name: /submit/i });

userEvent.click(fromAccount);
userEvent.click(getByRole('option', { name: /staff account/i }));

userEvent.click(toAccount);
userEvent.click(getByRole('option', { name: /staff savings/i }));

Expand Down Expand Up @@ -274,55 +263,25 @@
expect(getByLabelText(/end date/i)).toHaveValue('');
});

it('should validate that from and to accounts are different', async () => {
const { getByRole, queryByRole, getAllByRole } = render(<Components />);
it('locks the From Account to the launching card and excludes it from To Account options', async () => {
const { getByRole, queryByRole } = render(<Components />);

const [fromAccount, toAccount] = getAllByRole('combobox');
const fromAccount = getByRole('combobox', { name: /from account/i });
expect(fromAccount).toHaveTextContent('Staff Account');

userEvent.click(fromAccount);
expect(
getByRole('option', { name: /staff account/i }),
).toBeInTheDocument();
userEvent.click(getByRole('option', { name: /staff account/i }));
expect(queryByRole('listbox')).not.toBeInTheDocument();

userEvent.click(getByRole('combobox', { name: /to account/i }));

userEvent.click(toAccount);
await waitFor(() =>
expect(
queryByRole('option', { name: /staff account/i }),
).not.toBeInTheDocument(),
);

userEvent.click(getByRole('option', { name: /staff savings/i }));
userEvent.click(getByRole('button', { name: /submit/i }));
});

it('should swap accounts when swap button is clicked', async () => {
const { getByRole, getByTestId } = render(<Components />);

const icon = getByTestId('SwapHorizIcon');
const swapButton = icon.closest('button');

expect(swapButton).toBeDisabled();

const fromAccount = getByRole('combobox', { name: /from account/i });
const toAccount = getByRole('combobox', { name: /to account/i });

userEvent.click(fromAccount);
userEvent.click(getByRole('option', { name: /staff account/i }));

userEvent.click(toAccount);
userEvent.click(getByRole('option', { name: /staff savings/i }));

await waitFor(() => {
expect(swapButton).not.toBeDisabled();
});

userEvent.click(swapButton as HTMLButtonElement);

await waitFor(() => {
expect(fromAccount).toHaveTextContent('Staff Savings');
expect(toAccount).toHaveTextContent('Staff Account');
});
expect(
getByRole('option', { name: /staff savings/i }),
).toBeInTheDocument();
});

it('should show/hide end date based on schedule selection', async () => {
Expand Down Expand Up @@ -393,12 +352,8 @@
it('should show information box when amount exceeds limit', async () => {
const { getByRole, findByRole } = render(<Components />);

const fromAccount = getByRole('combobox', { name: /from account/i });
const toAccount = getByRole('combobox', { name: /to account/i });

userEvent.click(fromAccount);
userEvent.click(getByRole('option', { name: /staff account/i }));

userEvent.click(toAccount);
userEvent.click(getByRole('option', { name: /staff savings/i }));

Expand Down Expand Up @@ -441,9 +396,6 @@

const amountField = getByRole('spinbutton', { name: /amount/i });

userEvent.click(getByRole('combobox', { name: /from account/i }));
userEvent.click(getByRole('option', { name: /staff account/i }));

userEvent.click(getByRole('combobox', { name: /to account/i }));
userEvent.click(getByRole('option', { name: /staff savings/i }));

Expand Down Expand Up @@ -474,9 +426,6 @@

const amountField = getByRole('spinbutton', { name: /amount/i });

userEvent.click(getByRole('combobox', { name: /from account/i }));
userEvent.click(getByRole('option', { name: /staff account/i }));

userEvent.click(getByRole('combobox', { name: /to account/i }));
userEvent.click(getByRole('option', { name: /staff savings/i }));

Expand Down
40 changes: 8 additions & 32 deletions src/components/HrTools/SavingsFundTransfer/TransferModal/TransferModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { useState } from 'react';
import East from '@mui/icons-material/East';
import SwapHorizIcon from '@mui/icons-material/SwapHoriz';
import { ArrowRightAlt } from '@mui/icons-material';
import {
Alert,
Box,
Expand All @@ -11,12 +10,10 @@
FormHelperText,
FormLabel,
Grid,
IconButton,
InputLabel,
Radio,
RadioGroup,
TextField,
Tooltip,
Typography,
} from '@mui/material';
import { Formik } from 'formik';
Expand Down Expand Up @@ -309,225 +306,204 @@
{t('From Account')}
</InputLabel>
<TransferModalSelect
notSelected={transferTo}
funds={funds}
funds={funds.filter(
(fund) => fund.fundType === transferFrom,
)}
label={t('From Account')}
labelId="transferFrom"
name="transferFrom"
value={transferFrom}
disabled={!isNew}
onChange={handleChange}
onBlur={handleBlur}
error={
touched.transferFrom &&
Boolean(errors.transferFrom)
}
readOnly

@kegrimes kegrimes Jun 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we add something like this to make it unclickable?

Suggested change
readOnly
readOnly
tabIndex={-1}
sx={{ pointerEvents: 'none' }}

@zweatshirt zweatshirt Jun 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am leaning toward making it clickable (hoverable) for accessibility if that's ok. Otherwise users with screen readers can't know what the from account is. At least that was the intention. With that being said, I should test what the screen reader actually produces on hover here.

IconComponent={() => null}
required
/>
{touched.transferFrom && errors.transferFrom && (
<FormHelperText error={true}>
{errors.transferFrom}
</FormHelperText>
)}
</FormControl>
</Grid>

<Grid
item
xs={12}
sm={1}
sx={{
display: 'flex',
justifyContent: 'center',
alignItems: 'center',
}}
>
<IconButton
onClick={() => {
setFieldValue('transferFrom', transferTo);
setFieldValue('transferTo', transferFrom);
}}
color="primary"
disabled={!transferFrom || !transferTo}
>
<Tooltip title={t('Swap')}>
<SwapHorizIcon />
</Tooltip>
</IconButton>
<ArrowRightAlt />
</Grid>

<Grid item xs={12} sm={5.5}>
<FormControl fullWidth>
<InputLabel id="transferTo">
{t('To Account')}
</InputLabel>
<TransferModalSelect
notSelected={transferFrom}
funds={funds}
label={t('To Account')}
labelId="transferTo"
name="transferTo"
value={transferTo}
onChange={handleChange}
onBlur={handleBlur}
error={
touched.transferTo && Boolean(errors.transferTo)
}
required
/>
{touched.transferTo && errors.transferTo && (
<FormHelperText error={true}>
{errors.transferTo}
</FormHelperText>
)}
</FormControl>
</Grid>
</Grid>
) : (
<Grid container spacing={2} alignItems="center">
<Grid
item
xs={12}
sm={5.5}
sx={{
display: 'flex',
justifyContent: 'flex-end',
alignItems: 'center',
}}
>
<FundInfoDisplay fund={fund} />
</Grid>
<Grid item xs={12} sm={1} sx={{ textAlign: 'center' }}>
<East />
<ArrowRightAlt />
</Grid>
<Grid item xs={12} sm={5.5}>
<FundInfoDisplay
fund={funds.find((f) => f.fundType === transferTo)}
/>
</Grid>
</Grid>
)}
</Box>

{isNew && (
<Box sx={{ mb: 3 }}>
<FormControl
component="fieldset"
error={touched.schedule && Boolean(errors.schedule)}
>
<FormLabel component="legend">{t('Schedule')}</FormLabel>
<RadioGroup
row
name="schedule"
value={schedule}
onChange={(_event, value) => {
setFieldValue('schedule', value);
if (value === ScheduleEnum.Monthly) {
setFieldTouched('transferDate', true, false);
}
validateField('transferDate');
}}
>
<FormControlLabel
value={ScheduleEnum.OneTime}
control={<Radio />}
label={t('One Time')}
/>
<FormControlLabel
value={ScheduleEnum.Monthly}
control={<Radio />}
label={t('Monthly')}
/>
</RadioGroup>
{touched.schedule && errors.schedule && (
<FormHelperText error={true}>
{errors.schedule}
</FormHelperText>
)}
</FormControl>
</Box>
)}

<Box sx={{ mb: 3 }}>
<Grid container spacing={2}>
{schedule === ScheduleEnum.OneTime ? (
<Grid item xs={12}>
<TextField
fullWidth
label={t('Transfer Date')}
value={transferDate.toFormat('MM/dd/yyyy')}
disabled={true}
/>
</Grid>
) : (
<>
<Grid item xs={6}>
<CustomDateField
label={t('Transfer Date')}
value={transferDate}
onChange={(date) => {
setFieldValue('transferDate', date);
setFieldTouched('transferDate', true, false);
}}
error={
touched.transferDate &&
Boolean(errors.transferDate)
}
helperText={
touched.transferDate &&
(errors.transferDate as string)
}
required
/>
</Grid>

<Grid item xs={6}>
<CustomDateField
label={t('End Date (Optional)')}
value={endDate}
onChange={(date) => {
setFieldValue('endDate', date);
setFieldTouched('endDate', true, false);
}}
error={touched.endDate && Boolean(errors.endDate)}
helperText={
touched.endDate && (errors.endDate as string)
}
/>
</Grid>
</>
)}
</Grid>
</Box>

<Box sx={{ mb: 2 }}>
<TextField
fullWidth
label={t('Amount')}
name="amount"
type="number"
value={amount}
onChange={handleChange}
onBlur={handleBlur}
error={touched.amount && Boolean(errors.amount)}
helperText={
errors.amount && touched.amount ? errors.amount : ''
}
InputProps={{
startAdornment: <Typography sx={{ mr: 1 }}>$</Typography>,
}}
required
/>
</Box>

{schedule === ScheduleEnum.OneTime && (
<Box sx={{ mb: 2 }}>
<FormControl fullWidth>
<TextField
id="transfer-note"
label={t('Note (Optional)')}
name="note"

Check notice on line 506 in src/components/HrTools/SavingsFundTransfer/TransferModal/TransferModal.tsx

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ Getting better: Complex Method 

TransferModal:React.FC<TransferModalProps> decreases in cyclomatic complexity from 42 to 38, threshold = 15. 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.
disabled={!isNew}
value={note}
onChange={handleChange}
fullWidth
Expand Down
3 changes: 0 additions & 3 deletions src/components/HrTools/SavingsFundTransfer/TransfersPage/TransfersPage.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,6 @@ describe('TransfersPage', () => {
expect(
await findAllByRole('button', { name: 'TRANSFER FROM' }),
).toHaveLength(2);
expect(await findAllByRole('button', { name: 'TRANSFER TO' })).toHaveLength(
2,
);

const tables = await findAllByRole('grid');
expect(tables.length).toBe(2);
Expand Down
Loading