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

ui: add option to filter for jobs that are packs #17226

Merged
merged 3 commits into from
May 18, 2023
Merged

Conversation

ChaiWithJai
Copy link
Contributor

Resolves #15736

This PR adds a UI filter to the jobs.index view to enable filtering for jobs that are packs.

@github-actions
Copy link

Ember Asset Size action

As of e9dbda1

Files that got Bigger 🚨:

File raw gzip
nomad-ui.js +312 B +113 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

@github-actions
Copy link

Ember Test Audit comparison

main e9dbda1 change
passes 1492 1492 0
failures 4 4 0
flaky 0 0 0
duration 000ms 000ms -000ms

Copy link
Contributor

@philrenaud philrenaud left a comment

Choose a reason for hiding this comment

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

Pack doesn't seem to have the same exclusivity rules in filtering as to the job types

Comment on lines +228 to +234
const shouldShowPack = types.includes('pack') && job.displayType.isPack;

if (types.length && shouldShowPack) {
return true;
}

if (types.length && !types.includes(job.get('displayType.type'))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Something seems different about pack here; shown is a non-pack job where I have service + pack selected. Shouldn't this be hidden?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The filter should work as an or when using multi-select. A job cannot simultaneously be system and service and both would show in the list.

Conversely, pack is not a job type. A job could be pack and be service at the same time.

However, this is a multi-select filter so options are applied as or here. And if we applied another separate filter like datacenter, then that would be applied as an and operator.

Could you describe in pseudocode what you're expecting? Because I think the or and and mental model applies here and that this is working as expected.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess my confusion is from Pack not really being a job type as much as an attribute of the job / making it the only non-mutually-exclusive item here. I don't think this will lead to much user error, so I think leave it in, but another way we might have tackled this is under a separate dropdown for job attributes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it really does deserve its own filter. But then we just clutter the page. Our filtering is pretty far off from the rest of the other UIs. So it might be worth addressing later on down the line.

Comment on lines +228 to +234
const shouldShowPack = types.includes('pack') && job.displayType.isPack;

if (types.length && shouldShowPack) {
return true;
}

if (types.length && !types.includes(job.get('displayType.type'))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess my confusion is from Pack not really being a job type as much as an attribute of the job / making it the only non-mutually-exclusive item here. I don't think this will lead to much user error, so I think leave it in, but another way we might have tackled this is under a separate dropdown for job attributes.

@ChaiWithJai ChaiWithJai merged commit 2917231 into main May 18, 2023
13 checks passed
@ChaiWithJai ChaiWithJai deleted the 15736/pack-filter branch May 18, 2023 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add type filter option for packs
2 participants