Non-shippable items contribute to order weight

uc_order_condition_products_weight() is not correctly determining an order's total weight when it includes file downloads.

We sell books on our site, and those books have attributes that allow people to buy a print book or an ebook. The "base" book SKU (say, "SampleBook") is shippable, but the adjusted SKU (say "SampleBook-E") is set as not shippable. Let's say for instance that the "base" book is 2 lbs.

Here's how to reproduce the scenario I'm talking about:

1. Add one print book to your cart (SKU: SampleBook)
2. Add one ebook to your cart (SKU: SampleBook-E)
3. Checkout
4. The total weight of the order according to the uc_order conditional action is now 4lbs, and it should be 2lbs.

Because of this, we overcharged a number of customers for shipping until I noticed the problem and threw in a quick patch. Here's how I fixed the problem. Probably not the most elegant solution...

diff -ur ubercart/uc_order/uc_order.ca.inc ubercart-ktf/uc_order/uc_order.ca.inc
--- ubercart/uc_order/uc_order.ca.inc	2011-07-29 16:00:20.000000000 -0700
+++ ubercart-ktf/uc_order/uc_order.ca.inc	2011-08-11 11:18:49.175718505 -0700
@@ -733,9 +733,17 @@
   $totals = array('all' => 0);
   $total = 0;
   foreach ($order->products as $product) {
+
+    // When someone buys a non-shippable item (ie a file), it should NOT have a weight.
+    $result = db_query("SELECT * FROM {uc_file_products} WHERE model = '%s'", $product->model);
+    $actually_shippable = 1; // default to 1 -- if it's not in this table, it's physical goods and is shippable
+    while($data = db_fetch_object($result)) {
+      $actually_shippable = $data->shippable;
+    }
+
     $unit_conversion = uc_weight_conversion($product->weight_units, $settings['weight_units']);
-    $totals['all'] += $product->qty * $product->weight * $unit_conversion;
-    $totals[$product->model] = $product->qty * $product->weight * $unit_conversion;
+    $totals['all'] += $product->qty * $product->weight * $unit_conversion * $actually_shippable; // if it's not shippable, this sets weight to 0, otherwise unchanged
+    $totals[$product->model] = $product->qty * $product->weight * $unit_conversion * $actually_shippable; // same here
   }
   if (in_array('all', $settings['products'])) {
     $total = $totals['all'];

Note that the uc_order_is_shippable() function works correctly, because it allows other modules (namely uc_file) to hook into it with hook_cart_item(). So this bug only rears its head when an order is shippable, but contains one or more products that are not.

This exact same problem occurs in uc_taxes, by the way. Taxes are charged for non-shippable goods even when Ubercart is configured not to do so, because it only checks the "base" product and doesn't account for attributes and file downloads. I'm not sure if I should report that as a separate issue or not, but I solved that problem with a nearly identical version of the above patch.