diff options
| -rw-r--r-- | Bugzilla/Bug.pm | 3 | ||||
| -rw-r--r-- | Bugzilla/Group.pm | 49 | ||||
| -rw-r--r-- | Bugzilla/Search.pm | 1499 | ||||
| -rw-r--r-- | template/en/default/global/code-error.html.tmpl | 4 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 22 | ||||
| -rw-r--r-- | xt/lib/Bugzilla/Test/Search/Constants.pm | 125 |
6 files changed, 771 insertions, 931 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index e32add2e1..efee14826 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -1632,8 +1632,7 @@ sub _check_keywords { my %keywords; foreach my $keyword (@$keyword_array) { next unless $keyword; - my $obj = new Bugzilla::Keyword({ name => $keyword }); - ThrowUserError("unknown_keyword", { keyword => $keyword }) if !$obj; + my $obj = Bugzilla::Keyword->check($keyword); $keywords{$obj->id} = $obj; } return [values %keywords]; diff --git a/Bugzilla/Group.pm b/Bugzilla/Group.pm index f047ef365..29b2a2f1c 100644 --- a/Bugzilla/Group.pm +++ b/Bugzilla/Group.pm @@ -185,6 +185,16 @@ sub products { #### Methods #### ############################### +sub check_members_are_visible { + my $self = shift; + my $user = Bugzilla->user; + return if !Bugzilla->params->{'usevisibilitygroups'}; + my $is_visible = grep { $_->id == $_ } @{ $user->visible_groups_inherited }; + if (!$is_visible) { + ThrowUserError('group_not_visible', { group => $self }); + } +} + sub set_description { $_[0]->set('description', $_[1]); } sub set_is_active { $_[0]->set('isactive', $_[1]); } sub set_name { $_[0]->set('name', $_[1]); } @@ -407,25 +417,6 @@ sub create { return $group; } -sub ValidateGroupName { - my ($name, @users) = (@_); - my $dbh = Bugzilla->dbh; - my $query = "SELECT id FROM groups " . - "WHERE name = ?"; - if (Bugzilla->params->{'usevisibilitygroups'}) { - my @visible = (-1); - foreach my $user (@users) { - $user && push @visible, @{$user->visible_groups_direct}; - } - my $visible = join(', ', @visible); - $query .= " AND id IN($visible)"; - } - my $sth = $dbh->prepare($query); - $sth->execute($name); - my ($ret) = $sth->fetchrow_array(); - return $ret; -} - ############################### ### Validators ### ############################### @@ -486,7 +477,6 @@ Bugzilla::Group - Bugzilla group class. my $icon_url = $group->icon_url; my $is_active_bug_group = $group->is_active_bug_group; - my $group_id = Bugzilla::Group::ValidateGroupName('admin', @users); my @groups = Bugzilla::Group->get_all; =head1 DESCRIPTION @@ -506,24 +496,15 @@ normally does, this function also makes the new group be inherited by the C<admin> group. That is, the C<admin> group will automatically be a member of this group. -=item C<ValidateGroupName($name, @users)> - - Description: ValidateGroupName checks to see if ANY of the users - in the provided list of user objects can see the - named group. - - Params: $name - String with the group name. - @users - An array with Bugzilla::User objects. - - Returns: It returns the group id if successful - and undef otherwise. - -=back - =head1 METHODS =over +=item C<check_members_are_visible> + +Throws an error if this group is not visible (according to +visibility groups) to the currently-logged-in user. + =item C<check_remove> =over diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 89e2dfa61..7128b8751 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -60,20 +60,20 @@ use Storable qw(dclone); # If you specify a search type in the boolean charts, this describes # which operator maps to which internal function here. use constant OPERATORS => { - equals => \&_equals, - notequals => \&_notequals, + equals => \&_simple_operator, + notequals => \&_simple_operator, casesubstring => \&_casesubstring, substring => \&_substring, substr => \&_substring, notsubstring => \&_notsubstring, regexp => \&_regexp, notregexp => \&_notregexp, - lessthan => \&_lessthan, - lessthaneq => \&_lessthaneq, + lessthan => \&_simple_operator, + lessthaneq => \&_simple_operator, matches => sub { ThrowUserError("search_content_without_matches"); }, notmatches => sub { ThrowUserError("search_content_without_matches"); }, - greaterthan => \&_greaterthan, - greaterthaneq => \&_greaterthaneq, + greaterthan => \&_simple_operator, + greaterthaneq => \&_simple_operator, anyexact => \&_anyexact, anywordssubstr => \&_anywordsubstr, allwordssubstr => \&_allwordssubstr, @@ -85,7 +85,35 @@ use constant OPERATORS => { changedafter => \&_changedbefore_changedafter, changedfrom => \&_changedfrom_changedto, changedto => \&_changedfrom_changedto, - changedby => \&_changedby, + changedby => \&_changedby, +}; + +# Some operators are really just standard SQL operators, and are +# all implemented by the _simple_operator function, which uses this +# constant. +use constant SIMPLE_OPERATORS => { + equals => '=', + notequals => '!=', + greaterthan => '>', + greaterthaneq => '>=', + lessthan => '<', + lessthaneq => "<=", +}; + +# Most operators just reverse by removing or adding "not" from/to them. +# However, some operators reverse in a different way, so those are listed +# here. +use constant OPERATOR_REVERSE => { + nowords => 'anywords', + nowordssubstr => 'anywordssubstr', + anywords => 'nowords', + anywordssubstr => 'nowordssubstr', + lessthan => 'greaterthaneq', + lessthaneq => 'greaterthan', + greaterthan => 'lessthaneq', + greaterthaneq => 'lessthan', + # The following don't currently have reversals: + # casesubstring, anyexact, allwords, allwordssubstr }; use constant OPERATOR_FIELD_OVERRIDE => { @@ -95,7 +123,7 @@ use constant OPERATOR_FIELD_OVERRIDE => { _default => \&_attachments_submitter, }, assigned_to => { - _non_changed => \&_assigned_to_reporter_nonchanged, + _non_changed => \&_contact_nonchanged, }, cc => { _non_changed => \&_cc_nonchanged, @@ -104,7 +132,7 @@ use constant OPERATOR_FIELD_OVERRIDE => { _default => \&_commenter, }, reporter => { - _non_changed => \&_assigned_to_reporter_nonchanged, + _non_changed => \&_contact_nonchanged, }, 'requestees.login_name' => { _default => \&_requestees_login_name, @@ -184,6 +212,7 @@ use constant OPERATOR_FIELD_OVERRIDE => { greaterthaneq => \&_owner_idle_time_greater_less, lessthan => \&_owner_idle_time_greater_less, lessthaneq => \&_owner_idle_time_greater_less, + _default => \&_invalid_combination, }, product => { @@ -207,7 +236,7 @@ use constant OPERATOR_FIELD_OVERRIDE => { # Timetracking Fields percentage_complete => { - _default => \&_percentage_complete, + _non_changed => \&_percentage_complete, }, work_time => { changedby => \&_work_time_changedby, @@ -929,7 +958,6 @@ sub init { $col++) { my $field = $params->param("field$chart-$row-$col") || "noop"; - my $original_field = $field; # Saved for search_description my $operator = $params->param("type$chart-$row-$col") || "noop"; my $value = $params->param("value$chart-$row-$col"); $value = "" if !defined $value; @@ -950,39 +978,40 @@ sub init { my $quoted = $dbh->quote($value); trick_taint($quoted); - my $term; my $full_field = $field =~ /\./ ? $field : "bugs.$field"; - $self->do_search_function({ - chartid => \$chartid, - sequence => \$sequence, - f => \$field, - ff => \$full_field, - t => \$operator, - v => \$value, - q => \$quoted, - term => \$term, + my %search_args = ( + chart_id => $chartid, + sequence => $sequence, + field => $field, + full_field => $full_field, + operator => $operator, + value => $value, + quoted => $quoted, multi_fields => \@multi_select_fields, - supptables => \@supptables, - wherepart => \@wherepart, + joins => \@supptables, + where => \@wherepart, having => \@having, - groupby => \@groupby, - chartfields => \%chartfields, + group_by => \@groupby, fields => \@fields, - }); - - if ($term) { + chart_fields => \%chartfields, + ); + # This should add a "term" selement to %search_args. + $self->do_search_function(\%search_args); + + if ($search_args{term}) { + # All the things here that don't get pulled out of + # %search_args are their original values before + # do_search_function modified them. $self->search_description({ - field => $original_field, type => $operator, - value => $value, term => $term, + field => $field, type => $operator, + value => $value, term => $search_args{term}, }); - push(@orlist, $term); + push(@orlist, $search_args{term}); } else { # This field and this type don't work together. - ThrowCodeError("field_type_mismatch", - { field => $params->param("field$chart-$row-$col"), - type => $params->param("type$chart-$row-$col"), - }); + ThrowUserrror("search_field_operator_invalid", + { field => $field, operator => $operator }); } } if (@orlist) { @@ -1116,16 +1145,17 @@ sub init { # it into SQL, using the constants at the top of this file. sub do_search_function { my ($self, $args) = @_; - my ($field, $operator, $value) = @$args{qw(f t v)}; + my ($field, $operator, $value) = @$args{qw(field operator value)}; - my $actual_field = FIELD_MAP->{$$field} || $$field; + my $actual_field = FIELD_MAP->{$field} || $field; + $args->{field} = $actual_field; if (my $parse_func = SPECIAL_PARSING->{$actual_field}) { - $self->$parse_func(%$args); + $self->$parse_func($args); # Some parsing functions set $term, though most do not. # For the ones that set $term, we don't need to do any further # parsing. - return if ${ $args->{term} }; + return if $args->{term}; } my $override = OPERATOR_FIELD_OVERRIDE->{$actual_field}; @@ -1142,14 +1172,14 @@ sub do_search_function { } if ($override) { - my $search_func = $self->_pick_override_function($override, $$operator); - $self->$search_func(%$args) if $search_func; + my $search_func = $self->_pick_override_function($override, $operator); + $self->$search_func($args) if $search_func; } # Some search functions set $term, and some don't. For the ones that # don't (or for fields that don't have overrides) we now call the # direct operator function from OPERATORS. - if (!${ $args->{term} }) { + if (!defined $args->{term}) { $self->_do_operator_function($args); } } @@ -1158,9 +1188,19 @@ sub do_search_function { # functions directly. sub _do_operator_function { my ($self, $func_args) = @_; - my $operator = $func_args->{t}; - my $operator_func = OPERATORS->{$$operator}; - $self->$operator_func(%$func_args); + my $operator = $func_args->{operator}; + my $operator_func = OPERATORS->{$operator}; + $self->$operator_func($func_args); +} + +sub _reverse_operator { + my ($self, $operator) = @_; + my $reverse = OPERATOR_REVERSE->{$operator}; + return $reverse if $reverse; + if ($operator =~ s/^not//) { + return $operator; + } + return "not$operator"; } sub _pick_override_function { @@ -1415,180 +1455,185 @@ sub translate_old_column { # Search Functions ##################################################################### +sub _invalid_combination { + my ($self, $args) = @_; + my ($field, $operator) = @$args{qw(field operator)}; + ThrowUserError('search_field_operator_invalid', + { field => $field, operator => $operator }); +} + sub _contact_pronoun { - my $self = shift; - my %func_args = @_; - my ($value, $quoted) = @func_args{qw(v q)}; + my ($self, $args) = @_; + my ($value, $quoted) = @$args{qw(value quoted)}; my $user = $self->{'user'}; - if ($$value =~ /^\%group/) { - $self->_contact_exact_group(%func_args); + if ($value =~ /^\%group/) { + $self->_contact_exact_group($args); } - elsif ($$value =~ /^(%\w+%)$/) { - $$value = pronoun($1, $user); - $$quoted = $$value; + elsif ($value =~ /^(%\w+%)$/) { + $args->{value} = pronoun($1, $user); + $args->{quoted} = $args->{value}; } - } sub _contact_exact_group { - my $self = shift; - my %func_args = @_; - my ($chartid, $supptables, $f, $t, $v, $term) = - @func_args{qw(chartid supptables f t v term)}; - my $user = $self->{'user'}; + my ($self, $args) = @_; + my ($value, $operator, $field, $chart_id, $joins) = + @$args{qw(value operator field chart_id joins)}; + my $dbh = Bugzilla->dbh; - $$v =~ /\%group\.([^%]+)%/; - my $group = $1; - my $groupid = Bugzilla::Group::ValidateGroupName( $group, ($user)); - $groupid || ThrowUserError('invalid_group_name',{name => $group}); - my @childgroups = @{Bugzilla::Group->flatten_group_membership($groupid)}; - my $table = "user_group_map_$$chartid"; - push (@$supptables, "LEFT JOIN user_group_map AS $table " . - "ON $table.user_id = bugs.$$f " . - "AND $table.group_id IN(" . - join(',', @childgroups) . ") " . - "AND $table.isbless = 0 " . - "AND $table.grant_type IN(" . - GRANT_DIRECT . "," . GRANT_REGEXP . ")" - ); - if ($$t =~ /^not/) { - $$term = "$table.group_id IS NULL"; - } else { - $$term = "$table.group_id IS NOT NULL"; + $value =~ /\%group\.([^%]+)%/; + my $group = Bugzilla::Group->check($1); + $group->check_members_are_visible(); + my $group_ids = Bugzilla::Group->flatten_group_membership($group->id); + my $table = "user_group_map_$chart_id"; + my $join_sql = + "LEFT JOIN user_group_map AS $table" + . " ON $table.user_id = bugs.$field" + . " AND " . $dbh->sql_in("$table.group_id", $group_ids) + . " AND $table.isbless = 0"; + push(@$joins, $join_sql); + if ($operator =~ /^not/) { + $args->{term} = "$table.group_id IS NULL"; + } + else { + $args->{term} = "$table.group_id IS NOT NULL"; } } -sub _assigned_to_reporter_nonchanged { - my $self = shift; - my %func_args = @_; - my ($f, $ff, $t, $term) = - @func_args{qw(f ff t term)}; +sub _contact_nonchanged { + my ($self, $args) = @_; + my $field = $args->{field}; - $$ff = "profiles.login_name"; - $self->_do_operator_function(\%func_args); - $$term = "bugs.$$f IN (SELECT userid FROM profiles WHERE $$term)"; + $args->{full_field} = "profiles.login_name"; + $self->_do_operator_function($args); + my $term = $args->{term}; + $args->{term} = "bugs.$field IN (SELECT userid FROM profiles WHERE $term)"; } sub _qa_contact_nonchanged { - my $self = shift; - my %func_args = @_; - my ($supptables, $f, $ff) = - @func_args{qw(supptables f ff)}; + my ($self, $args) = @_; + my $joins = $args->{joins}; - push(@$supptables, "LEFT JOIN profiles AS map_qa_contact " . - "ON bugs.qa_contact = map_qa_contact.userid"); - $$ff = "COALESCE(map_$$f.login_name,'')"; + push(@$joins, "LEFT JOIN profiles AS map_qa_contact " . + "ON bugs.qa_contact = map_qa_contact.userid"); + $args->{full_field} = "COALESCE(map_qa_contact.login_name,'')"; } sub _cc_pronoun { - my $self = shift; - my %func_args = @_; - my ($full_field, $value, $quoted) = @func_args{qw(ff v q)}; + my ($self, $args) = @_; + my ($full_field, $value) = @$args{qw(full_field value)}; my $user = $self->{'user'}; - if ($$value =~ /\%group/) { - return $self->_cc_exact_group(%func_args); + if ($value =~ /\%group/) { + return $self->_cc_exact_group($args); + } + elsif ($value =~ /^(%\w+%)$/) { + $args->{value} = pronoun($1, $user); + $args->{quoted} = $args->{value}; + $args->{full_field} = "profiles.userid"; } - elsif ($$value =~ /^(%\w+%)$/) { - $$value = pronoun($1, $user); - $$quoted = $$value; - $$full_field = "profiles.userid"; - } } sub _cc_exact_group { - my $self = shift; - my %func_args = @_; - my ($chartid, $sequence, $supptables, $t, $v, $term) = - @func_args{qw(chartid sequence supptables t v term)}; + my ($self, $args) = @_; + my ($chart_id, $sequence, $joins, $operator, $value) = + @$args{qw(chart_id sequence joins operator value)}; my $user = $self->{'user'}; + my $dbh = Bugzilla->dbh; - $$v =~ m/%group\.([^%]+)%/; - my $group = $1; - my $groupid = Bugzilla::Group::ValidateGroupName( $group, ($user)); - $groupid || ThrowUserError('invalid_group_name',{name => $group}); - my @childgroups = @{Bugzilla::Group->flatten_group_membership($groupid)}; - my $chartseq = $$chartid; - if ($$chartid eq "") { - $chartseq = "CC$$sequence"; - $$sequence++; - } - my $table = "user_group_map_$chartseq"; - push(@$supptables, "LEFT JOIN cc AS cc_$chartseq " . - "ON bugs.bug_id = cc_$chartseq.bug_id"); - push(@$supptables, "LEFT JOIN user_group_map AS $table " . - "ON $table.user_id = cc_$chartseq.who " . - "AND $table.group_id IN(" . - join(',', @childgroups) . ") " . - "AND $table.isbless = 0 " . - "AND $table.grant_type IN(" . - GRANT_DIRECT . "," . GRANT_REGEXP . ")" - ); - if ($$t =~ /^not/) { - $$term = "$table.group_id IS NULL"; - } else { - $$term = "$table.group_id IS NOT NULL"; + $value =~ m/%group\.([^%]+)%/; + my $group = Bugzilla::Group->check($1); + $group->check_members_are_visible(); + my $all_groups = Bugzilla::Group->flatten_group_membership($group->id); + + # This is for the email1, email2, email3 fields from query.cgi. + if ($chart_id eq "") { + $chart_id = "CC$$sequence"; + $args->{sequence}++; + } + + my $group_table = "user_group_map_$chart_id"; + my $cc_table = "cc_$chart_id"; + push(@$joins, "LEFT JOIN cc AS $cc_table " . + "ON bugs.bug_id = $cc_table.bug_id"); + my $join_sql = + "LEFT JOIN user_group_map AS $group_table" + . " ON $group_table.user_id = $cc_table.who" + . " AND " . $dbh->sql_in("$group_table.group_id", $all_groups) + . " AND $group_table.isbless = 0 "; + push(@$joins, $join_sql); + if ($operator =~ /^not/) { + $args->{term} = "$group_table.group_id IS NULL"; + } + else { + $args->{term} = "$group_table.group_id IS NOT NULL"; } } sub _cc_nonchanged { - my $self = shift; - my %func_args = @_; - my ($chartid, $sequence, $f, $ff, $t, $supptables, $term, $v) = - @func_args{qw(chartid sequence f ff t supptables term v)}; + my ($self, $args) = @_; + my ($chart_id, $sequence, $field, $full_field, $operator, $joins, $value) = + @$args{qw(chart_id sequence field full_field operator joins value)}; - my $chartseq = $$chartid; - if ($$chartid eq "") { - $chartseq = "CC$$sequence"; - $$sequence++; + # This is for the email1, email2, email3 fields from query.cgi. + if ($chart_id eq "") { + $chart_id = "CC$$sequence"; + $args->{sequence}++; } - if ($$ff eq 'bugs.cc') { - $$ff = "profiles.login_name"; + + # $full_field might have been changed by one of the cc_pronoun + # functions, in which case we leave it alone. + if ($full_field eq 'bugs.cc') { + $args->{full_field} = "profiles.login_name"; } - $self->_do_operator_function(\%func_args); - push(@$supptables, "LEFT JOIN cc AS cc_$chartseq " . - "ON bugs.bug_id = cc_$chartseq.bug_id " . - "AND cc_$chartseq.who IN" . - "(SELECT userid FROM profiles WHERE $$term)" - ); - $$term = "cc_$chartseq.who IS NOT NULL"; + + $self->_do_operator_function($args); + + my $term = $args->{term}; + my $table = "cc_$chart_id"; + my $join_sql = + "LEFT JOIN cc AS $table" + . " ON bugs.bug_id = $table.bug_id" + . " AND $table.who IN (SELECT userid FROM profiles WHERE $term)"; + push(@$joins, $join_sql); + + $args->{term} = "$table.who IS NOT NULL"; } +# XXX This duplicates having Commenter as a search field. sub _long_desc_changedby { - my $self = shift; - my %func_args = @_; - my ($chartid, $supptables, $term, $v) = - @func_args{qw(chartid supptables term v)}; + my ($self, $args) = @_; + my ($chart_id, $joins, $value) = @$args{qw(chart_id joins value)}; - my $table = "longdescs_$$chartid"; - push(@$supptables, "LEFT JOIN longdescs AS $table " . - "ON $table.bug_id = bugs.bug_id"); - my $id = login_to_id($$v, THROW_ERROR); - $$term = "$table.who = $id"; + my $table = "longdescs_$chart_id"; + push(@$joins, "LEFT JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id"); + my $user_id = login_to_id($value, THROW_ERROR); + $args->{term} = "$table.who = $user_id"; } sub _long_desc_changedbefore_after { - my $self = shift; - my %func_args = @_; - my ($chartid, $t, $v, $supptables, $term) = - @func_args{qw(chartid t v supptables term)}; + my ($self, $args) = @_; + my ($chart_id, $operator, $value, $joins) = + @$args{qw(chart_id operator value joins)}; my $dbh = Bugzilla->dbh; - my $operator = ($$t =~ /before/) ? '<' : '>'; - my $table = "longdescs_$$chartid"; - push(@$supptables, "LEFT JOIN longdescs AS $table " . - "ON $table.bug_id = bugs.bug_id " . - "AND $table.bug_when $operator " . - $dbh->quote(SqlifyDate($$v)) ); - $$term = "($table.bug_when IS NOT NULL)"; + my $sql_operator = ($operator =~ /before/) ? '<' : '>'; + my $table = "longdescs_$chart_id"; + my $sql_date = $dbh->quote(SqlifyDate($value)); + my $join_sql = + "LEFT JOIN longdescs AS $table " + . " ON $table.bug_id = bugs.bug_id" + . " AND $table.bug_when $sql_operator $sql_date"; + push(@$joins, $join_sql); + $args->{term} = "$table.bug_when IS NOT NULL"; } sub _content_matches { - my $self = shift; - my %func_args = @_; - my ($chartid, $supptables, $term, $groupby, $fields, $t, $v) = - @func_args{qw(chartid supptables term groupby fields t v)}; + my ($self, $args) = @_; + my ($chart_id, $joins, $group_by, $fields, $operator, $value) = + @$args{qw(chart_id joins group_by fields operator value)}; my $dbh = Bugzilla->dbh; # "content" is an alias for columns containing text for which we @@ -1599,25 +1644,26 @@ sub _content_matches { # index searches. # Add the fulltext table to the query so we can search on it. - my $table = "bugs_fulltext_$$chartid"; + my $table = "bugs_fulltext_$chart_id"; my $comments_col = "comments"; $comments_col = "comments_noprivate" unless $self->{'user'}->is_insider; - push(@$supptables, "LEFT JOIN bugs_fulltext AS $table " . - "ON bugs.bug_id = $table.bug_id"); + push(@$joins, "LEFT JOIN bugs_fulltext AS $table " . + "ON bugs.bug_id = $table.bug_id"); # Create search terms to add to the SELECT and WHERE clauses. - my ($term1, $rterm1) = $dbh->sql_fulltext_search("$table.$comments_col", - $$v, 1); - my ($term2, $rterm2) = $dbh->sql_fulltext_search("$table.short_desc", - $$v, 2); + my ($term1, $rterm1) = + $dbh->sql_fulltext_search("$table.$comments_col", $value, 1); + my ($term2, $rterm2) = + $dbh->sql_fulltext_search("$table.short_desc", $value, 2); $rterm1 = $term1 if !$rterm1; $rterm2 = $term2 if !$rterm2; # The term to use in the WHERE clause. - $$term = "$term1 > 0 OR $term2 > 0"; - if ($$t =~ /not/i) { - $$term = "NOT($$term)"; + my $term = "$term1 > 0 OR $term2 > 0"; + if ($operator =~ /not/i) { + $term = "NOT($term)"; } + $args->{term} = $term; # In order to sort by relevance (in case the user requests it), # we SELECT the relevance value so we can add it to the ORDER BY @@ -1629,282 +1675,262 @@ sub _content_matches { my $current = COLUMNS->{'relevance'}->{name}; $current = $current ? "$current + " : ''; # For NOT searches, we just add 0 to the relevance. - my $select_term = $$t =~ /not/ ? 0 : "($current$rterm1 + $rterm2)"; + my $select_term = $operator =~ /not/ ? 0 : "($current$rterm1 + $rterm2)"; COLUMNS->{'relevance'}->{name} = $select_term; } sub _timestamp_translate { - my $self = shift; - my %func_args = @_; - my ($value, $quoted) = @func_args{qw(v q)}; + my ($self, $args) = @_; + my $value = $args->{value}; my $dbh = Bugzilla->dbh; - return if $$value !~ /^[\+\-]?\d+[hdwmy]$/i; + return if $value !~ /^[\+\-]?\d+[hdwmy]$/i; - $$value = SqlifyDate($$value); - $$quoted = $dbh->quote($$value); + $args->{value} = SqlifyDate($value); + $args->{quoted} = $dbh->quote($args->{value}); } +# XXX This should probably be merged with cc_pronoun. sub _commenter_pronoun { - my $self = shift; - my %func_args = @_; - my ($full_field, $value, $quoted) = @func_args{qw(ff v q)}; + my ($self, $args) = @_; + my $value = $args->{value}; my $user = $self->{'user'}; - if ($$value =~ /^(%\w+%)$/) { - $$value = pronoun($1, $user); - $$quoted = $$value; - $$full_field = "profiles.userid"; + if ($value =~ /^(%\w+%)$/) { + $args->{value} = pronoun($1, $user); + $args->{quoted} = $args->{value}; + $args->{full_field} = "profiles.userid"; } } sub _commenter { - my $self = shift; - my %func_args = @_; - my ($chartid, $sequence, $supptables, $f, $ff, $t, $term) = - @func_args{qw(chartid sequence supptables f ff t term)}; + my ($self, $args) = @_; + my ($chart_id, $sequence, $joins, $field, $full_field, $operator) = + @$args{qw(chart_id sequence joins field full_field operator)}; - my $chartseq = $$chartid; - if ($$chartid eq "") { - $chartseq = "LD$$sequence"; - $$sequence++; + if ($chart_id eq "") { + $chart_id = "LD$sequence"; + $args->{sequence}++; } - my $table = "longdescs_$chartseq"; - my $extra = $self->{'user'}->is_insider ? "" : "AND $table.isprivate < 1"; - if ($$ff eq 'bugs.commenter') { - $$ff = "profiles.login_name"; + my $table = "longdescs_$chart_id"; + + my $extra = $self->{'user'}->is_insider ? "" : "AND $table.isprivate = 0"; + # commenter_pronoun could have changed $full_field to something else, + # so we only set this if commenter_pronoun hasn't set it. + if ($full_field eq 'bugs.commenter') { + $args->{full_field} = "profiles.login_name"; } - $self->_do_operator_function(\%func_args); - push(@$supptables, "LEFT JOIN longdescs AS $table " . - "ON $table.bug_id = bugs.bug_id $extra " . - "AND $table.who IN" . - "(SELECT userid FROM profiles WHERE $$term)" - ); - $$term = "$table.who IS NOT NULL"; + $self->_do_operator_function($args); + my $term = $args->{term}; + my $join_sql = + "LEFT JOIN longdescs AS $table" + . " ON $table.bug_id = bugs.bug_id $extra" + . " AND $table.who IN (SELECT userid FROM profiles WHERE $term)"; + push(@$joins, $join_sql); + $args->{term} = "$table.who IS NOT NULL"; } sub _long_desc { - my $self = shift; - my %func_args = @_; - my ($chartid, $supptables, $ff) = - @func_args{qw(chartid supptables ff)}; + my ($self, $args) = @_; + my ($chart_id, $joins) = @$args{qw(chart_id joins)}; - my $table = "longdescs_$$chartid"; - my $extra = $self->{'user'}->is_insider ? "" : "AND $table.isprivate < 1"; - push(@$supptables, "LEFT JOIN longdescs AS $table " . - "ON $table.bug_id = bugs.bug_id $extra"); - $$ff = "$table.thetext"; + my $table = "longdescs_$chart_id"; + my $extra = $self->{'user'}->is_insider ? "" : "AND $table.isprivate = 0"; + push(@$joins, "LEFT JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id $extra"); + $args->{full_field} = "$table.thetext"; } sub _longdescs_isprivate { - my $self = shift; - my %func_args = @_; - my ($chartid, $supptables, $ff) = - @func_args{qw(chartid supptables ff)}; + my ($self, $args) = @_; + my ($chart_id, $joins) = @$args{qw(chart_id joins)}; - my $table = "longdescs_$$chartid"; - my $extra = $self->{'user'}->is_insider ? "" : "AND $table.isprivate < 1"; - push(@$supptables, "LEFT JOIN longdescs AS $table " . - "ON $table.bug_id = bugs.bug_id $extra"); - $$ff = "$table.isprivate"; + my $table = "longdescs_$chart_id"; + my $extra = $self->{'user'}->is_insider ? "" : "AND $table.isprivate = 0"; + push(@$joins, "LEFT JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id $extra"); + $args->{quoted} = $args->{value} ? 1 : 0; + $args->{full_field} = "$table.isprivate"; } sub _work_time_changedby { - my $self = shift; - my %func_args = @_; - my ($chartid, $supptables, $v, $term) = - @func_args{qw(chartid supptables v term)}; + my ($self, $args) = @_; + my ($chart_id, $joins, $value) = @$args{qw(chart_id joins value)}; - my $table = "longdescs_$$chartid"; - push(@$supptables, "LEFT JOIN longdescs AS $table " . - "ON $table.bug_id = bugs.bug_id"); - my $id = login_to_id($$v, THROW_ERROR); - $$term = "(($table.who = $id"; - $$term .= ") AND ($table.work_time <> 0))"; + my $table = "longdescs_$chart_id"; + push(@$joins, "LEFT JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id"); + my $user_id = login_to_id($value, THROW_ERROR); + $args->{term} = "$table.who = $user_id AND $table.work_time != 0"; } sub _work_time_changedbefore_after { - my $self = shift; - my %func_args = @_; - my ($chartid, $t, $v, $supptables, $term) = - @func_args{qw(chartid t v supptables term)}; + my ($self, $args) = @_; + my ($chart_id, $operator, $value, $joins) = + @$args{qw(chart_id operator value joins)}; my $dbh = Bugzilla->dbh; - my $operator = ($$t =~ /before/) ? '<' : '>'; - my $table = "longdescs_$$chartid"; - push(@$supptables, "LEFT JOIN longdescs AS $table " . - "ON $table.bug_id = bugs.bug_id " . - "AND $table.work_time <> 0 " . - "AND $table.bug_when $operator " . - $dbh->quote(SqlifyDate($$v)) ); - $$term = "($table.bug_when IS NOT NULL)"; + my $table = "longdescs_$chart_id"; + my $sql_operator = ($operator =~ /before/) ? '<' : '>'; + my $sql_date = $dbh->quote(SqlifyDate($value)); + my $join_sql = + "LEFT JOIN longdescs AS $table" + . " ON $table.bug_id = bugs.bug_id" + . " AND $table.work_time != 0" + . " AND $table.bug_when $sql_operator $sql_date"; + push(@$joins, $join_sql); + + $args->{term} = "$table.bug_when IS NOT NULL"; } sub _work_time { - my $self = shift; - my %func_args = @_; - my ($chartid, $supptables, $ff) = - @func_args{qw(chartid supptables ff)}; + my ($self, $args) = @_; + my ($chart_id, $joins) = @$args{qw(chart_id joins)}; - my $table = "longdescs_$$chartid"; - push(@$supptables, "LEFT JOIN longdescs AS $table " . - "ON $table.bug_id = bugs.bug_id"); - $$ff = "$table.work_time"; + my $table = "longdescs_$chart_id"; + push(@$joins, "LEFT JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id"); + $args->{full_field} = "$table.work_time"; } sub _percentage_complete { - my $self = shift; - my %func_args = @_; - my ($t, $chartid, $supptables, $fields, $q, $v, $having, $groupby, $term) = - @func_args{qw(t chartid supptables fields q v having groupby term)}; - my $dbh = Bugzilla->dbh; - - my $oper; - if ($$t eq "equals") { - $oper = "="; - } elsif ($$t eq "greaterthan") { - $oper = ">"; - } elsif ($$t eq "greaterthaneq") { - $oper = ">="; - } elsif ($$t eq "lessthan") { - $oper = "<"; - } elsif ($$t eq "lessthaneq") { - $oper = "<="; - } elsif ($$t eq "notequal") { - $oper = "<>"; - } elsif ($$t eq "regexp") { - # This is just a dummy to help catch bugs- $oper won't be used - # since "regexp" is treated as a special case below. But - # leaving $oper uninitialized seems risky... - $oper = "sql_regexp"; - } elsif ($$t eq "notregexp") { - # This is just a dummy to help catch bugs- $oper won't be used - # since "notregexp" is treated as a special case below. But - # leaving $oper uninitialized seems risky... - $oper = "sql_not_regexp"; - } else { - $oper = "noop"; - } - if ($oper ne "noop") { - my $table = "longdescs_$$chartid"; - if (!grep($_ eq 'remaining_time', @$fields)) { - push(@$fields, "remaining_time"); - } - push(@$supptables, "LEFT JOIN longdescs AS $table " . - "ON $table.bug_id = bugs.bug_id"); - my $expression = "(100 * ((SUM($table.work_time) * - COUNT(DISTINCT $table.bug_when) / - COUNT(bugs.bug_id)) / - ((SUM($table.work_time) * - COUNT(DISTINCT $table.bug_when) / - COUNT(bugs.bug_id)) + - bugs.remaining_time)))"; - $$q = $dbh->quote($$v); - trick_taint($$q); - if ($$t eq "regexp") { - push(@$having, $dbh->sql_regexp($expression, $$q)); - } elsif ($$t eq "notregexp") { - push(@$having, $dbh->sql_not_regexp($expression, $$q)); - } else { - push(@$having, "$expression $oper " . $$q); - } - push(@$groupby, "bugs.remaining_time"); + my ($self, $args) = @_; + my ($chart_id, $joins, $operator, $value, $having, $fields) = + @$args{qw(chart_id joins operator value having fields)}; + + my $table = "longdescs_$chart_id"; + + # We can't just use "percentage_complete" as the field, because + # (a) PostgreSQL doesn't accept it in the HAVING clause + # and (b) it wouldn't work in multiple chart rows, because it uses + # a fixed name for the table, "ldtime". + my $expression = COLUMNS->{percentage_complete}->{name}; + $expression =~ s/\bldtime\b/$table/g; + $args->{full_field} = "($expression)"; + push(@$joins, "LEFT JOIN longdescs AS $table " . + "ON $table.bug_id = bugs.bug_id"); + + # We need remaining_time in @fields, otherwise we can't use + # it in the expression for creating percentage_complete. + if (!grep { $_ eq 'remaining_time' } @$fields) { + push(@$fields, 'remaining_time'); } - $$term = "0=0"; + + $self->_do_operator_function($args); + push(@$having, $args->{term}); + + # We put something into $args->{term} so that do_search_function + # stops processing. + $args->{term} = "0=0"; } sub _bug_group_nonchanged { - my $self = shift; - my %func_args = @_; - my ($supptables, $chartid, $ff, $t, $term) = - @func_args{qw(supptables chartid ff t term)}; - - push(@$supptables, - "LEFT JOIN bug_group_map AS bug_group_map_$$chartid " . - "ON bugs.bug_id = bug_group_map_$$chartid.bug_id"); - $$ff = "groups_$$chartid.name"; - $self->_do_operator_function(\%func_args); - push(@$supptables, - "LEFT JOIN groups AS groups_$$chartid " . - "ON groups_$$chartid.id = bug_group_map_$$chartid.group_id " . - "AND $$term"); - $$term = "$$ff IS NOT NULL"; + my ($self, $args) = @_; + my ($chart_id, $joins, $field) = @$args{qw(chart_id joins field)}; + + my $map_table = "bug_group_map_$chart_id"; + push(@$joins, + "LEFT JOIN bug_group_map AS $map_table " . + "ON bugs.bug_id = $map_table.bug_id"); + + my $groups_table = "groups_$chart_id"; + my $full_field = "$groups_table.name"; + $args->{full_field} = $full_field; + $self->_do_operator_function($args); + my $term = $args->{term}; + push(@$joins, + "LEFT JOIN groups AS $groups_table " . + "ON $groups_table.id = $map_table.group_id AND $term"); + $args->{term} = "$full_field IS NOT NULL"; } sub _attach_data_thedata { - my $self = shift; - my %func_args = @_; - my ($chartid, $supptables, $ff) = - @func_args{qw(chartid supptables ff)}; + my ($self, $args) = @_; + my ($chart_id, $joins) = @$args{qw(chart_id joins)}; - my $atable = "attachments_$$chartid"; - my $dtable = "attachdata_$$chartid"; - my $extra = $self->{'user'}->is_insider ? "" : "AND $atable.isprivate = 0"; - push(@$supptables, "LEFT JOIN attachments AS $atable " . - "ON bugs.bug_id = $atable.bug_id $extra"); - push(@$supptables, "LEFT JOIN attach_data AS $dtable " . - "ON $dtable.id = $atable.attach_id"); - $$ff = "$dtable.thedata"; + my $attach_table = "attachments_$chart_id"; + my $data_table = "attachdata_$chart_id"; + my $extra = $self->{'user'}->is_insider + ? "" : "AND $attach_table.isprivate = 0"; + push(@$joins, "LEFT JOIN attachments AS $attach_table " . + "ON bugs.bug_id = $attach_table.bug_id $extra"); + push(@$joins, "LEFT JOIN attach_data AS $data_table " . + "ON $data_table.id = $attach_table.attach_id"); + $args->{full_field} = "$data_table.thedata"; } sub _attachments_submitter { - my $self = shift; - my %func_args = @_; - my ($chartid, $supptables, $ff) = - @func_args{qw(chartid supptables ff)}; + my ($self, $args) = @_; + my ($chart_id, $joins) = @$args{qw(chart_id joins)}; + + my $attach_table = "attachment_submitter_$chart_id"; + my $extra = $self->{'user'}->is_insider + ? "" : "AND $attach_table.isprivate = 0"; + push(@$joins, "LEFT JOIN attachments AS $attach_table " . + "ON bugs.bug_id = $attach_table.bug_id $extra"); - my $atable = "map_attachment_submitter_$$chartid"; - my $extra = $self->{'user'}->is_insider ? "" : "AND $atable.isprivate = 0"; - push(@$supptables, "LEFT JOIN attachments AS $atable " . - "ON bugs.bug_id = $atable.bug_id $extra"); - push(@$supptables, "LEFT JOIN profiles AS attachers_$$chartid " . - "ON $atable.submitter_id = attachers_$$chartid.userid"); - $$ff = "attachers_$$chartid.login_name"; + my $map_table = "map_attachment_submitter_$chart_id"; + push(@$joins, "LEFT JOIN profiles AS $map_table " . + "ON $attach_table.submitter_id = $map_table.userid"); + $args->{full_field} = "$map_table.login_name"; } sub _attachments { - my $self = shift; - my %func_args = @_; - my ($chartid, $supptables, $f, $ff, $t, $v, $q) = - @func_args{qw(chartid supptables f ff t v q)}; + my ($self, $args) = @_; + my ($chart_id, $joins, $field, $operator, $value) = + @$args{qw(chart_id joins field operator value)}; my $dbh = Bugzilla->dbh; - my $table = "attachments_$$chartid"; + my $table = "attachments_$chart_id"; my $extra = $self->{'user'}->is_insider ? "" : "AND $table.isprivate = 0"; - push(@$supptables, "LEFT JOIN attachments AS $table " . - "ON bugs.bug_id = $table.bug_id $extra"); - $$f =~ m/^attachments\.(.*)$/; - my $field = $1; - if ($$t eq "changedby") { - $$v = login_to_id($$v, THROW_ERROR); - $$q = $dbh->quote($$v); - $field = "submitter_id"; - $$t = "equals"; - } elsif ($$t eq "changedbefore") { - $$v = SqlifyDate($$v); - $$q = $dbh->quote($$v); - $field = "creation_ts"; - $$t = "lessthan"; - } elsif ($$t eq "changedafter") { - $$v = SqlifyDate($$v); - $$q = $dbh->quote($$v); - $field = "creation_ts"; - $$t = "greaterthan"; - } - if ($field eq "ispatch" && $$v ne "0" && $$v ne "1") { - ThrowUserError("illegal_attachment_is_patch"); - } - if ($field eq "isobsolete" && $$v ne "0" && $$v ne "1") { - ThrowUserError("illegal_is_obsolete"); - } - $$ff = "$table.$field"; + push(@$joins, "LEFT JOIN attachments AS $table " . + "ON bugs.bug_id = $table.bug_id $extra"); + + $field =~ /^attachments\.(.+)$/; + my $attach_field = $1; + # XXX This is not actually the correct method of searching for + # changes in attachment values--this just tells you who posted an + # attachment. + if ($operator eq "changedby") { + $args->{value} = login_to_id($value, THROW_ERROR); + $args->{quoted} = $args->{value}; + $attach_field = "submitter_id"; + $args->{operator} = "equals"; + } + elsif ($operator eq 'changedbefore' or $operator eq 'changedafter') { + $args->{value} = SqlifyDate($value); + $args->{quoted} = $dbh->quote($args->{value}); + $attach_field = "creation_ts"; + $args->{operator} = $operator eq 'changedbefore' ? "lessthan" + : "greaterthan"; + } + + $args->{full_field} = "$table.$attach_field"; +} + +sub _join_flag_tables { + my ($self, $args) = @_; + my ($joins, $chart_id) = @$args{qw(joins chart_id)}; + + my $attachments = "attachments_$chart_id"; + my $extra = $self->{'user'}->is_insider + ? "" : "AND $attachments.isprivate = 0"; + push(@$joins, "LEFT JOIN attachments AS $attachments " . + "ON bugs.bug_id = $attachments.bug_id $extra"); + my $flags = "flags_$chart_id"; + # We join both the bugs and the attachments table in separately, + # and then the join code will later combine the terms. + push(@$joins, "LEFT JOIN flags AS $flags " . + "ON bugs.bug_id = $flags.bug_id "); + push(@$joins, "LEFT JOIN flags AS $flags " . + "ON $flags.attach_id = $attachments.attach_id " . + "OR $flags.attach_id IS NULL"); } sub _flagtypes_name { - my $self = shift; - my %func_args = @_; - my ($t, $chartid, $supptables, $ff, $having, $term) = - @func_args{qw(t chartid supptables ff having term)}; + my ($self, $args) = @_; + my ($chart_id, $operator, $joins, $field, $having) = + @$args{qw(chart_id operator joins field having)}; my $dbh = Bugzilla->dbh; # Matches bugs by flag name/status. @@ -1914,31 +1940,25 @@ sub _flagtypes_name { # Don't do anything if this condition is about changes to flags, # as the generic change condition processors can handle those. - return if ($$t =~ m/^changed/); + return if $operator =~ /^changed/; # Add the flags and flagtypes tables to the query. We do # a left join here so bugs without any flags still match # negative conditions (f.e. "flag isn't review+"). - my $attachments = "attachments_$$chartid"; - my $extra = $self->{'user'}->is_insider ? "" : "AND $attachments.isprivate = 0"; - push(@$supptables, "LEFT JOIN attachments AS $attachments " . - "ON bugs.bug_id = $attachments.bug_id $extra"); - my $flags = "flags_$$chartid"; - push(@$supptables, "LEFT JOIN flags AS $flags " . - "ON bugs.bug_id = $flags.bug_id "); - my $flagtypes = "flagtypes_$$chartid"; - push(@$supptables, "LEFT JOIN flagtypes AS $flagtypes " . - "ON $flags.type_id = $flagtypes.id"); - push(@$supptables, "LEFT JOIN flags AS $flags " . - "ON $flags.attach_id = $attachments.attach_id " . - "OR $flags.attach_id IS NULL"); + $self->_join_flag_tables($args); + my $flags = "flags_$chart_id"; + my $flagtypes = "flagtypes_$chart_id"; + push(@$joins, "LEFT JOIN flagtypes AS $flagtypes " . + "ON $flags.type_id = $flagtypes.id"); # Generate the condition by running the operator-specific - # function. Afterwards the condition resides in the global $term + # function. Afterwards the condition resides in the $args->{term} # variable. - $$ff = $dbh->sql_string_concat("${flagtypes}.name", - "$flags.status"); - $self->_do_operator_function(\%func_args); + my $full_field = $dbh->sql_string_concat("$flagtypes.name", + "$flags.status"); + $args->{full_field} = $full_field; + $self->_do_operator_function($args); + my $term = $args->{term}; # If this is a negative condition (f.e. flag isn't "review+"), # we only want bugs where all flags match the condition, not @@ -1948,542 +1968,433 @@ sub _flagtypes_name { # of flags on each bug, then compare them in a HAVING clause. # If the numbers are the same, all flags match the condition, # so this bug should be included. - if ($$t =~ m/not/) { + if ($operator =~ /^not/) { push(@$having, - "SUM(CASE WHEN $$ff IS NOT NULL THEN 1 ELSE 0 END) = " . - "SUM(CASE WHEN $$term THEN 1 ELSE 0 END)"); - $$term = "0=0"; + "SUM(CASE WHEN $full_field IS NOT NULL THEN 1 ELSE 0 END) = " . + "SUM(CASE WHEN $term THEN 1 ELSE 0 END)"); + $args->{term} = "0=0"; } } +# XXX These two functions can probably be joined (requestees and setters). sub _requestees_login_name { - my $self = shift; - my %func_args = @_; - my ($ff, $chartid, $supptables) = @func_args{qw(ff chartid supptables)}; + my ($self, $args) = @_; + my ($chart_id, $joins) = @$args{qw(chart_id joins)}; - my $attachments = "attachments_$$chartid"; - my $extra = $self->{'user'}->is_insider ? "" : "AND $attachments.isprivate = 0"; - push(@$supptables, "LEFT JOIN attachments AS $attachments " . - "ON bugs.bug_id = $attachments.bug_id $extra"); - my $flags = "flags_$$chartid"; - push(@$supptables, "LEFT JOIN flags AS $flags " . - "ON bugs.bug_id = $flags.bug_id "); - push(@$supptables, "LEFT JOIN profiles AS requestees_$$chartid " . - "ON $flags.requestee_id = requestees_$$chartid.userid"); - push(@$supptables, "LEFT JOIN flags AS $flags " . - "ON $flags.attach_id = $attachments.attach_id " . - "OR $flags.attach_id IS NULL"); + $self->_join_flag_tables($args); + my $flags = "flags_$chart_id"; + my $map_table = "map_flag_requestees_$chart_id"; + push(@$joins, "LEFT JOIN profiles AS $map_table " . + "ON $flags.requestee_id = $map_table.userid"); - $$ff = "requestees_$$chartid.login_name"; + $args->{full_field} = "$map_table.login_name"; } sub _setters_login_name { - my $self = shift; - my %func_args = @_; - my ($ff, $chartid, $supptables) = @func_args{qw(ff chartid supptables)}; - - my $attachments = "attachments_$$chartid"; - my $extra = $self->{'user'}->is_insider ? "" : "AND $attachments.isprivate = 0"; - push(@$supptables, "LEFT JOIN attachments AS $attachments " . - "ON bugs.bug_id = $attachments.bug_id $extra"); - my $flags = "flags_$$chartid"; - push(@$supptables, "LEFT JOIN flags AS $flags " . - "ON bugs.bug_id = $flags.bug_id "); - push(@$supptables, "LEFT JOIN profiles AS setters_$$chartid " . - "ON $flags.setter_id = setters_$$chartid.userid"); - push(@$supptables, "LEFT JOIN flags AS $flags " . - "ON $flags.attach_id = $attachments.attach_id " . - "OR $flags.attach_id IS NULL"); + my ($self, $args) = @_; + my ($chart_id, $joins) = @$args{qw(chart_id joins)}; + + $self->_join_flag_tables($args); + my $flags = "flags_$chart_id"; + my $map_table = "map_flag_setters_$chart_id"; + push(@$joins, "LEFT JOIN profiles AS $map_table " . + "ON $flags.setter_id = $map_table.userid"); - $$ff = "setters_$$chartid.login_name"; + $args->{full_field} = "$map_table.login_name"; } sub _changedin_days_elapsed { - my $self = shift; - my %func_args = @_; - my ($ff) = @func_args{qw(ff)}; + my ($self, $args) = @_; my $dbh = Bugzilla->dbh; - $$ff = "(" . $dbh->sql_to_days('NOW()') . " - " . - $dbh->sql_to_days('bugs.delta_ts') . ")"; + $args->{full_field} = "(" . $dbh->sql_to_days('NOW()') . " - " . + $dbh->sql_to_days('bugs.delta_ts') . ")"; } sub _component_nonchanged { - my $self = shift; - my %func_args = @_; - my ($ff, $t, $term) = @func_args{qw(ff t term)}; + my ($self, $args) = @_; - $$ff = "components.name"; - $self->_do_operator_function(\%func_args); - $$term = build_subselect("bugs.component_id", - "components.id", - "components", - $$term); + $args->{full_field} = "components.name"; + $self->_do_operator_function($args); + my $term = $args->{term}; + $args->{term} = build_subselect("bugs.component_id", + "components.id", "components", $args->{term}); } + sub _product_nonchanged { - my $self = shift; - my %func_args = @_; - my ($ff, $t, $term) = @func_args{qw(ff t term)}; + my ($self, $args) = @_; # Generate the restriction condition - $$ff = "products.name"; - $self->_do_operator_function(\%func_args); - $$term = build_subselect("bugs.product_id", - "products.id", - "products", - $$term); + $args->{full_field} = "products.name"; + $self->_do_operator_function($args); + my $term = $args->{term}; + $args->{term} = build_subselect("bugs.product_id", + "products.id", "products", $term); } sub _classification_nonchanged { - my $self = shift; - my %func_args = @_; - my ($chartid, $v, $ff, $t, $supptables, $term) = - @func_args{qw(chartid v ff t supptables term)}; + my ($self, $args) = @_; + my $joins = $args->{joins}; # Generate the restriction condition - push @$supptables, "INNER JOIN products AS map_products " . - "ON bugs.product_id = map_products.id"; - $$ff = "classifications.name"; - $self->_do_operator_function(\%func_args); - $$term = build_subselect("map_products.classification_id", - "classifications.id", - "classifications", - $$term); + push(@$joins, "INNER JOIN products AS map_products " . + "ON bugs.product_id = map_products.id"); + $args->{full_field} = "classifications.name"; + $self->_do_operator_function($args); + my $term = $args->{term}; + $args->{term} = build_subselect("map_products.classification_id", + "classifications.id", "classifications", $term); } sub _keywords_exact { - my $self = shift; - my %func_args = @_; - my ($chartid, $v, $ff, $f, $t, $term, $supptables) = - @func_args{qw(chartid v ff f t term supptables)}; - - my @list; - my $table = "keywords_$$chartid"; - foreach my $value (split(/[\s,]+/, $$v)) { - if ($value eq '') { - next; - } - my $keyword = new Bugzilla::Keyword({name => $value}); - if ($keyword) { - push(@list, "$table.keywordid = " . $keyword->id); - } - else { - ThrowUserError("unknown_keyword", - { keyword => $$v }); - } - } - my $haveawordterm; - if (@list) { - $haveawordterm = "(" . join(' OR ', @list) . ")"; - if ($$t eq "anywords") { - $$term = $haveawordterm; - } elsif ($$t eq "allwords") { - $self->_allwords; - if ($$term && $haveawordterm) { - $$term = "(($$term) AND $haveawordterm)"; - } - } + my ($self, $args) = @_; + my ($chart_id, $joins, $value, $operator) = + @$args{qw(chart_id joins value operator)}; + my $dbh = Bugzilla->dbh; + + my @keyword_ids; + foreach my $value (split(/[\s,]+/, $value)) { + next if $value eq ''; + my $keyword = Bugzilla::Keyword->check($value); + push(@keyword_ids, $keyword->id); } - if ($$term) { - push(@$supptables, "LEFT JOIN keywords AS $table " . - "ON $table.bug_id = bugs.bug_id"); + + # XXX We probably should instead throw an error here if there were + # just commas in the field. + if (!@keyword_ids) { + $args->{term} = "0=0"; + return; } - else { - $self->_keywords_nonchanged(%func_args); + + # This is an optimization for anywords, since we already know + # the keyword id from having checked it above. + if ($operator eq 'anywords') { + my $table = "keywords_$chart_id"; + $args->{term} = $dbh->sql_in("$table.keywordid", \@keyword_ids); + push(@$joins, "LEFT JOIN keywords AS $table" + . " ON $table.bug_id = bugs.bug_id"); + return; } + + $self->_keywords_nonchanged($args); } sub _keywords_nonchanged { - my $self = shift; - my %func_args = @_; - my ($chartid, $v, $ff, $t, $term, $supptables) = - @func_args{qw(chartid v ff t term supptables)}; + my ($self, $args) = @_; + my ($chart_id, $joins, $value, $operator) = + @$args{qw(chart_id joins value operator)}; - my $k_table = "keywords_$$chartid"; - my $kd_table = "keyworddefs_$$chartid"; + my $k_table = "keywords_$chart_id"; + my $kd_table = "keyworddefs_$chart_id"; - push(@$supptables, "LEFT JOIN keywords AS $k_table " . - "ON $k_table.bug_id = bugs.bug_id"); - push(@$supptables, "LEFT JOIN keyworddefs AS $kd_table " . - "ON $kd_table.id = $k_table.keywordid"); + push(@$joins, "LEFT JOIN keywords AS $k_table " . + "ON $k_table.bug_id = bugs.bug_id"); + push(@$joins, "LEFT JOIN keyworddefs AS $kd_table " . + "ON $kd_table.id = $k_table.keywordid"); - $$ff = "$kd_table.name"; + $args->{full_field} = "$kd_table.name"; } +# XXX This should be combined with blocked_nonchanged. sub _dependson_nonchanged { - my $self = shift; - my %func_args = @_; - my ($chartid, $ff, $f, $t, $term, $supptables) = - @func_args{qw(chartid ff f t term supptables)}; + my ($self, $args) = @_; + my ($chart_id, $joins, $field, $operator) = + @$args{qw(chart_id joins field operator)}; - my $table = "dependson_" . $$chartid; - $$ff = "$table.$$f"; - $self->_do_operator_function(\%func_args); - push(@$supptables, "LEFT JOIN dependencies AS $table " . - "ON $table.blocked = bugs.bug_id " . - "AND ($$term)"); - $$term = "$$ff IS NOT NULL"; + my $table = "dependson_$chart_id"; + my $full_field = "$table.$field"; + $args->{full_field} = $full_field; + $self->_do_operator_function($args); + my $term = $args->{term}; + push(@$joins, "LEFT JOIN dependencies AS $table " . + "ON $table.blocked = bugs.bug_id AND ($term)"); + $args->{term} = "$full_field IS NOT NULL"; } sub _blocked_nonchanged { - my $self = shift; - my %func_args = @_; - my ($chartid, $ff, $f, $t, $term, $supptables) = - @func_args{qw(chartid ff f t term supptables)}; + my ($self, $args) = @_; + my ($chart_id, $joins, $field, $operator) = + @$args{qw(chart_id joins field operator)}; - my $table = "blocked_" . $$chartid; - $$ff = "$table.$$f"; - $self->_do_operator_function(\%func_args); - push(@$supptables, "LEFT JOIN dependencies AS $table " . - "ON $table.dependson = bugs.bug_id " . - "AND ($$term)"); - $$term = "$$ff IS NOT NULL"; + my $table = "blocked_$chart_id"; + my $full_field = "$table.$field"; + $args->{full_field} = $full_field; + $self->_do_operator_function($args); + my $term = $args->{term}; + push(@$joins, "LEFT JOIN dependencies AS $table " . + "ON $table.dependson = bugs.bug_id AND ($term)"); + $args->{term} = "$full_field IS NOT NULL"; } sub _alias_nonchanged { - my $self = shift; - my %func_args = @_; - my ($ff, $t, $term) = @func_args{qw(ff t term)}; - - $$ff = "COALESCE(bugs.alias, '')"; - - $self->_do_operator_function(\%func_args); + my ($self, $args) = @_; + $args->{full_field} = "COALESCE(bugs.alias, '')"; + $self->_do_operator_function($args); } sub _owner_idle_time_greater_less { - my $self = shift; - my %func_args = @_; - my ($chartid, $v, $supptables, $t, $wherepart, $term) = - @func_args{qw(chartid v supptables t wherepart term)}; + my ($self, $args) = @_; + my ($chart_id, $joins, $value, $operator) = + @$args{qw(chart_id joins value operator)}; my $dbh = Bugzilla->dbh; - my $table = "idle_" . $$chartid; - $$v =~ /^(\d+)\s*([hHdDwWmMyY])?$/; - my $quantity = $1 || 0; - my $unit = lc $2; - my $unitinterval = 'DAY'; - if ($unit eq 'h') { - $unitinterval = 'HOUR'; - } elsif ($unit eq 'w') { - $unitinterval = ' * 7 DAY'; - } elsif ($unit eq 'm') { - $unitinterval = 'MONTH'; - } elsif ($unit eq 'y') { - $unitinterval = 'YEAR'; - } - my $cutoff = "NOW() - " . - $dbh->sql_interval($quantity, $unitinterval); + my $table = "idle_$chart_id"; + my $quoted = $dbh->quote(SqlifyDate($value)); + + my $ld_table = "comment_$table"; + my $comments_join = + "LEFT JOIN longdescs AS $ld_table" + . " ON $ld_table.who = bugs.assigned_to" + . " AND $ld_table.bug_id = bugs.bug_id" + . " AND $ld_table.bug_when > $quoted"; + push(@$joins, $comments_join); + + my $act_table = "activity_$table"; my $assigned_fieldid = get_field_id('assigned_to'); - push(@$supptables, "LEFT JOIN longdescs AS comment_$table " . - "ON comment_$table.who = bugs.assigned_to " . - "AND comment_$table.bug_id = bugs.bug_id " . - "AND comment_$table.bug_when > $cutoff"); - push(@$supptables, "LEFT JOIN bugs_activity AS activity_$table " . - "ON (activity_$table.who = bugs.assigned_to " . - "OR activity_$table.fieldid = $assigned_fieldid) " . - "AND activity_$table.bug_id = bugs.bug_id " . - "AND activity_$table.bug_when > $cutoff"); - if ($$t =~ /greater/) { - push(@$wherepart, "(comment_$table.who IS NULL " . - "AND activity_$table.who IS NULL)"); + + # XXX Why are we joining using $assignedto_fieldid here? It shouldn't + # matter when or if the assignee changed. + my $activity_join = + "LEFT JOIN bugs_activity AS $act_table" + . " ON ( $act_table.who = bugs.assigned_to" + . " OR $act_table.fieldid = $assigned_fieldid )" + . " AND $act_table.bug_id = bugs.bug_id" + . " AND $act_table.bug_when > $quoted"; + push(@$joins, $activity_join); + + if ($operator =~ /greater/) { + $args->{term} = + "$ld_table.who IS NULL AND $act_table.who IS NULL)"; } else { - push(@$wherepart, "(comment_$table.who IS NOT NULL " . - "OR activity_$table.who IS NOT NULL)"); + $args->{term} = + "$ld_table.who IS NOT NULL OR $act_table.who IS NOT NULL"; } - $$term = "0=0"; } sub _multiselect_negative { - my $self = shift; - my %func_args = @_; - my ($f, $ff, $t, $term) = @func_args{qw(f ff t term)}; - - my %map = ( - notequals => 'equals', - notregexp => 'regexp', - notsubstring => 'substring', - nowords => 'anywords', - nowordssubstr => 'anywordssubstr', - ); + my ($self, $args) = @_; + my ($field, $operator) = @$args{qw(field operator)}; my $table; - if ($$f eq 'keywords') { + if ($field eq 'keywords') { $table = "keywords LEFT JOIN keyworddefs" . " ON keywords.keywordid = keyworddefs.id"; - $$ff = "keyworddefs.name"; + $args->{full_field} = "keyworddefs.name"; } - else { - $table = "bug_$$f"; - $$ff = "$table.value"; + else { + $table = "bug_$field"; + $args->{full_field} = "$table.value"; } - - $$t = $map{$$t}; - $self->_do_operator_function(\%func_args); - $$term = "bugs.bug_id NOT IN (SELECT bug_id FROM $table WHERE $$term)"; + $args->{operator} = $self->_reverse_operator($operator); + $self->_do_operator_function($args); + my $term = $args->{term}; + $args->{term} = + "bugs.bug_id NOT IN (SELECT bug_id FROM $table WHERE $term)"; } sub _multiselect_multiple { - my $self = shift; - my %func_args = @_; - my ($f, $ff, $t, $v, $term) = @func_args{qw(f ff t v term)}; + my ($self, $args) = @_; + my ($chart_id, $joins, $field, $operator, $value) + = @$args{qw(chart_id joins field operator value)}; - my @terms; - my $table = "bug_$$f"; - $$ff = "$table.value"; + my $table = "bug_$field"; + $args->{full_field} = "$table.value"; - foreach my $word (split(/[\s,]+/, $$v)) { - $$v = $word; - $self->_do_operator_function(\%func_args); - push(@terms, "bugs.bug_id IN - (SELECT bug_id FROM $table WHERE $$term)"); + my @terms; + foreach my $word (split(/[\s,]+/, $value)) { + $args->{value} = $word; + $self->_do_operator_function($args); + my $term = $args->{term}; + push(@terms, "bugs.bug_id IN (SELECT bug_id FROM $table WHERE $term)"); } - if ($$t eq 'anyexact') { - $$term = "(" . join(" OR ", @terms) . ")"; + if ($operator eq 'anyexact') { + $args->{term} = join(" OR ", @terms); } else { - $$term = "(" . join(" AND ", @terms) . ")"; + $args->{term} = join(" AND ", @terms); } } sub _multiselect_nonchanged { - my $self = shift; - my %func_args = @_; - my ($chartid, $f, $ff, $t, $supptables) = - @func_args{qw(chartid f ff t supptables)}; - - my $table = $$f."_".$$chartid; - $$ff = "$table.value"; - - $self->_do_operator_function(\%func_args); - push(@$supptables, "LEFT JOIN bug_$$f AS $table " . - "ON $table.bug_id = bugs.bug_id "); -} + my ($self, $args) = @_; + my ($chart_id, $joins, $field, $operator) = + @$args{qw(chart_id joins field operator)}; -sub _equals { - my $self = shift; - my %func_args = @_; - my ($ff, $q, $term) = @func_args{qw(ff q term)}; - - $$term = "$$ff = $$q"; + my $table = "${field}_$chart_id"; + $args->{full_field} = "$table.value"; + push(@$joins, "LEFT JOIN bug_$field AS $table " . + "ON $table.bug_id = bugs.bug_id "); } -sub _notequals { - my $self = shift; - my %func_args = @_; - my ($ff, $q, $term) = @func_args{qw(ff q term)}; - - $$term = "$$ff != $$q"; +sub _simple_operator { + my ($self, $args) = @_; + my ($full_field, $quoted, $operator) = + @$args{qw(full_field quoted operator)}; + my $sql_operator = SIMPLE_OPERATORS->{$operator}; + $args->{term} = "$full_field $sql_operator $quoted"; } sub _casesubstring { - my $self = shift; - my %func_args = @_; - my ($ff, $q, $term) = @func_args{qw(ff q term)}; + my ($self, $args) = @_; + my ($full_field, $quoted) = @$args{qw(full_field quoted)}; my $dbh = Bugzilla->dbh; - $$term = $dbh->sql_position($$q, $$ff) . " > 0"; + $args->{term} = $dbh->sql_position($quoted, $full_field) . " > 0"; } sub _substring { - my $self = shift; - my %func_args = @_; - my ($ff, $q, $term) = @func_args{qw(ff q term)}; + my ($self, $args) = @_; + my ($full_field, $quoted) = @$args{qw(full_field quoted)}; my $dbh = Bugzilla->dbh; - $$term = $dbh->sql_iposition($$q, $$ff) . " > 0"; + # XXX This should probably be changed to just use LIKE + $args->{term} = $dbh->sql_iposition($quoted, $full_field) . " > 0"; } sub _notsubstring { - my $self = shift; - my %func_args = @_; - my ($ff, $q, $term) = @func_args{qw(ff q term)}; + my ($self, $args) = @_; + my ($full_field, $quoted) = @$args{qw(full_field quoted)}; my $dbh = Bugzilla->dbh; - $$term = $dbh->sql_iposition($$q, $$ff) . " = 0"; + # XXX This should probably be changed to just use NOT LIKE + $args->{term} = $dbh->sql_iposition($quoted, $full_field) . " = 0"; } sub _regexp { - my $self = shift; - my %func_args = @_; - my ($ff, $q, $term) = @func_args{qw(ff q term)}; + my ($self, $args) = @_; + my ($full_field, $quoted) = @$args{qw(full_field quoted)}; my $dbh = Bugzilla->dbh; - $$term = $dbh->sql_regexp($$ff, $$q); + $args->{term} = $dbh->sql_regexp($full_field, $quoted); } sub _notregexp { - my $self = shift; - my %func_args = @_; - my ($ff, $q, $term) = @func_args{qw(ff q term)}; + my ($self, $args) = @_; + my ($full_field, $quoted) = @$args{qw(full_field quoted)}; my $dbh = Bugzilla->dbh; - $$term = $dbh->sql_not_regexp($$ff, $$q); -} - -sub _lessthan { - my $self = shift; - my %func_args = @_; - my ($ff, $q, $term) = @func_args{qw(ff q term)}; - - $$term = "$$ff < $$q"; -} - -sub _lessthaneq { - my $self = shift; - my %func_args = @_; - my ($ff, $q, $term) = @func_args{qw(ff q term)}; - - $$term = "$$ff <= $$q"; -} - -sub _greaterthan { - my $self = shift; - my %func_args = @_; - my ($ff, $q, $term) = @func_args{qw(ff q term)}; - - $$term = "$$ff > $$q"; -} - -sub _greaterthaneq { - my $self = shift; - my %func_args = @_; - my ($ff, $q, $term) = @func_args{qw(ff q term)}; - - $$term = "$$ff >= $$q"; + $args->{term} = $dbh->sql_not_regexp($full_field, $quoted); } sub _anyexact { - my $self = shift; - my %func_args = @_; - my ($f, $ff, $v, $q, $term) = @func_args{qw(f ff v q term)}; + my ($self, $args) = @_; + my ($field, $value, $full_field) = @$args{qw(field value full_field)}; my $dbh = Bugzilla->dbh; my @list; - foreach my $w (split(/,/, $$v)) { - if ($w eq "---" && $$f =~ /resolution/) { - $w = ""; + foreach my $word (split(/,/, $value)) { + if ($word eq "---" && $field eq 'resolution') { + $word = ""; } - $$q = $dbh->quote($w); - trick_taint($$q); - push(@list, $$q); + my $quoted_word = $dbh->quote($word); + trick_taint($quoted_word); + push(@list, $quoted_word); } + if (@list) { - $$term = $dbh->sql_in($$ff, \@list); + $args->{term} = $dbh->sql_in($full_field, \@list); + } + # XXX Perhaps if it's all commas, we should just throw an error. + else { + $args->{term} = "0=0"; } } sub _anywordsubstr { - my $self = shift; - my %func_args = @_; - my ($ff, $v, $term) = @func_args{qw(ff v term)}; + my ($self, $args) = @_; + my ($full_field, $value) = @$args{qw(full_field value)}; - $$term = join(" OR ", @{GetByWordListSubstr($$ff, $$v)}); + my $list = GetByWordListSubstr($full_field, $value); + $args->{term} = join(" OR ", @$list); } sub _allwordssubstr { - my $self = shift; - my %func_args = @_; - my ($ff, $v, $term) = @func_args{qw(ff v term)}; + my ($self, $args) = @_; + my ($full_field, $value) = @$args{qw(full_field value)}; - $$term = join(" AND ", @{GetByWordListSubstr($$ff, $$v)}); + my $list = GetByWordListSubstr($full_field, $value); + $args->{term} = join(" AND ", @$list); } sub _nowordssubstr { - my $self = shift; - my %func_args = @_; - my ($ff, $v, $term) = @func_args{qw(ff v term)}; - - my @list = @{GetByWordListSubstr($$ff, $$v)}; - if (@list) { - $$term = "NOT (" . join(" OR ", @list) . ")"; - } + my ($self, $args) = @_; + $self->_anywordsubstr($args); + my $term = $args->{term}; + $args->{term} = "NOT($term)"; } sub _anywords { - my $self = shift; - my %func_args = @_; - my ($ff, $v, $term) = @func_args{qw(ff v term)}; + my ($self, $args) = @_; + my ($full_field, $value) = @$args{qw(full_field value)}; - $$term = join(" OR ", @{GetByWordList($$ff, $$v)}); + my $list = GetByWordList($full_field, $value); + $args->{term} = join(" OR ", @$list); } sub _allwords { - my $self = shift; - my %func_args = @_; - my ($ff, $v, $term) = @func_args{qw(ff v term)}; + my ($self, $args) = @_; + my ($full_field, $value) = @$args{qw(full_field value)}; - $$term = join(" AND ", @{GetByWordList($$ff, $$v)}); + my $list = GetByWordList($full_field, $value); + $args->{term} = join(" AND ", @$list); } sub _nowords { - my $self = shift; - my %func_args = @_; - my ($ff, $v, $term) = @func_args{qw(ff v term)}; - - my @list = @{GetByWordList($$ff, $$v)}; - if (@list) { - $$term = "NOT (" . join(" OR ", @list) . ")"; - } + my ($self, $args) = @_; + $self->_anywords($args); + my $term = $args->{term}; + $args->{term} = "NOT($term)"; } sub _changedbefore_changedafter { - my $self = shift; - my %func_args = @_; - my ($chartid, $f, $ff, $t, $v, $chartfields, $supptables, $term) = - @func_args{qw(chartid f ff t v chartfields supptables term)}; + my ($self, $args) = @_; + my ($chart_id, $joins, $field, $operator, $value) = + @$args{qw(chart_id joins field operator value)}; my $dbh = Bugzilla->dbh; - my $operator = ($$t =~ /before/) ? '<' : '>'; - my $table = "act_$$chartid"; - my $fieldid = $$chartfields{$$f}; - if (!$fieldid) { - ThrowCodeError("invalid_field_name", {field => $$f}); - } - push(@$supptables, "LEFT JOIN bugs_activity AS $table " . - "ON $table.bug_id = bugs.bug_id " . - "AND $table.fieldid = $fieldid " . - "AND $table.bug_when $operator " . - $dbh->quote(SqlifyDate($$v)) ); - $$term = "($table.bug_when IS NOT NULL)"; + my $sql_operator = ($operator =~ /before/) ? '<' : '>'; + my $table = "act_$chart_id"; + my $field_id = get_field_id($field); + my $sql_date = $dbh->quote(SqlifyDate($value)); + push(@$joins, + "LEFT JOIN bugs_activity AS $table" + . " ON $table.bug_id = bugs.bug_id" + . " AND $table.fieldid = $field_id" + . " AND $table.bug_when $sql_operator $sql_date"); + $args->{term} = "$table.bug_when IS NOT NULL"; } sub _changedfrom_changedto { - my $self = shift; - my %func_args = @_; - my ($chartid, $chartfields, $f, $t, $v, $q, $supptables, $term) = - @func_args{qw(chartid chartfields f t v q supptables term)}; + my ($self, $args) = @_; + my ($chart_id, $joins, $field, $operator, $quoted) = + @$args{qw(chart_id joins field operator quoted)}; - my $operator = ($$t =~ /from/) ? 'removed' : 'added'; - my $table = "act_$$chartid"; - my $fieldid = $$chartfields{$$f}; - if (!$fieldid) { - ThrowCodeError("invalid_field_name", {field => $$f}); - } - push(@$supptables, "LEFT JOIN bugs_activity AS $table " . - "ON $table.bug_id = bugs.bug_id " . - "AND $table.fieldid = $fieldid " . - "AND $table.$operator = $$q"); - $$term = "($table.bug_when IS NOT NULL)"; + my $column = ($operator =~ /from/) ? 'removed' : 'added'; + my $table = "act_$chart_id"; + my $field_id = get_field_id($field); + push(@$joins, + "LEFT JOIN bugs_activity AS $table" + . " ON $table.bug_id = bugs.bug_id" + . " AND $table.fieldid = $field_id" + . " AND $table.$column = $quoted"); + $args->{term} = "$table.bug_when IS NOT NULL"; } sub _changedby { - my $self = shift; - my %func_args = @_; - my ($chartid, $chartfields, $f, $v, $supptables, $term) = - @func_args{qw(chartid chartfields f v supptables term)}; - - my $table = "act_$$chartid"; - my $fieldid = $$chartfields{$$f}; - if (!$fieldid) { - ThrowCodeError("invalid_field_name", {field => $$f}); - } - my $id = login_to_id($$v, THROW_ERROR); - push(@$supptables, "LEFT JOIN bugs_activity AS $table " . - "ON $table.bug_id = bugs.bug_id " . - "AND $table.fieldid = $fieldid " . - "AND $table.who = $id"); - $$term = "($table.bug_when IS NOT NULL)"; + my ($self, $args) = @_; + my ($chart_id, $joins, $field, $operator, $value) = + @$args{qw(chart_id joins field operator value)}; + + my $table = "act_$chart_id"; + my $field_id = get_field_id($field); + my $user_id = login_to_id($value, THROW_ERROR); + push(@$joins, + "LEFT JOIN bugs_activity AS $table" + . " ON $table.bug_id = bugs.bug_id" + . " AND $table.fieldid = $field_id" + . " AND $table.who = $user_id"); + $args->{term} = "$table.bug_when IS NOT NULL"; } 1; diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl index 43644f703..849008adb 100644 --- a/template/en/default/global/code-error.html.tmpl +++ b/template/en/default/global/code-error.html.tmpl @@ -184,10 +184,6 @@ '[% field.description FILTER html %]' ([% field.name FILTER html %]) is not a custom field. - [% ELSIF error == "field_type_mismatch" %] - Cannot seem to handle <code>[% field FILTER html %]</code> - and <code>[% type FILTER html %]</code> together. - [% ELSIF error == "field_type_not_specified" %] [% title = "Field Type Not Specified" %] You must specify a type when creating a custom field. diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index 490941807..3f7248c40 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -744,11 +744,15 @@ [% title = "Group not specified" %] No group was specified. + [% ELSIF error == "group_not_visible" %] + [% title = "Group Not Allowed" %] + You are not allowed to see members of the [% group.name FILTER html %] + group. + [% ELSIF error == "system_group_not_deletable" %] [% title = "System Groups not deletable" %] <em>[% name FILTER html %]</em> is a system group. This group cannot be deleted. - [% ELSIF error == "group_unknown" %] [% title = "Unknown Group" %] The group [% name FILTER html %] does not exist. Please specify @@ -1288,6 +1292,9 @@ [% IF class == "Bugzilla::User" %] Either you mis-typed the name or that user has not yet registered for a [% terms.Bugzilla %] account. + [% ELSIF class == "Bugzilla::Keyword" %] + The legal keyword names are <a href="describekeywords.cgi">listed + here</a>. [% END %] [% ELSIF error == "old_password_incorrect" %] @@ -1477,6 +1484,12 @@ and the "matches" search can only be used with the "content" field. + [% ELSIF error == "search_field_operator_invalid" %] + [% terms.Bugzilla %] does not support using the + "[%+ field_descs.$field FILTER html %]" ([% field FILTER html %]) + field with the "[% search_descs.$operator %]" ([% operator FILTER html %]) + search type. + [% ELSIF error == "series_already_exists" %] [% title = "Series Already Exists" %] [% docslinks = {'reporting.html' => 'Reporting'} %] @@ -1562,11 +1575,6 @@ I could not figure out what you wanted to do. [% END %] - [% ELSIF error == "unknown_keyword" %] - [% title = "Unknown Keyword" %] - <code>[% keyword FILTER html %]</code> is not a known keyword. - The legal keyword names are <a href="describekeywords.cgi">listed here</a>. - [% ELSIF error == "unknown_tab" %] [% title = "Unknown Tab" %] <code>[% current_tab_name FILTER html %]</code> is not a legal tab name. @@ -1759,6 +1767,8 @@ field [% ELSIF class == "Bugzilla::Group" %] group + [% ELSIF class == "Bugzilla::Keyword" %] + keyword [% ELSIF class == "Bugzilla::Product" %] product [% ELSIF class == "Bugzilla::Search::Recent" %] diff --git a/xt/lib/Bugzilla/Test/Search/Constants.pm b/xt/lib/Bugzilla/Test/Search/Constants.pm index 86e195ae0..7f35b3719 100644 --- a/xt/lib/Bugzilla/Test/Search/Constants.pm +++ b/xt/lib/Bugzilla/Test/Search/Constants.pm @@ -179,22 +179,6 @@ use constant FIELD_SUBSTR_SIZE => { # See the KNOWN_BROKEN constant for a general description of these # "_BROKEN" constants. -# Search.pm currently enforces "this must be a 0 or 1" in situations -# where it should not, with two of the attachment booleans. -use constant ATTACHMENT_BOOLEANS_SEARCH_BROKEN => ( - 'attachments.ispatch' => { search => 1 }, - 'attachments.isobsolete' => { search => 1 }, -); - -# Sometimes the search for attachment booleans works, but then contains -# the wrong results, because it does not contain bugs that fully lack -# attachments. -use constant ATTACHMENT_BOOLEANS_CONTAINS_BROKEN => ( - 'attachments.isobsolete' => { contains => [5] }, - 'attachments.ispatch' => { contains => [5] }, - 'attachments.isprivate' => { contains => [5] }, -); - # Certain fields fail all the "negative" search tests: # # Blocked and Dependson "notequals" only finds bugs that have @@ -223,7 +207,9 @@ use constant ATTACHMENT_BOOLEANS_CONTAINS_BROKEN => ( # # requestees.login_name doesn't find bugs that fully lack requestees. use constant NEGATIVE_BROKEN => ( - ATTACHMENT_BOOLEANS_CONTAINS_BROKEN, + 'attachments.isobsolete' => { contains => [5] }, + 'attachments.ispatch' => { contains => [5] }, + 'attachments.isprivate' => { contains => [5] }, 'attach_data.thedata' => { contains => [5] }, 'attachments.description' => { contains => [5] }, 'attachments.filename' => { contains => [5] }, @@ -237,7 +223,6 @@ use constant NEGATIVE_BROKEN => ( dependson => { contains => [2,4,5] }, longdesc => { contains => [1] }, 'longdescs.isprivate' => { contains => [1] }, - percentage_complete => { contains => [1] }, 'requestees.login_name' => { contains => [3,4,5] }, 'setters.login_name' => { contains => [5] }, work_time => { contains => [1] }, @@ -271,16 +256,12 @@ use constant GREATERTHAN_BROKEN => ( # allwordssubstr work_time only matches against a single comment, # instead of matching against all comments on a bug. Same is true # for the other longdesc fields, cc, keywords, and bug_group. -# -# percentage_complete just drops in 0=0 for the term. use constant ALLWORDS_BROKEN => ( - ATTACHMENT_BOOLEANS_SEARCH_BROKEN, bug_group => { contains => [1] }, cc => { contains => [1] }, keywords => { contains => [1] }, longdesc => { contains => [1] }, work_time => { contains => [1] }, - percentage_complete => { contains => [2,3,4,5] }, ); # nowords and nowordssubstr have these broken tests in common. @@ -306,7 +287,7 @@ use constant NOWORDS_BROKEN => ( use constant CHANGED_BROKEN => ( classification => { contains => [1] }, commenter => { contains => [1] }, - percentage_complete => { contains => [2,3,4,5] }, + percentage_complete => { contains => [1] }, 'requestees.login_name' => { contains => [1] }, 'setters.login_name' => { contains => [1] }, delta_ts => { contains => [1] }, @@ -346,28 +327,9 @@ use constant CHANGED_VALUE_BROKEN => ( # while the other fails. In this case, we have a special override for # "operator-value", which uniquely identifies tests. use constant KNOWN_BROKEN => { - notequals => { NEGATIVE_BROKEN }, - # percentage_complete substring matches every bug, regardless of - # its percentage_complete value. - substring => { - percentage_complete => { contains => [2,3,4,5] }, - }, - casesubstring => { - percentage_complete => { contains => [2,3,4,5] }, - }, + notequals => { NEGATIVE_BROKEN }, notsubstring => { NEGATIVE_BROKEN }, - - # Attachment noolean fields don't work with regexes, right now, - # because they throw an error that regexes are not valid booleans. - 'regexp-^1-' => { ATTACHMENT_BOOLEANS_SEARCH_BROKEN }, - - # percentage_complete notregexp fails to match bugs that - # fully lack hours worked. - notregexp => { - NEGATIVE_BROKEN, - percentage_complete => { contains => [5] }, - }, - 'notregexp-^1-' => { ATTACHMENT_BOOLEANS_SEARCH_BROKEN }, + notregexp => { NEGATIVE_BROKEN }, # percentage_complete doesn't match bugs with 0 hours worked or remaining. # @@ -376,19 +338,13 @@ use constant KNOWN_BROKEN => { # also broken in this way, but all our comments come from the same user.) # Also, the attachments ones don't find bugs that have no attachments # at all (which might be OK?). - # - # attachments.isprivate lessthan doesn't find bugs without attachments. lessthan => { - ATTACHMENT_BOOLEANS_SEARCH_BROKEN, - 'attachments.isprivate' => { contains => [5] }, 'longdescs.isprivate' => { contains => [1] }, - percentage_complete => { contains => [5] }, work_time => { contains => [1,2,3,4] }, }, # The lessthaneq tests are broken for the same reasons, but they work # slightly differently so they have a different set of broken tests. lessthaneq => { - ATTACHMENT_BOOLEANS_CONTAINS_BROKEN, 'longdescs.isprivate' => { contains => [1] }, work_time => { contains => [2,3,4] }, }, @@ -401,24 +357,18 @@ use constant KNOWN_BROKEN => { percentage_complete => { contains => [2] }, }, - # percentage_complete just throws 0=0 into the search term, returning - # all bugs. + # percentage_complete doesn't do a numeric comparison, so + # it doesn't find decimal values. anyexact => { - ATTACHMENT_BOOLEANS_SEARCH_BROKEN, - percentage_complete => { contains => [3,4,5] }, + percentage_complete => { contains => [2] }, }, # bug_group anywordssubstr returns all our bugs. Not sure why. anywordssubstr => { - ATTACHMENT_BOOLEANS_SEARCH_BROKEN, - percentage_complete => { contains => [3,4,5] }, + percentage_complete => { contains => [2] }, bug_group => { contains => [3,4,5] }, }, 'allwordssubstr-<1>' => { ALLWORDS_BROKEN }, - 'allwordssubstr-<1>,<2>' => { - ATTACHMENT_BOOLEANS_SEARCH_BROKEN, - percentage_complete => { contains => [1,2,3,4,5] }, - }, # flagtypes.name does not work here, probably because they all try to # match against a single flag. # Same for attach_data.thedata. @@ -427,10 +377,6 @@ use constant KNOWN_BROKEN => { 'attach_data.thedata' => { contains => [1] }, 'flagtypes.name' => { contains => [1] }, }, - 'allwords-<1> <2>' => { - ATTACHMENT_BOOLEANS_SEARCH_BROKEN, - percentage_complete => { contains => [1,2,3,4,5] }, - }, nowordssubstr => { NOWORDS_BROKEN }, # attach_data.thedata doesn't match properly with any of the plain @@ -446,15 +392,13 @@ use constant KNOWN_BROKEN => { # attach_data doesn't work (perhaps because it's the entire # data, or some problem with the regex?). anywords => { - ATTACHMENT_BOOLEANS_SEARCH_BROKEN, 'attach_data.thedata' => { contains => [1] }, bug_group => { contains => [2,3,4,5] }, - percentage_complete => { contains => [2,3,4,5] }, work_time => { contains => [1] }, }, 'anywords-<1> <2>' => { bug_group => { contains => [3,4,5] }, - percentage_complete => { contains => [3,4,5] }, + percentage_complete => { contains => [2] }, 'attach_data.thedata' => { contains => [1,2] }, work_time => { contains => [1,2] }, }, @@ -496,7 +440,7 @@ use constant KNOWN_BROKEN => { commenter => { contains => [2,3,4] }, creation_ts => { contains => [2,3,4] }, delta_ts => { contains => [2,3,4] }, - percentage_complete => { contains => [1,5] }, + percentage_complete => { contains => [2,3,4] }, 'requestees.login_name' => { contains => [2,3,4] }, 'setters.login_name' => { contains => [2,3,4] }, }, @@ -552,11 +496,13 @@ use constant REGEX_OVERRIDE => { blocked => { value => '^<1>$' }, dependson => { value => '^<1>$' }, bug_id => { value => '^<1>$' }, - 'attachments.isprivate' => { value => '^1' }, - cclist_accessible => { value => '^1' }, - reporter_accessible => { value => '^1' }, - everconfirmed => { value => '^1' }, - 'longdescs.isprivate' => { value => '^1' }, + 'attachments.isobsolete' => { value => '^1'}, + 'attachments.ispatch' => { value => '^1'}, + 'attachments.isprivate' => { value => '^1' }, + cclist_accessible => { value => '^1' }, + reporter_accessible => { value => '^1' }, + everconfirmed => { value => '^1' }, + 'longdescs.isprivate' => { value => '^1' }, creation_ts => { value => '^2037-01-01' }, delta_ts => { value => '^2037-01-01' }, deadline => { value => '^2037-02-01' }, @@ -734,11 +680,13 @@ use constant TESTS => { blocked => { value => '<4-id>', contains => [1,2] }, dependson => { value => '<3-id>', contains => [1,3] }, bug_id => { value => '<2-id>' }, - 'attachments.isprivate' => { value => 1, contains => [2,3,4,5] }, - cclist_accessible => { value => 1, contains => [2,3,4,5] }, - reporter_accessible => { value => 1, contains => [2,3,4,5] }, - 'longdescs.isprivate' => { value => 1, contains => [2,3,4,5] }, - everconfirmed => { value => 1, contains => [2,3,4,5] }, + 'attachments.isprivate' => { value => 1, contains => [2,3,4] }, + 'attachments.isobsolete' => { value => 1, contains => [2,3,4] }, + 'attachments.ispatch' => { value => 1, contains => [2,3,4] }, + cclist_accessible => { value => 1, contains => [2,3,4,5] }, + reporter_accessible => { value => 1, contains => [2,3,4,5] }, + 'longdescs.isprivate' => { value => 1, contains => [2,3,4,5] }, + everconfirmed => { value => 1, contains => [2,3,4,5] }, creation_ts => { value => '2037-01-02', contains => [1,5] }, delta_ts => { value => '2037-01-02', contains => [1,5] }, deadline => { value => '2037-02-02' }, @@ -755,9 +703,9 @@ use constant TESTS => { lessthaneq => [ { contains => [1], value => '<1>', override => { - 'attachments.ispatch' => { value => 0, contains => [2,3,4,5] }, - 'attachments.isobsolete' => { value => 0, contains => [2,3,4,5] }, - 'attachments.isprivate' => { value => 0, contains => [2,3,4,5] }, + 'attachments.isobsolete' => { value => 0, contains => [2,3,4] }, + 'attachments.ispatch' => { value => 0, contains => [2,3,4] }, + 'attachments.isprivate' => { value => 0, contains => [2,3,4] }, cclist_accessible => { value => 0, contains => [2,3,4,5] }, reporter_accessible => { value => 0, contains => [2,3,4,5] }, 'longdescs.isprivate' => { value => 0, contains => [2,3,4,5] }, @@ -768,6 +716,7 @@ use constant TESTS => { delta_ts => { contains => [1,5] }, remaining_time => { contains => [1,5] }, longdesc => { contains => [1,5] }, + percentage_complete => { contains => [1,5] }, work_time => { value => 1, contains => [1,5] }, LESSTHAN_OVERRIDE, }, @@ -936,13 +885,7 @@ use constant TESTS => { # operator_ok overrides the "brokenness" of certain operators, so that they # are always OK for that field/operator combination. use constant INJECTION_BROKEN_FIELD => { - 'attachments.isobsolete' => { search => 1 }, - 'attachments.ispatch' => { search => 1 }, - owner_idle_time => { - sql_error => qr/bugs\.owner_idle_time.+where clause/, - operator_ok => [qw(changedfrom changedto greaterthan greaterthaneq - lessthan lessthaneq)] - }, + owner_idle_time => { search => 1 }, keywords => { search => 1, operator_ok => [qw(allwordssubstr anywordssubstr casesubstring @@ -957,9 +900,9 @@ use constant INJECTION_BROKEN_FIELD => { # search => 1 means the Bugzilla::Search creation fails, but # field_ok contains fields that it does actually succeed for. use constant INJECTION_BROKEN_OPERATOR => { - changedafter => { search => 1, field_ok => ['percentage_complete'] }, - changedbefore => { search => 1, field_ok => ['percentage_complete'] }, - changedby => { search => 1, field_ok => ['percentage_complete'] }, + changedafter => { search => 1 }, + changedbefore => { search => 1 }, + changedby => { search => 1 }, }; # Tests run by Bugzilla::Test::Search::InjectionTest. |