Display Rules for In house and Community Billboards (#19189)

* init

* Update app/queries/display_ads/filtered_ads_query.rb

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update app/queries/display_ads/filtered_ads_query.rb

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* add display ad query and update caller

* set as keyword args and add org id to callers

* Update app/queries/display_ads/filtered_ads_query.rb

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update app/queries/display_ads/filtered_ads_query.rb

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update app/queries/display_ads/filtered_ads_query.rb

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update app/views/articles/_sticky_nav.html.erb

Co-authored-by: Joshua Wehner <joshua@forem.com>

* Update app/views/articles/show.html.erb

Co-authored-by: Joshua Wehner <joshua@forem.com>

* Update app/queries/display_ads/filtered_ads_query.rb

Spacing

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update app/queries/display_ads/filtered_ads_query.rb

More spacing

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update app/queries/display_ads/filtered_ads_query.rb

Co-authored-by: Joshua Wehner <joshua@forem.com>

* Update app/queries/display_ads/filtered_ads_query.rb

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* fix some broken specs

* add updated spec for billboard rules

* missing comma

* Update spec/queries/display_ads/filtered_ads_query_spec.rb

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* add placement in spec

* fix external spec

* cleanup

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Joshua Wehner <joshua@forem.com>
This commit is contained in:
Lawrence 2023-03-06 16:15:14 -06:00 committed by GitHub
parent 0fc6a455de
commit cfad413ca1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 57 additions and 18 deletions

View file

@ -37,10 +37,11 @@ class DisplayAd < ApplicationRecord
search: "%#{term}%"
}
def self.for_display(area, user_signed_in, article_tags = [])
def self.for_display(area:, user_signed_in:, organization_id: nil, article_tags: [])
DisplayAds::FilteredAdsQuery.call(
display_ads: self,
area: area,
organization_id: organization_id,
user_signed_in: user_signed_in,
article_tags: article_tags,
)

View file

@ -4,10 +4,11 @@ module DisplayAds
new(...).call
end
def initialize(display_ads:, area:, user_signed_in:, article_tags: [])
def initialize(display_ads:, area:, user_signed_in:, organization_id: nil, article_tags: [])
@filtered_display_ads = display_ads
@area = area
@user_signed_in = user_signed_in
@organization_id = organization_id
@article_tags = article_tags
end
@ -29,6 +30,8 @@ module DisplayAds
authenticated_ads(%w[all logged_out])
end
@filtered_display_ads = community_or_in_house_ads
@filtered_display_ads = @filtered_display_ads.order(success_rate: :desc)
@filtered_display_ads = sample_ads
end
@ -56,6 +59,15 @@ module DisplayAds
@filtered_display_ads.where(display_to: display_auth_audience)
end
def community_or_in_house_ads
@filtered_display_ads.where(
"(type_of = :in_house) OR
(type_of = :community AND organization_id = :organization_id) OR
(type_of = :external AND organization_id != :organization_id)",
DisplayAd.type_ofs.merge({ organization_id: @organization_id }),
)
end
# Business Logic Context:
# 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. 1 out of every 8 times we show an ad (12.5%), it is totally random. This gives "not yet

View file

@ -6,13 +6,13 @@
<%= render "layouts/sidebar_tags" %>
<% end %>
<% cache("display-area-left-#{rand(5)}-#{user_signed_in?}", expires_in: 5.minutes) do %>
<% @left_sidebar_ad = DisplayAd.for_display("sidebar_left", user_signed_in?) %>
<% @left_sidebar_ad = DisplayAd.for_display(area: "sidebar_left", user_signed_in: user_signed_in?) %>
<% if @left_sidebar_ad %>
<%= render partial: "shared/display_ad", locals: {display_ad: @left_sidebar_ad, data_context_type: DisplayAdEvent::CONTEXT_TYPE_HOME } %>
<% end %>
<% end %>
<% cache("display-area-left-2-#{rand(5)}-#{user_signed_in?}", expires_in: 5.minutes) do %>
<% @second_left_sidebar_ad = DisplayAd.for_display("sidebar_left_2", user_signed_in?) %>
<% @second_left_sidebar_ad = DisplayAd.for_display(area: "sidebar_left_2", user_signed_in: user_signed_in?) %>
<% if @second_left_sidebar_ad %>
<div class="pt-4">
<%= render partial: "shared/display_ad", locals: {display_ad: @second_left_sidebar_ad, data_context_type: DisplayAdEvent::CONTEXT_TYPE_HOME } %>

View file

@ -56,7 +56,7 @@
</div>
<%# cache("article-sidebar-content-#{rand(5)}-#{@article.id}-#{user_signed_in?}-#{(@organization || @user).latest_article_updated_at}", expires_in: 15.minutes) do %>
<% sidebar_ad = DisplayAd.for_display("post_sidebar", user_signed_in?, @article.decorate.cached_tag_list_array) %>
<% sidebar_ad = DisplayAd.for_display(area: "post_sidebar", user_signed_in: user_signed_in?, organization_id: @article.organization_id, article_tags: @article.decorate.cached_tag_list_array) %>
<% if @article.permit_adjacent_sponsors? && sidebar_ad %>
<div class="crayons-article-sticky grid gap-4 break-word pt-4">
<%= render partial: "shared/display_ad", locals: {display_ad: sidebar_ad, data_context_type: DisplayAdEvent::CONTEXT_TYPE_ARTICLE } %>

View file

@ -193,7 +193,7 @@
</article>
<% cache("article-bottom-content-#{rand(5)}-#{@article.id}-#{user_signed_in?}-#{(@organization || @user).latest_article_updated_at}", expires_in: 15.minutes) do %>
<% @display_ad = DisplayAd.for_display("post_comments", user_signed_in?, @article.decorate.cached_tag_list_array) %>
<% @display_ad = DisplayAd.for_display(area: "post_comments", user_signed_in: user_signed_in?, organization_id: @article.organization_id, article_tags: @article.decorate.cached_tag_list_array) %>
<% if @display_ad %>
<div class="pb-4">
<%= render partial: "shared/display_ad", locals: {display_ad: @display_ad, data_context_type: DisplayAdEvent::CONTEXT_TYPE_ARTICLE } %>

View file

@ -1,6 +1,6 @@
<aside class="side-bar sidebar-additional showing grid gap-4" id="sidebar-additional">
<% 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("sidebar_right", user_signed_in?) %>
<% @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 } %>
<% end %>

View file

@ -9,19 +9,19 @@ RSpec.describe DisplayAds::FilteredAdsQuery, type: :query do
it "does not return unpublished ads" do
display_ad.update!(published: false, approved: true)
expect(described_class.call(display_ads: DisplayAd.all, area: display_ad.placement_area,
user_signed_in: false)).to be_nil
organization_id: nil, user_signed_in: false)).to be_nil
end
it "does not return unapproved ads" do
display_ad.update!(published: true, approved: false)
expect(described_class.call(display_ads: DisplayAd.all, area: display_ad.placement_area,
user_signed_in: false)).to be_nil
organization_id: nil, user_signed_in: false)).to be_nil
end
it "returns published and approved ads" do
display_ad.update!(published: true, approved: true)
expect(described_class.call(display_ads: DisplayAd.all, area: display_ad.placement_area,
user_signed_in: false)).to eq(display_ad)
organization_id: nil, user_signed_in: false)).to eq(display_ad)
end
end
@ -41,7 +41,7 @@ RSpec.describe DisplayAds::FilteredAdsQuery, type: :query do
article_tags = %w[linux productivity]
expect(described_class.call(display_ads: DisplayAd.all, area: "post_comments", user_signed_in: false,
article_tags: article_tags)).to eq(display_ad)
organization_id: nil, article_tags: article_tags)).to eq(display_ad)
end
it "will show display ads that have no tags set" do
@ -59,7 +59,7 @@ RSpec.describe DisplayAds::FilteredAdsQuery, type: :query do
article_tags = %w[productivity java]
expect(described_class.call(display_ads: DisplayAd.all, area: "post_comments", user_signed_in: false,
article_tags: article_tags)).to eq(display_ad)
organization_id: display_ad.organization.id, article_tags: article_tags)).to eq(display_ad)
end
it "will show no display ads if the available display ads have no tags set or do not contain matching tags" do
@ -70,7 +70,7 @@ RSpec.describe DisplayAds::FilteredAdsQuery, type: :query do
cached_tag_list: "productivity")
article_tags = %w[javascript]
expect(described_class.call(display_ads: DisplayAd.all, area: "post_comments", user_signed_in: false,
article_tags: article_tags)).to be_nil
organization_id: nil, article_tags: article_tags)).to be_nil
end
it "will show display ads with no tags set if there are no article tags" do
@ -87,7 +87,7 @@ RSpec.describe DisplayAds::FilteredAdsQuery, type: :query do
cached_tag_list: "")
expect(described_class.call(display_ads: DisplayAd.all, area: "post_comments",
user_signed_in: false)).to eq(display_ad_without_tags)
organization_id: nil, user_signed_in: false)).to eq(display_ad_without_tags)
end
end
@ -101,12 +101,12 @@ RSpec.describe DisplayAds::FilteredAdsQuery, type: :query do
it "shows ads that have a display_to of 'logged_in' if a user is signed in" do
expect(described_class.call(display_ads: DisplayAd.all, area: display_ad2.placement_area,
user_signed_in: true)).to eq(display_ad2)
organization_id: display_ad2.organization.id, user_signed_in: true)).to eq(display_ad2)
end
it "shows ads that have a display_to of 'logged_out' if a user is signed in" do
expect(described_class.call(display_ads: DisplayAd.all, area: display_ad3.placement_area,
user_signed_in: false)).to eq(display_ad3)
organization_id: display_ad3.organization.id, user_signed_in: false)).to eq(display_ad3)
end
end
@ -117,12 +117,38 @@ RSpec.describe DisplayAds::FilteredAdsQuery, type: :query do
it "shows ads that have a display_to of 'all' if a user is signed in" do
expect(described_class.call(display_ads: DisplayAd.all, area: display_ad.placement_area,
user_signed_in: true)).to eq(display_ad)
organization_id: display_ad.organization.id, user_signed_in: true)).to eq(display_ad)
end
it "shows ads that have a display_to of 'all' if a user is not signed in" do
expect(described_class.call(display_ads: DisplayAd.all, area: display_ad.placement_area,
user_signed_in: false)).to eq(display_ad)
organization_id: display_ad.organization.id, user_signed_in: false)).to eq(display_ad)
end
end
context "when organization is set on ad" do
it "shows ads that have that are of type community and associated with the organization" do
display_ad = create(:display_ad, organization_id: organization.id,
published: true,
approved: true,
placement_area: "post_comments",
type_of: "community",
display_to: "all")
expect(described_class.call(display_ads: DisplayAd.all, area: display_ad.placement_area,
organization_id: display_ad.organization.id, user_signed_in: false)).to eq(display_ad)
end
it "shows ads that have that are of type in house" do
display_ad = create(:display_ad, organization_id: organization.id,
published: true,
approved: true,
placement_area: "post_comments",
type_of: "in_house",
display_to: "all")
expect(described_class.call(display_ads: DisplayAd.all, area: display_ad.placement_area,
organization_id: display_ad.organization.id, user_signed_in: false)).to eq(display_ad)
end
end
end