Order product category condition matches non-taxonomy ids

This is an issue with the product module and affects anything that uses the condition plugin (promotion, shipping, etc).

This also affects the Order product item category condition plugin

  public function evaluate(EntityInterface $entity) {
    $this->assertEntity($entity);
    /** @var \Drupal\commerce_order\Entity\OrderItemInterface $order_item */
    $order_item = $entity;
    /** @var \Drupal\commerce_product\Entity\ProductVariationInterface $purchased_entity */
    $purchased_entity = $order_item->getPurchasedEntity();
    if (!$purchased_entity || $purchased_entity->getEntityTypeId() != 'commerce_product_variation') {
      return FALSE;
    }
    $term_ids = $this->getTermIds();
    $referenced_ids = $this->getReferencedIds($purchased_entity->getProduct());

    return (bool) array_intersect($term_ids, $referenced_ids);
  }

It looks like the getReferenceIds() function is only used by the Order Product Category & Order Product Item Category condition plugins so I'm inclined to think the getReferenceIds() function should be refactored to only retrieve taxonomy ids. Either that or it should compare UUIDs.

Can a maintainer chime in on how best to address this bug?

I agree this is a bug. At first I thought the method name needed to be updated to signify that it's only for terms, but then I realized it's part of there ProductCategoryTrait. Based on that we should update the API/docblock definition and limit the related field API calls to term reference fields.

I encounter same bug. But this time, it was the ID of a product (which is too an entity reference field 'product_id') which was the same ID of a product variation category, in a custom condition plugin for variation category

The fix was to update the getEntityReferenceFieldMap() method (applied on variation entities

protected function getEntityReferenceFieldMap() {
    $ignore_fields = ['type', 'uid', 'product_id'];
    $ignore_fields = array_combine($ignore_fields, $ignore_fields);
    $field_map = $this->entityFieldManager->getFieldMapByFieldType('entity_reference');
    $field_map = array_diff_key($field_map['commerce_product_variation'], $ignore_fields);
    foreach ($field_map as $field_name => $field_info) {
      foreach ($field_info['bundles'] as $bundle) {
        $field_definitions = $this->entityFieldManager->getFieldDefinitions('commerce_product_variation', $bundle);
        $field_definition = $field_definitions[$field_name];
        $target_type = $field_definition->getSetting('target_type');
        if ($target_type !== 'taxonomy_term') {
          unset($field_map[$field_name]);
        }
      }
    }
    return $field_map;
  }

Let's the bot test this. I wonder about performance implications (because of the call of FieldDefinitions() for the bundle inside the loop) about this patch, and if there is not a lighter way ?

Oups. commerce_product_variation instead of commerce_product

Another approach simplier.

Yes. But I don't understand why the $field_definition is NULL in the tests. It shoudn't.

I think we can all agree folks use Terms for categorization, and if someone uses any other kind of entity, well.. they can open a feature request. So we need to make sure the trait only returns entity references to taxonomy terms, per #10. So that looks good, but test failure is a mystery. Probably a test setup WTF.

EDIT: It is a Unit test which failed. So definitely a problem with our mocks.

This fixes the test. I'm slightly concerned we don't test the bug in question – verify it only works on taxonmy_term entity references.

EDIT: I forgot the other unit test.

Fixes the other unit test

Also, for any concern about being specific to taxonomy terms, see

  /**
   * Gets the vocabulary IDs used by products.
   *
   * @return string[]
   *   The vocabulary IDs.
   */
  protected function getVocabularyIds() {
    $vocabulary_ids = [];
    foreach ($this->getEntityReferenceFieldMap() as $field_name => $field_info) {
      foreach ($field_info['bundles'] as $bundle) {
        $field_definitions = $this->entityFieldManager->getFieldDefinitions('commerce_product', $bundle);
        $field_definition = $field_definitions[$field_name];
        if ($field_definition->getSetting('target_type') == 'taxonomy_term') {
          $target_bundles = $field_definition->getSetting('handler_settings')['target_bundles'];
          if (!empty($target_bundles)) {
            $vocabulary_ids = array_merge($vocabulary_ids, $target_bundles);
          }
        }
      }
    }
    $vocabulary_ids = array_unique($vocabulary_ids);

    return $vocabulary_ids;
  }

This looks good to me!

🥳 Thanks, everyone! Committed.