Add article id and geolocation info to billboard events (#20329)

* Add article id and geolocation info to billboard events

* Confirm destroyed in rollup

app/assets/javascripts/initializers/initializeBillboardVisibility.js

@@ -68,6 +68,7 @@ function trackAdClick(adBox) {
       billboard_id: adBox.dataset.id,
       context_type: adBox.dataset.contextType,
       category: adBox.dataset.categoryClick,
+      article_id: adBox.dataset.articleId,
     },
   };
   window.fetch('/billboard_events', {

app/controllers/billboard_events_controller.rb

@@ -37,6 +37,18 @@ class BillboardEventsController < ApplicationMetalController
     # keeping while we may receive data in the "old" format from cached js
     billboard_id = event_params.delete(:display_ad_id)
     event_params[:billboard_id] ||= billboard_id
-    event_params.slice(:context_type, :category, :billboard_id)
+    event_params[:article_id] = params[:article_id] if params[:article_id].present?
+    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

app/javascript/packs/billboardAfterRenderActions.js

@@ -88,6 +88,7 @@ function trackAdImpression(adBox) {
       billboard_id: adBox.dataset.id,
       context_type: adBox.dataset.contextType,
       category: adBox.dataset.categoryImpression,
+      article_id: adBox.dataset.articleId,
     },
   };
 

app/models/billboard_event.rb

@@ -4,6 +4,8 @@
 class BillboardEvent < ApplicationRecord
   belongs_to :billboard, class_name: "Billboard", foreign_key: :display_ad_id, inverse_of: :billboard_events
   belongs_to :user, optional: true
+  # We also have an article_id param, but not belongs_to because it is not indexed and not designed to be
+  # consistently referenced within the application.
 
   self.table_name = "display_ad_events"
 

app/services/billboard_event_rollup.rb

@@ -1,6 +1,6 @@
 class BillboardEventRollup
   ATTRIBUTES_PRESERVED = %i[user_id display_ad_id category context_type created_at].freeze
-  ATTRIBUTES_DESTROYED = %i[id counts_for updated_at].freeze
+  ATTRIBUTES_DESTROYED = %i[id counts_for updated_at article_id geolocation].freeze
 
   class EventAggregator
     Compact = Struct.new(:events, :user_id, :billboard_id, :category, :context_type) do

app/views/shared/_billboard.html.erb

@@ -4,6 +4,7 @@
     data-category-click="<%= BillboardEvent::CATEGORY_CLICK %>"
     data-category-impression="<%= BillboardEvent::CATEGORY_IMPRESSION %>"
     data-context-type="<%= data_context_type %>"
+    data-article-id="<%= @article&.id %>"
     data-type-of="<%= billboard.type_of %>">
       <%= billboard.processed_html.html_safe %>
   </div>
@@ -13,6 +14,7 @@
     data-category-click="<%= BillboardEvent::CATEGORY_CLICK %>"
     data-category-impression="<%= BillboardEvent::CATEGORY_IMPRESSION %>"
     data-context-type="<%= data_context_type %>"
+    data-article-id="<%= @article&.id %>"
     data-type-of="<%= billboard.type_of %>">
     <div class="crayons-story__body">
       <div class="crayons-story__top">
@@ -31,6 +33,7 @@
     data-category-click="<%= BillboardEvent::CATEGORY_CLICK %>"
     data-category-impression="<%= BillboardEvent::CATEGORY_IMPRESSION %>"
     data-context-type="<%= data_context_type %>"
+    data-article-id="<%= @article&.id %>"
     data-type-of="<%= billboard.type_of %>">
     <%= render partial: "shared/billboard_header", locals: { billboard: billboard } %>
     <div class="p-1 text-styles text-styles--billboard">
@@ -43,6 +46,7 @@
     data-category-click="<%= BillboardEvent::CATEGORY_CLICK %>"
     data-category-impression="<%= BillboardEvent::CATEGORY_IMPRESSION %>"
     data-context-type="<%= data_context_type %>"
+    data-article-id="<%= @article&.id %>"
     data-type-of="<%= billboard.type_of %>">
     <%= render partial: "shared/billboard_header", locals: { billboard: billboard } %>
     <div class="p-1 pt-3 text-styles text-styles--billboard">

db/migrate/20231108153011_add_new_fields_to_billboard_events.rb

@@ -0,0 +1,6 @@
+class AddNewFieldsToBillboardEvents < ActiveRecord::Migration[7.0]
+  def change
+    add_column :display_ad_events, :article_id, :integer
+    add_column :display_ad_events, :geolocation, :string
+  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: 2023_10_26_152356) do
+ActiveRecord::Schema[7.0].define(version: 2023_11_08_153011) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
   enable_extension "ltree"
@@ -454,11 +454,13 @@ ActiveRecord::Schema[7.0].define(version: 2023_10_26_152356) do
   end
 
   create_table "display_ad_events", force: :cascade do |t|
+    t.integer "article_id"
     t.string "category"
     t.string "context_type"
     t.integer "counts_for", default: 1, null: false
     t.datetime "created_at", precision: nil, null: false
     t.bigint "display_ad_id"
+    t.string "geolocation"
     t.datetime "updated_at", precision: nil, null: false
     t.bigint "user_id"
     t.index ["display_ad_id"], name: "index_display_ad_events_on_display_ad_id"

spec/requests/billboard_events_spec.rb

@@ -68,6 +68,33 @@ RSpec.describe "BillboardEvents" do
         expect(BillboardEvent.last.user_id).to eq(user.id)
       end
 
+      it "assigns event to passed article_id" do
+        article = create(:article)
+        post "/billboard_events", params: {
+          billboard_event: {
+            billboard_id: billboard.id,
+            context_type: BillboardEvent::CONTEXT_TYPE_HOME,
+            category: BillboardEvent::CATEGORY_IMPRESSION,
+            article_id: article.id
+          }
+        }
+        expect(BillboardEvent.last.article_id)
+          .to eq(article.id)
+      end
+
+      it "assigns event to passed current geolocation" do
+        article = create(:article)
+        post "/billboard_events", params: {
+          billboard_event: {
+            billboard_id: billboard.id,
+            context_type: BillboardEvent::CONTEXT_TYPE_HOME,
+            category: BillboardEvent::CATEGORY_IMPRESSION,
+            article_id: article.id
+          }
+        }, headers: { "X-Client-Geo" => "CA-AB", "X-Cacheable-Client-Geo" => "CA" }
+        expect(BillboardEvent.last.geolocation).to eq("CA-AB")
+      end
+
       it "uses a ThrottledCall for data updates" do
         post "/billboard_events", params: {
           billboard_event: {