maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Aliased DisplayAdEvent.display_ad_id to billboard_id (#19805)

Browse source 
* Aliased DisplayAdEvent.display_ad_id to billboard_id

* Allow :display_ad_id instead of :billboard_id for billboard_events in case of cache
This commit is contained in:
Anna Buianova 2023-07-24 17:54:39 +03:00 • committed by GitHub
parent c28bafccfe
commit 963eea8126
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 28 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
app/assets/javascripts/initializers/initializeDisplayAdVisibility.js
View file

@ -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,
},

7
app/controllers/billboard_events_controller.rb
View file

@ -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

4
app/javascript/packs/billboardAfterRenderActions.js
View file

@ -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,
},

4
app/models/display_ad_event.rb
View file

@ -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

14
app/services/billboard_event_rollup.rb
View file

@ -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

10
spec/requests/billboard_events_spec.rb
View file

@ -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
}

8
spec/services/billboard_event_rollup_spec.rb
View file

@ -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