Fixing invalid null-return in Google Analytics

[alexh-swdev]

Description / Fixed Issues (*)

See #3435 :
The problem is, that the _categorieIds contains ALL its categories, of all its subshops. The poped category is not from the subshop from where it is added to the cart. Probably that's why the category is not loaded and its name is null.

[empiricompany]

Hello @daboss84, thanks for reporting these issues. What about a more granular selection like this? Also, fix a case where the category is not active but is being incorrectly tracked. (we have some customers that put products in hidden categories for internal purpose)
I've tested it in a multi-website/store environment.

    /**
     * Returns last category name
     *
     * @param Mage_Catalog_Model_Product $product
     * @return ?string
     */
    public function getLastCategoryName($product): ?string
    {
        // force root category to current store
        $rootCategory = Mage::getModel('catalog/category')
            ->load(Mage::app()->getStore()->getRootCategoryId());

        $_lastCat = Mage::getResourceModel('catalog/category_collection')
            ->addAttributeToSelect('name')
            ->addIdFilter($product->getCategoryIds())
            ->addIsActiveFilter()
            ->addFieldToFilter('path', array('like' => $rootCategory->getPath() . '/%'))
            ->addOrder('level')
            ->getFirstItem();

        return $_lastCat->getName() ?: false;
    }

Reference: https://stackoverflow.com/questions/15505221/how-do-i-get-the-category-ids-that-a-product-is-in-with-respect-to-the-store-tha

Test conducted:

correct skip Categories of "Secondary Website" and disabled categories "New arrivals 1.2"

2023-08-10T08:43:33+00:00 DEBUG (7): Array
(
    [0] => gtag('event', 'add_to_cart', {"currency":"USD","value":"99.00","items":[{"item_id":"test-multi-shop","item_name":"Test multi webstire store product","price":"99.00","quantity":1,"item_brand":"","item_category":"New Arrivals"}]});
    [1] => gtag('event', 'view_item', {"currency":"USD","value":"99.00","items":[{"item_id":"test-multi-shop","item_name":"Test multi webstire store product","list_name":"Product Detail Page","item_category":"New Arrivals","price":"99.00"}]});
)

without excluding no active categories add_to_cart tracks it wrong:

2023-08-10T08:43:33+00:00 DEBUG (7): Array
(
    [0] => gtag('event', 'add_to_cart', {"currency":"USD","value":"99.00","items":[{"item_id":"test-multi-shop","item_name":"Test multi webstire store product","price":"99.00","quantity":1,"item_brand":"","item_category":"New Arrivals 1.2"}]});
    [1] => gtag('event', 'view_item', {"currency":"USD","value":"99.00","items":[{"item_id":"test-multi-shop","item_name":"Test multi webstire store product","list_name":"Product Detail Page","item_category":"New Arrivals","price":"99.00"}]});
)

(in this case view_item event picks category from current_category registry, while add_to_cart selects the last category but it is not active)

If I re-enable "New Arrivals 1.2," it correctly picks it.

[fballiano]

I think we should use a collection instead of a "load in a loop", the performance hit (in this case) wouldn't be big but a collection just looks better and safer

[alexh-swdev]

Hi,
sorry for the delay, I am on a business trip right now.
So if I get you right, @fballiano , we're going with @empiricompany's solution? Then I'll go forward and merge his pull request? However, I've no time for testing it, too. But his results look good.

[fballiano]

@daboss84 sorry for the delay, I'd merge @empiricompany's PR alexh-swdev#1 cause a collection it's always safer than loading a in a loop, I'll try to test it our as soon as it gets published here ;-)

[fballiano]

phpstan is not happy

[alexh-swdev]

What does that mean exaclty? What do we have to do?

(I am new to PR's and I only know PHPStan exists but not really what for unfortunately)

[empiricompany]

@daboss84 merge last review suggests from @fballiano and should be fix also phpstan

this is the correct version:

     /**
     * Returns last category name
     *
     * @param Mage_Catalog_Model_Product $product
     * @return string
     */
    public function getLastCategoryName($product): string
    {
        // force root category to current store
        $rootCategory = Mage::getModel('catalog/category')
            ->load(Mage::app()->getStore()->getRootCategoryId());

        $_lastCat = Mage::getResourceModel('catalog/category_collection')
            ->addAttributeToSelect('name')
            ->addIdFilter($product->getCategoryIds())
            ->addIsActiveFilter()
            ->addFieldToFilter('path', array('like' => $rootCategory->getPath() . '/%'))
            ->addOrder('level')
            ->getFirstItem();

        return $_lastCat->getName() ?: '';
    }

[alexh-swdev]

Ah ok, thanks. Yes I can do that in the evening (in a couple of hours)

[fballiano]

I've pushed a new version to this PR, let me know if it works for you

[alexh-swdev]

Hi @fballiano I just run a quick test and it looks to me, like the category is empty now.

[empiricompany]

the problem is this commit ad9d881

$storeRootCategory = Mage::app()->getStore()->getRootCategoryId();
then
->addFieldToFilter('path', ['like' => "{$storeRootCategory}/%"])

condition results: like '2/%' where 2 is the root category of store
but this not include the root category 1, the condition must be like '1/2/%'

in my version i'm using getPath() that return 1/2 including root category id avoiding this issues
we can simply change the condition to
->addFieldToFilter('path', ['like' => "1/{$storeRootCategory}/%"])
assuming that the root category has id 1 or there is same method to get the root correct category?
or simply restore the loading of root category model then getPath()`

$rootCategory = Mage::getModel('catalog/category')
            ->load(Mage::app()->getStore()->getRootCategoryId());
[..]
->addFieldToFilter('path', array('like' => $rootCategory->getPath() . '/%'))

[alexh-swdev]

I just committed those changes. Now it looks like it's working in my test environment.

[alexh-swdev]

Do I have anything to do with this? Or can I leave it as it is? If I have to merge, I can do it in a couple of hours (european night time)

[fballiano]

I think the first time I've to approve the workflow run.

sorry about the rootCategory thing, I tested it in my environment and it was working fine and I really wanted to avoid another model load...