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

Fix count SQL on products sold collection #8812

Merged

Conversation

jameshalsall
Copy link

@jameshalsall jameshalsall commented Mar 7, 2017

No description provided.

@avoelkl avoelkl self-assigned this Mar 10, 2017
@avoelkl avoelkl added this to the March 2017 milestone Mar 10, 2017
@avoelkl
Copy link
Contributor

avoelkl commented Mar 14, 2017

Hi @jameshalsall!
Thanks for the PR!
Actually I was trying to reproduce what your PR solves, can you tell me where/in which cases you had an issue?
It seems the Ordered Quantity works correctly?

@jameshalsall
Copy link
Author

jameshalsall commented Mar 15, 2017

If you use Magento\Reports\Model\ResourceModel\Product\Sold\Collection to fetch the most sold products anywhere in user land code, it breaks on the count SQL as the table alias is incorrect.

Example code from my project:

<?php

use Magento\Reports\Model\ResourceModel\Product\Sold\CollectionFactory as ProductsSoldCollectionFactory;

class MostPurchasedProductSkuProvider
{
    private $collectionFactory;
    private $storeManager;

    public function __construct(ProductsSoldCollectionFactory $productsSoldCollectionFactory)
    {
        $this->collectionFactory = $productsSoldCollectionFactory;
    }

    public function getSkus(int $limit, \DateTimeInterface $from, \DateTimeInterface $to): array
    {
        if ($limit < 1) {
            return [];
        }

        $collection = $this->collectionFactory->create();

        $collection->addOrderedQty($from->format('Y-m-d H:i:s'), $to->format('Y-m-d H:i:s'));
        $collection->setOrder('ordered_qty');
        $collection->setPageSize($limit);

        $collection->getSelect()->columns('order_items.sku');

        $skus = [];
        foreach ($collection as $item) {
            $skus[] = $item['sku'];
        }

        return $skus;
    }
}

@avoelkl
Copy link
Contributor

avoelkl commented Mar 15, 2017

Thanks for the update @jameshalsall!
Mind to add a test as well, like in Magento\Reports\Test\Unit\Model\ResourceModel\Report\Quote\CollectionTest for example? That would be supercool. Let me know if you need help.

@jameshalsall
Copy link
Author

jameshalsall commented Mar 16, 2017 via email

@jameshalsall jameshalsall force-pushed the fix-product-sold-collection-sql branch from 5e988f3 to dc333b4 Compare March 16, 2017 21:52
@jameshalsall
Copy link
Author

I've added a test to cover this functionality now.

@magento-team magento-team merged commit dc333b4 into magento:develop Mar 25, 2017
@vrann
Copy link
Contributor

vrann commented Mar 25, 2017

@jameshalsall Thank you for the contribution!

@jameshalsall jameshalsall deleted the fix-product-sold-collection-sql branch March 27, 2017 11:39
magento-devops-reposync-svc pushed a commit that referenced this pull request Apr 9, 2024
ACPT-1837: Removing unused use from ResetterInterface
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants