From 2f9769a89d120863ca6c57261e71deb4e113be43 Mon Sep 17 00:00:00 2001 From: Ben Halpern Date: Thu, 8 Feb 2024 11:55:48 -0500 Subject: [PATCH] Add new sidebar billboard locations and remove legacy campaign area (#20562) * Add new sidebar billboard locations * Remove some specs * Remove campaign tests * Adjust tag-allowed billboards * Fix up targeting functionality * Fix sidebar third render --- app/controllers/application_controller.rb | 8 ++ .../application_metal_controller.rb | 8 ++ .../billboard_events_controller.rb | 10 -- app/controllers/billboards_controller.rb | 8 -- app/helpers/billboard_helper.rb | 20 ++++ app/javascript/packs/admin/billboards.jsx | 3 + app/models/billboard.rb | 6 +- app/views/sidebars/_homepage_content.html.erb | 16 ++-- .../billboards/editBillboards.spec.js | 1 - .../listingFlows/viewListing.spec.js | 15 --- spec/requests/listings_spec.rb | 7 -- spec/requests/sidebars_spec.rb | 1 - spec/requests/stories_index_spec.rb | 91 ++++--------------- 13 files changed, 70 insertions(+), 124 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index c2ba92307..25b6a124f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -271,6 +271,14 @@ class ApplicationController < ActionController::Base end end + def client_geolocation + if session_current_user_id + request.headers["X-Client-Geo"] + else + request.headers["X-Cacheable-Client-Geo"] + end + end + def forward_to_app_config_domain # Let's only redirect get requests for this purpose. return unless request.get? && diff --git a/app/controllers/application_metal_controller.rb b/app/controllers/application_metal_controller.rb index 633494b81..ce26da2eb 100644 --- a/app/controllers/application_metal_controller.rb +++ b/app/controllers/application_metal_controller.rb @@ -15,4 +15,12 @@ class ApplicationMetalController < ActionController::Metal def logger ActionController::Base.logger end + + def client_geolocation + if session_current_user_id + request.headers["X-Client-Geo"] + else + request.headers["X-Cacheable-Client-Geo"] + end + end end diff --git a/app/controllers/billboard_events_controller.rb b/app/controllers/billboard_events_controller.rb index 1d4b90878..d8b147b2c 100644 --- a/app/controllers/billboard_events_controller.rb +++ b/app/controllers/billboard_events_controller.rb @@ -41,14 +41,4 @@ class BillboardEventsController < ApplicationMetalController event_params[:geolocation] = client_geolocation event_params.slice(:context_type, :category, :billboard_id, :article_id, :geolocation) end - - def client_geolocation - # Copied here instead of re-used due to this controller - # inhereting from ApplicationMetalController instead of ApplicationController - if session_current_user_id - request.headers["X-Client-Geo"] - else - request.headers["X-Cacheable-Client-Geo"] - end - end end diff --git a/app/controllers/billboards_controller.rb b/app/controllers/billboards_controller.rb index 852af9040..175c5d80e 100644 --- a/app/controllers/billboards_controller.rb +++ b/app/controllers/billboards_controller.rb @@ -59,14 +59,6 @@ class BillboardsController < ApplicationController current_user&.cached_followed_tag_names&.first(rand(RANDOM_USER_TAG_RANGE_MIN..RANDOM_USER_TAG_RANGE_MAX)) end - def client_geolocation - if session_current_user_id - request.headers["X-Client-Geo"] - else - request.headers["X-Cacheable-Client-Geo"] - end - end - def return_test_billboard? param_present = params[:bb_test_placement_area] == placement_area && params[:bb_test_id].present? present_and_admin = param_present && current_user&.any_admin? diff --git a/app/helpers/billboard_helper.rb b/app/helpers/billboard_helper.rb index 3ee631a7a..3a397bd58 100644 --- a/app/helpers/billboard_helper.rb +++ b/app/helpers/billboard_helper.rb @@ -1,4 +1,7 @@ module BillboardHelper + RANDOM_USER_TAG_RANGE_MIN = 5 + RANDOM_USER_TAG_RANGE_MAX = 32 + def billboards_placement_area_options_array Billboard::ALLOWED_PLACEMENT_AREAS_HUMAN_READABLE.zip(Billboard::ALLOWED_PLACEMENT_AREAS) end @@ -25,4 +28,21 @@ module BillboardHelper def feed_targeted_tag_placement?(area) Billboard::HOME_FEED_PLACEMENTS.include?(area) end + + # Including here because multiple controllers render this in different contexts + # When signed in, this is uncached, but it is cached for signed-out contexts + def get_homepage_sidebar_billboards + user_tags = current_user&.cached_followed_tag_names + &.first(rand(RANDOM_USER_TAG_RANGE_MIN..RANDOM_USER_TAG_RANGE_MAX)) + common_params = { + user_signed_in: user_signed_in?, + user_id: current_user&.id, + user_tags: user_tags + } + common_params[:location] = client_geolocation if user_signed_in? && FeatureFlag.enabled?(Geolocation::FEATURE_FLAG) + billboards = [] + billboards << Billboard.for_display(**common_params, area: "sidebar_right") + billboards << Billboard.for_display(**common_params, area: "sidebar_right_second") + billboards << Billboard.for_display(**common_params, area: "sidebar_right_third") + end end diff --git a/app/javascript/packs/admin/billboards.jsx b/app/javascript/packs/admin/billboards.jsx index 179211982..85a692098 100644 --- a/app/javascript/packs/admin/billboards.jsx +++ b/app/javascript/packs/admin/billboards.jsx @@ -144,6 +144,9 @@ document.ready.then(() => { const targetedTagPlacements = [ 'post_comments', 'post_sidebar', + 'sidebar_right', + 'sidebar_right_second', + 'sidebar_right_third', 'feed_first', 'feed_second', 'feed_third', diff --git a/app/models/billboard.rb b/app/models/billboard.rb index f6ddea5ec..7d8c77894 100644 --- a/app/models/billboard.rb +++ b/app/models/billboard.rb @@ -6,11 +6,13 @@ class Billboard < ApplicationRecord belongs_to :audience_segment, optional: true # rubocop:disable Layout/LineLength - ALLOWED_PLACEMENT_AREAS = %w[sidebar_left sidebar_left_2 sidebar_right feed_first feed_second feed_third home_hero post_sidebar post_comments].freeze + ALLOWED_PLACEMENT_AREAS = %w[sidebar_left sidebar_left_2 sidebar_right sidebar_right_second sidebar_right_third feed_first feed_second feed_third home_hero post_sidebar post_comments].freeze # rubocop:enable Layout/LineLength ALLOWED_PLACEMENT_AREAS_HUMAN_READABLE = ["Sidebar Left (First Position)", "Sidebar Left (Second Position)", - "Sidebar Right (Home)", + "Sidebar Right (Home first position)", + "Sidebar Right (Home second position)", + "Sidebar Right (Home third position)", "Home Feed First", "Home Feed Second", "Home Feed Third", diff --git a/app/views/sidebars/_homepage_content.html.erb b/app/views/sidebars/_homepage_content.html.erb index 225253513..45c4d4808 100644 --- a/app/views/sidebars/_homepage_content.html.erb +++ b/app/views/sidebars/_homepage_content.html.erb @@ -1,7 +1,7 @@ +<% @billboards = get_homepage_sidebar_billboards %>