-
Notifications
You must be signed in to change notification settings - Fork 4
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
fix: builds filter #474
fix: builds filter #474
Conversation
f80afb5
to
5411648
Compare
please explain in the commit why this is fixing the problem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
working great, just some minor pointers
@WilsonNet Before the changes, the |
5411648
to
8c573a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
backend/kernelCI_app/utils.py
Outdated
grouped_filters[field] = f | ||
grouped_filters[field]['value'] = value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f already has the value
property right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, f
is something like this {'field': 'valid', 'value': 'false', 'comparison_op': 'exact'}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I got it, line 164 is unnecessary
backend/kernelCI_app/utils.py
Outdated
value = f['value'] | ||
if field not in grouped_filters: | ||
grouped_filters[field] = f | ||
grouped_filters[field]['value'] = value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for this line?
@Francisco2002 In this PR I only worked on Views builds, so the changes do not affect the data in the boots/tests tab. Furthermore, I also tested the graphs now and they have the same staging behavior. How do you get this error?
|
8c573a7
to
7165072
Compare
My bad. |
Before the changes, the treeCommitsHistory and treeDetailsView views were unable to handle more than one filter using the same field (and even if it worked it wouldn't be able to deal with where to put AND or OR). So I created get_grouped_filters to group all values in an array into the same filter object. However, I received some errors in the query when dealing with a filter that only had one value in the array. Therefore, to avoid changing every query to always check whether it has one value or more, I decided to change the code in just one place, defining a string at first and changing it to an array when adding more values Closes #438
7165072
to
cdf4333
Compare
How to test
go to http://localhost:5173/tree/6c455c44f0ac22ab9f237ee520d0dad4ad1bda1e?tableFilter=%7B%22bootsTable%22%3A%22all%22%2C%22buildsTable%22%3A%22all%22%2C%22testsTable%22%3A%22all%22%7D&origin=maestro¤tTreeDetailsTab=treeDetails.builds&diffFilter=%7B%7D&treeInfo=%7B%22gitUrl%22%3A%22https%3A%2F%2Fandroid.googlesource.com%2Fkernel%2Fcommon%22%2C%22gitBranch%22%3A%22android-mainline%22%2C%22treeName%22%3A%22android%22%2C%22commitName%22%3A%22v6.12-rc3-6464-g6c455c44f0ac2%22%2C%22headCommitHash%22%3A%226c455c44f0ac22ab9f237ee520d0dad4ad1bda1e%22%7D
apply one or more of these filters
Build Status
Hardware
Configs
Architecture
Compilers
Closes Tree Details: Bug on filter builds #438