summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormkanat%bugzilla.org <>2007-08-24 04:31:19 +0200
committermkanat%bugzilla.org <>2007-08-24 04:31:19 +0200
commit845aacfb3652a18ef12828628e68180591f6baf6 (patch)
tree32f462212ee80be748c073f221489d4a935f1aea
parentd721954afb4458f240d3be8e020c619c57c9c1ed (diff)
downloadbugzilla-845aacfb3652a18ef12828628e68180591f6baf6.tar.gz
bugzilla-845aacfb3652a18ef12828628e68180591f6baf6.tar.xz
Bug 373440: Make "check" into a generic function in Bugzilla::Object
Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit
-rwxr-xr-xBugzilla/Bug.pm4
-rw-r--r--Bugzilla/Milestone.pm35
-rw-r--r--Bugzilla/Object.pm52
-rw-r--r--Bugzilla/User.pm8
-rw-r--r--Bugzilla/Version.pm26
-rwxr-xr-xeditmilestones.cgi24
-rwxr-xr-xeditversions.cgi17
-rw-r--r--template/en/default/global/user-error.html.tmpl32
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 %]