magento2: Repository Filter Groups Applied Inconsistently

Per conventional wisdom that’s floating around the Magento programmer community, it’s my understanding that, when creating object repositories and implementing a getList method that

  1. Filters within a group should be applied as OR filters
  2. Filter groups themselves should be applied as AND filter

However, this informal guideline is not followed consistently in the core Magento systems code. For example, the CMS page repository does not apply this AND/OR behavior, and applies each filter as an AND

#File: vendor/magento/module-cms/Model/PageRepository.php
foreach ($criteria->getFilterGroups() as $filterGroup) {
    foreach ($filterGroup->getFilters() as $filter) {
        if ($filter->getField() === 'store_id') {
            $collection->addStoreFilter($filter->getValue(), false);
            continue;
        }
        $condition = $filter->getConditionType() ?: 'eq';
        $collection->addFieldToFilter($filter->getField(), [$condition => $filter->getValue()]);
    }
}

Whereas the coupon repository does follow this guideline.

#File: vendor/magento/module-sales-rule/Model/CouponRepository.php

protected function addFilterGroupToCollection(
    \Magento\Framework\Api\Search\FilterGroup $filterGroup,
    Collection $collection
) {
    $fields = [];
    $conditions = [];
    foreach ($filterGroup->getFilters() as $filter) {
        $condition = $filter->getConditionType() ? $filter->getConditionType() : 'eq';
        $fields[] = $filter->getField();
        $conditions[] = [$condition => $filter->getValue()];
    }
    if ($fields) {
        $collection->addFieldToFilter($fields, $conditions);
    }
}

This makes it unclear how filters and filter groups should be applied.

Expected Behavior: Ideally, all repositories with a getList method that accepts search criteria should behave the same, and apply a “filters are OR”, “filter groups are AND” logic. If you really wanted to knock this out of the park, a helper or trait that provided and ventral consistent place for filter group adding logic would be ideal. This would remove the burden from the repository creator as to how filters and filter groups should be applied.

If neither is possible, then the behavior of individual repositories should b documented.

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Reactions: 5
  • Comments: 15 (9 by maintainers)

Commits related to this issue

Most upvoted comments

Consist filtering behavior over the API (Web API and PHP API) is very important.

all the repos which use SearchCriteria works consistently

But I want to repeat that the only repos that worked inconsistently were fixed. Since that the problem doesn’t exist anymore.

@slavvka @piotrekkaminski I’m not sure if I understand the above correctly – but those don’t appear to be true statements. While the CMS Page Repository in the develop branch does appear fixed

https://github.com/magento/magento2/blob/develop/app/code/Magento/Cms/Model/PageRepository.php#L228
private function addFilterGroupToCollection(
    \Magento\Framework\Api\Search\FilterGroup $filterGroup,
    \Magento\Cms\Model\ResourceModel\Page\Collection $collection
) {
    $fields = [];
    $conditions = [];
    foreach ($filterGroup->getFilters() as $filter) {
        if ($filter->getField() === 'store_id') {
            $collection->addStoreFilter($filter->getValue(), false);
            continue;
        }
        $condition = $filter->getConditionType() ?: 'eq';
        $fields[] = $filter->getField();
        $conditions[] = [$condition => $filter->getValue()];
    }
    if ($fields) {
        $collection->addFieldToFilter($fields, $conditions);
    }
}

This is far from “all the repos which use SearchCriteria”. For example, if I look at the Payment Token Repository

protected function addFilterGroupToCollection(FilterGroup $filterGroup, Collection $collection)
{
    foreach ($filterGroup->getFilters() as $filter) {
        $condition = $filter->getConditionType() ? $filter->getConditionType() : 'eq';
        $collection->addFieldToFilter($filter->getField(), [$condition => $filter->getValue()]);
    }
}

This sure looks like the behavior where every filter in a single group is applied as an AND query. I’d bet even money there are other repositories like this as well.

Am I missing something?