Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OrderGridCollectionFilter Plugin Breaks Mapping for created_at Column #36439

Closed
1 of 5 tasks
nwcasebolt opened this issue Nov 8, 2022 · 16 comments
Closed
1 of 5 tasks
Assignees
Labels
Area: Order Component: Admin Issue: needs update Additional information is require, waiting for response Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: done Reported on 2.4.5-p1 Indicates original Magento version for the Issue report. Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it

Comments

@nwcasebolt
Copy link

Preconditions and environment

  • Magento 2.4.5-p1

Steps to reproduce

  1. Add an afterSearch plugin on Magento\Framework\View\Element\UiComponent\DataProvider\Reporting that joins the sales_order table to the sales_order_grid table.

Example:

$salesOrderTable = $collection->getConnection()->getTableName('sales_order');
            $collection->getSelect()->joinLeft(
                ['sales' => $salesOrderTable],
                'sales.increment_id = main_table.increment_id',
                'your_field'
            );
  1. Filter the Order grid on purchase date.

Expected result

The orders are filtered by purchase date.

Actual result

  • The order grid does not filter on purchase date
  • An alert box says, "Attention: Something went wrong"
  • A message generates that says, "Something went wrong with processing the default view and we have restored the filter to its original state"
  • Error logs say, "Integrity constraint violation: 1052 Column 'created_at' in where clause is ambiguous."

Additional information

A bug was introduced in 2.4.5 when the OrderGridCollectionFilter plugin was added in December 2021 to convert the created_at field to UTC. The plugin's aroundAddFieldToFilter filter returns the field without calling $proceed. This means that created_at never gets processed through \Magento\Framework\Data\Collection\AbstractDb::addFieldToFilter, which in turns prevents created_at from getting mapped to main_table.

It's also worth pointing out that the new plugin duplicates code which operates on created_at in \Magento\Sales\Model\ResourceModel\Order\Grid\Collection::addFieldToFilter, leaving everything in that collection's if statement unreachable.

Release note

No response

Triage and priority

  • Severity: S0 - Affects critical data or functionality and leaves users without workaround.
  • Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
  • Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
  • Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
  • Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.
@m2-assistant
Copy link

m2-assistant bot commented Nov 8, 2022

Hi @nwcasebolt. Thank you for your report.
To speed up processing of this issue, make sure that you provided the following information:

  • Summary of the issue
  • Information on your environment
  • Steps to reproduce
  • Expected and actual results

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:

@magento give me 2.4-develop instance - upcoming 2.4.x release

For more details, review the Magento Contributor Assistant documentation.

Add a comment to assign the issue: @magento I am working on this

To learn more about issue processing workflow, refer to the Code Contributions.


⚠️ According to the Magento Contribution requirements, all issues must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of issues happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

@MyroslavDobra
Copy link

I also found it an hour ago. It's impossible that people that make such mistakes are working for a corporation like Adobe.

@engcom-November engcom-November added the Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it label Nov 15, 2022
@zaximus84
Copy link

Despite the name of the plugin referencing the order collection, the plugin is on a more generic class, so it can affect just about any UI admin grid that joins a table with "created_at". On my installation, I experienced this error on the customer grid (because we customized it to join the B2B company table) as well as a third party module table that was joining the quote table.

I fixed it by patching \Magento\Sales\Plugin\Model\ResourceModel\Order\OrderGridCollectionFilter and removing these lines:

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

return $subject;

That allows it to call through to $proceed and execute all of the normal code.

@mohitnavik
Copy link

mohitnavik commented Dec 16, 2022

Hi @nwcasebolt,

I was also getting the error related to 'created_at' column when trying to apply "purchase date" filter on order grid. I have created below patch to fix this issue. I was not unable to upload with '.patch' extension. So, the '.txt' extension can be changed to '.patch'. Then the patch can be directly applied to Magento setup.

Patch-Fixed-CreatedAt-Filter-Issue-At-OrderGrid.txt

Thanks,

@engcom-Bravo
Copy link
Contributor

Hi @nwcasebolt,

Thank you for reporting and collaboration.
Verified the issue on Magento 2.4-develop instance and the issue is reproducible. KIndly refer the screenshots.

Steps to reproduce

1.Add an afterSearch plugin on Magento\Framework\View\Element\UiComponent\DataProvider\Reporting that joins the sales_order table to the sales_order_grid table.
Example:

$salesOrderTable = $collection->getConnection()->getTableName('sales_order');
            $collection->getSelect()->joinLeft(
                ['sales' => $salesOrderTable],
                'sales.increment_id = main_table.increment_id',
                'your_field'
            );

2.Filter the Order grid on purchase date.

Screenshot 2023-01-16 at 2 48 26 PM

We are getting a message "Something went wrong with processing the default view and we have restored the filter to its original state"

Hence Confirming the issue.

Thanks.

@engcom-Bravo engcom-Bravo added Area: Order Component: Admin Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed labels Jan 16, 2023
@github-jira-sync-bot
Copy link

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

@m2-assistant
Copy link

m2-assistant bot commented Jan 16, 2023

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

@engcom-Hotel engcom-Hotel added Reported on 2.4.5-p1 Indicates original Magento version for the Issue report. Priority: P2 A defect with this priority could have functionality issues which are not to expectations. labels Jan 17, 2023
@atty31
Copy link

atty31 commented Feb 1, 2023

For me replacing:
$field='sales_order.'.$field; with $field ='main_table.'.$field;
from the patch, mentioned by @mohitnavik worked for me.

@shmVan
Copy link
Contributor

shmVan commented Mar 23, 2023

@magento I am working on this

@engcom-Bravo engcom-Bravo self-assigned this Jul 20, 2023
@m2-assistant
Copy link

m2-assistant bot commented Jul 20, 2023

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

    1. Add/Edit Area: XXXXX label to the ticket, indicating the functional areas it may be related to.
    1. Verify that the issue is reproducible on 2.4-develop branch
      Details- Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
      - If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
      - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
    1. If the issue is not relevant or is not reproducible any more, feel free to close it.

@engcom-Bravo engcom-Bravo removed the Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed label Jul 20, 2023
@m2-community-project m2-community-project bot added Progress: dev in progress and removed Progress: PR Created Indicates that Pull Request has been created to fix issue labels Jul 20, 2023
@engcom-Bravo engcom-Bravo added Issue: needs update Additional information is require, waiting for response and removed Progress: dev in progress labels Jul 20, 2023
@engcom-Bravo engcom-Bravo removed the Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch label Jul 20, 2023
@github-jira-sync-bot github-jira-sync-bot added Progress: PR Created Indicates that Pull Request has been created to fix issue and removed Progress: dev in progress Progress: ready for dev labels Jul 20, 2023
@engcom-Bravo
Copy link
Contributor

Hi @nwcasebolt,

We have noticed that this issue has not been updated since long time.
Hence we assume that this issue is fixed now, so we are closing it. Please feel to raise a fresh ticket or reopen this ticket if you need more assistance on this.

Thanks.

@m2-community-project m2-community-project bot added Progress: done and removed Progress: PR Created Indicates that Pull Request has been created to fix issue labels Aug 1, 2023
@sanderjongsma
Copy link
Contributor

I also ran into this problem. Because my custom grid was joining some other tables with created_at. I resolved it by creating a before plugin for: \Magento\Sales\Plugin\Model\ResourceModel\Order\OrderGridCollectionFilter::aroundAddFieldToFilter:

public function beforeAroundAddFieldToFilter(
        OrderGridCollectionFilter $subject,
        SearchResult $searchResult,
        Closure $proceed,
        $field,
        $condition = null
    ): array {
        if (! $searchResult instanceof Collection) {
            return [$searchResult, $proceed, $field, $condition];
        }

        if ($field === 'created_at') {
            $field = 'main_table.created_at';
        }

        return [$searchResult, $proceed, $field, $condition];
    }

In this way the original plugin won't be affected. And in this before plugin you can check if the searchResult is an instance of the desired collection. And to eventually not hit the original around plugin, just add the table to the created_at field like: main_table.created_at or if you want the joined table created_at

@mtytula
Copy link

mtytula commented Jan 11, 2024

index 995bb83..3765a1d 100644
--- a/Plugin/Model/ResourceModel/Order/OrderGridCollectionFilter.php
+++ b/Plugin/Model/ResourceModel/Order/OrderGridCollectionFilter.php
@@ -52,7 +52,7 @@ class OrderGridCollectionFilter
                 }
             }

-            $fieldName = $subject->getConnection()->quoteIdentifier($field);
+            $fieldName = $subject->getConnection()->quoteIdentifier($field === 'created_at' ? 'main_table.created_at' : $field);
             $condition = $subject->getConnection()->prepareSqlCondition($fieldName, $condition);
             $subject->getSelect()->where($condition, null, Select::TYPE_CONDITION);

Here is a quick patch for getting it to work with any custom made that joins another tables that contains created_at column. Works on 2.4.5-p4.

@LucScu
Copy link

LucScu commented Mar 15, 2024

Hi @nwcasebolt,

Verified the issue on Magento 2.4-develop instance and the issue is not reproducible.Kindly refer the screenshots.

Steps to reproduce:

  1. Added an afterSearch plugin on Magento\Framework\View\Element\UiComponent\DataProvider\Reporting that joins the sales_order table to the sales_order_grid table.

Screenshot 1945-04-29 at 12 17 40 PM

  1. Filtered the Order grid on purchase date.

Screenshot 1945-04-29 at 12 20 41 PM

Screenshot 1945-04-29 at 12 20 53 PM

So here we are not getting error messages like something went wrong.We are able apply the filter properly.

Hence issue is not able to reproduce.It seems like issue is already fixed.

Thanks.

The issue is still present in 2.4.6-p3, your test are wrong.
Please reopen.

@atIOCrON
Copy link

For anyone else looking for a fix, please see here:

#38916

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Order Component: Admin Issue: needs update Additional information is require, waiting for response Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: done Reported on 2.4.5-p1 Indicates original Magento version for the Issue report. Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it
Projects
Development

No branches or pull requests