[bug fix] N+1 query in Analytics V2 (#1451)

donrestarone · alis-khadka · web-flow · commit 336836eb35f1 · 2023-03-10T22:20:38.000-05:00
Addresses: #1399 Co-authored-by: Prashant <25191509+alis-khadka@users.noreply.github.com>
diff --git a/.env.test b/.env.test
@@ -9,4 +9,4 @@ APP_HOST=lvh.me:5250
 RECAPTCHA_SITE_KEY=6Lc6BAAAAAAAAChqRbQZcn_yyyyyyyyyyyyyyyyy
 RECAPTCHA_SECRET_KEY=6Lc6BAAAAAAAAKN3DRm6VA_xxxxxxxxxxxxxxxxx
 SECRET_KEY_BASE='38c72586473e364229897f24f1892f1dc5565776878aa4d8c6bf051258622bd2e923b926ab59b40f912b661216f764d993e8d6b8bbfbc33026e5c954b6c51f9b'
-RACK_TIMEOUT_SERVICE_TIMEOUT=8
+RACK_TIMEOUT_SERVICE_TIMEOUT=80
diff --git a/app/controllers/comfy/admin/v2/dashboard_controller.rb b/app/controllers/comfy/admin/v2/dashboard_controller.rb
@@ -8,13 +8,15 @@ def dashboard
     @end_date = params[:end_date]&.to_date || Date.today.end_of_month
     date_range = @start_date.beginning_of_day..@end_date.end_of_day
 
-    @visits = Ahoy::Visit.where(started_at: @start_date.beginning_of_day..@end_date.end_of_day)
+    @visits = Ahoy::Visit.where(started_at: date_range)
+
+    filtered_events = Ahoy::Event.joins(:visit)
 
     Ahoy::Event::EVENT_CATEGORIES.values.each do |event_category|
       if event_category == Ahoy::Event::EVENT_CATEGORIES[:page_visit]
-        events = Ahoy::Event.where(name: 'comfy-cms-page-visit').joins(:visit)
+        events = filtered_events.where(name: 'comfy-cms-page-visit')
       else
-        events = Ahoy::Event.jsonb_search(:properties, { category: event_category }).joins(:visit)
+        events = filtered_events.jsonb_search(:properties, { category: event_category })
       end
       events = events.jsonb_search(:properties, { page_id: params[:page] }) if params[:page].present?
       instance_variable_set("@previous_period_#{event_category}_events", events.where(time: previous_period(params[:interval], @start_date, @end_date)))
@@ -23,7 +25,7 @@ def dashboard
 
     # legacy and system events does not have category 
     # separating out 'comfy-cms-page-visit' event since we have a seprate section
-    @legacy_and_system_events = Ahoy::Event.where.not('properties::jsonb ? :key', key: 'category').where.not(name: 'comfy-cms-page-visit').joins(:visit)
+    @legacy_and_system_events = filtered_events.where.not('properties::jsonb ? :key', key: 'category').where.not(name: 'comfy-cms-page-visit')
     @previous_period_legacy_and_system_events = @legacy_and_system_events.where(time: previous_period(params[:interval], @start_date, @end_date))
     @legacy_and_system_events = @legacy_and_system_events.where(time: date_range)
   end
diff --git a/app/helpers/dashboard_helper.rb b/app/helpers/dashboard_helper.rb
@@ -24,19 +24,29 @@ def session_detail_title
 
   def page_visit_chart_data(page_visit_events, start_date, end_date)
     period, format = split_into(start_date, end_date)
-    page_visit_events.where.not('ahoy_visits.device_type': nil).group_by { |u| u.visit.device_type }.map do |key, value|
-      { name: key, data: Ahoy::Event.where(id: value.pluck(:id)).group_by_period(period, :time, range: start_date..end_date, format: format).count }
-    end
-  end
 
+    page_visit_events
+      .where.not(visit: {device_type: nil})
+      .group("visit.device_type")
+      .group_by_period(period, :time, range: start_date..end_date, format: format)
+      .size
+      .group_by {|k, v| k.first}
+      .map do |k,  v| 
+        {
+          name: k,
+          data: v.map {|item| [item.first.last, item.last]}.to_h
+        }
+      end 
+  end 
+    
   def page_name(page_id)
     return 'Website' if page_id.blank?
 
     Comfy::Cms::Page.find_by(id: page_id)&.label
   end
 
   def visitors_chart_data(visits)
-    visitors_by_token = visits.group(:visitor_token).count
+    visitors_by_token = visits.group(:visitor_token).size
     recurring_visitors = visitors_by_token.values.count { |v| v > 1 }
     single_time_visitors = visitors_by_token.keys.count - recurring_visitors
     {"Single time visitor": single_time_visitors, "Recurring visitors" => recurring_visitors  }
@@ -61,46 +71,55 @@ def tooltip_content(current_count, prev_count, interval, start_date, end_date)
   end
 
   def total_watch_time(video_view_events)
-    video_view_events.sum { |event| event.properties['watch_time'].to_i }
+    video_view_events.pluck(Arel.sql("SUM((#{Ahoy::Event.table_name}.properties ->> 'watch_time')::bigint)")).sum
   end
 
   def to_minutes(time_in_milisecond)
     "#{number_with_delimiter((time_in_milisecond.to_f / (1000 * 60)).round(2) , :delimiter => ',')} min"
   end
 
   def total_views(video_view_events)
-    video_view_events.select { |event| event.properties['video_start'] }.size
+    video_view_events.pluck(Arel.sql("SUM(CASE WHEN (#{Ahoy::Event.table_name}.properties ->> 'video_start')::boolean THEN 1 ELSE 0 END)")).sum
   end
 
   def avg_view_duration(video_view_events)
     total_watch_time(video_view_events).to_f / (total_views(video_view_events).nonzero? || 1)
   end
 
   def avg_view_percentage(video_view_events)
-    view_percentage_arr = video_view_events.group_by { |event| event.properties['resource_id'] }.map do |_resource_id, events|
-      (events.sum { |event| event.properties['watch_time'].to_f  / event.properties['total_duration'].to_f }) * 100
-    end
-    view_percentage_arr.sum / (total_views(video_view_events).nonzero? || 1)
-  end
-
-  def top_three_videos(video_view_events, previous_video_view_events) 
-    video_view_events.group_by { |event| event.properties['resource_id'] }.map do |resource_id, events|
-      previous_period_event = previous_video_view_events.jsonb_search(:properties, { resource_id: resource_id })
-      api_resource = ApiResource.find_by(id: resource_id) 
-      { 
-        total_views: total_views(events),
-        total_watch_time:  total_watch_time(events),
-        previous_period_total_views: total_views(previous_period_event),
-        previous_period_total_watch_time: total_watch_time(previous_period_event),
-        resource_title: api_resource&.properties.dig(api_resource&.api_namespace.analytics_metadata&.dig("title")) || "Resource Id: #{resource_id}",
-        resource_author: api_resource&.properties.dig(api_resource&.api_namespace.analytics_metadata&.dig("author")),
-        resource_image: api_resource&.non_primitive_properties.find_by(field_type: "file", label: api_resource&.api_namespace.analytics_metadata&.dig("thumbnail"))&.file_url,
-        resource_id: api_resource&.id,
-        namespace_id: api_resource&.api_namespace.id,
-        duration: events.first.properties['total_duration'],
-        name: events.first.name
-      }
-    end.sort_by {|event| event[:total_views]}.reverse.first(3)
+    video_view_events.pluck(Arel.sql("((properties ->> 'watch_time')::float / (properties ->> 'total_duration')::float) * 100")).sum / (total_views(video_view_events).nonzero? || 1)
+  end
+
+  def top_three_videos(video_view_events, previous_video_view_events)
+    video_view_events
+      .with_api_resource
+      .group(:resource_id)
+      .reorder("SUM(is_viewed) DESC", "total_watch_time DESC")
+      .select(:resource_id,
+        "SUM(watch_time)::INT AS total_watch_time",
+        "SUM(is_viewed) AS total_views",
+        "MAX(total_duration)::float AS duration",
+        "json_agg(ahoy_events.name) AS names",
+        "json_agg(namespace_id) AS namespace_ids")
+      .limit(3)
+      .as_json
+      .map(&:with_indifferent_access)
+      .each do |video_event|
+        previous_period_event = previous_video_view_events.jsonb_search(:properties, { resource_id: video_event[:resource_id] })
+        api_resource = ApiResource.find_by(id: video_event[:resource_id])
+
+        video_event[:name] = video_event[:names].uniq.first
+        video_event[:namespace_id] = video_event[:namespace_ids].uniq.first
+        video_event[:previous_period_total_views] = total_views(previous_period_event)
+        video_event[:previous_period_total_watch_time] = total_watch_time(previous_period_event)
+        video_event[:resource_title] = api_resource&.properties.dig(api_resource&.api_namespace.analytics_metadata&.dig("title")) || "Resource Id: #{video_event[:resource_id]}"
+        video_event[:resource_author] = api_resource&.properties.dig(api_resource&.api_namespace.analytics_metadata&.dig("author"))
+        video_event[:resource_image] = api_resource&.non_primitive_properties.find_by(field_type: "file", label: api_resource&.api_namespace.analytics_metadata&.dig("thumbnail"))&.file_url
+
+        video_event.delete(:names)
+        video_event.delete(:namespace_ids)
+        video_event.delete(:id)
+      end
   end
 
   private
diff --git a/app/models/ahoy/event.rb b/app/models/ahoy/event.rb
@@ -27,6 +27,33 @@ class Ahoy::Event < ApplicationRecord
   belongs_to :visit
   belongs_to :user, optional: true
 
+  scope :with_label , -> {
+    # Build a subquery SQL
+    subquery = self.unscoped.select("(case when #{table_name}.properties->>'label' is not NULL then #{table_name}.properties->>'label' else #{table_name}.name end) as label, #{table_name}.id").to_sql
+
+    # join the subquery to base model
+    joins("INNER JOIN (#{subquery}) as labelled_events ON labelled_events.id = #{table_name}.id")
+  }
+
+  scope :with_api_resource , -> {
+    # Build a subquery SQL
+    subquery = self
+                .unscoped
+                .joins("INNER JOIN #{ApiResource.table_name} ON ahoy_events.properties->>'resource_id' IS NOT NULL AND (ahoy_events.properties ->> 'resource_id')::int = #{ApiResource.table_name}.id")
+                .select(
+                  "(#{self.table_name}.properties ->> 'resource_id')::int AS resource_id",
+                  "#{self.table_name}.id",
+                  "#{ApiResource.table_name}.api_namespace_id AS namespace_id",
+                  "(#{self.table_name}.properties ->> 'watch_time')::bigint AS watch_time",
+                  "round((#{self.table_name}.properties->>'total_duration')::numeric, 3) AS total_duration",
+                  "CASE WHEN (#{self.table_name}.properties ->> 'video_start')::boolean THEN 1 ELSE 0 END AS is_viewed",
+                  )
+                  .to_sql
+
+    # join the subquery to base model
+    joins("INNER JOIN (#{subquery}) as api_resourced_events ON api_resourced_events.id = #{table_name}.id")
+  }
+
   # For events_list page, sorting on the grouped query
   # https://stackoverflow.com/a/35987240
   ransacker :count do
diff --git a/app/views/comfy/admin/v2/dashboard/_events.html.haml b/app/views/comfy/admin/v2/dashboard/_events.html.haml
@@ -3,28 +3,27 @@
     .d-md-flex.align-items-center
       .vr-analytics-sub-title
         = title
-      - if events.present?
+      - if events_exists
         .d-flex.align-items-center.mt-2.mt-md-0
           .vr-analytics-count
-            = events.count 
+            = events_count 
             = "total #{type}"
           .vr-analytics-percent-change
-            = display_percent_change(events.count, previous_period_events.count)
-          .vr-analytics-tooltips{ data: { toggle: "tooltip", placement: "right" }, title: tooltip_content(events.count, previous_period_events.count, params[:interval], @start_date, @end_date) }
+            = display_percent_change(events_count, previous_period_events_count)
+          .vr-analytics-tooltips{ data: { toggle: "tooltip", placement: "right" }, title: tooltip_content(events_count, previous_period_events_count, params[:interval], @start_date, @end_date) }
             ?
 
-  - if events.present?
+  - if events_exists
     .vr-analytics-section-body.d-flex.align-items-center
       .vr-analytics-events-grid.row.w-100
-        - previous_grouped_events = previous_period_events.group(:name).size
-        - events.group_by(&:label).each do |key, value|
+        - label_grouped_event.each do |key, value|
           .vr-analytics-events-grid-item.col.col-12.col-sm-6.col-md-4.col-lg-3.mb-4
             .d-flex.mr-4.align-items-center.mb-2
               .vr-analytics-count-lg
-                = value.count 
+                = value.size 
               .vr-analytics-percent-change
-                = display_percent_change(value.count, previous_grouped_events[key].to_i)
+                = display_percent_change(value.size, previous_grouped_events[key].to_i)
               - if previous_grouped_events[key].to_i == 0
-                .vr-analytics-tooltips{ data: { toggle: "tooltip", placement: "right" }, title: tooltip_content(value.count, 0, params[:interval], @start_date, @end_date) }
+                .vr-analytics-tooltips{ data: { toggle: "tooltip", placement: "right" }, title: tooltip_content(value.size, 0, params[:interval], @start_date, @end_date) }
                   ?
-            = link_to key, dashboard_events_path(ahoy_event_type: value.first&.name), class: 'vr-analytics-event-label'  
+            = link_to key, dashboard_events_path(ahoy_event_type: value.first), class: 'vr-analytics-event-label'  
diff --git a/app/views/comfy/admin/v2/dashboard/dashboard.html.haml b/app/views/comfy/admin/v2/dashboard/dashboard.html.haml
@@ -22,22 +22,22 @@
 
   %main{class: 'my-5'}
     %section.row.vr-analytics-section.vr-analytics-page-visit-events
-      .col{ class: @page_visit_events.present? ? "col-lg-9 mb-4" : "col-12" }
+      .col{ class: @page_visit_events.load.any? ? "col-lg-9 mb-4" : "col-12" }
         .d-flex.justify-content-between
           .vr-analytics-section-header.d-md-flex.justify-content-between.align-items-center
             .vr-analytics-sub-title
               = "#{page_name(params[:page])} visits"
-            - if @page_visit_events.present?
+            - if @page_visit_events.load.any?
               .d-flex.justify-content-between.align-items-center.mt-2.mt-md-0
                 .vr-analytics-count
-                  = @page_visit_events.count 
+                  = @page_visit_events.size 
                   total visits
                 .vr-analytics-percent-change
-                  = display_percent_change(@page_visit_events.count, @previous_period_page_visit_events.count)
-                .vr-analytics-tooltips{ data: { toggle: "tooltip", placement: "right" }, title: tooltip_content(@page_visit_events.count, @previous_period_page_visit_events.count, params[:interval], @start_date, @end_date) }
+                  = display_percent_change(@page_visit_events.size, @previous_period_page_visit_events.size)
+                .vr-analytics-tooltips{ data: { toggle: "tooltip", placement: "right" }, title: tooltip_content(@page_visit_events.size, @previous_period_page_visit_events.size, params[:interval], @start_date, @end_date) }
                   ?
         .vr-analytics-section-body
-          - if @page_visit_events.present?
+          - if @page_visit_events.load.any?
             = column_chart page_visit_chart_data(@page_visit_events, @start_date, @end_date), colors: ["#88DAE3FF", "#D785E3FF", "#F7C85CFF"], library: { plugins: { legend: { position: "top", align: "end", labels: { padding: 24, boxWidth: 8, usePointStyle: true, font: { size: 16 } } } }  }
           - else
             .vr-analytics-blank-states
@@ -58,16 +58,16 @@
                 %br
                 :escaped
                   </main>
-      - if @page_visit_events.present?
+      - if @page_visit_events.load.any?
         .col.col-lg-3
           .vr-analytics-card.vr-analytics-page-visit-events-donut-chart
             %h5 Website visitors
-            = pie_chart visitors_chart_data(@visits), colors: ['#F7A47B', '#B5E69A'], donut: true, library: { cutout: 85, plugins: { legend: { position: "bottom",  align: 'start', labels: {boxWidth: 8, usePointStyle: true, font: { size: 16 } } } } }
+            = pie_chart visitors_chart_data(@page_visit_events), colors: ['#F7A47B', '#B5E69A'], donut: true, library: { cutout: 85, plugins: { legend: { position: "bottom",  align: 'start', labels: {boxWidth: 8, usePointStyle: true, font: { size: 16 } } } } }
 
   
     %hr.m-0
-    = render partial: 'events', locals: { events: @click_events, previous_period_events: @previous_period_click_events, title: 'Clicks', type: 'clickables' }
-    - unless @click_events.present?
+    = render partial: 'events', locals: { events_exists: @click_events.load.any?, events_count: @click_events.size, label_grouped_event: @click_events.with_label.group(:label).pluck(:label, Arel.sql('json_agg(ahoy_events.name)')), previous_period_events_count: @previous_period_click_events.size, previous_grouped_events: @previous_period_click_events.with_label.group(:label).size, title: 'Clicks', type: 'clickables' }
+    - unless @click_events.load.any?
       .vr-analytics-blank-states
         There are no click events within the selected date range. 
         %br
@@ -86,7 +86,7 @@
           .vr-analytics-sub-title.col.col-12.col-md-2
             Watch time
 
-          - if @video_view_events.present? 
+          - if @video_view_events.load.any? 
             .col.col-md-10
               .row
                 - watch_time = total_watch_time(@video_view_events)
@@ -121,11 +121,11 @@
                     = display_percent_change(view_percent, previous_view_percent)
                   .vr-analytics-tooltips{ data: { toggle: "tooltip", placement: "right" }, title: tooltip_content(view_percent, previous_view_percent, params[:interval], @start_date, @end_date) }
                     ?
-      - if @video_view_events.present? 
+      - if @video_view_events.load.any? 
         .vr-analytics-section-body
           .vr-analytics-event-label 
             - top_videos = top_three_videos(@video_view_events, @previous_period_video_view_events)
-            = "Top #{top_videos.count} videos"
+            = "Top #{top_videos.size} videos"
 
           .row.mt-4
             - top_videos.each do |event|
@@ -184,8 +184,8 @@
           Please make sure 'data-violet-resource-id' is present and Analytics mapping is populated
         
     %hr.m-0
-    = render partial: 'events', locals: { events: @form_submit_events, previous_period_events: @previous_period_form_submit_events, title: 'Form Submissions', type: 'submitables' }
-    - unless @form_submit_events.present?
+    = render partial: 'events', locals: { events_exists: @form_submit_events.load.any?, events_count: @form_submit_events.size, label_grouped_event: @form_submit_events.with_label.group(:label).pluck(:label, Arel.sql('json_agg(ahoy_events.name)')), previous_period_events_count: @previous_period_form_submit_events.size, previous_grouped_events: @previous_period_form_submit_events.with_label.group(:label).size, title: 'Form Submissions', type: 'submitables' }
+    - unless @form_submit_events.load.any?
       .vr-analytics-blank-states
         There are no form submission events within the selected date range. 
         %br
@@ -217,8 +217,8 @@
             </form>
 
     %hr.m-0
-    = render partial: 'events', locals: { events: @section_view_events, previous_period_events: @previous_period_section_view_events, title: 'Section Views', type: 'viewables' }
-    - unless @section_view_events.present?
+    = render partial: 'events', locals: { events_exists: @section_view_events.load.any?, events_count: @section_view_events.size, label_grouped_event: @section_view_events.with_label.group(:label).pluck(:label, Arel.sql('json_agg(ahoy_events.name)')), previous_period_events_count: @previous_period_section_view_events.size, previous_grouped_events: @previous_period_section_view_events.with_label.group(:label).size, title: 'Section Views', type: 'viewables' }
+    - unless @section_view_events.load.any?
       .vr-analytics-blank-states
         There are no section view events within the selected date range. 
         %br
@@ -252,4 +252,4 @@
         By default, the threshold value is 0.75
 
     %hr.m-0
-    = render partial: 'events', locals: { events: @legacy_and_system_events, previous_period_events: @previous_period_legacy_and_system_events, title: 'Events', type: 'events' }   
+    = render partial: 'events', locals: { events_exists: @legacy_and_system_events.load.any?, events_count: @legacy_and_system_events.size, label_grouped_event: @legacy_and_system_events.with_label.group(:label).pluck(:label, Arel.sql('json_agg(ahoy_events.name)')), previous_period_events_count: @previous_period_legacy_and_system_events.size, previous_grouped_events: @previous_period_legacy_and_system_events.with_label.group(:label).size, title: 'Events', type: 'events' }
diff --git a/test/controllers/admin/comfy/v2/dashboard_controller_test.rb b/test/controllers/admin/comfy/v2/dashboard_controller_test.rb
diff --git a/test/helpers/dashboard_helper_test.rb b/test/helpers/dashboard_helper_test.rb
diff --git a/test/tasks/purge_unattached_task_test.rb b/test/tasks/purge_unattached_task_test.rb
