{"action":"reopened","issue":{"url":"https://api.github.com/repos/magento/magento2/issues/4287","repository_url":"https://api.github.com/repos/magento/magento2","labels_url":"https://api.github.com/repos/magento/magento2/issues/4287/labels{/name}","comments_url":"https://api.github.com/repos/magento/magento2/issues/4287/comments","events_url":"https://api.github.com/repos/magento/magento2/issues/4287/events","html_url":"https://github.com/magento/magento2/issues/4287","id":150705650,"number":4287,"title":"Repository Filter Groups Applied Inconsistently","user":{"login":"astorm","id":184372,"avatar_url":"https://avatars.githubusercontent.com/u/184372?v=3","gravatar_id":"","url":"https://api.github.com/users/astorm","html_url":"https://github.com/astorm","followers_url":"https://api.github.com/users/astorm/followers","following_url":"https://api.github.com/users/astorm/following{/other_user}","gists_url":"https://api.github.com/users/astorm/gists{/gist_id}","starred_url":"https://api.github.com/users/astorm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/astorm/subscriptions","organizations_url":"https://api.github.com/users/astorm/orgs","repos_url":"https://api.github.com/users/astorm/repos","events_url":"https://api.github.com/users/astorm/events{/privacy}","received_events_url":"https://api.github.com/users/astorm/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/magento/magento2/labels/bug","name":"bug","color":"d93f0b"}],"state":"open","locked":false,"assignee":{"login":"piotrekkaminski","id":1714545,"avatar_url":"https://avatars.githubusercontent.com/u/1714545?v=3","gravatar_id":"","url":"https://api.github.com/users/piotrekkaminski","html_url":"https://github.com/piotrekkaminski","followers_url":"https://api.github.com/users/piotrekkaminski/followers","following_url":"https://api.github.com/users/piotrekkaminski/following{/other_user}","gists_url":"https://api.github.com/users/piotrekkaminski/gists{/gist_id}","starred_url":"https://api.github.com/users/piotrekkaminski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/piotrekkaminski/subscriptions","organizations_url":"https://api.github.com/users/piotrekkaminski/orgs","repos_url":"https://api.github.com/users/piotrekkaminski/repos","events_url":"https://api.github.com/users/piotrekkaminski/events{/privacy}","received_events_url":"https://api.github.com/users/piotrekkaminski/received_events","type":"User","site_admin":false},"assignees":[{"login":"piotrekkaminski","id":1714545,"avatar_url":"https://avatars.githubusercontent.com/u/1714545?v=3","gravatar_id":"","url":"https://api.github.com/users/piotrekkaminski","html_url":"https://github.com/piotrekkaminski","followers_url":"https://api.github.com/users/piotrekkaminski/followers","following_url":"https://api.github.com/users/piotrekkaminski/following{/other_user}","gists_url":"https://api.github.com/users/piotrekkaminski/gists{/gist_id}","starred_url":"https://api.github.com/users/piotrekkaminski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/piotrekkaminski/subscriptions","organizations_url":"https://api.github.com/users/piotrekkaminski/orgs","repos_url":"https://api.github.com/users/piotrekkaminski/repos","events_url":"https://api.github.com/users/piotrekkaminski/events{/privacy}","received_events_url":"https://api.github.com/users/piotrekkaminski/received_events","type":"User","site_admin":false},{"login":"mbrinton01","id":10635536,"avatar_url":"https://avatars.githubusercontent.com/u/10635536?v=3","gravatar_id":"","url":"https://api.github.com/users/mbrinton01","html_url":"https://github.com/mbrinton01","followers_url":"https://api.github.com/users/mbrinton01/followers","following_url":"https://api.github.com/users/mbrinton01/following{/other_user}","gists_url":"https://api.github.com/users/mbrinton01/gists{/gist_id}","starred_url":"https://api.github.com/users/mbrinton01/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mbrinton01/subscriptions","organizations_url":"https://api.github.com/users/mbrinton01/orgs","repos_url":"https://api.github.com/users/mbrinton01/repos","events_url":"https://api.github.com/users/mbrinton01/events{/privacy}","received_events_url":"https://api.github.com/users/mbrinton01/received_events","type":"User","site_admin":false}],"milestone":null,"comments":10,"created_at":"2016-04-24T22:16:05Z","updated_at":"2016-08-05T20:40:10Z","closed_at":null,"body":"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 \r\n\r\n1. Filters within a group should be applied as `OR` filters\r\n2. Filter groups themselves should be applied as `AND` filter\r\n\r\nHowever, 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`\r\n\r\n    #File: vendor/magento/module-cms/Model/PageRepository.php\r\n    foreach ($criteria->getFilterGroups() as $filterGroup) {\r\n        foreach ($filterGroup->getFilters() as $filter) {\r\n            if ($filter->getField() === 'store_id') {\r\n                $collection->addStoreFilter($filter->getValue(), false);\r\n                continue;\r\n            }\r\n            $condition = $filter->getConditionType() ?: 'eq';\r\n            $collection->addFieldToFilter($filter->getField(), [$condition => $filter->getValue()]);\r\n        }\r\n    }\r\n\r\nWhereas the coupon repository does follow this guideline.\r\n\r\n    #File: vendor/magento/module-sales-rule/Model/CouponRepository.php\r\n\r\n    protected function addFilterGroupToCollection(\r\n        \\Magento\\Framework\\Api\\Search\\FilterGroup $filterGroup,\r\n        Collection $collection\r\n    ) {\r\n        $fields = [];\r\n        $conditions = [];\r\n        foreach ($filterGroup->getFilters() as $filter) {\r\n            $condition = $filter->getConditionType() ? $filter->getConditionType() : 'eq';\r\n            $fields[] = $filter->getField();\r\n            $conditions[] = [$condition => $filter->getValue()];\r\n        }\r\n        if ($fields) {\r\n            $collection->addFieldToFilter($fields, $conditions);\r\n        }\r\n    }\r\n\r\nThis makes it unclear how filters and filter groups should be applied.\r\n\r\n**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.\r\n\r\nIf neither is possible, then the behavior of individual repositories should b documented."}}
{"actor":{"display_login":"piotrekkaminski"}}