From bcbce3eff0bec489c718ea8cf414f3c38f54527a Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Mon, 24 Jun 2013 16:53:01 -0400 Subject: [PATCH 1/4] Add handful of events to the Segment.io whitelist --- common/lib/xmodule/xmodule/js/src/capa/display.coffee | 8 ++++---- common/static/coffee/src/logger.coffee | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.coffee b/common/lib/xmodule/xmodule/js/src/capa/display.coffee index f773fc81c4..6b355459e9 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/capa/display.coffee @@ -138,7 +138,7 @@ class @Problem # maybe preferable to consolidate all dispatches to use FormData ### check_fd: => - Logger.log 'problem_check', @answers + Logger.log 'problem_check', answers: @answers # If there are no file inputs in the problem, we can fall back on @check if $('input:file').length == 0 @@ -212,7 +212,7 @@ class @Problem $.ajaxWithPrefix("#{@url}/problem_check", settings) check: => - Logger.log 'problem_check', @answers + Logger.log 'problem_check', answers: @answers $.postWithPrefix "#{@url}/problem_check", @answers, (response) => switch response.success when 'incorrect', 'correct' @@ -224,7 +224,7 @@ class @Problem @gentle_alert response.success reset: => - Logger.log 'problem_reset', @answers + Logger.log 'problem_reset', answers: @answers $.postWithPrefix "#{@url}/problem_reset", id: @id, (response) => @render(response.html) @updateProgress response @@ -284,7 +284,7 @@ class @Problem @el.find('.capa_alert').css(opacity: 0).animate(opacity: 1, 700) save: => - Logger.log 'problem_save', @answers + Logger.log 'problem_save', answers: @answers $.postWithPrefix "#{@url}/problem_save", @answers, (response) => saveMessage = response.msg @gentle_alert saveMessage diff --git a/common/static/coffee/src/logger.coffee b/common/static/coffee/src/logger.coffee index 6da4929fb0..dbc2b8e004 100644 --- a/common/static/coffee/src/logger.coffee +++ b/common/static/coffee/src/logger.coffee @@ -1,6 +1,6 @@ class @Logger # events we want sent to Segment.io for tracking - SEGMENT_IO_WHITELIST = ["seq_goto", "seq_next", "seq_prev"] + SEGMENT_IO_WHITELIST = ["seq_goto", "seq_next", "seq_prev", "problem_check", "problem_reset", "problem_show", "problem_save"] @log: (event_type, data) -> if event_type in SEGMENT_IO_WHITELIST From 84f4361d522c9898b69fa3e16c0540126673b5cc Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Tue, 25 Jun 2013 15:15:28 -0400 Subject: [PATCH 2/4] Avoid changing format of data sent to our logs, and prevent problem_check event from firing twice --- common/lib/xmodule/xmodule/js/src/capa/display.coffee | 9 +++++---- common/static/coffee/src/logger.coffee | 8 ++++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.coffee b/common/lib/xmodule/xmodule/js/src/capa/display.coffee index 6b355459e9..bf6aba0a21 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/capa/display.coffee @@ -138,7 +138,7 @@ class @Problem # maybe preferable to consolidate all dispatches to use FormData ### check_fd: => - Logger.log 'problem_check', answers: @answers + Logger.log 'problem_check', @answers # If there are no file inputs in the problem, we can fall back on @check if $('input:file').length == 0 @@ -212,7 +212,8 @@ class @Problem $.ajaxWithPrefix("#{@url}/problem_check", settings) check: => - Logger.log 'problem_check', answers: @answers + # Calling check from within check_fd will result in firing the 'problem_check' event twice + # Logger.log 'problem_check', @answers $.postWithPrefix "#{@url}/problem_check", @answers, (response) => switch response.success when 'incorrect', 'correct' @@ -224,7 +225,7 @@ class @Problem @gentle_alert response.success reset: => - Logger.log 'problem_reset', answers: @answers + Logger.log 'problem_reset', @answers $.postWithPrefix "#{@url}/problem_reset", id: @id, (response) => @render(response.html) @updateProgress response @@ -284,7 +285,7 @@ class @Problem @el.find('.capa_alert').css(opacity: 0).animate(opacity: 1, 700) save: => - Logger.log 'problem_save', answers: @answers + Logger.log 'problem_save', @answers $.postWithPrefix "#{@url}/problem_save", @answers, (response) => saveMessage = response.msg @gentle_alert saveMessage diff --git a/common/static/coffee/src/logger.coffee b/common/static/coffee/src/logger.coffee index dbc2b8e004..f2dfef5132 100644 --- a/common/static/coffee/src/logger.coffee +++ b/common/static/coffee/src/logger.coffee @@ -3,9 +3,13 @@ class @Logger SEGMENT_IO_WHITELIST = ["seq_goto", "seq_next", "seq_prev", "problem_check", "problem_reset", "problem_show", "problem_save"] @log: (event_type, data) -> + # Segment.io event tracking if event_type in SEGMENT_IO_WHITELIST - # Segment.io event tracking - analytics.track event_type, data + # to avoid changing the format of data sent to our servers, we only massage it here + if typeof data isnt 'object' or data is null + analytics.track event_type, value: data + else + analytics.track event_type, data $.getWithPrefix '/event', event_type: event_type From 881d63dae7177adc4ff9e0cad595eac37d18a604 Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Tue, 25 Jun 2013 16:04:00 -0400 Subject: [PATCH 3/4] Fixed Jasmine tests in light of Logger changes, and wrote test to cover case where data passed is not a dictionary --- common/static/coffee/spec/logger_spec.coffee | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/common/static/coffee/spec/logger_spec.coffee b/common/static/coffee/spec/logger_spec.coffee index 8866daa570..7fe734d8b5 100644 --- a/common/static/coffee/spec/logger_spec.coffee +++ b/common/static/coffee/spec/logger_spec.coffee @@ -3,10 +3,15 @@ describe 'Logger', -> expect(window.log_event).toBe Logger.log describe 'log', -> - it 'sends an event to Segment.io, if the event is whitelisted', -> + it 'sends an event to Segment.io, if the event is whitelisted and the data is not a dictionary', -> spyOn(analytics, 'track') Logger.log 'seq_goto', 'data' - expect(analytics.track).toHaveBeenCalledWith 'seq_goto', 'data' + expect(analytics.track).toHaveBeenCalledWith 'seq_goto', value: 'data' + + it 'sends an event to Segment.io, if the event is whitelisted and the data is a dictionary', -> + spyOn(analytics, 'track') + Logger.log 'seq_goto', value: 'data' + expect(analytics.track).toHaveBeenCalledWith 'seq_goto', value: 'data' it 'send a request to log event', -> spyOn $, 'getWithPrefix' From 3f49da385f1275f5cf24959008103e89a29e050d Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Tue, 25 Jun 2013 17:22:05 -0400 Subject: [PATCH 4/4] Swap Logger call from check_fd to check --- common/lib/xmodule/xmodule/js/src/capa/display.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.coffee b/common/lib/xmodule/xmodule/js/src/capa/display.coffee index bf6aba0a21..1f3be9e5e9 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/capa/display.coffee @@ -138,7 +138,8 @@ class @Problem # maybe preferable to consolidate all dispatches to use FormData ### check_fd: => - Logger.log 'problem_check', @answers + # Calling check from check_fd will result in firing the 'problem_check' event twice, since it is also called in the check function. + #Logger.log 'problem_check', @answers # If there are no file inputs in the problem, we can fall back on @check if $('input:file').length == 0 @@ -212,8 +213,7 @@ class @Problem $.ajaxWithPrefix("#{@url}/problem_check", settings) check: => - # Calling check from within check_fd will result in firing the 'problem_check' event twice - # Logger.log 'problem_check', @answers + Logger.log 'problem_check', @answers $.postWithPrefix "#{@url}/problem_check", @answers, (response) => switch response.success when 'incorrect', 'correct'