diff options
| -rw-r--r-- | Bugzilla/DB.pm | 1 | ||||
| -rw-r--r-- | Bugzilla/DB/Oracle.pm | 1 | ||||
| -rw-r--r-- | Bugzilla/Search.pm | 274 | ||||
| -rwxr-xr-x | buglist.cgi | 192 | ||||
| -rwxr-xr-x | collectstats.pl | 2 | ||||
| -rwxr-xr-x | duplicates.cgi | 16 | ||||
| -rwxr-xr-x | report.cgi | 65 | ||||
| -rw-r--r-- | template/en/default/list/table.html.tmpl | 28 | ||||
| -rwxr-xr-x | whine.pl | 20 |
9 files changed, 305 insertions, 294 deletions
diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm index f7d00f3b3..4bf064375 100644 --- a/Bugzilla/DB.pm +++ b/Bugzilla/DB.pm @@ -52,7 +52,6 @@ use Storable qw(dclone); use constant BLOB_TYPE => DBI::SQL_BLOB; use constant ISOLATION_LEVEL => 'REPEATABLE READ'; -use constant GROUPBY_REGEXP => '(?:.*\s+AS\s+|SUBSTRING\()?(\w+(\.\w+)?)(?:\s+(ASC|DESC|FROM\s+.+))?$'; # Set default values for what used to be the enum types. These values # are no longer stored in localconfig. If we are upgrading from a diff --git a/Bugzilla/DB/Oracle.pm b/Bugzilla/DB/Oracle.pm index 23b97a0d2..a2c78e094 100644 --- a/Bugzilla/DB/Oracle.pm +++ b/Bugzilla/DB/Oracle.pm @@ -52,7 +52,6 @@ use base qw(Bugzilla::DB); use constant EMPTY_STRING => '__BZ_EMPTY_STR__'; use constant ISOLATION_LEVEL => 'READ COMMITTED'; use constant BLOB_TYPE => { ora_type => ORA_BLOB }; -use constant GROUPBY_REGEXP => '((CASE\s+WHEN.+END)|(SUBSTR.+\))|(TO_CHAR\(.+\))|(\(SCORE.+\))|(\(MATCH.+\))|(\w+(\.\w+)?))(\s+AS\s+)?(.*)?$'; sub new { my ($class, $user, $pass, $host, $dbname, $port) = @_; diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 0c0a76562..c606b774d 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -33,7 +33,13 @@ use strict; package Bugzilla::Search; use base qw(Exporter); -@Bugzilla::Search::EXPORT = qw(IsValidQueryType); +@Bugzilla::Search::EXPORT = qw( + EMPTY_COLUMN + + IsValidQueryType + split_order_term + translate_old_column +); use Bugzilla::Error; use Bugzilla::Util; @@ -47,21 +53,126 @@ use Bugzilla::Keyword; use Date::Format; use Date::Parse; +# A SELECTed expression that we use as a placeholder if somebody selects +# <none> for the X, Y, or Z axis in report.cgi. +use constant EMPTY_COLUMN => '-1'; + # Some fields are not sorted on themselves, but on other fields. # We need to have a list of these fields and what they map to. # Each field points to an array that contains the fields mapped # to, in order. use constant SPECIAL_ORDER => { - 'bugs.target_milestone' => [ 'ms_order.sortkey','ms_order.value' ], + 'target_milestone' => [ 'ms_order.sortkey','ms_order.value' ], }; # When we add certain fields to the ORDER BY, we need to then add a # table join to the FROM statement. This hash maps input fields to # the join statements that need to be added. use constant SPECIAL_ORDER_JOIN => { - 'bugs.target_milestone' => 'LEFT JOIN milestones AS ms_order ON ms_order.value = bugs.target_milestone AND ms_order.product_id = bugs.product_id', + 'target_milestone' => 'LEFT JOIN milestones AS ms_order ON ms_order.value = bugs.target_milestone AND ms_order.product_id = bugs.product_id', }; +# This constant defines the columns that can be selected in a query +# and/or displayed in a bug list. Column records include the following +# fields: +# +# 1. id: a unique identifier by which the column is referred in code; +# +# 2. name: The name of the column in the database (may also be an expression +# that returns the value of the column); +# +# 3. title: The title of the column as displayed to users. +# +# Note: There are a few hacks in the code that deviate from these definitions. +# In particular, when the list is sorted by the "votes" field the word +# "DESC" is added to the end of the field to sort in descending order, +# and the redundant short_desc column is removed when the client +# requests "all" columns. +# +# This is really a constant--that is, once it's been called once, the value +# will always be the same unless somebody adds a new custom field. But +# we have to do a lot of work inside the subroutine to get the data, +# and we don't want it to happen at compile time, so we have it as a +# subroutine. +sub COLUMNS { + my $dbh = Bugzilla->dbh; + my $cache = Bugzilla->request_cache; + return $cache->{search_columns} if defined $cache->{search_columns}; + + # These are columns that don't exist in fielddefs, but are valid buglist + # columns. (Also see near the bottom of this function for the definition + # of short_short_desc.) + my %columns = ( + relevance => { title => 'Relevance' }, + assigned_to_realname => { title => 'Assignee' }, + reporter_realname => { title => 'Reporter' }, + qa_contact_realname => { title => 'QA Contact' }, + ); + + # Next we define columns that have special SQL instead of just something + # like "bugs.bug_id". + my $actual_time = '(SUM(ldtime.work_time)' + . ' * COUNT(DISTINCT ldtime.bug_when)/COUNT(bugs.bug_id))'; + my %special_sql = ( + deadline => $dbh->sql_date_format('bugs.deadline', '%Y-%m-%d'), + actual_time => $actual_time, + + percentage_complete => + "(CASE WHEN $actual_time + bugs.remaining_time = 0.0" + . " THEN 0.0" + . " ELSE 100" + . " * ($actual_time / ($actual_time + bugs.remaining_time))" + . " END)", + ); + + # Backward-compatibility for old field names. Goes new_name => old_name. + # These are here and not in translate_old_column because the rest of the + # code actually still uses the old names, while the fielddefs table uses + # the new names (which is not the case for the fields handled by + # translate_old_column). + my %old_names = ( + creation_ts => 'opendate', + delta_ts => 'changeddate', + work_time => 'actual_time', + ); + + # Fields that are email addresses + my @email_fields = qw(assigned_to reporter qa_contact); + # Other fields that are stored in the bugs table as an id, but + # should be displayed using their name. + my @id_fields = qw(product component classification); + + foreach my $col (@email_fields) { + my $sql = "map_${col}.login_name"; + if (!Bugzilla->user->id) { + $sql = $dbh->sql_string_until($sql, $dbh->quote('@')); + } + $special_sql{$col} = $sql; + $columns{"${col}_realname"}->{name} = "map_${col}.realname"; + } + + foreach my $col (@id_fields) { + $special_sql{$col} = "map_${col}s.name"; + } + + # Do the actual column-getting from fielddefs, now. + foreach my $field (Bugzilla->get_fields({ obsolete => 0, buglist => 1 })) { + my $id = $field->name; + $id = $old_names{$id} if exists $old_names{$id}; + my $sql = 'bugs.' . $field->name; + $sql = $special_sql{$id} if exists $special_sql{$id}; + $columns{$id} = { name => $sql, title => $field->description }; + } + + # The short_short_desc column is identical to short_desc + $columns{'short_short_desc'} = $columns{'short_desc'}; + + Bugzilla::Hook::process("buglist-columns", { columns => \%columns }); + + $cache->{search_columns} = \%columns; + return $cache->{search_columns}; +} + # Create a new Search # Note that the param argument may be modified by Bugzilla::Search sub new { @@ -78,22 +189,18 @@ sub new { sub init { my $self = shift; - my $fieldsref = $self->{'fields'}; + my @fields = @{ $self->{'fields'} || [] }; my $params = $self->{'params'}; $self->{'user'} ||= Bugzilla->user; my $user = $self->{'user'}; - my $orderref = $self->{'order'} || 0; - my @inputorder; - @inputorder = @$orderref if $orderref; + my @inputorder = @{ $self->{'order'} || [] }; my @orderby; - my @fields; my @supptables; my @wherepart; my @having; my @groupby; - @fields = @$fieldsref if $fieldsref; my @specialchart; my @andlist; my %chartfields; @@ -109,48 +216,56 @@ sub init { obsolete => 0 }); foreach my $field (@select_fields) { my $name = $field->name; - $special_order{"bugs.$name"} = [ "$name.sortkey", "$name.value" ], - $special_order_join{"bugs.$name"} = - "LEFT JOIN $name ON $name.value = bugs.$name"; + next if $name eq 'product'; # products don't have sortkeys. + $special_order{$name} = [ "$name.sortkey", "$name.value" ], + $special_order_join{$name} = + "LEFT JOIN $name ON $name.value = bugs.$name"; } my $dbh = Bugzilla->dbh; - + + # All items that are in the ORDER BY must be in the SELECT. + foreach my $orderitem (@inputorder) { + my $column_name = split_order_term($orderitem); + if (!grep($_ eq $column_name, @fields)) { + push(@fields, $column_name); + } + } + # First, deal with all the old hard-coded non-chart-based poop. - if (grep(/map_assigned_to/, @$fieldsref)) { + if (grep(/^assigned_to/, @fields)) { push @supptables, "INNER JOIN profiles AS map_assigned_to " . "ON bugs.assigned_to = map_assigned_to.userid"; } - if (grep(/map_reporter/, @$fieldsref)) { + if (grep(/^reporter/, @fields)) { push @supptables, "INNER JOIN profiles AS map_reporter " . "ON bugs.reporter = map_reporter.userid"; } - if (grep(/map_qa_contact/, @$fieldsref)) { + if (grep(/^qa_contact/, @fields)) { push @supptables, "LEFT JOIN profiles AS map_qa_contact " . "ON bugs.qa_contact = map_qa_contact.userid"; } - if (lsearch($fieldsref, 'map_products.name') >= 0) { + if (grep($_ eq 'product' || $_ eq 'classification', @fields)) + { push @supptables, "INNER JOIN products AS map_products " . "ON bugs.product_id = map_products.id"; } - if (lsearch($fieldsref, 'map_classifications.name') >= 0) { - push @supptables, "INNER JOIN products AS map_products " . - "ON bugs.product_id = map_products.id"; + if (grep($_ eq 'classification', @fields)) { push @supptables, "INNER JOIN classifications AS map_classifications " . "ON map_products.classification_id = map_classifications.id"; } - if (lsearch($fieldsref, 'map_components.name') >= 0) { + if (grep($_ eq 'component', @fields)) { push @supptables, "INNER JOIN components AS map_components " . "ON bugs.component_id = map_components.id"; } - if (grep($_ =~/AS (actual_time|percentage_complete)$/, @$fieldsref)) { + if (grep($_ eq 'actual_time' || $_ eq 'percentage_complete', @fields)) { push(@supptables, "LEFT JOIN longdescs AS ldtime " . "ON ldtime.bug_id = bugs.bug_id"); } @@ -758,13 +873,6 @@ sub init { # to other parts of the query, so we want to create it before we # write the FROM clause. foreach my $orderitem (@inputorder) { - # Some fields have 'AS' aliases. The aliases go in the ORDER BY, - # not the whole fields. - # XXX - Ideally, we would get just the aliases in @inputorder, - # and we'd never have to deal with this. - if ($orderitem =~ /\s+AS\s+(.+)$/i) { - $orderitem = $1; - } BuildOrderBy(\%special_order, $orderitem, \@orderby); } # Now JOIN the correct tables in the FROM clause. @@ -772,9 +880,9 @@ sub init { # cleaner to do it this way. foreach my $orderitem (@inputorder) { # Grab the part without ASC or DESC. - my @splitfield = split(/\s+/, $orderitem); - if ($special_order_join{$splitfield[0]}) { - push(@supptables, $special_order_join{$splitfield[0]}); + my $column_name = split_order_term($orderitem); + if ($special_order_join{$column_name}) { + push(@supptables, $special_order_join{$column_name}); } } @@ -803,7 +911,9 @@ sub init { # Make sure we create a legal SQL query. @andlist = ("1 = 1") if !@andlist; - my $query = "SELECT " . join(', ', @fields) . + my @sql_fields = map { $_ eq EMPTY_COLUMN ? EMPTY_COLUMN + : COLUMNS->{$_}->{name} . ' AS ' . $_ } @fields; + my $query = "SELECT " . join(', ', @sql_fields) . " FROM $suppstring" . " LEFT JOIN bug_group_map " . " ON bug_group_map.bug_id = bugs.bug_id "; @@ -830,17 +940,21 @@ sub init { } } - foreach my $field (@fields, @orderby) { - next if ($field =~ /(AVG|SUM|COUNT|MAX|MIN|VARIANCE)\s*\(/i || - $field =~ /^\d+$/ || $field eq "bugs.bug_id" || - $field =~ /^(relevance|actual_time|percentage_complete)/); - # The structure of fields is of the form: - # [foo AS] {bar | bar.baz} [ASC | DESC] - # Only the mandatory part bar OR bar.baz is of interest. - # But for Oracle, it needs the real name part instead. - my $regexp = $dbh->GROUPBY_REGEXP; - if ($field =~ /$regexp/i) { - push(@groupby, $1) if !grep($_ eq $1, @groupby); + # For some DBs, every field in the SELECT must be in the GROUP BY. + foreach my $field (@fields) { + # These fields never go into the GROUP BY (bug_id goes in + # explicitly, below). + next if (grep($_ eq $field, EMPTY_COLUMN, + qw(bug_id actual_time percentage_complete))); + my $col = COLUMNS->{$field}->{name}; + push(@groupby, $col) if !grep($_ eq $col, @groupby); + } + # And all items from ORDER BY must be in the GROUP BY. The above loop + # doesn't catch items that were put into the ORDER BY from SPECIAL_ORDER. + foreach my $item (@inputorder) { + my $column_name = split_order_term($item); + if ($special_order{$column_name}) { + push(@groupby, @{ $special_order{$column_name} }); } } $query .= ") " . $dbh->sql_group_by("bugs.bug_id", join(', ', @groupby)); @@ -1023,9 +1137,7 @@ sub IsValidQueryType sub BuildOrderBy { my ($special_order, $orderitem, $stringlist, $reverseorder) = (@_); - my @twopart = split(/\s+/, $orderitem); - my $orderfield = $twopart[0]; - my $orderdirection = $twopart[1] || ""; + my ($orderfield, $orderdirection) = split_order_term($orderitem); if ($reverseorder) { # If orderdirection is empty or ASC... @@ -1052,6 +1164,40 @@ sub BuildOrderBy { push(@$stringlist, trim($orderfield . ' ' . $orderdirection)); } +# Splits out "asc|desc" from a sort order item. +sub split_order_term { + my $fragment = shift; + $fragment =~ /^(.+?)(?:\s+(ASC|DESC))?$/i; + my ($column_name, $direction) = (lc($1), uc($2)); + $direction ||= ""; + return wantarray ? ($column_name, $direction) : $column_name; +} + +# Used to translate old SQL fragments from buglist.cgi's "order" argument +# into our modern field IDs. +sub translate_old_column { + my ($column) = @_; + # All old SQL fragments have a period in them somewhere. + return $column if $column !~ /\./; + + if ($column =~ /\bAS\s+(\w+)$/i) { + return $1; + } + # product, component, classification, assigned_to, qa_contact, reporter + elsif ($column =~ /map_(\w+?)s?\.(login_)?name/i) { + return $1; + } + + # If it doesn't match the regexps above, check to see if the old + # SQL fragment matches the SQL of an existing column + foreach my $key (%{ COLUMNS() }) { + next unless exists COLUMNS->{$key}->{name}; + return $key if COLUMNS->{$key}->{name} eq $column; + } + + return $column; +} + ##################################################################### # Search Functions ##################################################################### @@ -1276,9 +1422,9 @@ sub _content_matches { "ON bugs.bug_id = $table.bug_id"); # Create search terms to add to the SELECT and WHERE clauses. - my ($term1, $rterm1) = $dbh->sql_fulltext_search("$table.$comments_col", + my ($term1, $rterm1) = $dbh->sql_fulltext_search("$table.$comments_col", $$v, 1); - my ($term2, $rterm2) = $dbh->sql_fulltext_search("$table.short_desc", + my ($term2, $rterm2) = $dbh->sql_fulltext_search("$table.short_desc", $$v, 2); $rterm1 = $term1 if !$rterm1; $rterm2 = $term2 if !$rterm2; @@ -1287,20 +1433,18 @@ sub _content_matches { $$term = "$term1 > 0 OR $term2 > 0"; # In order to sort by relevance (in case the user requests it), - # we SELECT the relevance value and give it an alias so we can - # add it to the SORT BY clause when we build it in buglist.cgi. - my $select_term = "($rterm1 + $rterm2) AS relevance"; - - # Users can specify to display the relevance field, in which case - # it'll show up in the list of fields being selected, and we need - # to replace that occurrence with our select term. Otherwise - # we can just add the term to the list of fields being selected. - if (grep($_ eq "relevance", @$fields)) { - @$fields = map($_ eq "relevance" ? $select_term : $_ , @$fields); - } - else { - push(@$fields, $select_term); - } + # we SELECT the relevance value so we can add it to the ORDER BY + # clause. Every time a new fulltext chart isadded, this adds more + # terms to the relevance sql. (That doesn't make sense in + # "NOT" charts, but Bugzilla never uses those with fulltext + # by default.) + # + # We build the relevance SQL by modifying the COLUMNS list directly, + # which is kind of a hack but works. + my $current = COLUMNS->{'relevance'}->{name}; + $current = $current ? "$current + " : ''; + my $select_term = "($current$rterm1 + $rterm2)"; + COLUMNS->{'relevance'}->{name} = $select_term; } sub _timestamp_compare { @@ -1459,8 +1603,8 @@ sub _percentage_complete { } if ($oper ne "noop") { my $table = "longdescs_$$chartid"; - if(lsearch($fields, "bugs.remaining_time") == -1) { - push(@$fields, "bugs.remaining_time"); + if (!grep($_ eq 'remaining_time', @$fields)) { + push(@$fields, "remaining_time"); } push(@$supptables, "LEFT JOIN longdescs AS $table " . "ON $table.bug_id = bugs.bug_id"); diff --git a/buglist.cgi b/buglist.cgi index edee13bde..35d0c0291 100755 --- a/buglist.cgi +++ b/buglist.cgi @@ -640,92 +640,7 @@ if (!$params->param('query_format')) { # Column Definition ################################################################################ -# Define the columns that can be selected in a query and/or displayed in a bug -# list. Column records include the following fields: -# -# 1. ID: a unique identifier by which the column is referred in code; -# -# 2. Name: The name of the column in the database (may also be an expression -# that returns the value of the column); -# -# 3. Title: The title of the column as displayed to users. -# -# Note: There are a few hacks in the code that deviate from these definitions. -# In particular, when the list is sorted by the "votes" field the word -# "DESC" is added to the end of the field to sort in descending order, -# and the redundant short_desc column is removed when the client -# requests "all" columns. -# Note: For column names using aliasing (SQL "<field> AS <alias>"), the column -# ID needs to be identical to the field ID for list ordering to work. - -my $columns = { relevance => {name => 'relevance', title => 'Relevance'}, - short_short_desc => {name => 'bugs.short_desc', title => 'Summary'} }; - -foreach my $field (Bugzilla->get_fields({ obsolete => 0, buglist => 1 })) { - # Rename some field names for backward compatibility - my $id = $field->name; - if ($id eq 'creation_ts') { - $id = 'opendate'; - } - elsif ($id eq 'delta_ts') { - $id = 'changeddate'; - } - elsif ($id eq 'work_time') { - $id = 'actual_time'; - } - - # Database column names and expressions - # XXX Move these to fielddefs/Field.pm or Search.pm? - my $name = 'bugs.' . $field->name; - if ($id eq 'assigned_to' || $id eq 'reporter' || $id eq 'qa_contact') { - $name = 'map_' . $field->name . '.login_name'; - if (!Bugzilla->user->id) { - $name = $dbh->sql_string_until($name, $dbh->quote('@')); - } - } - elsif ($id eq 'product' || $id eq 'component' || $id eq 'classification') { - $name = 'map_' . $field->name . 's.name'; - } - elsif ($id eq 'deadline') { - $name = $dbh->sql_date_format('bugs.deadline', '%Y-%m-%d') . " AS deadline"; - } - elsif ($id eq 'actual_time') { - $name = '(SUM(ldtime.work_time)*COUNT(DISTINCT ldtime.bug_when)/COUNT(bugs.bug_id)) AS actual_time'; - } - elsif ($id eq 'percentage_complete') { - $name = - "(CASE WHEN (SUM(ldtime.work_time)*COUNT(DISTINCT ldtime.bug_when)/COUNT(bugs.bug_id)) " . - " + bugs.remaining_time = 0.0 " . - "THEN 0.0 " . - "ELSE 100*((SUM(ldtime.work_time)*COUNT(DISTINCT ldtime.bug_when)/COUNT(bugs.bug_id)) " . - " /((SUM(ldtime.work_time)*COUNT(DISTINCT ldtime.bug_when)/COUNT(bugs.bug_id)) + bugs.remaining_time)) " . - "END) AS percentage_complete" - } - - $columns->{$id} = { 'name' => $name, 'title' => $field->description }; -} - -foreach my $col (qw(assigned_to reporter qa_contact)) { - my $colname = "${col}_realname"; - if ($format->{'extension'} eq 'html') { - my $login = "map_${col}.login_name"; - if (!Bugzilla->user->id) { - $login = $dbh->sql_string_until($login, $dbh->quote('@')); - } - $columns->{$colname}->{name} = - "CASE WHEN map_${col}.realname = '' - THEN $login ELSE map_${col}.realname - END AS $colname"; - } - else { - $columns->{$colname}->{name} = "map_${col}.realname AS $colname"; - } -} -$columns->{assigned_to_realname}->{title} = "Assignee"; -$columns->{reporter_realname}->{title} = "Reporter"; -$columns->{qa_contact_realname}->{title} = "QA Contact"; - -Bugzilla::Hook::process("buglist-columns", {'columns' => $columns} ); +my $columns = Bugzilla::Search::COLUMNS; ################################################################################ # Display Column Determination @@ -823,6 +738,18 @@ if (lsearch(\@displaycolumns, "percentage_complete") >= 0) { push (@selectcolumns, "actual_time"); } +# Make sure that the login_name version of a field is always also +# requested if the realname version is requested, so that we can +# display the login name when the realname is empty. +my @realname_fields = grep(/_realname$/, @displaycolumns); +foreach my $item (@realname_fields) { + my $login_field = $item; + $login_field =~ s/_realname$//; + if (!grep($_ eq $login_field, @selectcolumns)) { + push(@selectcolumns, $login_field); + } +} + # Display columns are selected because otherwise we could not display them. foreach my $col (@displaycolumns) { push (@selectcolumns, $col) if !grep($_ eq $col, @selectcolumns); @@ -866,17 +793,6 @@ if ($format->{'extension'} eq 'atom') { } ################################################################################ -# Query Generation -################################################################################ - -# Convert the list of columns being selected into a list of column names. -my @selectnames = map($columns->{$_}->{'name'}, @selectcolumns); - -# Remove columns with no names, such as percentage_complete -# (or a removed *_time column due to permissions) -@selectnames = grep($_ ne '', @selectnames); - -################################################################################ # Sort Order Determination ################################################################################ @@ -897,39 +813,48 @@ if (!$order || $order =~ /^reuse/i) { } } -my $db_order = ""; # Modified version of $order for use with SQL query if ($order) { # Convert the value of the "order" form field into a list of columns # by which to sort the results. ORDER: for ($order) { /^Bug Number$/ && do { - $order = "bugs.bug_id"; + $order = "bug_id"; last ORDER; }; /^Importance$/ && do { - $order = "bugs.priority, bugs.bug_severity"; + $order = "priority,bug_severity"; last ORDER; }; /^Assignee$/ && do { - $order = "map_assigned_to.login_name, bugs.bug_status, bugs.priority, bugs.bug_id"; + $order = "assigned_to,bug_status,priority,bug_id"; last ORDER; }; /^Last Changed$/ && do { - $order = "bugs.delta_ts, bugs.bug_status, bugs.priority, map_assigned_to.login_name, bugs.bug_id"; + $order = "changeddate,bug_status,priority,assigned_to,bug_id"; last ORDER; }; do { my (@order, @invalid_fragments); - my @columnnames = map($columns->{lc($_)}->{'name'}, keys(%$columns)); + # A custom list of columns. Make sure each column is valid. foreach my $fragment (split(/,/, $order)) { $fragment = trim($fragment); next unless $fragment; - # Accept an order fragment matching a column name, with - # asc|desc optionally following (to specify the direction) - if (grep($fragment =~ /^\Q$_\E(\s+(asc|desc))?$/, @columnnames, keys(%$columns))) { - next if $fragment =~ /\brelevance\b/ && !$fulltext; - push(@order, $fragment); + my ($column_name, $direction) = split_order_term($fragment); + $column_name = translate_old_column($column_name); + + # Special handlings for certain columns + next if $column_name eq 'relevance' && !$fulltext; + + # If we are sorting by votes, sort in descending order if + # no explicit sort order was given. + if ($column_name eq 'votes' && !$direction) { + $direction = "DESC"; + } + + if (exists $columns->{$column_name}) { + $direction = " $direction" if $direction; + push(@order, "$column_name$direction"); } else { push(@invalid_fragments, $fragment); @@ -950,58 +875,13 @@ if ($order) { if (!$order) { # DEFAULT - $order = "bugs.bug_status, bugs.priority, map_assigned_to.login_name, bugs.bug_id"; + $order = "bug_status,priority,assigned_to,bug_id"; } -# Make sure ORDER BY columns are included in the field list. -foreach my $fragment (split(/,/, $order)) { - $fragment = trim($fragment); - if (!grep($fragment =~ /^\Q$_\E(\s+(asc|desc))?$/, @selectnames)) { - # Add order columns to selectnames - # The fragment has already been validated - $fragment =~ s/\s+(asc|desc)$//; - - # While newer fragments contain IDs for aliased columns, older - # LASTORDER cookies (or bookmarks) may contain full names. - # Convert them to an ID here. - if ($fragment =~ / AS (\w+)/) { - $fragment = $1; - } - - $fragment =~ tr/a-zA-Z\.0-9\-_//cd; - - # If the order fragment is an ID, we need its corresponding name - # to be in the field list. - if (exists($columns->{$fragment})) { - $fragment = $columns->{$fragment}->{'name'}; - } - - push(@selectnames, $fragment) unless (grep { $fragment eq $_ } @selectnames); - } -} - -$db_order = $order; # Copy $order into $db_order for use with SQL query - -# If we are sorting by votes, sort in descending order if no explicit -# sort order was given -$db_order =~ s/bugs.votes\s*(,|$)/bugs.votes desc$1/i; - -# the 'actual_time' field is defined as an aggregate function, but -# for order we just need the column name 'actual_time' -my $aggregate_search = quotemeta($columns->{'actual_time'}->{'name'}); -$db_order =~ s/$aggregate_search/actual_time/g; - -# the 'percentage_complete' field is defined as an aggregate too -$aggregate_search = quotemeta($columns->{'percentage_complete'}->{'name'}); -$db_order =~ s/$aggregate_search/percentage_complete/g; - -# Now put $db_order into a format that Bugzilla::Search can use. -# (We create $db_order as a string first because that's the way -# we did it before Bugzilla::Search took an "order" argument.) -my @orderstrings = split(/,\s*/, $db_order); +my @orderstrings = split(/,\s*/, $order); # Generate the basic SQL query that will be used to generate the bug list. -my $search = new Bugzilla::Search('fields' => \@selectnames, +my $search = new Bugzilla::Search('fields' => \@selectcolumns, 'params' => $params, 'order' => \@orderstrings); my $query = $search->getSQL(); diff --git a/collectstats.pl b/collectstats.pl index 761c648c8..bcb0fac5b 100755 --- a/collectstats.pl +++ b/collectstats.pl @@ -561,7 +561,7 @@ sub CollectSeriesData { # login name or a renamed product or component, etc. eval { my $search = new Bugzilla::Search('params' => $cgi, - 'fields' => ["bugs.bug_id"], + 'fields' => ["bug_id"], 'user' => $user); my $sql = $search->getSQL(); $data = $shadow_dbh->selectall_arrayref($sql); diff --git a/duplicates.cgi b/duplicates.cgi index 06334e22b..af239d632 100755 --- a/duplicates.cgi +++ b/duplicates.cgi @@ -199,14 +199,14 @@ if (scalar(%count)) { $params->param('product', join(',', @query_products)); } - my $query = new Bugzilla::Search('fields' => [qw(bugs.bug_id - map_components.name - bugs.bug_severity - bugs.op_sys - bugs.target_milestone - bugs.short_desc - bugs.bug_status - bugs.resolution + my $query = new Bugzilla::Search('fields' => [qw(bug_id + component + bug_severity + op_sys + target_milestone + short_desc + bug_status + resolution ) ], 'params' => $params, diff --git a/report.cgi b/report.cgi index fd2f28943..2f950948a 100755 --- a/report.cgi +++ b/report.cgi @@ -102,53 +102,42 @@ else { } } -my %columns; -$columns{'bug_severity'} = "bugs.bug_severity"; -$columns{'priority'} = "bugs.priority"; -$columns{'rep_platform'} = "bugs.rep_platform"; -$columns{'assigned_to'} = "map_assigned_to.login_name"; -$columns{'reporter'} = "map_reporter.login_name"; -$columns{'qa_contact'} = "map_qa_contact.login_name"; -$columns{'bug_status'} = "bugs.bug_status"; -$columns{'resolution'} = "bugs.resolution"; -$columns{'component'} = "map_components.name"; -$columns{'product'} = "map_products.name"; -$columns{'classification'} = "map_classifications.name"; -$columns{'version'} = "bugs.version"; -$columns{'op_sys'} = "bugs.op_sys"; -$columns{'votes'} = "bugs.votes"; -$columns{'keywords'} = "bugs.keywords"; -$columns{'target_milestone'} = "bugs.target_milestone"; -# Single-select fields are also accepted as valid column names. -my @single_select_fields = - grep { $_->type == FIELD_TYPE_SINGLE_SELECT } Bugzilla->active_custom_fields; - -foreach my $custom_field (@single_select_fields) { - my $field_name = $custom_field->name; - $columns{$field_name} = "bugs.$field_name"; -} - -# One which means "nothing". Any number would do, really. It just gets SELECTed -# so that we always select 3 items in the query. -$columns{''} = "42217354"; +# Valid bug fields that can be reported on. +my @columns = qw( + assigned_to + reporter + qa_contact + component + classification + version + votes + keywords + target_milestone +); +# Single-select fields (custom or not) are also accepted as valid. +my @single_selects = Bugzilla->get_fields({ type => FIELD_TYPE_SINGLE_SELECT, + obsolete => 0 }); +push(@columns, map { $_->name } @single_selects); +my %valid_columns = map { $_ => 1 } @columns; # Validate the values in the axis fields or throw an error. !$row_field - || ($columns{$row_field} && trick_taint($row_field)) + || ($valid_columns{$row_field} && trick_taint($row_field)) || ThrowCodeError("report_axis_invalid", {fld => "x", val => $row_field}); !$col_field - || ($columns{$col_field} && trick_taint($col_field)) + || ($valid_columns{$col_field} && trick_taint($col_field)) || ThrowCodeError("report_axis_invalid", {fld => "y", val => $col_field}); !$tbl_field - || ($columns{$tbl_field} && trick_taint($tbl_field)) + || ($valid_columns{$tbl_field} && trick_taint($tbl_field)) || ThrowCodeError("report_axis_invalid", {fld => "z", val => $tbl_field}); -my @axis_fields = ($row_field, $col_field, $tbl_field); -my @selectnames = map($columns{$_}, @axis_fields); +my @axis_fields = ($row_field || EMPTY_COLUMN, + $col_field || EMPTY_COLUMN, + $tbl_field || EMPTY_COLUMN); # Clone the params, so that Bugzilla::Search can modify them my $params = new Bugzilla::CGI($cgi); -my $search = new Bugzilla::Search('fields' => \@selectnames, +my $search = new Bugzilla::Search('fields' => \@axis_fields, 'params' => $params); my $query = $search->getSQL(); @@ -179,9 +168,9 @@ foreach my $result (@$results) { $col = ' ' if ($col eq ''); $tbl = ' ' if ($tbl eq ''); - $row = "" if ($row eq $columns{''}); - $col = "" if ($col eq $columns{''}); - $tbl = "" if ($tbl eq $columns{''}); + $row = "" if ($row eq EMPTY_COLUMN); + $col = "" if ($col eq EMPTY_COLUMN); + $tbl = "" if ($tbl eq EMPTY_COLUMN); # account for the fact that names may start with '_' or '.'. Change this # so the template doesn't hide hash elements with those keys diff --git a/template/en/default/list/table.html.tmpl b/template/en/default/list/table.html.tmpl index ed1f3de3f..10a50b33f 100644 --- a/template/en/default/list/table.html.tmpl +++ b/template/en/default/list/table.html.tmpl @@ -87,11 +87,11 @@ [% END %] <th colspan="[% splitheader ? 2 : 1 %]" class="first-child"> [% desc = '' %] - [% IF (om = order.match("^bugs\.bug_id( desc)?")) %] - [% desc = ' desc' IF NOT om.0 %] + [% IF (om = order.match("^bug_id( DESC)?")) %] + [% desc = ' DESC' IF NOT om.0 %] [% END %] <a href="buglist.cgi? - [% urlquerypart FILTER html %]&order=bugs.bug_id[% desc FILTER url_quote %] + [% urlquerypart FILTER html %]&order=bug_id[% desc FILTER url_quote %] [%-#%]&query_based_on= [% defaultsavename OR searchname FILTER url_quote %]">ID</a> </th> @@ -130,20 +130,13 @@ [% BLOCK columnheader %] <th colspan="[% splitheader ? 2 : 1 %]"> - [% IF column.name.match('\s+AS\s+') %] - [%# For aliased columns, use their ID for sorting. %] - [% column.sortalias = id %] - [% ELSE %] - [%# Other columns may sort on their name directly. %] - [% column.sortalias = column.name %] - [% END %] [% desc = '' %] - [% IF (om = order.match("$column.sortalias( desc)?")) %] - [% desc = ' desc' IF NOT om.0 %] + [% IF (om = order.match("$id( DESC)?")) %] + [% desc = ' DESC' IF NOT om.0 %] [% END %] - [% order = order.remove("$column.sortalias( desc)?,?") %] + [% order = order.remove("$id( DESC)?,?") %] <a href="buglist.cgi?[% urlquerypart FILTER html %]&order= - [% column.sortalias FILTER url_quote %][% desc FILTER url_quote %] + [% id FILTER url_quote %][% desc FILTER url_quote %] [% ",$order" FILTER url_quote IF order %] [%-#%]&query_based_on= [% defaultsavename OR searchname FILTER url_quote %]"> @@ -204,6 +197,13 @@ [%- get_status(bug.$column).truncate(abbrev.$column.maxlength, abbrev.$column.ellipsis) FILTER html %] [% ELSIF column == 'resolution' %] [%- get_resolution(bug.$column).truncate(abbrev.$column.maxlength, abbrev.$column.ellipsis) FILTER html %] + + [%# Display the login name of the user if their real name is empty. %] + [% ELSIF column.match('_realname$') && bug.$column == '' %] + [% SET login_column = column.remove('_realname$') %] + [% bug.${login_column}.truncate(abbrev.$column.maxlength, + abbrev.$column.ellipsis) FILTER html %] + [% ELSE %] [%- bug.$column.truncate(abbrev.$column.maxlength, abbrev.$column.ellipsis) FILTER html -%] [% END %] @@ -431,16 +431,16 @@ sub run_queries { next unless $savedquery; # silently ignore missing queries # Execute the saved query - my @searchfields = ( - 'bugs.bug_id', - 'bugs.bug_severity', - 'bugs.priority', - 'bugs.rep_platform', - 'bugs.assigned_to', - 'bugs.bug_status', - 'bugs.resolution', - 'bugs.short_desc', - 'map_assigned_to.login_name', + my @searchfields = qw( + bug_id + bug_severity + priority + rep_platform + assigned_to + bug_status + resolution + short_desc + assigned_to ); # A new Bugzilla::CGI object needs to be created to allow # Bugzilla::Search to execute a saved query. It's exceedingly weird, |