diff options
| -rw-r--r-- | Bugzilla/DB.pm | 2 | ||||
| -rw-r--r-- | Bugzilla/Field.pm | 260 | ||||
| -rwxr-xr-x | checksetup.pl | 2 | ||||
| -rw-r--r-- | editfields.cgi | 84 | ||||
| -rw-r--r-- | template/en/default/admin/custom_fields/create.html.tmpl | 2 | ||||
| -rw-r--r-- | template/en/default/global/messages.html.tmpl | 4 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 48 |
7 files changed, 237 insertions, 165 deletions
diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm index f851e4533..cefd361ae 100644 --- a/Bugzilla/DB.pm +++ b/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. diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm index 602ab5e4d..782ee5581 100644 --- a/Bugzilla/Field.pm +++ b/Bugzilla/Field.pm @@ -43,8 +43,8 @@ Bugzilla::Field - a particular piece of information about bugs print Dumper(Bugzilla::Field->match({ obsolete => 1, custom => 1 })); # Create or update a custom field or field definition. - my $field = Bugzilla::Field::create_or_update( - {name => 'hilarity', desc => 'Hilarity', custom => 1}); + my $field = Bugzilla::Field->create( + {name => 'cf_silly', description => 'Silly', custom => 1}); # Instantiate a Field object for an existing field. my $field = new Bugzilla::Field({name => 'qacontact_accessible'}); @@ -99,6 +99,26 @@ use constant DB_COLUMNS => ( 'enter_bug', ); +use constant REQUIRED_CREATE_FIELDS => qw(name description); + +use constant VALIDATORS => { + custom => \&_check_custom, + description => \&_check_description, + enter_bug => \&_check_enter_bug, + mailhead => \&_check_mailhead, + obsolete => \&_check_obsolete, + sortkey => \&_check_sortkey, + type => \&_check_type, +}; + +use constant UPDATE_COLUMNS => qw( + description + mailhead + sortkey + obsolete + enter_bug +); + # How various field types translate into SQL data definitions. use constant SQL_DEFINITIONS => { # Using commas because these are constants and they shouldn't @@ -168,6 +188,76 @@ use constant DEFAULT_FIELDS => ( {name => "owner_idle_time", desc => "Time Since Assignee Touched"}, ); +############## +# Validators # +############## + +sub _check_custom { return $_[1] ? 1 : 0; } + +sub _check_description { + my ($invocant, $desc) = @_; + $desc = clean_text($desc); + $desc || ThrowUserError('field_missing_description'); + return $desc; +} + +sub _check_enter_bug { return $_[1] ? 1 : 0; } + +sub _check_mailhead { return $_[1] ? 1 : 0; } + +sub _check_name { + my ($invocant, $name, $is_custom) = @_; + $name = clean_text($name); + $name || ThrowUserError('field_missing_name'); + + # Don't want to allow a name that might mess up SQL. + my $name_regex = qr/^[\w\.]+$/; + # Custom fields have more restrictive name requirements than + # standard fields. + $name_regex = qr/^\w+$/ if $is_custom; + # Custom fields can't be named just "cf_", and there is no normal + # field named just "cf_". + ($name =~ $name_regex && $name ne "cf_") + || ThrowUserError('field_invalid_name', { name => $name }); + + # If it's custom, prepend cf_ to the custom field name to distinguish + # it from standard fields. + if ($name !~ /^cf_/ && $is_custom) { + $name = 'cf_' . $name; + } + + # Assure the name is unique. Names can't be changed, so we don't have + # to worry about what to do on updates. + my $field = new Bugzilla::Field({ name => $name }); + ThrowUserError('field_already_exists', {'field' => $field }) if $field; + + return $name; +} + +sub _check_obsolete { return $_[1] ? 1 : 0; } + +sub _check_sortkey { + my ($invocant, $sortkey) = @_; + my $skey = $sortkey; + if (!defined $skey || $skey eq '') { + ($sortkey) = Bugzilla->dbh->selectrow_array( + 'SELECT MAX(sortkey) + 100 FROM fielddefs') || 100; + } + detaint_natural($sortkey) + || ThrowUserError('field_invalid_sortkey', { sortkey => $skey }); + return $sortkey; +} + +sub _check_type { + my ($invocant, $type) = @_; + my $saved_type = $type; + # FIELD_TYPE_SINGLE_SELECT here should be updated every time a new, + # higher field type is added. + (detaint_natural($type) && $type <= FIELD_TYPE_SINGLE_SELECT) + || ThrowCodeError('invalid_customfield_type', { type => $saved_type }); + return $type; +} + =pod =head2 Instance Properties @@ -290,98 +380,105 @@ sub legal_values { =pod -=head2 Class Methods +=head2 Instance Mutators + +These set the particular field that they are named after. + +They take a single value--the new value for that field. + +They will throw an error if you try to set the values to something invalid. =over -=item C<create_or_update({name => $name, desc => $desc, in_new_bugmail => 0, custom => 0})> +=item C<set_description> -Description: Creates a new field, or updates one that - already exists with the same name. +=item C<set_enter_bug> -Params: This function takes named parameters in a hashref: - C<name> - string - The name of the field. - C<desc> - string - The field label to display in the UI. - C<in_new_bugmail> - boolean - Whether this field appears at the - top of the bugmail for a newly-filed bug. +=item C<set_obsolete> - The following parameters are optional: - C<custom> - boolean - True if this is a Custom Field. The field - will be added to the C<bugs> table if it does not exist. - C<sortkey> - integer - The sortkey of the field. - C<editable_on_enter_bug> - boolean - Whether this field is - editable on the bug creation form. - C<is_obsolete> - boolean - Whether this field is obsolete. +=item C<set_sortkey> -Returns: a C<Bugzilla::Field> object. +=item C<set_in_new_bugmail> =back =cut -sub create_or_update { - my ($params) = @_; - my $dbh = Bugzilla->dbh; +sub set_description { $_[0]->set('description', $_[1]); } +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]); } - my $name = $params->{name}; - my $custom = $params->{custom} ? 1 : 0; - my $in_new_bugmail = $params->{in_new_bugmail} ? 1 : 0; - my $type = $params->{type} || FIELD_TYPE_UNKNOWN; +=pod - my $field = new Bugzilla::Field({name => $name}); - if ($field) { - # Both fields are mandatory. - my @columns = ('description', 'mailhead'); - my @values = ($params->{desc}, $in_new_bugmail); +=head2 Class Methods - if (exists $params->{sortkey}) { - push(@columns, 'sortkey'); - push(@values, $params->{sortkey} || 0); - } - if (exists $params->{editable_on_enter_bug}) { - push(@columns, 'enter_bug'); - push(@values, $params->{editable_on_enter_bug} ? 1 : 0); - } - if (exists $params->{is_obsolete}) { - push(@columns, 'obsolete'); - push(@values, $params->{is_obsolete} ? 1 : 0); - } - my $columns = join(', ', map {"$_ = ?"} @columns); - # Update the already-existing definition. - $dbh->do("UPDATE fielddefs SET $columns WHERE id = ?", - undef, (@values, $field->id)); - } - else { - my $sortkey = $params->{sortkey} || 0; - my $enter_bug = $params->{editable_on_enter_bug} ? 1 : 0; - my $is_obsolete = $params->{is_obsolete} ? 1 : 0; +=over - $sortkey ||= $dbh->selectrow_array( - "SELECT MAX(sortkey) + 100 FROM fielddefs") || 100; +=item C<create> - # Add the field to the list of fields at this Bugzilla installation. - $dbh->do("INSERT INTO fielddefs (name, description, sortkey, type, - custom, mailhead, obsolete, enter_bug) - VALUES (?, ?, ?, ?, ?, ?, ?, ?)", undef, - $name, $params->{desc}, $sortkey, $type, $custom, - $in_new_bugmail, $is_obsolete, $enter_bug); - } +Just like L<Bugzilla::Object/create>. Takes the following parameters: + +=over + +=item C<name> B<Required> - The name of the field. + +=item C<description> B<Required> - The field label to display in the UI. + +=item C<mailhead> - boolean - Whether this field appears at the +top of the bugmail for a newly-filed bug. Defaults to 0. + +=item C<custom> - boolean - True if this is a Custom Field. The field +will be added to the C<bugs> table if it does not exist. Defaults to 0. + +=item C<sortkey> - integer - The sortkey of the field. Defaults to 0. + +=item C<enter_bug> - boolean - Whether this field is +editable on the bug creation form. Defaults to 0. + +C<obsolete> - boolean - Whether this field is obsolete. Defaults to 0. + +=back + +=back + +=cut + +sub create { + my $class = shift; + my $field = $class->SUPER::create(@_); - if (!$dbh->bz_column_info('bugs', $name) && $custom) { + my $dbh = Bugzilla->dbh; + if ($field->custom) { + my $name = $field->name; + my $type = $field->type; # Create the database column that stores the data for this field. $dbh->bz_add_column('bugs', $name, SQL_DEFINITIONS->{$type}); + + if ($type == 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 ('---')"); + } } - 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 $field; +} + +sub run_create_validators { + my $class = shift; + my $dbh = Bugzilla->dbh; + my $params = $class->SUPER::run_create_validators(@_); + + $params->{name} = $class->_check_name($params->{name}, $params->{custom}); + if (!exists $params->{sortkey}) { + $params->{sortkey} = $dbh->selectrow_array( + "SELECT MAX(sortkey) + 100 FROM fielddefs") || 100; } - return new Bugzilla::Field({name => $name}); + return $params; } @@ -508,8 +605,22 @@ sub populate_field_definitions { my $dbh = Bugzilla->dbh; # ADD and UPDATE field definitions - foreach my $definition (DEFAULT_FIELDS) { - create_or_update($definition); + foreach my $def (DEFAULT_FIELDS) { + my $field = new Bugzilla::Field({ name => $def->{name} }); + if ($field) { + $field->set_description($def->{desc}); + $field->set_in_new_bugmail($def->{in_new_bugmail}); + $field->update(); + } + else { + if (exists $def->{in_new_bugmail}) { + $def->{mailhead} = $def->{in_new_bugmail}; + delete $def->{in_new_bugmail}; + } + $def->{description} = $def->{desc}; + delete $def->{desc}; + Bugzilla::Field->create($def); + } } # DELETE fields which were added only accidentally, or which @@ -583,8 +694,9 @@ sub populate_field_definitions { # This field has to be created separately, or the above upgrade code # might not run properly. - Bugzilla::Field::create_or_update( - {name => $new_field_name, desc => $field_description}); + Bugzilla::Field->create({ name => $new_field_name, + description => $field_description }) + unless new Bugzilla::Field({ name => $new_field_name }); } diff --git a/checksetup.pl b/checksetup.pl index 2a67e8b6e..d2313c92b 100755 --- a/checksetup.pl +++ b/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(); diff --git a/editfields.cgi b/editfields.cgi index f7a059016..e57e1952c 100644 --- a/editfields.cgi +++ b/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) diff --git a/template/en/default/admin/custom_fields/create.html.tmpl b/template/en/default/admin/custom_fields/create.html.tmpl index 995c4d0a9..a67e94680 100644 --- a/template/en/default/admin/custom_fields/create.html.tmpl +++ b/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> </th> diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl index 3d05214dc..a3b29bee7 100644 --- a/template/en/default/global/messages.html.tmpl +++ b/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" %] diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index d6b596ea5..ef53b2b8e 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/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 |