diff --git a/app/models/account.rb b/app/models/account.rb index 2367c92b2..f8b9e7344 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -85,7 +85,7 @@ def create_careerplug_webhook webhook_urls.create!( url: ENV.fetch('CAREERPLUG_WEBHOOK_URL'), - events: %w[form.started form.completed submission.completed form.changes_requested template.preferences_updated], + events: WebhookUrl::CAREERPLUG_ACCOUNT_EVENTS, secret: { 'X-CareerPlug-Secret' => ENV.fetch('CAREERPLUG_WEBHOOK_SECRET') } ) end diff --git a/app/models/partnership.rb b/app/models/partnership.rb index ad17847e8..8208f1698 100644 --- a/app/models/partnership.rb +++ b/app/models/partnership.rb @@ -47,7 +47,7 @@ def create_careerplug_webhook webhook_urls.create!( url: ENV.fetch('CAREERPLUG_WEBHOOK_URL'), - events: %w[template.preferences_updated], + events: WebhookUrl::PARTNERSHIP_EVENTS, secret: { 'X-CareerPlug-Secret' => ENV.fetch('CAREERPLUG_WEBHOOK_SECRET') } ) end diff --git a/app/models/webhook_url.rb b/app/models/webhook_url.rb index 3a495755e..9512096b8 100644 --- a/app/models/webhook_url.rb +++ b/app/models/webhook_url.rb @@ -46,6 +46,17 @@ class WebhookUrl < ApplicationRecord template.preferences_updated ].freeze + # Events the ATS-pointed webhook must register for an Account-owned row. + # Single source of truth for both the on-create callback and the backfill task + # so the two can never drift (drift here is what caused missing submission.completed). + CAREERPLUG_ACCOUNT_EVENTS = %w[ + form.started + form.completed + submission.completed + form.changes_requested + template.preferences_updated + ].freeze + belongs_to :account, optional: true belongs_to :partnership, optional: true diff --git a/lib/careerplug_webhook_backfill.rb b/lib/careerplug_webhook_backfill.rb new file mode 100644 index 000000000..3641d9c4f --- /dev/null +++ b/lib/careerplug_webhook_backfill.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require 'digest' + +# Backfills and normalizes the ATS-pointed WebhookUrl for every Account and Partnership. +# +# The on-create provisioning callback (Account#create_careerplug_webhook, +# Partnership#create_careerplug_webhook) only fires for records created after +# CAREERPLUG_WEBHOOK_URL/SECRET were set. Pre-existing owners were never covered, +# and rows from the PopulateWebhookUrls migration carry a stale event set missing +# submission.completed. This one-time task closes both gaps. +# +# Idempotent: re-running with correct config produces zero changes. +# ALWAYS run with DRY_RUN=1 first and inspect the output before the live run +# (see lib/tasks/webhooks.rake). +class CareerplugWebhookBackfill + # Outcome of a run. duplicate_warnings lists owners with more than one + # ATS-pointed row (detected, never auto-deleted). + Result = Struct.new(:created, :updated, :unchanged, :duplicate_warnings, keyword_init: true) do + def self.empty + new(created: 0, updated: 0, unchanged: 0, duplicate_warnings: []) + end + end + + def self.run(dry_run: false) = new(dry_run:).run + + def initialize(dry_run: false) + @dry_run = dry_run + @result = Result.empty + end + + def run + return @result unless configured? + + Account.find_each { |account| upsert(account, WebhookUrl::CAREERPLUG_ACCOUNT_EVENTS) } + Partnership.find_each { |partnership| upsert(partnership, WebhookUrl::PARTNERSHIP_EVENTS) } + @result + end + + private + + def upsert(owner, events) + existing = owner.webhook_urls.where(sha1: target_sha1).to_a + + if existing.size > 1 + warn_duplicates(owner, existing) + return + end + + webhook = existing.first || owner.webhook_urls.new + webhook.assign_attributes(url: target_url, events: events, secret: target_secret) + + if webhook.new_record? then persist(webhook, :created) + elsif webhook.changed? then persist(webhook, :updated) + else + @result.unchanged += 1 + end + end + + def warn_duplicates(owner, existing) + @result.duplicate_warnings << { owner_class: owner.class.name, owner_id: owner.id, + webhook_ids: existing.map(&:id) } + log "DUPLICATE (no delete): #{owner.class} id=#{owner.id} has #{existing.size} " \ + "ATS-pointed rows: #{existing.map(&:id).join(', ')}" + end + + def persist(webhook, counter) + label = owner_label(webhook) + action = counter == :created ? 'Would create' : 'Would update' + log "#{@dry_run ? '[DRY RUN] ' : ''}#{action} #{label}" + @result[counter] += 1 + return if @dry_run + + webhook.save! + end + + def owner_label(webhook) + webhook.account_id ? "Account=#{webhook.account_id}" : "Partnership=#{webhook.partnership_id}" + end + + def configured? + return true if target_url.present? && ENV['CAREERPLUG_WEBHOOK_SECRET'].present? + + log 'CAREERPLUG_WEBHOOK_URL/SECRET not set — no-op' + false + end + + def target_url = ENV.fetch('CAREERPLUG_WEBHOOK_URL', nil) + def target_sha1 = Digest::SHA1.hexdigest(target_url.to_s) + def target_secret = { 'X-CareerPlug-Secret' => ENV.fetch('CAREERPLUG_WEBHOOK_SECRET') } + + def log(message) + $stdout.puts(message) + Rails.logger.info(message) + end +end diff --git a/lib/tasks/webhooks.rake b/lib/tasks/webhooks.rake index f6a42caa2..bab458823 100644 --- a/lib/tasks/webhooks.rake +++ b/lib/tasks/webhooks.rake @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'careerplug_webhook_backfill' + namespace :webhooks do desc 'Configure CareerPlug webhook secret from CAREERPLUG_WEBHOOK_SECRET env var' task configure_careerplug: :environment do @@ -92,4 +94,28 @@ namespace :webhooks do puts "Done: #{updated} webhook URL(s) updated" end + + desc <<~DESC + Backfill + normalize the ATS-pointed WebhookUrl for every Account and Partnership. + Creates missing rows, normalizes events to the canonical set, syncs URL/secret. + Idempotent. Reports duplicate ATS-pointed rows per owner (no auto-delete). + + ALWAYS run DRY_RUN=1 first and inspect counts + duplicate warnings before the live run. + A large `created` count for accounts that already have webhooks means the URL has + drifted (the task would create siblings instead of normalizing) — STOP and switch + to secret-identity matching instead of running live. + DESC + task backfill_careerplug: :environment do + dry_run = %w[1 true TRUE yes].include?(ENV.fetch('DRY_RUN', nil)) + result = CareerplugWebhookBackfill.run(dry_run: dry_run) + + puts "created=#{result.created} updated=#{result.updated} unchanged=#{result.unchanged} " \ + "duplicate_warnings=#{result.duplicate_warnings.length}" + + if dry_run + puts 'Dry run only — re-run without DRY_RUN to apply.' + elsif result.duplicate_warnings.any? + puts 'WARNING: duplicate ATS-pointed rows found (not deleted). Inspect and clean up manually.' + end + end end diff --git a/spec/lib/careerplug_webhook_backfill_spec.rb b/spec/lib/careerplug_webhook_backfill_spec.rb new file mode 100644 index 000000000..b05f174f6 --- /dev/null +++ b/spec/lib/careerplug_webhook_backfill_spec.rb @@ -0,0 +1,211 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'careerplug_webhook_backfill' + +RSpec.describe CareerplugWebhookBackfill do + # The on-create callback on Account/Partnership provisions a canonical webhook + # whenever CAREERPLUG_WEBHOOK_URL/SECRET are present. To test the backfill in + # isolation we disable the callback during owner creation (ENV absent), then + # restore ENV for the run. Mirrors the pattern in webhook_url_spec.rb. + let(:target_url) { 'http://localhost:3000/api/docuseal/events' } + let(:target_secret) { 'development_webhook_secret' } + let(:configured_env) do + ENV.to_h.merge('CAREERPLUG_WEBHOOK_URL' => target_url, 'CAREERPLUG_WEBHOOK_SECRET' => target_secret) + end + let(:callback_disabled_env) { ENV.to_h.except('CAREERPLUG_WEBHOOK_URL', 'CAREERPLUG_WEBHOOK_SECRET') } + + def run(dry_run: false) + described_class.run(dry_run: dry_run) + end + + # Create owners without the on-create callback firing (it no-ops when env vars are absent), + # then restore the configured ENV so the subsequent `run` sees the real target URL/secret. + def create_owner(factory, **attrs) + stub_const('ENV', callback_disabled_env) + owner = create(factory, **attrs) + stub_const('ENV', configured_env) + owner + end + + def build_stale_webhook(owner) + owner_attrs = owner.is_a?(Account) ? { account: owner, partnership: nil } : { partnership: owner, account: nil } + build(:webhook_url, + **owner_attrs, + url: target_url, + events: %w[form.viewed form.started form.completed form.declined], + secret: { 'X-CareerPlug-Secret' => target_secret }).tap(&:save!) + end + + describe '.run' do + before { stub_const('ENV', configured_env) } + + context 'when an Account has no ATS-pointed webhook' do + it 'creates one with the canonical account events and secret' do + account = create_owner(:account) + + expect { run }.to change(WebhookUrl, :count).by(1) + + webhook = account.webhook_urls.last + expect(webhook.url).to eq(target_url) + expect(webhook.events).to eq(WebhookUrl::CAREERPLUG_ACCOUNT_EVENTS) + expect(webhook.secret).to eq('X-CareerPlug-Secret' => target_secret) + end + end + + context 'when an Account has a stale webhook (old default events)' do + it 'updates events to include submission.completed without creating a second row' do + account = create_owner(:account) + webhook = build_stale_webhook(account) + original_id = webhook.id + + result = run + + expect(WebhookUrl.where(account_id: account.id).count).to eq(1) + expect(WebhookUrl.find(original_id).events).to include('submission.completed') + expect(WebhookUrl.find(original_id).events).to eq(WebhookUrl::CAREERPLUG_ACCOUNT_EVENTS) + expect(result.updated).to eq(1) + expect(result.created).to eq(0) + end + end + + context 'when an Account webhook is already canonical' do + it 'is unchanged' do + account = create_owner(:account) + webhook = build(:webhook_url, account: account, url: target_url, + events: WebhookUrl::CAREERPLUG_ACCOUNT_EVENTS, + secret: { 'X-CareerPlug-Secret' => target_secret }).tap(&:save!) + original_updated_at = webhook.updated_at + + result = run + + expect(WebhookUrl.where(account_id: account.id).count).to eq(1) + expect(WebhookUrl.find(webhook.id).updated_at).to eq(original_updated_at) + expect(result.unchanged).to eq(1) + expect(result.created).to eq(0) + expect(result.updated).to eq(0) + end + end + + context 'when a Partnership has no ATS-pointed webhook' do + it 'creates one with only template.preferences_updated' do + partnership = create_owner(:partnership) + + expect { run }.to change(WebhookUrl, :count).by(1) + + webhook = partnership.webhook_urls.last + expect(webhook.events).to eq(WebhookUrl::PARTNERSHIP_EVENTS) + expect(webhook.events).to eq(%w[template.preferences_updated]) + end + end + + context 'when a Partnership webhook is already canonical' do + it 'is unchanged' do + partnership = create_owner(:partnership) + webhook = build(:webhook_url, partnership: partnership, account: nil, url: target_url, + events: WebhookUrl::PARTNERSHIP_EVENTS, + secret: { 'X-CareerPlug-Secret' => target_secret }).tap(&:save!) + + result = run + + expect(result.unchanged).to eq(1) + expect(WebhookUrl.find(webhook.id).events).to eq(WebhookUrl::PARTNERSHIP_EVENTS) + end + end + + describe 'idempotency' do + it 'a second run changes nothing' do + create_owner(:account) + + run + count_after_first_run = WebhookUrl.count + updated_at_after_first_run = WebhookUrl.first.updated_at + + result = run + + expect(WebhookUrl.count).to eq(count_after_first_run) + expect(WebhookUrl.first.updated_at).to eq(updated_at_after_first_run) + expect(result.created).to eq(0) + expect(result.updated).to eq(0) + expect(result.unchanged).to eq(1) + end + end + + context 'when an owner has duplicate ATS-pointed rows' do + it 'records a warning and mutates nothing' do + account = create_owner(:account) + build(:webhook_url, account: account, url: target_url, + events: WebhookUrl::CAREERPLUG_ACCOUNT_EVENTS, + secret: { 'X-CareerPlug-Secret' => target_secret }).tap(&:save!) + build(:webhook_url, account: account, url: target_url, + events: WebhookUrl::CAREERPLUG_ACCOUNT_EVENTS, + secret: { 'X-CareerPlug-Secret' => target_secret }).tap(&:save!) + + expect { run }.not_to change(WebhookUrl, :count) + expect(WebhookUrl.where(account_id: account.id).count).to eq(2) + end + + it 'reports exactly one duplicate_warning with both webhook ids' do + account = create_owner(:account) + first = build(:webhook_url, account: account, url: target_url, + events: WebhookUrl::CAREERPLUG_ACCOUNT_EVENTS, + secret: { 'X-CareerPlug-Secret' => target_secret }).tap(&:save!) + second = build(:webhook_url, account: account, url: target_url, + events: WebhookUrl::CAREERPLUG_ACCOUNT_EVENTS, + secret: { 'X-CareerPlug-Secret' => target_secret }).tap(&:save!) + + result = run + + expect(result.duplicate_warnings.length).to eq(1) + warning = result.duplicate_warnings.first + expect(warning[:owner_class]).to eq('Account') + expect(warning[:owner_id]).to eq(account.id) + expect(warning[:webhook_ids]).to contain_exactly(first.id, second.id) + end + end + + context 'with DRY_RUN=1' do + it 'writes nothing but reports the would-be action' do + create_owner(:account) + + expect { run(dry_run: true) }.not_to change(WebhookUrl, :count) + + # Re-running for real creates it, proving the dry run did not. + result = run + expect(result.created).to eq(1) + end + end + + context 'when env vars are missing' do + it 'is a no-op that touches no rows' do + create_owner(:account) + stub_const('ENV', callback_disabled_env) # disable env AFTER owner creation, BEFORE run + + expect { run }.not_to change(WebhookUrl, :count) + + result = run + expect(result.created).to eq(0) + expect(result.updated).to eq(0) + expect(result.unchanged).to eq(0) + end + end + end + + describe 'WebhookUrl::CAREERPLUG_ACCOUNT_EVENTS' do + it 'includes submission.completed (the root-cause event this ticket fixes)' do + expect(WebhookUrl::CAREERPLUG_ACCOUNT_EVENTS).to include('submission.completed') + end + + it 'matches the events the on-create Account callback provisions' do + # The Account callback references this constant directly; this guards + # against the drift that originally caused the missing event. + expect(WebhookUrl::CAREERPLUG_ACCOUNT_EVENTS).to eq(%w[ + form.started + form.completed + submission.completed + form.changes_requested + template.preferences_updated + ]) + end + end +end