From ddfc4a11ccf7ba810eadaa4ce978305a59dabfbb Mon Sep 17 00:00:00 2001 From: Caleb Alldrin Date: Thu, 27 Jun 2024 18:34:12 -0700 Subject: [PATCH 1/5] Add updateDuplicate mutation and fix query --- .../Tool/MergePeople/GetPersonDuplicates.graphql | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/components/Tool/MergePeople/GetPersonDuplicates.graphql b/src/components/Tool/MergePeople/GetPersonDuplicates.graphql index 3aaa009ec..88a937a07 100644 --- a/src/components/Tool/MergePeople/GetPersonDuplicates.graphql +++ b/src/components/Tool/MergePeople/GetPersonDuplicates.graphql @@ -2,9 +2,17 @@ mutation MergePeopleBulk($input: MergePeopleBulkInput!) { mergePeopleBulk(input: $input) } +mutation UpdateDuplicate($input: DuplicatesUpdateMutationInput!) { + updateDuplicate(input: $input) { + duplicate { + id + } + } +} + query GetPersonDuplicates($accountListId: ID!) { # TODO: Eventually needs pagination (Jira issue: MPDX-7642) - personDuplicates(accountListId: $accountListId, first: 50) { + personDuplicates(accountListId: $accountListId, ignore: false, first: 10) { nodes { id reason From 37c7d495df9a2669a1f6d42357168a3f2ba3ff8d Mon Sep 17 00:00:00 2001 From: Caleb Alldrin Date: Thu, 27 Jun 2024 19:03:25 -0700 Subject: [PATCH 2/5] pass duplicateId for ignoring duplicates --- .../Tool/MergeContacts/ContactPair.tsx | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/components/Tool/MergeContacts/ContactPair.tsx b/src/components/Tool/MergeContacts/ContactPair.tsx index cab5c1598..1f7373aae 100644 --- a/src/components/Tool/MergeContacts/ContactPair.tsx +++ b/src/components/Tool/MergeContacts/ContactPair.tsx @@ -282,14 +282,21 @@ const ContactItem: React.FC = ({ interface Props { contact1: RecordInfoFragment | PersonInfoFragment; contact2: RecordInfoFragment | PersonInfoFragment; - update: (id1: string, id2: string, action: string) => void; + update: ( + id1: string, + id2: string, + duplicateId: string, + action: string, + ) => void; updating: boolean; setContactFocus: SetContactFocus; + duplicateId: string; } const ContactPair: React.FC = ({ contact1, contact2, + duplicateId, update, updating, setContactFocus, @@ -306,19 +313,19 @@ const ContactPair: React.FC = ({ switch (side) { case 'left': setSelected('left'); - update(contact1.id, contact2.id, 'merge'); + update(contact1.id, contact2.id, duplicateId, 'merge'); break; case 'right': setSelected('right'); - update(contact2.id, contact1.id, 'merge'); + update(contact2.id, contact1.id, duplicateId, 'merge'); break; - case 'cancel': - setSelected('cancel'); - update(contact1.id, contact2.id, 'cancel'); + case 'ignore': + setSelected('ignore'); + update(contact1.id, contact2.id, duplicateId, 'ignore'); break; default: setSelected(''); - update(contact1.id, contact2.id, 'cancel'); + update(contact1.id, contact2.id, duplicateId, 'ignore'); } } }; @@ -382,9 +389,9 @@ const ContactPair: React.FC = ({ updateState('cancel')} + onClick={() => updateState('ignore')} className={ - selected === 'cancel' ? classes.red : classes.grey + selected === 'ignore' ? classes.red : classes.grey } data-testid="ignoreButton" > From ee12c354812a0e602885581f3161235ed86d0c97 Mon Sep 17 00:00:00 2001 From: Caleb Alldrin Date: Thu, 27 Jun 2024 19:06:02 -0700 Subject: [PATCH 3/5] handle multiple mutations with handleBulkUpdateDuplicates() --- .../Tool/MergeContacts/MergeContacts.tsx | 168 +++++++++++++----- .../Tool/MergePeople/MergePeople.tsx | 168 +++++++++++++----- 2 files changed, 248 insertions(+), 88 deletions(-) diff --git a/src/components/Tool/MergeContacts/MergeContacts.tsx b/src/components/Tool/MergeContacts/MergeContacts.tsx index d9c8bf2b9..96af8f260 100644 --- a/src/components/Tool/MergeContacts/MergeContacts.tsx +++ b/src/components/Tool/MergeContacts/MergeContacts.tsx @@ -11,8 +11,10 @@ import { Trans, useTranslation } from 'react-i18next'; import { makeStyles } from 'tss-react/mui'; import { SetContactFocus } from 'pages/accountLists/[accountListId]/tools/useToolsHelper'; import { useMassActionsMergeMutation } from 'src/components/Contacts/MassActions/Merge/MassActionsMerge.generated'; +import { TypeEnum } from 'src/graphql/types.generated'; import useGetAppSettings from 'src/hooks/useGetAppSettings'; import theme from '../../../theme'; +import { useUpdateDuplicateMutation } from '../MergePeople/GetPersonDuplicates.generated'; import NoData from '../NoData'; import ContactPair from './ContactPair'; import { useGetContactDuplicatesQuery } from './GetContactDuplicates.generated'; @@ -66,6 +68,7 @@ const MergeContacts: React.FC = ({ }); const { appName } = useGetAppSettings(); const [contactsMerge, { loading: updating }] = useMassActionsMergeMutation(); + const [updateDuplicates] = useUpdateDuplicateMutation(); const disabled = useMemo( () => updating || !Object.entries(actions).length, [actions, updating], @@ -73,13 +76,19 @@ const MergeContacts: React.FC = ({ const totalCount = data?.contactDuplicates.totalCount || 0; const duplicatesDisplayedCount = data?.contactDuplicates.nodes.length || 0; - const updateActions = (id1: string, id2: string, action: string): void => { + const updateActions = ( + id1: string, + id2: string, + duplicateId: string, + action: string, + ): void => { if (!updating) { - if (action === 'cancel') { + if (action === 'ignore') { setActions((prevState) => ({ ...prevState, [id1]: { action: '' }, [id2]: { action: '' }, + [duplicateId]: { action: 'ignore' }, })); } else { setActions((prevState) => ({ @@ -91,40 +100,112 @@ const MergeContacts: React.FC = ({ } }; - const mergeContacts = async () => { - const mergeActions = Object.entries(actions).filter( - (action) => action[1].action === 'merge', - ); - if (mergeActions.length) { - const winnersAndLosers: { winnerId: string; loserId: string }[] = - mergeActions.map((action) => { - return { winnerId: action[0], loserId: action[1].mergeId || '' }; - }); - await contactsMerge({ - variables: { - input: { - winnersAndLosers, - }, - }, - update: (cache) => { - // Delete the loser contacts and remove dangling references to them - winnersAndLosers.forEach((contact) => { - cache.evict({ id: `Contact:${contact.loserId}` }); + const handleBulkUpdateDuplicates = async () => { + try { + const callsByDuplicate: (() => Promise<{ success: boolean }>)[] = []; + + const mergeActions = Object.entries(actions).filter( + (action) => action[1].action === 'merge', + ); + + if (mergeActions.length) { + const winnersAndLosers: { winnerId: string; loserId: string }[] = + mergeActions.map((action) => { + return { winnerId: action[0], loserId: action[1].mergeId || '' }; }); - cache.gc(); - }, - onCompleted: () => { - enqueueSnackbar(t('Success!'), { + + const callMergeDuplicatesMutation = () => + mergeDuplicates(winnersAndLosers); + callsByDuplicate.push(callMergeDuplicatesMutation); + } + + const duplicatesToIgnore = Object.entries(actions) + .filter((action) => action[1].action === 'ignore') + .map((action) => action[0]); + + if (duplicatesToIgnore.length) { + duplicatesToIgnore.forEach((duplicateId) => { + const callIgnoreDuplicateMutation = () => + ignoreDuplicates(duplicateId); + callsByDuplicate.push(callIgnoreDuplicateMutation); + }); + } + + if (callsByDuplicate.length) { + const results = await Promise.all( + callsByDuplicate.map((call) => call()), + ); + + const failedUpdates = results.filter( + (result) => !result.success, + ).length; + const successfulUpdates = results.length - failedUpdates; + + if (successfulUpdates) { + enqueueSnackbar(t(`Updated ${successfulUpdates} duplicate(s)`), { variant: 'success', }); + } + if (failedUpdates) { + enqueueSnackbar( + t(`Error when updating ${failedUpdates} duplicate(s)`), + { + variant: 'error', + }, + ); + } + } else { + enqueueSnackbar(t(`No duplicates were updated`), { + variant: 'warning', + }); + } + } catch (error) { + enqueueSnackbar(t(`Error updating duplicates`), { variant: 'error' }); + } + }; + + const ignoreDuplicates = async (duplicateId: string) => { + await updateDuplicates({ + variables: { + input: { + attributes: { + ignore: true, + }, + type: TypeEnum.Contact, + id: duplicateId, }, - onError: (err) => { - enqueueSnackbar(t('A server error occurred. {{err}}', { err }), { - variant: 'error', - }); + }, + update: (cache) => { + // Delete the duplicate + cache.evict({ id: `ContactDuplicate:${duplicateId}` }); + cache.gc(); + }, + onError: () => { + return { success: false }; + }, + }); + return { success: true }; + }; + + const mergeDuplicates = async (winnersAndLosers) => { + await contactsMerge({ + variables: { + input: { + winnersAndLosers, }, - }); - } + }, + update: (cache) => { + // Delete the contacts and remove dangling references to them + winnersAndLosers.forEach((contact) => { + cache.evict({ id: `Contact:${contact.loserId}` }); + }); + cache.gc(); + }, + onError: () => { + return { success: false }; + }, + }); + return { success: true }; }; return ( @@ -170,22 +251,21 @@ const MergeContacts: React.FC = ({ duplicatesDisplayedCount={duplicatesDisplayedCount} disabled={disabled} totalCount={totalCount} - confirmAction={mergeContacts} + confirmAction={handleBulkUpdateDuplicates} setActions={setActions} /> - {data?.contactDuplicates.nodes - .map((duplicate) => ( - - )) - .reverse()} + {data?.contactDuplicates.nodes.map((duplicate) => ( + + ))} ) : ( diff --git a/src/components/Tool/MergePeople/MergePeople.tsx b/src/components/Tool/MergePeople/MergePeople.tsx index 4eab3baed..22bff7e83 100644 --- a/src/components/Tool/MergePeople/MergePeople.tsx +++ b/src/components/Tool/MergePeople/MergePeople.tsx @@ -10,6 +10,7 @@ import { useSnackbar } from 'notistack'; import { Trans, useTranslation } from 'react-i18next'; import { makeStyles } from 'tss-react/mui'; import { SetContactFocus } from 'pages/accountLists/[accountListId]/tools/useToolsHelper'; +import { TypeEnum } from 'src/graphql/types.generated'; import useGetAppSettings from 'src/hooks/useGetAppSettings'; import theme from '../../../theme'; import ContactPair from '../MergeContacts/ContactPair'; @@ -18,6 +19,7 @@ import NoData from '../NoData'; import { useGetPersonDuplicatesQuery, useMergePeopleBulkMutation, + useUpdateDuplicateMutation, } from './GetPersonDuplicates.generated'; const useStyles = makeStyles()(() => ({ @@ -68,6 +70,7 @@ const MergePeople: React.FC = ({ }); const { appName } = useGetAppSettings(); const [peopleMerge, { loading: updating }] = useMergePeopleBulkMutation(); + const [updateDuplicates] = useUpdateDuplicateMutation(); const disabled = useMemo( () => updating || !Object.entries(actions).length, [actions, updating], @@ -75,13 +78,19 @@ const MergePeople: React.FC = ({ const totalCount = data?.personDuplicates.totalCount || 0; const duplicatesDisplayedCount = data?.personDuplicates.nodes.length || 0; - const updateActions = (id1: string, id2: string, action: string): void => { + const updateActions = ( + id1: string, + id2: string, + duplicateId: string, + action: string, + ): void => { if (!updating) { - if (action === 'cancel') { + if (action === 'ignore') { setActions((prevState) => ({ ...prevState, [id1]: { action: '' }, [id2]: { action: '' }, + [duplicateId]: { action: 'ignore' }, })); } else { setActions((prevState) => ({ @@ -93,40 +102,112 @@ const MergePeople: React.FC = ({ } }; - const mergePeople = async () => { - const mergeActions = Object.entries(actions).filter( - (action) => action[1].action === 'merge', - ); - if (mergeActions.length) { - const winnersAndLosers: { winnerId: string; loserId: string }[] = - mergeActions.map((action) => { - return { winnerId: action[0], loserId: action[1].mergeId || '' }; - }); - await peopleMerge({ - variables: { - input: { - winnersAndLosers, - }, - }, - update: (cache) => { - // Delete the loser people and remove dangling references to them - winnersAndLosers.forEach((person) => { - cache.evict({ id: `Person:${person.loserId}` }); + const handleBulkUpdateDuplicates = async () => { + try { + const callsByDuplicate: (() => Promise<{ success: boolean }>)[] = []; + + const mergeActions = Object.entries(actions).filter( + (action) => action[1].action === 'merge', + ); + + if (mergeActions.length) { + const winnersAndLosers: { winnerId: string; loserId: string }[] = + mergeActions.map((action) => { + return { winnerId: action[0], loserId: action[1].mergeId || '' }; }); - cache.gc(); - }, - onCompleted: () => { - enqueueSnackbar(t('Success!'), { + + const callMergeDuplicatesMutation = () => + mergeDuplicates(winnersAndLosers); + callsByDuplicate.push(callMergeDuplicatesMutation); + } + + const duplicatesToIgnore = Object.entries(actions) + .filter((action) => action[1].action === 'ignore') + .map((action) => action[0]); + + if (duplicatesToIgnore.length) { + duplicatesToIgnore.forEach((duplicateId) => { + const callIgnoreDuplicateMutation = () => + ignoreDuplicates(duplicateId); + callsByDuplicate.push(callIgnoreDuplicateMutation); + }); + } + + if (callsByDuplicate.length) { + const results = await Promise.all( + callsByDuplicate.map((call) => call()), + ); + + const failedUpdates = results.filter( + (result) => !result.success, + ).length; + const successfulUpdates = results.length - failedUpdates; + + if (successfulUpdates) { + enqueueSnackbar(t(`Updated ${successfulUpdates} duplicate(s)`), { variant: 'success', }); + } + if (failedUpdates) { + enqueueSnackbar( + t(`Error when updating ${failedUpdates} duplicate(s)`), + { + variant: 'error', + }, + ); + } + } else { + enqueueSnackbar(t(`No duplicates were updated`), { + variant: 'warning', + }); + } + } catch (error) { + enqueueSnackbar(t(`Error updating duplicates`), { variant: 'error' }); + } + }; + + const ignoreDuplicates = async (duplicateId: string) => { + await updateDuplicates({ + variables: { + input: { + attributes: { + ignore: true, + }, + type: TypeEnum.Person, + id: duplicateId, }, - onError: (err) => { - enqueueSnackbar(t('A server error occurred. {{err}}', { err }), { - variant: 'error', - }); + }, + update: (cache) => { + // Delete the duplicate + cache.evict({ id: `PersonDuplicate:${duplicateId}` }); + cache.gc(); + }, + onError: () => { + return { success: false }; + }, + }); + return { success: true }; + }; + + const mergeDuplicates = async (winnersAndLosers) => { + await peopleMerge({ + variables: { + input: { + winnersAndLosers, }, - }); - } + }, + update: (cache) => { + // Delete the people and remove dangling references to them + winnersAndLosers.forEach((person) => { + cache.evict({ id: `Person:${person.loserId}` }); + }); + cache.gc(); + }, + onError: () => { + return { success: false }; + }, + }); + return { success: true }; }; return ( @@ -172,22 +253,21 @@ const MergePeople: React.FC = ({ duplicatesDisplayedCount={duplicatesDisplayedCount} disabled={disabled} totalCount={totalCount} - confirmAction={mergePeople} + confirmAction={handleBulkUpdateDuplicates} setActions={setActions} /> - {data?.personDuplicates.nodes - .map((duplicate) => ( - - )) - .reverse()} + {data?.personDuplicates.nodes.map((duplicate) => ( + + ))} ) : ( From 54ad2d35f5d121247c49a59c9af841aa3b311d94 Mon Sep 17 00:00:00 2001 From: Caleb Alldrin Date: Thu, 27 Jun 2024 19:06:12 -0700 Subject: [PATCH 4/5] Add tests --- .../Tool/MergeContacts/MergeContacts.test.tsx | 44 ++++++++++++++++-- .../Tool/MergePeople/MergePeople.test.tsx | 46 ++++++++++++++++--- 2 files changed, 80 insertions(+), 10 deletions(-) diff --git a/src/components/Tool/MergeContacts/MergeContacts.test.tsx b/src/components/Tool/MergeContacts/MergeContacts.test.tsx index ffe524e35..438118401 100644 --- a/src/components/Tool/MergeContacts/MergeContacts.test.tsx +++ b/src/components/Tool/MergeContacts/MergeContacts.test.tsx @@ -6,6 +6,7 @@ import { SnackbarProvider } from 'notistack'; import TestRouter from '__tests__/util/TestRouter'; import { GqlMockedProvider } from '__tests__/util/graphqlMocking'; import { ContactsProvider } from 'src/components/Contacts/ContactsContext/ContactsContext'; +import { TypeEnum } from 'src/graphql/types.generated'; import theme from 'src/theme'; import { GetContactDuplicatesQuery } from './GetContactDuplicates.generated'; import MergeContacts from './MergeContacts'; @@ -97,7 +98,7 @@ describe('Tools - MergeContacts', () => { userEvent.click(getByRole('button', { name: 'Confirm and Continue' })); await waitFor(() => - expect(mockEnqueue).toHaveBeenCalledWith('Success!', { + expect(mockEnqueue).toHaveBeenCalledWith('Updated 1 duplicate(s)', { variant: 'success', }), ); @@ -121,7 +122,14 @@ describe('Tools - MergeContacts', () => { it('should ignore contacts', async () => { const mutationSpy = jest.fn(); - const { queryByText, queryAllByTestId, findByText, getByRole } = render( + const { + queryByText, + queryByTestId, + queryAllByTestId, + findByText, + getByText, + getByRole, + } = render( , @@ -136,10 +144,38 @@ describe('Tools - MergeContacts', () => { userEvent.click(queryAllByTestId('rightButton')[0]); expect(await findByText('Use this one')).toBeInTheDocument(); userEvent.click(queryAllByTestId('ignoreButton')[0]); + userEvent.click(queryAllByTestId('ignoreButton')[1]); expect(queryByText('Use this one')).not.toBeInTheDocument(); + + userEvent.click(getByRole('button', { name: 'Confirm and Continue' })); + await waitFor(() => + expect(mockEnqueue).toHaveBeenCalledWith('Updated 2 duplicate(s)', { + variant: 'success', + }), + ); + + const mergeCalls = mutationSpy.mock.calls + .map(([{ operation }]) => operation) + .filter(({ operationName }) => operationName === 'MassActionsMerge'); + expect(mergeCalls).toHaveLength(0); + + const ignoreCalls = mutationSpy.mock.calls + .map(([{ operation }]) => operation) + .filter(({ operationName }) => operationName === 'UpdateDuplicate'); + expect(ignoreCalls).toHaveLength(2); + expect(ignoreCalls[0].variables).toEqual({ + input: { + attributes: { + ignore: true, + }, + type: TypeEnum.Contact, + id: '1', + }, + }); + expect(queryByTestId('ignoreButton')).not.toBeInTheDocument(); expect( - getByRole('button', { name: 'Confirm and Continue' }), - ).not.toBeDisabled(); + getByText('No duplicate contacts need attention'), + ).toBeInTheDocument(); }); describe('setContactFocus()', () => { diff --git a/src/components/Tool/MergePeople/MergePeople.test.tsx b/src/components/Tool/MergePeople/MergePeople.test.tsx index 4b73561b5..e45162479 100644 --- a/src/components/Tool/MergePeople/MergePeople.test.tsx +++ b/src/components/Tool/MergePeople/MergePeople.test.tsx @@ -6,6 +6,7 @@ import { SnackbarProvider } from 'notistack'; import TestRouter from '__tests__/util/TestRouter'; import { GqlMockedProvider } from '__tests__/util/graphqlMocking'; import { ContactsProvider } from 'src/components/Contacts/ContactsContext/ContactsContext'; +import { TypeEnum } from 'src/graphql/types.generated'; import theme from 'src/theme'; import { GetPersonDuplicatesQuery } from './GetPersonDuplicates.generated'; import MergePeople from './MergePeople'; @@ -99,7 +100,7 @@ describe('Tools - MergePeople', () => { userEvent.click(getByRole('button', { name: 'Confirm and Continue' })); await waitFor(() => - expect(mockEnqueue).toHaveBeenCalledWith('Success!', { + expect(mockEnqueue).toHaveBeenCalledWith('Updated 1 duplicate(s)', { variant: 'success', }), ); @@ -120,10 +121,17 @@ describe('Tools - MergePeople', () => { }); }); - it('should ignore contacts', async () => { + it('should ignore duplicates', async () => { const mutationSpy = jest.fn(); - const { queryByText, queryAllByTestId, findByText, getByRole } = render( + const { + queryByText, + queryAllByTestId, + findByText, + getByRole, + queryByTestId, + getByText, + } = render( , @@ -138,10 +146,36 @@ describe('Tools - MergePeople', () => { userEvent.click(queryAllByTestId('rightButton')[0]); expect(await findByText('Use this one')).toBeInTheDocument(); userEvent.click(queryAllByTestId('ignoreButton')[0]); + userEvent.click(queryAllByTestId('ignoreButton')[1]); expect(queryByText('Use this one')).not.toBeInTheDocument(); - expect( - getByRole('button', { name: 'Confirm and Continue' }), - ).not.toBeDisabled(); + + userEvent.click(getByRole('button', { name: 'Confirm and Continue' })); + await waitFor(() => + expect(mockEnqueue).toHaveBeenCalledWith('Updated 2 duplicate(s)', { + variant: 'success', + }), + ); + + const mergeCalls = mutationSpy.mock.calls + .map(([{ operation }]) => operation) + .filter(({ operationName }) => operationName === 'MergePeopleBulk'); + expect(mergeCalls).toHaveLength(0); + + const ignoreCalls = mutationSpy.mock.calls + .map(([{ operation }]) => operation) + .filter(({ operationName }) => operationName === 'UpdateDuplicate'); + expect(ignoreCalls).toHaveLength(2); + expect(ignoreCalls[0].variables).toEqual({ + input: { + attributes: { + ignore: true, + }, + type: TypeEnum.Person, + id: '1', + }, + }); + expect(queryByTestId('ignoreButton')).not.toBeInTheDocument(); + expect(getByText('No duplicate people need attention')).toBeInTheDocument(); }); describe('setContactFocus()', () => { From 25f26e1da839698d4d7df878530e253cb13dfb84 Mon Sep 17 00:00:00 2001 From: Caleb Alldrin Date: Fri, 28 Jun 2024 22:16:00 -0700 Subject: [PATCH 5/5] Test Merge Duplicates error state and move mutations into helper file --- .../Tool/MergeContacts/MergeContacts.test.tsx | 67 +++++++- .../Tool/MergeContacts/MergeContacts.tsx | 118 ++----------- .../Tool/MergeContacts/MergeContactsMock.ts | 4 +- .../MergeContacts/mergeDuplicatesHelper.ts | 157 ++++++++++++++++++ .../Tool/MergePeople/MergePeople.test.tsx | 57 ++++++- .../Tool/MergePeople/MergePeople.tsx | 118 ++----------- .../Tool/MergePeople/PersonDuplicatesMock.ts | 4 +- 7 files changed, 298 insertions(+), 227 deletions(-) create mode 100644 src/components/Tool/MergeContacts/mergeDuplicatesHelper.ts diff --git a/src/components/Tool/MergeContacts/MergeContacts.test.tsx b/src/components/Tool/MergeContacts/MergeContacts.test.tsx index 438118401..6cef1ebf7 100644 --- a/src/components/Tool/MergeContacts/MergeContacts.test.tsx +++ b/src/components/Tool/MergeContacts/MergeContacts.test.tsx @@ -2,6 +2,7 @@ import React from 'react'; import { ThemeProvider } from '@mui/material/styles'; import { render, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import { ApolloErgonoMockMap } from 'graphql-ergonomock'; import { SnackbarProvider } from 'notistack'; import TestRouter from '__tests__/util/TestRouter'; import { GqlMockedProvider } from '__tests__/util/graphqlMocking'; @@ -30,10 +31,12 @@ jest.mock('notistack', () => ({ interface MergeContactsWrapperProps { mutationSpy?: () => void; + mocks?: ApolloErgonoMockMap; } const MergeContactsWrapper: React.FC = ({ mutationSpy, + mocks = getContactDuplicatesMocks, }) => { return ( @@ -41,7 +44,7 @@ const MergeContactsWrapper: React.FC = ({ - mocks={getContactDuplicatesMocks} + mocks={mocks} onCall={mutationSpy} > { const { getByText, queryAllByTestId, findByText, getByRole } = render( - + { + return { mergeContacts: ['contact-2'] }; + }, + }} + /> , ); @@ -90,7 +102,16 @@ describe('Tools - MergeContacts', () => { const confirmButton = getByRole('button', { name: 'Confirm and Continue' }); expect(confirmButton).toBeDisabled(); - userEvent.click(getByText('123 John St Orlando, FL 32832')); + userEvent.click(queryAllByTestId('ignoreButton')[1]); + userEvent.click( + getByText('(contact-2 address) 123 John St Orlando, FL 32832'), + ); + userEvent.click( + getByText('(contact-1 address) 123 Main St Orlando, FL 32832'), + ); + userEvent.click( + getByText('(contact-2 address) 123 John St Orlando, FL 32832'), + ); expect(await findByText('Use this one')).toBeInTheDocument(); expect( getByRole('button', { name: 'Confirm and Continue' }), @@ -98,7 +119,7 @@ describe('Tools - MergeContacts', () => { userEvent.click(getByRole('button', { name: 'Confirm and Continue' })); await waitFor(() => - expect(mockEnqueue).toHaveBeenCalledWith('Updated 1 duplicate(s)', { + expect(mockEnqueue).toHaveBeenCalledWith('Updated 2 duplicate(s)', { variant: 'success', }), ); @@ -111,8 +132,8 @@ describe('Tools - MergeContacts', () => { input: { winnersAndLosers: [ { - loserId: 'contact-1', winnerId: 'contact-2', + loserId: 'contact-1', }, ], }, @@ -178,6 +199,42 @@ describe('Tools - MergeContacts', () => { ).toBeInTheDocument(); }); + it('should show error', async () => { + const mutationSpy = jest.fn(); + + const { queryAllByTestId, getByRole } = render( + + { + throw new Error('Server Error'); + }, + MassActionsMerge: () => { + throw new Error('Server Error'); + }, + }} + /> + , + ); + await waitFor(() => + expect(queryAllByTestId('MergeContactPair')).toHaveLength(2), + ); + userEvent.click(queryAllByTestId('ignoreButton')[0]); + + userEvent.click(getByRole('button', { name: 'Confirm and Continue' })); + await waitFor(() => + expect(mockEnqueue).toHaveBeenCalledWith( + 'Failed to update 1 duplicate(s)', + { + variant: 'error', + }, + ), + ); + }); + describe('setContactFocus()', () => { it('should open up contact details', async () => { const mutationSpy = jest.fn(); diff --git a/src/components/Tool/MergeContacts/MergeContacts.tsx b/src/components/Tool/MergeContacts/MergeContacts.tsx index 96af8f260..499cd29a2 100644 --- a/src/components/Tool/MergeContacts/MergeContacts.tsx +++ b/src/components/Tool/MergeContacts/MergeContacts.tsx @@ -19,6 +19,7 @@ import NoData from '../NoData'; import ContactPair from './ContactPair'; import { useGetContactDuplicatesQuery } from './GetContactDuplicates.generated'; import { StickyConfirmButtons } from './StickyConfirmButtons'; +import { bulkUpdateDuplicates } from './mergeDuplicatesHelper'; const useStyles = makeStyles()(() => ({ container: { @@ -100,112 +101,15 @@ const MergeContacts: React.FC = ({ } }; - const handleBulkUpdateDuplicates = async () => { - try { - const callsByDuplicate: (() => Promise<{ success: boolean }>)[] = []; - - const mergeActions = Object.entries(actions).filter( - (action) => action[1].action === 'merge', - ); - - if (mergeActions.length) { - const winnersAndLosers: { winnerId: string; loserId: string }[] = - mergeActions.map((action) => { - return { winnerId: action[0], loserId: action[1].mergeId || '' }; - }); - - const callMergeDuplicatesMutation = () => - mergeDuplicates(winnersAndLosers); - callsByDuplicate.push(callMergeDuplicatesMutation); - } - - const duplicatesToIgnore = Object.entries(actions) - .filter((action) => action[1].action === 'ignore') - .map((action) => action[0]); - - if (duplicatesToIgnore.length) { - duplicatesToIgnore.forEach((duplicateId) => { - const callIgnoreDuplicateMutation = () => - ignoreDuplicates(duplicateId); - callsByDuplicate.push(callIgnoreDuplicateMutation); - }); - } - - if (callsByDuplicate.length) { - const results = await Promise.all( - callsByDuplicate.map((call) => call()), - ); - - const failedUpdates = results.filter( - (result) => !result.success, - ).length; - const successfulUpdates = results.length - failedUpdates; - - if (successfulUpdates) { - enqueueSnackbar(t(`Updated ${successfulUpdates} duplicate(s)`), { - variant: 'success', - }); - } - if (failedUpdates) { - enqueueSnackbar( - t(`Error when updating ${failedUpdates} duplicate(s)`), - { - variant: 'error', - }, - ); - } - } else { - enqueueSnackbar(t(`No duplicates were updated`), { - variant: 'warning', - }); - } - } catch (error) { - enqueueSnackbar(t(`Error updating duplicates`), { variant: 'error' }); - } - }; - - const ignoreDuplicates = async (duplicateId: string) => { - await updateDuplicates({ - variables: { - input: { - attributes: { - ignore: true, - }, - type: TypeEnum.Contact, - id: duplicateId, - }, - }, - update: (cache) => { - // Delete the duplicate - cache.evict({ id: `ContactDuplicate:${duplicateId}` }); - cache.gc(); - }, - onError: () => { - return { success: false }; - }, - }); - return { success: true }; - }; - - const mergeDuplicates = async (winnersAndLosers) => { - await contactsMerge({ - variables: { - input: { - winnersAndLosers, - }, - }, - update: (cache) => { - // Delete the contacts and remove dangling references to them - winnersAndLosers.forEach((contact) => { - cache.evict({ id: `Contact:${contact.loserId}` }); - }); - cache.gc(); - }, - onError: () => { - return { success: false }; - }, - }); - return { success: true }; + const handleSubmit = () => { + bulkUpdateDuplicates( + TypeEnum.Contact, + actions, + contactsMerge, + updateDuplicates, + enqueueSnackbar, + t, + ); }; return ( @@ -251,7 +155,7 @@ const MergeContacts: React.FC = ({ duplicatesDisplayedCount={duplicatesDisplayedCount} disabled={disabled} totalCount={totalCount} - confirmAction={handleBulkUpdateDuplicates} + confirmAction={handleSubmit} setActions={setActions} /> diff --git a/src/components/Tool/MergeContacts/MergeContactsMock.ts b/src/components/Tool/MergeContacts/MergeContactsMock.ts index 813973e6a..f3962e240 100644 --- a/src/components/Tool/MergeContacts/MergeContactsMock.ts +++ b/src/components/Tool/MergeContacts/MergeContactsMock.ts @@ -15,7 +15,7 @@ export const getContactDuplicatesMocks = { status: null, primaryAddress: { id: 'address-1', - street: '123 Main St', + street: '(contact-1 address) 123 Main St', city: 'Orlando', state: 'FL', postalCode: '32832', @@ -30,7 +30,7 @@ export const getContactDuplicatesMocks = { status: null, primaryAddress: { id: 'address-1', - street: '123 John St', + street: '(contact-2 address) 123 John St', city: 'Orlando', state: 'FL', postalCode: '32832', diff --git a/src/components/Tool/MergeContacts/mergeDuplicatesHelper.ts b/src/components/Tool/MergeContacts/mergeDuplicatesHelper.ts new file mode 100644 index 000000000..4ec008e26 --- /dev/null +++ b/src/components/Tool/MergeContacts/mergeDuplicatesHelper.ts @@ -0,0 +1,157 @@ +import { TFunction } from 'i18next'; +import { ProviderContext } from 'notistack'; +import { MassActionsMergeMutationHookResult } from 'src/components/Contacts/MassActions/Merge/MassActionsMerge.generated'; +import { TypeEnum } from 'src/graphql/types.generated'; +import { + MergePeopleBulkMutationHookResult, + UpdateDuplicateMutationHookResult, +} from '../MergePeople/GetPersonDuplicates.generated'; +import { ActionType } from './MergeContacts'; + +type MergeMutation = + | MergePeopleBulkMutationHookResult[0] + | MassActionsMergeMutationHookResult[0]; + +export const bulkUpdateDuplicates = async ( + type: TypeEnum, + actions: Record, + mergeMutation: MergeMutation, + updateDuplicate: UpdateDuplicateMutationHookResult[0], + enqueueSnackbar: ProviderContext['enqueueSnackbar'], + t: TFunction, +) => { + const ignoreDuplicate = async (duplicateId: string) => { + let status = true; + await updateDuplicate({ + variables: { + input: { + attributes: { + ignore: true, + }, + type: type, + id: duplicateId, + }, + }, + update: (cache) => { + // Delete the duplicate + type === TypeEnum.Contact + ? cache.evict({ id: `ContactDuplicate:${duplicateId}` }) + : cache.evict({ id: `PersonDuplicate:${duplicateId}` }); + cache.gc(); + }, + onCompleted: () => { + status = true; + }, + onError: () => { + status = false; + }, + }); + return { success: status }; + }; + + const mergeDuplicates = async ( + winnersAndLosers, + mergeMutation: MergeMutation, + ) => { + let status = true; + let successResponse = 0; + await mergeMutation({ + variables: { + input: { + winnersAndLosers, + }, + }, + update: (cache) => { + // Delete the contacts and remove dangling references to them + winnersAndLosers.forEach((contact) => { + type === TypeEnum.Contact + ? cache.evict({ id: `Contact:${contact.loserId}` }) + : cache.evict({ id: `Person:${contact.loserId}` }); + }); + cache.gc(); + }, + onCompleted: (response) => { + status = true; + successResponse = + type === TypeEnum.Contact + ? response.mergeContacts.length + : response.mergePeopleBulk.length; + }, + onError: () => { + status = false; + }, + }); + return { success: status, successfulMerges: successResponse }; + }; + + try { + const callsByDuplicate: (() => Promise<{ + success: boolean; + successfulMerges?: number; + }>)[] = []; + + const mergeActions = Object.entries(actions).filter( + (action) => action[1].action === 'merge', + ); + if (mergeActions.length) { + const winnersAndLosers: { winnerId: string; loserId: string }[] = + mergeActions.map((action) => { + return { winnerId: action[0], loserId: action[1].mergeId || '' }; + }); + const callMergeDuplicatesMutation = () => + mergeDuplicates(winnersAndLosers, mergeMutation); + callsByDuplicate.push(callMergeDuplicatesMutation); + } + const duplicatesToIgnore = Object.entries(actions) + .filter((action) => action[1].action === 'ignore') + .map((action) => action[0]); + + if (duplicatesToIgnore.length) { + duplicatesToIgnore.forEach((duplicateId) => { + const callIgnoreDuplicateMutation = () => ignoreDuplicate(duplicateId); + callsByDuplicate.push(callIgnoreDuplicateMutation); + }); + } + if (callsByDuplicate.length) { + const results = await Promise.all(callsByDuplicate.map((call) => call())); + let totalSuccessful = 0; + results.forEach((result) => { + //if the result has successfulMerges then add that number, otherwise just add 1 (for the individual ignoreDuplicate mutations) + totalSuccessful += + result.success && result?.successfulMerges + ? result.successfulMerges + : result.success + ? 1 + : 0; + }); + const totalDuplicatesAttempted = + mergeActions.length + duplicatesToIgnore.length; + + if (totalSuccessful) { + const message = t(`Updated ${totalSuccessful} duplicate(s)`); + enqueueSnackbar(`${message}`, { + variant: 'success', + }); + } + if (totalDuplicatesAttempted !== totalSuccessful) { + const message = t( + `Failed to update ${ + totalDuplicatesAttempted - totalSuccessful + } duplicate(s)`, + ); + enqueueSnackbar(message, { + variant: 'error', + }); + } + } else { + enqueueSnackbar(`${t('No duplicates were updated')}`, { + variant: 'warning', + }); + } + } catch (error) { + const message = t(`Error updating duplicates: ${error}`); + enqueueSnackbar(message, { + variant: 'error', + }); + } +}; diff --git a/src/components/Tool/MergePeople/MergePeople.test.tsx b/src/components/Tool/MergePeople/MergePeople.test.tsx index e45162479..8b5f6fb74 100644 --- a/src/components/Tool/MergePeople/MergePeople.test.tsx +++ b/src/components/Tool/MergePeople/MergePeople.test.tsx @@ -2,6 +2,7 @@ import React from 'react'; import { ThemeProvider } from '@mui/material/styles'; import { render, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import { ApolloErgonoMockMap } from 'graphql-ergonomock'; import { SnackbarProvider } from 'notistack'; import TestRouter from '__tests__/util/TestRouter'; import { GqlMockedProvider } from '__tests__/util/graphqlMocking'; @@ -30,10 +31,12 @@ jest.mock('notistack', () => ({ interface MergePeopleWrapperProps { mutationSpy?: () => void; + mocks?: ApolloErgonoMockMap; } const MergePeopleWrapper: React.FC = ({ mutationSpy, + mocks = getPersonDuplicatesMocks, }) => { return ( @@ -41,7 +44,7 @@ const MergePeopleWrapper: React.FC = ({ - mocks={getPersonDuplicatesMocks} + mocks={mocks} onCall={mutationSpy} > { const { getByText, queryAllByTestId, findByText, getByRole } = render( - + { + return { mergePeopleBulk: ['person-1'] }; + }, + }} + /> , ); @@ -92,7 +103,10 @@ describe('Tools - MergePeople', () => { expect( getByRole('button', { name: 'Confirm and Continue' }), ).toBeDisabled(); - userEvent.click(getByText('555-555-5555')); + userEvent.click(queryAllByTestId('ignoreButton')[1]); + userEvent.click(getByText('(person-1 phone) 555-555-5555')); + userEvent.click(getByText('(person-1.5 phone) 444-444-4444')); + userEvent.click(getByText('(person-1 phone) 555-555-5555')); expect(await findByText('Use this one')).toBeInTheDocument(); expect( getByRole('button', { name: 'Confirm and Continue' }), @@ -100,7 +114,7 @@ describe('Tools - MergePeople', () => { userEvent.click(getByRole('button', { name: 'Confirm and Continue' })); await waitFor(() => - expect(mockEnqueue).toHaveBeenCalledWith('Updated 1 duplicate(s)', { + expect(mockEnqueue).toHaveBeenCalledWith('Updated 2 duplicate(s)', { variant: 'success', }), ); @@ -178,6 +192,41 @@ describe('Tools - MergePeople', () => { expect(getByText('No duplicate people need attention')).toBeInTheDocument(); }); + it('should show error', async () => { + const mutationSpy = jest.fn(); + + const { queryAllByTestId, getByRole } = render( + + { + throw new Error('Server Error'); + }, + MergePeopleBulk: () => { + throw new Error('Server Error'); + }, + }} + /> + , + ); + await waitFor(() => + expect(queryAllByTestId('MergeContactPair')).toHaveLength(2), + ); + userEvent.click(queryAllByTestId('ignoreButton')[0]); + + userEvent.click(getByRole('button', { name: 'Confirm and Continue' })); + await waitFor(() => + expect(mockEnqueue).toHaveBeenCalledWith( + 'Failed to update 1 duplicate(s)', + { + variant: 'error', + }, + ), + ); + }); + describe('setContactFocus()', () => { it('should open up contact details', async () => { const mutationSpy = jest.fn(); diff --git a/src/components/Tool/MergePeople/MergePeople.tsx b/src/components/Tool/MergePeople/MergePeople.tsx index 22bff7e83..3031516e2 100644 --- a/src/components/Tool/MergePeople/MergePeople.tsx +++ b/src/components/Tool/MergePeople/MergePeople.tsx @@ -15,6 +15,7 @@ import useGetAppSettings from 'src/hooks/useGetAppSettings'; import theme from '../../../theme'; import ContactPair from '../MergeContacts/ContactPair'; import { StickyConfirmButtons } from '../MergeContacts/StickyConfirmButtons'; +import { bulkUpdateDuplicates } from '../MergeContacts/mergeDuplicatesHelper'; import NoData from '../NoData'; import { useGetPersonDuplicatesQuery, @@ -102,112 +103,15 @@ const MergePeople: React.FC = ({ } }; - const handleBulkUpdateDuplicates = async () => { - try { - const callsByDuplicate: (() => Promise<{ success: boolean }>)[] = []; - - const mergeActions = Object.entries(actions).filter( - (action) => action[1].action === 'merge', - ); - - if (mergeActions.length) { - const winnersAndLosers: { winnerId: string; loserId: string }[] = - mergeActions.map((action) => { - return { winnerId: action[0], loserId: action[1].mergeId || '' }; - }); - - const callMergeDuplicatesMutation = () => - mergeDuplicates(winnersAndLosers); - callsByDuplicate.push(callMergeDuplicatesMutation); - } - - const duplicatesToIgnore = Object.entries(actions) - .filter((action) => action[1].action === 'ignore') - .map((action) => action[0]); - - if (duplicatesToIgnore.length) { - duplicatesToIgnore.forEach((duplicateId) => { - const callIgnoreDuplicateMutation = () => - ignoreDuplicates(duplicateId); - callsByDuplicate.push(callIgnoreDuplicateMutation); - }); - } - - if (callsByDuplicate.length) { - const results = await Promise.all( - callsByDuplicate.map((call) => call()), - ); - - const failedUpdates = results.filter( - (result) => !result.success, - ).length; - const successfulUpdates = results.length - failedUpdates; - - if (successfulUpdates) { - enqueueSnackbar(t(`Updated ${successfulUpdates} duplicate(s)`), { - variant: 'success', - }); - } - if (failedUpdates) { - enqueueSnackbar( - t(`Error when updating ${failedUpdates} duplicate(s)`), - { - variant: 'error', - }, - ); - } - } else { - enqueueSnackbar(t(`No duplicates were updated`), { - variant: 'warning', - }); - } - } catch (error) { - enqueueSnackbar(t(`Error updating duplicates`), { variant: 'error' }); - } - }; - - const ignoreDuplicates = async (duplicateId: string) => { - await updateDuplicates({ - variables: { - input: { - attributes: { - ignore: true, - }, - type: TypeEnum.Person, - id: duplicateId, - }, - }, - update: (cache) => { - // Delete the duplicate - cache.evict({ id: `PersonDuplicate:${duplicateId}` }); - cache.gc(); - }, - onError: () => { - return { success: false }; - }, - }); - return { success: true }; - }; - - const mergeDuplicates = async (winnersAndLosers) => { - await peopleMerge({ - variables: { - input: { - winnersAndLosers, - }, - }, - update: (cache) => { - // Delete the people and remove dangling references to them - winnersAndLosers.forEach((person) => { - cache.evict({ id: `Person:${person.loserId}` }); - }); - cache.gc(); - }, - onError: () => { - return { success: false }; - }, - }); - return { success: true }; + const handleSubmit = () => { + bulkUpdateDuplicates( + TypeEnum.Person, + actions, + peopleMerge, + updateDuplicates, + enqueueSnackbar, + t, + ); }; return ( @@ -253,7 +157,7 @@ const MergePeople: React.FC = ({ duplicatesDisplayedCount={duplicatesDisplayedCount} disabled={disabled} totalCount={totalCount} - confirmAction={handleBulkUpdateDuplicates} + confirmAction={handleSubmit} setActions={setActions} /> diff --git a/src/components/Tool/MergePeople/PersonDuplicatesMock.ts b/src/components/Tool/MergePeople/PersonDuplicatesMock.ts index 21dc5c85b..95197e6a9 100644 --- a/src/components/Tool/MergePeople/PersonDuplicatesMock.ts +++ b/src/components/Tool/MergePeople/PersonDuplicatesMock.ts @@ -13,7 +13,7 @@ export const getPersonDuplicatesMocks = { lastName: 'Doe', createdAt: '2022-09-06T00:00:00-05:00', primaryPhoneNumber: { - number: '555-555-5555', + number: '(person-1 phone) 555-555-5555', source: 'MPDX', }, primaryEmailAddress: { @@ -29,7 +29,7 @@ export const getPersonDuplicatesMocks = { lastName: 'Doe', createdAt: '2021-09-06T00:00:00-05:00', primaryPhoneNumber: { - number: '444-444-4444', + number: '(person-1.5 phone) 444-444-4444', source: 'MPDX', }, primaryEmailAddress: {