summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMax Kanat-Alexander <mkanat@bugzilla.org>2010-10-05 10:39:01 +0200
committerMax Kanat-Alexander <mkanat@bugzilla.org>2010-10-05 10:39:01 +0200
commit7307c8c748e3245d65a25c016e7d92c6c7ae2aa4 (patch)
tree97fa8ea6b7482fad71cecc5195c2a81e6cc866c8
parentdaa533e7c6d1c6ff2e8600c5178ac75bf7a2538c (diff)
downloadbugzilla-7307c8c748e3245d65a25c016e7d92c6c7ae2aa4.tar.gz
bugzilla-7307c8c748e3245d65a25c016e7d92c6c7ae2aa4.tar.xz
Bug 26074 - Ability to limit search by number of Comments
r=mkanat, a=mkanat (module owner)
-rw-r--r--Bugzilla/Comment.pm5
-rw-r--r--Bugzilla/Field.pm2
-rw-r--r--Bugzilla/Search.pm44
-rw-r--r--template/en/default/global/field-descs.none.tmpl1
-rw-r--r--template/en/default/list/table.html.tmpl1
-rw-r--r--xt/lib/Bugzilla/Test/Search.pm9
-rw-r--r--xt/lib/Bugzilla/Test/Search/Constants.pm19
-rw-r--r--xt/lib/Bugzilla/Test/Search/FieldTest.pm5
-rw-r--r--xt/lib/Bugzilla/Test/Search/InjectionTest.pm6
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) {