From 129628b2c22e61c552417297c95abe4f58662549 Mon Sep 17 00:00:00 2001 From: Duke Greene Date: Mon, 24 Jul 2023 13:21:13 -0400 Subject: [PATCH] render left sidebar billboards ('display ads') asynchronously (#19797) * render left sidebar billboards ('display ads') asynchronously * remove request spec expectations for ads that are now rendered async * don't set up path helper for old name style * refactor from global to imported observeDisplayAds call * revert refactor attempt, bad export syntax, may fast follow --- app/views/articles/_sidebar.html.erb | 18 ++++-------------- config/routes.rb | 2 +- spec/requests/stories_index_spec.rb | 27 ++++++++++----------------- 3 files changed, 15 insertions(+), 32 deletions(-) diff --git a/app/views/articles/_sidebar.html.erb b/app/views/articles/_sidebar.html.erb index 2af17ff10..4dce740ae 100644 --- a/app/views/articles/_sidebar.html.erb +++ b/app/views/articles/_sidebar.html.erb @@ -5,19 +5,9 @@ <%= render partial: "layouts/main_nav", locals: { context: "sidebar" } %> <%= render "layouts/sidebar_tags" %> <% end %> - <% cache("display-area-left-#{rand(5)}-#{user_signed_in?}", expires_in: 5.minutes) do %> - <% @left_sidebar_ad = DisplayAd.for_display(area: "sidebar_left", user_signed_in: user_signed_in?) %> - <% if @left_sidebar_ad %> - <%= render partial: "shared/display_ad", locals: { display_ad: @left_sidebar_ad, data_context_type: DisplayAdEvent::CONTEXT_TYPE_HOME } %> - <% end %> - <% end %> - <% cache("display-area-left-2-#{rand(5)}-#{user_signed_in?}", expires_in: 5.minutes) do %> - <% @second_left_sidebar_ad = DisplayAd.for_display(area: "sidebar_left_2", user_signed_in: user_signed_in?) %> - <% if @second_left_sidebar_ad %> -
- <%= render partial: "shared/display_ad", locals: { display_ad: @second_left_sidebar_ad, data_context_type: DisplayAdEvent::CONTEXT_TYPE_HOME } %> -
- <% end %> - <% end %> +
+
+ +<%= javascript_packs_with_chunks_tag "billboard", defer: true %> diff --git a/config/routes.rb b/config/routes.rb index 0ea699205..ed1f53db8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -212,7 +212,7 @@ Rails.application.routes.draw do # temporary keeping both routes while transitioning (renaming) display_ads => billboards get "/display_ads/:placement_area", to: "billboards#show" end - get "/billboards/:placement_area", to: "billboards#show" + get "/billboards/:placement_area", to: "billboards#show", as: :billboard # temporary keeping both routes while transitioning (renaming) display_ads => billboards get "/display_ads/:placement_area", to: "billboards#show" diff --git a/spec/requests/stories_index_spec.rb b/spec/requests/stories_index_spec.rb index ce17a9719..0c133251e 100644 --- a/spec/requests/stories_index_spec.rb +++ b/spec/requests/stories_index_spec.rb @@ -66,39 +66,32 @@ RSpec.describe "StoriesIndex" do expect(response.body).to include("This is a landing page!") end - it "renders all display_ads of different placements when published and approved" do + it "renders display_ads when published and approved" do org = create(:organization) - ad = create(:display_ad, published: true, approved: true, organization: org, placement_area: "sidebar_left") - second_left_ad = create(:display_ad, published: true, approved: true, organization: org, - placement_area: "sidebar_left_2") - right_ad = create(:display_ad, published: true, approved: true, placement_area: "sidebar_right", - organization: org) + ad = create(:display_ad, published: true, approved: true, placement_area: "sidebar_right", + organization: org) get "/" expect(response.body).to include(ad.processed_html) - expect(response.body).to include(second_left_ad.processed_html) - expect(response.body).to include(right_ad.processed_html) end it "does not render display_ads when not approved" do org = create(:organization) - ad = create(:display_ad, published: true, approved: false, organization: org) - right_ad = create(:display_ad, published: true, approved: false, placement_area: "sidebar_right", - organization: org) + ad = create(:display_ad, published: true, approved: false, placement_area: "sidebar_right", + organization: org) get "/" expect(response.body).not_to include(ad.processed_html) - expect(response.body).not_to include(right_ad.processed_html) end - it "renders only one display ad of placement" do + it "renders only one display ad per placement" do org = create(:organization) - left_ad = create(:display_ad, published: true, approved: true, placement_area: "sidebar_left", organization: org) - second_left_ad = create(:display_ad, published: true, approved: true, placement_area: "sidebar_left", - organization: org) + 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) get "/" - expect(response.body).to include(left_ad.processed_html).or(include(second_left_ad.processed_html)) + expect(response.body).to include(ad.processed_html).or(include(second_ad.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