getDefaultVariation does not fire a FILTER_VARIATIONS event

Created on 22 October 2024, 2 months ago

Better solution to PRODUCT_DEFAULT_VARIATION event

Currently the FILTER_VARIATIONS event doesn't fire when getDefaultVariation is called. This is confusing as for example if you implement a subscribe on FILTER_VARIATIONS and had sucessfully filtered a list of variations - if all by one variation which shouldn't be shown are removed this would now be visible on the product page.

When trying to implement a filter on variations such that users are not displayed variations that meet some criteria for example variations that show only to logged in users with role 'x' there are various guides on achieving this via subscribing to the FILTER_VARIATIONS event. And this works nicely except in the case where the defaultVariation would not pass the filter criteria.

In https://www.drupal.org/project/commerce/issues/2835629#comment-13610533 there is some discussion about which event should be fired and it is noted that some options trigger an ProductEvents::FILTER_VARIATIONS event.

I think some options are to consider using:
A) "However, we usually recommend using \Drupal\commerce_product\ProductVariationStorage::loadFromContext and \Drupal\commerce_product\ProductVariationStorage::loadEnabled " note that the other related issue flags some issues with the loadFromContext route

B) Or possibly as we have an event maybe the solution here is to update the documentation to make it clear that FILTER_VARIATIONS Event - doesn't apply to loading the default variation - and as such if you are subscribing to FILTER_VARIATIONS you will also need to subscribe to PRODUCT_DEFAULT_VARIATION.

C) Trigger a FILTER_VARIATIONS event prior to looping over the list of variations. This seems like we then have two events to do the same thing - however as noted on https://www.drupal.org/project/commerce/issues/2835629 it might be the case that the PRODUCT_DEFAULT_VARIATION subscribe is reordering rather than filtering.

Feature request
Status

Active

Version

3.0

Component

Developer experience

Created by

🇳🇿New Zealand luke.stewart

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @luke.stewart
  • 🇵🇭Philippines ambot112

    C is a good approach.

    However, other modules can subscribe the PRODUCT_DEFAULT_VARIATION event and do additional variation filtering.

  • 🇮🇱Israel jsacksick

    hm... The patch doesn't look correct, as you're not even looping on $enabled_variations... And btw the variable has a confusing name... $this->getVariations() returns all variations, regardless on whether they're enabled or not.

    Not sure yet whether we should proceed with that.

  • 🇵🇭Philippines ambot112

    Update the patch to use a function getFilteredVariations.

Production build 0.71.5 2024