Conversation
|
Looks like it's not going to be possible to detect 404 on Task Search right now. @tpetr @kwm4385 @wolfd This PR is good to be reviewed/merged from my perspective now. |
| url: `/history/task/${taskId}` | ||
| (taskId, isMainApiCall) => ({ | ||
| url: `/history/task/${taskId}`, | ||
| isMainApiCall |
There was a problem hiding this comment.
Unfortunately the meaning snuck into another PR - that's on me, I implemented it with the Deploy Detail page fixes.
It's basically an idiomatic way of saying it ignores 404s. It signifies that if the API call fails with a 404 then the page should 404 (but it can also be extended to mean other things later).
There was a problem hiding this comment.
isMainApiCall doesn't convey that information at all -- if that's all that this variable controls, can we think of a more descriptive name for it?
There was a problem hiding this comment.
Naming things isn't my strong suit. Maybe pageCantRenderIfThisFails? I don't really like that name either though.
There was a problem hiding this comment.
Or page404sIfThis404s.
There was a problem hiding this comment.
It's cool, naming is hard 😄 -- I'd suggest something like renderNotFoundIf404
|
LGTM |
1 similar comment
|
LGTM |
Handles 404s on pages for specific requests, deploys, and tasks by showing the 404 page rather than rendering something meaningless and showing an error.
The following pages are affected:
Please let me know if I've missed any @tpetr @kwm4385 @wolfd