From 2f9d1ed3d6aa799bd1378d767702bc818d9b2446 Mon Sep 17 00:00:00 2001 From: Daniel Bisgrove Date: Tue, 10 Sep 2024 17:06:39 -0400 Subject: [PATCH 1/4] Remove zero amount confirmation and logic and replace with a error message stating pledges cannot be set to zero. In this commit I also add snackbar to the graphQL request to show the user what is going on and I fixed am issue with the new commitment currency. --- .../UpdateDonationsModal.test.tsx | 89 +++---------------- .../UpdateDonationsModal.tsx | 76 ++++++++-------- 2 files changed, 51 insertions(+), 114 deletions(-) diff --git a/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.test.tsx b/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.test.tsx index a7b6befb2..a56d9a7ca 100644 --- a/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.test.tsx +++ b/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.test.tsx @@ -419,9 +419,9 @@ describe('UpdateDonationsModal', () => { describe('Update donations functionality', () => { const { appeal: _appeal, ...restOfPledge } = defaultPledge; describe('Zero amount', () => { - it('should show zero amount confirmation', async () => { - const { getByRole, getByTestId, findByRole, findAllByRole } = render( - , + it('should show error message if total is zero and no pledge', async () => { + const { getByRole, findByRole, findAllByRole } = render( + , ); const checkboxes = await findAllByRole('checkbox'); @@ -440,92 +440,31 @@ describe('UpdateDonationsModal', () => { userEvent.click(getByRole('button', { name: 'Save' })); - expect(getByRole('heading', { name: 'Confirm' })).toBeInTheDocument(); - - expect(getByTestId('confirmModalMessage')).toHaveTextContent( - 'The total amount is zero.', - ); - }); - - it('should move contact to Received when pledge is present', async () => { - const { getByRole, findAllByRole } = render(); - const checkboxes = await findAllByRole('checkbox'); - userEvent.click(checkboxes[0]); - userEvent.click(checkboxes[2]); - userEvent.click(getByRole('button', { name: 'Save' })); - - expect(getByRole('heading', { name: 'Confirm' })).toBeInTheDocument(); - - userEvent.click(getByRole('button', { name: 'Yes' })); - await waitFor(() => - expect(mutationSpy).toHaveGraphqlOperation( - 'UpdateAccountListPledge', + expect(mockEnqueue).toHaveBeenCalledWith( + 'Unable to create a pledge with the amount of $0', { - input: { - pledgeId: defaultPledge.id, - attributes: { - ...restOfPledge, - appealId: appealId, - contactId: defaultContact.id, - status: PledgeStatusEnum.ReceivedNotProcessed, - }, - }, + variant: 'error', }, ), ); - - expect(mutationSpy).toHaveGraphqlOperation('UpdateDonations', { - input: { - accountListId, - attributes: [ - { - id: 'donation-1', - appealId: 'none', - }, - { - id: 'donation-3', - appealId: 'appealId', - }, - ], - }, - }); }); - it('should move contact to Asked when pledge is NOT present', async () => { - const { getByRole } = render(); - - await waitFor(() => - expect(getByRole('button', { name: 'Save' })).not.toBeDisabled(), - ); - + it('should show error message if total is zero and has a pledge', async () => { + const { getByRole, findAllByRole } = render(); + const checkboxes = await findAllByRole('checkbox'); + userEvent.click(checkboxes[0]); + userEvent.click(checkboxes[2]); userEvent.click(getByRole('button', { name: 'Save' })); - expect(getByRole('heading', { name: 'Confirm' })).toBeInTheDocument(); - - userEvent.click(getByRole('button', { name: 'Yes' })); - await waitFor(() => - expect(mutationSpy).toHaveGraphqlOperation( - 'CreateAccountListPledge', + expect(mockEnqueue).toHaveBeenCalledWith( + 'Unable to create a pledge with the amount of $0', { - input: { - accountListId, - attributes: { - appealId: appealId, - contactId: defaultContact.id, - expectedDate: '2020-01-01', - amount: 0, - }, - }, + variant: 'error', }, ), ); - - expect(mutationSpy).not.toHaveGraphqlOperation('UpdateDonations'); - expect(mutationSpy).not.toHaveGraphqlOperation( - 'UpdateAccountListPledge', - ); }); }); diff --git a/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.tsx b/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.tsx index 15c5d1098..8e665d94d 100644 --- a/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.tsx +++ b/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.tsx @@ -1,6 +1,7 @@ import React, { ReactNode, useEffect, useMemo, useState } from 'react'; import { DialogActions, DialogContent, DialogContentText } from '@mui/material'; import { DateTime } from 'luxon'; +import { useSnackbar } from 'notistack'; import { Trans, useTranslation } from 'react-i18next'; import { useGetContactDonationsQuery } from 'src/components/Contacts/ContactDetails/ContactDonationsTab/ContactDonationsTab.generated'; import { DonationRow } from 'src/components/DonationTable/DonationTable'; @@ -48,18 +49,16 @@ export const UpdateDonationsModal: React.FC = ({ }) => { const locale = useLocale(); const { t } = useTranslation(); + const { enqueueSnackbar } = useSnackbar(); const { accountListId, appealId } = React.useContext( AppealsContext, ) as AppealsType; const [selectedDonations, setSelectedDonations] = useState([]); - const [zeroAmountConfirmationMessage, setZeroAmountConfirmationMessage] = - useState(null); const [ lessThanPledgeConfirmationMessage, setLessThanPledgeConfirmationMessage, ] = useState(null); const [saveDisabled, setSaveDisabled] = useState(true); - const [status, setStatus] = useState(); const [pageSize, setPageSize] = useState(25); const { data } = useGetContactDonationsQuery({ @@ -108,18 +107,10 @@ export const UpdateDonationsModal: React.FC = ({ const handleUpdateDonations = async () => { if (!totalSelectedDonationsAmount) { - if (pledge) { - setStatus(PledgeStatusEnum.ReceivedNotProcessed); - } - setZeroAmountConfirmationMessage( - }} - />, - ); + enqueueSnackbar(t('Unable to create a pledge with the amount of $0'), { + variant: 'error', + }); + return; } else if (pledge && totalSelectedDonationsAmount < pledge.amount) { setLessThanPledgeConfirmationMessage( = ({ components={{ bold: }} />, ); - setStatus(PledgeStatusEnum.Processed); } else { if (pledge) { await commit({ @@ -144,20 +134,6 @@ export const UpdateDonationsModal: React.FC = ({ } }; - const zeroAmountConfirmationMutation = async () => { - if (pledge) { - await commit({ - updatedPledge: { - ...pledge, - status, - }, - }); - } else { - await commit({}); - } - setZeroAmountConfirmationMessage(null); - }; - const lessThanPledgeConfirmationMutation = async () => { if (pledge) { try { @@ -224,6 +200,16 @@ export const UpdateDonationsModal: React.FC = ({ }, }, refetchQueries: ['Contacts', 'Appeal'], + onCompleted: () => { + enqueueSnackbar(t('Successfully updated donations'), { + variant: 'success', + }); + }, + onError: () => { + enqueueSnackbar(t('Error while updating donations'), { + variant: 'error', + }); + }, }); } if (updatedPledge?.id) { @@ -241,6 +227,16 @@ export const UpdateDonationsModal: React.FC = ({ }, }, refetchQueries: ['Contacts', 'Appeal'], + onCompleted: () => { + enqueueSnackbar(t('Successfully updated commitment'), { + variant: 'success', + }); + }, + onError: () => { + enqueueSnackbar(t('Error while updating commitment'), { + variant: 'error', + }); + }, }); } else if (!donationsAttributes.length) { // Create pledge if no pledge and no donations @@ -253,10 +249,21 @@ export const UpdateDonationsModal: React.FC = ({ contactId: contact.id, expectedDate: DateTime.local().startOf('day').toISODate(), amount: totalSelectedDonationsAmount, + amountCurrency: 'USD', }, }, }, refetchQueries: ['Contacts', 'Appeal'], + onCompleted: () => { + enqueueSnackbar(t('Successfully created a new commitment'), { + variant: 'success', + }); + }, + onError: () => { + enqueueSnackbar(t('Error while trying to create a commitment'), { + variant: 'error', + }); + }, }); } handleClose(); @@ -345,15 +352,6 @@ export const UpdateDonationsModal: React.FC = ({ - {zeroAmountConfirmationMessage && ( - setZeroAmountConfirmationMessage(null)} - message={zeroAmountConfirmationMessage} - mutation={zeroAmountConfirmationMutation} - /> - )} {lessThanPledgeConfirmationMessage && ( Date: Wed, 11 Sep 2024 09:34:30 -0400 Subject: [PATCH 2/4] Moving accountCurrency to parent so the parent can use it when creating a new commitment/pledge. --- .../DonationTable/DonationTable.tsx | 14 ++----- .../UpdateDonationsModal.test.tsx | 40 +++++++++++++++++++ .../UpdateDonationsModal.tsx | 13 ++++-- 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/src/components/Tool/Appeal/Modals/UpdateDonationsModal/DonationTable/DonationTable.tsx b/src/components/Tool/Appeal/Modals/UpdateDonationsModal/DonationTable/DonationTable.tsx index b02a86e4c..dd4312d05 100644 --- a/src/components/Tool/Appeal/Modals/UpdateDonationsModal/DonationTable/DonationTable.tsx +++ b/src/components/Tool/Appeal/Modals/UpdateDonationsModal/DonationTable/DonationTable.tsx @@ -22,7 +22,6 @@ import { } from 'src/components/DonationTable/DonationTable'; import { DonationTableQueryVariables, - useAccountListCurrencyQuery, useDonationTableQuery, } from 'src/components/DonationTable/DonationTable.generated'; import { useFetchAllPages } from 'src/hooks/useFetchAllPages'; @@ -33,13 +32,13 @@ import { currencyFormat, dateFormatShort } from 'src/lib/intlFormat'; type RenderCell = GridColDef['renderCell']; export interface DonationTableProps { - accountListId: string; appealId: string; filter: Partial; loading?: boolean; onSelectContact?: (contactId: string) => void; visibleColumnsStorageKey: string; emptyPlaceholder: React.ReactElement; + accountCurrency: string; selectedDonations: DonationRow[]; setSelectedDonations: React.Dispatch>; totalSelectedDonationsAmount: number; @@ -69,12 +68,12 @@ const StyledListItemIcon = styled(ListItemIcon)(() => ({ })); export const DonationTable: React.FC = ({ - accountListId, appealId, filter, loading: skipped = false, visibleColumnsStorageKey, emptyPlaceholder, + accountCurrency, selectedDonations = [], setSelectedDonations, totalSelectedDonationsAmount = 0, @@ -106,15 +105,8 @@ export const DonationTable: React.FC = ({ setSelectedDonations(preselectedDonations); }, [data]); - const { data: accountListData, loading: loadingAccountListData } = - useAccountListCurrencyQuery({ - variables: { accountListId }, - }); - const nodes = data?.donations.nodes || []; - const accountCurrency = accountListData?.accountList.currency || 'USD'; - const donations = useMemo(() => nodes.map(createDonationRow), [nodes]); const date: RenderCell = ({ row }) => dateFormatShort(row.date, locale); @@ -269,7 +261,7 @@ export const DonationTable: React.FC = ({ )} - ) : loadingAccountListData || loading || skipped ? ( + ) : loading || skipped ? ( diff --git a/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.test.tsx b/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.test.tsx index a56d9a7ca..158febd57 100644 --- a/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.test.tsx +++ b/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.test.tsx @@ -643,4 +643,44 @@ describe('UpdateDonationsModal', () => { }); }); }); + describe('Creating a new pledge', () => { + it('should show create pledge with pre-selected donations', async () => { + const { getByRole, findByRole, findAllByRole } = render( + , + ); + + const checkboxes = await findAllByRole('checkbox'); + expect(checkboxes).toHaveLength(3); + + const totalRow = within( + await findByRole('table', { name: 'Donation Totals' }), + ).getByRole('row'); + expect(totalRow.children[0]).toHaveTextContent('Total Donations: CA$10'); + + userEvent.click(getByRole('button', { name: 'Save' })); + + await waitFor(() => + expect(mutationSpy).toHaveGraphqlOperation('CreateAccountListPledge', { + input: { + accountListId, + attributes: { + appealId: appealId, + contactId: defaultContact.id, + expectedDate: '2020-01-01', + amount: 10, + amountCurrency: 'CAD', + }, + }, + }), + ); + + expect(mockEnqueue).toHaveBeenCalledWith( + 'Successfully created a new commitment', + { + variant: 'success', + }, + ); + }); + + }); }); diff --git a/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.tsx b/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.tsx index 8e665d94d..4110f8b25 100644 --- a/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.tsx +++ b/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.tsx @@ -7,6 +7,7 @@ import { useGetContactDonationsQuery } from 'src/components/Contacts/ContactDeta import { DonationRow } from 'src/components/DonationTable/DonationTable'; import { DonationTableQueryVariables, + useAccountListCurrencyQuery, useDonationTableQuery, } from 'src/components/DonationTable/DonationTable.generated'; import { EmptyDonationsTable } from 'src/components/common/EmptyDonationsTable/EmptyDonationsTable'; @@ -61,6 +62,12 @@ export const UpdateDonationsModal: React.FC = ({ const [saveDisabled, setSaveDisabled] = useState(true); const [pageSize, setPageSize] = useState(25); + const { data: accountListData, loading: loadingAccountListData } = + useAccountListCurrencyQuery({ + variables: { accountListId: accountListId ?? '' }, + }); + const accountCurrency = accountListData?.accountList.currency || 'USD'; + const { data } = useGetContactDonationsQuery({ variables: { accountListId: accountListId ?? '', @@ -249,7 +256,7 @@ export const UpdateDonationsModal: React.FC = ({ contactId: contact.id, expectedDate: DateTime.local().startOf('day').toISODate(), amount: totalSelectedDonationsAmount, - amountCurrency: 'USD', + amountCurrency: accountCurrency, }, }, }, @@ -320,10 +327,9 @@ export const UpdateDonationsModal: React.FC = ({ = ({ })} /> } + accountCurrency={accountCurrency} visibleColumnsStorageKey="contact-donations" selectedDonations={selectedDonations} setSelectedDonations={setSelectedDonations} From 96c31f8ae6525bb20bcc03d27c0f1a4aa5880304 Mon Sep 17 00:00:00 2001 From: Daniel Bisgrove Date: Wed, 11 Sep 2024 09:35:12 -0400 Subject: [PATCH 3/4] Adding success message when creating a new pledge and adding better test coverage. --- .../UpdateDonationsModal.test.tsx | 48 +++++++++++++++++++ .../UpdateDonationsModal.tsx | 5 ++ 2 files changed, 53 insertions(+) diff --git a/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.test.tsx b/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.test.tsx index 158febd57..ba104bc68 100644 --- a/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.test.tsx +++ b/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.test.tsx @@ -682,5 +682,53 @@ describe('UpdateDonationsModal', () => { ); }); + it('should show create pledge via updating donations', async () => { + const { getByRole, findByRole, findAllByRole } = render( + , + ); + + const checkboxes = await findAllByRole('checkbox'); + userEvent.click(checkboxes[0]); + userEvent.click(checkboxes[1]); + + const totalRow = within( + await findByRole('table', { name: 'Donation Totals' }), + ).getByRole('row'); + expect(totalRow.children[0]).toHaveTextContent('Total Donations: CA$10'); + + userEvent.click(getByRole('button', { name: 'Save' })); + + await waitFor(() => + expect(mutationSpy).toHaveGraphqlOperation('UpdateDonations', { + input: { + accountListId, + attributes: [ + { + id: 'donation-1', + appealId: 'none', + }, + { + id: 'donation-2', + appealId: 'appealId', + }, + ], + }, + }), + ); + + expect(mockEnqueue).toHaveBeenCalledWith( + 'Successfully updated donations', + { + variant: 'success', + }, + ); + + expect(mockEnqueue).toHaveBeenCalledWith( + 'Successfully created a new commitment', + { + variant: 'success', + }, + ); + }); }); }); diff --git a/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.tsx b/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.tsx index 4110f8b25..1c1630ba3 100644 --- a/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.tsx +++ b/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.tsx @@ -211,6 +211,11 @@ export const UpdateDonationsModal: React.FC = ({ enqueueSnackbar(t('Successfully updated donations'), { variant: 'success', }); + if (!updatedPledge) { + enqueueSnackbar(t('Successfully created a new commitment'), { + variant: 'success', + }); + } }, onError: () => { enqueueSnackbar(t('Error while updating donations'), { From 537448008df272db8c3e0a4c9b009582555cfe26 Mon Sep 17 00:00:00 2001 From: Daniel Bisgrove Date: Wed, 11 Sep 2024 10:17:20 -0400 Subject: [PATCH 4/4] Boost codecoverage --- .../UpdateDonationsModal.test.tsx | 98 ++++++++++++++++++- 1 file changed, 94 insertions(+), 4 deletions(-) diff --git a/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.test.tsx b/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.test.tsx index ba104bc68..6952798d8 100644 --- a/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.test.tsx +++ b/src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsModal.test.tsx @@ -67,6 +67,8 @@ interface ComponentsProps { hasForeignCurrency?: boolean; hasMultiplePages?: boolean; zeroAmount?: boolean; + noDonationsMatchAppeal?: boolean; + mutationsThrowErrors?: boolean; } const Components = ({ @@ -75,6 +77,8 @@ const Components = ({ hasForeignCurrency = false, hasMultiplePages = false, zeroAmount = false, + noDonationsMatchAppeal = false, + mutationsThrowErrors = false, }: ComponentsProps) => { return ( @@ -89,6 +93,21 @@ const Components = ({ DonationTable: DonationTableQuery; }> mocks={{ + UpdateDonations: mutationsThrowErrors + ? () => { + throw new Error('Server Error'); + } + : {}, + UpdateAccountListPledge: mutationsThrowErrors + ? () => { + throw new Error('Server Error'); + } + : {}, + CreateAccountListPledge: mutationsThrowErrors + ? () => { + throw new Error('Server Error'); + } + : {}, AccountListCurrency: { accountList: { currency: 'CAD', @@ -112,7 +131,9 @@ const Components = ({ currency: 'CAD', }, appeal: { - id: appealId, + id: noDonationsMatchAppeal + ? 'appeal-99' + : appealId, name: 'Appeal 1', }, designationAccount: { @@ -220,12 +241,14 @@ const Components = ({ describe('UpdateDonationsModal', () => { it('default', () => { - const { getByRole } = render(); + const { getByRole, getByTestId } = render(); expect( getByRole('heading', { name: 'Update Donations' }), ).toBeInTheDocument(); + expect(getByTestId('LoadingBox')).toBeInTheDocument(); + expect(getByRole('button', { name: 'Cancel' })).toBeInTheDocument(); expect(getByRole('button', { name: 'Save' })).toBeInTheDocument(); expect(getByRole('button', { name: 'Save' })).toBeDisabled(); @@ -263,11 +286,15 @@ describe('UpdateDonationsModal', () => { describe('Donations', () => { it('should show no donations', async () => { - const { findByText, getByRole } = render(); + const { findByText, getByRole, queryByTestId } = render( + , + ); expect( await findByText(`No donations received for ${defaultContact.name}`), ).toBeInTheDocument(); + expect(queryByTestId('LoadingBox')).not.toBeInTheDocument(); + expect( getByRole('button', { name: 'Add New Donation', @@ -311,12 +338,26 @@ describe('UpdateDonationsModal', () => { }); describe('Donations - Table functionality', () => { + it('uses the default total and disabled save when no donations are selected', async () => { + const { findByRole, getByRole } = render( + , + ); + + const totalRow = within( + await findByRole('table', { name: 'Donation Totals' }), + ).getByRole('row'); + expect(totalRow.children[0]).toHaveTextContent('Total Donations: CA$0'); + + expect(getByRole('button', { name: 'Save' })).toBeDisabled(); + }); + it('hides currency column when all currencies match the account currency', async () => { - const { queryByRole, findByRole } = render(); + const { queryByRole, findByRole, queryByTestId } = render(); expect( await findByRole('cell', { name: 'Designation Account 1' }), ).toBeInTheDocument(); + expect(queryByTestId('LoadingBox')).not.toBeInTheDocument(); expect( queryByRole('columnheader', { name: 'Foreign Amount' }), ).not.toBeInTheDocument(); @@ -416,6 +457,55 @@ describe('UpdateDonationsModal', () => { }); }); + describe('Handle mutation errors', () => { + it('Update pledge errors', async () => { + const { getByRole, findAllByRole } = render( + , + ); + + const checkboxes = await findAllByRole('checkbox'); + expect(checkboxes).toHaveLength(3); + userEvent.click(checkboxes[1]); + + userEvent.click(getByRole('button', { name: 'Save' })); + + await waitFor(() => { + expect(mockEnqueue).toHaveBeenCalledWith( + 'Error while updating donations', + { + variant: 'error', + }, + ); + expect(mockEnqueue).toHaveBeenCalledWith( + 'Error while updating commitment', + { + variant: 'error', + }, + ); + }); + }); + + it('Create pledge errors', async () => { + const { getByRole, findAllByRole } = render( + , + ); + + const checkboxes = await findAllByRole('checkbox'); + expect(checkboxes).toHaveLength(3); + + userEvent.click(getByRole('button', { name: 'Save' })); + + await waitFor(() => { + expect(mockEnqueue).toHaveBeenCalledWith( + 'Error while trying to create a commitment', + { + variant: 'error', + }, + ); + }); + }); + }); + describe('Update donations functionality', () => { const { appeal: _appeal, ...restOfPledge } = defaultPledge; describe('Zero amount', () => {