Bug 350307: Split out the create and update functionality of Bugzilla::Field::create_or_update

Bugzilla/DB.pm

@@ -587,6 +587,8 @@ sub _bz_add_table_raw {
 sub bz_add_field_table {
     my ($self, $name) = @_;
     my $table_schema = $self->_bz_schema->FIELD_TABLE_SCHEMA;
+    # We do nothing if the table already exists.
+    return if $self->bz_table_info($name);
     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.

checksetup.pl

@@ -199,7 +199,7 @@ check_graphviz(!$silent) if Bugzilla->params->{'webdotbase'};
 # Changes to the fielddefs --TABLE--
 ###########################################################################
 
-# Calling Bugzilla::Field::create_or_update depends on the
+# Using Bugzilla::Field's create() or update() depends on the
 # fielddefs table having a modern definition. So, we have to make
 # these particular schema changes before we make any other schema changes.
 Bugzilla::Install::DB::update_fielddefs_definition();

editfields.cgi

@@ -55,49 +55,18 @@ elsif ($action eq 'add') {
 }
 elsif ($action eq 'new') {
     check_token_data($token, 'add_field');
-    my $name = clean_text($cgi->param('name') || '');
-    my $desc = clean_text($cgi->param('desc') || '');
-    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+$/ && $name ne "cf_"
-      || 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;
-    }
-    my $field = new Bugzilla::Field({'name' => $name});
-    ThrowUserError('customfield_already_exists', {'field' => $field }) if $field;
-
-    $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,
-                                                        'sortkey' => $skey});
-
-    # All fields have been validated. We can create this new custom field.
-    trick_taint($name);
-    trick_taint($desc);
-
-    $vars->{'name'} = $name;
-    $vars->{'desc'} = $desc;
-    $vars->{'sortkey'} = $sortkey;
-    $vars->{'type'} = $type;
-    $vars->{'custom'} = 1;
-    $vars->{'in_new_bugmail'} = $cgi->param('new_bugmail') ? 1 : 0;
-    $vars->{'editable_on_enter_bug'} = $cgi->param('enter_bug') ? 1 : 0;
-    $vars->{'is_obsolete'} = $cgi->param('obsolete') ? 1 : 0;
-
-    Bugzilla::Field::create_or_update($vars);
+
+    $vars->{'field'} = Bugzilla::Field->create({
+        name        => scalar $cgi->param('name'),
+        description => scalar $cgi->param('desc'),
+        type        => scalar $cgi->param('type'),
+        sortkey     => scalar $cgi->param('sortkey'),
+        mailhead    => scalar $cgi->param('new_bugmail'),
+        enter_bug   => scalar $cgi->param('enter_bug'),
+        obsolete    => scalar $cgi->param('obsolete'),
+        custom      => 1,
+    });
+
     delete_token($token);
 
     $vars->{'message'} = 'custom_field_created';
@@ -106,7 +75,7 @@ elsif ($action eq 'new') {
         || ThrowTemplateError($template->error());
 }
 elsif ($action eq 'edit') {
-    my $name = $cgi->param('name') || ThrowUserError('customfield_missing_name');
+    my $name = $cgi->param('name') || ThrowUserError('field_missing_name');
     # Custom field names must start with "cf_".
     if ($name !~ /^cf_/) {
         $name = 'cf_' . $name;
@@ -123,11 +92,9 @@ elsif ($action eq 'edit') {
 elsif ($action eq 'update') {
     check_token_data($token, 'edit_field');
     my $name = $cgi->param('name');
-    my $desc = clean_text($cgi->param('desc') || '');
-    my $sortkey = $cgi->param('sortkey') || 0;
 
     # Validate fields.
-    $name || ThrowUserError('customfield_missing_name');
+    $name || ThrowUserError('field_missing_name');
     # Custom field names must start with "cf_".
     if ($name !~ /^cf_/) {
         $name = 'cf_' . $name;
@@ -135,25 +102,16 @@ elsif ($action eq 'update') {
     my $field = new Bugzilla::Field({'name' => $name});
     $field || ThrowUserError('customfield_nonexistent', {'name' => $name});
 
-    $desc || ThrowUserError('customfield_missing_description', {'name' => $name});
-    trick_taint($desc);
-
-    my $skey = $sortkey;
-    detaint_natural($sortkey)
-      || ThrowUserError('customfield_invalid_sortkey', {'name'    => $name,
-                                                        'sortkey' => $skey});
-
-    $vars->{'name'} = $field->name;
-    $vars->{'desc'} = $desc;
-    $vars->{'sortkey'} = $sortkey;
-    $vars->{'custom'} = 1;
-    $vars->{'in_new_bugmail'} = $cgi->param('new_bugmail') ? 1 : 0;
-    $vars->{'editable_on_enter_bug'} = $cgi->param('enter_bug') ? 1 : 0;
-    $vars->{'is_obsolete'} = $cgi->param('obsolete') ? 1 : 0;
+    $field->set_description($cgi->param('desc'));
+    $field->set_sortkey($cgi->param('sortkey'));
+    $field->set_in_new_bugmail($cgi->param('new_bugmail'));
+    $field->set_enter_bug($cgi->param('enter_bug'));
+    $field->set_obsolete($cgi->param('obsolete'));
+    $field->update();
 
-    Bugzilla::Field::create_or_update($vars);
     delete_token($token);
 
+    $vars->{'field'}   = $field;
     $vars->{'message'} = 'custom_field_updated';
 
     $template->process('admin/custom_fields/list.html.tmpl', $vars)

template/en/default/admin/custom_fields/create.html.tmpl

@@ -93,7 +93,7 @@
     <tr>
       <th align="right"><label for="sortkey">Sortkey:</label></th>
       <td>
-        <input type="text" id="sortkey" name="sortkey" value="0" size="6" maxlength="6">
+        <input type="text" id="sortkey" name="sortkey" size="6" maxlength="6">
       </td>
 
       <th>&nbsp;</th>

template/en/default/global/messages.html.tmpl

@@ -168,12 +168,12 @@
 
   [% ELSIF message_tag == "custom_field_created" %]
     [% title = "Custom Field Created" %]
-    The new custom field '[% name FILTER html %]' has been
+    The new custom field '[% field.name FILTER html %]' has been
     successfully created.
 
   [% ELSIF message_tag == "custom_field_updated" %]
     [% title = "Custom Field Updated" %]
-    Properties of the '[% name FILTER html %]' field have been
+    Properties of the '[% field.name FILTER html %]' field have been
     successfully updated.
 
   [% ELSIF message_tag == "emailold_change_cancelled" %]

template/en/default/global/user-error.html.tmpl

@@ -311,34 +311,10 @@
     Product [% product FILTER html %] does not have a component
     named [% name FILTER html %].
 
-  [% ELSIF error == "customfield_already_exists" %]
-    [% title = "Field Already Exists" %]
-    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. The
-    name should also be different from 'cf_'.
-
   [% ELSIF error == "customfield_nonexistent" %]
     [% title = "Unknown Custom Field" %]
     There is no custom field with the name '[% name FILTER html %]'.
 
-  [% ELSIF error == "customfield_invalid_sortkey" %]
-    [% title = "Invalid Sortkey for Field" %]
-    The sortkey [% sortkey FILTER html %] that you have provided for
-    the '[% name FILTER html %]' field is not a valid positive integer.
-
-  [% ELSIF error == "customfield_missing_description" %]
-    [% title = "Missing Description for Field" %]
-    You must enter a description for the '[% name FILTER html %]' field.
-
-  [% ELSIF error == "customfield_missing_name" %]
-    [% title = "Missing Name for Field" %]
-    You must enter a name for this field.
-
   [% ELSIF error == "dependency_loop_multi" %]
     [% title = "Dependency Loop Detected" %]
     The following [% terms.bug %](s) would appear on both the "depends on"
@@ -400,6 +376,30 @@
     does not exist or you aren't authorized to
     enter [% terms.abug %] into it.
 
+  [% ELSIF error == "field_already_exists" %]
+    [% title = "Field Already Exists" %]
+    The field '[% field.name FILTER html %]'     
+    ([% field.description FILTER html %]) already exists. Please
+    choose another name.
+
+  [% ELSIF error == "field_invalid_name" %]
+    [% title = "Invalid Field Name" %]
+    '[% name FILTER html %]' is not a valid name for a field.
+    A name may contain only letters, numbers, and the underscore character.
+
+  [% ELSIF error == "field_invalid_sortkey" %]
+    [% title = "Invalid Sortkey for Field" %]
+    The sortkey [% sortkey FILTER html %] that you have provided for
+    this field is not a valid positive integer.
+
+  [% ELSIF error == "field_missing_description" %]
+    [% title = "Missing Description for Field" %]
+    You must enter a description for this field.
+
+  [% ELSIF error == "field_missing_name" %]
+    [% title = "Missing Name for Field" %]
+    You must enter a name for this field.
+
   [% ELSIF error == "fieldname_invalid" %]
     [% title = "Specified Field Does Not Exist" %]
     The field '[% field FILTER html %]' does not exist or