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

[Analytics] Implement tracking for user profile changes #11176

Merged
merged 4 commits into from
Jul 8, 2022

Conversation

jakobhero
Copy link
Member

@jakobhero jakobhero commented Jul 6, 2022

Description

Implements tracking for user-reported user profile changes implemented in #11023 by:

  • firing a profile_changed event with the old and new profile properties whenever the users submit a change to their profile
  • firing an identify call with the traits { name, company, email} whenever users submit a change to their profile

Note that this change could potentially overwrite emails in tools such as customer.io with an unverified email address reported by the user.

How to test

  1. Open a preview environment, sign up, and then visit the profile section. In the account tab, make changes to the vales in the fields Name, Email, and/or Company, then click Update Profile
  2. Go to the debugger of Staging Untrusted and ensure that a profile_changed event that details the changes and an identify event that contains the respective user attributes are sent
  3. Repeat 1 and 2 with other arbitrary combinations of the Name, Email, and Company fields to ensure stability

Release Notes

None

Documentation

Werft options:

  • /werft with-preview
  • /werft analytics=segment|TEZnsG4QbLSxLfHfNieLYGF4cDwyFWoe

@jakobhero
Copy link
Member Author

jakobhero commented Jul 6, 2022

/werft run

👍 started the job as gitpod-build-jh-analytics-profile-changes.1
(with .werft/ from main)

@jakobhero jakobhero force-pushed the jh/analytics-profile-changes branch from 30364c0 to 7f63454 Compare July 6, 2022 13:37
@jakobhero jakobhero marked this pull request as ready for review July 6, 2022 13:41
@jakobhero jakobhero requested a review from a team July 6, 2022 13:41
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Jul 6, 2022
@jakobhero
Copy link
Member Author

@chrifro i added you as reviewer because of

Note that this change could potentially overwrite emails in tools such as customer.io with an unverified email address reported by the user.

@jakobhero jakobhero requested a review from chrifro July 6, 2022 13:42
@geropl geropl self-assigned this Jul 6, 2022
const newProfile = { name: partialUser.fullName, ...partialUser.additionalData?.profile };
if (newProfile) {
if (
!oldProfile ||
Copy link
Member

Choose a reason for hiding this comment

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

Note: We already have similar functionality here. Actually, this whole section should be going into protocol.ts, so it can be used from dashboard and server alike.

@jakobhero Do you want to do that? Or should I add a commit as suggestion on top?

Copy link
Member

Choose a reason for hiding this comment

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

Here is a commit if you want to 🍒-pick: b230334

Copy link
Member Author

Choose a reason for hiding this comment

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

this whole section should be going into protocol.ts, so it can be used from dashboard and server alike.

makes sense!👍🏼

Here is a commit if you want to 🍒-pick: b230334

thanks for the 🍒!🙏🏼 i used hasChanges to check before sending events, looks much cleaner now!

@chrifro
Copy link

chrifro commented Jul 7, 2022

Note that this change could potentially overwrite emails in tools such as customer.io with an unverified email address reported by the user.

I guess we can only test this once the PR is merged?

I'm not sure what you mean by "unverified email address reported by the user."

Expected behavior: the information in customer.io is in sync with the dashboard. Once any information is changed in the dashboard, the information will be overwritten in customer.io as well.

@roboquat roboquat added size/L and removed size/S labels Jul 7, 2022
@jakobhero
Copy link
Member Author

I guess we can only test this once the PR is merged?

no, apologies for not being clearer. with this change, emails in customer.io will be overwritten by new email addresses that might not be real. when users are changing their emails in the profile, we are not taking any steps to verify that they're real nor have users confirm their new email address if they're changed in the profile section, which is a step down from using the trusted emails we get through the git authenticators (i assume they have to be verified for all of them before an account is created).

Expected behavior: the information in customer.io is in sync with the dashboard. Once any information is changed in the dashboard, the information will be overwritten in customer.io as well.

this is what's going to happen if this is merged.

@jakobhero jakobhero requested a review from geropl July 7, 2022 17:15
Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

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

🎉 Nice, looks good!

@roboquat roboquat merged commit cfe358b into main Jul 8, 2022
@roboquat roboquat deleted the jh/analytics-profile-changes branch July 8, 2022 07:23
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/L team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants