diff --git a/app/assets/javascripts/initializers/initializeBillboardVisibility.js b/app/assets/javascripts/initializers/initializeBillboardVisibility.js index 52ad315b6..627c9f96d 100644 --- a/app/assets/javascripts/initializers/initializeBillboardVisibility.js +++ b/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', { diff --git a/app/controllers/billboard_events_controller.rb b/app/controllers/billboard_events_controller.rb index 7405e5ca6..1d4b90878 100644 --- a/app/controllers/billboard_events_controller.rb +++ b/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 diff --git a/app/javascript/packs/billboardAfterRenderActions.js b/app/javascript/packs/billboardAfterRenderActions.js index 63b3cddb2..2374d0264 100644 --- a/app/javascript/packs/billboardAfterRenderActions.js +++ b/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, }, }; diff --git a/app/models/billboard_event.rb b/app/models/billboard_event.rb index 445ed2859..234713ce6 100644 --- a/app/models/billboard_event.rb +++ b/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" diff --git a/app/services/billboard_event_rollup.rb b/app/services/billboard_event_rollup.rb index 35f6d1d16..06234d058 100644 --- a/app/services/billboard_event_rollup.rb +++ b/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 diff --git a/app/views/shared/_billboard.html.erb b/app/views/shared/_billboard.html.erb index 854fe4521..785e5de41 100644 --- a/app/views/shared/_billboard.html.erb +++ b/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 %> @@ -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 %>">
@@ -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 } %>
@@ -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 } %>
diff --git a/db/migrate/20231108153011_add_new_fields_to_billboard_events.rb b/db/migrate/20231108153011_add_new_fields_to_billboard_events.rb new file mode 100644 index 000000000..d4073fceb --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 7d3e4f87f..85d0a2cee 100644 --- a/db/schema.rb +++ b/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" diff --git a/spec/requests/billboard_events_spec.rb b/spec/requests/billboard_events_spec.rb index 25b4fd1ee..3affbc0c4 100644 --- a/spec/requests/billboard_events_spec.rb +++ b/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: {