diff options
| -rw-r--r-- | Bugzilla/DB/Schema.pm | 2 | ||||
| -rw-r--r-- | Bugzilla/Install/DB.pm | 3 | ||||
| -rw-r--r-- | Bugzilla/Keyword.pm | 34 | ||||
| -rw-r--r-- | Bugzilla/Object.pm | 71 | ||||
| -rwxr-xr-x | editkeywords.cgi | 62 | ||||
| -rw-r--r-- | template/en/default/global/code-error.html.tmpl | 6 |
6 files changed, 120 insertions, 58 deletions
diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index e997e4187..50785c5b7 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -346,7 +346,7 @@ use constant ABSTRACT_SCHEMA => { keyworddefs => { FIELDS => [ - id => {TYPE => 'INT2', NOTNULL => 1, + id => {TYPE => 'SMALLSERIAL', NOTNULL => 1, PRIMARYKEY => 1}, name => {TYPE => 'varchar(64)', NOTNULL => 1}, description => {TYPE => 'MEDIUMTEXT'}, diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm index dfe7f957a..a4ab54260 100644 --- a/Bugzilla/Install/DB.pm +++ b/Bugzilla/Install/DB.pm @@ -468,6 +468,9 @@ sub update_table_definitions { $dbh->bz_alter_column('flagtypes', 'id', {TYPE => 'SMALLSERIAL', NOTNULL => 1, PRIMARYKEY => 1}); + $dbh->bz_alter_column('keyworddefs', 'id', + {TYPE => 'SMALLSERIAL', NOTNULL => 1, PRIMARYKEY => 1}); + ################################################################ # New --TABLE-- changes should go *** A B O V E *** this point # ################################################################ diff --git a/Bugzilla/Keyword.pm b/Bugzilla/Keyword.pm index 2e0f19f4c..fead77821 100644 --- a/Bugzilla/Keyword.pm +++ b/Bugzilla/Keyword.pm @@ -20,6 +20,9 @@ package Bugzilla::Keyword; use base qw(Bugzilla::Object); +use Bugzilla::Error; +use Bugzilla::Util; + ############################### #### Initialization #### ############################### @@ -32,6 +35,13 @@ use constant DB_COLUMNS => qw( use constant DB_TABLE => 'keyworddefs'; +use constant REQUIRED_CREATE_FIELDS => qw(name description); + +use constant VALIDATORS => { + name => \&_check_name, + description => \&_check_description, +}; + ############################### #### Accessors ###### ############################### @@ -81,6 +91,30 @@ sub get_all_with_bug_count { return $keywords; } +############################### +### Validators ### +############################### + +sub _check_name { + my ($name) = @_; + $name = trim($name); + $name eq "" && ThrowUserError("keyword_blank_name"); + if ($name =~ /[\s,]/) { + ThrowUserError("keyword_invalid_name"); + } + my $keyword = new Bugzilla::Keyword({ name => $name }); + ThrowUserError("keyword_already_exists", { name => $name }) if $keyword; + + return $name; +} + +sub _check_description { + my ($desc) = @_; + $desc = trim($desc); + $desc eq '' && ThrowUserError("keyword_blank_description"); + return $desc; +} + 1; __END__ diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm index 56789f584..c250f80fd 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -108,6 +108,45 @@ sub name { return $_[0]->{'name'}; } #### Subroutines ###### ############################### +sub create { + my ($class, $params) = @_; + my $dbh = Bugzilla->dbh; + + my $required = $class->REQUIRED_CREATE_FIELDS; + my $validators = $class->VALIDATORS; + my $table = $class->DB_TABLE; + + foreach my $field ($class->REQUIRED_CREATE_FIELDS) { + ThrowCodeError('param_required', + { function => "${class}->create", param => $field }) + if !exists $params->{$field}; + } + + my (@field_names, @values); + # We do the sort just to make sure that validation always + # happens in a consistent order. + foreach my $field (sort keys %$params) { + my $value; + if (exists $validators->{$field}) { + $value = &{$validators->{$field}}($params->{$field}); + } + else { + $value = $params->{$field}; + } + trick_taint($value); + push(@field_names, $field); + push(@values, $value); + } + + my $qmarks = '?,' x @values; + chop($qmarks); + $dbh->do("INSERT INTO $table (" . join(', ', @field_names) + . ") VALUES ($qmarks)", undef, @values); + my $id = $dbh->bz_last_key($table, 'id'); + + return $class->new($id); +} + sub get_all { my $class = shift; my $dbh = Bugzilla->dbh; @@ -173,6 +212,19 @@ The order that C<new_from_list> and C<get_all> should return objects in. This should be the name of a database column. Defaults to C<name>. +=item C<REQUIRED_CREATE_FIELDS> + +The list of fields that B<must> be specified when the user calls +C<create()>. This should be an array. + +=item C<VALIDATORS> + +A hashref that points to a function that will validate each param to +C<create()>. Each function in this hashref will be passed a single +argument, the value passed to C<create()> for that field. These +functions should call L<Bugzilla::Error/ThrowUserError> if they fail. +They must return the validated value. + =back =head1 METHODS @@ -210,6 +262,25 @@ C<name>. =over +=item C<create($params)> + +Description: Creates a new item in the database. + Throws a User Error if any of the passed-in params + are invalid. + +Params: C<$params> - hashref - A value to put in each database + field for this object. Certain values must be set (the + ones specified in L</REQUIRED_CREATE_FIELDS>), and + the function will throw a Code Error if you don't set + them. + +Returns: The Object just created in the database. + +Notes: In order for this function to work in your subclass, + your subclass's C<id> field must be of C<SERIAL> + type in the database. Your subclass also must + define L</REQUIRED_CREATE_FIELDS> and L</VALIDATORS>. + =item C<get_all> Description: Returns all objects in this table from the database. diff --git a/editkeywords.cgi b/editkeywords.cgi index 7b906c30b..7b94dbbe3 100755 --- a/editkeywords.cgi +++ b/editkeywords.cgi @@ -34,25 +34,6 @@ my $dbh = Bugzilla->dbh; my $template = Bugzilla->template; my $vars = {}; -sub Validate { - my ($name, $description) = @_; - if ($name eq "") { - ThrowUserError("keyword_blank_name"); - } - if ($name =~ /[\s,]/) { - ThrowUserError("keyword_invalid_name"); - } - if ($description eq "") { - ThrowUserError("keyword_blank_description"); - } - # It is safe to detaint these values as they are only - # used in placeholders. - trick_taint($name); - $_[0] = $name; - trick_taint($description); - $_[1] = $description; -} - sub ValidateKeyID { my $id = shift; @@ -102,49 +83,16 @@ if ($action eq 'add') { # # action='new' -> add keyword entered in the 'action=add' screen # - if ($action eq 'new') { - # Cleanups and validity checks - - my $name = trim($cgi->param('name') || ''); - my $description = trim($cgi->param('description') || ''); - - Validate($name, $description); - - my $id = $dbh->selectrow_array('SELECT id FROM keyworddefs - WHERE name = ?', undef, $name); - - if ($id) { - $vars->{'name'} = $name; - ThrowUserError("keyword_already_exists", $vars); - } - - - # Pick an unused number. Be sure to recycle numbers that may have been - # deleted in the past. This code is potentially slow, but it happens - # rarely enough, and there really aren't ever going to be that many - # keywords anyway. - - my $existing_ids = - $dbh->selectcol_arrayref('SELECT id FROM keyworddefs ORDER BY id'); + my $name = $cgi->param('name') || ''; + my $desc = $cgi->param('description') || ''; - my $newid = 1; - - foreach my $oldid (@$existing_ids) { - if ($oldid > $newid) { - last; - } - $newid = $oldid + 1; - } - - # Add the new keyword. - $dbh->do('INSERT INTO keyworddefs - (id, name, description) VALUES (?, ?, ?)', - undef, ($newid, $name, $description)); + my $keyword = Bugzilla::Keyword->create( + { name => $name, description => $desc }); print $cgi->header(); - $vars->{'name'} = $name; + $vars->{'name'} = $keyword->name; $template->process("admin/keywords/created.html.tmpl", $vars) || ThrowTemplateError($template->error()); diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl index f64cf6411..9d039a07c 100644 --- a/template/en/default/global/code-error.html.tmpl +++ b/template/en/default/global/code-error.html.tmpl @@ -294,6 +294,12 @@ Invalid parameter passed to [% function FILTER html %]. It must be numeric. + [% ELSIF error == "param_required" %] + [% title = "Missing Parameter" %] + The function <code>[% function FILTER html %]</code> requires + a <code>[% param FILTER html %]</code> argument, and that + argument was not set. + [% ELSIF error == "unknown_comparison_type" %] Specified comparison type is not supported. |