diff --git a/app/controllers/api/templates_clone_controller.rb b/app/controllers/api/templates_clone_controller.rb index dc40b70ea..343201606 100644 --- a/app/controllers/api/templates_clone_controller.rb +++ b/app/controllers/api/templates_clone_controller.rb @@ -86,7 +86,7 @@ def finalize_and_render_response(cloned_template) 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 682e6e242..cd25da887 100644 --- a/app/controllers/api/templates_controller.rb +++ b/app/controllers/api/templates_controller.rb @@ -220,9 +220,9 @@ def enqueue_template_updated_webhooks 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 b5332622a..c98836d18 100644 --- a/app/controllers/templates_controller.rb +++ b/app/controllers/templates_controller.rb @@ -160,14 +160,14 @@ def maybe_redirect_to_template(template) 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 85ad99355..f50dbfa77 100644 --- a/app/controllers/templates_uploads_controller.rb +++ b/app/controllers/templates_uploads_controller.rb @@ -72,7 +72,7 @@ def create_file_params_from_url 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 79bf345eb..950d9db7a 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 @@ def perform(params = {}) 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 6897746b0..e4a1340d6 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 @@ def perform(params = {}) 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 1c7a1e32c..e571c8fd1 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 @@ def perform(params = {}) 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 44510f463..e3b1d1384 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 @@ def perform(params = {}) 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 162743e43..5218461d3 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 @@ def perform(params = {}) 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 82fc271f4..56f3c0be0 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 @@ def perform(params = {}) 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 375bfa759..847933601 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 @@ def perform(params = {}) 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 d798e76a8..2ae47e2e1 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 @@ def perform(params = {}) 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 c2a5691b8..c361ba381 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 @@ def perform(params = {}) 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 353ecb6d5..7fa493792 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 @@ def perform(params = {}) 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 30623e15b..59d945a4a 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 @@ def perform(params = {}) 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 c0591395c..782317fe0 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 6f3ec5ae7..430b9e236 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 000000000..53a358a21 --- /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 b1e01cf03..f06135eb2 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 @@ 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 @@ 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 @@ 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 @@ 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 000000000..b839b8593 --- /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 e57c802ba..6f40354ac 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 bb2b51c63..336b3e266 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 @@ 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 2edcd2800..68c368fa6 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 @@ 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 000000000..ebb67ffdb --- /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 000000000..4707ca2cf --- /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 00bfa3f3e..48fd37ea3 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 000000000..318a53366 --- /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