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

Rename DisplayAdEvent to BillboardEvent (#19833)

Browse source
This commit is contained in:
Anna Buianova 2023-07-26 20:57:02 +03:00 • committed by GitHub
parent 569dbb4834
commit ffb560c2a7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 78 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
app/controllers/billboard_events_controller.rb
View file

@ -5,7 +5,7 @@ class BillboardEventsController < ApplicationMetalController
def create
# Only tracking for logged in users at the moment
billboard_event_create_params = billboard_event_params.merge(user_id: session_current_user_id)
@billboard_event = DisplayAdEvent.create(billboard_event_create_params)
@billboard_event = BillboardEvent.create(billboard_event_create_params)
update_billboards_data
@ -35,8 +35,8 @@ class BillboardEventsController < ApplicationMetalController
def billboard_event_params
event_params = params[:billboard_event] || params[:display_ad_event]
# 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
billboard_id = event_params.delete(:display_ad_id)
event_params[:billboard_id] ||= billboard_id
event_params.slice(:context_type, :category, :billboard_id)
end
end

4
app/models/display_ad_event.rb → app/models/billboard_event.rb
View file

@ -1,10 +1,12 @@
# @note When we destroy the related user, it's using dependent:
# :delete for the relationship. That means no before/after
# destroy callbacks will be called on this object.
class DisplayAdEvent < ApplicationRecord
class BillboardEvent < ApplicationRecord
belongs_to :billboard, class_name: "DisplayAd", foreign_key: :display_ad_id, inverse_of: :billboard_events
belongs_to :user, optional: true
self.table_name = "display_ad_events"
alias_attribute :billboard_id, :display_ad_id
CATEGORY_IMPRESSION = "impression".freeze

2
app/models/display_ad.rb
View file

@ -31,7 +31,7 @@ class DisplayAd < ApplicationRecord
enum type_of: { in_house: 0, community: 1, external: 2 }
belongs_to :organization, optional: true
has_many :billboard_events, class_name: "DisplayAdEvent", dependent: :destroy
has_many :billboard_events, dependent: :destroy
validates :placement_area, presence: true,
inclusion: { in: ALLOWED_PLACEMENT_AREAS }

2
app/models/user.rb
View file

@ -70,7 +70,7 @@ class User < ApplicationRecord
has_many :created_podcasts, class_name: "Podcast", foreign_key: :creator_id, inverse_of: :creator, dependent: :nullify
has_many :credits, dependent: :destroy
has_many :discussion_locks, dependent: :delete_all, inverse_of: :locking_user, foreign_key: :locking_user_id
has_many :billboard_events, class_name: "DisplayAdEvent", dependent: :nullify
has_many :billboard_events, dependent: :nullify
has_many :email_authorizations, dependent: :delete_all
has_many :email_messages, class_name: "Ahoy::Message", dependent: :destroy
has_many :field_test_memberships, class_name: "FieldTest::Membership", as: :participant, dependent: :destroy

2
app/services/billboard_event_rollup.rb
View file

@ -44,7 +44,7 @@ class BillboardEventRollup
attr_reader :aggregator
end
def self.rollup(date, relation: DisplayAdEvent)
def self.rollup(date, relation: BillboardEvent)
new(relation: relation).rollup(date)
end

2
app/views/articles/index.html.erb
View file

@ -27,7 +27,7 @@
<%= javascript_packs_with_chunks_tag "homePage", defer: true %>
<% cache(release_adjusted_cache_key("main-stories-index-#{params}-#{user_signed_in?}"), expires_in: 90.seconds) do %>
<% if @hero_display_ad && FeatureFlag.enabled?(:hero_billboard) %>
<%= render partial: "shared/display_ad", locals: { display_ad: @hero_display_ad, data_context_type: DisplayAdEvent::CONTEXT_TYPE_HOME } %>
<%= render partial: "shared/display_ad", locals: { display_ad: @hero_display_ad, data_context_type: BillboardEvent::CONTEXT_TYPE_HOME } %>
<% end %>
<div class="crayons-layout crayons-layout--3-cols crayons-layout--3-cols--drop-right-left" id="index-container"
data-params="<%= params.merge(sort_by: "hotness_score", sort_direction: "desc").to_json(only: %i[tag username q sort_by sort_direction]) %>" data-which="<%= @list_of %>"

4
app/views/billboards/show.html.erb
View file

@ -1,9 +1,9 @@
<% if @display_ad %>
<% if user_signed_in? %>
<%= render partial: "shared/display_ad", locals: { display_ad: @display_ad, data_context_type: DisplayAdEvent::CONTEXT_TYPE_ARTICLE } %>
<%= render partial: "shared/display_ad", locals: { display_ad: @display_ad, data_context_type: BillboardEvent::CONTEXT_TYPE_ARTICLE } %>
<% else %>
<% cache([params[:username], params[:slug], params[:placement_area]], expires_in: 15.minutes) do %>
<%= render partial: "shared/display_ad", locals: { display_ad: @display_ad, data_context_type: DisplayAdEvent::CONTEXT_TYPE_ARTICLE } %>
<%= render partial: "shared/display_ad", locals: { display_ad: @display_ad, data_context_type: BillboardEvent::CONTEXT_TYPE_ARTICLE } %>
<% end %>
<% end %>
<% end %>

12
app/views/shared/_display_ad.html.erb
View file

@ -1,8 +1,8 @@
<% if display_ad.placement_area.start_with?("feed_") %>
<div class="crayons-story crayons-story__display-ad billboard"
data-display-unit data-id="<%= display_ad.id %>"
data-category-click="<%= DisplayAdEvent::CATEGORY_CLICK %>"
data-category-impression="<%= DisplayAdEvent::CATEGORY_IMPRESSION %>"
data-category-click="<%= BillboardEvent::CATEGORY_CLICK %>"
data-category-impression="<%= BillboardEvent::CATEGORY_IMPRESSION %>"
data-context-type="<%= data_context_type %>"
data-type-of="<%= display_ad.type_of %>">
<div class="crayons-story__body">
@ -19,8 +19,8 @@
<% elsif display_ad.placement_area == "home_hero" %>
<div class="crayons-card crayons-card--secondary crayons-sponsorship crayons-story__display-ad hero-billboard"
data-display-unit data-id="<%= display_ad.id %>"
data-category-click="<%= DisplayAdEvent::CATEGORY_CLICK %>"
data-category-impression="<%= DisplayAdEvent::CATEGORY_IMPRESSION %>"
data-category-click="<%= BillboardEvent::CATEGORY_CLICK %>"
data-category-impression="<%= BillboardEvent::CATEGORY_IMPRESSION %>"
data-context-type="<%= data_context_type %>"
data-type-of="<%= display_ad.type_of %>">
<%= render partial: "shared/display_ad_header", locals: { display_ad: display_ad } %>
@ -31,8 +31,8 @@
<% else %>
<div class="crayons-card crayons-card--secondary crayons-sponsorship billboard"
data-display-unit data-id="<%= display_ad.id %>"
data-category-click="<%= DisplayAdEvent::CATEGORY_CLICK %>"
data-category-impression="<%= DisplayAdEvent::CATEGORY_IMPRESSION %>"
data-category-click="<%= BillboardEvent::CATEGORY_CLICK %>"
data-category-impression="<%= BillboardEvent::CATEGORY_IMPRESSION %>"
data-context-type="<%= data_context_type %>"
data-type-of="<%= display_ad.type_of %>">
<%= render partial: "shared/display_ad_header", locals: { display_ad: display_ad } %>

2
app/views/sidebars/_homepage_content.html.erb
View file

@ -2,7 +2,7 @@
<% cache(release_adjusted_cache_key("main-article-right-sidebar-discussions-#{params[:timeframe]}-#{user_signed_in?}"), expires_in: (params[:timeframe].blank? ? 120 : 360).seconds) do %>
<% @sidebar_ad = DisplayAd.for_display(area: "sidebar_right", user_signed_in: user_signed_in?) %>
<% if @sidebar_ad %>
<%= render partial: "shared/display_ad", locals: { display_ad: @sidebar_ad, data_context_type: DisplayAdEvent::CONTEXT_TYPE_HOME } %>
<%= render partial: "shared/display_ad", locals: { display_ad: @sidebar_ad, data_context_type: BillboardEvent::CONTEXT_TYPE_HOME } %>
<% end %>
<%= render "articles/sidebar_campaign" if Campaign.current.show_in_sidebar? %>

6
app/views/stories/_main_stories_feed.html.erb
View file

@ -10,7 +10,7 @@
<% if !user_signed_in? %>
<% @sidebar_ad_first = DisplayAd.for_display(area: "feed_first", user_signed_in: user_signed_in?) %>
<% if @sidebar_ad_first %>
<%= render partial: "shared/display_ad", locals: { display_ad: @sidebar_ad_first, data_context_type: DisplayAdEvent::CONTEXT_TYPE_HOME } %>
<%= render partial: "shared/display_ad", locals: { display_ad: @sidebar_ad_first, data_context_type: BillboardEvent::CONTEXT_TYPE_HOME } %>
<% end %>
<% end %>
@ -38,14 +38,14 @@
<% if !user_signed_in? && i == second_display_ad_position %>
<% @sidebar_ad_second = DisplayAd.for_display(area: "feed_second", user_signed_in: user_signed_in?) %>
<% if @sidebar_ad_second %>
<%= render partial: "shared/display_ad", locals: { display_ad: @sidebar_ad_second, data_context_type: DisplayAdEvent::CONTEXT_TYPE_HOME } %>
<%= render partial: "shared/display_ad", locals: { display_ad: @sidebar_ad_second, data_context_type: BillboardEvent::CONTEXT_TYPE_HOME } %>
<% end %>
<% end %>
<% if !user_signed_in? && i == third_display_ad_position %>
<% @sidebar_ad_third = DisplayAd.for_display(area: "feed_third", user_signed_in: user_signed_in?) %>
<% if @sidebar_ad_third %>
<%= render partial: "shared/display_ad", locals: { display_ad: @sidebar_ad_third, data_context_type: DisplayAdEvent::CONTEXT_TYPE_HOME } %>
<%= render partial: "shared/display_ad", locals: { display_ad: @sidebar_ad_third, data_context_type: BillboardEvent::CONTEXT_TYPE_HOME } %>
<% end %>
<% end %>
<% if story.id != @featured_story.id && story.id != @pinned_article&.id %>

4
spec/factories/display_ad_events.rb → spec/factories/billboard_events.rb
View file

@ -1,6 +1,6 @@
FactoryBot.define do
factory :display_ad_event do
category { DisplayAdEvent::CATEGORY_IMPRESSION }
factory :billboard_event do
category { BillboardEvent::CATEGORY_IMPRESSION }
context_type { "home" }
association :billboard, factory: :display_ad
end

2
spec/models/display_ad_event_spec.rb → spec/models/billboard_event_spec.rb
View file

@ -1,6 +1,6 @@
require "rails_helper"
RSpec.describe DisplayAdEvent do
RSpec.describe BillboardEvent do
it { is_expected.to validate_inclusion_of(:category).in_array(described_class::VALID_CATEGORIES) }
it { is_expected.to validate_inclusion_of(:context_type).in_array(described_class::VALID_CONTEXT_TYPES) }

30
spec/requests/billboard_events_spec.rb
View file

@ -15,8 +15,8 @@ RSpec.describe "BillboardEvents" do
post "/billboard_events", params: {
billboard_event: {
billboard_id: display_ad.id,
context_type: DisplayAdEvent::CONTEXT_TYPE_HOME,
category: DisplayAdEvent::CATEGORY_CLICK
context_type: BillboardEvent::CONTEXT_TYPE_HOME,
category: BillboardEvent::CATEGORY_CLICK
}
}
expect(display_ad.reload.clicks_count).to eq(1)
@ -26,8 +26,8 @@ RSpec.describe "BillboardEvents" do
post "/billboard_events", params: {
display_ad_event: {
display_ad_id: display_ad.id,
context_type: DisplayAdEvent::CONTEXT_TYPE_HOME,
category: DisplayAdEvent::CATEGORY_CLICK
context_type: BillboardEvent::CONTEXT_TYPE_HOME,
category: BillboardEvent::CATEGORY_CLICK
}
}
expect(display_ad.reload.clicks_count).to eq(1)
@ -37,21 +37,21 @@ RSpec.describe "BillboardEvents" do
post "/billboard_events", params: {
billboard_event: {
billboard_id: display_ad.id,
context_type: DisplayAdEvent::CONTEXT_TYPE_HOME,
category: DisplayAdEvent::CATEGORY_IMPRESSION
context_type: BillboardEvent::CONTEXT_TYPE_HOME,
category: BillboardEvent::CATEGORY_IMPRESSION
}
}
expect(display_ad.reload.impressions_count).to eq(1)
end
it "creates a display ad success rate" do
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)
ad_event_params = { billboard_id: display_ad.id, context_type: BillboardEvent::CONTEXT_TYPE_HOME }
impression_params = ad_event_params.merge(category: BillboardEvent::CATEGORY_IMPRESSION, user: user)
create_list(:billboard_event, 4, impression_params)
post(
"/billboard_events",
params: { billboard_event: ad_event_params.merge(category: DisplayAdEvent::CATEGORY_CLICK) },
params: { billboard_event: ad_event_params.merge(category: BillboardEvent::CATEGORY_CLICK) },
)
expect(display_ad.reload.success_rate).to eq(0.25)
@ -61,19 +61,19 @@ RSpec.describe "BillboardEvents" do
post "/billboard_events", params: {
billboard_event: {
billboard_id: display_ad.id,
context_type: DisplayAdEvent::CONTEXT_TYPE_HOME,
category: DisplayAdEvent::CATEGORY_IMPRESSION
context_type: BillboardEvent::CONTEXT_TYPE_HOME,
category: BillboardEvent::CATEGORY_IMPRESSION
}
}
expect(DisplayAdEvent.last.user_id).to eq(user.id)
expect(BillboardEvent.last.user_id).to eq(user.id)
end
it "uses a ThrottledCall for data updates" do
post "/billboard_events", params: {
billboard_event: {
billboard_id: display_ad.id,
context_type: DisplayAdEvent::CONTEXT_TYPE_HOME,
category: DisplayAdEvent::CATEGORY_IMPRESSION
context_type: BillboardEvent::CONTEXT_TYPE_HOME,
category: BillboardEvent::CATEGORY_IMPRESSION
}
}

76
spec/services/billboard_event_rollup_spec.rb
View file

@ -7,9 +7,9 @@ RSpec.describe BillboardEventRollup, type: :service do
let(:user2) { create(:user) }
def override_timestamps
DisplayAdEvent.record_timestamps = false
BillboardEvent.record_timestamps = false
yield
DisplayAdEvent.record_timestamps = true
BillboardEvent.record_timestamps = true
end
def days_ago_as_range(num)
@ -18,24 +18,24 @@ RSpec.describe BillboardEventRollup, type: :service do
it "fails if new attributes would be lost" do
attributes_considered = described_class::ATTRIBUTES_PRESERVED + described_class::ATTRIBUTES_DESTROYED
expect(DisplayAdEvent.column_names.map(&:to_sym)).to match_array(attributes_considered)
expect(BillboardEvent.column_names.map(&:to_sym)).to match_array(attributes_considered)
end
context "when compacting many rows" do
before do
override_timestamps do
create(:display_ad_event, created_at: Date.current - 2, billboard: ad1, user_id: nil, updated_at: Date.current)
create(:display_ad_event, created_at: Date.current - 2, billboard: ad1, user_id: nil, updated_at: Date.current)
create(:display_ad_event, created_at: Date.current - 2, billboard: ad1, user_id: nil, updated_at: Date.current)
create(:billboard_event, created_at: Date.current - 2, billboard: ad1, user_id: nil, updated_at: Date.current)
create(:billboard_event, created_at: Date.current - 2, billboard: ad1, user_id: nil, updated_at: Date.current)
create(:billboard_event, created_at: Date.current - 2, billboard: ad1, user_id: nil, updated_at: Date.current)
create(:display_ad_event, created_at: Date.current - 2, billboard: ad1, user_id: user1.id,
updated_at: Date.current)
create(:display_ad_event, created_at: Date.current - 2, billboard: ad2, user_id: nil, updated_at: Date.current)
create(:billboard_event, created_at: Date.current - 2, billboard: ad1, user_id: user1.id,
updated_at: Date.current)
create(:billboard_event, created_at: Date.current - 2, billboard: ad2, user_id: nil, updated_at: Date.current)
end
end
it "compacts one day's billboard_events" do
expect(DisplayAdEvent.where(created_at: days_ago_as_range(2)).count).to eq(5)
expect(BillboardEvent.where(created_at: days_ago_as_range(2)).count).to eq(5)
described_class.rollup(Date.current - 2)
@ -44,7 +44,7 @@ RSpec.describe BillboardEventRollup, type: :service do
[ad1.id, user1.id, 1],
[ad2.id, nil, 1],
]
results_mapped = DisplayAdEvent.where(created_at: days_ago_as_range(2)).map do |event|
results_mapped = BillboardEvent.where(created_at: days_ago_as_range(2)).map do |event|
[event.billboard_id, event.user_id, event.counts_for]
end
expect(results_mapped).to match_array(expectations)
@ -53,14 +53,14 @@ RSpec.describe BillboardEventRollup, type: :service do
# separate category
it "groups by category" do
create(:display_ad_event, category: "impression", billboard: ad1, user_id: nil)
create(:display_ad_event, category: "impression", billboard: ad1, user_id: nil)
create(:display_ad_event, category: "impression", billboard: ad1, user_id: nil)
create(:display_ad_event, category: "click", billboard: ad1, user_id: nil)
create(:display_ad_event, category: "click", billboard: ad1, user_id: nil)
create(:billboard_event, category: "impression", billboard: ad1, user_id: nil)
create(:billboard_event, category: "impression", billboard: ad1, user_id: nil)
create(:billboard_event, category: "impression", billboard: ad1, user_id: nil)
create(:billboard_event, category: "click", billboard: ad1, user_id: nil)
create(:billboard_event, category: "click", billboard: ad1, user_id: nil)
described_class.rollup(Date.current)
results = DisplayAdEvent.where(created_at: Date.current.all_day)
results = BillboardEvent.where(created_at: Date.current.all_day)
by_category = results.index_by { |r| r["category"] }
expect(by_category["impression"]["counts_for"]).to eq(3)
expect(by_category["click"]["counts_for"]).to eq(2)
@ -68,14 +68,14 @@ RSpec.describe BillboardEventRollup, type: :service 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)
create(:display_ad_event, billboard: ad2, user_id: nil)
create(:display_ad_event, billboard: ad2, user_id: nil)
create(:billboard_event, billboard: ad1, user_id: nil)
create(:billboard_event, billboard: ad1, user_id: nil)
create(:billboard_event, billboard: ad1, user_id: nil)
create(:billboard_event, billboard: ad2, user_id: nil)
create(:billboard_event, billboard: ad2, user_id: nil)
described_class.rollup(Date.current)
results = DisplayAdEvent.where(created_at: Date.current.all_day)
results = BillboardEvent.where(created_at: Date.current.all_day)
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)
@ -83,17 +83,17 @@ RSpec.describe BillboardEventRollup, type: :service do
# separate user_id / null
it "groups by user_id (including null / logged-out user)" do
create(:display_ad_event, billboard: ad1, user: user1)
create(:display_ad_event, billboard: ad1, user: user2)
create(:display_ad_event, billboard: ad1, user: user2)
create(:display_ad_event, billboard: ad1, user: nil)
create(:display_ad_event, billboard: ad1, user: nil)
create(:display_ad_event, billboard: ad1, user: nil)
create(:display_ad_event, billboard: ad1, user: nil)
create(:display_ad_event, billboard: ad1, user: nil)
create(:billboard_event, billboard: ad1, user: user1)
create(:billboard_event, billboard: ad1, user: user2)
create(:billboard_event, billboard: ad1, user: user2)
create(:billboard_event, billboard: ad1, user: nil)
create(:billboard_event, billboard: ad1, user: nil)
create(:billboard_event, billboard: ad1, user: nil)
create(:billboard_event, billboard: ad1, user: nil)
create(:billboard_event, billboard: ad1, user: nil)
described_class.rollup(Date.current)
results = DisplayAdEvent.where(created_at: Date.current.all_day)
results = BillboardEvent.where(created_at: Date.current.all_day)
by_user = results.index_by { |r| r["user_id"] }
expect(by_user[user1.id]["counts_for"]).to eq(1)
expect(by_user[user2.id]["counts_for"]).to eq(2)
@ -102,14 +102,14 @@ RSpec.describe BillboardEventRollup, type: :service do
# sums counts_for > 1
it "counts previously crunched" do
create(:display_ad_event, billboard: ad1, counts_for: 10, user_id: nil)
create(:display_ad_event, billboard: ad1, counts_for: 15, user_id: nil)
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)
create(:billboard_event, billboard: ad1, counts_for: 10, user_id: nil)
create(:billboard_event, billboard: ad1, counts_for: 15, user_id: nil)
create(:billboard_event, billboard: ad1, user_id: nil)
create(:billboard_event, billboard: ad1, user_id: nil)
create(:billboard_event, billboard: ad1, user_id: nil)
described_class.rollup(Date.current)
results = DisplayAdEvent.where(created_at: Date.current.all_day)
results = BillboardEvent.where(created_at: Date.current.all_day)
expect(results.count).to eq(1)
expect(results.first.counts_for).to eq(28)
end