Expose shipping data to views

Looks like you're missing the actual handlers ...

Wonderful!

This is on my to-do list, but I have other stuff in the way. This is something that's glaringly missing from Ubercart.

Once this is done, some standard views can be added to the shipping interface, I think.

Using "git status -v" often helps when making a diff that deals with new files. Give it a try and look at the output.

I note that you don't expose packages. I'll just weigh in and say that it's important.

I would like to be able to set up a view of packages and add a Views Bulk Operations field to it. From there, I would like to bulk-print packing slips. Apart from the Views integration, this requires adding a template and an action for printing packing slips, but it's just something to keep in mind as you develop this. It would make Ubercart shipping much more practical.

Do you need separate tables for origin and destination country codes?

I think you have too many t()s here:

      'help' => t('The !field of the !address of the shipment.', array('!field' => t('full name'), '!address' => drupal_strtolower($address))),

Some minor naming issues:

+   * Override init function to provide generic option to link to user.

"shipment", not "user".

The 'link_to_sid' should be 'link_to_shipment'.

Bravo! I wish I'd had this when I started writing the Stamps.com module.

A few more notes:

  // Pull in ordered product fields directly.
  $data['uc_order_products']['table']['join']['uc_packaged_products'] = array(
    'left_field' => 'order_product_id',
    'field' => 'order_product_id',
  );

See #1884512: Make ordered products entity fieldable. I don't know much about constructing Views, but I think that this does not work for fieldable entities. Or maybe it does. Can you check?

  // For each order can be only one shippment so is safe to expose fields
  // directly.

This is not true. If an order has multiple packages, you can put some of the packages in one shipment, and some of the packages in a different shipment. In fact, this is necessary when some of the products are back-ordered.

  // Shipment Transaction ID field
  $data['uc_shipments']['transaction_id'] = array(
    'title' => t('Transaction ID'),
    'help' => t('The carrier\'s shipment identifier.'),
    'field' => array(
      'handler' => 'uc_shipping_handler_field_sid',
      'click sortable' => TRUE,
    ),
    'sort' => array(
      'handler' => 'views_handler_sort',
    ),
    'filter' => array(
      'handler' => 'views_handler_filter_numeric',
    ),
    'argument' => array(
      'handler' => 'views_handler_argument_numeric',
      'name field' => 'title',
      'numeric' => TRUE,
      'validate type' => 'transaction_id',
    ),
  );

Since this comes from the carrier, I don't think you can guarantee that it is numeric. It's a varchar(255) in the database schema. The same is true for the tracking ID.

Technically, there shouldn't even be a tracking ID on the shipment. It should be on the package. However, it's in both places in the schema....

  // Shipment Tracking Number field
  // @todo An option to external URL for tracking

That's not required. Any module that inserts that number can override it. Or, anyone who builds the view manually can use a rewrite rule to put in a URL.

  // @todo Why order_id exists in the uc_packages shcema. In order for a package
  //       to exist there must be a shippment first so the package should link
  //       only to it's shippment not directly to order.

It exists because the packages are created before the shipments are created. When there are no shipments, the packages still have to be tied to the orders.

  // @todo Package weight maybe is useful here.

Yes, it is essential. Unfortunately, it is not kept in the database. The weight is computed by uc_shipping_package_load() by just summing up the weight of the individual packaged products. This is incorrect, as it does not account for tare weight. Ideally, the {uc_packages} schema would include either the tare weight or the total weight.

That said, $package->weight could be put in as "total product weight".

Also, Views filters, sorts, and aggregation are based entirely on SQL. The value has to be either be in the database or have a handler that uses an SQL formula to compute the value. Anything computed with PHP doesn't work with aggregation. It should have a handler that contains this:

  function use_group_by() {
    return FALSE;
  }

About package weight I think the best way is to make a handler that use a subquery directly on SQL to get weight on products and so it will not be php and will be sortable.

This is already done for Ordered Products. See #1876496: Calculate additional Ordered Product fields for Views. Perhaps you can use that technique. Keep in mind that it still can't calculate total weight for the package, because Ubercart still doesn't have tare weight. Call it "net weight" or "total product weight" or something like that.

TODO: I did not have enaugh time to look at your code, in fact at the parent code to see how to SUM not just one product with qty but all products. Some help here will be great.

See the uc_order_handler_field_weight_total handler class for some hints. It overrides the query() method to use an SQL formula ($table.$field * $table.qty) to calculate the total weight.

Summing over all the different products in the package will require an SQL subquery. I don't know how to do that, much less with Views. I'll research it and let you know if I figure it out.

Fixed: Is defined as string now. My mistake I also have invoices for shipments like URG-XXXXX or DIV-XXXX.

You still have the shipment transaction ID and tracking number fields as numeric. I also don't think you want to use 'handler' => 'uc_shipping_handler_field_sid' for the package tracking number.

  $data['uc_packaged_products']['packaged_ordered_products'] = array(

This should be:

  $data['uc_packaged_products']['uc_order_products'] = array(

...for consistency with the definitions in uc_order_views_data().

Also, #1785434: Ubercart is not implementing the foreign keys setting correctly in schema needs to be finished when this work is done.

As you know, Drupal 7 does not use foreign keys. You can define them in the schema, but they don't do anything. They are essentially just documentation. This may change for Drupal 8. See http://drupal.org/node/146939.

Adding an index to a table requires a schema update. db_add_index() in a hook_update_N() would do it.

Thanks for working on this. I have a 90 minute train ride later today during which I will try and review this patch and the comments raised above.

Ok, I've given this an initial look.

/**
 * Add indexes for relationship fields.
 */
function uc_shipping_update_7002() {

Per hook_update_N() docs, I think this should be named uc_shipping_update_7300().

The header block comments appear on the page when update.php runs. They should be a little more descriptive, so:

/**
 * Add indexes to package and shipment tables.
 */
function uc_shipping_update_7300() {

db_add_index('uc_packaged_products', 'order_product_id', array('order_product_id'));
...
  // Expose packed products on their package as a relationship.
  $data['uc_packages']['packaged_products'] = array(

Does there also need to be an index on package_id? That will be necessary for finding out what products are in a package.

package weight (this cannot be based on SQL subquery because there is no way to determine in SQL weight_units for each product in package).

Actually, it can be done. It requires converting everything to a common weight unit in SQL, which requires conversion tables and mathematical formulas. This would be some difficult code, so it might be a good idea to add add a field with that later.

Note that package length/width/height fields have the same issue.

It would be nice to have a field that combined the weight and weight unit values into one formatted display with uc_weight_format(). It would also be nice to have a field that combined the length values and dimensions into one display field. (This could be done with rewriting, but it would be nice to have.)

Gotta go. I'll review it more later.

This looks really good. I barely use the shipping module, so real-world testing from those of you that do use this would be appreciated! As this is entirely new and is very unlikely to break any existing implementations, I'm also thinking to commit this early and deal with fixes and additional feature requests in followup issues.

An updated patch and interdiff are attached, the changes are mostly string and comment fixes, plus the addition of hook_date_views_* instead of using date_views_filter_handler_simple, which I believe is the correct way to integrate Date and Views into other Drupal modules. (uc_order does use the simple handler but we can't revert this now without affecting existing users)

I think you have too many t()s here:

      'help' => t('The !field of the !address of the shipment.', array('!field' => t('full name'), '!address' => drupal_strtolower($address))),

This looks odd, but is correct, and saves a bit of work for the translators (possibly at the expense of slightly incorrect grammar in some languages, but that doesn't really matter too much here).

Does there also need to be an index on package_id? That will be necessary for finding out what products are in a package.

As I understand it, when you use a composite primary key, the first field in the key (which here is the package_id) is automatically indexed.

It would also be nice to have a field that combined the length values and dimensions into one display field.

I suggested this way back in #730120-15: Dimension and package quantity fields in Views but it has not yet been implemented - and as this isn't yet available for products, it's probably outside the scope of this issue. As noted, it's already possible using Views' field rewriting.

I think this update should be run for all upgrades of ubercart including 6.x-2.x to 7.x-4.x for example not only for 7.x-3.x, or not?! I don't know, maybe a maintainer could explain more here.

hook_update_N numbering doesn't actually matter, it's just a convention; all updates are run in order. However, using 7300 here leaves 7002 to 7099 free to allow us to fix any as-yet unidentified bugs in the 6.x to 7.x upgrade process.

I suppose this is as good a place to bring this up as any. First, some definitions:

Tare weight

The weight of packaging: I.e., the box and the packing material, such as paper or styrofoam.

Net weight

The weight of the stuff being shipped. I.e., the products that your customer paid for.

Gross weight

The weight of the entire package. This is tare weight plus net weight

Ubercart only considers net weight. It gets it by adding up the weights of the products being shipped. It has no concept of gross weight or tare weight.

Shippers (UPS, Fedex, etc.) only care about gross weight. They absolutely require it for calculating shipping costs. Gross weight can be significantly different than net weight.

This is a problem, because it means that Ubercart shipping doesn't fully support any shipper. Shipping modules have to add on gross weight functionality in order to get the numbers right, as the Stamps.com module does. It really ought to be in Ubercart proper, since most shipping methods have to worry about this.

Unfortunately, fixing this is a big problem. It requires overhauling the shipping system, updating the schema to store tare or gross weight, etc. It may have to wait until D8/U4.

So why am I bringing this up here, if it won't be fixed here? Here's why: Leave room for Ubercart to do it right in the future without breaking existing views. The field that calculates the weight of all the products shouldn't be called "Weight". It should be called "Net weight". The machine name (array index) should be 'net_weight'.

I don't see why and how tare weight will be calculated or even how ubercart should store it in db.

That's an design/architecture discussion for a different issue. There are some decisions to be made.

For today, I just want you to leave room for it by calling the package net weight "Net weight", not "Weight" in Views.

Well, see the definition. "Net weight" only applies to packages. You can see net weight on products you buy at the grocery store.

I don't think that calling the net weight "Net weight" will be confusing. More importantly, it is correct. If it gets called "weight" here, that will be confusing. Is that "weight" the net weight or the gross weight? It is better to do it correctly.

You would expect "Package weight" to be the weight of the package. Well, here, it's not. It's the weight of the products in the package. So, calling it just "weight" is more confusing, because it's not true.

It could also be called "Total product weight", but that is longer. "Net weight" is the common English term for this concept.

In other news, I intend to see if I can use these views with the Stamps.com module. That module uses the package tracking number field when processing packages. It can store the value "queued" or "exported" before the package gets a shipping label. This stores the state of the shipping process for those packages. The module displays a table of all queued packages and exports their data in an XML file. It has queries to look up all the queued packages.

So, it would be useful to have an index on the tracking number field in the packages table, at least for that module. Is it worth doing adding this index here? Admins who want this to run faster can always add the index to the database manually, right?

Regardless, I'll be testing this new feature to see how it works with the Stamps.com module.

I have no real preference for "weight" vs "net weight" and can see arguments for both sides. However at present it seems unlikely we will implement tare weight until D8 (and perhaps even then that would be better off left to Packaging module?), we have not used the term "net weight" in Ubercart until now (consider translations), the machine name doesn't ultimately matter that much, and changing labels is possible at any point without breaking existing code, so perhaps "weight" will do for now.

FYI, I'll set up some views to test this new code this weekend.

Re-rolling #28 so it will apply to current -dev.

Just a heads-up. I've confirmed my suspicions about the hook_update_N() run sequence.

#1916806: Packages not deleted when order is deleted contains uc_shipping_update_7301(). Installing it first interferes with this patch, which contains uc_shipping_update_7300(). It prevents uc_shipping_update_7300() from running.

The two patches need to be pushed at the same time, or this one needs to be first, or the hook_update_N() numbers need to be adjusted.

Well, here's a rather basic problem: I can't make a Shipment view or Package view.

I made a test site and created some orders, packages, and shipments. I applied the patch, and can see the new files, and the update ran. I cleared the caches. I have the Shipping module enabled. I even rebooted.

However, when I go to create a new view, I can see all sorts of options (like Content and Orders), but Shipment and Package aren't there. So, I can't create a Shipment view or a Package view.

I don't know if that's intentional or not, but package views would be very very useful for my shipping module.

I created an order view, and added a shipment relationship. I then added all the shipment fields. That worked fine.

Did the same thing with an order->packages relationship. That also worked.

I note that the names of some of the field were a little weird. I don't know if this is a Views thing.

 Order: Order ID (Order ID)
(package) Package: Height (Height)
(package) Package: Length (Length)
Package: Package ID (Package ID)
Package: Package type (Package type)
Package: Shipment type (Shipment type)
Package: Tracking number (Tracking number)
Package: Value (Value)
(package) Package: Weight (Weight)
Package: Width (Width) 

Why is there both "(package) Package: Height (Height)" and "Package: Width (Width)"?

I'll be away from my test host until next weekend. I can run more tests then.

This needs lines like data['uc_shipments']['table']['base'] = array(...); to be defined. That's why shipments and packages aren't showing up as base tables.

I'm out of time for now, but I'll try to take a shot at it later if I get a chance.

The stamps.com shipping module would do very well with a view of packages awaiting export to XML. By the time packages get there, it doesn't need the order information. So, it's better for the packages to be the base table.

Also, I hope to make views of packages for VBO, for batch printing packing lists and/or shipping labels, exporting to stamps.com, etc. I believe that the packages have to be a base table for VBO to work on them.

In another issue, the update 7300 fails if those table indexes already exist (unlikely, but it could happen).

I'm testing a solution for both of these and trying to figure out how to make an interdiff when files are being added as part of the patch.... I'll post it when I'm done.

I found a bug:

  // Expose shipments to their packages as a relationship.
  $data['uc_packages']['shipments'] = array(
    'relationship' => array(
      'title' => t('Shipment'),
      'help' => t('Relate shipment to package.'),
      'handler' => 'views_handler_relationship',
      'base' => 'uc_packages',
      'base field' => 'sid',
      'relationship field' => 'sid',
      'label' => t('shipment'),
    ),
  );

That's supposed to be:

      'base' => 'uc_shipments',

Also, since it's possible to have packages without shipments, there needs to be a relationship from packages directly to orders.

I'm working on this now....

This patch update:

Adds a check to see if the table indexes already exist before creating them.

Establishes packages as a base table.

Establishes shipments as a base table.

Fixes the uc_packages => shipments bug mentioned in #43.

Fixes some comment language and punctuation style issues.

Adds a relationship from packages to their orders.

Adds a relationship from shipments to their orders.

Exposes the sid field to packages. This allows a link to the shipment edit page without setting up the relationship and its join.

Exposes the order_id field for packages and shipments. This allows a link to the order page without setting up the relationship and its join.

Rearranges data so that the order for each type is: group declaration, base table declaration, relationships, then fields.

Fixes the sid link handler to work with non-shipment (i.e., package) tables, so long as "order_id" is present.

I made the relationships separate declarations. I suspect that the declarations of relationships from packages to shipments and from packages and shipments to orders should have been wrapped into the declarations for the fields. This way works, though.... Should they be together or separate?

For example, I added the field for the sid to the uc_packages data. Should I have moved the relationship from packages to shipments into the declarations for that field, or just left them separate?

...and I think this interdiff will work....

I have only very briefly tested this but it looks good to me. If both DanZ and fiftyz are happy I think this can be committed, and any remaining bugs or features can be ironed out in followups.

Should I have moved the relationship from packages to shipments into the declarations for that field, or just left them separate?

I don't think this matters either way really, it's consistent whichever way you look at it and should be fine as long as we don't go changing the declarations around after people start making Views with this.

The only part I haven't tested yet is with the packaged products. After I or someone else test that, yes, I'd like to see it committed so I can build some new features into the stamps.com module.

One strange thing I noticed.... Why can order ID numbers be made to link to the order list page, instead of to the actual individual orders? Just curious.

Ok, I tested the packaged products, and it looks great. I'll attach the test views I used.

One thing I noticed is that the relationship to the packaged products isn't real useful, as it only gives quantity. It becomes useful when you add an additional relationship to ordered products. That also has a quantity, but it's the quantity ordered, not the quantity packaged. I guess this is the best way.

The next step is to add a field to packages that lists the contents in a simple way. There should be options for listing by SKU or product title, and for using a comma-separated list or a bullet list. It should be themable, of course. That can go into a separate issue, though.

Committed to 7.x-3.x. Thanks to both fiftyz and DanZ for working on this.