]> git.ipfire.org Git - thirdparty/bugzilla.git/commitdiff
Bug 288663: The inclusion and exclusion lists behave incorrectly when a product or...
authorlpsolit%gmail.com <>
Fri, 6 May 2005 02:25:56 +0000 (02:25 +0000)
committerlpsolit%gmail.com <>
Fri, 6 May 2005 02:25:56 +0000 (02:25 +0000)
Bugzilla/FlagType.pm
editflagtypes.cgi
template/en/default/admin/flag-type/edit.html.tmpl

index a428d5389acc45c284603355cc8d23079fffc76e..b28a40c143c54dcd77e72ae0fcd53a04106a537d 100644 (file)
@@ -89,26 +89,38 @@ sub get_exclusions {
     return get_clusions($id, "ex");
 }
 
+# Return a hash of product/component IDs and names
+# associated with the flagtype:
+# $clusions{'product_name:component_name'} = "product_ID:component_ID"
+
 sub get_clusions {
     my ($id, $type) = @_;
     
-    &::PushGlobalSQLState();
-    &::SendSQL("SELECT products.name, components.name " . 
-               "FROM flagtypes, flag${type}clusions " . 
-               "LEFT OUTER JOIN products ON flag${type}clusions.product_id = products.id " . 
-               "LEFT OUTER JOIN components ON flag${type}clusions.component_id = components.id " . 
-               "WHERE flagtypes.id = $id AND flag${type}clusions.type_id = flagtypes.id");
-    my @clusions = ();
-    while (&::MoreSQLData()) {
-        my ($product, $component) = &::FetchSQLData();
-        $product ||= "Any";
-        $component ||= "Any";
-        push(@clusions, "$product:$component");
-    }
-    &::PopGlobalSQLState();
+    my $dbh = Bugzilla->dbh;
+
+    my $list =
+        $dbh->selectall_arrayref("SELECT products.id, products.name, " .
+                                 "       components.id, components.name " . 
+                                 "FROM flagtypes, flag${type}clusions " . 
+                                 "LEFT OUTER JOIN products " .
+                                 "  ON flag${type}clusions.product_id = products.id " . 
+                                 "LEFT OUTER JOIN components " .
+                                 "  ON flag${type}clusions.component_id = components.id " . 
+                                 "WHERE flagtypes.id = ? " .
+                                 " AND flag${type}clusions.type_id = flagtypes.id",
+                                 undef, $id);
+    my %clusions;
+    foreach my $data (@$list) {
+        my ($product_id, $product_name, $component_id, $component_name) = @$data;
+        $product_id ||= 0;
+        $product_name ||= "__Any__";
+        $component_id ||= 0;
+        $component_name ||= "__Any__";
+        $clusions{"$product_name:$component_name"} = "$product_id:$component_id";
+     }
     
-    return \@clusions;
-}
+    return \%clusions;
+ }
 
 sub match {
     # Queries the database for flag types matching the given criteria
index b172cd996a3a8acce1550d887ff7014edff92328..f63369ce08a4a63350583cfdbf60f2b4965e7282 100755 (executable)
@@ -133,9 +133,11 @@ sub edit {
     # Otherwise set the target type (the minimal information about the type
     # that the template needs to know) from the URL parameter and default
     # the list of inclusions to all categories.
-    else { 
+    else {
+        my %inclusions;
+        $inclusions{"__Any__:__Any__"} = "0:0";
         $vars->{'type'} = { 'target_type' => $::FORM{'target_type'} , 
-                            'inclusions'  => ["__Any__:__Any__"] };
+                            'inclusions'  => \%inclusions };
     }
     
     # Return the appropriate HTTP response headers.
@@ -158,13 +160,13 @@ sub processCategoryChange {
     if ($categoryAction eq 'include') {
         validateProduct();
         validateComponent();
-        my $category = ($::FORM{'product'} || "__Any__") . ":" . ($::FORM{'component'} || "__Any__");
+        my $category = ($product_id || 0) . ":" . ($component_id || 0);
         push(@inclusions, $category) unless grep($_ eq $category, @inclusions);
     }
     elsif ($categoryAction eq 'exclude') {
         validateProduct();
         validateComponent();
-        my $category = ($::FORM{'product'} || "__Any__") . ":" . ($::FORM{'component'} || "__Any__");
+        my $category = ($product_id || 0) . ":" . ($component_id || 0);
         push(@exclusions, $category) unless grep($_ eq $category, @exclusions);
     }
     elsif ($categoryAction eq 'removeInclusion') {
@@ -173,7 +175,12 @@ sub processCategoryChange {
     elsif ($categoryAction eq 'removeExclusion') {
         @exclusions = map(($_ eq $::FORM{'exclusion_to_remove'} ? () : $_), @exclusions);
     }
-    
+
+    # Convert the array @clusions('prod_ID:comp_ID') back to a hash of
+    # the form %clusions{'prod_name:comp_name'} = 'prod_ID:comp_ID'
+    my %inclusions = clusion_array_to_hash(\@inclusions);
+    my %exclusions = clusion_array_to_hash(\@exclusions);
+
     # Get this installation's products and components.
     GetVersionTable();
 
@@ -186,8 +193,8 @@ sub processCategoryChange {
     $vars->{'action'} = $::FORM{'action'};
     my $type = {};
     foreach my $key (keys %::FORM) { $type->{$key} = $::FORM{$key} }
-    $type->{'inclusions'} = \@inclusions;
-    $type->{'exclusions'} = \@exclusions;
+    $type->{'inclusions'} = \%inclusions;
+    $type->{'exclusions'} = \%exclusions;
     $vars->{'type'} = $type;
     
     # Return the appropriate HTTP response headers.
@@ -198,6 +205,21 @@ sub processCategoryChange {
       || ThrowTemplateError($template->error());
 }
 
+# Convert the array @clusions('prod_ID:comp_ID') back to a hash of
+# the form %clusions{'prod_name:comp_name'} = 'prod_ID:comp_ID'
+sub clusion_array_to_hash {
+    my $array = shift;
+    my %hash;
+    foreach my $ids (@$array) {
+        trick_taint($ids);
+        my ($product_id, $component_id) = split(":", $ids);
+        my $product_name = get_product_name($product_id) || "__Any__";
+        my $component_name = get_component_name($component_id) || "__Any__";
+        $hash{"$product_name:$component_name"} = $ids;
+    }
+    return %hash;
+}
+
 sub insert {
     validateName();
     validateDescription();
@@ -231,16 +253,7 @@ sub insert {
                  $::FORM{'is_multiplicable'})");
     
     # Populate the list of inclusions/exclusions for this flag type.
-    foreach my $category_type ("inclusions", "exclusions") {
-        foreach my $category (@{$::MFORM{$category_type}}) {
-          my ($product, $component) = split(/:/, $category);
-          my $product_id = get_product_id($product) || "NULL";
-          my $component_id = 
-            get_component_id($product_id, $component) || "NULL";
-          SendSQL("INSERT INTO flag$category_type (type_id, product_id, " . 
-                  "component_id) VALUES ($id, $product_id, $component_id)");
-        }
-    }
+    validateAndSubmit($id);
     
     SendSQL("UNLOCK TABLES");
 
@@ -286,18 +299,7 @@ sub update {
               WHERE  id = $::FORM{'id'}");
     
     # Update the list of inclusions/exclusions for this flag type.
-    foreach my $category_type ("inclusions", "exclusions") {
-        SendSQL("DELETE FROM flag$category_type WHERE type_id = $::FORM{'id'}");
-        foreach my $category (@{$::MFORM{$category_type}}) {
-          my ($product, $component) = split(/:/, $category);
-          my $product_id = get_product_id($product) || "NULL";
-          my $component_id = 
-            get_component_id($product_id, $component) || "NULL";
-          SendSQL("INSERT INTO flag$category_type (type_id, product_id, " . 
-                  "component_id) VALUES ($::FORM{'id'}, $product_id, " . 
-                  "$component_id)");
-        }
-    }
+    validateAndSubmit($::FORM{'id'});
 
     SendSQL("UNLOCK TABLES");
     
@@ -500,3 +502,36 @@ sub validateAllowMultiple {
     $::FORM{'is_multiplicable'} = $::FORM{'is_multiplicable'} ? 1 : 0;
 }
 
+# At this point, values either come the DB itself or have been recently
+# added by the user and have passed all validation tests.
+# The only way to have invalid product/component combinations is to
+# hack the URL. So we silently ignore them, if any.
+sub validateAndSubmit ($) {
+    my ($id) = @_;
+    my $dbh = Bugzilla->dbh;
+
+    foreach my $category_type ("inclusions", "exclusions") {
+        # Will be used several times below.
+        my $sth = $dbh->prepare("INSERT INTO flag$category_type " .
+                                "(type_id, product_id, component_id) " .
+                                "VALUES (?, ?, ?)");
+
+        $dbh->do("DELETE FROM flag$category_type WHERE type_id = ?", undef, $id);
+        foreach my $category (@{$::MFORM{$category_type}}) {
+            trick_taint($category);
+            my ($product_id, $component_id) = split(":", $category);
+            # The product does not exist.
+            next if ($product_id && !get_product_name($product_id));
+            # A component was selected without a product being selected.
+            next if (!$product_id && $component_id);
+            # The component does not belong to this product.
+            next if ($component_id
+                     && !$dbh->selectrow_array("SELECT id FROM components
+                                                WHERE id = ? AND product_id = ?",
+                                                undef, ($component_id, $product_id)));
+            $product_id ||= undef;
+            $component_id ||= undef;
+            $sth->execute($id, $product_id, $component_id);
+        }
+    }
+}
index b5da2f52342d6589836f2d714a8c8b59b55623ee..b09c70cc9d8b934d87162ee5eb404b53cb46b47f 100644 (file)
   <input type="hidden" name="id" value="[% type.id %]">
   <input type="hidden" name="target_type" value="[% type.target_type %]">
   [% FOREACH category = type.inclusions %]
-    <input type="hidden" name="inclusions" value="[% category FILTER html %]">
+    <input type="hidden" name="inclusions" value="[% category.value FILTER html %]">
   [% END %]
   [% FOREACH category = type.exclusions %]
-    <input type="hidden" name="exclusions" value="[% category FILTER html %]">
+    <input type="hidden" name="exclusions" value="[% category.value FILTER html %]">
   [% END %]
 
   <table id="form" cellspacing="0" cellpadding="4" border="0">