Since all user-defined roles are authenticated user, ticking the "authenticated user" in the configuration means all roles are tracked. Should they be ANDed such that one it tracked only if all his roles are ticked?

Comments

neopoet’s picture

What is the status of this?

Perhaps we should offer the opposite- track everyone EXCEPT the specified roles. In most cases, admins want to track everyone but themselves (and any test users they've created). Doesn't this make more sense?

budda’s picture

I'm open to patches. I don't have the time to implement and test the proposed changes at present.

brmassa’s picture

Title: role-based tracking should be ANDed instead of ORed » roles that are NOT tracked
Assigned: Unassigned » brmassa
Status: Active » Needs review
StatusFileSize
new3.65 KB

Guys,

there is a patch agains DRUPAL-5 branch.
1* users chose roles that are NOT tracked
2* create a new "role": user 1 for admins

it fix the installation process, form fields and (invert) all variables.

regards,

massa

PS: all variables are now based on role id instead role name. its because a role called "role 1" would conflict with "role_1". role id is unique and incremental, so its safer.

budda’s picture

Has anybody else been able to test this and confirm it upgraded and worked as planned? I've not got a live 5.x site to test on still.

brmassa’s picture

Status: Needs review » Closed (duplicate)

Guys,

i did a series of tests and its working fine. i just some modifications and the final patch is on issue http://drupal.org/node/142514.

follow there all updates.

best regards,

massa

aclight’s picture

Title: roles that are NOT tracked » Allow certain roles to NOT be tracked
Status: Closed (duplicate) » Needs review

Subscribing and changing status back to cnr. I agree with JohnAlbin's comments at http://drupal.org/node/142514 about not combining several issues into one patch. I would like the ability to not track my administrative roles but have no interest in nor the background to review the patch there which deals with adding integration functionality to the ECommerce module.

budda’s picture

I would like the ability to not track my administrative roles

Surely that means you just untick the role from the tracking list.....?

aclight’s picture

Unfortunately it's not that simple.

I want to track ALL users except users that have the "site administrator" role that I have created.

So I have checked "anonymous users" and "authenticated users" to be tracked, but since all users with the "site administrator" role also have the "authenticated users" role, they are tracked as well.

But maybe I'm missing something simple here?

denney’s picture

I've tested the code in #3 and it appears to work fine.

+1 for this to get committed.

budda’s picture

How are you applying this patch? It's failing for me against $Id: googleanalytics.module,v 1.14.2.6

$ patch -p0 -u < googleanalytics_2.patch

patching file googleanalytics.install
Hunk #1 FAILED at 5.
1 out of 1 hunk FAILED -- saving rejects to file googleanalytics.install.rej
patching file googleanalytics.module
Hunk #1 FAILED at 43.
Hunk #2 FAILED at 113.
2 out of 2 hunks FAILED -- saving rejects to file googleanalytics.module.rej
aclight’s picture

I was able to apply the patch cleanly (with offset) to the 5.x-1.x-dev version.

I used:

# patch -p0 < googleanalytics_2.patch
patching file googleanalytics.install
patching file googleanalytics.module
Hunk #2 succeeded at 117 (offset 4 lines).
aclight’s picture

StatusFileSize
new3.64 KB

I cleaned up the patch (slightly) by making the following changes:
1. googleanalytics_update_1() did not return an array, which resulted in an array_merge error upon running update.php.
2. These lines below did not follow Drupal coding conventions so I fixed those

+    if (variable_get("googleanalytics_track_". $role, FALSE)) {$track = FALSE;}
   }
+  if ($user->uid == 1 and variable_get("googleanalytics_track__user1", FALSE)) {$track = FALSE;}

I also rerolled the patch against the most recent dev version.

Otherwise, I tested this on a Drupal 5.2 installation and it works great. I also tested that the analytics .js tracker wasn't being added to roles that I set up to be excluded, and that worked as expected.

budda’s picture

Status: Needs review » Fixed

Thanks for taking the time to redo the patch - was in a bit of a weird format and still didn't work. However, had a spare 15 mins this evening so picked through the patch code manually and applied the changes.

hass’s picture

Status: Fixed » Needs work

You should never use double quotes if not required... use singe quotes only and use double quotes if you need a "\n" or you have single quotes in a translateable string for e.g. t("foo'd.").

Drupal coding standards... :-)

hass’s picture

Status: Needs work » Fixed

i fixed this in the issue http://drupal.org/node/169153

Anonymous’s picture

Status: Fixed » Closed (fixed)
nicholas.alipaz’s picture

Version: 5.x-1.x-dev » 6.x-2.x-dev
Status: Closed (fixed) » Active

This issue has NOT been fixed. I am not really sure what hass was referring to when he closed it two years ago.

The Issue

To show code for users based on role, one must select the roles that they want to have the code shown to. This causes a few issues:

  • Showing the code to "authenticated users" means that all users get the code, even if they are in a role that is not checked. Example: check "authenticated" but uncheck "your custom role" and you will find that "your custom role" still gets the code.
  • If one does decide to use this feature then when adding new roles they would have to remember to update this setting so that the new role will have the code

Solution

Invert setting to be "opt-out." Meaning that checking roles would remove the code for the role. Please see http://drupal.org/project/live_person module for an example. I wrote that module based on google analytics module and I reversed this option for these very reasons.

Thanks for reading.

hass’s picture

Version: 6.x-2.x-dev » 5.x-1.x-dev
Status: Active » Closed (fixed)
nicholas.alipaz’s picture

OK, well thanks for the link even though I still think that this might need to be part of the module rather than as a separate script. I am sorry if you might have found it offensive with your rather short "RTFM" it felt as though you might. I didn't mean to imply you don't know what you are doing or anything, just to say that I wasn't sure what the link was referring to when saying it fixed the issue.

hass’s picture

I still think that this might need to be part of the module rather than as a separate script

The module implements the Block visibility logic from core. Until core changes I'm not changing the logic as it was MUCH MUCH MUCH MORE confusing in past than today. Today it simply works as expected.