-
Notifications
You must be signed in to change notification settings - Fork 72
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
Rename user_id
--> ga_client_id
#2356
Rename user_id
--> ga_client_id
#2356
Conversation
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.
this is great thank you
@@ -767,7 +767,7 @@ class ProvidedIdentityType(EnumType): | |||
|
|||
email = "email" | |||
phone_number = "phone_number" | |||
user_id = "user_id" | |||
ga_client_id = "ga_client_id" |
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.
one more question, are we sure that it is the client_id
and not the app_instance_id
?
My options in the API request are:
Type of user. One of APP_INSTANCE_ID, CLIENT_ID or USER_ID.
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.
oh! I know that the cookie only has the client_id
. I'm guessing the app_instance_id
might be the GA value customers have to put in their javascript, but let me take a look at the docs and get back to you.
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.
okay great 👍
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.
okay yes, looking at this, https://developers.google.com/analytics/devguides/config/userdeletion/v3, it should be client_id
we want here
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.
excellent
Patches up #2304 after some more conversation with @pattisdr
Code Changes
user_id
toga_client_id
. Since this was merged to a feature branch, should be safe to make these changes directly instead of making another alembic migrationSteps to Confirm
Pre-Merge Checklist
CHANGELOG.md
Description Of Changes
Will also need to update #2337