Merge pull request #24722 from code-dot-org/sign-up-tracking

Track sign up details

apps/src/sites/studio/pages/signup.js

@@ -6,6 +6,7 @@ import CountryAutocompleteDropdown from '@cdo/apps/templates/CountryAutocomplete
 import { COUNTRIES } from '@cdo/apps/geographyConstants';
 import SchoolNotFound from '@cdo/apps/templates/SchoolNotFound';
 import i18n from "@cdo/locale";
+import firehoseClient from "../../../lib/util/firehose";
 
 const SCHOOL_TYPES_HAVING_NCES_SEARCH = ['charter', 'private', 'public'];
 
@@ -139,13 +140,24 @@ window.SignupManager = function (options) {
 
   $("#user_user_type").change(function () {
     var value = $(this).val();
+    trackUserTypeSelected(value);
     if (value === "student") {
       showStudent();
     } else if (value === "teacher") {
       showTeacher();
     }
   });
 
+  function trackUserTypeSelected(type) {
+    const eventName = "select-" + type;
+    firehoseClient.putRecord({
+      study: 'account-sign-up',
+      study_group: 'control',
+      event: eventName,
+      data_string: self.options.signUpUID,
+    });
+  }
+
   function setSchoolInfoVisibility(state) {
     if (state) {
       $("#schooldropdown-block").fadeIn();

dashboard/app/controllers/application_controller.rb

@@ -24,6 +24,8 @@ class ApplicationController < ActionController::Base
 
   before_action :fix_crawlers_with_bad_accept_headers
 
+  before_action :clear_sign_up_session_vars
+
   def fix_crawlers_with_bad_accept_headers
     # append text/html as an acceptable response type for Edmodo and weebly-agent's malformed HTTP_ACCEPT header.
     if request.formats.include?("image/*") &&
@@ -271,4 +273,12 @@ def pairing_user_ids
     # TODO(asher): Determine whether we need to guard against it being nil.
     session[:pairings].nil? ? [] : session[:pairings]
   end
+
+  def clear_sign_up_session_vars
+    if session[:sign_up_uid] || session[:sign_up_type] || session[:sign_up_tracking_expiration]
+      session.delete(:sign_up_uid)
+      session.delete(:sign_up_type)
+      session.delete(:sign_up_tracking_expiration)
+    end
+  end
 end

dashboard/app/controllers/home_controller.rb

@@ -8,6 +8,10 @@ class HomeController < ApplicationController
   # clicking on a link.
   skip_before_action :verify_authenticity_token, only: 'set_locale'
 
+  # The terms_and_privacy page gets loaded in an iframe on the signup page, so skip
+  # clearing the sign up tracking variables
+  skip_before_action :clear_sign_up_session_vars, only: [:terms_and_privacy]
+
   def set_locale
     set_locale_cookie(params[:locale]) if params[:locale]
     if params[:i18npath]

dashboard/app/controllers/omniauth_callbacks_controller.rb

@@ -4,6 +4,8 @@
 class OmniauthCallbacksController < Devise::OmniauthCallbacksController
   include UsersHelper
 
+  skip_before_action :clear_sign_up_session_vars
+
   # GET /users/auth/:provider/callback
   def all
     if should_connect_provider?
@@ -59,6 +61,7 @@ def login
     auth_hash = request.env['omniauth.auth']
     auth_params = request.env['omniauth.params']
     provider = auth_hash.provider.to_s
+    session[:sign_up_type] = provider
 
     # Fiddle with data if it's a Powerschool request (other OpenID 2.0 providers might need similar treatment if we add any)
     if provider == 'powerschool'
@@ -70,7 +73,7 @@ def login
       auth_hash = extract_microsoft_data(request.env["omniauth.auth"])
     end
 
-    @user = User.from_omniauth(auth_hash, auth_params)
+    @user = User.from_omniauth(auth_hash, auth_params, session)
 
     # Set user-account locale only if no cookie is already set.
     if @user.locale &&
@@ -117,6 +120,11 @@ def self.get_cache_key(oauth_param, user)
   def register_new_user(user)
     move_oauth_params_to_cache(user)
     session["devise.user_attributes"] = user.attributes
+
+    # For some providers, signups can happen without ever having hit the sign_up page, where
+    # our tracking data is usually populated, so do it here
+    SignUpTracking.begin_sign_up_tracking(session)
+
     redirect_to new_user_registration_url
   end
 
@@ -281,6 +289,10 @@ def silent_takeover(oauth_user, auth_hash)
 
   def sign_in_user
     flash.notice = I18n.t('auth.signed_in')
+
+    # Will only log if the sign_up page session cookie is set, so this is safe to call in all cases
+    SignUpTracking.log_sign_in(resource, session, request)
+
     sign_in_and_redirect @user
   end
 

dashboard/app/controllers/registrations_controller.rb

@@ -7,10 +7,19 @@ class RegistrationsController < Devise::RegistrationsController
     :migrate_to_multi_auth, :demigrate_from_multi_auth
   ]
   skip_before_action :verify_authenticity_token, only: [:set_age]
+  skip_before_action :clear_sign_up_session_vars, only: [:new, :create]
 
   def new
     session[:user_return_to] ||= params[:user_return_to]
     @already_hoc_registered = params[:already_hoc_registered]
+
+    SignUpTracking.begin_sign_up_tracking(session)
+    FirehoseClient.instance.put_record(
+      study: 'account-sign-up',
+      event: 'load-sign-up-page',
+      data_string: session[:sign_up_uid]
+    )
+
     super
   end
 
@@ -51,6 +60,22 @@ def create
       storage_id = take_storage_id_ownership_from_cookie(current_user.id)
       current_user.generate_progress_from_storage_id(storage_id) if storage_id
     end
+
+    sign_up_type = session[:sign_up_type]
+    sign_up_type ||= resource.email ? 'email' : 'other'
+    if session[:sign_up_tracking_expiration]&.future?
+      result = resource.persisted? ? 'success' : 'error'
+      tracking_data = {
+        study: 'account-sign-up',
+        event: "#{sign_up_type}-sign-up-#{result}",
+        data_string: session[:sign_up_uid],
+        data_json: {
+          detail: resource.to_json,
+          errors: resource.errors&.full_messages
+        }.to_json
+      }
+      FirehoseClient.instance.put_record(tracking_data)
+    end
   end
 
   #

dashboard/app/models/user.rb

@@ -75,6 +75,7 @@
 require 'cdo/race_interstitial_helper'
 require 'cdo/shared_cache'
 require 'school_info_interstitial_helper'
+require 'sign_up_tracking'
 
 class User < ActiveRecord::Base
   include SerializedProperties
@@ -707,10 +708,14 @@ def self.name_from_omniauth(raw_name)
   end
 
   CLEVER_ADMIN_USER_TYPES = ['district_admin', 'school_admin'].freeze
-  def self.from_omniauth(auth, params)
+  def self.from_omniauth(auth, params, session = nil)
     omniauth_user = find_by_credential(type: auth.provider, id: auth.uid)
-    omniauth_user ||= create do |user|
-      initialize_new_oauth_user(user, auth, params)
+
+    unless omniauth_user
+      omniauth_user = create do |user|
+        initialize_new_oauth_user(user, auth, params)
+      end
+      SignUpTracking.log_sign_up_result(omniauth_user, session)
     end
 
     omniauth_user.update_oauth_credential_tokens(auth)

dashboard/app/views/devise/registrations/new.html.haml

@@ -182,6 +182,9 @@
   $(document).ready(function() {
     // Send through some values that the JavaScript will need.
     window.signupManager = new SignupManager({
+      signUpType: "#{session[:sign_up_type]}",
+      signUpUID: "#{session[:sign_up_uid]}",
+      signUpTrackingExpiration: "#{session[:sign_up_tracking_expiration]}",
       isTeacher: "#{resource.teacher?}",
       returnToUrl: "#{user_return_to_url}",
       teacherDashboardUrl: "#{teacher_dashboard_url}",

dashboard/lib/sign_up_tracking.rb

@@ -0,0 +1,42 @@
+require 'cdo/firehose'
+
+module SignUpTracking
+  def self.begin_sign_up_tracking(session)
+    unless session[:sign_up_tracking_expiration]&.future?
+      session[:sign_up_uid] = SecureRandom.uuid.to_s
+      session[:sign_up_tracking_expiration] = 5.minutes.from_now
+    end
+  end
+
+  def self.log_sign_in(user, session, request)
+    return unless user && session && request
+    provider = request.env['omniauth.auth'].provider.to_s
+    if session[:sign_up_tracking_expiration]&.future?
+      tracking_data = {
+        study: 'account-sign-up',
+        event: "#{provider}-sign-in",
+        data_string: session[:sign_up_uid]
+      }
+      FirehoseClient.instance.put_record(tracking_data)
+    end
+  end
+
+  def self.log_sign_up_result(user, session)
+    return unless user && session
+    sign_up_type = session[:sign_up_type]
+    sign_up_type ||= user.email ? 'email' : 'other'
+    if session[:sign_up_tracking_expiration]&.future?
+      result = user.persisted? ? 'success' : 'error'
+      tracking_data = {
+        study: 'account-sign-up',
+        event: "#{sign_up_type}-sign-up-#{result}",
+        data_string: session[:sign_up_uid],
+        data_json: {
+          detail: user.to_json,
+          errors: user.errors&.messages
+        }.to_json
+      }
+      FirehoseClient.instance.put_record(tracking_data)
+    end
+  end
+end

dashboard/test/controllers/registrations_controller/new_account_tracking_test.rb

@@ -0,0 +1,158 @@
+require 'test_helper'
+
+module RegistrationsControllerTests
+  class NewAccountTrackingTest < ActionDispatch::IntegrationTest
+    EMAIL = 'upgraded@code.org'
+    PASSWORD = '1234567'
+    DEFAULT_UID = '1111' # sso uid
+    UUID = 'abcdefg1234567' # tracking uuid
+    STUDY = 'account-sign-up'
+
+    USER_PARAMS_GOOD = {
+      name: 'A name',
+      password: PASSWORD,
+      password_confirmation: PASSWORD,
+      email: EMAIL,
+      hashed_email: User.hash_email(EMAIL),
+      gender: 'F',
+      age: '18',
+      user_type: 'student',
+      terms_of_service_version: 1
+    }
+
+    USER_PARAMS_ERROR = {
+      name: 'A name',
+      password: PASSWORD,
+      password_confirmation: PASSWORD + "oops!",
+      email: EMAIL,
+      hashed_email: User.hash_email(EMAIL),
+      gender: 'F',
+      age: '18',
+      user_type: 'student',
+      terms_of_service_version: 1
+    }
+
+    setup do
+      SecureRandom.stubs(:uuid).returns(UUID)
+      OmniAuth.config.test_mode = true
+      OmniAuth.config.mock_auth[:google_oauth2] = generate_auth_user_hash(provider: 'google_oauth2')
+    end
+
+    test 'loading sign up page sends Firehose page load event' do
+      FirehoseClient.instance.expects(:put_record).once.with do |data|
+        data[:study] == STUDY &&
+          data[:event] == 'load-sign-up-page' &&
+          data[:data_string] == UUID
+      end
+      get '/users/sign_up'
+    end
+
+    test 'successful email sign up sends Firehose success event' do
+      events = %w(load-sign-up-page email-sign-up-success)
+      FirehoseClient.instance.expects(:put_record).times(2).with do |data|
+        data[:study] == STUDY &&
+          data[:event] == events.shift &&
+          data[:data_string] == UUID
+      end
+
+      get '/users/sign_up'
+
+      assert_creates(User) do
+        post '/users.json', params: {
+          user: USER_PARAMS_GOOD
+        }
+      end
+    end
+
+    test 'email sign up with wrong password confirmation sends Firehose error event' do
+      events = %w(load-sign-up-page email-sign-up-error)
+      FirehoseClient.instance.expects(:put_record).times(2).with do |data|
+        data[:study] == STUDY &&
+          data[:event] == events.shift &&
+          data[:data_string] == UUID
+      end
+
+      get '/users/sign_up'
+
+      assert_does_not_create(User) do
+        post '/users.json', params: {
+          user: USER_PARAMS_ERROR
+        }
+      end
+    end
+
+    test 'successful oauth sign up after hitting sign up page sends Firehose success events' do
+      events = %w(load-sign-up-page google_oauth2-sign-up-success)
+      FirehoseClient.instance.expects(:put_record).times(2).with do |data|
+        data[:study] == STUDY &&
+          data[:event] == events.shift &&
+          data[:data_string] == UUID
+      end
+
+      get '/users/sign_up'
+
+      @request.env["devise.mapping"] = Devise.mappings[:user]
+      assert_creates(User) do
+        get '/users/auth/google_oauth2'
+        follow_redirect!
+      end
+    end
+
+    test 'login to existing account after hitting sign up page sends Firehose sign in event' do
+      create :teacher, :unmigrated_google_sso, uid: DEFAULT_UID, email: EMAIL
+      events = %w(load-sign-up-page google_oauth2-sign-in)
+      FirehoseClient.instance.expects(:put_record).times(2).with do |data|
+        data[:study] == STUDY &&
+            data[:event] == events.shift &&
+            data[:data_string] == UUID
+      end
+
+      get '/users/sign_up'
+
+      assert_does_not_create(User) do
+        get '/users/auth/google_oauth2'
+        follow_redirect!
+      end
+    end
+
+    test 'tracking cookie is cleared when hitting another random page on the site' do
+      create :teacher, :unmigrated_google_sso, uid: DEFAULT_UID, email: EMAIL
+      events = %w(load-sign-up-page)
+      FirehoseClient.instance.expects(:put_record).once.with do |data|
+        data[:study] == STUDY &&
+            data[:event] == events.shift &&
+            data[:data_string] == UUID
+      end
+
+      get '/users/sign_up'
+      get '/courses' # should clear tracking variable, events should not be sent afterwards
+
+      @request.env["devise.mapping"] = Devise.mappings[:user]
+      assert_does_not_create(User) do
+        get '/users/auth/google_oauth2'
+        follow_redirect!
+      end
+    end
+
+    private
+
+    def generate_auth_user_hash(args)
+      OmniAuth::AuthHash.new(
+        uid: args[:uid] || DEFAULT_UID,
+        provider: args[:provider] || 'facebook',
+        info: {
+          name: args[:name] || 'someone',
+          email: args[:email] || EMAIL,
+          user_type: args[:user_type] || 'teacher',
+          dob: args[:dob] || Date.today - 20.years,
+          gender: args[:gender] || 'f'
+        },
+        credentials: {
+          token: args[:token] || '12345',
+          expires_at: args[:expires_at] || 'some-future-time',
+          refresh_token: args[:refresh_token] || nil
+        }
+      )
+    end
+  end
+end