Follow up 20d3c5027f
- Fixes #342 - prevent deletion of Webhooks that are used by Job/Macro/Trigger
This commit is contained in:
parent
78da2d5062
commit
9a9c2ed719
4 changed files with 91 additions and 3 deletions
|
@ -7,6 +7,7 @@ class Webhook < ApplicationModel
|
||||||
|
|
||||||
before_create :validate_endpoint
|
before_create :validate_endpoint
|
||||||
before_update :validate_endpoint
|
before_update :validate_endpoint
|
||||||
|
before_destroy Webhook::EnsureNoRelatedObjects
|
||||||
|
|
||||||
validates :name, presence: true
|
validates :name, presence: true
|
||||||
|
|
||||||
|
@ -19,5 +20,4 @@ class Webhook < ApplicationModel
|
||||||
rescue URI::InvalidURIError
|
rescue URI::InvalidURIError
|
||||||
raise Exceptions::UnprocessableEntity, 'Invalid endpoint!'
|
raise Exceptions::UnprocessableEntity, 'Invalid endpoint!'
|
||||||
end
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
|
|
69
app/models/webhook/ensure_no_related_objects.rb
Normal file
69
app/models/webhook/ensure_no_related_objects.rb
Normal file
|
@ -0,0 +1,69 @@
|
||||||
|
# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
|
||||||
|
|
||||||
|
class Webhook::EnsureNoRelatedObjects
|
||||||
|
|
||||||
|
attr_reader :record
|
||||||
|
|
||||||
|
def self.before_destroy(record)
|
||||||
|
new(record).before_destroy
|
||||||
|
end
|
||||||
|
|
||||||
|
def self.referencing_models
|
||||||
|
# this cache doesn't need to be cleared as the result won't change
|
||||||
|
@referencing_models ||= Models
|
||||||
|
.all
|
||||||
|
.keys
|
||||||
|
.select { |klass| klass.column_names.include? 'perform' }
|
||||||
|
end
|
||||||
|
|
||||||
|
def initialize(record)
|
||||||
|
@record = record
|
||||||
|
end
|
||||||
|
|
||||||
|
def before_destroy
|
||||||
|
return if record.new_record?
|
||||||
|
|
||||||
|
ensure_no_related_objects!
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def ensure_no_related_objects!
|
||||||
|
return if related_objects.blank?
|
||||||
|
|
||||||
|
raise Exceptions::UnprocessableEntity, "Cannot delete! This webhook is referenced by #{references_text}"
|
||||||
|
end
|
||||||
|
|
||||||
|
def related_objects
|
||||||
|
@related_objects ||= self.class.referencing_models.each_with_object({}) do |model, result|
|
||||||
|
performables = referencing_performables(model)
|
||||||
|
next if performables.blank?
|
||||||
|
|
||||||
|
result[model.name] = performables
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def referencing_performables(model)
|
||||||
|
model.find_each.with_object([]) do |performable, result|
|
||||||
|
next if !webhook_referenced?(performable)
|
||||||
|
|
||||||
|
result.push({
|
||||||
|
id: performable.id,
|
||||||
|
name: performable.name
|
||||||
|
})
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def webhook_referenced?(performable)
|
||||||
|
record.id == performable.perform
|
||||||
|
&.dig('notification.webhook', 'webhook_id')
|
||||||
|
&.to_i
|
||||||
|
end
|
||||||
|
|
||||||
|
def references_text
|
||||||
|
related_objects.map do |model, performables|
|
||||||
|
performables_text = performables.map { |performable| "#{performable[:name]} (##{performable[:id]})" }.join(', ')
|
||||||
|
"#{model}: #{performables_text}"
|
||||||
|
end.join(', ')
|
||||||
|
end
|
||||||
|
end
|
|
@ -1,6 +1,7 @@
|
||||||
FactoryBot.define do
|
FactoryBot.define do
|
||||||
factory :webhook do
|
factory :webhook do
|
||||||
sequence(:name) { |n| "Test webhook #{n}" }
|
sequence(:name) { |n| "Test webhook #{n}" }
|
||||||
|
endpoint { 'http://example.com/endpoint' }
|
||||||
ssl_verify { true }
|
ssl_verify { true }
|
||||||
active { true }
|
active { true }
|
||||||
created_by_id { 1 }
|
created_by_id { 1 }
|
||||||
|
|
|
@ -38,4 +38,22 @@ RSpec.describe Webhook, type: :model do
|
||||||
end
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '#destroy' do
|
||||||
|
subject(:webhook) { create(:webhook) }
|
||||||
|
|
||||||
|
context 'when no dependencies' do
|
||||||
|
it 'removes the object' do
|
||||||
|
expect { webhook.destroy }.to change(webhook, :destroyed?).to true
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when related object exists' do
|
||||||
|
let!(:trigger) { create(:trigger, perform: { 'notification.webhook' => { 'webhook_id' => webhook.id.to_s } }) }
|
||||||
|
|
||||||
|
it 'raises error with details' do
|
||||||
|
expect { webhook.destroy }.to raise_error(Exceptions::UnprocessableEntity, /#{Regexp.escape("Trigger: #{trigger.name} (##{trigger.id})")}/)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in a new issue