Add ability to mark billboard (display_ad) as "priority" (boolean) (#19599)

* Add ability to mark billboard (display_ad) as "priority" (boolean)

* Fix invocation

* Update spec/models/display_ad_spec.rb

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update spec/models/display_ad_spec.rb

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update spec/models/display_ad_spec.rb

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update spec/models/display_ad_spec.rb

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

app/controllers/admin/display_ads_controller.rb

@@ -60,7 +60,7 @@ module Admin
 
     def display_ad_params
       params.permit(:organization_id, :body_markdown, :placement_area, :published, :approved, :name, :display_to,
-                    :tag_list, :type_of, :exclude_article_ids, :audience_segment_id)
+                    :tag_list, :type_of, :exclude_article_ids, :audience_segment_id, :priority)
     end
 
     def authorize_admin

app/controllers/api/v1/display_ads_controller.rb

@@ -53,7 +53,8 @@ module Api
         params.permit :approved, :body_markdown, :creator_id, :display_to,
                       :name, :organization_id, :placement_area, :published,
                       :tag_list, :type_of, :exclude_article_ids,
-                      :audience_segment_type, :audience_segment_id
+                      :audience_segment_type, :audience_segment_id,
+                      :priority
       end
     end
   end

app/models/display_ad.rb

@@ -54,7 +54,7 @@ class DisplayAd < ApplicationRecord
                              search: "%#{term}%"
                      }
 
-  scope :seldom_seen, -> { where("impressions_count < ?", LOW_IMPRESSION_COUNT) }
+  scope :seldom_seen, -> { where("impressions_count < ?", LOW_IMPRESSION_COUNT).or(where(priority: true)) }
 
   def self.for_display(area:, user_signed_in:, user_id: nil, article: nil)
     permit_adjacent = article ? article.permit_adjacent_sponsors? : true

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

@@ -101,6 +101,11 @@
       <%= label_tag :approved, "Approved:", class: "crayons-field__label" %>
       <%= select_tag :approved, options_for_select([false, true], selected: @display_ad.approved), class: "crayons-select" %>
     </div>
+
+    <div class="crayons-field">
+      <%= label_tag :priority, "Prioritized:", class: "crayons-field__label" %>
+      <%= select_tag :priority, options_for_select([false, true], selected: @display_ad.priority), class: "crayons-select" %>
+    </div>
   </div>
   <div>
     <div class="crayons-card crayons-card--secondary crayons-sponsorship text-styles text-styles--display-ad">

db/migrate/20230608154310_add_priority_to_display_ads.rb

@@ -0,0 +1,5 @@
+class AddPriorityToDisplayAds < ActiveRecord::Migration[7.0]
+  def change
+    add_column :display_ads, :priority, :boolean, default: false
+  end
+end

db/schema.rb

@@ -10,10 +10,9 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2023_05_17_132219) do
+ActiveRecord::Schema[7.0].define(version: 2023_06_08_154310) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
-  enable_extension "pg_stat_statements"
   enable_extension "pg_trgm"
   enable_extension "pgcrypto"
   enable_extension "plpgsql"
@@ -473,6 +472,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_05_17_132219) do
     t.string "name"
     t.bigint "organization_id"
     t.string "placement_area"
+    t.boolean "priority", default: false
     t.text "processed_html"
     t.boolean "published", default: false
     t.float "success_rate", default: 0.0

spec/factories/display_ads.rb

@@ -3,5 +3,6 @@ FactoryBot.define do
     placement_area { "sidebar_left" }
     sequence(:body_markdown) { |n| "Hello _hey_ Hey hey #{n}" }
     organization
+    priority { false }
   end
 end

spec/models/display_ad_spec.rb

@@ -332,4 +332,27 @@ RSpec.describe DisplayAd do
       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!(:priority_ad) { create(:display_ad, 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).to include(low_impression_ad)
+    end
+
+    it "excludes ads with impressions count greater than or equal to LOW_IMPRESSION_COUNT" do
+      expect(described_class.seldom_seen).not_to include(high_impression_ad)
+    end
+
+    it "includes ads with priority set to true" do
+      expect(described_class.seldom_seen).to include(priority_ad)
+    end
+
+    it "includes both priority to be proper size when two qualifying ads exist" do
+      expect(described_class.seldom_seen.size).to be 2
+    end
+  end
 end

spec/requests/admin/display_ads_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe "/admin/customization/display_ads" do
   let(:org) { create(:organization) }
   let(:params) do
     { organization_id: org.id, body_markdown: "[Click here!](https://example.com)", placement_area: "sidebar_left",
-      approved: true, published: true }
+      approved: true, published: true, priority: true }
   end
   let(:post_resource) { post admin_display_ads_path, params: params }
 
@@ -74,6 +74,14 @@ RSpec.describe "/admin/customization/display_ads" do
         end
       end
 
+      it "updates DisplayAd's priority value" do
+        Timecop.freeze(Time.current) do
+          expect do
+            put admin_display_ad_path(display_ad.id), params: params
+          end.to change { display_ad.reload.priority }.from(false).to(true)
+        end
+      end
+
       it "redirects back to edit path" do
         put admin_display_ad_path(display_ad.id), params: params
         expect(response.body).to redirect_to edit_admin_display_ad_path(display_ad.id)

spec/requests/api/v1/display_ads_spec.rb

@@ -50,7 +50,8 @@ RSpec.describe "Api::V1::DisplayAds" do
                           "placement_area", "processed_html", "published",
                           "success_rate", "tag_list", "type_of", "updated_at",
                           "creator_id", "exclude_article_ids",
-                          "audience_segment_type", "audience_segment_id")
+                          "audience_segment_type", "audience_segment_id",
+                          "priority")
       end
 
       it "returns a malformed response" do
@@ -72,6 +73,7 @@ RSpec.describe "Api::V1::DisplayAds" do
           "id" => ad1.id,
           "published" => true,
           "approved" => true,
+          "priority" => false,
           "type_of" => "in_house",
           "cached_tag_list" => "",
           "clicks_count" => 0,
@@ -97,7 +99,8 @@ RSpec.describe "Api::V1::DisplayAds" do
                           "placement_area", "processed_html", "published",
                           "success_rate", "tag_list", "type_of", "updated_at",
                           "creator_id", "exclude_article_ids",
-                          "audience_segment_type", "audience_segment_id")
+                          "audience_segment_type", "audience_segment_id",
+                          "priority")
       end
     end