Update PingAdmins to show triggering event (#3853)

app/controllers/comments_controller.rb

@@ -51,7 +51,7 @@ class CommentsController < ApplicationController
   # POST /comments.json
   def create
     authorize Comment
-    raise if RateLimitChecker.new(current_user).limit_by_situation("comment_creation")
+    raise if RateLimitChecker.new(current_user).limit_by_action("comment_creation")
 
     @comment = Comment.new(permitted_attributes(Comment))
     @comment.user_id = current_user.id

app/controllers/image_uploads_controller.rb

@@ -5,7 +5,7 @@ class ImageUploadsController < ApplicationController
   def create
     authorize :image_upload
     begin
-      raise RateLimitChecker::UploadRateLimitReached if RateLimitChecker.new(current_user).limit_by_situation("image_upload")
+      raise RateLimitChecker::UploadRateLimitReached if RateLimitChecker.new(current_user).limit_by_action("image_upload")
       raise CarrierWave::IntegrityError if params[:image].blank?
 
       uploaders = image_upload(params[:image])

app/jobs/rate_limit_checker_job.rb

@@ -1,8 +1,8 @@
 class RateLimitCheckerJob < ApplicationJob
   queue_as :rate_limit_checker
 
-  def perform(user_id)
+  def perform(user_id, action)
     user = User.find_by(id: user_id)
-    PingAdmins.call(user) if user
+    PingAdmins.call(user, action) if user
   end
 end

app/labor/rate_limit_checker.rb

@@ -1,13 +1,14 @@
 class RateLimitChecker
-  attr_accessor :user
+  attr_reader :user, :action
+
   def initialize(user = nil)
     @user = user
   end
 
   class UploadRateLimitReached < StandardError; end
 
-  def limit_by_situation(situation)
-    result = case situation
+  def limit_by_action(action)
+    result = case action
              when "comment_creation"
                user.comments.where("created_at > ?", 30.seconds.ago).size > 9
              when "published_article_creation"
@@ -17,7 +18,10 @@ class RateLimitChecker
              else
                false
              end
-    ping_admins if result == true
+    if result
+      @action = action
+      ping_admins
+    end
     result
   end
 
@@ -28,16 +32,16 @@ class RateLimitChecker
   end
 
   def limit_by_email_recipient_address(address)
-    # This is related to the recipient, not the "user" initiator, like in situation.
+    # This is related to the recipient, not the "user" initiator, like in action.
     EmailMessage.where(to: address).
       where("sent_at > ?", 2.minutes.ago).size > 5
   end
 
   def ping_admins
-    RateLimitCheckerJob.perform_later(user.id)
+    RateLimitCheckerJob.perform_later(user.id, action)
   end
 
   def ping_admins_without_delay
-    RateLimitCheckerJob.perform_now(user.id)
+    RateLimitCheckerJob.perform_now(user.id, action)
   end
 end

app/services/articles/creator.rb

@@ -10,7 +10,7 @@ module Articles
     end
 
     def call
-      raise if RateLimitChecker.new(user).limit_by_situation("published_article_creation")
+      raise if RateLimitChecker.new(user).limit_by_action("published_article_creation")
 
       tags = article_params[:tags]
       series = article_params[:series]

app/services/ping_admins.rb

@@ -1,6 +1,7 @@
 class PingAdmins
-  def initialize(user)
+  def initialize(user, action = "unknown")
     @user = user
+    @action = action
   end
 
   def self.call(*args)
@@ -11,7 +12,7 @@ class PingAdmins
     return unless user && Rails.env.production?
 
     SlackBot.ping(
-      "Rate limit exceeded. https://dev.to#{user.path}",
+      "Rate limit exceeded (#{action}). https://dev.to#{user.path}",
       channel: "abuse-reports",
       username: "rate_limit",
       icon_emoji: ":hand:",
@@ -20,5 +21,5 @@ class PingAdmins
 
   private
 
-  attr_reader :user
+  attr_reader :user, :action
 end

spec/jobs/rate_limit_checker_job_spec.rb

@@ -17,15 +17,15 @@ RSpec.describe RateLimitCheckerJob, type: :job do
 
     it "calls a service" do
       perform_enqueued_jobs do
-        described_class.perform_now(user.id)
+        described_class.perform_now(user.id, "test")
 
-        expect(service).to have_received(:call).with(user).once
+        expect(service).to have_received(:call).with(user, "test").once
       end
     end
 
     it "does nothing for non-existent user" do
       perform_enqueued_jobs do
-        described_class.perform_now(nil)
+        described_class.perform_now(nil, "test")
 
         expect(service).not_to have_received(:call)
       end

spec/labor/rate_limit_checker_spec.rb

@@ -4,41 +4,48 @@ RSpec.describe RateLimitChecker do
   let(:user) { create(:user) }
   let(:article) { create(:article, user_id: user.id) }
 
-  it ".limit_by_situation returns false for invalid case" do
-    expect(described_class.new(user).limit_by_situation("random-nothing")).to eq(false)
-  end
-
-  it ".limit_by_situation returns true if too many comments at once" do
-    create_list(:comment, 10, user_id: user.id, commentable_id: article.id)
-    expect(described_class.new(user).limit_by_situation("comment_creation")).to eq(true)
-  end
-
-  it ".limit_by_situation returns false if allowed comment" do
-    create_list(:comment, 2, user_id: user.id, commentable_id: article.id)
-    expect(described_class.new(user).limit_by_situation("comment_creation")).to eq(false)
-  end
-
-  it ". limit_by_situation returns true if too many published articles at once" do
-    create_list(:article, 10, user_id: user.id, published: true)
-    expect(described_class.new(user).limit_by_situation("published_article_creation")).to eq(true)
-  end
-
-  it ".limit_by_situation returns false if published articles comment" do
-    create_list(:article, 2, user_id: user.id, published: true)
-    expect(described_class.new(user).limit_by_situation("published_article_creation")).to eq(false)
-  end
-
-  it ".limit_by_email_recipient_address returns true if too many published articles at once" do
-    10.times do
-      EmailMessage.create(to: user.email, sent_at: Time.current)
+  describe "#limit_by_action" do
+    it "returns false for invalid action" do
+      expect(described_class.new(user).limit_by_action("random-nothing")).to eq(false)
+    end
+
+    it "returns true if too many comments at once" do
+      create_list(:comment, 10, user_id: user.id, commentable_id: article.id)
+      expect(described_class.new(user).limit_by_action("comment_creation")).to eq(true)
+    end
+
+    it "triggers ping admin when too many comments" do
+      allow(RateLimitCheckerJob).to receive(:perform_later)
+      create_list(:comment, 10, user_id: user.id, commentable_id: article.id)
+      described_class.new(user).limit_by_action("comment_creation")
+      expect(RateLimitCheckerJob).to have_received(:perform_later).with(user.id, "comment_creation")
+    end
+
+    it "returns false if allowed comment" do
+      create_list(:comment, 2, user_id: user.id, commentable_id: article.id)
+      expect(described_class.new(user).limit_by_action("comment_creation")).to eq(false)
+    end
+
+    it "returns true if too many published articles at once" do
+      create_list(:article, 10, user_id: user.id, published: true)
+      expect(described_class.new(user).limit_by_action("published_article_creation")).to eq(true)
+    end
+
+    it "returns false if published articles comment" do
+      create_list(:article, 2, user_id: user.id, published: true)
+      expect(described_class.new(user).limit_by_action("published_article_creation")).to eq(false)
     end
-    expect(described_class.new.limit_by_email_recipient_address(user.email)).to eq(true)
   end
 
-  it ".limit_by_email_recipient_address returns false if published articles comment" do
-    2.times do
-      EmailMessage.create(to: user.email, sent_at: Time.current)
+  describe "#limit_by_email_recipient_address" do
+    it "returns true if too many published articles at once" do
+      10.times { EmailMessage.create(to: user.email, sent_at: Time.current) }
+      expect(described_class.new.limit_by_email_recipient_address(user.email)).to eq(true)
+    end
+
+    it "returns false if published articles comment" do
+      2.times { EmailMessage.create(to: user.email, sent_at: Time.current) }
+      expect(described_class.new.limit_by_email_recipient_address(user.email)).to eq(false)
     end
-    expect(described_class.new.limit_by_email_recipient_address(user.email)).to eq(false)
   end
 end