Allow LOW_IMPRESSION_COUNT and related values set by ENV (#19678)
* Allow LOW_IMPRESSION_COUNT and related values set by ENV * Update app/models/display_ad.rb Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Update app/models/display_ad.rb Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Update app/models/display_ad.rb Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Adjust test and move methods to non-private * Change fallback numbers to constants * Add global config fallbacks * Update spec/models/display_ad_spec.rb Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Update app/models/display_ad.rb Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This commit is contained in:
Ben Halpern
GitHub
parent
01e1557583
commit
7ecb72413d
2 changed files with 60 additions and 13 deletions
|
|
@ -22,8 +22,9 @@ class DisplayAd < ApplicationRecord
|
|||
POST_WIDTH = 775
|
||||
SIDEBAR_WIDTH = 350
|
||||
LOW_IMPRESSION_COUNT = 1_000
|
||||
RARELY = (0...5) # 5 percent chance
|
||||
SELDOM = (5...35) # 30 percent chance
|
||||
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 }
|
||||
|
|
@ -54,7 +55,7 @@ class DisplayAd < ApplicationRecord
|
|||
search: "%#{term}%"
|
||||
}
|
||||
|
||||
scope :seldom_seen, -> { where("impressions_count < ?", LOW_IMPRESSION_COUNT).or(where(priority: true)) }
|
||||
scope :seldom_seen, ->(area) { where("impressions_count < ?", low_impression_count(area)).or(where(priority: true)) }
|
||||
|
||||
def self.for_display(area:, user_signed_in:, user_id: nil, article: nil)
|
||||
permit_adjacent = article ? article.permit_adjacent_sponsors? : true
|
||||
|
|
@ -70,16 +71,16 @@ class DisplayAd < ApplicationRecord
|
|||
)
|
||||
|
||||
case rand(99) # output integer from 0-99
|
||||
when RARELY # smallest range, 5%
|
||||
when (0..random_range_max(area)) # smallest range, 5%
|
||||
# We are always showing more of the good stuff — but we are also always testing the system to give any a chance to
|
||||
# rise to the top. 5 out of every 100 times we show an ad (5%), it is totally random. This gives "not yet
|
||||
# evaluated" stuff a chance to get some engagement and start showing up more. If it doesn't get engagement, it
|
||||
# stays in this area.
|
||||
ads_for_display.sample
|
||||
when SELDOM # medium range, 30%
|
||||
when (random_range_max(area)..new_and_priority_range_max(area)) # medium range, 30%
|
||||
# Here we sample from only billboards with fewer than 1000 impressions (with a fallback
|
||||
# if there are none of those, causing an extra query, but that shouldn't happen very often).
|
||||
ads_for_display.seldom_seen.sample || ads_for_display.sample
|
||||
ads_for_display.seldom_seen(area).sample || ads_for_display.sample
|
||||
else # large range, 65%
|
||||
|
||||
# Ads that get engagement have a higher "success rate", and among this category, we sample from the top 15 that
|
||||
|
|
@ -138,6 +139,27 @@ 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
|
||||
|
|
|
|||
|
|
@ -340,19 +340,44 @@ RSpec.describe DisplayAd do
|
|||
let!(:priority_ad) { 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).to include(low_impression_ad)
|
||||
expect(described_class.seldom_seen("sidebar_left")).to include(low_impression_ad)
|
||||
end
|
||||
|
||||
it "does not include ads with impression count more than env area override of LOW_IMPRESSION_COUNT" do
|
||||
allow(ApplicationConfig).to receive(:[]).with("LOW_IMPRESSION_COUNT_FOR_SIDEBAR_LEFT").and_return("990")
|
||||
expect(described_class.seldom_seen("sidebar_left")).not_to include(low_impression_ad)
|
||||
end
|
||||
|
||||
it "includes ads with impression count less than than env area override of LOW_IMPRESSION_COUNT" do
|
||||
allow(ApplicationConfig).to receive(:[]).with("LOW_IMPRESSION_COUNT_FOR_SIDEBAR_LEFT").and_return("1010")
|
||||
expect(described_class.seldom_seen("sidebar_left")).to include(low_impression_ad)
|
||||
end
|
||||
|
||||
it "does not include ads with impression count more than env GLOBAL override of LOW_IMPRESSION_COUNT" do
|
||||
allow(ApplicationConfig).to receive(:[]).with("LOW_IMPRESSION_COUNT").and_return("990")
|
||||
allow(ApplicationConfig).to receive(:[]).with("LOW_IMPRESSION_COUNT_FOR_SIDEBAR_LEFT").and_return(nil)
|
||||
expect(described_class.seldom_seen("sidebar_left")).not_to include(low_impression_ad)
|
||||
end
|
||||
|
||||
it "includes ads with impression count less than than env GLOBAL override of LOW_IMPRESSION_COUNT" do
|
||||
allow(ApplicationConfig).to receive(:[]).with("LOW_IMPRESSION_COUNT").and_return("1010")
|
||||
allow(ApplicationConfig).to receive(:[]).with("LOW_IMPRESSION_COUNT_FOR_SIDEBAR_LEFT").and_return(nil)
|
||||
expect(described_class.seldom_seen("sidebar_left")).to include(low_impression_ad)
|
||||
end
|
||||
|
||||
it "ignores override of LOW_IMPRESSION_COUNT to a different location" do
|
||||
allow(ApplicationConfig).to receive(:[]).with("LOW_IMPRESSION_COUNT_FOR_POST_COMMENTS").and_return("990")
|
||||
allow(ApplicationConfig).to receive(:[]).with("LOW_IMPRESSION_COUNT_FOR_SIDEBAR_LEFT").and_return(nil)
|
||||
allow(ApplicationConfig).to receive(:[]).with("LOW_IMPRESSION_COUNT").and_return(nil)
|
||||
expect(described_class.seldom_seen("sidebar_left")).to include(low_impression_ad)
|
||||
end
|
||||
|
||||
it "excludes ads with impressions count greater than or equal to LOW_IMPRESSION_COUNT" do
|
||||
expect(described_class.seldom_seen).not_to include(high_impression_ad)
|
||||
end
|
||||
|
||||
it "includes ads with priority set to true" do
|
||||
expect(described_class.seldom_seen).to include(priority_ad)
|
||||
expect(described_class.seldom_seen("sidebar_left")).not_to include(high_impression_ad)
|
||||
end
|
||||
|
||||
it "includes both priority to be proper size when two qualifying ads exist" do
|
||||
expect(described_class.seldom_seen.size).to be 2
|
||||
expect(described_class.seldom_seen("sidebar_left").size).to be 2
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue