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.5k stars 9.3k forks source link

getSize() of product collection returns incorrect value after applying addAttributeToFilter() #7730

Closed heyqule closed 6 years ago

heyqule commented 7 years ago

The pagination became incorrect. For example, The pagination would show 10 page when there are only 3 pages of product on product listing page.

Preconditions

Magento version 2.1.2

Steps to reproduce

  1. Create a frontend plugin for product collection's load function, beforeLoad()
  2. Add a new addAttributeToFitler() to $subject
  3. get the value of getSize()

Expected result

  1. the getSize() should reflect the total in the collection with new attribute filter.

Actual result

  1. the _totalRecords is not correct for filtered collection. getSize() returns incorrect value.

Investigation: The total records was pulled from a different model. When I added new attribute filter to the collection, it did not update the search criteria in catalogsearch models. Hence, the total records did not represent the total in the collection.

Magento\CatalogSearch\Model\ResourceModel\Fulltext\Collection::_renderFiltersBefore() Line 347: $this->_totalRecords = $this->searchResult->getTotalCount();

heyqule commented 7 years ago

My current solution was to remove the above line with a function override in new extension. But I'm not sure the side effect of this change.

daniel-ifrim commented 7 years ago

@heyqule This issue looks related to this one https://github.com/magento/magento2/issues/9807

cmuench commented 7 years ago

We have the same issue in one of our projects. I can confirm that the "getSize" method contains no own filter. So the "getCount" method returns a different value than the "getSize" method.

magento-engcom-team commented 6 years ago

@heyqule, thank you for your report. The issue is already fixed in develop branch, 2.2.0

drew7721 commented 6 years ago

The fix above was never merged. Has this been fixed in any version?

Hexmage commented 6 years ago

@magento-engcom-team I agree with @drew7721 this is still an issue in 2.2.2

seyuf commented 5 years ago

@magento-engcom-team this is a major issue. And seems to had not been fixed (in any version to date). Please at least reopen corresponding the issue(s)?... Thanks

onepack commented 5 years ago

I'm trying to figure out how to get around this issue. This is closed by Magento as they see this as normal behavior. The proposed solution never made it to production: https://github.com/magento/magento2/pull/10246/commits/676930547154a428fc2b5ce2565b1d6e8edb524f Also doesn't it fix a problem that is currently present with and without the fix but with custom filters: Example: The page will show 1-7 of 7 products with a page setting of 12 max per page. However I know there are 8 products that should be visible. When I set the max per page to 16 per page the page will show the missing product and the page calculation will become products 1-8 of 8.

What to do? Add getSize() myself in my custom layer.php plugin?

Hexmage commented 5 years ago

@onepack I think the reason they haven't merged the commit is because the actual problem is deeper. The original unfiltered collection returns 12 products. Then proceeds to filter those 12 products which results in the 7 products you see. The 8th product apparently is in the next 12 products.

So the actual problem is that the collection is filtered too late.

onepack commented 5 years ago

@Hexmage Thanks for the reply! For now I will stick with the proposed solution combined with a high items per grid setting. This way I can get around 80% of the issue that some products are not shown as they would be in the second page collection. I will spend more time on this when time allows it.

seyuf commented 5 years ago

Hi, yep @Hexmage the problem is surely deeper. The thing is i had the same issue somewhere else ie here. Someone found a fix that better and had nothing to do with mine :). @onepack may be you can take a look? My solution at the time was to reset the total count if i remember correctly :).

Hope this helps.

@shikhamis11 maybe you'll be interested in this (even though its marked as closed ;))? I believe it's similar to the issue mentioned above that you solved.

m2-assistant[bot] commented 5 years ago

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

onepack commented 5 years ago

@seyuf, Thank you for your reply. Resetting the count seems like a good approach. Will have a test with that now.

seyuf commented 5 years ago

@onepack you can also take a look at this approach https://github.com/magento/magento2/pull/22291/commits/b881a5d98db719eae436d80965ea41365b53295e

onepack commented 5 years ago

I tried a combination of both fixes: 

            $itemCount = count($this->_items);
            if ($this->_totalRecords === null || ($itemCount > 0 && $itemCount !== $this->_totalRecords)) {   
            $sql = $this->getSelectCountSql();  
            $sql->reset(\Zend\Db\Sql\Select::GROUP);  
            $this->_totalRecords = (int) $this->getConnection()->fetchOne($sql, $this->_bindParams);

But the result stays like it was. Products are missing that are supposed to be on the next page if the filter was not in place. Any suggestion on the code?

seyuf commented 5 years ago

Humm just curious can you replace fetchOne by fetchAll? And tell me what it gives you?

onepack commented 5 years ago

It gives me the total count of one in the toolbar and shows 11 items that should be 12. Setting the page limit to 32 products shows me 12 products but the count is still one. I reverted that last change so it is fetchOne again.

seyuf commented 5 years ago

Alright thanks, hmm so it's not what i was thinking then if the total stays at one. I will take a look at this when i have a bit more time. But hopefully, @shikhamis11 will have found a fix by then :)

seyuf commented 5 years ago

@onepack what's your magento version?

onepack commented 5 years ago

M2 version 2.3.2 in development mode.

seyuf commented 5 years ago

^^ what is this supposed to mean? https://github.com/magento/magento2/blob/2.3-develop/app/code/Magento/CatalogSearch/Model/ResourceModel/Fulltext/Collection/TotalRecordsResolver.php

onepack commented 5 years ago

This is the first time I see that. Will check tomorrow morning.

onepack commented 5 years ago

@magento-engcom-team This serious issue is still present until today. When I set pagecount to 8 I get 5 products but when I set pagecount to 20 I get all 12 products. In real live shops this will affect sales as products are not visible for customers that should be visible.

Why is there no activity at all to to solve this major bug?

onepack commented 5 years ago

I filter the collection now on layer.php. It seems too late for the correct count. Via the search didn't help either. Is it possible to change the moment the counting is done. Anyone?

bytes-commerce commented 3 years ago

Experiencing a similar issue in Magento 2.4.3. I have a collection with a group filter, and if I debug the query it adds a AND (NULL) to the query, so ->getSize() remains 0.

hassanas commented 2 years ago

experienceing same issue in 2.4 as well why issue closed ?

heyqule commented 2 years ago

rofl. Isn't this issue fixed in 2.2 and 2.3? Did they break it again in 2.4?

Adobe probably gonna fix it in Magento 3.

simonrl commented 2 years ago

@heyqule I noticed that sometimes Flat-Index was enabled, and disabling it solved the problem. It's not recommended to enable it anyways.