Added specs for webhooks

app/controllers/users/settings/webhooks_controller.rb

@@ -72,7 +72,7 @@ module Users
       end
 
       def webhook_params
-        params.require(:webhook).permit(:method, :url, :active)
+        params.require(:webhook).permit(:http_method, :url, :active)
       end
 
       def load_filter_elements(filter)

app/jobs/activities/dispatch_webhooks_job.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module Activities
-  class DispatchWebhooksJobs < ApplicationJob
+  class DispatchWebhooksJob < ApplicationJob
     queue_as :high_priority
 
     def perform(activity)

app/jobs/activities/send_webhook_job.rb

@@ -5,7 +5,7 @@ module Activities
     queue_as :high_priority
 
     def perform(webhook, activity)
-      Activities::WebhookService.new(webhook, activity).send_webhook
+      Activities::ActivityWebhookService.new(webhook, activity).send_webhook
     end
   end
 end

app/models/activity.rb

@@ -155,6 +155,6 @@ class Activity < ApplicationRecord
   end
 
   def dispatch_webhooks
-    Activities::DispatchWebhooksJobs.perform_later(self)
+    Activities::DispatchWebhooksJob.perform_later(self)
   end
 end

app/models/webhook.rb

@@ -1,10 +1,10 @@
 # frozen_string_literal: true
 
 class Webhook < ApplicationRecord
-  enum method: { get: 0, post: 1, patch: 2 }
+  enum http_method: { get: 0, post: 1, patch: 2 }
 
   belongs_to :activity_filter
-  validates :method, presence: true
+  validates :http_method, presence: true
   validates :url, presence: true
   validate :valid_url
 

app/services/activities/activity_webhook_service.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module Activities
+  class ActivityWebhookService
+    def initialize(webhook, activity)
+      @webhook = webhook
+      @activity = activity
+    end
+
+    def send_webhook
+      WebhookService.new(@webhook, activity_payload).send_webhook
+    end
+
+    def activity_payload
+      @activity.values.merge(
+        type: @activity.type_of,
+        created_at: @activity.created_at
+      )
+    end
+  end
+end

app/services/activities/webhook_service.rb

@@ -1,56 +0,0 @@
-# frozen_string_literal: true
-
-module Activities
-  class WebhookService
-    DISABLE_WEBHOOK_ERROR_THRESHOLD = 10
-
-    def initialize(webhook, activity)
-      @webhook = webhook
-      @activity = activity
-    end
-
-    def send_webhook
-      raise "Cannot send inactive webhook." unless @webhook.active?
-
-      response = HTTParty.send(
-        @webhook.method,
-        @webhook.url,
-        {
-          headers: { 'Content-Type' => 'application/json' },
-          body: activity_payload
-        }
-      )
-
-      unless response.success?
-        log_error!("#{response.status}: #{response.message}")
-      end
-    rescue Net::ReadTimeout, SocketError => error
-      log_error!(error)
-    ensure
-      disable_webhook_if_broken!
-    end
-
-    private
-
-    def activity_payload
-      @activity.values.merge(
-        type: @activity.type_of,
-        created_at: @activity.created_at
-      )
-    end
-
-    def log_error!(message)
-      error_count = @webhook.error_count + 1
-
-      @webhook.update(
-        error_count: error_count,
-        last_error: message
-      )
-    end
-
-    def disable_webhook_if_broken!
-      return if @webhook.error_count < DISABLE_WEBHOOK_ERROR_THRESHOLD
-      @webhook.update(active: false)
-    end
-  end
-end

app/services/webhook_service.rb

@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+class WebhookService
+  class InactiveWebhookSendException < StandardError; end
+
+  DISABLE_WEBHOOK_ERROR_THRESHOLD = 10
+
+  def initialize(webhook, payload)
+    @webhook = webhook
+    @payload = payload
+  end
+
+  def send_webhook
+    unless @webhook.active?
+      raise(
+        Activities::WebhooksService::InactiveWebhookSendException.new(
+          "Refused to send inactive webhook."
+        )
+      )
+    end
+
+    response = HTTParty.send(
+      @webhook.http_method,
+      @webhook.url,
+      {
+        headers: { 'Content-Type' => 'application/json' },
+        body: @payload
+      }
+    )
+
+    unless response.success?
+      log_error!("#{response.code}: #{response.message}")
+    end
+
+    response
+  rescue Net::ReadTimeout, Net::OpenTimeout, SocketError => error
+    log_error!(error)
+    raise error
+  ensure
+    disable_webhook_if_broken!
+  end
+
+  private
+
+
+  def log_error!(message)
+    error_count = @webhook.error_count + 1
+
+    @webhook.update(
+      error_count: error_count,
+      last_error: message
+    )
+  end
+
+  def disable_webhook_if_broken!
+    return if @webhook.error_count < DISABLE_WEBHOOK_ERROR_THRESHOLD
+    @webhook.update(active: false)
+  end
+end

app/views/users/settings/webhooks/_webhook_form.html.erb

@@ -1,6 +1,6 @@
 <span class="form-text"><%= t("webhooks.index.webhook_trigger") %></span>
 <div class="webhook-method-container">
-  <%= f.select :method, options_for_select(Webhook.methods.map{ |k,_v| [k.upcase, k] }, f.object.method) %>
+  <%= f.select :http_method, options_for_select(Webhook.http_methods.map{ |k,_v| [k.upcase, k] }, f.object.http_method) %>
 </div>
 <span class="form-text"><%= t("webhooks.index.target") %></span>
 <div class="sci-input-container url-input-container form-group">

app/views/users/settings/webhooks/index.html.erb

@@ -63,7 +63,7 @@
             <li class="webhook <%= 'active' if webhook.active? %>">
               <div class="view-mode">
                 <span class="webhook-text">
-                  <%= t('webhooks.index.webhook_text_html', method: webhook.method.upcase, url: webhook.url) %>
+                  <%= t('webhooks.index.webhook_text_html', method: webhook.http_method.upcase, url: webhook.url) %>
                 </span>
                 <% if webhook.active? %>
                   <span class="active-webhook">

db/migrate/20210617111749_rename_webhook_method_column.rb

@@ -0,0 +1,5 @@
+class RenameWebhookMethodColumn < ActiveRecord::Migration[6.1]
+  def change
+    rename_column :webhooks, :method, :http_method
+  end
+end

db/structure.sql

@@ -2799,7 +2799,7 @@ CREATE TABLE public.webhooks (
     activity_filter_id bigint NOT NULL,
     active boolean DEFAULT true NOT NULL,
     url character varying NOT NULL,
-    method integer NOT NULL,
+    http_method integer NOT NULL,
     created_at timestamp(6) without time zone NOT NULL,
     updated_at timestamp(6) without time zone NOT NULL,
     error_count integer DEFAULT 0 NOT NULL,
@@ -7351,6 +7351,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20210506125657'),
 ('20210531114633'),
 ('20210603152345'),
-('20210616071836');
+('20210616071836'),
+('20210617111749');
 
 

spec/factories/activity_filters.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :activity_filter do
+    name { "type filter 1" }
+    filter { {"types" => ["0"], "from_date" => "", "to_date" => ""} }
+  end
+end

spec/factories/webhooks.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :webhook do
+    activity_filter
+    http_method { "post" }
+    url { "https://www.example.com" }
+  end
+end

spec/jobs/activities/dispatch_webhooks_job_spec.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Activities::DispatchWebhooksJob do
+  let!(:activity_filter_1) { create :activity_filter }
+  let!(:activity_filter_2) { create :activity_filter }
+  let!(:non_matching_activity_filter) do
+    create(:activity_filter,
+      filter: {"types" => ["163"], "from_date" => "", "to_date" => ""}
+    )
+  end
+  let!(:webhook_1) { create :webhook, activity_filter: activity_filter_1 }
+  let!(:webhook_2) { create :webhook, activity_filter: activity_filter_2 }
+  let!(:webhook_3) { create :webhook, activity_filter: non_matching_activity_filter }
+  let!(:activity) { create :activity }
+
+  it 'enqueues webhook jobs' do
+    ActiveJob::Base.queue_adapter = :test
+
+    expect {
+      Activities::DispatchWebhooksJob.new(activity).perform_now
+    }.to have_enqueued_job(Activities::SendWebhookJob).exactly(2).times
+  end
+end

spec/jobs/activities/send_webhook_job_spec.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Activities::SendWebhookJob do
+  let!(:webhook) { create :webhook }
+  let!(:activity) { create :activity }
+
+  it 'sends the webhook' do
+    stub_request(:post, webhook.url).to_return(status: 200, body: "", headers: {})
+
+    expect(Activities::SendWebhookJob.new(webhook, activity).perform_now.response.code).to eq("200")
+  end
+end

spec/models/activity_spec.rb

@@ -4,6 +4,9 @@ require 'rails_helper'
 
 describe Activity, type: :model do
   let(:activity) { build :activity }
+  let(:user) { create :user }
+  let(:team) { create :team }
+
   let(:old_activity) { build :activity, :old }
 
   it 'should be of class Activity' do
@@ -61,6 +64,15 @@ describe Activity, type: :model do
     end
   end
 
+  describe '.create' do
+    it 'enqueues webhook dispatch job' do
+      ActiveJob::Base.queue_adapter = :test
+      expect {
+        Activity.create(owner: user, team: team, type_of: "generate_pdf_report")
+      }.to have_enqueued_job(Activities::DispatchWebhooksJob)
+    end
+  end
+
   describe '.save' do
     it 'adds user to message items' do
       activity.save

spec/services/webhook_service_spec.rb

@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Activities::CreateActivityService do
+  let(:webhook) { create :webhook }
+
+  context 'when webhook is valid' do
+    it 'executes webhook' do
+      stub_request(:post, webhook.url).to_return(status: 200, body: "", headers: {})
+
+      expect(WebhookService.new(webhook, { payload: "payload" }).send_webhook.response.code).to eq("200")
+    end
+  end
+
+  context 'when webhook gets non-success HTTP response' do
+    it 'logs error' do
+      stub_request(:post, webhook.url).to_return(status: 500, body: "", headers: {})
+
+      expect(WebhookService.new(webhook, { payload: "payload" }).send_webhook.response.code).to eq("500")
+      expect(webhook.error_count).to eq(1)
+      expect(webhook.last_error).to eq("500: ")
+    end
+  end
+
+  context 'when webhook times out' do
+    it 'logs error' do
+      stub_request(:post, webhook.url).to_timeout
+
+      expect { WebhookService.new(webhook, { payload: "payload" }).send_webhook }.to raise_error(Net::OpenTimeout)
+      expect(webhook.error_count).to eq(1)
+      expect(webhook.last_error).to eq("execution expired")
+    end
+  end
+
+  context 'when webhook url cannot be resolved' do
+    it 'logs error' do
+      stub_request(:post, webhook.url).to_raise(SocketError)
+
+      expect { WebhookService.new(webhook, { payload: "payload" }).send_webhook }.to raise_error(SocketError)
+      expect(webhook.error_count).to eq(1)
+      expect(webhook.last_error).to eq("Exception from WebMock")
+    end
+  end
+
+  context 'when webhook failed too many times' do
+    it 'disables webhook' do
+      stub_request(:post, webhook.url).to_raise(SocketError)
+
+      webhook.update_columns(error_count: WebhookService::DISABLE_WEBHOOK_ERROR_THRESHOLD - 1, active: true)
+
+      expect { WebhookService.new(webhook, { payload: "payload" }).send_webhook }.to raise_error(SocketError)
+      expect(webhook.error_count).to eq(WebhookService::DISABLE_WEBHOOK_ERROR_THRESHOLD)
+      expect(webhook.active).to be false
+    end
+  end
+end