Trivial patch. Whether this is correct or needs deeper work I do not know. I can't find anything broken at least.

CommentFileSizeAuthor
notice_1093.patch800 byteschx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rszrama’s picture

Thanks, chx. I'm worried about what would happen throughout line item price calculation / order total calculation if we have an empty price. Theoretically speaking, products should never not have a value in the commerce_price field... hmm, unless it was unset during calculation but a line item was still created based on it. I don't suppose you can share how you ran into the notice so I can play with it locally?

Zach Harkey’s picture

This fixed the issue for me. Thanks.

(Note: I had trouble applying the patch — ended up applying the changes manually.)

jordojuice’s picture

I ran into it after I created test products and orders using Commerce Devel, so I was assuming this notice was a result of improperly configured orders being generated by that module. Thought it may help you get on the right track to figuring out if this is a result of Drupal Commerce or of another module's usage of the API.

jordojuice’s picture

A little more information. This seems to happen with Commerce Devel when I generate products and related product displays when the product display's product reference field does not support the type of product being generated. So, in my experience this seems to be a result of some issue with the product reference field, which I know from working with the code deals in line items (though I'm admittedly not that savvy about what's going on in there). Of course, Commerce Devel can do things that a user may not be able to do through the UI, but hopefully this little bit of information can help you track down what might be the cause in a scenario where a user can cause this issue (if it exists).

rszrama’s picture

That's a good lead; technically speaking, you shouldn't end up with a product type that doesn't have a commerce_price field, but it sounds like perhaps Commerce Devel is generating products that have don't end up getting a commerce_price property on them even if the field should be there.

I guess the question then is do we want to pass the same problem along to the created line item? Or should we perhaps set $commerce_line_item->commerce_unit_price to some alternate value, such as a price with a NULL amount which we do actually use elsewhere to represent the inability to find a price for a product.

madelyncruz’s picture

notice_1093.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, notice_1093.patch, failed testing.

rszrama’s picture

Status: Needs work » Closed (cannot reproduce)

I think for now I'm going to postpone making a change to commerce_product_line_item_populate(), as I can't really figure out how to create this scenario. I took a clean install of Kickstart 1.x and first manually deleted the price data for a product from the database and then deleted the value of a required product reference field but got different notices, not the one reported.

However, having fixed a separate issue with chx, I believe the original report may have been caused in some roundabout way by a migration updating a product but from entity_create() (i.e. with a half-formed product entity, one without its price field) and perhaps saved that incomplete product entity in the cache and tried to create a line item using that cached product later. That's a pretty roundabout process, but I was also unable to reproduce the precise notice by simple using entity_create() to whip up a new product and pass it through commerce_product_line_item_new().

In the end, I think I want to fall back on the function's doc block, which specifies that $product should be a "fully loaded product entity." There can be no fully loaded product entity that doesn't have a commerce_price field, and if a site is generating one and trying to pass it through this function, I think it's better for there to be a notice than to silently assume the price is NULL as I suggested above.

@chx - perhaps we need to file a bug report in Commerce Migrate if you're using that to create your products / product displays?

To the others in the thread, if you continue to have issues with Commerce Devel, I'd file a bug report in its issue queue.

To anyone who's using the patch in production, I'd advise you to get to the source of why you have a product without the commerce_price field. I don't think you'll have any negative consequences from a user facing standpoint, because Commerce will just assume the price is NULL and therefore render the product unpurchasable in the Add to Cart form.

madelyncruz’s picture

Status: Closed (cannot reproduce) » Needs review

notice_1093.patch queued for re-testing.

rszrama’s picture

Status: Needs review » Closed (cannot reproduce)

Reopening this without interacting with previous comments is not helpful.

ArAnManDapCel’s picture

Issue summary: View changes

Apparently the issue still occurs in version 7.x-1.11.
I've applied the same solution, but now the code is at row 1374.

Arne Slabbinck’s picture

I'm facing the same problem in version 7.x-1.11 .
It occured when transferring configuration from a development site to a live site using features. I did enable all commerce modules & dependencies in one go. My products don't have a commerce_price field so I can't add a price..

EntityMetadataWrapperException: Unknown data property commerce_product. in EntityStructureWrapper->getPropertyInfo() (regel 335 van /home/welzijnszorg/drupal/sites/all/modules/entity/includes/entity.wrapper.inc).

commerc_repaire module didn't fix this

philsward’s picture

Not sure if I should comment here or somewhere else, but I got something "similar" when trying to apply a rule to a panel pane, to hide the pane if commerce price was 0.00. Now I can't get into the content of the panel variant :-/

error:

Notice: Undefined property: stdClass::$sku in commerce_product_line_item_populate() (line 1376 of /home/user/public_html/sites/all/modules/contrib/commerce/modules/product_reference/commerce_product_reference.module).
Notice: Undefined property: stdClass::$product_id in commerce_product_line_item_populate() (line 1382 of /home/user/public_html/sites/all/modules/contrib/commerce/modules/product_reference/commerce_product_reference.module).
EntityMetadataWrapperException: Invalid data value given. Be sure it matches the required data type and format. Value at commerce_line_item()->commerce_product: . in EntityDrupalWrapper->set() (line 756 of /home/user/public_html/sites/all/modules/contrib/entity/includes/entity.wrapper.inc).

EntityMetadataWrapperException: Invalid data value given. Be sure it matches the required data type and format. This is the error I would receive as a 500 response error when trying to apply the rule on the pane. I'm guessing it saved it in the DB, but since the code is goofy, it's throwing everything else off.

rszrama’s picture

@Phil Unfortunately, it's hard to say what's gone wrong from your comment here. Whatever function in your codebase is calling commerce_product_line_item_populate() isn't passing in the required parameters, a line item object and a product object. That's why you're getting those undefined property errors ... otherwise every line item object has a line_item_label property and every product a product_id. Could it be an issue with some module intending to integrate Commerce + Panels?

philsward’s picture

@rszrama Other than the core commerce integration with panels, I don't have anything else that is trying to insert commerce into panels...

Off the top of my head, I forget the exact setup I was shooting for, but I believe I had the pricing (custom MSRP as commerce price field, commerce Price field, custom text field "call for price") all in a mini panel. On the main panel page, I was trying to hide the mini panel pane "if" commerce price was 0.00. At least I think that's what I was trying to accomplish. I've slept since then. Maybe I was trying to hide it on the mini panel, or maybe I had all of the pricing panes on the actual panel. Either way, trying to set a rule against commerce price if the price was 0.00 (empty), blew up the variants content. It's odd that a price of 0.00 was blowing it up seeing that 0.00 technically isn't "empty".

The work around was to set a custom Boolean field as "Hide Price?" on the content type and if yes, hide the commerce price through a panels rule. More overhead and more human error, but it works for now.

Point is, panels rules don't like to work if there is an "empty" price of 0.00. Just wanted to throw out the errors I was receiving in case it helps.

FYI: Did not test the patch.

philsward’s picture

(original comment replaced)

Ran into the same problem I described above on a different website. Created new issue to reflect the problem: #2919200: Adding A Panels Rule Checked Against Price Blows Up The Panel Variant