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

Renamed display_ad_events variables and associations (#19788)

Browse source 
* Renamed stuff related to display_ad_events (except the model)

* Updated user billboard_events association spec

* Fixed display_ad_event factory
This commit is contained in:
Anna Buianova 2023-07-21 14:59:28 +03:00 • committed by GitHub
parent 9a6f04bd37
commit e659dbfc00
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 94 additions and 94 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -29,7 +29,7 @@ function trackAdImpression(adBox) {
var csrfToken = tokenMeta && tokenMeta.getAttribute('content');
var dataBody = {
display_ad_event: {
billboard_event: {
display_ad_id: adBox.dataset.id,
context_type: adBox.dataset.contextType,
category: adBox.dataset.categoryImpression,
@ -64,7 +64,7 @@ function trackAdClick(adBox) {
var csrfToken = tokenMeta && tokenMeta.getAttribute('content');
var dataBody = {
display_ad_event: {
billboard_event: {
display_ad_id: adBox.dataset.id,
context_type: adBox.dataset.contextType,
category: adBox.dataset.categoryClick,

24
app/controllers/billboard_events_controller.rb
View file

@ -4,27 +4,27 @@ class BillboardEventsController < ApplicationMetalController
def create
# Only tracking for logged in users at the moment
display_ad_event_create_params = display_ad_event_params.merge(user_id: session_current_user_id)
@display_ad_event = DisplayAdEvent.create(display_ad_event_create_params)
billboard_event_create_params = billboard_event_params.merge(user_id: session_current_user_id)
@billboard_event = DisplayAdEvent.create(billboard_event_create_params)
update_display_ads_data
update_billboards_data
head :ok
end
private
def update_display_ads_data
display_ad_event_id = display_ad_event_params[:display_ad_id]
def update_billboards_data
billboard_event_id = billboard_event_params[:display_ad_id]
ThrottledCall.perform("display_ads_data_update-#{display_ad_event_id}", throttle_for: 15.minutes) do
@display_ad = DisplayAd.find(display_ad_event_id)
ThrottledCall.perform("billboards_data_update-#{billboard_event_id}", throttle_for: 15.minutes) do
@billboard = DisplayAd.find(billboard_event_id)
num_impressions = @display_ad.display_ad_events.impressions.sum(:counts_for)
num_clicks = @display_ad.display_ad_events.clicks.sum(:counts_for)
num_impressions = @billboard.billboard_events.impressions.sum(:counts_for)
num_clicks = @billboard.billboard_events.clicks.sum(:counts_for)
rate = num_clicks.to_f / num_impressions
@display_ad.update_columns(
@billboard.update_columns(
success_rate: rate,
clicks_count: num_clicks,
impressions_count: num_impressions,
@ -32,7 +32,7 @@ class BillboardEventsController < ApplicationMetalController
end
end
def display_ad_event_params
params[:display_ad_event].slice(:context_type, :category, :display_ad_id)
def billboard_event_params
params[:billboard_event].slice(:context_type, :category, :display_ad_id)
end
end

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

@ -60,7 +60,7 @@ function trackAdImpression(adBox) {
const csrfToken = tokenMeta && tokenMeta.getAttribute('content');
const dataBody = {
display_ad_event: {
billboard_event: {
display_ad_id: adBox.dataset.id,
context_type: adBox.dataset.contextType,
category: adBox.dataset.categoryImpression,
@ -96,7 +96,7 @@ function trackAdClick(adBox) {
const csrfToken = tokenMeta && tokenMeta.getAttribute('content');
const dataBody = {
display_ad_event: {
billboard_event: {
display_ad_id: adBox.dataset.id,
context_type: adBox.dataset.contextType,
category: adBox.dataset.categoryClick,

45
app/models/display_ad.rb
View file

@ -27,12 +27,11 @@ class DisplayAd < ApplicationRecord
RANDOM_RANGE_MAX_FALLBACK = 5
NEW_AND_PRIORITY_RANGE_MAX_FALLBACK = 35
enum display_to: { all: 0, logged_in: 1, logged_out: 2 }, _prefix: true
enum type_of: { in_house: 0, community: 1, external: 2 }
belongs_to :organization, optional: true
has_many :display_ad_events, dependent: :destroy
has_many :billboard_events, class_name: "DisplayAdEvent", dependent: :destroy
validates :placement_area, presence: true,
inclusion: { in: ALLOWED_PLACEMENT_AREAS }
@ -97,6 +96,27 @@ class DisplayAd < ApplicationRecord
end
end
# Temporary ENV configs, to eventually be replaced by permanent configurations
# once we determine what the appropriate long-term config approach is.
def self.low_impression_count(placement_area)
ApplicationConfig["LOW_IMPRESSION_COUNT_FOR_#{placement_area.upcase}"] ||
ApplicationConfig["LOW_IMPRESSION_COUNT"] ||
LOW_IMPRESSION_COUNT
end
def self.random_range_max(placement_area)
ApplicationConfig["SELDOM_SEEN_MIN_FOR_#{placement_area.upcase}"] ||
ApplicationConfig["SELDOM_SEEN_MIN"] ||
RANDOM_RANGE_MAX_FALLBACK
end
def self.new_and_priority_range_max(placement_area)
ApplicationConfig["SELDOM_SEEN_MAX_FOR_#{placement_area.upcase}"] ||
ApplicationConfig["SELDOM_SEEN_MAX"] ||
NEW_AND_PRIORITY_RANGE_MAX_FALLBACK
end
def human_readable_placement_area
ALLOWED_PLACEMENT_AREAS_HUMAN_READABLE[ALLOWED_PLACEMENT_AREAS.find_index(placement_area)]
end
@ -143,27 +163,6 @@ class DisplayAd < ApplicationRecord
write_attribute :exclude_article_ids, (adjusted_input || [])
end
# Temporary ENV configs, to eventually be replaced by permanent configurations
# once we determine what the appropriate long-term config approach is.
def self.low_impression_count(placement_area)
ApplicationConfig["LOW_IMPRESSION_COUNT_FOR_#{placement_area.upcase}"] ||
ApplicationConfig["LOW_IMPRESSION_COUNT"] ||
LOW_IMPRESSION_COUNT
end
def self.random_range_max(placement_area)
ApplicationConfig["SELDOM_SEEN_MIN_FOR_#{placement_area.upcase}"] ||
ApplicationConfig["SELDOM_SEEN_MIN"] ||
RANDOM_RANGE_MAX_FALLBACK
end
def self.new_and_priority_range_max(placement_area)
ApplicationConfig["SELDOM_SEEN_MAX_FOR_#{placement_area.upcase}"]||
ApplicationConfig["SELDOM_SEEN_MAX"] ||
NEW_AND_PRIORITY_RANGE_MAX_FALLBACK
end
private
def generate_display_ad_name

2
app/models/display_ad_event.rb
View file

@ -2,7 +2,7 @@
# :delete for the relationship. That means no before/after
# destroy callbacks will be called on this object.
class DisplayAdEvent < ApplicationRecord
belongs_to :display_ad
belongs_to :billboard, class_name: "DisplayAd", foreign_key: "display_ad_id", inverse_of: :billboard_events
belongs_to :user, optional: true
CATEGORY_IMPRESSION = "impression".freeze

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 :display_ad_events, dependent: :nullify
has_many :billboard_events, class_name: "DisplayAdEvent", 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/display_ad_event_rollup.rb → app/services/billboard_event_rollup.rb
View file

@ -1,4 +1,4 @@
class DisplayAdEventRollup
class BillboardEventRollup
ATTRIBUTES_PRESERVED = %i[user_id display_ad_id category context_type created_at].freeze
ATTRIBUTES_DESTROYED = %i[id counts_for updated_at].freeze

2
app/services/users/delete_activity.rb
View file

@ -10,7 +10,7 @@ module Users
user.blocker_blocks.delete_all
user.blocked_blocks.delete_all
user.authored_notes.delete_all
user.display_ad_events.delete_all
user.billboard_events.delete_all
user.email_messages.delete_all
user.html_variants.delete_all
user.poll_skips.delete_all

4
app/workers/display_ad_event_rollup_worker.rb → app/workers/billboard_event_rollup_worker.rb
View file

@ -1,10 +1,10 @@
class DisplayAdEventRollupWorker
class BillboardEventRollupWorker
include Sidekiq::Worker
sidekiq_options queue: :low_priority
def perform
month_ago = Date.current - 32.days
DisplayAdEventRollup.rollup month_ago
BillboardEventRollup.rollup month_ago
end
end

4
config/schedule.yml
View file

@ -146,10 +146,10 @@ notify_about_published_articles:
description: "Sends notifications about the new published articles (runs every minute)"
cron: "*/5 * * * *"
class: "Articles::PublishWorker"
display_ad_event_rollup:
billboard_event_rollup:
description: "Compact rows in the display_ad_events table"
cron: "5 3 * * *"
class: "DisplayAdEventRollupWorker"
class: "BillboardEventRollupWorker"
audience_segment_refresh:
description: "Refresh audience segmentation for all segments"
cron: "10 3 * * *"

2
spec/factories/display_ad_events.rb
View file

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

5
spec/models/display_ad_spec.rb
View file

@ -14,7 +14,7 @@ RSpec.describe DisplayAd do
subject { display_ad }
it { is_expected.to belong_to(:organization).optional }
it { is_expected.to have_many(:display_ad_events).dependent(:destroy) }
it { is_expected.to have_many(:billboard_events).dependent(:destroy) }
it { is_expected.to validate_presence_of(:placement_area) }
it { is_expected.to validate_presence_of(:body_markdown) }
@ -337,7 +337,8 @@ RSpec.describe DisplayAd do
let(:low_impression_count) { DisplayAd::LOW_IMPRESSION_COUNT }
let!(:low_impression_ad) { create(:display_ad, impressions_count: low_impression_count - 1) }
let!(:high_impression_ad) { create(:display_ad, impressions_count: low_impression_count + 1) }
let!(:priority_ad) { create(:display_ad, priority: true, impressions_count: low_impression_count + 1) }
before { create(:display_ad, priority: true, impressions_count: low_impression_count + 1) }
it "includes ads with impressions count less than LOW_IMPRESSION_COUNT" do
expect(described_class.seldom_seen("sidebar_left")).to include(low_impression_ad)

2
spec/models/user_spec.rb
View file

@ -79,7 +79,7 @@ RSpec.describe User do
it { is_expected.to have_many(:comments).dependent(:destroy) }
it { is_expected.to have_many(:credits).dependent(:destroy) }
it { is_expected.to have_many(:discussion_locks).dependent(:delete_all) }
it { is_expected.to have_many(:display_ad_events).dependent(:nullify) }
it { is_expected.to have_many(:billboard_events).dependent(:nullify) }
it { is_expected.to have_many(:email_authorizations).dependent(:delete_all) }
it { is_expected.to have_many(:email_messages).class_name("Ahoy::Message").dependent(:destroy) }
it { is_expected.to have_many(:field_test_memberships).class_name("FieldTest::Membership").dependent(:destroy) }

12
spec/requests/billboard_events_spec.rb
View file

@ -13,7 +13,7 @@ RSpec.describe "BillboardEvents" do
it "creates a display ad click event" do
post "/billboard_events", params: {
display_ad_event: {
billboard_event: {
display_ad_id: display_ad.id,
context_type: DisplayAdEvent::CONTEXT_TYPE_HOME,
category: DisplayAdEvent::CATEGORY_CLICK
@ -24,7 +24,7 @@ RSpec.describe "BillboardEvents" do
it "creates a display ad impression event" do
post "/billboard_events", params: {
display_ad_event: {
billboard_event: {
display_ad_id: display_ad.id,
context_type: DisplayAdEvent::CONTEXT_TYPE_HOME,
category: DisplayAdEvent::CATEGORY_IMPRESSION
@ -40,7 +40,7 @@ RSpec.describe "BillboardEvents" do
post(
"/billboard_events",
params: { display_ad_event: ad_event_params.merge(category: DisplayAdEvent::CATEGORY_CLICK) },
params: { billboard_event: ad_event_params.merge(category: DisplayAdEvent::CATEGORY_CLICK) },
)
expect(display_ad.reload.success_rate).to eq(0.25)
@ -48,7 +48,7 @@ RSpec.describe "BillboardEvents" do
it "assigns event to current user" do
post "/billboard_events", params: {
display_ad_event: {
billboard_event: {
display_ad_id: display_ad.id,
context_type: DisplayAdEvent::CONTEXT_TYPE_HOME,
category: DisplayAdEvent::CATEGORY_IMPRESSION
@ -59,7 +59,7 @@ RSpec.describe "BillboardEvents" do
it "uses a ThrottledCall for data updates" do
post "/billboard_events", params: {
display_ad_event: {
billboard_event: {
display_ad_id: display_ad.id,
context_type: DisplayAdEvent::CONTEXT_TYPE_HOME,
category: DisplayAdEvent::CATEGORY_IMPRESSION
@ -67,7 +67,7 @@ RSpec.describe "BillboardEvents" do
}
expect(ThrottledCall).to have_received(:perform)
.with("display_ads_data_update-#{display_ad.id}", throttle_for: instance_of(ActiveSupport::Duration))
.with("billboards_data_update-#{display_ad.id}", throttle_for: instance_of(ActiveSupport::Duration))
end
end
end

66
spec/services/display_ad_event_rollup_spec.rb → spec/services/billboard_event_rollup_spec.rb
View file

@ -1,6 +1,6 @@
require "rails_helper"
RSpec.describe DisplayAdEventRollup, type: :service do
RSpec.describe BillboardEventRollup, type: :service do
let(:ad1) { create(:display_ad) }
let(:ad2) { create(:display_ad) }
let(:user1) { create(:user) }
@ -17,24 +17,24 @@ RSpec.describe DisplayAdEventRollup, type: :service do
end
it "fails if new attributes would be lost" do
attributes_considered = DisplayAdEventRollup::ATTRIBUTES_PRESERVED + DisplayAdEventRollup::ATTRIBUTES_DESTROYED
expect(DisplayAdEvent.column_names.map(&:to_sym)).to contain_exactly(*attributes_considered)
attributes_considered = described_class::ATTRIBUTES_PRESERVED + described_class::ATTRIBUTES_DESTROYED
expect(DisplayAdEvent.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, display_ad: ad1, user_id: nil, updated_at: Date.current)
create(:display_ad_event, created_at: Date.current - 2, display_ad: ad1, user_id: nil, updated_at: Date.current)
create(:display_ad_event, created_at: Date.current - 2, display_ad: 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(: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, display_ad: ad1, user_id: user1.id,
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, display_ad: ad2, user_id: nil, updated_at: Date.current)
create(:display_ad_event, created_at: Date.current - 2, billboard: ad2, user_id: nil, updated_at: Date.current)
end
end
it "compacts one day's display_ad_events" do
it "compacts one day's billboard_events" do
expect(DisplayAdEvent.where(created_at: days_ago_as_range(2)).count).to eq(5)
described_class.rollup(Date.current - 2)
@ -47,17 +47,17 @@ RSpec.describe DisplayAdEventRollup, type: :service do
results_mapped = DisplayAdEvent.where(created_at: days_ago_as_range(2)).map do |event|
[event.display_ad_id, event.user_id, event.counts_for]
end
expect(results_mapped).to contain_exactly(*expectations)
expect(results_mapped).to match_array(expectations)
end
end
# separate category
it "groups by category" do
create(:display_ad_event, category: "impression", display_ad: ad1, user_id: nil)
create(:display_ad_event, category: "impression", display_ad: ad1, user_id: nil)
create(:display_ad_event, category: "impression", display_ad: ad1, user_id: nil)
create(:display_ad_event, category: "click", display_ad: ad1, user_id: nil)
create(:display_ad_event, category: "click", display_ad: 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: "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)
described_class.rollup(Date.current)
results = DisplayAdEvent.where(created_at: Date.current.all_day)
@ -68,11 +68,11 @@ RSpec.describe DisplayAdEventRollup, type: :service do
# separate display_ad_id
it "groups by display_ad_id" do
create(:display_ad_event, display_ad: ad1, user_id: nil)
create(:display_ad_event, display_ad: ad1, user_id: nil)
create(:display_ad_event, display_ad: ad1, user_id: nil)
create(:display_ad_event, display_ad: ad2, user_id: nil)
create(:display_ad_event, display_ad: ad2, 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(:display_ad_event, billboard: ad2, user_id: nil)
create(:display_ad_event, billboard: ad2, user_id: nil)
described_class.rollup(Date.current)
results = DisplayAdEvent.where(created_at: Date.current.all_day)
@ -83,14 +83,14 @@ RSpec.describe DisplayAdEventRollup, type: :service do
# separate user_id / null
it "groups by user_id (including null / logged-out user)" do
create(:display_ad_event, display_ad: ad1, user: user1)
create(:display_ad_event, display_ad: ad1, user: user2)
create(:display_ad_event, display_ad: ad1, user: user2)
create(:display_ad_event, display_ad: ad1, user: nil)
create(:display_ad_event, display_ad: ad1, user: nil)
create(:display_ad_event, display_ad: ad1, user: nil)
create(:display_ad_event, display_ad: ad1, user: nil)
create(:display_ad_event, display_ad: ad1, user: nil)
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)
described_class.rollup(Date.current)
results = DisplayAdEvent.where(created_at: Date.current.all_day)
@ -102,11 +102,11 @@ RSpec.describe DisplayAdEventRollup, type: :service do
# sums counts_for > 1
it "counts previously crunched" do
create(:display_ad_event, display_ad: ad1, counts_for: 10, user_id: nil)
create(:display_ad_event, display_ad: ad1, counts_for: 15, user_id: nil)
create(:display_ad_event, display_ad: ad1, user_id: nil)
create(:display_ad_event, display_ad: ad1, user_id: nil)
create(:display_ad_event, display_ad: ad1, user_id: nil)
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)
described_class.rollup(Date.current)
results = DisplayAdEvent.where(created_at: Date.current.all_day)

2
spec/services/users/delete_spec.rb
View file

@ -85,7 +85,7 @@ RSpec.describe Users::Delete, type: :service do
audit_logs
banished_users
created_podcasts
display_ad_events
billboard_events
offender_feedback_messages
page_views
rating_votes

6
spec/workers/display_ad_event_rollup_worker_spec.rb → spec/workers/billboard_event_rollup_worker_spec.rb
View file

@ -1,10 +1,10 @@
require "rails_helper"
RSpec.describe DisplayAdEventRollupWorker, type: :worker do
RSpec.describe BillboardEventRollupWorker, type: :worker do
subject(:worker) { described_class.new }
before do
allow(DisplayAdEventRollup).to receive(:rollup)
allow(BillboardEventRollup).to receive(:rollup)
end
include_examples "#enqueues_on_correct_queue", "low_priority"
@ -13,7 +13,7 @@ RSpec.describe DisplayAdEventRollupWorker, type: :worker do
it "rollups one month ago" do
month_ago = Date.current - 32.days
worker.perform
expect(DisplayAdEventRollup).to have_received(:rollup).with(month_ago)
expect(BillboardEventRollup).to have_received(:rollup).with(month_ago)
end
end
end