From 3da039b74e81a7cadf99e48642e6411635857553 Mon Sep 17 00:00:00 2001 From: Konrad du Plessis Date: Fri, 24 Apr 2026 13:03:12 +0200 Subject: [PATCH] =?UTF-8?q?Revert=20"feat(webhooks):=20outbound=20payslip?= =?UTF-8?q?=20webhook=20=E2=86=92=20Make.com=20/=20Zapier=20/=20n8n"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit a52d841c00ad642942dd6de7bf54373ad9ea62d6. --- CLAUDE.md | 18 --------- config/settings.py | 8 ---- core/apps.py | 11 ------ core/signals.py | 93 ---------------------------------------------- core/tests.py | 90 -------------------------------------------- requirements.txt | 3 +- 6 files changed, 1 insertion(+), 222 deletions(-) delete mode 100644 core/signals.py diff --git a/CLAUDE.md b/CLAUDE.md index 0140c77..6914778 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -724,27 +724,9 @@ EMAIL_HOST_USER # Gmail address — required for any outbound ema EMAIL_HOST_PASSWORD # Gmail App Password (16 chars, no spaces/non-breaking-space) DEFAULT_FROM_EMAIL # Optional — falls back to EMAIL_HOST_USER if unset SPARK_RECEIPT_EMAIL # Optional — defaults to FoxFitt's Spark Receipt address -WEBHOOK_PAYSLIP_URL # Optional — Make.com/Zapier/n8n "Catch Hook" URL to receive JSON on each new payslip (empty = OFF) PROJECT_DESCRIPTION, PROJECT_IMAGE_URL # Flatlogic branding ``` -### Outbound payslip webhook -When a `PayrollRecord` is created, `core/signals.py::send_payslip_webhook` -posts a small JSON summary (payslip_id, worker_name, amount_paid, -admin_url, etc.) to `WEBHOOK_PAYSLIP_URL` if the env var is set. Unset = -feature OFF; no outbound network call happens. Typical destination is a -Make.com / Zapier / n8n "Catch Hook" URL that fans the event out to -Google Sheets / Airtable / Slack / etc. without any more Python code. - -Failures are non-fatal (logged at WARNING level, payslip still saved) — -the post_save handler runs AFTER the DB transaction commits, so a dead -webhook endpoint cannot break payroll. Make.com and Zapier both handle -retry on their side; there is no in-app retry queue. - -Tests: `PayslipWebhookTests` in `core/tests.py` covers the three -important contracts (fires when configured, no-op when unset, swallows -errors) + a "doesn't refire on .save() of an existing record" guard. - ### Email fallback behaviour `DEFAULT_FROM_EMAIL` is not strictly required — `config/settings.py` sets it as: diff --git a/config/settings.py b/config/settings.py index 15a2598..1a79554 100644 --- a/config/settings.py +++ b/config/settings.py @@ -244,14 +244,6 @@ CONTACT_EMAIL_TO = [ # via env var for flexibility. Set to empty string if you want to disable sending. SPARK_RECEIPT_EMAIL = os.getenv("SPARK_RECEIPT_EMAIL", "foxfitt-ed9wc+expense@to.sparkreceipt.com") -# === OUTBOUND WEBHOOKS === -# When a PayrollRecord is created, the app posts a small JSON summary to -# WEBHOOK_PAYSLIP_URL (if set). Typical destination: a Make.com / Zapier -# / n8n "Catch Hook" URL that then fans out to Google Sheets / Airtable / -# Slack / etc. Unset = feature OFF (no network call, no behaviour change). -# See core/signals.py::send_payslip_webhook for the handler + payload shape. -WEBHOOK_PAYSLIP_URL = os.getenv("WEBHOOK_PAYSLIP_URL", "") - # Fail loudly at startup in production if credentials are missing — catches the # "I forgot to set env vars on the new deploy platform" mistake before a user # triggers a payroll payment and the email silently fails. diff --git a/core/apps.py b/core/apps.py index 7d80240..8115ae6 100644 --- a/core/apps.py +++ b/core/apps.py @@ -4,14 +4,3 @@ from django.apps import AppConfig class CoreConfig(AppConfig): default_auto_field = 'django.db.models.BigAutoField' name = 'core' - - def ready(self): - """Import signal handlers so Django's dispatch table registers them. - - `core.signals` attaches a post_save receiver to PayrollRecord that - posts a JSON summary to WEBHOOK_PAYSLIP_URL (Make.com / Zapier / n8n - destination) when a payslip is created. Importing the module here is - the standard Django idiom — the @receiver decorators run at import - time and wire themselves into the signal dispatcher. - """ - from core import signals # noqa: F401 — imported for side effects diff --git a/core/signals.py b/core/signals.py deleted file mode 100644 index 4568bf8..0000000 --- a/core/signals.py +++ /dev/null @@ -1,93 +0,0 @@ -# === core/signals.py === -# Django signal handlers that bridge in-app events to the outside world. -# -# Currently hosts ONE handler: a post_save hook on PayrollRecord that -# posts a JSON summary to settings.WEBHOOK_PAYSLIP_URL whenever a new -# payslip is created. Intended to feed Make.com / Zapier / n8n scenarios -# that fan the event out to Google Sheets, Airtable, Slack, etc. -# -# Design notes: -# - Fires on `created=True` ONLY (post_save is also called on updates; -# we don't care about those — the payslip is a new event, not a diff). -# - Feature is OFF by default: if WEBHOOK_PAYSLIP_URL is empty/unset, -# the handler short-circuits before any network call. -# - Failures are NEVER fatal. By the time post_save fires, the database -# transaction has already committed — the payslip exists regardless -# of what happens here. So we log and move on. No retry, no queue. -# If you need retry semantics, add them in the receiving service -# (Make.com and Zapier both retry failed webhook deliveries by -# default). -# - Payload is deliberately small + stable. No unbounded text fields -# (adjustment descriptions, PDFs, etc.) — those stay in the app. -# Receivers can always fetch the admin_url for the full record. -import logging - -import requests -from django.conf import settings -from django.db.models.signals import post_save -from django.dispatch import receiver - -from core.models import PayrollRecord - -logger = logging.getLogger(__name__) - -# How long to wait for the webhook endpoint to respond before giving up. -# Short enough to not block the Django worker for long; long enough that -# a slow Make.com scenario still usually succeeds. -_WEBHOOK_TIMEOUT_SECONDS = 5 - - -@receiver(post_save, sender=PayrollRecord) -def send_payslip_webhook(sender, instance, created, **kwargs): - """Post a JSON summary to WEBHOOK_PAYSLIP_URL when a PayrollRecord is created. - - Non-fatal on any failure — the payslip is already saved by the - time we get here, so a missed webhook is a notification problem, - not a data-integrity problem. - """ - # Only fire on creation, not on every save (edits, reassignments, etc.) - if not created: - return - - # Feature flag: if the env var is empty/unset, do nothing. - url = (getattr(settings, "WEBHOOK_PAYSLIP_URL", "") or "").strip() - if not url: - return - - # --- Build the payload --- - # Every key is a simple scalar (string, int, null) so it survives - # JSON round-tripping cleanly. Decimals become strings to preserve - # the "R 5420.00" look downstream (Google Sheets, Make.com etc. - # don't always round-trip Decimal precision cleanly). - payload = { - "event": "payslip.created", - "payslip_id": instance.id, - "worker_id": instance.worker_id, - "worker_name": instance.worker.name, - "amount_paid": str(instance.amount_paid), - # instance.date is the payslip date, not a wall-clock timestamp - # (PayrollRecord has no created_at field). Good enough for a - # "created on what day" marker on the receiving side. - "payslip_date": instance.date.isoformat() if instance.date else None, - # These counts require extra queries; fine at 1 payslip/second - # rates (we're never firing this in a loop). - "work_log_count": instance.work_logs.count(), - "adjustment_count": instance.adjustments.count(), # related_name on PayrollAdjustment.payroll_record FK - # Fully-qualified URL so a Slack/Sheets link works for the - # recipient without requiring them to know the host. - "admin_url": f"https://foxlog.flatlogic.app/payroll/payslip/{instance.id}/", - } - - # --- Make the network call --- - # Any exception is swallowed + logged. The bare `except Exception` - # is deliberate here: we want to catch ConnectionError, Timeout, - # SSL errors, malformed JSON responses, DNS failures, even a - # BaseException that some misconfigured middleware might raise. - # Nothing downstream of this handler depends on success. - try: - requests.post(url, json=payload, timeout=_WEBHOOK_TIMEOUT_SECONDS) - except Exception as e: # noqa: BLE001 — see comment above - logger.warning( - "Payslip webhook failed for PayrollRecord #%s: %s: %s", - instance.id, type(e).__name__, e, - ) diff --git a/core/tests.py b/core/tests.py index 317d771..28ec2d7 100644 --- a/core/tests.py +++ b/core/tests.py @@ -1379,93 +1379,3 @@ class PayrollDashboardAdjustmentAggregationTests(TestCase): "this fails with R1000 the project-attribution double-" "count bug has reappeared.", ) - - -# === PAYSLIP WEBHOOK TESTS === -# Verify the outbound webhook handler (core/signals.py) behaves correctly: -# 1. fires when WEBHOOK_PAYSLIP_URL is set, with the right payload shape -# 2. is a no-op when the env var is unset (feature OFF by default) -# 3. swallows network errors so payslip creation never fails -from unittest.mock import patch - - -class PayslipWebhookTests(TestCase): - """Outbound webhook fired on PayrollRecord creation (core/signals.py).""" - - def setUp(self): - # Minimal fixture — a worker we can issue a payslip to. - # We don't actually NEED a work log linked; PayrollRecord is creatable - # with just worker + amount_paid (date defaults to today). - self.worker = Worker.objects.create( - name='WebhookTestWorker', - id_number='WHK1', - monthly_salary=Decimal('10000'), - ) - - @patch('core.signals.requests.post') - def test_webhook_fires_when_url_configured(self, mock_post): - """If WEBHOOK_PAYSLIP_URL is set, creating a payslip triggers ONE - POST to that URL with the expected payload shape.""" - with self.settings(WEBHOOK_PAYSLIP_URL='https://example.invalid/hook/abc123'): - record = PayrollRecord.objects.create( - worker=self.worker, - amount_paid=Decimal('5420.00'), - ) - mock_post.assert_called_once() - # First positional arg = URL; payload is in the json= kwarg - call_args, call_kwargs = mock_post.call_args - self.assertEqual(call_args[0], 'https://example.invalid/hook/abc123') - payload = call_kwargs['json'] - self.assertEqual(payload['event'], 'payslip.created') - self.assertEqual(payload['payslip_id'], record.id) - self.assertEqual(payload['worker_id'], self.worker.id) - self.assertEqual(payload['worker_name'], 'WebhookTestWorker') - self.assertEqual(payload['amount_paid'], '5420.00') - self.assertEqual(payload['work_log_count'], 0) - self.assertEqual(payload['adjustment_count'], 0) - self.assertIn('/payroll/payslip/', payload['admin_url']) - - @patch('core.signals.requests.post') - def test_webhook_is_noop_when_url_unset(self, mock_post): - """With WEBHOOK_PAYSLIP_URL='' (the default), no outbound call - should be made — feature must be OFF by default so local dev and - fresh deploys don't accidentally leak to an old/stale hook URL.""" - with self.settings(WEBHOOK_PAYSLIP_URL=''): - PayrollRecord.objects.create( - worker=self.worker, - amount_paid=Decimal('100.00'), - ) - mock_post.assert_not_called() - - @patch( - 'core.signals.requests.post', - side_effect=ConnectionError('simulated network failure'), - ) - def test_webhook_failure_does_not_break_save(self, mock_post): - """If the webhook endpoint is down/slow/unreachable, the PayrollRecord - must still be created successfully. The webhook is a notification, - not a transactional dependency — we log the failure and move on.""" - with self.settings(WEBHOOK_PAYSLIP_URL='https://example.invalid/hook/abc123'): - # Must NOT raise — save completes, returning a real record - record = PayrollRecord.objects.create( - worker=self.worker, - amount_paid=Decimal('250.00'), - ) - self.assertIsNotNone(record.id) - self.assertEqual(record.amount_paid, Decimal('250.00')) - mock_post.assert_called_once() # The call WAS attempted, just failed - - @patch('core.signals.requests.post') - def test_webhook_does_not_fire_on_update(self, mock_post): - """post_save fires on every save, but the handler should only - fire on creation (created=True). Confirm a subsequent .save() - doesn't double-post.""" - with self.settings(WEBHOOK_PAYSLIP_URL='https://example.invalid/hook/abc123'): - record = PayrollRecord.objects.create( - worker=self.worker, - amount_paid=Decimal('100.00'), - ) - self.assertEqual(mock_post.call_count, 1) - # Re-save with no changes — post_save fires but created=False - record.save() - self.assertEqual(mock_post.call_count, 1, 'Updates must NOT refire the webhook') diff --git a/requirements.txt b/requirements.txt index a63567c..c24208f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,5 +3,4 @@ mysqlclient==2.2.7 python-dotenv==1.1.1 pillow==12.1.1 weasyprint==68.1 -django-debug-toolbar==6.0.0 # dev-only — gated in config/settings.py, never active in prod -requests>=2.32.0 \ No newline at end of file +django-debug-toolbar==6.0.0 # dev-only — gated in config/settings.py, never active in prod \ No newline at end of file