maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Extracting constants and tidying tests (#15946)

Browse source 
Prior to this commit, we had a couple of references to magic strings
regarding DisplayAdEvent objects.  This commit seeks to consolidate that
behavior.

In a handful of cases we had javascript variables, but as these JS files
are `erb` templates, we can inject the constant as well.  This reduces
repetition of knowledge.
This commit is contained in:
Jeremy Friesen 2022-01-05 09:19:39 -05:00 • committed by GitHub
parent 59eea24244
commit b56ae3acbb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 35 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
app/assets/javascripts/initializers/initializeBaseTracking.js.erb
View file

@ -206,8 +206,8 @@ function trackAdImpression(token, adBox) {
var dataBody = {
display_ad_event: {
display_ad_id: adBox.dataset.id,
context_type: "home",
category: "impression",
context_type: "<%= DisplayAdEvent::CONTEXT_TYPE_HOME %>",
category: "<%= DisplayAdEvent::CATEGORY_IMPRESSION %>",
}
}
window.fetch('/display_ad_events', {
@ -227,8 +227,8 @@ function trackAdClick(token, clickedElement) {
var dataBody = {
display_ad_event: {
display_ad_id: adBox.dataset.id,
context_type: "home",
category: "click",
context_type: "<%= DisplayAdEvent::CONTEXT_TYPE_HOME %>",
category: "<%= DisplayAdEvent::CATEGORY_CLICK %>",
}
}
window.fetch('/display_ad_events', {

4
app/controllers/display_ad_events_controller.rb
View file

@ -18,8 +18,8 @@ class DisplayAdEventsController < ApplicationMetalController
ThrottledCall.perform(:display_ads_data_update, throttle_for: 15.minutes) do
@display_ad = DisplayAd.find(display_ad_event_params[:display_ad_id])
num_impressions = @display_ad.display_ad_events.where(category: "impression").size
num_clicks = @display_ad.display_ad_events.where(category: "click").size
num_impressions = @display_ad.display_ad_events.impressions.size
num_clicks = @display_ad.display_ad_events.clicks.size
rate = num_clicks.to_f / num_impressions
@display_ad.update_columns(

14
app/models/display_ad_event.rb
View file

@ -5,6 +5,16 @@ class DisplayAdEvent < ApplicationRecord
belongs_to :display_ad
belongs_to :user
validates :category, inclusion: { in: %w[impression click] }
validates :context_type, inclusion: { in: %w[home] }
CATEGORY_IMPRESSION = "impression".freeze
CATEGORY_CLICK = "click".freeze
VALID_CATEGORIES = [CATEGORY_CLICK, CATEGORY_IMPRESSION].freeze
CONTEXT_TYPE_HOME = "home".freeze
VALID_CONTEXT_TYPES = [CONTEXT_TYPE_HOME].freeze
validates :category, inclusion: { in: VALID_CATEGORIES }
validates :context_type, inclusion: { in: VALID_CONTEXT_TYPES }
scope :impressions, -> { where(category: CATEGORY_IMPRESSION) }
scope :clicks, -> { where(category: CATEGORY_CLICK) }
end

2
spec/factories/display_ad_events.rb
View file

@ -1,6 +1,6 @@
FactoryBot.define do
factory :display_ad_event do
category { "impression" }
category { DisplayAdEvent::CATEGORY_IMPRESSION }
context_type { "home" }
display_ad
end

21
spec/models/display_ad_event_spec.rb
View file

@ -1,24 +1,7 @@
require "rails_helper"
RSpec.describe DisplayAdEvent, type: :model do
let(:user) { build(:user) }
let(:organization) { build(:organization) }
let(:display_ad) { build(:display_ad, organization: organization) }
it { is_expected.to validate_inclusion_of(:category).in_array(described_class::VALID_CATEGORIES) }
describe "#category" do
it "is valid with a click category" do
event = build(:display_ad_event, category: "click", user: user, display_ad: display_ad)
expect(event).to be_valid
end
it "is valid with an impression category" do
event = build(:display_ad_event, category: "impression", user: user, display_ad: display_ad)
expect(event).to be_valid
end
it "is not valid with an unknown category" do
event = build(:display_ad_event, category: "wazoo", user: user, display_ad: display_ad)
expect(event).not_to be_valid
end
end
it { is_expected.to validate_inclusion_of(:context_type).in_array(described_class::VALID_CONTEXT_TYPES) }
end

25
spec/requests/display_ad_events_spec.rb
View file

@ -15,8 +15,8 @@ RSpec.describe "DisplayAdEvents", type: :request do
post "/display_ad_events", params: {
display_ad_event: {
display_ad_id: display_ad.id,
context_type: "home",
category: "click"
context_type: DisplayAdEvent::CONTEXT_TYPE_HOME,
category: DisplayAdEvent::CATEGORY_CLICK
}
}
expect(display_ad.reload.clicks_count).to eq(1)
@ -26,19 +26,22 @@ RSpec.describe "DisplayAdEvents", type: :request do
post "/display_ad_events", params: {
display_ad_event: {
display_ad_id: display_ad.id,
context_type: "home",
category: "impression"
context_type: DisplayAdEvent::CONTEXT_TYPE_HOME,
category: DisplayAdEvent::CATEGORY_IMPRESSION
}
}
expect(display_ad.reload.impressions_count).to eq(1)
end
it "creates a display ad success rate" do
ad_event_params = { display_ad_id: display_ad.id, context_type: "home" }
impression_params = ad_event_params.merge(category: "impression", user: user)
ad_event_params = { display_ad_id: display_ad.id, context_type: DisplayAdEvent::CONTEXT_TYPE_HOME }
impression_params = ad_event_params.merge(category: DisplayAdEvent::CATEGORY_IMPRESSION, user: user)
create_list(:display_ad_event, 4, impression_params)
post "/display_ad_events", params: { display_ad_event: ad_event_params.merge(category: "click") }
post(
"/display_ad_events",
params: { display_ad_event: ad_event_params.merge(category: DisplayAdEvent::CATEGORY_CLICK) },
)
expect(display_ad.reload.success_rate).to eq(0.25)
end
@ -47,8 +50,8 @@ RSpec.describe "DisplayAdEvents", type: :request do
post "/display_ad_events", params: {
display_ad_event: {
display_ad_id: display_ad.id,
context_type: "home",
category: "impression"
context_type: DisplayAdEvent::CONTEXT_TYPE_HOME,
category: DisplayAdEvent::CATEGORY_IMPRESSION
}
}
expect(DisplayAdEvent.last.user_id).to eq(user.id)
@ -58,8 +61,8 @@ RSpec.describe "DisplayAdEvents", type: :request do
post "/display_ad_events", params: {
display_ad_event: {
display_ad_id: display_ad.id,
context_type: "home",
category: "impression"
context_type: DisplayAdEvent::CONTEXT_TYPE_HOME,
category: DisplayAdEvent::CATEGORY_IMPRESSION
}
}