GCW-3432 Authentication routing #6
Conversation
Essential for testing routing by allowing us to mock router history
Ionic uses this to handle animations related to navigation/routing
Component required by Ionic so rather not have to mock this out in tests
Components do not show up as expected on JSDOM with IonReactRouter
Test belongs to MainRouter
Latest version has compatibility issues esp. redirects
Reverting history ver. to 4 fixes the issue with redirects
Routing should be based on user auth status
Rely on actual DOM elements for testing rather than data-testid
index.ts convention makes it harder to distinguish tabs in editors
Don't rely on data-testid
Change function signature of render function to allow for currying
BboyStatix
requested review from
patrixr,
swatijadhav,
abulsayyad123,
bharat619 and
shreyas098
| const renderComponent = (initialAuthState: boolean) => ( | ||
| initialPath: string | ||
| ) => { | ||
| const history = createMemoryHistory({ initialEntries: [initialPath] }); | ||
| return { | ||
| history, | ||
| ...render( | ||
| <AuthProvider initialAuthState={initialAuthState}> | ||
| <Router history={history}> | ||
| <MainRouter /> | ||
| </Router> | ||
| </AuthProvider> | ||
| ), | ||
| }; | ||
| }; | ||
|
|
||
| // TODO extend jest matchers instead | ||
| const expectToBeOnPage = ( | ||
| container: HTMLElement, | ||
| myPath: string, | ||
| expectedPage: string | ||
| ) => { | ||
| const expectedPath = `/${expectedPage}`; | ||
| expect(myPath).toEqual(expectedPath); | ||
| expect(container.querySelector(".ion-page")).toHaveAttribute( | ||
| "title", | ||
| expectedPage | ||
| ); | ||
| }; |
There was a problem hiding this comment.
If any of those helpers will be useful in the future, don't hesitate to put them in a helper file.
There was a problem hiding this comment.
Yeah good point, but I feel that at this point this function is not abstract enough to warrant its own file?
| { initialPath: "/login", expectedPage: "login" }, | ||
| { initialPath: "/login/1234", expectedPage: "login" }, | ||
| { initialPath: "/", expectedPage: "login" }, | ||
| { initialPath: "/bad-route", expectedPage: "login" }, |
There was a problem hiding this comment.
do we want a 404 page ? 🤔 (just thinking out loud)
There was a problem hiding this comment.
@patrixr @bharat619 any reason for 404 page? Current implementation makes it impossible to reach such a page. What are the possible disadvantages of this?
There was a problem hiding this comment.
I'm talking about future implementation, not current.
and I just like 404s
@bharat619 yeah good point but I saw this stack overflow answer from the react team https://stackoverflow.com/a/44872787 |
Ticket Link
https://jira.crossroads.org.hk/browse/GCW-3432
What does this PR do?