Skip to content

Bug - Magento_Eav - eav_attribute_group (repository) filtering #23172

@LucianMihalache

Description

@LucianMihalache

There is a bug on the Magento_Eav Module, to be more precise, there is a bug when you want to filter the attribute_group collection by multiple values (IN).

*The bug affects rest api too

Preconditions (*)

  1. Any Magento 2.3.1 that does not overwrite Magento_Eav module
  2. Some data on the site, I tested with sampledata

Steps to reproduce (*)

  1. Try to filter the eav_attribute_group (AttributeGroupRepository) by multiple attribute_set_id's or by multiple attribute_group_code

If you try to filter by attribute_group_code, you won't be able to filter at all.

/**
 * @var \Magento\Eav\Api\AttributeGroupRepositoryInterface
 */
protected $attributeGroupRepository;

/**
 * @var \Magento\Framework\Api\Search\SearchCriteriaBuilderFactory
 */
protected $searchCriteriaBuilderFactory;

/**
 * @var \Magento\Framework\Api\FilterBuilder
 */
protected $filterBuilder;

public function __construct(
    \Magento\Eav\Api\AttributeGroupRepositoryInterface $attributeGroupRepository,
    \Magento\Framework\Api\Search\SearchCriteriaBuilderFactory >$searchCriteriaBuilderFactory,
    \Magento\Framework\Api\FilterBuilder $filterBuilder
) {
    $this->attributeGroupRepository = $attributeGroupRepository;
    $this->searchCriteriaBuilderFactory = $searchCriteriaBuilderFactory;
    $this->filterBuilder = $filterBuilder;
}

public function test()
{
    /** @var \Magento\Framework\Api\Search\SearchCriteriaBuilder $criteriaBuilder */
    $criteriaBuilder = $this->searchCriteriaBuilderFactory->create();
    $setIdFilter = $this->filterBuilder
        ->setField('attribute_set_id')
        ->setValue('15,10,4,14,11,12,13,9') // All the attribute_set_id from that are present on clean install with sampledata
        ->setConditionType('in')
        ->create();

    $groupCodeFilter = $this->filterBuilder
        ->setField('attribute_group_code')
        ->setValue('"advanced-pricing", "autosettings", "bundle-items", "content"')
        ->setConditionType('in')
        ->create();

/* I am using directly imploded strings because this is the format I need when I am doing rest api 
   get, and since is the same repository, it works the same way.*/

    $criteria = $criteriaBuilder->addFilter($setIdFilter)->create();
    //$criteria = $criteriaBuilder->addFilter($groupCodeFilter)->create();

    $groupList = $this->attributeGroupRepository->getList($criteria);
    //Do something with $groupList
    var_dump($groupList->getSearchCriteria()->getFilterGroups()[0]->getFilters());
    var_dump($groupList->getTotalCount());
    foreach ($groupList->getItems() as $group) {
        var_dump('AttributeSetId: ' . $group->getAttributeSetId());
    }
    die('asdfasfasdf');
}

Expected result (*)

  1. Returning all the groups assigned to all the attribute_set_id's given.
    In the database, that means, 80 records (sampledata)

select

Actual result (*)

    ....php:102:
    array (size=1)
   0 => 
     object(Magento\Framework\Api\Filter)[1784]
       protected '_data' => 
         array (size=3)
           'field' => string 'attribute_set_id' (length=16)
           'value' => string '15,10,4,14,11,12,13,9' (length=21)
           'condition_type' => string 'in' (length=2)
     ....php:103:int 10
     ....php:105:string 'AttributeSetId: 15' (length=18)
     ....php:105:string 'AttributeSetId: 15' (length=18)
     ....php:105:string 'AttributeSetId: 15' (length=18)
     ....php:105:string 'AttributeSetId: 15' (length=18)
     ....php:105:string 'AttributeSetId: 15' (length=18)
     ....php:105:string 'AttributeSetId: 15' (length=18)
     ....php:105:string 'AttributeSetId: 15' (length=18)
     ....php:105:string 'AttributeSetId: 15' (length=18)
     ....php:105:string 'AttributeSetId: 15' (length=18)
     ....php:105:string 'AttributeSetId: 15' (length=18)

The query string after applying the filters:

 SELECT `main_table`.* FROM `eav_attribute_group` AS `main_table` WHERE (`attribute_set_id` = '15,10,4,14,11,12,13,9')

The problem, as I said in some other post (#2892 (comment)) is:

After some debugging, I found the problem:
When the AttributeGroupRepository processes the collection, the process goes through:

vendor/magento/framework/Api/SearchCriteria/CollectionProcessor/FilterProcessor.php
    addFilterGroupToCollection(...)

Here, on the line 66-70:

        $customFilter = $this->getCustomFilterForField($filter->getField());
        if ($customFilter) {
            $isApplied = $customFilter->apply($filter, $collection);
        }

There IS a custom field for my search (attribute_set_id), and the code goes in:

 vendor/magento/module-eav/Model/Api/SearchCriteria/CollectionProcessor/FilterProcessor/AttributeGroupAttributeSetIdFilter.php

Here there is:

public function apply(Filter $filter, AbstractDb $collection)
{
    /** @var \Magento\Eav\Model\ResourceModel\Entity\Attribute\Group\Collection $collection */
    $collection->setAttributeSetFilter($filter->getValue());

    return true;
}

Which goes on:

 vendor/magento/module-eav/Model/ResourceModel/Entity/Attribute/Group/Collection.php
public function setAttributeSetFilter($setId)
{
    $this->addFieldToFilter('attribute_set_id', ['eq' => $setId]);
    $this->setOrder('sort_order');
    return $this;
}

Here we can see that the eav_attribute_group collection does not even care about what we want us to do with the filter, it simply forces us on equal.

I don't want to believe that this was intentional, but instead, I believe that this was some old code that was done before the whole process-filter-thing, and it was forgotten.

The same goes for all filters from the eav_attribute_group:

      vendor/magento/module-eav/Model/Api/SearchCriteria/CollectionProcessor/FilterProcessor/AttributeGroupAttributeSetIdFilter.php
vendor/magento/module-eav/Model/Api/SearchCriteria/CollectionProcessor/FilterProcessor/AttributeGroupCodeFilter.php
vendor/magento/module-eav/Model/Api/SearchCriteria/CollectionProcessor/FilterProcessor/AttributeSetEntityTypeCodeFilter.php

All of them, don't bother to see what condition type we want to use on the collection.

For fixes, I think that the best solution is to let the processor do its job. That means:

vendor/magento/module-eav/etc/di.xml

Remove the lines 184-154:

<virtualType name="Magento\Eav\Model\Api\SearchCriteria\CollectionProcessor\AttributeSetFilterProcessor" type="Magento\Framework\Api\SearchCriteria\CollectionProcessor\FilterProcessor">
    <arguments>
        <argument name="customFilters" xsi:type="array">
            <item name="entity_type_code" xsi:type="object">Magento\Eav\Model\Api\SearchCriteria\CollectionProcessor\FilterProcessor\AttributeSetEntityTypeCodeFilter</item>
        </argument>
    </arguments>
</virtualType>

Remove the lines: 169-179

<virtualType name="Magento\Eav\Model\Api\SearchCriteria\CollectionProcessor\AttributeGroupFilterProcessor" type="Magento\Framework\Api\SearchCriteria\CollectionProcessor\FilterProcessor">
    <arguments>
        <argument name="fieldMapping" xsi:type="array">
            <item name="attribute_group_code" xsi:type="string">main_table.attribute_group_code</item>
        </argument>
        <argument name="customFilters" xsi:type="array">
            <item name="attribute_set_id" xsi:type="object">Magento\Eav\Model\Api\SearchCriteria\CollectionProcessor\FilterProcessor\AttributeGroupAttributeSetIdFilter</item>
            <item name="attribute_group_code" xsi:type="object">Magento\Eav\Model\Api\SearchCriteria\CollectionProcessor\FilterProcessor\AttributeGroupCodeFilter</item>
        </argument>
    </arguments>
</virtualType>

And of course, remove the not used (anymore) code...

Metadata

Metadata

Assignees

No one assigned

    Labels

    Component: EavIssue: Clear DescriptionGate 2 Passed. Manual verification of the issue description passedIssue: ConfirmedGate 3 Passed. Manual verification of the issue completed. Issue is confirmedIssue: Format is validGate 1 Passed. Automatic verification of issue format passedIssue: Ready for WorkGate 4. Acknowledged. Issue is added to backlog and ready for developmentPriority: P3May be fixed according to the position in the backlog.Progress: doneReproduced on 2.3.xThe issue has been reproduced on latest 2.3 releaseSeverity: S3Affects non-critical data or functionality and does not force users to employ a workaround.Triage: Dev.ExperienceIssue related to Developer Experience and needs help with Triage to Confirm or Reject itstale issue

    Type

    No type

    Projects

    Status

    Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions