From d4f1fd5168130441b735497bac94810a3ccc34c3 Mon Sep 17 00:00:00 2001 From: "lpsolit%gmail.com" <> Date: Wed, 6 Feb 2008 22:15:34 +0000 Subject: Bug 349369: Allow unused custom fields to be deleted from editfields.cgi - Patch by Alex Eiser r/a=LpSolit --- Bugzilla/DB.pm | 7 ++ Bugzilla/Field.pm | 83 ++++++++++++++++++++++ docs/xml/administration.xml | 8 +-- editfields.cgi | 43 +++++++++++ .../en/default/admin/custom_fields/edit.html.tmpl | 9 +++ .../en/default/admin/custom_fields/list.html.tmpl | 18 +++++ template/en/default/global/messages.html.tmpl | 5 ++ template/en/default/global/user-error.html.tmpl | 15 ++++ 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 + +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 Deleting Custom Fields - 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. 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 @@ +[% IF field.obsolete %] +

+ Remove + this custom field from the database.
+ This action will only be successful if the custom field has never been used + in [% terms.abug %].
+

+[% END %] +

Back to the list of existing custom fields

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&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" -- cgit v1.2.3-24-g4f1b