Skip to content

Commit

Permalink
feat(webhooks): only trigger enabled webhooks
Browse files Browse the repository at this point in the history
  • Loading branch information
bethesque committed May 23, 2019
1 parent ce452ec commit cb2b032
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 30 deletions.
10 changes: 10 additions & 0 deletions db/migrations/20190524_set_webhooks_enabled.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
require 'pact_broker/db/data_migrations/set_webhooks_enabled'

Sequel.migration do
up do
PactBroker::DB::DataMigrations::SetWebhooksEnabled.call(self)
end

down do
end
end
11 changes: 11 additions & 0 deletions lib/pact_broker/db/data_migrations/helpers.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module PactBroker
module DB
module DataMigrations
module Helpers
def column_exists?(connection, table, column)
connection.table_exists?(table) && connection.schema(table).find{|col| col.first == column }
end
end
end
end
end
17 changes: 17 additions & 0 deletions lib/pact_broker/db/data_migrations/set_webhooks_enabled.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
require 'pact_broker/db/data_migrations/helpers'

module PactBroker
module DB
module DataMigrations
class SetWebhooksEnabled
extend Helpers

def self.call(connection)
if column_exists?(connection, :webhooks, :enabled)
connection[:webhooks].where(enabled: nil).update(enabled: true)
end
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/pact_broker/db/migrate_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def self.call database_connection, options = {}
DataMigrations::SetPacticipantIdsForVerifications.call(database_connection)
DataMigrations::SetConsumerIdsForPactPublications.call(database_connection)
DataMigrations::SetLatestVersionSequenceValue.call(database_connection)
DataMigrations::SetWebhooksEnabled.call(database_connection)
end
end
end
Expand Down
5 changes: 1 addition & 4 deletions lib/pact_broker/webhooks/repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,6 @@ def delete_by_consumer_and_provider consumer, provider
Webhook.where(consumer: consumer, provider: provider).destroy
end

def find_for_pact_and_event_name pact, event_name
find_by_consumer_and_or_provider_and_event_name(pact.consumer, pact.provider, event_name)
end

def find_by_consumer_and_or_provider_and_event_name consumer, provider, event_name
find_by_consumer_and_provider_and_event_name(consumer, provider, event_name) +
find_by_consumer_and_provider_and_event_name(nil, provider, event_name) +
Expand All @@ -100,6 +96,7 @@ def find_by_consumer_and_provider_and_event_name consumer, provider, event_name
}
Webhook
.select_all_qualified
.enabled
.where(criteria)
.join(:webhook_events, { webhook_id: :id })
.where(Sequel[:webhook_events][:name] => event_name)
Expand Down
6 changes: 5 additions & 1 deletion lib/pact_broker/webhooks/webhook.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ class Webhook < Sequel::Model

dataset_module do
include PactBroker::Repositories::Helpers

def enabled
where(enabled: true)
end
end

def before_destroy
Expand Down Expand Up @@ -86,7 +90,7 @@ def self.properties_hash_from_domain webhook
url: webhook.request.url,
username: webhook.request.username,
password: not_plain_text_password(webhook.request.password),
enabled: webhook.enabled,
enabled: webhook.enabled.nil? ? true : webhook.enabled,
body: (is_json_request_body ? webhook.request.body.to_json : webhook.request.body),
is_json_request_body: is_json_request_body
}
Expand Down
32 changes: 7 additions & 25 deletions spec/lib/pact_broker/webhooks/repository_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,6 @@ module Webhooks
subject { Repository.new.find_by_consumer_and_provider_and_event_name td.consumer, td.provider, 'something_happened' }

context "when a webhook exists with a matching consumer and provider and event name" do

before do
td
.create_consumer("Consumer")
Expand All @@ -347,32 +346,15 @@ module Webhooks
it "returns an array of webhooks" do
expect(subject.collect(&:uuid).sort).to eq ['1', '2']
end
end
end

describe "find_for_pact_and_event_name" do
context "when a webhook exists with a matching consumer and provider and event name" do
before do
td
.create_consumer("Consumer")
.create_consumer_version("1")
.create_provider("Another Provider")
.create_webhook
.create_provider("Provider")
.create_pact
.create_webhook(uuid: '1', events: [{ name: 'something_happened' }])
.create_webhook(uuid: '2', events: [{ name: 'something_happened' }])
.create_webhook(uuid: '3', events: [{ name: 'something_else_happened' }])
.create_consumer_webhook(uuid: '4', events: [{ name: 'something_happened' }])
.create_provider_webhook(uuid: '5', events: [{ name: 'something_happened' }])
.create_global_webhook(uuid: '6', events: [{ name: 'something_happened' }])
.create_global_webhook(uuid: '7', events: [{ name: 'something_else_happened' }])
end

subject { Repository.new.find_for_pact_and_event_name(td.pact, 'something_happened') }
context "when the webhook is not enabled" do
before do
Webhook.where(uuid: '2').update(enabled: false)
end

it "returns an array of webhooks" do
expect(subject.collect(&:uuid).sort).to eq ['1', '2', '4', '5', '6']
it "is not returned" do
expect(subject.collect(&:uuid).sort).to_not include('2 ')
end
end
end
end
Expand Down

0 comments on commit cb2b032

Please sign in to comment.