Problem/Motivation

Currently, the product variation fields are added to the Product render array in commerce_product_commerce_product_view() function.

When someone needs to rewrite that logic, it's necessary to implement that hook, or the _alter() version to override that behavior. That also forces to redo that logic again, hitting performance.

That's why we think a good improvement would be to add a service where that task in performed. In this way, this logic can be replaced without need to redo all the processing when users need to rewrite the default behavior.

Proposed resolution

Add the concept of ProductVariationRenderer, as tagged service where implement the logic explained above
Create a ProductVariationRendererManager service that takes care of managing the different Product Variation Renderers
Create a default service ProductVariationRendererDefault that implements the code the currently is in commerce_product_commerce_product_view()

Remaining tasks

User interface changes

None

API changes

Add a new service to manage Product Variation rendering inside products

Data model changes

CommentFileSizeAuthor
#4 add_a_new_service_to-2805625-4.patch3.98 KBplopesc
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

plopesc created an issue. See original summary.

bojanz’s picture

What's the use case for this? How are you customizing the rendering? Are you customizing it per product type, or just once per site?

I am not convinced about the resolver architecture here, feels like it might make more sense to have a single swappable service.

mglaman’s picture

Status: Active » Needs work

We have PR. I don't think we need resolver architecture either. but either a single service, or EntityViewBuilder implementation that can be altered/swapped out.

plopesc’s picture

Title: Add a new service to manage the product variation rendering » Add a new ProductViewBuilder to manage the product variation rendering
Status: Needs work » Needs review
FileSize
3.98 KB

Attached patch and changing issue name to match the requirements included in the Github PR.

Thanks

mglaman’s picture

:D Looks good! Let's see what Travis thinks.

  • bojanz committed ca260b9 on 8.x-2.x authored by plopesc
    Issue #2805625: Add a new service to manage the product variation...
bojanz’s picture

Status: Needs review » Fixed

Boom!

Status: Fixed » Closed (fixed)

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