diff --git a/app/controllers/display_ad_events_controller.rb b/app/controllers/display_ad_events_controller.rb index ce4799894..b01370297 100644 --- a/app/controllers/display_ad_events_controller.rb +++ b/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 diff --git a/spec/lib/throttled_call_spec.rb b/spec/lib/throttled_call_spec.rb index aa69866c2..3684629c4 100644 --- a/spec/lib/throttled_call_spec.rb +++ b/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 } diff --git a/spec/requests/display_ad_events_spec.rb b/spec/requests/display_ad_events_spec.rb index dfb21c482..f6c51034b 100644 --- a/spec/requests/display_ad_events_spec.rb +++ b/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 diff --git a/spec/support/shared_context.rb b/spec/support/shared_context.rb index 4e9117e6d..59300a98a 100644 --- a/spec/support/shared_context.rb +++ b/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 diff --git a/spec/workers/reactions/update_relevant_scores_worker_spec.rb b/spec/workers/reactions/update_relevant_scores_worker_spec.rb index d5ef70c97..0717c809b 100644 --- a/spec/workers/reactions/update_relevant_scores_worker_spec.rb +++ b/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