Skip to content

Fixing issue with getSize function not recalculating after adding filters. #10246

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

Conversation

KevinDonaldson
Copy link

@KevinDonaldson KevinDonaldson commented Jul 14, 2017

Should fix several issues with getSize() returning incorrect values after filters have been applied to a collection.

Description

  • Adds additional guard to check if items on collection is equal to the totalRecords on collection.

Fixed Issues

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

Manual testing scenarios

  1. Load a collection.
  2. Apply a filter to the collection.
  3. Get size of collection using getSize() function and validate that value is correct.

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Jul 14, 2017

CLA assistant check
All committers have signed the CLA.

@okorshenko
Copy link
Contributor

Hi @KevinDonaldson Could you please cover your fix with the integration test? This is a very low-level class that must be covered with the tests. Thank you

@okorshenko okorshenko self-assigned this Jul 14, 2017
@okorshenko okorshenko added this to the July 2017 milestone Jul 14, 2017
@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Jul 15, 2017

@KevinDonaldson @okobchenko it's incorrect behavior because getSize method will return total records in DB, while in $this->_items we will have amount of products that was limited. Please do not merge these changes.
I think in order to fix this issue we need to have some "hash" of current filters and if it's different (added new filters) - total records should be added , but if we have any results in $this->_items - we don't need to calculate new total records value again and use the last one.

Also there are might be negative effect because getSize result might be already used in UI, as example in pagination, so as for me - you need to find another way to add your filters instead of using before load plugin

@okorshenko
Copy link
Contributor

Hi @KevinDonaldson Thank you for your contribution. Please consider the previous comment
Closing this PR.

@KevinDonaldson
Copy link
Author

Thanks for the feedback.

@drew7721
Copy link
Member

Is there a new issue created for this? Has this been fixed? From what I see the PR was rejected but the issue was also closed...

@magento-engcom-team magento-engcom-team added bugfix Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release labels Nov 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Progress: needs update Progress: reject Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants