Add Tally onboarding intake workflow#335
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Tally form webhook ingestion to the existing intake pipeline: HMAC signature verification, form-ID allowlisting, Tally-to-intake payload conversion, and job enqueueing. Introduces durable ChangesTally Intake Webhook System
Sequence Diagram(s)sequenceDiagram
participant TallyForm
participant APIHandler as tally_intake_webhook_handler
participant HMACVerify as HMAC / X-API-Secret auth
participant Allowlist as form_id allowlist
participant Worker as process_intake_form_job
participant Processor as IntakeFormProcessor
participant Postgres as onboarding_intake_submissions
participant CRM
TallyForm->>APIHandler: POST /webhooks/tally/onboarding (raw body + Tally-Signature)
APIHandler->>HMACVerify: verify signature against ONBOARDING_TALLY_WEBHOOK_SIGNING_SECRET
HMACVerify-->>APIHandler: 401 if invalid
APIHandler->>Allowlist: check formId in ONBOARDING_TALLY_ALLOWED_FORM_IDS
Allowlist-->>APIHandler: 403 if unset or unapproved
APIHandler->>APIHandler: convert Tally fields → GoogleFormsIntakePayload
APIHandler->>Worker: enqueue process_intake_form_job (tally: idempotency key)
APIHandler-->>TallyForm: 202 queued
Worker->>Processor: process_intake(payload)
alt last_name_is_placeholder
Processor->>Postgres: _persist_intake_submission (contact_id=None)
Processor-->>Worker: pending_review=True
else normal flow
Processor->>CRM: create or update prospect contact
Processor->>Postgres: _persist_intake_submission (contact_id)
Processor->>Processor: _scan_resume_content (subprocess virus scan)
Processor-->>Worker: success
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9666b38a48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adds a Tally-based onboarding intake workflow end-to-end (API webhook → queue job → worker CRM updates + durable submission storage), plus admin-dashboard configuration and onboarding-queue surfacing of the latest application details.
Changes:
- Add Tally onboarding intake webhook handler with signature verification, form allowlisting, and legacy
TALLY_*env aliases. - Persist normalized/raw intake submissions into a new
onboarding_intake_submissionstable and surface the latest submission in the onboarding queue API/UI. - Require a configured malware scan command before parsing downloaded resume files (while still accepting/storing intake data).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_worker_config.py | Adds coverage for Tally allowed-form parsing, legacy env aliases, and virus-scan timeout validation. |
| tests/unit/test_runtime_config.py | Verifies new onboarding Tally settings are dashboard-configurable (secrets + CSV allowlist). |
| tests/unit/test_intake_form_processor.py | Updates resume-processing tests for new scan gate; adds coverage for website/weekly-hours intake fields and scan-failure behavior. |
| tests/unit/test_backend_api.py | Adds unit tests for Tally webhook auth, allowlisting, signature verification, and field normalization/enqueueing. |
| packages/shared/src/five08/runtime_config.py | Introduces runtime-config definitions for onboarding Tally API key, signing secret, and allowed form IDs (with legacy env aliases). |
| docs/configuration.md | Documents new Tally intake settings and resume malware scan settings. |
| apps/worker/src/five08/worker/models.py | Adds Pydantic models for Tally webhooks and expands intake aliasing/fields for onboarding questions. |
| apps/worker/src/five08/worker/migrations/versions/20260613_0200_create_onboarding_intake_submissions.py | Creates onboarding_intake_submissions table, indexes, and updated-at trigger. |
| apps/worker/src/five08/worker/crm/intake_form_processor.py | Adds persistence of intake submissions, website link mapping, and malware scan enforcement before resume parsing. |
| apps/worker/src/five08/worker/config.py | Adds onboarding Tally settings + resume scan settings to worker configuration. |
| apps/api/src/five08/backend/api.py | Adds Tally webhook endpoint(s), signature validation, field mapping to intake payload, and onboarding queue enrichment with latest submission JSON. |
| apps/admin_dashboard/src/main.tsx | Surfaces latest intake submission summary in onboarding queue; adds signing-secret generate/copy/hide UX; adjusts config save to return success boolean. |
| .env.example | Adds example env vars for onboarding Tally intake and resume malware scan configuration. |
Comments suppressed due to low confidence (1)
apps/admin_dashboard/src/main.tsx:7483
onSavenow returns a Promise, but the Save button click handler doesn't await it or mark it as intentionally fire-and-forget. This can trigger lint warnings (and makes it easier to accidentally introduce unhandled rejections ifonSavechanges).
<Button
type="button"
size="sm"
onClick={() => onSave(item.key, draft)}
disabled={
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
tests/unit/test_backend_api.py (1)
8717-8792: ⚡ Quick winAdd signature enforcement tests for the canonical onboarding path.
Line 8738 and Line 8763 validate signatures only on
/webhooks/tally, but Line 8677 uses/webhooks/tally/onboardingfor the canonical flow. A route-specific auth regression on/webhooks/tally/onboardingwould currently slip through.Suggested test hardening
+@pytest.mark.parametrize("path", ["/webhooks/tally", "/webhooks/tally/onboarding"]) +def test_tally_intake_accepts_valid_tally_signature_for_all_routes( + client: TestClient, + monkeypatch: pytest.MonkeyPatch, + path: str, +) -> None: + monkeypatch.setattr(api.settings, "onboarding_tally_webhook_signing_secret", "signing-secret") + monkeypatch.setattr(api.settings, "onboarding_tally_allowed_form_ids", "tally-form-1") + body = json.dumps(_TALLY_INTAKE_PAYLOAD, separators=(",", ":")).encode("utf-8") + signature = base64.b64encode( + hmac.new(b"signing-secret", body, hashlib.sha256).digest() + ).decode("ascii") + + with patch("five08.backend.api.enqueue_job") as mock_enqueue: + mock_enqueue.return_value = Mock(id="job-tally-1") + response = client.post( + path, + content=body, + headers={"Content-Type": "application/json", "Tally-Signature": signature}, + ) + + assert response.status_code == 202 + mock_enqueue.assert_called_once()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_backend_api.py` around lines 8717 - 8792, Add parallel signature-enforcement tests for the canonical onboarding endpoint /webhooks/tally/onboarding: duplicate the logic from test_tally_intake_accepts_valid_tally_signature and test_tally_intake_rejects_invalid_tally_signature but target the route "/webhooks/tally/onboarding" and use the same setup of monkeypatching api.settings.onboarding_tally_webhook_signing_secret and computing/setting the Tally-Signature header (or sending an invalid signature) while asserting the expected status codes (202 for valid, 401 for invalid) and enqueue_job behavior (mock_enqueue.assert_called_once / assert_not_called). Ensure you reuse the same body payload encoding/hmac computation as in the existing tests so the verification covers the onboarding route.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/admin_dashboard/src/main.tsx`:
- Around line 7295-7304: The copyGeneratedSecret function currently calls
navigator.clipboard.writeText(secret) without handling rejections, so failing
writes reject and prevent the fallback; update copyGeneratedSecret to wrap the
writeText call in a try/catch (or handle its Promise rejection) and on error
fall back to locating the input by id `generatedSecret-${key}` and calling
select() (or log/surface an error) so the UX still works when clipboard API
fails; reference the function name copyGeneratedSecret and the DOM id pattern
generatedSecret- to locate where to add the try/catch and fallback logic.
In `@apps/api/src/five08/backend/api.py`:
- Around line 7086-7116: The handler currently enqueues only normalized_payload,
dropping the original Tally webhook body; modify the enqueue call so the
canonical/raw Tally payload is passed through to the worker (e.g., add a
raw_payload argument alongside normalized_payload in the args passed to
enqueue_job), using the original payload/tally_payload dump (preserve
aliases/None handling as needed), and keep the idempotency_key logic unchanged;
update the worker (process_intake_form_job) to expect and persist raw_payload
instead of reconstructing it from normalized_payload.
- Around line 743-752: The current _validate_tally_submission treats an empty
settings.onboarding_tally_allowed_form_ids_set as permissive; change it to fail
closed by returning JSONResponse({"error":"invalid_form_id"}, status_code=403)
when allowed_form_ids is empty or missing. In _validate_tally_submission,
reference settings.onboarding_tally_allowed_form_ids_set and
payload.data.form_id: if the set is falsy or empty, immediately return the error
response; otherwise strip payload.data.form_id and allow only when the stripped
form_id is non-empty and present in the allowed set, returning the same 403
JSONResponse for all other cases.
In
`@apps/worker/src/five08/worker/migrations/versions/20260613_0200_create_onboarding_intake_submissions.py`:
- Around line 56-61: The current
sa.UniqueConstraint("source","form_id","submission_id",
name="uq_onboarding_intake_submissions_source_form_submission") allows NULLs and
breaks UPSERTs; replace this UniqueConstraint with a database-level unique index
that normalizes NULLs using COALESCE (e.g., index on source plus
COALESCE(form_id, '') and COALESCE(submission_id, '')) in the migration (replace
the UniqueConstraint declaration in the migration creating
onboarding_intake_submissions), and update the processor SQL that uses ON
CONFLICT to match the index expressions (use ON CONFLICT (source,
COALESCE(form_id, ''), COALESCE(submission_id, '')) so conflicts fire when
form_id or submission_id are NULL).
In `@tests/unit/test_worker_config.py`:
- Around line 153-168: The test
test_legacy_tally_env_aliases_still_populate_onboarding_settings is flaky
because existing ONBOARDING_TALLY_* env vars can override the legacy TALLY_*
aliases; before setting TALLY_API_KEY, TALLY_WEBHOOK_SIGNING_SECRET, and
TALLY_ALLOWED_FORM_IDS, ensure you clear any ONBOARDING_TALLY_API_KEY,
ONBOARDING_TALLY_WEBHOOK_SIGNING_SECRET, and ONBOARDING_TALLY_ALLOWED_FORM_IDS
from the environment (use monkeypatch.delenv or equivalent) so WorkerSettings()
will read the legacy aliases and the assertions on onboarding_tally_api_key,
onboarding_tally_webhook_signing_secret, and
onboarding_tally_allowed_form_ids_set reliably validate the compatibility
behavior.
---
Nitpick comments:
In `@tests/unit/test_backend_api.py`:
- Around line 8717-8792: Add parallel signature-enforcement tests for the
canonical onboarding endpoint /webhooks/tally/onboarding: duplicate the logic
from test_tally_intake_accepts_valid_tally_signature and
test_tally_intake_rejects_invalid_tally_signature but target the route
"/webhooks/tally/onboarding" and use the same setup of monkeypatching
api.settings.onboarding_tally_webhook_signing_secret and computing/setting the
Tally-Signature header (or sending an invalid signature) while asserting the
expected status codes (202 for valid, 401 for invalid) and enqueue_job behavior
(mock_enqueue.assert_called_once / assert_not_called). Ensure you reuse the same
body payload encoding/hmac computation as in the existing tests so the
verification covers the onboarding route.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 88109ddd-b953-47eb-9d5a-668528e4f817
📒 Files selected for processing (13)
.env.exampleapps/admin_dashboard/src/main.tsxapps/api/src/five08/backend/api.pyapps/worker/src/five08/worker/config.pyapps/worker/src/five08/worker/crm/intake_form_processor.pyapps/worker/src/five08/worker/migrations/versions/20260613_0200_create_onboarding_intake_submissions.pyapps/worker/src/five08/worker/models.pydocs/configuration.mdpackages/shared/src/five08/runtime_config.pytests/unit/test_backend_api.pytests/unit/test_intake_form_processor.pytests/unit/test_runtime_config.pytests/unit/test_worker_config.py
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7e048e627
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…m-crm-onboarding # Conflicts: # apps/api/src/five08/backend/static/dashboard/.vite/manifest.json # apps/api/src/five08/backend/static/dashboard/assets/index-BOSn0NiV.js # apps/api/src/five08/backend/static/dashboard/index.html
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a4a979235
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00cf41cd0c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2173ff3d28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def _build_resume_updates( | ||
| self, | ||
| payload: Mapping[str, Any], | ||
| *, | ||
| resume_file: IntakeResumeFile | None = None, | ||
| ) -> dict[str, Any]: | ||
| if resume_file is None: | ||
| resume_file = self._prepare_resume_file(payload) | ||
| if resume_file is None: | ||
| return {} |
| if isinstance(raw_payload_candidate, Mapping): | ||
| raw_payload = dict(raw_payload_candidate) | ||
| elif isinstance(raw_tally_fields, list): | ||
| raw_payload = {"fields": raw_tally_fields} | ||
| else: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ca55be148
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if isinstance(submitted_at, datetime): | ||
| return submitted_at | ||
| created_at = latest_intake_submission.get("created_at") | ||
| if isinstance(created_at, datetime): |
There was a problem hiding this comment.
Even though the SQL now selects latest_intake_sort_at, the later merge with orphan rows sorts through this helper. For rows fetched from Postgres, latest_intake_submission is built with jsonb_build_object, so submitted_at/created_at arrive as JSON strings rather than datetime objects; these checks fall through to stale onboarding_updated_at. When a fresh Tally/Google submission matches an existing prospect and orphan rows fill the limit, that application can still be sorted below older rows and sliced out of the default queue. Parse the ISO strings here or carry the selected sort timestamp into the Python merge.
Useful? React with 👍 / 👎.
Summary
/webhooks/tally/onboardingwith signature verification, form allowlisting, and compatibility aliases for existingTALLY_*env varsonboarding_intake_submissionstable and surface latest application details in the onboarding queueNotes
Tests
./scripts/test.sh./scripts/lint.shgit diff --checkSummary by CodeRabbit
Release Notes
New Features
Documentation