Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions apps/api/src/five08/backend/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -812,22 +812,24 @@ def _tally_intake_dry_run_mode(
return "none"


def _strip_url_query(value: str) -> str:
def _strip_url_query_and_fragment(value: str) -> str:
parsed = urlsplit(value)
if parsed.scheme not in {"http", "https"} or not parsed.netloc or not parsed.query:
if parsed.scheme not in {"http", "https"} or not parsed.netloc:
return value
return urlunsplit((parsed.scheme, parsed.netloc, parsed.path, "", parsed.fragment))
if not parsed.query and not parsed.fragment:
return value
return urlunsplit((parsed.scheme, parsed.netloc, parsed.path, "", ""))


def _sanitize_tally_raw_payload(value: Any) -> Any:
def _sanitize_intake_raw_payload(value: Any) -> Any:
if isinstance(value, Mapping):
return {
str(key): _sanitize_tally_raw_payload(item) for key, item in value.items()
str(key): _sanitize_intake_raw_payload(item) for key, item in value.items()
}
if isinstance(value, list):
return [_sanitize_tally_raw_payload(item) for item in value]
return [_sanitize_intake_raw_payload(item) for item in value]
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if isinstance(value, str):
return _strip_url_query(value)
return _strip_url_query_and_fragment(value)
return value


Expand Down Expand Up @@ -7701,6 +7703,7 @@ async def google_forms_intake_webhook_handler(request: Request) -> JSONResponse:
submitted_at=payload.submitted_at,
payload=normalized_payload,
)
normalized_payload["raw_payload"] = _sanitize_intake_raw_payload(payload_data)

queue = request.app.state.queue
try:
Expand Down Expand Up @@ -7789,8 +7792,8 @@ async def tally_intake_webhook_handler(request: Request) -> JSONResponse:
"email": email,
"first_name": first_name,
"last_name": last_name,
"raw_payload": _sanitize_tally_raw_payload(payload_data),
"raw_tally_fields": _sanitize_tally_raw_payload(raw_tally_fields),
"raw_payload": _sanitize_intake_raw_payload(payload_data),
"raw_tally_fields": _sanitize_intake_raw_payload(raw_tally_fields),
}
)

Expand Down
32 changes: 23 additions & 9 deletions apps/worker/src/five08/worker/crm/intake_form_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from dataclasses import dataclass
from datetime import datetime, timezone
from pathlib import Path
from typing import Any
from typing import Any, cast
from urllib.parse import urljoin, urlsplit, urlunsplit
from uuid import NAMESPACE_URL, uuid4, uuid5

Expand Down Expand Up @@ -94,6 +94,13 @@ class IntakeResumeFile:
source_url: str


class _ResumeFileNotProvided:
pass


_RESUME_FILE_NOT_PROVIDED = _ResumeFileNotProvided()


class IntakeFormProcessor:
"""Process a Google Forms member intake submission against CRM."""

Expand Down Expand Up @@ -464,7 +471,9 @@ def _build_intake_updates(
payload: Mapping[str, Any],
include_email: bool = True,
include_last_name: bool = True,
resume_file: IntakeResumeFile | None = None,
resume_file: IntakeResumeFile | None | _ResumeFileNotProvided = (
_RESUME_FILE_NOT_PROVIDED
),
) -> dict[str, Any]:
updates: dict[str, Any] = {"firstName": first_name}
if include_last_name:
Expand Down Expand Up @@ -698,22 +707,27 @@ def _build_resume_updates(
self,
payload: Mapping[str, Any],
*,
resume_file: IntakeResumeFile | None = None,
resume_file: IntakeResumeFile | None | _ResumeFileNotProvided = (
_RESUME_FILE_NOT_PROVIDED
),
) -> dict[str, Any]:
if resume_file is None:
resume_file = self._prepare_resume_file(payload)
if resume_file is None:
prepared_resume_file: IntakeResumeFile | None
if resume_file is _RESUME_FILE_NOT_PROVIDED:
prepared_resume_file = self._prepare_resume_file(payload)
else:
prepared_resume_file = cast(IntakeResumeFile | None, resume_file)
if prepared_resume_file is None:
return {}

try:
resume_text = self.document_processor.extract_text(
resume_file.content,
resume_file.filename,
prepared_resume_file.content,
prepared_resume_file.filename,
)
except Exception as exc:
logger.warning(
"Failed to parse resume masked_url=%s error=%s",
self._mask_resume_url_for_log(resume_file.source_url),
self._mask_resume_url_for_log(prepared_resume_file.source_url),
exc,
)
return {}
Expand Down
22 changes: 19 additions & 3 deletions tests/unit/test_backend_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -9286,6 +9286,10 @@ def test_google_forms_intake_enqueues_job(
"first_name": " Jane ",
"last_name": " Doe ",
"form_id": "form-1",
"resume_url": (
"https://drive.google.com/resume.pdf?signature=secret"
"#token=fragment-secret"
),
},
headers=auth_headers,
)
Expand All @@ -9302,6 +9306,17 @@ def test_google_forms_intake_enqueues_job(
assert call_kwargs["args"][0]["email"] == "member@example.com"
assert call_kwargs["args"][0]["first_name"] == "Jane"
assert call_kwargs["args"][0]["last_name"] == "Doe"
assert call_kwargs["args"][0]["resume_url"] == (
"https://drive.google.com/resume.pdf?signature=secret#token=fragment-secret"
)
assert call_kwargs["args"][0]["raw_payload"] == {
**_GOOGLE_FORMS_INTAKE_PAYLOAD,
"email": " member@example.com ",
"first_name": " Jane ",
"last_name": " Doe ",
"form_id": "form-1",
"resume_url": "https://drive.google.com/resume.pdf",
}


def test_google_forms_intake_rejects_unapproved_form_id(
Expand Down Expand Up @@ -9637,15 +9652,16 @@ def test_tally_intake_strips_signed_urls_from_raw_payload_before_enqueue(
"""Queued raw Tally payloads should not retain signed URL query tokens."""
tally_payload = json.loads(json.dumps(_TALLY_INTAKE_PAYLOAD))
tally_payload["data"]["submissionPdfUrl"] = (
"https://tally.so/r/abc.pdf?accessToken=secret&signature=sig"
"https://tally.so/r/abc.pdf?accessToken=secret&signature=sig#token=frag"
)
tally_payload["data"]["submissionPreviewUrl"] = (
"https://tally.so/r/abc?accessToken=secret&signature=sig"
"https://tally.so/r/abc?accessToken=secret&signature=sig#token=frag"
)
for field in tally_payload["data"]["fields"]:
if field["key"] == "question_resume":
field["value"][0]["url"] = (
"https://storage.googleapis.com/tally/resume.pdf?signature=sig"
"#token=frag"
)

with (
Expand All @@ -9664,7 +9680,7 @@ def test_tally_intake_strips_signed_urls_from_raw_payload_before_enqueue(
intake_payload = mock_enqueue.call_args.kwargs["args"][0]
assert (
intake_payload["resume_url"]
== "https://storage.googleapis.com/tally/resume.pdf?signature=sig"
== "https://storage.googleapis.com/tally/resume.pdf?signature=sig#token=frag"
)
raw_payload = intake_payload["raw_payload"]
assert raw_payload["data"]["submissionPdfUrl"] == "https://tally.so/r/abc.pdf"
Expand Down
55 changes: 55 additions & 0 deletions tests/unit/test_intake_form_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,33 @@ def test_intake_form_processor_dry_run_create_reports_resume_upload_plan() -> No
mock_persist.assert_not_called()


def test_create_prospect_does_not_retry_failed_resume_prepare() -> None:
"""Create flow should not download/scan a failed resume more than once."""
processor = IntakeFormProcessor()
processor.api = MagicMock()
processor.api.request.side_effect = [
{"list": []},
{"id": "contact-1"},
]

with (
patch.object(processor, "_prepare_resume_file", return_value=None) as prepare,
patch.object(processor, "_persist_intake_submission"),
):
result = processor.process_intake(
payload={
"email": "new@example.com",
"first_name": "New",
"last_name": "Person",
"resume_url": "https://tally.so/resume.pdf",
"form_id": "form-1",
}
)

assert result["success"] is True
prepare.assert_called_once()


def test_intake_form_processor_dry_run_update_does_not_write_crm_or_db() -> None:
"""Dry-run update should return planned updates without PUT or persistence."""
processor = IntakeFormProcessor()
Expand Down Expand Up @@ -173,6 +200,34 @@ def test_intake_form_processor_dry_run_update_does_not_write_crm_or_db() -> None
mock_persist.assert_not_called()


def test_update_prospect_does_not_retry_failed_resume_prepare() -> None:
"""Update flow should not download/scan a failed resume more than once."""
processor = IntakeFormProcessor()
processor.api = MagicMock()
processor.api.request.side_effect = [
{"list": [{"id": "contact-1", "type": "Prospect"}]},
{},
]

with (
patch.object(processor, "_prepare_resume_file", return_value=None) as prepare,
patch.object(processor, "_persist_intake_submission"),
):
result = processor.process_intake(
payload={
"email": "existing@example.com",
"first_name": "Existing",
"last_name": "Person",
"github_username": "existing-dev",
"resume_url": "https://tally.so/resume.pdf",
"form_id": "form-1",
}
)

assert result["success"] is True
prepare.assert_called_once()


def test_intake_form_processor_uploads_resume_after_create() -> None:
"""Created prospects should receive the downloaded Tally resume attachment."""
processor = IntakeFormProcessor()
Expand Down