Feature/better 404 page handling #189
Conversation
e2e/spec/404.spec.e2e.ts
Outdated
|
|
||
| //region 404 page for non-existing News resource | ||
| Scenario('Renders (SSR) global 404 page for non-existing News resource', (I) => { | ||
| I.amOnPage('/news/article/abc-news-au34'); |
There was a problem hiding this comment.
I propose:
- to move all urls to config injectors(eg e2e/spec/pages/news.ts)
- and inside those files create 2 objects: urls and selectors
I think it's more obviously and ease to read tests.
There was a problem hiding this comment.
Did some refactoring with news urls, pls take a look.
I think we need a separate PR for more refactoring around this topic.
| document.removeEventListener('click', this.#onClickLink); | ||
| } | ||
|
|
||
| #onSingleSpaRoutingEvents = () => { | ||
| this.#prevRoute = this.#currentRoute; | ||
|
|
||
| const newUrl = this.#getCurrUrl(); | ||
| if (this.#forceSpecialRoute !== null && this.#forceSpecialRoute.url === newUrl) { | ||
| this.#currentRoute = this.#router.matchSpecial(newUrl, this.#forceSpecialRoute.id); |
There was a problem hiding this comment.
Can we reset "#forceSpecialRoute" immediately after using it here?
Why we should do it in separate else if?
| }; | ||
|
|
||
| #getIlcState = (request) => { | ||
| const state = request.ilcState || {}; |
There was a problem hiding this comment.
I don't understand when we would not have ilcState
Because I see ilcState is always expected here is ilc/server/tailor/error-handler.js line 16 and it would be added to the request here is ilc/server/app.js line 30
e2e/spec/404.spec.e2e.ts
Outdated
|
|
||
| Scenario('Renders (SSR) overridden 404 page for non-existing News resource', (I, newsPage: newsPage) => { | ||
| I.amOnPage(newsPage.url.nonExistingResourceWithOverride); | ||
| I.waitForText('404 not found component', 1000, 'body > div#body'); |
There was a problem hiding this comment.
Do we exactly need to wait here for 1000 seconds?
e2e/spec/404.spec.e2e.ts
Outdated
|
|
||
| //region 404 page for non-existing News app route | ||
| Scenario('Renders (SSR) global 404 page for non-existing News app route', (I) => { | ||
| I.amOnPage('/news/nonExisting'); |
There was a problem hiding this comment.
I see you added URLs here is e2e/spec/pages/news.ts
So you can use newsPage.url.nonExistingRoute here
No description provided.