diff options
| -rw-r--r-- | Bugzilla/Bug.pm | 5 | ||||
| -rw-r--r-- | Bugzilla/Search.pm | 56 | ||||
| -rw-r--r-- | xt/lib/Bugzilla/Test/Search/Constants.pm | 98 | ||||
| -rw-r--r-- | xt/lib/Bugzilla/Test/Search/FieldTest.pm | 9 |
4 files changed, 80 insertions, 88 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index fccf94a02..327e55ccc 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -3327,7 +3327,10 @@ sub percentage_complete { my $actual = $self->actual_time; my $total = $remaining + $actual; return undef if $total == 0; - return 100 * ($actual / $total); + # Search.pm truncates this value to an integer, so we want to as well, + # since this is mostly used in a test where its value needs to be + # identical to what the database will return. + return int(100 * ($actual / $total)); } sub product { diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index d887677b0..da6f57bf3 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -406,6 +406,11 @@ use constant COLUMN_DEPENDS => { # DB::Schema to figure out what needs to be joined, but for some # fields it needs a little help. use constant COLUMN_JOINS => { + actual_time => { + table => '(SELECT bug_id, SUM(work_time) AS total' + . ' FROM longdescs GROUP BY bug_id)', + join => 'INNER', + }, assigned_to => { from => 'assigned_to', to => 'userid', @@ -441,10 +446,6 @@ use constant COLUMN_JOINS => { to => 'id', join => 'INNER', }, - actual_time => { - table => 'longdescs', - join => 'INNER', - }, 'flagtypes.name' => { as => 'map_flags', table => 'flags', @@ -504,18 +505,20 @@ sub COLUMNS { # Next we define columns that have special SQL instead of just something # like "bugs.bug_id". - my $actual_time = '(SUM(map_actual_time.work_time)' - . ' * COUNT(DISTINCT map_actual_time.bug_when)/COUNT(bugs.bug_id))'; + my $total_time = "(map_actual_time.total + bugs.remaining_time)"; my %special_sql = ( deadline => $dbh->sql_date_format('bugs.deadline', '%Y-%m-%d'), - actual_time => $actual_time, + actual_time => 'map_actual_time.total', + # "FLOOR" is in there to turn this into an integer, making searches + # totally predictable. Otherwise you get floating-point numbers that + # are rather hard to search reliably if you're asking for exact + # numbers. percentage_complete => - "(CASE WHEN $actual_time + bugs.remaining_time = 0.0" - . " THEN 0.0" - . " ELSE 100" - . " * ($actual_time / ($actual_time + bugs.remaining_time))" - . " END)", + "(CASE WHEN $total_time = 0" + . " THEN 0" + . " ELSE FLOOR(100 * (map_actual_time.total / $total_time))" + . " END)", 'flagtypes.name' => $dbh->sql_group_concat('DISTINCT ' . $dbh->sql_string_concat('map_flagtypes.name', 'map_flags.status')), @@ -614,7 +617,6 @@ sub REPORT_COLUMNS { # is here because it *always* goes into the GROUP BY as the first item, # so it should be skipped when determining extra GROUP BY columns. use constant GROUP_BY_SKIP => EMPTY_COLUMN, qw( - actual_time bug_id flagtypes.name keywords @@ -2240,30 +2242,12 @@ sub _work_time { sub _percentage_complete { my ($self, $args) = @_; - my ($chart_id, $joins, $operator, $having, $fields) = - @$args{qw(chart_id joins operator having fields)}; - - my $table = "longdescs_$chart_id"; - - # We can't just use "percentage_complete" as the field, because - # (a) PostgreSQL doesn't accept it in the HAVING clause - # and (b) it wouldn't work in multiple chart rows, because it uses - # a fixed name for the table, "ldtime". - my $expression = COLUMNS->{percentage_complete}->{name}; - $expression =~ s/\bldtime\b/$table/g; - $args->{full_field} = "($expression)"; - push(@$joins, { table => 'longdescs', as => $table }); - - # We need remaining_time in _select_columns, otherwise we can't use - # it in the expression for creating percentage_complete. - $self->_add_extra_column('remaining_time'); + + $args->{full_field} = COLUMNS->{percentage_complete}->{name}; - $self->_do_operator_function($args); - push(@$having, $args->{term}); - - # We put something into $args->{term} so that do_search_function - # stops processing. - $args->{term} = ''; + # We need actual_time in _select_columns, otherwise we can't use + # it in the expression for searching percentage_complete. + $self->_add_extra_column('actual_time'); } sub _bug_group_nonchanged { diff --git a/xt/lib/Bugzilla/Test/Search/Constants.pm b/xt/lib/Bugzilla/Test/Search/Constants.pm index aeb392c70..c0dfe30c8 100644 --- a/xt/lib/Bugzilla/Test/Search/Constants.pm +++ b/xt/lib/Bugzilla/Test/Search/Constants.pm @@ -140,7 +140,6 @@ use constant SKIP_FIELDS => qw( # right in OR tests, and it's too much work to document the exact tests # that they cause to fail. use constant OR_SKIP => qw( - percentage_complete flagtypes.name ); @@ -167,7 +166,7 @@ use constant FIELD_SUBSTR_SIZE => { deadline => -5, creation_ts => -8, delta_ts => -8, - percentage_complete => 7, + percentage_complete => 1, work_time => 3, remaining_time => 3, target_milestone => 15, @@ -361,19 +360,8 @@ use constant KNOWN_BROKEN => { work_time => { contains => [2,3,4] }, }, - greaterthan => { GREATERTHAN_BROKEN }, - - # percentage_complete is broken -- it won't match equal values. - greaterthaneq => { - GREATERTHAN_BROKEN, - percentage_complete => { contains => [2] }, - }, - - # percentage_complete doesn't do a numeric comparison, so - # it doesn't find decimal values. - anyexact => { - percentage_complete => { contains => [2] }, - }, + greaterthan => { GREATERTHAN_BROKEN }, + greaterthaneq => { GREATERTHAN_BROKEN }, 'allwordssubstr-<1>' => { ALLWORDS_BROKEN }, # flagtypes.name does not work here, probably because they all try to @@ -399,7 +387,6 @@ use constant KNOWN_BROKEN => { work_time => { contains => [1] }, }, 'anywords-<1> <2>' => { - percentage_complete => { contains => [2] }, work_time => { contains => [1,2] }, }, @@ -481,16 +468,6 @@ use constant PG_BROKEN => { notregexp => { contains => [5] }, nowords => { contains => [5] }, }, - percentage_complete => { - 'allwordssubstr-<1>' => { contains => [3] }, - anywordssubstr => { contains => [2,3] }, - casesubstring => { contains => [3] }, - 'notregexp-<1>' => { contains => [3] }, - notsubstring => { contains => [3] }, - nowordssubstr => { contains => [3] }, - 'regexp-<1>' => { contains => [3] }, - substring => { contains => [3] }, - }, }; ################### @@ -512,7 +489,6 @@ use constant COMMON_BROKEN_NOT => ( "flagtypes.name" => { contains => [5] }, "keywords" => { contains => [5] }, "longdescs.isprivate" => { contains => [1] }, - "percentage_complete" => { contains => [1 .. 5] }, "see_also" => { contains => [5] }, FIELD_TYPE_BUG_ID, { contains => [5] }, FIELD_TYPE_DATETIME, { contains => [5] }, @@ -527,7 +503,7 @@ use constant CHANGED_BROKEN_NOT => ( "classification" => { contains => [1] }, "commenter" => { contains => [1] }, "delta_ts" => { contains => [1] }, - "percentage_complete" => { contains => [1] }, + percentage_complete => { contains => [1] }, "requestees.login_name" => { contains => [1] }, "setters.login_name" => { contains => [1] }, "work_time" => { contains => [1] }, @@ -549,7 +525,6 @@ use constant NEGATIVE_BROKEN_NOT => ( "bug_group" => { contains => [5] }, "dependson" => { contains => [2, 4, 5] }, "flagtypes.name" => { contains => [1 .. 5] }, - "percentage_complete" => { contains => [1 .. 5] }, ); # These are field/operator combinations that are broken when run under NOT(). @@ -603,7 +578,6 @@ use constant BROKEN_NOT => { }, 'anyexact-<1>, <2>' => { bug_group => { contains => [1] }, - percentage_complete => { contains => [1,3,4,5] }, keywords => { contains => [1,5] }, see_also => { }, FIELD_TYPE_MULTI_SELECT, { }, @@ -616,7 +590,6 @@ use constant BROKEN_NOT => { }, 'anywords-<1> <2>' => { 'attach_data.thedata' => { contains => [5] }, - "percentage_complete" => { contains => [1,3,4,5] }, work_time => { contains => [1,2] }, }, anywordssubstr => { @@ -624,7 +597,6 @@ use constant BROKEN_NOT => { "work_time" => { contains => [1, 2] }, }, 'anywordssubstr-<1> <2>' => { - percentage_complete => { contains => [1,2,3,4,5] }, FIELD_TYPE_MULTI_SELECT, { contains => [5] }, }, casesubstring => { @@ -645,8 +617,8 @@ use constant BROKEN_NOT => { "attach_data.thedata" => { contains => [2, 3, 4] }, "classification" => { contains => [2, 3, 4] }, "commenter" => { contains => [2, 3, 4] }, + percentage_complete => { contains => [2, 3, 4] }, "delta_ts" => { contains => [2, 3, 4] }, - "percentage_complete" => { contains => [2, 3, 4] }, "requestees.login_name" => { contains => [2, 3, 4] }, "setters.login_name" => { contains => [2, 3, 4] }, }, @@ -693,7 +665,6 @@ use constant BROKEN_NOT => { cc => { contains => [1] }, "flagtypes.name" => { contains => [2, 5] }, "work_time" => { contains => [2, 3, 4] }, - percentage_complete => { contains => [1,3,4,5] },, FIELD_TYPE_MULTI_SELECT, { contains => [5] }, }, lessthan => { @@ -717,14 +688,14 @@ use constant BROKEN_NOT => { notsubstring => { NEGATIVE_BROKEN_NOT }, nowords => { NEGATIVE_BROKEN_NOT, - "work_time" => { contains => [2, 3, 4] }, + "work_time" => { contains => [2, 3, 4] }, "flagtypes.name" => { }, }, nowordssubstr => { NEGATIVE_BROKEN_NOT, "attach_data.thedata" => { }, "flagtypes.name" => { }, - "work_time" => { contains => [2, 3, 4] }, + "work_time" => { contains => [2, 3, 4] }, }, regexp => { COMMON_BROKEN_NOT, @@ -774,7 +745,7 @@ use constant REGEX_OVERRIDE => { remaining_time => { value => '^9.0' }, work_time => { value => '^1.0' }, longdesc => { value => '^1-' }, - percentage_complete => { value => '^10.0' }, + percentage_complete => { value => '^10' }, FIELD_TYPE_BUG_ID, { value => '^<1>$' }, FIELD_TYPE_DATETIME, { value => '^2037-03-01' } }; @@ -915,22 +886,46 @@ use constant TESTS => { { contains => [2,3,4,5], value => '<1>' }, ], substring => [ - { contains => [1], value => '<1>' }, + { contains => [1], value => '<1>', + override => { + percentage_complete => { contains => [1,2,3] }, + } + }, ], casesubstring => [ - { contains => [1], value => '<1>' }, + { contains => [1], value => '<1>', + override => { + percentage_complete => { contains => [1,2,3] }, + } + }, { contains => [], value => '<1>', transform => sub { lc($_[0]) }, - extra_name => 'lc', if_equal => { contains => [1] } }, + extra_name => 'lc', if_equal => { contains => [1] }, + override => { + percentage_complete => { contains => [1,2,3] }, + } + }, ], notsubstring => [ - { contains => [2,3,4,5], value => '<1>' }, + { contains => [2,3,4,5], value => '<1>', + override => { + percentage_complete => { contains => [4,5] }, + }, + } ], regexp => [ - { contains => [1], value => '<1>', escape => 1 }, + { contains => [1], value => '<1>', escape => 1, + override => { + percentage_complete => { value => '^10' }, + } + }, { contains => [1], value => '^1-', override => REGEX_OVERRIDE }, ], notregexp => [ - { contains => [2,3,4,5], value => '<1>', escape => 1 }, + { contains => [2,3,4,5], value => '<1>', escape => 1, + override => { + percentage_complete => { value => '^10' }, + } + }, { contains => [2,3,4,5], value => '^1-', override => REGEX_OVERRIDE }, ], lessthan => [ @@ -1031,14 +1026,26 @@ use constant TESTS => { ], anywordssubstr => [ { contains => [1,2], value => '<1> <2>', - override => { ANY_OVERRIDE } }, + override => { + ANY_OVERRIDE, + percentage_complete => { contains => [1,2,3] }, + } + }, ], allwordssubstr => [ { contains => [1], value => '<1>', - override => { MULTI_BOOLEAN_OVERRIDE } }, + override => { + MULTI_BOOLEAN_OVERRIDE, + # We search just the number "1" for percentage_complete, + # which matches a lot of bugs. + percentage_complete => { contains => [1,2,3] }, + }, + }, { contains => [], value => '<1>,<2>', 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] } } }, ], @@ -1048,6 +1055,7 @@ use constant TESTS => { # longdescs.isprivate translates to "1 0", so no bugs should # 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. diff --git a/xt/lib/Bugzilla/Test/Search/FieldTest.pm b/xt/lib/Bugzilla/Test/Search/FieldTest.pm index 3e7fd2521..532936e91 100644 --- a/xt/lib/Bugzilla/Test/Search/FieldTest.pm +++ b/xt/lib/Bugzilla/Test/Search/FieldTest.pm @@ -166,7 +166,9 @@ sub transformed_value_was_equal { sub bug_is_contained { my ($self, $number) = @_; my $contains = $self->test->{contains}; - if ($self->transformed_value_was_equal($number)) { + if ($self->transformed_value_was_equal($number) + and !$self->test->{override}->{$self->field}->{contains}) + { $contains = $self->test->{if_equal}->{contains}; } return grep($_ == $number, @$contains) ? 1 : 0; @@ -482,11 +484,6 @@ sub _substr_value { $substr_size += length($field); } my $string = substr($value, 0, $substr_size); - # Make percentage_complete substrings strings match integers uniquely, - # by searching for the full decimal number. - if ($field eq 'percentage_complete' and length($string) < $substr_size) { - $string .= ".000"; - } return $string; } return substr($value, $substr_size); |