Notice Undefined property stdClass::$type in uc_product_kit_uc_order_product_alter()

I can confirm that I have the same php notice with today's dev.

The attached patch fixes the issue, but I wonder if this is the right place to do the node_load() ...

I don't think that will work as intended, as $product could contain data such as attributes, which node_load() will wipe out. The problem is higher up somewhere, I think related to the initial conversion of cart items into actual products.

The (pre-patch) code is pulling the $product object out of the $order, and node-specific fields like ->type or ->created etc. are not stored in the order (specifically, they're not stored in {uc_order_products}). I agree node_load() is overkill here, when all we want is the type. So to get the type here, it's either going to have to be pulled from the DB (e.g. SELECT type FROM node WHERE nid = $product->nid) or saved in {uc_order_products} earlier on in the process. We certainly don't want to do a node_load() earlier then store and carry around every single node field as part of the order.

Maybe it's important to add a type column in {uc_order_products} so that we have sufficient information to be able to run reports on things like type-based taxes from the information in our database. Currently, if the product class of a product is changed, that change would implicitly change all orders in the history and prevent us from properly reporting historical sales. Looking at the full $product object, the other field I think we should probably be saving is vid. Or, if we had the vid and really implemented product versioning, we wouldn't need to store type because we'd be able to reconstruct it from the nid and the vid.

Note that $product->data['module'] is available, so perhaps uc_product_kit should be checking that rather than the node type?

Also, the comment for uc_product_kit_uc_order_product_alter() is "The hookups for making product kits work on the order edit admin screen" - but drupal_alter('uc_order_product') actually gets called for all products every time an order is saved.. really we should consolidate this code somehow so "add to cart" and "add to order" use the same functions for splitting up the product kit when necessary, and maybe get rid of that "skip_save" hack for product kits.

@#5:

I've always disliked the way important information is serialized into the data column as part of the order - this makes it difficult and expensive to use - you can't for example do a simple query to find orders containing a particular atrribute, you have to retrieve all the orders and unserialize that column then filter in code. And the 'module' part of that array will only equal 'uc_product' or 'uc_product_kit' - there is no way to tell the product class. So while that might solve the instant problem, we really need the actual Drupal node type in general. I think it's totally reasonable, even expected (!), for an implementation of hook_uc_order_product_alter() to query the product type before taking action, so implementing a workaround that only works for product kits is just postponing the necessary general fix.

@#6:

I agree.

Tagging.

I don't like how the code is currently operating, but I think we can at least patch this so it works properly and so the PHP notice doesn't show. What I've done in this patch is to just add the type property to the $product instead of overwriting the whole product from the DB like I did in #2. I propose committing this patch to get it working, as a short-term solution, then revisiting the issue at our leisure in order to improve the code.

I don't think a node_load() is necessary there. I believe that hook_uc_order_product_alter() was originally created for products that were added by admins to orders that already existed. In this case, the $product has been constructed from a node so it has a type and all of the other node data that should be expected.

Furthermore, during checkout, the items in the cart will never be product kits anyway because they will already be separated. For a temporary fix until we can unify all of the alterations for items added to orders, I prefer just checking if type exists first.

Committed to 7.x-3.x.

Let's open another issue to try to deal with the hook_uc_order_product_alter() confusion.