This is aimed at improving Commerce Product URLs module by removing commerce_product_urls_entity_view() function - a huge chunk of code executed twice for each entity view (first time in commerce_product_reference_entity_view(), and then in commerce_product_urls_entity_view()).

This code currently needs replicating, as commerce_product_urls_entity_view() does not allow to specify default product which should be selected, so commerce_product_urls_entity_view() had to be introduced to accommodate for this.

Once attached patch makes its way into commerce_product_reference module, commerce_product_reference_entity_view() can be removed, which should be a significant performance gain. (Also, a first step of implementing unique URLs for particular products on product displays in Commerce core.)

Patch to follow.

CommentFileSizeAuthor
#8 commerce-product_reference_default_product-1861170-8.patch4.49 KBmaciej.zgadzaj
#5 commerce-product_reference_default_product-1861170-5.patch4.48 KBmaciej.zgadzaj
#4 commerce-product_reference_default_product-1861170-4.patch3.23 KBmaciej.zgadzaj
#3 commerce-product_reference_default_product-1861170-3.patch3.22 KBmaciej.zgadzaj
#2 commerce-product_reference_default_product-1861170-2.patch3.2 KBmaciej.zgadzaj
#1 commerce-product_reference_default_product-1861170-1.patch1.77 KBmaciej.zgadzaj

Comments

maciej.zgadzaj’s picture

maciej.zgadzaj commented
Status: Active » Needs review
StatusFileSize
new commerce-product_reference_default_product-1861170-1.patch1.77 KB
maciej.zgadzaj’s picture

maciej.zgadzaj commented
StatusFileSize
new commerce-product_reference_default_product-1861170-2.patch3.2 KB

As discussed over IRC, new patch with new approach.

maciej.zgadzaj’s picture

maciej.zgadzaj commented
StatusFileSize
new commerce-product_reference_default_product-1861170-3.patch3.22 KB

Small changes to docs - added a couple of "the"s and new lines...

maciej.zgadzaj’s picture

maciej.zgadzaj commented
StatusFileSize
new commerce-product_reference_default_product-1861170-4.patch3.23 KB

And slightly better version, passing array of product entities instead of wrappers as a second param to hook_commerce_product_reference_default_product_alter().

maciej.zgadzaj’s picture

maciej.zgadzaj commented
StatusFileSize
new commerce-product_reference_default_product-1861170-5.patch4.48 KB

New version provides the same option to change default product for commerce_cart_field_formatter_view() too.

fearlsgroove’s picture

fearlsgroove commented

This is working well for me for a similar need. I applied the patch and looked over the code (briefly) and didn't find anything to object to. Doesn't seem like much performance impact here, and the default behavior is unaffected.

Is there anything else holding this up?

summit’s picture

summit commented
Status: Needs review » Reviewed & tested by the community

Hi, would love to see this committed also!
Greetings, Martijn

maciej.zgadzaj’s picture

maciej.zgadzaj commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new commerce-product_reference_default_product-1861170-8.patch4.49 KB

Since it's still not committed, here is version not generating strict warnings (Only variables should be passed by reference) in commerce_product_reference_default_product() (in line with core reset() / key() approach).

Changing to needs review for bot to re-test, just in case.

rszrama’s picture

rszrama commented
Status: Needs review » Fixed

Committed this with a small change to make it explicit that the alter function is actually altering the delta and not the product itself.

Commit: http://drupalcode.org/project/commerce.git/commitdiff/b63b0d1

Status: Fixed » Closed (fixed)

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