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.46k stars 9.28k forks source link

addFilterToMap in Collection class not working #39000

Open dchaykas opened 1 month ago

dchaykas commented 1 month ago

Preconditions and environment

This commit introduces a "aroundAddFieldToFilter" plugin which diverts the logic from using created mappings using "addFilterToMap" function for all columns in custom joins labeled as "created_at".

Steps to reproduce

  1. Create a report which uses UI component Magento\Framework\View\Element\UiComponent\DataProvider\SearchResult , setting main table as sales_creditmemo_item
  2. In the same Collection class, join the creditmemo table which has the created_at column.
  3. In the same Collection class, apply mapping for the created_at column, like this $this->addFilterToMap('created_at', 'creditmemo.created_at');
  4. Load the report, try to filter by created_at column.
  5. The error received is SQLSTATE[23000]: Integrity constraint violation: 1052 Column 'created_at' in where clause is ambiguous

Expected result

The addFilterToMap function should add the appropriate mapping alias to the joined table. The system should recognize the mapping and render the report with filters correctly. In the current bug the WHERE clause should have the creditmemo.created_at column as a filter instead of created_at.

Actual result

Error shown:

SQLSTATE[23000]: Integrity constraint violation: 1052 Column 'created_at' in where clause is ambiguous, query was: SELECT `main_table`.*, `creditmemo`.`created_at`, `order_item`.`name` AS `product_name`, `order_item`.`sku`, `order`.`increment_id` AS `order_increment_id`, `order`.`status` AS `order_status`, `order_payment`.`method` AS `payment_method`, `order_payment`.`cc_type` AS `payment_type`, ... WHERE (((`created_at` >= '2024-08-01 12:00:00'))) GROUP BY `main_table`.`order_item_id` LIMIT 20
Exception in /var/www/html/magento/vendor/magento/framework/DB/Statement/Pdo/Mysql.php:109

Due to the missed mapping logic the code assumes the created_at potentially belongs to the main table (which creditmemo item table doesn't have) and throws an error if more tables joined having the same column name as created_at.

The plugin below intercepts mapper logic when fields name is created_at: vendor/magento/module-sales/etc/adminhtml/di.xml:52

<type name="Magento\Framework\View\Element\UiComponent\DataProvider\SearchResult">
        <plugin name="orderGridCollectionFilterPlugin" type="Magento\Sales\Plugin\Model\ResourceModel\Order\OrderGridCollectionFilter"/>
    </type>

The IF condition in the plugin does not use mapper logic to determine if a field has been mapped to something else:

    public function aroundAddFieldToFilter(
        SearchResult $subject,
        \Closure $proceed,
        $field,
        $condition = null
    ) {

        if ($field === 'created_at' || $field === 'order_created_at') {
            if (is_array($condition)) {
                foreach ($condition as $key => $value) {
                    $condition[$key] = $this->timeZone->convertConfigTimeToUtc($value);
                }
            }

            $fieldName = $subject->getConnection()->quoteIdentifier($field);
            $condition = $subject->getConnection()->prepareSqlCondition($fieldName, $condition);
            $subject->getSelect()->where($condition, null, Select::TYPE_CONDITION);

            return $subject;
        }

        return $proceed($field, $condition);
    }

For all other field names the plugin is skipped and go to the $proceed(...) call which continues to evaluate mapper fields code.

Additional information

public function _construct()
    {
        $this->_init($this->document, 'Magento\Sales\Model\ResourceModel\Order\Creditmemo\Item');
        $this->setMainTable('sales_creditmemo_item');
    }

    protected function _initSelect()
    {
        parent::_initSelect();

        $this->addCreditmemoToCollection();

        $this->addFilterToMap('created_at', 'creditmemo.created_at');

        return $this;
    }
    protected function addCreditmemoToCollection()
    {
        $this->getSelect()
            ->joinInner(
                ['creditmemo' => $this->getTable('sales_creditmemo')],
                'creditmemo.entity_id = main_table.parent_id',
                ['created_at' => 'creditmemo.created_at']
            );
    }

Release note

No response

Triage and priority

m2-assistant[bot] commented 1 month ago

Hi @dchaykas. Thank you for your report. To speed up processing of this issue, make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, Add a comment to the issue:

m2-assistant[bot] commented 1 month ago

Hi @engcom-Hotel. 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:

engcom-Hotel commented 1 month ago

Hello @dchaykas,

Thanks for the report and collaboration!

We have tried to reproduce the issue in the latest development branch i.e. 2.4-develop but it seems the issue is not reproducible for us.

Please find below the module for reference and let us know if we missed anything:

https://github.com/engcom-Hotel/magento2/tree/issue39000/app/code/Magz/Issue39000

Thanks

dchaykas commented 1 month ago

Thank you for the quick tetst @engcom-Hotel , please update the report logic to use UIComponent:

<type name="Magento\Framework\View\Element\UiComponent\DataProvider\SearchResult">
        <plugin name="orderGridCollectionFilterPlugin" type="Magento\Sales\Plugin\Model\ResourceModel\Order\OrderGridCollectionFilter"/>
    </type>

The above code is the snippet where order plugin will catch all fields named created_at preventing code from continue to mapper logic.

    public function aroundAddFieldToFilter(
        SearchResult $subject,
        \Closure $proceed,
        $field,
        $condition = null
    ) {

        if ($field === 'created_at' || $field === 'order_created_at') {
            if (is_array($condition)) {
                foreach ($condition as $key => $value) {
                    $condition[$key] = $this->timeZone->convertConfigTimeToUtc($value);
                }
            }

            $fieldName = $subject->getConnection()->quoteIdentifier($field);
            $condition = $subject->getConnection()->prepareSqlCondition($fieldName, $condition);
            $subject->getSelect()->where($condition, null, Select::TYPE_CONDITION);

            return $subject;
        }

        return $proceed($field, $condition);
    }

I'll update the description, my bad for missing this piece.

engcom-Hotel commented 1 month ago

Hello @dchaykas,

Thanks for the updates!

The issue is now reproducible after making the changes with the latest updates. Hence confirming the issue.

Thanks

github-jira-sync-bot commented 1 month ago

:white_check_mark: Jira issue https://jira.corp.adobe.com/browse/AC-12771 is successfully created for this GitHub issue.

m2-assistant[bot] commented 1 month ago

:white_check_mark: Confirmed by @engcom-Hotel. Thank you for verifying the issue.
Issue Available: @engcom-Hotel, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

dchaykas commented 3 days ago

Related issues: