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

Track sign up details #24722

Merged
merged 5 commits into from Sep 17, 2018
Merged

Track sign up details #24722

merged 5 commits into from Sep 17, 2018

Conversation

ewjordan
Copy link
Contributor

@ewjordan ewjordan commented Sep 11, 2018

Logs the following events into Firehose/Redshift:

load-sign-up-page (when user loads sign up page)
select-teacher (when user selects teacher user type)
select-student (when user selects student user type)
email-sign-up-success (when user successfully signs up with email / password account)
google-sign-up-success (when user successfully signs up through Google account)
microsoft-sign-up-success (when user successfully signs up through Microsoft account)
facebook-sign-up-success (when user successfully signs up through Facebook account)
clever-sign-up-success (when user successfully signs up through Clever account)
powerschool-sign-up-success (when user successfully signs up through PowerSchool account)
other-sign-up-success (when user successfully signs up through some other means we’re not tracking, e.g. some other third party provider used internationally)
email-sign-up-error (when user hits an error after trying to sign up through email / password account)
google-sign-up-error (when user hits an error after trying to sign up through Google account)
microsoft-sign-up-error (when user hits an error after trying to sign up through Microsoft account)
facebook-sign-up-error (when user hits an error after trying to sign up through Facebook account)
clever-sign-up-error (when user hits an error after trying to sign up through Clever account)
powerschool-sign-up-error (when user hits an error after trying to sign up through PowerSchool account)
other-sign-up-error (when user hits an error after trying to sign up through some other means we’re not tracking)
google-sign-in (when user signs in through Google from the sign up page)
microsoft-sign-in (when user signs in through Microsoft from the sign up page)
facebook-sign-in (when user signs in through Facebook from the sign up page)

unless session[:sign_up_tracking_expiration]&.future?
session[:sign_up_uid] = SecureRandom.uuid.to_s
session[:sign_up_tracking_expiration] = 5.minutes.from_now
FirehoseClient.instance.put_record(
Copy link
Contributor

Choose a reason for hiding this comment

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

Tracking question: We're only putting the 'load-sign-up-page' event if we didn't already start tracking. Are we intentionally squelching the case where a user hits the signup page then reloads it within the 5-minute window? I'm wondering if this put_record should be outside the if-statement, or in a separate if-statement.

# our tracking data is usually populated, so do it here
unless session[:sign_up_tracking_expiration]&.future?
session[:sign_up_uid] = SecureRandom.uuid.to_s
session[:sign_up_tracking_expiration] = 5.minutes.from_now
Copy link
Contributor

Choose a reason for hiding this comment

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

This two- or three-step session setup happens a couple of times in this PR - might be a good candidate for extraction to something like SignUpTracking.begin_sign_up_tracking(session).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

data_string: session[:sign_up_uid],
data_json: {
detail: resource.to_json,
errors: resource.errors&.messages
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 be full_messages instead of mesages? not sure that it matters, just changes the formatting of the error messages

Copy link
Contributor

@islemaster islemaster left a comment

Choose a reason for hiding this comment

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

🎉

@ewjordan ewjordan merged commit b65f5be into staging Sep 17, 2018
@ewjordan ewjordan deleted the sign-up-tracking branch September 17, 2018 18:23
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.

None yet

3 participants