-
Notifications
You must be signed in to change notification settings - Fork 26
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
Editor: allow filtering by check categories #80
Editor: allow filtering by check categories #80
Conversation
self.checks = kwargs.get("checks") | ||
self.category = kwargs.get("category") | ||
|
||
def filter_checks(self): | ||
if self.unit_filter != 'checks': |
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.
I'm thinking this whole change to keep track of unit_filter
is unnecessary, as the filter
method in BaseUnit
basically makes sure filter_<unit_filter>
will be called. Therefore in this specific line (which is the only place where self.unit_filter
is used) it's assured that unit_filter == 'checks'
and hence this branch is dead code.
9f26753
to
08a0369
Compare
@@ -42,11 +43,14 @@ def filter_checks(self): | |||
return self.qs.filter( | |||
qualitycheck__false_positive=False, | |||
qualitycheck__name__in=self.checks).distinct() | |||
elif self.category: | |||
elif self.category or self.category is False: |
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.
As I understand self.category is False
was meant to be used to use no category at all (i.e. fall back to all categories), but as it turns out a) self.category
is never assigned the False
value and b) the default return
which is below already takes care of the fallback.
if (value in checks) { | ||
groupTotal += checks[value]; | ||
} else { | ||
$opt.remove(); |
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.
I am well aware this is removing an indentation level from old code, but we also need to be aware this disallows updating the list of checks without page refreshes, as it's being removed from the DOM and that is being used as the data source.
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.
Filed #89 to track this specific issue.
this.$filterStatus.select2('val', this.filter); | ||
} | ||
if (groupTotal === 0) { | ||
$gr.hide(); |
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.
This is also a similar case as above (indentation level decreased), but we need to be aware that hide()
on optgroup
s doesn't work reliably across browsers.
08a0369
to
4a3732f
Compare
4a3732f
to
2f5d2d2
Compare
Allows selecting and filtering by an entire group of checks in the editor.