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.

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-7935 add mailing address #955

Merged
merged 7 commits into from
Jun 17, 2024
Merged

MPDX-7935 add mailing address #955

merged 7 commits into from
Jun 17, 2024

Conversation

dr-bizz
Copy link
Contributor

@dr-bizz dr-bizz commented Jun 13, 2024

Description

In this PR, I added a model for adding a mailing address, cleaned up the add button, and made some improvements to AddAddressModal, which I then did on EditContactAddressModal. I've also added tests for editing, deleting, and adding an address.

I also cleaned up the FixMailingAddress component, switched the modal state props to separate state values, and refactored the modal open/close handling.

Checklist:

  • I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
  • I have applied the appropriate labels. (Add the label "On Staging" to get the branch automatically merged into staging.)
  • I have requested a review from another person on the project

Copy link
Contributor

github-actions bot commented Jun 13, 2024

Bundle sizes [mpdx-react]

Compared against a9d552f

Route Size (gzipped) Diff
/accountLists/[accountListId]/tools/fixMailingAddresses 167.55 KB +1008 B
Dynamic import Size (gzipped) Diff
../src/components/Contacts/ContactDetails/ContactDetailsTab/DynamicContactDetailsTab.tsx -> ./ContactDetailsTab 66.39 KB +2.43 KB
../src/components/Contacts/ContactDetails/ContactDetailsTab/Mailing/AddAddressModal/DynamicAddAddressModal.tsx -> ./AddAddressModal 3.75 KB added
../src/components/Contacts/ContactDetails/ContactDetailsTab/Mailing/EditContactAddressModal/DynamicEditContactAddressModal.tsx -> ./EditContactAddressModal no change added

@dr-bizz dr-bizz changed the base branch from main to MPDX-7930-fix-mailing-addresses-style-modals June 13, 2024 18:48
Base automatically changed from MPDX-7930-fix-mailing-addresses-style-modals to main June 13, 2024 18:50
@dr-bizz dr-bizz requested a review from caleballdrin June 13, 2024 20:55
@caleballdrin
Copy link
Contributor

@dr-bizz The code looks good! Now that you have the add address button you can remove the "MPDX" text. Could you also align the button with the address boxes?
Screenshot 2024-06-13 at 5 02 53 PM

@caleballdrin
Copy link
Contributor

I also noticed that Tnt Imported addresses have a lock next to them. They should show an edit pen icon because they are editable.

@dr-bizz
Copy link
Contributor Author

dr-bizz commented Jun 14, 2024

I also noticed that Tnt Imported addresses have a lock next to them. They should show an edit pen icon because they are editable.

I don't think they should be editable, as they weren't before on the old MPDX. I think this is due to ensuring TNT is the only place people can update their info if it's imported from TNT.
Only is the source is MPDX, can it be edited on MPDX.

@caleballdrin
Copy link
Contributor

caleballdrin commented Jun 14, 2024

I don't think they should be editable, as they weren't before on the old MPDX. I think this is due to ensuring TNT is the only place people can update their info if it's imported from TNT. Only is the source is MPDX, can it be edited on MPDX.

@dr-bizz Tnt addresses are editable in the old MPDX, it just also incorrectly shows a lock. There is no automatic update from Tnt like there is for Google and Siebel. Tnt import is more similar to a csv import where it is a one-time upload of data and then users will want to be able to edit that data in MPDX rather than going back to the csv/tnt file to re-import.

@dr-bizz
Copy link
Contributor Author

dr-bizz commented Jun 14, 2024

I don't think they should be editable, as they weren't before on the old MPDX. I think this is due to ensuring TNT is the only place people can update their info if it's imported from TNT. Only is the source is MPDX, can it be edited on MPDX.

@dr-bizz Tnt addresses are editable in the old MPDX, it just also incorrectly shows a lock. There is no automatic update from Tnt like there is for Google and Siebel. Tnt import is more similar to a csv import where it is a one-time upload of data and then users will want to be able to edit that data in MPDX rather than going back to the csv/tnt file to re-import.

@caleballdrin Fixed

@caleballdrin
Copy link
Contributor

@dr-bizz I'm pretty sure Tnt is different than DataServer. If you look at Pedros account you can see my contact was imported from Tnt.

Copy link
Contributor

@caleballdrin caleballdrin left a comment

Choose a reason for hiding this comment

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

Nice work!

@@ -233,7 +235,9 @@ const Contact: React.FC<Props> = ({
<strong>{t('Source')}: </strong>
</Typography>
</Hidden>
<Typography display="inline">MPDX</Typography>
<Typography display="inline">
Copy link
Contributor

@caleballdrin caleballdrin Jun 14, 2024

Choose a reason for hiding this comment

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

@dr-bizz Can you remove this? "MPDX" is still showing up across from the "Add Address" button. See my screenshot above.

@dr-bizz dr-bizz force-pushed the MPDX-7935-add-mailing-address branch from 6343904 to 7893bf6 Compare June 17, 2024 14:48
@caleballdrin caleballdrin self-requested a review June 17, 2024 14:49
@dr-bizz dr-bizz enabled auto-merge June 17, 2024 14:49
@dr-bizz dr-bizz merged commit 96f17f7 into main Jun 17, 2024
18 checks passed
@dr-bizz dr-bizz deleted the MPDX-7935-add-mailing-address branch June 17, 2024 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants