summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--Bugzilla/Bug.pm8
-rw-r--r--Bugzilla/Constants.pm4
-rw-r--r--Bugzilla/Field.pm4
-rw-r--r--Bugzilla/Field/Choice.pm237
-rw-r--r--Bugzilla/Status.pm55
-rwxr-xr-xeditvalues.cgi55
-rw-r--r--template/en/default/global/code-error.html.tmpl6
-rw-r--r--template/en/default/global/user-error.html.tmpl21
8 files changed, 231 insertions, 159 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 1d12ee2cf..d620f222c 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -54,7 +54,6 @@ use base qw(Bugzilla::Object Exporter);
RemoveVotes CheckIfVotedConfirmed
LogActivityEntry
editable_bug_fields
- SPECIAL_STATUS_WORKFLOW_ACTIONS
);
#####################################################################
@@ -234,13 +233,6 @@ use constant MAX_LINE_LENGTH => 254;
# Used in _check_comment(). Gives the max length allowed for a comment.
use constant MAX_COMMENT_LENGTH => 65535;
-use constant SPECIAL_STATUS_WORKFLOW_ACTIONS => qw(
- none
- duplicate
- change_resolution
- clearresolution
-);
-
#####################################################################
sub new {
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index abe1fe248..601ea52b2 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -151,6 +151,7 @@ use File::Basename;
MAX_PRODUCT_SIZE
MAX_MILESTONE_SIZE
MAX_COMPONENT_SIZE
+ MAX_FIELD_VALUE_SIZE
MAX_FREETEXT_LENGTH
);
@@ -427,6 +428,9 @@ use constant MAX_MILESTONE_SIZE => 20;
# The longest component name allowed.
use constant MAX_COMPONENT_SIZE => 64;
+# The maximum length for values of <select> fields.
+use constant MAX_FIELD_VALUE_SIZE => 64;
+
# Maximum length allowed for free text fields.
use constant MAX_FREETEXT_LENGTH => 255;
diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm
index bb516b8f4..5d8e5adc0 100644
--- a/Bugzilla/Field.pm
+++ b/Bugzilla/Field.pm
@@ -75,7 +75,6 @@ use base qw(Exporter Bugzilla::Object);
use Bugzilla::Constants;
use Bugzilla::Error;
-use Bugzilla::Field::Choice;
use Bugzilla::Util;
###############################
@@ -376,7 +375,8 @@ sub legal_values {
my $self = shift;
if (!defined $self->{'legal_values'}) {
- my @values = Bugzilla::Field::Choice->get_all({ field => $self });
+ require Bugzilla::Field::Choice;
+ my @values = Bugzilla::Field::Choice->type($self)->get_all();
$self->{'legal_values'} = \@values;
}
return $self->{'legal_values'};
diff --git a/Bugzilla/Field/Choice.pm b/Bugzilla/Field/Choice.pm
index 6ef911941..7705258eb 100644
--- a/Bugzilla/Field/Choice.pm
+++ b/Bugzilla/Field/Choice.pm
@@ -26,6 +26,8 @@ use base qw(Bugzilla::Object);
use Bugzilla::Constants;
use Bugzilla::Error;
+use Bugzilla::Field;
+use Bugzilla::Util qw(trim detaint_natural);
use Scalar::Util qw(blessed);
@@ -42,104 +44,141 @@ use constant DB_COLUMNS => qw(
use constant NAME_FIELD => 'value';
use constant LIST_ORDER => 'sortkey, value';
-##########################################
-# Constructors and Database Manipulation #
-##########################################
+use constant REQUIRED_CREATE_FIELDS => qw(value);
-# When calling class methods, we aren't based on just one table,
-# so we need some slightly hacky way to do DB_TABLE. We do it by overriding
-# class methods and making them specify $_new_table. This is possible
-# because we're always called from inside Bugzilla::Field, so it always
-# has a $self to pass us which contains info about which field we're
-# attached to.
-#
-# This isn't thread safe, but Bugzilla isn't a threaded application.
-our $_new_table;
-our $_current_field;
-sub DB_TABLE {
- my $invocant = shift;
- if (blessed $invocant) {
- return $invocant->field->name;
+use constant VALIDATORS => {
+ value => \&_check_value,
+ sortkey => \&_check_sortkey,
+};
+
+use constant CLASS_MAP => {
+ bug_status => 'Bugzilla::Status',
+};
+
+#################
+# Class Factory #
+#################
+
+# Bugzilla::Field::Choice is actually an abstract base class. Every field
+# type has its own dynamically-generated class for its values. This allows
+# certain fields to have special types, like how bug_status's values
+# are Bugzilla::Status objects.
+
+sub type {
+ my ($class, $field) = @_;
+ my $field_obj = blessed $field ? $field : Bugzilla::Field->check($field);
+ my $field_name = $field_obj->name;
+
+ if ($class->CLASS_MAP->{$field_name}) {
+ return $class->CLASS_MAP->{$field_name};
}
- return $_new_table;
+
+ # For generic classes, we use a lowercase class name, so as
+ # not to interfere with any real subclasses we might make some day.
+ my $package = "Bugzilla::Field::Choice::$field_name";
+
+ # We check defined so that the package and the stored field are only
+ # created once globally (at least per request). We prefix it with
+ # field_ (instead of suffixing it) so that we don't somehow conflict
+ # with the names of custom fields.
+ if (!defined Bugzilla->request_cache->{"field_$package"}) {
+ eval <<EOC;
+ package $package;
+ use base qw(Bugzilla::Field::Choice);
+ use constant DB_TABLE => '$field_name';
+EOC
+ Bugzilla->request_cache->{"field_$package"} = $field_obj;
+ }
+
+ return $package;
}
+################
+# Constructors #
+################
+
+# We just make new() enforce this, which should give developers
+# the understanding that you can't use Bugzilla::Field::Choice
+# without calling type().
sub new {
my $class = shift;
- my ($params) = @_;
- _check_field_arg($params);
- my $self = $class->SUPER::new($params);
- _fix_return($self);
- return $self;
+ if ($class eq 'Bugzilla::Field::Choice') {
+ ThrowCodeError('field_choice_must_use_type');
+ }
+ $class->SUPER::new(@_);
}
-sub new_from_list {
- my $class = shift;
- my ($ids, $params) = @_;
- _check_field_arg($params);
- my $list = $class->SUPER::new_from_list(@_);
- _fix_return($list);
- return $list;
-}
+#########################
+# Database Manipulation #
+#########################
-sub match {
+# Our subclasses can take more arguments than we normally accept.
+# So, we override create() to remove arguments that aren't valid
+# columns. (Normally Bugzilla::Object dies if you pass arguments
+# that aren't valid columns.)
+sub create {
my $class = shift;
my ($params) = @_;
- _check_field_arg($params);
- my $results = $class->SUPER::match(@_);
- _fix_return($results);
- return $results;
+ foreach my $key (keys %$params) {
+ if (!grep {$_ eq $key} $class->DB_COLUMNS) {
+ delete $params->{$key};
+ }
+ }
+ return $class->SUPER::create(@_);
}
-sub get_all {
- my $class = shift;
- _check_field_arg(@_);
- my @list = $class->SUPER::get_all(@_);
- _fix_return(\@list);
- return @list;
-}
+#############
+# Accessors #
+#############
-sub _check_field_arg {
- my ($params) = @_;
- my ($class, undef, undef, $func) = caller(1);
- if (!defined $params->{field}) {
- ThrowCodeError('param_required',
- { function => "${class}::$func",
- param => 'field' });
- }
- elsif (!blessed $params->{field}) {
- ThrowCodeError('bad_arg', { function => "${class}::$func",
- argument => 'field' });
- }
- $_new_table = $params->{field}->name;
- $_current_field = $params->{field};
- delete $params->{field};
+sub sortkey { return $_[0]->{'sortkey'}; }
+sub field {
+ my $invocant = shift;
+ my $class = ref $invocant || $invocant;
+ my $cache = Bugzilla->request_cache;
+ # This is just to make life easier for subclasses. Our auto-generated
+ # subclasses from type() already have this set.
+ $cache->{"field_$class"} ||=
+ new Bugzilla::Field({ name => $class->DB_TABLE });
+ return $cache->{"field_$class"};
}
-sub _fix_return {
- my $retval = shift;
- if (ref $retval eq 'ARRAY') {
- foreach my $obj (@$retval) {
- $obj->{field} = $_current_field;
- }
- }
- elsif (defined $retval) {
- $retval->{field} = $_current_field;
+##############
+# Validators #
+##############
+
+sub _check_value {
+ my ($invocant, $value) = @_;
+
+ my $field = $invocant->field;
+
+ $value = trim($value);
+ ThrowUserError('fieldvalue_undefined') if !defined $value || $value eq "";
+ ThrowUserError('fieldvalue_name_too_long', { value => $value })
+ if length($value) > MAX_FIELD_VALUE_SIZE;
+
+ my $exists = $invocant->type($field)->new({ name => $value });
+ if ($exists) {
+ ThrowUserError('fieldvalue_already_exists',
+ { field => $field, value => $value });
}
- # We do this just to avoid any possible bugs where $_new_table or
- # $_current_field are set from a previous call. It also might save
- # a little memory under mod_perl by releasing $_current_field explicitly.
- undef $_new_table;
- undef $_current_field;
+ return $value;
}
-#############
-# Accessors #
-#############
+sub _check_sortkey {
+ my ($invocant, $value) = @_;
+ $value = trim($value);
+ return 0 if !$value;
+ # Store for the error message in case detaint_natural clears it.
+ my $orig_value = $value;
+ detaint_natural($value)
+ || ThrowUserError('fieldvalue_sortkey_invalid',
+ { sortkey => $orig_value,
+ field => $invocant->field });
+ return $value;
+}
-sub sortkey { return $_[0]->{'sortkey'}; }
-sub field { return $_[0]->{'field'}; }
1;
@@ -153,27 +192,47 @@ Bugzilla::Field::Choice - A legal value for a <select>-type field.
my $field = new Bugzilla::Field({name => 'bug_status'});
- my $choice = new Bugzilla::Field::Choice({ field => $field, id => 1 });
- my $choice = new Bugzilla::Field::Choice({ field => $field, name => 'NEW' });
+ my $choice = new Bugzilla::Field::Choice->type($field)->new(1);
- my $choices = Bugzilla::Field::Choice->new_from_list([1,2,3],
- { field => $field});
- my $choices = Bugzilla::Field::Choice->get_all({ field => $field });
- my $choices = Bugzilla::Field::Choice->match({ sortkey => 10,
- field => $field });
+ my $choices = Bugzilla::Field::Choice->type($field)->new_from_list([1,2,3]);
+ my $choices = Bugzilla::Field::Choice->type($field)->get_all();
+ my $choices = Bugzilla::Field::Choice->type($field->match({ sortkey => 10 });
=head1 DESCRIPTION
This is an implementation of L<Bugzilla::Object>, but with a twist.
-All the class methods require that you specify an additional C<field>
-argument, which is a L<Bugzilla::Field> object that represents the
-field whose value this is.
+You can't call any class methods (such as C<new>, C<create>, etc.)
+directly on C<Bugzilla::Field::Choice> itself. Instead, you have to
+call C<Bugzilla::Field::Choice-E<gt>type($field)> to get the class
+you're going to instantiate, and then you call the methods on that.
+
+We do that because each field has its own database table for its values, so
+each value type needs its own class.
-You can look at the L</SYNOPSIS> to see where this extra C<field>
-argument goes in each function.
+See the L</SYNOPSIS> for examples of how this works.
=head1 METHODS
+=head2 Class Factory
+
+In object-oriented design, a "class factory" is a method that picks
+and returns the right class for you, based on an argument that you pass.
+
+=over
+
+=item C<type>
+
+Takes a single argument, which is either the name of a field from the
+C<fielddefs> table, or a L<Bugzilla::Field> object representing a field.
+
+Returns an appropriate subclass of C<Bugzilla::Field::Choice> that you
+can now call class methods on (like C<new>, C<create>, C<match>, etc.)
+
+B<NOTE>: YOU CANNOT CALL CLASS METHODS ON C<Bugzilla::Field::Choice>. You
+must call C<type> to get a class you can call methods on.
+
+=back
+
=head2 Accessors
These are in addition to the standard L<Bugzilla::Object> accessors.
diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm
index f8b77331c..5f37974e7 100644
--- a/Bugzilla/Status.pm
+++ b/Bugzilla/Status.pm
@@ -22,13 +22,28 @@ use strict;
package Bugzilla::Status;
-use base qw(Bugzilla::Object Exporter);
-@Bugzilla::Status::EXPORT = qw(BUG_STATE_OPEN is_open_state closed_bug_statuses);
+use Bugzilla::Error;
+
+use base qw(Bugzilla::Field::Choice Exporter);
+@Bugzilla::Status::EXPORT = qw(
+ BUG_STATE_OPEN
+ SPECIAL_STATUS_WORKFLOW_ACTIONS
+
+ is_open_state
+ closed_bug_statuses
+);
################################
##### Initialization #####
################################
+use constant SPECIAL_STATUS_WORKFLOW_ACTIONS => qw(
+ none
+ duplicate
+ change_resolution
+ clearresolution
+);
+
use constant DB_TABLE => 'bug_status';
use constant DB_COLUMNS => qw(
@@ -39,8 +54,24 @@ use constant DB_COLUMNS => qw(
is_open
);
-use constant NAME_FIELD => 'value';
-use constant LIST_ORDER => 'sortkey, value';
+sub VALIDATORS {
+ my $invocant = shift;
+ my $validators = $invocant->SUPER::VALIDATORS;
+ $validators->{is_open} = \&Bugzilla::Object::check_boolean;
+ $validators->{value} = \&_check_value;
+ return $validators;
+}
+
+#########################
+# Database Manipulation #
+#########################
+
+sub create {
+ my $class = shift;
+ my $self = $class->SUPER::create(@_);
+ add_missing_bug_status_transitions();
+ return $self;
+}
###############################
##### Accessors ####
@@ -51,6 +82,22 @@ sub sortkey { return $_[0]->{'sortkey'}; }
sub is_active { return $_[0]->{'isactive'}; }
sub is_open { return $_[0]->{'is_open'}; }
+##############
+# Validators #
+##############
+
+sub _check_value {
+ my $invocant = shift;
+ my $value = $invocant->SUPER::_check_value(@_);
+
+ if (grep { lc($value) eq lc($_) } SPECIAL_STATUS_WORKFLOW_ACTIONS) {
+ ThrowUserError('fieldvalue_reserved_word',
+ { field => $invocant->field, value => $value });
+ }
+ return $value;
+}
+
+
###############################
##### Methods ####
###############################
diff --git a/editvalues.cgi b/editvalues.cgi
index ffb7ce576..4525774e1 100755
--- a/editvalues.cgi
+++ b/editvalues.cgi
@@ -29,7 +29,7 @@ use Bugzilla::Config qw(:admin);
use Bugzilla::Token;
use Bugzilla::Field;
use Bugzilla::Bug;
-use Bugzilla::Status;
+use Bugzilla::Status qw(SPECIAL_STATUS_WORKFLOW_ACTIONS);
# List of different tables that contain the changeable field values
# (the old "enums.") Keep them in alphabetical order by their
@@ -172,12 +172,8 @@ trick_taint($field);
sub display_field_values {
my $template = Bugzilla->template;
my $field = $vars->{'field'}->name;
- my $fieldvalues =
- Bugzilla->dbh->selectall_arrayref("SELECT value AS name, sortkey"
- . " FROM $field ORDER BY sortkey, value",
- {Slice =>{}});
- $vars->{'values'} = $fieldvalues;
+ $vars->{'values'} = $vars->{'field'}->legal_values;
$vars->{'default'} = Bugzilla->params->{$defaults{$field}} if defined $defaults{$field};
$vars->{'static'} = $static{$field} if exists $static{$field};
@@ -212,53 +208,14 @@ if ($action eq 'add') {
if ($action eq 'new') {
check_token_data($token, 'add_field_value');
- # Cleanups and validity checks
- $value || ThrowUserError('fieldvalue_undefined');
-
- if (length($value) > 60) {
- ThrowUserError('fieldvalue_name_too_long',
- {'value' => $value});
- }
- # Need to store in case detaint_natural() clears the sortkey
- my $stored_sortkey = $sortkey;
- if (!detaint_natural($sortkey)) {
- ThrowUserError('fieldvalue_sortkey_invalid',
- {'name' => $field,
- 'sortkey' => $stored_sortkey});
- }
- if (ValueExists($field, $value)) {
- ThrowUserError('fieldvalue_already_exists',
- {'field' => $field_obj,
- 'value' => $value});
- }
- if ($field eq 'bug_status'
- && (grep { lc($value) eq $_ } SPECIAL_STATUS_WORKFLOW_ACTIONS))
- {
- $vars->{'value'} = $value;
- ThrowUserError('fieldvalue_reserved_word', $vars);
- }
-
- # Value is only used in a SELECT placeholder and through the HTML filter.
- trick_taint($value);
-
- # Add the new field value.
- $dbh->do("INSERT INTO $field (value, sortkey) VALUES (?, ?)",
- undef, ($value, $sortkey));
-
- if ($field eq 'bug_status') {
- unless ($cgi->param('is_open')) {
- # The bug status is a closed state, but they are open by default.
- $dbh->do('UPDATE bug_status SET is_open = 0 WHERE value = ?', undef, $value);
- }
- # Allow the transition from this new bug status to the one used
- # by the 'duplicate_or_move_bug_status' parameter.
- Bugzilla::Status::add_missing_bug_status_transitions();
- }
+ my $created_value = Bugzilla::Field::Choice->type($field_obj)->create(
+ { value => $value, sortkey => $sortkey,
+ is_open => scalar $cgi->param('is_open') });
delete_token($token);
$vars->{'message'} = 'field_value_created';
- $vars->{'value'} = $value;
+ $vars->{'value'} = $created_value->name;
display_field_values();
}
diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl
index b93b92efd..f85375849 100644
--- a/template/en/default/global/code-error.html.tmpl
+++ b/template/en/default/global/code-error.html.tmpl
@@ -145,6 +145,12 @@
in the database for '[% username FILTER html %]', but your
account source says that '[% extern_user FILTER html %]' has that ID.
+ [% ELSIF error == "field_choice_must_use_type" %]
+ When you call a class method on <code>Bugzilla::Field::Choice</code>,
+ you must call <code>Bugzilla::Field::Choice-&gt;type('some_field')</code>
+ to generate the right class (you can't call class methods directly
+ on Bugzilla::Field::Choice).
+
[% ELSIF error == "field_type_mismatch" %]
Cannot seem to handle <code>[% field FILTER html %]</code>
and <code>[% type FILTER html %]</code> together.
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index c61a1f42a..bcb6b6630 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -432,7 +432,7 @@
[% ELSIF error == "fieldvalue_already_exists" %]
[% title = "Field Value Already Exists" %]
The value '[% value FILTER html %]' already exists for the
- '[%- field.description FILTER html %]' field.
+ [%+ field.description FILTER html %] field.
[% ELSIF error == "fieldvalue_doesnt_exist" %]
[% title = "Specified Field Value Does Not Exist" %]
@@ -450,8 +450,9 @@
[% ELSIF error == "fieldvalue_name_too_long" %]
[% title = "Field Value Is Too Long" %]
- The value of a field is limited to 60 characters.
- '[% value FILTER html %]' is too long ([% value.length %] characters).
+ The value of a field is limited to [% constants.FIELD_VALUE_MAX_SIZE %]
+ characters. '[% value FILTER html %]' is too long ([% value.length %]
+ characters).
[% ELSIF error == "fieldvalue_not_editable" %]
[% title = "Field Value Not Editable" %]
@@ -475,8 +476,9 @@
[% ELSIF error == "fieldvalue_sortkey_invalid" %]
[% title = "Invalid Field Value Sortkey" %]
- The sortkey '[% sortkey FILTER html %]' for the '[% name FILTER html %]'
- field is not a valid (positive) number.
+ The sortkey '[% sortkey FILTER html %]' for the
+ [%+ field.description FILTER html %] field is not a valid
+ (positive) number.
[% ELSIF error == "fieldvalue_still_has_bugs" %]
[% title = "You Cannot Delete This Field Value" %]
@@ -1178,13 +1180,13 @@
is less than the minimum allowable value of '[% min_num FILTER html %]'.
[% ELSIF error == "object_name_not_specified" %]
- [% type = BLOCK %][% PROCESS object_name %][% END %]
+ [% type = BLOCK %][% INCLUDE object_name class = class %][% END %]
[% title = BLOCK %][% type FILTER ucfirst FILTER html %] Not
Specified[% END %]
You must select/enter a [% type FILTER html %].
[% ELSIF error == "object_does_not_exist" %]
- [% type = BLOCK %][% PROCESS object_name %][% END %]
+ [% type = BLOCK %][% INCLUDE object_name class = class %][% END %]
[% title = BLOCK %]Invalid [% type FILTER ucfirst FILTER html %][% END %]
There is no [% type FILTER html %] named '[% name FILTER html %]'
[% IF product.defined %]
@@ -1672,5 +1674,10 @@
milestone
[% ELSIF class == "Bugzilla::Status" %]
status
+ [% ELSIF class == "Bugzilla::Field" %]
+ field
+ [% ELSIF ( matches = class.match('^Bugzilla::Field::Choice::(.+)') ) %]
+ [% SET field_name = matches.0 %]
+ [% field_descs.$field_name %]
[% END %]
[% END %]
generated by cgit v1.2.3-24-g4f1b (git 2.32.0) at 2024-11-21 20:14:03 +0100