From 9c9663a5b5b60c2e2216152942ceba549c13a910 Mon Sep 17 00:00:00 2001 From: Anna Buianova Date: Fri, 11 Aug 2023 22:00:07 +0300 Subject: [PATCH] Renamed display_ads to billboards: variables and descriptions in tests (#19920) * Renamed display ad variables and descriptions in tests * Renamed api docs display_ads => billboards * Updated billboard cypress seeds * Fixed api billboards specs * Update db/schema.rb Co-authored-by: Duke Greene --------- Co-authored-by: Ben Halpern Co-authored-by: Duke Greene --- .../billboards/deleteBillboards.spec.js | 8 +- .../billboards/editBillboards.spec.js | 2 +- spec/factories/billboard_events.rb | 2 +- .../{display_ads.rb => billboards.rb} | 2 +- spec/helpers/billboard_helper_spec.rb | 4 +- .../generate_display_ad_names_spec.rb | 10 +- spec/models/display_ad_spec.rb | 235 +++++++++--------- .../billboards/filtered_ads_query_spec.rb | 126 +++++----- spec/requests/admin/billboards_spec.rb | 32 +-- .../requests/api/v1/audience_segments_spec.rb | 2 +- spec/requests/api/v1/billboards_spec.rb | 35 +-- .../api/v1/docs/audience_segments_spec.rb | 2 +- ...display_ads_spec.rb => billboards_spec.rb} | 52 ++-- spec/requests/billboard_events_spec.rb | 24 +- spec/requests/billboards_spec.rb | 2 +- spec/requests/stories_index_spec.rb | 27 +- spec/services/billboard_event_rollup_spec.rb | 76 +++--- spec/services/content_renderer_spec.rb | 2 +- spec/support/seeds/seeds_e2e.rb | 2 +- spec/views/admin/billboards/new_spec.rb | 2 +- ...udience_segment_refresh_all_worker_spec.rb | 10 +- 21 files changed, 332 insertions(+), 325 deletions(-) rename spec/factories/{display_ads.rb => billboards.rb} (71%) rename spec/requests/api/v1/docs/{display_ads_spec.rb => billboards_spec.rb} (82%) diff --git a/cypress/e2e/seededFlows/adminFlows/billboards/deleteBillboards.spec.js b/cypress/e2e/seededFlows/adminFlows/billboards/deleteBillboards.spec.js index 1f9268b80..b450123d3 100644 --- a/cypress/e2e/seededFlows/adminFlows/billboards/deleteBillboards.spec.js +++ b/cypress/e2e/seededFlows/adminFlows/billboards/deleteBillboards.spec.js @@ -1,4 +1,4 @@ -describe('Delete Display Ads', () => { +describe('Delete Billboards', () => { beforeEach(() => { cy.testSetup(); cy.fixture('users/adminUser.json').as('user'); @@ -7,7 +7,7 @@ describe('Delete Display Ads', () => { cy.loginAndVisit(user, '/admin/customization/billboards'); cy.findByRole('table').within(() => { - cy.contains('Tests Display Ad') + cy.contains('Tests Billboard') .closest('tr') .findByRole('button', { name: 'Destroy' }) .as('deleteButton') @@ -16,7 +16,7 @@ describe('Delete Display Ads', () => { }); }); - describe('delete a display ad', () => { + describe('delete a billboard', () => { it('should display confirmation modal', () => { cy.findByRole('dialog').contains('Confirm changes').should('be.visible'); }); @@ -36,7 +36,7 @@ describe('Delete Display Ads', () => { cy.get('@deleteButton').should('be.visible'); }); - it('should remove display ad if confirmation text matches', () => { + it('should remove billboard if confirmation text matches', () => { cy.get('@user').then((user) => { cy.findByRole('dialog').within(() => { cy.get('input').type( diff --git a/cypress/e2e/seededFlows/adminFlows/billboards/editBillboards.spec.js b/cypress/e2e/seededFlows/adminFlows/billboards/editBillboards.spec.js index 821b6b0f8..9097a87b7 100644 --- a/cypress/e2e/seededFlows/adminFlows/billboards/editBillboards.spec.js +++ b/cypress/e2e/seededFlows/adminFlows/billboards/editBillboards.spec.js @@ -78,7 +78,7 @@ describe('Billboards Form', () => { }); context( - 'when editing a display ad with a manually managed audience', + 'when editing a billboard with a manually managed audience', () => { beforeEach(() => { cy.visit('/admin/customization/billboards'); diff --git a/spec/factories/billboard_events.rb b/spec/factories/billboard_events.rb index d9279465d..54c36f4b5 100644 --- a/spec/factories/billboard_events.rb +++ b/spec/factories/billboard_events.rb @@ -2,6 +2,6 @@ FactoryBot.define do factory :billboard_event do category { BillboardEvent::CATEGORY_IMPRESSION } context_type { "home" } - association :billboard, factory: :display_ad + billboard end end diff --git a/spec/factories/display_ads.rb b/spec/factories/billboards.rb similarity index 71% rename from spec/factories/display_ads.rb rename to spec/factories/billboards.rb index 529d65136..67381045e 100644 --- a/spec/factories/display_ads.rb +++ b/spec/factories/billboards.rb @@ -1,5 +1,5 @@ FactoryBot.define do - factory :display_ad do + factory :billboard, class: "DisplayAd", aliases: %i[display_ad] do placement_area { "sidebar_left" } sequence(:body_markdown) { |n| "Hello _hey_ Hey hey #{n}" } organization diff --git a/spec/helpers/billboard_helper_spec.rb b/spec/helpers/billboard_helper_spec.rb index 2f7f16b7f..fd8c9ca58 100644 --- a/spec/helpers/billboard_helper_spec.rb +++ b/spec/helpers/billboard_helper_spec.rb @@ -36,7 +36,7 @@ describe BillboardHelper do let(:target_segment) { create(:audience_segment, type_of: "manual") } let!(:different_segment) { create(:audience_segment, type_of: "manual") } - let(:billboard) { build(:display_ad, name: "Manual Test", audience_segment: target_segment) } + let(:billboard) { build(:billboard, name: "Manual Test", audience_segment: target_segment) } it "returns a single option with the billboard's audience segment" do expect(options).to include(["Managed elsewhere", target_segment.id]) @@ -44,7 +44,7 @@ describe BillboardHelper do end context "when the billboard doesn't have an audience segment" do - let(:billboard) { build(:display_ad, name: "No Audience Segment") } + let(:billboard) { build(:billboard, name: "No Audience Segment") } it "raises ArgumentError" do expect { options }.to raise_error(ArgumentError, /must have a target audience segment/) diff --git a/spec/lib/data_update_scripts/generate_display_ad_names_spec.rb b/spec/lib/data_update_scripts/generate_display_ad_names_spec.rb index 1970e50f6..5f43887f8 100644 --- a/spec/lib/data_update_scripts/generate_display_ad_names_spec.rb +++ b/spec/lib/data_update_scripts/generate_display_ad_names_spec.rb @@ -4,22 +4,22 @@ require Rails.root.join( ) describe DataUpdateScripts::GenerateDisplayAdNames do - context "when there is no name for a Display Ad" do + context "when there is no name for a billboard" do it "generates a name for an existing Display Ad" do - display_ad = create(:display_ad, name: nil) + billboard = create(:billboard, name: nil) described_class.new.run - expect(display_ad.reload.name).to eq("Billboard #{display_ad.id}") + expect(billboard.reload.name).to eq("Billboard #{billboard.id}") end end context "when there is a name for the Display Ad" do it "does not change the name" do - display_ad = create(:display_ad, name: "Test") + billboard = create(:billboard, name: "Test") expect do described_class.new.run - end.not_to change { display_ad.reload.name } + end.not_to change { billboard.reload.name } end end end diff --git a/spec/models/display_ad_spec.rb b/spec/models/display_ad_spec.rb index 8e35f8f0f..189bf6c88 100644 --- a/spec/models/display_ad_spec.rb +++ b/spec/models/display_ad_spec.rb @@ -2,7 +2,7 @@ require "rails_helper" RSpec.describe DisplayAd do let(:organization) { build(:organization) } - let(:display_ad) { build(:display_ad, organization: nil) } + let(:billboard) { build(:billboard, organization: nil) } let(:audience_segment) { create(:audience_segment) } before { allow(FeatureFlag).to receive(:enabled?).with(:consistent_rendering, any_args).and_return(true) } @@ -11,7 +11,7 @@ RSpec.describe DisplayAd do describe "validations" do describe "builtin validations" do - subject { display_ad } + subject { billboard } it { is_expected.to belong_to(:organization).optional } it { is_expected.to have_many(:billboard_events).dependent(:destroy) } @@ -22,88 +22,88 @@ RSpec.describe DisplayAd do end it "allows sidebar_right" do - display_ad.placement_area = "sidebar_right" - expect(display_ad).to be_valid + billboard.placement_area = "sidebar_right" + expect(billboard).to be_valid end it "allows sidebar_left" do - display_ad.placement_area = "sidebar_left" - expect(display_ad).to be_valid + billboard.placement_area = "sidebar_left" + expect(billboard).to be_valid end it "allows home_hero with in_house" do - display_ad.placement_area = "home_hero" - display_ad.type_of = "in_house" - expect(display_ad).to be_valid + billboard.placement_area = "home_hero" + billboard.type_of = "in_house" + expect(billboard).to be_valid end it "does not allow home_hero with community" do - display_ad.placement_area = "home_hero" - display_ad.type_of = "community" - expect(display_ad).not_to be_valid - expect(display_ad.errors[:type_of]) + billboard.placement_area = "home_hero" + billboard.type_of = "community" + expect(billboard).not_to be_valid + expect(billboard.errors[:type_of]) .to include("must be in_house if billboard is a Home Hero") end it "disallows unacceptable placement_area" do - display_ad.placement_area = "tsdsdsdds" - expect(display_ad).not_to be_valid + billboard.placement_area = "tsdsdsdds" + expect(billboard).not_to be_valid end it "returns human readable name" do - display_ad.placement_area = "sidebar_left_2" - expect(display_ad.human_readable_placement_area).to eq("Sidebar Left (Second Position)") + billboard.placement_area = "sidebar_left_2" + expect(billboard.human_readable_placement_area).to eq("Sidebar Left (Second Position)") end it "allows creator_id to be set" do - display_ad.creator = build(:user) - expect(display_ad).to be_valid + billboard.creator = build(:user) + expect(billboard).to be_valid end it "does not require creator to be valid" do - display_ad.creator = nil - expect(display_ad).to be_valid + billboard.creator = nil + expect(billboard).to be_valid end it "requires organization_id for community-type ads" do - expect(display_ad).to be_valid + expect(billboard).to be_valid - display_ad.type_of = "community" - expect(display_ad).not_to be_valid - expect(display_ad.errors[:organization]).not_to be_blank + billboard.type_of = "community" + expect(billboard).not_to be_valid + expect(billboard.errors[:organization]).not_to be_blank - display_ad.organization = organization - expect(display_ad).to be_valid - expect(display_ad.errors[:organization]).to be_blank + billboard.organization = organization + expect(billboard).to be_valid + expect(billboard.errors[:organization]).to be_blank end it "allows clear audience segment" do - display_ad.audience_segment_id = audience_segment.id - expect(display_ad).to be_valid + billboard.audience_segment_id = audience_segment.id + expect(billboard).to be_valid - display_ad.audience_segment_id = audience_segment.id - display_ad.audience_segment_type = audience_segment.type_of - expect(display_ad).to be_valid + billboard.audience_segment_id = audience_segment.id + billboard.audience_segment_type = audience_segment.type_of + expect(billboard).to be_valid end it "disallows invalid audience segment" do - display_ad.audience_segment_id = audience_segment.id - display_ad.audience_segment_type = "something_invalid_here" + billboard.audience_segment_id = audience_segment.id + billboard.audience_segment_type = "something_invalid_here" expect(audience_segment.type_of).not_to eq("something_invalid_here") - expect(display_ad).not_to be_valid + expect(billboard).not_to be_valid end it "disallows valid but ambiguous audience segment & id mismatch" do - display_ad.audience_segment_id = audience_segment.id - display_ad.audience_segment_type = "posted" + billboard.audience_segment_id = audience_segment.id + billboard.audience_segment_type = "posted" expect(audience_segment.type_of).not_to eq("posted") - expect(display_ad).not_to be_valid + expect(billboard).not_to be_valid end it "disallows valid but imprecise manual audience segment type" do - display_ad.audience_segment = nil - display_ad.audience_segment_type = "manual" - expect(display_ad).not_to be_valid + billboard.audience_segment = nil + billboard.audience_segment_type = "manual" + expect(billboard).not_to be_valid end end @@ -112,35 +112,35 @@ RSpec.describe DisplayAd do user = create(:user) url = "#{URL.url}/#{user.username}" allow(UnifiedEmbed::Tag).to receive(:validate_link).with(any_args).and_return(url) - username_ad = create(:display_ad, body_markdown: "Hello! {% embed #{url}} %}") + username_ad = create(:billboard, body_markdown: "Hello! {% embed #{url}} %}") expect(username_ad.processed_html).to include("/#{user.username}") expect(username_ad.processed_html).to include("ltag__user__link") end end context "when callbacks are triggered before save" do - before { display_ad.save! } + before { billboard.save! } it "generates #processed_html from #body_markdown" do - expect(display_ad.processed_html).to start_with("

Hello hey Hey hey") + expect(billboard.processed_html).to start_with("

Hello hey Hey hey") end it "does not render

" do div_html = "
Good morning, how are you?
" p_html = "

Good morning, how are you?

" - display_ad.update(body_markdown: div_html) - display_ad.reload - expect(display_ad.processed_html).to eq(p_html) + billboard.update(body_markdown: div_html) + billboard.reload + expect(billboard.processed_html).to eq(p_html) end it "does not render disallowed tags" do - display_ad.update(body_markdown: " Hey hey") - expect(display_ad.processed_html).to eq("

body { color: black} Hey hey

") + billboard.update(body_markdown: " Hey hey") + expect(billboard.processed_html).to eq("

body { color: black} Hey hey

") end it "does not render disallowed attributes" do - display_ad.update(body_markdown: "

Hello hey Hey hey

") - expect(display_ad.processed_html).to start_with("

Hello hey Hey hey

") + billboard.update(body_markdown: "

Hello hey Hey hey

") + expect(billboard.processed_html).to start_with("

Hello hey Hey hey

") end end @@ -154,9 +154,9 @@ RSpec.describe DisplayAd do allow(FastImage).to receive(:size) allow(Images::Optimizer).to receive(:call).and_return(image_url) image_md = "![Image description](#{image_url})

Hello hey Hey hey

" - create(:display_ad, 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 }) - # width is display_ad.prefix_width + # width is billboard.prefix_width expect(Images::Optimizer).to have_received(:call).with(image_url, width: DisplayAd::POST_WIDTH) # Images::Optimizer.call(source, width: width) end @@ -166,7 +166,7 @@ RSpec.describe DisplayAd do allow(FastImage).to receive(:size) allow(Images::Optimizer).to receive(:call).and_return(image_url) image_md = "![Image description](#{image_url})

Hello hey Hey hey

" - create(:display_ad, 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) end @@ -175,47 +175,49 @@ RSpec.describe DisplayAd do allow(FastImage).to receive(:size) allow(Images::Optimizer).to receive(:call).and_return(image_url) image_md = "![Image description](#{image_url})

Hello hey Hey hey

" - create(:display_ad, 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) end it "keeps the same processed_html if markdown was not changed" do - display_ad = create(:display_ad) - html = display_ad.processed_html - display_ad.update(name: "Sample billboard") - display_ad.reload - expect(display_ad.processed_html).to eq(html) + billboard = create(:billboard) + html = billboard.processed_html + billboard.update(name: "Sample billboard") + billboard.reload + expect(billboard.processed_html).to eq(html) end end describe "after_save callbacks" do - let!(:display_ad) { create(:display_ad, name: nil) } + let!(:billboard) { create(:billboard, name: nil) } it "generates a name when one does not exist" do - display_ad_with_name = create(:display_ad, name: "Test") + billboard_with_name = create(:billboard, name: "Test") - expect(display_ad.name).to eq("Billboard #{display_ad.id}") - expect(display_ad_with_name.name).to eq("Test") + expect(billboard.name).to eq("Billboard #{billboard.id}") + expect(billboard_with_name.name).to eq("Test") end end describe ".search_ads" do - let!(:ad) { create(:display_ad, name: "This is an Ad", body_markdown: "Ad Body", placement_area: "post_comments") } + let!(:billboard) do + create(:billboard, name: "This is a billboard", body_markdown: "Billboard Body", placement_area: "post_comments") + end it "finds via name" do - expect(described_class.search_ads("this").ids).to contain_exactly(ad.id) + expect(described_class.search_ads("this").ids).to contain_exactly(billboard.id) end it "finds via body" do - expect(described_class.search_ads("body").ids).to contain_exactly(ad.id) + expect(described_class.search_ads("body").ids).to contain_exactly(billboard.id) end it "finds via placement_area" do - expect(described_class.search_ads("comment").ids).to contain_exactly(ad.id) + expect(described_class.search_ads("comment").ids).to contain_exactly(billboard.id) end it "finds just one result via multiple criteria" do - expect(described_class.search_ads("ad").ids).to contain_exactly(ad.id) + expect(described_class.search_ads("billboard").ids).to contain_exactly(billboard.id) end it "returns empty when no match" do @@ -226,27 +228,27 @@ RSpec.describe DisplayAd do describe ".validate_tag" do it "rejects more than 10 tags" do eleven_tags = "one, two, three, four, five, six, seven, eight, nine, ten, eleven" - expect(build(:display_ad, - name: "This is an Ad", - body_markdown: "Ad Body", + expect(build(:billboard, + name: "This is a billboard", + body_markdown: "Billboard Body", placement_area: "post_comments", tag_list: eleven_tags).valid?).to be(false) end it "rejects tags with length > 30" do tags = "'testing tag length with more than 30 chars', tag" - expect(build(:display_ad, - name: "This is an Ad", - body_markdown: "Ad Body", + expect(build(:billboard, + name: "This is a billboard", + body_markdown: "Billboard Body", placement_area: "post_comments", tag_list: tags).valid?).to be(false) end it "rejects tag with non-alphanumerics" do expect do - build(:display_ad, - name: "This is an Ad", - body_markdown: "Ad Body", + build(:billboard, + name: "This is a billboard", + body_markdown: "Billboard Body", placement_area: "post_comments", tag_list: "c++").validate! end.to raise_error(ActiveRecord::RecordInvalid) @@ -254,21 +256,20 @@ RSpec.describe DisplayAd do it "always downcase tags" do tags = "UPPERCASE, CAPITALIZE" - display_ad = create(:display_ad, - name: "This is an Ad", - body_markdown: "Ad Body", - placement_area: "post_comments", - tag_list: tags) - expect(display_ad.tag_list).to eq(tags.downcase.split(", ")) + billboard = create(:billboard, name: "This is a billboard", + body_markdown: "Billboard Body", + placement_area: "post_comments", + tag_list: tags) + expect(billboard.tag_list).to eq(tags.downcase.split(", ")) end end describe ".validate_geolocations" do subject(:billboard) do build( - :display_ad, - name: "This is an Ad", - body_markdown: "Ad Body", + :billboard, + name: "This is a billboard", + body_markdown: "Billboard Body", placement_area: "post_comments", target_geolocations: geo_input, ) @@ -357,41 +358,41 @@ RSpec.describe DisplayAd do describe "#exclude_articles_ids" do it "processes array of integer ids as expected" do - display_ad.exclude_article_ids = ["11"] - expect(display_ad.exclude_article_ids).to contain_exactly(11) + billboard.exclude_article_ids = ["11"] + expect(billboard.exclude_article_ids).to contain_exactly(11) - display_ad.exclude_article_ids = %w[11 12 13 14] - expect(display_ad.exclude_article_ids).to contain_exactly(11, 12, 13, 14) + billboard.exclude_article_ids = %w[11 12 13 14] + expect(billboard.exclude_article_ids).to contain_exactly(11, 12, 13, 14) - display_ad.exclude_article_ids = "11,12,13,14" - expect(display_ad.exclude_article_ids).to contain_exactly(11, 12, 13, 14) + billboard.exclude_article_ids = "11,12,13,14" + expect(billboard.exclude_article_ids).to contain_exactly(11, 12, 13, 14) - display_ad.exclude_article_ids = "" - expect(display_ad.exclude_article_ids).to eq([]) + billboard.exclude_article_ids = "" + expect(billboard.exclude_article_ids).to eq([]) - display_ad.exclude_article_ids = [] - expect(display_ad.exclude_article_ids).to eq([]) + billboard.exclude_article_ids = [] + expect(billboard.exclude_article_ids).to eq([]) - display_ad.exclude_article_ids = ["", "", ""] - expect(display_ad.exclude_article_ids).to eq([]) + billboard.exclude_article_ids = ["", "", ""] + expect(billboard.exclude_article_ids).to eq([]) - display_ad.exclude_article_ids = [nil] - expect(display_ad.exclude_article_ids).to eq([]) + billboard.exclude_article_ids = [nil] + expect(billboard.exclude_article_ids).to eq([]) - display_ad.exclude_article_ids = nil - expect(display_ad.exclude_article_ids).to eq([]) + billboard.exclude_article_ids = nil + expect(billboard.exclude_article_ids).to eq([]) end it "round-trips to the database as expected" do - display_ad.exclude_article_ids = [11] - display_ad.save! - expect(display_ad.exclude_article_ids).to contain_exactly(11) + billboard.exclude_article_ids = [11] + billboard.save! + expect(billboard.exclude_article_ids).to contain_exactly(11) - display_ad.update(exclude_article_ids: "11,12,13,14") - expect(display_ad.exclude_article_ids).to contain_exactly(11, 12, 13, 14) + billboard.update(exclude_article_ids: "11,12,13,14") + expect(billboard.exclude_article_ids).to contain_exactly(11, 12, 13, 14) - display_ad.update(exclude_article_ids: nil) - expect(display_ad.exclude_article_ids).to eq([]) + billboard.update(exclude_article_ids: nil) + expect(billboard.exclude_article_ids).to eq([]) end end @@ -405,10 +406,10 @@ RSpec.describe DisplayAd do before { allow(AudienceSegmentRefreshWorker).to receive(:perform_async) } it "refreshes audience segment as an asynchronous callback" do - display_ad.save! + billboard.save! expect(AudienceSegmentRefreshWorker).not_to have_received(:perform_async) - display_ad.update audience_segment: audience_segment + billboard.update audience_segment: audience_segment expect(AudienceSegmentRefreshWorker).to have_received(:perform_async) .with(audience_segment.id) end @@ -420,17 +421,17 @@ RSpec.describe DisplayAd do before { allow(AudienceSegmentRefreshWorker).to receive(:perform_async) } it "does not need to refresh audience segment" do - display_ad.update audience_segment: audience_segment + billboard.update audience_segment: audience_segment expect(AudienceSegmentRefreshWorker).not_to have_received(:perform_async) end end describe "seldom_seen scope" do let(:low_impression_count) { DisplayAd::LOW_IMPRESSION_COUNT } - let!(:low_impression_ad) { create(:display_ad, impressions_count: low_impression_count - 1) } - let!(:high_impression_ad) { create(:display_ad, 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) } - before { create(:display_ad, priority: true, impressions_count: low_impression_count + 1) } + before { create(:billboard, 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("sidebar_left")).to include(low_impression_ad) diff --git a/spec/queries/billboards/filtered_ads_query_spec.rb b/spec/queries/billboards/filtered_ads_query_spec.rb index a67b37ece..91ced2069 100644 --- a/spec/queries/billboards/filtered_ads_query_spec.rb +++ b/spec/queries/billboards/filtered_ads_query_spec.rb @@ -3,17 +3,17 @@ require "rails_helper" RSpec.describe Billboards::FilteredAdsQuery, type: :query do let(:placement_area) { "post_sidebar" } - def create_display_ad(**options) + def create_billboard(**options) defaults = { approved: true, published: true, placement_area: placement_area, display_to: :all } - create(:display_ad, **options.reverse_merge(defaults)) + create(:billboard, **options.reverse_merge(defaults)) end - def filter_ads(**options) + def filter_billboards(**options) defaults = { billboards: DisplayAd, area: placement_area, user_signed_in: false } @@ -21,39 +21,39 @@ RSpec.describe Billboards::FilteredAdsQuery, type: :query do end context "when ads are not approved or published" do - let!(:unapproved) { create_display_ad approved: false } - let!(:unpublished) { create_display_ad published: false } - let!(:display_ad) { create_display_ad } + let!(:unapproved) { create_billboard approved: false } + let!(:unpublished) { create_billboard published: false } + let!(:billboard) { create_billboard } it "does not display unapproved or unpublished ads" do - filtered = filter_ads + filtered = filter_billboards expect(filtered).not_to include(unapproved) expect(filtered).not_to include(unpublished) - expect(filtered).to include(display_ad) + expect(filtered).to include(billboard) end end context "when considering article_tags" do - let!(:no_tags) { create_display_ad cached_tag_list: "" } - let!(:mismatched) { create_display_ad cached_tag_list: "career" } + let!(:no_tags) { create_billboard cached_tag_list: "" } + let!(:mismatched) { create_billboard cached_tag_list: "career" } it "will show no-tag display ads if the article tags do not contain matching tags" do - filtered = filter_ads(article_id: 11, article_tags: %w[javascript]) + filtered = filter_billboards(article_id: 11, article_tags: %w[javascript]) expect(filtered).not_to include(mismatched) expect(filtered).to include(no_tags) end it "will show display ads with no tags set if there are no article tags" do - filtered = filter_ads(article_id: 11, article_tags: []) + filtered = filter_billboards(article_id: 11, article_tags: []) expect(filtered).not_to include(mismatched) expect(filtered).to include(no_tags) end context "when available ads have matching tags" do - let!(:matching) { create_display_ad cached_tag_list: "linux, git, go" } + let!(:matching) { create_billboard cached_tag_list: "linux, git, go" } it "will show the display ads that contain tags that match any of the article tags" do - filtered = filter_ads article_id: 11, article_tags: %w[linux productivity] + filtered = filter_billboards article_id: 11, article_tags: %w[linux productivity] expect(filtered).not_to include(mismatched) expect(filtered).to include(matching) expect(filtered).to include(no_tags) @@ -62,26 +62,26 @@ RSpec.describe Billboards::FilteredAdsQuery, type: :query do end context "when considering user_tags" do - let!(:no_tags) { create_display_ad placement_area: "feed_first", cached_tag_list: "" } - let!(:mismatched) { create_display_ad placement_area: "feed_first", cached_tag_list: "career" } + let!(:no_tags) { create_billboard placement_area: "feed_first", cached_tag_list: "" } + let!(:mismatched) { create_billboard placement_area: "feed_first", cached_tag_list: "career" } it "will show no-tag display ads if the user tags do not contain matching tags" do - filtered = filter_ads(area: "feed_first", user_tags: %w[javascript]) + filtered = filter_billboards(area: "feed_first", user_tags: %w[javascript]) expect(filtered).not_to include(mismatched) expect(filtered).to include(no_tags) end it "will show display ads with no tags set if there are no user tags" do - filtered = filter_ads(area: "feed_first", user_tags: []) + filtered = filter_billboards(area: "feed_first", user_tags: []) expect(filtered).not_to include(mismatched) expect(filtered).to include(no_tags) end context "when available ads have matching tags" do - let!(:matching) { create_display_ad placement_area: "feed_first", cached_tag_list: "linux, git, go" } + let!(:matching) { create_billboard placement_area: "feed_first", cached_tag_list: "linux, git, go" } it "will show the display ads that contain tags that match any of the user tags" do - filtered = filter_ads area: "feed_first", user_tags: %w[linux productivity] + filtered = filter_billboards area: "feed_first", user_tags: %w[linux productivity] expect(filtered).not_to include(mismatched) expect(filtered).to include(matching) expect(filtered).to include(no_tags) @@ -90,35 +90,35 @@ RSpec.describe Billboards::FilteredAdsQuery, type: :query do end context "when considering users_signed_in" do - let!(:for_logged_in) { create_display_ad display_to: :logged_in } - let!(:for_logged_out) { create_display_ad display_to: :logged_out } - let!(:for_all_users) { create_display_ad display_to: :all } + let!(:for_logged_in) { create_billboard display_to: :logged_in } + let!(:for_logged_out) { create_billboard display_to: :logged_out } + let!(:for_all_users) { create_billboard display_to: :all } it "always shows :all, only shows -in/-out appropriately" do - filtered = filter_ads user_signed_in: true + filtered = filter_billboards user_signed_in: true expect(filtered).to contain_exactly(for_logged_in, for_all_users) - filtered = filter_ads user_signed_in: false + filtered = filter_billboards user_signed_in: false expect(filtered).to contain_exactly(for_logged_out, for_all_users) end end context "when considering article_exclude_ids" do - let!(:ex_article1) { create_display_ad exclude_article_ids: "11,12" } - let!(:another_ex_article2) { create_display_ad exclude_article_ids: "12,13" } - let!(:no_excludes) { create_display_ad } + let!(:ex_article1) { create_billboard exclude_article_ids: "11,12" } + let!(:another_ex_article2) { create_billboard exclude_article_ids: "12,13" } + let!(:no_excludes) { create_billboard } it "will show display ads that exclude articles appropriately" do - filtered = filter_ads article_id: 11 + filtered = filter_billboards article_id: 11 expect(filtered).to contain_exactly(another_ex_article2, no_excludes) - filtered = filter_ads article_id: 12 + filtered = filter_billboards article_id: 12 expect(filtered).to contain_exactly(no_excludes) - filtered = filter_ads article_id: 13 + filtered = filter_billboards article_id: 13 expect(filtered).to contain_exactly(ex_article1, no_excludes) - filtered = filter_ads article_id: 14 + filtered = filter_billboards article_id: 14 expect(filtered).to contain_exactly(ex_article1, another_ex_article2, no_excludes) end end @@ -126,73 +126,73 @@ RSpec.describe Billboards::FilteredAdsQuery, type: :query do context "when considering audience segmentation" do let!(:in_segment) { create(:user) } let!(:audience_segment) { create(:audience_segment, type_of: :no_posts_yet) } - let!(:targets_segment) { create_display_ad audience_segment: audience_segment } - let!(:no_targets) { create_display_ad display_to: :all } + let!(:targets_segment) { create_billboard audience_segment: audience_segment } + let!(:no_targets) { create_billboard display_to: :all } let!(:not_in_segment) { create(:user) } # won't be in any segment before do - _targets_other = create_display_ad audience_segment: create(:audience_segment) + _targets_other = create_billboard audience_segment: create(:audience_segment) end it "targets users in/out of segment appropriately" do - filtered = filter_ads user_signed_in: true, user_id: in_segment + filtered = filter_billboards user_signed_in: true, user_id: in_segment expect(filtered).to contain_exactly(targets_segment, no_targets) - filtered = filter_ads user_signed_in: true, user_id: not_in_segment + filtered = filter_billboards user_signed_in: true, user_id: not_in_segment expect(filtered).to contain_exactly(no_targets) - filtered = filter_ads user_signed_in: false + filtered = filter_billboards user_signed_in: false expect(filtered).to contain_exactly(no_targets) end end context "when considering ads with organization_id" do - let!(:in_house_ad) { create_display_ad type_of: :in_house } + let!(:in_house_ad) { create_billboard type_of: :in_house } let(:organization) { create(:organization) } let(:other_org) { create(:organization) } let(:no_ads_org) { create(:organization) } - let!(:community_ad) { create_display_ad organization_id: organization.id, type_of: :community } - let!(:other_community) { create_display_ad organization_id: other_org.id, type_of: :community } + let!(:community_ad) { create_billboard organization_id: organization.id, type_of: :community } + let!(:other_community) { create_billboard organization_id: other_org.id, type_of: :community } - let!(:external_ad) { create_display_ad organization_id: organization.id, type_of: :external } - let!(:other_external) { create_display_ad organization_id: other_org.id, type_of: :external } + let!(:external_ad) { create_billboard organization_id: organization.id, type_of: :external } + let!(:other_external) { create_billboard organization_id: other_org.id, type_of: :external } it "always shows :community ad if matching, otherwise shows in_house/external", :aggregate_failure do - filtered = filter_ads organization_id: organization.id + filtered = filter_billboards organization_id: organization.id expect(filtered).to contain_exactly(community_ad) expect(filtered).not_to include(other_community) - filtered = filter_ads organization_id: no_ads_org.id + filtered = filter_billboards organization_id: no_ads_org.id expect(filtered).to contain_exactly(in_house_ad) expect(filtered).not_to include(other_community) - filtered = filter_ads organization_id: nil + filtered = filter_billboards organization_id: nil expect(filtered).to contain_exactly(in_house_ad, external_ad, other_external) expect(filtered).not_to include(community_ad, other_community) end it "suppresses external ads when permit_adjacent_sponsors is false" do - filtered = filter_ads organization_id: organization.id, permit_adjacent_sponsors: false + filtered = filter_billboards organization_id: organization.id, permit_adjacent_sponsors: false expect(filtered).to contain_exactly(community_ad) expect(filtered).not_to include(other_community) - filtered = filter_ads organization_id: nil, permit_adjacent_sponsors: false + filtered = filter_billboards organization_id: nil, permit_adjacent_sponsors: false expect(filtered).to contain_exactly(in_house_ad) expect(filtered).not_to include(other_community) end end context "when considering home hero ads" do - let!(:in_house_ad) { create_display_ad placement_area: "home_hero", type_of: :in_house } + let!(:in_house_ad) { create_billboard placement_area: "home_hero", type_of: :in_house } let(:organization) { create(:organization) } let(:other_org) { create(:organization) } - let!(:community_ad) { create_display_ad organization_id: organization.id, type_of: :community } - let!(:other_community) { create_display_ad organization_id: other_org.id, type_of: :community } + let!(:community_ad) { create_billboard organization_id: organization.id, type_of: :community } + let!(:other_community) { create_billboard organization_id: other_org.id, type_of: :community } it "always shows home hero ads only" do - filtered = filter_ads(area: "home_hero") + filtered = filter_billboards(area: "home_hero") expect(filtered).to contain_exactly(in_house_ad) expect(filtered).not_to include(other_community) expect(filtered).not_to include(community_ad) @@ -200,12 +200,12 @@ RSpec.describe Billboards::FilteredAdsQuery, type: :query do end context "with target geolocations" do - let!(:no_targets) { create_display_ad } - let!(:targets_canada) { create_display_ad(target_geolocations: "CA") } - let!(:targets_new_york_and_canada) { create_display_ad(target_geolocations: "US-NY, CA") } - let!(:targets_california_and_texas) { create_display_ad(target_geolocations: "US-CA, US-TX") } - let!(:targets_quebec_and_newfoundland) { create_display_ad(target_geolocations: "CA-QC, CA-NL") } - let!(:targets_maine_alberta_and_ontario) { create_display_ad(target_geolocations: "US-ME, CA-AB, CA-ON") } + let!(:no_targets) { create_billboard } + let!(:targets_canada) { create_billboard(target_geolocations: "CA") } + let!(:targets_new_york_and_canada) { create_billboard(target_geolocations: "US-NY, CA") } + let!(:targets_california_and_texas) { create_billboard(target_geolocations: "US-CA, US-TX") } + let!(:targets_quebec_and_newfoundland) { create_billboard(target_geolocations: "CA-QC, CA-NL") } + let!(:targets_maine_alberta_and_ontario) { create_billboard(target_geolocations: "US-ME, CA-AB, CA-ON") } context "when location targeting feature is not enabled" do before do @@ -213,7 +213,7 @@ RSpec.describe Billboards::FilteredAdsQuery, type: :query do end it "ignores the target geolocations" do - filtered = filter_ads(location: "CA-NL") # User is in Newfoundland, Canada + filtered = filter_billboards(location: "CA-NL") # User is in Newfoundland, Canada expect(filtered).to include( no_targets, @@ -232,7 +232,7 @@ RSpec.describe Billboards::FilteredAdsQuery, type: :query do end it "shows only billboards with no targeting if no location is provided" do - filtered = filter_ads + filtered = filter_billboards expect(filtered).to include(no_targets) expect(filtered).not_to include( targets_canada, @@ -244,7 +244,7 @@ RSpec.describe Billboards::FilteredAdsQuery, type: :query do end it "shows only billboards whose target location includes the specified location" do - filtered = filter_ads(location: "CA-NL") # User is in Newfoundland, Canada + filtered = filter_billboards(location: "CA-NL") # User is in Newfoundland, Canada expect(filtered).to include( no_targets, @@ -257,7 +257,7 @@ RSpec.describe Billboards::FilteredAdsQuery, type: :query do targets_maine_alberta_and_ontario, ) - filtered = filter_ads(location: "US-CA") # User is in California, USA + filtered = filter_billboards(location: "US-CA") # User is in California, USA expect(filtered).to include( no_targets, targets_california_and_texas, @@ -271,7 +271,7 @@ RSpec.describe Billboards::FilteredAdsQuery, type: :query do end it "shows only billboards targeting the country specifically if no region is provided" do - filtered = filter_ads(location: "CA") # User is in "Canada" + filtered = filter_billboards(location: "CA") # User is in "Canada" expect(filtered).to include( no_targets, diff --git a/spec/requests/admin/billboards_spec.rb b/spec/requests/admin/billboards_spec.rb index 78aa7262e..f0abc345a 100644 --- a/spec/requests/admin/billboards_spec.rb +++ b/spec/requests/admin/billboards_spec.rb @@ -47,7 +47,7 @@ RSpec.describe "/admin/customization/billboards" do end describe "POST /admin/customization/billboards" do - it "creates a new display_ad" do + it "creates a new billboard" do expect do post_resource end.to change { DisplayAd.all.count }.by(1) @@ -78,36 +78,36 @@ RSpec.describe "/admin/customization/billboards" do end describe "PUT /admin/customization/billboards" do - let!(:display_ad) { create(:display_ad, approved: false) } + let!(:billboard) { create(:billboard, approved: false) } it "updates DisplayAd's approved value" do Timecop.freeze(Time.current) do expect do - put admin_billboard_path(display_ad.id), params: params - end.to change { display_ad.reload.approved }.from(false).to(true) + put admin_billboard_path(billboard.id), params: params + end.to change { billboard.reload.approved }.from(false).to(true) end end it "updates DisplayAd's priority value" do Timecop.freeze(Time.current) do expect do - put admin_billboard_path(display_ad.id), params: params - end.to change { display_ad.reload.priority }.from(false).to(true) + put admin_billboard_path(billboard.id), params: params + end.to change { billboard.reload.priority }.from(false).to(true) end end it "redirects back to edit path" do - put admin_billboard_path(display_ad.id), params: params - expect(response.body).to redirect_to edit_admin_billboard_path(display_ad.id) + put admin_billboard_path(billboard.id), params: params + expect(response.body).to redirect_to edit_admin_billboard_path(billboard.id) end end describe "DELETE /admin/billboards/:id" do - let!(:display_ad) { create(:display_ad) } + let!(:billboard) { create(:billboard) } it "deletes the Display Ad" do expect do - delete admin_billboard_path(display_ad.id) + delete admin_billboard_path(billboard.id) end.to change { DisplayAd.all.count }.by(-1) end end @@ -126,7 +126,7 @@ RSpec.describe "/admin/customization/billboards" do end describe "POST /admin/customization/billboards" do - it "creates a new display_ad" do + it "creates a new billboard" do expect do post_resource end.to change { DisplayAd.all.count }.by(1) @@ -139,23 +139,23 @@ RSpec.describe "/admin/customization/billboards" do end describe "PUT /admin/customization/billboards" do - let!(:display_ad) { create(:display_ad, approved: false) } + let!(:billboard) { create(:billboard, approved: false) } it "updates DisplayAd's approved value" do Timecop.freeze(Time.current) do expect do - put admin_billboard_path(display_ad.id), params: params - end.to change { display_ad.reload.approved }.from(false).to(true) + put admin_billboard_path(billboard.id), params: params + end.to change { billboard.reload.approved }.from(false).to(true) end end end describe "DELETE /admin/billboards/:id" do - let!(:display_ad) { create(:display_ad) } + let!(:billboard) { create(:billboard) } it "deletes the Display Ad" do expect do - delete admin_billboard_path(display_ad.id) + delete admin_billboard_path(billboard.id) end.to change { DisplayAd.all.count }.by(-1) end end diff --git a/spec/requests/api/v1/audience_segments_spec.rb b/spec/requests/api/v1/audience_segments_spec.rb index 1972121a9..38605c23a 100644 --- a/spec/requests/api/v1/audience_segments_spec.rb +++ b/spec/requests/api/v1/audience_segments_spec.rb @@ -321,7 +321,7 @@ RSpec.describe "Api::V1::AudienceSegments" do let(:segment) { AudienceSegment.create!(type_of: "manual") } let(:users) { create_list(:user, 3) } - let(:billboard) { create(:display_ad, published: true, approved: true, type_of: "community") } + let(:billboard) { create(:billboard, published: true, approved: true, type_of: "community") } it_behaves_like "an admin-only protected resource" diff --git a/spec/requests/api/v1/billboards_spec.rb b/spec/requests/api/v1/billboards_spec.rb index cd3c6a9a6..d339b79ab 100644 --- a/spec/requests/api/v1/billboards_spec.rb +++ b/spec/requests/api/v1/billboards_spec.rb @@ -17,7 +17,7 @@ RSpec.describe "Api::V1::Billboards" do target_geolocations: "US-WA, CA-BC" } end - let!(:ad1) { create(:display_ad, published: true, approved: true, type_of: "in_house") } + let!(:billboard1) { create(:billboard, published: true, approved: true, type_of: "in_house") } shared_context "when user is authorized" do let(:api_secret) { create(:api_secret) } @@ -39,7 +39,7 @@ RSpec.describe "Api::V1::Billboards" do end describe "POST /api/billboards" do - it "creates a new display_ad" do + it "creates a new billboard" do post api_billboards_path, params: billboard_params.to_json, headers: auth_header expect(response).to have_http_status(:created) @@ -100,12 +100,12 @@ RSpec.describe "Api::V1::Billboards" do describe "GET /api/billboards/:id" do it "returns json response" do - get api_billboard_path(ad1.id), headers: auth_header + get api_billboard_path(billboard1.id), headers: auth_header expect(response).to have_http_status(:success) expect(response.media_type).to eq("application/json") expect(response.parsed_body).to include( - "id" => ad1.id, + "id" => billboard1.id, "published" => true, "approved" => true, "priority" => false, @@ -117,16 +117,16 @@ RSpec.describe "Api::V1::Billboards" do end describe "PUT /api/billboards/:id" do - it "updates an existing display_ad" do - put api_billboard_path(ad1.id), + it "updates an existing billboard" do + put api_billboard_path(billboard1.id), params: billboard_params.merge(name: "Updated!", type_of: "external").to_json, headers: auth_header expect(response).to have_http_status(:success) expect(response.media_type).to eq("application/json") - ad1.reload - expect(ad1.name).to eq("Updated!") - expect(ad1.type_of).to eq("external") + billboard1.reload + expect(billboard1.name).to eq("Updated!") + expect(billboard1.type_of).to eq("external") expect(response.parsed_body.keys).to \ contain_exactly("approved", "body_markdown", "cached_tag_list", "clicks_count", "created_at", "display_to", "id", @@ -139,19 +139,20 @@ RSpec.describe "Api::V1::Billboards" do end it "also accepts target geolocations as an array" do - put api_billboard_path(ad1.id), params: { target_geolocations: %w[US-FL US-GA] }.to_json, headers: auth_header + put api_billboard_path(billboard1.id), params: { target_geolocations: %w[US-FL US-GA] }.to_json, + headers: auth_header expect(response).to have_http_status(:success) expect(response.media_type).to eq("application/json") - ad1.reload - expect(ad1.target_geolocations).to contain_exactly( + billboard1.reload + expect(billboard1.target_geolocations).to contain_exactly( Geolocation.from_iso3166("US-FL"), Geolocation.from_iso3166("US-GA"), ) end it "returns a malformed response with invalid geolocation" do - put api_billboard_path(ad1.id), params: { target_geolocations: "US-FAKE" }.to_json, headers: auth_header + put api_billboard_path(billboard1.id), params: { target_geolocations: "US-FAKE" }.to_json, headers: auth_header expect(response).to have_http_status(:unprocessable_entity) expect(response.media_type).to eq("application/json") @@ -161,11 +162,11 @@ RSpec.describe "Api::V1::Billboards" do end describe "PUT /api/billboards/:id/unpublish" do - it "unpublishes the display_ad" do - put unpublish_api_billboard_path(ad1.id), headers: auth_header + it "unpublishes the billboard" do + put unpublish_api_billboard_path(billboard1.id), headers: auth_header expect(response).to have_http_status(:success) - expect(ad1.reload).not_to be_published + expect(billboard1.reload).not_to be_published end end end @@ -179,7 +180,7 @@ RSpec.describe "Api::V1::Billboards" do context "when unauthorized and get to show" do it "returns unauthorized" do - get api_billboards_path, params: { id: ad1.id }, + get api_billboards_path, params: { id: billboard1.id }, headers: v1_headers.merge({ "api-key" => "invalid api key" }) expect(response).to have_http_status(:unauthorized) end diff --git a/spec/requests/api/v1/docs/audience_segments_spec.rb b/spec/requests/api/v1/docs/audience_segments_spec.rb index e34ec345a..ba7bd55cd 100644 --- a/spec/requests/api/v1/docs/audience_segments_spec.rb +++ b/spec/requests/api/v1/docs/audience_segments_spec.rb @@ -213,7 +213,7 @@ The endpoint supports pagination, and each page will contain `30` segments by de response "409", "Audience segment could not be deleted" do let(:"api-key") { admin_api_secret.secret } let(:id) { segment.id } - let(:billboard) { create(:display_ad, published: true, approved: true) } + let(:billboard) { create(:billboard, published: true, approved: true) } before do billboard.update!(audience_segment: segment) diff --git a/spec/requests/api/v1/docs/display_ads_spec.rb b/spec/requests/api/v1/docs/billboards_spec.rb similarity index 82% rename from spec/requests/api/v1/docs/display_ads_spec.rb rename to spec/requests/api/v1/docs/billboards_spec.rb index 4d8903865..c3bde031f 100644 --- a/spec/requests/api/v1/docs/display_ads_spec.rb +++ b/spec/requests/api/v1/docs/billboards_spec.rb @@ -4,18 +4,18 @@ require "swagger_helper" # rubocop:disable RSpec/EmptyExampleGroup # rubocop:disable RSpec/VariableName -RSpec.describe "api/v1/display_ads" do +RSpec.describe "api/v1/billboards" do let(:Accept) { "application/vnd.forem.api-v1+json" } let(:api_secret) { create(:api_secret) } - let(:display_ad) { create(:display_ad) } + let(:billboard) { create(:billboard) } let(:user) { api_secret.user } before { user.add_role(:admin) } - path "/api/display_ads" do - describe "GET /display_ads" do + path "/api/billboards" do + describe "GET /billboards" do get("Billboards") do - tags "display ads" + tags "billboards" description(<<-DESCRIBE.strip) This endpoint allows the client to retrieve a list of all billboards. DESCRIBE @@ -40,22 +40,22 @@ RSpec.describe "api/v1/display_ads" do end end - describe "POST /display_ads" do + describe "POST /billboards" do post "Create a billboard" do - tags "display ads" + tags "billboards" description(<<-DESCRIBE.strip) This endpoint allows the client to create a new billboard. DESCRIBE produces "application/json" consumes "application/json" - parameter name: :display_ad, in: :body, schema: { type: :object, - items: { "$ref": "#/components/schemas/DisplayAd" } } + parameter name: :billboard, in: :body, schema: { type: :object, + items: { "$ref": "#/components/schemas/DisplayAd" } } - let(:display_ad) do + let(:billboard) do { body_markdown: "# Hi, this is ad\nYep, it's an ad", - name: "Example Ad", + name: "Example Billboard", display_to: "all", approved: true, published: true, @@ -93,10 +93,10 @@ RSpec.describe "api/v1/display_ads" do end end - path "/api/display_ads/{id}" do - describe "GET /display_ads/:id" do + path "/api/billboards/{id}" do + describe "GET /billboards/:id" do get "A billboard (by id)" do - tags "display ads" + tags "billboards" description(<<-DESCRIBE.strip) This endpoint allows the client to retrieve a single billboard, via its id. DESCRIBE @@ -114,7 +114,7 @@ RSpec.describe "api/v1/display_ads" do example: 123 response(200, "successful") do - let(:id) { display_ad.id } + let(:id) { billboard.id } let(:"api-key") { api_secret.secret } add_examples @@ -139,9 +139,9 @@ RSpec.describe "api/v1/display_ads" do end end - describe "PUT /display_ads/:id" do + describe "PUT /billboards/:id" do put "Update a billboard by ID" do - tags "display ads" + tags "billboards" description(<<-DESCRIBE.strip) This endpoint allows the client to update the attributes of a single billboard, via its id. DESCRIBE @@ -160,8 +160,8 @@ RSpec.describe "api/v1/display_ads" do }, example: 123 - parameter name: :display_ad, in: :body, schema: { type: :object, - items: { "$ref": "#/components/schemas/DisplayAd" } } + parameter name: :billboard, in: :body, schema: { type: :object, + items: { "$ref": "#/components/schemas/DisplayAd" } } let(:placement_area) { "post_comments" } @@ -169,7 +169,7 @@ RSpec.describe "api/v1/display_ads" do schema type: :object, items: { "$ref": "#/components/schemas/DisplayAd" } let(:"api-key") { api_secret.secret } - let(:id) { display_ad.id } + let(:id) { billboard.id } add_examples run_test! @@ -185,7 +185,7 @@ RSpec.describe "api/v1/display_ads" do response "401", "unauthorized" do let(:"api-key") { "invalid" } - let(:id) { display_ad.id } + let(:id) { billboard.id } add_examples run_test! @@ -194,10 +194,10 @@ RSpec.describe "api/v1/display_ads" do end end - path "/api/display_ads/{id}/unpublish" do - describe "PUT /display_ads/:id/unpublish" do + path "/api/billboards/{id}/unpublish" do + describe "PUT /billboards/:id/unpublish" do put "Unpublish a billboard" do - tags "display ads" + tags "billboards" description(<<-DESCRIBE.strip) This endpoint allows the client to remove a billboard from rotation by un-publishing it. DESCRIBE @@ -217,7 +217,7 @@ RSpec.describe "api/v1/display_ads" do response(204, "no content") do let(:"api-key") { api_secret.secret } - let(:id) { display_ad.id } + let(:id) { billboard.id } add_examples run_test! @@ -233,7 +233,7 @@ RSpec.describe "api/v1/display_ads" do response "401", "unauthorized" do let(:"api-key") { "invalid" } - let(:id) { display_ad.id } + let(:id) { billboard.id } add_examples run_test! diff --git a/spec/requests/billboard_events_spec.rb b/spec/requests/billboard_events_spec.rb index f9377ef69..74e56c989 100644 --- a/spec/requests/billboard_events_spec.rb +++ b/spec/requests/billboard_events_spec.rb @@ -3,7 +3,7 @@ require "rails_helper" RSpec.describe "BillboardEvents" do let(:user) { create(:user, :trusted) } let(:organization) { create(:organization) } - let(:display_ad) { create(:display_ad, organization_id: organization.id) } + let(:billboard) { create(:billboard, organization_id: organization.id) } describe "POST /billboard_events", throttled_call: true do context "when user signed in" do @@ -14,38 +14,38 @@ RSpec.describe "BillboardEvents" do it "creates a display ad click event" do post "/billboard_events", params: { billboard_event: { - billboard_id: display_ad.id, + billboard_id: billboard.id, context_type: BillboardEvent::CONTEXT_TYPE_HOME, category: BillboardEvent::CATEGORY_CLICK } } - expect(display_ad.reload.clicks_count).to eq(1) + expect(billboard.reload.clicks_count).to eq(1) end it "creates a display ad click event with old params" do post "/billboard_events", params: { display_ad_event: { - display_ad_id: display_ad.id, + display_ad_id: billboard.id, context_type: BillboardEvent::CONTEXT_TYPE_HOME, category: BillboardEvent::CATEGORY_CLICK } } - expect(display_ad.reload.clicks_count).to eq(1) + expect(billboard.reload.clicks_count).to eq(1) end it "creates a display ad impression event" do post "/billboard_events", params: { billboard_event: { - billboard_id: display_ad.id, + billboard_id: billboard.id, context_type: BillboardEvent::CONTEXT_TYPE_HOME, category: BillboardEvent::CATEGORY_IMPRESSION } } - expect(display_ad.reload.impressions_count).to eq(1) + expect(billboard.reload.impressions_count).to eq(1) end it "creates a display ad success rate" do - ad_event_params = { billboard_id: display_ad.id, context_type: BillboardEvent::CONTEXT_TYPE_HOME } + ad_event_params = { billboard_id: billboard.id, context_type: BillboardEvent::CONTEXT_TYPE_HOME } impression_params = ad_event_params.merge(category: BillboardEvent::CATEGORY_IMPRESSION, user: user) create_list(:billboard_event, 4, impression_params) @@ -54,13 +54,13 @@ RSpec.describe "BillboardEvents" do params: { billboard_event: ad_event_params.merge(category: BillboardEvent::CATEGORY_CLICK) }, ) - expect(display_ad.reload.success_rate).to eq(0.25) + expect(billboard.reload.success_rate).to eq(0.25) end it "assigns event to current user" do post "/billboard_events", params: { billboard_event: { - billboard_id: display_ad.id, + billboard_id: billboard.id, context_type: BillboardEvent::CONTEXT_TYPE_HOME, category: BillboardEvent::CATEGORY_IMPRESSION } @@ -71,14 +71,14 @@ RSpec.describe "BillboardEvents" do it "uses a ThrottledCall for data updates" do post "/billboard_events", params: { billboard_event: { - billboard_id: display_ad.id, + billboard_id: billboard.id, context_type: BillboardEvent::CONTEXT_TYPE_HOME, category: BillboardEvent::CATEGORY_IMPRESSION } } expect(ThrottledCall).to have_received(:perform) - .with("billboards_data_update-#{display_ad.id}", throttle_for: instance_of(ActiveSupport::Duration)) + .with("billboards_data_update-#{billboard.id}", throttle_for: instance_of(ActiveSupport::Duration)) end end end diff --git a/spec/requests/billboards_spec.rb b/spec/requests/billboards_spec.rb index 41e5b874b..bfa42f880 100644 --- a/spec/requests/billboards_spec.rb +++ b/spec/requests/billboards_spec.rb @@ -13,7 +13,7 @@ RSpec.describe "Billboards" do published: true, display_to: :all } - create(:display_ad, **options.reverse_merge(defaults)) + create(:billboard, **options.reverse_merge(defaults)) end describe "GET /:username/:slug/billboards/:placement_area" do diff --git a/spec/requests/stories_index_spec.rb b/spec/requests/stories_index_spec.rb index b01a9be50..d72c3e6b8 100644 --- a/spec/requests/stories_index_spec.rb +++ b/spec/requests/stories_index_spec.rb @@ -67,36 +67,37 @@ RSpec.describe "StoriesIndex" do expect(response.body).to include("This is a landing page!") end - it "renders display_ads when published and approved" do - ad = create(:display_ad, published: true, approved: true, placement_area: "sidebar_right", - organization: org) + it "renders billboards when published and approved" do + ad = create(:billboard, published: true, approved: true, placement_area: "sidebar_right", + organization: org) get "/" expect(response.body).to include(ad.processed_html) end - it "does not render display_ads when not approved" do - ad = create(:display_ad, published: true, approved: false, placement_area: "sidebar_right", - organization: org) + it "does not render billboards when not approved" do + ad = create(:billboard, published: true, approved: false, placement_area: "sidebar_right", + organization: org) get "/" expect(response.body).not_to include(ad.processed_html) end - it "renders only one display ad per placement" do - ad = create(:display_ad, published: true, approved: true, placement_area: "sidebar_right", organization: org) - second_ad = create(:display_ad, published: true, approved: true, placement_area: "sidebar_right", - organization: org) + it "renders only one billboard per placement" do + billboard = create(:billboard, published: true, approved: true, placement_area: "sidebar_right", + organization: org) + second_billboard = create(:billboard, published: true, approved: true, placement_area: "sidebar_right", + organization: org) get "/" - expect(response.body).to include(ad.processed_html).or(include(second_ad.processed_html)) + expect(response.body).to include(billboard.processed_html).or(include(second_billboard.processed_html)) expect(response.body).to include("crayons-card crayons-card--secondary crayons-sponsorship").once expect(response.body).to include("sponsorship-dropdown-trigger-").once end - it "renders a hero display ad" do + it "renders a hero billboard" do allow(FeatureFlag).to receive(:enabled?).with(:hero_billboard).and_return(true) - billboard = create(:display_ad, published: true, approved: true, placement_area: "home_hero", organization: org) + billboard = create(:billboard, published: true, approved: true, placement_area: "home_hero", organization: org) get "/" expect(response.body).to include(billboard.processed_html) end diff --git a/spec/services/billboard_event_rollup_spec.rb b/spec/services/billboard_event_rollup_spec.rb index bf4da6672..9be485448 100644 --- a/spec/services/billboard_event_rollup_spec.rb +++ b/spec/services/billboard_event_rollup_spec.rb @@ -1,8 +1,8 @@ require "rails_helper" RSpec.describe BillboardEventRollup, type: :service do - let(:ad1) { create(:display_ad) } - let(:ad2) { create(:display_ad) } + let(:billboard1) { create(:billboard) } + let(:billboard2) { create(:billboard) } let(:user1) { create(:user) } let(:user2) { create(:user) } @@ -24,13 +24,17 @@ RSpec.describe BillboardEventRollup, type: :service do context "when compacting many rows" do before do override_timestamps do - create(:billboard_event, created_at: Date.current - 2, billboard: ad1, user_id: nil, updated_at: Date.current) - create(:billboard_event, created_at: Date.current - 2, billboard: ad1, user_id: nil, updated_at: Date.current) - create(:billboard_event, created_at: Date.current - 2, billboard: ad1, user_id: nil, updated_at: Date.current) - - create(:billboard_event, created_at: Date.current - 2, billboard: ad1, user_id: user1.id, + create(:billboard_event, created_at: Date.current - 2, billboard: billboard1, user_id: nil, + updated_at: Date.current) + create(:billboard_event, created_at: Date.current - 2, billboard: billboard1, user_id: nil, + updated_at: Date.current) + create(:billboard_event, created_at: Date.current - 2, billboard: billboard1, user_id: nil, + updated_at: Date.current) + + create(:billboard_event, created_at: Date.current - 2, billboard: billboard1, user_id: user1.id, + updated_at: Date.current) + create(:billboard_event, created_at: Date.current - 2, billboard: billboard2, user_id: nil, updated_at: Date.current) - create(:billboard_event, created_at: Date.current - 2, billboard: ad2, user_id: nil, updated_at: Date.current) end end @@ -40,9 +44,9 @@ RSpec.describe BillboardEventRollup, type: :service do described_class.rollup(Date.current - 2) expectations = [ - [ad1.id, nil, 3], - [ad1.id, user1.id, 1], - [ad2.id, nil, 1], + [billboard1.id, nil, 3], + [billboard1.id, user1.id, 1], + [billboard2.id, nil, 1], ] results_mapped = BillboardEvent.where(created_at: days_ago_as_range(2)).map do |event| [event.billboard_id, event.user_id, event.counts_for] @@ -53,11 +57,11 @@ RSpec.describe BillboardEventRollup, type: :service do # separate category it "groups by category" do - create(:billboard_event, category: "impression", billboard: ad1, user_id: nil) - create(:billboard_event, category: "impression", billboard: ad1, user_id: nil) - create(:billboard_event, category: "impression", billboard: ad1, user_id: nil) - create(:billboard_event, category: "click", billboard: ad1, user_id: nil) - create(:billboard_event, category: "click", billboard: ad1, user_id: nil) + create(:billboard_event, category: "impression", billboard: billboard1, user_id: nil) + create(:billboard_event, category: "impression", billboard: billboard1, user_id: nil) + create(:billboard_event, category: "impression", billboard: billboard1, user_id: nil) + create(:billboard_event, category: "click", billboard: billboard1, user_id: nil) + create(:billboard_event, category: "click", billboard: billboard1, user_id: nil) described_class.rollup(Date.current) results = BillboardEvent.where(created_at: Date.current.all_day) @@ -68,29 +72,29 @@ RSpec.describe BillboardEventRollup, type: :service do # separate billboard_id it "groups by billboard_id" do - create(:billboard_event, billboard: ad1, user_id: nil) - create(:billboard_event, billboard: ad1, user_id: nil) - create(:billboard_event, billboard: ad1, user_id: nil) - create(:billboard_event, billboard: ad2, user_id: nil) - create(:billboard_event, billboard: ad2, user_id: nil) + create(:billboard_event, billboard: billboard1, user_id: nil) + create(:billboard_event, billboard: billboard1, user_id: nil) + create(:billboard_event, billboard: billboard1, user_id: nil) + create(:billboard_event, billboard: billboard2, user_id: nil) + create(:billboard_event, billboard: billboard2, user_id: nil) described_class.rollup(Date.current) results = BillboardEvent.where(created_at: Date.current.all_day) by_ad = results.index_by { |r| r["billboard_id"] } - expect(by_ad[ad1.id]["counts_for"]).to eq(3) - expect(by_ad[ad2.id]["counts_for"]).to eq(2) + expect(by_ad[billboard1.id]["counts_for"]).to eq(3) + expect(by_ad[billboard2.id]["counts_for"]).to eq(2) end # separate user_id / null it "groups by user_id (including null / logged-out user)" do - create(:billboard_event, billboard: ad1, user: user1) - create(:billboard_event, billboard: ad1, user: user2) - create(:billboard_event, billboard: ad1, user: user2) - create(:billboard_event, billboard: ad1, user: nil) - create(:billboard_event, billboard: ad1, user: nil) - create(:billboard_event, billboard: ad1, user: nil) - create(:billboard_event, billboard: ad1, user: nil) - create(:billboard_event, billboard: ad1, user: nil) + create(:billboard_event, billboard: billboard1, user: user1) + create(:billboard_event, billboard: billboard1, user: user2) + create(:billboard_event, billboard: billboard1, user: user2) + create(:billboard_event, billboard: billboard1, user: nil) + create(:billboard_event, billboard: billboard1, user: nil) + create(:billboard_event, billboard: billboard1, user: nil) + create(:billboard_event, billboard: billboard1, user: nil) + create(:billboard_event, billboard: billboard1, user: nil) described_class.rollup(Date.current) results = BillboardEvent.where(created_at: Date.current.all_day) @@ -102,11 +106,11 @@ RSpec.describe BillboardEventRollup, type: :service do # sums counts_for > 1 it "counts previously crunched" do - create(:billboard_event, billboard: ad1, counts_for: 10, user_id: nil) - create(:billboard_event, billboard: ad1, counts_for: 15, user_id: nil) - create(:billboard_event, billboard: ad1, user_id: nil) - create(:billboard_event, billboard: ad1, user_id: nil) - create(:billboard_event, billboard: ad1, user_id: nil) + create(:billboard_event, billboard: billboard1, counts_for: 10, user_id: nil) + create(:billboard_event, billboard: billboard1, counts_for: 15, user_id: nil) + create(:billboard_event, billboard: billboard1, user_id: nil) + create(:billboard_event, billboard: billboard1, user_id: nil) + create(:billboard_event, billboard: billboard1, user_id: nil) described_class.rollup(Date.current) results = BillboardEvent.where(created_at: Date.current.all_day) diff --git a/spec/services/content_renderer_spec.rb b/spec/services/content_renderer_spec.rb index cf1c4242f..2dec68309 100644 --- a/spec/services/content_renderer_spec.rb +++ b/spec/services/content_renderer_spec.rb @@ -98,7 +98,7 @@ RSpec.describe ContentRenderer do end it "raises ContentParsingError for display ad" do - source = build(:display_ad) + source = build(:billboard) expect do described_class.new(markdown, source: source, user: user).process end.to raise_error(ContentRenderer::ContentParsingError, /This liquid tag can only be used in Articles/) diff --git a/spec/support/seeds/seeds_e2e.rb b/spec/support/seeds/seeds_e2e.rb index 26d3fb603..389e2b82b 100644 --- a/spec/support/seeds/seeds_e2e.rb +++ b/spec/support/seeds/seeds_e2e.rb @@ -1099,7 +1099,7 @@ seeder.create_if_none(DisplayAd) do organization_id: org_id, body_markdown: "

This is a regular billboard

", placement_area: "sidebar_left", - name: "Tests Display Ad", + name: "Tests Billboard", published: true, approved: true, ) diff --git a/spec/views/admin/billboards/new_spec.rb b/spec/views/admin/billboards/new_spec.rb index f2e7bc4b6..059121f78 100644 --- a/spec/views/admin/billboards/new_spec.rb +++ b/spec/views/admin/billboards/new_spec.rb @@ -4,7 +4,7 @@ RSpec.describe "admin/billboards/new" do let(:admin) { build(:user, :super_admin) } before do - assign(:billboard, build(:display_ad)) + assign(:billboard, build(:billboard)) end context "when signed-in" do diff --git a/spec/workers/audience_segment_refresh_all_worker_spec.rb b/spec/workers/audience_segment_refresh_all_worker_spec.rb index c7ab7635f..6acb38f13 100644 --- a/spec/workers/audience_segment_refresh_all_worker_spec.rb +++ b/spec/workers/audience_segment_refresh_all_worker_spec.rb @@ -3,7 +3,7 @@ require "rails_helper" RSpec.describe AudienceSegmentRefreshAllWorker, type: :worker do let(:worker) { subject } let(:approved_and_published_ad) do - create(:display_ad, + create(:billboard, approved: true, published: true, audience_segment: create(:audience_segment)) @@ -11,20 +11,20 @@ RSpec.describe AudienceSegmentRefreshAllWorker, type: :worker do before do allow(AudienceSegmentRefreshWorker).to receive(:perform_bulk) - create(:display_ad, + create(:billboard, approved: false, published: true, audience_segment: create(:audience_segment)) - create(:display_ad, + create(:billboard, approved: true, published: false, audience_segment: create(:audience_segment)) - create(:display_ad, + create(:billboard, approved: false, published: false, audience_segment: create(:audience_segment)) - create(:display_ad, + create(:billboard, approved: true, published: true, audience_segment: approved_and_published_ad.audience_segment)