diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index 2f7dfdcad10f9e5fded70a53f28e5f33c9c4d5e8..59a358a6fbd6ba28dbe773540905b1a5cca01550 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 b509cf76b5f74bc611d975d119cbc603a636c1c9..3c68f69d2cee23d7ba3262d31b73ce563dedb830 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] },
},