coreshop / CoreShop

CoreShop - Pimcore enhanced eCommerce
http://www.coreshop.org
Other
275 stars 157 forks source link

DataObjectBatchListing ignores ->setOffset() #2714

Closed hethehe closed 3 weeks ago

hethehe commented 4 weeks ago
Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no

If we create a Listingand set a offset with $listing->setOffset(100) and use it with the CoreShop\Component\Pimcore\BatchProcessing\DataObjectBatchListing it gets ignored.

I think the problem is that you get the loadIdList() and the getTotalCount() from the DAO instead directly from the $list

Is there a special reason you use the $dao instead the $list directly?

dpfaffenbauer commented 4 weeks ago

@hethehe honestly, not sure... we never actually had in mind that the BatchList can use a offset :D.

hethehe commented 4 weeks ago

@dpfaffenbauer We tried to set an offset for the coreshop:index command, as sometimes it would be nice not to include all products ;)

Are you interested in a PR that changes the $dao to $list? I don't see anything that could break, WDYT?

dpfaffenbauer commented 4 weeks ago

@hethehe yes, we are interested. it should be tested anyway, maybe you can add another test for offests?

hethehe commented 4 weeks ago

Just realized, that this is not a problem of your BatchListingbut from Pimcore's getTotalCount() see: https://github.com/pimcore/pimcore/issues/17618

dpfaffenbauer commented 4 weeks ago

gotcha, then we can close this?

hethehe commented 4 weeks ago

yes, closed

kingjia90 commented 4 weeks ago

Can this issue be opened again please? I believe the initial issue would be fixed if we replace getTotalCount() to getCount() in there

https://github.com/coreshop/CoreShop/blob/38d6eca36bc3116a0c23218007e79f3f24e5b0bc/src/CoreShop/Component/Pimcore/BatchProcessing/DataObjectBatchListing.php#L86-L94

see also https://github.com/pimcore/pimcore/issues/17618#issuecomment-2353334021

dpfaffenbauer commented 4 weeks ago

@hethehe can you give it a test?