Repository Filter Groups Applied Inconsistently

[astorm]

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.

[astorm]

Pinging because two weeks and no acknowledgment.

Is this issue not a priority for Magento 2 at this time? Consistent behavior of the filters in the Web API methods seems like it should be, but if it's not it would still be good to know so we can work around it.

If the issue isn't clear please let me know -- it's not the sort of thing that lends itself to easy, clear, and reproducible test steps, and as it's a core part of how Magento's API works it's not something that an outsider should really be making calls on.

/cc @piotrekkaminski

[Vinai]

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

[pboisvert]

@mbrinton01 can you pass this along for investigation?

[piotrekkaminski]

internal issue MAGETWO-53262 to address that

[slavvka]

Hi all. The fix for CMS Pages and Blocks Repositories has been merged to mainline.

[astorm]

@slavvka This bug was reporting that filter groups are applied inconsistently across the entire repository system in Magento -- i.e. it's my assumption that filters and groups would behave the same regardless of where they're at. You've closed this issue reporting that the CMS Pages and Blocks have been "fixed". However, all the other repositories remain.

Is Magento saying that it's intended behavior that repositories act differently?

Or was this ticket incorrectly closed?

[piotrekkaminski]

Reopening for investigation @choukalos