From 41e18f9484f877a2d8c2f90d057b2757d6a8d0a4 Mon Sep 17 00:00:00 2001 From: Ryan Arakawa Date: Wed, 18 Feb 2026 14:37:08 -0600 Subject: [PATCH] CP-12376 extend webhooks partnerships (#55) * add partnership_id to webhook_urls - add migration to make account_id OR partnership_id required, you can use either, but can and must use at least one - add PARTNERSHIP_EVENTS constant to constrain webhook firing to just template events * extract duplicated webhook retry logic for webhook jobs - 11 webhook jobs all used the same retry logic (except one file that had 12 max retries instead of 10. - remove and replace duplicated code - add retry logic for partnership templates * refactor WebhookUrls to support partnerships - add for_template method to support account/partnership templates. - for_account_id will still work for submissions - update controllers with new method - I'm not great with Arel, so I refactored since I wanted account/partnership to use a shared method. * a automatic webhook creation for new partnerships * fix rubocop violations * update spec to expect raised error instead of empty array * fix rubocop/rspec for HTTP requests in test * remove after commit partnership webhook temporarily The immediately following PR will add this `after_commit` back. We only really need the upcoming template.preferences_updated webhook event that will be in the next PR, so even though it's unlikely anyone will be testing this at the Partnership level right now, better to just remove it for the time being for a cleaner PR. * validate incoming events against WebhookUrl::EVENTS constant * safety against SQL injection This method does not accept user input, but adding this just to be safe. * add form.changes_requested to events * since we added the events constant checker, we need to make sure this event is part of the constant list --- .../api/templates_clone_controller.rb | 2 +- app/controllers/api/templates_controller.rb | 4 +- app/controllers/templates_controller.rb | 4 +- .../templates_uploads_controller.rb | 2 +- ...m_changes_requested_webhook_request_job.rb | 19 +-- ...send_form_completed_webhook_request_job.rb | 17 +- .../send_form_declined_webhook_request_job.rb | 19 +-- .../send_form_started_webhook_request_job.rb | 19 +-- .../send_form_viewed_webhook_request_job.rb | 19 +-- ...submission_archived_webhook_request_job.rb | 19 +-- ...ubmission_completed_webhook_request_job.rb | 17 +- ..._submission_created_webhook_request_job.rb | 19 +-- ..._submission_expired_webhook_request_job.rb | 19 +-- ...nd_template_created_webhook_request_job.rb | 17 +- ...nd_template_updated_webhook_request_job.rb | 17 +- app/models/partnership.rb | 1 + app/models/webhook_url.rb | 42 +++-- ...1605_add_partnership_id_to_webhook_urls.rb | 14 ++ db/schema.rb | 17 +- lib/webhook_retry_logic.rb | 37 +++++ lib/webhook_urls.rb | 36 +++- ...mplate_created_webhook_request_job_spec.rb | 68 ++++++++ ...mplate_updated_webhook_request_job_spec.rb | 68 ++++++++ spec/lib/webhook_retry_logic_spec.rb | 45 +++++ spec/lib/webhook_urls_spec.rb | 157 ++++++++++++++++++ spec/models/partnership_spec.rb | 2 - spec/models/webhook_url_spec.rb | 94 +++++++++++ 27 files changed, 646 insertions(+), 148 deletions(-) create mode 100644 db/migrate/20260206171605_add_partnership_id_to_webhook_urls.rb create mode 100644 lib/webhook_retry_logic.rb create mode 100644 spec/lib/webhook_retry_logic_spec.rb create mode 100644 spec/lib/webhook_urls_spec.rb create mode 100644 spec/models/webhook_url_spec.rb diff --git a/app/controllers/api/templates_clone_controller.rb b/app/controllers/api/templates_clone_controller.rb index dc40b70e..34320160 100644 --- a/app/controllers/api/templates_clone_controller.rb +++ b/app/controllers/api/templates_clone_controller.rb @@ -86,7 +86,7 @@ module Api end def enqueue_webhooks(template) - WebhookUrls.for_account_id(template.account_id, 'template.created').each do |webhook_url| + WebhookUrls.for_template(template, 'template.created').each do |webhook_url| SendTemplateCreatedWebhookRequestJob.perform_async('template_id' => template.id, 'webhook_url_id' => webhook_url.id) end diff --git a/app/controllers/api/templates_controller.rb b/app/controllers/api/templates_controller.rb index 682e6e24..cd25da88 100644 --- a/app/controllers/api/templates_controller.rb +++ b/app/controllers/api/templates_controller.rb @@ -220,9 +220,9 @@ module Api end def enqueue_template_webhooks(template, event_type, job_class) - return if template.account_id.blank? + return if template.partnership.blank? && template.account.blank? - WebhookUrls.for_account_id(template.account_id, event_type).each do |webhook_url| + WebhookUrls.for_template(template, event_type).each do |webhook_url| job_class.perform_async('template_id' => template.id, 'webhook_url_id' => webhook_url.id) end end diff --git a/app/controllers/templates_controller.rb b/app/controllers/templates_controller.rb index b5332622..c98836d1 100644 --- a/app/controllers/templates_controller.rb +++ b/app/controllers/templates_controller.rb @@ -160,14 +160,14 @@ class TemplatesController < ApplicationController end def enqueue_template_created_webhooks(template) - WebhookUrls.for_account_id(template.account_id, 'template.created').each do |webhook_url| + WebhookUrls.for_template(template, 'template.created').each do |webhook_url| SendTemplateCreatedWebhookRequestJob.perform_async('template_id' => template.id, 'webhook_url_id' => webhook_url.id) end end def enqueue_template_updated_webhooks(template) - WebhookUrls.for_account_id(template.account_id, 'template.updated').each do |webhook_url| + WebhookUrls.for_template(template, 'template.updated').each do |webhook_url| SendTemplateUpdatedWebhookRequestJob.perform_async('template_id' => template.id, 'webhook_url_id' => webhook_url.id) end diff --git a/app/controllers/templates_uploads_controller.rb b/app/controllers/templates_uploads_controller.rb index 85ad9935..f50dbfa7 100644 --- a/app/controllers/templates_uploads_controller.rb +++ b/app/controllers/templates_uploads_controller.rb @@ -72,7 +72,7 @@ class TemplatesUploadsController < ApplicationController end def enqueue_template_created_webhooks(template) - WebhookUrls.for_account_id(template.account_id, 'template.created').each do |webhook_url| + WebhookUrls.for_template(template, 'template.created').each do |webhook_url| SendTemplateCreatedWebhookRequestJob.perform_async('template_id' => template.id, 'webhook_url_id' => webhook_url.id) end diff --git a/app/jobs/send_form_changes_requested_webhook_request_job.rb b/app/jobs/send_form_changes_requested_webhook_request_job.rb index 79bf345e..950d9db7 100644 --- a/app/jobs/send_form_changes_requested_webhook_request_job.rb +++ b/app/jobs/send_form_changes_requested_webhook_request_job.rb @@ -5,8 +5,6 @@ class SendFormChangesRequestedWebhookRequestJob sidekiq_options queue: :webhooks - MAX_ATTEMPTS = 10 - def perform(params = {}) submitter = Submitter.find(params['submitter_id']) webhook_url = WebhookUrl.find(params['webhook_url_id']) @@ -20,14 +18,13 @@ class SendFormChangesRequestedWebhookRequestJob resp = SendWebhookRequest.call(webhook_url, event_type: 'form.changes_requested', data: Submitters::SerializeForWebhook.call(submitter)) - if (resp.nil? || resp.status.to_i >= 400) && attempt <= MAX_ATTEMPTS && - (!Docuseal.multitenant? || submitter.account.account_configs.exists?(key: :plan)) - SendFormChangesRequestedWebhookRequestJob.perform_in((2**attempt).minutes, { - 'submitter_id' => submitter.id, - 'webhook_url_id' => webhook_url.id, - 'attempt' => attempt + 1, - 'last_status' => resp&.status.to_i - }) - end + return unless WebhookRetryLogic.should_retry?(response: resp, attempt: attempt, record: submitter) + + SendFormChangesRequestedWebhookRequestJob.perform_in((2**attempt).minutes, { + 'submitter_id' => submitter.id, + 'webhook_url_id' => webhook_url.id, + 'attempt' => attempt + 1, + 'last_status' => resp&.status.to_i + }) end end diff --git a/app/jobs/send_form_completed_webhook_request_job.rb b/app/jobs/send_form_completed_webhook_request_job.rb index 6897746b..e4a1340d 100644 --- a/app/jobs/send_form_completed_webhook_request_job.rb +++ b/app/jobs/send_form_completed_webhook_request_job.rb @@ -5,8 +5,6 @@ class SendFormCompletedWebhookRequestJob sidekiq_options queue: :webhooks - MAX_ATTEMPTS = 12 - def perform(params = {}) submitter = Submitter.find(params['submitter_id']) webhook_url = WebhookUrl.find(params['webhook_url_id']) @@ -28,14 +26,13 @@ class SendFormCompletedWebhookRequestJob resp = SendWebhookRequest.call(webhook_url, event_type: 'form.completed', data: webhook_data) - if (resp.nil? || resp.status.to_i >= 400) && attempt <= MAX_ATTEMPTS && - (!Docuseal.multitenant? || submitter.account.account_configs.exists?(key: :plan)) - SendFormCompletedWebhookRequestJob.perform_in((2**attempt).minutes, { - **params, - 'attempt' => attempt + 1, - 'last_status' => resp&.status.to_i - }) - end + return unless WebhookRetryLogic.should_retry?(response: resp, attempt: attempt, record: submitter) + + SendFormCompletedWebhookRequestJob.perform_in((2**attempt).minutes, { + **params, + 'attempt' => attempt + 1, + 'last_status' => resp&.status.to_i + }) end private diff --git a/app/jobs/send_form_declined_webhook_request_job.rb b/app/jobs/send_form_declined_webhook_request_job.rb index 1c7a1e32..e571c8fd 100644 --- a/app/jobs/send_form_declined_webhook_request_job.rb +++ b/app/jobs/send_form_declined_webhook_request_job.rb @@ -5,8 +5,6 @@ class SendFormDeclinedWebhookRequestJob sidekiq_options queue: :webhooks - MAX_ATTEMPTS = 10 - def perform(params = {}) submitter = Submitter.find(params['submitter_id']) webhook_url = WebhookUrl.find(params['webhook_url_id']) @@ -20,14 +18,13 @@ class SendFormDeclinedWebhookRequestJob resp = SendWebhookRequest.call(webhook_url, event_type: 'form.declined', data: Submitters::SerializeForWebhook.call(submitter)) - if (resp.nil? || resp.status.to_i >= 400) && attempt <= MAX_ATTEMPTS && - (!Docuseal.multitenant? || submitter.account.account_configs.exists?(key: :plan)) - SendFormDeclinedWebhookRequestJob.perform_in((2**attempt).minutes, { - 'submitter_id' => submitter.id, - 'webhook_url_id' => webhook_url.id, - 'attempt' => attempt + 1, - 'last_status' => resp&.status.to_i - }) - end + return unless WebhookRetryLogic.should_retry?(response: resp, attempt: attempt, record: submitter) + + SendFormDeclinedWebhookRequestJob.perform_in((2**attempt).minutes, { + 'submitter_id' => submitter.id, + 'webhook_url_id' => webhook_url.id, + 'attempt' => attempt + 1, + 'last_status' => resp&.status.to_i + }) end end diff --git a/app/jobs/send_form_started_webhook_request_job.rb b/app/jobs/send_form_started_webhook_request_job.rb index 44510f46..e3b1d138 100644 --- a/app/jobs/send_form_started_webhook_request_job.rb +++ b/app/jobs/send_form_started_webhook_request_job.rb @@ -5,8 +5,6 @@ class SendFormStartedWebhookRequestJob sidekiq_options queue: :webhooks - MAX_ATTEMPTS = 10 - def perform(params = {}) submitter = Submitter.find(params['submitter_id']) webhook_url = WebhookUrl.find(params['webhook_url_id']) @@ -20,14 +18,13 @@ class SendFormStartedWebhookRequestJob resp = SendWebhookRequest.call(webhook_url, event_type: 'form.started', data: Submitters::SerializeForWebhook.call(submitter)) - if (resp.nil? || resp.status.to_i >= 400) && attempt <= MAX_ATTEMPTS && - (!Docuseal.multitenant? || submitter.account.account_configs.exists?(key: :plan)) - SendFormStartedWebhookRequestJob.perform_in((2**attempt).minutes, { - 'submitter_id' => submitter.id, - 'webhook_url_id' => webhook_url.id, - 'attempt' => attempt + 1, - 'last_status' => resp&.status.to_i - }) - end + return unless WebhookRetryLogic.should_retry?(response: resp, attempt: attempt, record: submitter) + + SendFormStartedWebhookRequestJob.perform_in((2**attempt).minutes, { + 'submitter_id' => submitter.id, + 'webhook_url_id' => webhook_url.id, + 'attempt' => attempt + 1, + 'last_status' => resp&.status.to_i + }) end end diff --git a/app/jobs/send_form_viewed_webhook_request_job.rb b/app/jobs/send_form_viewed_webhook_request_job.rb index 162743e4..5218461d 100644 --- a/app/jobs/send_form_viewed_webhook_request_job.rb +++ b/app/jobs/send_form_viewed_webhook_request_job.rb @@ -5,8 +5,6 @@ class SendFormViewedWebhookRequestJob sidekiq_options queue: :webhooks - MAX_ATTEMPTS = 10 - def perform(params = {}) submitter = Submitter.find(params['submitter_id']) webhook_url = WebhookUrl.find(params['webhook_url_id']) @@ -20,14 +18,13 @@ class SendFormViewedWebhookRequestJob resp = SendWebhookRequest.call(webhook_url, event_type: 'form.viewed', data: Submitters::SerializeForWebhook.call(submitter)) - if (resp.nil? || resp.status.to_i >= 400) && attempt <= MAX_ATTEMPTS && - (!Docuseal.multitenant? || submitter.account.account_configs.exists?(key: :plan)) - SendFormViewedWebhookRequestJob.perform_in((2**attempt).minutes, { - 'submitter_id' => submitter.id, - 'webhook_url_id' => webhook_url.id, - 'attempt' => attempt + 1, - 'last_status' => resp&.status.to_i - }) - end + return unless WebhookRetryLogic.should_retry?(response: resp, attempt: attempt, record: submitter) + + SendFormViewedWebhookRequestJob.perform_in((2**attempt).minutes, { + 'submitter_id' => submitter.id, + 'webhook_url_id' => webhook_url.id, + 'attempt' => attempt + 1, + 'last_status' => resp&.status.to_i + }) end end diff --git a/app/jobs/send_submission_archived_webhook_request_job.rb b/app/jobs/send_submission_archived_webhook_request_job.rb index 82fc271f..56f3c0be 100644 --- a/app/jobs/send_submission_archived_webhook_request_job.rb +++ b/app/jobs/send_submission_archived_webhook_request_job.rb @@ -5,8 +5,6 @@ class SendSubmissionArchivedWebhookRequestJob sidekiq_options queue: :webhooks - MAX_ATTEMPTS = 10 - def perform(params = {}) submission = Submission.find(params['submission_id']) webhook_url = WebhookUrl.find(params['webhook_url_id']) @@ -18,14 +16,13 @@ class SendSubmissionArchivedWebhookRequestJob resp = SendWebhookRequest.call(webhook_url, event_type: 'submission.archived', data: submission.as_json(only: %i[id archived_at])) - if (resp.nil? || resp.status.to_i >= 400) && attempt <= MAX_ATTEMPTS && - (!Docuseal.multitenant? || submission.account.account_configs.exists?(key: :plan)) - SendSubmissionArchivedWebhookRequestJob.perform_in((2**attempt).minutes, { - 'submission_id' => submission.id, - 'webhook_url_id' => webhook_url.id, - 'attempt' => attempt + 1, - 'last_status' => resp&.status.to_i - }) - end + return unless WebhookRetryLogic.should_retry?(response: resp, attempt: attempt, record: submission) + + SendSubmissionArchivedWebhookRequestJob.perform_in((2**attempt).minutes, { + 'submission_id' => submission.id, + 'webhook_url_id' => webhook_url.id, + 'attempt' => attempt + 1, + 'last_status' => resp&.status.to_i + }) end end diff --git a/app/jobs/send_submission_completed_webhook_request_job.rb b/app/jobs/send_submission_completed_webhook_request_job.rb index 375bfa75..84793360 100644 --- a/app/jobs/send_submission_completed_webhook_request_job.rb +++ b/app/jobs/send_submission_completed_webhook_request_job.rb @@ -5,8 +5,6 @@ class SendSubmissionCompletedWebhookRequestJob sidekiq_options queue: :webhooks - MAX_ATTEMPTS = 10 - def perform(params = {}) submission = Submission.find(params['submission_id']) webhook_url = WebhookUrl.find(params['webhook_url_id']) @@ -18,13 +16,12 @@ class SendSubmissionCompletedWebhookRequestJob resp = SendWebhookRequest.call(webhook_url, event_type: 'submission.completed', data: Submissions::SerializeForApi.call(submission)) - if (resp.nil? || resp.status.to_i >= 400) && attempt <= MAX_ATTEMPTS && - (!Docuseal.multitenant? || submission.account.account_configs.exists?(key: :plan)) - SendSubmissionCompletedWebhookRequestJob.perform_in((2**attempt).minutes, { - **params, - 'attempt' => attempt + 1, - 'last_status' => resp&.status.to_i - }) - end + return unless WebhookRetryLogic.should_retry?(response: resp, attempt: attempt, record: submission) + + SendSubmissionCompletedWebhookRequestJob.perform_in((2**attempt).minutes, { + **params, + 'attempt' => attempt + 1, + 'last_status' => resp&.status.to_i + }) end end diff --git a/app/jobs/send_submission_created_webhook_request_job.rb b/app/jobs/send_submission_created_webhook_request_job.rb index d798e76a..2ae47e2e 100644 --- a/app/jobs/send_submission_created_webhook_request_job.rb +++ b/app/jobs/send_submission_created_webhook_request_job.rb @@ -5,8 +5,6 @@ class SendSubmissionCreatedWebhookRequestJob sidekiq_options queue: :webhooks - MAX_ATTEMPTS = 10 - def perform(params = {}) submission = Submission.find(params['submission_id']) webhook_url = WebhookUrl.find(params['webhook_url_id']) @@ -18,14 +16,13 @@ class SendSubmissionCreatedWebhookRequestJob resp = SendWebhookRequest.call(webhook_url, event_type: 'submission.created', data: Submissions::SerializeForApi.call(submission)) - if (resp.nil? || resp.status.to_i >= 400) && attempt <= MAX_ATTEMPTS && - (!Docuseal.multitenant? || submission.account.account_configs.exists?(key: :plan)) - SendSubmissionCreatedWebhookRequestJob.perform_in((2**attempt).minutes, { - 'submission_id' => submission.id, - 'webhook_url_id' => webhook_url.id, - 'attempt' => attempt + 1, - 'last_status' => resp&.status.to_i - }) - end + return unless WebhookRetryLogic.should_retry?(response: resp, attempt: attempt, record: submission) + + SendSubmissionCreatedWebhookRequestJob.perform_in((2**attempt).minutes, { + 'submission_id' => submission.id, + 'webhook_url_id' => webhook_url.id, + 'attempt' => attempt + 1, + 'last_status' => resp&.status.to_i + }) end end diff --git a/app/jobs/send_submission_expired_webhook_request_job.rb b/app/jobs/send_submission_expired_webhook_request_job.rb index c2a5691b..c361ba38 100644 --- a/app/jobs/send_submission_expired_webhook_request_job.rb +++ b/app/jobs/send_submission_expired_webhook_request_job.rb @@ -5,8 +5,6 @@ class SendSubmissionExpiredWebhookRequestJob sidekiq_options queue: :webhooks - MAX_ATTEMPTS = 10 - def perform(params = {}) submission = Submission.find(params['submission_id']) webhook_url = WebhookUrl.find(params['webhook_url_id']) @@ -18,14 +16,13 @@ class SendSubmissionExpiredWebhookRequestJob resp = SendWebhookRequest.call(webhook_url, event_type: 'submission.expired', data: Submissions::SerializeForApi.call(submission)) - if (resp.nil? || resp.status.to_i >= 400) && attempt <= MAX_ATTEMPTS && - (!Docuseal.multitenant? || submission.account.account_configs.exists?(key: :plan)) - SendSubmissionExpiredWebhookRequestJob.perform_in((2**attempt).minutes, { - 'submission_id' => submission.id, - 'webhook_url_id' => webhook_url.id, - 'attempt' => attempt + 1, - 'last_status' => resp&.status.to_i - }) - end + return unless WebhookRetryLogic.should_retry?(response: resp, attempt: attempt, record: submission) + + SendSubmissionExpiredWebhookRequestJob.perform_in((2**attempt).minutes, { + 'submission_id' => submission.id, + 'webhook_url_id' => webhook_url.id, + 'attempt' => attempt + 1, + 'last_status' => resp&.status.to_i + }) end end diff --git a/app/jobs/send_template_created_webhook_request_job.rb b/app/jobs/send_template_created_webhook_request_job.rb index 353ecb6d..7fa49379 100644 --- a/app/jobs/send_template_created_webhook_request_job.rb +++ b/app/jobs/send_template_created_webhook_request_job.rb @@ -18,14 +18,13 @@ class SendTemplateCreatedWebhookRequestJob resp = SendWebhookRequest.call(webhook_url, event_type: 'template.created', data: Templates::SerializeForApi.call(template)) - if (resp.nil? || resp.status.to_i >= 400) && attempt <= MAX_ATTEMPTS && - (!Docuseal.multitenant? || template.account.account_configs.exists?(key: :plan)) - SendTemplateCreatedWebhookRequestJob.perform_in((2**attempt).minutes, { - 'template_id' => template.id, - 'webhook_url_id' => webhook_url.id, - 'attempt' => attempt + 1, - 'last_status' => resp&.status.to_i - }) - end + return unless WebhookRetryLogic.should_retry?(response: resp, attempt: attempt, record: template) + + SendTemplateCreatedWebhookRequestJob.perform_in((2**attempt).minutes, { + 'template_id' => template.id, + 'webhook_url_id' => webhook_url.id, + 'attempt' => attempt + 1, + 'last_status' => resp&.status.to_i + }) end end diff --git a/app/jobs/send_template_updated_webhook_request_job.rb b/app/jobs/send_template_updated_webhook_request_job.rb index 30623e15..59d945a4 100644 --- a/app/jobs/send_template_updated_webhook_request_job.rb +++ b/app/jobs/send_template_updated_webhook_request_job.rb @@ -18,14 +18,13 @@ class SendTemplateUpdatedWebhookRequestJob resp = SendWebhookRequest.call(webhook_url, event_type: 'template.updated', data: Templates::SerializeForApi.call(template)) - if (resp.nil? || resp.status.to_i >= 400) && attempt <= MAX_ATTEMPTS && - (!Docuseal.multitenant? || template.account.account_configs.exists?(key: :plan)) - SendTemplateUpdatedWebhookRequestJob.perform_in((2**attempt).minutes, { - 'template_id' => template.id, - 'webhook_url_id' => webhook_url.id, - 'attempt' => attempt + 1, - 'last_status' => resp&.status.to_i - }) - end + return unless WebhookRetryLogic.should_retry?(response: resp, attempt: attempt, record: template) + + SendTemplateUpdatedWebhookRequestJob.perform_in((2**attempt).minutes, { + 'template_id' => template.id, + 'webhook_url_id' => webhook_url.id, + 'attempt' => attempt + 1, + 'last_status' => resp&.status.to_i + }) end end diff --git a/app/models/partnership.rb b/app/models/partnership.rb index c0591395..782317fe 100644 --- a/app/models/partnership.rb +++ b/app/models/partnership.rb @@ -17,6 +17,7 @@ class Partnership < ApplicationRecord has_many :templates, dependent: :destroy has_many :template_folders, dependent: :destroy + has_many :webhook_urls, dependent: :destroy validates :external_partnership_id, presence: true, uniqueness: true validates :name, presence: true diff --git a/app/models/webhook_url.rb b/app/models/webhook_url.rb index 6f3ec5ae..430b9e23 100644 --- a/app/models/webhook_url.rb +++ b/app/models/webhook_url.rb @@ -4,23 +4,26 @@ # # Table name: webhook_urls # -# id :bigint not null, primary key -# events :text not null -# secret :text not null -# sha1 :string not null -# url :text not null -# created_at :datetime not null -# updated_at :datetime not null -# account_id :integer not null +# id :bigint not null, primary key +# events :text not null +# secret :text not null +# sha1 :string not null +# url :text not null +# created_at :datetime not null +# updated_at :datetime not null +# account_id :integer +# partnership_id :bigint # # Indexes # -# index_webhook_urls_on_account_id (account_id) -# index_webhook_urls_on_sha1 (sha1) +# index_webhook_urls_on_account_id (account_id) +# index_webhook_urls_on_partnership_id (partnership_id) +# index_webhook_urls_on_sha1 (sha1) # # Foreign Keys # # fk_rails_... (account_id => accounts.id) +# fk_rails_... (partnership_id => partnerships.id) # class WebhookUrl < ApplicationRecord EVENTS = %w[ @@ -28,6 +31,7 @@ class WebhookUrl < ApplicationRecord form.started form.completed form.declined + form.changes_requested submission.created submission.completed submission.expired @@ -36,7 +40,14 @@ class WebhookUrl < ApplicationRecord template.updated ].freeze - belongs_to :account + # Partnership webhooks can only use template events since partnerships don't have submissions/submitters + PARTNERSHIP_EVENTS = %w[ + template.created + template.updated + ].freeze + + belongs_to :account, optional: true + belongs_to :partnership, optional: true attribute :events, :string, default: -> { %w[form.viewed form.started form.completed form.declined] } attribute :secret, :string, default: -> { {} } @@ -45,10 +56,19 @@ class WebhookUrl < ApplicationRecord serialize :secret, coder: JSON before_validation :set_sha1 + validate :validate_owner_presence encrypts :url, :secret def set_sha1 self.sha1 = Digest::SHA1.hexdigest(url) end + + private + + def validate_owner_presence + return if account_id.present? ^ partnership_id.present? + + errors.add(:base, 'Must have either account_id or partnership_id, but not both') + end end diff --git a/db/migrate/20260206171605_add_partnership_id_to_webhook_urls.rb b/db/migrate/20260206171605_add_partnership_id_to_webhook_urls.rb new file mode 100644 index 00000000..53a358a2 --- /dev/null +++ b/db/migrate/20260206171605_add_partnership_id_to_webhook_urls.rb @@ -0,0 +1,14 @@ +class AddPartnershipIdToWebhookUrls < ActiveRecord::Migration[8.0] + def change + # Make account_id nullable since webhooks can now belong to either account or partnership + change_column_null :webhook_urls, :account_id, true + + # Add partnership_id as optional reference + add_reference :webhook_urls, :partnership, null: true, foreign_key: true + + # Add check constraint to ensure exactly one of account_id or partnership_id is set + add_check_constraint :webhook_urls, + '(account_id IS NOT NULL AND partnership_id IS NULL) OR (account_id IS NULL AND partnership_id IS NOT NULL)', + name: 'webhook_urls_owner_check' + end +end diff --git a/db/schema.rb b/db/schema.rb index b1e01cf0..f06135eb 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2026_01_21_191632) do +ActiveRecord::Schema[8.0].define(version: 2026_02_06_171605) do # These are extensions that must be enabled in order to support this database enable_extension "btree_gin" enable_extension "pg_catalog.plpgsql" @@ -181,7 +181,7 @@ ActiveRecord::Schema[8.0].define(version: 2026_01_21_191632) do t.datetime "created_at", null: false t.index ["account_id", "event_datetime"], name: "index_email_events_on_account_id_and_event_datetime" t.index ["email"], name: "index_email_events_on_email" - t.index ["email"], name: "index_email_events_on_email_event_types", where: "((event_type)::text = ANY (ARRAY[('bounce'::character varying)::text, ('soft_bounce'::character varying)::text, ('complaint'::character varying)::text, ('soft_complaint'::character varying)::text]))" + t.index ["email"], name: "index_email_events_on_email_event_types", where: "((event_type)::text = ANY ((ARRAY['bounce'::character varying, 'soft_bounce'::character varying, 'complaint'::character varying, 'soft_complaint'::character varying])::text[]))" t.index ["emailable_type", "emailable_id"], name: "index_email_events_on_emailable" t.index ["message_id"], name: "index_email_events_on_message_id" end @@ -290,11 +290,10 @@ ActiveRecord::Schema[8.0].define(version: 2026_01_21_191632) do t.tsvector "tsvector", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.index ["account_id"], name: "index_search_entries_on_account_id" + t.index ["account_id", "tsvector"], name: "index_search_entries_on_account_id_tsvector_submission", where: "((record_type)::text = 'Submission'::text)", using: :gin + t.index ["account_id", "tsvector"], name: "index_search_entries_on_account_id_tsvector_submitter", where: "((record_type)::text = 'Submitter'::text)", using: :gin + t.index ["account_id", "tsvector"], name: "index_search_entries_on_account_id_tsvector_template", where: "((record_type)::text = 'Template'::text)", using: :gin t.index ["record_id", "record_type"], name: "index_search_entries_on_record_id_and_record_type", unique: true - t.index ["tsvector"], name: "index_search_entries_on_account_id_tsvector_submission", where: "((record_type)::text = 'Submission'::text)", using: :gin - t.index ["tsvector"], name: "index_search_entries_on_account_id_tsvector_submitter", where: "((record_type)::text = 'Submitter'::text)", using: :gin - t.index ["tsvector"], name: "index_search_entries_on_account_id_tsvector_template", where: "((record_type)::text = 'Template'::text)", using: :gin end create_table "submission_events", force: :cascade do |t| @@ -470,15 +469,18 @@ ActiveRecord::Schema[8.0].define(version: 2026_01_21_191632) do end create_table "webhook_urls", force: :cascade do |t| - t.integer "account_id", null: false + t.integer "account_id" t.text "url", null: false t.text "events", null: false t.string "sha1", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.text "secret", null: false + t.bigint "partnership_id" t.index ["account_id"], name: "index_webhook_urls_on_account_id" + t.index ["partnership_id"], name: "index_webhook_urls_on_partnership_id" t.index ["sha1"], name: "index_webhook_urls_on_sha1" + t.check_constraint "account_id IS NOT NULL AND partnership_id IS NULL OR account_id IS NULL AND partnership_id IS NOT NULL", name: "webhook_urls_owner_check" end add_foreign_key "access_tokens", "users" @@ -516,4 +518,5 @@ ActiveRecord::Schema[8.0].define(version: 2026_01_21_191632) do add_foreign_key "user_configs", "users" add_foreign_key "users", "accounts" add_foreign_key "webhook_urls", "accounts" + add_foreign_key "webhook_urls", "partnerships" end diff --git a/lib/webhook_retry_logic.rb b/lib/webhook_retry_logic.rb new file mode 100644 index 00000000..b839b859 --- /dev/null +++ b/lib/webhook_retry_logic.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +# Shared logic for determining if webhook requests should be retried +# Used across all Send*WebhookRequestJob classes +module WebhookRetryLogic + module_function + + MAX_ATTEMPTS = 10 + + # Determines if a failed webhook request should be retried + # + # @param response [HTTP::Response, nil] The HTTP response from the webhook request + # @param attempt [Integer] Current retry attempt number + # @param record [Template, Submission, Submitter] The record triggering the webhook + # @return [Boolean] true if the webhook should be retried + def should_retry?(response:, attempt:, record:) + return false unless response.nil? || response.status.to_i >= 400 + return false if attempt > MAX_ATTEMPTS + return true unless Docuseal.multitenant? + + eligible_for_retries?(record) + end + + # Checks if a record is eligible for webhook retries in multitenant mode + # @param record [Template, Submission, Submitter] The record to check + # @return [Boolean] true if eligible for retries + def eligible_for_retries?(record) + case record + when Template + record.partnership_id.present? || record.account&.account_configs&.exists?(key: :plan) + when Submission, Submitter + record.account.account_configs.exists?(key: :plan) + else + false + end + end +end diff --git a/lib/webhook_urls.rb b/lib/webhook_urls.rb index e57c802b..6f40354a 100644 --- a/lib/webhook_urls.rb +++ b/lib/webhook_urls.rb @@ -3,25 +3,47 @@ module WebhookUrls module_function - def for_account_id(account_id, events) - events = Array.wrap(events) + def for_template(template, events) + if template.partnership_id.present? + for_partnership_id(template.partnership_id, events) + elsif template.account_id.present? + for_account_id(template.account_id, events) + else + raise ArgumentError, 'Template must have either account_id or partnership_id' + end + end + def for_account_id(account_id, events) rel = WebhookUrl.where(account_id:) - event_arel = events.map { |event| Arel::Table.new(:webhook_urls)[:events].matches("%\"#{event}\"%") }.reduce(:or) - if Docuseal.multitenant? || account_id == 1 - rel.where(event_arel) + rel.where(event_matcher(events)) else linked_account_rel = AccountLinkedAccount.where(linked_account_id: account_id).where.not(account_type: :testing).select(:account_id) - webhook_urls = rel.or(WebhookUrl.where(account_id: linked_account_rel).where(event_arel)) + webhook_urls = rel.or(WebhookUrl.where(account_id: linked_account_rel).where(event_matcher(events))) account_urls, linked_urls = webhook_urls.partition { |w| w.account_id == account_id } - account_urls.select { |w| w.events.intersect?(events) }.presence || + account_urls.select { |w| w.events.intersect?(Array.wrap(events)) }.presence || (account_urls.present? ? WebhookUrl.none : linked_urls) end end + + def for_partnership_id(partnership_id, events) + WebhookUrl.where(partnership_id:).where(event_matcher(events)) + end + + def event_matcher(events) + events = Array.wrap(events) + + # Validate against known events constant + invalid_events = events - WebhookUrl::EVENTS + raise ArgumentError, "Invalid events: #{invalid_events.join(', ')}" if invalid_events.any? + + conditions = events.map { 'events LIKE ?' }.join(' OR ') + values = events.map { |event| "%\"#{event}\"%" } + [conditions, *values] + end end diff --git a/spec/jobs/send_template_created_webhook_request_job_spec.rb b/spec/jobs/send_template_created_webhook_request_job_spec.rb index bb2b51c6..336b3e26 100644 --- a/spec/jobs/send_template_created_webhook_request_job_spec.rb +++ b/spec/jobs/send_template_created_webhook_request_job_spec.rb @@ -84,5 +84,73 @@ RSpec.describe SendTemplateCreatedWebhookRequestJob do expect(WebMock).to have_requested(:post, webhook_url.url).once end + + context 'with partnership template' do + let(:partnership) { create(:partnership) } + let(:partnership_template) { create(:template, partnership: partnership, account: nil, author: user) } + let(:partnership_webhook) do + create(:webhook_url, + partnership: partnership, + account: nil, + events: ['template.created'], + url: 'https://partnership.example.com/webhook') + end + + before do + stub_request(:post, partnership_webhook.url).to_return(status: 200) + end + + it 'sends a webhook request for partnership template' do + described_class.new.perform( + 'template_id' => partnership_template.id, + 'webhook_url_id' => partnership_webhook.id + ) + + expect(WebMock).to have_requested(:post, partnership_webhook.url).with( + body: { + 'event_type' => 'template.created', + 'timestamp' => /.*/, + 'data' => JSON.parse(Templates::SerializeForApi.call(partnership_template.reload).to_json) + }, + headers: { + 'Content-Type' => 'application/json', + 'User-Agent' => 'DocuSeal.com Webhook' + } + ).once + end + + it 'sends a webhook request with the partnership secret' do + partnership_webhook.update(secret: { 'X-Partnership-Secret' => 'partnership_secret' }) + described_class.new.perform( + 'template_id' => partnership_template.id, + 'webhook_url_id' => partnership_webhook.id + ) + + expect(WebMock).to have_requested(:post, partnership_webhook.url).with( + headers: { + 'Content-Type' => 'application/json', + 'User-Agent' => 'DocuSeal.com Webhook', + 'X-Partnership-Secret' => 'partnership_secret' + } + ).once + end + + it 'retries on failure for partnership template' do + stub_request(:post, partnership_webhook.url).to_return(status: 500) + + expect do + described_class.new.perform( + 'template_id' => partnership_template.id, + 'webhook_url_id' => partnership_webhook.id + ) + end.to change(described_class.jobs, :size).by(1) + + expect(WebMock).to have_requested(:post, partnership_webhook.url).once + + args = described_class.jobs.last['args'].first + expect(args['attempt']).to eq(1) + expect(args['last_status']).to eq(500) + end + end end end diff --git a/spec/jobs/send_template_updated_webhook_request_job_spec.rb b/spec/jobs/send_template_updated_webhook_request_job_spec.rb index 2edcd280..68c368fa 100644 --- a/spec/jobs/send_template_updated_webhook_request_job_spec.rb +++ b/spec/jobs/send_template_updated_webhook_request_job_spec.rb @@ -84,5 +84,73 @@ RSpec.describe SendTemplateUpdatedWebhookRequestJob do expect(WebMock).to have_requested(:post, webhook_url.url).once end + + context 'with partnership template' do + let(:partnership) { create(:partnership) } + let(:partnership_template) { create(:template, partnership: partnership, account: nil, author: user) } + let(:partnership_webhook) do + create(:webhook_url, + partnership: partnership, + account: nil, + events: ['template.updated'], + url: 'https://partnership.example.com/webhook') + end + + before do + stub_request(:post, partnership_webhook.url).to_return(status: 200) + end + + it 'sends a webhook request for partnership template' do + described_class.new.perform( + 'template_id' => partnership_template.id, + 'webhook_url_id' => partnership_webhook.id + ) + + expect(WebMock).to have_requested(:post, partnership_webhook.url).with( + body: { + 'event_type' => 'template.updated', + 'timestamp' => /.*/, + 'data' => JSON.parse(Templates::SerializeForApi.call(partnership_template.reload).to_json) + }, + headers: { + 'Content-Type' => 'application/json', + 'User-Agent' => 'DocuSeal.com Webhook' + } + ).once + end + + it 'sends a webhook request with the partnership secret' do + partnership_webhook.update(secret: { 'X-Partnership-Secret' => 'partnership_secret' }) + described_class.new.perform( + 'template_id' => partnership_template.id, + 'webhook_url_id' => partnership_webhook.id + ) + + expect(WebMock).to have_requested(:post, partnership_webhook.url).with( + headers: { + 'Content-Type' => 'application/json', + 'User-Agent' => 'DocuSeal.com Webhook', + 'X-Partnership-Secret' => 'partnership_secret' + } + ).once + end + + it 'retries on failure for partnership template' do + stub_request(:post, partnership_webhook.url).to_return(status: 500) + + expect do + described_class.new.perform( + 'template_id' => partnership_template.id, + 'webhook_url_id' => partnership_webhook.id + ) + end.to change(described_class.jobs, :size).by(1) + + expect(WebMock).to have_requested(:post, partnership_webhook.url).once + + args = described_class.jobs.last['args'].first + expect(args['attempt']).to eq(1) + expect(args['last_status']).to eq(500) + end + end end end diff --git a/spec/lib/webhook_retry_logic_spec.rb b/spec/lib/webhook_retry_logic_spec.rb new file mode 100644 index 00000000..ebb67ffd --- /dev/null +++ b/spec/lib/webhook_retry_logic_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +# Simple response object for testing +FakeResponse = Struct.new(:status) + +RSpec.describe WebhookRetryLogic do + let(:account) { create(:account) } + let(:user) { create(:user, account: account) } + let(:template) { create(:template, account: account, author: user) } + + describe '.should_retry?' do + context 'with successful response' do + it 'does not retry' do + response = FakeResponse.new(200) + result = described_class.should_retry?(response: response, attempt: 1, record: template) + expect(result).to be false + end + end + + context 'with failed response' do + it 'retries on 4xx errors within attempt limit' do + response = FakeResponse.new(400) + result = described_class.should_retry?(response: response, attempt: 5, record: template) + expect(result).to be true + end + + it 'retries on 5xx errors within attempt limit' do + response = FakeResponse.new(500) + result = described_class.should_retry?(response: response, attempt: 5, record: template) + expect(result).to be true + end + + it 'retries on nil response within attempt limit' do + result = described_class.should_retry?(response: nil, attempt: 5, record: template) + expect(result).to be true + end + + it 'does not retry when max attempts exceeded' do + response = FakeResponse.new(500) + result = described_class.should_retry?(response: response, attempt: 11, record: template) + expect(result).to be false + end + end + end +end diff --git a/spec/lib/webhook_urls_spec.rb b/spec/lib/webhook_urls_spec.rb new file mode 100644 index 00000000..4707ca2c --- /dev/null +++ b/spec/lib/webhook_urls_spec.rb @@ -0,0 +1,157 @@ +# frozen_string_literal: true + +RSpec.describe WebhookUrls do + describe '.for_template' do + let(:user) { create(:user) } + + context 'with a partnership template' do + let(:partnership) { create(:partnership) } + let(:template) { create(:template, partnership: partnership, account: nil, author: user) } + let!(:partnership_webhook) do + create(:webhook_url, + partnership: partnership, + account: nil, + events: ['template.created'], + url: 'https://partnership.example.com/webhook') + end + + it 'returns partnership webhooks' do + webhooks = described_class.for_template(template, 'template.created') + expect(webhooks).to include(partnership_webhook) + end + + it 'does not return account webhooks' do + account_webhook = create(:webhook_url, account: create(:account), events: ['template.created']) + webhooks = described_class.for_template(template, 'template.created') + expect(webhooks).not_to include(account_webhook) + end + + it 'filters by event type' do + non_matching_webhook = create(:webhook_url, + partnership: partnership, + account: nil, + events: ['template.updated']) + + webhooks = described_class.for_template(template, 'template.created') + expect(webhooks).to include(partnership_webhook) + expect(webhooks).not_to include(non_matching_webhook) + end + end + + context 'with an account template' do + let(:account) { create(:account) } + let(:account_user) { create(:user, account: account) } + let(:template) { create(:template, account: account, partnership: nil, author: account_user) } + let!(:account_webhook) do + create(:webhook_url, + account: account, + partnership: nil, + events: ['template.created']) + end + + it 'returns account webhooks' do + webhooks = described_class.for_template(template, 'template.created') + expect(webhooks).to include(account_webhook) + end + + it 'does not return partnership webhooks' do + partnership_webhook = create(:webhook_url, + partnership: create(:partnership), + account: nil, + events: ['template.created']) + webhooks = described_class.for_template(template, 'template.created') + expect(webhooks).not_to include(partnership_webhook) + end + end + + context 'with a template that has neither account nor partnership' do + let(:template) { build(:template, account: nil, partnership: nil, author: user) } + + it 'raises an ArgumentError' do + expect do + described_class.for_template(template, 'template.created') + end.to raise_error(ArgumentError, 'Template must have either account_id or partnership_id') + end + end + end + + describe '.for_partnership_id' do + let(:partnership) { create(:partnership) } + let!(:webhook) do + create(:webhook_url, + partnership: partnership, + account: nil, + events: ['template.created', 'template.updated']) + end + let!(:webhook_update_only) do + create(:webhook_url, + partnership: partnership, + account: nil, + events: ['template.updated']) + end + + it 'returns webhooks matching the event' do + webhooks = described_class.for_partnership_id(partnership.id, 'template.created') + expect(webhooks).to include(webhook) + expect(webhooks).not_to include(webhook_update_only) + end + + it 'returns webhooks matching any of multiple events' do + webhooks = described_class.for_partnership_id(partnership.id, ['template.created', 'template.updated']) + expect(webhooks).to include(webhook, webhook_update_only) + end + + it 'does not return webhooks from other partnerships' do + other_partnership = create(:partnership) + other_webhook = create(:webhook_url, + partnership: other_partnership, + account: nil, + events: ['template.created']) + + webhooks = described_class.for_partnership_id(partnership.id, 'template.created') + expect(webhooks).not_to include(other_webhook) + end + + it 'handles single event as string' do + webhooks = described_class.for_partnership_id(partnership.id, 'template.updated') + expect(webhooks).to include(webhook, webhook_update_only) + end + end + + describe '.for_account_id' do + let(:account) { create(:account) } + let!(:webhook) do + create(:webhook_url, + account: account, + partnership: nil, + events: ['template.created']) + end + + it 'returns webhooks for the account' do + webhooks = described_class.for_account_id(account.id, 'template.created') + expect(webhooks).to include(webhook) + end + + it 'does not return webhooks from other accounts' do + other_account = create(:account) + other_webhook = create(:webhook_url, + account: other_account, + partnership: nil, + events: ['template.created']) + + webhooks = described_class.for_account_id(account.id, 'template.created') + expect(webhooks).not_to include(other_webhook) + end + + it 'filters by event type' do + non_matching = create(:webhook_url, + account: account, + partnership: nil, + events: ['template.updated']) + + webhooks = described_class.for_account_id(account.id, 'template.created') + expect(webhooks).to include(webhook) + expect(webhooks).not_to include(non_matching) + end + end +end diff --git a/spec/models/partnership_spec.rb b/spec/models/partnership_spec.rb index 00bfa3f3..48fd37ea 100644 --- a/spec/models/partnership_spec.rb +++ b/spec/models/partnership_spec.rb @@ -15,8 +15,6 @@ # index_partnerships_on_external_partnership_id (external_partnership_id) UNIQUE # describe Partnership do - let(:partnership) { create(:partnership) } - describe 'validations' do it 'validates presence of external_partnership_id' do partnership = build(:partnership, external_partnership_id: nil) diff --git a/spec/models/webhook_url_spec.rb b/spec/models/webhook_url_spec.rb new file mode 100644 index 00000000..318a5336 --- /dev/null +++ b/spec/models/webhook_url_spec.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: webhook_urls +# +# id :bigint not null, primary key +# events :text not null +# secret :text not null +# sha1 :string not null +# url :text not null +# created_at :datetime not null +# updated_at :datetime not null +# account_id :integer +# partnership_id :bigint +# +# Indexes +# +# index_webhook_urls_on_account_id (account_id) +# index_webhook_urls_on_partnership_id (partnership_id) +# index_webhook_urls_on_sha1 (sha1) +# +# Foreign Keys +# +# fk_rails_... (account_id => accounts.id) +# fk_rails_... (partnership_id => partnerships.id) +# +describe WebhookUrl do + describe 'validations' do + context 'with owner presence' do + it 'is valid with account_id and no partnership_id' do + webhook = build(:webhook_url, account: create(:account), partnership: nil) + expect(webhook).to be_valid + end + + it 'is valid with partnership_id and no account_id' do + # Disable webhook creation callback by removing env vars + stub_const('ENV', ENV.to_hash.except('CAREERPLUG_WEBHOOK_URL', 'CAREERPLUG_WEBHOOK_SECRET')) + + partnership = create(:partnership) + webhook = build(:webhook_url, + account: nil, + partnership: partnership, + events: WebhookUrl::PARTNERSHIP_EVENTS) + expect(webhook).to be_valid + end + + it 'is invalid with both account_id and partnership_id' do + stub_const('ENV', ENV.to_hash.except('CAREERPLUG_WEBHOOK_URL', 'CAREERPLUG_WEBHOOK_SECRET')) + + webhook = build(:webhook_url, account: create(:account), partnership: create(:partnership)) + expect(webhook).not_to be_valid + expect(webhook.errors[:base]).to include('Must have either account_id or partnership_id, but not both') + end + + it 'is invalid with neither account_id nor partnership_id' do + webhook = build(:webhook_url, account: nil, partnership: nil) + expect(webhook).not_to be_valid + expect(webhook.errors[:base]).to include('Must have either account_id or partnership_id, but not both') + end + end + + context 'with partnership events constraint' do + it 'only includes template.* events in PARTNERSHIP_EVENTS' do + expect(WebhookUrl::PARTNERSHIP_EVENTS).to all(start_with('template.')) + end + + it 'PARTNERSHIP_EVENTS is a subset of EVENTS' do + expect(WebhookUrl::PARTNERSHIP_EVENTS).to all(be_in(WebhookUrl::EVENTS)) + end + end + end + + describe 'callbacks' do + describe '#set_sha1' do + it 'sets sha1 based on url' do + webhook = build(:webhook_url, url: 'https://example.com/webhook') + webhook.valid? + expect(webhook.sha1).to eq(Digest::SHA1.hexdigest('https://example.com/webhook')) + end + + it 'updates sha1 when url changes' do + webhook = create(:webhook_url, url: 'https://example.com/webhook') + original_sha1 = webhook.sha1 + + webhook.url = 'https://example.com/new-webhook' + webhook.valid? + + expect(webhook.sha1).not_to eq(original_sha1) + expect(webhook.sha1).to eq(Digest::SHA1.hexdigest('https://example.com/new-webhook')) + end + end + end +end