From 2b8db9971cd029465b994ba4da5e4a896c206035 Mon Sep 17 00:00:00 2001
From: Max Kanat-Alexander <mkanat@bugzilla.org>
Date: Wed, 15 Dec 2010 14:06:01 -0800
Subject: [PATCH] Bug 619466: Make searching by work_time search the total time
on the bug instead of searching the time on individual comments. r=mkanat,
a=mkanat (module owner)
---
Bugzilla/Search.pm | 7 +--
xt/lib/Bugzilla/Test/Search.pm | 2 +-
xt/lib/Bugzilla/Test/Search/Constants.pm | 66 ++++--------------------
xt/lib/Bugzilla/Test/Search/FieldTest.pm | 3 ++
4 files changed, 16 insertions(+), 62 deletions(-)
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index 7dc0f2c00..2220abf1e 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -2323,11 +2323,8 @@ sub _work_time_changedbefore_after {
sub _work_time {
my ($self, $args) = @_;
- my ($chart_id, $joins) = @$args{qw(chart_id joins)};
-
- my $table = "longdescs_$chart_id";
- push(@$joins, { table => 'longdescs', as => $table });
- $args->{full_field} = "$table.work_time";
+ $self->_add_extra_column('actual_time');
+ $args->{full_field} = COLUMNS->{actual_time}->{name};
}
sub _percentage_complete {
diff --git a/xt/lib/Bugzilla/Test/Search.pm b/xt/lib/Bugzilla/Test/Search.pm
index 42882bea9..857af6cb3 100644
--- a/xt/lib/Bugzilla/Test/Search.pm
+++ b/xt/lib/Bugzilla/Test/Search.pm
@@ -617,7 +617,7 @@ sub _create_one_bug {
my $extra_values = $self->_extra_bug_create_values->{$number};
foreach my $field (qw(comments remaining_time percentage_complete
keyword_objects everconfirmed dependson blocked
- groups_in classification))
+ groups_in classification actual_time))
{
$extra_values->{$field} = $bug->$field;
}
diff --git a/xt/lib/Bugzilla/Test/Search/Constants.pm b/xt/lib/Bugzilla/Test/Search/Constants.pm
index 54a4464a4..ef0220ed1 100644
--- a/xt/lib/Bugzilla/Test/Search/Constants.pm
+++ b/xt/lib/Bugzilla/Test/Search/Constants.pm
@@ -108,7 +108,6 @@ use constant COLUMN_TRANSLATION => {
# Make comment field names to their Bugzilla::Comment accessor.
use constant COMMENT_FIELDS => {
longdesc => 'body',
- work_time => 'work_time',
commenter => 'author',
'longdescs.isprivate' => 'is_private',
};
@@ -234,7 +233,6 @@ use constant NEGATIVE_BROKEN => (
dependson => { contains => [2,4,5] },
longdesc => { contains => [1] },
'longdescs.isprivate' => { contains => [1] },
- work_time => { contains => [1] },
# Custom fields are busted because they can be NULL.
FIELD_TYPE_FREETEXT, { contains => [5] },
FIELD_TYPE_BUG_ID, { contains => [5] },
@@ -262,15 +260,14 @@ use constant GREATERTHAN_BROKEN => (
# allwords and allwordssubstr have these broken tests in common.
#
-# allwordssubstr work_time only matches against a single comment,
+# allwordssubstr on longdescs fields matches against a single comment,
# instead of matching against all comments on a bug. Same is true
-# for the other longdesc fields, cc, keywords, and bug_group.
+# for cc, keywords, and bug_group.
use constant ALLWORDS_BROKEN => (
bug_group => { contains => [1] },
cc => { contains => [1] },
keywords => { contains => [1] },
longdesc => { contains => [1] },
- work_time => { contains => [1] },
);
# nowords and nowordssubstr have these broken tests in common.
@@ -279,14 +276,13 @@ use constant ALLWORDS_BROKEN => (
# 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
+# 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] },
bug_group => { contains => [5] },
longdesc => {},
- work_time => {},
'longdescs.isprivate' => {},
);
@@ -342,22 +338,16 @@ use constant KNOWN_BROKEN => {
notsubstring => { NEGATIVE_BROKEN },
notregexp => { NEGATIVE_BROKEN },
- # percentage_complete doesn't match bugs with 0 hours worked or remaining.
- #
# longdescs.isprivate matches if any comment matches, instead of if all
- # comments match. Same for longdescs and work_time. (Commenter is probably
+ # comments match. (Commenter is probably
# also broken in this way, but all our comments come from the same user.)
- # Also, the attachments ones don't find bugs that have no attachments
- # at all (which might be OK?).
lessthan => {
'longdescs.isprivate' => { contains => [1] },
- work_time => { contains => [1,2,3,4] },
},
# 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] },
- work_time => { contains => [2,3,4] },
},
greaterthan => { GREATERTHAN_BROKEN },
@@ -380,16 +370,6 @@ use constant KNOWN_BROKEN => {
NOWORDS_BROKEN,
},
- # anywords searches don't work on decimal values.
- # attach_data doesn't work (perhaps because it's the entire
- # data, or some problem with the regex?).
- anywords => {
- work_time => { contains => [1] },
- },
- 'anywords-<1> <2>' => {
- work_time => { contains => [1,2] },
- },
-
# setters.login_name and requestees.login name aren't tracked individually
# in bugs_activity, so can't be searched using this method.
#
@@ -432,8 +412,8 @@ use constant KNOWN_BROKEN => {
work_time => { contains => [1] },
FIELD_TYPE_BUG_ID, { contains => [5] },
},
- # changeto doesn't find work_time changes (probably due to decimal/string
- # stuff). Same for remaining_time and estimated_time.
+ # changeto doesn't find remaining_time changes (possibly due to us not
+ # tracking that data properly).
#
# multi-valued fields are stored as comma-separated strings, so you
# can't do changedfrom/to on them.
@@ -507,7 +487,6 @@ use constant CHANGED_BROKEN_NOT => (
percentage_complete => { contains => [1] },
"requestees.login_name" => { contains => [1] },
"setters.login_name" => { contains => [1] },
- "work_time" => { contains => [1] },
);
# For changedfrom and changedto.
@@ -539,7 +518,6 @@ use constant BROKEN_NOT => {
keywords => { contains => [1,5] },
longdesc => { contains => [1] },
'see_also' => { },
- work_time => { contains => [1] },
FIELD_TYPE_MULTI_SELECT, { },
},
'allwords-<1> <2>' => {
@@ -550,7 +528,6 @@ use constant BROKEN_NOT => {
'keywords' => { contains => [5] },
'longdesc' => { },
'longdescs.isprivate' => { },
- work_time => { },
},
allwordssubstr => {
COMMON_BROKEN_NOT,
@@ -559,7 +536,6 @@ use constant BROKEN_NOT => {
keywords => { contains => [1,5] },
longdesc => { contains => [1] },
see_also => { },
- work_time => { contains => [1] },
FIELD_TYPE_MULTI_SELECT, { },
},
'allwordssubstr-<1>,<2>' => {
@@ -570,13 +546,11 @@ use constant BROKEN_NOT => {
"longdesc" => { },
"longdescs.isprivate" => { },
"see_also" => { },
- "work_time" => { },
},
anyexact => {
COMMON_BROKEN_NOT,
"flagtypes.name" => { contains => [1, 2, 5] },
"longdesc" => { contains => [1, 2] },
- "work_time" => { contains => [1, 2] }
},
'anyexact-<1>, <2>' => {
bug_group => { contains => [1] },
@@ -586,17 +560,13 @@ use constant BROKEN_NOT => {
},
anywords => {
COMMON_BROKEN_NOT,
- "work_time" => { contains => [1, 2] },
- "work_time" => { contains => [1] },
FIELD_TYPE_MULTI_SELECT, { contains => [5] },
},
'anywords-<1> <2>' => {
'attach_data.thedata' => { contains => [5] },
- work_time => { contains => [1,2] },
},
anywordssubstr => {
COMMON_BROKEN_NOT,
- "work_time" => { contains => [1, 2] },
},
'anywordssubstr-<1> <2>' => {
FIELD_TYPE_MULTI_SELECT, { contains => [5] },
@@ -606,7 +576,6 @@ use constant BROKEN_NOT => {
bug_group => { contains => [1] },
keywords => { contains => [1,5] },
longdesc => { contains => [1] },
- work_time => { contains => [1] } ,
FIELD_TYPE_MULTI_SELECT, { contains => [1,5] },
},
'casesubstring-<1>-lc' => {
@@ -626,11 +595,11 @@ use constant BROKEN_NOT => {
},
changedbefore=> {
CHANGED_BROKEN_NOT,
- work_time => { }
},
changedby => {
CHANGED_BROKEN_NOT,
creation_ts => { contains => [1] },
+ work_time => { contains => [1] },
},
changedfrom => {
CHANGED_BROKEN_NOT,
@@ -639,13 +608,13 @@ use constant BROKEN_NOT => {
blocked => { contains => [1, 2] },
dependson => { contains => [1, 3] },
longdesc => { },
+ work_time => { contains => [1] },
FIELD_TYPE_BUG_ID, { contains => [1 .. 4] },
},
changedto => {
CHANGED_BROKEN_NOT,
CHANGED_FROM_TO_BROKEN_NOT,
- work_time => { },
longdesc => { contains => [1] },
"remaining_time" => { contains => [1] },
},
@@ -655,19 +624,16 @@ use constant BROKEN_NOT => {
"flagtypes.name" => { contains => [1, 5] },
keywords => { contains => [1,5] },
longdesc => { contains => [1] },
- work_time => { contains => [1] }
},
greaterthan => {
COMMON_BROKEN_NOT,
cc => { contains => [1] },
- work_time => { contains => [2, 3, 4] },
FIELD_TYPE_MULTI_SELECT, { contains => [5] },
},
greaterthaneq => {
COMMON_BROKEN_NOT,
cc => { contains => [1] },
"flagtypes.name" => { contains => [2, 5] },
- "work_time" => { contains => [2, 3, 4] },
FIELD_TYPE_MULTI_SELECT, { contains => [5] },
},
lessthan => {
@@ -691,14 +657,12 @@ use constant BROKEN_NOT => {
notsubstring => { NEGATIVE_BROKEN_NOT },
nowords => {
NEGATIVE_BROKEN_NOT,
- "work_time" => { contains => [2, 3, 4] },
"flagtypes.name" => { },
},
nowordssubstr => {
NEGATIVE_BROKEN_NOT,
"attach_data.thedata" => { },
"flagtypes.name" => { },
- "work_time" => { contains => [2, 3, 4] },
},
regexp => {
COMMON_BROKEN_NOT,
@@ -706,7 +670,6 @@ use constant BROKEN_NOT => {
"flagtypes.name" => { contains => [1,5] },
keywords => { contains => [1,5] },
longdesc => { contains => [1] },
- work_time => { contains => [1] },
},
'regexp-^1-' => {
"flagtypes.name" => { contains => [5] },
@@ -716,7 +679,6 @@ use constant BROKEN_NOT => {
bug_group => { contains => [1] },
keywords => { contains => [1,5] },
longdesc => { contains => [1] },
- work_time => { contains => [1] },
},
};
@@ -1065,10 +1027,7 @@ use constant TESTS => {
# show up.
'longdescs.isprivate' => { contains => [] },
percentage_complete => { contains => [4,5] },
- # 1.0 0.0 exludes bug 5.
- # XXX However, it also shouldn't match 2, 3, or 4, because
- # they contain at least one comment with 0.0 work_time.
- work_time => { contains => [2,3,4] },
+ work_time => { contains => [2,3,4,5] },
}
},
],
@@ -1076,7 +1035,6 @@ use constant TESTS => {
{ contains => [1], value => '<1>',
override => {
MULTI_BOOLEAN_OVERRIDE,
- work_time => { value => '1.0', contains => [1] },
}
},
{ contains => [1,2], value => '<1> <2>',
@@ -1084,7 +1042,6 @@ use constant TESTS => {
MULTI_BOOLEAN_OVERRIDE,
dependson => { value => '<1> <3>', contains => [1,3] },
'longdescs.count' => { contains => [1,2,3,4] },
- work_time => { value => '1.0 2.0' },
},
},
],
@@ -1103,10 +1060,7 @@ use constant TESTS => {
# longdescs.isprivate translates to "1 0", so no bugs should
# show up.
'longdescs.isprivate' => { contains => [] },
- # 1.0 0.0 exludes bug 5.
- # XXX However, it also shouldn't match 2, 3, or 4, because
- # they contain at least one comment with 0.0 work_time.
- work_time => { contains => [2,3,4] },
+ work_time => { contains => [2,3,4,5] },
}
},
],
diff --git a/xt/lib/Bugzilla/Test/Search/FieldTest.pm b/xt/lib/Bugzilla/Test/Search/FieldTest.pm
index fd3c7b6c7..56c0a57d6 100644
--- a/xt/lib/Bugzilla/Test/Search/FieldTest.pm
+++ b/xt/lib/Bugzilla/Test/Search/FieldTest.pm
@@ -335,6 +335,9 @@ sub _field_values_for_bug {
elsif ($field eq 'longdescs.count') {
@values = scalar(@{ $self->bug($number)->comments });
}
+ elsif ($field eq 'work_time') {
+ @values = $self->_values_for($number, 'actual_time');
+ }
elsif ($field eq 'bug_group') {
@values = $self->_values_for($number, 'groups_in', 'name');
}
--
2.24.1