search

magento / meta-for-magento2

33 stars 19 forks source link

Potential errors and performance issues in Tracker/InitiateCheckout #72

Open snoop0x7b opened 2 weeks ago

snoop0x7b commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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}