SAA-645 Require note on savings fund transfer form by jeff-clay · Pull Request #1772 · CruGlobal/mpdx-react

jeff-clay · 2026-05-12T15:09:02Z

Description

Makes the Note field required on one-time fund transfers (previously labeled "Note (Optional)", which produced a confusing SAA 422 when left blank).
Defaults the Note to <lastName> Savings Fund Transfer in MPDX using the current user's lastName. Editable — clearing it triggers the required-field error.
Front-end enforcement only; recurring transfers are unaffected (their description is server-synthesized).

Related: SAA-645

Testing

Open HR Tools → Savings Fund Transfer → New Transfer, One Time schedule.
Confirm Note is pre-filled with <your-last-name> Savings Fund Transfer in MPDX.
Submit unchanged → succeeds.
Open another new transfer, clear Note (or enter only whitespace), submit → "Note is required" inline error, no submission.
Switch to Monthly schedule — no Note field shown, recurring submits normally.

Checklist:

I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)" — using SAA-645 since this work is tracked in the SAA project's Jira
I have applied the appropriate labels (Add the label "Preview" to automatically create a preview environment)
I have run the Claude Code /pr-review command locally and fixed any relevant suggestions
I have requested a review from another person on the project
I have tested my changes in preview or in staging
I have cleaned up my commit history

github-actions · 2026-05-12T15:12:29Z

Preview branch generated at https://SAA-645-require-note-on-transfer-form.d3dytjb8adxkk5.amplifyapp.com

github-actions · 2026-05-12T15:13:10Z

Bundle sizes [mpdx-react]

Compared against 1dea58b

No significant changes found

jeff-clay

Small, well-scoped form/validation change. All SAA-645 acceptance criteria met (required field with visual asterisk, whitespace-only rejected via .trim().min(1), mutation never fires when invalid). Default-note feature is a documented scope addition per product owner. Refactor to useGetUserQuery over parsing staffAccount.name is the right architectural call — Apollo's cache makes the duplicate hook call free. No must-fix issues; a few nice-to-have hardening suggestions and one optional whitespace test below.

jeff-clay · 2026-05-12T17:07:12Z

  const type = data.type || TransferTypeEnum.New;
  const isNew = type === TransferTypeEnum.New;

+  const defaultNote = lastName


[Nice-to-have] Consider trimming lastName before interpolating, so an upstream value like ' Sleight ' doesn't produce a default with leading/trailing whitespace. const trimmed = lastName?.trim(); const defaultNote = trimmed ? t(...) : '';

dr-bizz

This is looking good, but you don't need to add the min onto the yup logic as required does that already.

jeff-clay · 2026-05-13T15:37:17Z

@dr-bizz Changes made. I know you approved already, but not sure if you want to do a review of changes. Not sure of protocol.

dr-bizz · 2026-05-14T13:11:43Z

+const buildCacheWithUser = (lastName: string) => {
+  const cache = createCache();
+  cache.writeQuery<GetUserQuery>({
+    query: GetUserDocument,
+    data: {
+      user: {
+        __typename: 'User',
+        id: 'test-user-id',
+        firstName: '',
+        lastName,
+        avatar: '',
+        staffAccountId: null,
+        primaryDesignation: null,
+        userType: UserTypeEnum.UsStaff,
+        preferences: null,
+      },
+    },
+  });
+  return cache;
+};
+


There is a different way to do this. Let me type it up

dr-bizz

Thank you for fixing that issue. We have a few more issues that have arisen.

dr-bizz · 2026-05-14T13:34:51Z

+  lastName?: string;
 }

+const buildCacheWithUser = (lastName: string) => {


We don't need to build the cache. We can do the same as we've done here: src/components/DonationTable/DonationTable.test.tsx

We create the mocks on the GqlMockedProvider, which will mock the GQL queries

dr-bizz · 2026-05-14T13:37:32Z

      .min(0.01, i18n.t('Amount must be at least $0.01')),
-    note: yup.string().nullable(),
+    note: yup.string().when('schedule', {
+      is: ScheduleEnum.OneTime,


Are we only adding the required note if it is a one time payment?

Yeah, a few reasons:

The note field currently only renders for one time transfers.

The recurring GraphQL mutation doesn't accept a description

SAA's recurring transfer synthesizes its own description server-side from the fund/account name.

dr-bizz · 2026-05-14T13:40:36Z

            createTransfer: CreateTransferMutation;
            updateRecurringTransfer: UpdateRecurringTransferMutation;
          }>
+            cache={buildCacheWithUser(lastName)}


We can mock the response of the GQL query here so we don't need to store it in the cache. (I don't know if this will work out the box, as I haven't tested it, you might need to edit the GetUser)

We only need to enter the values we want to mock as we are randomly generating everything else.

Suggested change

cache={buildCacheWithUser(lastName)}

mocks={{

GetUser {

user {

id

firstName

lastName

}

}

}}

Tried your mocks suggestion. It works for the mocks setup, but with useGetUserQuery inside the modal the data resolves async, and Formik captures initialValues at mount, so the default note ends up empty in that brief window.

Added a render-guard (if (userLoading) return null) as the smallest fix. In production it never fires (cache is warmed at app boot by UserPreferenceProvider). In tests it lets the mock resolve before assertions run, and let me drop the cache builder entirely.

If you'd prefer a different shape (e.g. compute the default via useEffect + setFieldValue, or not handle the loading case in the modal at all) happy to change. Just let me know.

dr-bizz

Thank you for making those edits. MPDX React is a large app, which means we need to keep it consistent throughout.

I noticed some more issues and have added my suggestions for you to implement.

The loading from GraphQL gets triggered every time it loads, both during the initial load and subsequent sequential loads.

I believe we can resolve this issue with a useMemo, which updates the default value when userData changes.

dr-bizz · 2026-05-15T15:34:21Z

+  if (userLoading) {
+    return null;
+  }


We don't want to do this, as it can refetch data, and then we would no longer render the component.

See the next comment

dr-bizz · 2026-05-15T15:40:32Z

+  const { data: userData, loading: userLoading } = useGetUserQuery();
+
+  if (userLoading) {
+    return null;
+  }
+
+  const trimmedLastName = userData?.user?.lastName?.trim();
+  const defaultNote = trimmedLastName
+    ? t('{{lastName}} Savings Fund Transfer in MPDX', {
+        lastName: trimmedLastName,
+      })
+    : '';
+
  const title =
    type === TransferTypeEnum.New


We can use useMemo to reinitialise the variable defaultNote upon userData changing.

Suggested change

const { data: userData, loading: userLoading } = useGetUserQuery();

if (userLoading) {

return null;

}

const trimmedLastName = userData?.user?.lastName?.trim();

const defaultNote = trimmedLastName

? t('{{lastName}} Savings Fund Transfer in MPDX', {

lastName: trimmedLastName,

})

: '';

const title =

type === TransferTypeEnum.New

const { data: userData } = useGetUserQuery();

const defaultNote = useMemo(() => {

const trimmedLastName = userData?.user?.lastName?.trim();

return trimmedLastName

? t('{{lastName}} Savings Fund Transfer in MPDX', {

lastName: trimmedLastName,

})

: '';

}, [userData, t]);

const title =

type === TransferTypeEnum.New

dr-bizz · 2026-05-15T15:49:26Z

+const renderModal = async (props: ComponentsProps = {}) => {
+  const result = render(<Components {...props} />);
+  await result.findByText(/Fund Transfer/);
+  return result;
+};


With now we're using useMemo, I'm unsure if you need this.

Require note on savings fund transfer form

9912aa4

jeff-clay added the Preview Environment Add this label to create an Amplify Preview label May 12, 2026

Default note to <lastName> Savings Fund Transfer in MPDX

edda24d

jeff-clay commented May 12, 2026

View reviewed changes

Trim lastName and lock in whitespace-only note rejection

417b0af

jeff-clay requested a review from dr-bizz May 12, 2026 17:29

Consolidate empty/whitespace note tests with it.each

b05637e

dr-bizz approved these changes May 12, 2026

View reviewed changes

Comment thread src/components/HrTools/SavingsFundTransfer/TransferModal/TransferModal.tsx Outdated

Comment thread src/components/HrTools/SavingsFundTransfer/TransfersPage/TransfersPage.tsx Outdated

Move user query into modal, drop redundant min

6f5c546

dr-bizz reviewed May 14, 2026

View reviewed changes

dr-bizz requested changes May 14, 2026

View reviewed changes

jeff-clay added 2 commits May 14, 2026 10:01

Use mocks instead of cache; guard modal while loading

fcda991

Run prettier on test file

2ae7c7d

jeff-clay requested a review from dr-bizz May 15, 2026 15:02

dr-bizz requested changes May 15, 2026

View reviewed changes

jeff-clay assigned BradleySchlather May 15, 2026

Conversation

jeff-clay commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Checklist:

Uh oh!

github-actions Bot commented May 12, 2026

Uh oh!

github-actions Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bundle sizes [mpdx-react]

Uh oh!

jeff-clay left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dr-bizz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jeff-clay commented May 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dr-bizz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeff-clay May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dr-bizz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jeff-clay commented May 12, 2026 •

edited

Loading

github-actions Bot commented May 12, 2026 •

edited

Loading

jeff-clay May 14, 2026 •

edited

Loading