it just add you the ability to paste the codesnippet from the google tag manager right above the html end section
you can config what role you want to track.

config and add tag manager code via admin/settings/gtm
the first .dev is for drupal 6

..some plannings but need really help!

Project page: http://drupal.org/sandbox/ekn33/1892672
Git: git clone --recursive --branch 6.x-1.x http://git.drupal.org/sandbox/ekn33/1892672.git gtm

Manual review other Modules:

  1. http://drupal.org/node/1871326#comment-6975494
  2. http://drupal.org/node/1740956#comment-6975848
  3. http://drupal.org/node/1784544#comment-6976022

Comments

eule’s picture

eule commented
Status: Active » Needs review
Ignigena’s picture

Ignigena commented

You're missing a README.txt in the module.

There are quite a few warnings and errors found here:
http://ventral.org/pareview/httpgitdrupalorgsandboxekn331802214git

You should run the coder module and just refresh yourself on the Drupal Coding Standards found here:
http://drupal.org/node/318

Also, it's a good idea to put a direct link to your GIT repository (git clone...) see here:
http://drupal.org/node/1011698

Just a few things that should be fixed before a full review can be made.

Good luck!

Ignigena’s picture

Ignigena commented
Status: Needs review » Needs work
eule’s picture

eule commented

thank you

eule’s picture

eule commented
Status: Needs work » Needs review

please add if you can again a link with the help for the warnings and errors. i add here the readme.txt and change issue

brajendrasingh’s picture

brajendrasingh commented

Please add some more description about your project.
Also mention git repositary path in your issue details.

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.

Run coder to check your style, some issues were found (please check the Drupal coding standards).
Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards).

eule’s picture

eule commented

yes i work in the master branch on git...u drupal heros make my live not easyer ;/
will see what i can do! maybe i see someone on the irc chan thats can help...

the "coder" project - i see this is only aviable on drupal 7...to understand..i setup a d7 install and run "coder" here
can i check than d6 modules ???

varunmishra2006’s picture

varunmishra2006 commented
Status: Needs review » Needs work

Manual Review

In gtm.install and gtm.admin.inc file why used $result = db_query('SELECT * FROM {role} ORDER BY name');
You can use user_roles function .

The README.txt is not proper . Check http://drupal.org/node/161085
The project page is quite messed up. Please add some screen shot and categorized description. see http://drupal.org/node/997024

Online review.
The module failed to meet drupal coding standards. Please fix them. You can use coder module or ventral.org to see errors and warnings. See http://ventral.org/pareview/httpgitdrupalorgsandboxekn331802214git

You are using master branch.You should really be working in a version specific branch.

eule’s picture

eule commented

Guys how i can get this to the new branch? i setup in git a new one for .dev release but with this document http://drupal.org/node/1127732 it is not possibel to get a new brach in drupal on a sandbox project.
i can´t change under drupal my versions control, here is only master aviable.
http://drupal.org/project/1802214/git-instructions

eule’s picture

eule commented
Status: Needs work » Needs review

hello, sorry for the needs review but i need help with the setup for a new branch! i descript my problem here but i get no answer http://drupal.org/node/1819164
the guys on the git chan have no clue about it..so i need help by you drupaler´s

eule’s picture

eule commented

Sandbox: http://drupal.org/sandbox/ekn33/1802214
Git Clone: ekn33@git.drupal.org:sandbox/ekn33/1802214.git
Drupalcode tree: http://drupalcode.org/sandbox/ekn33/1802214.git/tree
New Branch: https://github.com/ekn33/GTM/tree/6.x-1.x
Version: 6.x

plazik’s picture

plazik commented

This module http://drupal.org/sandbox/sgerrand/1809298 seems to be more powerful.

eule’s picture

eule commented

i try it.
first. go and use it if u have a d7 install
second..who is the power?

this module here gives u more options! and came up FiRST

eule’s picture

eule commented

switching branch to 6.x-1.x
updating project page

indydas’s picture

indydas commented

Hi there,

Just initially ran your module and this is the results.

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732

http://ventral.org/pareview/httpgitdrupalorgsandboxekn331802214git. Not too many errors if you go to this link.

udaksh’s picture

udaksh commented

Hi Eule,

Manual Review :
README.txt is not in proper format.

Usage of t() at line 38 in gtm.admin.inc is incorrect. Use placeholder for passing variables to t() http://api.drupal.org/api/drupal/includes%21common.inc/function/t/6 .

change all db_query() to db_select().

Function documentation is not present for function gtm_footer() in gtm.module .

Your commenting style is incorrect in file gtm.module. You are presently using
/**
*
*/
Please Use
/**
*
*/
and correct similar coding style issues raised by automated review tool.

Thanks

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

eule’s picture

eule commented

master branch purged

benjy’s picture

benjy commented

@eule, how are you going with this?

Do you need some help finishing it off because I'm keen to use this module.

eule’s picture

eule commented

hi, i found one who will help me..so i add a new co-maintainer. i test it a lot and have now troubel with tracking, dunno whats up with the tag manager...i join today the google help forum.

and yes Help is always welcome. looking for someone who can rewrite it for d7.

cheers

eule’s picture

eule commented

tracking works again ..lags on tag manager settings

eule’s picture

eule commented
Issue summary: View changes

spelling, my english is not the best

eule’s picture

eule commented
Issue summary: View changes

changing project name

ain’s picture

ain commented
Status: Needs review » Needs work

Automated review of both D6 and D7 branches: all clear.
Manual review of D6 and D7: all fine.

I also tested the D7 module and found a problem. From Google Tag Manager documentation:

Container Snippet
Copy the following code then paste it onto every page of your website. Place it immediately after the opening <body> tag.

Currently the snippet is being added not right after the <body> tag:

<body class="html front not-logged-in one-sidebar sidebar-first page-node">
  <div id="skip-link">
    <a href="#main-content" class="element-invisible element-focusable">Skip to main content</a>
  </div>
    <div class="region region-page-top">
    <!-- Google Tag Manager -->
<noscript>&lt;iframe src="//www.googletagmanager.com/ns.html?id=GTM-5DLX"
height="0" width="0" style="display:none;visibility:hidden"&gt;&lt;/iframe&gt;</noscript>
<script>(function(w,d,s,l,i){w[l]=w[l]||[];w[l].push({'gtm.start':
new Date().getTime(),event:'gtm.js'});var f=d.getElementsByTagName(s)[0],
j=d.createElement(s),dl=l!='dataLayer'?'&l='+l:'';j.async=true;j.src=
'//www.googletagmanager.com/gtm.js?id='+i+dl;f.parentNode.insertBefore(j,f);
})(window,document,'script','dataLayer','GTM-5DLX');</script>
<!-- End Google Tag Manager -->
  </div>
  <div id="page-wrapper"><div id="page">
…

It's not likely to affect the functionality, but to keep the high quality of our modules, it's worth to make the correction. Please also check if this is the case on D6 module.

Additional recommendations:

  1. A more comprehensive README that is more like the info on your project page.
  2. Implement hook_help() to increase UX of the module administration.
  3. Provide better description for GTM snippet. Currently it only offers a link to Google Tag Manager, but should more importantly say what should be copied there. Even just a direct link to http://support.google.com/tagmanager/answer/2574370?hl=et&ref_topic=2574303 would be nice.

Generally I think the module is ready for prime time.

NB! Please consider reviewing 3 other projects yourself to get this ticket closure on a fast track.

benjy’s picture

benjy commented

It looks like you have a D7 branch in the repo. Have you started work on this? Otherwise let me know and i'll have a look this week.

eule’s picture

eule commented

@benjy yes the start is alive thanks to drupalrv but if you have some tips let me know please

thanks

benjy’s picture

benjy commented

@eule, i'll give it a look over this week. Do you plan on doing a few reviews to get this module into contrib?

eule’s picture

eule commented

i plan to get this module into contrib ..sure....but my coding experience @all not the best, i got the idee and have make the module for my d6 sites without any experience on drupal coding standarts..for me the module was working so i think me i give this to the community. for now i have learned about that git and drupal stuff :)
thats why help / patches always welcome to make it stable for the community

eule’s picture

eule commented
Issue summary: View changes

upd

eule’s picture

eule commented
Issue summary: View changes

review added

eule’s picture

eule commented
Issue summary: View changes

review added

eule’s picture

eule commented
Status: Needs work » Needs review

adding 3 manual reviews for the bonus

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

Don't forget to add the "PAReview: review bonus" tag as indicated in #1410826: [META] Review bonus, otherwise you won't show up on my high priority list.

eule’s picture

eule commented
Issue tags: +PAreview: review bonus

3 manual reviews for the bonus

yannickoo’s picture

yannickoo
Location Berlin
commented
Status: Needs review » Needs work

You should remove the author statement from the module file (6.x-1.x, 7.x-1.x).

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs work » Needs review

That is surely no application blocker, any other issues you found in your manual review?

yannickoo’s picture

yannickoo
Location Berlin
commented
Status: Needs review » Needs work

Oh, he doesn't use filter_xss when getting the variable. variable_get should be wrapped by filter_xss.

eule’s picture

eule commented
Status: Needs work » Needs review

This vulnerability is mitigated by the fact that an attacker must have a role with the permission "administer google tag manager".

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Issue tags: -PAreview: review bonus

Removing review bonus tag, you have not done any manual review, you just posted the output of an automated review tool. Make sure to read through the source code of the other projects.

And please review applications in the "needs review" state, applications in "needs work" are blocked already and need a maintainer response anyway.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Issue summary: View changes

manual review added

ain’s picture

ain commented

I don't think filter_xss() should be used for variable_get() since the latter just fetches the data from the database. There should be no malicious content stored in the first place even if the content shouldn't be mingled with on storage. The general principle should always be to only store valid data.

ain’s picture

ain commented
Issue summary: View changes

removed non-manual reviews

ain’s picture

ain commented
Status: Needs review » Needs work

Automated review

FILE: /Users/ain/NetBeansProjects/Drupal7/sites/all/modules/gtm/README.txt
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 29 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

Manual review

Not all aforementioned issues have been resolved.

ain’s picture

ain commented
Issue summary: View changes

manual review

eule’s picture

eule commented
Issue summary: View changes

second manual review

eule’s picture

eule commented
Issue summary: View changes

3´te manual review

eule’s picture

eule commented
Status: Needs work » Needs review
Issue tags: +PAreview: review bonus
eule’s picture

eule commented

3 manual reviews done and square eyes

benjy’s picture

benjy commented
Status: Needs review » Reviewed & tested by the community

@eule, please post the links to three projects you've reviewed manually to help the admins see the work you've done.

  • You need a new line at the end of the README
  • You're currently injecting into page_top which seems fine to me. GTM does say it should be in immediately after body but it won't cause any issues where you have.

Overall this module is simple and works exactly as described. The above points should not stop this module moving to contrib.

eule’s picture

eule commented
Issue summary: View changes

ripping out "not done yet"

eule’s picture

eule commented

commited :: reviews on the first entry

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: -PAreview: review bonus

manual review of the 7.x branch:

  1. "'access arguments' => array('administer google tag manager'),": since that page callback allows users to inject arbitrary javascript you need to mark that permission as 'restrict access' => TRUE. Otherwise you are considered to be vulnerable to XSS, see also http://drupal.org/node/28984 . 'restrict access' does not exist in Drupal 6, so you have to make an explicit warning in the README.
  2. Is the snippet that users have to paste Javascript? Then you should use drupal_add_js() and not #markup.

The first point is a security blocker, otherwise looks pretty ready to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

eule’s picture

eule commented

i do not again 3 reviews...moving to wordpress..

eule’s picture

eule commented

other modules like foo table working in the master, and 7.x-1.x and have no readme.txt, comment_goodness a module sponsored by acquia is still unsupported - fixes makes a other guy but dont get in branch - but is live. i do not understud why this is happend. Module development still suxxs and makes no one who will help drupal more happy.

i do maybe a rewrite, but i dont have the time to make another 3 reviews. i have done 6 reviews now. nothing is accepted. i a´m done.

sorry

ain’s picture

ain commented

eule, this is not about this particular project getting accepted and confirmed, it is about the authority to create full projects.

Citing Apply for permission to create full projects:

After contributing a useful and working module or theme as a sandbox project, you may go through a one-time approval process to get permission to promote it (and future projects) to a full project.

Additionally, I find this process one of the best possible places for professional and free training. No developer has all the skills in the world, you can only improve by the experience from the others. Sure the hurdle has been long, but it's well worth it knowledge-wise. I hope you agree.

eule’s picture

eule commented

7.x-1.x i add restrict access => TRUE in gtm.module

benjy’s picture

benjy commented

@eule, I moved from WordPress to Drupal and I can tell you you're almost certainly going in the wrong direction :)

eule’s picture

eule commented

@benjy,@ain ok. i will finaly do the steps ..but for a none programmer it is not easy. it takes some time and i will work on a better security solution for the Google Tag Manager aka GTM. The thing is i don´t have think thats anyone will give permissions to a anon to setup GTM. For now i mean the module is useable. Try, Test and save my Live!

eule’s picture

eule commented

and someone know why on my profil page my project not appear?

benjy’s picture

benjy commented

@eule, I can see my projects here: http://drupal.org/project/user but I don't have any sandbox projects so I can't comment if they should also appear.

You've added restrict access but you still need to change #markup to drupal_add_js. Once that's done set back to RTBC.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

The Git commits are not connected to your user account. You need to specify an email address. See http://drupal.org/node/1022156 and http://drupal.org/node/1051722

PA robot’s picture

PA robot commented
Status: Needs work » Closed (won't fix)

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

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

Vic_’s picture

Vic_ commented
Issue tags: +Status update

Hi guys

I need this for D7 install, so im porting it.
il start with this module

il look into the security part. But for now. Porting it to D7 is the most important thing.

benjy’s picture

benjy commented

There already is a 7.x branch

eule’s picture

eule commented

@Vic_ if u want to rewrite please let me know
security issue is a good point to start, and yes there is a d7 working version of it

i want some for d8

please let me know ..i can give rights to support this module

iamEAP’s picture

iamEAP commented

I've got a custom (though generic) GTM module that has dataLayer integrations that I'd like to deprecate in favor of something hosted on d.o. I'd be more than happy to help/contribute.

benjy’s picture

benjy commented

I've thought about taking over this module also but it does very little other than inject one JS file into the page. Do we want to start by outlining what else this module can offer?

Or is the fact it overs a UI for non-technical users to use GTM enough? If so, as iamEAP mentioned, maybe a generic solution is a better alternative?

iamEAP’s picture

iamEAP commented

In addition to injecting the script in the proper way, the module should, at its base, handle GTM's datalayer in a similar way to how Drupal handles Drupal.settings.

Would need to include:

  • Ability to change datalayer variable name (and reflect it in the injected script snippet),
  • Provide a single datalayer object injected in the head,
  • Easy API for adding datalayer properties (e.g. gtm_add_datalayer() or somesuch)
  • Provide a few core datalayer properties present on all pages, perhaps even some fancier stuff like...
    • Current user info: authentication status? role? id?
    • Entity info: entity type, entity ID, entity bundle (content type)?
    • Language; maybe even just a generic property to indicate the webpage was generated by Drupal, etc.

Like I said, I have a lot of this built out in a custom module that I'd like to incorporate into this module.

Off of that, there might be contrib module that could: provide a Context reaction datalayer push, provide some other UI (maybe with token support) for datalayer stuff.

I think the module should also probably handle GTM macros, but I'm not very familiar with those, currently.

lsolesen’s picture

lsolesen commented

I concur with Plazik in #12 that another sandbox seems a little more powerful with tests included.

rwohleb’s picture

rwohleb
he/him
Primary language English
commented

Both this and the sandbox mentioned in #12 have not been updated in quite a while. I'm keen to start working on the GTM code, so we need to come to a consensus on who is doing what. Don't make me create my own sandbox :P

rp7’s picture

rp7 commented

@57 / iamEAP

This looks like a very good starting point. Why not add your code to this module/or completely replace it?

webbroidrupal’s picture

webbroidrupal commented

Are you guys thinking about getting this project off the ground? It would really be nice to have a Google Task Manager Module. One of my developers could take on some development tasks if someone out there leads.

https://drupal.org/user/2705195

eule’s picture

eule commented

@webbroidrupal hello, glad to hear this. I´m not a coder and sometimes its very hard to get all the stuff running from git - to drupal -and coding standards. drupalrv have me helped a lot in the beginning. so it would be nice if your develop will help here.

webbroidrupal’s picture

webbroidrupal commented

@eule

If you don't mind we are going to spin it up using Pantheon Multdev next week. It makes it easier to test and collaborate on the code as a team in the same place. Certainly save you some time. I might have a few others who can help too.

Who else is working with you on this?

Still the group listed on project Page:

iamEAP
drupalrv
gnuget
benjy

If your curious about the functionality of Pantheon MultiDev check out this link:

https://www.getpantheon.com/multidev

eule’s picture

eule commented

hi @webbroidrupal i work alone on it. i will purge the guys. but i´m sure drupalrv helps too. i just can give you guys permission on it and perhaps credits on the project page. would be nice so i can learn from you ;-)

eule’s picture

eule commented

@webbroidrupal i update the project page ..give me a user/coder...i will setup permission for you guys

benjy’s picture

benjy commented

Hi all, yeah I just don't have time to commit to this and I don't actually use GTM anymore.

@webbroidrupal Multidev looks great but starting at $400 a month puts it out of reach for most of us.

webbroidrupal’s picture

webbroidrupal commented

@benjy

No problem. What are you using for Event Tracking?

MultiDev won't cost you guys a thing. You will use my account, I can invite other developers in for Free.

@benjy,

The developer on this project will be Andrew_Emelianenko

eule’s picture

eule commented

@webbroidrupal he is ready to rock i setup all.

rwohleb’s picture

rwohleb
he/him
Primary language English
commented
Status: Closed (won't fix) » Needs work

Where are we at with this? There is another project application for a GTM module (#2070317: [D7] Google Tag Manager). I added a comment pointing them to this thread. Hopefully all efforts are combined.

Speaking of efforts, if you are doing your work outside of Drupal GIT, please add a link to the respository in this thread and on the original sandbox. People may want to submit patches and if we can't see what's up, there will be overlaps.

I'm more than happy to act as a co-maintainer on this project. This isn't my first rodeo, so I can review everything and get it up to standards and best practices.

FYI, I'm setting this back to Needs Work since there is new activity.

webbroidrupal’s picture

webbroidrupal commented

Robert,

We will be starting this week. We will connect with the other team. Give us a week or so to get going on this, then we can call you in to review and standardize everything.

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented
Title: Google Tag Manager (GTM) » [D6] Google Tag Manager (GTM)

Just fixing title.

eule’s picture

eule commented
Title: [D6] Google Tag Manager (GTM) » Google Tag Manager (GTM)

WTF?? Both Version for D6 and D7 available

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented

Oh, sorry, I was just looking at the initial project application. We only need to evaluate one branch for the application, do you want that to be the D6 or D7 version?

eule’s picture

eule commented

we work with @webbroidrupal soon on it...so ...please still test the stuff.

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented

When you're ready for review, set this issue to "needs review" and someone will look at it. Also, if you could let us know which branch you'd like to have reviewed, that would be great.

Andrew_Emelianenko’s picture

Andrew_Emelianenko commented

Hi guys,

I just refactored the code. You can look at this on github: https://github.com/MrWEST/drupal-gtm
I allowed access for: drupalrv and ekn33. Anyone else will need the access?

Guys do you registered at www.getpantheon.com? We can use Multidev system for developing this module.
If you are registered, please, give me your account names and I will give you access.

If your not registered just create a free account, and give me your email address.

ryan osītis’s picture

ryan osītis commented

Andrew,

Would you add me as well?

rositis@gmail.com

Andrew_Emelianenko’s picture

Andrew_Emelianenko commented

Rositis,

Done!
Also, I added access to repo for your GitHub account: https://github.com/rositis

webbroidrupal’s picture

webbroidrupal commented

Robert,

I think we are ready to get going on this. Thank you for offering to help work as a co-maintainer on this. If you have some time we are going to need some help on PM side too.

https://github.com/MrWEST/drupal-gtm

I will need your email to invite you into the Dev Environment.

webbroidrupal’s picture

webbroidrupal commented
Issue summary: View changes

strong manual reviews

PA robot’s picture

PA robot commented
Issue summary: View changes
Status: Needs work » Closed (won't fix)

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

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

chris burge’s picture

chris burge commented

There is now a Google Tag Manager project: http://drupal.org/project/google_tag