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

Files: 
CommentFileSizeAuthor
notice_1093.patch800 byteschx
PASSED: [[SimpleTest]]: [MySQL] 3,557 pass(es).
[ View ]

Comments

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?

This fixed the issue for me. Thanks.

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

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.

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

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.

notice_1093.patch queued for re-testing.

Status:Needs review» Needs work

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

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.

Status:Closed (cannot reproduce)» Needs review

notice_1093.patch queued for re-testing.

Status:Needs review» Closed (cannot reproduce)

Reopening this without interacting with previous comments is not helpful.