CP-14164 - Backfill + normalize ATS-pointed webhooks

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

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

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
 

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

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

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