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

Add logged-in vs. logged-out targeting option for Display Ads #18390 (#18400)

Browse source 
* feat: add a migration that creates a display_to column on the display_ads table

* feat: add an enum for display_to

* feat/WIP: add rough draft of the field to the UI and permit it in the controller

* feat: take into account on whether an ad needs to be shown on logged in or out or both + update cache keys on sign in

* fix: if a user is not signed in then we want to display all and logged out

* fix: display_for spec in model

* feat: add some tests for display_to

* feat: style the label

* feat: show the display_to on the index page

* feat: add a seed for e2e

* chore: force true
This commit is contained in:
Ridhwana 2022-09-07 17:21:27 +02:00 • committed by GitHub
parent d37834b0e1
commit 172dfef4bf
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 95 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/controllers/admin/display_ads_controller.rb
View file

@ -60,7 +60,7 @@ module Admin
private
def display_ad_params
params.permit(:organization_id, :body_markdown, :placement_area, :published, :approved, :name)
params.permit(:organization_id, :body_markdown, :placement_area, :published, :approved, :name, :display_to)
end
def authorize_admin

12
app/models/display_ad.rb
View file

@ -10,6 +10,8 @@ class DisplayAd < ApplicationRecord
POST_WIDTH = 775
SIDEBAR_WIDTH = 350
enum display_to: { all: 0, logged_in: 1, logged_out: 2 }, _prefix: true
belongs_to :organization, optional: true
has_many :display_ad_events, dependent: :destroy
@ -21,9 +23,17 @@ class DisplayAd < ApplicationRecord
scope :approved_and_published, -> { where(approved: true, published: true) }
def self.for_display(area)
def self.for_display(area, user_signed_in)
relation = approved_and_published.where(placement_area: area).order(success_rate: :desc)
relation = if user_signed_in
relation.where(display_to: %w[all logged_in])
else
relation.where(display_to: %w[all logged_out])
end
relation.order(success_rate: :desc)
if rand(8) == 1
relation.sample
else

22
app/views/admin/display_ads/_form.html.erb
View file

@ -20,6 +20,28 @@
<%= select_tag :placement_area, options_for_select(display_ads_placement_area_options_array, selected: @display_ad.placement_area), include_blank: "Select...", class: "crayons-select" %>
</div>
<div class="crayons-field">
<fieldset aria-describedby="section-description" aria-describedby="display-to-description">
<legend class="crayons-field crayons-field__label pl-0">Display to user group</legend>
<p id="display-to-description" class="crayons-field__description mb-2">Determines which user group will be able to see the Display Ad</p>
<label class="crayons-field crayons-field--radio mb-2">
<%= radio_button_tag :display_to, "all", @display_ad.display_to_all?, class: "crayons-radio" %>
<div class="crayons-field__label">All users</div>
</label>
<label class="crayons-field crayons-field--radio mb-2">
<%= radio_button_tag :display_to, "logged_in", @display_ad.display_to_logged_in?, class: "crayons-radio" %>
<div class="crayons-field__label">Only logged in users</div>
</label>
<label class="crayons-field crayons-field--radio mb-2">
<%= radio_button_tag :display_to, "logged_out", @display_ad.display_to_logged_out?, class: "crayons-radio" %>
<div class="crayons-field__label">Only logged out users</div>
</label>
</fieldset>
</div>
<div class="crayons-field">
<%= label_tag :published, "Published:", class: "crayons-field__label" %>
<%= select_tag :published, options_for_select([false, true], selected: @display_ad.published), class: "crayons-select" %>

2
app/views/admin/display_ads/index.html.erb
View file

@ -22,6 +22,7 @@
<tr>
<th scope="col">Name</th>
<th scope="col">Placement Area</th>
<th scope="col">Display to User Group</th>
<th scope="col">Published</th>
<th scope="col">Approved</th>
<th scope="col">Success Rate</th>
@ -32,6 +33,7 @@
<tr data-row-id="<%= display_ad.id %>">
<td><%= link_to display_ad.name, edit_admin_display_ad_path(display_ad) %></td>
<td><%= display_ad.human_readable_placement_area %></td>
<td><%= display_ad.display_to %></td>
<td><%= display_ad.published %></td>
<td><%= display_ad.approved %></td>
<td><%= display_ad.success_rate %></td>

8
app/views/articles/_sidebar.html.erb
View file

@ -19,16 +19,16 @@
</div>
<% end %>
<% end %>
<% cache("display-area-left-#{rand(5)}", expires_in: 5.minutes) do %>
<% @left_sidebar_ad = DisplayAd.for_display("sidebar_left") %>
<% 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?) %>
<% if @left_sidebar_ad %>
<div class="crayons-card crayons-card--secondary p-3 crayons-sponsorship-widget" data-display-unit data-id="<%= @left_sidebar_ad.id %>">
<%= @left_sidebar_ad.processed_html.html_safe %>
</div>
<% end %>
<% end %>
<% cache("display-area-left-2-#{rand(5)}", expires_in: 5.minutes) do %>
<% @second_left_sidebar_ad = DisplayAd.for_display("sidebar_left_2") %>
<% 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?) %>
<% if @second_left_sidebar_ad %>
<div class="mt-4 crayons-card crayons-card--secondary p-3 crayons-sponsorship-widget" data-display-unit data-id="<%= @second_left_sidebar_ad.id %>">
<%= @second_left_sidebar_ad.processed_html.html_safe %>

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

@ -191,7 +191,7 @@
</article>
<% cache("article-bottom-content-#{@article.id}-#{user_signed_in?}-#{(@organization || @user).latest_article_updated_at}", expires_in: 18.hours) do %>
<% @display_ad = DisplayAd.for_display("post_comments") %>
<% @display_ad = DisplayAd.for_display("post_comments", user_signed_in?) %>
<% if @display_ad %>
<div class="crayons-card crayons-card--secondary p-4 crayons-sponsorship-article" data-display-unit data-id="<%= @display_ad.id %>">
<%= @display_ad.processed_html.html_safe %>

4
app/views/sidebars/_homepage_content.html.erb
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]}"), expires_in: (params[:timeframe].blank? ? 120 : 360).seconds) do %>
<% @sidebar_ad = DisplayAd.for_display("sidebar_right") %>
<% 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?) %>
<% if @sidebar_ad %>
<div class="crayons-card crayons-card--secondary p-4 crayons-sponsorship-widget" data-display-unit data-id="<%= @sidebar_ad.id %>">
<%= @sidebar_ad.processed_html.html_safe %>

2
cypress/e2e/seededFlows/adminFlows/displayAds/displayAds.spec.js
View file

@ -7,7 +7,7 @@ describe('Display Ads', () => {
cy.loginAndVisit(user, '/admin/customization/display_ads');
cy.findByRole('table').within(() => {
cy.findByRole('button', { name: 'Destroy' }).click();
cy.findByRole('button', { name: 'Destroy' }).click({ force: true });
});
});
});

5
db/migrate/20220831091245_add_display_to_to_display_ad.rb Normal file
View file

@ -0,0 +1,5 @@
class AddDisplayToToDisplayAd < ActiveRecord::Migration[7.0]
def change
add_column :display_ads, :display_to, :integer, default: 0, null: false
end
end

3
db/schema.rb
View file

@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.0].define(version: 2022_08_30_141143) do
ActiveRecord::Schema[7.0].define(version: 2022_08_31_091245) do
# These are extensions that must be enabled in order to support this database
enable_extension "citext"
enable_extension "pg_stat_statements"
@ -459,6 +459,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_08_30_141143) do
t.text "body_markdown"
t.integer "clicks_count", default: 0
t.datetime "created_at", precision: nil, null: false
t.integer "display_to", default: 0, null: false
t.integer "impressions_count", default: 0
t.string "name"
t.bigint "organization_id"

53
spec/models/display_ad_spec.rb
View file

@ -63,21 +63,54 @@ RSpec.describe DisplayAd, type: :model do
end
describe ".for_display" do
let!(:display_ad) { create(:display_ad, organization_id: organization.id) }
context "when updating the published and approved values" do
let!(:display_ad) { create(:display_ad, organization_id: organization.id) }
it "does not return unpublished ads" do
display_ad.update!(published: false, approved: true)
expect(described_class.for_display(display_ad.placement_area)).to be_nil
it "does not return unpublished ads" do
display_ad.update!(published: false, approved: true)
expect(described_class.for_display(display_ad.placement_area, false)).to be_nil
end
it "does not return unapproved ads" do
display_ad.update!(published: true, approved: false)
expect(described_class.for_display(display_ad.placement_area, false)).to be_nil
end
it "returns published and approved ads" do
display_ad.update!(published: true, approved: true)
expect(described_class.for_display(display_ad.placement_area, false)).to eq(display_ad)
end
end
it "does not return unapproved ads" do
display_ad.update!(published: true, approved: false)
expect(described_class.for_display(display_ad.placement_area)).to be_nil
context "when display_to is set to 'logged_in' or 'logged_out'" do
let!(:display_ad2) do
create(:display_ad, organization_id: organization.id, published: true, approved: true, display_to: "logged_in")
end
let!(:display_ad3) do
create(:display_ad, organization_id: organization.id, published: true, approved: true, display_to: "logged_out")
end
it "shows ads that have a display_to of 'logged_in' if a user is signed in" do
expect(described_class.for_display(display_ad2.placement_area, 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.for_display(display_ad3.placement_area, false)).to eq(display_ad3)
end
end
it "returns published and approved ads" do
display_ad.update!(published: true, approved: true)
expect(described_class.for_display(display_ad.placement_area)).to eq(display_ad)
context "when display_to is set to 'all'" do
let!(:display_ad) do
create(:display_ad, organization_id: organization.id, published: true, approved: true, display_to: "all")
end
it "shows ads that have a display_to of 'all' if a user is signed in" do
expect(described_class.for_display(display_ad.placement_area, 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.for_display(display_ad.placement_area, false)).to eq(display_ad)
end
end
end
end

1
spec/support/seeds/seeds_e2e.rb
View file

@ -966,6 +966,7 @@ seeder.create_if_none(DisplayAd) do
organization_id: org_id,
body_markdown: "<h1>This is an add</h1>",
placement_area: "sidebar_left",
name: "Tests Display Ad",
published: true,
approved: true,
)