diff --git a/CLAUDE.md b/CLAUDE.md index d6c9d19..be6d1fd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -134,6 +134,15 @@ Two scripts live in `scripts/` for diagnosing and backfilling team rosters on `/ - `cleanup_bogus_imported_users.py [--event-id ] [--apply]` — finds and removes the user docs left behind by the older off-by-one `parse_projects` bug. Fingerprint: `imported=True` AND `propel_id=""` AND `email_address` present but not a valid email AND `import_source` starts with `projects-`. For each matched user it prunes the doc-ref from every team's `users[]` that references it, then deletes the user doc. Dry-run by default. After running, re-run `import_hackathon_users_from_csv.py --csv-type projects` against the affected events to import the real members. - `backfill_devpost_winners.py --event-id --devpost-url [--projects-csv ] [--apply]` — scrapes the Devpost project gallery for EVERY project tile, flagging winners (`aside.entry-badge img.winner`). For each project it matches to a Firestore team via a layered strategy: `teams.devpost_link` exact-URL → team name (case-insensitive) → email-overlap via Devpost projects CSV (auto-discovered from `/tmp/devpost_files//projects-*.csv`). Two backfills happen in one pass: (1) any matched team with an empty `devpost_link` gets the gallery URL written; (2) matched WINNERS additionally get `/software/` fetched for prize text + member names, with prize strings mapped to status — "1st place" → `FOUNDING_ENGINEERS`, "Completion" or "2nd place" → `COMPLETION_SUPPORT`, anything else marked Winner → `CATEGORY_WINNER` (rank-based; multi-prize teams get the best status, all prize text retained in `awards: []`). Conflicts (team already has a different `devpost_link`) are logged but never overwritten. Unmatched winners exit with code 2 so a human notices; unmatched non-winners are listed for visibility but don't fail the run (typical for teams that registered only on Devpost). Only sets `status`, `awards`, `winners_backfilled_at/source`, and `devpost_link`; never touches `users[]`. Re-runnable. Adds `beautifulsoup4` to requirements. +## Resend API constraints +`GET /emails` (list) supports ONLY `limit` (max 100)/`after`/`before` — no recipient or date filter; results newest-first. Per-recipient status = `Emails.get(id)` using the `resend_id` stored in volunteer `sent_emails`, or webhooks. Rate limit is low single-digit req/s per team and shared with email sending. + +### Resend status architecture (implemented 2026-06-10) +- **Primary path**: `GET /api/admin/emails/resend-status` — per-ID `Emails.get` with Redis cache (`resend:status:{id}`). Terminal events (`delivered`, `bounced`, `complained`, etc.) TTL 7 days; transient (`sent`, `queued`, etc.) TTL 120 s. Cap 100 IDs, 0.3 s throttle between calls. +- **List crawl**: `POST /api/admin/emails/resend-list` — never blocks on page load. Always returns from cache immediately. Pass `force: true` to kick a background crawl. Date-bounded to 90 days; max 30 pages; 0.5 s between pages. Cache TTL 15 min fresh / 1 hour stale. +- **Admin UI**: on volunteer load, bulk-fetches IDs from `sent_emails` via the status endpoint. "Sync from Resend" button sends `force: true` and shows a 30 s snackbar if the sync is still running. +- **Confirmation email tracking**: `send_volunteer_confirmation_email` now accepts `volunteer_id` and appends a `sent_emails` record with `recipient_type: 'application_confirmation'` after sending. + ## Resend audience sync `scripts/sync_resend_audience.py --source {all|profiles|volunteers|mentors|judges|sponsors|helpers|leads} --audience "" [--event-id ] [--selected-only] [--apply]` — pulls emails from Firestore (`users.email_address`, `volunteers.email` filtered by `volunteer_type`, `leads.email`) and upserts contacts into a Resend audience (creates if missing). Dry-run by default. Re-runnable: lists existing audience contacts first and only POSTs new emails. Needs `RESEND_API_KEY` with audiences scope — the existing `RESEND_WELCOME_EMAIL_KEY` is send-only and will 401. Uses the deprecated `resend.Audiences` SDK class (now an alias for Segments) — fine for now, but if it breaks switch to `resend.Segments`. diff --git a/api/messages/messages_service.py b/api/messages/messages_service.py index 5c8cbb2..98e9eb4 100644 --- a/api/messages/messages_service.py +++ b/api/messages/messages_service.py @@ -668,6 +668,7 @@ def upload_image_to_cdn(request): logger.debug(f"Saving file to temporary location: {temp_filepath}") file.save(temp_filepath) + _optimize_image_for_web(temp_filepath) # Get just the file name without the path as the destination destination_filename = os.path.basename(temp_filepath) @@ -724,6 +725,7 @@ def upload_image_to_cdn(request): logger.debug(f"Saving base64 image to temporary file: {temp_filepath}") with open(temp_filepath, 'wb') as temp_file: temp_file.write(image_data) + _optimize_image_for_web(temp_filepath) # Get just the file name without the path as the destination destination_filename = os.path.basename(temp_filepath) @@ -769,6 +771,7 @@ def upload_image_to_cdn(request): logger.debug(f"Saving binary image to temporary file: {temp_filepath}") with open(temp_filepath, 'wb') as temp_file: temp_file.write(binary_data) + _optimize_image_for_web(temp_filepath) # Get just the file name without the path as the destination destination_filename = os.path.basename(temp_filepath) @@ -807,6 +810,7 @@ def upload_image_to_cdn(request): logger.debug(f"Saving raw image to temporary file: {temp_filepath}") with open(temp_filepath, 'wb') as temp_file: temp_file.write(image_data) + _optimize_image_for_web(temp_filepath) # Get just the file name without the path as the destination destination_filename = os.path.basename(temp_filepath) @@ -845,3 +849,41 @@ def _is_image_file(filename): logger.debug(f"File extension check for {filename}: {'valid' if is_image else 'invalid'} image file") return is_image + +def _optimize_image_for_web(filepath: str, max_dimension: int = 2048, jpeg_quality: int = 85) -> None: + """Resize and compress an image in-place when it exceeds web-friendly limits. + + Uses Pillow (already in requirements.txt). Skips files that are already + small (< 500 KB) and within max_dimension on both axes. On failure the + original file is left untouched so the upload can still proceed. + """ + from PIL import Image, ImageOps + try: + original_size = os.path.getsize(filepath) + with Image.open(filepath) as _raw: + # Apply EXIF orientation before anything else so portrait photos + # from phones (which store pixels sideways + an EXIF rotation tag) + # are not saved rotated after the tag is stripped on re-save. + img = ImageOps.exif_transpose(_raw) + needs_resize = img.width > max_dimension or img.height > max_dimension + if not needs_resize and original_size < 500_000: + return # already web-friendly + + if needs_resize: + img.thumbnail((max_dimension, max_dimension), Image.LANCZOS) + + ext = os.path.splitext(filepath)[1].lower() + if ext in ('.jpg', '.jpeg'): + if img.mode in ('RGBA', 'P', 'LA'): + img = img.convert('RGB') + img.save(filepath, format='JPEG', quality=jpeg_quality, optimize=True) + else: + img.save(filepath, optimize=True) + + new_size = os.path.getsize(filepath) + logger.info( + f"_optimize_image_for_web: {original_size // 1024}KB → {new_size // 1024}KB" + ) + except Exception as exc: + logger.warning(f"_optimize_image_for_web failed for {filepath}, uploading original: {exc}") + diff --git a/api/volunteers/volunteers_views.py b/api/volunteers/volunteers_views.py index 7da1401..c8a97af 100644 --- a/api/volunteers/volunteers_views.py +++ b/api/volunteers/volunteers_views.py @@ -817,9 +817,11 @@ def admin_list_resend_emails(): else: filter_emails = None - logger.info("Listing Resend emails, filter_count=%d", len(filter_emails) if filter_emails else 0) + force = bool(body.get('force', False)) + logger.info("Listing Resend emails, filter_count=%d, force=%s", + len(filter_emails) if filter_emails else 0, force) - result = list_all_resend_emails(filter_emails=filter_emails) + result = list_all_resend_emails(filter_emails=filter_emails, force=force) if result['success']: return _success_response(result, "Resend email list fetched successfully") diff --git a/common/utils/redis_cache.py b/common/utils/redis_cache.py index 2eaa772..627b7f7 100644 --- a/common/utils/redis_cache.py +++ b/common/utils/redis_cache.py @@ -1,34 +1,102 @@ import os -from typing import Any, Optional, Callable, TypeVar import json from functools import wraps +from typing import Any, Callable, TypeVar +from urllib.parse import urlparse, urlunparse + from cachetools import TTLCache +from common.log import get_logger, info, warning + T = TypeVar('T') +logger = get_logger("redis_cache") + +LOCAL_CACHE_MAXSIZE = 1000 +LOCAL_CACHE_TTL_SECONDS = 600 + + +def _redact_redis_url(redis_url: str) -> str: + """Remove credentials from Redis URLs before logging them.""" + parsed = urlparse(redis_url) + if not parsed.netloc or "@" not in parsed.netloc: + return redis_url + + user_info, host_info = parsed.netloc.rsplit("@", 1) + username = user_info.split(":", 1)[0] if ":" in user_info else user_info + redacted_user_info = f"{username}:***" if username else "***" + return urlunparse(parsed._replace(netloc=f"{redacted_user_info}@{host_info}")) + + +def _disable_redis(operation: str, exc: Exception) -> None: + """Disable Redis after a runtime failure and fall back to local cache.""" + global REDIS_ENABLED, REDIS_CLIENT + + if REDIS_ENABLED: + warning( + logger, + "Redis cache operation failed; falling back to local TTL cache", + operation=operation, + error=str(exc), + ) + + REDIS_ENABLED = False + REDIS_CLIENT = None + # Check if Redis is available and import if it is REDIS_ENABLED = False REDIS_CLIENT = None +redis_url = os.environ.get('REDIS_URL') + +if redis_url: + try: + import redis + from redis.exceptions import RedisError -try: - import redis - from redis.exceptions import RedisError - - # Get Redis URL from environment (Fly.io provides this for Redis instances) - redis_url = os.environ.get('REDIS_URL') - - if redis_url: REDIS_CLIENT = redis.from_url(redis_url) # Test the connection REDIS_CLIENT.ping() REDIS_ENABLED = True -except (ImportError, RedisError, Exception): - # Either redis is not installed or connection failed - # We'll fall back to local caching - pass + info( + logger, + "Redis cache enabled", + cache_backend="redis", + redis_url=_redact_redis_url(redis_url), + ) + except ImportError as exc: + warning( + logger, + "Redis client import failed; using local TTL cache", + cache_backend="local_ttl", + redis_url=_redact_redis_url(redis_url), + error=str(exc), + ) + except RedisError as exc: + warning( + logger, + "Redis connection failed during startup; using local TTL cache", + cache_backend="local_ttl", + redis_url=_redact_redis_url(redis_url), + error=str(exc), + ) + except Exception as exc: + warning( + logger, + "Unexpected Redis startup failure; using local TTL cache", + cache_backend="local_ttl", + redis_url=_redact_redis_url(redis_url), + error=str(exc), + ) +else: + info( + logger, + "Redis cache disabled; REDIS_URL not configured, using local TTL cache", + cache_backend="local_ttl", + local_ttl_seconds=LOCAL_CACHE_TTL_SECONDS, + ) # Fallback local cache with 10 minute TTL -local_cache = TTLCache(maxsize=1000, ttl=600) +local_cache = TTLCache(maxsize=LOCAL_CACHE_MAXSIZE, ttl=LOCAL_CACHE_TTL_SECONDS) def cache_key(*args, **kwargs) -> str: """Generate a consistent cache key from arguments.""" @@ -54,9 +122,8 @@ def get_cached(key: str, default: Any = None) -> Any: if value: return json.loads(value) return default - except Exception: - # Fall back to local cache if Redis fails - pass + except Exception as exc: + _disable_redis("get", exc) return local_cache.get(key, default) @@ -80,9 +147,8 @@ def set_cached(key: str, value: Any, ttl: int = 600) -> bool: try: REDIS_CLIENT.setex(key, ttl, json_value) return True - except Exception: - # Fall back to local cache if Redis fails - pass + except Exception as exc: + _disable_redis("set", exc) # Use local cache local_cache[key] = value @@ -104,8 +170,8 @@ def delete_cached(key: str) -> bool: if REDIS_ENABLED: try: REDIS_CLIENT.delete(key) - except Exception: - pass + except Exception as exc: + _disable_redis("delete", exc) # Also remove from local cache if key in local_cache: @@ -131,8 +197,8 @@ def clear_pattern(pattern: str) -> bool: keys = REDIS_CLIENT.keys(pattern) if keys: REDIS_CLIENT.delete(*keys) - except Exception: - pass + except Exception as exc: + _disable_redis("clear_pattern", exc) # For local cache, scan and remove matching keys keys_to_delete = [k for k in local_cache if k.startswith(pattern)] diff --git a/common/utils/validators.py b/common/utils/validators.py index 36f4174..4bb505f 100644 --- a/common/utils/validators.py +++ b/common/utils/validators.py @@ -376,11 +376,11 @@ def validate_meals(meals): MAX_EVENT_PHOTOS = 100 MAX_SOCIAL_POSTS = 25 -ALLOWED_SOCIAL_PLATFORMS = {"linkedin", "instagram", "threads"} +ALLOWED_SOCIAL_PLATFORMS = {"linkedin", "instagram", "threads", "article"} # article = freeform URL with no platform-host restrictions, SOCIAL_PLATFORM_HOSTS = { "linkedin": ("linkedin.com",), "instagram": ("instagram.com",), - "threads": ("threads.net", "threads.com"), + "threads": ("threads.net", "threads.com"), } @@ -424,11 +424,13 @@ def validate_social_posts(posts): raise ValueError(f"social_posts[{i}].url must be a valid URL") host = (urlparse(url).netloc or "").lower() host = host[4:] if host.startswith("www.") else host - allowed_hosts = SOCIAL_PLATFORM_HOSTS[platform] - if not any(host == h or host.endswith("." + h) for h in allowed_hosts): - raise ValueError( - f"social_posts[{i}].url host must match platform '{platform}' ({allowed_hosts})" - ) + # For non-article platforms, enforce that the URL's host matches the expected platform hosts. + if platform != "article": + allowed_hosts = SOCIAL_PLATFORM_HOSTS.get(platform, ()) + if not any(host == h or host.endswith("." + h) for h in allowed_hosts): + raise ValueError( + f"social_posts[{i}].url host must match platform '{platform}' ({allowed_hosts})" + ) caption = post.get("caption") if caption is not None and not isinstance(caption, str): raise ValueError(f"social_posts[{i}].caption must be a string") diff --git a/docs/resend-status-optimization-plan.md b/docs/resend-status-optimization-plan.md new file mode 100644 index 0000000..e39e8d8 --- /dev/null +++ b/docs/resend-status-optimization-plan.md @@ -0,0 +1,107 @@ +# Plan: Stop full Resend email sync on volunteer admin page load + +## Problem +`services/volunteers_service.py:_fetch_and_cache_resend_emails()` crawls the ENTIRE Resend +account (up to 100 pages × 100 emails) whenever the 5-min freshness flag expires and an admin +loads `/admin/volunteers`. The frontend (`frontend-ohack.dev/src/components/admin/VolunteerTable.js`) +eagerly POSTs all volunteer emails to `/api/admin/emails/resend-list` on mount, but the backend +ignores that filter for fetching — it's applied AFTER crawling everything. First uncached request +blocks for the whole crawl. Rate limit is shared with actual email sending. + +## Hard API constraint (verified June 2026, resend SDK 2.22.0) +Resend's List Emails API (`GET /emails`) supports ONLY `limit` (max 100, default 10), `after`, +`before` cursors. **There is no recipient/`to` filter, no date filter.** Results are newest-first. +So "fetch only emails for users on the page" is impossible via the list endpoint. The per-recipient +primitives available are: +1. `GET /emails/{id}` (`resend.Emails.get`) — works because we already store `resend_id` per send. +2. Webhooks (`email.delivered/bounced/complained/...`, svix-signed) — push, no polling. +3. List crawl — only way to discover emails whose ids we never recorded. + +Rate limit: low single-digit req/s per team (docs say default ~2–5 rps, shared across keys and +with email sending). 429 on exceed. Throttle accordingly. + +## Existing data we already have (use it) +- Volunteer docs have `sent_emails: [{resend_id, subject, timestamp, sent_by, recipient_type}]` + for all admin-composed sends (volunteers_service.py ~2394–2428, ~2545–2601). The frontend + already receives these in the volunteers payload (`getSentEmails` in VolunteerTable.js). +- The bulk list adds only: (a) live `last_event` for stored ids, (b) emails sent by paths that + don't record ids (e.g. `send_confirmation_email` ~line 304, certificates, contact). +- `last_event` values delivered/bounced/complained/failed/canceled are TERMINAL — cache forever. + `sent`/`delivery_delayed`/`queued` are transient — short TTL. + +## Confirmed design (decided 2026-06-10) +1. DB-first: render each volunteer's email list from Firestore `sent_emails` (already in the + volunteers payload) — zero Resend calls. +2. Hydrate delivery status (`last_event`) via per-ID `Emails.get` behind the existing + `/api/admin/emails/resend-status` endpoint, Redis-cached per id. +3. Full Resend list crawl becomes a MANUAL sync (admin button), backend-cached (Redis SWR), + background-only, date-bounded. It exists only to surface emails with no stored resend_id + (legacy `messages_sent`, application confirmation emails). + +## Phase 1 — make per-ID status the primary path; stop eager full sync + +### Backend: `services/volunteers_service.py` +1. Rework `get_resend_email_statuses(email_ids)`: + - Per-id Redis cache (`common/utils/redis_cache.py` get_cached/set_cached), key + `resend:status:{id}`. Terminal `last_event` → TTL 7 days; non-terminal → TTL 120 s. + - Only call `resend.Emails.get` for cache misses; `time.sleep(0.3)` between API calls; + on a 429/exception, retry once with backoff then mark that id `last_event: 'unknown'` + WITHOUT caching the failure. Keep the request cap (raise to 100 ids; the frontend chunks). +2. Rework `list_all_resend_emails` (keep route + SWR cache keys): + - NEVER fetch inline. If cache empty → set lock, start the existing background thread, + return `{'success': True, 'emails_by_recipient': {}, 'total_fetched': 0, 'syncing': True}`. + - Accept `force: true` in the POST body (manual sync button): if no refresh lock is held, + start a background refresh regardless of the fresh flag; always respond immediately with + current cached data + `syncing` flag. Cache stays shared across all admins. + - Date-bound the crawl in `_fetch_and_cache_resend_emails`: stop paginating when the last + item of a page has `created_at` older than a cutoff (param `since_days`, default 90). + Results are newest-first so this is safe. Lower `max_pages` 100 → 30. Add + `time.sleep(0.5)` between pages. + - Bump `_RESEND_FRESH_TTL` 300 → 900 (it becomes on-demand, not per-page-load). + +### Frontend: `frontend-ohack.dev/src/components/admin/VolunteerTable.js` +3. Remove the eager `useEffect` that calls `fetchResendEmailList()` when volunteers load + (~line 352). Keep the function. +4. New effect: when volunteers load, collect unique `resend_id`s from `getSentEmails(v)` across + all rows (table is unpaginated — all rows render), POST to existing + `/api/admin/emails/resend-status` in chunks of ≤100 via the existing `fetchResendStatuses` + (it already dedupes against state). Delivery chips then render from `resendStatuses` — this + covers every tracked email with mostly Redis cache hits. +5. Keep `fetchResendEmailList` behind an explicit small "Sync from Resend" refresh button + (e.g. next to the checked-in filter), sending `force: true`, for discovering untracked + emails (confirmation emails etc.). If response has `syncing: true`, show a snackbar + "Sync started — refreshing in ~30s" and re-fetch once (without force) after ~30 s. + Keep tooltip `onOpen` per-id fetch as-is (now cheap due to caching). + +### Optional later upgrade (skip for now) +When `resend-status` sees a terminal event, write `last_event`/`last_event_at` back into the +matching volunteer `sent_emails` entry in Firestore so status ships with the volunteers payload +and that id never hits Resend again. Needs an id→volunteer mapping in the request (Firestore +can't query inside arrays of maps by one field) and adds writes — Redis caching already captures +most of the win, so defer. + +## Phase 2 — record resend_id at send time (closes most of the gap) +- `send_confirmation_email` (volunteers_service.py ~304): `resend.Emails.send()` returns + `{'id': ...}`; capture it and append a `sent_emails` record + (`recipient_type: 'application_confirmation'`) to the volunteer doc right after creation/update. + Mirror the existing tracking pattern at ~2394. Then per-ID status covers ~everything shown on + this page and the list crawl becomes a rarely-used backfill. + +## Phase 3 (optional end state) — Resend webhooks, zero polling +- New blueprint per CLAUDE.md pattern: `api/resend_webhooks/resend_webhooks_views.py` + + service. `POST /api/resend/webhook`, svix signature verification (headers svix-id, + svix-timestamp, svix-signature; secret env `RESEND_WEBHOOK_SECRET`; add `svix` to + requirements.txt or implement the HMAC-SHA256 check manually — payload is + `{msg_id}.{timestamp}.{body}` signed with the base64 secret). +- On `email.*` events: write `resend:status:{id}` to Redis (terminal TTL 7d) and optionally + update the matching volunteer `sent_emails` entry. Read path then never calls Resend. +- Requires registering the endpoint in the Resend dashboard (manual step — note in PR). + +## Tests + verification +- Tests in `api/volunteers/tests/` or `test/`; pre-mock `resend`, `db.*`, `common.utils.*` in + `sys.modules` per CLAUDE.md testing notes. Cover: terminal-vs-transient TTL choice, + cache-hit skips API call, date-bounded pagination stop, `syncing: True` response when cold. +- Python 3.9: use `Optional[X]`, never `X | None`. +- Boot check: `source /opt/homebrew/Caskroom/miniconda/base/etc/profile.d/conda.sh && conda + activate py39_ohack_backend && python -c "import api"` (or `flask run` smoke) before claiming done. +- Frontend: `npm run build` in `../frontend-ohack.dev`. diff --git a/services/hackathons_service.py b/services/hackathons_service.py index 790ab46..46e6bae 100644 --- a/services/hackathons_service.py +++ b/services/hackathons_service.py @@ -502,6 +502,53 @@ def get_hackathon_funnel_aggregate(): return result +def _enrich_teams_users_batch(teams, db): + """ + Replace users[] on every team dict with slim profile dicts in a single + batched Firestore get_all call (one round-trip for all teams combined). + Mirrors services/teams_service.py::_enrich_team_users but operates on a + list so the caller doesn't pay N round-trips. + """ + uid_to_refs = {} # user_id -> DocumentReference (deduped) + team_user_ids = [] # parallel to teams: list of user_id lists per team + for team in teams: + if team is None: + team_user_ids.append([]) + continue + uids = [u for u in (team.get("users") or []) if isinstance(u, str)] + team_user_ids.append(uids) + for uid in uids: + if uid not in uid_to_refs: + uid_to_refs[uid] = db.collection("users").document(uid) + + if not uid_to_refs: + return teams + + try: + snapshots = db.get_all(list(uid_to_refs.values())) + snap_by_id = {} + for snap in snapshots: + if snap.exists: + d = snap.to_dict() or {} + snap_by_id[snap.id] = { + "id": snap.id, + "user_id": d.get("user_id"), + "name": d.get("name"), + "nickname": d.get("nickname"), + "profile_image": d.get("profile_image"), + } + else: + snap_by_id[snap.id] = {"id": snap.id, "user_id": None, "name": None, "nickname": None, "profile_image": None} + except Exception as e: + logger.warning(f"_enrich_teams_users_batch get_all failed, leaving user ids in place: {e}") + return teams + + for team, uids in zip(teams, team_user_ids): + team["users"] = [snap_by_id.get(uid, {"id": uid, "user_id": None, "name": None, "nickname": None, "profile_image": None}) for uid in uids] + + return teams + + @cached(cache=TTLCache(maxsize=100, ttl=600)) @limits(calls=2000, period=ONE_MINUTE) def get_single_hackathon_event(hackathon_id): @@ -517,7 +564,8 @@ def get_single_hackathon_event(hackathon_id): else: result["nonprofits"] = [] if "teams" in result and result["teams"]: - result["teams"] = [doc_to_json(doc=team, docid=team.id) for team in result["teams"]] + teams = [t for t in (doc_to_json(doc=team, docid=team.id) for team in result["teams"]) if t is not None] + result["teams"] = _enrich_teams_users_batch(teams, _get_db()) else: result["teams"] = [] diff --git a/services/teams_service.py b/services/teams_service.py index 36a47e1..2cb0989 100644 --- a/services/teams_service.py +++ b/services/teams_service.py @@ -204,11 +204,10 @@ def get_teams_batch(json): def save_team(propel_user_id, json): send_slack_audit(action="save_team", message="Saving", payload=json) - email, user_id, last_login, profile_image, name, nickname = get_propel_user_details_by_id(propel_user_id) - slack_user_id = user_id + email, slack_user_id, last_login, profile_image, name, nickname = get_propel_user_details_by_id(propel_user_id) - root_slack_user_id = extract_slack_user_id(slack_user_id) - user = get_user_doc_reference(root_slack_user_id) + slack_member_id = extract_slack_user_id(slack_user_id) + user = get_user_doc_reference(slack_user_id) db = get_db() logger.debug("Team Save") @@ -304,7 +303,7 @@ def save_team(propel_user_id, json): *Channel:* #{team_slack_channel} *Nonprofit:* <{nonprofit_url}|{nonprofit_name}> *Project:* <{project_url}|{raw_problem_statement_title}> -*Created by:* <@{root_slack_user_id}> (add your other team members here) +*Created by:* <@{slack_member_id}> (add your other team members here) :github_parrot: *GitHub Repository:* {repo['full_url']} All code goes here! Remember, we're building for the public good (MIT license). @@ -410,7 +409,18 @@ def join_team(propel_user_id, json): db = get_db() slack_user = get_slack_user_from_propel_user_id(propel_user_id) - userid = get_user_from_slack_id(slack_user["sub"]).id + if not slack_user or "sub" not in slack_user: + logger.error(f"Could not resolve Slack user for propel_user_id={propel_user_id}") + return Message("Error: User not found") + + slack_user_id = slack_user["sub"] + slack_member_id = extract_slack_user_id(slack_user_id) + user = get_user_from_slack_id(slack_user_id) + if user is None: + logger.error(f"Could not resolve database user for slack_user_id={slack_user_id}") + return Message("Error: User not found") + + userid = user.id team_ref = db.collection('teams').document(team_id) user_ref = db.collection('users').document(userid) @@ -450,8 +460,10 @@ def update_team_and_user(transaction): if success: send_slack_audit(action="join_team", message="Added", payload=json) message = "Joined Team" - if team_slack_channel: - invite_user_to_channel(userid, team_slack_channel) + if team_slack_channel and slack_user_id: + invite_user_to_channel(slack_user_id, team_slack_channel) + # Send a simple message that pings the user in the channel to let them know they were added + send_slack(f"<@{slack_member_id}> has joined the team!", team_slack_channel) else: message = "User was already in the team" except Exception as e: @@ -471,7 +483,18 @@ def unjoin_team(propel_user_id, json): db = get_db() slack_user = get_slack_user_from_propel_user_id(propel_user_id) - userid = get_user_from_slack_id(slack_user["sub"]).id + if not slack_user or "sub" not in slack_user: + logger.error(f"Could not resolve Slack user for propel_user_id={propel_user_id}") + return Message("Error: User not found") + + slack_user_id = slack_user["sub"] + slack_member_id = extract_slack_user_id(slack_user_id) + user = get_user_from_slack_id(slack_user_id) + if user is None: + logger.error(f"Could not resolve database user for slack_user_id={slack_user_id}") + return Message("Error: User not found") + + userid = user.id team_ref = db.collection('teams').document(team_id) user_ref = db.collection('users').document(userid) @@ -492,6 +515,8 @@ def update_team_and_user(transaction): user_list = team_data.get("users", []) user_teams = user_data.get("teams", []) + team_slack_channel = team_data.get("slack_channel") + if user_ref not in user_list: logger.warning(f"User {userid} not found in team {team_id}") return False @@ -502,6 +527,9 @@ def update_team_and_user(transaction): transaction.update(team_ref, {"users": new_user_list}) transaction.update(user_ref, {"teams": new_user_teams}) + if team_slack_channel and slack_member_id: + send_slack(f"<@{slack_member_id}> has left the team.", team_slack_channel) + logger.debug(f"User {userid} removed from team {team_id}") return True @@ -510,7 +538,7 @@ def update_team_and_user(transaction): success = update_team_and_user(transaction) if success: send_slack_audit(action="unjoin_team", message="Removed", payload=json) - message = "Removed from Team" + message = "Removed from Team" else: message = "User was not in the team" except Exception as e: diff --git a/services/volunteers_service.py b/services/volunteers_service.py index 86e1fc5..72d86ba 100644 --- a/services/volunteers_service.py +++ b/services/volunteers_service.py @@ -2,7 +2,7 @@ import uuid import time import threading -from datetime import datetime +from datetime import datetime, timedelta import pytz from functools import lru_cache from ratelimiter import RateLimiter @@ -179,9 +179,9 @@ def verify_recaptcha(token: str) -> bool: exception(logger, "Error verifying recaptcha", exc_info=e) return False -def send_volunteer_confirmation_email(first_name: str, last_name: str, email: str, volunteer_type: str, - calendar_attachments: Optional[List[Dict[str, Any]]] = None, event_id: Optional[str] = None - ) -> bool: +def send_volunteer_confirmation_email(first_name: str, last_name: str, email: str, volunteer_type: str, + calendar_attachments: Optional[List[Dict[str, Any]]] = None, event_id: Optional[str] = None, + volunteer_id: Optional[str] = None) -> Optional[str]: """ Send a confirmation email to the volunteer who submitted the form. @@ -301,12 +301,33 @@ def send_volunteer_confirmation_email(first_name: str, last_name: str, email: st if calendar_attachments and len(calendar_attachments) > 0: params["attachments"] = calendar_attachments - resend.Emails.send(params) - info(logger, "Sent confirmation email to volunteer", email=email, attachment_count=len(calendar_attachments) if calendar_attachments else 0) - return True + email_result = resend.Emails.send(params) + resend_id = email_result.get('id') if isinstance(email_result, dict) else getattr(email_result, 'id', None) + info(logger, "Sent confirmation email to volunteer", email=email, + attachment_count=len(calendar_attachments) if calendar_attachments else 0, + resend_id=resend_id) + if resend_id and volunteer_id: + try: + subject = params.get('subject', '') + sent_record = { + 'resend_id': resend_id, + 'subject': subject, + 'timestamp': _get_current_timestamp(), + 'sent_by': 'system', + 'recipient_type': 'application_confirmation', + } + get_db().collection('volunteers').document(volunteer_id).update( + {'sent_emails': firestore.ArrayUnion([sent_record])} + ) + info(logger, "Tracked confirmation email in sent_emails", + volunteer_id=volunteer_id, resend_id=resend_id) + except Exception as track_err: + warning(logger, "Failed to track confirmation email in sent_emails", + exc_info=track_err, volunteer_id=volunteer_id) + return resend_id except Exception as e: exception(logger, "Error sending email via Resend", exc_info=e, email=email) - return False + return None def send_admin_notification_email(volunteer_data: Dict[str, Any], is_update: bool = False) -> bool: """ @@ -931,7 +952,7 @@ def create_or_update_volunteer( else: warning(logger, "No calendar attachments generated despite availability data", email=email) - send_volunteer_confirmation_email(first_name, last_name, email, volunteer_type, calendar_attachments, event_id) + send_volunteer_confirmation_email(first_name, last_name, email, volunteer_type, calendar_attachments, event_id, volunteer_id=volunteer_id) info(logger, "Sent notifications about updated volunteer", email=email) except Exception as e: @@ -1004,7 +1025,7 @@ def create_or_update_volunteer( else: warning(logger, "No calendar attachments generated despite availability data", email=email) - send_volunteer_confirmation_email(first_name, last_name, email, volunteer_type, calendar_attachments, event_id) + send_volunteer_confirmation_email(first_name, last_name, email, volunteer_type, calendar_attachments, event_id, volunteer_id=volunteer_id) send_admin_notification_email(volunteer_doc) send_slack_volunteer_notification(volunteer_doc) info(logger, "Sent notifications for new volunteer", email=email) @@ -2653,12 +2674,7 @@ def send_email_to_address( def get_resend_email_statuses(email_ids: list) -> Dict[str, Any]: """ Fetch delivery status for a list of Resend email IDs. - - Args: - email_ids: List of Resend email IDs to look up - - Returns: - Dict with 'statuses' mapping each email ID to its Resend status + Results are cached per-id: terminal events for 7 days, transient for 2 minutes. """ try: resend.api_key = os.environ.get('RESEND_EMAIL_STATUS_KEY') @@ -2666,24 +2682,47 @@ def get_resend_email_statuses(email_ids: list) -> Dict[str, Any]: return {'success': False, 'error': 'Resend API key not configured'} statuses = {} - for eid in email_ids[:50]: # Cap at 50 to avoid excessive API calls - try: - email_data = resend.Emails.get(eid) - statuses[eid] = { - 'id': eid, - 'to': getattr(email_data, 'to', []) if not isinstance(email_data, dict) else email_data.get('to', []), - 'subject': getattr(email_data, 'subject', '') if not isinstance(email_data, dict) else email_data.get('subject', ''), - 'created_at': getattr(email_data, 'created_at', '') if not isinstance(email_data, dict) else email_data.get('created_at', ''), - 'last_event': getattr(email_data, 'last_event', '') if not isinstance(email_data, dict) else email_data.get('last_event', ''), - } - except Exception as fetch_error: - warning(logger, "Failed to fetch Resend email status", - resend_email_id=eid, exc_info=fetch_error) - statuses[eid] = { - 'id': eid, - 'last_event': 'unknown', - 'error': str(fetch_error) - } + ids_to_fetch = [] + + for eid in email_ids[:100]: + cached = get_cached(f"resend:status:{eid}") + if cached is not None: + statuses[eid] = cached + else: + ids_to_fetch.append(eid) + + first_call = True + for eid in ids_to_fetch: + if not first_call: + time.sleep(0.3) + first_call = False + + retries = 0 + result = None + while retries < 2: + try: + email_data = resend.Emails.get(eid) + last_event = getattr(email_data, 'last_event', '') if not isinstance(email_data, dict) else email_data.get('last_event', '') + result = { + 'id': eid, + 'to': getattr(email_data, 'to', []) if not isinstance(email_data, dict) else email_data.get('to', []), + 'subject': getattr(email_data, 'subject', '') if not isinstance(email_data, dict) else email_data.get('subject', ''), + 'created_at': getattr(email_data, 'created_at', '') if not isinstance(email_data, dict) else email_data.get('created_at', ''), + 'last_event': last_event, + } + ttl = _RESEND_STATUS_TERMINAL_TTL if last_event in _RESEND_STATUS_TERMINAL_EVENTS else _RESEND_STATUS_TRANSIENT_TTL + set_cached(f"resend:status:{eid}", result, ttl=ttl) + break + except Exception as fetch_error: + retries += 1 + if retries < 2: + time.sleep(0.6) + continue + warning(logger, "Failed to fetch Resend email status after retry", + resend_email_id=eid, exc_info=fetch_error) + result = {'id': eid, 'last_event': 'unknown', 'error': str(fetch_error)} + + statuses[eid] = result return {'success': True, 'statuses': statuses} @@ -2698,28 +2737,39 @@ def get_resend_email_statuses(email_ids: list) -> Dict[str, Any]: _RESEND_LOCK_KEY = "resend:all_emails_refreshing" # background-refresh lock _RESEND_STALE_TTL = 3600 # keep data for 1 hour -_RESEND_FRESH_TTL = 300 # re-fetch after 5 minutes +_RESEND_FRESH_TTL = 900 # re-fetch after 15 minutes (on-demand sync, not per-page-load) _RESEND_LOCK_TTL = 120 # lock expires after 2 minutes (prevents stampede) +# Per-ID status cache TTLs +_RESEND_STATUS_TERMINAL_TTL = 7 * 24 * 3600 # terminal events don't change — 7 days +_RESEND_STATUS_TRANSIENT_TTL = 120 # transient events may update — 2 minutes +_RESEND_STATUS_TERMINAL_EVENTS = frozenset({ + 'delivered', 'bounced', 'complained', 'clicked', 'opened', 'unsubscribed', 'failed', 'canceled', +}) -def _fetch_and_cache_resend_emails() -> Optional[Dict[str, Any]]: + +def _fetch_and_cache_resend_emails(since_days: int = 90) -> Optional[Dict[str, Any]]: """ - Fetch all pages from Resend and update both cache keys. + Fetch recent pages from Resend and update both cache keys. Returns the full payload dict on success, None on failure. - Called either inline (first load) or from a background thread. + Called only from a background thread — never inline. """ resend.api_key = os.environ.get('RESEND_EMAIL_STATUS_KEY') if not resend.api_key: return None + cutoff_dt = datetime.now(pytz.utc) - timedelta(days=since_days) all_emails = [] - max_pages = 100 + max_pages = 30 max_retries = 2 params = {"limit": 100} page_error_occurred = False truncated = False + date_cutoff_reached = False for page in range(max_pages): + if page > 0: + time.sleep(0.5) retries = 0 while True: try: @@ -2733,7 +2783,21 @@ def _fetch_and_cache_resend_emails() -> Optional[Dict[str, Any]]: if not has_more or len(email_list) == 0: break + # Stop when the oldest item on this page predates cutoff (results are newest-first) last_item = email_list[-1] + last_created = last_item.get('created_at', '') if isinstance(last_item, dict) else getattr(last_item, 'created_at', '') + if last_created: + try: + last_dt_str = last_created.rstrip('Z').split('.')[0] + last_dt = datetime.fromisoformat(last_dt_str).replace(tzinfo=pytz.utc) + if last_dt < cutoff_dt: + date_cutoff_reached = True + except (ValueError, AttributeError): + pass + + if date_cutoff_reached: + break + last_id = last_item.get('id', '') if isinstance(last_item, dict) else getattr(last_item, 'id', '') if not last_id: break @@ -2751,7 +2815,7 @@ def _fetch_and_cache_resend_emails() -> Optional[Dict[str, Any]]: page=page, exc_info=page_error) page_error_occurred = True break - if page_error_occurred: + if page_error_occurred or date_cutoff_reached: break else: truncated = True @@ -2823,21 +2887,22 @@ def _background_refresh_resend_emails() -> None: delete_cached(_RESEND_LOCK_KEY) -def list_all_resend_emails(filter_emails=None): +def list_all_resend_emails(filter_emails=None, force: bool = False): """ - Fetch all sent emails from Resend's List Emails API, cached in Redis. - Returns an index of {recipient_email: [{id, subject, created_at, last_event}, ...]}. + Return the cached Resend email index. Never blocks on a full list crawl. - Uses a stale-while-revalidate strategy: - - Fresh cache hit → return immediately (fast path) - - Stale cache hit → return stale data immediately + refresh in background - - No cache at all → block on first fetch then return + Strategies: + - force=True → kick off a background refresh (ignoring freshness), return current data + syncing=True + - Stale cache → kick off background refresh, return stale data immediately + - No cache → kick off background refresh, return empty data + syncing=True + - Fresh cache → return immediately Args: filter_emails: Optional list of email addresses to filter results for. + force: If True, always start a background refresh. Returns: - Dict with 'success', 'emails_by_recipient', and 'total_fetched'. + Dict with 'success', 'emails_by_recipient', 'total_fetched', 'syncing'. """ try: resend.api_key = os.environ.get('RESEND_EMAIL_STATUS_KEY') @@ -2847,62 +2912,62 @@ def list_all_resend_emails(filter_emails=None): is_fresh = get_cached(_RESEND_FRESH_KEY) is not None cached = get_cached(_RESEND_INDEX_KEY) - if cached is not None: - if not is_fresh: - # Stale — trigger a background refresh if one isn't already running - if get_cached(_RESEND_LOCK_KEY) is None: - set_cached(_RESEND_LOCK_KEY, True, ttl=_RESEND_LOCK_TTL) - t = threading.Thread( - target=_background_refresh_resend_emails, - daemon=True - ) - t.start() - info(logger, "Stale Resend email cache — background refresh triggered", - total_fetched=cached.get('total_fetched', 0)) - else: - info(logger, "Stale Resend email cache — refresh already in progress", - total_fetched=cached.get('total_fetched', 0)) - else: - info(logger, "Using fresh cached Resend email index", - total_emails=cached.get('total_fetched', 0)) - - index = cached['emails_by_recipient'] - if filter_emails: - filter_set = {e.lower() for e in filter_emails} - index = {k: v for k, v in index.items() if k in filter_set} + def _maybe_start_background(): + if get_cached(_RESEND_LOCK_KEY) is None: + set_cached(_RESEND_LOCK_KEY, True, ttl=_RESEND_LOCK_TTL) + threading.Thread(target=_background_refresh_resend_emails, daemon=True).start() + return True + return False + + syncing = False + + if force: + started = _maybe_start_background() + # Report syncing=True even if lock was already held (another admin's sync is running) + syncing = started or (get_cached(_RESEND_LOCK_KEY) is not None) + info(logger, "Force-sync requested for Resend email cache", + started_new=started, syncing=syncing) + elif cached is None: + syncing = _maybe_start_background() + info(logger, "No Resend email cache — background sync started", syncing=syncing) return { 'success': True, - 'emails_by_recipient': index, - 'total_fetched': cached['total_fetched'], - 'truncated': cached.get('truncated', False), - 'from_cache': True, - 'stale': not is_fresh + 'emails_by_recipient': {}, + 'total_fetched': 0, + 'truncated': False, + 'from_cache': False, + 'syncing': True, } + elif not is_fresh: + syncing = _maybe_start_background() + info(logger, "Stale Resend email cache — background refresh triggered", + total_fetched=cached.get('total_fetched', 0), syncing=syncing) + else: + info(logger, "Using fresh cached Resend email index", + total_emails=cached.get('total_fetched', 0)) - # No cached data at all — block on initial fetch - info(logger, "No Resend email cache found — fetching synchronously") - set_cached(_RESEND_LOCK_KEY, True, ttl=_RESEND_LOCK_TTL) - payload = _fetch_and_cache_resend_emails() - if payload is None: + if cached is None: return { - 'success': False, - 'error': 'Failed to fetch email pages from Resend', + 'success': True, 'emails_by_recipient': {}, - 'total_fetched': 0 + 'total_fetched': 0, + 'truncated': False, + 'from_cache': False, + 'syncing': syncing, } - index = payload['emails_by_recipient'] + index = cached['emails_by_recipient'] if filter_emails: filter_set = {e.lower() for e in filter_emails} index = {k: v for k, v in index.items() if k in filter_set} - return { 'success': True, 'emails_by_recipient': index, - 'total_fetched': payload['total_fetched'], - 'truncated': payload['truncated'], - 'from_cache': False, - 'stale': False + 'total_fetched': cached['total_fetched'], + 'truncated': cached.get('truncated', False), + 'from_cache': True, + 'stale': not is_fresh, + 'syncing': syncing, } except Exception as e: diff --git a/test/common/utils/test_validators.py b/test/common/utils/test_validators.py new file mode 100644 index 0000000..ac349df --- /dev/null +++ b/test/common/utils/test_validators.py @@ -0,0 +1,23 @@ +import pytest + +from common.utils.validators import validate_social_posts + + +def test_validate_social_posts_allows_article_platform_urls(): + validate_social_posts([ + {"platform": "article", "url": "https://example.com/story"}, + ]) + + +def test_validate_social_posts_rejects_invalid_article_url(): + with pytest.raises(ValueError, match=r"social_posts\[0\]\.url must be a valid URL"): + validate_social_posts([ + {"platform": "article", "url": "not-a-url"}, + ]) + + +def test_validate_social_posts_keeps_platform_host_checks(): + with pytest.raises(ValueError, match=r"social_posts\[0\]\.url host must match platform 'linkedin'"): + validate_social_posts([ + {"platform": "linkedin", "url": "https://example.com/story"}, + ]) \ No newline at end of file