Bug 309749: Remove get_x_by_y functions from the new .pm files, in favor of object methods

Patch By André Batosti <batosti@async.com.br> r=LpSolit, a=justdave

8 files changed, 52 insertions, 162 deletions

diff --git a/Bugzilla/Component.pm b/Bugzilla/Component.pm
index cd6722993..74ea60d1b 100644
--- a/Bugzilla/Component.pm
+++ b/Bugzilla/Component.pm
@@ -145,32 +145,6 @@ sub product_id  { return $_[0]->{'product_id'};  }
 ####      Subroutines      ####
 ###############################
 
-sub get_components_by_product {
-    my ($product_id) = @_;
-    my $dbh = Bugzilla->dbh;
-
-    my $stored_product_id = $product_id;
-    unless (detaint_natural($product_id)) {
-        ThrowCodeError(
-            'invalid_numeric_argument',
-            {argument => 'product_id',
-             value    => $stored_product_id,
-             function =>
-                'Bugzilla::Component::get_components_by_product'}
-        );
-    }
-
-    my $ids = $dbh->selectcol_arrayref(q{
-        SELECT id FROM components
-        WHERE product_id = ?}, undef, $product_id);
-
-    my @components;
-    foreach my $id (@$ids) {
-        push @components, new Bugzilla::Component($id);
-    }
-    return @components;
-}
-
 sub check_component {
     my ($product, $comp_name) = @_;
 
@@ -217,7 +191,6 @@ Bugzilla::Component - Bugzilla product component class.
     my $default_assignee   = $component->default_assignee;
     my $default_qa_contact = $component->default_qa_contact;
 
-    my @components = Bugzilla::Component::get_components_by_product($id);
     my $component  = Bugzilla::Component::check_component($product, 'AcmeComp');
 
 =head1 DESCRIPTION
@@ -282,15 +255,6 @@ Component.pm represents a Product Component object.
 
 =over
 
-=item C<get_components_by_product($product_id)>
-
- Description: Returns all components that belong to the supplied product.
-
- Params:      $product_id - Integer with a Bugzilla product id.
-
- Returns:     An array of Bugzilla::Component objects.
- 
-
 =item C<check_component($product, $comp_name)>
 
  Description: Checks if the component name was passed in and if it is a valid
diff --git a/Bugzilla/Group.pm b/Bugzilla/Group.pm
index 54fff489b..cc57fca69 100644
--- a/Bugzilla/Group.pm
+++ b/Bugzilla/Group.pm
@@ -124,27 +124,6 @@ sub ValidateGroupName {
     return $ret;
 }
 
-sub get_group_controls_by_product {
-    my ($product_id) = @_;
-    my $dbh = Bugzilla->dbh;
-
-    my $query = qq{SELECT
-                       $columns,
-                       group_control_map.entry,
-                       group_control_map.membercontrol,
-                       group_control_map.othercontrol,
-                       group_control_map.canedit
-                  FROM groups
-                  LEFT JOIN group_control_map
-                        ON groups.id = group_control_map.group_id
-                  WHERE group_control_map.product_id = ?
-                  AND   groups.isbuggroup != 0
-                  ORDER BY groups.name};
-    my $groups = $dbh->selectall_hashref($query, 'id', undef,
-                                         ($product_id));
-    return $groups;
-}
-
 sub get_all_groups {
     my $dbh = Bugzilla->dbh;
 
@@ -181,7 +160,6 @@ Bugzilla::Group - Bugzilla group class.
     my $is_active    = $group->is_active;
 
     my $group_id = Bugzilla::Group::ValidateGroupName('admin', @users);
-    my $groups = Bugzilla::Group::get_group_controls_by_product(1);
     my @groups = Bugzilla::get_all_groups();
 
 =head1 DESCRIPTION
@@ -223,17 +201,6 @@ Group.pm represents a Bugzilla Group object.
  Returns:     It returns the group id if successful
               and undef otherwise.
 
-=item C<get_group_controls_by_product($product_id)>
-
- Description: Returns all group controls of a specific product.
-              It is encouraged to use Bugzilla::Product object
-              instead of directly calling this routine.
-
- Params:      $product_id - Integer with a Bugzilla product id.
-
- Returns:     A hash with group id as key and hash containing the
-              group data as value.
-
 =item C<get_all_groups()>
 
  Description: Returns all groups available, including both
diff --git a/Bugzilla/Milestone.pm b/Bugzilla/Milestone.pm
index 6a5eeb48f..7b5d47d49 100644
--- a/Bugzilla/Milestone.pm
+++ b/Bugzilla/Milestone.pm
@@ -98,32 +98,6 @@ sub sortkey    { return $_[0]->{'sortkey'};    }
 #####     Subroutines      #####
 ################################
 
-sub get_milestones_by_product {
-    my ($product_id) = @_;
-    my $dbh = Bugzilla->dbh;
-
-    my $stored_product_id = $product_id;
-    unless (detaint_natural($product_id)) {
-        ThrowCodeError(
-            'invalid_numeric_argument',
-            {argument => 'product_id',
-             value    => $stored_product_id,
-             function =>
-                'Bugzilla::Milestone::get_milestones_by_product'}
-        );
-    }
-
-    my $values = $dbh->selectcol_arrayref(q{
-        SELECT value FROM milestones
-        WHERE product_id = ?}, undef, $product_id);
-
-    my @milestones;
-    foreach my $value (@$values) {
-        push @milestones, new Bugzilla::Milestone($product_id, $value);
-    }
-    return @milestones;
-}
-
 sub check_milestone {
     my ($product, $milestone_name) = @_;
 
@@ -172,7 +146,6 @@ Bugzilla::Milestone - Bugzilla product milestone class.
     my $product_id = $milestone->product_id;
     my $value = $milestone->value;
 
-    my $hash_ref = Bugzilla::Milestone::get_milestones_by_product(1);
     my $milestone = $hash_ref->{'milestone_value'};
 
 =head1 DESCRIPTION
@@ -207,15 +180,6 @@ Milestone.pm represents a Product Milestone object.
 
 =over
 
-=item C<get_milestones_by_product($product_id)>
-
- Description: Returns all product milestones that belong
-              to the supplied product.
-
- Params:      $product_id - Integer with a product id.
-
- Returns:     Bugzilla::Milestone object list.
-
 =item C<check_milestone($product, $milestone_name)>
 
  Description: Checks if a milestone name was passed in
diff --git a/Bugzilla/Product.pm b/Bugzilla/Product.pm
index 514620258..c6dd698b6 100644
--- a/Bugzilla/Product.pm
+++ b/Bugzilla/Product.pm
@@ -96,15 +96,22 @@ sub _init {
 
 sub components {
     my $self = shift;
+    my $dbh = Bugzilla->dbh;
 
     if (!defined $self->{components}) {
-        my @components =
-            Bugzilla::Component::get_components_by_product($self->id);
+        my $ids = $dbh->selectcol_arrayref(q{
+            SELECT id FROM components
+            WHERE product_id = ?}, undef, $self->id);
+
+        my @components;
+        foreach my $id (@$ids) {
+            push @components, new Bugzilla::Component($id);
+        }
         $self->{components} = \@components;
     }
     return $self->{components};
 }
-
+ 
 sub classification {
     my $self = shift;
 
@@ -117,20 +124,46 @@ sub classification {
 
 sub group_controls {
     my $self = shift;
+    my $dbh = Bugzilla->dbh;
 
     if (!defined $self->{group_controls}) {
-        $self->{group_controls} =
-            Bugzilla::Group::get_group_controls_by_product($self->id);
+        my $query = qq{SELECT
+                       groups.id,
+                       groups.name,
+                       groups.description,
+                       groups.isbuggroup,
+                       groups.last_changed,
+                       groups.userregexp,
+                       groups.isactive,
+                       group_control_map.entry,
+                       group_control_map.membercontrol,
+                       group_control_map.othercontrol,
+                       group_control_map.canedit
+                  FROM groups
+                  LEFT JOIN group_control_map
+                        ON groups.id = group_control_map.group_id
+                  WHERE group_control_map.product_id = ?
+                  AND   groups.isbuggroup != 0
+                  ORDER BY groups.name};
+        my $self->{group_controls} = 
+            $dbh->selectall_hashref($query, 'id', undef, $self->id);
     }
     return $self->{group_controls};
 }
 
 sub versions {
     my $self = shift;
+    my $dbh = Bugzilla->dbh;
 
     if (!defined $self->{versions}) {
-        my @versions =
-            Bugzilla::Version::get_versions_by_product($self->id);
+        my $values = $dbh->selectcol_arrayref(q{
+            SELECT value FROM versions
+            WHERE product_id = ?}, undef, $self->id);
+
+        my @versions;
+        foreach my $value (@$values) {
+            push @versions, new Bugzilla::Version($self->id, $value);
+        }
         $self->{versions} = \@versions;
     }
     return $self->{versions};
@@ -138,10 +171,17 @@ sub versions {
 
 sub milestones {
     my $self = shift;
+    my $dbh = Bugzilla->dbh;
 
     if (!defined $self->{milestones}) {
-        my @milestones =
-            Bugzilla::Milestone::get_milestones_by_product($self->id);
+        my $values = $dbh->selectcol_arrayref(q{
+            SELECT value FROM milestones
+            WHERE product_id = ?}, undef, $self->id);
+ 
+        my @milestones;
+        foreach my $value (@$values) {
+            push @milestones, new Bugzilla::Milestone($self->id, $value);
+        }
         $self->{milestones} = \@milestones;
     }
     return $self->{milestones};
diff --git a/Bugzilla/Version.pm b/Bugzilla/Version.pm
index c6c4bdae2..9492e8135 100644
--- a/Bugzilla/Version.pm
+++ b/Bugzilla/Version.pm
@@ -96,32 +96,6 @@ sub product_id { return $_[0]->{'product_id'}; }
 #####     Subroutines       ###
 ###############################
 
-sub get_versions_by_product {
-    my ($product_id) = @_;
-    my $dbh = Bugzilla->dbh;
-
-    my $stored_product_id = $product_id;
-    unless (detaint_natural($product_id)) {
-        ThrowCodeError(
-            'invalid_numeric_argument',
-            {argument => 'product_id',
-             value    => $stored_product_id,
-             function =>
-                'Bugzilla::Version::get_versions_by_product'}
-        );
-    }
-
-    my $values = $dbh->selectcol_arrayref(q{
-        SELECT value FROM versions
-        WHERE product_id = ?}, undef, $product_id);
-
-    my @versions;
-    foreach my $value (@$values) {
-        push @versions, new Bugzilla::Version($product_id, $value);
-    }
-    return @versions;
-}
-
 sub check_version {
     my ($product, $version_name) = @_;
 
@@ -152,7 +126,6 @@ Bugzilla::Version - Bugzilla product version class.
     my $product_id = $version->product_id;
     my $value = $version->value;
 
-    my $hash_ref = Bugzilla::Version::get_versions_by_product(1);
     my $version = $hash_ref->{'version_value'};
 
     my $version = Bugzilla::Version::check_version($product_obj,
@@ -190,15 +163,6 @@ Version.pm represents a Product Version object.
 
 =over
 
-=item C<get_versions_by_product($product_id)>
-
- Description: Returns all product versions that belong
-              to the supplied product.
-
- Params:      $product_id - Integer with a product id.
-
- Returns:     Bugzilla::Version object list.
-
 =item C<check_version($product, $version_name)>
 
  Description: Checks if the version name exists for the product name.
diff --git a/editcomponents.cgi b/editcomponents.cgi
index 096570d82..5b5333e31 100755
--- a/editcomponents.cgi
+++ b/editcomponents.cgi
@@ -92,12 +92,9 @@ my $product = Bugzilla::Product::check_product($product_name);
 
 unless ($action) {
 
-    my @components =
-        Bugzilla::Component::get_components_by_product($product->id);
-
     $vars->{'showbugcounts'} = $showbugcounts;
     $vars->{'product'} = $product->name;
-    $vars->{'components'} = \@components;
+    $vars->{'components'} = $product->components;
     $template->process("admin/components/list.html.tmpl", $vars)
         || ThrowTemplateError($template->error());
 
diff --git a/editmilestones.cgi b/editmilestones.cgi
index 3e484bbfc..3ca7ea998 100755
--- a/editmilestones.cgi
+++ b/editmilestones.cgi
@@ -80,12 +80,9 @@ my $product = Bugzilla::Product::check_product($product_name);
 
 unless ($action) {
 
-    my @milestones =
-        Bugzilla::Milestone::get_milestones_by_product($product->id);
-
     $vars->{'showbugcounts'} = $showbugcounts;
     $vars->{'product'} = $product->name;
-    $vars->{'milestones'} = \@milestones;
+    $vars->{'milestones'} = $product->milestones;
     $vars->{'default_milestone'} = $product->default_milestone;
     $template->process("admin/milestones/list.html.tmpl",
                        $vars)
diff --git a/editversions.cgi b/editversions.cgi
index d29e4d850..bc7420b19 100755
--- a/editversions.cgi
+++ b/editversions.cgi
@@ -88,12 +88,9 @@ my $product = Bugzilla::Product::check_product($product_name);
 #
 
 unless ($action) {
-    my @versions =
-        Bugzilla::Version::get_versions_by_product($product->id);
-
     $vars->{'showbugcounts'} = $showbugcounts;
     $vars->{'product'} = $product->name;
-    $vars->{'versions'} = \@versions;
+    $vars->{'versions'} = $product->versions;
     $template->process("admin/versions/list.html.tmpl",
                        $vars)
       || ThrowTemplateError($template->error());