-
Notifications
You must be signed in to change notification settings - Fork 34
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Whelp, that'd be bad. intval()ing our user ID, which is an email addr…
…ess, ain't gonna fly.
- Loading branch information
1 parent
de45c5c
commit 80d750a
Showing
2 changed files
with
2 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
<script type="text/javascript"> | ||
analytics.identify( <?php echo '"' . intval( $user_id ) . '"' ?><?php if ( ! empty( $traits ) ) { echo ', ' . json_encode( Segment_Analytics_WordPress::esc_js_deep( $traits ) ); } else { echo ', {}'; } ?><?php if ( ! empty( $options ) ) { echo ', ' . json_encode( Segment_Analytics_WordPress::esc_js_deep( $options ) ); } ?>); | ||
analytics.identify( <?php echo '"' . esc_js( $user_id ) . '"' ?><?php if ( ! empty( $traits ) ) { echo ', ' . json_encode( Segment_Analytics_WordPress::esc_js_deep( $traits ) ); } else { echo ', {}'; } ?><?php if ( ! empty( $options ) ) { echo ', ' . json_encode( Segment_Analytics_WordPress::esc_js_deep( $options ) ); } ?>); | ||
</script> |
80d750a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JustinSainton is the user_id always an email? GA user-id can't be personally identifiable (https://cloudup.com/cWuXRNeAeXO) so it would be better if it were the db id to take advantage of the user id views. Make sense? cc/ @ianstormtaylor
80d750a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh oh! That's no good. I'll touch base with @ianstormtaylor and see if there's a way we can handle this in a backwards compatible manner.
80d750a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yup, happy to help figure out what we need here. Related to this thread: #9
I think our default going forward should be to
identify
by the database ID, but then have a setting (that defaults tofalse
for new installs) for "Identify by email address." that they can toggle on in the WordPress admin. Does that make sense Justin?