I made a patch that expose all (almost all for now) shipping data to views.

CommentFileSizeAuthor
#47 package_contents_view.txt4.4 KBDanZ
#47 package_view.txt6.38 KBDanZ
#47 shipment_view.txt5.54 KBDanZ
#44 ubercart_shipping_views-1911278-44.patch30.69 KBDanZ
PASSED: [[SimpleTest]]: [MySQL] 2,842 pass(es). View
#44 interdiff-37-44.txt14.98 KBDanZ
#37 ubercart_shipping_views-1911278-37.patch26.95 KBDanZ
PASSED: [[SimpleTest]]: [MySQL] 2,833 pass(es). View
#28 1911278-shipping-views-28.patch26.46 KBlongwave
PASSED: [[SimpleTest]]: [MySQL] 2,837 pass(es). View
#28 1911278-shipping-views-28-interdiff.txt18.08 KBlongwave
#23 uc_shipping-expose_fields_to_views-1911278-23.patch26.47 KBfiftyz
PASSED: [[SimpleTest]]: [MySQL] 2,839 pass(es). View
#16 uc_shipping-expose_fields_to_views-1911278-12-13.patch21.61 KBfiftyz
PASSED: [[SimpleTest]]: [MySQL] 2,799 pass(es). View
#11 expose_shipping_data_to_views_with_packages-7.patch19.96 KBfiftyz
PASSED: [[SimpleTest]]: [MySQL] 2,797 pass(es). View
#9 expose_shipping_data_to_views_with_packages-5.patch19.51 KBfiftyz
PASSED: [[SimpleTest]]: [MySQL] 2,803 pass(es). View
#7 expose_shipping_data_to_views_with_packages-4.patch19.58 KBfiftyz
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in expose_shipping_data_to_views_with_packages-4.patch. View
#5 expose_shipping_data_to_views_with_packages-3.patch19.52 KBfiftyz
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in expose_shipping_data_to_views_with_packages-3.patch. View
#2 expose_shipping_data_to_views-2.patch13.52 KBfiftyz
PASSED: [[SimpleTest]]: [MySQL] 2,797 pass(es). View
expose_shipping_data_to_views-1.patch967 bytesfiftyz
PASSED: [[SimpleTest]]: [MySQL] 2,795 pass(es). View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

TR’s picture

Status: Needs review » Needs work

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

fiftyz’s picture

FileSize
13.52 KB
PASSED: [[SimpleTest]]: [MySQL] 2,797 pass(es). View

:-) I didn't know that when I add new files to git i have to make the patch like this:

git diff --cached > expose_shipping_data_to_views-2.patch

Is it the correct way?

fiftyz’s picture

Status: Needs work » Needs review
DanZ’s picture

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'.

fiftyz’s picture

FileSize
19.52 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in expose_shipping_data_to_views_with_packages-3.patch. View

Packages support added.

Status: Needs review » Needs work

The last submitted patch, expose_shipping_data_to_views_with_packages-3.patch, failed testing.

fiftyz’s picture

Status: Needs work » Needs review
FileSize
19.58 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in expose_shipping_data_to_views_with_packages-4.patch. View

Fixed line endings.

Status: Needs review » Needs work

The last submitted patch, expose_shipping_data_to_views_with_packages-4.patch, failed testing.

fiftyz’s picture

Status: Needs work » Needs review
FileSize
19.51 KB
PASSED: [[SimpleTest]]: [MySQL] 2,803 pass(es). View

I realy don't get it... I converted all the lines to UNIX style. Another try.

fiftyz’s picture

fiftyz’s picture

FileSize
19.96 KB
PASSED: [[SimpleTest]]: [MySQL] 2,797 pass(es). View

And the last one made like mentioned in Advanced patch contributor guide. Hope this one is ok.

DanZ’s picture

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;
  }
fiftyz’s picture

You are right about shipment, I'll make it relation to order and not expose fields directly. This way will work also for any fieldable entity, including ordered products.

Transaction should exist for order / invoice or so at the carrier, because you have one order / invoice for multiple packages. This way you can keep track of this also. You are right about transaction as string. I'll make it string.

About order_id on packages then I'll expose it as relation to order.

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.

fiftyz’s picture

Assigned: Unassigned » fiftyz
Status: Needs review » Needs work
DanZ’s picture

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.

fiftyz’s picture

FileSize
21.61 KB
PASSED: [[SimpleTest]]: [MySQL] 2,799 pass(es). View

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?

Fixed: Exposed as relation so it will work for sure with entities.

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.

Fixed: Exposed as relation so an order can have unlimited number of shipments. It will create one row for each shipment.

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.

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

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.

:-))) Removed the TODO.

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.

Fixed: There are 2 relations now, one on Order: Packages and other on Shipment: Packages. So it should work fine now for any scenario.

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.

fiftyz’s picture

Status: Needs work » Needs review

Just to force test bot to check if everything is fine by now. Still needs work.

DanZ’s picture

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.

DanZ’s picture

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().

fiftyz’s picture

TODO: Because we now expose order_id or order_product_id or any other field from shipping tables used in a relationship we need to make them indexes because, if not, all querys where they are joined are very slow. Tested on mysql that scans the whole table on join. I've got querys with 40000ms exec time for a view with same records as orders but with a field that requires a relationship to any of this tables.

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

The same problem seems to be present in uc_payment module where fields were exposed to views recently too.

fiftyz’s picture

Status: Needs review » Needs work
DanZ’s picture

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.

fiftyz’s picture

Assigned: fiftyz » Unassigned
Status: Needs work » Needs review
FileSize
26.47 KB
PASSED: [[SimpleTest]]: [MySQL] 2,839 pass(es). View

This is almost the final patch. There are still few todo left for your consideration.

Added:
- dimensions fields;
- 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).
- indexes for uc_shipping, uc_packages and cu_packaged_products in schema.
- update indexes using hook_update_N.

longwave’s picture

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.

fiftyz’s picture

With pleasure longwave. I'll always try to help where I can and when I'll have time. And by the way, thank you for hard work on maintaining this vast project!!!

DanZ’s picture

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.

fiftyz’s picture

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

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.

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_id is the primary key so it is already indexed.

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.

This issue is "Expose shipping data to views" so we should keep focusing on this. It can be done but it should be done also in products. This is a very complex task and should have it's own issue. Weight untis should not be stored in db for products or any other tables. There should be an accepted standard weight unit for all weights and conversion to any other metric should be made before and after the standard weight is inserted / extracted from db.

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

The implementation is similar to uc_product so I don't see where is the issue here?!

It would be nice to have a field that combined the weight and weight unit values into one formatted display with uc_weight_format().

Already done that. Do you mean something else?

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.)

I don't realy see the use of this because you can rewrite dimensions as you like. Anyway it is easy to implement if maintainers want to.

longwave’s picture

FileSize
18.08 KB
26.46 KB
PASSED: [[SimpleTest]]: [MySQL] 2,837 pass(es). View

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.

DanZ’s picture

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'.

fiftyz’s picture

I don't see why and how tare weight will be calculated or even how ubercart should store it in db. Is simple to add a field for package and update the schema but the calculation differ from carrier to carrier so this is how I propose to implement it:
Product weight = product gross weight
You can calculate trough rules for example the tare weight. If a product has certain dimenssion you will need a diffrent box or packing materials. Also if the product has a certain weight you need a different box. All this calculations are a report of dimensions and weight right?
Uc should provide the quote via rules and we can set custom rules for every carrier based on type (fragile, regular), dimmensions and weight. In some cases also the cod value and insurance value. From those 5 factors exposed we could calculate any shipping from any courier, right?
So, why uc should store tare weight different than weight and if yes how you see this in a real life scenario? I implemented the rules calculations from here #1902622: Allow shipping quotes to use rules for shipping cost calculation and I can calculate all my shippings from interface.

DanZ’s picture

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.

fiftyz’s picture

At least, at currently state of weight in uc is confusing to name a fielde "Net Weight" when everywhere is named just "Weight". The user will ask his self "Hmmm, but where is the weight that I entered in product form? There was no net weight...".
I think that we need to build uc around what already is not around something MAYBE at some point will be and create confusions by doing so.

DanZ’s picture

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.

longwave’s picture

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.

fiftyz’s picture

It is easy, if we implement gross / tare weight we set that machine name tare_weight / gross_weight and let existing field as weight. Still I don't understand why uc should care about tare weight unless as package info which are 20 lines of code to implement?!

DanZ’s picture

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

DanZ’s picture

FileSize
26.95 KB
PASSED: [[SimpleTest]]: [MySQL] 2,833 pass(es). View

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

DanZ’s picture

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.

DanZ’s picture

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.

DanZ’s picture

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.

fiftyz’s picture

The packages are dependent to orders. I don't think they should be defined as base. This is why I didn't defined them in first place.

DanZ’s picture

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.

DanZ’s picture

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....

DanZ’s picture

FileSize
14.98 KB
30.69 KB
PASSED: [[SimpleTest]]: [MySQL] 2,842 pass(es). View

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....

longwave’s picture

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.

DanZ’s picture

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.

DanZ’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
5.54 KB
6.38 KB
4.4 KB

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.

longwave’s picture

Status: Reviewed & tested by the community » Fixed

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

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.