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

Renamed DisplayAd model to Billboard (#19993)

Browse source 
* Renamed Resource Admin: DisplayAd to Billboard

* Renamed DisplayAd to Billboard

* Added a data update script for roles
This commit is contained in:
Anna Buianova 2023-08-28 15:09:36 +03:00 • committed by GitHub
parent 567aba455a
commit 2d6b740940
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
34 changed files with 116 additions and 93 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
app/controllers/admin/billboards_controller.rb
View file

@ -5,7 +5,7 @@ module Admin
after_action :bust_ad_caches, only: %i[create update destroy] after_action :bust_ad_caches, only: %i[create update destroy]
def index def index
@billboards = DisplayAd.order(id: :desc) @billboards = Billboard.order(id: :desc)
.page(params[:page]).per(50) .page(params[:page]).per(50)
return if params[:search].blank? return if params[:search].blank?
@ -14,15 +14,15 @@ module Admin
end end
def new def new
@billboard = DisplayAd.new @billboard = Billboard.new
end end
def edit def edit
@billboard = DisplayAd.find(params[:id]) @billboard = Billboard.find(params[:id])
end end
def create def create
@billboard = DisplayAd.new(billboard_params) @billboard = Billboard.new(billboard_params)
@billboard.creator = current_user @billboard.creator = current_user
if @billboard.save if @billboard.save
@ -35,7 +35,7 @@ module Admin
end end
def update def update
@billboard = DisplayAd.find(params[:id]) @billboard = Billboard.find(params[:id])
if @billboard.update(billboard_params) if @billboard.update(billboard_params)
flash[:success] = I18n.t("admin.billboards_controller.updated") flash[:success] = I18n.t("admin.billboards_controller.updated")
@ -47,7 +47,7 @@ module Admin
end end
def destroy def destroy
@billboard = DisplayAd.find(params[:id]) @billboard = Billboard.find(params[:id])
if @billboard.destroy if @billboard.destroy
render json: { message: I18n.t("admin.billboards_controller.deleted") }, status: :ok render json: { message: I18n.t("admin.billboards_controller.deleted") }, status: :ok
@ -65,7 +65,7 @@ module Admin
end end
def authorize_admin def authorize_admin
authorize DisplayAd, :access?, policy_class: InternalPolicy authorize Billboard, :access?, policy_class: InternalPolicy
end end
def bust_ad_caches def bust_ad_caches

2
app/controllers/api/v1/audience_segments_controller.rb
View file

@ -34,7 +34,7 @@ module Api
def destroy def destroy
@segment = scope.find(params[:id]) @segment = scope.find(params[:id])
if DisplayAd.where(audience_segment_id: @segment.id).any? if Billboard.where(audience_segment_id: @segment.id).any?
render json: { error: "Segments cannot be deleted while in use by any billboards" }, status: :conflict render json: { error: "Segments cannot be deleted while in use by any billboards" }, status: :conflict
else else
@segment.segmented_users.in_batches.delete_all @segment.segmented_users.in_batches.delete_all

12
app/controllers/api/v1/billboards_controller.rb
View file

@ -8,30 +8,30 @@ module Api
rescue_from ArgumentError, with: :error_unprocessable_entity rescue_from ArgumentError, with: :error_unprocessable_entity
def index def index
@billboards = DisplayAd.order(id: :desc).page(params[:page]).per(50) @billboards = Billboard.order(id: :desc).page(params[:page]).per(50)
@billboards = @billboards.search_ads(params[:search]) if params[:search].present? @billboards = @billboards.search_ads(params[:search]) if params[:search].present?
render json: @billboards render json: @billboards
end end
def show def show
@billboard = DisplayAd.find(params[:id]) @billboard = Billboard.find(params[:id])
render json: @billboard render json: @billboard
end end
def create def create
@billboard = DisplayAd.new(permitted_params) @billboard = Billboard.new(permitted_params)
@billboard.save! @billboard.save!
render json: @billboard, status: :created render json: @billboard, status: :created
end end
def update def update
@billboard = DisplayAd.find(params[:id]) @billboard = Billboard.find(params[:id])
@billboard.update!(permitted_params) @billboard.update!(permitted_params)
render json: @billboard render json: @billboard
end end
def unpublish def unpublish
@billboard = DisplayAd.find(params[:id]) @billboard = Billboard.find(params[:id])
result = @billboard.update(published: false) result = @billboard.update(published: false)
if result if result
head :no_content head :no_content
@ -43,7 +43,7 @@ module Api
private private
def require_admin def require_admin
authorize DisplayAd, :access?, policy_class: InternalPolicy authorize Billboard, :access?, policy_class: InternalPolicy
end end
def permitted_params def permitted_params

2
app/controllers/billboard_events_controller.rb
View file

@ -18,7 +18,7 @@ class BillboardEventsController < ApplicationMetalController
billboard_event_id = billboard_event_params[:billboard_id] billboard_event_id = billboard_event_params[:billboard_id]
ThrottledCall.perform("billboards_data_update-#{billboard_event_id}", throttle_for: 15.minutes) do ThrottledCall.perform("billboards_data_update-#{billboard_event_id}", throttle_for: 15.minutes) do
@billboard = DisplayAd.find(billboard_event_id) @billboard = Billboard.find(billboard_event_id)
num_impressions = @billboard.billboard_events.impressions.sum(:counts_for) num_impressions = @billboard.billboard_events.impressions.sum(:counts_for)
num_clicks = @billboard.billboard_events.clicks.sum(:counts_for) num_clicks = @billboard.billboard_events.clicks.sum(:counts_for)

2
app/controllers/billboards_controller.rb
View file

@ -19,7 +19,7 @@ class BillboardsController < ApplicationController
@article = Article.find_by(slug: params[:slug]) @article = Article.find_by(slug: params[:slug])
end end
@billboard = DisplayAd.for_display( @billboard = Billboard.for_display(
area: placement_area, area: placement_area,
user_signed_in: user_signed_in?, user_signed_in: user_signed_in?,
user_id: current_user&.id, user_id: current_user&.id,

2
app/controllers/stories_controller.rb
View file

@ -54,7 +54,7 @@ class StoriesController < ApplicationController
end end
def assign_hero_banner def assign_hero_banner
@hero_billboard = DisplayAd.for_display(area: "home_hero", user_signed_in: user_signed_in?) @hero_billboard = Billboard.for_display(area: "home_hero", user_signed_in: user_signed_in?)
end end
def assign_hero_html def assign_hero_html

6
app/helpers/billboard_helper.rb
View file

@ -1,6 +1,6 @@
module BillboardHelper module BillboardHelper
def billboards_placement_area_options_array def billboards_placement_area_options_array
DisplayAd::ALLOWED_PLACEMENT_AREAS_HUMAN_READABLE.zip(DisplayAd::ALLOWED_PLACEMENT_AREAS) Billboard::ALLOWED_PLACEMENT_AREAS_HUMAN_READABLE.zip(Billboard::ALLOWED_PLACEMENT_AREAS)
end end
def automatic_audience_segments_options_array def automatic_audience_segments_options_array
@ -20,9 +20,9 @@ module BillboardHelper
# #
# @return [Boolean] true or false on whether the area is a targeted tag placement on the feed. # @return [Boolean] true or false on whether the area is a targeted tag placement on the feed.
# #
# @note An area of "sidebar_left_2" will return false as it is not part of DisplayAd::HOME_FEED_PLACEMENTS # @note An area of "sidebar_left_2" will return false as it is not part of Billboard::HOME_FEED_PLACEMENTS
# whilst an area of "feed_first" will return false. # whilst an area of "feed_first" will return false.
def feed_targeted_tag_placement?(area) def feed_targeted_tag_placement?(area)
DisplayAd::HOME_FEED_PLACEMENTS.include?(area) Billboard::HOME_FEED_PLACEMENTS.include?(area)
end end
end end

2
app/lib/constants/role.rb
View file

@ -22,7 +22,7 @@ module Constants
"Resource Admin: Broadcast" => { name: "single_resource_admin", resource_type: "Broadcast" }, "Resource Admin: Broadcast" => { name: "single_resource_admin", resource_type: "Broadcast" },
"Resource Admin: Comment" => { name: "single_resource_admin", resource_type: "Comment" }, "Resource Admin: Comment" => { name: "single_resource_admin", resource_type: "Comment" },
"Resource Admin: Config" => { name: "single_resource_admin", resource_type: "Config" }, "Resource Admin: Config" => { name: "single_resource_admin", resource_type: "Config" },
"Resource Admin: DisplayAd" => { name: "single_resource_admin", resource_type: "DisplayAd" }, "Resource Admin: Billboard" => { name: "single_resource_admin", resource_type: "Billboard" },
"Resource Admin: DataUpdateScript" => { name: "single_resource_admin", resource_type: "DataUpdateScript" }, "Resource Admin: DataUpdateScript" => { name: "single_resource_admin", resource_type: "DataUpdateScript" },
"Resource Admin: FeedbackMessage" => { name: "single_resource_admin", resource_type: "FeedbackMessage" }, "Resource Admin: FeedbackMessage" => { name: "single_resource_admin", resource_type: "FeedbackMessage" },
"Resource Admin: HtmlVariant" => { name: "single_resource_admin", resource_type: "HtmlVariant" }, "Resource Admin: HtmlVariant" => { name: "single_resource_admin", resource_type: "HtmlVariant" },

6
app/models/display_ad.rb → app/models/billboard.rb
View file

@ -1,4 +1,4 @@
class DisplayAd < ApplicationRecord class Billboard < ApplicationRecord
include Taggable include Taggable
acts_as_taggable_on :tags acts_as_taggable_on :tags
resourcify resourcify
@ -32,7 +32,7 @@ class DisplayAd < ApplicationRecord
enum type_of: { in_house: 0, community: 1, external: 2 } enum type_of: { in_house: 0, community: 1, external: 2 }
belongs_to :organization, optional: true belongs_to :organization, optional: true
has_many :billboard_events, dependent: :destroy has_many :billboard_events, foreign_key: :display_ad_id, inverse_of: :billboard, dependent: :destroy
validates :placement_area, presence: true, validates :placement_area, presence: true,
inclusion: { in: ALLOWED_PLACEMENT_AREAS } inclusion: { in: ALLOWED_PLACEMENT_AREAS }
@ -60,6 +60,8 @@ class DisplayAd < ApplicationRecord
scope :seldom_seen, ->(area) { where("impressions_count < ?", low_impression_count(area)).or(where(priority: true)) } scope :seldom_seen, ->(area) { where("impressions_count < ?", low_impression_count(area)).or(where(priority: true)) }
self.table_name = "display_ads"
def self.for_display(area:, user_signed_in:, user_id: nil, article: nil, user_tags: nil, location: nil) def self.for_display(area:, user_signed_in:, user_id: nil, article: nil, user_tags: nil, location: nil)
permit_adjacent = article ? article.permit_adjacent_sponsors? : true permit_adjacent = article ? article.permit_adjacent_sponsors? : true

2
app/models/billboard_event.rb
View file

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

2
app/models/organization.rb
View file

@ -28,7 +28,7 @@ class Organization < ApplicationRecord
has_many :articles, dependent: :nullify has_many :articles, dependent: :nullify
has_many :collections, dependent: :nullify has_many :collections, dependent: :nullify
has_many :credits, dependent: :restrict_with_error has_many :credits, dependent: :restrict_with_error
has_many :billboards, class_name: "DisplayAd", dependent: :destroy has_many :billboards, class_name: "Billboard", dependent: :destroy
has_many :listings, dependent: :destroy has_many :listings, dependent: :destroy
has_many :notifications, dependent: :delete_all has_many :notifications, dependent: :delete_all
has_many :organization_memberships, dependent: :delete_all has_many :organization_memberships, dependent: :delete_all

2
app/models/tag.rb
View file

@ -38,7 +38,7 @@ class Tag < ActsAsTaggableOn::Tag
belongs_to :badge, optional: true belongs_to :badge, optional: true
has_many :articles, through: :taggings, source: :taggable, source_type: "Article" has_many :articles, through: :taggings, source: :taggable, source_type: "Article"
has_many :billboards, class_name: "DisplayAd", through: :taggings, source: :taggable, source_type: "DisplayAd" has_many :billboards, class_name: "Billboard", through: :taggings, source: :taggable, source_type: "Billboard"
mount_uploader :profile_image, ProfileImageUploader mount_uploader :profile_image, ProfileImageUploader
mount_uploader :social_image, ProfileImageUploader mount_uploader :social_image, ProfileImageUploader

8
app/queries/billboards/filtered_ads_query.rb
View file

@ -7,10 +7,10 @@ module Billboards
# @param area [String] the site area where the ad is visible # @param area [String] the site area where the ad is visible
# @param user_signed_in [Boolean] whether or not the visitor is signed-in # @param user_signed_in [Boolean] whether or not the visitor is signed-in
# @param billboards [DisplayAd] can be a filtered scope or Arel relationship # @param billboards [Billboard] can be a filtered scope or Arel relationship
# @param location [Geolocation|String] the visitor's geographic location # @param location [Geolocation|String] the visitor's geographic location
def initialize(area:, user_signed_in:, organization_id: nil, article_tags: [], def initialize(area:, user_signed_in:, organization_id: nil, article_tags: [],
permit_adjacent_sponsors: true, article_id: nil, billboards: DisplayAd, permit_adjacent_sponsors: true, article_id: nil, billboards: Billboard,
user_id: nil, user_tags: nil, location: nil) user_id: nil, user_tags: nil, location: nil)
@filtered_billboards = billboards.includes([:organization]) @filtered_billboards = billboards.includes([:organization])
@area = area @area = area
@ -118,7 +118,7 @@ module Billboards
def type_of_ads def type_of_ads
# If this is an organization article and community-type ads exist, show them # If this is an organization article and community-type ads exist, show them
if @organization_id.present? if @organization_id.present?
community = @filtered_billboards.where(type_of: DisplayAd.type_ofs[:community], community = @filtered_billboards.where(type_of: Billboard.type_ofs[:community],
organization_id: @organization_id) organization_id: @organization_id)
return community if community.any? return community if community.any?
end end
@ -135,7 +135,7 @@ module Billboards
types_matching << :external types_matching << :external
end end
@filtered_billboards.where(type_of: DisplayAd.type_ofs.slice(*types_matching).values) @filtered_billboards.where(type_of: Billboard.type_ofs.slice(*types_matching).values)
end end
end end
end end

6
app/services/content_renderer.rb
View file

@ -1,4 +1,4 @@
# renders markdown for Articles, DisplayAds, Comments # renders markdown for Articles, Billboards, Comments
class ContentRenderer class ContentRenderer
Result = Struct.new(:front_matter, :reading_time, :processed_html, keyword_init: true) Result = Struct.new(:front_matter, :reading_time, :processed_html, keyword_init: true)
@ -9,8 +9,8 @@ class ContentRenderer
end end
# @param input [String] body_markdown to process # @param input [String] body_markdown to process
# @param source [Article, Comment, DisplayAd] # @param source [Article, Comment, Billboard]
# @param user [User, NilClass] article's or comment's user, nil for DisplayAd # @param user [User, NilClass] article's or comment's user, nil for Billboard
# @param fixer [Object] fixes the input markdown # @param fixer [Object] fixes the input markdown
def initialize(input, source:, user: nil, fixer: MarkdownProcessor::Fixer::FixAll) def initialize(input, source:, user: nil, fixer: MarkdownProcessor::Fixer::FixAll)
@input = input || "" @input = input || ""

2
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"> <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 %> <% 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_billboard = DisplayAd.for_display(area: "sidebar_right", user_signed_in: user_signed_in?) %> <% @sidebar_billboard = Billboard.for_display(area: "sidebar_right", user_signed_in: user_signed_in?) %>
<% if @sidebar_billboard %> <% if @sidebar_billboard %>
<%= render partial: "shared/billboard", locals: { billboard: @sidebar_billboard, data_context_type: BillboardEvent::CONTEXT_TYPE_HOME } %> <%= render partial: "shared/billboard", locals: { billboard: @sidebar_billboard, data_context_type: BillboardEvent::CONTEXT_TYPE_HOME } %>
<% end %> <% end %>

6
app/views/stories/_main_stories_feed.html.erb
View file

@ -8,7 +8,7 @@
<% end %> <% end %>
<% if !user_signed_in? %> <% if !user_signed_in? %>
<% @sidebar_billboard_first = DisplayAd.for_display(area: "feed_first", user_signed_in: user_signed_in?) %> <% @sidebar_billboard_first = Billboard.for_display(area: "feed_first", user_signed_in: user_signed_in?) %>
<% if @sidebar_billboard_first %> <% if @sidebar_billboard_first %>
<%= render partial: "shared/billboard", locals: { billboard: @sidebar_billboard_first, data_context_type: BillboardEvent::CONTEXT_TYPE_HOME } %> <%= render partial: "shared/billboard", locals: { billboard: @sidebar_billboard_first, data_context_type: BillboardEvent::CONTEXT_TYPE_HOME } %>
<% end %> <% end %>
@ -36,14 +36,14 @@
<% if @stories.any? %> <% if @stories.any? %>
<% @stories.each_with_index do |story, i| %> <% @stories.each_with_index do |story, i| %>
<% if !user_signed_in? && i == second_billboard_position %> <% if !user_signed_in? && i == second_billboard_position %>
<% @sidebar_billboard_second = DisplayAd.for_display(area: "feed_second", user_signed_in: user_signed_in?) %> <% @sidebar_billboard_second = Billboard.for_display(area: "feed_second", user_signed_in: user_signed_in?) %>
<% if @sidebar_billboard_second %> <% if @sidebar_billboard_second %>
<%= render partial: "shared/billboard", locals: { billboard: @sidebar_billboard_second, data_context_type: BillboardEvent::CONTEXT_TYPE_HOME } %> <%= render partial: "shared/billboard", locals: { billboard: @sidebar_billboard_second, data_context_type: BillboardEvent::CONTEXT_TYPE_HOME } %>
<% end %> <% end %>
<% end %> <% end %>
<% if !user_signed_in? && i == third_billboard_position %> <% if !user_signed_in? && i == third_billboard_position %>
<% @sidebar_billboard_third = DisplayAd.for_display(area: "feed_third", user_signed_in: user_signed_in?) %> <% @sidebar_billboard_third = Billboard.for_display(area: "feed_third", user_signed_in: user_signed_in?) %>
<% if @sidebar_billboard_third %> <% if @sidebar_billboard_third %>
<%= render partial: "shared/billboard", locals: { billboard: @sidebar_billboard_third, data_context_type: BillboardEvent::CONTEXT_TYPE_HOME } %> <%= render partial: "shared/billboard", locals: { billboard: @sidebar_billboard_third, data_context_type: BillboardEvent::CONTEXT_TYPE_HOME } %>
<% end %> <% end %>

2
app/workers/audience_segment_refresh_all_worker.rb
View file

@ -4,7 +4,7 @@ class AudienceSegmentRefreshAllWorker
sidekiq_options queue: :low_priority sidekiq_options queue: :low_priority
def perform def perform
ids = DisplayAd.approved_and_published ids = Billboard.approved_and_published
.distinct(:audience_segment_id) .distinct(:audience_segment_id)
.pluck(:audience_segment_id).compact .pluck(:audience_segment_id).compact

2
config/environments/development.rb
View file

@ -143,7 +143,7 @@ Rails.application.configure do
# There were some warnings about eager loading the organization for a billboard, however since the code goes down # There were some warnings about eager loading the organization for a billboard, however since the code goes down
# different paths (in_house where we don’t need the organization info vs external/community where we need the # different paths (in_house where we don’t need the organization info vs external/community where we need the
# organization info), bullet was getting confused on whether we need the eager loading or not. # organization info), bullet was getting confused on whether we need the eager loading or not.
Bullet.add_safelist(type: :unused_eager_loading, class_name: "DisplayAd", association: :organization) Bullet.add_safelist(type: :unused_eager_loading, class_name: "Billboard", association: :organization)
# TODO: Temporarily ignoring this while working out user - profile relationship # TODO: Temporarily ignoring this while working out user - profile relationship
Bullet.add_safelist(type: :n_plus_one_query, class_name: "User", association: :profile) Bullet.add_safelist(type: :n_plus_one_query, class_name: "User", association: :profile)

2
config/locales/views/admin/en.yml
View file

@ -390,7 +390,7 @@ en:
"Resource Admin: Broadcast": "Resource Admin: Broadcast" "Resource Admin: Broadcast": "Resource Admin: Broadcast"
"Resource Admin: Comment": "Resource Admin: Comment" "Resource Admin: Comment": "Resource Admin: Comment"
"Resource Admin: Config": "Resource Admin: Config" "Resource Admin: Config": "Resource Admin: Config"
"Resource Admin: DisplayAd": "Resource Admin: DisplayAd" "Resource Admin: Billboard": "Resource Admin: Billboard"
"Resource Admin: DataUpdateScript": "Resource Admin: DataUpdateScript" "Resource Admin: DataUpdateScript": "Resource Admin: DataUpdateScript"
"Resource Admin: FeedbackMessage": "Resource Admin: FeedbackMessage" "Resource Admin: FeedbackMessage": "Resource Admin: FeedbackMessage"
"Resource Admin: HtmlVariant": "Resource Admin: HtmlVariant" "Resource Admin: HtmlVariant": "Resource Admin: HtmlVariant"

2
config/locales/views/admin/fr.yml
View file

@ -388,7 +388,7 @@ fr:
"Resource Admin: Broadcast": "Resource Admin: Broadcast" "Resource Admin: Broadcast": "Resource Admin: Broadcast"
"Resource Admin: Comment": "Resource Admin: Comment" "Resource Admin: Comment": "Resource Admin: Comment"
"Resource Admin: Config": "Resource Admin: Config" "Resource Admin: Config": "Resource Admin: Config"
"Resource Admin: DisplayAd": "Resource Admin: DisplayAd" "Resource Admin: Billboard": "Resource Admin: Billboard"
"Resource Admin: DataUpdateScript": "Resource Admin: DataUpdateScript" "Resource Admin: DataUpdateScript": "Resource Admin: DataUpdateScript"
"Resource Admin: FeedbackMessage": "Resource Admin: FeedbackMessage" "Resource Admin: FeedbackMessage": "Resource Admin: FeedbackMessage"
"Resource Admin: HtmlVariant": "Resource Admin: HtmlVariant" "Resource Admin: HtmlVariant": "Resource Admin: HtmlVariant"

10
db/seeds.rb
View file

@ -559,9 +559,9 @@ end
############################################################################## ##############################################################################
seeder.create_if_none(DisplayAd) do seeder.create_if_none(Billboard) do
DisplayAd::ALLOWED_PLACEMENT_AREAS.each do |placement_area| Billboard::ALLOWED_PLACEMENT_AREAS.each do |placement_area|
DisplayAd.create!( Billboard.create!(
name: "#{Faker::Lorem.word} #{placement_area}", name: "#{Faker::Lorem.word} #{placement_area}",
body_markdown: Faker::Lorem.sentence, body_markdown: Faker::Lorem.sentence,
published: true, published: true,
@ -571,12 +571,12 @@ seeder.create_if_none(DisplayAd) do
end end
segment = AudienceSegment.create!(type_of: :manual) segment = AudienceSegment.create!(type_of: :manual)
DisplayAd.create!( Billboard.create!(
name: "#{Faker::Lorem.word} (Manually Managed Audience)", name: "#{Faker::Lorem.word} (Manually Managed Audience)",
body_markdown: Faker::Lorem.sentence, body_markdown: Faker::Lorem.sentence,
published: true, published: true,
approved: true, approved: true,
placement_area: DisplayAd::ALLOWED_PLACEMENT_AREAS.sample, placement_area: Billboard::ALLOWED_PLACEMENT_AREAS.sample,
audience_segment: segment, audience_segment: segment,
) )
end end

4
lib/data_update_scripts/20220830153942_generate_display_ad_names.rb
View file

@ -1,8 +1,8 @@
module DataUpdateScripts module DataUpdateScripts
class GenerateDisplayAdNames class GenerateDisplayAdNames
def run def run
DisplayAd.where(name: nil).find_each do |display_ad| Billboard.where(name: nil).find_each do |display_ad|
display_ad.update(name: "Display Ad #{display_ad.id}") display_ad.update(name: "Billboard #{display_ad.id}")
end end
end end
end end

7
lib/data_update_scripts/20230828081018_rename_display_ad_roles_to_billboards.rb Normal file
View file

@ -0,0 +1,7 @@
module DataUpdateScripts
class RenameDisplayAdRolesToBillboards
def run
Role.where(resource_type: "DisplayAd").update_all(resource_type: "Billboard")
end
end
end

2
spec/factories/billboards.rb
View file

@ -1,5 +1,5 @@
FactoryBot.define do FactoryBot.define do
factory :billboard, class: "DisplayAd", aliases: %i[display_ad] do factory :billboard, class: "Billboard", aliases: %i[display_ad] do
placement_area { "sidebar_left" } placement_area { "sidebar_left" }
sequence(:body_markdown) { |n| "Hello _hey_ Hey hey #{n}" } sequence(:body_markdown) { |n| "Hello _hey_ Hey hey #{n}" }
organization organization

14
spec/lib/data_update_scripts/rename_display_ad_roles_to_billboards_spec.rb Normal file
View file

@ -0,0 +1,14 @@
require "rails_helper"
require Rails.root.join(
"lib/data_update_scripts/20230828081018_rename_display_ad_roles_to_billboards.rb",
)
describe DataUpdateScripts::RenameDisplayAdRolesToBillboards do
it "updates role" do
role = build(:role, resource_type: "DisplayAd")
role.save(validate: false)
described_class.new.run
role.reload
expect(role.resource_type).to eq("Billboard")
end
end

12
spec/models/display_ad_spec.rb → spec/models/billboard_spec.rb
View file

@ -1,6 +1,6 @@
require "rails_helper" require "rails_helper"
RSpec.describe DisplayAd do RSpec.describe Billboard do
let(:organization) { build(:organization) } let(:organization) { build(:organization) }
let(:billboard) { build(:billboard, organization: nil) } let(:billboard) { build(:billboard, organization: nil) }
let(:audience_segment) { create(:audience_segment) } let(:audience_segment) { create(:audience_segment) }
@ -164,7 +164,7 @@ RSpec.describe DisplayAd do
describe "#process_markdown" do describe "#process_markdown" do
# FastImage.size is called when synchronous_detail_detection: true is passed to Html::Parser#prefix_all_images # FastImage.size is called when synchronous_detail_detection: true is passed to Html::Parser#prefix_all_images
# which should be the case for DisplayAd # which should be the case for Billboard
# Images::Optimizer is also called with widht # Images::Optimizer is also called with widht
it "calls Html::Parser#prefix_all_images with parameters" do it "calls Html::Parser#prefix_all_images with parameters" do
# Html::Parser.new(html).prefix_all_images(prefix_width, synchronous_detail_detection: true).html # Html::Parser.new(html).prefix_all_images(prefix_width, synchronous_detail_detection: true).html
@ -175,7 +175,7 @@ RSpec.describe DisplayAd do
create(:billboard, body_markdown: image_md, placement_area: "post_comments") create(:billboard, body_markdown: image_md, placement_area: "post_comments")
expect(FastImage).to have_received(:size).with(image_url, { timeout: 10 }) expect(FastImage).to have_received(:size).with(image_url, { timeout: 10 })
# width is billboard.prefix_width # width is billboard.prefix_width
expect(Images::Optimizer).to have_received(:call).with(image_url, width: DisplayAd::POST_WIDTH) expect(Images::Optimizer).to have_received(:call).with(image_url, width: Billboard::POST_WIDTH)
# Images::Optimizer.call(source, width: width) # Images::Optimizer.call(source, width: width)
end end
@ -185,7 +185,7 @@ RSpec.describe DisplayAd do
allow(Images::Optimizer).to receive(:call).and_return(image_url) allow(Images::Optimizer).to receive(:call).and_return(image_url)
image_md = "![Image description](#{image_url})<p style='margin-top:100px'>Hello <em>hey</em> Hey hey</p>" image_md = "![Image description](#{image_url})<p style='margin-top:100px'>Hello <em>hey</em> Hey hey</p>"
create(:billboard, body_markdown: image_md, placement_area: "post_sidebar") create(:billboard, body_markdown: image_md, placement_area: "post_sidebar")
expect(Images::Optimizer).to have_received(:call).with(image_url, width: DisplayAd::SIDEBAR_WIDTH) expect(Images::Optimizer).to have_received(:call).with(image_url, width: Billboard::SIDEBAR_WIDTH)
end end
it "uses post width for feed location" do it "uses post width for feed location" do
@ -194,7 +194,7 @@ RSpec.describe DisplayAd do
allow(Images::Optimizer).to receive(:call).and_return(image_url) allow(Images::Optimizer).to receive(:call).and_return(image_url)
image_md = "![Image description](#{image_url})<p style='margin-top:100px'>Hello <em>hey</em> Hey hey</p>" image_md = "![Image description](#{image_url})<p style='margin-top:100px'>Hello <em>hey</em> Hey hey</p>"
create(:billboard, body_markdown: image_md, placement_area: "feed_second") create(:billboard, body_markdown: image_md, placement_area: "feed_second")
expect(Images::Optimizer).to have_received(:call).with(image_url, width: DisplayAd::POST_WIDTH) expect(Images::Optimizer).to have_received(:call).with(image_url, width: Billboard::POST_WIDTH)
end end
it "keeps the same processed_html if markdown was not changed" do it "keeps the same processed_html if markdown was not changed" do
@ -445,7 +445,7 @@ RSpec.describe DisplayAd do
end end
describe "seldom_seen scope" do describe "seldom_seen scope" do
let(:low_impression_count) { DisplayAd::LOW_IMPRESSION_COUNT } let(:low_impression_count) { Billboard::LOW_IMPRESSION_COUNT }
let!(:low_impression_ad) { create(:billboard, impressions_count: low_impression_count - 1) } let!(:low_impression_ad) { create(:billboard, impressions_count: low_impression_count - 1) }
let!(:high_impression_ad) { create(:billboard, impressions_count: low_impression_count + 1) } let!(:high_impression_ad) { create(:billboard, impressions_count: low_impression_count + 1) }

6
spec/queries/admin/users_query_spec.rb
View file

@ -122,10 +122,10 @@ RSpec.describe Admin::UsersQuery, type: :query do
end end
context "when given multiple single_resource_admin roles" do context "when given multiple single_resource_admin roles" do
let(:roles) { ["Admin", "Super Admin", "Resource Admin: DataUpdateScript", "Resource Admin: DisplayAd"] } let(:roles) { ["Admin", "Super Admin", "Resource Admin: DataUpdateScript", "Resource Admin: Billboard"] }
let!(:user8) { create(:user).tap { |u| u.add_role(:single_resource_admin, DisplayAd) } } let!(:user8) { create(:user).tap { |u| u.add_role(:single_resource_admin, Billboard) } }
# This user is provided to ensure our test looks for unique users even if they have duplicate roles # This user is provided to ensure our test looks for unique users even if they have duplicate roles
let!(:user9) { create(:user, :super_admin).tap { |u| u.add_role(:single_resource_admin, DisplayAd) } } let!(:user9) { create(:user, :super_admin).tap { |u| u.add_role(:single_resource_admin, Billboard) } }
it { is_expected.to eq([user9, user8, user7, user6, user5, user4]) } it { is_expected.to eq([user9, user8, user7, user6, user5, user4]) }
end end

2
spec/queries/billboards/filtered_ads_query_spec.rb
View file

@ -15,7 +15,7 @@ RSpec.describe Billboards::FilteredAdsQuery, type: :query do
def filter_billboards(**options) def filter_billboards(**options)
defaults = { defaults = {
billboards: DisplayAd, area: placement_area, user_signed_in: false billboards: Billboard, area: placement_area, user_signed_in: false
} }
described_class.call(**options.reverse_merge(defaults)) described_class.call(**options.reverse_merge(defaults))
end end

26
spec/requests/admin/billboards_spec.rb
View file

@ -12,7 +12,7 @@ RSpec.describe "/admin/customization/billboards" do
end end
let(:post_resource) { post admin_billboards_path, params: params } let(:post_resource) { post admin_billboards_path, params: params }
it_behaves_like "an InternalPolicy dependant request", DisplayAd do it_behaves_like "an InternalPolicy dependant request", Billboard do
let(:request) { get_resource } let(:request) { get_resource }
end end
@ -50,7 +50,7 @@ RSpec.describe "/admin/customization/billboards" do
it "creates a new billboard" do it "creates a new billboard" do
expect do expect do
post_resource post_resource
end.to change { DisplayAd.all.count }.by(1) end.to change { Billboard.all.count }.by(1)
end end
it "busts sidebar" do it "busts sidebar" do
@ -61,26 +61,26 @@ RSpec.describe "/admin/customization/billboards" do
it "sets creator to current_user" do it "sets creator to current_user" do
post_resource post_resource
expect(DisplayAd.last.creator_id).to eq(super_admin.id) expect(Billboard.last.creator_id).to eq(super_admin.id)
end end
it "fails to create a new billboard with invalid target geolocations" do it "fails to create a new billboard with invalid target geolocations" do
expect do expect do
post admin_billboards_path, params: params.merge(target_geolocations: "US-UM, CA-UH") post admin_billboards_path, params: params.merge(target_geolocations: "US-UM, CA-UH")
end.not_to change { DisplayAd.all.count } end.not_to change { Billboard.all.count }
end end
it "creates a new billboard with no target geolocations" do it "creates a new billboard with no target geolocations" do
expect do expect do
post admin_billboards_path, params: params.merge(target_geolocations: nil) post admin_billboards_path, params: params.merge(target_geolocations: nil)
end.to change { DisplayAd.all.count }.by(1) end.to change { Billboard.all.count }.by(1)
end end
end end
describe "PUT /admin/customization/billboards" do describe "PUT /admin/customization/billboards" do
let!(:billboard) { create(:billboard, approved: false) } let!(:billboard) { create(:billboard, approved: false) }
it "updates DisplayAd's approved value" do it "updates Billboard's approved value" do
Timecop.freeze(Time.current) do Timecop.freeze(Time.current) do
expect do expect do
put admin_billboard_path(billboard.id), params: params put admin_billboard_path(billboard.id), params: params
@ -88,7 +88,7 @@ RSpec.describe "/admin/customization/billboards" do
end end
end end
it "updates DisplayAd's priority value" do it "updates Billboard's priority value" do
Timecop.freeze(Time.current) do Timecop.freeze(Time.current) do
expect do expect do
put admin_billboard_path(billboard.id), params: params put admin_billboard_path(billboard.id), params: params
@ -108,13 +108,13 @@ RSpec.describe "/admin/customization/billboards" do
it "deletes the Display Ad" do it "deletes the Display Ad" do
expect do expect do
delete admin_billboard_path(billboard.id) delete admin_billboard_path(billboard.id)
end.to change { DisplayAd.all.count }.by(-1) end.to change { Billboard.all.count }.by(-1)
end end
end end
end end
context "when the user is a single resource admin" do context "when the user is a single resource admin" do
let(:single_resource_admin) { create(:user, :single_resource_admin, resource: DisplayAd) } let(:single_resource_admin) { create(:user, :single_resource_admin, resource: Billboard) }
before { sign_in single_resource_admin } before { sign_in single_resource_admin }
@ -129,19 +129,19 @@ RSpec.describe "/admin/customization/billboards" do
it "creates a new billboard" do it "creates a new billboard" do
expect do expect do
post_resource post_resource
end.to change { DisplayAd.all.count }.by(1) end.to change { Billboard.all.count }.by(1)
end end
it "sets creator to current_user" do it "sets creator to current_user" do
post_resource post_resource
expect(DisplayAd.last.creator_id).to eq(single_resource_admin.id) expect(Billboard.last.creator_id).to eq(single_resource_admin.id)
end end
end end
describe "PUT /admin/customization/billboards" do describe "PUT /admin/customization/billboards" do
let!(:billboard) { create(:billboard, approved: false) } let!(:billboard) { create(:billboard, approved: false) }
it "updates DisplayAd's approved value" do it "updates Billboard's approved value" do
Timecop.freeze(Time.current) do Timecop.freeze(Time.current) do
expect do expect do
put admin_billboard_path(billboard.id), params: params put admin_billboard_path(billboard.id), params: params
@ -156,7 +156,7 @@ RSpec.describe "/admin/customization/billboards" do
it "deletes the Display Ad" do it "deletes the Display Ad" do
expect do expect do
delete admin_billboard_path(billboard.id) delete admin_billboard_path(billboard.id)
end.to change { DisplayAd.all.count }.by(-1) end.to change { Billboard.all.count }.by(-1)
end end
end end
end end

2
spec/requests/api/v1/billboards_spec.rb
View file

@ -89,7 +89,7 @@ RSpec.describe "Api::V1::Billboards" do
post api_billboards_path, post api_billboards_path,
params: billboard_params.merge(target_geolocations: "US-FAKE").to_json, params: billboard_params.merge(target_geolocations: "US-FAKE").to_json,
headers: auth_header headers: auth_header
end.not_to change(DisplayAd, :count) end.not_to change(Billboard, :count)
expect(response).to have_http_status(:unprocessable_entity) expect(response).to have_http_status(:unprocessable_entity)
expect(response.media_type).to eq("application/json") expect(response.media_type).to eq("application/json")

10
spec/requests/api/v1/docs/billboards_spec.rb
View file

@ -24,7 +24,7 @@ RSpec.describe "api/v1/billboards" do
response(200, "successful") do response(200, "successful") do
schema type: :array, schema type: :array,
items: { "$ref": "#/components/schemas/DisplayAd" } items: { "$ref": "#/components/schemas/Billboard" }
let(:"api-key") { api_secret.secret } let(:"api-key") { api_secret.secret }
add_examples add_examples
@ -50,7 +50,7 @@ RSpec.describe "api/v1/billboards" do
produces "application/json" produces "application/json"
consumes "application/json" consumes "application/json"
parameter name: :billboard, in: :body, schema: { type: :object, parameter name: :billboard, in: :body, schema: { type: :object,
items: { "$ref": "#/components/schemas/DisplayAd" } } items: { "$ref": "#/components/schemas/Billboard" } }
let(:billboard) do let(:billboard) do
{ {
@ -68,7 +68,7 @@ RSpec.describe "api/v1/billboards" do
response "201", "A billboard" do response "201", "A billboard" do
schema type: :object, schema type: :object,
items: { "$ref": "#/components/schemas/DisplayAd" } items: { "$ref": "#/components/schemas/Billboard" }
let(:"api-key") { api_secret.secret } let(:"api-key") { api_secret.secret }
add_examples add_examples
@ -161,13 +161,13 @@ RSpec.describe "api/v1/billboards" do
example: 123 example: 123
parameter name: :billboard, in: :body, schema: { type: :object, parameter name: :billboard, in: :body, schema: { type: :object,
items: { "$ref": "#/components/schemas/DisplayAd" } } items: { "$ref": "#/components/schemas/Billboard" } }
let(:placement_area) { "post_comments" } let(:placement_area) { "post_comments" }
response(200, "successful") do response(200, "successful") do
schema type: :object, schema type: :object,
items: { "$ref": "#/components/schemas/DisplayAd" } items: { "$ref": "#/components/schemas/Billboard" }
let(:"api-key") { api_secret.secret } let(:"api-key") { api_secret.secret }
let(:id) { billboard.id } let(:id) { billboard.id }
add_examples add_examples

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

@ -1100,9 +1100,9 @@ end
############################################################################## ##############################################################################
seeder.create_if_none(DisplayAd) do seeder.create_if_none(Billboard) do
org_id = Organization.find_by(slug: "bachmanity").id org_id = Organization.find_by(slug: "bachmanity").id
DisplayAd.create!( Billboard.create!(
organization_id: org_id, organization_id: org_id,
body_markdown: "<h1>This is a regular billboard</h1>", body_markdown: "<h1>This is a regular billboard</h1>",
placement_area: "sidebar_left", placement_area: "sidebar_left",
@ -1111,7 +1111,7 @@ seeder.create_if_none(DisplayAd) do
approved: true, approved: true,
) )
DisplayAd.create!( Billboard.create!(
organization_id: org_id, organization_id: org_id,
body_markdown: "<h1>This is a billboard with a manually managed audience</h1>", body_markdown: "<h1>This is a billboard with a manually managed audience</h1>",
placement_area: "sidebar_left", placement_area: "sidebar_left",
@ -1121,7 +1121,7 @@ seeder.create_if_none(DisplayAd) do
audience_segment: AudienceSegment.where(type_of: :manual).first, audience_segment: AudienceSegment.where(type_of: :manual).first,
) )
DisplayAd.create!( Billboard.create!(
organization_id: org_id, organization_id: org_id,
body_markdown: "<h1>This is a billboard shown to people in Ontario</h1>", body_markdown: "<h1>This is a billboard shown to people in Ontario</h1>",
placement_area: "feed_first", placement_area: "feed_first",
@ -1131,7 +1131,7 @@ seeder.create_if_none(DisplayAd) do
target_geolocations: "CA-ON", target_geolocations: "CA-ON",
) )
DisplayAd.create!( Billboard.create!(
organization_id: org_id, organization_id: org_id,
body_markdown: "<h1>This is a billboard shown to people in the US</h1>", body_markdown: "<h1>This is a billboard shown to people in the US</h1>",
placement_area: "feed_first", placement_area: "feed_first",

8
spec/swagger_helper.rb
View file

@ -398,7 +398,7 @@ The default maximum value can be overridden by \"API_PER_PAGE_MAX\" environment
name: { type: :string, nullable: true } name: { type: :string, nullable: true }
} }
}, },
DisplayAd: { Billboard: {
description: "A Display Ad, aka Billboard, aka Widget", description: "A Display Ad, aka Billboard, aka Widget",
type: :object, type: :object,
properties: { properties: {
@ -409,7 +409,7 @@ The default maximum value can be overridden by \"API_PER_PAGE_MAX\" environment
published: { type: :boolean, description: "Ad must be both published and approved to be in rotation" }, published: { type: :boolean, description: "Ad must be both published and approved to be in rotation" },
organization_id: { type: :integer, description: "Identifies the organization to which the ad belongs", nullable: true }, organization_id: { type: :integer, description: "Identifies the organization to which the ad belongs", nullable: true },
creator_id: { type: :integer, description: "Identifies the user who created the ad.", nullable: true }, creator_id: { type: :integer, description: "Identifies the user who created the ad.", nullable: true },
placement_area: { type: :string, enum: DisplayAd::ALLOWED_PLACEMENT_AREAS, placement_area: { type: :string, enum: Billboard::ALLOWED_PLACEMENT_AREAS,
description: "Identifies which area of site layout the ad can appear in" }, description: "Identifies which area of site layout the ad can appear in" },
tag_list: { type: :string, description: "Tags on which this ad can be displayed (blank is all/any tags)" }, tag_list: { type: :string, description: "Tags on which this ad can be displayed (blank is all/any tags)" },
exclude_article_ids: { type: :string, exclude_article_ids: { type: :string,
@ -423,9 +423,9 @@ The default maximum value can be overridden by \"API_PER_PAGE_MAX\" environment
target_geolocations: { type: :array, target_geolocations: { type: :array,
items: { type: :string }, items: { type: :string },
description: "Locations to show this billboard in (blank means it will be shown in all locations). Specified as a comma-separated list or array of ISO 3166-2 country and optionally region codes)" }, description: "Locations to show this billboard in (blank means it will be shown in all locations). Specified as a comma-separated list or array of ISO 3166-2 country and optionally region codes)" },
display_to: { type: :string, enum: DisplayAd.display_tos.keys, default: "all", display_to: { type: :string, enum: Billboard.display_tos.keys, default: "all",
description: "Potentially limits visitors to whom the ad is visible" }, description: "Potentially limits visitors to whom the ad is visible" },
type_of: { type: :string, enum: DisplayAd.type_ofs.keys, default: "in_house", type_of: { type: :string, enum: Billboard.type_ofs.keys, default: "in_house",
description: <<~DESCRIBE description: <<~DESCRIBE
Types of the billboards: Types of the billboards:
in_house (created by admins), in_house (created by admins),

12
swagger/v1/api_v1.json
View file

@ -1543,7 +1543,7 @@
"schema": { "schema": {
"type": "array", "type": "array",
"items": { "items": {
"$ref": "#/components/schemas/DisplayAd" "$ref": "#/components/schemas/Billboard"
} }
} }
} }
@ -1600,7 +1600,7 @@
"schema": { "schema": {
"type": "object", "type": "object",
"items": { "items": {
"$ref": "#/components/schemas/DisplayAd" "$ref": "#/components/schemas/Billboard"
} }
} }
} }
@ -1635,7 +1635,7 @@
"schema": { "schema": {
"type": "object", "type": "object",
"items": { "items": {
"$ref": "#/components/schemas/DisplayAd" "$ref": "#/components/schemas/Billboard"
} }
} }
} }
@ -1770,7 +1770,7 @@
"schema": { "schema": {
"type": "object", "type": "object",
"items": { "items": {
"$ref": "#/components/schemas/DisplayAd" "$ref": "#/components/schemas/Billboard"
} }
} }
} }
@ -1805,7 +1805,7 @@
"schema": { "schema": {
"type": "object", "type": "object",
"items": { "items": {
"$ref": "#/components/schemas/DisplayAd" "$ref": "#/components/schemas/Billboard"
} }
} }
} }
@ -4213,7 +4213,7 @@
} }
} }
}, },
"DisplayAd": { "Billboard": {
"description": "A Display Ad, aka Billboard, aka Widget", "description": "A Display Ad, aka Billboard, aka Widget",
"type": "object", "type": "object",
"properties": { "properties": {