[15-minute fix] Add ThrottledCall to DisplayAdEventsController (#12924)

* Add ThrottledCall to DisplayAdEventsController

* Update old spec

app/controllers/display_ad_events_controller.rb

@@ -15,27 +15,22 @@ class DisplayAdEventsController < ApplicationMetalController
   private
 
   def update_display_ads_data
-    return if skip_ad_update?
+    ThrottledCall.perform(:display_ads_data_update, throttle_for: 15.minutes) do
+      @display_ad = DisplayAd.find(display_ad_event_params[:display_ad_id])
 
-    @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
+      rate = num_clicks.to_f / num_impressions
 
-    num_impressions = @display_ad.display_ad_events.where(category: "impression").size
-    num_clicks = @display_ad.display_ad_events.where(category: "click").size
-    rate = num_clicks.to_f / num_impressions
-
-    @display_ad.update_columns(
-      success_rate: rate,
-      clicks_count: num_clicks,
-      impressions_count: num_impressions,
-    )
+      @display_ad.update_columns(
+        success_rate: rate,
+        clicks_count: num_clicks,
+        impressions_count: num_impressions,
+      )
+    end
   end
 
   def display_ad_event_params
     params[:display_ad_event].slice(:context_type, :category, :display_ad_id)
   end
-
-  def skip_ad_update?
-    # We need to do this operation only once in a while.
-    rand(20) != 1
-  end
 end

spec/lib/throttled_call_spec.rb

@@ -4,17 +4,13 @@ class TestService
   def call(*); end
 end
 
-RSpec.describe ThrottledCall do
-  let(:redis_cache) { ActiveSupport::Cache.lookup_store(:redis_cache_store) }
+RSpec.describe ThrottledCall, throttled_call: true do
   let(:service) { TestService.new }
 
   before do
-    allow(Rails).to receive(:cache).and_return(redis_cache)
     allow(service).to receive(:call)
   end
 
-  after { Rails.cache.clear }
-
   it "calls the block if the call is not currently throttled" do
     described_class.perform(:test, throttle_for: 1.minute) { service.call }
 

spec/requests/display_ad_events_spec.rb

@@ -5,13 +5,10 @@ RSpec.describe "DisplayAdEvents", type: :request do
   let(:organization) { create(:organization) }
   let(:display_ad) { create(:display_ad, organization_id: organization.id) }
 
-  describe "POST /display_ad_events" do
+  describe "POST /display_ad_events", throttled_call: true do
     context "when user signed in" do
       before do
         sign_in user
-        # rubocop:disable RSpec/AnyInstance
-        allow_any_instance_of(DisplayAdEventsController).to receive(:skip_ad_update?).and_return(false)
-        # rubocop:enable RSpec/AnyInstance
       end
 
       it "creates a display ad click event" do
@@ -38,10 +35,8 @@ RSpec.describe "DisplayAdEvents", type: :request do
 
       it "creates a display ad success rate" do
         ad_event_params = { display_ad_id: display_ad.id, context_type: "home" }
-
-        4.times do
-          post "/display_ad_events", params: { display_ad_event: ad_event_params.merge(category: "impression") }
-        end
+        impression_params = ad_event_params.merge(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") }
 
@@ -58,6 +53,19 @@ RSpec.describe "DisplayAdEvents", type: :request do
         }
         expect(DisplayAdEvent.last.user_id).to eq(user.id)
       end
+
+      it "uses a ThrottledCall for data updates" do
+        post "/display_ad_events", params: {
+          display_ad_event: {
+            display_ad_id: display_ad.id,
+            context_type: "home",
+            category: "impression"
+          }
+        }
+
+        expect(ThrottledCall).to have_received(:perform)
+          .with(:display_ads_data_update, throttle_for: instance_of(ActiveSupport::Duration))
+      end
     end
   end
 end

spec/support/shared_context.rb

@@ -13,3 +13,18 @@ end
 RSpec.configure do |rspec|
   rspec.include_context "when proper status", proper_status: true
 end
+
+RSpec.shared_context "with ThrottledCall" do
+  let(:redis_cache) { ActiveSupport::Cache.lookup_store(:redis_cache_store) }
+
+  before do
+    allow(Rails).to receive(:cache).and_return(redis_cache)
+    allow(ThrottledCall).to receive(:perform).and_call_original
+  end
+
+  after { Rails.cache.clear }
+end
+
+RSpec.configure do |rspec|
+  rspec.include_context "with ThrottledCall", throttled_call: true
+end

spec/workers/reactions/update_relevant_scores_worker_spec.rb

@@ -1,6 +1,6 @@
 require "rails_helper"
 
-RSpec.describe Reactions::UpdateRelevantScoresWorker, type: :worker do
+RSpec.describe Reactions::UpdateRelevantScoresWorker, type: :worker, throttled_call: true do
   describe "#perform" do
     let(:article) { create(:article) }
     let(:reaction) { create(:reaction, reactable: article) }
@@ -47,8 +47,10 @@ RSpec.describe Reactions::UpdateRelevantScoresWorker, type: :worker do
     end
 
     it "uses a throttled call for syncing the reactions count" do
-      allow(ThrottledCall).to receive(:perform)
-        .with(:sync_reactions_count, instance_of(ActiveSupport::Duration))
+      worker.perform(reaction.id)
+
+      expect(ThrottledCall).to have_received(:perform)
+        .with(:sync_reactions_count, throttle_for: instance_of(ActiveSupport::Duration))
     end
   end
 end