search

magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.44k stars 9.29k forks source link

OR Condition in searchCriteria (UI Component DataProvider) doesn't work #18329

Closed martin-cod closed 1 year ago

martin-cod commented 5 years ago

Preconditions

  1. Magento Commerce 2.2.5
  2. PHP version 7.1.20

Steps to reproduce

  1. I want to extend Order Status filter and allow to search both "Pending" and "Pending Payment" statuses when I select "Pending" in the filter. I extend filters for Status column of Admin Order's grid UI component so that the searchCriteria has two filters inside one filterGroup 
    "search_criteria": {
"filter_groups": [
    {
        "filters": [
            {
                "field": "status",
                "value": "pending",
                "condition_type": "eq"
            },
            {
                "field": "status",
                "value": "pending_payment",
                "condition_type": "eq"
            }
        ]
    }
]
}

    It can be extended vi URL params or using plugin for \Magento\Framework\View\Element\UiComponent\DataProvider\DataProvider::getSearchCriteria() method

  2. Apply Filter by Status field

Expected result

  1. Orders with both Pending and Pending Payment statuses are shown

Actual result

  1. No orders is selected

The reason is in the wrong processing of searchCriteria in \Magento\Framework\View\Element\UiComponent\DataProvider\FilterPool::applyFilters() method. The searchCriteria above should be transformed to the following SQL statement: WHERE (status="pending" OR status="pending_payment") but it is transformed to the following: WHERE status="pending" AND status="pending_payment"

The correct processing of searchCriteria is provided here \Magento\Framework\Api\SearchCriteria\CollectionProcessor\FilterProcessor::process()

magento-engcom-team commented 5 years ago

Hi @martin-cod. Thank you for your report. To help us process this issue please make sure that you provided the following information:

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento-engcom-team give me $VERSION instance

where $VERSION is version tags (starting from 2.2.0+) or develop branches (for example: 2.3-develop). For more details, please, review the Magento Contributor Assistant documentation.

@martin-cod do you confirm that you was able to reproduce the issue on vanilla Magento instance following steps to reproduce?

yaroslav-zenin commented 5 years ago

I can confirm that behavior. Reproduced also on 2.3-develop That's happen because 

class Magento\Framework\View\Element\UiComponent\DataProvider\DataProvider implements DataProviderInterface 
{
    // only one way to add any filters
    public function addFilter(\Magento\Framework\Api\Filter $filter)
    {
        $this->searchCriteriaBuilder->addFilter($filter);
    }

    // unable to use searchcriteria because next filters will not be applied
    public function getSearchCriteria()
    {
        if (!$this->searchCriteria) {
            $this->searchCriteria = $this->searchCriteriaBuilder->create();
            $this->searchCriteria->setRequestName($this->name);
        }
        return $this->searchCriteria;
    }
}

class Magento\Framework\Api\Search\SearchCriteriaBuilder extends AbstractSimpleObjectBuilder
{
    // when creating criteria all filters goes in different groups, so only AND conditions
    public function create()
    {
        foreach ($this->filters as $filter) {
            $this->data[SearchCriteria::FILTER_GROUPS][] = $this->filterGroupBuilder->setFilters([])
                ->addFilter($filter)
                ->create();
        }
        $this->data[SearchCriteria::SORT_ORDERS] = [$this->sortOrderBuilder->create()];
        return parent::create();
    }
}

Any suggestions how to make a nice fix here?

magento-engcom-team commented 5 years ago

Hi @engcom-backlog-nazar. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

magento-engcom-team commented 5 years ago

@engcom-backlog-nazar Thank you for verifying the issue. Based on the provided information internal tickets MAGETWO-96171, MAGETWO-96172 were created

irajneeshgupta commented 5 years ago

Hi @martin-cod @yaroslav-zenin @engcom-backlog-nazar, How you are able to reproduce the issue? The Magento UI grid uses "condition_type": "in" incase of multi-select, and it will solve the purpose. I didn't find any Filter in UI grid which create 2 eq condition_type on same field.

"search_criteria": {
    "filter_groups": [
        {
            "filters": [
                {
                    "field": "status",
                    "value": [ "pending_payment", "pending" ],
                    "condition_type": "in"
                }
            ]
        }
    ]
}
martin-cod commented 5 years ago

Hi @irajneeshgupta

I didn't try "condition_type": "in" but the SearchCriteria filter logic (described in the ticket details) is the different then it is described in Dev docs https://devdocs.magento.com/guides/v2.2/extension-dev-guide/searching-with-repositories.html https://devdocs.magento.com/guides/v2.2/rest/performing-searches.html

I assume, that the SearchCriteria filtering logic how it is applied in \Magento\Framework\View\Element\UiComponent\DataProvider\FilterPool should be fixed to be consistent with DevDocs OR the exceptional filtering logic in UI DataProvider should be documented as well

hugodeaguiar commented 4 years ago

Hello!

I changed the following sections to work OR filter:

In magento/framework/View/Element/UiComponent/DataProvider/FilterPool.php line 37

    /**
     * @param Collection $collection
     * @param SearchCriteriaInterface $criteria
     * @return void
     */
    public function applyFilters(Collection $collection, SearchCriteriaInterface $criteria)
    {
        foreach ($criteria->getFilterGroups() as $filterGroup) {
            $filters = $filterGroup->getFilters();
            $groupedFilters = [];
            $isGrouped = count($filters) > 1 ? true : false;

            foreach ($filterGroup->getFilters() as $filter) {
                $filterApplier = false;

                /** @var $filterApplier FilterApplierInterface*/
                if (isset($this->appliers[$filter->getConditionType()])) {
                    $filterApplier = $this->appliers[$filter->getConditionType()];
                } else {
                    if (!$isGrouped) {
                        $filterApplier = $this->appliers['regular'];
                    } else {
                        $groupedFilters[] = $filter;
                    }
                }
                if ($filterApplier) {
                    $filterApplier->apply($collection, $filter);
                }
            }

            if (count($groupedFilters) > 0) {
                $filterApplier = $this->appliers['regular'];
                $filterApplier->applyFilterGroup($collection, $groupedFilters);
                $groupedFilters = [];
            }
        }
    }

In magento/framework/View/Element/UiComponent/DataProvider/RegularFilter.php i create a function applyFilterGroup

    /**
     * Apply regular filter groups like collection filters
     *
     * @param Collection $collection
     * @param Array $filters
     * @return void
     */
    public function applyFilterGroup(Collection $collection, Array $filters)
    {
        $filterGroupAttrs = [];
        $filterGroupCond  = [];
        foreach ($filters as $filter) {
            $filterGroupAttrs[] = $filter->getField();
            $filterGroupCond[] = [$filter->getConditionType() => $filter->getValue()];
        }

        $collection->addFieldToFilter($filterGroupAttrs, $filterGroupCond);
    }

After this change the filters are working fine.

Regards, Hugo Aguiar.

espressobytes commented 4 years ago

@hugodeaguiar Thanks very much. I created a patch-file for this issue, which is a bug in my eyes.

framework_ui_component_data_provider_fix.patch.zip

To apply this patch via composer:

{
   ...
   "require": {
        "magento/product-community-edition": "2.3.3-p1",
        ... 
   },
   ...
   "extra": {
        ...
        "patches": {
            "magento/framework": {
                "Bugfix for OR-filtering in adminhtml": "patches/composer/framework_ui_component_data_provider_fix.patch"
            },
            ...
        }
    }
}
m2-assistant[bot] commented 4 years ago

Hi @engcom-Charlie. Thank you for working on this issue. Looks like this issue is already verified and confirmed. But if you want to validate it one more time, please, go though the following instruction:

gwharton commented 3 years ago

I can confirm, still broken in 2.4.0. The above patch is still a good workaround, albeit needing slight alteration to apply to 2.4 codebase.

Related to https://github.com/magento/magento2/issues/24576

gwharton commented 3 years ago

Well, someone else will need to confirm this, but I have just upgraded to 2.4.2, and my code that needed the above patches to implement an "OR" search criteria filter now works without the patch.

Release notes for 2.4.2 state "Customized filter groups are now applied correctly when searching on customers. Previously, the afterSearch function did not OR filters as expected. #24576"

So, i'm gonna say this is now fixed.

JamesFX2 commented 2 years ago

I'm adding to this issue because it's basically duplication of effort to have something excellent like CollectionProcessor / FilterProcessor

And instead use FilterPool

CollectionProcessor just works as per the documentation. FilterPool seemingly doesn't. Backwards compatibility or something?

engcom-Hotel commented 1 year ago

Hello @martin-cod,

As per the comment below in https://github.com/magento/magento2/issues/18329#issuecomment-776291249, it seems the issue has been resolved. Hence we are closing this issue.

Well, someone else will need to confirm this, but I have just upgraded to 2.4.2, and my code that needed the above patches to implement an "OR" search criteria filter now works without the patch.

Release notes for 2.4.2 state "Customized filter groups are now applied correctly when searching on customers. Previously, the afterSearch function did not OR filters as expected. #24576"

So, i'm gonna say this is now fixed.

Please let us know if you are still facing this issue.

Thanks