Bug 373440: Make "check" into a generic function in Bugzilla::Object

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit

5 files changed, 51 insertions, 74 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.