-
Notifications
You must be signed in to change notification settings - Fork 1
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-8044 Fix Send Newsletter - add pagination #996
Conversation
Bundle sizes [mpdx-react]Compared against e8dc8c1
|
You can use TTM John Doe Exclusive to test this. It has many Newsletters to fix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. I requested some changes but nothing much. Great work on this.
pages/accountLists/[accountListId]/tools/fixSendNewsletter/[[...contactId]].page.test.tsx
Show resolved
Hide resolved
}, | ||
}, | ||
}, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should check that the contact gets removed from the page after a successful request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I'm using refetchQueries
it is hard to test that an item was removed. Because refetchQueries brings in the same mock data. Do you have any ideas on how to test this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the test is too hard, move onto the next item and leave a comment. We can come back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great. I'm going to approve this. It has some minor changes required, but I will not prevent you from deploying.
Description
(https://jira.cru.org/secure/RapidBoard.jspa?rapidView=3&view=detail&selectedIssue=MPDX-8044#)
Checklist: