From c86970b52fb7c5ac8af0c44231e42d5e2d1a1738 Mon Sep 17 00:00:00 2001 From: Greg V <6913307+gregv@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:04:40 -0700 Subject: [PATCH 1/3] Add public portfolio profile stack (Aug 2026 launch) and profile API cutover Introduces the canonical /api/users/* profile stack (registry-driven PROFILE_FIELD_SPECS, privacy-filtered public portfolio, vanity slug claim/rename, bio video upload) backing the new /u/ and /profile/ public portfolio pages, and retires the legacy /api/messages/profile* handlers to thin delegates pending deletion. Also fixes review findings from the profile API cutover: - get_profile_by_db_id() dropped `github` entirely when trimmed to safe_public_fields; restored it via a new internal_lookup_fields list (safe_public_fields + github) so team rosters, peer feedback, and the admin giveaway selector keep showing GitHub usernames, without reopening github on the fully public/privacy-gated portfolio path. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 17 + api/certificates/certificate_service.py | 52 +- api/certificates/certificate_views.py | 51 +- api/messages/messages_service.py | 274 ++------- .../tests/test_profile_characterization.py | 152 +++++ api/messages/tests/test_profile_delegates.py | 74 +++ api/users/tests/test_field_registry.py | 124 +++++ api/users/tests/test_helping.py | 91 +++ api/users/tests/test_portfolio.py | 213 +++++++ api/users/tests/test_profile_identity.py | 68 +++ api/users/tests/test_profile_roundtrip.py | 70 +++ api/users/users_views.py | 123 +++- common/utils/cdn.py | 70 ++- common/utils/firebase.py | 46 +- db/db.py | 37 +- db/firestore.py | 211 +++++-- db/interface.py | 8 +- db/mem.py | 76 +++ firestore.indexes.json | 13 +- model/user.py | 178 +++++- .../backfill_certificate_github_usernames.py | 95 ++++ services/giveaway_service.py | 4 +- services/hearts_service.py | 61 +- services/news_service.py | 14 + services/problem_statements_service.py | 166 +++--- services/user_slug_service.py | 124 +++++ services/users_service.py | 523 ++++++++++++++++-- test/services/test_portfolio_helpers.py | 55 ++ 28 files changed, 2519 insertions(+), 471 deletions(-) create mode 100644 api/messages/tests/test_profile_characterization.py create mode 100644 api/messages/tests/test_profile_delegates.py create mode 100644 api/users/tests/test_field_registry.py create mode 100644 api/users/tests/test_helping.py create mode 100644 api/users/tests/test_portfolio.py create mode 100644 api/users/tests/test_profile_identity.py create mode 100644 api/users/tests/test_profile_roundtrip.py create mode 100644 scripts/backfill_certificate_github_usernames.py create mode 100644 services/user_slug_service.py create mode 100644 test/services/test_portfolio_helpers.py diff --git a/CLAUDE.md b/CLAUDE.md index 89b4df0..3b62d1d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -174,3 +174,20 @@ Config store for the Slack praise-bot (repo `ohack-slack-bot/praise-bot`): the b - Doc types in one collection: fixed doc id `global` (dry_run/llm_enabled/timezone), `github_watcher` (source `mode: hackathon|repos`, digest + optional rollup crons), `calendar_reminder`, and singleton `community` (intro matchmaker + weekly digest). Audit fields `created_at/updated_at/updated_by` on every doc. - `GET /api/praise-bot/config` — bot-facing, authed via `X-Api-Key` against `BACKEND_BOT_CONFIG_TOKEN` (falls back to `BACKEND_PRAISE_TOKEN`) through the shared `common/utils/api_key.py:check_api_key` (hmac.compare_digest; new code should use this instead of the inline checks in messages_views). Returns `configured: false` when the collection is empty → bot uses its env defaults. - `GET/POST /api/praise-bot/admin/config`, `PATCH/DELETE /api/praise-bot/admin/config/` — `volunteer.admin`-gated. Validation is whitelist-per-type (`_ALLOWED_KEYS` — the enforcement point that keeps secrets out of docs); crons validated as 5 fields (bot re-validates with `cron.validate()`); repos normalized to `owner/repo`; `global` is upsert-only (no DELETE); `community` is a singleton (POST 400s if one exists). `source.orgs` is accepted/stored but ignored by the bot until org-watching ships. 15s TTL cache on the assembled config, cleared on mutation. Tests: `api/praisebot/tests/` (mockfirestore, run with `ENVIRONMENT=test`). + +## Public portfolio (profile → portfolio, Aug 2026) + +The public profile payload (`GET /api/users//profile/public`) is now the "portfolio" payload. Load-bearing contracts: + +- **`model/user.py` is the control surface.** `metadata_list` += `bio`, `headline`, `portfolio_links` (savable via the generic profile POSTs — BOTH `api/users` and the legacy `api/messages` paths, which each call `_sanitize_portfolio_metadata` from users_service: bio ≤2000, headline ≤80, links ≤10 × {label ≤40, url} with https autoprefix + whitespace-URL rejection since `validate_url` lets spaces through). `privacy_fields` += `bio`, `bio_video_url`, `portfolio_links`, `teams`, `certificates`, `github_history`, `hearts` — **all default private**; `get_privacy_settings()` backfills lazily. `safe_public_fields` += `id`, `profile_slug` (they ARE the public URLs). `headline` rides the `bio` privacy toggle. `profile_visibility` (`private`|`public`, `DEFAULT_PROFILE_VISIBILITY="private"`) is always emitted; **private does NOT strip content** — it only tells the frontend to noindex (old links keep today's per-field rendering; `public` = indexable + sitemap and **requires a claimed slug**, enforced in `set_profile_visibility`). +- **Profile fields have ONE source of truth: `PROFILE_FIELD_SPECS` in `model/user.py`** (Aug 2026 profile-stack retirement). Adding a flat profile field = one spec entry `(name, default, owner_editable, persisted)` (+ a `privacy_fields` entry if privacy-gated). The registry generates `metadata_list`, `OWNER_EDITABLE_FIELDS` (what POST /profile accepts), `PROFILE_PERSISTED_FIELDS` (what the generic upsert writes), and `serialize_profile_fields()`; `users_service.build_profile_response(user)` is THE canonical own-profile response (hackathons attendance-derived, NOT the deprecated users.hackathons refs). Guards in `api/users/tests/`: `test_field_registry.py` fails CI when any hand-list (privacy/pii/safe, `_ADMIN_PROFILE_LEAN_FIELDS`) drifts; `test_profile_roundtrip.py` asserts every editable field survives POST→GET. `volunteering` is deliberately NOT in the generic write set (dedicated `update_user_volunteering` — a profile save racing a volunteering log must not clobber it); `propel_id`/`bio_video_url`/`profile_slug`/`profile_visibility` are never owner-editable via metadata. Profile get/save resolve identity via `_resolve_and_ensure_user` (3-tier; OAuth outage can't 404 the profile). The legacy `/api/messages/profile*` routes are THIN DELEGATES onto this stack (parity-tested in `api/messages/tests/test_profile_delegates.py`) pending final deletion once the migrated frontend deploy is confirmed live — the old hand-built `_old` bodies are gone; don't add fields to delegates. +- **Dedicated writers only** for `profile_slug` (`POST /api/users/profile/slug`, check at `GET .../slug/check/`), `profile_visibility` (`PATCH /api/users/profile/visibility`), `bio_video_url` (`POST /api/users/profile/bio-video`) — never add these to `metadata_list` (uniqueness/validation would be bypassed via merge writes). +- **Slugs** (`services/user_slug_service.py`): `user_slugs/{slug}` pointer collection, slug IS the doc id → uniqueness via `DocumentReference.create()` (MockFirestore fallback: get+set). Old slugs stay as `is_primary: False` aliases forever (links never break, no slug-jacking); rename throttled 1/24h via pointer `created_at`, max 5 pointers/user; `RESERVED_SLUGS` + regex `^[a-z0-9](?:[a-z0-9-]{1,28}[a-z0-9])?$` + reject 32-hex (db-id shadowing). Resolution: `_get_user_profile_by_db_id_or_slug` tries the doc id first (db-id always wins), then the pointer — wired into profile/public, privacy-settings, and praises getters. +- **Attach functions** in users_service (same try/except pattern as `_attach_hackathon_history`): `_attach_teams` (raw-doc team refs → `db.get_all` — deserialize drops `teams`; allowlisted team fields + trimmed `event` via `get_hackathon_by_event_id`; `@redis_cached("portfolio:teams", 900)`), `_attach_certificates` (heart certs from `history.certificates` + git-fame via `get_certificates_by_github_username`; **never expose `author_email`**), `_attach_github_contributions`, `_attach_hearts` (`hearts_service.get_hearts_summary` = what+how sum only, matches frontend HeartGauge; tiers mirror `src/lib/heartTiers.js` — keep in lockstep). +- **Caching**: whole payload `@redis_cached("portfolio:profile", 300)` (`get_portfolio_profile`); `clear_portfolio_caches()` (pattern-clears portfolio:* prefixes) is called from profile save (both paths), privacy PATCH, visibility PATCH, slug claim, bio-video set, and `give_hearts_to_user`. `redis_cached` never caches `None` and silently skips non-JSON-serializable results. +- **Certificates by user**: cert docs now stamped with `github_username` at generation (`_extract_github_username`: noreply-email regex, fallback bare author_name, lowercased). `GET /api/certificates?github=` (bare prefix route; sort/dedupe in Python — no order_by, avoids a composite index). **Run `scripts/backfill_certificate_github_usernames.py --apply` once** so pre-existing certs match. +- **GitHub contributions**: `get_github_contributions_for_user` no longer hardcodes the 2025 org — collection-group query on `login` only. **Requires the `github_contributors.login` COLLECTION_GROUP fieldOverride in firestore.indexes.json — deploy with `firebase deploy --only firestore:indexes` BEFORE shipping.** Timestamps ISO-ified for redis; `get_github_profile` TTL 10s→1h. +- **Bio video**: never proxy bytes through Flask. `POST /api/users/profile/bio-video/upload-url` mints a V4 signed GCS PUT URL (`common/utils/cdn.py::generate_signed_upload_url`, `x-goog-content-length-range` enforces the 100MB cap server-side; client must echo `required_headers`). The setter accepts own-CDN `users/{db_id}/…` URLs (blob verified via `get_blob_metadata`) or YouTube/Vimeo/Loom hosts; replaces best-effort-delete the previous own-CDN blob. **One-time ops: set bucket CORS to allow PUT from www.ohack.dev/ohack.dev/localhost** (`gcloud storage buckets update gs://$GCLOUD_CDN_BUCKET --cors-file=…`). +- **Sitemap feed**: `GET /api/users/portfolio/sitemap` (public, `@redis_cached("portfolio:sitemap", 3600)`) → `[{slug, last_login}]` for `profile_visibility == "public"` users; consumed by the frontend `server-sitemap.xml.js`. +- **Security fixes shipped with this**: `get_profile_by_db_id` now returns only `safe_public_fields` (was leaking `propel_id` + privacy-ignoring `github`); `get_praises_about_user` privacy-gates the raw-Slack-id praise route inside the service (messages_views is frozen); `POST /api/certificates/generate` requires login, and its `slack_channel` batch mode requires `volunteer.admin` (checked manually via `org_id_to_org_member_info` — pattern from hackathon_planning_service.is_admin). +- Tests: `api/users/tests/test_portfolio.py` (slug validation/claims/throttle, visibility gating, privacy matrix, sanitizer) + `test/services/test_portfolio_helpers.py` (hearts summary/tiers, cert username extraction). Run per-directory (`pytest api/users/tests test/…`) — running `pytest api/` wholesale hits a pre-existing `tests`-package name collision. diff --git a/api/certificates/certificate_service.py b/api/certificates/certificate_service.py index 337ca1a..930221f 100644 --- a/api/certificates/certificate_service.py +++ b/api/certificates/certificate_service.py @@ -1,5 +1,6 @@ import uuid import os +import re from PIL import ImageFont from os import getenv, path, remove @@ -14,7 +15,8 @@ import resend # Import get_team_by_slack_channel -from common.utils.firebase import get_team_by_slack_channel, save_certificate, get_certficate_by_file_id, get_recent_certs_from_db +from common.utils.firebase import get_team_by_slack_channel, save_certificate, get_certficate_by_file_id, get_recent_certs_from_db, fetch_certificates_by_github_username +from common.utils.redis_cache import redis_cached from common.utils.cdn import upload_to_cdn from common.utils.slack import async_send_slack from common.log import get_logger @@ -43,6 +45,45 @@ # Organizer names/usernames excluded from certificate generation (lowercase) EXCLUDED_AUTHORS = {"gregv", "greg v"} +# GitHub noreply emails look like 123456+login@users.noreply.github.com +GITHUB_NOREPLY_PATTERN = re.compile(r"^(?:\d+\+)?(?P[^@+]+)@users\.noreply\.github\.com$", re.IGNORECASE) + + +def _extract_github_username(author_email, author_name): + """Best-effort GitHub login for a cert, lowercased. + + Prefers the noreply-email encoding; falls back to author_name when it looks + like a bare login (no spaces). Returns None when neither works — the + backfill script logs those. + """ + if author_email: + m = GITHUB_NOREPLY_PATTERN.match(author_email.strip()) + if m: + return m.group("login").lower() + if author_name and " " not in author_name.strip(): + return author_name.strip().lower() + return None + + +@redis_cached(prefix="certs:by_github", ttl=900) +def get_certificates_by_github_username(github_username): + """Public list of a user's git-fame certificates, newest first.""" + if not github_username: + return [] + certs = fetch_certificates_by_github_username(github_username.strip().lower()) + # Sort + dedupe in Python — an order_by alongside the where() would force a + # composite index. + certs.sort(key=lambda c: c.get("date") or "", reverse=True) + deduped = [] + seen_file_ids = set() + for cert in certs: + file_id = cert.get("file_id") + if file_id in seen_file_ids: + continue + seen_file_ids.add(file_id) + deduped.append(cert) + return deduped + def get_cert_info(id): return get_certficate_by_file_id(id) @@ -334,15 +375,22 @@ def generate_certificate(repositoryURL: str, username: str) -> str: "certificate_url" : file_url, "author_name": username, "author_email" : authorWithEmailData.author, + # Stamped so certs can be queried per-user (portfolio page); the + # backfill script computes this for pre-existing docs. + "github_username": _extract_github_username(authorWithEmailData.author, username), "stats": stats_json, "totals": totals_json, "file_id": file_id, "file_id_hash": file_id_hash, "date": iso_date, - "repository_url": repositoryURL + "repository_url": repositoryURL } save_certificate(result) + try: + get_certificates_by_github_username.cache_clear() + except Exception: + pass return result def validateCertificate(certificateBase64Str: str) -> bool: diff --git a/api/certificates/certificate_views.py b/api/certificates/certificate_views.py index ca56e92..6efbbf7 100644 --- a/api/certificates/certificate_views.py +++ b/api/certificates/certificate_views.py @@ -2,17 +2,48 @@ from flask import Blueprint from flask import request -from api.certificates.certificate_service import generate_certificate, validateCertificate, generate_certificate_from_slack, get_cert_info, get_recent_certs +from api.certificates.certificate_service import generate_certificate, validateCertificate, generate_certificate_from_slack, get_cert_info, get_recent_certs, get_certificates_by_github_username +from common.auth import auth, auth_user bp_name = "api-certificates" bp_url_prefix = "/api/certificates" bp = Blueprint(bp_name, __name__, url_prefix=bp_url_prefix) +ADMIN_PERMISSION = "volunteer.admin" + + +def _user_has_admin_permission(propel_user): + """True when the logged-in user holds volunteer.admin in any org. + + Same pattern as services/hackathon_planning_service.is_admin — OHack has a + single org, so any-org membership is sufficient. + """ + if not propel_user or not getattr(propel_user, "user_id", None): + return False + org_id_to_org_member_info = ( + getattr(propel_user, "org_id_to_org_member_info", None) or {} + ) + for org_info in org_id_to_org_member_info.values(): + try: + if org_info.user_has_permission(ADMIN_PERMISSION): + return True + except Exception: + permissions = getattr(org_info, "user_permissions", None) or [] + if ADMIN_PERMISSION in permissions: + return True + return False + + @bp.route("/generate", methods=["POST"]) -def generateCertificate(): +@auth.require_user +def generateCertificate(): form = request.get_json() - if "slack_channel" in form: - return {"images": generate_certificate_from_slack(slack_channel=form["slack_channel"])} + if "slack_channel" in form: + # Batch mode clones every team repo and notifies people via Slack + + # email — admin only. + if not _user_has_admin_permission(auth_user): + return {"error": "Admin permission required for batch generation"}, 403 + return {"images": generate_certificate_from_slack(slack_channel=form["slack_channel"])} if ("repoURL" not in form or "username" not in form): return {} @@ -32,7 +63,15 @@ def getCert(id): return get_cert_info(id) @bp.route("/recent", methods=["GET"]) -def getRecentCerts(): +def getRecentCerts(): return { "certs": get_recent_certs() - } \ No newline at end of file + } + +@bp.route("", methods=["GET"]) +def getCertsByGithub(): + """GET /api/certificates?github= — a user's certificates.""" + github = request.args.get("github", "").strip() + if not github: + return {"error": "github query param is required"}, 400 + return {"certs": get_certificates_by_github_username(github)} \ No newline at end of file diff --git a/api/messages/messages_service.py b/api/messages/messages_service.py index c11ea09..4b147d4 100644 --- a/api/messages/messages_service.py +++ b/api/messages/messages_service.py @@ -80,130 +80,15 @@ def clear_cache(): # --------------------------- Problem Statement functions to be deleted ----------------- # @limits(calls=100, period=ONE_MINUTE) def save_helping_status_old(propel_user_id, json): - logger.info(f"save_helping_status {propel_user_id} // {json}") - slack_user = get_slack_user_from_propel_user_id(propel_user_id) - if slack_user is None: - logger.warning(f"Could not resolve Slack user for propel_user_id={propel_user_id}") + """Thin delegate to the canonical helping service (profile-stack + retirement). Deleted once the frontend calls /api/users/profile/helping.""" + logger.info("legacy-profile-route hit: POST /api/messages/profile/helping") + from services.problem_statements_service import save_helping_status + result = save_helping_status(propel_user_id, json) + if result is None: return None - user_id = slack_user["sub"] + return Message("Updated helping status") - helping_status = json["status"] # helping or not_helping - - problem_statement_id = json["problem_statement_id"] - mentor_or_hacker = json["type"] - - npo_id = json["npo_id"] if "npo_id" in json else "" - - user_obj = fetch_user_by_user_id(user_id) - my_date = datetime.now() - - - to_add = { - "user": user_obj.id, - "slack_user": user_id, - "type": mentor_or_hacker, - "timestamp": my_date.isoformat() - } - - db = get_db() - problem_statement_doc = db.collection( - 'problem_statements').document(problem_statement_id) - - ps_dict = problem_statement_doc.get().to_dict() - helping_list = [] - if "helping" in ps_dict: - helping_list = ps_dict["helping"] - logger.debug(f"Start Helping list: {helping_list}") - - if "helping" == helping_status: - helping_list.append(to_add) - else: - helping_list = [ - d for d in helping_list if d['user'] not in user_obj.id] - - else: - logger.debug(f"Start Helping list: {helping_list} * New list created for this problem") - if "helping" == helping_status: - helping_list.append(to_add) - - - logger.debug(f"End Helping list: {helping_list}") - problem_result = problem_statement_doc.update({ - "helping": helping_list - }) - - clear_cache() - - - send_slack_audit(action="helping", message=user_id, payload=to_add) - - # Determine how to identify this user in the Slack post. - # Slack logins get a real <@Uxxx> mention + auto-invite to the project channel. - # Non-Slack logins (Google, etc.) fall back to their display name so we don't - # render a broken "@oauth2" mention, and get a follow-up email asking them to - # join the Slack workspace. - display_name = (user_obj.name or user_obj.nickname or user_obj.email_address or "A volunteer").strip() - profile_url = f"https://ohack.dev/profile/{user_obj.id}" if user_obj.id else None - - if is_slack_user_id(user_id): - slack_user_id = extract_slack_user_id(user_id) - if profile_url: - mention = f"<@{slack_user_id}> (<{profile_url}|profile>)" - else: - mention = f"<@{slack_user_id}>" - is_slack_login = True - logger.info(f"save_helping_status_old: Slack login, mention={slack_user_id}") - else: - slack_user_id = None - mention = f"<{profile_url}|{display_name}>" if profile_url else display_name - is_slack_login = False - logger.info(f"save_helping_status_old: non-Slack login ({user_id}), mention={display_name}") - - problem_statement_title = ps_dict["title"] - - if "slack_channel" in ps_dict: - problem_statement_slack_channel = ps_dict["slack_channel"] - - project_link = f"" - if npo_id: - suffix = f" for " - else: - suffix = "" - - if "helping" == helping_status: - slack_message = f"{mention} is helping as a *{mentor_or_hacker}* on *{project_link}*{suffix}" - else: - slack_message = f"{mention} is _no longer able to help_ on *{project_link}*{suffix}" - - if is_slack_login and slack_user_id: - try: - invite_user_to_channel(user_id=slack_user_id, - channel_name=problem_statement_slack_channel) - except Exception as e: - logger.warning(f"invite_user_to_channel failed for {slack_user_id}: {e}") - - send_slack(message=slack_message, - channel=problem_statement_slack_channel) - - # For non-Slack users who are signing up to help, email them with a Slack join CTA - # so their project team can actually reach them. Swallow errors so a Resend - # outage never breaks the help toggle. - if not is_slack_login and helping_status == "helping" and user_obj.email_address: - try: - send_project_help_slack_invite_email( - name=user_obj.name or user_obj.nickname, - email=user_obj.email_address, - problem_statement_title=ps_dict.get("title"), - mentor_or_hacker=mentor_or_hacker, - npo_id=npo_id or None, - problem_statement_id=problem_statement_id, - ) - except Exception as e: - logger.warning(f"send_project_help_slack_invite_email failed for {user_obj.email_address}: {e}") - - return Message( - "Updated helping status" - ) @limits(calls=50, period=ONE_MINUTE) def save_problem_statement_old(json): @@ -288,7 +173,9 @@ def get_problem_statement_list_old(): logger.debug(results) return { "problem_statements": results } -@cached(cache=TTLCache(maxsize=100, ttl=10), lock=threading.Lock()) +# Contribution data comes from a batch scraper — 1h cache, matching the +# redis cache on get_github_contributions_for_user (was ttl=10, absurdly short) +@cached(cache=TTLCache(maxsize=100, ttl=3600), lock=threading.Lock()) @limits(calls=100, period=ONE_MINUTE) def get_github_profile(github_username): logger.debug(f"Getting Github Profile for {github_username}") @@ -304,49 +191,17 @@ def get_github_profile(github_username): @cached(cache=TTLCache(maxsize=100, ttl=600), lock=threading.Lock()) @limits(calls=100, period=ONE_MINUTE) def get_profile_metadata_old(propel_id): - logger.debug("Profile Metadata") - - email, user_id, last_login, profile_image, name, nickname = get_propel_user_details_by_id(propel_id) - - if user_id is None: - logger.warning("Could not resolve user details from PropelAuth for propel_id=%s", propel_id) + """Thin delegate to the canonical users-service profile read, preserving + the legacy {"text": ...} envelope and auth_failed error shape. + (Profile-stack retirement — deleted once the frontend is migrated.)""" + logger.info("legacy-profile-route hit: GET /api/messages/profile") + from services.users_service import get_profile_metadata + response = get_profile_metadata(propel_id) + if response is None: return {"error": "Unable to resolve user profile", "status": "auth_failed"} + return {"text": response} - send_slack_audit( - action="login", message=f"User went to profile: {user_id} with email: {email}") - - logger.debug(f"Account Details:\ - \nEmail: {email}\nSlack User ID: {user_id}\n\ - Last Login:{last_login}\ - Image:{profile_image}") - - # Call firebase to see if account exists and save these details - db_id = save_user_old( - user_id=user_id, - email=email, - last_login=last_login, - profile_image=profile_image, - name=name, - nickname=nickname, - propel_id=propel_id - ) - - # Get all of the user history and profile data from the DB - response = get_history_old(db_id) - logger.debug(f"get_profile_metadata {response}") - - - return { - "text" : response - } - - -# Fields returned to the admin /admin/profiles consumers (page + UserSearchDialog). -# Keep this in sync with frontend src/pages/admin/profile/index.js and -# src/components/admin/UserSearchDialog.js. Drop anything heavy (history) or -# unused (mailing address, propel_id, want_stickers) — those routes have their -# own /profile/ fetch when a row is opened. _ADMIN_PROFILE_LEAN_FIELDS = ( "name", "nickname", @@ -410,6 +265,8 @@ def get_all_profiles(): # Caching is not needed because the parent method already is caching + + @limits(calls=100, period=ONE_MINUTE) def get_history_old(db_id): logger.debug("Get History Start") @@ -481,6 +338,13 @@ def get_history_old(db_id): "postal_code": res["postal_code"] if "postal_code" in res else "", "country": res["country"] if "country" in res else "", "want_stickers": res["want_stickers"] if "want_stickers" in res else "", + # Portfolio fields (Profile.js PortfolioTab) + "bio": res.get("bio", ""), + "headline": res.get("headline", ""), + "bio_video_url": res.get("bio_video_url", ""), + "portfolio_links": res.get("portfolio_links") or [], + "profile_slug": res.get("profile_slug"), + "profile_visibility": res.get("profile_visibility", "private"), } # Clear cache @@ -554,82 +418,24 @@ def save_user_old( return doc_id def save_profile_metadata_old(propel_id, json): - send_slack_audit(action="save_profile_metadata", message="Saving", payload=json) - db = get_db() # this connects to our Firestore database - oauth_user = get_slack_user_from_propel_user_id(propel_id) - if oauth_user is None: - logger.warning(f"Could not get OAuth user details for propel_id: {propel_id}") + """Thin delegate to the canonical users-service profile write. + Returns None when identity can't be resolved (route 404s, as before).""" + logger.info("legacy-profile-route hit: POST /api/messages/profile") + from services.users_service import save_profile_metadata + result = save_profile_metadata(propel_id, json) + if result is None: return None - oauth_user_id = oauth_user["sub"] - - logger.info(f"Save Profile Metadata for {oauth_user_id} {json}") - - json = json["metadata"] + return Message("Saved Profile Metadata") - # See if the user exists - user = get_user_from_slack_id(oauth_user_id) - if user is None: - return - else: - logger.info(f"User exists: {user.id}") - - # Only update metadata that is in the json - metadataList = [ - "role", "expertise", "education", "company", "why", "shirt_size", "github", "linkedin_url", "instagram_url", "propel_id", - "street_address", "street_address_2", "city", "state", "postal_code", "country", "want_stickers" - ] - - d = {} - - for m in metadataList: - if m in json: - d[m] = json[m] - - logger.info(f"Metadata: {d}") - update_res = db.collection("users").document(user.id).set( d, merge=True) - - logger.info(f"Update Result: {update_res}") - - # Clear cache for get_profile_metadata - get_profile_metadata_old.cache_clear() - get_user_by_id_old.cache_clear() - - return Message( - "Saved Profile Metadata" - ) @cached(cache=TTLCache(maxsize=100, ttl=600), lock=threading.Lock(), key=lambda id: id) def get_user_by_id_old(id): - logger.debug(f"Attempting to get user by ID: {id}") - db = get_db() - doc_ref = db.collection('users').document(id) - - try: - doc = doc_ref.get() - if not doc.exists: - logger.warning(f"User with ID {id} not found") - return {} - - fields = ["name", "profile_image", "user_id", "nickname", "github"] - res = {} - for field in fields: - try: - value = doc.get(field) - if value is not None: - res[field] = value - except KeyError: - logger.info(f"Field '{field}' not found for user {id}") - - res["id"] = doc.id - logger.debug(f"Successfully retrieved user data: {res}") - return res - - except NotFound: - logger.info(f"Document with ID {id} not found in 'users' collection") - return {} - except Exception as e: - logger.error(f"Error retrieving user data for ID {id}: {str(e)}") - return {} + """Thin delegate to the privacy-safe public profile getter. Deliberate + deltas vs the old body: no longer leaks `github` (privacy-gated field), + now includes `profile_slug`.""" + logger.info("legacy-profile-route hit: GET /api/messages/profile/") + from services.users_service import get_profile_by_db_id + return get_profile_by_db_id(id) or {} def upload_image_to_cdn(request): diff --git a/api/messages/tests/test_profile_characterization.py b/api/messages/tests/test_profile_characterization.py new file mode 100644 index 0000000..3ed9ad1 --- /dev/null +++ b/api/messages/tests/test_profile_characterization.py @@ -0,0 +1,152 @@ +"""Characterization tests for the LEGACY profile routes (`_old` delegates). + +Originally pinned the hand-built legacy bodies; the `_old` functions are now +thin delegates onto the canonical users-service stack, and these tests pin +the OBSERVABLE legacy contract that must survive the delegation: + - GET keeps the {"text": ...} envelope and every historical key + - partial saves never clobber unrelated fields + - the public by-id route stays flat and PII-free +Deleted together with the legacy routes in the final cleanup PR. +""" +import os + +os.environ.setdefault("ENVIRONMENT", "test") # -> MockFirestore; no network at import + +import pytest + +import api.messages.messages_service as ms +import services.users_service as us +from db.db import get_db + +# The exact key set the legacy editor read path has always returned. +LEGACY_PROFILE_KEYS = frozenset({ + "id", "user_id", "profile_image", "email_address", "history", "badges", + "hackathons", "hackathon_history", + "expertise", "education", "shirt_size", "linkedin_url", "instagram_url", + "github", "why", "role", "company", "propel_id", + "street_address", "street_address_2", "city", "state", "postal_code", + "country", "want_stickers", + "bio", "headline", "bio_video_url", "portfolio_links", + "profile_slug", "profile_visibility", +}) + + +def _boom(_propel_id): + raise AssertionError("OAuth round-trip must not be required (propel_id resolves)") + + +def _seed_user(db_id, user_id, email, propel_id=None, **extra): + doc = { + "user_id": user_id, + "email_address": email, + "profile_image": "https://i.imgur.com/RdOsE7s.png", + "name": "Char Test", + "nickname": "Char", + "last_login": "2026-01-01T00:00:00Z", + "role": "volunteer", + "github": "chartest", + "city": "Tempe", + "want_stickers": "yes", + "badges": [], + "teams": [], + } + if propel_id: + doc["propel_id"] = propel_id + doc.update(extra) + get_db().collection("users").document(db_id).set(doc) + return doc + + +@pytest.fixture(autouse=True) +def _quiet(monkeypatch): + monkeypatch.setattr(ms, "send_slack_audit", lambda *a, **k: None) + monkeypatch.setattr(us, "send_slack_audit", lambda *a, **k: None) + monkeypatch.setattr(us, "get_propel_user_details_by_id", _boom) + monkeypatch.setattr(us, "get_oauth_user_from_propel_user_id", _boom) + yield + + +# ----------------------------- read path ------------------------------------ + +def test_get_history_old_returns_legacy_key_superset(): + """The frozen legacy read body (uncalled since delegation; deleted in the + cleanup PR). Kept as the executable definition of LEGACY_PROFILE_KEYS.""" + db_id = "char1111char1111char1111char1111" + _seed_user(db_id, "oauth2|slack|T123-UCHAR1", "char1@example.com") + + result = ms.get_history_old(db_id) + assert result is not None + assert LEGACY_PROFILE_KEYS <= set(result.keys()) + assert result["id"] == db_id + + +def test_get_profile_metadata_old_keeps_text_envelope_and_keys(): + db_id = "char2222char2222char2222char2222" + propel_id = "propel-char-2-unique" + _seed_user(db_id, "oauth2|slack|T123-UCHAR2", "char2@example.com", propel_id=propel_id) + + us.get_profile_metadata.cache_clear() + response = ms.get_profile_metadata_old(propel_id) + assert set(response.keys()) == {"text"}, "legacy GET must keep the {'text': ...} envelope" + assert LEGACY_PROFILE_KEYS <= set(response["text"].keys()), ( + f"delegate lost legacy keys: {LEGACY_PROFILE_KEYS - set(response['text'].keys())}" + ) + assert response["text"]["id"] == db_id + assert response["text"]["city"] == "Tempe" + assert response["text"]["want_stickers"] == "yes" + + +def test_get_profile_metadata_old_unresolvable_keeps_auth_failed_shape(monkeypatch): + monkeypatch.setattr(us, "_resolve_and_ensure_user", lambda pid: (None, None)) + us.get_profile_metadata.cache_clear() + response = ms.get_profile_metadata_old("propel-char-unresolvable-unique") + assert response == {"error": "Unable to resolve user profile", "status": "auth_failed"} + + +# ----------------------------- write path ----------------------------------- + +def test_save_profile_metadata_old_partial_save_does_not_clobber(): + db_id = "char3333char3333char3333char3333" + propel_id = "propel-char-3-unique" + _seed_user(db_id, "oauth2|slack|T123-UCHAR3", "char3@example.com", propel_id=propel_id, + company="Keep Me Inc", github="keepme", bio="keep bio") + + result = ms.save_profile_metadata_old(propel_id, {"metadata": {"city": "Phoenix"}}) + assert result is not None + + saved = get_db().collection("users").document(db_id).get().to_dict() + assert saved["city"] == "Phoenix" + # Partial saves must never clobber unrelated fields + assert saved["company"] == "Keep Me Inc" + assert saved["github"] == "keepme" + assert saved["bio"] == "keep bio" + assert saved["email_address"] == "char3@example.com" + + +def test_save_profile_metadata_old_unresolvable_user_returns_none(monkeypatch): + monkeypatch.setattr(us, "_resolve_and_ensure_user", lambda pid: (None, None)) + assert ms.save_profile_metadata_old("propel-char-4-unique", {"metadata": {"city": "X"}}) is None + + +# ----------------------------- public by-id path ---------------------------- + +def test_get_user_by_id_old_flat_safe_shape(): + db_id = "char5555char5555char5555char5555" + _seed_user(db_id, "oauth2|slack|T123-UCHAR5", "char5@example.com") + + result = ms.get_user_by_id_old(db_id) + for key in ("name", "profile_image", "user_id", "nickname", "id", "github"): + assert key in result, f"missing {key}" + assert result["id"] == db_id + assert result["github"] == "chartest" + # Never any PII. github IS included here (internal_lookup_fields) — this + # route backs team rosters/feedback/giveaways, which have always shown a + # participant's GitHub username regardless of the fully-public-portfolio + # privacy toggle. + assert "propel_id" not in result + assert "email_address" not in result + assert "profile_slug" in result + + +def test_get_user_by_id_old_unknown_id_returns_empty(): + assert ms.get_user_by_id_old("nope6666nope6666nope6666nope6666") == {} diff --git a/api/messages/tests/test_profile_delegates.py b/api/messages/tests/test_profile_delegates.py new file mode 100644 index 0000000..dcfc73b --- /dev/null +++ b/api/messages/tests/test_profile_delegates.py @@ -0,0 +1,74 @@ +"""Parity: the legacy delegates serve EXACTLY the canonical service's data. + +If these fail, the two URL families have diverged — the precise bug class the +profile-stack retirement exists to kill. +""" +import os + +os.environ.setdefault("ENVIRONMENT", "test") + +import pytest + +import api.messages.messages_service as ms +import services.users_service as us +from db.db import get_db + +from api.messages.tests.test_profile_characterization import LEGACY_PROFILE_KEYS + + +DB_ID = "par11111par11111par11111par11111" +PROPEL_ID = "propel-parity-unique" + + +@pytest.fixture(autouse=True) +def _seed(monkeypatch): + get_db().collection("users").document(DB_ID).set({ + "user_id": "oauth2|slack|T123-UPAR1", + "email_address": "parity@example.com", + "profile_image": "https://i.imgur.com/RdOsE7s.png", + "name": "Parity Test", + "nickname": "Parity", + "propel_id": PROPEL_ID, + "role": "mentor", + "city": "Mesa", + "badges": [], + "teams": [], + }) + monkeypatch.setattr(us, "send_slack_audit", lambda *a, **k: None) + monkeypatch.setattr(us, "get_propel_user_details_by_id", + lambda pid: (_ for _ in ()).throw(RuntimeError("no oauth"))) + monkeypatch.setattr(us, "get_oauth_user_from_propel_user_id", + lambda pid: (_ for _ in ()).throw(RuntimeError("no oauth"))) + yield + + +def _without_login_stamp(d): + # Every GET stamps a fresh last_login — irrelevant to shape parity + return {k: v for k, v in d.items() if k != "last_login"} + + +def test_legacy_get_equals_canonical_get(): + us.get_profile_metadata.cache_clear() + canonical = us.get_profile_metadata(PROPEL_ID) + + ms.get_profile_metadata_old.cache_clear() + us.get_profile_metadata.cache_clear() + legacy = ms.get_profile_metadata_old(PROPEL_ID) + + assert _without_login_stamp(legacy["text"]) == _without_login_stamp(canonical) + assert LEGACY_PROFILE_KEYS <= set(legacy["text"].keys()) + + +def test_legacy_by_id_equals_canonical_by_id(): + legacy = ms.get_user_by_id_old(DB_ID) + canonical = us.get_profile_by_db_id(DB_ID) + assert legacy == canonical + + +def test_write_via_legacy_visible_via_canonical(): + result = ms.save_profile_metadata_old(PROPEL_ID, {"metadata": {"headline": "Parity headline"}}) + assert result is not None + + us.get_profile_metadata.cache_clear() + canonical = us.get_profile_metadata(PROPEL_ID) + assert canonical["headline"] == "Parity headline" diff --git a/api/users/tests/test_field_registry.py b/api/users/tests/test_field_registry.py new file mode 100644 index 0000000..f5321de --- /dev/null +++ b/api/users/tests/test_field_registry.py @@ -0,0 +1,124 @@ +"""The CI guard for the profile field registry. + +Adding a profile field = one PROFILE_FIELD_SPECS entry. These tests fail when +any remaining hand-written list (privacy lists, admin lean projection, the +canonical response) drifts out of sync — turning the old "silently dropped +field" bug class into a red build. +""" +import os + +os.environ.setdefault("ENVIRONMENT", "test") + +from model.user import ( + User, + PROFILE_FIELD_SPECS, + OWNER_EDITABLE_FIELDS, + PROFILE_PERSISTED_FIELDS, + PROFILE_READONLY_RESPONSE_FIELDS, + metadata_list, + privacy_fields, + pii_fields, + safe_public_fields, +) + +SPEC_NAMES = {n for (n, _d, _e, _p) in PROFILE_FIELD_SPECS} +RESPONSE_NAMES = SPEC_NAMES | set(PROFILE_READONLY_RESPONSE_FIELDS) + +# Privacy fields that are DERIVED sections (attached by users_service from +# other collections/computations), not flat storage fields on the user doc. +DERIVED_PRIVACY_FIELDS = { + "badges", "what", "how", "feedback", "hackathon_history", "praises", + "teams", "certificates", "github_history", "hearts", +} + +# The frozen legacy editor contract (see the retirement plan): the canonical +# response must remain a SUPERSET of what /api/messages/profile always served. +LEGACY_PROFILE_KEYS = frozenset({ + "id", "user_id", "profile_image", "email_address", "history", "badges", + "hackathons", "hackathon_history", + "expertise", "education", "shirt_size", "linkedin_url", "instagram_url", + "github", "why", "role", "company", "propel_id", + "street_address", "street_address_2", "city", "state", "postal_code", + "country", "want_stickers", + "bio", "headline", "bio_video_url", "portfolio_links", + "profile_slug", "profile_visibility", +}) + + +def _minimal_user(): + return User.deserialize({"id": "reg11111reg11111reg11111reg11111"}) + + +def test_every_spec_field_is_an_instance_attr_after_deserialize(): + user = _minimal_user() + for name in SPEC_NAMES: + assert name in vars(user), f"deserialize must always set {name}" + + +def test_metadata_list_matches_registry(): + assert set(metadata_list) == SPEC_NAMES + + +def test_owner_editable_excludes_system_and_dedicated_fields(): + for forbidden in ("propel_id", "volunteering", "bio_video_url", "profile_slug", "profile_visibility"): + assert forbidden not in OWNER_EDITABLE_FIELDS, ( + f"{forbidden} must never be settable via POST /profile metadata" + ) + + +def test_volunteering_not_in_generic_write_set(): + # volunteering has a dedicated writer; the generic profile upsert writing + # it is the lost-update hazard this registry exists to prevent. + assert "volunteering" not in PROFILE_PERSISTED_FIELDS + user = _minimal_user() + assert "volunteering" not in user.serialize_profile_metadata() + + +def test_update_from_metadata_ignores_non_editable_fields(): + user = _minimal_user() + original_propel = user.propel_id + user.update_from_metadata({ + "propel_id": "attacker-propel-id", + "volunteering": [{"hours": 999}], + "profile_slug": "stolen-slug", + "city": "Tempe", + }) + assert user.propel_id == original_propel + assert user.volunteering == [] + assert user.profile_slug is None + assert user.city == "Tempe" + + +def test_privacy_lists_only_reference_known_fields(): + for field in privacy_fields: + assert field in SPEC_NAMES | set(PROFILE_READONLY_RESPONSE_FIELDS) | DERIVED_PRIVACY_FIELDS, ( + f"privacy_fields entry {field!r} is neither a registry field nor a known derived section" + ) + for field in pii_fields: + assert field in RESPONSE_NAMES, f"pii_fields entry {field!r} unknown" + for field in safe_public_fields: + assert field in RESPONSE_NAMES, f"safe_public_fields entry {field!r} unknown" + + +def test_admin_lean_fields_are_known(): + from api.messages.messages_service import _ADMIN_PROFILE_LEAN_FIELDS + allowed = RESPONSE_NAMES | {"badges", "teams", "hackathons", "volunteering"} + for field in _ADMIN_PROFILE_LEAN_FIELDS: + assert field in allowed, ( + f"_ADMIN_PROFILE_LEAN_FIELDS entry {field!r} is not a known profile field" + ) + + +def test_canonical_response_is_superset_of_legacy_contract(monkeypatch): + import services.users_service as us + monkeypatch.setattr(us, "warning", lambda *a, **k: None) + + user = _minimal_user() + response = us.build_profile_response(user) + missing = LEGACY_PROFILE_KEYS - set(response.keys()) + assert not missing, f"canonical response lost legacy keys: {missing}" + # And the sane defaults hold for a bare doc + assert response["profile_visibility"] == "private" + assert response["portfolio_links"] == [] + assert response["badges"] == [] + assert isinstance(response["history"], dict) diff --git a/api/users/tests/test_helping.py b/api/users/tests/test_helping.py new file mode 100644 index 0000000..423cdcc --- /dev/null +++ b/api/users/tests/test_helping.py @@ -0,0 +1,91 @@ +"""Canonical helping toggle (POST /api/users/profile/helping service path). + +Ports of the legacy behavior worth pinning: add/remove round-trip, the +exact-match removal fix (the legacy body used a substring test that could +remove OTHER users' entries), and OAuth-outage resilience via the resolver. +""" +import os + +os.environ.setdefault("ENVIRONMENT", "test") + +import pytest + +import services.problem_statements_service as pss +import services.users_service as us +from db.db import get_db + + +DB_ID = "help1111help1111help1111help1111" +PROPEL_ID = "propel-helping-unique" +USER_ID = "oauth2|slack|T123-UHELP1" +PS_ID = "ps-help-1" + + +def _boom(_propel_id): + raise AssertionError("OAuth round-trip must NOT be required when propel_id resolves") + + +@pytest.fixture(autouse=True) +def _seed(monkeypatch): + get_db().collection("users").document(DB_ID).set({ + "user_id": USER_ID, + "email_address": "helper@example.com", + "profile_image": "x", + "name": "Helper One", + "nickname": "Helper", + "propel_id": PROPEL_ID, + "badges": [], + "teams": [], + }) + get_db().collection("problem_statements").document(PS_ID).set({ + "title": "Helping Test Project", + "slack_channel": "npo-helping-test", + # Pre-existing entry from ANOTHER user whose id is a SUBSTRING of ours — + # the legacy `not in` removal would have wrongly deleted this. + "helping": [{"user": "help1111", "slack_user": "U0OTHER", "type": "hacker", + "timestamp": "2026-01-01T00:00:00"}], + }) + monkeypatch.setattr(us, "get_oauth_user_from_propel_user_id", _boom) + monkeypatch.setattr(pss, "send_slack", lambda *a, **k: None) + monkeypatch.setattr(pss, "send_slack_audit", lambda *a, **k: None) + monkeypatch.setattr(pss, "invite_user_to_channel", lambda *a, **k: None) + yield + + +def _helping_list(): + return (get_db().collection("problem_statements").document(PS_ID).get().to_dict() or {}).get("helping", []) + + +def test_helping_add_then_remove_roundtrip(): + result = pss.save_helping_status(PROPEL_ID, { + "status": "helping", "problem_statement_id": PS_ID, "type": "hacker", "npo_id": "npo1", + }) + assert result is not None + mine = [h for h in _helping_list() if h["user"] == DB_ID] + assert len(mine) == 1 + assert mine[0]["type"] == "hacker" + assert mine[0]["slack_user"] == USER_ID + + result = pss.save_helping_status(PROPEL_ID, { + "status": "not_helping", "problem_statement_id": PS_ID, "type": "hacker", + }) + assert result is not None + assert [h for h in _helping_list() if h["user"] == DB_ID] == [] + + +def test_remove_is_exact_match_not_substring(): + pss.save_helping_status(PROPEL_ID, { + "status": "not_helping", "problem_statement_id": PS_ID, "type": "hacker", + }) + survivors = _helping_list() + # The other user's entry (id "help1111", a substring of ours) must survive + assert any(h["user"] == "help1111" for h in survivors), ( + "exact-match removal regressed to the legacy substring bug" + ) + + +def test_unknown_problem_statement_returns_none(): + result = pss.save_helping_status(PROPEL_ID, { + "status": "helping", "problem_statement_id": "nope-does-not-exist", "type": "hacker", + }) + assert result is None diff --git a/api/users/tests/test_portfolio.py b/api/users/tests/test_portfolio.py new file mode 100644 index 0000000..f881d25 --- /dev/null +++ b/api/users/tests/test_portfolio.py @@ -0,0 +1,213 @@ +"""Tests for the public-portfolio backend: slug claims, visibility gating, +and the privacy matrix of get_public_profile_data. + +ENVIRONMENT=test -> MockFirestore; no network at import. +""" +import os + +os.environ.setdefault("ENVIRONMENT", "test") + +import pytest + +import services.users_service as us +import services.user_slug_service as slugs +from model.user import User, DEFAULT_PROFILE_VISIBILITY + + +def _make_user(db_id, slug=None): + user = User() + user.id = db_id + user.name = "Test User" + user.nickname = "Tester" + user.profile_image = "https://i.imgur.com/RdOsE7s.png" + user.user_id = f"oauth2|slack|T123-{db_id}" + user.profile_slug = slug + return user + + +def _patch_resolve(monkeypatch, user): + monkeypatch.setattr(us, "_resolve_and_ensure_user", lambda _pid: (user, user.user_id)) + + +# ----------------------------- slug validation ----------------------------- + +@pytest.mark.parametrize("slug,valid", [ + ("gregv", True), + ("greg-v", True), + ("a1b2c3", True), + ("ab", False), # too short + ("a" * 31, False), # too long + ("-greg", False), # leading hyphen + ("greg-", False), # trailing hyphen + ("Greg", False), # normalize first — validate_slug expects lowercase + ("greg v", False), # space + ("admin", False), # reserved + ("profile", False), # reserved + ("u", False), # reserved + short + ("0123456789abcdef0123456789abcdef", False), # 32-hex — could shadow a db id +]) +def test_validate_slug(slug, valid): + ok, _reason = slugs.validate_slug(slug) + assert ok is valid + + +def test_normalize_slug(): + assert slugs.normalize_slug(" GregV ") == "gregv" + + +# ----------------------------- claim + conflict ----------------------------- + +def test_claim_and_conflict(monkeypatch): + alice = _make_user("aaaa1111aaaa1111aaaa1111aaaa1111") + _patch_resolve(monkeypatch, alice) + + payload, status = slugs.claim_profile_slug("propel-alice", "alice-portfolio") + assert status == 200 + assert payload["slug"] == "alice-portfolio" + assert payload["previous_slug"] is None + + # A different user cannot take it + bob = _make_user("bbbb2222bbbb2222bbbb2222bbbb2222") + _patch_resolve(monkeypatch, bob) + payload, status = slugs.claim_profile_slug("propel-bob", "Alice-Portfolio") + assert status == 409 + + # The pointer resolves to alice + from db.db import fetch_user_db_id_by_slug + pointer = fetch_user_db_id_by_slug("alice-portfolio") + assert pointer["user_db_id"] == alice.id + assert pointer["is_primary"] is True + + +def test_claim_same_slug_is_idempotent(monkeypatch): + carol = _make_user("cccc3333cccc3333cccc3333cccc3333") + _patch_resolve(monkeypatch, carol) + + payload, status = slugs.claim_profile_slug("propel-carol", "carol") + assert status == 200 + payload, status = slugs.claim_profile_slug("propel-carol", "carol") + assert status == 200 + assert payload.get("previous_slug") is None + + +def test_rename_is_throttled(monkeypatch): + dave = _make_user("dddd4444dddd4444dddd4444dddd4444") + _patch_resolve(monkeypatch, dave) + + payload, status = slugs.claim_profile_slug("propel-dave", "dave-one") + assert status == 200 + # Immediate rename hits the 24h cooldown + payload, status = slugs.claim_profile_slug("propel-dave", "dave-two") + assert status == 429 + + +def test_invalid_slug_rejected(monkeypatch): + erin = _make_user("eeee5555eeee5555eeee5555eeee5555") + _patch_resolve(monkeypatch, erin) + _payload, status = slugs.claim_profile_slug("propel-erin", "admin") + assert status == 400 + + +# ----------------------------- visibility gating ---------------------------- + +def test_public_visibility_requires_slug(monkeypatch): + frank = _make_user("ffff6666ffff6666ffff6666ffff6666", slug=None) + _patch_resolve(monkeypatch, frank) + + payload, status = us.set_profile_visibility("propel-frank", "public") + assert status == 400 + + payload, status = us.set_profile_visibility("propel-frank", "bogus") + assert status == 400 + + frank.profile_slug = "frank" + payload, status = us.set_profile_visibility("propel-frank", "public") + assert status == 200 + assert payload["profile_visibility"] == "public" + + payload, status = us.set_profile_visibility("propel-frank", "private") + assert status == 200 + + +# ----------------------------- privacy matrix ------------------------------- + +def test_public_profile_defaults_are_private(): + user = _make_user("9999aaaa9999aaaa9999aaaa9999aaaa", slug="niner") + user.github = "someuser" + user.bio = "My bio" + user.headline = "Builder" + user.portfolio_links = [{"label": "Site", "url": "https://example.com"}] + user.history = {"what": {"code_quality": 2}, "how": {"standups_completed": 1}} + + data = user.get_public_profile_data() + + # Safe fields always present + assert data["name"] == "Test User" + assert data["id"] == user.id + assert data["profile_slug"] == "niner" + # Master toggle defaults private + assert data["profile_visibility"] == DEFAULT_PROFILE_VISIBILITY == "private" + # Privacy-gated fields absent by default + for hidden in ("github", "bio", "headline", "portfolio_links", "history", + "teams", "certificates", "github_history", "hearts"): + assert hidden not in data, f"{hidden} leaked with default privacy" + + +def test_public_profile_opt_ins(): + user = _make_user("8888bbbb8888bbbb8888bbbb8888bbbb") + user.bio = "My bio" + user.headline = "Builder" + user.portfolio_links = [{"label": "Site", "url": "https://example.com"}] + user.privacy_settings = {"bio": "public", "portfolio_links": "public"} + + data = user.get_public_profile_data() + assert data["bio"] == "My bio" + assert data["headline"] == "Builder" # rides the bio toggle + assert data["portfolio_links"] == [{"label": "Site", "url": "https://example.com"}] + + +def test_headline_hidden_when_bio_private(): + user = _make_user("7777cccc7777cccc7777cccc7777cccc") + user.headline = "Builder" + user.privacy_settings = {"bio": "private"} + data = user.get_public_profile_data() + assert "headline" not in data + + +def test_get_profile_by_db_id_never_leaks_propel_id_but_includes_github(monkeypatch): + user = _make_user("6666dddd6666dddd6666dddd6666dddd") + user.propel_id = "propel-secret" + user.github = "somegithub" + monkeypatch.setattr(us, "get_user_profile_by_db_id", lambda _id: user) + + result = us.get_profile_by_db_id(user.id) + assert "propel_id" not in result + # github is part of internal_lookup_fields (not safe_public_fields) — + # this internal by-id route backs team rosters/feedback/giveaways, which + # have always shown a participant's GitHub username regardless of the + # fully-public-portfolio privacy toggle tested above. + assert result["github"] == "somegithub" + assert result["name"] == "Test User" + + +# ----------------------------- metadata sanitization ------------------------ + +def test_sanitize_portfolio_metadata(): + metadata = { + "bio": " x" + "y" * 3000, + "headline": "h" * 200, + "portfolio_links": [ + {"label": "ok", "url": "example.com/portfolio"}, + {"label": "bad", "url": "not a url at all"}, + "not-a-dict", + {"label": "l" * 100, "url": "https://good.example.org"}, + ], + } + us._sanitize_portfolio_metadata(metadata) + assert len(metadata["bio"]) <= us.MAX_BIO_LENGTH + assert len(metadata["headline"]) == us.MAX_HEADLINE_LENGTH + urls = [l["url"] for l in metadata["portfolio_links"]] + assert "https://example.com/portfolio" in urls # https auto-prefixed + assert all(u.startswith("http") for u in urls) + assert len(metadata["portfolio_links"]) == 2 # invalid entries dropped + assert all(len(l["label"]) <= us.MAX_LINK_LABEL_LENGTH for l in metadata["portfolio_links"]) diff --git a/api/users/tests/test_profile_identity.py b/api/users/tests/test_profile_identity.py new file mode 100644 index 0000000..5091b1c --- /dev/null +++ b/api/users/tests/test_profile_identity.py @@ -0,0 +1,68 @@ +"""Profile read/write must survive an OAuth-provider outage. + +get_profile_metadata / save_profile_metadata used to depend SOLELY on the live +OAuth round-trip (like volunteering once did — see test_volunteer_resolve.py). +They now resolve through _resolve_and_ensure_user, whose tier-1 path (stored +propel_id) makes no external call. +""" +import os + +os.environ.setdefault("ENVIRONMENT", "test") # -> MockFirestore; no network at import + +import services.users_service as us +from db.db import get_db + + +def _boom(_propel_id): + raise AssertionError("OAuth round-trip must NOT be required when propel_id resolves") + + +def _seed_user(db_id, propel_id): + get_db().collection("users").document(db_id).set({ + "user_id": f"oauth2|slack|T123-U{db_id[:6].upper()}", + "email_address": f"{db_id[:8]}@example.com", + "profile_image": "https://i.imgur.com/RdOsE7s.png", + "name": "Identity Test", + "nickname": "Ident", + "propel_id": propel_id, + "badges": [], + "teams": [], + }) + + +def test_profile_read_works_with_oauth_down(monkeypatch): + db_id = "ident111ident111ident111ident111" + propel_id = "propel-ident-read-unique" + _seed_user(db_id, propel_id) + + monkeypatch.setattr(us, "get_oauth_user_from_propel_user_id", _boom) + monkeypatch.setattr(us, "get_propel_user_details_by_id", _boom) + monkeypatch.setattr(us, "send_slack_audit", lambda *a, **k: None) + + result = us.get_profile_metadata(propel_id) + assert result is not None + assert result["id"] == db_id + assert result["name"] == "Identity Test" + + +def test_profile_save_works_with_oauth_down(monkeypatch): + db_id = "ident222ident222ident222ident222" + propel_id = "propel-ident-save-unique" + _seed_user(db_id, propel_id) + + monkeypatch.setattr(us, "get_oauth_user_from_propel_user_id", _boom) + monkeypatch.setattr(us, "send_slack_audit", lambda *a, **k: None) + + result = us.save_profile_metadata(propel_id, {"metadata": {"company": "Resilient Corp"}}) + assert result is not None + assert result["company"] == "Resilient Corp" + + saved = get_db().collection("users").document(db_id).get().to_dict() + assert saved["company"] == "Resilient Corp" + # Untouched fields survive the save + assert saved["name"] == "Identity Test" + + +def test_profile_save_without_metadata_key_returns_none(monkeypatch): + monkeypatch.setattr(us, "send_slack_audit", lambda *a, **k: None) + assert us.save_profile_metadata("propel-ident-bad-unique", {}) is None diff --git a/api/users/tests/test_profile_roundtrip.py b/api/users/tests/test_profile_roundtrip.py new file mode 100644 index 0000000..4f3d990 --- /dev/null +++ b/api/users/tests/test_profile_roundtrip.py @@ -0,0 +1,70 @@ +"""Round-trip guarantee: EVERY owner-editable profile field survives +POST /profile -> GET /profile. + +This is the test that makes the "saves fine, renders blank" bug class +structurally impossible: if a field is in the registry but any layer drops it +(write set, read path, serializer), this fails. +""" +import os + +os.environ.setdefault("ENVIRONMENT", "test") + +import pytest + +import services.users_service as us +from db.db import get_db +from model.user import OWNER_EDITABLE_FIELDS + + +def _sentinel_for(field): + if field == "portfolio_links": + return ( + [{"label": "Site", "url": "https://example.com/rt"}], + [{"label": "Site", "url": "https://example.com/rt"}], + ) + if field == "want_stickers": + return (True, True) + if field == "expertise": + return (["Data Science", "Mentor"], ["Data Science", "Mentor"]) + if field == "linkedin_url": + return ("https://www.linkedin.com/in/roundtrip", "https://www.linkedin.com/in/roundtrip") + sent = f"rt-{field}" + return (sent, sent) + + +DB_ID = "rt111111rt111111rt111111rt111111" +PROPEL_ID = "propel-roundtrip-unique" + + +@pytest.fixture(autouse=True) +def _seed(monkeypatch): + get_db().collection("users").document(DB_ID).set({ + "user_id": "oauth2|slack|T123-URT1", + "email_address": "roundtrip@example.com", + "profile_image": "https://i.imgur.com/RdOsE7s.png", + "name": "Round Trip", + "nickname": "RT", + "propel_id": PROPEL_ID, + "badges": [], + "teams": [], + }) + monkeypatch.setattr(us, "send_slack_audit", lambda *a, **k: None) + monkeypatch.setattr(us, "get_propel_user_details_by_id", + lambda pid: (_ for _ in ()).throw(RuntimeError("no oauth in tests"))) + yield + + +@pytest.mark.parametrize("field", OWNER_EDITABLE_FIELDS) +def test_every_editable_field_survives_write_then_read(field): + submitted, expected = _sentinel_for(field) + + save_result = us.save_profile_metadata(PROPEL_ID, {"metadata": {field: submitted}}) + assert save_result is not None, f"save failed for {field}" + assert save_result.get(field) == expected, f"save response dropped {field}" + + us.get_profile_metadata.cache_clear() + read_result = us.get_profile_metadata(PROPEL_ID) + assert read_result is not None + assert read_result.get(field) == expected, ( + f"{field} was saved but came back {read_result.get(field)!r} — a projection dropped it" + ) diff --git a/api/users/users_views.py b/api/users/users_views.py index 42e7f5f..c9e4ebd 100644 --- a/api/users/users_views.py +++ b/api/users/users_views.py @@ -1,5 +1,7 @@ from model.user import User from services import users_service +from services import user_slug_service +from services import problem_statements_service from common.utils import safe_get_env_var from common.auth import auth, auth_user @@ -21,29 +23,30 @@ def getOrgId(req): # Used to provide profile details - user must be logged in @bp.route("/profile", methods=["GET"]) @auth.require_user -def profile(): - # user_id is a uuid from Propel Auth - if auth_user and auth_user.user_id: - u: User | None = users_service.get_profile_metadata(auth_user.user_id) - if u is None: - return None - # vars(u) exposes u.hackathons as raw Hackathon objects, which Flask - # can't JSON-encode (TypeError: Object of type Hackathon is not JSON - # serializable). Shallow-copy and replace it with serialized dicts. - result = dict(vars(u)) - result["hackathons"] = u.serialize_hackathons() - return result - else: - return None +def profile(): + """Canonical own-profile read: the flat build_profile_response dict.""" + if not (auth_user and auth_user.user_id): + return {"error": "Unauthorized"}, 401 + profile_data = users_service.get_profile_metadata(auth_user.user_id) + if profile_data is None: + # Identity couldn't be resolved by any tier (propel_id, OAuth, metadata) + return {"error": "Unable to resolve user profile"}, 503 + return profile_data + @bp.route("/profile", methods=["POST"]) @auth.require_user -def save_profile(): - if auth_user and auth_user.user_id: - u: User | None = users_service.save_profile_metadata(auth_user.user_id, request.get_json()) - return vars(u) if u is not None else None - else: - return None +def save_profile(): + """Canonical own-profile write. Returns the updated flat profile dict.""" + if not (auth_user and auth_user.user_id): + return {"error": "Unauthorized"}, 401 + data = request.get_json(silent=True) + if not data or "metadata" not in data: + return {"error": "metadata is required"}, 400 + result = users_service.save_profile_metadata(auth_user.user_id, data) + if result is None: + return {"error": "Unable to resolve user profile"}, 404 + return result # Get user profile by user id @@ -108,6 +111,82 @@ def get_all_volunteering_time(): return None +@bp.route("/profile/helping", methods=["POST"]) +@auth.require_user +def register_helping_status(): + """Canonical helping toggle (replaces POST /api/messages/profile/helping).""" + if not (auth_user and auth_user.user_id): + return {"error": "Unauthorized"}, 401 + data = request.get_json(silent=True) or {} + for required in ("status", "problem_statement_id", "type"): + if required not in data: + return {"error": f"{required} is required"}, 400 + result = problem_statements_service.save_helping_status(auth_user.user_id, data) + if result is None: + return {"error": "Unable to resolve user or problem statement"}, 404 + return result + + +# Vanity profile slug (portfolio URL) — dedicated routes so slugs can never be +# set through the generic profile metadata POST (uniqueness would be bypassed). +@bp.route("/profile/slug", methods=["POST"]) +@auth.require_user +def claim_profile_slug(): + if not (auth_user and auth_user.user_id): + return {"error": "Unauthorized"}, 401 + data = request.get_json() or {} + payload, status = user_slug_service.claim_profile_slug(auth_user.user_id, data.get("slug")) + return payload, status + + +@bp.route("/profile/slug/check/", methods=["GET"]) +@auth.require_user +def check_profile_slug(slug): + if not (auth_user and auth_user.user_id): + return {"error": "Unauthorized"}, 401 + return user_slug_service.check_slug_availability(slug) + + +@bp.route("/profile/visibility", methods=["PATCH"]) +@auth.require_user +def set_profile_visibility(): + """Portfolio master toggle: private (default) or public (search-indexable).""" + if not (auth_user and auth_user.user_id): + return {"error": "Unauthorized"}, 401 + data = request.get_json() or {} + payload, status = users_service.set_profile_visibility(auth_user.user_id, data.get("visibility")) + return payload, status + + +@bp.route("/portfolio/sitemap", methods=["GET"]) +def get_portfolio_sitemap(): + """Public feed of opted-in portfolio slugs for the frontend server-sitemap.""" + return {"portfolios": users_service.get_searchable_portfolio_sitemap()} + + +@bp.route("/profile/bio-video/upload-url", methods=["POST"]) +@auth.require_user +def create_bio_video_upload_url(): + """Mint a signed GCS PUT URL — video bytes never pass through this API.""" + if not (auth_user and auth_user.user_id): + return {"error": "Unauthorized"}, 401 + data = request.get_json() or {} + payload, status = users_service.create_bio_video_upload_url( + auth_user.user_id, data.get("content_type"), data.get("content_length")) + return payload, status + + +@bp.route("/profile/bio-video", methods=["POST"]) +@auth.require_user +def set_bio_video_url(): + """Set (own-CDN upload or YouTube/Vimeo/Loom link) or clear the bio video.""" + if not (auth_user and auth_user.user_id): + return {"error": "Unauthorized"}, 401 + data = request.get_json() or {} + payload, status = users_service.set_bio_video_url(auth_user.user_id, data.get("url")) + return payload, status + + @bp.route("/profile/privacy-settings", methods=["GET"]) @auth.require_user def get_privacy_settings(): @@ -142,8 +221,8 @@ def update_privacy_settings(): # Privacy-aware public profile endpoints @bp.route("//profile/public", methods=["GET"]) def get_public_profile_by_db_id(user_id): - """Get privacy-filtered public profile by database ID""" - profile_data = users_service.get_privacy_filtered_profile_by_db_id(user_id) + """Get privacy-filtered public profile by database ID or vanity slug (cached)""" + profile_data = users_service.get_portfolio_profile(user_id) if profile_data: return profile_data return {"error": "User not found"}, 404 diff --git a/common/utils/cdn.py b/common/utils/cdn.py index 6956b75..ccbf9b9 100644 --- a/common/utils/cdn.py +++ b/common/utils/cdn.py @@ -25,14 +25,74 @@ GCLOUD_CDN_BUCKET = os.getenv("GCLOUD_CDN_BUCKET") GOOGLE_APPLICATION_CREDENTIALS = os.getenv("GOOGLE_APPLICATION_CREDENTIALS") -def upload_to_cdn(directory, source_file_name, destination_file_name=None): - """Uploads a file to the bucket.""" +def _get_bucket(): + """Storage bucket handle using the service-account credentials.""" gcp_json_credentials_dict = json.loads(GOOGLE_APPLICATION_CREDENTIALS) creds = service_account.Credentials.from_service_account_info(gcp_json_credentials_dict) project_name = GCLOUD_CDN_BUCKET.split("_")[0] - storage_client = storage.Client(project=project_name,credentials=creds) - bucket = storage_client.bucket(GCLOUD_CDN_BUCKET) - + storage_client = storage.Client(project=project_name, credentials=creds) + return storage_client.bucket(GCLOUD_CDN_BUCKET) + + +def generate_signed_upload_url(directory, filename, content_type, max_bytes, expiration_minutes=15): + """V4 signed PUT URL so large files (videos) upload straight to GCS. + + Never proxy video bytes through Flask — a multi-MB body pins a worker + thread + RAM for the whole transfer on our small Fly box. + + The x-goog-content-length-range header makes GCS itself enforce the size + cap; the client MUST send the same header on the PUT or the signature + check fails. + """ + from datetime import timedelta + + bucket = _get_bucket() + blob = bucket.blob(f"{directory}/{filename}") + signed_url = blob.generate_signed_url( + version="v4", + expiration=timedelta(minutes=expiration_minutes), + method="PUT", + content_type=content_type, + headers={"x-goog-content-length-range": f"0,{max_bytes}"}, + ) + return { + "upload_url": signed_url, + "blob_path": f"{directory}/{filename}", + "final_url": f"{CDN_SERVER}/{directory}/{filename}", + "required_headers": { + "Content-Type": content_type, + "x-goog-content-length-range": f"0,{max_bytes}", + }, + } + + +def get_blob_metadata(path): + """{exists, size, content_type} for a blob path like 'users//video.mp4'.""" + bucket = _get_bucket() + blob = bucket.get_blob(path) + if blob is None: + return {"exists": False, "size": None, "content_type": None} + return {"exists": True, "size": blob.size, "content_type": blob.content_type} + + +def delete_from_cdn(path): + """Best-effort delete of a blob path. Returns True when deleted.""" + try: + bucket = _get_bucket() + blob = bucket.blob(path) + if blob.exists(): + blob.delete() + logger.info(f"Deleted {path} from CDN bucket") + return True + except Exception as e: + logger.warning(f"Failed to delete {path} from CDN bucket: {e}") + return False + + +def upload_to_cdn(directory, source_file_name, destination_file_name=None): + """Uploads a file to the bucket.""" + bucket = _get_bucket() + # Use destination_file_name if provided, otherwise use source_file_name blob_filename = destination_file_name if destination_file_name else source_file_name blob = bucket.blob(f"{directory}/{blob_filename}") diff --git a/common/utils/firebase.py b/common/utils/firebase.py index 1dff814..f887026 100644 --- a/common/utils/firebase.py +++ b/common/utils/firebase.py @@ -8,6 +8,7 @@ from google.cloud.firestore import FieldFilter # Import OAuth utilities for handling multiple providers (Slack, Google, etc.) from common.utils.oauth_providers import SLACK_PREFIX, normalize_slack_user_id, is_oauth_user_id +from common.utils.redis_cache import redis_cached cert_env = json.loads(safe_get_env_var("FIREBASE_CERT_CONFIG")) @@ -161,15 +162,22 @@ def get_users_by_emails(email_addresses): return user_map +@redis_cached(prefix="github:contrib", ttl=3600) def get_github_contributions_for_user(login): + """All stored GitHub contributions for a login, across every org/event. + + Collection-group query on login only (needs the github_contributors.login + COLLECTION_GROUP fieldOverride in firestore.indexes.json). Contribution + data comes from a batch scraper, so a 1h cache is appropriate. + """ logger.info(f"Getting github contributions for user {login}") db = get_db() # this connects to our Firestore database - - # Use a collection group query to search across all contributor subcollections - # FIXME: Hardcoded org_name filter, make this dynamic later - contributors_ref = db.collection_group('github_contributors').where("login", "==", login).where("org_name", "==", "2025-Arizona-Opportunity-Hack") - + + # Collection group query across all contributor subcollections — every + # org/hackathon the user contributed to, not just one hardcoded event. + contributors_ref = db.collection_group('github_contributors').where("login", "==", login) + docs = contributors_ref.stream() github_history = [] @@ -182,11 +190,15 @@ def get_github_contributions_for_user(login): org_ref = repo_ref.parent.parent contribution["repo_name"] = repo_ref.id contribution["org_name"] = org_ref.id + # Firestore timestamps aren't JSON-serializable (blocks redis caching) + ts = contribution.get("timestamp") + if hasattr(ts, "isoformat"): + contribution["timestamp"] = ts.isoformat() github_history.append(contribution) logger.info(f"Found {len(github_history)} contributions for user {login}") - return github_history + return github_history @@ -602,6 +614,28 @@ def get_certficate_by_file_id(file_id): adict["id"] = doc.id return adict +def fetch_certificates_by_github_username(github_username): + """All certificate docs stamped with this github_username (lowercased). + + Single-field equality query — auto-indexed, no composite needed (sorting + happens in the service layer). Pre-backfill docs without the field simply + don't match. + """ + if not github_username: + return [] + db = get_db() + certs = [] + try: + docs = db.collection('certificates').where("github_username", "==", github_username).stream() + for doc in docs: + adict = doc.to_dict() + adict["id"] = doc.id + certs.append(adict) + except Exception as e: + logger.warning(f"Failed to fetch certificates for github user {github_username}: {e}") + return certs + + def get_recent_certs_from_db(): db = get_db() # this connects to our Firestore database # Get recent certificates by date diff --git a/db/db.py b/db/db.py index 66aa561..9c2d70a 100644 --- a/db/db.py +++ b/db/db.py @@ -66,6 +66,34 @@ def fetch_users(): def fetch_user_by_github(github_username): return db.fetch_user_by_github(github_username) +# User slugs (vanity profile URLs) +def create_user_slug(slug, user_db_id, previous_slug=None): + return db.create_user_slug(slug, user_db_id, previous_slug=previous_slug) + +def fetch_user_db_id_by_slug(slug): + return db.fetch_user_db_id_by_slug(slug) + +def fetch_user_slugs_by_db_id(user_db_id): + return db.fetch_user_slugs_by_db_id(user_db_id) + +def fetch_user_portfolio_teams(user_db_id): + return db.fetch_user_portfolio_teams(user_db_id) + +def update_user_profile_visibility(user_db_id, visibility): + return db.update_user_profile_visibility(user_db_id, visibility) + +def fetch_public_portfolio_users(): + return db.fetch_public_portfolio_users() + +def update_user_bio_video(user_db_id, url): + return db.update_user_bio_video(user_db_id, url) + +def update_user_login(user_db_id, payload): + return db.update_user_login(user_db_id, payload) + +def update_user_volunteering(user): + return db.update_user_volunteering(user) + # Problem Statements def fetch_problem_statement(id): return db.fetch_problem_statement(id) @@ -82,11 +110,10 @@ def update_problem_statement(problem_statement: ProblemStatement): def delete_problem_statement(id): return db.delete_problem_statement(id) -def insert_helping(problem_statement_id, user: User, mentor_or_hacker, helping_date): - return db.insert_helping(problem_statement_id, user, mentor_or_hacker, helping_date) - -def delete_helping(problem_statement_id, user: User): - return db.delete_helping(problem_statement_id, user) +# NOTE: insert_helping/delete_helping dispatchers were removed — the pair was +# broken (4-arg call into a 3-arg firestore method; delete had no firestore +# impl at all) and its only caller was replaced by +# problem_statements_service.save_helping_status. # Hackathons diff --git a/db/firestore.py b/db/firestore.py index 609e060..63c39c5 100644 --- a/db/firestore.py +++ b/db/firestore.py @@ -227,7 +227,9 @@ def get_user_profile_by_db_id(self, db_id): user = None - if temp is not None: + # Missing doc: real Firestore's to_dict() returns None, but + # MockFirestore returns {} — the exists check covers both. + if temp is not None and getattr(temp, "exists", True): d = temp.to_dict() @@ -290,13 +292,176 @@ def get_user_profile_by_db_id(self, db_id): return user def upsert_profile_metadata(self, user:User): - + db = self.get_db() # this connects to our Firestore database data = user.serialize_profile_metadata() - update_res = db.collection("users").document(user.id).set( data, merge=True) + update_res = db.collection("users").document(user.id).set( data, merge=True) logger.info(f"Update Result: {update_res}") - + return + + # ----------------------- User slugs --------------------------------------- + # user_slugs/{slug} = {user_db_id, is_primary, created_at}. The slug IS the + # doc id, so uniqueness is enforced by DocumentReference.create() (atomic, + # raises AlreadyExists when the slug is taken) — no index, no transaction. + + def create_user_slug(self, slug, user_db_id, previous_slug=None): + """Claim `slug` for `user_db_id`. Returns True on success, False when taken. + + Old slugs are kept as aliases (is_primary=False) so shared links never + break and nobody else can claim them. + """ + db = self.get_db() + slug_ref = db.collection('user_slugs').document(slug) + payload = { + "user_db_id": user_db_id, + "is_primary": True, + "created_at": datetime.now().isoformat() + "Z", + } + try: + if hasattr(slug_ref, "create"): + slug_ref.create(payload) + else: + # MockFirestore has no create(); emulate (non-atomic, test-only) + if slug_ref.get().exists: + raise ValueError("already exists") + slug_ref.set(payload) + except Exception as e: + # google.api_core.exceptions.AlreadyExists in prod; ValueError in tests. + # Re-claiming one of your own aliases is allowed — flip it primary. + existing = slug_ref.get() + existing_dict = existing.to_dict() if getattr(existing, "exists", False) else None + if existing_dict and existing_dict.get("user_db_id") == user_db_id: + slug_ref.set({"is_primary": True}, merge=True) + else: + info(logger, "Slug already taken", slug=slug, error=str(e)) + return False + + db.collection("users").document(user_db_id).set({ + "profile_slug": slug, + "slug_updated_at": datetime.now().isoformat() + "Z", + }, merge=True) + + if previous_slug and previous_slug != slug: + # set(merge) rather than update() so a missing legacy pointer is + # (re)created as an alias instead of erroring. + db.collection('user_slugs').document(previous_slug).set({ + "user_db_id": user_db_id, + "is_primary": False, + }, merge=True) + + return True + + def fetch_user_portfolio_teams(self, db_id): + """Allowlisted team docs for every team the user is on. + + Reads the RAW user doc because User.deserialize drops the `teams` + DocumentReference array. Uses db.get_all (no `in`-query 10-item cap). + """ + db = self.get_db() + user_doc = self.fetch_user_by_db_id_raw(db, db_id) + if user_doc is None or not getattr(user_doc, "exists", False): + return [] + d = user_doc.to_dict() or {} + team_refs = d.get("teams") or [] + if not team_refs: + return [] + + allow = ("name", "slack_channel", "demo_video_url", "devpost_link", + "github_links", "awards", "status", "hackathon_event_id", + "team_number", "active") + teams = [] + try: + for t_doc in db.get_all(team_refs): + if not getattr(t_doc, "exists", False): + continue + t = t_doc.to_dict() or {} + trimmed = {k: t[k] for k in allow if k in t} + trimmed["id"] = t_doc.id + teams.append(trimmed) + except Exception as e: + warning(logger, "Failed to fetch portfolio teams", db_id=db_id, error=str(e)) + return teams + + def fetch_user_db_id_by_slug(self, slug): + """Resolve a slug (primary or alias) to {slug, user_db_id, is_primary} or None.""" + if not slug: + return None + db = self.get_db() + doc = db.collection('user_slugs').document(slug).get() + if doc is None or not getattr(doc, "exists", False): + return None + d = doc.to_dict() or {} + d["slug"] = doc.id + return d + + def fetch_user_slugs_by_db_id(self, user_db_id): + """All slug pointers (primary + aliases) owned by a user.""" + db = self.get_db() + results = [] + try: + docs = db.collection('user_slugs').where("user_db_id", "==", user_db_id).stream() + for doc in docs: + d = doc.to_dict() or {} + d["slug"] = doc.id + results.append(d) + except Exception as e: + warning(logger, "Failed to fetch user slugs", user_db_id=user_db_id, error=str(e)) + return results + + def update_user_profile_visibility(self, user_db_id, visibility): + """Set the portfolio master-visibility field on the user doc.""" + db = self.get_db() + db.collection("users").document(user_db_id).set( + {"profile_visibility": visibility}, merge=True) + return True + + def update_user_volunteering(self, user): + """Targeted write of the volunteering array only (dedicated writer — + deliberately not part of the generic profile upsert).""" + db = self.get_db() + db.collection("users").document(user.id).set( + {"volunteering": user.volunteering or []}, merge=True) + return True + + def update_user_login(self, user_db_id, payload): + """Targeted merge write of login-refresh fields (last_login, and + provider avatar/name when available). Used by the profile GET path so + the propel_id fast-path resolver still refreshes these.""" + allowed = ("last_login", "profile_image", "name", "nickname") + data = {k: v for k, v in (payload or {}).items() if k in allowed and v} + if not data: + return False + db = self.get_db() + db.collection("users").document(user_db_id).set(data, merge=True) + return True + + def update_user_bio_video(self, user_db_id, url): + """Set (or clear) the validated bio_video_url on the user doc.""" + db = self.get_db() + db.collection("users").document(user_db_id).set( + {"bio_video_url": url or ""}, merge=True) + return True + + def fetch_public_portfolio_users(self): + """Slugs of all users who opted into a public (search-indexable) portfolio. + + Single-field equality query — auto-indexed. Only slug + last_login are + projected (sitemap needs nothing else; no PII). + """ + db = self.get_db() + results = [] + try: + docs = db.collection('users').where("profile_visibility", "==", "public").stream() + for doc in docs: + d = doc.to_dict() or {} + slug = d.get("profile_slug") + if not slug: + continue # public requires a slug; skip inconsistent docs + results.append({"slug": slug, "last_login": d.get("last_login")}) + except Exception as e: + warning(logger, "Failed to fetch public portfolio users", error=str(e)) + return results def finish_deleting_user(self, db, user, user_id): @@ -527,44 +692,6 @@ def delete_problem_statement(self, problem_statement_id): return p - def insert_helping(self, problem_statement_id, user: User, mentor_or_hacker): - - my_date = datetime.now() - - to_add = { - "user": user.id, - "slack_user": user.user_id, - "type": mentor_or_hacker, - "timestamp": my_date.isoformat() - } - - db = self.get_db() - - problem_statement_doc = db.collection( - 'problem_statements').document(problem_statement_id) - - ps_dict = problem_statement_doc.get().to_dict() - helping_list = [] - if "helping" in ps_dict: - helping_list = ps_dict["helping"] - logger.debug(f"Helping list: {helping_list}") - - helping_list.append(to_add) - - else: - logger.debug(f"Start Helping list: {helping_list} * New list created for this problem") - helping_list.append(to_add) - - - logger.debug(f"End Helping list: {helping_list}") - problem_result = problem_statement_doc.update({ - "helping": helping_list - }) - - return ProblemStatement.deserialize(ps_dict) - - # ----------------------- Hackathons ------------------------------------------ - def fetch_hackathons(self): hackathons = [] db = self.get_db() # this connects to our Firestore database diff --git a/db/interface.py b/db/interface.py index 64f2ee2..4437e8c 100644 --- a/db/interface.py +++ b/db/interface.py @@ -21,7 +21,13 @@ def __subclasshook__(cls, __subclass: type) -> bool: hasattr(__subclass, 'delete_user_by_user_id') and callable(__subclass.delete_user_by_user_id) and hasattr(__subclass, 'delete_user_by_db_id') and - callable(__subclass.delete_user_by_db_id)) + callable(__subclass.delete_user_by_db_id) and + hasattr(__subclass, 'create_user_slug') and + callable(__subclass.create_user_slug) and + hasattr(__subclass, 'fetch_user_db_id_by_slug') and + callable(__subclass.fetch_user_db_id_by_slug) and + hasattr(__subclass, 'fetch_user_slugs_by_db_id') and + callable(__subclass.fetch_user_slugs_by_db_id)) #Team: #get_team_by_name diff --git a/db/mem.py b/db/mem.py index ae1bd1d..94d32f6 100644 --- a/db/mem.py +++ b/db/mem.py @@ -60,6 +60,8 @@ class InMemoryDatabaseInterface(DatabaseInterface): def __init__(self): super().__init__() + # slug -> {"slug", "user_db_id", "is_primary", "created_at"} + self.user_slugs = {} self.init_users() self.init_problem_statements() self.init_problem_statement_helping() @@ -71,6 +73,80 @@ def __init__(self): self.init_problem_statement_hackathons() self.init_nonprofits() + # ----------------------- User slugs ---------------------------------------- # + + def create_user_slug(self, slug, user_db_id, previous_slug=None): + existing = self.user_slugs.get(slug) + if existing is not None and existing.get("user_db_id") != str(user_db_id): + return False + self.user_slugs[slug] = { + "slug": slug, + "user_db_id": str(user_db_id), + "is_primary": True, + "created_at": datetime.now().isoformat() + "Z", + } + if previous_slug and previous_slug != slug and previous_slug in self.user_slugs: + self.user_slugs[previous_slug]["is_primary"] = False + try: + u = self.users.by.id[int(user_db_id)] + setattr(u, "profile_slug", slug) + except (KeyError, ValueError, TypeError): + pass + return True + + def fetch_user_db_id_by_slug(self, slug): + entry = self.user_slugs.get(slug) + return dict(entry) if entry else None + + def fetch_user_portfolio_teams(self, db_id): + # The in-memory fixture data carries no team references. + return [] + + def update_user_profile_visibility(self, user_db_id, visibility): + try: + u = self.users.by.id[int(user_db_id)] + setattr(u, "profile_visibility", visibility) + return True + except (KeyError, ValueError, TypeError): + return False + + def fetch_public_portfolio_users(self): + results = [] + for u in self.users: + if getattr(u, "profile_visibility", None) == "public" and getattr(u, "profile_slug", None): + results.append({"slug": u.profile_slug, "last_login": getattr(u, "last_login", None)}) + return results + + def update_user_volunteering(self, user): + try: + u = self.users.by.id[int(user.id)] + setattr(u, "volunteering", user.volunteering or []) + return True + except (KeyError, ValueError, TypeError): + return False + + def update_user_login(self, user_db_id, payload): + allowed = ("last_login", "profile_image", "name", "nickname") + try: + u = self.users.by.id[int(user_db_id)] + except (KeyError, ValueError, TypeError): + return False + for k, v in (payload or {}).items(): + if k in allowed and v: + setattr(u, k, v) + return True + + def update_user_bio_video(self, user_db_id, url): + try: + u = self.users.by.id[int(user_db_id)] + setattr(u, "bio_video_url", url or "") + return True + except (KeyError, ValueError, TypeError): + return False + + def fetch_user_slugs_by_db_id(self, user_db_id): + return [dict(e) for e in self.user_slugs.values() if e.get("user_db_id") == str(user_db_id)] + # ----------------------- Users -------------------------------------------- # def fetch_user_by_user_id_raw(self, user_id): diff --git a/firestore.indexes.json b/firestore.indexes.json index 45e84e8..ace7a96 100644 --- a/firestore.indexes.json +++ b/firestore.indexes.json @@ -41,5 +41,16 @@ ] } ], - "fieldOverrides": [] + "fieldOverrides": [ + { + "collectionGroup": "github_contributors", + "fieldPath": "login", + "indexes": [ + { "queryScope": "COLLECTION", "order": "ASCENDING" }, + { "queryScope": "COLLECTION", "order": "DESCENDING" }, + { "queryScope": "COLLECTION", "arrayConfig": "CONTAINS" }, + { "queryScope": "COLLECTION_GROUP", "order": "ASCENDING" } + ] + } + ] } diff --git a/model/user.py b/model/user.py index 2f838bc..3835a72 100644 --- a/model/user.py +++ b/model/user.py @@ -1,5 +1,68 @@ -metadata_list = ["role", "expertise", "education", "company", "why", "shirt_size", "github", "volunteering", "linkedin_url", "instagram_url", "propel_id"] -privacy_fields = ["github", "role", "company", "badges", "expertise", "education", "why", "linkedin_url", "instagram_url", "what", "how", "feedback", "hackathon_history", "praises"] +# --------------------------------------------------------------------------- +# PROFILE FIELD REGISTRY — the single source of truth for flat, user-owned +# profile storage fields. Adding a profile field = ONE entry here (plus a +# privacy_fields entry if it's privacy-gated). The registry generates the +# read set (deserialize), the owner-write set (update_from_metadata), the +# persistence set (serialize_profile_metadata), and the canonical response +# serializer (serialize_profile_fields). api/users/tests/test_field_registry.py +# fails CI when any remaining hand-list drifts out of sync. +# +# Tuple: (name, default, owner_editable, persisted) +# owner_editable — POST /api/users/profile may set it +# persisted — the generic profile upsert writes it +# Derived collections (badges/teams/hackathons/history), identity fields +# (id/user_id/email_address/name/nickname/profile_image/last_login), and +# dedicated-route fields (bio_video_url/profile_slug/profile_visibility) are +# deliberately NOT specs — see PROFILE_READONLY_RESPONSE_FIELDS below. +# --------------------------------------------------------------------------- +PROFILE_FIELD_SPECS = [ + ("role", "", True, True), + ("expertise", "", True, True), + ("education", "", True, True), + ("company", "", True, True), + ("why", "", True, True), + ("shirt_size", "", True, True), + ("github", "", True, True), + ("linkedin_url", "", True, True), + ("instagram_url", "", True, True), + ("street_address", "", True, True), + ("street_address_2", "", True, True), + ("city", "", True, True), + ("state", "", True, True), + ("postal_code", "", True, True), + ("country", "", True, True), + ("want_stickers", "", True, True), + ("bio", "", True, True), + ("headline", "", True, True), + ("portfolio_links", list, True, True), + # System-managed: persisted (the tier-3 resolver backfills it) but a + # POSTed metadata.propel_id must never be accepted from the client. + ("propel_id", None, False, True), + # Dedicated writer (save_volunteering_time -> update_user_volunteering). + # NEVER in the generic upsert write set — a profile save racing a + # volunteering log must not clobber entries. + ("volunteering", list, False, False), +] + + +def _default(dv): + return dv() if callable(dv) else dv + + +# Read set — name kept for backwards compatibility with existing importers +metadata_list = [n for (n, _d, _e, _p) in PROFILE_FIELD_SPECS] +# What POST /profile may set +OWNER_EDITABLE_FIELDS = [n for (n, _d, e, _p) in PROFILE_FIELD_SPECS if e] +# What the generic profile upsert persists +PROFILE_PERSISTED_FIELDS = [n for (n, _d, _e, p) in PROFILE_FIELD_SPECS if p] + +# Read-only extras every canonical profile response also carries. These are +# either identity fields, derived data, or dedicated-route fields. +PROFILE_READONLY_RESPONSE_FIELDS = [ + "id", "user_id", "email_address", "name", "nickname", "profile_image", + "last_login", "history", "bio_video_url", "profile_slug", "profile_visibility", +] +privacy_fields = ["github", "role", "company", "badges", "expertise", "education", "why", "linkedin_url", "instagram_url", "what", "how", "feedback", "hackathon_history", "praises", "bio", "bio_video_url", "portfolio_links", "teams", "certificates", "github_history", "hearts"] # Privacy fields that default to "public" for new/legacy users (everything else defaults private). default_public_privacy_fields = {"praises"} @@ -7,8 +70,25 @@ # Fields that should NEVER be shared publicly regardless of privacy settings pii_fields = ["email_address", "last_login", "propel_id", "volunteering"] -# Fields that are always safe to share publicly (basic profile info) -safe_public_fields = ["name", "nickname", "profile_image", "user_id"] +# Fields that are always safe to share publicly (basic profile info). +# id + profile_slug are the public URL identifiers; the frontend needs both to +# canonicalize /profile/ <-> /u/. +safe_public_fields = ["name", "nickname", "profile_image", "user_id", "id", "profile_slug"] + +# Fields exposed by the internal by-id profile lookup (GET /api/users//profile), +# consumed by team rosters, peer feedback, and admin giveaway UIs to show a +# GitHub username alongside name/avatar — a lower bar than the fully public, +# search-indexable portfolio (get_public_profile_data), which independently +# gates `github` behind that user's own privacy toggle. github is NOT added to +# safe_public_fields itself because get_public_profile_data also reads that +# list unconditionally — doing so would leak github there regardless of privacy. +internal_lookup_fields = safe_public_fields + ["github"] + +# Portfolio visibility master toggle: "private" (default — shareable link, +# noindex, today's per-field rendering) or "public" (search-indexable, listed +# in the sitemap; requires a claimed slug). +DEFAULT_PROFILE_VISIBILITY = "private" +PROFILE_VISIBILITY_VALUES = ("private", "public") def _default_privacy_value(field): @@ -29,6 +109,21 @@ class User: role = "" company = "" why = "" + bio = "" + headline = "" + linkedin_url = "" + instagram_url = "" + street_address = "" + street_address_2 = "" + city = "" + state = "" + postal_code = "" + country = "" + want_stickers = "" + bio_video_url = "" + portfolio_links = [] + profile_slug = None + profile_visibility = None badges = [] teams = [] hackathons = [] @@ -47,7 +142,14 @@ def deserialize(cls, d): u.badges = [] u.hackathons = [] u.teams = [] - u.volunteering = [] + + # Registry-driven: every spec field is ALWAYS set as an instance attr + # (kills the old dir(self) fragility where linkedin_url/instagram_url + # only existed after deserialize happened to set them). + for field_name, default_value, _editable, _persisted in PROFILE_FIELD_SPECS: + setattr(u, field_name, d.get(field_name, _default(default_value))) + + # Identity + dedicated-route fields (hand-written by design) u.id = d['id'] u.email_address = d.get('email_address', '') u.last_login = d.get('last_login') @@ -55,17 +157,9 @@ def deserialize(cls, d): u.profile_image = d.get('profile_image') u.name = d['name'] if 'name' in d else '' u.nickname = d['nickname'] if 'nickname' in d else '' - u.expertise = d['expertise'] if 'expertise' in d else '' - u.education = d['education'] if 'education' in d else '' - u.shirt_size = d['shirt_size'] if 'shirt_size' in d else '' - u.linkedin_url = d['linkedin_url'] if 'linkedin_url' in d else '' - u.instagram_url = d['instagram_url'] if 'instagram_url' in d else '' - u.github = d['github'] if 'github' in d else '' - u.role = d['role'] if 'role' in d else '' - u.company = d['company'] if 'company' in d else '' - u.why = d['why'] if 'why' in d else '' - u.volunteering = d['volunteering'] if 'volunteering' in d else [] - u.propel_id = d['propel_id'] if 'propel_id' in d else None + u.bio_video_url = d.get('bio_video_url', '') + u.profile_slug = d.get('profile_slug') + u.profile_visibility = d.get('profile_visibility') u.privacy_settings = d['privacy_settings'] if 'privacy_settings' in d else {} # Handle history in a generic way @@ -121,23 +215,36 @@ def serialize_hackathons(self): return [h.serialize() for h in self.hackathons] if self.hackathons else [] def serialize_profile_metadata(self): - d = {} - props = dir(self) - for m in metadata_list: - if m in props: - d[m] = getattr(self, m) + """What the generic profile upsert persists. Registry-driven — + `volunteering` is deliberately NOT here (dedicated writer).""" + d = {m: getattr(self, m) for m in PROFILE_PERSISTED_FIELDS} # Add privacy settings d['privacy_settings'] = self.get_privacy_settings() return d - + def update_from_metadata(self, d): - props = dir(self) - for m in metadata_list: - if m in d and m in props: + """Apply a client-submitted metadata dict. Registry-driven — only + OWNER_EDITABLE_FIELDS are accepted (a POSTed propel_id/volunteering + is ignored).""" + for m in OWNER_EDITABLE_FIELDS: + if m in d: setattr(self, m, d[m]) return + def serialize_profile_fields(self): + """THE one flat profile serializer — every canonical profile response + (new stack AND legacy delegates) is built from this.""" + d = {n: getattr(self, n, _default(dv)) for n, dv, _e, _p in PROFILE_FIELD_SPECS} + for n in PROFILE_READONLY_RESPONSE_FIELDS: + d[n] = getattr(self, n, None) + d["profile_visibility"] = d.get("profile_visibility") or DEFAULT_PROFILE_VISIBILITY + if not isinstance(d.get("history"), dict): + d["history"] = {} + d["badges"] = self.badges or [] + d["privacy_settings"] = self.get_privacy_settings() + return d + def get_privacy_settings(self): """Get privacy settings, initializing defaults if needed. @@ -171,8 +278,13 @@ def get_public_profile_data(self): if hasattr(self, field) and getattr(self, field) is not None: public_data[field] = getattr(self, field) - # Fields that need special handling (not simple attribute lookups) - special_fields = {"hackathon_history", "what", "how", "badges"} + # Fields that need special handling (not simple attribute lookups). + # teams/certificates/github_history/hearts are attached by + # users_service (they need extra reads); portfolio_links is list-valued; + # headline rides the bio privacy field below. + special_fields = {"hackathon_history", "what", "how", "badges", + "teams", "certificates", "github_history", "hearts", + "portfolio_links"} # Include privacy-controlled fields only if user made them public for field in privacy_fields: @@ -206,6 +318,18 @@ def get_public_profile_data(self): if privacy_settings.get("badges", False) == "public" and hasattr(self, 'badges') and self.badges: public_data["badges"] = self.badges + # Portfolio links: list-valued, only emitted when non-empty + if privacy_settings.get("portfolio_links", False) == "public" and self.portfolio_links: + public_data["portfolio_links"] = self.portfolio_links + + # Headline rides the bio privacy field (one "About" toggle covers both) + if privacy_settings.get("bio", False) == "public" and self.headline: + public_data["headline"] = self.headline + + # Master visibility toggle is always emitted so the frontend can decide + # robots/index behavior. Default private (never indexed without opt-in). + public_data["profile_visibility"] = self.profile_visibility or DEFAULT_PROFILE_VISIBILITY + # Include privacy settings themselves for the frontend to know what's public public_data["privacy_settings"] = privacy_settings diff --git a/scripts/backfill_certificate_github_usernames.py b/scripts/backfill_certificate_github_usernames.py new file mode 100644 index 0000000..a5100ba --- /dev/null +++ b/scripts/backfill_certificate_github_usernames.py @@ -0,0 +1,95 @@ +#!/usr/bin/env python3 +""" +Backfill script to stamp `github_username` on existing certificate docs. + +New certificates get github_username at generation time +(api/certificates/certificate_service.py::generate_certificate); this script +computes it for pre-existing docs so GET /api/certificates?github= +(and the portfolio page) can find them. + +Usage: + python scripts/backfill_certificate_github_usernames.py # dry-run + python scripts/backfill_certificate_github_usernames.py --apply +""" + +import argparse +import os +import sys + +from dotenv import load_dotenv +load_dotenv() + +# Add parent directory to path to import from project modules +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +# NOTE: user-facing output uses print(), not logging — importing the app +# modules below reconfigures logging (common/log) and disables loggers +# created here, which made an earlier version of this script appear to do +# nothing at all. + + +def main(): + parser = argparse.ArgumentParser(description="Stamp github_username on certificate docs") + parser.add_argument("--apply", action="store_true", help="Write changes (default is dry-run)") + parser.add_argument("--dry-run", action="store_true", help="Log what would change (default)") + args = parser.parse_args() + apply_changes = args.apply and not args.dry_run + + from common.utils.firebase import get_db + from api.certificates.certificate_service import _extract_github_username + + db = get_db() + docs = list(db.collection("certificates").stream()) + print(f"Scanning {len(docs)} certificate docs (mode: {'APPLY' if apply_changes else 'DRY-RUN'})") + + # Second-chance matching: many certs carry a personal git email (not the + # GitHub noreply form). Join those against users.email_address -> github. + email_to_github = {} + try: + for udoc in db.collection("users").where("github", ">", "").stream(): + u = udoc.to_dict() or {} + email = (u.get("email_address") or "").strip().lower() + github = (u.get("github") or "").strip().lower() + if email and github: + email_to_github[email] = github + print(f"Loaded {len(email_to_github)} users with a github username for email matching") + except Exception as e: + print(f"WARNING could not load users for email matching: {e}") + + stamped = 0 + already = 0 + unparseable = [] + for doc in docs: + cert = doc.to_dict() or {} + if cert.get("github_username"): + already += 1 + continue + + username = _extract_github_username(cert.get("author_email"), cert.get("author_name")) + if not username: + username = email_to_github.get((cert.get("author_email") or "").strip().lower()) + if not username: + unparseable.append({ + "doc_id": doc.id, + "author_name": cert.get("author_name"), + "author_email": cert.get("author_email"), + }) + continue + + stamped += 1 + if apply_changes: + doc.reference.update({"github_username": username}) + print(f"Stamped {doc.id}: github_username={username}") + else: + print(f"[dry-run] Would stamp {doc.id}: github_username={username}") + + print(f"Done. stamped={stamped} already_stamped={already} unparseable={len(unparseable)}") + for entry in unparseable: + print(f"WARNING unparseable cert {entry['doc_id']}: name={entry['author_name']!r} email={entry['author_email']!r}") + + if not apply_changes: + print("Dry-run only — re-run with --apply to write changes.") + + +if __name__ == "__main__": + main() diff --git a/services/giveaway_service.py b/services/giveaway_service.py index a87a16f..1c870f4 100644 --- a/services/giveaway_service.py +++ b/services/giveaway_service.py @@ -74,8 +74,8 @@ def get_all_giveaways(): if not user_id: continue if user_id not in giveaways: - from api.messages.messages_service import get_user_by_id_old - user = get_user_by_id_old(user_id) + from services.users_service import get_profile_by_db_id + user = get_profile_by_db_id(user_id) giveaway["user"] = user if user is not None else {} giveaways[user_id] = giveaway diff --git a/services/hearts_service.py b/services/hearts_service.py index 71ec0f7..004176f 100644 --- a/services/hearts_service.py +++ b/services/hearts_service.py @@ -33,7 +33,59 @@ from services.users_service import save_user -def get_hearts_for_all_users(): +# Heart tiers — thresholds mirror the frontend's src/lib/heartTiers.js. +# Keep both in lockstep if rewards change. +HEART_TIERS = [ + ("Diamond", 48), + ("Platinum", 24), + ("Gold", 10), + ("Silver", 5), + ("Bronze", 2), +] + + +def get_heart_tier(total): + """Highest tier name reached for a heart total, or None below Bronze.""" + try: + total = float(total or 0) + except (TypeError, ValueError): + return None + for name, threshold in HEART_TIERS: + if total >= threshold: + return name + return None + + +def get_hearts_summary(history): + """Pure summary of a user's hearts from their `history` map. + + Sum rule = the explicit `what` + `how` maps only — matches the frontend + HeartGauge and stays deterministic if unrelated keys (certificates, etc.) + are added to history later. Note the leaderboard sum + (get_hearts_leaderboard) instead skips keys containing "certificates" — + TODO: unify it onto this function. + """ + breakdown = {"what": {}, "how": {}} + total = 0.0 + for section in ("what", "how"): + values = (history or {}).get(section) or {} + if not isinstance(values, dict): + continue + for key, value in values.items(): + try: + amount = float(value or 0) + except (TypeError, ValueError): + continue + breakdown[section][key] = amount + total += amount + return { + "total": total, + "breakdown": breakdown, + "tier": get_heart_tier(total), + } + + +def get_hearts_for_all_users(): users = fetch_users() result = [] @@ -212,6 +264,13 @@ def give_hearts_to_user(slack_user_id, amount, reasons, create_certificate_image for reason in reasons: add_hearts_for_user(id, amount, reason) + # New hearts must show up on the public portfolio promptly + try: + from services.users_service import clear_portfolio_caches + clear_portfolio_caches(id) + except Exception as e: + warning(logger, "Failed to clear portfolio caches after hearts", error=str(e)) + reasons_string = ", ".join(get_reason_pretty(reason) for reason in reasons) plural = "s" if amount > 1 else "" diff --git a/services/news_service.py b/services/news_service.py index 8565106..7f79d16 100644 --- a/services/news_service.py +++ b/services/news_service.py @@ -235,6 +235,20 @@ def get_all_praises(): @cached(cache=TTLCache(maxsize=100, ttl=600), lock=threading.Lock()) def get_praises_about_user(user_id): + # Privacy gate: praises key off raw Slack ids, but the receiving user may + # have set their "praises" privacy field to private. Users not in our DB + # (or with no explicit setting) default to public — matches + # model/user.py default_public_privacy_fields. + try: + receiver = get_user_by_user_id(user_id) + if receiver: + praises_setting = (receiver.get("privacy_settings") or {}).get("praises", "public") + if praises_setting != "public": + logger.info(f"Praises for {user_id} are private — returning empty list") + return Message([]) + except Exception as e: + logger.warning(f"Praise privacy check failed for {user_id}: {e}") + results = get_praises_by_user_id(user_id) slack_ids = set() diff --git a/services/problem_statements_service.py b/services/problem_statements_service.py index 93aea12..8133587 100644 --- a/services/problem_statements_service.py +++ b/services/problem_statements_service.py @@ -5,8 +5,8 @@ from common.utils.oauth_providers import extract_slack_user_id, is_slack_user_id from model.problem_statement import ProblemStatement from model.user import User -from db.db import (delete_helping, fetch_hackathon, fetch_problem_statements, - insert_helping, delete_problem_statement, fetch_problem_statement, +from db.db import (fetch_hackathon, fetch_problem_statements, get_db, + delete_problem_statement, fetch_problem_statement, insert_problem_statement, update_problem_statement, insert_problem_statement_hackathon, update_problem_statement_hackathons) import logging @@ -119,89 +119,123 @@ def update_problem_statement_fields(d): @limits(calls=100, period=ONE_MINUTE) def save_helping_status(propel_user_id, d): - info(logger, "save_helping_status", propel_user_id=propel_user_id, data=d) - user = users_service.get_user_from_propel_user_id(propel_user_id) - - slack_message = None - - # Do the actual data wrangling - problem_statement = save_user_helping_status(user, d) + """Toggle the caller's "helping" status on a problem statement. - problem_statement_title = problem_statement.title - problem_statement_slack_channel = problem_statement.slack_channel + Port of the legacy messages_service.save_helping_status_old body (the + rich version: Slack mention + channel invite for Slack logins, profile + link + Slack-join email CTA for non-Slack logins, npo suffix), with + identity via the 3-tier resolver so a broken OAuth token can't 404 the + toggle. Returns a plain dict, or None when identity can't be resolved. + """ + info(logger, "save_helping_status", propel_user_id=propel_user_id, data=d) - helping_status = d["status"] # helping or not_helping + user, user_id = users_service._resolve_and_ensure_user(propel_user_id) + if user is None or not getattr(user, "id", None): + warning(logger, "Could not resolve user for helping toggle", propel_user_id=propel_user_id) + return None + helping_status = d["status"] # helping or not_helping problem_statement_id = d["problem_statement_id"] + mentor_or_hacker = d["type"] + npo_id = d.get("npo_id", "") - npo_id = d["npo_id"] if "npo_id" in d else "" + to_add = { + "user": user.id, + "slack_user": user.user_id, + "type": mentor_or_hacker, + "timestamp": datetime.now().isoformat(), + } - mentor_or_hacker = d["type"] + db = get_db() + problem_statement_doc = db.collection('problem_statements').document(problem_statement_id) + ps_dict = problem_statement_doc.get().to_dict() + # Missing doc: real Firestore yields None, MockFirestore yields {} — treat both as unknown + if not ps_dict: + warning(logger, "Helping toggle on unknown problem statement", problem_statement_id=problem_statement_id) + return None - url = "" - if npo_id == "": - url = f"for project https://ohack.dev/project/{problem_statement_id}" + helping_list = ps_dict.get("helping", []) + if "helping" == helping_status: + helping_list.append(to_add) else: - url = f"for nonprofit https://ohack.dev/nonprofit/{npo_id} on project https://ohack.dev/project/{problem_statement_id}" + # NOTE: the legacy body used `d['user'] not in user.id` — a substring + # test that could remove other users' entries. Exact match only. + helping_list = [h for h in helping_list if h.get('user') != user.id] + problem_statement_doc.update({"helping": helping_list}) - slack_user_id = None + # Project pages read helping through the messages-side caches try: - # Extract raw Slack user ID for Slack API calls (handles OAuth formats) - if is_slack_user_id(user.user_id): - slack_user_id = extract_slack_user_id(user.user_id) - invite_user_to_channel(user_id=slack_user_id, - channel_name=problem_statement_slack_channel) - except Exception: - pass # Don't return error if slack invite fails. - - - if slack_user_id is not None: - try: - slack_message = f"<@{slack_user_id}>" + from api.messages import messages_service + messages_service.clear_cache() + except Exception as e: + warning(logger, "Failed to clear messages caches after helping toggle", error=str(e)) - if "helping" == helping_status: - slack_message = f"{slack_message} is helping as a *{mentor_or_hacker}* on *{problem_statement_title}* {url}" - else: - slack_message = f"{slack_message} is _no longer able to help_ on *{problem_statement_title}* {url}" + try: + send_slack_audit(action="helping", message=user.user_id, payload=to_add) + except Exception: + pass - send_slack(message=slack_message, - channel=problem_statement_slack_channel) - except: - pass # Don't return error if slack message fails. + # Determine how to identify this user in the Slack post. + # Slack logins get a real <@Uxxx> mention + auto-invite to the project + # channel. Non-Slack logins (Google, etc.) fall back to their display name + # so we don't render a broken "@oauth2" mention, and get a follow-up email + # asking them to join the Slack workspace. + display_name = (user.name or user.nickname or user.email_address or "A volunteer").strip() + profile_url = f"https://ohack.dev/profile/{user.id}" if user.id else None + + if is_slack_user_id(user.user_id): + slack_user_id = extract_slack_user_id(user.user_id) + mention = f"<@{slack_user_id}> (<{profile_url}|profile>)" if profile_url else f"<@{slack_user_id}>" + is_slack_login = True + else: + slack_user_id = None + mention = f"<{profile_url}|{display_name}>" if profile_url else display_name + is_slack_login = False - return problem_statement + problem_statement_title = ps_dict.get("title", "") -def save_user_helping_status(user: User, d): + if "slack_channel" in ps_dict: + problem_statement_slack_channel = ps_dict["slack_channel"] - info(logger, "save_user_helping_status", user=user.serialize(), data=d) - helping_status = d["status"] # helping or not_helping - - problem_statement_id = d["problem_statement_id"] - mentor_or_hacker = d["type"] + project_link = f"" + suffix = f" for " if npo_id else "" - helping_date = datetime.now().isoformat() - - to_add = { - "user": user.id, - "slack_user": user.user_id, - "type": mentor_or_hacker, - "timestamp": helping_date - } - - try: - send_slack_audit(action="helping", message=user.user_id, payload=to_add) - except Exception: - pass + if "helping" == helping_status: + slack_message = f"{mention} is helping as a *{mentor_or_hacker}* on *{project_link}*{suffix}" + else: + slack_message = f"{mention} is _no longer able to help_ on *{project_link}*{suffix}" - problem_statement: ProblemStatement | None = None - - if "helping" == helping_status: - problem_statement = insert_helping(problem_statement_id, user, mentor_or_hacker, helping_date) - else: - problem_statement = delete_helping(problem_statement_id, user) + if is_slack_login and slack_user_id: + try: + invite_user_to_channel(user_id=slack_user_id, + channel_name=problem_statement_slack_channel) + except Exception as e: + warning(logger, "invite_user_to_channel failed", slack_user_id=slack_user_id, error=str(e)) - return problem_statement + try: + send_slack(message=slack_message, channel=problem_statement_slack_channel) + except Exception as e: + warning(logger, "helping Slack post failed", error=str(e)) + + # For non-Slack users signing up to help, email them a Slack join CTA so + # their project team can actually reach them. Swallow errors so a Resend + # outage never breaks the help toggle. + if not is_slack_login and helping_status == "helping" and user.email_address: + try: + from services.email_service import send_project_help_slack_invite_email + send_project_help_slack_invite_email( + name=user.name or user.nickname, + email=user.email_address, + problem_statement_title=ps_dict.get("title"), + mentor_or_hacker=mentor_or_hacker, + npo_id=npo_id or None, + problem_statement_id=problem_statement_id, + ) + except Exception as e: + warning(logger, "send_project_help_slack_invite_email failed", error=str(e)) + + return {"message": "Updated helping status"} @limits(calls=100, period=ONE_MINUTE) diff --git a/services/user_slug_service.py b/services/user_slug_service.py new file mode 100644 index 0000000..4a694c7 --- /dev/null +++ b/services/user_slug_service.py @@ -0,0 +1,124 @@ +"""Vanity profile slugs (portfolio URLs like ohack.dev/u/). + +Slugs live in the `user_slugs` collection where the slug IS the document id — +uniqueness is enforced atomically by DocumentReference.create(). Old slugs stay +behind as aliases (is_primary=False) so shared links never break and nobody can +claim a slug you previously used (prevents slug-jacking). +""" +import re +from datetime import datetime, timedelta + +from common.log import get_logger, info, warning + +logger = get_logger("user_slug_service") + +SLUG_PATTERN = re.compile(r"^[a-z0-9](?:[a-z0-9-]{1,28}[a-z0-9])?$") +# users.id values are uuid.uuid1().hex — a slug must never shadow one +DB_ID_PATTERN = re.compile(r"^[0-9a-f]{32}$") + +MAX_SLUGS_PER_USER = 5 +SLUG_CHANGE_COOLDOWN_HOURS = 24 + +RESERVED_SLUGS = frozenset({ + "about", "admin", "api", "app", "blog", "cert", "certs", "certificates", + "community", "community-champions", "contact", "cdn", "dev", "docs", + "donate", "edit", "faq", "feedback", "giveaway", "hack", "hackathon", + "hackathons", "hacker", "hackers", "help", "hearts", "home", "index", + "internships", "jobs", "join", "judge", "judges", "leaderboard", "legal", + "letters", "login", "logout", "me", "media", "mentor", "mentors", + "myfeedback", "myprofile", "new", "news", "nonprofit", "nonprofits", + "null", "office-hours", "ohack", "onboarding", "opportunity-hack", + "portfolio", "praise", "praises", "privacy", "profile", "profiles", + "project", "projects", "search", "settings", "signup", "sitemap", + "slack", "sponsor", "sponsors", "staff", "static", "store", "support", + "team", "teams", "terms", "test", "undefined", "user", "users", "u", + "volunteer", "volunteers", "www", +}) + + +def normalize_slug(slug): + return (slug or "").strip().lower() + + +def validate_slug(slug): + """Returns (is_valid, reason). `slug` must already be normalized.""" + if not slug: + return False, "Slug is required" + if len(slug) < 3 or len(slug) > 30: + return False, "Slug must be 3-30 characters" + if not SLUG_PATTERN.match(slug): + return False, "Use lowercase letters, numbers, and hyphens (no leading/trailing hyphen)" + if slug in RESERVED_SLUGS: + return False, "This name is reserved" + if DB_ID_PATTERN.match(slug): + return False, "This name is reserved" + return True, None + + +def check_slug_availability(slug): + """Availability check for the editor's live validation.""" + from db.db import fetch_user_db_id_by_slug + + normalized = normalize_slug(slug) + valid, reason = validate_slug(normalized) + if not valid: + return {"slug": normalized, "valid": False, "available": False, "reason": reason} + + existing = fetch_user_db_id_by_slug(normalized) + if existing is not None: + return {"slug": normalized, "valid": True, "available": False, "reason": "Already taken"} + return {"slug": normalized, "valid": True, "available": True, "reason": None} + + +def claim_profile_slug(propel_id, slug): + """Claim (or change to) `slug` for the authenticated user. + + Returns (payload, http_status). Old slug is kept as an alias. + """ + from db.db import create_user_slug, fetch_user_db_id_by_slug, fetch_user_slugs_by_db_id + from services.users_service import _resolve_and_ensure_user, clear_portfolio_caches + + normalized = normalize_slug(slug) + valid, reason = validate_slug(normalized) + if not valid: + return {"error": reason}, 400 + + user, _user_id = _resolve_and_ensure_user(propel_id) + if user is None or not getattr(user, "id", None): + return {"error": "Could not resolve your account"}, 404 + + existing = fetch_user_db_id_by_slug(normalized) + if existing is not None and existing.get("user_db_id") != user.id: + return {"error": "Already taken"}, 409 + + my_slugs = fetch_user_slugs_by_db_id(user.id) + previous_primary = next((s.get("slug") for s in my_slugs if s.get("is_primary")), None) + + if previous_primary == normalized: + return {"slug": normalized, "previous_slug": None, "message": "Already your URL"}, 200 + + if previous_primary is not None: + # Rename path: throttle to one change per 24h, cap total aliases. + owned_slugs = {s.get("slug") for s in my_slugs} + if normalized not in owned_slugs and len(my_slugs) >= MAX_SLUGS_PER_USER: + return {"error": f"You can hold at most {MAX_SLUGS_PER_USER} URLs (old ones stay as aliases)"}, 400 + newest = max((s.get("created_at") or "" for s in my_slugs), default="") + if newest: + try: + newest_dt = datetime.fromisoformat(newest.replace("Z", "")) + if datetime.now() - newest_dt < timedelta(hours=SLUG_CHANGE_COOLDOWN_HOURS): + return {"error": "You can change your URL once every 24 hours"}, 429 + except ValueError: + pass + + created = create_user_slug(normalized, user.id, previous_slug=previous_primary) + if not created: + return {"error": "Already taken"}, 409 + + info(logger, "Slug claimed", slug=normalized, user_db_id=user.id, previous=previous_primary) + try: + clear_portfolio_caches(user.id) + except Exception as e: + warning(logger, "Failed to clear portfolio caches after slug claim", error=str(e)) + + return {"slug": normalized, "previous_slug": previous_primary}, 200 diff --git a/services/users_service.py b/services/users_service.py index 726071a..5bd1a88 100644 --- a/services/users_service.py +++ b/services/users_service.py @@ -4,11 +4,13 @@ from ratelimit import limits import requests from common.utils.slack import send_slack_audit, get_slack_user_by_email -from model.user import User +from model.user import User, internal_lookup_fields from db.db import delete_user_by_db_id, delete_user_by_user_id, fetch_user_by_user_id, fetch_user_by_db_id, fetch_user_by_propel_id, fetch_user_by_email, fetch_users, insert_user, update_user, get_user_profile_by_db_id, upsert_profile_metadata, fetch_user_by_github import pytz from cachetools import cached, LRUCache, TTLCache from cachetools.keys import hashkey +from common.utils.redis_cache import redis_cached +from common.utils.validators import sanitize_string, validate_url from common.log import get_logger, info, debug, warning, error, exception import uuid @@ -31,8 +33,33 @@ # USER_ID_PREFIX is now imported from oauth_providers module for consistency # Note: This maintains backward compatibility with Slack-specific code -def clear_cache(): +def clear_cache(): get_profile_metadata.cache_clear() + clear_portfolio_caches() + + +def clear_portfolio_caches(db_id=None): + """Invalidate the public-portfolio caches after any profile-affecting write. + + Redis prefixes are cleared wholesale (clear_pattern) — entries are few and + per-key deletion would need the exact hashed cache_key. + """ + from common.utils.redis_cache import clear_pattern + for prefix in ("portfolio:profile", "portfolio:resolve", "portfolio:teams", "portfolio:sitemap"): + try: + clear_pattern(f"{prefix}:*") + except Exception as e: + warning(logger, "Failed to clear portfolio cache", prefix=prefix, error=str(e)) + + # The profile EDITOR reads through the legacy /api/messages/profile path, + # which has its own TTL caches — slug/visibility/bio-video writes must + # flush those too or the editor shows stale data for up to 10 minutes. + try: + from api.messages import messages_service + messages_service.get_profile_metadata_old.cache_clear() + messages_service.get_user_by_id_old.cache_clear() + except Exception as e: + warning(logger, "Failed to clear legacy profile caches", error=str(e)) def finish_saving_insert( user_id=None, @@ -307,8 +334,13 @@ def get_profile_by_db_id(id): res = None - # Only keep these fields since this is a public api - fields = ["name", "profile_image", "user_id", "nickname", "github", "propel_id"] #TODO: Wait. We are getting extended profile data (e.g. hackathons above just to pitch it out here?) + # internal_lookup_fields = safe_public_fields + github. github is included + # here (unlike the fully public/privacy-filtered portfolio) because this + # route backs internal features — team rosters, peer feedback, admin + # giveaways — that have always shown a participant's GitHub username. + # propel_id is PII and everything else beyond this list is privacy-gated; + # those need get_privacy_filtered_profile_by_db_id instead. + fields = list(internal_lookup_fields) if u is not None: # Check if the field is in the response first @@ -319,43 +351,61 @@ def get_profile_by_db_id(id): logger.debug(f"Get User By ID Result: {res}") return res +def _refresh_login_details(user, propel_id): + """Best-effort login refresh (last_login + provider avatar/name). + + The 3-tier resolver's fast path (stored propel_id) makes no external call + and therefore doesn't refresh these like save_user used to. The OAuth + round-trip here is strictly optional — when it's down we still stamp + last_login and move on. Never raises. + """ + payload = {"last_login": datetime.now().isoformat() + "Z"} + try: + _email, user_id, _last_login, profile_image, name, nickname = \ + get_propel_user_details_by_id(propel_id) + if user_id: + payload.update({ + "profile_image": profile_image, + "name": name, + "nickname": nickname, + }) + except Exception as e: + warning(logger, "Login-detail refresh skipped (provider unavailable)", + propel_id=propel_id, error=str(e)) + try: + from db.db import update_user_login + update_user_login(user.id, payload) + except Exception as e: + warning(logger, "Failed to write login refresh", propel_id=propel_id, error=str(e)) + + # 10 minute cache for 100 objects LRU @cached(cache=TTLCache(maxsize=100, ttl=600), lock=threading.Lock()) @limits(calls=100, period=ONE_MINUTE) def get_profile_metadata(propel_id): + """Own-profile read. Identity via the 3-tier resolver — a broken OAuth + provider token can no longer 404 the profile page (the old path depended + solely on the live OAuth round-trip).""" logger.debug("Profile Metadata") - - email, user_id, last_login, profile_image, name, nickname = get_propel_user_details_by_id(propel_id) - + + user, user_id = _resolve_and_ensure_user(propel_id) + if user is None or not getattr(user, "id", None): + warning(logger, "Could not resolve user for profile read", propel_id=propel_id) + return None + send_slack_audit( - action="login", message=f"User went to profile: {user_id} with email: {email}") - + action="login", message=f"User went to profile: {user_id} with email: {user.email_address}") - logger.debug(f"Account Details:\ - \nEmail: {email}\nUser ID: {user_id}\n\ - Last Login:{last_login}\ - Image:{profile_image}") - - # Call firebase to see if account exists and save these details - db_id = save_user( - user_id=user_id, - email=email, - last_login=last_login, - profile_image=profile_image, - name=name, - nickname=nickname, - propel_id=propel_id - ) + _refresh_login_details(user, propel_id) - if db_id is None: - warning(logger, "save_user returned None — PropelAuth provided empty values", propel_id=propel_id) + # Re-read through the profile loader (resolves badge refs) and serialize + full_user = get_history(user.id) + if full_user is None: return None - - # Get all of the user history and profile data from the DB - response = get_history(db_id.id) + response = build_profile_response(full_user) logger.debug(f"get_profile_metadata {response}") - return response #TODO: Breaking API change + return response # Caching is not needed because the parent method already is caching @limits(calls=100, period=ONE_MINUTE) @@ -366,37 +416,108 @@ def get_history(db_id): logger.debug(f"RESULT\n{result}") return result + +def build_profile_response(user): + """THE canonical own-profile response dict. Both /api/users/profile and + the legacy /api/messages/profile delegates serve exactly this (the legacy + route wraps it in its historical {"text": ...} envelope). + + hackathons/hackathon_history are attendance-derived from the volunteers + collection (the documented source of truth) — NOT the deprecated + users.hackathons ref array. + """ + d = user.serialize_profile_fields() + try: + from services.volunteers_service import get_user_hackathon_attendance + hackathons = get_user_hackathon_attendance( + user_id=getattr(user, "user_id", None), + email=getattr(user, "email_address", None), + ) + except Exception as e: + warning(logger, "Failed to load hackathon attendance for profile response", + db_id=getattr(user, "id", None), error=str(e)) + hackathons = [] + d["hackathons"] = hackathons + d["hackathon_history"] = hackathons + return d + +MAX_BIO_LENGTH = 2000 +MAX_HEADLINE_LENGTH = 80 +MAX_PORTFOLIO_LINKS = 10 +MAX_LINK_LABEL_LENGTH = 40 +MAX_LINK_URL_LENGTH = 300 + + +def _sanitize_portfolio_metadata(metadata): + """Sanitize the portfolio-specific metadata fields in place. + + bio/headline are length-capped; portfolio_links is rebuilt as a clean + [{label, url}] array (invalid URLs dropped, https:// auto-prefixed). + """ + if "bio" in metadata: + metadata["bio"] = sanitize_string(metadata.get("bio") or "", MAX_BIO_LENGTH) + if "headline" in metadata: + metadata["headline"] = sanitize_string(metadata.get("headline") or "", MAX_HEADLINE_LENGTH) + if "portfolio_links" in metadata: + raw = metadata.get("portfolio_links") + links = [] + if isinstance(raw, list): + for item in raw[:MAX_PORTFOLIO_LINKS]: + if not isinstance(item, dict): + continue + url = (item.get("url") or "").strip() + if not url or any(c.isspace() for c in url): + continue # validate_url's urlparse check lets spaces through + if not url.lower().startswith(("http://", "https://")): + url = f"https://{url}" + if not validate_url(url): + continue + links.append({ + "label": sanitize_string(item.get("label") or "", MAX_LINK_LABEL_LENGTH), + "url": url[:MAX_LINK_URL_LENGTH], + }) + metadata["portfolio_links"] = links + return metadata + + def save_profile_metadata(propel_id, json): + """Own-profile write. Identity via the 3-tier resolver (lazily creates the + doc for brand-new users) — no longer blocked by a broken OAuth token.""" send_slack_audit(action="save_profile_metadata", message="Saving", payload=json) - oauth_user = get_oauth_user_from_propel_user_id(propel_id) - if oauth_user is None: - warning(logger, "Could not get OAuth user from PropelAuth", propel_id=propel_id) + if not json or "metadata" not in json: + warning(logger, "save_profile_metadata called without metadata", propel_id=propel_id) return None - user_id = oauth_user["sub"] + user, user_id = _resolve_and_ensure_user(propel_id) + if user is None or not getattr(user, "id", None): + warning(logger, "Could not resolve user for profile save", propel_id=propel_id) + return None logger.info(f"Save Profile Metadata for {user_id} {json}") json = json["metadata"] - # See if the user exists - user = fetch_user_by_user_id(user_id) - if user is None: - return - else: - logger.info(f"User exists: {user.id}") - user.update_from_metadata(json) - upsert_profile_metadata(user) + _sanitize_portfolio_metadata(json) + user.update_from_metadata(json) + upsert_profile_metadata(user) - # Clear cache for get_profile_metadata - get_profile_metadata.cache_clear() + # Clear cache for get_profile_metadata + get_profile_metadata.cache_clear() + clear_portfolio_caches(user.id) - return user #TODO: Breaking API change + return build_profile_response(user) def get_user_by_db_id(id): - return fetch_user_by_db_id(id) + user = fetch_user_by_db_id(id) + if user is not None: + return user + # Accept vanity slugs anywhere a db id is accepted (public routes) + resolved = resolve_user_db_id(id) + if resolved and resolved != id: + return fetch_user_by_db_id(resolved) + return None def get_slack_user_id_by_github(github_username): """Look up a Slack user ID given a GitHub username.""" @@ -589,7 +710,10 @@ def _clean_hours(value): entry["manual"] = True user.volunteering.append(entry) - upsert_profile_metadata(user) + # Targeted write — volunteering is NOT in the generic profile write set + # (a concurrent profile save must never clobber a volunteering log). + from db.db import update_user_volunteering + update_user_volunteering(user) # Clear cache for get_profile_metadata get_profile_metadata.cache_clear() @@ -732,6 +856,7 @@ def update_privacy_settings(propel_id, data): # Clear cache for get_profile_metadata get_profile_metadata.cache_clear() + clear_portfolio_caches(user.id) return user.get_privacy_settings() @@ -739,6 +864,33 @@ def update_privacy_settings(propel_id, data): PUBLIC_PRAISES_PREVIEW_LIMIT = 3 +def resolve_user_db_id(id_or_slug): + """Resolve a /profile URL param (db id OR vanity slug) to a db id. + + Direct doc ids always win (legacy links); the slug pointer collection is + only consulted when no user doc has that id. Returns None when neither + resolves. + """ + if not id_or_slug: + return None + from db.db import fetch_user_db_id_by_slug + pointer = fetch_user_db_id_by_slug(str(id_or_slug).strip().lower()) + if pointer and pointer.get("user_db_id"): + return pointer["user_db_id"] + return None + + +def _get_user_profile_by_db_id_or_slug(id_or_slug): + """get_user_profile_by_db_id that transparently accepts a vanity slug.""" + user = get_user_profile_by_db_id(id_or_slug) + if user is not None: + return user + resolved = resolve_user_db_id(id_or_slug) + if resolved and resolved != id_or_slug: + return get_user_profile_by_db_id(resolved) + return None + + def _attach_hackathon_history(user, public_data, privacy_settings): """Replace the legacy hackathons array with attendance derived from volunteers.""" if privacy_settings.get("hackathon_history") != "public": @@ -759,6 +911,119 @@ def _attach_hackathon_history(user, public_data, privacy_settings): public_data["hackathons"] = history +def _trim_event_for_portfolio(event): + if not event: + return None + keep = ("event_id", "title", "start_date", "end_date", "location", "image_url") + return {k: event.get(k) for k in keep if event.get(k) is not None} + + +@redis_cached(prefix="portfolio:teams", ttl=900) +def _fetch_portfolio_teams_cached(db_id): + """User's teams (allowlisted) with a trimmed `event` object attached.""" + from db.db import fetch_user_portfolio_teams + from common.utils.firebase import get_hackathon_by_event_id + + teams = fetch_user_portfolio_teams(db_id) + if not teams: + return [] + + events = {} + for event_id in {t.get("hackathon_event_id") for t in teams if t.get("hackathon_event_id")}: + try: + events[event_id] = _trim_event_for_portfolio(get_hackathon_by_event_id(event_id)) + except Exception as e: + warning(logger, "Failed to enrich portfolio team event", event_id=event_id, error=str(e)) + + for team in teams: + team["event"] = events.get(team.get("hackathon_event_id")) + + # Newest event first; teams with no event date sink to the end + teams.sort(key=lambda t: ((t.get("event") or {}).get("start_date") or ""), reverse=True) + return teams + + +def _attach_teams(user, public_data, privacy_settings): + """Attach hackathon teams (demo videos, repos, awards) when opted in.""" + if privacy_settings.get("teams") != "public": + return + try: + public_data["teams"] = _fetch_portfolio_teams_cached(user.id) + except Exception as e: + warning(logger, "Failed to load teams for public profile", + db_id=getattr(user, 'id', None), exc_info=e) + + +def _attach_certificates(user, public_data, privacy_settings): + """Attach heart certificates (from history) + git-fame GitHub certificates.""" + if privacy_settings.get("certificates") != "public": + return + + cdn_server = os.getenv("CDN_SERVER", "https://cdn.ohack.dev") + certificates = {"heart_certificates": [], "github_certificates": []} + + history = getattr(user, "history", {}) or {} + for entry in (history.get("certificates") or []): + if isinstance(entry, str): + certificates["heart_certificates"].append({"url": f"{cdn_server}/certificates/{entry}"}) + elif isinstance(entry, dict): + url = entry.get("url") + if not url and entry.get("filename"): + url = f"{cdn_server}/certificates/{entry['filename']}" + if not url: + continue + certificates["heart_certificates"].append({ + "url": url, + "timestamp": entry.get("timestamp"), + "reasons": entry.get("reasons"), + "hearts": entry.get("hearts"), + }) + + github = getattr(user, "github", "") or "" + if github: + try: + from api.certificates.certificate_service import get_certificates_by_github_username + cert_allow = ("certificate_url", "date", "repository_url", "stats", "file_id") + for cert in (get_certificates_by_github_username(github) or []): + # Allowlist — author_email must never leak to the public payload + certificates["github_certificates"].append( + {k: cert.get(k) for k in cert_allow if cert.get(k) is not None} + ) + except Exception as e: + warning(logger, "Failed to load GitHub certificates for public profile", + github=github, exc_info=e) + + if certificates["heart_certificates"] or certificates["github_certificates"]: + public_data["certificates"] = certificates + + +def _attach_github_contributions(user, public_data, privacy_settings): + """Attach stored GitHub contribution history when opted in.""" + if privacy_settings.get("github_history") != "public": + return + github = getattr(user, "github", "") or "" + if not github: + return + try: + from common.utils.firebase import get_github_contributions_for_user + public_data["github_history"] = get_github_contributions_for_user(github) + except Exception as e: + warning(logger, "Failed to load GitHub contributions for public profile", + github=github, exc_info=e) + + +def _attach_hearts(user, public_data, privacy_settings): + """Attach the hearts total + tier summary when opted in. Zero extra reads.""" + if privacy_settings.get("hearts") != "public": + return + try: + from services.hearts_service import get_hearts_summary + public_data["hearts"] = get_hearts_summary(getattr(user, "history", {}) or {}) + except Exception as e: + warning(logger, "Failed to compute hearts summary for public profile", + db_id=getattr(user, 'id', None), exc_info=e) + + def _attach_received_praises(user, public_data, privacy_settings): """Attach praises_count + praises_recent when the user opts in.""" if privacy_settings.get("praises") != "public": @@ -786,9 +1051,9 @@ def _attach_received_praises(user, public_data, privacy_settings): def get_privacy_filtered_profile_by_db_id(db_id): - """Get privacy-filtered profile data by database ID""" + """Get privacy-filtered profile data by database ID or vanity slug""" logger.debug(f"Get Privacy-Filtered Profile By DB ID: {db_id}") - user = get_user_profile_by_db_id(db_id) + user = _get_user_profile_by_db_id_or_slug(db_id) if user is None: logger.debug("User not found") @@ -800,11 +1065,167 @@ def get_privacy_filtered_profile_by_db_id(db_id): _attach_hackathon_history(user, public_data, privacy_settings) _attach_received_praises(user, public_data, privacy_settings) + _attach_teams(user, public_data, privacy_settings) + _attach_certificates(user, public_data, privacy_settings) + _attach_github_contributions(user, public_data, privacy_settings) + _attach_hearts(user, public_data, privacy_settings) logger.debug(f"Privacy-Filtered Profile Result: {public_data}") return public_data +# Bio video: either uploaded to our CDN (signed-URL direct upload) or a link +# to an allowlisted video provider (rendered via the frontend's VideoDisplay). +ALLOWED_VIDEO_CONTENT_TYPES = { + "video/mp4": "mp4", + "video/webm": "webm", + "video/quicktime": "mov", +} +MAX_BIO_VIDEO_BYTES = 100 * 1024 * 1024 # 100MB +ALLOWED_VIDEO_LINK_HOSTS = { + "youtube.com", "www.youtube.com", "youtu.be", + "vimeo.com", "player.vimeo.com", "www.vimeo.com", + "loom.com", "www.loom.com", +} + + +def _cdn_server(): + return os.getenv("CDN_SERVER", "https://cdn.ohack.dev").rstrip("/") + + +def create_bio_video_upload_url(propel_id, content_type, content_length): + """Mint a signed GCS PUT URL for a bio video. Returns (payload, status).""" + if content_type not in ALLOWED_VIDEO_CONTENT_TYPES: + return {"error": f"content_type must be one of {sorted(ALLOWED_VIDEO_CONTENT_TYPES)}"}, 400 + try: + content_length = int(content_length) + except (TypeError, ValueError): + return {"error": "content_length is required"}, 400 + if content_length <= 0 or content_length > MAX_BIO_VIDEO_BYTES: + return {"error": f"Video must be under {MAX_BIO_VIDEO_BYTES // (1024 * 1024)}MB"}, 400 + + user, _user_id = _resolve_and_ensure_user(propel_id) + if user is None or not getattr(user, "id", None): + return {"error": "Could not resolve your account"}, 404 + + from common.utils.cdn import generate_signed_upload_url + ext = ALLOWED_VIDEO_CONTENT_TYPES[content_type] + filename = f"bio_video_{uuid.uuid4().hex}.{ext}" + try: + payload = generate_signed_upload_url( + directory=f"users/{user.id}", + filename=filename, + content_type=content_type, + max_bytes=MAX_BIO_VIDEO_BYTES, + ) + except Exception as e: + exception(logger, "Failed to generate signed upload URL", error=str(e)) + return {"error": "Could not create an upload URL"}, 500 + return payload, 200 + + +def set_bio_video_url(propel_id, url): + """The single writer for bio_video_url. Returns (payload, status). + + Accepts: null/"" (clear), an own-CDN URL under users// (verified to + exist), or an allowlisted provider link (YouTube/Vimeo/Loom). + bio_video_url is deliberately NOT in metadata_list — arbitrary URLs must + never reach the public page. + """ + user, _user_id = _resolve_and_ensure_user(propel_id) + if user is None or not getattr(user, "id", None): + return {"error": "Could not resolve your account"}, 404 + + url = (url or "").strip() + previous = getattr(user, "bio_video_url", "") or "" + cdn_prefix = f"{_cdn_server()}/users/{user.id}/" + + if url: + if url.startswith(cdn_prefix): + blob_path = url[len(_cdn_server()) + 1:] + try: + from common.utils.cdn import get_blob_metadata + meta = get_blob_metadata(blob_path) + except Exception as e: + exception(logger, "Failed to verify uploaded bio video", error=str(e)) + return {"error": "Could not verify the uploaded video"}, 500 + if not meta.get("exists"): + return {"error": "Upload not found — did the upload finish?"}, 400 + if meta.get("content_type") not in ALLOWED_VIDEO_CONTENT_TYPES: + return {"error": "Uploaded file is not an allowed video type"}, 400 + if (meta.get("size") or 0) > MAX_BIO_VIDEO_BYTES: + return {"error": "Uploaded video exceeds the size limit"}, 400 + else: + if not validate_url(url): + return {"error": "Invalid URL"}, 400 + from urllib.parse import urlparse + host = (urlparse(url).netloc or "").lower().split(":")[0] + if host not in ALLOWED_VIDEO_LINK_HOSTS: + return {"error": "Video links must be YouTube, Vimeo, or Loom (or an upload)"}, 400 + + from db.db import update_user_bio_video + update_user_bio_video(user.id, url) + + # Best-effort cleanup of a replaced/removed own-CDN upload + if previous and previous.startswith(cdn_prefix) and previous != url: + try: + from common.utils.cdn import delete_from_cdn + delete_from_cdn(previous[len(_cdn_server()) + 1:]) + except Exception as e: + warning(logger, "Failed to delete previous bio video", error=str(e)) + + send_slack_audit(action="set_bio_video_url", message=f"User {user.id} set bio video") + get_profile_metadata.cache_clear() + clear_portfolio_caches(user.id) + return {"bio_video_url": url}, 200 + + +def set_profile_visibility(propel_id, visibility): + """Set the portfolio master toggle. Returns (payload, http_status). + + "public" (search-indexable + sitemap-listed) requires a claimed slug so + every indexed portfolio has a clean /u/ URL. + """ + from model.user import PROFILE_VISIBILITY_VALUES + + if visibility not in PROFILE_VISIBILITY_VALUES: + return {"error": f"visibility must be one of {list(PROFILE_VISIBILITY_VALUES)}"}, 400 + + user, _user_id = _resolve_and_ensure_user(propel_id) + if user is None or not getattr(user, "id", None): + return {"error": "Could not resolve your account"}, 404 + + if visibility == "public" and not getattr(user, "profile_slug", None): + return {"error": "Claim a portfolio URL before making your portfolio public"}, 400 + + from db.db import update_user_profile_visibility + update_user_profile_visibility(user.id, visibility) + + send_slack_audit(action="set_profile_visibility", + message=f"User {user.id} set portfolio visibility to {visibility}") + get_profile_metadata.cache_clear() + clear_portfolio_caches(user.id) + return {"profile_visibility": visibility}, 200 + + +@redis_cached(prefix="portfolio:sitemap", ttl=3600) +def get_searchable_portfolio_sitemap(): + """[{slug, last_login}] for every opted-in public portfolio (sitemap feed).""" + from db.db import fetch_public_portfolio_users + return fetch_public_portfolio_users() + + +@redis_cached(prefix="portfolio:profile", ttl=300) +def get_portfolio_profile(id_or_slug): + """Cached public portfolio payload (the fat response the profile page SSRs). + + Cache is keyed on the requested param (db id, slug, or alias each get their + own 300s entry); clear_portfolio_caches() wipes the whole prefix on any + profile-affecting write. Misses (None) are not cached by redis_cached. + """ + return get_privacy_filtered_profile_by_db_id(id_or_slug) + + def get_received_praises_by_db_id(db_id, limit=20, offset=0): """Return praises received by the user identified by `db_id`, honoring privacy. @@ -812,7 +1233,7 @@ def get_received_praises_by_db_id(db_id, limit=20, offset=0): not found / has praises set to private. """ logger.debug(f"Get Received Praises By DB ID: {db_id} limit={limit} offset={offset}") - user = get_user_profile_by_db_id(db_id) + user = _get_user_profile_by_db_id_or_slug(db_id) if user is None: return None @@ -849,7 +1270,7 @@ def get_received_praises_by_db_id(db_id, limit=20, offset=0): def get_public_privacy_settings_by_db_id(db_id): """Get only the privacy settings for a user by database ID (for public profile views)""" logger.debug(f"Get Public Privacy Settings By DB ID: {db_id}") - user = get_user_profile_by_db_id(db_id) + user = _get_user_profile_by_db_id_or_slug(db_id) if user is None: logger.debug("User not found") diff --git a/test/services/test_portfolio_helpers.py b/test/services/test_portfolio_helpers.py new file mode 100644 index 0000000..0a96611 --- /dev/null +++ b/test/services/test_portfolio_helpers.py @@ -0,0 +1,55 @@ +"""Pure-function tests: hearts summary/tiers + certificate github_username extraction.""" +import os + +os.environ.setdefault("ENVIRONMENT", "test") + +from services.hearts_service import get_hearts_summary, get_heart_tier + + +def test_hearts_summary_sums_what_and_how_only(): + history = { + "what": {"code_quality": 1.5, "documentation": 0.5, "judge": 1}, + "how": {"standups_completed": 2}, + "certificates": [{"filename": "x.png", "hearts": 99}], # must be ignored + "unrelated_future_key": {"foo": 100}, # must be ignored + } + summary = get_hearts_summary(history) + assert summary["total"] == 5.0 + assert summary["breakdown"]["what"]["code_quality"] == 1.5 + assert summary["tier"] == "Silver" + + +def test_hearts_summary_handles_missing_and_malformed(): + assert get_hearts_summary(None)["total"] == 0 + assert get_hearts_summary({})["tier"] is None + summary = get_hearts_summary({"what": {"a": "not-a-number", "b": 2}, "how": "bogus"}) + assert summary["total"] == 2.0 + + +def test_heart_tiers(): + assert get_heart_tier(0) is None + assert get_heart_tier(2) == "Bronze" + assert get_heart_tier(5) == "Silver" + assert get_heart_tier(10) == "Gold" + assert get_heart_tier(24) == "Platinum" + assert get_heart_tier(48) == "Diamond" + assert get_heart_tier(100) == "Diamond" + assert get_heart_tier("nan-ish") is None + + +def test_extract_github_username(): + from api.certificates.certificate_service import _extract_github_username + + # Standard noreply encoding + assert _extract_github_username( + "123714233+aitzeng@users.noreply.github.com", "Anthony Tzeng") == "aitzeng" + # Old-style noreply (no numeric id) + assert _extract_github_username( + "someuser@users.noreply.github.com", None) == "someuser" + # Fallback: author_name that looks like a login + assert _extract_github_username("real@example.com", "gregv") == "gregv" + # Full name is not a login; unrelated email -> None + assert _extract_github_username("real@example.com", "Greg V") is None + assert _extract_github_username(None, None) is None + # Case is normalized + assert _extract_github_username("1+AiTzEnG@users.noreply.github.com", None) == "aitzeng" From db80be86759c07992dfa0a35e57c84ee03e7a2e6 Mon Sep 17 00:00:00 2001 From: Greg V <6913307+gregv@users.noreply.github.com> Date: Sun, 9 Aug 2026 11:35:32 -0700 Subject: [PATCH 2/3] Settings --- .vscode/settings.json | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 5e6d34a..a9fb220 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -16,12 +16,19 @@ "statusBar.background": "#dd0531", "statusBar.foreground": "#e7e7e7", "statusBarItem.hoverBackground": "#fa1b49", - "statusBarItem.remoteBackground": "#dd0531", + "statusBarItem.remoteBackground": "#0e3e01", "statusBarItem.remoteForeground": "#e7e7e7", "titleBar.activeBackground": "#dd0531", "titleBar.activeForeground": "#e7e7e7", "titleBar.inactiveBackground": "#dd053199", - "titleBar.inactiveForeground": "#e7e7e799" + "titleBar.inactiveForeground": "#e7e7e799", + "activityBarTop.activeBackground": "#fa1b49", + "activityBarTop.background": "#fa1b49", + "activityBarTop.foreground": "#e7e7e7", + "activityBarTop.inactiveForeground": "#e7e7e799", + "commandCenter.foreground": "#e7e7e7", + "statusBar.debuggingBackground": "#dd0531", + "statusBar.debuggingForeground": "#e7e7e7" }, "peacock.color": "#dd0531" } \ No newline at end of file From 0c76469e2ba43f12c6438624b856d1ef4ee64323 Mon Sep 17 00:00:00 2001 From: Greg V <6913307+gregv@users.noreply.github.com> Date: Tue, 11 Aug 2026 20:54:06 -0700 Subject: [PATCH 3/3] Accept judge_judging_{start,end}_time hackathon constraints New per-event HH:MM constraints driving the judging-window copy on the judge application (frontend falls back to 3:00/5:30 PM when unset). Generalizes the judge_venue_arrival_time validation into a shared JUDGE_TIME_CONSTRAINT_KEYS loop in both validate_hackathon_data and validate_hackathon_data_partial. Co-Authored-By: Claude Fable 5 --- common/utils/validators.py | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/common/utils/validators.py b/common/utils/validators.py index 4bb505f..6aa20ff 100644 --- a/common/utils/validators.py +++ b/common/utils/validators.py @@ -7,6 +7,14 @@ logger = logging.getLogger(__name__) +# Per-event judge time constraints (HH:MM 24-hour, nullable). Admin UI in +# frontend JudgesSection.js; consumed by the judge application page. +JUDGE_TIME_CONSTRAINT_KEYS = ( + "judge_venue_arrival_time", + "judge_judging_start_time", + "judge_judging_end_time", +) + # Regular expression for email validation # This regex follows the RFC 5322 standard for email addresses EMAIL_REGEX = re.compile(r"""(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])""", re.IGNORECASE) @@ -127,11 +135,14 @@ def validate_hackathon_data(data): if not isinstance(q.get("error"), str) or not q.get("error"): raise ValueError(f"Question {i} must have a non-empty 'error' string") - # Validate judge_venue_arrival_time if present (HH:MM 24-hour string or null) - arrival = constraints.get("judge_venue_arrival_time") - if arrival not in (None, ""): - if not isinstance(arrival, str) or not re.match(r"^([01]\d|2[0-3]):[0-5]\d$", arrival): - raise ValueError("judge_venue_arrival_time must be HH:MM (24-hour)") + # Validate judge time constraints if present (HH:MM 24-hour string or null). + # judge_judging_{start,end}_time drive the judging-window copy on the + # judge application (frontend falls back to 15:00/17:30 when unset). + for judge_time_key in JUDGE_TIME_CONSTRAINT_KEYS: + judge_time = constraints.get(judge_time_key) + if judge_time not in (None, ""): + if not isinstance(judge_time, str) or not re.match(r"^([01]\d|2[0-3]):[0-5]\d$", judge_time): + raise ValueError(f"{judge_time_key} must be HH:MM (24-hour)") # Validate hacker_deposit if present hacker_deposit = constraints.get("hacker_deposit") @@ -243,11 +254,12 @@ def _skip(field, reason): _skip("constraints.hacker_required_questions", str(e)) c.pop("hacker_required_questions") - arrival = c.get("judge_venue_arrival_time") - if arrival not in (None, ""): - if not isinstance(arrival, str) or not re.match(r"^([01]\d|2[0-3]):[0-5]\d$", arrival): - _skip("constraints.judge_venue_arrival_time", "must be HH:MM (24-hour)") - c.pop("judge_venue_arrival_time") + for judge_time_key in JUDGE_TIME_CONSTRAINT_KEYS: + judge_time = c.get(judge_time_key) + if judge_time not in (None, ""): + if not isinstance(judge_time, str) or not re.match(r"^([01]\d|2[0-3]):[0-5]\d$", judge_time): + _skip(f"constraints.{judge_time_key}", "must be HH:MM (24-hour)") + c.pop(judge_time_key) if "hacker_deposit" in c and c["hacker_deposit"] is not None: try: