diff options
| -rw-r--r-- | Bugzilla/Comment.pm | 5 | ||||
| -rw-r--r-- | Bugzilla/Field.pm | 2 | ||||
| -rw-r--r-- | Bugzilla/Search.pm | 44 | ||||
| -rw-r--r-- | template/en/default/global/field-descs.none.tmpl | 1 | ||||
| -rw-r--r-- | template/en/default/list/table.html.tmpl | 1 | ||||
| -rw-r--r-- | xt/lib/Bugzilla/Test/Search.pm | 9 | ||||
| -rw-r--r-- | xt/lib/Bugzilla/Test/Search/Constants.pm | 19 | ||||
| -rw-r--r-- | xt/lib/Bugzilla/Test/Search/FieldTest.pm | 5 | ||||
| -rw-r--r-- | xt/lib/Bugzilla/Test/Search/InjectionTest.pm | 6 |
9 files changed, 81 insertions, 11 deletions
diff --git a/Bugzilla/Comment.pm b/Bugzilla/Comment.pm index 074f28dd6..b4f94e213 100644 --- a/Bugzilla/Comment.pm +++ b/Bugzilla/Comment.pm @@ -56,7 +56,10 @@ use constant UPDATE_COLUMNS => qw( use constant DB_TABLE => 'longdescs'; use constant ID_FIELD => 'comment_id'; -use constant LIST_ORDER => 'bug_when'; +# In some rare cases, two comments can have identical timestamps. If +# this happens, we want to be sure that the comment added later shows up +# later in the sequence. +use constant LIST_ORDER => 'bug_when, comment_id'; use constant VALIDATORS => { extra_data => \&_check_extra_data, diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm index 41b90a644..1229d31cb 100644 --- a/Bugzilla/Field.pm +++ b/Bugzilla/Field.pm @@ -218,6 +218,8 @@ use constant DEFAULT_FIELDS => ( buglist => 1}, {name => 'longdesc', desc => 'Comment'}, {name => 'longdescs.isprivate', desc => 'Comment is private'}, + {name => 'longdescs.count', desc => 'Number of Comments', + buglist => 1}, {name => 'alias', desc => 'Alias', buglist => 1}, {name => 'everconfirmed', desc => 'Ever Confirmed'}, {name => 'reporter_accessible', desc => 'Reporter Accessible'}, diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index da6f57bf3..cc1354dcf 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -272,6 +272,14 @@ use constant OPERATOR_FIELD_OVERRIDE => { changedafter => \&_long_desc_changedbefore_after, _default => \&_long_desc, }, + 'longdescs.count' => { + changedby => \&_long_desc_changedby, + changedbefore => \&_long_desc_changedbefore_after, + changedafter => \&_long_desc_changedbefore_after, + changedfrom => \&_invalid_combination, + changedto => \&_invalid_combination, + _default => \&_long_descs_count, + }, 'longdescs.isprivate' => { _default => \&_longdescs_isprivate, }, @@ -466,6 +474,10 @@ use constant COLUMN_JOINS => { to => 'id', }, }, + 'longdescs.count' => { + table => 'longdescs', + join => 'INNER', + }, }; # This constant defines the columns that can be selected in a query @@ -524,6 +536,8 @@ sub COLUMNS { . $dbh->sql_string_concat('map_flagtypes.name', 'map_flags.status')), 'keywords' => $dbh->sql_group_concat('DISTINCT map_keyworddefs.name'), + + 'longdescs.count' => 'COUNT(DISTINCT map_longdescs_count.comment_id)', ); # Backward-compatibility for old field names. Goes new_name => old_name. @@ -620,6 +634,7 @@ use constant GROUP_BY_SKIP => EMPTY_COLUMN, qw( bug_id flagtypes.name keywords + longdescs.count percentage_complete ); @@ -2171,7 +2186,7 @@ sub _content_matches { COLUMNS->{'relevance'}->{name} = $select_term; } -sub _long_desc { +sub _join_longdescs { my ($self, $args) = @_; my ($chart_id, $joins) = @$args{qw(chart_id joins)}; @@ -2182,22 +2197,37 @@ sub _long_desc { 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_desc { + my ($self, $args) = @_; + my $table = $self->_join_longdescs($args); $args->{full_field} = "$table.thetext"; } -sub _longdescs_isprivate { +sub _long_descs_count { 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 $table = "longdescs_count_$chart_id"; + my $extra = $self->_user->is_insider ? "" : "WHERE isprivate = 0"; my $join = { - table => 'longdescs', + table => "(SELECT bug_id, COUNT(*) AS num" + . " FROM longdescs $extra GROUP BY bug_id)", as => $table, - extra => $extra, }; push(@$joins, $join); + $args->{full_field} = "${table}.num"; +} + +sub _longdescs_isprivate { + my ($self, $args) = @_; + my $table = $self->_join_longdescs($args); $args->{full_field} = "$table.isprivate"; } diff --git a/template/en/default/global/field-descs.none.tmpl b/template/en/default/global/field-descs.none.tmpl index 741eecf83..efcce6c64 100644 --- a/template/en/default/global/field-descs.none.tmpl +++ b/template/en/default/global/field-descs.none.tmpl @@ -133,6 +133,7 @@ "flagtypes.name" => "Flags", "keywords" => "Keywords", "longdesc" => "Comment", + "longdescs.count" => "Number of Comments", "longdescs.isprivate" => "Comment is private", "newcc" => "CC", "op_sys" => "OS", diff --git a/template/en/default/list/table.html.tmpl b/template/en/default/list/table.html.tmpl index bc2dadd98..ffde712fe 100644 --- a/template/en/default/list/table.html.tmpl +++ b/template/en/default/list/table.html.tmpl @@ -66,6 +66,7 @@ "op_sys" => { maxlength => 4 } , "bug_file_loc" => { maxlength => 30 } , "target_milestone" => { title => "TargetM" } , + "longdescs.count" => { title => "# Comments" }, "percentage_complete" => { format_value => "%d %%" } , } %] diff --git a/xt/lib/Bugzilla/Test/Search.pm b/xt/lib/Bugzilla/Test/Search.pm index 02ff48a80..73bb86dfd 100644 --- a/xt/lib/Bugzilla/Test/Search.pm +++ b/xt/lib/Bugzilla/Test/Search.pm @@ -667,6 +667,13 @@ sub _create_one_bug { undef, $bug->id); } + # Bug 1 gets three comments, so that longdescs.count matches it + # uniquely. The third comment is added in the middle, so that the + # last comment contains all of the important data, like work_time. + if ($number == 1) { + $bug->add_comment("1-comment-" . random(100)); + } + my %update_params = %{ $self->_bug_update_values->{$number} }; my %reverse_map = reverse %{ Bugzilla::Bug->FIELD_MAP }; foreach my $db_name (keys %reverse_map) { @@ -712,7 +719,7 @@ sub _create_one_bug { $update_params{reporter_accessible} = $number == 1 ? 1 : 0; $update_params{cclist_accessible} = $number == 1 ? 1 : 0; $update_params{alias} = $update_alias; - + $bug->set_all(\%update_params); my $flags = $self->bug_create_value($number, 'set_flags')->{b}; $bug->set_flags([], $flags); diff --git a/xt/lib/Bugzilla/Test/Search/Constants.pm b/xt/lib/Bugzilla/Test/Search/Constants.pm index c0dfe30c8..de96ba1d2 100644 --- a/xt/lib/Bugzilla/Test/Search/Constants.pm +++ b/xt/lib/Bugzilla/Test/Search/Constants.pm @@ -309,6 +309,7 @@ use constant CHANGED_VALUE_BROKEN => ( estimated_time => { contains => [1] }, 'flagtypes.name' => { contains => [1] }, keywords => { contains => [1] }, + 'longdescs.count' => { search => 1 }, work_time => { contains => [1] }, FIELD_TYPE_MULTI_SELECT, { contains => [1] }, ); @@ -511,6 +512,7 @@ use constant CHANGED_BROKEN_NOT => ( # For changedfrom and changedto. use constant CHANGED_FROM_TO_BROKEN_NOT => ( + 'longdescs.count' => { search => 1 }, "bug_group" => { contains => [1] }, "cc" => { contains => [1] }, "cf_multi_select" => { contains => [1] }, @@ -737,6 +739,7 @@ use constant REGEX_OVERRIDE => { cclist_accessible => { value => '^1' }, reporter_accessible => { value => '^1' }, everconfirmed => { value => '^1' }, + 'longdescs.count' => { value => '^3' }, 'longdescs.isprivate' => { value => '^1' }, creation_ts => { value => '^2037-01-01' }, delta_ts => { value => '^2037-01-01' }, @@ -808,6 +811,7 @@ use constant NEGATIVE_MULTI_BOOLEAN_OVERRIDE => ( # For anyexact and anywordssubstr use constant ANY_OVERRIDE => ( + 'longdescs.count' => { contains => [1,2,3,4] }, 'work_time' => { value => '1.0,2.0' }, dependson => { value => '<1>,<3>', contains => [1,3] }, MULTI_BOOLEAN_OVERRIDE, @@ -944,6 +948,7 @@ use constant TESTS => { '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.count' => { value => 3, 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] }, @@ -967,6 +972,7 @@ use constant TESTS => { '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.count' => { value => 2, contains => [2,3,4,5] }, 'longdescs.isprivate' => { value => 0, contains => [2,3,4,5] }, everconfirmed => { value => 0, contains => [2,3,4,5] }, blocked => { contains => [1,2] }, @@ -991,6 +997,7 @@ use constant TESTS => { 'attachments.isprivate' => { value => 0, contains => [1] }, cclist_accessible => { value => 0, contains => [1] }, reporter_accessible => { value => 0, contains => [1] }, + 'longdescs.count' => { value => 2, contains => [1] }, 'longdescs.isprivate' => { value => 0, contains => [1] }, everconfirmed => { value => 0, contains => [1] }, 'flagtypes.name' => { value => 2, contains => [2,3,4] }, @@ -1006,6 +1013,7 @@ use constant TESTS => { 'attachments.isprivate' => { value => 1, contains => [1] }, cclist_accessible => { value => 1, contains => [1] }, reporter_accessible => { value => 1, contains => [1] }, + 'longdescs.count' => { value => 3, contains => [1] }, 'longdescs.isprivate' => { value => 1, contains => [1] }, everconfirmed => { value => 1, contains => [1] }, dependson => { value => '<3>', contains => [1,3] }, @@ -1074,6 +1082,7 @@ use constant TESTS => { override => { MULTI_BOOLEAN_OVERRIDE, dependson => { value => '<1> <3>', contains => [1,3] }, + 'longdescs.count' => { contains => [1,2,3,4] }, work_time => { value => '1.0 2.0' }, }, }, @@ -1109,6 +1118,7 @@ use constant TESTS => { blocked => { contains => [1,2] }, dependson => { contains => [1,3] }, longdesc => { contains => [1,5] }, + 'longdescs.count' => { contains => [1,5] }, } }, ], @@ -1195,6 +1205,15 @@ use constant INJECTION_BROKEN_FIELD => { FIELD_TYPE_BUG_ID, { db_skip => ['Pg'] }, FIELD_TYPE_DATETIME, { db_skip => ['Pg'] }, owner_idle_time => { search => 1 }, + 'longdescs.count' => { + search => 1, + db_skip => ['Pg'], + operator_ok => [qw(allwordssubstr anywordssubstr casesubstring + changedbefore changedafter greaterthan greaterthaneq + lessthan lessthaneq notregexp notsubstring + nowordssubstr regexp substring anywords + notequals nowords)], + }, keywords => { search => 1, operator_ok => [qw(allwordssubstr anywordssubstr casesubstring diff --git a/xt/lib/Bugzilla/Test/Search/FieldTest.pm b/xt/lib/Bugzilla/Test/Search/FieldTest.pm index 532936e91..fd3c7b6c7 100644 --- a/xt/lib/Bugzilla/Test/Search/FieldTest.pm +++ b/xt/lib/Bugzilla/Test/Search/FieldTest.pm @@ -332,6 +332,9 @@ sub _field_values_for_bug { # searches use the last comment. @values = reverse @values; } + elsif ($field eq 'longdescs.count') { + @values = scalar(@{ $self->bug($number)->comments }); + } elsif ($field eq 'bug_group') { @values = $self->_values_for($number, 'groups_in', 'name'); } @@ -515,7 +518,7 @@ sub do_tests { my $search_broken = $self->search_known_broken; my $search = $self->_test_search_object_creation(); - + my $sql; TODO: { local $TODO = $search_broken if $search_broken; diff --git a/xt/lib/Bugzilla/Test/Search/InjectionTest.pm b/xt/lib/Bugzilla/Test/Search/InjectionTest.pm index 1bd9fd82c..41f5fcdc2 100644 --- a/xt/lib/Bugzilla/Test/Search/InjectionTest.pm +++ b/xt/lib/Bugzilla/Test/Search/InjectionTest.pm @@ -54,7 +54,11 @@ sub sql_error_ok { return $_[0]->_known_broken->{sql_error} } # Injection tests only skip fields on certain dbs. sub field_not_yet_implemented { my ($self) = @_; - my $skip_for_dbs = $self->_known_broken->{db_skip}; + # We use the constant directly because we don't want operator_ok + # or field_ok to stop us. + my $broken = INJECTION_BROKEN_FIELD->{$self->field} + || INJECTION_BROKEN_FIELD->{$self->field_object->type}; + my $skip_for_dbs = $broken->{db_skip}; return undef if !$skip_for_dbs; my $dbh = Bugzilla->dbh; if (my ($skip) = grep { $dbh->isa("Bugzilla::DB::$_") } @$skip_for_dbs) { |