Bug 349369: Allow unused custom fields to be deleted from editfields.cgi - Patch by Alex Eiser <aeiser@arc.nasa.gov> r/a=LpSolit

8 files changed, 184 insertions, 4 deletions

diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index 4a0303287..6763da06b 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -687,6 +687,13 @@ sub bz_add_field_tables {
 
 }
 
+sub bz_drop_field_tables {
+    my ($self, $field) = @_;
+    if ($field->type == FIELD_TYPE_MULTI_SELECT) {
+        $self->bz_drop_table('bug_' . $field->name);
+    }
+    $self->bz_drop_table($field->name);
+}
 
 sub bz_drop_column {
     my ($self, $table, $column) = @_;
diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm
index 5ad5e51d3..5272f0ed6 100644
--- a/Bugzilla/Field.pm
+++ b/Bugzilla/Field.pm
@@ -415,6 +415,89 @@ sub set_in_new_bugmail { $_[0]->set('mailhead',    $_[1]); }
 
 =pod
 
+=head2 Instance Method
+
+=over
+
+=item C<remove_from_db>
+
+Attempts to remove the passed in field from the database.
+Deleting a field is only successful if the field is obsolete and
+there are no values specified (or EVER specified) for the field.
+
+=back
+
+=cut
+
+sub remove_from_db {
+    my $self = shift;
+    my $dbh = Bugzilla->dbh;
+
+    my $name = $self->name;
+
+    if (!$self->custom) {
+        ThrowCodeError('field_not_custom', {'name' => $name });
+    }
+
+    if (!$self->obsolete) {
+        ThrowUserError('customfield_not_obsolete', {'name' => $self->name });
+    }
+
+    $dbh->bz_start_transaction();
+
+    # Check to see if bug activity table has records (should be fast with index)
+    my $has_activity = $dbh->selectrow_array("SELECT COUNT(*) FROM bugs_activity
+                                      WHERE fieldid = ?", undef, $self->id);
+    if ($has_activity) {
+        ThrowUserError('customfield_has_activity', {'name' => $name });
+    }
+
+    # Check to see if bugs table has records (slow)
+    my $bugs_query = "";
+
+    if ($self->type == FIELD_TYPE_MULTI_SELECT) {
+        $bugs_query = "SELECT COUNT(*) FROM bug_$name";
+    }
+    else {
+        $bugs_query = "SELECT COUNT(*) FROM bugs WHERE $name IS NOT NULL
+                                AND $name != ''";
+        # Ignore the default single select value
+        if ($self->type == FIELD_TYPE_SINGLE_SELECT) {
+            $bugs_query .= " AND $name != '---'";
+        }
+        # Ignore blank dates.
+        if ($self->type == FIELD_TYPE_DATETIME) {
+            $bugs_query .= " AND $name != '00-00-00 00:00:00'";
+        }
+    }
+
+    my $has_bugs = $dbh->selectrow_array($bugs_query);
+    if ($has_bugs) {
+        ThrowUserError('customfield_has_contents', {'name' => $name });
+    }
+
+    # Once we reach here, we should be OK to delete.
+    $dbh->do('DELETE FROM fielddefs WHERE id = ?', undef, $self->id);
+
+    my $type = $self->type;
+
+    # the values for multi-select are stored in a seperate table
+    if ($type != FIELD_TYPE_MULTI_SELECT) {
+        $dbh->bz_drop_column('bugs', $name);
+    }
+
+    if ($type == FIELD_TYPE_SINGLE_SELECT
+        || $type == FIELD_TYPE_MULTI_SELECT)
+    {
+        # Delete the table that holds the legal values for this field.
+        $dbh->bz_drop_field_tables($self);
+    }
+
+    $dbh->bz_commit_transaction()
+}
+
+=pod
+
 =head2 Class Methods
 
 =over
diff --git a/docs/xml/administration.xml b/docs/xml/administration.xml
index 6a692c264..b7897215d 100644
--- a/docs/xml/administration.xml
+++ b/docs/xml/administration.xml
@@ -1679,10 +1679,10 @@ foo: ENTRY, MANDATORY/MANDATORY, CANEDIT
       <title>Deleting Custom Fields</title>
 
       <para>
-        At this point, it is not possible to delete custom fields from
-        your web browser. If you don't want to make one available anymore,
-        mark it as obsolete. This way, you will preserve your DB
-        referential integrity.
+        It is only possible to delete obsolete custom fields 
+        if the field has never been used in the database.
+        If you want to remove a field which already has content,
+        mark it as obsolete.
       </para>
     </section>
   </section>
diff --git a/editfields.cgi b/editfields.cgi
index 50564c190..138c6b729 100644
--- a/editfields.cgi
+++ b/editfields.cgi
@@ -117,6 +117,49 @@ elsif ($action eq 'update') {
     $template->process('admin/custom_fields/list.html.tmpl', $vars)
         || ThrowTemplateError($template->error());
 }
+elsif ($action eq 'del') {
+    my $name = $cgi->param('name');
+
+    # Validate field.
+    $name || ThrowUserError('field_missing_name');
+    # Custom field names must start with "cf_".
+    if ($name !~ /^cf_/) {
+        $name = 'cf_' . $name;
+    }
+    my $field = new Bugzilla::Field({'name' => $name});
+    $field || ThrowUserError('customfield_nonexistent', {'name' => $name});
+
+    $vars->{'field'} = $field;
+    $vars->{'token'} = issue_session_token('delete_field');
+
+    $template->process('admin/custom_fields/confirm-delete.html.tmpl', $vars)
+            || ThrowTemplateError($template->error());
+}
+elsif ($action eq 'delete') {
+    check_token_data($token, 'delete_field');
+    my $name = $cgi->param('name');
+
+    # Validate fields.
+    $name || ThrowUserError('field_missing_name');
+    # Custom field names must start with "cf_".
+    if ($name !~ /^cf_/) {
+        $name = 'cf_' . $name;
+    }
+    my $field = new Bugzilla::Field({'name' => $name});
+    $field || ThrowUserError('customfield_nonexistent', {'name' => $name});
+
+    # Calling remove_from_db will check if field can be deleted.
+    # If the field cannot be deleted, it will throw an error.
+    $field->remove_from_db();
+    
+    $vars->{'field'}   = $field;
+    $vars->{'message'} = 'custom_field_deleted';
+    
+    delete_token($token);
+
+    $template->process('admin/custom_fields/list.html.tmpl', $vars)
+        || ThrowTemplateError($template->error());
+}
 else {
     ThrowUserError('no_valid_action', {'field' => 'custom_field'});
 }
diff --git a/template/en/default/admin/custom_fields/edit.html.tmpl b/template/en/default/admin/custom_fields/edit.html.tmpl
index 596e7e704..02334ab13 100644
--- a/template/en/default/admin/custom_fields/edit.html.tmpl
+++ b/template/en/default/admin/custom_fields/edit.html.tmpl
@@ -104,6 +104,15 @@
   <input type="submit" id="edit" value="Submit">
 </form>
 
+[% IF field.obsolete %]
+<p>
+  <a href="editfields.cgi?action=del&amp;name=[% field.name FILTER html %]">Remove
+    this custom field from the database.</a><br>
+  This action will only be successful if the custom field has never been used
+  in [% terms.abug %].<br>
+</p>
+[% END %]
+
 <p>
   <a href="editfields.cgi">Back to the list of existing custom fields</a>
 </p>
diff --git a/template/en/default/admin/custom_fields/list.html.tmpl b/template/en/default/admin/custom_fields/list.html.tmpl
index acb3f680d..6f2e68be7 100644
--- a/template/en/default/admin/custom_fields/list.html.tmpl
+++ b/template/en/default/admin/custom_fields/list.html.tmpl
@@ -24,6 +24,8 @@
   doc_section = "custom-fields.html"
 %]
 
+[% delete_contentlink = BLOCK %]editfields.cgi?action=del&amp;name=%%name%%[% END %]
+
 [% columns = [
      {
        name => "name"
@@ -53,6 +55,11 @@
      {
        name => "obsolete"
        heading => "Is Obsolete"
+     },
+     {
+       name => "action"
+       heading => "Action"
+       content => ""
      }
    ]
 %]
@@ -73,6 +80,17 @@
   %]
 [% END %]
 
+
+[% overrides.action = [ {
+     match_value => 1
+     match_field => 'obsolete'
+     override_content => 1
+     content => "Delete"
+     override_contentlink => 1
+     contentlink => delete_contentlink
+   } ]
+%] 
+
 [% PROCESS admin/table.html.tmpl
      columns = columns
      overrides = overrides
diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl
index 007aab4d4..dfa4f78f4 100644
--- a/template/en/default/global/messages.html.tmpl
+++ b/template/en/default/global/messages.html.tmpl
@@ -254,6 +254,11 @@
     The new custom field '[% field.name FILTER html %]' has been
     successfully created.
 
+  [% ELSIF message_tag == "custom_field_deleted" %]
+    [% title = "Custom Field Deleted" %]
+    The custom field '[% field.name FILTER html %]' has been
+    successfully deleted.
+
   [% ELSIF message_tag == "custom_field_updated" %]
     [% title = "Custom Field Updated" %]
     Properties of the '[% field.name FILTER html %]' field have been
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 73d0f1d1f..8b3b5e20e 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -328,6 +328,21 @@
     [% title = "Unknown Custom Field" %]
     There is no custom field with the name '[% name FILTER html %]'.
 
+  [% ELSIF error == "customfield_not_obsolete" %]
+    [% title = "Custom Field Not Obsolete" %]
+    The custom field '[% name FILTER html %]' is not obsolete.
+    Please obsolete a custom field before attempting to delete it.
+
+  [% ELSIF error == "customfield_has_activity" %]
+    [% title = "Custom Field Has Activity" %]
+    The custom field '[% name FILTER html %]' cannot be deleted because
+    it has recorded activity.
+
+  [% ELSIF error == "customfield_has_contents" %]
+    [% title = "Custom Field Has Contents" %]
+    The custom field '[% name FILTER html %]' cannot be deleted because
+    at least one [% terms.bug %] has a non empty value for this field.
+
   [% ELSIF error == "dependency_loop_multi" %]
     [% title = "Dependency Loop Detected" %]
     The following [% terms.bug %](s) would appear on both the "depends on"