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

Potential errors and performance issues in Tracker/InitiateCheckout #72

Closed
snoop0x7b opened this issue Jun 12, 2024 · 3 comments
Closed

Comments

@snoop0x7b
Copy link

Preconditions (*)

  1. Any version of Magento, any version of PHP.

Steps to reproduce (*)

  1. Purchase an item that isn't in any category.

Expected result (*)

  1. Checkout/conversion should work correctly

Actual result (*)

  1. SQL error.

Basically what's going on is

` private function getCartCategories(CartInterface $quote): string
{
if (!$quote) {
return '';
}

    $items = $quote->getAllVisibleItems();
    foreach ($items as $item) {
        $product = $item->getProduct();
        $categoryIds = $product->getCategoryIds();
        $categories = $this->categoryCollection->create()
            ->addAttributeToSelect('*')
            ->addAttributeToFilter('entity_id', $categoryIds);
        $categoryNames = [];
        foreach ($categories as $category) {
            $categoryNames[] = $category->getName();
        }
    }
    return implode(',', $categoryNames); /** @phpstan-ignore-line */
}`

In this code -

It's assumed that the quote item is in a category. This should be checked prior to doing this query.

Second, this is a query in a loop. You could easily aggregate the category IDs and do the query after.

Third, you're selecting ALL attributes even though you only use name. It's a best practice in Magento to only fetch the attributes you actually need.

@snoop0x7b
Copy link
Author

Fourth logic error I noticed...

$categoryNames = []; foreach ($categories as $category) { $categoryNames[] = $category->getName(); }

You guys overwrite categoryNames for each item in the loop, so as a result you only get category names from the last item in the quote.

@snoop0x7b
Copy link
Author

Also FYI - This is what that SQL error log looks like.

#0 /home/jetrails/XXXX/html/vendor/magento/framework/DB/Statement/Pdo/Mysql.php(89): Magento\Framework\DB\Statement\Pdo\Mysql->tryExecute(Object(Closure))
#1 /home/jetrails/XXXX/html/vendor/magento/zend-db/library/Zend/Db/Statement.php(313): Magento\Framework\DB\Statement\Pdo\Mysql->_execute(Array)
#2 /home/jetrails/XXXX/html/vendor/magento/zend-db/library/Zend/Db/Adapter/Abstract.php(480): Zend_Db_Statement->execute(Array)
#3 /home/jetrails/XXXX/html/vendor/magento/zend-db/library/Zend/Db/Adapter/Pdo/Abstract.php(242): Zend_Db_Adapter_Abstract->query('SELECT e.* FR...', Array)
#4 /home/jetrails/XXXX/html/vendor/magento/framework/DB/Adapter/Pdo/Mysql.php(564): Zend_Db_Adapter_Pdo_Abstract->query('SELECT e.* FR...', Array)
#5 /home/jetrails/XXXX/html/vendor/magento/framework/DB/Adapter/Pdo/Mysql.php(634): Magento\Framework\DB\Adapter\Pdo\Mysql->_query('SELECT e.* FR...', Array)
#6 /home/jetrails/XXXX/html/vendor/magento/zend-db/library/Zend/Db/Adapter/Abstract.php(737): Magento\Framework\DB\Adapter\Pdo\Mysql->query(Object(Magento\Framework\DB\Select), Array)
#7 /home/jetrails/XXXX/html/vendor/magento/framework/Data/Collection/Db/FetchStrategy/Query.php(21): Zend_Db_Adapter_Abstract->fetchAll(Object(Magento\Framework\DB\Select), Array)
#8 /home/jetrails/XXXX/html/vendor/magento/framework/Data/Collection/AbstractDb.php(782): Magento\Framework\Data\Collection\Db\FetchStrategy\Query->fetchAll(Object(Magento\Framework\DB\Select), Array)
#9 /home/jetrails/XXXX/html/vendor/magento/module-eav/Model/Entity/Collection/AbstractCollection.php(1137): Magento\Framework\Data\Collection\AbstractDb->_fetchAll(Object(Magento\Framework\DB\Select))
#10 /home/jetrails/XXXX/html/vendor/magento/module-eav/Model/Entity/Collection/AbstractCollection.php(933): Magento\Eav\Model\Entity\Collection\AbstractCollection->_loadEntities(false, false)
#11 /home/jetrails/XXXX/html/vendor/magento/module-catalog/Model/ResourceModel/Category/Collection.php(246): Magento\Eav\Model\Entity\Collection\AbstractCollection->load(false, false)
#12 /home/jetrails/XXXX/html/vendor/magento/framework/Data/Collection.php(840): Magento\Catalog\Model\ResourceModel\Category\Collection->load()
#13 /home/jetrails/XXXX/html/app/code/Meta/Conversion/Model/Tracker/InitiateCheckout.php(122): Magento\Framework\Data\Collection->getIterator()
#14 /home/jetrails/XXXX/html/app/code/Meta/Conversion/Model/Tracker/InitiateCheckout.php(163): Meta\Conversion\Model\Tracker\InitiateCheckout->getCartCategories(Object(Magento\Quote\Model\Quote\Interceptor))
#15 /home/jetrails/XXXX/html/app/code/Meta/Conversion/Controller/Pixel/Tracker.php(87): Meta\Conversion\Model\Tracker\InitiateCheckout->getPayload(Array)
#16 /home/jetrails/XXXX/html/vendor/magento/framework/Interception/Interceptor.php(58): Meta\Conversion\Controller\Pixel\Tracker->execute()
#17 /home/jetrails/XXXX/html/vendor/magento/framework/Interception/Interceptor.php(138): Meta\Conversion\Controller\Pixel\Tracker\Interceptor->___callParent('execute', Array)
#18 /home/jetrails/XXXX/html/vendor/magento/framework/Interception/Interceptor.php(153): Meta\Conversion\Controller\Pixel\Tracker\Interceptor->Magento\Framework\Interception{closure}()
#19 /home/jetrails/XXXX/html/generated/code/Meta/Conversion/Controller/Pixel/Tracker/Interceptor.php(23): Meta\Conversion\Controller\Pixel\Tracker\Interceptor->___callPlugins('execute', Array, Array)
#20 /home/jetrails/XXXX/html/vendor/magento/framework/App/FrontController.php(248): Meta\Conversion\Controller\Pixel\Tracker\Interceptor->execute()
#21 /home/jetrails/XXXX/html/vendor/magento/framework/App/FrontController.php(212): Magento\Framework\App\FrontController->getActionResponse(Object(Meta\Conversion\Controller\Pixel\Tracker\Interceptor), Object(Magento\Framework\App\Request\Http))
#22 /home/jetrails/XXXX/html/vendor/magento/framework/App/FrontController.php(146): Magento\Framework\App\FrontController->processRequest(Object(Magento\Framework\App\Request\Http), Object(Meta\Conversion\Controller\Pixel\Tracker\Interceptor))
#23 /home/jetrails/XXXX/html/vendor/magento/framework/Interception/Interceptor.php(58): Magento\Framework\App\FrontController->dispatch(Object(Magento\Framework\App\Request\Http))
#24 /home/jetrails/XXXX/html/vendor/magento/framework/Interception/Interceptor.php(138): Magento\Framework\App\FrontController\Interceptor->___callParent('dispatch', Array)
#25 /home/jetrails/XXXX/html/vendor/magento/module-store/App/FrontController/Plugin/RequestPreprocessor.php(99): Magento\Framework\App\FrontController\Interceptor->Magento\Framework\Interception{closure}(Object(Magento\Framework\App\Request\Http))
#26 /home/jetrails/XXXX/html/vendor/magento/framework/Interception/Interceptor.php(135): Magento\Store\App\FrontController\Plugin\RequestPreprocessor->aroundDispatch(Object(Magento\Framework\App\FrontController\Interceptor), Object(Closure), Object(Magento\Framework\App\Request\Http))
#27 /home/jetrails/XXXX/html/vendor/fastly/magento2/Model/FrontControllerPlugin.php(135): Magento\Framework\App\FrontController\Interceptor->Magento\Framework\Interception{closure}(Object(Magento\Framework\App\Request\Http))
#28 /home/jetrails/XXXX/html/vendor/magento/framework/Interception/Interceptor.php(135): Fastly\Cdn\Model\FrontControllerPlugin->aroundDispatch(Object(Magento\Framework\App\FrontController\Interceptor), Object(Closure), Object(Magento\Framework\App\Request\Http))
#29 /home/jetrails/XXXX/html/vendor/magento/module-page-cache/Model/App/FrontController/BuiltinPlugin.php(71): Magento\Framework\App\FrontController\Interceptor->Magento\Framework\Interception{closure}(Object(Magento\Framework\App\Request\Http))
#30 /home/jetrails/XXXX/html/vendor/magento/framework/Interception/Interceptor.php(135): Magento\PageCache\Model\App\FrontController\BuiltinPlugin->aroundDispatch(Object(Magento\Framework\App\FrontController\Interceptor), Object(Closure), Object(Magento\Framework\App\Request\Http))
#31 /home/jetrails/XXXX/html/vendor/magento/framework/Interception/Interceptor.php(153): Magento\Framework\App\FrontController\Interceptor->Magento\Framework\Interception{closure}(Object(Magento\Framework\App\Request\Http))
#32 /home/jetrails/XXXX/html/generated/code/Magento/Framework/App/FrontController/Interceptor.php(23): Magento\Framework\App\FrontController\Interceptor->___callPlugins('dispatch', Array, NULL)
#33 /home/jetrails/XXXX/html/vendor/magento/framework/App/Http.php(116): Magento\Framework\App\FrontController\Interceptor->dispatch(Object(Magento\Framework\App\Request\Http))
#34 /home/jetrails/XXXX/html/vendor/magento/framework/App/Bootstrap.php(264): Magento\Framework\App\Http->launch()
#35 /home/jetrails/XXXX/html/pub/index.php(29): Magento\Framework\App\Bootstrap->run(Object(Magento\Framework\App\Http\Interceptor))
#36 {main}

@snoop0x7b snoop0x7b mentioned this issue Jun 12, 2024
4 tasks
@zlik
Copy link
Contributor

zlik commented Sep 12, 2024

This has been fixed in v1.3.3. Please let us know if this becomes an issue again. Thanks!

@zlik zlik closed this as completed Sep 12, 2024
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

No branches or pull requests

2 participants