- Issue created by @dharizza
- πΊπΈUnited States phenaproxima Massachusetts
Seems like a good start, but a few things jump out to me as being a little odd.
This also needs test coverage; at minimum it has to have a ComponentValidationTest for idempotency. (There are examples in all the other recipes which you can take inspiration from.)
- πΈπ°Slovakia poker10
I know that the google_analytics module still does not have D11 compatibility fixed, but I am curious, what is the more common way to add tracking to the sites for ambitious marketers? Is it to use the GA module directly (where the Google Analytics configuration is easier and more straightforward) or using the Google Tag manager, where you need a bit more experience to configure it right?
- π¨π·Costa Rica dharizza
@poker10 The Google Analytics module has been marked as obsolete and its use as deprecated, the recommendation now is to use google_tag. Also, considering the persona we're trying to reach with Drupal CMS is marketers and the market share reflects the majority of them are using Google Analytics for web analytics and Google Tag Manager for tag management, the use of the google_tag module is more future proof as it allows them to keep using both.
- πΊπΈUnited States phenaproxima Massachusetts
This is really looking good! I think there are a few more things we should do to punch it up, but it's close to ready, IMHO.
Also, to make sure tests run, please add this component to the components list in
.gitlab-ci.yml
. - π©πͺGermany jurgenhaas Gottmadingen
We also need to have a small enhancement in the recipe.yml file to enable the privacy settings for GA and GTM. Those are:
config: actions: klaro.klaro_app.ga: simpleConfigUpdate: status: true klaro.klaro_app.gtm: simpleConfigUpdate: status: true
The Klaro module and the apps are all available by default in Drupal CMS, but the apps remain disabled until something gets installed which requires them to become enabled. This is what those to config actions achieve.
Do you want me to open a separate issue and MR for that, or could this be handled as part of this one?
- πΊπΈUnited States phenaproxima Massachusetts
@jurgenhaas I think that needs to be a separate issue. Can you open that, and postpone it on this one?
I think it might make sense for the privacy recipe to enable both of those apps by default, rather than have this analytics recipe do it.
- π©πͺGermany jurgenhaas Gottmadingen
I've opened π Enable privacy apps when applying Goggle Tag recipe Active and explained there why the privacy recipe can not enable all potential apps by itself.
- πΊπΈUnited States phenaproxima Massachusetts
@dharizza and I worked hard on this today and we are both completely satisfied with the code of the MR. There is the same basic test coverage that we would expect in any recipe, and this is a strong foundation on which to build further. The privacy stuff, as agreed, will be dealt with in another issue.
So, from my perspective, this is ready to go. Giving it to Pam for manual testing and final review/approval.
- π¦πΊAustralia pameeela
Thanks for getting it so close! Manually tested but I hit a bit of an issue.
When I apply the recipe, there are all kinds of JS errors that break things. I thought this might be because the ID was fake but I tried with a real GTM ID and still had the same problem. The errors go away if I either disable JS aggregation, or uninstall GTM. Did either of you see anything similar?
- πΊπΈUnited States tim.plunkett Philadelphia
#13 is potentially caused by / related to π Google Tag makes CSS aggregates uncacheable Active
- π¦πΊAustralia pameeela
Awesome! Thanks for sorting out that weird JS issue.
The only thing is I'd like to remove it from the installer and PB for now given the dependency on the input. We are getting enough questions/complaints about things that are not quite finished, I'd like to limit the exposure!
Other than that, RTBC from me.
-
phenaproxima β
committed 79ae25c1 on 0.x authored by
dharizza β
Issue #3487542 by phenaproxima, dharizza: Add Google Tag recipe
-
phenaproxima β
committed 79ae25c1 on 0.x authored by
dharizza β
- πΊπΈUnited States phenaproxima Massachusetts
And just like that, Drupal CMS has its first input-requiring recipe. :) Congrats, everyone! This is some very fine work. Merged into 0.x.
Automatically closed - issue fixed for 2 weeks with no activity.