Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

namespace refactoring #12082

Merged
merged 24 commits into from
Feb 17, 2022
Merged

namespace refactoring #12082

merged 24 commits into from
Feb 17, 2022

Conversation

ChaiWithJai
Copy link
Contributor

This PR refactors namespaces in the Nomad UI. Namespaces in the Nomad UI were designed as query parameters and they will now be serialized in the URL with job ids and csi volumes.

RFC: https://docs.google.com/document/d/1ssx1q0k1c3wYhSBS0DIY08njk3jfUdy2cODhgkC7c24/edit

ChaiWithJai and others added 24 commits February 10, 2022 09:11
We need to adapt the test due to recent namespace changes.
This will give us 'correct' URLs for free when we only pass a `job`-model
to a `LinkTo` that links to the `jobs.job.*`-routes.
The new ID handling gives us this behavior for free and we don't need
to drill the namespace down through all the route-layers anymore.
* less clever™ metaprogramming when checking for expectedURL
* clicking slices job-client-status-summary needs to change its
  behavior and not pass the namespace query-param anymore.
Default namespaced jobs don't append the `@default`-id anymore due
to recent `jobs.job#serialize` changes.
Recent changes changed the behavior of not adding the `@default`
-namespace - we need to adapt the tests accordingly
There is no need to check the namespace query-param anymore with
`urlWithNamespace` but some tests still are using this. We refactor
the tests to be less clever and check the URL in a more manual approach
by explicitly defining how the URL should look like if a job belongs
to a namespace.
We need to access job-details differently when they have a namespace
due to recent namespace changes - we need to make the tests reflect
that.
We need to change the way we access `JobDetail`-pages based on recent
namespace changes.
* change the breadcrumbs generation to use `idWithNamespace`
* adapt tests to reflect new URLs for jobs with namespaces
URLs have changed - tests need to reflect that.
@github-actions
Copy link

Ember Asset Size action

As of 02218c6

Files that got Bigger 🚨:

File raw gzip
nomad-ui.js +2.58 kB +248 B

Files that stayed the same size 🤷‍:

File raw gzip
vendor.js 0 B 0 B
nomad-ui.css 0 B 0 B
vendor.css 0 B 0 B

Copy link
Contributor

@LevelbossMike LevelbossMike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After pairing on this together this looks good to me - excited to see this go in. 👏

@github-actions
Copy link

Ember Test Audit comparison

main 02218c6 change
passes 1269 1270 +1
failures 0 0 0
flaky 1 0 -1
duration 9m 23s 593ms 10m 27s 655ms +1m 04s 062ms

@github-actions
Copy link

Ember Test Audit flaky tests

Ember Test Audit detected these flaky tests on main:

  • Acceptance | optimize search and facets: the Prefix facet has the correct options

@ChaiWithJai ChaiWithJai merged commit cc3a7bf into main Feb 17, 2022
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI: namespace not preserved across clicks
2 participants