Merge pull request #5986 from CartoDB/5981_missing_event

5981 missing event

app/controllers/api/json/visualizations_controller.rb

@@ -58,8 +58,7 @@ def create
 
         vis = set_visualization_prev_next(vis, prev_id, next_id)
 
-        custom_properties = {'privacy' => vis.privacy, 'type' => vis.type,  'vis_id' => vis.id}
-        Cartodb::EventTracker.new.send_event(current_user, 'Created map', custom_properties)
+        track_event(vis, 'Created')
 
         render_jsonp(vis)
       rescue CartoDB::InvalidMember
@@ -149,15 +148,18 @@ def destroy
         return(head 404) unless vis
         return(head 403) unless vis.is_owner?(current_user)
 
-        custom_properties = {'privacy' => vis.privacy, 'type' => vis.type,  'vis_id' => vis.id}
-        event_type = vis.type == Visualization::Member::TYPE_DERIVED ? 'map' : 'dataset' 
+        track_event(vis, 'Deleted')
+        unless vis.table.nil?
+          vis.table.dependent_visualizations.each { |dependent_vis|
+            # Remove dependent visualizations as well, if any
+            track_event(dependent_vis, 'Deleted')
+          }
+        end 
 
         @stats_aggregator.timing('delete') do
           vis.delete
         end
 
-        Cartodb::EventTracker.new.send_event(current_user, "Deleted #{event_type}", custom_properties)
-
         return head 204
       rescue KeyError
         head(404)
@@ -464,5 +466,12 @@ def set_visualization_prev_next(vis, prev_id, next_id)
     vis
   end
 
+  def track_event(vis, action)
+    custom_properties = {'privacy' => vis.privacy, 'type' => vis.type,  'vis_id' => vis.id}
+    event_type = vis.type == Visualization::Member::TYPE_DERIVED ? 'map' : 'dataset'
+
+    Cartodb::EventTracker.new.send_event(current_user, "#{action} #{event_type}", custom_properties)
+  end
+
 end
 

lib/cartodb/event_tracker.rb

@@ -4,6 +4,7 @@ class EventTracker
 
     def send_event(user, event_name, custom_properties = {})
       return unless is_tracking_active?
+      return unless user_valid?(user, event_name, custom_properties)
 
       # Some events register custom properties
       # Monitary values associated with the event should use 'revenue' reserved key	
@@ -28,5 +29,16 @@ def is_tracking_active?
       !Cartodb.config[:segment].blank? and !Cartodb.config[:segment]['api_key'].blank?
     end
 
+    def user_valid?(user, event_name, custom_properties)
+      if user.nil?
+        Rollbar.report_message('EventTracker: null user error', 'Error', { event: event_name,
+                                                                           custom_properties: custom_properties,
+                                                                           error_message: 'Provided user is null'})
+        false
+      else
+        true
+      end
+    end
+
   end
 end

lib/cartodb/segment_tracker.rb

@@ -10,11 +10,12 @@ def initialize
     end
 
     def enabled?
-      !@api_key.nil?
+      !@api_key.blank?
     end
 
     def track_event(user_id, event, properties)
       return unless enabled?
+      return if user_id.blank?
 
       begin
         @analytics.track(
@@ -23,15 +24,18 @@ def track_event(user_id, event, properties)
           properties: properties
         )
       rescue => e
-      Rollbar.report_message('Segment error tracking event', 'error', { user: user_id, event: event })
+        Rollbar.report_message('EventTracker: segment event tracking error', 'error', { user_id: user_id, 
+                                                                                        event: event,
+                                                                                        properties: properties, 
+                                                                                        error_message: e.inspect })
       end
     end
 
     def flush
       begin
         @analytics.flush
       rescue => e
-        Rollbar.report_message('Segment error flush', 'error')
+        Rollbar.report_message('EventTracker: segment flushing error', 'error', { error_message: e.inspect })
       end
     end
   end