When editing a product, and "Create new revision" is checked - Ubercart keeps adding the revision information to the uc_products table without cleaning up the other versions within that table. This becomes an issue when packaging an order:

-- uc_shipping.module - line 350
$result = db_query("SELECT op.order_product_id, pp.qty, pp.qty * op.weight AS weight, p.weight_units, op.nid, op.title, op.model, op.price FROM {uc_packaged_products} AS pp LEFT JOIN {uc_order_products} AS op ON op.order_product_id = pp.order_product_id LEFT JOIN {uc_products} AS p ON op.nid = p.nid WHERE pp.package_id = %d", $package_id);

The above query will select everything from the uc_products table.

The issue is if you have multiple revisions of a product, it will show up multiple times in the admin/store/orders/{order#}/shipments/{shipment#}/view page i.e.

Package 25:
Contents: 1 x 123456789-1, 1 x 123456789-1, 1 x 123456789-1, 3 x abcdefg-1, 3 x abcdefg-1
Package type: Customer Supplied Package
Insured value: $199.00

1 x 123456789-1 - Revision 1
1 x 123456789-1 - Revision 2
1 x 123456789-1 - Revision 3
3 x abcdefg-1 - Revision 1
3 x abcdefg-1 - Revision 2

This is a problem because Ubercart will calculate the total weight including the revisions then creates a label with that incorrect weight on it.

The query should be:

$result = db_query("SELECT DISTINCT op.order_product_id, pp.qty, pp.qty * op.weight AS weight, p.weight_units, op.nid, op.title, op.model, op.price FROM {uc_packaged_products} AS pp LEFT JOIN {uc_order_products} AS op ON op.order_product_id = pp.order_product_id LEFT JOIN {uc_products} AS p ON op.nid = p.nid WHERE pp.package_id = %d", $package_id);

OR

$result = db_query("SELECT op.order_product_id, pp.qty, pp.qty * op.weight AS weight, p.weight_units, MAX(p.vid) as version, op.nid, op.title, op.model, op.price FROM {uc_packaged_products} AS pp LEFT JOIN {uc_order_products} AS op ON op.order_product_id = pp.order_product_id LEFT JOIN {uc_products} AS p ON op.nid = p.nid WHERE pp.package_id = %d GROUP BY op.title", $package_id);

Comments

Island Usurper’s picture

StatusFileSize
new2.47 KB

Instead of using GROUP BY or DISTINCT, I think JOINing on the node table is more efficient. At least, that's how I read the EXPLAIN queries. I also noticed that I had somehow missed an index on uc_products.nid, which is very important for making the JOIN fast.

I think the index disappeared during update 6000, because it's in hook_schema() correctly. It's impossible to cleanly add or remove an index from only a subset of installations, though, so I don't think I'm going to do anything about current installations. However, if someone still needs to upgrade from Drupal 5, they won't have this problem.

McBastard’s picture

I rejiggered the query a little. I ran into an issue where the weight units were not being pulled correctly based on the product version.
** Anything in {uc_products} table should really be queried directly (i.e. weight_units) because versioning produces a lot of false positives and the GROUP BY and/or DISTINCT clauses does not ensure you will get the newest records. JOINing to the node table didn't help fix the issue. I didn't explore this in any great detail but the query below solves the issue.

$result = db_query("SELECT op.order_product_id, pp.qty, pp.qty * op.weight AS weight, (SELECT weight_units FROM {uc_products} WHERE vid = MAX(p.vid)) AS weight_units, MAX(p.vid) as version, op.nid, op.title, op.model, op.price FROM {uc_packaged_products} AS pp LEFT JOIN {uc_order_products} AS op ON op.order_product_id = pp.order_product_id LEFT JOIN {uc_products} AS p ON op.nid = p.nid WHERE pp.package_id = %d GROUP BY op.title", $package_id);

McBastard’s picture

StatusFileSize
new1.5 KB

I've added a patch with my re-jiggered query for anyone who needs it.

McBastard’s picture

Status: Needs review » Patch (to be ported)
tr’s picture

Status: Patch (to be ported) » Needs review

"needs review" is the correct status still.

Island Usurper’s picture

I'd really like to know why using the JOIN on the node table didn't work. {node}.vid should always be the correct vid for {uc_products}, so unless the data isn't being saved correctly between revisions, that query ought to work.

wamilton’s picture

Deleting node revisions does not delete the revision in the uc_products table.

danny englander’s picture

Status: Needs work » Needs review

I just discovered I am having the same issue. It was like finding a needle in a haystack but at least now I have an explanation. So based on comment #7 above, simply deleting the node revision within Drupal does not solve my issue?
----------
Update: confirmed, deleting node revision(s) has no effect.

Status: Needs review » Needs work

The last submitted patch, uc_shipping.module.patch, failed testing.

wamilton’s picture

Status: Needs review » Needs work

Sorry, my comment was needlessly obtuse: you need to apply the hack from comment 2 that makes it count the weight properly. After I did that I had no more problems (with this part of Ubercart).

jrussell’s picture

Version: 6.x-2.2 » 6.x-2.x-dev
StatusFileSize
new1.36 KB

I just ran into this too. I added the JOIN from comment #1 to the current dev version. It is working here. I did not make the change in uc_products.install since I'm not certain of it's effects.

wamilton’s picture

Well, it's not exactly surprising that applying a hack meant for one file to another didn't have the desired effect. ;)

I'm not trying to nit-pick your grammar, but just to be clear, you CHANGED the offending join right? You didn't just add another one? I hazily recall also that there were two similar looking joins in the non-dev.

Have you tried using the hack on the given version AND deleting the revisions from {uc_products} and {node}, not all at once but just one at a time?

longwave’s picture

Implementing #1118036: add weight_unit to {uc_order_products} table would make this query much simpler and solve the problem of the weight units being unknown if the product is deleted after it has been ordered.

gabor_h’s picture

Status: Needs work » Needs review

I just ran into this too.

It seems to me that there are two separate problems mixed in this issue:
1) Wrong order weight when creating packages with revisioned products
2) Weight units not being pulled correctly based on the product version

It seems to me that 1) is not solved yet, because solving 2) is not so easy.

And according to the comment at http://drupal.org/node/1118036#comment-5036108 it seems to me that 2) will not be solved very soon. (TR wrote: "What needs to be done is to enforce versioning on {uc_products}" ... "Because this involves a significant change in the code and the data, it should be addressed in 7.x-3.x first. It's unlikely to be backported to 6.x-2.x.")

However, 1) has been a problem since January 2010, and although it is easy to fix and there are suggestions for that, it is still an open problem.

The last submitted patch of #11 works, and it solves the original problem of 1).
True, the patch does not solve 2), and I am aware that if the weight unit of a revisioned product changes, the weight unit might not be pulled correctly when creating packages. Later, when the time is ripe, the problem of 2) can be solved as well, and the affected query can be improved.

Could you please test patch #11 and include it in the next dev version, so that we can move forward?
This way, at least the weight value will be calculated correctly when using versioned products.

Thank you,
Gabor

Island Usurper’s picture

Version: 6.x-2.x-dev » 7.x-3.x-dev

#1118036 got accidentally committed and added to rc1, so this is probably fixed. Note that you need to get a followup patch in that issue for the order edit form to not set your product weights to 1, because I screwed up the release.

Status: Needs review » Needs work

The last submitted patch, 679094_package_query-11.patch, failed testing.

longwave’s picture

Status: Needs work » Closed (fixed)