Hello,

I created a module which integrates Drupal Commerce transactions to Google Tag Manager. Popularity of this combination is growing but there is no module for that so far.

Code is based on well tested Commerce Google Analytics module. Module was developed for commerce site Expomat.cz (please do not test it there :-)

To ensure the module is working properly and complies with Drupal code standards I checked it via

Motivation

My name is Petr Parimucha and I've been working with Drupal since 2009. My speciality is Ubercart / Drupal Commerce. I'm developing many small modules that might be useful to the Drupal community. I'm eager to share them.

Links

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/Parisek/2263083.git
commerce_google_tag_manager

Link to project page: https://drupal.org/sandbox/parisek/2263083

Reviews of other projects

https://drupal.org/node/2261421#comment-8777063
https://drupal.org/node/2263319#comment-8777165
https://drupal.org/node/2257809#comment-8777779

Comments

parisek’s picture

Issue summary: View changes
scotthooker’s picture

Hi apologies but this functionality already exists in https://drupal.org/project/google_tag

PA robot’s picture

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

parisek’s picture

scotthooker: Thanks for comment, but how to achieve this functionality in https://drupal.org/project/google_tag ? I checked whole code and couldn't find it! :-)

parisek’s picture

Issue summary: View changes
Issue tags: +PAreview: review bonus
scotthooker’s picture

Ah I see. Ok then this could be a good addition! You need to update the git link as thats your personal git clone link .

parisek’s picture

Issue summary: View changes

Corrected git link

parisek’s picture

Issue summary: View changes
mpdonadio’s picture

Manual Review; I can't install as I don't have a working Commerce site at the moment.

1. You mention footer in the comment on line 18, but you are adding it to the header scope on line 125.

2. The label in commerce_google_tag_manager_default_rules_configuration() is untranslated, but
I am not 100% sure if this needs to be. I have also never see this directly built up this way; I have only seen
exported rules as strings that then get run through rules_import().

3. There is some extra space in commerce_google_tag_manager_enable() on the chained database methods.

4. Is there a better hook that you can use than commerce_google_tag_manager_init()? This gets called on every
non-cached page that Drupal handles, which includes image styles, AJAX, etc. That could wipe out the session info. Also,
as drupal_add_js() is gone
from Drupal 8 (https://drupal.org/node/2169605), you should move everything to #attached. Perhaps using hook_page_build,
and adding it as #attached there would be best.

5. The unfiltered data going into the inline Javascript gave me pause, but drupal_json_encode() will escape everything, so
there are no XSS concerns. May want a comment here.

I am leaving this as Needs Review, as I don't know whether #4 constitutes a blocking issue. The rest is minor.

heddn’s picture

commerce_google_tag_manager_enable
Rather than adjusting module weights, using hook_module_implements_alter() is a better solution.

A dependency on entity module should probably be listed. I know that commerce depends on it too, but what if it stopped using it (not that it is a risk)? As a general rule, a module should list all its direct dependencies.

No need to revert to old school method to obtain the entity id. ex.
$line_item->value()->commerce_product[LANGUAGE_NONE][0]['product_id']
Replace with:
$line_item->commerce_product->getIdentifier()

Replace the hook_init() with hook_page_build()

The un-translated string should be translated in commerce_google_tag_manager.rules_defaults.inc.

Moving to needs work for these small fixes. This module is almost ready for RTBC.

heddn’s picture

Status: Needs review » Needs work
parisek’s picture

mpdonadio: Thank you for your review.

1. Fixed
2. Label is now translatable
3. Actually when I was thinking how to make it better via hook_module_implements_alter() I found out, that this step is not neccessary and deleted this file
4. Rewrited to hook_page_build() function
5. Added comment

heddn Thanks for review, I added dependency and remove "old school" method to access ID :-) Other was fixed based on mpdonadio review

parisek’s picture

Status: Needs work » Needs review
mpdonadio’s picture

Status: Needs review » Reviewed & tested by the community

Automated Review

FILE: .../www/drupal-7-pareview/pareview_temp/commerce_google_tag_manager.module
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
30 | WARNING | Unused variable $name.

Super minor warning that can be ignored.

Manual Review

In commerce_google_tag_manager_ecommerce_js:73, the default value that core uses is 'Drupal'

Not seeing any module duplication, third-party code, security, or major API issues. I think this this is RBTC.

klausi’s picture

Status: Reviewed & tested by the community » Fixed

manual review:

  1. Could you describe the differences to Commerce Google Analytics on the project page?
  2. drupal_json_encode($transaction) makes sure that all data is escaped correctly, so I see no XSS vulnerabilities there, that should be fine.

Thanks for your contribution, parisek!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

parisek’s picture

Status: Fixed » Closed (fixed)

Thank you guys for reviews and promoting me

Issue closed