Merge pull request #13605 from CartoDB/1997-segment-events

add new attributes to segment events

NEWS.md

@@ -154,6 +154,7 @@ ion for time-series (#12670)
 * Add marker size to layer cartocss props to reinstantiate torque map (#13590)
 * Fix select geometries dropdown in JOIN analysis ([Support#1281](https://github.com/CartoDB/support/issues/1281))
 * Fix IE11 Drag&Drop ([Support#876](https://github.com/CartoDB/support/issues/876))
+* Add new attributes to events ([Central#1997](https://github.com/CartoDB/cartodb-central#1997))
 * Removed references to analytics JS files in static pages (#13543)
 * Add titles (and description) to embeds in mobile viewports (#13517)
 * Support several versions of pg_dump and pg_restore binaries (CartoDB/cartodb-central#2073)

lib/assets/javascripts/builder/editor/layers/layer-content-views/data/data-view.js

@@ -240,7 +240,7 @@ module.exports = DatasetBaseView.extend({
     });
 
     MetricsTracker.track('Used advanced mode', {
-      type: 'sql'
+      mode_type: 'sql'
     });
   },
 

lib/assets/javascripts/builder/editor/layers/layer-content-views/infowindow/infowindow-base-view.js

@@ -194,7 +194,7 @@ module.exports = CoreView.extend({
       });
 
       MetricsTracker.track('Used advanced mode', {
-        type: 'popup'
+        mode_type: 'popup'
       });
 
       this._userActions.saveLayer(this._layerDefinitionModel);

lib/assets/javascripts/builder/editor/layers/layer-content-views/legend/legend-base-type-view.js

@@ -311,7 +311,7 @@ module.exports = CoreView.extend({
     }
 
     MetricsTracker.track('Used advanced mode', {
-      type: 'legend'
+      mode_type: 'legend'
     });
 
     LegendFactory.createLegend(this._layerDefinitionModel, type);

lib/assets/javascripts/builder/editor/style/style-content-view.js

@@ -166,8 +166,8 @@ module.exports = CoreView.extend({
           this._styleModel.setDefaultPropertiesByType(styleType, currentGeometryType);
 
           MetricsTracker.track('Aggregated geometries', {
-            previus_type: previousType,
-            type: styleType
+            previous_agg_type: previousType,
+            agg_type: styleType
           });
         }.bind(this);
 

lib/assets/javascripts/builder/editor/style/style-view.js

@@ -284,7 +284,7 @@ module.exports = CoreView.extend({
       });
 
       MetricsTracker.track('Used advanced mode', {
-        type: 'cartocss'
+        mode_type: 'cartocss'
       });
     } else {
       this._editorModel.get('edition') === false && CartoCSSNotifications.showErrorNotification(parser.parseError(errors));

lib/carto/tracking/events.rb

@@ -27,9 +27,20 @@ def self.required_properties(*required_properties)
         end
 
         def required_properties
-          these_required_properties = self.class.instance_eval { @required_properties }
+          these_required_properties = self.class.instance_eval { @required_properties || [] }
 
-          these_required_properties || self.class.superclass.required_properties
+          these_required_properties + self.class.superclass.required_properties
+        end
+
+        def self.optional_properties(*optional_properties)
+          @optional_properties ||= []
+          @optional_properties += optional_properties
+        end
+
+        def optional_properties
+          these_optional_properties = self.class.instance_eval { @optional_properties || [] }
+
+          these_optional_properties + self.class.superclass.optional_properties
         end
 
         def report
@@ -43,6 +54,7 @@ def report
 
         def report!
           check_required_properties!
+          check_no_extra_properties!
           authorize!
 
           report_to_methods = methods.select do |method_name|
@@ -66,6 +78,16 @@ def check_required_properties!
           end
         end
 
+        def check_no_extra_properties!
+          extra_properties = @format.to_hash.symbolize_keys.keys - required_properties - optional_properties
+
+          unless extra_properties.empty?
+            message = "#{name} is adding the following extra properties: #{extra_properties.join(', ')}"
+
+            raise Carto::UnprocesableEntityError.new(message)
+          end
+        end
+
         # Validators are modules that should be included in Event classes. These
         # modules contain methods that start with 'check_'. They raise an
         # exception if whatever condition they validate is not met. All methods
@@ -102,7 +124,10 @@ class MapEvent < Event
         required_properties :user_id, :visualization_id
       end
 
-      class CreatedMap < MapEvent; end
+      class CreatedMap < MapEvent
+        required_properties :origin
+      end
+
       class DeletedMap < MapEvent; end
 
       class PublishedMap < Event
@@ -161,7 +186,10 @@ class DatasetEvent < Event
         required_properties :user_id, :visualization_id
       end
 
-      class CreatedDataset < DatasetEvent; end
+      class CreatedDataset < DatasetEvent
+        required_properties :origin
+      end
+
       class DeletedDataset < DatasetEvent; end
 
       class AnalysisEvent < Event
@@ -184,7 +212,8 @@ class AppliedSql < Event
         include Carto::Tracking::Validators::Visualization::Writable
         include Carto::Tracking::Validators::User
 
-        required_properties :user_id, :visualization_id
+        required_properties :user_id, :visualization_id, :sql
+        optional_properties :node_id, :dataset_id
       end
 
       class AppliedCartocss < Event
@@ -193,16 +222,11 @@ class AppliedCartocss < Event
         include Carto::Tracking::Validators::Visualization::Writable
         include Carto::Tracking::Validators::User
 
-        required_properties :user_id, :visualization_id
+        required_properties :user_id, :visualization_id, :layer_id, :cartocss
       end
 
-      class ModifiedStyleForm < Event
-        include Carto::Tracking::Services::Hubspot
-
-        include Carto::Tracking::Validators::Visualization::Writable
-        include Carto::Tracking::Validators::User
-
-        required_properties :user_id, :visualization_id
+      class ModifiedStyleForm < AppliedCartocss
+        required_properties :style_properties
       end
 
       class CreatedWidget < Event
@@ -251,7 +275,7 @@ class CreatedLayer < Event
         include Carto::Tracking::Validators::Layer
         include Carto::Tracking::Validators::User
 
-        required_properties :user_id, :visualization_id, :layer_id
+        required_properties :user_id, :visualization_id, :layer_id, :empty
       end
 
       class ChangedDefaultGeometry < Event
@@ -269,7 +293,7 @@ class AggregatedGeometries < Event
         include Carto::Tracking::Validators::Visualization::Writable
         include Carto::Tracking::Validators::User
 
-        required_properties :user_id, :visualization_id, :previus_type, :type
+        required_properties :user_id, :visualization_id, :previous_agg_type, :agg_type
       end
 
       class UsedAdvancedMode < Event
@@ -278,7 +302,7 @@ class UsedAdvancedMode < Event
         include Carto::Tracking::Validators::Visualization::Writable
         include Carto::Tracking::Validators::User
 
-        required_properties :user_id, :visualization_id, :type
+        required_properties :user_id, :visualization_id, :mode_type
       end
 
       # Models a generic event for segment.

lib/carto/tracking/formats/segment.rb

@@ -15,6 +15,19 @@ def initialize(user: nil, visualization: nil, widget: nil, hash: {})
           @mapviews = hash[:mapviews]
           @action = hash[:action]
           @analysis = hash[:analysis]
+
+          # add anything else as it arrives
+          # add new properties in required_properties in events.rb for validation
+          @others = hash.symbolize_keys.except(:visualization_id,
+                                               :user_id,
+                                               :widget_id,
+                                               :connection,
+                                               :origin,
+                                               :page,
+                                               :quota_overage,
+                                               :mapviews,
+                                               :action,
+                                               :analysis)
         end
 
         def to_hash
@@ -27,6 +40,7 @@ def to_hash
           properties.merge!(trending_map_properties) if @mapviews
           properties.merge!(analysis_properties) if @analysis
           properties.merge!(widget_properties) if @widget
+          properties.merge!(@others) if @others
 
           properties[:page] = @page if @page
           properties[:quota_overage] = @quota_overage if @quota_overage