The form created by uc_shipping_shipment_edit() has editable fields for changing the origin and destination e-mail addresses. Also, for each package, it has fields for entering a custom weight.

However, when you click the "Save shipment" button, uc_shipping_shipment_edit_submit() does not save the values in these fields. Anything you type is lost. This is not due to a bad line of code. It just doesn't have any code to save the shipment information.

This is probably related to the fact that the uc_shipments table doesn't have any fields for storing e-mail addresses and the uc_packages table doesn't have any fields for saving package weights.

Doing this properly would require updating the schema for packages and shipments.

The fields are still useful, as uc_shipping_shipment_edit() can be used as the basis of a form for a custom module that might store the values in parallel tables or use the values directly to generate a label or such.

Comments

longwave’s picture

As the email addresses and package weight are not actually used anywhere, should we just drop them from this form? The weight is always calculated from the ordered products, so we have no need to store it separately, and I am not sure what use it is tracking an email address for the source and destination of a shipment anyway?!

Also this goes to show how few people use the shipping and packaging parts of Ubercart, as this code appears to be have been around forever, but nobody has noticed it doesn't actually work until now.

longwave’s picture

Priority: Normal » Minor

Reclassifying this as minor as it's ultimately a cosmetic issue; these fields are simply never used for anything.

Dan Z’s picture

I believe that just dropping it from the form would break shipping modules that use the form (or an alter() version of it) instead of re-inventing the wheel. I am working on just such a module for Stamps.com XML export, for example. None of the core Ubercart modules do this, though.

The e-mail address is used because some shipping APIs send a notification to the package sender or recipient when the label is generated, the package is picked up, and the package is delivered. A better question is whether anyone will need a last-moment change of the e-mail address instead of using the store and order e-mail addresses.

The weight field is useful to add-on modules because the calculated weight is always wrong. I think that the package weight is calculated in uc_shipping_package_load() by this SQL:

      $result = db_query("SELECT op.order_product_id, pp.qty, pp.qty * op.weight AS weight, op.weight_units, op.nid, op.title, op.model, op.price, op.data FROM {uc_packaged_products} AS pp LEFT JOIN {uc_order_products} AS op ON op.order_product_id = pp.order_product_id WHERE pp.package_id = :id ORDER BY op.order_product_id", array(':id' => $package_id));

I'm not an SQL expert, but I think that just means that it adds up the weight of the items in the package. Of course, that doesn't take into account packaging (tare weight) or errors. A shipper with a scale would be able to adjust and enter on the form.

The e-mail addresses come from uc_shipping_address_form(), and would have to be modified after that call.

The simplest thing to do would be to make the fields non-editable and warn module maintainers (if any) about the change.

SilviuChingaru’s picture

Status: Active » Closed (duplicate)

When custom fields will be available for shipment, packages and packaged products this will be solved so I'll mark it as duplicate of #2060649: Make uc_shipments fieldable entities.