[pull] main from forem:main

app/controllers/admin/billboards_controller.rb

@@ -67,7 +67,7 @@ def billboard_params
                     :published, :approved, :name, :display_to, :tag_list, :type_of, :color,
                     :exclude_article_ids, :audience_segment_id, :priority, :browser_context,
                     :exclude_role_names, :target_role_names, :include_subforem_ids,
-                    :render_mode, :template, :custom_display_label, :requires_cookies)
+                    :render_mode, :template, :custom_display_label, :requires_cookies, :expires_at)
     end
 
     def authorize_admin

app/controllers/api/v1/billboards_controller.rb

@@ -53,6 +53,7 @@ def permitted_params
                       :audience_segment_type, :audience_segment_id, :priority, :special_behavior,
                       :custom_display_label, :template, :render_mode, :preferred_article_ids,
                       :exclude_role_names, :target_role_names, :include_subforem_ids, :prefer_paired_with_billboard_id,
+                      :expires_at,
                       # Permitting twice allows both comma-separated string and array values
                       :target_geolocations, target_geolocations: []
       end

app/controllers/stories/feeds_controller.rb

@@ -13,6 +13,11 @@ def show
       @stories = assign_feed_stories
 
       add_pinned_article
+
+      # Add edge cache headers for signed out users only
+      unless user_signed_in?
+        set_cache_control_headers(60) # 1 minute expiration
+      end
     end
 
     private

app/models/billboard.rb

@@ -79,15 +79,16 @@ class Billboard < ApplicationRecord
            :validate_in_house_hero_ads,
            :valid_manual_audience_segment,
            :validate_tag,
-           :validate_geolocations
+           :validate_geolocations,
+           :validate_expiration_approval
 
   before_save :process_markdown
   after_save :generate_billboard_name
   after_save :refresh_audience_segment, if: :should_refresh_audience_segment?
   after_save :update_links_with_bb_param
   after_save :update_event_counts_when_taking_down, if: -> { being_taken_down? }
 
-  scope :approved_and_published, -> { where(approved: true, published: true) }
+  scope :approved_and_published, -> { where(approved: true, published: true).where("expires_at IS NULL OR expires_at > ?", Time.current) }
 
   scope :search_ads, lambda { |term|
                        where "name ILIKE :search OR processed_html ILIKE :search OR placement_area ILIKE :search",
@@ -273,6 +274,12 @@ def validate_in_house_hero_ads
     errors.add(:type_of, "must be in_house if billboard is a Home Hero")
   end
 
+  def validate_expiration_approval
+    return unless expires_at.present? && expires_at < Time.current && approved?
+
+    errors.add(:approved, "cannot be set to true if billboard has expired")
+  end
+
   def audience_segment_type
     @audience_segment_type ||= audience_segment&.type_of
   end
@@ -373,6 +380,12 @@ def score
     0 # Just to allow this to repond to .score for abuse reports
   end
 
+  def check_and_handle_expiration
+    return unless expires_at.present? && expires_at < Time.current && approved?
+
+    update_column(:approved, false)
+  end
+
   private
 
   def update_event_counts_when_taking_down

app/views/admin/billboards/_form.html.erb

@@ -157,6 +157,12 @@
       <%= select_tag :approved, options_for_select([false, true], selected: @billboard.approved), class: "crayons-select" %>
     </div>
 
+    <div class="crayons-field">
+      <%= label_tag :expires_at, "Expires at:", class: "crayons-field__label" %>
+      <%= datetime_local_field_tag :expires_at, @billboard.expires_at&.strftime("%Y-%m-%dT%H:%M"), class: "crayons-textfield", placeholder: "Leave blank for no expiration" %>
+      <p class="crayons-field__description">Billboard will automatically be marked as not approved after this time</p>
+    </div>
+
     <div class="crayons-field">
       <%= label_tag :priority, "Prioritized:", class: "crayons-field__label" %>
       <%= select_tag :priority, options_for_select([false, true], selected: @billboard.priority), class: "crayons-select" %>

app/workers/billboards/data_update_worker.rb

@@ -17,6 +17,9 @@ def perform_update(billboard_id)
       billboard = Billboard.find(billboard_id)
       timestamp = Time.current
 
+      # Check and handle expiration before processing other updates
+      billboard.check_and_handle_expiration
+
       return if rand(3) > 0 && billboard.impressions_count > 500_000
       return if rand(2).zero? && billboard.impressions_count > 100_000
 

db/migrate/20250902150028_add_expires_at_to_billboards.rb

@@ -0,0 +1,7 @@
+class AddExpiresAtToBillboards < ActiveRecord::Migration[7.0]
+  disable_ddl_transaction!
+
+  def change
+    add_column :display_ads, :expires_at, :datetime
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2025_08_28_133253) do
+ActiveRecord::Schema[7.0].define(version: 2025_09_02_150028) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
   enable_extension "ltree"
@@ -520,6 +520,7 @@
     t.integer "display_to", default: 0, null: false
     t.integer "exclude_article_ids", default: [], array: true
     t.string "exclude_role_names", default: [], array: true
+    t.datetime "expires_at"
     t.integer "impressions_count", default: 0
     t.integer "include_subforem_ids", default: [], array: true
     t.string "name"

spec/controllers/billboard_events_controller_spec.rb

@@ -0,0 +1,50 @@
+require "rails_helper"
+
+RSpec.describe BillboardEventsController, type: :controller do
+  include Devise::Test::ControllerHelpers
+
+  let(:user) { create(:user) }
+  let(:billboard) { create(:billboard, approved: true, published: true) }
+
+  before do
+    sign_in user
+  end
+
+  describe "POST #create" do
+    let(:valid_params) do
+      {
+        billboard_event: {
+          billboard_id: billboard.id,
+          context_type: "home",
+          category: "impression"
+        }
+      }
+    end
+
+    context "when creating an impression event" do
+      it "enqueues the DataUpdateWorker for processing" do
+        expect(Billboards::DataUpdateWorker).to receive(:perform_async).with(billboard.id.to_s)
+        post :create, params: valid_params
+        expect(response).to have_http_status(:ok)
+      end
+    end
+
+    context "when creating a non-impression event" do
+      let(:click_params) do
+        {
+          billboard_event: {
+            billboard_id: billboard.id,
+            context_type: "home",
+            category: "click"
+          }
+        }
+      end
+
+      it "enqueues the DataUpdateWorker for processing" do
+        expect(Billboards::DataUpdateWorker).to receive(:perform_async).with(billboard.id.to_s)
+        post :create, params: click_params
+        expect(response).to have_http_status(:ok)
+      end
+    end
+  end
+end

spec/models/billboard_spec.rb

@@ -169,6 +169,42 @@
       billboard.audience_segment_type = "manual"
       expect(billboard).not_to be_valid
     end
+
+    describe "expiration validation" do
+      it "allows setting approved to true when expires_at is nil" do
+        billboard.expires_at = nil
+        billboard.approved = true
+        expect(billboard).to be_valid
+      end
+
+      it "allows setting approved to true when expires_at is in the future" do
+        billboard.expires_at = 1.day.from_now
+        billboard.approved = true
+        expect(billboard).to be_valid
+      end
+
+      it "prevents setting approved to true when expires_at is in the past" do
+        billboard.expires_at = 1.day.ago
+        billboard.approved = true
+        expect(billboard).not_to be_valid
+        expect(billboard.errors[:approved]).to include("cannot be set to true if billboard has expired")
+      end
+
+      it "allows setting approved to false when expires_at is in the past" do
+        billboard.expires_at = 1.day.ago
+        billboard.approved = false
+        expect(billboard).to be_valid
+      end
+
+      it "prevents setting approved to true when expires_at is in the past" do
+        billboard.expires_at = 1.day.ago
+        billboard.approved = false # First set to false to pass validation
+        billboard.save!
+        billboard.approved = true # Now try to set to true
+        expect(billboard).not_to be_valid
+        expect(billboard.errors[:approved]).to include("cannot be set to true if billboard has expired")
+      end
+    end
   end
 
   context "when range env var is set" do
@@ -864,4 +900,49 @@
       end
     end
   end
+
+  describe "#check_and_handle_expiration" do
+    let(:billboard) { create(:billboard, approved: true, published: true) }
+
+    it "does nothing when expires_at is nil" do
+      billboard.expires_at = nil
+      expect { billboard.check_and_handle_expiration }.not_to change { billboard.reload.approved }
+    end
+
+    it "does nothing when expires_at is in the future" do
+      billboard.expires_at = 1.day.from_now
+      expect { billboard.check_and_handle_expiration }.not_to change { billboard.reload.approved }
+    end
+
+    it "does nothing when billboard is not approved" do
+      billboard.update!(approved: false)
+      billboard.expires_at = 1.day.ago
+      expect { billboard.check_and_handle_expiration }.not_to change { billboard.reload.approved }
+    end
+
+    it "marks billboard as not approved when expired and currently approved" do
+      billboard.update_column(:expires_at, 1.day.ago)
+      expect { billboard.check_and_handle_expiration }.to change { billboard.reload.approved }.from(true).to(false)
+    end
+  end
+
+  describe "scopes" do
+    describe ".approved_and_published" do
+      let!(:active_billboard) { create(:billboard, approved: true, published: true) }
+      let!(:expired_billboard) { create(:billboard, approved: false, published: true, expires_at: 1.day.ago) }
+      let!(:future_expiry_billboard) { create(:billboard, approved: true, published: true, expires_at: 1.day.from_now) }
+
+      it "includes billboards with no expiration" do
+        expect(described_class.approved_and_published).to include(active_billboard)
+      end
+
+      it "excludes expired billboards" do
+        expect(described_class.approved_and_published).not_to include(expired_billboard)
+      end
+
+      it "includes billboards with future expiration" do
+        expect(described_class.approved_and_published).to include(future_expiry_billboard)
+      end
+    end
+  end
 end

spec/requests/api/v1/billboards_spec.rb

@@ -55,7 +55,7 @@
                           "audience_segment_type", "audience_segment_id", "page_id", "counts_tabulated_at",
                           "exclude_role_names", "target_role_names", "include_subforem_ids", "prefer_paired_with_billboard_id",
                           "custom_display_label", "template", "render_mode", "preferred_article_ids",
-                          "priority", "weight", "target_geolocations", "requires_cookies", "special_behavior")
+                          "priority", "weight", "target_geolocations", "requires_cookies", "special_behavior", "expires_at")
         expect(response.parsed_body["target_geolocations"]).to contain_exactly("US-WA", "CA-BC")
       end
 
@@ -76,7 +76,7 @@
                           "audience_segment_type", "audience_segment_id", "page_id", "prefer_paired_with_billboard_id",
                           "exclude_role_names", "target_role_names", "include_subforem_ids",
                           "custom_display_label", "template", "render_mode", "preferred_article_ids",
-                          "priority", "weight", "target_geolocations", "requires_cookies", "special_behavior")
+                          "priority", "weight", "target_geolocations", "requires_cookies", "special_behavior", "expires_at")
         expect(response.parsed_body["target_geolocations"]).to contain_exactly("US-WA", "CA-BC")
       end
 
@@ -143,7 +143,7 @@
                           "audience_segment_type", "audience_segment_id", "special_behavior",
                           "exclude_role_names", "target_role_names", "include_subforem_ids",
                           "custom_display_label", "template", "render_mode", "preferred_article_ids",
-                          "priority", "weight", "target_geolocations", "prefer_paired_with_billboard_id")
+                          "priority", "weight", "target_geolocations", "prefer_paired_with_billboard_id", "expires_at")
       end
 
       it "also accepts target geolocations as an array" do

spec/requests/stories/feeds_spec.rb

@@ -78,6 +78,15 @@
           "tag_list" => article.decorate.cached_tag_list_array,
         )
       end
+
+      it "sets cache control headers for edge caching" do
+        get stories_feed_path
+
+        expect(response.headers["Cache-Control"]).to eq("public, no-cache")
+        expect(response.headers["X-Accel-Expires"]).to eq("60")
+        expect(response.headers["Surrogate-Control"]).to include("max-age=60")
+        expect(response.headers["Surrogate-Control"]).to include("stale-if-error=26400")
+      end
     end
 
     context "when rendering an article that is pinned" do
@@ -189,6 +198,24 @@
           "current_user_signed_in" => true
         )
       end
+
+      it "does not set cache control headers for edge caching" do
+        payload = {
+          user_id: user.id,
+          exp: 5.minutes.from_now.to_i # Token expires in 5 minutes
+        }
+        token = JWT.encode(payload, Rails.application.secret_key_base)
+        get stories_feed_path, headers: { "Authorization" => "Bearer #{token}" }
+
+        # Should not have the specific edge caching headers we set for signed out users
+        expect(response.headers["X-Accel-Expires"]).to be_nil
+        expect(response.headers["Surrogate-Control"]).to be_nil
+
+        # Cache-Control might be set by other parts of the system, so we check it doesn't contain our specific values
+        if response.headers["Cache-Control"]
+          expect(response.headers["Cache-Control"]).not_to include("public, no-cache")
+        end
+      end
     end
 
 
@@ -228,6 +255,19 @@
           "tag_list" => article.decorate.cached_tag_list_array,
         )
       end
+
+      it "does not set cache control headers for edge caching" do
+        get stories_feed_path
+
+        # Should not have the specific edge caching headers we set for signed out users
+        expect(response.headers["X-Accel-Expires"]).to be_nil
+        expect(response.headers["Surrogate-Control"]).to be_nil
+
+        # Cache-Control might be set by other parts of the system, so we check it doesn't contain our specific values
+        if response.headers["Cache-Control"]
+          expect(response.headers["Cache-Control"]).not_to include("public, no-cache")
+        end
+      end
     end
 
     context "when there are highly rated comments" do

spec/swagger_helper.rb

@@ -448,8 +448,10 @@
               id: { type: :integer, description: "The ID of the Billboard" },
               name: { type: :string, description: "For internal use, helps distinguish ads from one another" },
               body_markdown: { type: :string, description: "The text (in markdown) of the ad (required)" },
-              approved: { 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" },
+                        approved: { 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" },
+          expires_at: { type: :string, format: :"date-time", nullable: true,
+                        description: "Timestamp when the billboard expires. After this time, the billboard will automatically be marked as not approved." },
               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 },
               placement_area: { type: :string, enum: Billboard::ALLOWED_PLACEMENT_AREAS,

spec/views/admin/billboards/new_spec.rb

@@ -16,5 +16,11 @@
       render
       expect(rendered).to have_css("#placement_area")
     end
+
+    it "includes the expires_at field" do
+      render
+      expect(rendered).to have_css("input[name='expires_at']")
+      expect(rendered).to have_text("Billboard will automatically be marked as not approved after this time")
+    end
   end
 end