Skip to content
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

MPDX-8344 Tools Sources #1118

Merged
merged 15 commits into from
Oct 10, 2024
Merged

MPDX-8344 Tools Sources #1118

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
a078085
Return the appName when the source is MPDX
caleballdrin Oct 4, 2024
946c09e
FIx Email: Change the source value back to MPDX and use sourceToStr(…
caleballdrin Oct 4, 2024
4d9d668
Fix Phone: Change the source value back to MPDX and use sourceToStr(…
caleballdrin Oct 5, 2024
97528c0
Allow for other sources
caleballdrin Oct 5, 2024
c91cfc0
Fix Mailing: get correct Source names and use MPDX rather than appName
caleballdrin Oct 5, 2024
eb50c0c
Add comments about appName and check editableSources
caleballdrin Oct 5, 2024
5bc9aeb
Move editableSources to helper file
caleballdrin Oct 7, 2024
ed23126
Move helper functions to the same file
caleballdrin Oct 8, 2024
a575ee2
Use manualSourceValue variable and sourcesMatch function
caleballdrin Oct 8, 2024
10b3321
Update code with new functions where sources are used
caleballdrin Oct 8, 2024
4638271
Revert manualSourceValue
caleballdrin Oct 9, 2024
0f60d81
Add the source to the add new button
caleballdrin Oct 9, 2024
001f733
Remove source from new address data to avoid confusion that the UI sa…
caleballdrin Oct 9, 2024
e9339fb
Update comments
caleballdrin Oct 10, 2024
3e844d5
Merge branch 'main' into MPDX-8344-tools-sources
caleballdrin Oct 10, 2024
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
5 changes: 2 additions & 3 deletions ...tactDetails/ContactDetailsTab/Mailing/EditContactAddressModal/EditContactAddressModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { styled } from '@mui/material/styles';
import { Formik } from 'formik';
import { useSnackbar } from 'notistack';
import { useTranslation } from 'react-i18next';
import { isEditableSource } from 'src/components/Tool/helpers';
import {
CancelButton,
DeleteButton,
Expand Down Expand Up @@ -65,8 +66,6 @@ const LoadingIndicator = styled(CircularProgress)(({ theme }) => ({
margin: theme.spacing(0, 1, 0, 0),
}));

export const editableSources = ['MPDX', 'manual', 'TntImport'];

interface EditContactAddressModalProps {
accountListId: string;
address: ContactMailingFragment['addresses']['nodes'][0];
Expand Down Expand Up @@ -171,7 +170,7 @@ export const EditContactAddressModal: React.FC<
handleClose();
};

const editingDisabled = editableSources.indexOf(address.source) === -1;
const editingDisabled = !isEditableSource(address.source);
const { data: emailData } = useDonationServicesEmailQuery({
variables: {
accountListId,
Expand Down
11 changes: 8 additions & 3 deletions src/components/Tool/FixEmailAddresses/FixEmailAddressPerson/FixEmailAddressPerson.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ import { useLocale } from 'src/hooks/useLocale';
import i18n from 'src/lib/i18n';
import { dateFormatShort } from 'src/lib/intlFormat';
import theme from 'src/theme';
import { sourceToStr } from 'src/utils/sourceToStr';
import { isEditableSource } from '../../helpers';
import EmailValidationForm from '../EmailValidationForm';
import { EmailAddressData, PersonEmailAddresses } from '../FixEmailAddresses';
import { PersonInvalidEmailFragment } from '../FixEmailAddresses.generated';
Expand Down Expand Up @@ -359,7 +361,10 @@ export const FixEmailAddressPerson: React.FC<FixEmailAddressPersonProps> = ({
</Typography>
</Hidden>
<Typography display="inline" variant="body2">
{`${email.source} (${dateFormatShort(
{`${sourceToStr(
t,
email.source,
)} (${dateFormatShort(
DateTime.fromISO(email.updatedAt),
locale,
)})`}
Expand Down Expand Up @@ -453,7 +458,7 @@ export const FixEmailAddressPerson: React.FC<FixEmailAddressPersonProps> = ({
setFieldValue('newEmail', e.target.value);
handleSubmit();
}}
disabled={email.source !== appName}
disabled={!isEditableSource(email.source)}
/>
<FormHelperText
error={true}
Expand All @@ -463,7 +468,7 @@ export const FixEmailAddressPerson: React.FC<FixEmailAddressPersonProps> = ({
</FormHelperText>
</FormControl>

{email.source === appName ? (
{isEditableSource(email.source) ? (
<Box
display="flex"
alignItems="center"
Expand Down
12 changes: 2 additions & 10 deletions src/components/Tool/FixEmailAddresses/FixEmailAddresses.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,11 +596,7 @@ describe('FixEmailAddresses-Home', () => {
} as { [key: string]: PersonEmailAddresses };
const defaultSource = 'MPDX';

const dataToSend = determineBulkDataToSend(
dataState,
defaultSource,
'MPDX',
);
const dataToSend = determineBulkDataToSend(dataState, defaultSource);

const emails = dataToSend[0].emailAddresses ?? [];
expect(emails[0].primary).toEqual(true);
Expand All @@ -625,11 +621,7 @@ describe('FixEmailAddresses-Home', () => {
} as { [key: string]: PersonEmailAddresses };
const defaultSource = 'DonorHub';

const dataToSend = determineBulkDataToSend(
dataState,
defaultSource,
'MPDX',
);
const dataToSend = determineBulkDataToSend(dataState, defaultSource);
expect(dataToSend.length).toEqual(0);
});
});
Expand Down
38 changes: 21 additions & 17 deletions src/components/Tool/FixEmailAddresses/FixEmailAddresses.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
PersonEmailAddressInput,
PersonUpdateInput,
} from 'src/graphql/types.generated';
import { sourceToStr } from 'src/utils/sourceToStr';
import theme from '../../../theme';
import { ConfirmButtonIcon } from '../ConfirmButtonIcon';
import NoData from '../NoData';
Expand Down Expand Up @@ -105,15 +106,12 @@ export const determineBulkDataToSend = (
[key: string]: PersonEmailAddresses;
},
defaultSource: string,
appName: string,
): PersonUpdateInput[] => {
const dataToSend = [] as PersonUpdateInput[];

Object.entries(dataState).forEach((value) => {
const primaryEmailAddress = value[1].emailAddresses.find(
(email) =>
email.source === defaultSource ||
(defaultSource === appName && email.source === 'MPDX'),
(email) => email.source === defaultSource,
);
if (primaryEmailAddress) {
dataToSend.push({
Expand All @@ -137,8 +135,8 @@ export const FixEmailAddresses: React.FC<FixEmailAddressesProps> = ({
accountListId,
setContactFocus,
}) => {
const appName = process.env.APP_NAME ?? 'MPDX';
const [defaultSource, setDefaultSource] = useState(appName);
//Do NOT change "MPDX" to appName here. The source value needs to stay the same. The user will see their appName displayed since we use sourceToString()
const [defaultSource, setDefaultSource] = useState('MPDX');
const [showBulkConfirmModal, setShowBulkConfirmModal] = useState(false);
const { t } = useTranslation();
const { enqueueSnackbar } = useSnackbar();
Expand All @@ -152,12 +150,14 @@ export const FixEmailAddresses: React.FC<FixEmailAddressesProps> = ({
const [dataState, setDataState] = useState<{
[key: string]: PersonEmailAddresses;
}>({});
const [sourceOptions, setSourceOptions] = useState<string[]>([appName]);
//Do NOT change "MPDX" to appName here. The source value needs to stay the same. The user will see their appName displayed since we use sourceToString()
const [sourceOptions, setSourceOptions] = useState<string[]>(['MPDX']);

// Create a mutable copy of the query data and store in the state
useEffect(() => {
const existingSources = new Set<string>();
existingSources.add(appName);
//Do NOT change "MPDX" to appName here. The source value needs to stay the same. The user will see their appName displayed since we use sourceToString()
existingSources.add('MPDX');

const newDataState = data
? data.people.nodes?.reduce<{ [key: string]: PersonEmailAddresses }>(
Expand Down Expand Up @@ -255,11 +255,7 @@ export const FixEmailAddresses: React.FC<FixEmailAddressesProps> = ({
};

const handleBulkConfirm = async () => {
const dataToSend = determineBulkDataToSend(
dataState,
defaultSource ?? '',
appName,
);
const dataToSend = determineBulkDataToSend(dataState, defaultSource ?? '');

if (dataToSend.length) {
await updatePeople({
Expand Down Expand Up @@ -289,7 +285,12 @@ export const FixEmailAddresses: React.FC<FixEmailAddressesProps> = ({
});
} else {
enqueueSnackbar(
t(`No ${defaultSource} primary email address exists to update`),
t(
`No ${sourceToStr(
t,
defaultSource,
)} primary email address exists to update`,
),
caleballdrin marked this conversation as resolved.
Show resolved Hide resolved
{
variant: 'warning',
autoHideDuration: 7000,
Expand Down Expand Up @@ -328,7 +329,7 @@ export const FixEmailAddresses: React.FC<FixEmailAddressesProps> = ({
>
{sourceOptions.map((source) => (
<MenuItem key={source} value={source}>
{source}
{sourceToStr(t, source)}
</MenuItem>
))}
</SourceSelect>
Expand All @@ -339,7 +340,7 @@ export const FixEmailAddresses: React.FC<FixEmailAddressesProps> = ({
<ConfirmButtonIcon />
{t('Confirm {{amount}} as {{source}}', {
amount: data.people.nodes.length,
source: defaultSource,
source: sourceToStr(t, defaultSource),
})}
</ConfirmButton>
</DefaultSourceWrapper>
Expand Down Expand Up @@ -416,7 +417,10 @@ export const FixEmailAddresses: React.FC<FixEmailAddressesProps> = ({
handleClose={() => setShowBulkConfirmModal(false)}
mutation={handleBulkConfirm}
title={t('Confirm')}
message={t(`You are updating all contacts visible on this page, setting the first ${defaultSource} email address as the
message={t(`You are updating all contacts visible on this page, setting the first ${sourceToStr(
t,
defaultSource,
)} email address as the
primary email address. If no such email address exists the contact will not be updated.
Are you sure you want to do this?`)}
/>
Expand Down
20 changes: 9 additions & 11 deletions src/components/Tool/FixMailingAddresses/Contact.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import { useSnackbar } from 'notistack';
import { useTranslation } from 'react-i18next';
import { makeStyles } from 'tss-react/mui';
import { SetContactFocus } from 'pages/accountLists/[accountListId]/tools/useToolsHelper';
import { editableSources } from 'src/components/Contacts/ContactDetails/ContactDetailsTab/Mailing/EditContactAddressModal/EditContactAddressModal';
import { useSetContactPrimaryAddressMutation } from 'src/components/Contacts/ContactDetails/ContactDetailsTab/Mailing/SetPrimaryAddress.generated';
import {
AddButton,
Expand All @@ -37,7 +36,9 @@ import { useContactPartnershipStatuses } from 'src/hooks/useContactPartnershipSt
import { useLocale } from 'src/hooks/useLocale';
import { useUpdateCache } from 'src/hooks/useUpdateCache';
import { dateFormatShort } from 'src/lib/intlFormat';
import { sourceToStr } from 'src/utils/sourceToStr';
import theme from '../../../theme';
import { isEditableSource } from '../helpers';
import { HandleSingleConfirmProps, emptyAddress } from './FixMailingAddresses';
import { ContactAddressFragment } from './GetInvalidAddresses.generated';

Expand Down Expand Up @@ -112,7 +113,6 @@ interface Props {
name: string;
status: string;
addresses: ContactAddressFragment[];
appName: string;
openEditAddressModal: (address: ContactAddressFragment, id: string) => void;
openNewAddressModal: (address: ContactAddressFragment, id: string) => void;
setContactFocus: SetContactFocus;
Expand All @@ -128,7 +128,6 @@ const Contact: React.FC<Props> = ({
name,
status,
addresses,
appName,
openEditAddressModal,
openNewAddressModal,
setContactFocus,
Expand Down Expand Up @@ -245,7 +244,7 @@ const Contact: React.FC<Props> = ({
</Typography>
</Hidden>
<Typography display="inline">
{address.source}{' '}
{sourceToStr(t, address.source)}{' '}
</Typography>
<Typography display="inline">
{dateFormatShort(
Expand Down Expand Up @@ -296,14 +295,16 @@ const Contact: React.FC<Props> = ({
>
<Box className={classes.address}>
<Typography>
{`${address.street}, ${address.city} ${
address.state ? address.state : ''
}. ${address.postalCode}`}
{`${address.street ? address.street : ''}, ${
address.city ? address.city : ''
} ${address.state ? address.state : ''} ${
address.postalCode ? address.postalCode : ''
}`}
</Typography>
</Box>

<ContactIconContainer aria-label={t('Edit Icon')}>
{editableSources.indexOf(address.source) > -1 ? (
{isEditableSource(address.source) ? (
<EditIcon />
) : (
<LockIcon />
Expand All @@ -321,9 +322,6 @@ const Contact: React.FC<Props> = ({
<strong>{t('Source')}: </strong>
</Typography>
</Hidden>
<Typography display="inline">
{t('{{appName}}', { appName })}
</Typography>
</Box>
Comment on lines 335 to 336
Copy link
Contributor

Choose a reason for hiding this comment

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

You've removed the source?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is not needed there.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is needed. As we allow users to create new addresses, this source was letting the user know that the add address they would create would be on MPDX and not on DonorHub or a third party like that.
Screenshot 2024-10-08 at 8 23 58 AM
Screenshot 2024-10-08 at 8 26 22 AM

Copy link
Contributor Author

@caleballdrin caleballdrin Oct 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, I think it made sense to have it there with the old UI. But now that we have a button, I don't think it makes sense from a UX perspective. The button is not a part of the table from a users point of view. So the "MPDX" text over there becomes more confusing than helpful.

If we really need the user to know that they are making an address in MPDX, then we should do it a different way.

In my opinion, I think it is self explanatory that if they create the address in MPDX, then the source will be MPDX. Just like if they create a new contact or person or donation or anything.

Copy link
Contributor

Choose a reason for hiding this comment

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

The dropdown will be filled in with values that existing addresses have, and they can choose which source to create the address in. So, if they already have a DonorHub and an MPDX address, both will appear in the dropdown to choose. This impacts the logic for primary when saving as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is what Bill is referring to with the dropdown. There are 2 MPDX values. I assume this is due to one being something else like MPDX6 but then being changed to MPDX on the UI.

Screenshot 2024-10-09 at 9 24 25 AM

Copy link
Contributor

@dr-bizz dr-bizz Oct 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In my opinion, I think it is self explanatory that if they create the address in MPDX, then the source will be MPDX. Just like if they create a new contact or person or donation or anything

I think it's best if we make sure the user knows they are creating the address on MPDX. As there are other sources listed, and hopefully to avoid confusion.

I'm open to changing how we tell the user a new address will be created on MPDX.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I will add the source to the Add Address button so it will look like: Add Address (MPDX)

</Box>
</Grid>
Expand Down
17 changes: 7 additions & 10 deletions src/components/Tool/FixMailingAddresses/FixMailingAddresses.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ describe('FixMailingAddresses', () => {
expect(queryByTestId('loading')).not.toBeInTheDocument(),
);

userEvent.click(getByText('100 Lake Hart Drive, Orlando FL. 32832'));
userEvent.click(getByText('100 Lake Hart Drive, Orlando FL 32832'));

await waitFor(() => {
expect(getByText('Edit Address')).toBeInTheDocument();
Expand Down Expand Up @@ -210,7 +210,7 @@ describe('FixMailingAddresses', () => {
expect(queryByTestId('loading')).not.toBeInTheDocument(),
);

userEvent.click(getByText('100 Lake Hart Drive, Orlando FL. 32832'));
userEvent.click(getByText('100 Lake Hart Drive, Orlando FL 32832'));

await waitFor(() => {
expect(getByText('Edit Address')).toBeInTheDocument();
Expand All @@ -230,7 +230,7 @@ describe('FixMailingAddresses', () => {

await waitFor(() =>
expect(
queryByText('100 Lake Hart Drive, Orlando FL. 32832'),
queryByText('100 Lake Hart Drive, Orlando FL 32832'),
).not.toBeInTheDocument(),
);
});
Expand Down Expand Up @@ -313,7 +313,7 @@ describe('FixMailingAddresses', () => {

await waitFor(() =>
expect(
queryByText('Buckingham Palace, College Park England. SW1A 1AA'),
queryByText('Buckingham Palace, College Park England SW1A 1AA'),
).not.toBeInTheDocument(),
);

Expand Down Expand Up @@ -360,7 +360,7 @@ describe('FixMailingAddresses', () => {

await waitFor(() =>
expect(
getByText('Buckingham Palace, London . SW1A 1AA'),
getByText('Buckingham Palace, London SW1A 1AA'),
).toBeInTheDocument(),
);
}, 10000);
Expand Down Expand Up @@ -493,7 +493,6 @@ describe('FixMailingAddresses', () => {
const name2 = 'Gamgee, Samwise';

it('should handle Error', async () => {
process.env.APP_NAME = 'MPDX';
const { getByRole, getByText, queryByTestId } = render(
<Components
mocks={{
Expand Down Expand Up @@ -568,7 +567,6 @@ describe('FixMailingAddresses', () => {
});

it('should handle success and remove contacts', async () => {
process.env.APP_NAME = 'MPDX';
const { getByRole, queryByTestId, queryByText } = render(
<Components
mocks={{
Expand Down Expand Up @@ -623,7 +621,6 @@ describe('FixMailingAddresses', () => {
});

it('should not fire handleSingleConfirm', async () => {
process.env.APP_NAME = 'MPDX';
const { getByRole, queryByTestId, queryByText } = render(
<Components
mocks={{
Expand All @@ -638,9 +635,9 @@ describe('FixMailingAddresses', () => {
);
const name = 'Baggins, Frodo';
userEvent.click(getByRole('combobox'));
userEvent.click(getByRole('option', { name: 'DataServer' }));
userEvent.click(getByRole('option', { name: 'DonorHub' }));

userEvent.click(getByRole('button', { name: 'Confirm 1 as DataServer' }));
userEvent.click(getByRole('button', { name: 'Confirm 1 as DonorHub' }));

await waitFor(() =>
expect(getByRole('heading', { name: 'Confirm' })).toBeInTheDocument(),
Expand Down
Loading
Loading