kbase / kbase-ui

kbase-ui components
MIT License
10 stars 22 forks source link

UIP-35 Update to use anonymous user ids for Google Analytics call #1658

Closed briehl closed 11 months ago

briehl commented 1 year ago

Pull Request

Description

This updates the call to set up Google Analytics to use the shiny new anonid key provided by the /me endpoint of the auth service. This turns a KBase user id, which Google might perceive as PII, into an anonymous string unique to each user.

Note that this sorta depends on Auth2 v0.6.0 being released in production, otherwise null gets sent. Which isn't a terrible thing, but not the desired outcome.

Issues Resolved

Resolves https://kbase-jira.atlassian.net/browse/UIP-35

Provide details for testing status and how to test the PR:

Dev Checklist

Release Notes

eapearson commented 1 year ago

@briehl Gotcha. I'll get this merged in and then prepare a release so it is in the pipeline. Probably not today, but I'll try to get to it early next week. Are there other usages people are planning for the anonid? I think most usages where we are simply associating some data with a user would be better with the anonid ... Though usernames, just like text dates compared to integer dates, are kinder to the humans looking at the data. And, of course if we used anonids from the beginning, users could change their usernames if need be.

briehl commented 1 year ago

Thanks!

Are there other usages people are planning for the anonid?

Yeah, we should probably use this for any view that sends to GA. There are PRs in place for narrative right now, and static narrative assumes the user is logged out and doesn't send that.

Though usernames, just like text dates compared to integer dates, are kinder to the humans looking at the data.

Very true. There's an admin command in the auth service that'll do the translation back from anon id -> user name, put in for just that purpose!

MrCreosote commented 1 year ago

And, of course if we used anonids from the beginning, users could change their usernames if need be.

We floated that idea but the group decided not to do that: https://docs.google.com/document/d/1MXVu_2Vz2iF2EoFEmD8fxt6xVzrUI4o1Yby0lmN03Ug/edit#heading=h.u4b7skyzl5lj

IIRC the thinking was that the users already had a mutable name, their display name. They don't need two, and we preferred a user name that could be mnemonic.

briehl commented 12 months ago

@eapearson any chance of getting this merged and tested soon?

briehl commented 11 months ago

Closing this, already committed to the main3 branch.