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

[No-Jira] Change tools links #1003

Merged
merged 2 commits into from
Aug 22, 2024
Merged

[No-Jira] Change tools links #1003

merged 2 commits into from
Aug 22, 2024

Conversation

caleballdrin
Copy link
Contributor

Description

Change url of tools pages to the same as angular.

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

@caleballdrin caleballdrin self-assigned this Aug 21, 2024
@caleballdrin caleballdrin requested a review from dr-bizz August 21, 2024 13:27
@caleballdrin caleballdrin added the Preview Environment Add this label to create an Amplify Preview label Aug 21, 2024
Copy link
Contributor

Preview branch generated at https://tool-links.d3dytjb8adxkk5.amplifyapp.com

Copy link
Contributor

github-actions bot commented Aug 21, 2024

Bundle sizes [mpdx-react]

Compared against 9803ed9

Route Size (gzipped) Diff
/accountLists/[accountListId]/tools/fix/commitmentInfo/[[...contactId]] 110.88 KB added
/accountLists/[accountListId]/tools/fix/emailAddresses/[[...contactId]] 155.45 KB added
/accountLists/[accountListId]/tools/fix/mailingAddresses/[[...contactId]] 187.93 KB added
/accountLists/[accountListId]/tools/fix/phoneNumbers/[[...contactId]] 115.15 KB added
/accountLists/[accountListId]/tools/fix/sendNewsletter/[[...contactId]] 114.17 KB added
/accountLists/[accountListId]/tools/merge/contacts/[[...contactId]] 118.37 KB added
/accountLists/[accountListId]/tools/merge/people/[[...contactId]] 117.72 KB added
/accountLists/[accountListId]/tools/fixCommitmentInfo/[[...contactId]] no change removed
/accountLists/[accountListId]/tools/fixEmailAddresses/[[...contactId]] no change removed
/accountLists/[accountListId]/tools/fixMailingAddresses/[[...contactId]] no change removed
/accountLists/[accountListId]/tools/fixPhoneNumbers/[[...contactId]] no change removed
/accountLists/[accountListId]/tools/fixSendNewsletter/[[...contactId]] no change removed
/accountLists/[accountListId]/tools/mergeContacts/[[...contactId]] no change removed
/accountLists/[accountListId]/tools/mergePeople/[[...contactId]] no change removed

@caleballdrin
Copy link
Contributor Author

@dr-bizz I'm not sure why codecov failed. I haven't changed any logic.

@dr-bizz
Copy link
Contributor

dr-bizz commented Aug 22, 2024

It is because you've moved the pages around. You will need to create a test for /fix/commitmentInfo and /fix/phoneNumbers, which will help boost your score, but probably not by 30%.

Maybe try that and then if that doesn't boost the score, we'll go from there.

Copy link
Contributor

@dr-bizz dr-bizz left a comment

Choose a reason for hiding this comment

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

It all looks good, just need CodeCov to pass.

@caleballdrin
Copy link
Contributor Author

@dr-bizz can you approve this? There is a PR's already for Fix Commitment info that has the test but it hasn't been merged in yet. I don't think this PR should add tests for functionality that wasn't changed in this PR.

@caleballdrin caleballdrin requested a review from dr-bizz August 22, 2024 17:06
Copy link
Contributor

@dr-bizz dr-bizz left a comment

Choose a reason for hiding this comment

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

Great work on this

@caleballdrin caleballdrin merged commit 9af2c6b into main Aug 22, 2024
17 of 18 checks passed
@caleballdrin caleballdrin deleted the tool-links branch August 22, 2024 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Preview Environment Add this label to create an Amplify Preview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants