diff options
| -rw-r--r-- | Bugzilla/Search.pm | 293 | ||||
| -rw-r--r-- | xt/lib/Bugzilla/Test/Search/Constants.pm | 17 |
2 files changed, 153 insertions, 157 deletions
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 2f7dfdcad..59a358a6f 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -189,31 +189,30 @@ use constant OPERATOR_REVERSE => { }; use constant OPERATOR_FIELD_OVERRIDE => { - # User fields 'attachments.submitter' => { - _default => \&_attachments_submitter, + _non_changed => \&_user_nonchanged, }, assigned_to => { - _non_changed => \&_contact_nonchanged, + _non_changed => \&_user_nonchanged, }, cc => { - _non_changed => \&_cc_nonchanged, + _non_changed => \&_user_nonchanged, }, commenter => { - _default => \&_commenter, + _non_changed => \&_user_nonchanged, }, reporter => { - _non_changed => \&_contact_nonchanged, + _non_changed => \&_user_nonchanged, }, 'requestees.login_name' => { - _default => \&_requestees_login_name, + _non_changed => \&_user_nonchanged, }, 'setters.login_name' => { - _default => \&_setters_login_name, - }, + _non_changed => \&_user_nonchanged, + }, qa_contact => { - _non_changed => \&_qa_contact_nonchanged, + _non_changed => \&_user_nonchanged, }, # General Bug Fields @@ -332,6 +331,38 @@ use constant SPECIAL_PARSING => { delta_ts => \&_timestamp_translate, }; +# Information about fields that represent "users", used by _user_nonchanged. +# There are other user fields than the ones listed here, but those use +# defaults in _user_nonchanged. +use constant USER_FIELDS => { + 'attachments.submitter' => { + field => 'submitter_id', + join => { table => 'attachments' }, + isprivate => 1, + }, + cc => { + field => 'who', + join => { table => 'cc' }, + }, + commenter => { + field => 'who', + join => { table => 'longdescs', join => 'INNER' }, + isprivate => 1, + }, + qa_contact => { + nullable => 1, + }, + 'requestees.login_name' => { + nullable => 1, + field => 'requestee_id', + join => { table => 'flags' }, + }, + 'setters.login_name' => { + field => 'setter_id', + join => { table => 'flags' }, + }, +}; + # Backwards compatibility for times that we changed the names of fields # or URL parameters. use constant FIELD_MAP => { @@ -1786,7 +1817,7 @@ sub pronoun { return "bugs.assigned_to"; } if ($noun eq "%qacontact%") { - return "bugs.qa_contact"; + return "COALESCE(bugs.qa_contact,0)"; } return 0; } @@ -1802,6 +1833,7 @@ sub _contact_pronoun { elsif ($value =~ /^(%\w+%)$/) { $args->{value} = pronoun($1, $user); $args->{quoted} = $args->{value}; + $args->{value_is_id} = 1; } } @@ -1844,7 +1876,7 @@ sub _cc_pronoun { elsif ($value =~ /^(%\w+%)$/) { $args->{value} = pronoun($1, $user); $args->{quoted} = $args->{value}; - $args->{full_field} = "profiles.userid"; + $args->{value_is_id} = 1; } } @@ -1896,7 +1928,7 @@ sub _commenter_pronoun { if ($value =~ /^(%\w+%)$/) { $args->{value} = pronoun($1, $user); $args->{quoted} = $args->{value}; - $args->{full_field} = "profiles.userid"; + $args->{value_is_id} = 1; } } @@ -1911,53 +1943,116 @@ sub _invalid_combination { { field => $field, operator => $operator }); } -sub _contact_nonchanged { - my ($self, $args) = @_; - my $field = $args->{field}; - - $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, $args) = @_; - - # This will join in map_qa_contact for us. - $self->_add_extra_column('qa_contact'); - $args->{full_field} = "COALESCE(map_qa_contact.login_name,'')"; -} - -sub _cc_nonchanged { +# For all the "user" fields--assigned_to, reporter, qa_contact, +# cc, commenter, requestee, etc. +sub _user_nonchanged { my ($self, $args) = @_; - my ($chart_id, $sequence, $field, $full_field, $operator, $joins) = - @$args{qw(chart_id sequence field full_field operator joins)}; - - # This is for the email1, email2, email3 fields from query.cgi. - if ($chart_id eq "") { - $chart_id = "CC$sequence"; - $args->{sequence}++; + my ($field, $operator, $chart_id, $sequence, $joins) = + @$args{qw(field operator chart_id sequence joins)}; + + my $is_in_other_table; + if (my $join = USER_FIELDS->{$field}->{join}) { + $is_in_other_table = 1; + my $as = "${field}_$chart_id"; + # Needed for setters.login_name and requestees.login_name. + # Otherwise when we try to join "profiles" below, we'd get + # something like "setters.login_name.login_name" in the "from". + $as =~ s/\./_/g; + # This helps implement the email1, email2, etc. parameters. + if ($chart_id =~ /default/) { + $as .= "_$sequence"; + } + my $isprivate = USER_FIELDS->{$field}->{isprivate}; + my $extra = ($isprivate and !$self->_user->is_insider) + ? ["$as.isprivate = 0"] : []; + # We want to copy $join so as not to modify USER_FIELDS. + push(@$joins, { %$join, as => $as, extra => $extra }); + my $search_field = USER_FIELDS->{$field}->{field}; + $args->{full_field} = "$as.$search_field"; + } + + my $is_nullable = USER_FIELDS->{$field}->{nullable}; + my $null_alternate = "''"; + # When using a pronoun, we use the userid, and we don't have to + # join the profiles table. + if ($args->{value_is_id}) { + $null_alternate = 0; } - - # $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"; + else { + my $as = "name_${field}_$chart_id"; + # For fields with periods in their name. + $as =~ s/\./_/; + my $join = { + table => 'profiles', + as => $as, + from => $args->{full_field}, + to => 'userid', + join => (!$is_in_other_table and !$is_nullable) ? 'INNER' : undef, + }; + push(@$joins, $join); + $args->{full_field} = "$as.login_name"; } - $self->_do_operator_function($args); - - my $term = $args->{term}; - my $table = "cc_$chart_id"; - my $join = { - table => 'cc', - as => $table, - extra => ["$table.who IN (SELECT userid FROM profiles WHERE $term)"], - }; - push(@$joins, $join); + # We COALESCE fields that can be NULL, to make "not"-style operators + # continue to work properly. For example, "qa_contact is not equal to bob" + # should also show bugs where the qa_contact is NULL. With COALESCE, + # it does. + if ($is_nullable) { + $args->{full_field} = "COALESCE($args->{full_field}, $null_alternate)"; + } - $args->{term} = "$table.who IS NOT NULL"; + # For fields whose values are stored in other tables, negation (NOT) + # only works properly if we put the condition into the JOIN instead + # of the WHERE. + if ($is_in_other_table) { + # Using the last join works properly whether we're searching based + # on userid or login_name. + my $last_join = $joins->[-1]; + + # For negative operators, the system we're using here + # only works properly if we reverse the operator and check IS NULL + # in the WHERE. + my $is_negative = $operator =~ /^no/ ? 1 : 0; + if ($is_negative) { + $args->{operator} = $self->_reverse_operator($operator); + } + $self->_do_operator_function($args); + push(@{ $last_join->{extra} }, $args->{term}); + + # For login_name searches, we only want a single join. + # So we create a subselect table out of our two joins. This makes + # negation (NOT) work properly for values that are in other + # tables. + if ($last_join->{table} eq 'profiles') { + pop @$joins; + $last_join->{join} = 'INNER'; + my ($join_sql) = $self->_translate_join($last_join); + my $first_join = $joins->[-1]; + my $as = $first_join->{as}; + my $table = $first_join->{table}; + my $columns = "bug_id"; + $columns .= ",isprivate" if @{ $first_join->{extra} }; + my $new_table = "SELECT $columns FROM $table AS $as $join_sql"; + $first_join->{table} = "($new_table)"; + # We always want to LEFT JOIN the generated table. + delete $first_join->{join}; + # To support OR charts, we need multiple tables. + my $new_as = $first_join->{as} . "_$sequence"; + $_ =~ s/\Q$as\E/$new_as/ foreach @{ $first_join->{extra} }; + $first_join->{as} = $new_as; + $last_join = $first_join; + } + + # If we're joining the first table (we're using a pronoun and + # searching by user id) then we need to check $other_table->{field}. + my $check_field = $last_join->{as} . '.bug_id'; + if ($is_negative) { + $args->{term} = "$check_field IS NULL"; + } + else { + $args->{term} = "$check_field IS NOT NULL"; + } + } } # XXX This duplicates having Commenter as a search field. @@ -2037,34 +2132,6 @@ sub _content_matches { COLUMNS->{'relevance'}->{name} = $select_term; } -sub _commenter { - my ($self, $args) = @_; - my ($chart_id, $sequence, $joins, $field, $full_field, $operator) = - @$args{qw(chart_id sequence joins field full_field operator)}; - - if ($chart_id eq "") { - $chart_id = "LD$sequence"; - $args->{sequence}++; - } - 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($args); - my $term = $args->{term}; - my $join = { - table => 'longdescs', - as => $table, - extra => ["$table.who IN (SELECT userid FROM profiles WHERE $term)"], - }; - push(@$joins, $join); - $args->{term} = "$table.who IS NOT NULL"; -} - sub _long_desc { my ($self, $args) = @_; my ($chart_id, $joins) = @$args{qw(chart_id joins)}; @@ -2209,30 +2276,6 @@ sub _attach_data_thedata { $args->{full_field} = "$data_table.thedata"; } -sub _attachments_submitter { - my ($self, $args) = @_; - my ($chart_id, $joins) = @$args{qw(chart_id joins)}; - - my $attach_table = "attachments_$chart_id"; - my $profiles_table = "map_attachment_submitter_$chart_id"; - my $extra = $self->_user->is_insider - ? [] : ["$attach_table.isprivate = 0"]; - my $attachments_join = { - table => 'attachments', - as => $attach_table, - extra => $extra, - }; - my $profiles_join = { - table => 'profiles', - as => $profiles_table, - from => "$attach_table.submitter_id", - to => 'userid', - }; - push(@$joins, $attachments_join, $profiles_join); - - $args->{full_field} = "$profiles_table.login_name"; -} - sub _attachments { my ($self, $args) = @_; my ($chart_id, $joins, $field) = @@ -2329,42 +2372,6 @@ sub _flagtypes_name { } } -# XXX These two functions can probably be joined (requestees and setters). -sub _requestees_login_name { - 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_requestees_$chart_id"; - my $profiles_join = { - table => 'profiles', - as => $map_table, - from => "$flags.requestee_id", - to => 'userid', - }; - push(@$joins, $profiles_join); - - $args->{full_field} = "$map_table.login_name"; -} - -sub _setters_login_name { - 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"; - my $profiles_join = { - table => 'profiles', - as => $map_table, - from => "$flags.setter_id", - to => 'userid', - }; - push(@$joins, $profiles_join); - $args->{full_field} = "$map_table.login_name"; -} - sub _days_elapsed { my ($self, $args) = @_; my $dbh = Bugzilla->dbh; diff --git a/xt/lib/Bugzilla/Test/Search/Constants.pm b/xt/lib/Bugzilla/Test/Search/Constants.pm index b509cf76b..3c68f69d2 100644 --- a/xt/lib/Bugzilla/Test/Search/Constants.pm +++ b/xt/lib/Bugzilla/Test/Search/Constants.pm @@ -225,17 +225,13 @@ use constant NEGATIVE_BROKEN => ( 'attachments.description' => { contains => [5] }, 'attachments.filename' => { contains => [5] }, 'attachments.mimetype' => { contains => [5] }, - 'attachments.submitter' => { contains => [5] }, blocked => { contains => [3,4,5] }, bug_file_loc => { contains => [5] }, bug_group => { contains => [1,5] }, - cc => { contains => [1,5] }, deadline => { contains => [5] }, dependson => { contains => [2,4,5] }, longdesc => { contains => [1] }, 'longdescs.isprivate' => { contains => [1] }, - 'requestees.login_name' => { contains => [3,4,5] }, - 'setters.login_name' => { contains => [5] }, work_time => { contains => [1] }, # Custom fields are busted because they can be NULL. FIELD_TYPE_FREETEXT, { contains => [5] }, @@ -278,7 +274,7 @@ use constant ALLWORDS_BROKEN => ( # nowords and nowordssubstr have these broken tests in common. # # flagtypes.name doesn't match bugs without flags. -# cc, longdescs.isprivate, and bug_group actually work properly in +# longdescs.isprivate, and bug_group actually work properly in # terms of excluding bug 1 (since we exclude all values in the search, # on our test), but still fail at including bug 5. # The longdesc* and work_time fields, coincidentally, work completely @@ -287,7 +283,6 @@ use constant NOWORDS_BROKEN => ( NEGATIVE_BROKEN, 'flagtypes.name' => { contains => [5] }, bug_group => { contains => [5] }, - cc => { contains => [5] }, longdesc => {}, work_time => {}, 'longdescs.isprivate' => {}, @@ -508,16 +503,13 @@ use constant COMMON_BROKEN_NOT => ( "attachments.ispatch" => { contains => [5] }, "attachments.isprivate" => { contains => [5] }, "attachments.mimetype" => { contains => [5] }, - "attachments.submitter" => { contains => [5] }, "bug_file_loc" => { contains => [5] }, "deadline" => { contains => [5] }, "flagtypes.name" => { contains => [5] }, "keywords" => { contains => [5] }, "longdescs.isprivate" => { contains => [1] }, "percentage_complete" => { contains => [1 .. 5] }, - "requestees.login_name" => { contains => [3, 4, 5] }, "see_also" => { contains => [5] }, - "setters.login_name" => { contains => [5] }, FIELD_TYPE_BUG_ID, { contains => [5] }, FIELD_TYPE_DATETIME, { contains => [5] }, FIELD_TYPE_FREETEXT, { contains => [5] }, @@ -551,7 +543,6 @@ use constant CHANGED_FROM_TO_BROKEN_NOT => ( use constant NEGATIVE_BROKEN_NOT => ( "blocked" => { contains => [3, 4, 5] }, "bug_group" => { contains => [5] }, - "cc" => { contains => [1, 5] }, "dependson" => { contains => [2, 4, 5] }, "flagtypes.name" => { contains => [1 .. 5] }, "percentage_complete" => { contains => [1 .. 5] }, @@ -561,8 +552,8 @@ use constant NEGATIVE_BROKEN_NOT => ( use constant BROKEN_NOT => { allwords => { COMMON_BROKEN_NOT, - bug_group => { contains => [1] }, cc => { contains => [1] }, + bug_group => { contains => [1] }, "flagtypes.name" => { contains => [1,5] }, keywords => { contains => [1,5] }, longdesc => { contains => [1] }, @@ -592,7 +583,7 @@ use constant BROKEN_NOT => { }, 'allwordssubstr-<1>,<2>' => { bug_group => { }, - "cc" => { }, + cc => { }, FIELD_TYPE_MULTI_SELECT, { }, keywords => { contains => [5] }, "longdesc" => { }, @@ -725,13 +716,11 @@ use constant BROKEN_NOT => { nowords => { NEGATIVE_BROKEN_NOT, "work_time" => { contains => [2, 3, 4] }, - "cc" => { contains => [5] }, "flagtypes.name" => { }, }, nowordssubstr => { NEGATIVE_BROKEN_NOT, "attach_data.thedata" => { }, - "cc" => { contains => [5] }, "flagtypes.name" => { }, "work_time" => { contains => [2, 3, 4] }, }, |