diff options
| -rwxr-xr-x | Bugzilla/Bug.pm | 4 | ||||
| -rw-r--r-- | Bugzilla/Milestone.pm | 35 | ||||
| -rw-r--r-- | Bugzilla/Object.pm | 52 | ||||
| -rw-r--r-- | Bugzilla/User.pm | 8 | ||||
| -rw-r--r-- | Bugzilla/Version.pm | 26 | ||||
| -rwxr-xr-x | editmilestones.cgi | 24 | ||||
| -rwxr-xr-x | editversions.cgi | 17 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 32 |
8 files changed, 97 insertions, 101 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 0a2770a65..33bec5f71 100755 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -1282,7 +1282,7 @@ sub add_cc { my ($self, $user_or_name, $product) = @_; return if !$user_or_name; my $user = ref $user_or_name ? $user_or_name - : Bugzilla::User::check($user_or_name); + : Bugzilla::User->check($user_or_name); my $product_id = $product ? $product->id : $self->{product_id}; if (Bugzilla->params->{strict_isolation} @@ -1301,7 +1301,7 @@ sub add_cc { sub remove_cc { my ($self, $user_or_name) = @_; my $user = ref $user_or_name ? $user_or_name - : Bugzilla::User::check($user_or_name); + : Bugzilla::User->check($user_or_name); my $cc_users = $self->cc_users; @$cc_users = grep { $_->id != $user->id } @$cc_users; } diff --git a/Bugzilla/Milestone.pm b/Bugzilla/Milestone.pm index 2e70b4e2d..596c34bd8 100644 --- a/Bugzilla/Milestone.pm +++ b/Bugzilla/Milestone.pm @@ -96,23 +96,6 @@ sub sortkey { return $_[0]->{'sortkey'}; } ##### Subroutines ##### ################################ -sub check_milestone { - my ($product, $milestone_name) = @_; - - unless ($milestone_name) { - ThrowUserError('milestone_not_specified'); - } - - my $milestone = new Bugzilla::Milestone({ product => $product, - name => $milestone_name }); - unless ($milestone) { - ThrowUserError('milestone_not_valid', - {'product' => $product->name, - 'milestone' => $milestone_name}); - } - return $milestone; -} - sub check_sort_key { my ($milestone_name, $sortkey) = @_; # Keep a copy in case detaint_signed() clears the sortkey @@ -174,21 +157,3 @@ Milestone.pm represents a Product Milestone object. Returns: Integer with the number of bugs. =back - -=head1 SUBROUTINES - -=over - -=item C<check_milestone($product, $milestone_name)> - - Description: Checks if a milestone name was passed in - and if it is a valid milestone. - - Params: $product - Bugzilla::Product object. - $milestone_name - String with a milestone name. - - Returns: Bugzilla::Milestone object. - -=back - -=cut diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm index 4d54b04e1..9afad7633 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -104,6 +104,24 @@ sub _init { return $object; } +sub check { + my ($invocant, $param) = @_; + my $class = ref($invocant) || $invocant; + # If we were just passed a name, then just use the name. + if (!ref $param) { + $param = { name => $param }; + } + # Don't allow empty names. + if (exists $param->{name}) { + $param->{name} = trim($param->{name}); + $param->{name} || ThrowUserError('object_name_not_specified', + { class => $class }); + } + my $obj = $class->new($param) + || ThrowUserError('object_does_not_exist', {%$param, class => $class}); + return $obj; +} + sub new_from_list { my $invocant = shift; my $class = ref($invocant) || $invocant; @@ -463,7 +481,7 @@ during these comparisons. =over -=item C<new($param)> +=item C<new> =over @@ -478,7 +496,7 @@ If you pass an integer, the integer is the id of the object, from the database, that we want to read in. (id is defined as the value in the L</ID_FIELD> column). -If you pass in a hash, you can pass a C<name> key. The +If you pass in a hashref, you can pass a C<name> key. The value of the C<name> key is the case-insensitive name of the object (from L</NAME_FIELD>) in the DB. @@ -503,7 +521,35 @@ are intended B<only> for use by subclasses. =item B<Returns> -A fully-initialized object. +A fully-initialized object, or C<undef> if there is no object in the +database matching the parameters you passed in. + +=back + +=item C<check> + +=over + +=item B<Description> + +Checks if there is an object in the database with the specified name, and +throws an error if you specified an empty name, or if there is no object +in the database with that name. + +=item B<Params> + +The parameters are the same as for L</new>, except that if you don't pass +a hashref, the single argument is the I<name> of the object, not the id. + +=item B<Returns> + +A fully initialized object, guaranteed. + +=item B<Notes For Implementors> + +If you implement this in your subclass, make sure that you also update +the C<object_name> block at the bottom of the F<global/user-error.html.tmpl> +template. =back diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index cf8de0274..8ccd15a63 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -131,14 +131,6 @@ sub new { return $class->SUPER::new(@_); } -sub check { - my ($username) = @_; - $username = trim($username); - my $user = new Bugzilla::User({ name => $username }) - || ThrowUserError('invalid_username', { name => $username }); - return $user; -} - sub update { my $self = shift; my $changes = $self->SUPER::update(@_); diff --git a/Bugzilla/Version.pm b/Bugzilla/Version.pm index 69eee3752..a2ef6b01e 100644 --- a/Bugzilla/Version.pm +++ b/Bugzilla/Version.pm @@ -150,20 +150,6 @@ sub product_id { return $_[0]->{'product_id'}; } ##### Subroutines ### ############################### -sub check_version { - my ($product, $version_name) = @_; - - $version_name || ThrowUserError('version_not_specified'); - my $version = new Bugzilla::Version( - { product => $product, name => $version_name }); - unless ($version) { - ThrowUserError('version_not_valid', - {'product' => $product->name, - 'version' => $version_name}); - } - return $version; -} - sub create { my ($name, $product) = @_; my $dbh = Bugzilla->dbh; @@ -212,9 +198,6 @@ Bugzilla::Version - Bugzilla product version class. my $version = $hash_ref->{'version_value'}; - my $version = Bugzilla::Version::check_version($product_obj, - 'acme_version'); - my $version = Bugzilla::Version::create($version_name, $product); =head1 DESCRIPTION @@ -266,15 +249,6 @@ Version.pm represents a Product Version object. =over -=item C<check_version($product, $version_name)> - - Description: Checks if the version name exists for the product name. - - Params: $product - A Bugzilla::Product object. - $version_name - String with a version name. - - Returns: Bugzilla::Version object. - =item C<create($version_name, $product)> Description: Create a new version for the given product. diff --git a/editmilestones.cgi b/editmilestones.cgi index 17733bdb1..880e1d4a7 100755 --- a/editmilestones.cgi +++ b/editmilestones.cgi @@ -168,8 +168,8 @@ if ($action eq 'new') { # if ($action eq 'del') { - my $milestone = Bugzilla::Milestone::check_milestone($product, - $milestone_name); + my $milestone = Bugzilla::Milestone->check({ product => $product, + name => $milestone_name }); $vars->{'milestone'} = $milestone; $vars->{'product'} = $product; @@ -193,9 +193,8 @@ if ($action eq 'del') { if ($action eq 'delete') { check_token_data($token, 'delete_milestone'); - my $milestone = - Bugzilla::Milestone::check_milestone($product, - $milestone_name); + my $milestone = Bugzilla::Milestone->check({ product => $product, + name => $milestone_name }); $vars->{'milestone'} = $milestone; $vars->{'product'} = $product; @@ -245,9 +244,8 @@ if ($action eq 'delete') { if ($action eq 'edit') { - my $milestone = - Bugzilla::Milestone::check_milestone($product, - $milestone_name); + my $milestone = Bugzilla::Milestone->check({ product => $product, + name => $milestone_name }); $vars->{'milestone'} = $milestone; $vars->{'product'} = $product; @@ -269,9 +267,8 @@ if ($action eq 'edit') { if ($action eq 'update') { check_token_data($token, 'edit_milestone'); my $milestone_old_name = trim($cgi->param('milestoneold') || ''); - my $milestone_old = - Bugzilla::Milestone::check_milestone($product, - $milestone_old_name); + my $milestone_old = Bugzilla::Milestone->check( + { product => $product, name => $milestone_old_name }); if (length($milestone_name) > 20) { ThrowUserError('milestone_name_too_long', @@ -343,9 +340,8 @@ if ($action eq 'update') { $dbh->bz_unlock_tables(); - my $milestone = - Bugzilla::Milestone::check_milestone($product, - $milestone_name); + my $milestone = Bugzilla::Milestone->check({ product => $product, + name => $milestone_name }); delete_token($token); $vars->{'milestone'} = $milestone; diff --git a/editversions.cgi b/editversions.cgi index 7bda6215d..54f87457b 100755 --- a/editversions.cgi +++ b/editversions.cgi @@ -147,9 +147,8 @@ if ($action eq 'new') { # if ($action eq 'del') { - - my $version = Bugzilla::Version::check_version($product, $version_name); - + my $version = Bugzilla::Version->check({ product => $product, + name => $version_name }); $vars->{'version'} = $version; $vars->{'product'} = $product; $vars->{'token'} = issue_session_token('delete_version'); @@ -167,7 +166,8 @@ if ($action eq 'del') { if ($action eq 'delete') { check_token_data($token, 'delete_version'); - my $version = Bugzilla::Version::check_version($product, $version_name); + my $version = Bugzilla::Version->check({ product => $product, + name => $version_name }); $version->remove_from_db; delete_token($token); @@ -189,9 +189,8 @@ if ($action eq 'delete') { # if ($action eq 'edit') { - - my $version = Bugzilla::Version::check_version($product, $version_name); - + my $version = Bugzilla::Version->check({ product => $product, + name => $version_name }); $vars->{'version'} = $version; $vars->{'product'} = $product; $vars->{'token'} = issue_session_token('edit_version'); @@ -211,8 +210,8 @@ if ($action eq 'edit') { if ($action eq 'update') { check_token_data($token, 'edit_version'); my $version_old_name = trim($cgi->param('versionold') || ''); - my $version = - Bugzilla::Version::check_version($product, $version_old_name); + my $version = Bugzilla::Version->check({ product => $product, + name => $version_old_name }); $dbh->bz_lock_tables('bugs WRITE', 'versions WRITE'); diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index b5b7bf098..5cebb8166 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -902,10 +902,6 @@ The name of a milestone is limited to 20 characters. '[% name FILTER html %]' is too long ([% name.length %] characters). - [% ELSIF error == "milestone_not_specified" %] - [% title = "No Milestone Specified" %] - No milestone specified when trying to edit milestones. - [% ELSIF error == "milestone_not_valid" %] [% title = "Specified Milestone Does Not Exist" %] The milestone '[% milestone FILTER html %]' for product @@ -1139,6 +1135,24 @@ in the <em>[% field_descs.$field FILTER html %]</em> field is less than the minimum allowable value of '[% min_num FILTER html %]'. + [% ELSIF error == "object_name_not_specified" %] + [% type = BLOCK %][% PROCESS object_name %][% 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 %] + [% title = BLOCK %]Invalid [% type FILTER ucfirst FILTER html %][% END %] + There is no [% type FILTER html %] named '[% name FILTER html %]' + [% IF product.defined %] + in the '[% product.name FILTER html %]' product + [% END %]. + [% IF class == "Bugzilla::User" %] + Either you mis-typed the name or that user has not yet registered + for a [% terms.Bugzilla %] account. + [% END %] + [% ELSIF error == "old_password_incorrect" %] [% title = "Incorrect Old Password" %] You did not enter your old password correctly. @@ -1611,3 +1625,13 @@ [% END %] [% PROCESS global/footer.html.tmpl %] + +[% BLOCK object_name %] + [% IF class == "Bugzilla::User" %] + user + [% ELSIF class == "Bugzilla::Version" %] + version + [% ELSIF class == "Bugzilla::Milestone" %] + milestone + [% END %] +[% END %] |