UI: Refactor page chrome

[DingoEatingFuzz]

The only user-facing impact is the namespace switcher will redirect to the jobs list page for the selected namespace even when on non-job routes (e.g., client detail).

Namespaces are an enterprise feature.

---

By rethinking the way the namespace switcher works, I was able to generalize the usage of gutter-menu. Before it needed an onNamespaceChange action provided to it for the namespace switcher to do anything beyond setting the active namespace. Now it transitions to the jobs list page with the new namespace set in the query param.

Now that all usages of gutter-menu are the same, it could be moved up the route hierarchy. It now lives in a new page-layout component that contains both the global-header and the gutter-menu. So all that page chrome that's on most pages only needs to be included on the root template of each route hierarchy.

The logical continuation of this process would be to move the call site of page-layout to application.hbs. This, however, will never happen. Currently freestyle doesn't use the page chrome, and keeping the opportunity for non-page-layout pages to exist is important.

Final note: most of the diff is indentation change.

[johncowen]

Hey!

👍 Nice addition.

I had a brief look over the code, which LGTM from a code point of view. But then I tried checking it out for a play, set the mirageWithNamespaces setting, and I get an error in console after clicking around for a bit?

Seems to be related to the refreshRoute method that's deleted here?

I've deleted the entire repo and checked it all out again, and I can't replicate it so maybe it similar to what happened a couple of weeks ago? Could the ember tmp/ folder be affecting this?

Apart from that I found a few things that are maybe not related to this by clicking around on stuff in various places, the only one I can replicate reliably is the 'Logs: getReader' error.

This one was whilst navigating around using JS, not from typing a url into the url bar or manually refreshing the page.

If these aren't related then maybe this could be merged and those ^ sorted in another PR? The actual namespace switching thing seemed to work fine.

Shout me if you want me to try to replicate more, I can try on master also if you want? Actually just having checked again now, I can get the 404 one quite a bit also, is that maybe a mirage thing? Sorry bit more info in case it means something, I just noticed that I got a bunch of those 404 errors when I was on job/job-4 but I all the errors were for loads of weirdly numbered job urls (so job/27, job/32, job/28 etc etc)

[DingoEatingFuzz]

So that first refreshRoute error looks concerning. I'll try to replicate that. Also the last error is maybe concerning. It's absolutely possible to get a 404 when requesting a job when the namespace QP isn't present when it should be. Say job foo is in namespace my-namespace, but it's requested like /v1/job/foo instead of /v1/job/foo?namespace=my-namespace.

But as you noted, the ids for those 404s are weird in that the IDs don't start with job-. So it could very well be an issue with fixtures. I'll look into it either way, because both scenarios are bad.

[DingoEatingFuzz]

I got to the bottom of the job 404 errors. It was in fact due to some faulty Mirage code. I fixed it in this pr: #4539.

[johncowen]

Cool, just approved #4539 , lemme know when you want me to have a re-look at this one.

I just had a look at the getReader one on master and it's there also, so I'd be tempted to try and get this one closed and treat the getReader one as a separate PR? What do you think?

Thanks,

[DingoEatingFuzz]

This is ready for a re-review.

I'm not sure if there is a good way around the getReader error. The issue there is the log streamer wants to use the StreamReader interface on the fetch response (because Chrome supports it), but Mirage/Pretender does not support it.

So I could also guard in there against mirage being used in the environment, but the best solution would be for these mocking tools to support the newer interfaces.

If you manage to get that error against a real cluster, please do let me know!

[github-actions]

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.