Bug 287326: Ability to add custom single-select fields to a bug - Patch by Frédéric Buclin <LpSolit@gmail.com> and Max Kanat-Alexander <mkanat@bugzilla.org> r=mkanat a=myk

18 files changed, 251 insertions, 63 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 03a28bf5d..6e8079d27 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -110,20 +110,30 @@ use constant REQUIRED_CREATE_FIELDS => qw(
 
 # There are also other, more complex validators that are called
 # from run_create_validators.
-use constant VALIDATORS => {
-    alias          => \&_check_alias,
-    bug_file_loc   => \&_check_bug_file_loc,
-    bug_severity   => \&_check_bug_severity,
-    cc             => \&_check_cc,
-    deadline       => \&_check_deadline,
-    estimated_time => \&_check_estimated_time,
-    op_sys         => \&_check_op_sys,
-    priority       => \&_check_priority,
-    product        => \&_check_product,
-    remaining_time => \&_check_remaining_time,
-    rep_platform   => \&_check_rep_platform,
-    short_desc     => \&_check_short_desc,
-    status_whiteboard => \&_check_status_whiteboard,
+sub VALIDATORS {
+    my $validators = {
+        alias          => \&_check_alias,
+        bug_file_loc   => \&_check_bug_file_loc,
+        bug_severity   => \&_check_bug_severity,
+        cc             => \&_check_cc,
+        deadline       => \&_check_deadline,
+        estimated_time => \&_check_estimated_time,
+        op_sys         => \&_check_op_sys,
+        priority       => \&_check_priority,
+        product        => \&_check_product,
+        remaining_time => \&_check_remaining_time,
+        rep_platform   => \&_check_rep_platform,
+        short_desc     => \&_check_short_desc,
+        status_whiteboard => \&_check_status_whiteboard,
+    };
+
+    my @select_fields = Bugzilla->get_fields({custom => 1, obsolete => 0,
+                                              type => FIELD_TYPE_SINGLE_SELECT});
+
+    foreach my $field (@select_fields) {
+        $validators->{$field->name} = \&_check_select_field;
+    }
+    return $validators;
 };
 
 # Used in LogActivityEntry(). Gives the max length of lines in the
@@ -279,7 +289,7 @@ sub run_create_validators {
     $params->{remaining_time} = $params->{estimated_time};
 
     $class->_check_strict_isolation($product, $params->{cc},
-    $params->{assigned_to}, $params->{qa_contact});
+                                    $params->{assigned_to}, $params->{qa_contact});
 
     return $params;
 }
@@ -657,6 +667,12 @@ sub _check_version {
     return $version;
 }
 
+sub _check_select_field {
+    my ($invocant, $value, $field) = @_;
+    $value = trim($value);
+    check_field($field, $value);
+    return $value;
+}
 
 #####################################################################
 # Class Accessors
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index a0e869c33..9559dcae3 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -106,6 +106,7 @@ use File::Basename;
 
     FIELD_TYPE_UNKNOWN
     FIELD_TYPE_FREETEXT
+    FIELD_TYPE_SINGLE_SELECT
 
     BUG_STATE_OPEN
 
@@ -296,6 +297,7 @@ use constant SENDMAIL_EXE => '/usr/lib/sendmail.exe';
 
 use constant FIELD_TYPE_UNKNOWN   => 0;
 use constant FIELD_TYPE_FREETEXT  => 1;
+use constant FIELD_TYPE_SINGLE_SELECT => 2;
 
 # The maximum number of days a token will remain valid.
 use constant MAX_TOKEN_AGE => 3;
diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index 077f93cf7..5fceb961d 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -577,10 +577,25 @@ sub bz_add_table {
 sub _bz_add_table_raw {
     my ($self, $name) = @_;
     my @statements = $self->_bz_schema->get_table_ddl($name);
-    print "Adding new table $name ...\n";
+    print "Adding new table $name ...\n" unless i_am_cgi();
     $self->do($_) foreach (@statements);
 }
 
+sub bz_add_field_table {
+    my ($self, $name) = @_;
+    my $table_schema = $self->_bz_schema->FIELD_TABLE_SCHEMA;
+    my $indexes      = $table_schema->{INDEXES};
+    # $indexes is an arrayref, not a hash. In order to fix the keys,
+    # we have to fix every other item.
+    for (my $i = 0; $i < scalar @$indexes; $i++) {
+        next if ($i % 2 && $i != 0); # We skip 1, 3, 5, 7, etc.
+        $indexes->[$i] = $name . "_" . $indexes->[$i];
+    }
+    # We add this to the abstract schema so that bz_add_table can find it.
+    $self->_bz_schema->add_table($name, $table_schema);
+    $self->bz_add_table($name);
+}
+
 sub bz_drop_column {
     my ($self, $table, $column) = @_;
 
diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index 4c270e68c..384fb478b 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -1090,6 +1090,23 @@ use constant ABSTRACT_SCHEMA => {
     },
 
 };
+
+use constant FIELD_TABLE_SCHEMA => {
+    FIELDS => [
+        id       => {TYPE => 'SMALLSERIAL', NOTNULL => 1,
+                     PRIMARYKEY => 1},
+        value    => {TYPE => 'varchar(64)', NOTNULL => 1},
+        sortkey  => {TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0},
+        isactive => {TYPE => 'BOOLEAN', NOTNULL => 1,
+                     DEFAULT => 'TRUE'},
+    ],
+    # Note that bz_add_field_table should prepend the table name
+    # to these index names.
+    INDEXES => [
+        value_idx   => {FIELDS => ['value'], TYPE => 'UNIQUE'},
+        sortkey_idx => ['sortkey', 'value'],
+    ],
+};
 #--------------------------------------------------------------------------
 
 =head1 METHODS
diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm
index 870e93221..2dfd8aa6e 100644
--- a/Bugzilla/Field.pm
+++ b/Bugzilla/Field.pm
@@ -103,7 +103,9 @@ use constant DB_COLUMNS => (
 use constant SQL_DEFINITIONS => {
     # Using commas because these are constants and they shouldn't
     # be auto-quoted by the "=>" operator.
-    FIELD_TYPE_FREETEXT, { TYPE => 'varchar(255)' },
+    FIELD_TYPE_FREETEXT,      { TYPE => 'varchar(255)' },
+    FIELD_TYPE_SINGLE_SELECT, { TYPE => 'varchar(64)', NOTNULL => 1,
+                                DEFAULT => "'---'" },
 };
 
 # Field definitions for the fields that ship with Bugzilla.
@@ -266,6 +268,24 @@ enter_bug.cgi
 
 sub enter_bug { return $_[0]->{enter_bug} }
 
+=over
+
+=item C<legal_values>
+
+A reference to an array with valid active values for this field.
+
+=back
+
+=cut
+
+sub legal_values {
+    my $self = shift;
+
+    if (!defined $self->{'legal_values'}) {
+        $self->{'legal_values'} = get_legal_field_values($self->name);
+    }
+    return $self->{'legal_values'};
+}
 
 =pod
 
@@ -305,9 +325,7 @@ sub create_or_update {
     my $name           = $params->{name};
     my $custom         = $params->{custom} ? 1 : 0;
     my $in_new_bugmail = $params->{in_new_bugmail} ? 1 : 0;
-    # Some day we'll allow invocants to specify the field type.
-    # We don't care about $params->{type} yet.
-    my $type = $custom ? FIELD_TYPE_FREETEXT : FIELD_TYPE_UNKNOWN;
+    my $type           = $params->{type} || FIELD_TYPE_UNKNOWN;
 
     my $field = new Bugzilla::Field({name => $name});
     if ($field) {
@@ -353,6 +371,15 @@ sub create_or_update {
         $dbh->bz_add_column('bugs', $name, SQL_DEFINITIONS->{$type});
     }
 
+    if ($custom && !$dbh->bz_table_info($name)
+        && $type eq FIELD_TYPE_SINGLE_SELECT)
+    {
+        # Create the table that holds the legal values for this field.
+        $dbh->bz_add_field_table($name);
+        # And insert a default value of "---" into it.
+        $dbh->do("INSERT INTO $name (value) VALUES ('---')");
+    }
+
     return new Bugzilla::Field({name => $name});
 }
 
@@ -361,19 +388,45 @@ sub create_or_update {
 
 =over
 
-=item C<match($criteria)>
+=item C<match>
+
+=over
+
+=item B<Description>
 
-Description: returns a list of fields that match the specified criteria.
+Returns a list of fields that match the specified criteria.
 
-Params:    C<$criteria> - hash reference - the criteria to match against.
-           Hash keys represent field properties; hash values represent
-           their values.  All criteria are optional.  Valid criteria are
-           "custom" and "obsolete", and both take boolean values.
+You should be using L<Bugzilla/get_fields> and
+L<Bugzilla/get_custom_field_names> instead of this function.
 
-           Note: Bugzilla->get_fields() and Bugzilla->custom_field_names
-           wrap this method for most callers.
+=item B<Params>
 
-Returns:   A reference to an array of C<Bugzilla::Field> objects.
+Takes named parameters in a hashref:
+
+=over
+
+=item C<name> - The name of the field.
+
+=item C<custom> - Boolean. True to only return custom fields. False
+to only return non-custom fields. 
+
+=item C<obsolete> - Boolean. True to only return obsolete fields.
+False to not return obsolete fields.
+
+=item C<type> - The type of the field. A C<FIELD_TYPE> constant from
+L<Bugzilla::Constants>.
+
+=item C<enter_bug> - Boolean. True to only return fields that appear
+on F<enter_bug.cgi>. False to only return fields that I<don't> appear
+on F<enter_bug.cgi>.
+
+=back
+
+=item B<Returns>
+
+A reference to an array of C<Bugzilla::Field> objects.
+
+=back
 
 =back
 
@@ -395,8 +448,11 @@ sub match {
     if (defined $criteria->{enter_bug}) {
         push(@terms, "enter_bug=" . ($criteria->{enter_bug} ? '1' : '0'));
     }
+    if (defined $criteria->{type}) {
+        push(@terms, "type = " . $criteria->{type});
+    }
     my $where = (scalar(@terms) > 0) ? "WHERE " . join(" AND ", @terms) : "";
-  
+
     my $ids = Bugzilla->dbh->selectcol_arrayref(
         "SELECT id FROM fielddefs $where", {Slice => {}});
 
diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index 8de20d5f3..5d80a9d0f 100644
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -17,6 +17,7 @@
 # Everything Solved. All Rights Reserved.
 #
 # Contributor(s): Max Kanat-Alexander <mkanat@bugzilla.org>
+#                 Frédéric Buclin <LpSolit@gmail.com>
 
 use strict;
 
@@ -136,7 +137,7 @@ sub set {
     my $validators = $self->VALIDATORS;
     if (exists $validators->{$field}) {
         my $validator = $validators->{$field};
-        $value = $self->$validator($value);
+        $value = $self->$validator($value, $field);
     }
 
     $self->{$field} = $value;
@@ -196,7 +197,7 @@ sub run_create_validators {
         my $value;
         if (exists $validators->{$field}) {
             my $validator = $validators->{$field};
-            $value = $class->$validator($params->{$field});
+            $value = $class->$validator($params->{$field}, $field);
         }
         else {
             $value = $params->{$field};
@@ -328,6 +329,9 @@ a reference to the current object (what we normally call C<$self>).
 The second argument will be the value passed to L</create> or 
 L</set>for that field. 
 
+The third argument will be the name of the field being validated.
+This may be required by validators which validate several distinct fields.
+
 These functions should call L<Bugzilla::Error/ThrowUserError> if they fail.
 
 The validator must return the validated value.
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index d2d3b4978..0d72b5873 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -55,7 +55,7 @@ use constant SUMMARY_RELEVANCE_FACTOR => 7;
 # 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.
-our %specialorder = (
+use constant SPECIAL_ORDER => {
     'bugs.target_milestone' => [ 'ms_order.sortkey','ms_order.value' ],
     'bugs.bug_status' => [ 'bug_status.sortkey','bug_status.value' ],
     'bugs.rep_platform' => [ 'rep_platform.sortkey','rep_platform.value' ],
@@ -63,12 +63,12 @@ our %specialorder = (
     'bugs.op_sys' => [ 'op_sys.sortkey','op_sys.value' ],
     'bugs.resolution' => [ 'resolution.sortkey', 'resolution.value' ],
     'bugs.bug_severity' => [ 'bug_severity.sortkey','bug_severity.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.
-our %specialorderjoin = (
+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',
     'bugs.bug_status' => 'LEFT JOIN bug_status ON bug_status.value = bugs.bug_status',
     'bugs.rep_platform' => 'LEFT JOIN rep_platform ON rep_platform.value = bugs.rep_platform',
@@ -76,7 +76,7 @@ our %specialorderjoin = (
     'bugs.op_sys' => 'LEFT JOIN op_sys ON op_sys.value = bugs.op_sys',
     'bugs.resolution' => 'LEFT JOIN resolution ON resolution.value = bugs.resolution',
     'bugs.bug_severity' => 'LEFT JOIN bug_severity ON bug_severity.value = bugs.bug_severity'
-);
+};
 
 # Create a new Search
 # Note that the param argument may be modified by Bugzilla::Search
@@ -117,6 +117,18 @@ sub init {
     my @andlist;
     my %chartfields;
 
+    my %special_order      = %{SPECIAL_ORDER()};
+    my %special_order_join = %{SPECIAL_ORDER_JOIN()};
+
+    my @select_fields = Bugzilla->get_fields({ type => FIELD_TYPE_SINGLE_SELECT,
+                                               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";
+    }
+
     my $dbh = Bugzilla->dbh;
 
     # First, deal with all the old hard-coded non-chart-based poop.
@@ -213,6 +225,9 @@ sub init {
                         "assigned_to", "reporter", "component", "classification",
                         "target_milestone", "bug_group");
 
+    # Include custom select fields.
+    push(@legal_fields, map { $_->name } @select_fields);
+
     foreach my $field ($params->param()) {
         if (lsearch(\@legal_fields, $field) != -1) {
             push(@specialchart, [$field, "anyexact",
@@ -1368,7 +1383,7 @@ sub init {
         if ($orderitem =~ /\s+AS\s+(.+)$/i) {
             $orderitem = $1;
         }
-        BuildOrderBy($orderitem, \@orderby);
+        BuildOrderBy(\%special_order, $orderitem, \@orderby);
     }
     # Now JOIN the correct tables in the FROM clause.
     # This is done separately from the above because it's
@@ -1376,8 +1391,8 @@ sub init {
     foreach my $orderitem (@inputorder) {
         # Grab the part without ASC or DESC.
         my @splitfield = split(/\s+/, $orderitem);
-        if ($specialorderjoin{$splitfield[0]}) {
-            push(@supptables, $specialorderjoin{$splitfield[0]});
+        if ($special_order_join{$splitfield[0]}) {
+            push(@supptables, $special_order_join{$splitfield[0]});
         }
     }
 
@@ -1677,7 +1692,7 @@ sub IsValidQueryType
 # BuildOrderBy recursively, to let it know that we're "reversing" the 
 # order. That is, that we wanted "A DESC", not "A".
 sub BuildOrderBy {
-    my ($orderitem, $stringlist, $reverseorder) = (@_);
+    my ($special_order, $orderitem, $stringlist, $reverseorder) = (@_);
 
     my @twopart = split(/\s+/, $orderitem);
     my $orderfield = $twopart[0];
@@ -1695,11 +1710,12 @@ sub BuildOrderBy {
     }
 
     # Handle fields that have non-standard sort orders, from $specialorder.
-    if ($specialorder{$orderfield}) {
-        foreach my $subitem (@{$specialorder{$orderfield}}) {
+    if ($special_order->{$orderfield}) {
+        foreach my $subitem (@{$special_order->{$orderfield}}) {
             # DESC on a field with non-standard sort order means
             # "reverse the normal order for each field that we map to."
-            BuildOrderBy($subitem, $stringlist, $orderdirection =~ m/desc/i);
+            BuildOrderBy($special_order, $subitem, $stringlist,
+                         $orderdirection =~ m/desc/i);
         }
         return;
     }
diff --git a/editfields.cgi b/editfields.cgi
index d9f611a96..67b72e98d 100644
--- a/editfields.cgi
+++ b/editfields.cgi
@@ -41,26 +41,25 @@ print $cgi->header();
 
 # List all existing custom fields if no action is given.
 if (!$action) {
-    $vars->{'custom_fields'} = [Bugzilla->get_fields({'custom' => 1})];
-
     $template->process('admin/custom_fields/list.html.tmpl', $vars)
         || ThrowTemplateError($template->error());
 }
 # Interface to add a new custom field.
 elsif ($action eq 'add') {
-    $template->process('admin/custom_fields/create.html.tmpl')
+    $template->process('admin/custom_fields/create.html.tmpl', $vars)
         || ThrowTemplateError($template->error());
 }
 elsif ($action eq 'new') {
     my $name = clean_text($cgi->param('name') || '');
     my $desc = clean_text($cgi->param('desc') || '');
-    # For now, there is only one type available for custom fields.
-    # In the future, we will have to look at $cgi->param('type').
-    my $type = FIELD_TYPE_FREETEXT;
+    my $type = trim($cgi->param('type') || FIELD_TYPE_FREETEXT);
     my $sortkey = $cgi->param('sortkey') || 0;
 
     # Validate these fields.
     $name || ThrowUserError('customfield_missing_name');
+    # Don't want to allow a name that might mess up SQL.
+    $name =~ /^\w+$/ || ThrowUserError('customfield_invalid_name',
+                                       { name => $name });
     # Prepend cf_ to the custom field name to distinguish it from standard fields.
     if ($name !~ /^cf_/) {
         $name = 'cf_' . $name;
@@ -70,6 +69,11 @@ elsif ($action eq 'new') {
 
     $desc || ThrowUserError('customfield_missing_description', {'name' => $name});
 
+    # We hardcode valid values for $type. This doesn't matter.
+    my $typ = $type;
+    (detaint_natural($type) && $type < 3)
+      || ThrowCodeError('invalid_customfield_type', {'type' => $typ});
+
     my $skey = $sortkey;
     detaint_natural($sortkey)
       || ThrowUserError('customfield_invalid_sortkey', {'name'    => $name,
@@ -90,7 +94,6 @@ elsif ($action eq 'new') {
 
     Bugzilla::Field::create_or_update($vars);
 
-    $vars->{'custom_fields'} = [Bugzilla->get_fields({'custom' => 1})];
     $vars->{'message'} = 'custom_field_created';
 
     $template->process('admin/custom_fields/list.html.tmpl', $vars)
@@ -142,7 +145,6 @@ elsif ($action eq 'update') {
 
     Bugzilla::Field::create_or_update($vars);
 
-    $vars->{'custom_fields'} = [Bugzilla->get_fields({'custom' => 1})];
     $vars->{'message'} = 'custom_field_updated';
 
     $template->process('admin/custom_fields/list.html.tmpl', $vars)
diff --git a/editvalues.cgi b/editvalues.cgi
index dc35c40e0..2bcbf099f 100755
--- a/editvalues.cgi
+++ b/editvalues.cgi
@@ -35,6 +35,12 @@ use Bugzilla::Config qw(:admin);
 our @valid_fields = ('op_sys', 'rep_platform', 'priority', 'bug_severity',
                      'resolution');
 
+# Add custom select fields.
+my @custom_fields = Bugzilla->get_fields({custom => 1,
+                                          type => FIELD_TYPE_SINGLE_SELECT});
+
+push(@valid_fields, map { $_->name } @custom_fields);
+
 ######################################################################
 # Subroutines
 ######################################################################
@@ -128,6 +134,7 @@ $defaults{'bug_severity'} = 'defaultseverity';
 # In this case, only the sortkey can be altered.
 my %static;
 $static{'resolution'} = ['', 'FIXED', 'MOVED', 'DUPLICATE'];
+$static{$_->name} = ['---'] foreach (@custom_fields);
 
 #
 # field = '' -> Show nice list of fields
diff --git a/post_bug.cgi b/post_bug.cgi
index dffec2665..390718389 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -211,8 +211,16 @@ foreach my $group (@$groups) {
     }
 }
 
-my @bug_fields = map {$_->name} Bugzilla->get_fields(
-    { custom => 1, obsolete => 0, enter_bug => 1});
+# Include custom fields editable on bug creation.
+my @custom_bug_fields = Bugzilla->get_fields(
+    { custom => 1, obsolete => 0, enter_bug => 1 });
+
+my @bug_fields = map { $_->name } @custom_bug_fields;
+
+# Custom tables must be locked (required when validating custom fields).
+my @custom_tables = grep { $_->type == FIELD_TYPE_SINGLE_SELECT } @custom_bug_fields;
+@custom_tables = map { $_->name . ' READ' } @custom_tables;
+
 push(@bug_fields, qw(
     product
     component
@@ -251,7 +259,7 @@ $dbh->bz_lock_tables('bugs WRITE', 'bug_group_map WRITE', 'longdescs WRITE',
                      'products READ', 'versions READ', 'milestones READ',
                      'components READ', 'profiles READ', 'bug_severity READ',
                      'op_sys READ', 'priority READ', 'rep_platform READ',
-                     'group_control_map READ');
+                     'group_control_map READ', @custom_tables);
 
 my $bug = Bugzilla::Bug->create(\%bug_params);
 
diff --git a/process_bug.cgi b/process_bug.cgi
index bc49ce01f..4ca02b17b 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -775,14 +775,16 @@ foreach my $field ("rep_platform", "priority", "bug_severity",
 }
 
 # Add custom fields data to the query that will update the database.
-foreach my $field (Bugzilla->custom_field_names) {
-    if (defined $cgi->param($field)
+foreach my $field (Bugzilla->get_fields({custom => 1, obsolete => 0})) {
+    my $fname = $field->name;
+    if (defined $cgi->param($fname)
         && (!$cgi->param('dontchange')
-            || $cgi->param($field) ne $cgi->param('dontchange')))
+            || $cgi->param($fname) ne $cgi->param('dontchange')))
     {
         DoComma();
-        $::query .= "$field = ?";
-        my $value = $cgi->param($field);
+        $::query .= "$fname = ?";
+        my $value = $cgi->param($fname);
+        check_field($fname, $value) if ($field->type == FIELD_TYPE_SINGLE_SELECT);
         trick_taint($value);
         push(@values, $value);
     }
diff --git a/template/en/default/admin/custom_fields/create.html.tmpl b/template/en/default/admin/custom_fields/create.html.tmpl
index b366371bd..e8b66deca 100644
--- a/template/en/default/admin/custom_fields/create.html.tmpl
+++ b/template/en/default/admin/custom_fields/create.html.tmpl
@@ -79,10 +79,11 @@
     <tr>
       <th align="right"><label for="type">Type:</label></th>
       <td>
-        [%# Only one field type is valid right now. But let's prepare the UI
-          # for future new types. %]
         <select id="type" name="type">
-          <option value="FIELD_TYPE_FREETEXT">Free Text</option>
+          [% FOREACH type = field_types.keys %]
+            [% NEXT IF type == constants.FIELD_TYPE_UNKNOWN %]
+            <option value="[% type FILTER html %]">[% field_types.$type FILTER html %]</option>
+          [% END %]
         </select>
       </td>
 
diff --git a/template/en/default/admin/custom_fields/edit.html.tmpl b/template/en/default/admin/custom_fields/edit.html.tmpl
index def825e7c..6ffa3d89d 100644
--- a/template/en/default/admin/custom_fields/edit.html.tmpl
+++ b/template/en/default/admin/custom_fields/edit.html.tmpl
@@ -69,7 +69,7 @@
     </tr>
     <tr>
       <th align="right">Type:</th>
-      <td>Free Text</td>
+      <td>[% field_types.${field.type} FILTER html %]</td>
 
       <th align="right"><label for="obsolete">Is obsolete:</label></th>
       <td><input type="checkbox" id="obsolete" name="obsolete" value="1"
@@ -85,6 +85,15 @@
       <th>&nbsp;</th>
       <td>&nbsp;</td>
     </tr>
+    [% IF field.type == constants.FIELD_TYPE_SINGLE_SELECT %]
+      <tr>
+        <th>&nbsp;</th>
+        <td colspan="3">
+          <a href="editvalues.cgi?field=[% field.name FILTER url_quote %]">Edit
+            legal values for this field</a>.
+        </td>
+      </tr>
+    [% END %]
   </table>
   <br>
   <input type="hidden" name="action" value="update">
diff --git a/template/en/default/admin/custom_fields/list.html.tmpl b/template/en/default/admin/custom_fields/list.html.tmpl
index e02609dd3..2f64b0f06 100644
--- a/template/en/default/admin/custom_fields/list.html.tmpl
+++ b/template/en/default/admin/custom_fields/list.html.tmpl
@@ -37,6 +37,10 @@
        heading => "Sortkey"
      },
      {
+       name => "type_name"
+       heading => "Type"
+     },
+     {
        name => "enter_bug"
        heading => "Editable on Bug Creation"
      },
@@ -51,6 +55,14 @@
    ]
 %]
 
+[% USE Bugzilla %]
+[% custom_fields = Bugzilla.get_fields({ custom => 1 }) %]
+
+[%# We want to display the type name of fields, not their type ID. %]
+[% FOREACH cf_field = custom_fields %]
+  [% cf_field.type_name = field_types.${cf_field.type} %]
+[% END %]
+
 [% PROCESS admin/table.html.tmpl
      columns = columns
      data = custom_fields
diff --git a/template/en/default/bug/field.html.tmpl b/template/en/default/bug/field.html.tmpl
index 59020518b..008674d60 100644
--- a/template/en/default/bug/field.html.tmpl
+++ b/template/en/default/bug/field.html.tmpl
@@ -42,6 +42,14 @@
   [% SWITCH field.type %]
     [% CASE constants.FIELD_TYPE_FREETEXT %]
         <input name="[% field.name FILTER html %]" value="[% value FILTER html %]" size="60">
+    [% CASE constants.FIELD_TYPE_SINGLE_SELECT %]
+        <select id="[% field.name FILTER html %]" name="[% field.name FILTER html %]">
+          [% FOREACH legal_value = field.legal_values %]
+            <option value="[% legal_value FILTER html %]"
+                [%- " selected=\"selected\"" IF value == legal_value %]>
+                [%- legal_value FILTER html %]</option>
+          [% END %]
+        </select>
   [% END %]
 [% ELSE %]
   [% value FILTER html %]
diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl
index 9c504b827..b0d7fcda8 100644
--- a/template/en/default/global/code-error.html.tmpl
+++ b/template/en/default/global/code-error.html.tmpl
@@ -170,6 +170,10 @@
      The custom sort order specified in your form submission contains an
      invalid column name <em>[% fragment FILTER html %]</em>.
 
+  [% ELSIF error == "invalid_customfield_type" %]
+    [% title = "Invalid Field Type" %]
+    The type <em>[% type FILTER html %]</em> is not a valid field type.
+
   [% ELSIF error == "invalid_dimensions" %]
     [% title = "Invalid Dimensions" %]
     The width or height specified is not a positive integer.
diff --git a/template/en/default/global/field-descs.none.tmpl b/template/en/default/global/field-descs.none.tmpl
index 254bfef1d..94ba948d6 100644
--- a/template/en/default/global/field-descs.none.tmpl
+++ b/template/en/default/global/field-descs.none.tmpl
@@ -80,6 +80,10 @@
      IF !field_descs.${bz_field.name}.defined %]
 [% END %]
 
+[% field_types = { ${constants.FIELD_TYPE_UNKNOWN}       => "Unknown Type",
+                   ${constants.FIELD_TYPE_FREETEXT}      => "Free Text",
+                   ${constants.FIELD_TYPE_SINGLE_SELECT} => "Drop Down" } %]
+
 [% status_descs = { "UNCONFIRMED" => "UNCONFIRMED",
                     "NEW"         => "NEW",
                     "ASSIGNED"    => "ASSIGNED",
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 9a0d04555..a9706376b 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -314,6 +314,11 @@
     The field '[% field.name FILTER html %]' ([% field.description FILTER html %])
     already exists. Please choose another name.
 
+  [% ELSIF error == "customfield_invalid_name" %]
+    [% title = "Invalid Custom Field Name" %]
+    '[% name FILTER html %]' is not a valid name for a custom field.
+    A name may contain only letters, numbers, and the underscore character.
+
   [% ELSIF error == "customfield_nonexistent" %]
     [% title = "Unknown Custom Field" %]
     There is no custom field with the name '[% name FILTER html %]'.