diff options
| -rw-r--r-- | Bugzilla/Search.pm | 42 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 4 | ||||
| -rw-r--r-- | xt/lib/Bugzilla/Test/Search/Constants.pm | 36 |
3 files changed, 24 insertions, 58 deletions
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 81b459ee5..2bbd4e451 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -309,9 +309,7 @@ use constant OPERATOR_FIELD_OVERRIDE => { changedto => \&_invalid_combination, _default => \&_long_descs_count, }, - 'longdescs.isprivate' => { - _default => \&_longdescs_isprivate, - }, + 'longdescs.isprivate' => MULTI_SELECT_OVERRIDE, owner_idle_time => { greaterthan => \&_owner_idle_time_greater_less, greaterthaneq => \&_owner_idle_time_greater_less, @@ -2269,25 +2267,6 @@ sub _content_matches { COLUMNS->{'relevance'}->{name} = $select_term; } -sub _join_longdescs { - my ($self, $args) = @_; - my ($chart_id, $joins) = @$args{qw(chart_id joins)}; - - my $table = "longdescs_$chart_id"; - my $extra = $self->_user->is_insider ? [] : ["$table.isprivate = 0"]; - my $join = { - table => 'longdescs', - as => $table, - extra => $extra, - }; - # We only want to do an INNER JOIN if we're not checking isprivate. - # Otherwise we'd exclude all bugs with only private comments from - # the search entirely. - $join->{join} = 'INNER' if $self->_user->is_insider; - push(@$joins, $join); - return $table; -} - sub _long_descs_count { my ($self, $args) = @_; my ($chart_id, $joins) = @$args{qw(chart_id joins)}; @@ -2302,12 +2281,6 @@ sub _long_descs_count { $args->{full_field} = "${table}.num"; } -sub _longdescs_isprivate { - my ($self, $args) = @_; - my $table = $self->_join_longdescs($args); - $args->{full_field} = "$table.isprivate"; -} - sub _work_time_changedby { my ($self, $args) = @_; my ($chart_id, $joins, $value) = @$args{qw(chart_id joins value)}; @@ -2588,11 +2561,12 @@ sub _multiselect_multiple { push(@terms, $self->_multiselect_term($args)); } + # The spacing in the joins helps make the resulting SQL more readable. if ($operator =~ /^any/) { - $args->{term} = join(" OR ", @terms); + $args->{term} = join("\n OR ", @terms); } else { - $args->{term} = join(" AND ", @terms); + $args->{term} = join("\n AND ", @terms); } } @@ -2633,6 +2607,14 @@ sub _multiselect_table { $args->{full_field} = 'thetext'; return "longdescs"; } + elsif ($field eq 'longdescs.isprivate') { + ThrowUserError('auth_failure', { action => 'search', + object => 'bug_fields', + field => 'longdescs.isprivate' }) + if !$self->_user->is_insider; + $args->{full_field} = 'isprivate'; + return "longdescs"; + } my $table = "bug_$field"; $args->{full_field} = "bug_$field.value"; return $table; diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index 47219e3d9..16b66ca84 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -149,6 +149,8 @@ run [% ELSIF action == "schedule" %] schedule + [% ELSIF action == "search" %] + search [% ELSIF action == "use" %] use [% ELSIF action == "approve" %] @@ -167,6 +169,8 @@ [% END %] [% ELSIF object == "bugs" %] [%+ terms.bugs %] + [% ELSIF object == "bug_fields" %] + the [% field_descs.$field FILTER html %] field [% ELSIF object == "charts" %] the "New Charts" feature [% ELSIF object == "classifications" %] diff --git a/xt/lib/Bugzilla/Test/Search/Constants.pm b/xt/lib/Bugzilla/Test/Search/Constants.pm index ed7ff8f74..227203bc7 100644 --- a/xt/lib/Bugzilla/Test/Search/Constants.pm +++ b/xt/lib/Bugzilla/Test/Search/Constants.pm @@ -214,7 +214,6 @@ use constant NEGATIVE_BROKEN => ( 'attachments.mimetype' => { contains => [5] }, bug_file_loc => { contains => [5] }, deadline => { contains => [5] }, - 'longdescs.isprivate' => { contains => [1] }, # Custom fields are busted because they can be NULL. FIELD_TYPE_FREETEXT, { contains => [5] }, FIELD_TYPE_BUG_ID, { contains => [5] }, @@ -248,15 +247,9 @@ use constant ALLWORDS_BROKEN => ( # nowords and nowordssubstr have these broken tests in common. # # flagtypes.name doesn't match bugs without flags. -# longdescs.isprivate actually works properly in -# terms of excluding bug 1 (since we exclude all values in the search, -# on our test), but still fails at including bug 5. -# The longdesc* fields, coincidentally, work completely -# correctly, possibly because there's only one comment on bug 5. use constant NOWORDS_BROKEN => ( NEGATIVE_BROKEN, 'flagtypes.name' => { contains => [5] }, - 'longdescs.isprivate' => {}, ); # Fields that don't generally work at all with changed* searches, but @@ -311,18 +304,6 @@ use constant KNOWN_BROKEN => { notsubstring => { NEGATIVE_BROKEN }, notregexp => { NEGATIVE_BROKEN }, - # longdescs.isprivate matches if any comment matches, instead of if all - # comments match. (Commenter is probably - # also broken in this way, but all our comments come from the same user.) - lessthan => { - 'longdescs.isprivate' => { contains => [1] }, - }, - # The lessthaneq tests are broken for the same reasons, but they work - # slightly differently so they have a different set of broken tests. - lessthaneq => { - 'longdescs.isprivate' => { contains => [1] }, - }, - greaterthan => { GREATERTHAN_BROKEN }, greaterthaneq => { GREATERTHAN_BROKEN }, @@ -441,7 +422,6 @@ use constant COMMON_BROKEN_NOT => ( "bug_file_loc" => { contains => [5] }, "deadline" => { contains => [5] }, "flagtypes.name" => { contains => [5] }, - "longdescs.isprivate" => { contains => [1] }, FIELD_TYPE_BUG_ID, { contains => [5] }, FIELD_TYPE_DATETIME, { contains => [5] }, FIELD_TYPE_FREETEXT, { contains => [5] }, @@ -486,7 +466,6 @@ use constant BROKEN_NOT => { 'attach_data.thedata' => { contains => [5] }, cc => { }, 'flagtypes.name' => { contains => [5] }, - 'longdescs.isprivate' => { }, }, allwordssubstr => { COMMON_BROKEN_NOT, @@ -494,7 +473,6 @@ use constant BROKEN_NOT => { }, 'allwordssubstr-<1>,<2>' => { cc => { }, - "longdescs.isprivate" => { }, }, anyexact => { COMMON_BROKEN_NOT, @@ -560,11 +538,9 @@ use constant BROKEN_NOT => { }, lessthan => { COMMON_BROKEN_NOT, - 'longdescs.isprivate' => { }, }, lessthaneq => { COMMON_BROKEN_NOT, - 'longdescs.isprivate' => { }, }, notequals => { NEGATIVE_BROKEN_NOT }, notregexp => { NEGATIVE_BROKEN_NOT }, @@ -828,7 +804,7 @@ use constant TESTS => { cclist_accessible => { value => 1, contains => [2,3,4,5] }, reporter_accessible => { value => 1, contains => [2,3,4,5] }, 'longdescs.count' => { value => 3, contains => [2,3,4,5] }, - 'longdescs.isprivate' => { value => 1, contains => [2,3,4,5] }, + 'longdescs.isprivate' => { value => 1, contains => [1,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] }, @@ -852,7 +828,7 @@ use constant TESTS => { cclist_accessible => { value => 0, contains => [2,3,4,5] }, reporter_accessible => { value => 0, contains => [2,3,4,5] }, 'longdescs.count' => { value => 2, contains => [2,3,4,5] }, - 'longdescs.isprivate' => { value => 0, contains => [2,3,4,5] }, + 'longdescs.isprivate' => { value => -1, contains => [] }, everconfirmed => { value => 0, contains => [2,3,4,5] }, blocked => { contains => [1,2] }, dependson => { contains => [1,3] }, @@ -932,7 +908,9 @@ use constant TESTS => { override => { dependson => { value => '<1-id> <3-id>', contains => [] }, # bug 3 has the value "21" here, so matches "2,1" - percentage_complete => { value => '<2>,<3>', contains => [3] } + percentage_complete => { value => '<2>,<3>', contains => [3] }, + # 1 0 matches bug 1, which has both public and private comments. + 'longdescs.isprivate' => { contains => [1] }, } }, ], @@ -966,7 +944,9 @@ use constant TESTS => { override => { MULTI_BOOLEAN_OVERRIDE } }, { contains => [], value => '<1> <2>', override => { - dependson => { contains => [], value => '<2-id> <3-id>' } + dependson => { contains => [], value => '<2-id> <3-id>' }, + # 1 0 matches bug 1, which has both public and private comments. + 'longdescs.isprivate' => { contains => [1] }, } }, ], |