Bug 344878: Automatically generate list of acceptable columns for buglist.cgi from the database - Patch by Teemu Mannermaa <wicked@sci.fi> r/a=mkanat

5 files changed, 125 insertions, 77 deletions

diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index f11c86e75..3a4652c93 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -654,6 +654,8 @@ use constant ABSTRACT_SCHEMA => {
                             DEFAULT => 'FALSE'},
             enter_bug   => {TYPE => 'BOOLEAN', NOTNULL => 1,
                             DEFAULT => 'FALSE'},
+            buglist     => {TYPE => 'BOOLEAN', NOTNULL => 1,
+                            DEFAULT => 'FALSE'},
             visibility_field_id => {TYPE => 'INT3', 
                                     REFERENCES => {TABLE  => 'fielddefs',
                                                    COLUMN => 'id'}},
diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm
index 6d271fe11..848daf74a 100644
--- a/Bugzilla/Field.pm
+++ b/Bugzilla/Field.pm
@@ -96,6 +96,7 @@ use constant DB_COLUMNS => qw(
     sortkey
     obsolete
     enter_bug
+    buglist
     visibility_field_id
     visibility_value_id
     value_field_id
@@ -107,6 +108,7 @@ use constant VALIDATORS => {
     custom      => \&_check_custom,
     description => \&_check_description,
     enter_bug   => \&_check_enter_bug,
+    buglist     => \&Bugzilla::Object::check_boolean,
     mailhead    => \&_check_mailhead,
     obsolete    => \&_check_obsolete,
     sortkey     => \&_check_sortkey,
@@ -125,6 +127,7 @@ use constant UPDATE_COLUMNS => qw(
     sortkey
     obsolete
     enter_bug
+    buglist
     visibility_field_id
     visibility_value_id
     value_field_id
@@ -148,32 +151,42 @@ use constant SQL_DEFINITIONS => {
 # These are used by populate_field_definitions to populate
 # the fielddefs table.
 use constant DEFAULT_FIELDS => (
-    {name => 'bug_id',       desc => 'Bug #',      in_new_bugmail => 1},
-    {name => 'short_desc',   desc => 'Summary',    in_new_bugmail => 1},
-    {name => 'classification', desc => 'Classification', in_new_bugmail => 1},
-    {name => 'product',      desc => 'Product',    in_new_bugmail => 1},
-    {name => 'version',      desc => 'Version',    in_new_bugmail => 1},
+    {name => 'bug_id',       desc => 'Bug #',      in_new_bugmail => 1,
+     buglist => 1},
+    {name => 'short_desc',   desc => 'Summary',    in_new_bugmail => 1,
+     buglist => 1},
+    {name => 'classification', desc => 'Classification', in_new_bugmail => 1,
+     buglist => 1},
+    {name => 'product',      desc => 'Product',    in_new_bugmail => 1,
+     buglist => 1},
+    {name => 'version',      desc => 'Version',    in_new_bugmail => 1,
+     buglist => 1},
     {name => 'rep_platform', desc => 'Platform',   in_new_bugmail => 1,
-     type => FIELD_TYPE_SINGLE_SELECT},
+     type => FIELD_TYPE_SINGLE_SELECT, buglist => 1},
     {name => 'bug_file_loc', desc => 'URL',        in_new_bugmail => 1},
     {name => 'op_sys',       desc => 'OS/Version', in_new_bugmail => 1,
-     type => FIELD_TYPE_SINGLE_SELECT},
+     type => FIELD_TYPE_SINGLE_SELECT, buglist => 1},
     {name => 'bug_status',   desc => 'Status',     in_new_bugmail => 1,
-     type => FIELD_TYPE_SINGLE_SELECT},
+     type => FIELD_TYPE_SINGLE_SELECT, buglist => 1},
     {name => 'status_whiteboard', desc => 'Status Whiteboard',
-     in_new_bugmail => 1},
-    {name => 'keywords',     desc => 'Keywords',   in_new_bugmail => 1},
+     in_new_bugmail => 1, buglist => 1},
+    {name => 'keywords',     desc => 'Keywords',   in_new_bugmail => 1,
+     buglist => 1},
     {name => 'resolution',   desc => 'Resolution',
-     type => FIELD_TYPE_SINGLE_SELECT},
+     type => FIELD_TYPE_SINGLE_SELECT, buglist => 1},
     {name => 'bug_severity', desc => 'Severity',   in_new_bugmail => 1,
-     type => FIELD_TYPE_SINGLE_SELECT},
+     type => FIELD_TYPE_SINGLE_SELECT, buglist => 1},
     {name => 'priority',     desc => 'Priority',   in_new_bugmail => 1,
-     type => FIELD_TYPE_SINGLE_SELECT},
-    {name => 'component',    desc => 'Component',  in_new_bugmail => 1},
-    {name => 'assigned_to',  desc => 'AssignedTo', in_new_bugmail => 1},
-    {name => 'reporter',     desc => 'ReportedBy', in_new_bugmail => 1},
-    {name => 'votes',        desc => 'Votes'},
-    {name => 'qa_contact',   desc => 'QAContact',  in_new_bugmail => 1},
+     type => FIELD_TYPE_SINGLE_SELECT, buglist => 1},
+    {name => 'component',    desc => 'Component',  in_new_bugmail => 1,
+     buglist => 1},
+    {name => 'assigned_to',  desc => 'AssignedTo', in_new_bugmail => 1,
+     buglist => 1},
+    {name => 'reporter',     desc => 'ReportedBy', in_new_bugmail => 1,
+     buglist => 1},
+    {name => 'votes',        desc => 'Votes',      buglist => 1},
+    {name => 'qa_contact',   desc => 'QAContact',  in_new_bugmail => 1,
+     buglist => 1},
     {name => 'cc',           desc => 'CC',         in_new_bugmail => 1},
     {name => 'dependson',    desc => 'Depends on', in_new_bugmail => 1},
     {name => 'blocked',      desc => 'Blocks',     in_new_bugmail => 1},
@@ -186,25 +199,31 @@ use constant DEFAULT_FIELDS => (
     {name => 'attachments.isprivate',   desc => 'Attachment is private'},
     {name => 'attachments.submitter',   desc => 'Attachment creator'},
 
-    {name => 'target_milestone',      desc => 'Target Milestone'},
-    {name => 'creation_ts', desc => 'Creation date', in_new_bugmail => 1},
-    {name => 'delta_ts', desc => 'Last changed date', in_new_bugmail => 1},
+    {name => 'target_milestone',      desc => 'Target Milestone',
+     buglist => 1},
+    {name => 'creation_ts',           desc => 'Creation date',
+     in_new_bugmail => 1, buglist => 1},
+    {name => 'delta_ts',              desc => 'Last changed date',
+     in_new_bugmail => 1, buglist => 1},
     {name => 'longdesc',              desc => 'Comment'},
     {name => 'longdescs.isprivate',   desc => 'Comment is private'},
-    {name => 'alias',                 desc => 'Alias'},
+    {name => 'alias',                 desc => 'Alias', buglist => 1},
     {name => 'everconfirmed',         desc => 'Ever Confirmed'},
     {name => 'reporter_accessible',   desc => 'Reporter Accessible'},
     {name => 'cclist_accessible',     desc => 'CC Accessible'},
     {name => 'bug_group',             desc => 'Group', in_new_bugmail => 1},
-    {name => 'estimated_time', desc => 'Estimated Hours', in_new_bugmail => 1},
-    {name => 'remaining_time',        desc => 'Remaining Hours'},
-    {name => 'deadline',              desc => 'Deadline', in_new_bugmail => 1},
+    {name => 'estimated_time',        desc => 'Estimated Hours',
+     in_new_bugmail => 1, buglist => 1},
+    {name => 'remaining_time',        desc => 'Remaining Hours', buglist => 1},
+    {name => 'deadline',              desc => 'Deadline',
+     in_new_bugmail => 1, buglist => 1},
     {name => 'commenter',             desc => 'Commenter'},
     {name => 'flagtypes.name',        desc => 'Flag'},
     {name => 'requestees.login_name', desc => 'Flag Requestee'},
     {name => 'setters.login_name',    desc => 'Flag Setter'},
-    {name => 'work_time',             desc => 'Hours Worked'},
-    {name => 'percentage_complete',   desc => 'Percentage Complete'},
+    {name => 'work_time',             desc => 'Hours Worked', buglist => 1},
+    {name => 'percentage_complete',   desc => 'Percentage Complete',
+     buglist => 1},
     {name => 'content',               desc => 'Content'},
     {name => 'attach_data.thedata',   desc => 'Attachment data'},
     {name => 'attachments.isurl',     desc => 'Attachment is a URL'},
@@ -438,6 +457,19 @@ sub enter_bug { return $_[0]->{enter_bug} }
 
 =over
 
+=item C<buglist>
+
+A boolean specifying whether or not this field is selectable
+as a display or order column in buglist.cgi
+
+=back
+
+=cut
+
+sub buglist { return $_[0]->{buglist} }
+
+=over
+
 =item C<is_select>
 
 True if this is a C<FIELD_TYPE_SINGLE_SELECT> or C<FIELD_TYPE_MULTI_SELECT>
@@ -602,6 +634,8 @@ They will throw an error if you try to set the values to something invalid.
 
 =item C<set_in_new_bugmail>
 
+=item C<set_buglist>
+
 =item C<set_visibility_field>
 
 =item C<set_visibility_value>
@@ -617,6 +651,7 @@ sub set_enter_bug      { $_[0]->set('enter_bug',   $_[1]); }
 sub set_obsolete       { $_[0]->set('obsolete',    $_[1]); }
 sub set_sortkey        { $_[0]->set('sortkey',     $_[1]); }
 sub set_in_new_bugmail { $_[0]->set('mailhead',    $_[1]); }
+sub set_buglist        { $_[0]->set('buglist',     $_[1]); }
 sub set_visibility_field {
     my ($self, $value) = @_;
     $self->set('visibility_field_id', $value);
@@ -745,6 +780,9 @@ will be added to the C<bugs> table if it does not exist. Defaults to 0.
 =item C<enter_bug> - boolean - Whether this field is
 editable on the bug creation form. Defaults to 0.
 
+=item C<buglist> - boolean - Whether this field is
+selectable as a display or order column in bug lists. Defaults to 0.
+
 C<obsolete> - boolean - Whether this field is obsolete. Defaults to 0.
 
 =back
@@ -867,6 +905,7 @@ sub populate_field_definitions {
         if ($field) {
             $field->set_description($def->{desc});
             $field->set_in_new_bugmail($def->{in_new_bugmail});
+            $field->set_buglist($def->{buglist});
             $field->_set_type($def->{type}) if $def->{type};
             $field->update();
         }
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index b6bda167e..44ad4c814 100644
--- a/Bugzilla/Install/DB.pm
+++ b/Bugzilla/Install/DB.pm
@@ -92,6 +92,16 @@ sub update_fielddefs_definition {
     $dbh->bz_add_index('fielddefs', 'fielddefs_value_field_id_idx',
                        ['value_field_id']);
 
+    # Bug 344878
+    if (!$dbh->bz_column_info('fielddefs', 'buglist')) {
+        $dbh->bz_add_column('fielddefs', 'buglist',
+            {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'});
+        # Set non-multiselect custom fields as valid buglist fields
+        # Note that default fields will be handled in Field.pm
+        $dbh->do('UPDATE fielddefs SET buglist = 1 WHERE custom = 1 AND type != ' . FIELD_TYPE_MULTI_SELECT);
+    }
+
+
     # Remember, this is not the function for adding general table changes.
     # That is below. Add new changes to the fielddefs table above this
     # comment.
diff --git a/buglist.cgi b/buglist.cgi
index 32fb1b3f0..55bb8a70d 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -652,62 +652,58 @@ if (!$params->param('query_format')) {
 # 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.
 
-local our $columns = {};
-sub DefineColumn {
-    my ($id, $name, $title) = @_;
-    $columns->{$id} = { 'name' => $name , 'title' => $title };
-}
+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';
+    }
 
-# Column:     ID                    Name                           Title
-DefineColumn("bug_id"            , "bugs.bug_id"                , "ID"               );
-DefineColumn("alias"             , "bugs.alias"                 , "Alias"            );
-DefineColumn("opendate"          , "bugs.creation_ts"           , "Opened"           );
-DefineColumn("changeddate"       , "bugs.delta_ts"              , "Changed"          );
-DefineColumn("bug_severity"      , "bugs.bug_severity"          , "Severity"         );
-DefineColumn("priority"          , "bugs.priority"              , "Priority"         );
-DefineColumn("rep_platform"      , "bugs.rep_platform"          , "Hardware"         );
-DefineColumn("assigned_to"       , "map_assigned_to.login_name" , "Assignee"         );
-DefineColumn("reporter"          , "map_reporter.login_name"    , "Reporter"         );
-DefineColumn("qa_contact"        , "map_qa_contact.login_name"  , "QA Contact"       );
-if ($format->{'extension'} eq 'html') {
-    DefineColumn("assigned_to_realname", "CASE WHEN map_assigned_to.realname = '' THEN map_assigned_to.login_name ELSE map_assigned_to.realname END AS assigned_to_realname", "Assignee"  );
-    DefineColumn("reporter_realname"   , "CASE WHEN map_reporter.realname    = '' THEN map_reporter.login_name    ELSE map_reporter.realname    END AS reporter_realname"   , "Reporter"  );
-    DefineColumn("qa_contact_realname" , "CASE WHEN map_qa_contact.realname  = '' THEN map_qa_contact.login_name  ELSE map_qa_contact.realname  END AS qa_contact_realname" , "QA Contact");
-} else {
-    DefineColumn("assigned_to_realname", "map_assigned_to.realname AS assigned_to_realname", "Assignee"  );
-    DefineColumn("reporter_realname"   , "map_reporter.realname AS reporter_realname"      , "Reporter"  );
-    DefineColumn("qa_contact_realname" , "map_qa_contact.realname AS qa_contact_realname"  , "QA Contact");
-}
-DefineColumn("bug_status"        , "bugs.bug_status"            , "Status"           );
-DefineColumn("resolution"        , "bugs.resolution"            , "Resolution"       );
-DefineColumn("short_short_desc"  , "bugs.short_desc"            , "Summary"          );
-DefineColumn("short_desc"        , "bugs.short_desc"            , "Summary"          );
-DefineColumn("status_whiteboard" , "bugs.status_whiteboard"     , "Whiteboard"       );
-DefineColumn("component"         , "map_components.name"        , "Component"        );
-DefineColumn("product"           , "map_products.name"          , "Product"          );
-DefineColumn("classification"    , "map_classifications.name"   , "Classification"   );
-DefineColumn("version"           , "bugs.version"               , "Version"          );
-DefineColumn("op_sys"            , "bugs.op_sys"                , "OS"               );
-DefineColumn("target_milestone"  , "bugs.target_milestone"      , "Target Milestone" );
-DefineColumn("votes"             , "bugs.votes"                 , "Votes"            );
-DefineColumn("keywords"          , "bugs.keywords"              , "Keywords"         );
-DefineColumn("estimated_time"    , "bugs.estimated_time"        , "Estimated Hours"  );
-DefineColumn("remaining_time"    , "bugs.remaining_time"        , "Remaining Hours"  );
-DefineColumn("actual_time"       , "(SUM(ldtime.work_time)*COUNT(DISTINCT ldtime.bug_when)/COUNT(bugs.bug_id)) AS actual_time", "Actual Hours");
-DefineColumn("percentage_complete",
+    # 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';
+    }
+    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"                               , "% Complete"); 
-DefineColumn("relevance"         , "relevance"                  , "Relevance"        );
-DefineColumn("deadline"          , $dbh->sql_date_format('bugs.deadline', '%Y-%m-%d') . " AS deadline", "Deadline");
-
-foreach my $field (Bugzilla->active_custom_fields) {
-    # Multi-select fields are not (yet) supported in buglists.
-    next if $field->type == FIELD_TYPE_MULTI_SELECT;
-    DefineColumn($field->name, 'bugs.' . $field->name, $field->description);
+    "END) AS percentage_complete"
+    }
+
+    $columns->{$id} = { 'name' => $name, 'title' => $field->description };
+}
+
+if ($format->{'extension'} eq 'html') {
+    $columns->{assigned_to_realname} = { name => "CASE WHEN map_assigned_to.realname = '' THEN map_assigned_to.login_name ELSE map_assigned_to.realname END AS assigned_to_realname", title => "Assignee" };
+    $columns->{reporter_realname} = { name => "CASE WHEN map_reporter.realname    = '' THEN map_reporter.login_name    ELSE map_reporter.realname    END AS reporter_realname", title => "Reporter" };
+    $columns->{qa_contact_realname} = { name => "CASE WHEN map_qa_contact.realname  = '' THEN map_qa_contact.login_name  ELSE map_qa_contact.realname  END AS qa_contact_realname", title => "QA Contact" };
+} else {
+    $columns->{assigned_to_realname} = { name => "map_assigned_to.realname AS assigned_to_realname", title => "Assignee" };
+    $columns->{reporter_realname} = { name => "map_reporter.realname AS reporter_realname", title => "Reporter" };
+    $columns->{qa_contact_realname} = { name => "map_qa_contact.realname AS qa_contact_realname", title => "QA Contact" };
 }
 
 Bugzilla::Hook::process("buglist-columns", {'columns' => $columns} );
diff --git a/editfields.cgi b/editfields.cgi
index 446753044..09147d69a 100644
--- a/editfields.cgi
+++ b/editfields.cgi
@@ -64,6 +64,7 @@ elsif ($action eq 'new') {
         enter_bug   => scalar $cgi->param('enter_bug'),
         obsolete    => scalar $cgi->param('obsolete'),
         custom      => 1,
+        buglist     => (scalar $cgi->param('type') == FIELD_TYPE_MULTI_SELECT ? 0 : 1),
         visibility_field_id => scalar $cgi->param('visibility_field_id'),
         visibility_value_id => scalar $cgi->param('visibility_value_id'),
         value_field_id => scalar $cgi->param('value_field_id'),