Contrib.social

getDefaultVariation does not fire a FILTER_VARIATIONS event

Open on Drupal.org →
Created on 22 October 2024, 4 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
  • Comment 4 months ago →
    🇵🇭Philippines ambot112

    C is a good approach.

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

  • Comment 4 months ago →
    🇮🇱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.

  • Comment 4 months ago →
    🇵🇭Philippines ambot112

    Update the patch to use a function getFilteredVariations.

  • Status changed to Needs work 11 days ago
  • Comment 11 days ago →
    🇺🇦Ukraine lobodacyril

    For me, it seems obvious to select the default variation from the filtered variation list (option C). In my case, I remove some variations from the list in some conditions, so if I remove the first variation, then it can't be the default. In the case of the current code, I can pull the default variation, which doesn't consist in the current user list. Of course, it's possible to redefine using ProductEvents::PRODUCT_DEFAULT_VARIATION but I'm expecting another result.

    The patch looks fine to me but I can't apply it to drupal/commerce:2.40.0

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024