Hi there,

I would like to track my disqus commenting with google analytics, was wondering if by any chance it was on the list of things to add? I assume it could be quite straight forward: perhaps a checkbox to activate it and a text field/ textarea with the script command you want to trigger upon commenting?
In the meantime, which part can I include the required line at so that it works?
The line to include is:

function disqus_config() {
      this.callbacks.onNewComment = [function() { /* call tracking command here */ }];
  }

as specified on http://docs.disqus.com/help/60/

Thanks!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

telegraph’s picture

Category: feature » support
awolfey’s picture

I think the question here is also how to use drupal_add_js() to add the callback.

 if (module_exists('disqus')) {
    drupal_add_js(array('disqus' => array('callbacks' => array('onNewComment' => "function(){ _gas.push(['_trackEvent','Blog','New Comment',window.location.href]); }"))), 'setting');
  }

something like that should work, right?

girishmuraly’s picture

Version: 6.x-1.9 » 7.x-1.x-dev
Component: User interface » Code
Category: Support request » Feature request
Issue summary: View changes
Status: Active » Needs review
FileSize
2.92 KB

I have the same request and I think the best approach would be to implement the callback in a #post_render to the 'disqus' element. I've attached a patch that does this in a module called disqus_ga which I'm hoping can be a submodule to Disqus.

Other modules that want to do something different on the callback can either override the js callback using hook_js_alter() or follow the logic of the module to add their own callback.

slashrsm’s picture

Your patch looks like a whole new module. Did you want to include it in Disqus integration or create a standalone plugin?

girishmuraly’s picture

I thought it would benefit everybody if it was a submodule of Disqus. What do you think?

slashrsm’s picture

That would work, but we need a patch against module for that also.

girishmuraly’s picture

I thought this patch would work. Works for me. Is there something I need to follow? Could you point me in the right direction please? Thank you.

hugronaphor’s picture

Here is the patch which is going to integrate the GA Tracking without creating a sub-mudule as in my opinion having a separate module for that is superfluous.
Things to mention:
- This patch will support the both, the old GA api and the new analytics.js and Universal Analytics (see: https://developers.google.com/analytics/devguides/collection/upgrade/ref... ).
Inspired from http://cgit.drupalcode.org/google_analytics_et/tree/js/google_analytics_... ;
- As we have already 3 .js files it's worth to move theme in a separate directory "js";

Please review.

slashrsm’s picture

Version: 7.x-1.x-dev » 8.x-1.x-dev
Status: Needs review » Needs work

Thank you for your work. Looks very good. I'd suggest just few simple cosmetic improvements:

  1. +++ b/disqus.admin.inc
    @@ -97,6 +97,12 @@ function disqus_admin_settings() {
    +    '#default_value' => variable_get('disqus_track_newcomment_ga', FALSE),
    +  );
    

    Let's add this new configuration variable to the hook_uninstall().

  2. +++ b/js/disqus_ga.js
    @@ -0,0 +1,36 @@
    +  };
    +
    +})(jQuery);
    \ No newline at end of file
     
    

    Missing newline.

  3. It would be great to mention this new feature in README.txt too

It would be great to get this into 8.x-1.x first.

hugronaphor’s picture

FileSize
72.14 KB

Thanks for the improvements suggestion.
Attaching the improved patch for 7.x-1.x (I also changed the callback name to make it more descriptive).
In the main time I'll work on 8.x-1.x branch.

hugronaphor’s picture

FileSize
14.83 KB

I saw that variable deletion is sorted alphabetically - pushing new variable in the corespondent place :)

hugronaphor’s picture

Here is the patch for 8.x-1.x

hugronaphor’s picture

Status: Needs work » Needs review
slashrsm’s picture

Status: Needs review » Needs work

JS file seems to be missing in D8 patch.

slashrsm’s picture

@hugronaphor: Check https://www.drupal.org/documentation/git/interdiff. Interdiffs are great way to clearly show changes between individual patches and make it much easier for people to review your work.

hugronaphor’s picture

Status: Needs work » Needs review
FileSize
5.41 KB
1.11 KB

Here is the fixed patch with js file included and interdiff.
Thanks for pointing me into the right direction.

slashrsm’s picture

Status: Needs review » Needs work

Thanks! This looks great. I missed another critical and another cosmetic thing during my last review. I apologize for ping-pong....

+++ b/src/Element/Disqus.php
@@ -60,6 +60,14 @@ class Disqus extends RenderElement {
+    if (\Drupal::config('disqus.settings')->get('behavior.disqus_track_newcomment_ga')) {
+      // Add a callback when a new comment is posted.
+      $disqus['callbacks']['onNewComment'][] = 'disqusTrackNewComment';
+      // Attach the js with the callback implementation.
+      $element['#attached']['library'][] = 'disqus/disqus_ga';
+    }

Library name is actually "disqus/disqus". However, since GA integraton can be disabled it would be nicer to separate it's JS into a separate library (which could be called "disqus/ga" eventually).

See https://www.drupal.org/node/2201089 for more info about libraries and their definition.

+++ b/js/disqus_ga.js
@@ -0,0 +1,38 @@
+disqusTrackNewComment = function() {
+

Would be nice to have this function namespaced in "Drupal.disqus". This applies to D7 too.

hugronaphor’s picture

Before uploading the patch, would this be the correct way? :
disqus.libraries.yml

disqus:
  version: 1.x
  js:
    js/disqus.js: {}
    js/disqus_ga.js: {}
    js/disqus.settings.js: {}
  dependencies:
    - core/jquery
    - core/jquery.once
    - core/drupal
ga:
  version: 1.x
  js:
    js/disqus_ga.js: {}
  dependencies:
    - disqus/disqus

And I'm not sure I understand the meaning of "Drupal.disqus" namespace.
Do you mean as Drupal behavior maybe:

Drupal.behaviors.disqusTrackNewComment = {
  attach: function(context, settings) {
    // ...
  }
};

?

slashrsm’s picture

  1. That looks ok.
  2. No, I don't mean behaviours. They have a very specific meaning. I mean defining function inside Drupal.disqus subdocument instead on a global level. It is a good practice and part of Drupal's JS coding standards too. Check how it is done in Entity browser (http://cgit.drupalcode.org/entity_browser/tree/js/entity_browser.iframe....). You can also check appropriate part of coding standards doc (https://www.drupal.org/node/172169#functions).
hugronaphor’s picture

Status: Needs work » Active

This Issue can't be solved until the https://www.drupal.org/node/2476065 is not merged into 8.x-1.x

slashrsm’s picture

Status: Active » Needs work

#2476065 was merged.

hugronaphor’s picture

Title: integration with google analytics or other analytics » Integration with google analytics or other analytics
Status: Needs work » Needs review
FileSize
5.67 KB
1.96 KB

Thanks @slashrsm
Here is the latest patch against 8.x-1.x

  • slashrsm committed 70f76ac on 8.x-1.x authored by hugronaphor
    Issue #1507380 by hugronaphor: Integration with google analytics or...
slashrsm’s picture

Version: 8.x-1.x-dev » 7.x-1.x-dev
Status: Needs review » Patch (to be ported)

Committed. Thank you!

Now we can include this in D7 version too.

hugronaphor’s picture

Status: Patch (to be ported) » Needs review
FileSize
14.93 KB

Here is the last path against the 7.x-1.x

Even if you suggested to namespace the function disqusTrackNewComment in "Drupal.disqus" namespace - I found it a bit difficult to manage the order of added JS files as we did in D8 branch (by using the libraries "dependenctcies" property) and this may create problems in future.

slashrsm’s picture

There are no other JS functions using this namespace at the moment so this would not be a problem, right?

hugronaphor’s picture

Correct, there are no other JS functions using this namespace.
I'm concerned about where to declare the Drupal.disqus = {}; to make sure this will be available in any other part of APP. But you never can be sure without checking if the object exist already (this what I'm trying to avoid).

slashrsm’s picture

For now we could declare it in this file and try to fix this problem when we face it.

hugronaphor’s picture

FileSize
3.12 KB
14.83 KB

I see what you mean. Here it is.

slashrsm’s picture

Status: Needs review » Fixed

Committed. Thanks!

  • slashrsm committed 570d5ab on 7.x-1.x authored by hugronaphor
    Issue #1507380 by hugronaphor: Integration with google analytics or...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.