Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Google Analytics: TrackAdmin, AnonymizeIp, and TrackUserID Support #2785

Merged
merged 5 commits into from
May 15, 2019

Conversation

ohine
Copy link
Contributor

@ohine ohine commented May 14, 2019

Summary

This PR fixes #2782 and also implements AnonymizeIp and TrackUserID settings in Google Analytics.

@@ -105,7 +106,7 @@ define(["jquery", "knockout", "templatePath/scripts/config", "templatePath/scrip

// Set the isDeactivating flag to true to override the default save behaviour
// Temporary workaround until delete functionality on connectors is improved
conn.configurations[3].value("true");
conn.configurations[5].value("true");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way to get away from mysterious numbers defining these values?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, It's something I'd also like to fix, but I didn't have time this morning to look into the configuration property in the connector engine. I'm looking at it now to see if we can remove those and replace them with the name of the setting.

DNN Platform/Connectors/GoogleAnalytics/connector.htm Outdated Show resolved Hide resolved

if (trackingUserId)
{
sb.AppendFormat("ga('set', 'userId', {0});", UserController.Instance.GetCurrentUserInfo().UserID);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this check if they're logged in? or do we want to track -1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I grabbed that code from my old GA extension, and it was running for all users in that extension.

However, I can see pros/cons for either way. I'm good with -1 for guests or adding a check IsAuthenticated prior to setting the UserId var in GA. Whichever sounds best to the @dnnsoftware/approvers i'll make happen with another commit.

@bdukes bdukes merged commit 7676fb8 into dnnsoftware:development May 15, 2019
@ohine ohine added this to the 9.4.0 milestone May 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Google Analytics Connector - Track Admins Always True
3 participants