Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions app/models/billboard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ class Billboard < ApplicationRecord
def self.for_display(area:, user_signed_in:, user_id: nil, article: nil, user_tags: nil,
location: nil, cookies_allowed: false, page_id: nil, user_agent: nil,
role_names: nil, prefer_paired_with_billboard_id: nil)
# Check delivery rate configuration first - return nil early if rate check fails
return unless BillboardPlacementAreaConfig.should_fetch_billboard?(
placement_area: area,
user_signed_in: user_signed_in,
)

permit_adjacent = article ? article.permit_adjacent_sponsors? : true

billboards_for_display = Billboards::FilteredAdsQuery.call(
Expand Down
60 changes: 60 additions & 0 deletions app/models/billboard_placement_area_config.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
class BillboardPlacementAreaConfig < ApplicationRecord
validates :placement_area, presence: true, uniqueness: true,
inclusion: { in: Billboard::ALLOWED_PLACEMENT_AREAS }
validates :signed_in_rate, presence: true, numericality: {
greater_than_or_equal_to: 0,
less_than_or_equal_to: 100
}
validates :signed_out_rate, presence: true, numericality: {
greater_than_or_equal_to: 0,
less_than_or_equal_to: 100
}

# Cache key for storing all configs
CACHE_KEY = "billboard_placement_area_configs".freeze
CACHE_EXPIRY = 1.hour

after_destroy :bust_cache
after_save :bust_cache

# Get delivery rate for a specific placement area and user state
def self.delivery_rate_for(placement_area:, user_signed_in:)
config = config_for_placement_area(placement_area)
return 100 if config.blank? # Default to 100% if no config exists

user_signed_in ? config.signed_in_rate : config.signed_out_rate
end

# Check if we should fetch a billboard based on delivery rate
def self.should_fetch_billboard?(placement_area:, user_signed_in:)
rate = delivery_rate_for(placement_area: placement_area, user_signed_in: user_signed_in)
return true if rate >= 100 # Always fetch if rate is 100%
return false if rate <= 0 # Never fetch if rate is 0%

# Randomly decide based on the rate (e.g., 10% rate means 10% chance to fetch)
rand(100) < rate
end

# Get config for a specific placement area (cached)
def self.config_for_placement_area(placement_area)
all_configs[placement_area]
end

# Get all configs (cached)
def self.all_configs
Rails.cache.fetch(CACHE_KEY, expires_in: CACHE_EXPIRY) do
all.index_by(&:placement_area)
end
end

# Bust the cache when configs are modified
def self.bust_cache
Rails.cache.delete(CACHE_KEY)
end

private

def bust_cache
self.class.bust_cache
end
end
2 changes: 1 addition & 1 deletion app/views/shared/_billboard.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
</div>
</div>
<% elsif billboard.placement_area == "post_body_bottom" %>
<% long_bb = billboard.processed_html_final.size > 750 %>
<% long_bb = billboard.processed_html_final.size > 750 && billboard.processed_html_final.exclude?("ltag-survey") %>
<div class="crayons-card crayons-card--secondary crayons-bb js-billboard body-billboard mb-2 mt-6"
style="<%= billboard.style_string %>"
data-display-unit data-id="<%= billboard.id %>"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
class CreateBillboardPlacementAreaConfigs < ActiveRecord::Migration[7.0]
def change
create_table :billboard_placement_area_configs do |t|
t.string :placement_area, null: false
t.integer :signed_in_rate, null: false, default: 100
t.integer :signed_out_rate, null: false, default: 100

t.timestamps
end
add_index :billboard_placement_area_configs, :placement_area, unique: true
end
end
11 changes: 10 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.0].define(version: 2025_09_04_134417) do
ActiveRecord::Schema[7.0].define(version: 2025_09_04_154635) do
# These are extensions that must be enabled in order to support this database
enable_extension "citext"
enable_extension "ltree"
Expand Down Expand Up @@ -254,6 +254,15 @@
t.index ["username"], name: "index_banished_users_on_username", unique: true
end

create_table "billboard_placement_area_configs", force: :cascade do |t|
t.datetime "created_at", null: false
t.string "placement_area", null: false
t.integer "signed_in_rate", default: 100, null: false
t.integer "signed_out_rate", default: 100, null: false
t.datetime "updated_at", null: false
t.index ["placement_area"], name: "index_billboard_placement_area_configs_on_placement_area", unique: true
end

create_table "blazer_audits", force: :cascade do |t|
t.datetime "created_at", precision: nil
t.string "data_source"
Expand Down
175 changes: 175 additions & 0 deletions spec/models/billboard_placement_area_config_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
require "rails_helper"

RSpec.describe BillboardPlacementAreaConfig, type: :model do
describe "validations" do
it "validates presence of placement_area" do
config = described_class.new(signed_in_rate: 50, signed_out_rate: 50)
expect(config).not_to be_valid
expect(config.errors[:placement_area]).to include("can't be blank")
end

it "validates uniqueness of placement_area" do
described_class.create!(placement_area: "sidebar_left", signed_in_rate: 50, signed_out_rate: 50)
duplicate_config = described_class.new(placement_area: "sidebar_left", signed_in_rate: 75, signed_out_rate: 75)
expect(duplicate_config).not_to be_valid
expect(duplicate_config.errors[:placement_area]).to include("has already been taken")
end

it "validates inclusion of placement_area in allowed areas" do
config = described_class.new(placement_area: "invalid_area", signed_in_rate: 50, signed_out_rate: 50)
expect(config).not_to be_valid
expect(config.errors[:placement_area]).to include("is not included in the list")
end

it "validates presence of signed_in_rate" do
config = described_class.new(placement_area: "sidebar_left", signed_out_rate: 50, signed_in_rate: nil)
expect(config).not_to be_valid
expect(config.errors[:signed_in_rate]).to include("can't be blank")
end

it "validates presence of signed_out_rate" do
config = described_class.new(placement_area: "sidebar_left", signed_in_rate: 50, signed_out_rate: nil)
expect(config).not_to be_valid
expect(config.errors[:signed_out_rate]).to include("can't be blank")
end

it "validates signed_in_rate is between 0 and 100" do
config = described_class.new(placement_area: "sidebar_left", signed_in_rate: 150, signed_out_rate: 50)
expect(config).not_to be_valid
expect(config.errors[:signed_in_rate]).to include("must be less than or equal to 100")

config = described_class.new(placement_area: "sidebar_left", signed_in_rate: -10, signed_out_rate: 50)
expect(config).not_to be_valid
expect(config.errors[:signed_in_rate]).to include("must be greater than or equal to 0")
end

it "validates signed_out_rate is between 0 and 100" do
config = described_class.new(placement_area: "sidebar_left", signed_in_rate: 50, signed_out_rate: 150)
expect(config).not_to be_valid
expect(config.errors[:signed_out_rate]).to include("must be less than or equal to 100")

config = described_class.new(placement_area: "sidebar_left", signed_in_rate: 50, signed_out_rate: -10)
expect(config).not_to be_valid
expect(config.errors[:signed_out_rate]).to include("must be greater than or equal to 0")
end

it "is valid with valid attributes" do
config = described_class.new(placement_area: "sidebar_left", signed_in_rate: 50, signed_out_rate: 75)
expect(config).to be_valid
end
end

describe ".delivery_rate_for" do
let!(:config) { described_class.create!(placement_area: "sidebar_left", signed_in_rate: 80, signed_out_rate: 60) }

it "returns the signed_in_rate for signed in users" do
rate = described_class.delivery_rate_for(placement_area: "sidebar_left", user_signed_in: true)
expect(rate).to eq(80)
end

it "returns the signed_out_rate for signed out users" do
rate = described_class.delivery_rate_for(placement_area: "sidebar_left", user_signed_in: false)
expect(rate).to eq(60)
end

it "returns 100 for placement areas without config" do
rate = described_class.delivery_rate_for(placement_area: "nonexistent_area", user_signed_in: true)
expect(rate).to eq(100)
end
end

describe ".should_fetch_billboard?" do
let!(:config) { described_class.create!(placement_area: "sidebar_left", signed_in_rate: 50, signed_out_rate: 25) }

it "always returns true for 100% rate" do
config.update!(signed_in_rate: 100)
result = described_class.should_fetch_billboard?(placement_area: "sidebar_left", user_signed_in: true)
expect(result).to be true
end

it "always returns false for 0% rate" do
config.update!(signed_in_rate: 0)
result = described_class.should_fetch_billboard?(placement_area: "sidebar_left", user_signed_in: true)
expect(result).to be false
end

it "returns true for placement areas without config (default 100%)" do
result = described_class.should_fetch_billboard?(placement_area: "nonexistent_area", user_signed_in: true)
expect(result).to be true
end

it "uses the correct rate based on user sign-in status" do
# Mock rand to return 30 (which is < 50 but > 25)
allow_any_instance_of(Object).to receive(:rand).with(100).and_return(30)

signed_in_result = described_class.should_fetch_billboard?(placement_area: "sidebar_left", user_signed_in: true)
signed_out_result = described_class.should_fetch_billboard?(placement_area: "sidebar_left", user_signed_in: false)

expect(signed_in_result).to be true # 30 < 50
expect(signed_out_result).to be false # 30 > 25
end

it "returns consistent results for 50% rate over multiple calls" do
config.update!(signed_in_rate: 50, signed_out_rate: 50)

# Mock rand to return 30 (should return true) and 70 (should return false)
allow_any_instance_of(Object).to receive(:rand).with(100).and_return(30, 70)

result1 = described_class.should_fetch_billboard?(placement_area: "sidebar_left", user_signed_in: true)
result2 = described_class.should_fetch_billboard?(placement_area: "sidebar_left", user_signed_in: true)

expect(result1).to be true # 30 < 50
expect(result2).to be false # 70 > 50
end
end

describe "caching" do
let!(:config) { described_class.create!(placement_area: "sidebar_left", signed_in_rate: 50, signed_out_rate: 50) }

it "caches all_configs" do
# Clear cache first
described_class.bust_cache

# Verify the method works and returns expected structure
result = described_class.all_configs
expect(result).to be_a(Hash)
expect(result["sidebar_left"]).to eq(config)

# Verify it works multiple times (even if cache doesn't work in test env)
result2 = described_class.all_configs
expect(result2).to eq(result)
end

it "busts cache on save" do
# Populate cache
described_class.all_configs

# Save should bust cache - in test environment this will call delete but may not work as expected
# So we just verify the method is called without error
expect { config.update!(signed_in_rate: 75) }.not_to raise_error
end

it "busts cache on destroy" do
# Populate cache
described_class.all_configs

# Destroy should bust cache - in test environment this will call delete but may not work as expected
# So we just verify the method is called without error
expect { config.destroy! }.not_to raise_error
end
end

describe ".config_for_placement_area" do
let!(:config) { described_class.create!(placement_area: "sidebar_left", signed_in_rate: 50, signed_out_rate: 50) }

it "returns the config for the given placement area" do
result = described_class.config_for_placement_area("sidebar_left")
expect(result).to eq(config)
end

it "returns nil for non-existent placement area" do
result = described_class.config_for_placement_area("nonexistent_area")
expect(result).to be_nil
end
end
end
80 changes: 80 additions & 0 deletions spec/models/billboard_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,86 @@
)
end

context "when delivery rate configuration is set" do
let!(:config) do
BillboardPlacementAreaConfig.create!(placement_area: "digest_second", signed_in_rate: 0, signed_out_rate: 50)
end

it "returns nil when delivery rate is 0%" do
result = described_class.for_display(
area: "digest_second",
user_signed_in: true,
user_tags: nil,
user_id: nil,
)

expect(result).to be_nil
end

it "returns nil when delivery rate is 0% for signed out users" do
config.update!(signed_out_rate: 0)

result = described_class.for_display(
area: "digest_second",
user_signed_in: false,
user_tags: nil,
user_id: nil,
)

expect(result).to be_nil
end

it "returns a billboard when delivery rate is 100%" do
config.update!(signed_in_rate: 100, signed_out_rate: 100)

result = described_class.for_display(
area: "digest_second",
user_signed_in: true,
user_tags: nil,
user_id: nil,
)

expect(result).to be_present
expect([paired_bb, other_bb]).to include(result)
end

it "respects different rates for signed in vs signed out users" do
config.update!(signed_in_rate: 100, signed_out_rate: 0)

# Signed in user should get a billboard
signed_in_result = described_class.for_display(
area: "digest_second",
user_signed_in: true,
user_tags: nil,
user_id: nil,
)
expect(signed_in_result).to be_present

# Signed out user should get nil
signed_out_result = described_class.for_display(
area: "digest_second",
user_signed_in: false,
user_tags: nil,
user_id: nil,
)
expect(signed_out_result).to be_nil
end

it "uses default 100% rate when no config exists for placement area" do
config.destroy!

result = described_class.for_display(
area: "digest_second",
user_signed_in: true,
user_tags: nil,
user_id: nil,
)

expect(result).to be_present
expect([paired_bb, other_bb]).to include(result)
end
end

context "when prefer_paired_with_billboard_id is provided" do
xit "returns the billboard matching that ID" do
result = described_class.for_display(
Expand Down
Loading