WIP Allow excluding groupless jobs through filtering #5825
Conversation
| return [0] unless my $groups = $self->groups_for_globs; | ||
| return [map { $_->id } @$groups] if @$groups; | ||
|
|
||
| my $v = $self->validation; | ||
| $v->optional('groupid')->num(0, undef); | ||
| return $v->is_valid('groupid') ? $self->every_param('groupid') : undef; | ||
| $v->optional('not_groupid')->num(0, undef); | ||
|
|
||
| my $groupids = $v->is_valid('groupid') ? $self->every_param('groupid') : []; | ||
| my $not_groupids = $v->is_valid('not_groupid') ? $self->every_param('not_groupid') : []; | ||
|
|
||
| if (@$not_groupids) { | ||
| my %not_groupid_hash = map { $_ => 1 } @$not_groupids; | ||
| $groupids = [grep { !$not_groupid_hash{$_} } @$groupids]; | ||
| } | ||
|
|
||
| return $groupids if @$groupids; | ||
| return [0] unless my $groups = $self->groups_for_globs; | ||
| return [map { $_->id } @$groups]; |
There was a problem hiding this comment.
I'm wondering what the point of this change is. It is just filtering one set of query parameters by another set of query parameters.
| @@ -480,6 +481,15 @@ sub _compose_job_overview_search_args ($c) { | |||
| # allow filtering by group ID or group name | |||
| $search_args{groupids} = [map { $_->id } @groups] if @groups; | |||
|
|
|||
| # allow excluding jobs by group ID (0 to exclude groupless jobs) | |||
| if ($v->is_valid('not_groupid')) { | |||
| my @not_group_ids = @{$v->every_param('not_groupid')}; | |||
There was a problem hiding this comment.
It makes sense to avoid a copy in cases like this:
| my @not_group_ids = @{$v->every_param('not_groupid')}; | |
| my $not_group_ids = $v->every_param('not_groupid'); |
Of course you'll need to change the following lines to use $not_group_ids instead of @not_group_ids.
| my @not_group_ids = @{$v->every_param('not_groupid')}; | ||
| $search_args{groupid} = {-not_in => \@not_group_ids}; | ||
| if (grep { $_ == 0 } @not_group_ids) { | ||
| $search_args{groupid} = {-not_in => \@not_group_ids, '!=' => undef}; |
There was a problem hiding this comment.
This is just overriding the previous assignment. I'm not sure whether this is going to work as intended but we'll see when you've added tests.
| @@ -124,6 +125,11 @@ sub list ($self) { | |||
| my $latest = $validation->param('latest'); | |||
| my $schema = $self->schema; | |||
| my $rs = $schema->resultset('Jobs')->complex_query(%args); | |||
|
|
|||
| if (defined(my $not_groupid = $self->param('not_groupid'))) { | |||
There was a problem hiding this comment.
Not sure how other parameters are handled here but this should probably use the value from the validation object:
| if (defined(my $not_groupid = $self->param('not_groupid'))) { | |
| if (defined(my $not_groupid = $validation->param('not_groupid'))) { |
|
This pull request is now in conflicts. Could you fix it? 🙏 |
|
As this is no longer in scope of 155398, i'm closing this PR for now. |
Adds the ability to filter out (exclude) groupless jobs, as well as jobs of specific groups
Related ticket
155398