From 269db37f8cb29655c698d2871d96333791d75abf Mon Sep 17 00:00:00 2001 From: Greg V Date: Sat, 27 Jun 2026 22:31:18 -0700 Subject: [PATCH 1/4] Add event feedback surveys API (surveys collection) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New `api/surveys` blueprint for post-event / live-event feedback, stored in a separate `surveys` Firestore collection (distinct from the peer-to-peer `feedback` collection). - GET /api/surveys//context (public) — mode (live/post/upcoming), caller's eligible roles, requires_captcha, already_submitted - POST /api/surveys//responses (public) — selected volunteers are trusted and skip CAPTCHA; nonprofits/anonymous must pass reCAPTCHA (reuses the contact form's verify_recaptcha) - GET /api/surveys//responses, /summary (volunteer.admin) Mode is timezone-aware off start/end_date. Eligible roles come from the volunteers collection (hacker on application; mentor/judge/volunteer/sponsor require isSelected). Logged-in responses upsert by event+mode+user. Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 8 + api/__init__.py | 2 + api/surveys/__init__.py | 0 api/surveys/surveys_service.py | 373 ++++++++++++++++++++++ api/surveys/surveys_views.py | 70 ++++ api/surveys/tests/__init__.py | 0 api/surveys/tests/test_surveys_service.py | 67 ++++ 7 files changed, 520 insertions(+) create mode 100644 api/surveys/__init__.py create mode 100644 api/surveys/surveys_service.py create mode 100644 api/surveys/surveys_views.py create mode 100644 api/surveys/tests/__init__.py create mode 100644 api/surveys/tests/test_surveys_service.py diff --git a/CLAUDE.md b/CLAUDE.md index 3252a58..a45137f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -151,3 +151,11 @@ Two scripts live in `scripts/` for diagnosing and backfilling team rosters on `/ `scripts/sync_resend_audience.py --source {all|profiles|volunteers|mentors|judges|sponsors|helpers|leads} --audience "" [--event-id ] [--selected-only] [--apply]` — pulls emails from Firestore (`users.email_address`, `volunteers.email` filtered by `volunteer_type`, `leads.email`) and upserts contacts into a Resend audience (creates if missing). Dry-run by default. Re-runnable: lists existing audience contacts first and only POSTs new emails. Needs `RESEND_API_KEY` with audiences scope — the existing `RESEND_WELCOME_EMAIL_KEY` is send-only and will 401. Uses the deprecated `resend.Audiences` SDK class (now an alias for Segments) — fine for now, but if it breaks switch to `resend.Segments`. The frontend `/hack/` page's "Team Members:" list is `teams.users[]` (DocumentReferences). The bug pattern that motivated this: a team's `users[]` only contains the user who created the team on ohack.dev; everyone else registered via Devpost/JotForm and was never linked. Use `audit` first to confirm, then `import ... --csv-type roster` (or `projects` for old Devpost exports) to backfill. + +## Event Feedback Surveys (`surveys` collection) +Post-event / live-event feedback, stored in a NEW `surveys` collection — deliberately distinct from the peer-to-peer `feedback` collection. Blueprint `api/surveys/surveys_views.py` + `services` in `api/surveys/surveys_service.py`. Routes (`/api/surveys//...`): +- `GET context` — public (`@auth.optional_user`). Returns `mode` (`live|post|upcoming`), the caller's eligible `roles`, `primary_role`, `requires_captcha`, `already_submitted`, and a light `event` block. +- `POST responses` — public (`@auth.optional_user`). Logged-in volunteers who are eligible for the event are "trusted" and skip CAPTCHA; **everyone else (nonprofit partners — no flag yet — and anonymous) must pass reCAPTCHA** via the shared `verify_recaptcha` from `api.contact.contact_service`. +- `GET responses` / `GET summary` — `volunteer.admin`-gated. + +`compute_event_mode` is timezone-aware off `start_date`/`end_date` (mirrors the frontend `isHackathonExpired`; missing dates → `live`). Eligible roles come from the `volunteers` collection via `get_user_event_roles` — hacker counts on application, mentor/judge/volunteer/sponsor require `isSelected` — matched 3 ways (propel UUID / email / OAuth user_id) like `handle_get`. Logged-in responses **upsert** by doc id `{event_id}__{mode}__{propel_user_id}` with a full `set()` (NOT `merge=True`, which would deep-merge the `answers` map and keep cleared keys) carrying `created_at` forward; anonymous responses are random-uuid docs. Heavy/external deps (`get_hackathon_by_event_id`, `verify_recaptcha`, slack) are lazy-imported inside functions so the module imports cheaply (testable; pure date/mode logic covered in `api/surveys/tests/`). Question IDs/catalog live frontend-side; backend stores `answers` verbatim. Live-mode submissions ping the `#feedback` Slack channel; post-mode is audit-only. diff --git a/api/__init__.py b/api/__init__.py index b184d43..bbe44a9 100644 --- a/api/__init__.py +++ b/api/__init__.py @@ -187,6 +187,7 @@ def add_headers(response): from api.planning import planning_views from api.mentors import mentors_views from api.email_templates import email_templates_views + from api.surveys import surveys_views app.register_blueprint(messages_views.bp) app.register_blueprint(exception_views.bp) @@ -209,5 +210,6 @@ def add_headers(response): app.register_blueprint(planning_views.bp) app.register_blueprint(mentors_views.bp) app.register_blueprint(email_templates_views.bp) + app.register_blueprint(surveys_views.bp) return app diff --git a/api/surveys/__init__.py b/api/surveys/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/api/surveys/surveys_service.py b/api/surveys/surveys_service.py new file mode 100644 index 0000000..12e118e --- /dev/null +++ b/api/surveys/surveys_service.py @@ -0,0 +1,373 @@ +"""Post-event / live-event feedback surveys. + +Stores responses in the Firestore `surveys` collection (kept separate from the +existing peer-to-peer `feedback` collection). A response is keyed to an event +(`event_id`) and a mode: + - `live` — the event is currently happening (start <= now <= end) + - `post` — the event has ended (now > end) + +Eligibility: + - Logged-in volunteers who are `isSelected` for the event (mentor / judge / + volunteer / sponsor) or have a hacker application — they're "trusted" and + skip the CAPTCHA. Their role(s) are derived from the `volunteers` collection. + - Everyone else (nonprofit partners, who have no flag yet, and any anonymous + visitor) must pass the same Google reCAPTCHA v3 check the contact form uses. +""" +from typing import Dict, Any, List, Optional, Tuple +import uuid +import os +from datetime import datetime + +import pytz + +from db.db import get_db +from common.log import get_logger, warning + +logger = get_logger(__name__) + +SURVEY_COLLECTION = "surveys" +SURVEY_SLACK_CHANNEL = "feedback" +DEFAULT_TIMEZONE = "America/Phoenix" + +# Roles given by the volunteers collection. A "hacker" record counts as soon as +# the application exists; the rest require isSelected=True to count. +_SELECTED_ONLY_ROLES = {"mentor", "judge", "volunteer", "sponsor"} + +# Roles a submitted response may claim. +ALLOWED_ROLES = {"hacker", "mentor", "judge", "nonprofit", "volunteer", "organizer", "sponsor"} + + +def _now_iso() -> str: + return datetime.now(pytz.timezone(DEFAULT_TIMEZONE)).isoformat() + + +def _parse_event_date(date_str: Optional[str], tz, end_of_day: bool = False): + """Parse a 'YYYY-MM-DD' string into a tz-aware datetime, or None.""" + if not date_str or not isinstance(date_str, str): + return None + try: + parts = date_str.split("-") + year, month, day = int(parts[0]), int(parts[1]), int(parts[2]) + except (ValueError, IndexError): + return None + if end_of_day: + return tz.localize(datetime(year, month, day, 23, 59, 59)) + return tz.localize(datetime(year, month, day, 0, 0, 0)) + + +def compute_event_mode(event: Dict[str, Any]) -> str: + """Return 'live', 'post', or 'upcoming' for the event's current window. + + Comparisons use the event's own timezone so a viewer elsewhere doesn't flip + the mode early/late (mirrors the frontend `isHackathonExpired`). + """ + tz_name = event.get("timezone") or DEFAULT_TIMEZONE + try: + tz = pytz.timezone(tz_name) + except Exception: + tz = pytz.timezone(DEFAULT_TIMEZONE) + + now = datetime.now(tz) + start = _parse_event_date(event.get("start_date"), tz, end_of_day=False) + end = _parse_event_date(event.get("end_date"), tz, end_of_day=True) + + if end and now > end: + return "post" + if start and now < start: + return "upcoming" + # Within the window, or dates missing — treat as live so feedback is reachable. + return "live" + + +def _user_doc_id(event_id: str, mode: str, propel_user_id: str) -> str: + """Deterministic doc id so a logged-in user's response upserts (no dupes).""" + safe_uid = (propel_user_id or "anon").replace("/", "_") + return f"{event_id}__{mode}__{safe_uid}" + + +def _resolve_user_identity(propel_user_id: Optional[str]) -> Tuple[Optional[str], Optional[str]]: + """(email, oauth_user_id) for a PropelAuth user. Lazy import avoids a heavy + module load at import time and keeps this module cheap to unit-test.""" + if not propel_user_id: + return None, None + try: + from services.users_service import get_propel_user_details_by_id + details = get_propel_user_details_by_id(propel_user_id) or () + email = details[0] if len(details) > 0 else None + oauth_user_id = details[1] if len(details) > 1 else None + return email, oauth_user_id + except Exception as e: + warning(logger, "survey: could not resolve user identity", exc_info=e) + return None, None + + +def get_user_event_roles(propel_user_id: Optional[str], event_id: str) -> List[str]: + """Roles the user is eligible to give feedback as for this event. + + Hacker counts on application; mentor/judge/volunteer/sponsor need + isSelected=True. Matches the survey-eligibility rule. Uses equality-only + queries (no composite index needed). Identity is matched the same three + ways as `handle_get` in volunteers_views: propel UUID, email, OAuth user_id. + """ + if not propel_user_id or not event_id: + return [] + + email, oauth_user_id = _resolve_user_identity(propel_user_id) + db = get_db() + roles = set() + seen = set() + + def collect(field: str, value: Optional[str]): + if not value: + return + try: + query = ( + db.collection("volunteers") + .where(field, "==", value) + .where("event_id", "==", event_id) + ) + for snap in query.stream(): + if snap.id in seen: + continue + seen.add(snap.id) + vol = snap.to_dict() or {} + vtype = (vol.get("volunteer_type") or "").lower() + if not vtype: + continue + if vtype in _SELECTED_ONLY_ROLES and not vol.get("isSelected"): + continue + roles.add(vtype) + except Exception as e: + warning(logger, "survey: volunteer role query failed", exc_info=e) + + for uid in {propel_user_id, oauth_user_id}: + collect("user_id", uid) + collect("email", email) + return sorted(roles) + + +def get_survey_context(event_id: str, propel_user_id: Optional[str]) -> Tuple[Dict[str, Any], int]: + """What the frontend needs to render the right form: mode, the caller's + eligible roles, whether a CAPTCHA is required, and a basic event summary.""" + from common.utils.firebase import get_hackathon_by_event_id + + event = get_hackathon_by_event_id(event_id) + if not event: + return {"success": False, "error": "Event not found"}, 404 + + mode = compute_event_mode(event) + logged_in = bool(propel_user_id) + roles = get_user_event_roles(propel_user_id, event_id) if logged_in else [] + eligible = bool(roles) + trusted = logged_in and eligible + + already_submitted = False + if trusted and mode in ("live", "post"): + try: + doc_id = _user_doc_id(event_id, mode, propel_user_id) + already_submitted = get_db().collection(SURVEY_COLLECTION).document(doc_id).get().exists + except Exception as e: + warning(logger, "survey: already-submitted check failed", exc_info=e) + + return { + "success": True, + "mode": mode, + "logged_in": logged_in, + "eligible": eligible if logged_in else None, + "roles": roles, + "primary_role": roles[0] if roles else None, + "requires_captcha": not trusted, + "already_submitted": already_submitted, + "event": { + "event_id": event.get("event_id") or event_id, + "title": event.get("title"), + "start_date": event.get("start_date"), + "end_date": event.get("end_date"), + "timezone": event.get("timezone") or DEFAULT_TIMEZONE, + }, + }, 200 + + +def submit_survey_response( + event_id: str, + propel_user_id: Optional[str], + payload: Dict[str, Any], + ip_address: Optional[str] = None, +) -> Tuple[Dict[str, Any], int]: + """Validate + persist one feedback response. Returns (body, status_code).""" + from common.utils.firebase import get_hackathon_by_event_id + + event = get_hackathon_by_event_id(event_id) + if not event: + return {"success": False, "error": "Event not found"}, 404 + + mode = compute_event_mode(event) + if mode not in ("live", "post"): + return {"success": False, "error": "This event's feedback form is not open yet."}, 403 + + role = (payload.get("role") or "").strip().lower() + if role not in ALLOWED_ROLES: + return {"success": False, "error": "A valid role is required."}, 400 + + answers = payload.get("answers") + if not isinstance(answers, dict) or not answers: + return {"success": False, "error": "No answers were provided."}, 400 + + logged_in = bool(propel_user_id) + roles = get_user_event_roles(propel_user_id, event_id) if logged_in else [] + trusted = logged_in and bool(roles) + + # CAPTCHA gate for everyone who isn't a known, selected volunteer. + if not trusted: + from api.contact.contact_service import verify_recaptcha + token = payload.get("recaptchaToken") + if not verify_recaptcha(token) and os.environ.get("FLASK_ENV") != "development": + warning(logger, "survey: reCAPTCHA verification failed", event_id=event_id) + return {"success": False, "error": "reCAPTCHA verification failed"}, 400 + + email = None + if logged_in: + email, _ = _resolve_user_identity(propel_user_id) + if not email: + provided = payload.get("email") + if isinstance(provided, str) and "@" in provided: + email = provided.strip() + + now = _now_iso() + db = get_db() + record = { + "event_id": event_id, + "mode": mode, + "role": role, + "answers": answers, + "user_id": propel_user_id, + "email": email, + "is_anonymous": not logged_in, + "eligible_roles": roles, + "source": payload.get("source") or "survey", + "ip_address": ip_address, + "updated_at": now, + } + + is_update = False + if logged_in: + # Deterministic id → a user's response upserts. Write the whole doc + # (NOT merge=True — that deep-merges the answers map and would leave + # behind keys the user cleared) while carrying created_at forward. + doc_id = _user_doc_id(event_id, mode, propel_user_id) + ref = db.collection(SURVEY_COLLECTION).document(doc_id) + snap = ref.get() + if snap.exists: + is_update = True + record["created_at"] = (snap.to_dict() or {}).get("created_at") or now + else: + record["created_at"] = now + ref.set(record) + else: + doc_id = str(uuid.uuid4()) + record["created_at"] = now + db.collection(SURVEY_COLLECTION).document(doc_id).set(record) + + _notify_submission(event, record, doc_id) + return ( + {"success": True, "id": doc_id, "mode": mode, "updated": is_update}, + 200 if is_update else 201, + ) + + +def _notify_submission(event: Dict[str, Any], record: Dict[str, Any], doc_id: str) -> None: + """Audit every response; ping the #feedback Slack channel for *live* + feedback only, so organizers can react in real time without post-event spam.""" + try: + from common.utils.slack import send_slack_audit + send_slack_audit( + action="survey_response", + message=f"Survey response for {record.get('event_id')} ({record.get('mode')}, {record.get('role')})", + payload={"id": doc_id}, + ) + except Exception: + pass + + if record.get("mode") != "live": + return + + try: + from common.utils.slack import send_slack + answers = record.get("answers") or {} + lines = [ + f"*New live feedback* — {event.get('title') or record.get('event_id')}", + f"*Role:* {record.get('role')}", + ] + rating = answers.get("overall_rating") + if rating is not None: + lines.append(f"*How's it going:* {rating}/5") + for key, label in ( + ("hacker_blocked", "Blocked"), + ("mentor_team_concern", "Team concern"), + ("vol_live_issue", "Issue"), + ("npo_team_waiting", "NPO blocking a team"), + ("to_improve", "Should fix"), + ): + val = answers.get(key) + if isinstance(val, dict): + val = val.get("value") or val.get("note") + if val: + lines.append(f"*{label}:* {val}") + send_slack( + message="\n".join(lines), + channel=SURVEY_SLACK_CHANNEL, + username="Feedback Bot", + icon_emoji=":memo:", + ) + except Exception as e: + warning(logger, "survey: live Slack notify failed", exc_info=e) + + +def get_event_survey_responses(event_id: str, mode: Optional[str] = None) -> Dict[str, Any]: + """Admin: all responses for an event (PII-light — drops ip_address).""" + db = get_db() + query = db.collection(SURVEY_COLLECTION).where("event_id", "==", event_id) + if mode: + query = query.where("mode", "==", mode) + + responses = [] + for doc in query.stream(): + data = doc.to_dict() or {} + data["id"] = doc.id + data.pop("ip_address", None) + responses.append(data) + + responses.sort(key=lambda r: r.get("created_at") or "", reverse=True) + return {"success": True, "responses": responses, "count": len(responses)} + + +def get_event_survey_summary(event_id: str) -> Dict[str, Any]: + """Admin: light aggregate — counts by mode/role + averages of the two + cross-event segmenting scales (overall_rating, would_return).""" + responses = get_event_survey_responses(event_id)["responses"] + summary = { + "count": len(responses), + "by_mode": {}, + "by_role": {}, + "overall_rating": {"count": 0, "average": None}, + "would_return": {"count": 0, "average": None}, + } + rating_sum = 0.0 + return_sum = 0.0 + for resp in responses: + summary["by_mode"][resp.get("mode")] = summary["by_mode"].get(resp.get("mode"), 0) + 1 + summary["by_role"][resp.get("role")] = summary["by_role"].get(resp.get("role"), 0) + 1 + answers = resp.get("answers") or {} + overall = answers.get("overall_rating") + if isinstance(overall, (int, float)): + summary["overall_rating"]["count"] += 1 + rating_sum += overall + would_return = answers.get("would_return") + if isinstance(would_return, (int, float)): + summary["would_return"]["count"] += 1 + return_sum += would_return + if summary["overall_rating"]["count"]: + summary["overall_rating"]["average"] = round(rating_sum / summary["overall_rating"]["count"], 2) + if summary["would_return"]["count"]: + summary["would_return"]["average"] = round(return_sum / summary["would_return"]["count"], 2) + return {"success": True, "summary": summary} diff --git a/api/surveys/surveys_views.py b/api/surveys/surveys_views.py new file mode 100644 index 0000000..e6ff4dc --- /dev/null +++ b/api/surveys/surveys_views.py @@ -0,0 +1,70 @@ +from flask import Blueprint, jsonify, request +from common.log import get_logger +from common.auth import auth, auth_user, getOrgId +from api.surveys.surveys_service import ( + get_survey_context, + submit_survey_response, + get_event_survey_responses, + get_event_survey_summary, +) + +logger = get_logger(__name__) +bp = Blueprint("surveys", __name__, url_prefix="/api") + + +def _propel_user_id(): + user = auth_user + return getattr(user, "user_id", None) if user else None + + +@bp.route("/surveys//context", methods=["GET"]) +@auth.optional_user +def survey_context(event_id): + """Public. Tells the frontend the survey mode (live/post/upcoming), the + caller's eligible roles, and whether a CAPTCHA token is needed.""" + try: + result, status = get_survey_context(event_id, _propel_user_id()) + return jsonify(result), status + except Exception as e: + logger.exception("Error fetching survey context: %s", str(e)) + return jsonify({"success": False, "error": "Failed to load the feedback form."}), 500 + + +@bp.route("/surveys//responses", methods=["POST"]) +@auth.optional_user +def submit_survey(event_id): + """Public. Logged-in selected volunteers are trusted; everyone else + (nonprofit partners, anonymous) must pass reCAPTCHA.""" + data = request.get_json(silent=True) + if not data: + return jsonify({"success": False, "error": "Empty request body"}), 400 + try: + result, status = submit_survey_response( + event_id, _propel_user_id(), data, ip_address=request.remote_addr + ) + return jsonify(result), status + except Exception as e: + logger.exception("Error submitting survey response: %s", str(e)) + return jsonify({"success": False, "error": "An error occurred while saving your feedback."}), 500 + + +@bp.route("/surveys//responses", methods=["GET"]) +@auth.require_org_member_with_permission("volunteer.admin", req_to_org_id=getOrgId) +def list_survey_responses(event_id): + """Admin: all responses for an event (optional ?mode=live|post).""" + try: + return jsonify(get_event_survey_responses(event_id, request.args.get("mode"))), 200 + except Exception as e: + logger.exception("Error listing survey responses: %s", str(e)) + return jsonify({"success": False, "error": str(e), "responses": []}), 500 + + +@bp.route("/surveys//summary", methods=["GET"]) +@auth.require_org_member_with_permission("volunteer.admin", req_to_org_id=getOrgId) +def survey_summary(event_id): + """Admin: light aggregate of responses for an event.""" + try: + return jsonify(get_event_survey_summary(event_id)), 200 + except Exception as e: + logger.exception("Error building survey summary: %s", str(e)) + return jsonify({"success": False, "error": str(e)}), 500 diff --git a/api/surveys/tests/__init__.py b/api/surveys/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/api/surveys/tests/test_surveys_service.py b/api/surveys/tests/test_surveys_service.py new file mode 100644 index 0000000..b1eabbd --- /dev/null +++ b/api/surveys/tests/test_surveys_service.py @@ -0,0 +1,67 @@ +from datetime import datetime, timedelta + +import pytz + +from api.surveys.surveys_service import ( + compute_event_mode, + _parse_event_date, + _user_doc_id, + ALLOWED_ROLES, +) + +TZ = "America/Phoenix" + + +def _date(offset_days): + """A YYYY-MM-DD string offset from today in the event timezone.""" + now = datetime.now(pytz.timezone(TZ)) + return (now + timedelta(days=offset_days)).strftime("%Y-%m-%d") + + +class TestComputeEventMode: + def test_past_event_is_post(self): + event = {"start_date": _date(-10), "end_date": _date(-5), "timezone": TZ} + assert compute_event_mode(event) == "post" + + def test_current_event_is_live(self): + event = {"start_date": _date(-1), "end_date": _date(1), "timezone": TZ} + assert compute_event_mode(event) == "live" + + def test_future_event_is_upcoming(self): + event = {"start_date": _date(5), "end_date": _date(7), "timezone": TZ} + assert compute_event_mode(event) == "upcoming" + + def test_single_day_event_today_is_live(self): + today = _date(0) + assert compute_event_mode({"start_date": today, "end_date": today, "timezone": TZ}) == "live" + + def test_missing_dates_defaults_to_live(self): + assert compute_event_mode({}) == "live" + + def test_bad_timezone_falls_back(self): + event = {"start_date": _date(-10), "end_date": _date(-5), "timezone": "Not/AZone"} + assert compute_event_mode(event) == "post" + + +class TestHelpers: + def test_parse_event_date_end_of_day(self): + tz = pytz.timezone(TZ) + end = _parse_event_date("2026-03-01", tz, end_of_day=True) + assert end.hour == 23 and end.minute == 59 + start = _parse_event_date("2026-03-01", tz, end_of_day=False) + assert start.hour == 0 and start.minute == 0 + + def test_parse_event_date_invalid(self): + tz = pytz.timezone(TZ) + assert _parse_event_date(None, tz) is None + assert _parse_event_date("garbage", tz) is None + + def test_user_doc_id_is_deterministic_and_slash_safe(self): + a = _user_doc_id("evt", "post", "oauth2|slack|abc") + b = _user_doc_id("evt", "post", "oauth2|slack|abc") + assert a == b + assert "/" not in a + + def test_allowed_roles_cover_spec(self): + for role in ("hacker", "mentor", "judge", "nonprofit", "volunteer", "organizer"): + assert role in ALLOWED_ROLES From 5b41083c27697e3bd4f8f923eb11cca74d3d5979 Mon Sep 17 00:00:00 2001 From: Greg V Date: Sun, 28 Jun 2026 21:10:06 -0700 Subject: [PATCH 2/4] Restrict survey role to the volunteer's selected role(s) A trusted (logged-in, isSelected) volunteer may now only submit feedback for a role they were actually selected for; everyone else is restricted to nonprofit. Added allowed_roles_for(), enforced in submit (403), and surfaced allowed_roles in the context response so the frontend can scope its role picker. Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 2 +- api/surveys/surveys_service.py | 23 ++++++++++++++++++++++- api/surveys/tests/test_surveys_service.py | 12 ++++++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a45137f..30bc791 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -158,4 +158,4 @@ Post-event / live-event feedback, stored in a NEW `surveys` collection — delib - `POST responses` — public (`@auth.optional_user`). Logged-in volunteers who are eligible for the event are "trusted" and skip CAPTCHA; **everyone else (nonprofit partners — no flag yet — and anonymous) must pass reCAPTCHA** via the shared `verify_recaptcha` from `api.contact.contact_service`. - `GET responses` / `GET summary` — `volunteer.admin`-gated. -`compute_event_mode` is timezone-aware off `start_date`/`end_date` (mirrors the frontend `isHackathonExpired`; missing dates → `live`). Eligible roles come from the `volunteers` collection via `get_user_event_roles` — hacker counts on application, mentor/judge/volunteer/sponsor require `isSelected` — matched 3 ways (propel UUID / email / OAuth user_id) like `handle_get`. Logged-in responses **upsert** by doc id `{event_id}__{mode}__{propel_user_id}` with a full `set()` (NOT `merge=True`, which would deep-merge the `answers` map and keep cleared keys) carrying `created_at` forward; anonymous responses are random-uuid docs. Heavy/external deps (`get_hackathon_by_event_id`, `verify_recaptcha`, slack) are lazy-imported inside functions so the module imports cheaply (testable; pure date/mode logic covered in `api/surveys/tests/`). Question IDs/catalog live frontend-side; backend stores `answers` verbatim. Live-mode submissions ping the `#feedback` Slack channel; post-mode is audit-only. +`compute_event_mode` is timezone-aware off `start_date`/`end_date` (mirrors the frontend `isHackathonExpired`; missing dates → `live`). Eligible roles come from the `volunteers` collection via `get_user_event_roles` — hacker counts on application, mentor/judge/volunteer/sponsor require `isSelected` — matched 3 ways (propel UUID / email / OAuth user_id) like `handle_get`. **Role scope**: a trusted volunteer may submit ONLY for a role they're selected for; everyone else is restricted to `nonprofit` (`allowed_roles_for`). Enforced in `submit_survey_response` (403 on mismatch) and surfaced as `allowed_roles` in the context response so the frontend can scope its selector. Logged-in responses **upsert** by doc id `{event_id}__{mode}__{propel_user_id}` with a full `set()` (NOT `merge=True`, which would deep-merge the `answers` map and keep cleared keys) carrying `created_at` forward; anonymous responses are random-uuid docs. Heavy/external deps (`get_hackathon_by_event_id`, `verify_recaptcha`, slack) are lazy-imported inside functions so the module imports cheaply (testable; pure date/mode logic covered in `api/surveys/tests/`). Question IDs/catalog live frontend-side; backend stores `answers` verbatim. Live-mode submissions ping the `#feedback` Slack channel; post-mode is audit-only. diff --git a/api/surveys/surveys_service.py b/api/surveys/surveys_service.py index 12e118e..9668079 100644 --- a/api/surveys/surveys_service.py +++ b/api/surveys/surveys_service.py @@ -146,6 +146,17 @@ def collect(field: str, value: Optional[str]): return sorted(roles) +def allowed_roles_for(trusted: bool, eligible_roles: List[str]) -> List[str]: + """Roles a submitter may claim. + + - A trusted (logged-in, isSelected) volunteer may only submit for the role(s) + they were selected for — not every role. + - Everyone else (nonprofit partners, who have no flag, and anonymous visitors) + may only submit as a nonprofit, behind the CAPTCHA. + """ + return list(eligible_roles) if trusted else ["nonprofit"] + + def get_survey_context(event_id: str, propel_user_id: Optional[str]) -> Tuple[Dict[str, Any], int]: """What the frontend needs to render the right form: mode, the caller's eligible roles, whether a CAPTCHA is required, and a basic event summary.""" @@ -160,6 +171,7 @@ def get_survey_context(event_id: str, propel_user_id: Optional[str]) -> Tuple[Di roles = get_user_event_roles(propel_user_id, event_id) if logged_in else [] eligible = bool(roles) trusted = logged_in and eligible + allowed = allowed_roles_for(trusted, roles) already_submitted = False if trusted and mode in ("live", "post"): @@ -175,7 +187,8 @@ def get_survey_context(event_id: str, propel_user_id: Optional[str]) -> Tuple[Di "logged_in": logged_in, "eligible": eligible if logged_in else None, "roles": roles, - "primary_role": roles[0] if roles else None, + "allowed_roles": allowed, + "primary_role": allowed[0] if allowed else None, "requires_captcha": not trusted, "already_submitted": already_submitted, "event": { @@ -217,6 +230,14 @@ def submit_survey_response( roles = get_user_event_roles(propel_user_id, event_id) if logged_in else [] trusted = logged_in and bool(roles) + # Enforce role scope: selected volunteers may only submit for a role they + # were selected for; everyone else may only submit as a nonprofit. + if role not in set(allowed_roles_for(trusted, roles)): + return { + "success": False, + "error": "You can only submit feedback for the role you're selected for.", + }, 403 + # CAPTCHA gate for everyone who isn't a known, selected volunteer. if not trusted: from api.contact.contact_service import verify_recaptcha diff --git a/api/surveys/tests/test_surveys_service.py b/api/surveys/tests/test_surveys_service.py index b1eabbd..a201565 100644 --- a/api/surveys/tests/test_surveys_service.py +++ b/api/surveys/tests/test_surveys_service.py @@ -6,6 +6,7 @@ compute_event_mode, _parse_event_date, _user_doc_id, + allowed_roles_for, ALLOWED_ROLES, ) @@ -65,3 +66,14 @@ def test_user_doc_id_is_deterministic_and_slash_safe(self): def test_allowed_roles_cover_spec(self): for role in ("hacker", "mentor", "judge", "nonprofit", "volunteer", "organizer"): assert role in ALLOWED_ROLES + + +class TestAllowedRolesFor: + def test_trusted_volunteer_limited_to_selected_roles(self): + assert allowed_roles_for(True, ["mentor"]) == ["mentor"] + assert allowed_roles_for(True, ["hacker", "mentor"]) == ["hacker", "mentor"] + + def test_untrusted_can_only_be_nonprofit(self): + # anonymous / logged-in-but-not-selected + assert allowed_roles_for(False, []) == ["nonprofit"] + assert allowed_roles_for(False, ["hacker"]) == ["nonprofit"] From 99574beab570cdf0f12094855147692dfe645e29 Mon Sep 17 00:00:00 2001 From: Greg V Date: Mon, 29 Jun 2026 19:06:01 -0700 Subject: [PATCH 3/4] Add admin feedback review API + cross-event survey overview MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - api/feedback/: read-only admin aggregation for /admin/feedback (peer + onboarding), volunteer.admin-gated, PII-light (drops IP, hides anonymous giver). - api/surveys: new GET /api/surveys/overview — one scan grouped by event_id with per-event rating/would_return averages, mode/role counts, hackathon metadata join. Powers the 'Compare events' view. Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 8 +- api/__init__.py | 2 + api/feedback/__init__.py | 0 api/feedback/feedback_service.py | 170 +++++++++++++++++++++++++++++++ api/feedback/feedback_views.py | 40 ++++++++ api/surveys/surveys_service.py | 108 ++++++++++++++++++++ api/surveys/surveys_views.py | 13 +++ 7 files changed, 340 insertions(+), 1 deletion(-) create mode 100644 api/feedback/__init__.py create mode 100644 api/feedback/feedback_service.py create mode 100644 api/feedback/feedback_views.py diff --git a/CLAUDE.md b/CLAUDE.md index 30bc791..eeb3296 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -156,6 +156,12 @@ The frontend `/hack/` page's "Team Members:" list is `teams.users[]` ( Post-event / live-event feedback, stored in a NEW `surveys` collection — deliberately distinct from the peer-to-peer `feedback` collection. Blueprint `api/surveys/surveys_views.py` + `services` in `api/surveys/surveys_service.py`. Routes (`/api/surveys//...`): - `GET context` — public (`@auth.optional_user`). Returns `mode` (`live|post|upcoming`), the caller's eligible `roles`, `primary_role`, `requires_captcha`, `already_submitted`, and a light `event` block. - `POST responses` — public (`@auth.optional_user`). Logged-in volunteers who are eligible for the event are "trusted" and skip CAPTCHA; **everyone else (nonprofit partners — no flag yet — and anonymous) must pass reCAPTCHA** via the shared `verify_recaptcha` from `api.contact.contact_service`. -- `GET responses` / `GET summary` — `volunteer.admin`-gated. +- `GET responses` / `GET summary` (per-event) and `GET /api/surveys/overview` (cross-event) — `volunteer.admin`-gated. `overview` is **one** scan of the `surveys` collection grouped by `event_id` (`get_cross_event_survey_overview`): per-event `count`/`by_mode`/`by_role`, averages of the two universal scales (`overall_rating`, `would_return`), `first/last_response`, joined with hackathon `title`/dates/tz (lazy-import `get_hackathon_list`). Aggregates only — no per-response data, no PII. Powers the "Compare events" sub-view; static `/surveys/overview` doesn't collide with `/surveys//...`. `compute_event_mode` is timezone-aware off `start_date`/`end_date` (mirrors the frontend `isHackathonExpired`; missing dates → `live`). Eligible roles come from the `volunteers` collection via `get_user_event_roles` — hacker counts on application, mentor/judge/volunteer/sponsor require `isSelected` — matched 3 ways (propel UUID / email / OAuth user_id) like `handle_get`. **Role scope**: a trusted volunteer may submit ONLY for a role they're selected for; everyone else is restricted to `nonprofit` (`allowed_roles_for`). Enforced in `submit_survey_response` (403 on mismatch) and surfaced as `allowed_roles` in the context response so the frontend can scope its selector. Logged-in responses **upsert** by doc id `{event_id}__{mode}__{propel_user_id}` with a full `set()` (NOT `merge=True`, which would deep-merge the `answers` map and keep cleared keys) carrying `created_at` forward; anonymous responses are random-uuid docs. Heavy/external deps (`get_hackathon_by_event_id`, `verify_recaptcha`, slack) are lazy-imported inside functions so the module imports cheaply (testable; pure date/mode logic covered in `api/surveys/tests/`). Question IDs/catalog live frontend-side; backend stores `answers` verbatim. Live-mode submissions ping the `#feedback` Slack channel; post-mode is audit-only. + +## Admin feedback review (`api/feedback/`) +Read-only admin aggregation for the frontend `/admin/feedback` dashboard. New blueprint `api/feedback/feedback_views.py` (NOT messages_views — frozen) + `api/feedback/feedback_service.py` (the admin READ side; writes still live in `services/feedback_service.py` + `services/onboarding_service.py` + the surveys domain). Both routes `volunteer.admin`-gated, `?limit=` (default 500, cap 2000): +- `GET /api/admin/feedback/peer` — peer-to-peer `feedback` collection, newest first, giver/receiver names resolved via a one-shot `fetch_users()` directory (giver hidden when `is_anonymous`); light `by_relationship`/`by_role` summary. +- `GET /api/admin/feedback/onboarding` — `onboarding_feedbacks` collection, newest first, + rating & ease distributions. Keeps `clientInfo.userAgent`, drops the IP. **Field shape (load-bearing for the admin UI)**: maps `contactForFollowup` → `contact: {willing, firstName, email}` (the form stores `firstName`+`willing`, NOT `name` — don't revert to `name`); `overallRating` 0 = unrated (skipped in the average); timestamps are normalized to ISO, stripping a `__Timestamp__` export sentinel (see `scripts/sync_hackathons_from_csv.py`) if present. +Event surveys reuse the `api/surveys` admin routes (`/responses`, `/summary`, `/overview`) — not duplicated here. diff --git a/api/__init__.py b/api/__init__.py index bbe44a9..b181a04 100644 --- a/api/__init__.py +++ b/api/__init__.py @@ -188,6 +188,7 @@ def add_headers(response): from api.mentors import mentors_views from api.email_templates import email_templates_views from api.surveys import surveys_views + from api.feedback import feedback_views app.register_blueprint(messages_views.bp) app.register_blueprint(exception_views.bp) @@ -211,5 +212,6 @@ def add_headers(response): app.register_blueprint(mentors_views.bp) app.register_blueprint(email_templates_views.bp) app.register_blueprint(surveys_views.bp) + app.register_blueprint(feedback_views.bp) return app diff --git a/api/feedback/__init__.py b/api/feedback/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/api/feedback/feedback_service.py b/api/feedback/feedback_service.py new file mode 100644 index 0000000..9f17405 --- /dev/null +++ b/api/feedback/feedback_service.py @@ -0,0 +1,170 @@ +"""Admin-side read / aggregation for the feedback-review dashboard. + +The WRITE paths for these collections live elsewhere on purpose and are NOT +touched here: + - peer-to-peer `feedback` -> services/feedback_service.py (save_feedback) + - `onboarding_feedbacks` -> services/onboarding_service.py (save_onboarding_feedback) + - event `surveys` -> api/surveys/surveys_service.py (has its own admin reads) + +This module is only the admin REVIEW side: list + light aggregate for +`/admin/feedback`. Every caller is `volunteer.admin`-gated at the view layer. +Reads are PII-light: peer feedback hides the giver when the entry is anonymous; +onboarding keeps the user-agent but drops the IP address. +""" +from typing import Any, Dict, Optional + +from db.db import get_db, fetch_users +from common.log import get_logger, warning + +logger = get_logger(__name__) + +PEER_FEEDBACK_COLLECTION = "feedback" +ONBOARDING_COLLECTION = "onboarding_feedbacks" +DEFAULT_LIMIT = 500 +# Sentinel some docs carry when seeded from a Firestore CSV export +# (see scripts/sync_hackathons_from_csv.py); strip it so timestamps parse. +_TIMESTAMP_PREFIX = "__Timestamp__" + + +def _user_directory() -> Dict[str, Dict[str, Any]]: + """db_id -> light public identity, best-effort. + + One users-collection scan. The admin dashboard isn't a hot path, and + peer-feedback giver/receiver ids are arbitrary user doc ids, so a single + map is cheaper than N point reads. + """ + directory: Dict[str, Dict[str, Any]] = {} + try: + for u in fetch_users() or []: + uid = getattr(u, "id", None) + if not uid: + continue + directory[uid] = { + "name": getattr(u, "name", "") or getattr(u, "nickname", "") or "", + "profile_image": getattr(u, "profile_image", None), + } + except Exception as e: # pragma: no cover - defensive + warning(logger, "feedback-admin: user directory build failed", exc_info=e) + return directory + + +def _resolve(directory: Dict[str, Dict[str, Any]], db_id: Optional[str]): + if not db_id: + return None + info = directory.get(db_id) or {} + return { + "id": db_id, + "name": info.get("name") or "Unknown", + "profile_image": info.get("profile_image"), + } + + +def list_peer_feedback(limit: Optional[int] = None) -> Dict[str, Any]: + """All peer-to-peer feedback, newest first, names resolved (giver hidden + when anonymous), plus a light breakdown by relationship and role.""" + db = get_db() + limit = limit or DEFAULT_LIMIT + docs = list(db.collection(PEER_FEEDBACK_COLLECTION).stream()) + directory = _user_directory() + + items = [] + by_relationship: Dict[str, int] = {} + by_role: Dict[str, int] = {} + for doc in docs: + d = doc.to_dict() or {} + is_anon = bool(d.get("is_anonymous", False)) + fb = d.get("feedback") if isinstance(d.get("feedback"), dict) else {} + role = fb.get("role") + rel = d.get("relationship") + if rel: + by_relationship[rel] = by_relationship.get(rel, 0) + 1 + if role: + by_role[role] = by_role.get(role, 0) + 1 + items.append({ + "id": doc.id, + "receiver": _resolve(directory, d.get("feedback_receiver_id")), + "giver": None if is_anon else _resolve(directory, d.get("feedback_giver_id")), + "is_anonymous": is_anon, + "relationship": rel, + "duration": d.get("duration"), + "confidence_level": d.get("confidence_level"), + "role": role, + "feedback": fb, # nested: role + skill scores (0-100) + text fields + "timestamp": d.get("timestamp"), + }) + + items.sort(key=lambda x: x.get("timestamp") or "", reverse=True) + items = items[:limit] + return { + "success": True, + "count": len(items), + "summary": {"by_relationship": by_relationship, "by_role": by_role}, + "feedback": items, + } + + +def list_onboarding_feedback(limit: Optional[int] = None) -> Dict[str, Any]: + """All onboarding feedback, newest first, with a rating distribution. + + Drops `clientInfo.ipAddress`; keeps the user-agent for debugging dupes. + """ + db = get_db() + limit = limit or DEFAULT_LIMIT + docs = list(db.collection(ONBOARDING_COLLECTION).stream()) + + items = [] + rating_dist: Dict[str, int] = {} + ease_dist: Dict[str, int] = {} + rating_sum = 0.0 + rating_count = 0 + for doc in docs: + d = doc.to_dict() or {} + rating = d.get("overallRating") + if isinstance(rating, (int, float)) and rating: # 0 == unrated, don't count + rating_dist[str(int(rating))] = rating_dist.get(str(int(rating)), 0) + 1 + rating_sum += rating + rating_count += 1 + ease = d.get("easeOfUnderstanding") or "" + if ease: + ease_dist[ease] = ease_dist.get(ease, 0) + 1 + + ts = d.get("timestamp") + if hasattr(ts, "isoformat"): + ts = ts.isoformat() + elif isinstance(ts, str) and ts.startswith(_TIMESTAMP_PREFIX): + # CSV-export sentinel (see scripts/sync_hackathons_from_csv.py) -> clean ISO + ts = ts[len(_TIMESTAMP_PREFIX):] + + # contactForFollowup is {"willing": False} or {"willing": True, "firstName", "email"} + contact = d.get("contactForFollowup") or {} + client = d.get("clientInfo") or {} + items.append({ + "id": doc.id, + "overallRating": rating, + "usefulTopics": d.get("usefulTopics") or [], + "missingTopics": d.get("missingTopics") or "", + "easeOfUnderstanding": ease, + "improvements": d.get("improvements") or "", + "additionalFeedback": d.get("additionalFeedback") or "", + "contact": { + "willing": bool(contact.get("willing")), + "firstName": contact.get("firstName") or contact.get("name") or "", + "email": contact.get("email") or "", + }, + "userAgent": client.get("userAgent") or "", # IP intentionally dropped + "timestamp": ts, + }) + + items.sort(key=lambda x: x.get("timestamp") or "", reverse=True) + items = items[:limit] + avg = round(rating_sum / rating_count, 2) if rating_count else None + return { + "success": True, + "count": len(items), + "summary": { + "rating_distribution": rating_dist, + "ease_distribution": ease_dist, + "overall_rating": {"count": rating_count, "average": avg}, + }, + "onboarding_feedback": items, + } diff --git a/api/feedback/feedback_views.py b/api/feedback/feedback_views.py new file mode 100644 index 0000000..c595d83 --- /dev/null +++ b/api/feedback/feedback_views.py @@ -0,0 +1,40 @@ +from flask import Blueprint, jsonify, request + +from common.log import get_logger +from common.auth import auth, getOrgId +from api.feedback.feedback_service import ( + list_peer_feedback, + list_onboarding_feedback, +) + +logger = get_logger(__name__) +bp = Blueprint("feedback_admin", __name__, url_prefix="/api") + + +def _limit(default: int = 500, cap: int = 2000) -> int: + try: + return min(int(request.args.get("limit", default)), cap) + except (TypeError, ValueError): + return default + + +@bp.route("/admin/feedback/peer", methods=["GET"]) +@auth.require_org_member_with_permission("volunteer.admin", req_to_org_id=getOrgId) +def admin_peer_feedback(): + """Admin: all peer-to-peer feedback (newest first), names resolved.""" + try: + return jsonify(list_peer_feedback(_limit())), 200 + except Exception as e: + logger.exception("Error listing peer feedback: %s", str(e)) + return jsonify({"success": False, "error": str(e), "feedback": []}), 500 + + +@bp.route("/admin/feedback/onboarding", methods=["GET"]) +@auth.require_org_member_with_permission("volunteer.admin", req_to_org_id=getOrgId) +def admin_onboarding_feedback(): + """Admin: all onboarding feedback (newest first) + rating distribution.""" + try: + return jsonify(list_onboarding_feedback(_limit())), 200 + except Exception as e: + logger.exception("Error listing onboarding feedback: %s", str(e)) + return jsonify({"success": False, "error": str(e), "onboarding_feedback": []}), 500 diff --git a/api/surveys/surveys_service.py b/api/surveys/surveys_service.py index 9668079..97749a9 100644 --- a/api/surveys/surveys_service.py +++ b/api/surveys/surveys_service.py @@ -392,3 +392,111 @@ def get_event_survey_summary(event_id: str) -> Dict[str, Any]: if summary["would_return"]["count"]: summary["would_return"]["average"] = round(return_sum / summary["would_return"]["count"], 2) return {"success": True, "summary": summary} + + +def get_cross_event_survey_overview() -> Dict[str, Any]: + """Admin: cross-event aggregate for the 'Compare events' view. + + One scan of the surveys collection grouped by `event_id`, with per-event + averages of the two universal scales (overall_rating, would_return) plus + mode/role counts and the first/last response timestamps. Joined with + hackathon metadata (title / dates / timezone). Aggregates only — no + per-response data and no PII. + """ + db = get_db() + by_event: Dict[str, Dict[str, Any]] = {} + totals: Dict[str, Any] = {"responses": 0, "by_mode": {}, "by_role": {}} + + for doc in db.collection(SURVEY_COLLECTION).stream(): + data = doc.to_dict() or {} + event_id = data.get("event_id") + if not event_id: + continue + ev = by_event.get(event_id) + if ev is None: + ev = by_event[event_id] = { + "count": 0, + "by_mode": {}, + "by_role": {}, + "_rating_sum": 0.0, "_rating_n": 0, + "_return_sum": 0.0, "_return_n": 0, + "first_response": None, + "last_response": None, + } + ev["count"] += 1 + totals["responses"] += 1 + + mode = data.get("mode") + if mode: + ev["by_mode"][mode] = ev["by_mode"].get(mode, 0) + 1 + totals["by_mode"][mode] = totals["by_mode"].get(mode, 0) + 1 + role = data.get("role") + if role: + ev["by_role"][role] = ev["by_role"].get(role, 0) + 1 + totals["by_role"][role] = totals["by_role"].get(role, 0) + 1 + + answers = data.get("answers") or {} + overall = answers.get("overall_rating") + if isinstance(overall, (int, float)): + ev["_rating_sum"] += overall + ev["_rating_n"] += 1 + would_return = answers.get("would_return") + if isinstance(would_return, (int, float)): + ev["_return_sum"] += would_return + ev["_return_n"] += 1 + + created = data.get("created_at") + if isinstance(created, str) and created: + # Surveys are born-digital ISO; strip the CSV-export sentinel defensively. + if created.startswith("__Timestamp__"): + created = created[len("__Timestamp__"):] + if ev["first_response"] is None or created < ev["first_response"]: + ev["first_response"] = created + if ev["last_response"] is None or created > ev["last_response"]: + ev["last_response"] = created + + # Join event metadata (title / dates / tz). Lazy import avoids a heavy module + # load at import time and any circular import (mirrors _resolve_user_identity). + event_meta: Dict[str, Dict[str, Any]] = {} + try: + from services.hackathons_service import get_hackathon_list + for h in (get_hackathon_list().get("hackathons") or []): + hid = h.get("event_id") or h.get("id") + if hid: + event_meta[hid] = h + except Exception as e: # pragma: no cover - defensive + warning(logger, "survey-overview: hackathon metadata join failed", exc_info=e) + + events: List[Dict[str, Any]] = [] + for event_id, ev in by_event.items(): + meta = event_meta.get(event_id) or {} + rating_avg = round(ev["_rating_sum"] / ev["_rating_n"], 2) if ev["_rating_n"] else None + return_avg = round(ev["_return_sum"] / ev["_return_n"], 2) if ev["_return_n"] else None + events.append({ + "event_id": event_id, + "title": meta.get("title") or event_id, + "start_date": meta.get("start_date"), + "end_date": meta.get("end_date"), + "timezone": meta.get("timezone") or DEFAULT_TIMEZONE, + "count": ev["count"], + "by_mode": ev["by_mode"], + "by_role": ev["by_role"], + "overall_rating": {"count": ev["_rating_n"], "average": rating_avg}, + "would_return": {"count": ev["_return_n"], "average": return_avg}, + "first_response": ev["first_response"], + "last_response": ev["last_response"], + }) + + # Chronological; events with no known start_date fall to the end. + events.sort(key=lambda e: (e.get("start_date") or "9999", e.get("first_response") or "")) + + return { + "success": True, + "totals": { + "responses": totals["responses"], + "events": len(events), + "by_mode": totals["by_mode"], + "by_role": totals["by_role"], + }, + "events": events, + } diff --git a/api/surveys/surveys_views.py b/api/surveys/surveys_views.py index e6ff4dc..7ac6afd 100644 --- a/api/surveys/surveys_views.py +++ b/api/surveys/surveys_views.py @@ -6,6 +6,7 @@ submit_survey_response, get_event_survey_responses, get_event_survey_summary, + get_cross_event_survey_overview, ) logger = get_logger(__name__) @@ -68,3 +69,15 @@ def survey_summary(event_id): except Exception as e: logger.exception("Error building survey summary: %s", str(e)) return jsonify({"success": False, "error": str(e)}), 500 + + +@bp.route("/surveys/overview", methods=["GET"]) +@auth.require_org_member_with_permission("volunteer.admin", req_to_org_id=getOrgId) +def surveys_overview(): + """Admin: cross-event aggregate for the 'Compare events' view (one scan, + grouped by event_id; aggregates only, no PII).""" + try: + return jsonify(get_cross_event_survey_overview()), 200 + except Exception as e: + logger.exception("Error building cross-event survey overview: %s", str(e)) + return jsonify({"success": False, "error": str(e), "events": []}), 500 From 25245e5da46b8c190908577a9d3b76a24b1af647 Mon Sep 17 00:00:00 2001 From: Greg V Date: Mon, 29 Jun 2026 21:59:50 -0700 Subject: [PATCH 4/4] Fix volunteer time tracking for users without a profile doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /api/users/volunteering GET and POST both resolved identity and then required a pre-existing Firestore user doc. Users who had authenticated but never opened their profile page have no doc, so fetch_user_by_user_id returned None -> 404 on both read and write. On /volunteer/track this surfaced as "Failed to load your volunteer data" and also blocked them from starting a session ("not working for some people"). - Add _resolve_and_ensure_user(): lazily creates the users doc when missing (mirrors get_profile_metadata's save_user path). - get_volunteering_time now returns ([], 0, 0) instead of None/404 so the page shows a clean zero-state, and filters in a single pass (an entry may carry commitmentHours, finalHours, or BOTH — no duplicate rows). - save_volunteering_time accepts an optional timestamp (backdated manual logs) and a manual:true flag; hours are float-cleaned, non-negative, capped. Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 3 + services/users_service.py | 167 ++++++++++++++++++++------------------ 2 files changed, 91 insertions(+), 79 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index eeb3296..f4e9c74 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -123,6 +123,9 @@ To send a Slack DM, pass the Slack user ID as `channel`: `send_slack(message=... These are DIFFERENT VALUES. When bundling user data for the frontend, include both: `{user_id: propel_id, db_id: firestore_doc_id, name, profile_image}`. The frontend needs `db_id` to build profile links and `user_id` (propel) for matching against assignees/editors/mentions. +## Volunteer time tracking (`/api/users/volunteering`) +GET/POST in `api/users/users_views.py` → `services/users_service.py`. Both resolve identity through `_resolve_and_ensure_user(propel_id)`, which **lazily creates the `users` doc when missing** (the lazy-creation gotcha). Before this, a user who'd authenticated but never opened their profile had no doc, so `fetch_user_by_user_id` → None → 404 on BOTH read and write — surfaced on the frontend as "Failed to load your volunteer data" and blocked them from starting a session ("not working for some people"). `get_volunteering_time` now returns `([], 0, 0)` (never None/404) so the page shows a clean zero-state, and filters in a SINGLE pass — an entry may carry `commitmentHours`, `finalHours`, or BOTH (manual logs send both), no concat/duplicate. `save_volunteering_time` accepts an optional `timestamp` (backdated manual logs) + `manual:true` flag; hours are float-cleaned, non-negative, capped at 1000. + ## Admin Email Templates (`email_templates` collection) Powers the frontend `/admin/communication` template editor and the volunteer send-email dialogs. Blueprint: `api/email_templates/email_templates_views.py` (`/api/admin/templates*`, all `volunteer.admin`-gated); service: `services/email_templates_service.py`; seed data: `services/email_templates_seed.py`. - Layout: `email_templates/{slug}` main doc + `email_templates/{slug}/versions/{000N}` append-only content snapshots. `version` on the main doc = current version number; version doc ids are zero-padded for natural ordering. diff --git a/services/users_service.py b/services/users_service.py index 4aa8e33..24f1b76 100644 --- a/services/users_service.py +++ b/services/users_service.py @@ -399,56 +399,83 @@ def remove_user_by_slack_id(user_id): def get_users(): return fetch_users() -def save_volunteering_time(propel_id, json): - logger.info(f"Save Volunteering Time for {propel_id} {json}") +def _resolve_and_ensure_user(propel_id): + """Resolve a propel_id to a Firestore User, lazily creating the user doc + when the person has authenticated but never opened their profile page (the + documented lazy-creation gotcha). Returns (user, user_id); (None, None) only + when PropelAuth identity can't be resolved at all (transient OAuth failure). + + Before this, the volunteering endpoints returned None -> 404 for any user + without a pre-existing doc, which surfaced as "Failed to load your volunteer + data" and also blocked them from starting a session. New users now just work. + """ 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) - return None + return None, None user_id = oauth_user["sub"] + user = fetch_user_by_user_id(user_id) + if user is None: + info(logger, "Lazily creating user doc on first volunteering action", user_id=user_id) + save_user( + user_id=user_id, + email=oauth_user.get("email", ""), + last_login=datetime.now().isoformat() + "Z", + profile_image=( + oauth_user.get("https://slack.com/user_image_192") + or oauth_user.get("picture") + or "" + ), + name=oauth_user.get("name", ""), + nickname=oauth_user.get("given_name", ""), + propel_id=propel_id, + ) + user = fetch_user_by_user_id(user_id) + return user, user_id - logger.info(f"Save Volunteering Time for {user_id} {json}") - # Get the user - user = fetch_user_by_user_id(user_id) +def save_volunteering_time(propel_id, json): + logger.info(f"Save Volunteering Time for {propel_id} {json}") + user, user_id = _resolve_and_ensure_user(propel_id) if user is None: - warning(logger, "User not found", user_id=user_id) - return + warning(logger, "Could not resolve/create user for volunteering save", propel_id=propel_id) + return None - timestamp = datetime.now().isoformat() + "Z" - reason = json["reason"] # The kind of volunteering being done + # Allow backdating a manually-logged entry; default to now (UTC). + timestamp = json.get("timestamp") or (datetime.now().isoformat() + "Z") + reason = json.get("reason", "") # The kind of volunteering being done + + # A single entry can carry committed hours (set when a live session starts), + # actively-tracked hours (set when a session ends), or BOTH (manual log of + # actual time done away from the keyboard). Counting both on one entry is fine + # because get_volunteering_time no longer concatenates two filtered lists. + def _clean_hours(value): + try: + hours = round(float(value), 2) + except (TypeError, ValueError): + return None + if hours < 0: + return None + return min(hours, 1000) # defensive cap against garbage input - if "finalHours" in json: - finalHours = json["finalHours"] # This is sent at when volunteering is done - if finalHours is None: - error(logger, "finalHours is None", user_id=user_id) - return + commitment_hours = _clean_hours(json["commitmentHours"]) if "commitmentHours" in json else None + final_hours = _clean_hours(json["finalHours"]) if "finalHours" in json else None - user.volunteering.append({ - "timestamp": timestamp, - "finalHours": round(finalHours,2), - "reason": reason - }) + if commitment_hours is None and final_hours is None: + error(logger, "No valid hours provided for volunteering entry", user_id=user_id) + return None - # Add to the total - upsert_profile_metadata(user) + entry = {"timestamp": timestamp, "reason": reason} + if commitment_hours is not None: + entry["commitmentHours"] = commitment_hours + if final_hours is not None: + entry["finalHours"] = final_hours + if json.get("manual"): + entry["manual"] = True - # We keep track of what the user is committing to do but we don't show this - # The right way to do this is likely to get a session id when they start volunteering and the frontend uses that to close out the volunteering session when it is done - # But this way is simpler for now - elif "commitmentHours" in json: - commitmentHours = json["commitmentHours"] # This is sent at the start of volunteering - if commitmentHours is None: - error(logger, "commitmentHours is None", user_id=user_id) - return - - user.volunteering.append({ - "timestamp": timestamp, - "commitmentHours": round(commitmentHours,2), - "reason": reason - }) - upsert_profile_metadata(user) + user.volunteering.append(entry) + upsert_profile_metadata(user) # Clear cache for get_profile_metadata get_profile_metadata.cache_clear() @@ -457,50 +484,32 @@ def save_volunteering_time(propel_id, json): def get_volunteering_time(propel_id, start_date, end_date): logger.info(f"Get Volunteering Time for {propel_id} {start_date} {end_date}") - 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) - return None - - user_id = oauth_user["sub"] - - logger.info(f"Get Volunteering Time for {user_id} start: {start_date} end: {end_date}") - - # Get the user - user = fetch_user_by_user_id(user_id) + user, user_id = _resolve_and_ensure_user(propel_id) if user is None: - warning(logger, "User not found", user_id=user_id) - return None - - # Filter the volunteering data - volunteeringActiveTime = [] - for v in user.volunteering: - if "finalHours" in v: - if start_date is not None and end_date is not None: - if v["timestamp"] >= start_date and v["timestamp"] <= end_date: - volunteeringActiveTime.append(v) - else: - volunteeringActiveTime.append(v) - - volunteeringCommittmentTime = [] - for v in user.volunteering: - if "commitmentHours" in v: - if start_date is not None and end_date is not None: - if v["timestamp"] >= start_date and v["timestamp"] <= end_date: - volunteeringCommittmentTime.append(v) - else: - volunteeringCommittmentTime.append(v) - - totalActiveHours = sum([v["finalHours"] for v in volunteeringActiveTime]) - totalCommitmentHours = sum([v["commitmentHours"] for v in volunteeringCommittmentTime]) - - # Merge volunteeringActiveTime and volunteeringCommittmentTime - # This is a bit of a hack but it is easier to do it this way than to try to do it in the frontend - allVolunteering = volunteeringActiveTime + volunteeringCommittmentTime + # Identity couldn't be resolved (transient OAuth issue). Return an empty + # dataset rather than 404 so the page shows a clean zero-state. + warning(logger, "Could not resolve user for volunteering read; returning empty", propel_id=propel_id) + return [], 0, 0 + + def _in_range(v): + if start_date is None or end_date is None: + return True + ts = v.get("timestamp", "") + return start_date <= ts <= end_date + + # Single pass, no duplication. Each entry appears once and contributes to + # whichever totals its fields cover. + filtered = [v for v in (user.volunteering or []) if _in_range(v)] + total_active_hours = round(sum((v.get("finalHours") or 0) for v in filtered), 2) + total_commitment_hours = round(sum((v.get("commitmentHours") or 0) for v in filtered), 2) + + logger.debug( + f"volunteering entries: {len(filtered)} " + f"Total Active Hours: {total_active_hours} " + f"Total Commitment Hours: {total_commitment_hours}" + ) - logger.debug(f"allVolunteering: {allVolunteering} || volunteeringActiveTime: {volunteeringActiveTime} volunteeringCommittmentTime: {volunteeringCommittmentTime} Total Active Hours: {totalActiveHours} Total Commitment Hours: {totalCommitmentHours}") - - return allVolunteering, totalActiveHours, totalCommitmentHours + return filtered, total_active_hours, total_commitment_hours def get_all_volunteering_time(start_date=None, end_date=None): logger.info(f"Get All Volunteering Time for start: {start_date} end: {end_date}")