nystudio107 / craft-instantanalytics-ga4

Instant Analytics brings full Google GA4 server-side analytics support to your Twig templates and automatic Craft Commerce integration
Other
3 stars 4 forks source link

Missing `addCommerceProductDetailView` #6

Closed bossanova808 closed 1 year ago

bossanova808 commented 1 year ago

Unless I am missing something, I think you've missed re-implementing this one:

{% do instantAnalytics.addCommerceProductDetailView(PRODUCT_VARIANT) %}

These both work

{% do instantAnalytics.addCommerceProductListImpression(PAGE_PRODUCTS, LIST_NAME) %}
{% do instantAnalytics.addCommerceProductImpression(PRODUCT_VARIANT, INDEX, LIST_NAME) %}

But the detail view does not seem to achieve anything in the actual analytics, and I can't see it implemented when I poke around the code.

Romanavr commented 1 year ago

You can use this: https://github.com/nystudio107/craft-instantanalytics-ga4/blob/develop-v4/src/variables/InstantAnalyticsVariable.php#L77

{% if instantAnalytics is defined %}
    {% do instantAnalytics.addCommerceProductView(product)  %}
{% endif %}
bossanova808 commented 1 year ago

Yes that's probably the intention, but then the docs are incorrect. Not a hard fix anwyay!

bossanova808 commented 1 year ago

So that's a variable, not a service, so I think it would have to be

{{ instantAnalytics.addCommerceProductView(product)  }}

...but that is not working for me (no view_item event hits GA4), and nor is calling the corresponding service:

{% do instantAnalytics.addCommerceProductImpression(product) %}

Because that requires an index apparently (more like it's a list event).

All a bit broken currently, in a nutshell.

I'm sticking with my own created even here for now until @khalwat has a chance to tidy this up.

khalwat commented 1 year ago

Addressed in 4.0.0-beta.2

bossanova808 commented 1 year ago

(You'll want to update your docs here too as those are still wrong)

& maybe give an example showing you need to pass a 0 for index (although personally I recommend you default that, instead - as when adding a singular product, one expects it to the only thing on the 'list')

Currently, this works: {% do instantAnalytics.addCommerceProductImpression(product, 0) %}

khalwat commented 1 year ago

Yeah I think this:

{% do instantAnalytics.addCommerceProductImpression(product) %}

...should "just work" in that the second param should just default to 0.

What wrong with the docs, exactly?

bossanova808 commented 1 year ago

Has the old thing one in it still

image

And no, if you leave out that zero, you get:

ArgumentCountError: Too few arguments to function nystudio107\instantanalyticsGa4\ga4\Analytics::addCommerceProductImpression(), 1 passed in /var/www/html/vendor/twig/twig/src/Extension/CoreExtension.php on line 1607 and at least 2 expected in /var/www/html/vendor/nystudio107/craft-instantanalytics-ga4/src/ga4/Analytics.php:182
Stack trace:
#0 /var/www/html/vendor/twig/twig/src/Extension/CoreExtension.php(1607): nystudio107\instantanalyticsGa4\ga4\Analytics->addCommerceProductImpression(Object(craft\commerce\elements\Product))
#1 /var/www/html/vendor/craftcms/cms/src/helpers/Template.php(146): twig_get_attribute(Object(craft\web\twig\Environment), Object(Twig\Source), Object(nystudio107\instantanalyticsGa4\ga4\Analytics), 'addCommerceProd...', Array, 'method', false, false)
#2 /var/www/html/storage/runtime/compiled_templates/79/795fced158518896b71fed3bb0a697af.php(73): craft\helpers\Template::attribute(Object(craft\web\twig\Environment), Object(Twig\Source), Object(nystudio107\instantanalyticsGa4\ga4\Analytics), 'addCommerceProd...', Array, 'method')
#3 /var/www/html/vendor/twig/twig/src/Template.php(171): __TwigTemplate_56ec2f617fe489e0810d29b1a2624f55->block_structure(Array, Array)
#4 /var/www/html/storage/runtime/compiled_templates/2a/2a07f9d3c3d4cccfcffbf96963bd01b0.php(253): Twig\Template->displayBlock('structure', Array, Array)
#5 /var/www/html/vendor/twig/twig/src/Template.php(394): __TwigTemplate_71811f97b6dd8854e452d2604f89e572->doDisplay(Array, Array)
#6 /var/www/html/vendor/twig/twig/src/Template.php(367): Twig\Template->displayWithErrorHandling(Array, Array)
#7 /var/www/html/storage/runtime/compiled_templates/79/795fced158518896b71fed3bb0a697af.php(56): Twig\Template->display(Array, Array)
#8 /var/www/html/vendor/twig/twig/src/Template.php(394): __TwigTemplate_56ec2f617fe489e0810d29b1a2624f55->doDisplay(Array, Array)
#9 /var/www/html/vendor/twig/twig/src/Template.php(367): Twig\Template->displayWithErrorHandling(Array, Array)
#10 /var/www/html/vendor/twig/twig/src/Template.php(379): Twig\Template->display(Array)
#11 /var/www/html/vendor/twig/twig/src/TemplateWrapper.php(40): Twig\Template->render(Array, Array)
#12 /var/www/html/vendor/twig/twig/src/Environment.php(277): Twig\TemplateWrapper->render(Array)
#13 /var/www/html/vendor/craftcms/cms/src/web/View.php(465): Twig\Environment->render('_site/product', Array)
#14 /var/www/html/vendor/craftcms/cms/src/web/View.php(518): craft\web\View->renderTemplate('_site/product', Array)
#15 /var/www/html/vendor/craftcms/cms/src/web/TemplateResponseFormatter.php(56): craft\web\View->renderPageTemplate('_site/product', Array, 'site')
#16 /var/www/html/vendor/yiisoft/yii2/web/Response.php(1098): craft\web\TemplateResponseFormatter->format(Object(craft\web\Response))
#17 /var/www/html/vendor/craftcms/cms/src/web/Response.php(286): yii\web\Response->prepare()
#18 /var/www/html/vendor/yiisoft/yii2/web/Response.php(339): craft\web\Response->prepare()
#19 /var/www/html/vendor/yiisoft/yii2/base/Application.php(390): yii\web\Response->send()
#20 /var/www/html/web/index.php(12): yii\base\Application->run()
#21 {main}
bossanova808 commented 1 year ago

(There also this in the variable class - which will fail if called:

    public function addCommerceProductView($productVariant): void
    {
        InstantAnalytics::$plugin->commerce->addCommerceProductImpression($productVariant);
    }
khalwat commented 1 year ago

Right, I'm saying we should fix it so you can leave out that second param

bossanova808 commented 1 year ago

Ah right, sorry! Yep agree

khalwat commented 1 year ago

To clarify, due to the nature of the changes in GA4, this event:

{% do instantAnalytics.addCommerceProductDetailView(PRODUCT_VARIANT) %}

...no longer really does anything. Instead, you should be using:

{% do instantAnalytics.addCommerceProductImpression(PRODUCT_VARIANT, INDEX, LIST_NAME) %}

...but for backwards compatibility, we'd made addCommerceProductDetailView() alias to addCommerceProductImpression()

We just need to fix the implementation as noted above.

khalwat commented 1 year ago

Addressed in 4.0.0-beta.3 -> https://github.com/nystudio107/craft-instantanalytics-ga4/releases/tag/4.0.0-beta.3