From 963eea8126389c1a4761732b4362edb19f9ccacc Mon Sep 17 00:00:00 2001 From: Anna Buianova Date: Mon, 24 Jul 2023 17:54:39 +0300 Subject: [PATCH] Aliased DisplayAdEvent.display_ad_id to billboard_id (#19805) * Aliased DisplayAdEvent.display_ad_id to billboard_id * Allow :display_ad_id instead of :billboard_id for billboard_events in case of cache --- .../initializers/initializeDisplayAdVisibility.js | 4 ++-- app/controllers/billboard_events_controller.rb | 7 +++++-- .../packs/billboardAfterRenderActions.js | 4 ++-- app/models/display_ad_event.rb | 4 +++- app/services/billboard_event_rollup.rb | 14 +++++++------- spec/requests/billboard_events_spec.rb | 10 +++++----- spec/services/billboard_event_rollup_spec.rb | 8 ++++---- 7 files changed, 28 insertions(+), 23 deletions(-) diff --git a/app/assets/javascripts/initializers/initializeDisplayAdVisibility.js b/app/assets/javascripts/initializers/initializeDisplayAdVisibility.js index 8481a5cbf..8a9ff7732 100644 --- a/app/assets/javascripts/initializers/initializeDisplayAdVisibility.js +++ b/app/assets/javascripts/initializers/initializeDisplayAdVisibility.js @@ -30,7 +30,7 @@ function trackAdImpression(adBox) { var dataBody = { billboard_event: { - display_ad_id: adBox.dataset.id, + billboard_id: adBox.dataset.id, context_type: adBox.dataset.contextType, category: adBox.dataset.categoryImpression, }, @@ -65,7 +65,7 @@ function trackAdClick(adBox) { var dataBody = { billboard_event: { - display_ad_id: adBox.dataset.id, + billboard_id: adBox.dataset.id, context_type: adBox.dataset.contextType, category: adBox.dataset.categoryClick, }, diff --git a/app/controllers/billboard_events_controller.rb b/app/controllers/billboard_events_controller.rb index e2fcc494c..228cb087d 100644 --- a/app/controllers/billboard_events_controller.rb +++ b/app/controllers/billboard_events_controller.rb @@ -15,7 +15,7 @@ class BillboardEventsController < ApplicationMetalController private def update_billboards_data - billboard_event_id = billboard_event_params[:display_ad_id] + billboard_event_id = billboard_event_params[:billboard_id] ThrottledCall.perform("billboards_data_update-#{billboard_event_id}", throttle_for: 15.minutes) do @billboard = DisplayAd.find(billboard_event_id) @@ -34,6 +34,9 @@ class BillboardEventsController < ApplicationMetalController def billboard_event_params event_params = params[:billboard_event] || params[:display_ad_event] - event_params.slice(:context_type, :category, :display_ad_id) + # keeping while we may receive data in the "old" format from cached js + display_ad_id = event_params.delete(:display_ad_id) + event_params[:billboard_id] ||= display_ad_id + event_params.slice(:context_type, :category, :billboard_id) end end diff --git a/app/javascript/packs/billboardAfterRenderActions.js b/app/javascript/packs/billboardAfterRenderActions.js index 0de7c099b..84a977e41 100644 --- a/app/javascript/packs/billboardAfterRenderActions.js +++ b/app/javascript/packs/billboardAfterRenderActions.js @@ -61,7 +61,7 @@ function trackAdImpression(adBox) { const dataBody = { billboard_event: { - display_ad_id: adBox.dataset.id, + billboard_id: adBox.dataset.id, context_type: adBox.dataset.contextType, category: adBox.dataset.categoryImpression, }, @@ -97,7 +97,7 @@ function trackAdClick(adBox) { const dataBody = { billboard_event: { - display_ad_id: adBox.dataset.id, + billboard_id: adBox.dataset.id, context_type: adBox.dataset.contextType, category: adBox.dataset.categoryClick, }, diff --git a/app/models/display_ad_event.rb b/app/models/display_ad_event.rb index a5cbf59a3..06d24891c 100644 --- a/app/models/display_ad_event.rb +++ b/app/models/display_ad_event.rb @@ -2,9 +2,11 @@ # :delete for the relationship. That means no before/after # destroy callbacks will be called on this object. class DisplayAdEvent < ApplicationRecord - belongs_to :billboard, class_name: "DisplayAd", foreign_key: "display_ad_id", inverse_of: :billboard_events + belongs_to :billboard, class_name: "DisplayAd", foreign_key: :display_ad_id, inverse_of: :billboard_events belongs_to :user, optional: true + alias_attribute :billboard_id, :display_ad_id + CATEGORY_IMPRESSION = "impression".freeze CATEGORY_CLICK = "click".freeze VALID_CATEGORIES = [CATEGORY_CLICK, CATEGORY_IMPRESSION].freeze diff --git a/app/services/billboard_event_rollup.rb b/app/services/billboard_event_rollup.rb index 0cbff81a7..49494011a 100644 --- a/app/services/billboard_event_rollup.rb +++ b/app/services/billboard_event_rollup.rb @@ -3,7 +3,7 @@ class BillboardEventRollup ATTRIBUTES_DESTROYED = %i[id counts_for updated_at].freeze class EventAggregator - Compact = Struct.new(:events, :user_id, :display_ad_id, :category, :context_type) do + Compact = Struct.new(:events, :user_id, :billboard_id, :category, :context_type) do def to_h super.except(:events).merge({ counts_for: events.sum(&:counts_for) }) end @@ -11,8 +11,8 @@ class BillboardEventRollup def initialize @aggregator = Hash.new do |level1, user_id| - level1[user_id] = Hash.new do |level2, display_ad_id| - level2[display_ad_id] = Hash.new do |level3, category| + level1[user_id] = Hash.new do |level2, billboard_id| + level2[billboard_id] = Hash.new do |level3, category| level3[category] = Hash.new do |level4, context_type| level4[context_type] = [] end @@ -22,17 +22,17 @@ class BillboardEventRollup end def <<(event) - @aggregator[event.user_id][event.display_ad_id][event.category][event.context_type] << event + @aggregator[event.user_id][event.billboard_id][event.category][event.context_type] << event end def each @aggregator.each_pair do |user_id, grouped_by_user_id| - grouped_by_user_id.each_pair do |display_ad_id, grouped_by_display_ad_id| - grouped_by_display_ad_id.each_pair do |category, grouped_by_category| + grouped_by_user_id.each_pair do |billboard_id, grouped_by_billboard_id| + grouped_by_billboard_id.each_pair do |category, grouped_by_category| grouped_by_category.each_pair do |context_type, events| next unless events.size > 1 - yield Compact.new(events, user_id, display_ad_id, category, context_type) + yield Compact.new(events, user_id, billboard_id, category, context_type) end end end diff --git a/spec/requests/billboard_events_spec.rb b/spec/requests/billboard_events_spec.rb index 4b71ba3ad..a149ff8f0 100644 --- a/spec/requests/billboard_events_spec.rb +++ b/spec/requests/billboard_events_spec.rb @@ -14,7 +14,7 @@ RSpec.describe "BillboardEvents" do it "creates a display ad click event" do post "/billboard_events", params: { billboard_event: { - display_ad_id: display_ad.id, + billboard_id: display_ad.id, context_type: DisplayAdEvent::CONTEXT_TYPE_HOME, category: DisplayAdEvent::CATEGORY_CLICK } @@ -36,7 +36,7 @@ RSpec.describe "BillboardEvents" do it "creates a display ad impression event" do post "/billboard_events", params: { billboard_event: { - display_ad_id: display_ad.id, + billboard_id: display_ad.id, context_type: DisplayAdEvent::CONTEXT_TYPE_HOME, category: DisplayAdEvent::CATEGORY_IMPRESSION } @@ -45,7 +45,7 @@ RSpec.describe "BillboardEvents" do end it "creates a display ad success rate" do - ad_event_params = { display_ad_id: display_ad.id, context_type: DisplayAdEvent::CONTEXT_TYPE_HOME } + ad_event_params = { billboard_id: display_ad.id, context_type: DisplayAdEvent::CONTEXT_TYPE_HOME } impression_params = ad_event_params.merge(category: DisplayAdEvent::CATEGORY_IMPRESSION, user: user) create_list(:display_ad_event, 4, impression_params) @@ -60,7 +60,7 @@ RSpec.describe "BillboardEvents" do it "assigns event to current user" do post "/billboard_events", params: { billboard_event: { - display_ad_id: display_ad.id, + billboard_id: display_ad.id, context_type: DisplayAdEvent::CONTEXT_TYPE_HOME, category: DisplayAdEvent::CATEGORY_IMPRESSION } @@ -71,7 +71,7 @@ RSpec.describe "BillboardEvents" do it "uses a ThrottledCall for data updates" do post "/billboard_events", params: { billboard_event: { - display_ad_id: display_ad.id, + billboard_id: display_ad.id, context_type: DisplayAdEvent::CONTEXT_TYPE_HOME, category: DisplayAdEvent::CATEGORY_IMPRESSION } diff --git a/spec/services/billboard_event_rollup_spec.rb b/spec/services/billboard_event_rollup_spec.rb index a76bc74d9..5211fd120 100644 --- a/spec/services/billboard_event_rollup_spec.rb +++ b/spec/services/billboard_event_rollup_spec.rb @@ -45,7 +45,7 @@ RSpec.describe BillboardEventRollup, type: :service do [ad2.id, nil, 1], ] results_mapped = DisplayAdEvent.where(created_at: days_ago_as_range(2)).map do |event| - [event.display_ad_id, event.user_id, event.counts_for] + [event.billboard_id, event.user_id, event.counts_for] end expect(results_mapped).to match_array(expectations) end @@ -66,8 +66,8 @@ RSpec.describe BillboardEventRollup, type: :service do expect(by_category["click"]["counts_for"]).to eq(2) end - # separate display_ad_id - it "groups by display_ad_id" do + # separate billboard_id + it "groups by billboard_id" do create(:display_ad_event, billboard: ad1, user_id: nil) create(:display_ad_event, billboard: ad1, user_id: nil) create(:display_ad_event, billboard: ad1, user_id: nil) @@ -76,7 +76,7 @@ RSpec.describe BillboardEventRollup, type: :service do described_class.rollup(Date.current) results = DisplayAdEvent.where(created_at: Date.current.all_day) - by_ad = results.index_by { |r| r["display_ad_id"] } + by_ad = results.index_by { |r| r["billboard_id"] } expect(by_ad[ad1.id]["counts_for"]).to eq(3) expect(by_ad[ad2.id]["counts_for"]).to eq(2) end