MPDX-8167 Account List redirect #1084
Conversation
Co-authored-by: Caleb Cox <canac@users.noreply.github.com>
caleballdrin
added
the
Preview Environment
caleballdrin
changed the title
|
Preview branch generated at https://MPDX-8167-accountList-redirect.d3dytjb8adxkk5.amplifyapp.com |
Bundle sizes [mpdx-react]Compared against a8a2f6e No significant changes found |
| const nonAccountListError = | ||
| error instanceof ApolloError && | ||
| error.graphQLErrors.every( | ||
| (error) => !isAccountListNotFoundError(error), |
There was a problem hiding this comment.
@canac Codecov wants me to test lines 128 and 129. Do you have any ideas on how to do that?
It also wants coverage for many lines in client.ts. I'm not sure what to do there.
| req: { url: '/accountLists/[accountListId]/contacts' }, | ||
| query: { | ||
| accountListId: 'account-list-1', | ||
| }, |
There was a problem hiding this comment.
You could make a makeContext helper function that takes a URL and returns this context object. Then you could use makeContext in each test to reduce some of the duplication.
|
|
||
| it('replaces the default accountListId in the url', () => { | ||
| expect( | ||
| replaceUrlAccountList('/accountList/invalid-account-list/', '12345'), |
There was a problem hiding this comment.
Is this supposed to be /accountLists/... (pluralized)? If it's supposed to be an invalid pathname, let's use a url like /invalid-url and move it to returns to default url if defaultAccountListId is invalid.
Either way, it's probably good to have a test for /accountLists/invalid-account-list/ too.
|
I might be biased, but I think this is pretty good 😄 @dr-bizz Caleb and I pair-programmed it, so you may want to look at this on Monday. This PR probably won't have full test-coverage because we added lines to files that are hard to test, but I think you tested the lines that are easily testable. 👍 |
Description
https://jira.cru.org/secure/RapidBoard.jspa?rapidView=3&view=detail&selectedIssue=MPDX-8167&sprint=1259#
Checklist: