diff --git a/.github/workflows/deadline-reminders.yml b/.github/workflows/deadline-reminders.yml new file mode 100644 index 0000000..56bb354 --- /dev/null +++ b/.github/workflows/deadline-reminders.yml @@ -0,0 +1,28 @@ +name: ⏰ Submission deadline reminders + +# Hourly cron that pings POST /api/hackathons/deadlines/remind-due. The +# route itself iterates every currently-running event x {24,6,1} hour +# thresholds and no-ops (`only_if_due=True`) outside each due window, so +# calling it once an hour never double-sends reminders. Requires the +# BACKEND_CRON_TOKEN repo secret to match the backend's own +# BACKEND_CRON_TOKEN env var (checked via common.utils.api_key.check_api_key). + +on: + schedule: + - cron: "7 * * * *" + workflow_dispatch: {} + +permissions: + contents: read + +jobs: + remind: + name: πŸ”” Send due reminders + runs-on: ubuntu-latest + steps: + - name: πŸ“£ POST remind-due + run: | + curl -fsS -X POST "${BACKEND_URL:-https://api.ohack.dev}/api/hackathons/deadlines/remind-due" \ + -H "X-Api-Key: ${{ secrets.BACKEND_CRON_TOKEN }}" + env: + BACKEND_URL: ${{ vars.BACKEND_URL }} diff --git a/CLAUDE.md b/CLAUDE.md index 7e75484..ccdd387 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -71,6 +71,9 @@ Deployed to Fly.io (`fly.toml`, app: `backend-ohack`, region: `sjc`). Uses gunic ## Gotchas (load-bearing β€” every one of these has bitten us) +### `common.utils.firebase` needs `load_dotenv()` to have already run +`common/utils/firebase.py` reads `FIREBASE_CERT_CONFIG` at **module import time** (`json.loads(safe_get_env_var(...))`), with no lazy fallback β€” if that env var isn't in `os.environ` yet, the import raises `JSONDecodeError` immediately. `common/utils/slack.py` (and `cdn.py`, `github.py`, `openai_api.py`) call `load_dotenv()` at their own import time, which populates `.env` into the process env. **Import `common.utils.slack` (or another dotenv-calling module) before `common.utils.firebase`** in any new module that needs both β€” every existing service that imports both (`api/mentors/mentors_service.py`, `api/submissions/submissions_service.py`, `api/peer_votes/peer_votes_service.py`) does slack-then-firebase for exactly this reason. Getting the order backwards works fine in production (something else has already called `load_dotenv()` by the time your module loads) but breaks standalone script/test invocations that import your module directly. + ### Python 3.9 β€” no PEP 604 union syntax Backend runs on Python 3.9. `def foo() -> X | None:` raises `TypeError` at *import time*, blowing up every endpoint that imports the module. Use `Optional[X]` from `typing`. Audit any new `services/` module before committing. @@ -226,3 +229,44 @@ The public profile payload (`GET /api/users//profile/public`) is now ## Problem statement "Who's helping" roster (Sep 2026, frontend #359) `GET /api/problem-statements//helpers` (`api/problemstatements/problem_statement_views.py`, public) β†’ `services/problem_statements_service.py::get_problem_statement_helpers`. The raw `helping` array is append-only history (`{user: , slack_user, type, timestamp}`) and real docs carry the same person 2–3Γ— from double clicks, so `normalize_helping_entries` collapses to one row per person (earliest `timestamp` β†’ `since`, latest `type` wins, junk entries dropped, oldest first) and `_enrich_helpers_batch` attaches `name/nickname/profile_image` with ONE `db.get_all` (same public-safe field set as team rosters β€” no email/propel_id). 60s TTL cache (`_helpers_cache`, `clear_helpers_cache(ps_id)`) cleared inside `save_helping_status`, which now also updates a returning helper's entry in place (keeps their original timestamp, collapses their duplicates) instead of appending. 404 when the doc doesn't exist. Tests: `api/problemstatements/tests/test_helpers.py` (run per-directory). The frontend keeps a counts-only fallback when this route is missing, so deploy order doesn't matter. + +## Project submissions + Hackers' Choice (Sep 2026) + +Backend half of replacing DevPost with an in-house team dashboard (frontend plan: `docs/plans/team-dashboard-devpost-replacement.md`). Judging itself (rubric, rounds, scoring, results) is **completely unchanged** β€” the only judging-facing addition is the team's demo video (see the judging bug-fix note below). Two new blueprints, both registered in `api/__init__.py` after `broadcasts_views`: + +### `api/submissions/` β€” team project write-ups + submission deadlines +Self-serve, deadline-aware writes to a team's `project_*` fields, split deliberately from `api.teams.teams_service.edit_team` (the admin write path β€” no deadline gate, no membership check, org-permission gated at the route; an admin can override any `project_*` field, including `project_submission_status` β€” validated against `draft|submitted|late`, 400 on anything else, no write β€” via `PATCH /api/team/edit`; `project_tagline`/`project_story` get the same `sanitize_markdown` treatment there as the self-serve path, and a status *change* stamps `project_updated_at`). Every write goes through `_authorize_team_write(propel_user_id, team_id, admin, enforce_deadline)`: 404 unknown team β†’ 403 `not_team_member` (non-member, non-admin) β†’ 409 `submissions_closed{deadline,late_until,now}` once the event's submission window has closed, unless `admin=True` or the caller opted out (`enforce_deadline=False`, used only by mentor-availability). Full contract, sanitization rationale, and CDN-image validation rules: `api/submissions/README.md`. + +`submit_project` checks the already-submitted idempotent path **before** the deadline gate β€” a team that submitted on time and revisits the dashboard after the window closes still gets 200 `already_submitted:true`, never a spurious 409; only a *not-yet-submitted* team is blocked past close (unless admin). `self_serve_team_edit` (bridge for `/devpost` and `/demo-video`) calls this module's own `clear_cache()` after delegating to `edit_team` β€” `edit_team` alone only busts the generic per-function caches, not `services.hackathons_service`'s separately-cached `get_single_hackathon_event` (10-min TTL), so without this the event page kept showing a stale DevPost link/demo video for up to 10 minutes after a self-serve save. + +New team-doc fields (absent β‡’ legacy team, no dashboard implied): `project_tagline`, `project_story` (raw markdown; the frontend renders it via react-markdown **without** rehype-raw, so `sanitize_markdown` in `common/utils/validators.py` is defence-in-depth, not the primary XSS boundary β€” it loops its tag-strip to a fixpoint so a nested bypass like `ipt>` can't reassemble into a live tag on a single pass, matches `on*=` attributes glued to a `/` as well as whitespace [``], neutralizes `javascript:`/`vbscript:`/`data:` targets whether quoted or unquoted, also neutralizes the same targets in markdown link/image syntax [`](javascript:...)` β†’ `](#)`], and deliberately preserves generic `<` so `List`/`Map` in a write-up survives), `project_built_with`, `project_links`, `project_thumbnail_url`, `project_images`, `project_updated_at`, `project_submitted_at`, `project_submission_status` (`draft|submitted|late`), `mentor_help_wanted` (absent β‡’ **True**, a signal-only "open to mentors / heads-down" toggle with no deadline gate and no other behavior change). + +New hackathon-doc field: `deadlines` (`{submission, late_submission_until, voting_opens, voting_closes}`, each a tz-normalized ISO string or `None`). `common/utils/validators.py::validate_deadlines` normalizes naive datetimes into the event's own timezone (`normalize_deadline_iso`) and enforces `submission <= late_submission_until` / `voting_opens < voting_closes` when both sides of a pair are present in the same payload; an unknown key or a bad value skips the *whole* `deadlines` field in `validate_hackathon_data_partial` (goes into `skipped_fields`) rather than partially applying it. `services/hackathons_service.py::save_hackathon` turns an explicit `None` into a Firestore `DELETE_FIELD` sentinel on an **update** (so a cleared deadline actually clears, not just leaves the old value under `merge=True`) but simply omits it on **create** (nothing to delete yet). **`deadlines: {}` (and top-level `deadlines: null`) on an update is a no-op, not a clear-all** β€” an empty map merges zero sub-fields into the stored `deadlines` map under `merge=True`, leaving it untouched; to clear one deadline, send `{key: null}` for that key specifically. `get_single_hackathon_event` strips `project_story` off every team in the response (it can run to ~20k chars and the endpoint already fans out to every team on the event) β€” the dashboard/team page fetch it via the per-team routes instead. Both `compute_submission_window` (here) and `compute_voting_window` (`api/peer_votes/`) re-parse the *stored* deadline strings through `normalize_deadline_iso` before comparing them against `now` β€” a naive or `"Z"`-suffixed stored value used to raise (TypeError comparing naive-vs-aware, or ValueError on Python 3.9/3.10's `fromisoformat` rejecting `"Z"`) and reach the caller as a 500; an unparseable value is now logged and treated as absent (`no_deadline` / `closed`) instead. + +Deadline reminders (`build_reminder_message`, `send_deadline_reminders`, `send_due_reminders_for_current_events`, same file): a per-team Slack nudge naming only what THAT team still owes (never nags an already-submitted team β€” returns `None`). Idempotency key is `reminders_sent[f"{kind}_{hours_before}h"]` on the hackathon doc; `only_if_due=True` (used by the hourly cron, `.github/workflows/deadline-reminders.yml`, `X-Api-Key: BACKEND_CRON_TOKEN` via `common/utils/api_key.check_api_key`) no-ops outside a **one-hour-wide** `[deadline - hours_before, deadline - hours_before + 1h)` window instead of erroring, so the cron can safely call `POST /api/hackathons/deadlines/remind-due` every hour for every current event Γ— {24,6,1}h without spamming teams the other 23 hours. (The window used to extend all the way to the deadline itself β€” `[deadline - hours_before, deadline)` β€” which meant a deadline set with only a few hours' notice fell inside BOTH the 24h and 6h due windows on the very first cron tick and fired both reminders at once; each tier now gets its own narrow hour-wide slot, and a deadline set too close to fire a given tier's slot simply skips that tier rather than double-firing.) The admin "Send reminder now" button hits `POST /api/hackathons//deadlines/remind` with a Bearer token (`@auth.optional_user` + `is_admin(auth_user)`) instead of the API key. **`BACKEND_CRON_TOKEN` must be set on both Fly (backend env) and as a GitHub Actions secret** β€” this PR does not set either; do that before merging or the cron 403s. + +### `api/peer_votes/` β€” Hackers' Choice (assigned-slate approval vote) +Anti-popularity peer award, deliberately NOT a raw "vote for your favorite": each eligible voter (an `isSelected` hacker; `constraints.peer_vote_requires_submission` additionally requires the voter's own team to have submitted) gets a **deterministic, exposure-balanced slate** of `constraints.peer_vote_slate_size` (default 5) submitted projects, **never their own team** β€” seeded on `sha256(f"{event_id}:{propel_id}")` so the same voter always sees the same slate, sorted by current exposure ascending so under-shown projects surface first. Voters **approve, not rank**: pick up to `constraints.peer_vote_max_picks` (default 2, clamped in `_settings()` to at most `slate_size - 1` at READ time β€” `validate_hackathon_data_partial` only enforces that relationship when both fields are present in the same PATCH payload, so a doc can end up with an inconsistent stored pair; every voter-facing route re-clamps rather than trusting the stored value). Scoring is the **Wilson score interval lower bound** (z=1.96) of approvals/shown, not a raw rate β€” `wilson_lower_bound(0,0)==0`, `(5,5)β‰ˆ0.566`, `(1,1)β‰ˆ0.207` β€” so a team shown to 2 people who both approved doesn't outrank a team shown to 40 with a 90% rate. `shown` in `compute_results` counts **cast ballots** (non-voided, with picks) that included the team in their slate β€” NOT the raw exposure-doc count, which only reflects how many slates the team was ever persisted into (someone who opened the page but never voted). The raw count is kept separately as `exposure_shown`. **No tallies are ever shown to a voter**, only to admins (`GET .../peer-vote/results`, which also returns a `ballots_detail: [{voter_propel_id, voted_at, voided, picks_count}]` list β€” no names, emails, or picks β€” so the admin UI can drive `void` without seeing who voted for what). A voided ballot's own slate page renders `status: "voided"` with `picks: null` β€” never `"voted"` with stale picks. One ballot per voter (deterministic doc id `f"{event_id}__{propel_id}"` in the top-level `peer_votes` collection, `/` replaced with `_`), re-votable until close (both the pick-update in `submit_ballot` and the void in `void_ballot` do a **full `set()`**, not `set(merge=True)` β€” the whole doc is rebuilt from the existing one so it stays a complete, self-describing record), admin-voidable, admin-publishable. `POST .../peer-vote/publish` appends `"Hackers' Choice"` to the winning team's `awards[]` **once** (idempotent) and writes a public summary doc at `hackathons/{doc}/peer_vote/summary` (exposure counts live at `hackathons/{doc}/peer_vote/exposure`) β€” but refuses with 409 `no_ballots` when `results["ballots"] == 0` OR the computed rank-1 team has zero approvals (compute_results emits a row for every *submitted* team regardless of vote count, so with no ballots cast every row ties at a Wilson score of 0 and the sort falls through to team name, which would otherwise crown an arbitrary "winner" of a vote nobody voted in). `constraints.peer_vote_enabled` (default **False**) is read on every voter-facing route β€” a disabled or unconfigured event returns `{"status": "disabled"}` from the slate route and 403 `peer_vote_disabled` from the ballot route, regardless of anything else. Full contract: `api/peer_votes/peer_votes_service.py` module docstring + function docstrings. + +Firestore transaction note: `get_slate`'s first-ever materialization for a voter (persisting the slate + incrementing exposure) runs inside `_in_transaction(db, body)`, which re-checks the ballot doc's existence **inside** the transaction (not just the optimistic outer read before it) so two near-simultaneous requests from the same voter can't double-build a slate or double-increment exposure. `_in_transaction` is the one seam tests monkeypatch (real `@firestore.transactional` needs a live Firestore client) β€” swap it for `lambda db, body: body(FakeTransaction(...))`. + +### Judging β€” video-only change + 3 bug fixes (`api/judging/judging_service.py`) +No rubric, scoring, round, or results logic changed. `get_team_details`/`format_team_for_judge` now also return `demo_video_url` (the real field a team's dashboard writes) and fill the legacy `video_url` key from it (`team.get('demo_video_url') or team.get('video_url', '')`) so both old and new judge-page code read a working value. Bugs found and fixed along the way (all pre-existing, none related to the video change): +- `get_bulk_judge_details` always returned an empty judges list β€” it called an undefined name, `fetch_judge_scores_by_event` (the correctly-named `fetch_judge_scores_by_event_id` was imported but never used), which NameError'd straight into the function's own blanket `except`. One-line rename fixes it. +- `update_judge_assignment_details` (`PUT /api/judge/assignments/`) always 400'd "Assignment not found" β€” it looked assignments up via `fetch_judge_assignments_by_judge_id("")` (an empty judge_id can never match a real assignment) instead of by the assignment's own id. Added `fetch_judge_assignment_by_id` (direct doc-get, `db/firestore.py` + `db/db.py`) and used that instead. +- `api/github/github_views.py`'s `/issues` route let a request through with no `org` (the service then 400'd with a 200 status, since the view unconditionally returned `jsonify(...)` with no status code), and logged `len(issues)` where `issues` is the service's response **dict** β€” that logged the dict's key count, not the issue count. Both fixed while adding `/activity` (see below). + +### `GET /api/github/activity?org&repo` β€” team dashboard "Code activity" card +`common/utils/github.py::get_repo_activity(org, repo)` makes **exactly 3** GitHub API calls (rate-limit budget, not completeness β€” this is a live-ish snapshot): `g.get_repo(f"{org}/{repo}")`, `repo.get_commits().get_page(0)` (first page only, ≀100 commits; a 409 "empty repository" is a valid all-zeros result for a fresh team repo, not an error), `repo.get_pulls(state="open").totalCount`. Contributors are derived from that single commit page (a `Counter` over each commit's author), not a separate stats call. `api/github/github_service.py::get_github_activity` validates `org`/`repo` against `^[A-Za-z0-9_.-]{1,100}$`, caches successes only for 300s (`_ACTIVITY_CACHE`, separate from the existing `_ISSUES_CACHE`), and translates `UnknownObjectException`β†’404 `repo_not_found`, `RateLimitExceededException`β†’503 `github_rate_limited`, anything elseβ†’502 `github_unavailable`. + +### `GET /api/volunteer//me?type=hacker` +Widened the existing mentor self-check route. `services/volunteers_service.py::get_volunteer_self_status(propel_user_id, event_id, volunteer_type)` mirrors `api.mentors.mentors_service.get_mentor_self_status`'s shape/leanness (`{f"is_{type}": bool, "volunteer": {"name","isSelected"}|None}`) for any `volunteer_type`, via the existing `find_volunteer_by_caller_identity` resolver. Backs the team dashboard's "am I an approved hacker for this event" gate and Hackers' Choice eligibility. `type=mentor` still delegates to the original mentor-specific service unchanged. + +### Security fix: `/devpost` and `/demo-video` now check team membership +`POST /api/team//devpost` and `POST /api/team//demo-video` (`api/teams/teams_views.py`) used to call `edit_team` **directly with no membership check at all** β€” any logged-in user could overwrite any team's Devpost link or demo video. Both now lazy-import `api.submissions.submissions_service.self_serve_team_edit`, which runs the same `_authorize_team_write` gate as every other self-serve write in this feature (403 non-member, 409 closed-window unless admin). `api.teams.teams_service` itself must never import `api.submissions` (keep the dependency one-directional) β€” the import lives in the view function, not the service module. + +### Documented-only findings (not fixed β€” out of scope or not trivial) +- **Legacy `save_team`** (`services/teams_service.py`, backing the old `POST /api/messages/team` β€” the route's own docstring says "kept for backward compatibility, new teams should use /api/team/queue") calls `create_github_repo` with an outdated positional argument order/count (missing `org_name`/`devpost_url`, has two extra params the current signature doesn't take). Every call to this legacy path already fails. Not fixed here: the correct fix needs a hackathon-event lookup this function doesn't currently do, and the path appears to have no live callers left β€” a real fix belongs with a decision about whether to delete the legacy route instead of repairing it. +- **`hackathon.devpost_url`** is read in `services/volunteers_service.py` but `save_hackathon` never writes a top-level `devpost_url` field on the hackathon doc β€” it's `links[]`-only. Not a bug introduced or fixed here; DevPost is now optional/deprecated for the team dashboard anyway, so `links[]` stays the source of truth. +- **`GET /api/hacker/applications/`** (`api/volunteers/volunteers_views.py`, public, `@auth.optional_user`) exposes `user_id` and `isSelected` for every hacker applicant via `get_all_hackers_by_event_id`. `findteam.js` matchmaking on the frontend depends on `user_id` being present, so this isn't trivially fixable without a frontend change too β€” a lean projection (drop `user_id`/`isSelected` from the public shape, resolve matching some other way) is a good follow-up but out of scope here. +- **Judge-page field naming split**: some frontend judge-page code reads `devpost_url`/`video_url` while the team doc itself uses `devpost_link`/`demo_video_url`. The judging API now fills both `demo_video_url` and `video_url` (see above) so either naming convention on the frontend keeps working; a follow-up could rename one side for consistency but isn't required. diff --git a/api/__init__.py b/api/__init__.py index e3848d3..5378b85 100644 --- a/api/__init__.py +++ b/api/__init__.py @@ -192,6 +192,8 @@ def add_headers(response): from api.praisebot import praisebot_views from api.jobs import jobs_views from api.broadcasts import broadcasts_views + from api.submissions import submissions_views + from api.peer_votes import peer_votes_views app.register_blueprint(messages_views.bp) app.register_blueprint(exception_views.bp) @@ -219,5 +221,7 @@ def add_headers(response): app.register_blueprint(praisebot_views.bp) app.register_blueprint(jobs_views.bp) app.register_blueprint(broadcasts_views.bp) + app.register_blueprint(submissions_views.bp) + app.register_blueprint(peer_votes_views.bp) return app diff --git a/api/github/github_service.py b/api/github/github_service.py index 670779b..26f58a9 100644 --- a/api/github/github_service.py +++ b/api/github/github_service.py @@ -1,8 +1,9 @@ +import re import logging from typing import Dict, Any, List from cachetools import TTLCache from db.db import get_db -from common.utils.github import create_issue, get_issues +from common.utils.github import create_issue, get_issues, get_repo_activity logger = logging.getLogger("api.github.github_service") logger.setLevel(logging.DEBUG) @@ -13,6 +14,62 @@ # GitHub failures retry on the next request. _ISSUES_CACHE = TTLCache(maxsize=512, ttl=600) +# Team dashboard "Code activity" card (GET /api/github/activity). Separate, +# shorter-TTL cache from _ISSUES_CACHE since this is meant to feel live-ish. +_ACTIVITY_CACHE = TTLCache(maxsize=512, ttl=300) + +# GitHub org/repo name charset. Loose but enough to reject path-injection-ish +# input before it reaches PyGithub. +_GITHUB_NAME_RE = re.compile(r"^[A-Za-z0-9_.-]{1,100}$") + + +def get_github_activity(org_name: str, repo_name: str) -> Dict[str, Any]: + """ + GET /api/github/activity backing service. Success-only cache (mirrors + get_github_issues): a rate-limit or repo-not-found response is never + cached, so the next request retries against GitHub instead of pinning a + transient failure for the TTL window. + + Returns (payload, status): + 200 {success, repo, commits, contributors, open_prs} + 400 {"error": "invalid_repo"} β€” org/repo fails the name regex + 404 {"error": "repo_not_found"} + 503 {"error": "github_rate_limited", "reset_at"} + 502 {"error": "github_unavailable"} β€” anything else from PyGithub + """ + if not org_name or not repo_name or not _GITHUB_NAME_RE.match(org_name) or not _GITHUB_NAME_RE.match(repo_name): + return {"error": "invalid_repo"}, 400 + + cache_key = (org_name, repo_name) + cached = _ACTIVITY_CACHE.get(cache_key) + if cached is not None: + return cached, 200 + + from github import UnknownObjectException, RateLimitExceededException, GithubException + + try: + result = get_repo_activity(org_name, repo_name) + except UnknownObjectException: + logger.info("get_github_activity: repo not found org=%s repo=%s", org_name, repo_name) + return {"error": "repo_not_found"}, 404 + except RateLimitExceededException as e: + reset_at = None + try: + reset_at = e.headers.get("x-ratelimit-reset") if e.headers else None + except Exception: + reset_at = None + logger.warning("get_github_activity: rate limited org=%s repo=%s", org_name, repo_name) + return {"error": "github_rate_limited", "reset_at": reset_at}, 503 + except GithubException as e: + logger.error("get_github_activity: GitHub error org=%s repo=%s: %s", org_name, repo_name, e) + return {"error": "github_unavailable"}, 502 + except Exception as e: + logger.error("get_github_activity: unexpected error org=%s repo=%s: %s", org_name, repo_name, e) + return {"error": "github_unavailable"}, 502 + + _ACTIVITY_CACHE[cache_key] = result + return result, 200 + def get_github_organization_data(org_name: str) -> Dict[str, Any]: """ Get GitHub organization data including repositories and contributors. diff --git a/api/github/github_views.py b/api/github/github_views.py index e5ef428..75b8501 100644 --- a/api/github/github_views.py +++ b/api/github/github_views.py @@ -6,7 +6,8 @@ get_github_contributors_by_org, get_github_contributors_by_repo, create_github_issue, - get_github_issues + get_github_issues, + get_github_activity, ) from common.auth import auth, auth_user, getOrgId @@ -180,7 +181,9 @@ def get_issues_api(): Query Parameters: repo: Required. The repository name. - org: Optional. The organization name. + org: Required. The organization name (the service 400s without it + anyway β€” this used to let a missing org through to a 200 error + body instead of a real 400). state: Optional. The state of the issues ('open', 'closed', 'all'). Defaults to 'open'. Returns: @@ -193,13 +196,39 @@ def get_issues_api(): if not repo_name: return jsonify({"error": "repo parameter is required"}), 400 + if not org_name: + return jsonify({"error": "org parameter is required"}), 400 logger.info("Getting issues for repo: %s, org: %s, state: %s", repo_name, org_name, state) issues = get_github_issues(repo_name=repo_name, org_name=org_name, state=state) - logger.info("Retrieved %d issues for repo: %s", len(issues), repo_name) + # `issues` is the service's response dict ({"success", "issues": [...]} + # or {"error": ...}) β€” logging len(issues) here was logging the dict's + # KEY COUNT, not the issue count. + issue_count = len(issues.get("issues", [])) if isinstance(issues, dict) else 0 + logger.info("Retrieved %d issues for repo: %s", issue_count, repo_name) return jsonify(issues) except Exception as e: logger.error("Error getting issues: %s", str(e)) - return jsonify({"error": str(e)}), 500 \ No newline at end of file + return jsonify({"error": str(e)}), 500 + + +@bp.route("/activity", methods=["GET"]) +def get_activity_api(): + """ + Team dashboard "Code activity" card: last commit, commits in the last + 24h, top contributors, open PR count. Public (same trust level as + /issues β€” no team-membership check; the payload has nothing private). + + Query Parameters: + org: Required. The GitHub organization name. + repo: Required. The repository name. + """ + org_name = request.args.get('org') + repo_name = request.args.get('repo') + if not org_name or not repo_name: + return jsonify({"error": "org and repo parameters are required"}), 400 + + payload, status = get_github_activity(org_name, repo_name) + return jsonify(payload), status \ No newline at end of file diff --git a/api/github/tests/__init__.py b/api/github/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/api/github/tests/test_github_activity.py b/api/github/tests/test_github_activity.py new file mode 100644 index 0000000..a7eb6c8 --- /dev/null +++ b/api/github/tests/test_github_activity.py @@ -0,0 +1,275 @@ +""" +Tests for common.utils.github.get_repo_activity and its +api.github.github_service.get_github_activity wrapper. + +A fake Github client stands in for PyGithub so we can assert EXACTLY 3 +"API calls" are made (get_repo, get_commits().get_page(0), get_pulls().totalCount) +and control commit timestamps/authors precisely. +""" +import os +from datetime import datetime, timedelta, timezone + +os.environ.setdefault("ENVIRONMENT", "test") + +import pytest +from github import GithubException, UnknownObjectException, RateLimitExceededException + +import common.utils.github as github_utils + + +# --------------------------------------------------------------------------- +# Fakes +# --------------------------------------------------------------------------- + +class FakeAuthor: + def __init__(self, login=None, avatar_url=None): + self.login = login + self.avatar_url = avatar_url + + +class FakeGitAuthor: + def __init__(self, name, date): + self.name = name + self.date = date + + +class FakeGitCommit: + def __init__(self, message, author): + self.message = message + self.author = author + + +class FakeCommit: + def __init__(self, message, date, login=None, avatar_url=None, git_author_name="Someone"): + self.author = FakeAuthor(login, avatar_url) if login else None + self.commit = FakeGitCommit(message, FakeGitAuthor(git_author_name, date)) + + +class FakePullsList: + def __init__(self, calls, count): + self._calls = calls + self._count = count + + @property + def totalCount(self): + self._calls.append("get_pulls.totalCount") + return self._count + + +class FakeCommitsPaginator: + def __init__(self, calls, commits, raise_empty_409=False): + self._calls = calls + self._commits = commits + self._raise_empty_409 = raise_empty_409 + + def get_page(self, page): + self._calls.append(f"get_commits.get_page({page})") + if self._raise_empty_409: + raise GithubException(409, {"message": "Git Repository is empty."}, None) + return self._commits + + +class FakeRepo: + def __init__(self, calls, commits, open_prs=0, raise_empty_409=False, **attrs): + self._calls = calls + self._commits = commits + self._open_prs = open_prs + self._raise_empty_409 = raise_empty_409 + self.html_url = attrs.get("html_url", "https://github.com/opportunity-hack/test-repo") + self.default_branch = attrs.get("default_branch", "main") + self.pushed_at = attrs.get("pushed_at") + self.open_issues_count = attrs.get("open_issues_count", 0) + self.stargazers_count = attrs.get("stargazers_count", 0) + + def get_commits(self): + return FakeCommitsPaginator(self._calls, self._commits, raise_empty_409=self._raise_empty_409) + + def get_pulls(self, state="open"): + return FakePullsList(self._calls, self._open_prs) + + +class FakeGithubClient: + def __init__(self, calls, repo=None, unknown=False, rate_limited=False): + self._calls = calls + self._repo = repo + self._unknown = unknown + self._rate_limited = rate_limited + + def get_repo(self, full_name): + self._calls.append(f"get_repo({full_name})") + if self._unknown: + raise UnknownObjectException(404, {"message": "Not Found"}, None) + if self._rate_limited: + raise RateLimitExceededException(403, {"message": "rate limited"}, {"x-ratelimit-reset": "12345"}) + return self._repo + + +# --------------------------------------------------------------------------- +# get_repo_activity β€” exact call count, math, contributors, empty repo +# --------------------------------------------------------------------------- + +def _patch_github(monkeypatch, client): + monkeypatch.setattr(github_utils, "Github", lambda *a, **kw: client) + + +def test_get_repo_activity_makes_exactly_three_calls(monkeypatch): + calls = [] + now = datetime.now(timezone.utc) + commits = [FakeCommit("Fix bug", now - timedelta(hours=1), login="alice")] + repo = FakeRepo(calls, commits, open_prs=2) + _patch_github(monkeypatch, FakeGithubClient(calls, repo=repo)) + + result = github_utils.get_repo_activity("opportunity-hack", "test-repo") + + assert calls == ["get_repo(opportunity-hack/test-repo)", "get_commits.get_page(0)", "get_pulls.totalCount"] + assert result["open_prs"] == 2 + + +def test_get_repo_activity_last_24h_math(monkeypatch): + calls = [] + now = datetime.now(timezone.utc) + commits = [ + FakeCommit("Recent", now - timedelta(hours=2), login="alice"), + FakeCommit("Also recent", now - timedelta(hours=23), login="bob"), + FakeCommit("Old", now - timedelta(days=3), login="carol"), + ] + repo = FakeRepo(calls, commits) + _patch_github(monkeypatch, FakeGithubClient(calls, repo=repo)) + + result = github_utils.get_repo_activity("org", "repo") + + assert result["commits"]["total_recent"] == 3 + assert result["commits"]["last_24h"] == 2 + assert result["commits"]["last_author"] == "alice" + assert result["commits"]["last_commit_message"] == "Recent" + + +def test_get_repo_activity_top_contributors_capped_at_eight(monkeypatch): + calls = [] + now = datetime.now(timezone.utc) + commits = [FakeCommit(f"c{i}", now, login=f"user{i % 10}") for i in range(30)] + repo = FakeRepo(calls, commits) + _patch_github(monkeypatch, FakeGithubClient(calls, repo=repo)) + + result = github_utils.get_repo_activity("org", "repo") + + assert len(result["contributors"]) == 8 + logins = {c["login"] for c in result["contributors"]} + assert logins.issubset({f"user{i}" for i in range(10)}) + + +def test_get_repo_activity_falls_back_to_commit_author_name_without_github_login(monkeypatch): + calls = [] + now = datetime.now(timezone.utc) + commits = [FakeCommit("No github account", now, login=None, git_author_name="Jane Doe")] + repo = FakeRepo(calls, commits) + _patch_github(monkeypatch, FakeGithubClient(calls, repo=repo)) + + result = github_utils.get_repo_activity("org", "repo") + + assert result["commits"]["last_author"] == "Jane Doe" + assert result["contributors"][0]["login"] == "Jane Doe" + + +def test_get_repo_activity_empty_repo_returns_zeros_not_an_error(monkeypatch): + calls = [] + repo = FakeRepo(calls, [], raise_empty_409=True) + _patch_github(monkeypatch, FakeGithubClient(calls, repo=repo)) + + result = github_utils.get_repo_activity("org", "brand-new-repo") + + assert result["success"] is True + assert result["commits"]["total_recent"] == 0 + assert result["commits"]["last_24h"] == 0 + assert result["commits"]["last_commit_at"] is None + assert result["contributors"] == [] + + +def test_get_repo_activity_reraises_non_409_github_exception(monkeypatch): + calls = [] + repo = FakeRepo(calls, [], raise_empty_409=False) + repo.get_commits = lambda: (_ for _ in ()).throw(GithubException(500, {"message": "boom"}, None)) + _patch_github(monkeypatch, FakeGithubClient(calls, repo=repo)) + + with pytest.raises(GithubException): + github_utils.get_repo_activity("org", "repo") + + +def test_get_repo_activity_propagates_unknown_object(monkeypatch): + calls = [] + _patch_github(monkeypatch, FakeGithubClient(calls, unknown=True)) + with pytest.raises(UnknownObjectException): + github_utils.get_repo_activity("org", "nope") + + +def test_get_repo_activity_propagates_rate_limit(monkeypatch): + calls = [] + _patch_github(monkeypatch, FakeGithubClient(calls, rate_limited=True)) + with pytest.raises(RateLimitExceededException): + github_utils.get_repo_activity("org", "repo") + + +# --------------------------------------------------------------------------- +# api.github.github_service.get_github_activity β€” translation + caching +# --------------------------------------------------------------------------- + +import api.github.github_service as github_service + + +def test_get_github_activity_rejects_invalid_repo_name(): + payload, status = github_service.get_github_activity("org", "bad repo name!") + assert status == 400 + assert payload["error"] == "invalid_repo" + + +def test_get_github_activity_404s_on_unknown_repo(monkeypatch): + github_service._ACTIVITY_CACHE.clear() + monkeypatch.setattr(github_service, "get_repo_activity", lambda org, repo: (_ for _ in ()).throw(UnknownObjectException(404, {}, None))) + payload, status = github_service.get_github_activity("org", "repo") + assert status == 404 + assert payload["error"] == "repo_not_found" + + +def test_get_github_activity_503s_on_rate_limit(monkeypatch): + github_service._ACTIVITY_CACHE.clear() + monkeypatch.setattr( + github_service, + "get_repo_activity", + lambda org, repo: (_ for _ in ()).throw(RateLimitExceededException(403, {}, {"x-ratelimit-reset": "999"})), + ) + payload, status = github_service.get_github_activity("org", "repo") + assert status == 503 + assert payload["error"] == "github_rate_limited" + + +def test_get_github_activity_caches_success_only(monkeypatch): + github_service._ACTIVITY_CACHE.clear() + call_count = {"n": 0} + + def fake_activity(org, repo): + call_count["n"] += 1 + return {"success": True, "repo": {}, "commits": {}, "contributors": [], "open_prs": 0} + + monkeypatch.setattr(github_service, "get_repo_activity", fake_activity) + + payload1, status1 = github_service.get_github_activity("org", "repo") + payload2, status2 = github_service.get_github_activity("org", "repo") + + assert status1 == status2 == 200 + assert call_count["n"] == 1 # second call served from cache + + +def test_get_github_activity_does_not_cache_errors(monkeypatch): + github_service._ACTIVITY_CACHE.clear() + call_count = {"n": 0} + + def fake_activity(org, repo): + call_count["n"] += 1 + raise UnknownObjectException(404, {}, None) + + monkeypatch.setattr(github_service, "get_repo_activity", fake_activity) + + github_service.get_github_activity("org", "repo") + github_service.get_github_activity("org", "repo") + + assert call_count["n"] == 2 # never cached, so it's retried diff --git a/api/github/tests/test_github_views.py b/api/github/tests/test_github_views.py new file mode 100644 index 0000000..595eb9e --- /dev/null +++ b/api/github/tests/test_github_views.py @@ -0,0 +1,59 @@ +""" +Route-level tests for the /issues org-required fix (Part 9 bug #7) and the +new /activity route. +""" +import os + +os.environ.setdefault("ENVIRONMENT", "test") + +from flask import Flask +from unittest.mock import MagicMock + +from api.github import github_views + + +def _app(): + app = Flask(__name__) + app.register_blueprint(github_views.bp) + return app + + +def test_issues_requires_org(monkeypatch): + client = _app().test_client() + res = client.get("/api/github/issues?repo=some-repo") + assert res.status_code == 400 + assert "org" in res.get_json()["error"] + + +def test_issues_requires_repo(): + client = _app().test_client() + res = client.get("/api/github/issues?org=some-org") + assert res.status_code == 400 + assert "repo" in res.get_json()["error"] + + +def test_issues_logs_issue_count_not_dict_key_count(monkeypatch, caplog): + monkeypatch.setattr( + github_views, + "get_github_issues", + lambda repo_name, org_name, state: {"success": True, "issues": [{"issue_number": 1}, {"issue_number": 2}]}, + ) + client = _app().test_client() + with caplog.at_level("INFO"): + res = client.get("/api/github/issues?org=o&repo=r") + assert res.status_code == 200 + assert any("Retrieved 2 issues" in r.message for r in caplog.records) + + +def test_activity_route_requires_org_and_repo(): + client = _app().test_client() + assert client.get("/api/github/activity?repo=r").status_code == 400 + assert client.get("/api/github/activity?org=o").status_code == 400 + + +def test_activity_route_dispatches_to_service(monkeypatch): + monkeypatch.setattr(github_views, "get_github_activity", MagicMock(return_value=({"success": True}, 200))) + client = _app().test_client() + res = client.get("/api/github/activity?org=o&repo=r") + assert res.status_code == 200 + assert res.get_json()["success"] is True diff --git a/api/judging/judging_service.py b/api/judging/judging_service.py index c95f9c5..12125aa 100644 --- a/api/judging/judging_service.py +++ b/api/judging/judging_service.py @@ -4,6 +4,7 @@ from common.log import get_logger, debug, warning, error from db.db import ( fetch_judge_assignments_by_judge_id, + fetch_judge_assignment_by_id, fetch_judge_assignments_by_event_and_judge, fetch_judge_scores_by_judge_and_event, fetch_judge_score, @@ -257,12 +258,15 @@ def get_team_details(team_id: str) -> Dict: "members": members, # Now populated with actual member data "github_url": github_url, "devpost_url": team_data.get('devpost_link', ''), - "slack_channel": team_data.get('slack_channel', ''), - - - - # Not used - "video_url": team_data.get('video_url', ''), + "slack_channel": team_data.get('slack_channel', ''), + # demo_video_url is the real field written by the team dashboard's + # DemoVideoEditor / TeamStatusPanel; video_url is filled from it + # too so judge-side consumers reading either name keep working + # (Part 9 bug #2 β€” judges never received the team's demo video + # because this used to read a phantom `video_url` key that + # save/edit_team never writes). + "demo_video_url": team_data.get('demo_video_url', ''), + "video_url": team_data.get('demo_video_url') or team_data.get('video_url', ''), "demo_url": team_data.get('demo_url', ''), "technologies": team_data.get('technologies', []), "features": team_data.get('features', []) @@ -507,7 +511,10 @@ def format_team_for_judge(team: Dict, score_lookup: Dict = None, nonprofit_id: s "github_url": github_url, "devpost_url": team.get('devpost_link', ''), "slack_channel": team.get('slack_channel', ''), - "video_url": team.get('video_url', ''), + # See get_team_details' matching comment (Part 9 bug #2): demo_video_url + # is the real field; video_url is filled from it for legacy readers. + "demo_video_url": team.get('demo_video_url', ''), + "video_url": team.get('demo_video_url') or team.get('video_url', ''), "demo_time": None, # Will be overridden for round2 "judged": score_obj is not None, "score": score_obj.total_score if score_obj else None, @@ -579,14 +586,12 @@ def update_judge_assignment_details(assignment_id: str, demo_time: str = None, try: debug(logger, "Updating judge assignment", assignment_id=assignment_id) - # First fetch the existing assignment - # This is inefficient but works with current db interface - assignments = fetch_judge_assignments_by_judge_id("") - assignment = None - for a in assignments: - if a.id == assignment_id: - assignment = a - break + # First fetch the existing assignment. (Part 9 bug #4: this used to + # call fetch_judge_assignments_by_judge_id("") β€” an empty judge_id + # matches no real assignment β€” so `assignment` was always None and + # every call 400'd "Assignment not found". fetch_judge_assignment_by_id + # is a direct doc-get.) + assignment = fetch_judge_assignment_by_id(assignment_id) if not assignment: return {"success": False, "error": "Assignment not found"} @@ -954,8 +959,12 @@ def get_bulk_judge_details(event_id: str) -> Dict: # Get all assignments for the event assignments = fetch_judge_assignments_by_event_id(event_id) - # Get all of the scores for the event - scores = fetch_judge_scores_by_event(event_id) + # Get all of the scores for the event. (Part 9 bug #3: this called an + # undefined name, fetch_judge_scores_by_event β€” the correctly-named + # fetch_judge_scores_by_event_id was imported but never used here β€” + # so every call silently NameError'd into the outer except and this + # function always returned an empty judges list.) + scores = fetch_judge_scores_by_event_id(event_id) if "error" in judges_result: diff --git a/api/judging/tests/__init__.py b/api/judging/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/api/judging/tests/test_format_team.py b/api/judging/tests/test_format_team.py new file mode 100644 index 0000000..d9d70fa --- /dev/null +++ b/api/judging/tests/test_format_team.py @@ -0,0 +1,150 @@ +""" +Part 9 bug fixes in api/judging/judging_service.py: + #2 judges never received the team's demo video (get_team_details / + format_team_for_judge read a phantom `video_url` key) + #3 get_bulk_judge_details always returned empty (NameError on an undefined + function name, swallowed by a blanket try/except) + #4 update_judge_assignment_details always 400'd (looked assignments up by + an empty judge_id instead of the assignment's own id) + +No rubric/scoring/results behavior is touched by any of these. +""" +import os + +os.environ.setdefault("ENVIRONMENT", "test") + +from unittest.mock import patch + +import api.judging.judging_service as svc + + +# --------------------------------------------------------------------------- +# #2 β€” demo_video_url surfaced to judges (both formatters) +# --------------------------------------------------------------------------- + +def test_get_team_details_surfaces_demo_video_url(): + fake_team = {"team": { + "id": "team-1", "name": "Team One", "users": [], + "demo_video_url": "https://youtu.be/abc123", + "devpost_link": "https://devpost.com/x", + }} + with patch.object(svc, "get_team", return_value=fake_team): + result = svc.get_team_details("team-1") + + assert result["team"]["demo_video_url"] == "https://youtu.be/abc123" + assert result["team"]["video_url"] == "https://youtu.be/abc123" + + +def test_get_team_details_video_url_falls_back_to_legacy_field_when_no_demo_video(): + fake_team = {"team": {"id": "team-1", "name": "Team One", "users": [], "video_url": "https://legacy.example.com/v.mp4"}} + with patch.object(svc, "get_team", return_value=fake_team): + result = svc.get_team_details("team-1") + + assert result["team"]["demo_video_url"] == "" + assert result["team"]["video_url"] == "https://legacy.example.com/v.mp4" + + +def test_get_team_details_empty_when_no_video_at_all(): + fake_team = {"team": {"id": "team-1", "name": "Team One", "users": []}} + with patch.object(svc, "get_team", return_value=fake_team): + result = svc.get_team_details("team-1") + + assert result["team"]["demo_video_url"] == "" + assert result["team"]["video_url"] == "" + + +def test_format_team_for_judge_surfaces_demo_video_url(): + team = {"id": "team-1", "name": "Team One", "users": [], "demo_video_url": "https://vimeo.com/123"} + result = svc.format_team_for_judge(team) + assert result["demo_video_url"] == "https://vimeo.com/123" + assert result["video_url"] == "https://vimeo.com/123" + + +def test_format_team_for_judge_devpost_and_github_unchanged(): + """Confirms the fix is additive β€” existing judge-facing fields (github, + devpost) are untouched by the video_url change.""" + team = { + "id": "team-1", "name": "Team One", "users": [], + "devpost_link": "https://devpost.com/x", + "github_links": [{"link": "https://github.com/org/repo"}], + } + result = svc.format_team_for_judge(team) + assert result["devpost_url"] == "https://devpost.com/x" + assert result["github_url"] == "https://github.com/org/repo" + + +# --------------------------------------------------------------------------- +# #3 β€” get_bulk_judge_details no longer NameErrors into an always-empty result +# --------------------------------------------------------------------------- + +def test_get_bulk_judge_details_returns_judges_after_namefix(): + judges_result = {"data": [{"id": "j1", "user_id": "u1", "name": "Judge One", "event_id": "event-1"}]} + with patch.object(svc, "get_volunteer_from_db_by_event", return_value=judges_result), \ + patch.object(svc, "fetch_judge_assignments_by_event_id", return_value=[]), \ + patch.object(svc, "fetch_judge_scores_by_event_id", return_value=[]) as mock_scores: + result = svc.get_bulk_judge_details("event-1") + + # The bug: this call used to raise NameError before ever reaching here. + mock_scores.assert_called_once_with("event-1") + assert result.get("error") is None + assert len(result["judges"]) == 1 + assert result["judges"][0]["id"] == "j1" + + +def test_get_bulk_judge_details_still_handles_service_error_gracefully(): + with patch.object(svc, "get_volunteer_from_db_by_event", return_value={"error": "boom"}), \ + patch.object(svc, "fetch_judge_assignments_by_event_id", return_value=[]), \ + patch.object(svc, "fetch_judge_scores_by_event_id", return_value=[]): + result = svc.get_bulk_judge_details("event-1") + + assert result["judges"] == [] + assert "error" in result + + +# --------------------------------------------------------------------------- +# #4 β€” update_judge_assignment_details looks up by the assignment's own id +# --------------------------------------------------------------------------- + +class _FakeAssignment: + def __init__(self, assignment_id): + self.id = assignment_id + self.judge_id = "judge-1" + self.event_id = "event-1" + self.team_id = "team-1" + self.round = "round1" + self.demo_time = None + self.room = None + self.updated_at = None + + +def test_update_judge_assignment_details_finds_assignment_by_id(): + fake = _FakeAssignment("assignment-1") + with patch.object(svc, "fetch_judge_assignment_by_id", return_value=fake) as mock_fetch, \ + patch.object(svc, "update_judge_assignment", side_effect=lambda a: a): + result = svc.update_judge_assignment_details("assignment-1", demo_time="10:00 AM", room="Room A") + + mock_fetch.assert_called_once_with("assignment-1") + assert result["success"] is True + assert result["assignment"]["id"] == "assignment-1" + assert result["assignment"]["demo_time"] == "10:00 AM" + assert result["assignment"]["room"] == "Room A" + + +def test_update_judge_assignment_details_404_like_response_for_unknown_id(): + with patch.object(svc, "fetch_judge_assignment_by_id", return_value=None): + result = svc.update_judge_assignment_details("nope") + + assert result["success"] is False + assert result["error"] == "Assignment not found" + + +def test_update_judge_assignment_details_never_queries_by_empty_judge_id(): + """The regression itself: no code path here should call + fetch_judge_assignments_by_judge_id("") anymore.""" + fake = _FakeAssignment("assignment-1") + with patch.object(svc, "fetch_judge_assignment_by_id", return_value=fake), \ + patch.object(svc, "update_judge_assignment", side_effect=lambda a: a), \ + patch.object(svc, "fetch_judge_assignments_by_judge_id") as mock_by_judge_id: + svc.update_judge_assignment_details("assignment-1", demo_time="9:00 AM") + + mock_by_judge_id.assert_not_called() diff --git a/api/messages/tests/test_cache_invalidation.py b/api/messages/tests/test_cache_invalidation.py index 4b62e37..d6c2c80 100644 --- a/api/messages/tests/test_cache_invalidation.py +++ b/api/messages/tests/test_cache_invalidation.py @@ -85,23 +85,19 @@ def test_remove_nonprofit_from_hackathon_clears_cache(self, mock_db, mock_clear_ @patch('services.hackathons_service.clear_cache') @patch('services.hackathons_service._get_db') - @patch('services.hackathons_service.validate_hackathon_data') + @patch('services.hackathons_service.validate_hackathon_data_partial') def test_save_hackathon_clears_cache(self, mock_validate, mock_db, mock_clear_cache): - """Test that saving a hackathon clears the cache.""" + """Test that saving a hackathon clears the cache. + + LOW finding #14 / Part 9 bug #16: this used to patch + 'validate_hackathon_data', which was renamed + 'validate_hackathon_data_partial' β€” the patch silently no-op'd + (AttributeError on entering the context manager) because + hackathons_service no longer imports the old name at all. + """ # Setup mock_db_instance = MagicMock() mock_db.return_value = mock_db_instance - mock_validate.return_value = None - - # Mock transaction - mock_transaction = MagicMock() - mock_db_instance.transaction.return_value = mock_transaction - - # Mock collection and document - mock_hackathon_ref = MagicMock() - mock_collection = MagicMock() - mock_collection.document.return_value = mock_hackathon_ref - mock_db_instance.collection.return_value = mock_collection json_data = { "title": "Test Hackathon", @@ -113,6 +109,18 @@ def test_save_hackathon_clears_cache(self, mock_validate, mock_db, mock_clear_ca "image_url": "https://example.com/image.png", "event_id": "event123" } + # validate_hackathon_data_partial returns (cleaned_data, skipped_fields) + mock_validate.return_value = (json_data, []) + + # Mock transaction + mock_transaction = MagicMock() + mock_db_instance.transaction.return_value = mock_transaction + + # Mock collection and document + mock_hackathon_ref = MagicMock() + mock_collection = MagicMock() + mock_collection.document.return_value = mock_hackathon_ref + mock_db_instance.collection.return_value = mock_collection # Execute result = save_hackathon(json_data, "user123") diff --git a/api/messages/tests/test_hackathon_deadlines.py b/api/messages/tests/test_hackathon_deadlines.py new file mode 100644 index 0000000..0d0cea0 --- /dev/null +++ b/api/messages/tests/test_hackathon_deadlines.py @@ -0,0 +1,224 @@ +""" +Tests for the hackathon `deadlines` object: validation (unknown key, ordering, +naive->offset normalization) and the save_hackathon persistence path (DELETE_FIELD +on update for an explicit null, dropped entirely on create). + +Mirrors the mocking pattern in api/messages/tests/test_cache_invalidation.py +(patch services.hackathons_service._get_db + clear_cache; a MagicMock stands +in for the firestore.Transaction passed into the @firestore.transactional +inner function β€” google-cloud-firestore's _Transactional wrapper only touches +attributes on it, never asserts a real Transaction type). +""" +import pytest +from unittest.mock import patch, MagicMock +from firebase_admin import firestore + +from common.utils.validators import ( + DEADLINE_KEYS, + normalize_deadline_iso, + validate_deadlines, +) +from services.hackathons_service import save_hackathon + + +# --------------------------------------------------------------------------- +# normalize_deadline_iso / validate_deadlines β€” pure validator behavior. +# (Broader validator-suite coverage lives in test/common/utils/test_validators.py; +# these are the deadlines-specific cases called out by the plan.) +# --------------------------------------------------------------------------- + +def test_normalize_deadline_iso_naive_localizes_to_tz(): + result = normalize_deadline_iso("2026-10-10T15:00:00", "America/Phoenix") + assert result == "2026-10-10T15:00:00-07:00" + + +def test_normalize_deadline_iso_trailing_z_becomes_offset(): + result = normalize_deadline_iso("2026-10-10T22:00:00Z", "America/Phoenix") + assert result == "2026-10-10T22:00:00+00:00" + + +def test_normalize_deadline_iso_none_and_empty_string_clear(): + assert normalize_deadline_iso(None) is None + assert normalize_deadline_iso("") is None + + +def test_normalize_deadline_iso_rejects_garbage(): + with pytest.raises(ValueError): + normalize_deadline_iso("not-a-date") + + +def test_validate_deadlines_rejects_unknown_key(): + with pytest.raises(ValueError, match="Unknown deadlines key"): + validate_deadlines({"submitted_by": "2026-10-10T15:00:00"}, "America/Phoenix") + + +def test_validate_deadlines_enforces_submission_ordering(): + with pytest.raises(ValueError, match="late_submission_until must be on or after submission"): + validate_deadlines( + { + "submission": "2026-10-10T15:00:00", + "late_submission_until": "2026-10-10T10:00:00", + }, + "America/Phoenix", + ) + + +def test_validate_deadlines_enforces_voting_ordering(): + with pytest.raises(ValueError, match="voting_closes must be after voting_opens"): + validate_deadlines( + { + "voting_opens": "2026-10-12T00:00:00", + "voting_closes": "2026-10-12T00:00:00", + }, + "America/Phoenix", + ) + + +def test_validate_deadlines_accepts_full_valid_object(): + result = validate_deadlines( + { + "submission": "2026-10-10T15:00:00", + "late_submission_until": "2026-10-10T18:00:00", + "voting_opens": "2026-10-10T18:00:00", + "voting_closes": "2026-10-12T23:59:59", + }, + "America/Phoenix", + ) + assert set(result.keys()) == DEADLINE_KEYS + + +def test_validate_deadlines_preserves_explicit_none_as_clear_intent(): + result = validate_deadlines({"submission": None}, "America/Phoenix") + assert result == {"submission": None} + + +# --------------------------------------------------------------------------- +# save_hackathon persistence β€” DELETE_FIELD on update, dropped on create. +# --------------------------------------------------------------------------- + +def _base_json(**extra): + data = { + "title": "Test Hackathon", + "description": "Test Description", + "location": "Test Location", + "start_date": "2026-01-01", + "end_date": "2026-01-02", + "type": "hackathon", + "image_url": "https://example.com/image.png", + "event_id": "event123", + } + data.update(extra) + return data + + +def _mock_db(): + mock_db_instance = MagicMock() + mock_transaction = MagicMock() + mock_db_instance.transaction.return_value = mock_transaction + mock_hackathon_ref = MagicMock() + mock_collection = MagicMock() + mock_collection.document.return_value = mock_hackathon_ref + mock_db_instance.collection.return_value = mock_collection + return mock_db_instance, mock_transaction + + +@patch("services.hackathons_service.clear_cache") +@patch("services.hackathons_service._get_db") +def test_save_hackathon_create_drops_none_deadline_keys(mock_get_db, mock_clear_cache): + mock_db_instance, mock_transaction = _mock_db() + mock_get_db.return_value = mock_db_instance + + json_data = _base_json(deadlines={"submission": "2026-01-01T15:00:00", "voting_opens": None}) + save_hackathon(json_data, "user123") + + written = mock_transaction.set.call_args[0][1] + assert written["deadlines"] == {"submission": "2026-01-01T15:00:00-07:00"} + mock_clear_cache.assert_called_once() + + +@patch("services.hackathons_service.clear_cache") +@patch("services.hackathons_service._get_db") +def test_save_hackathon_update_writes_delete_field_for_none(mock_get_db, mock_clear_cache): + mock_db_instance, mock_transaction = _mock_db() + mock_get_db.return_value = mock_db_instance + + json_data = _base_json( + id="abc123", + deadlines={"submission": None, "late_submission_until": "2026-01-02T12:00:00-07:00"}, + ) + save_hackathon(json_data, "user123") + + written = mock_transaction.set.call_args[0][1] + assert written["deadlines"]["submission"] is firestore.DELETE_FIELD + assert written["deadlines"]["late_submission_until"] == "2026-01-02T12:00:00-07:00" + # merge=True on update so untouched fields on the doc survive + assert mock_transaction.set.call_args.kwargs.get("merge") is True + + +@patch("services.hackathons_service.clear_cache") +@patch("services.hackathons_service._get_db") +def test_save_hackathon_without_deadlines_key_omits_it(mock_get_db, mock_clear_cache): + mock_db_instance, mock_transaction = _mock_db() + mock_get_db.return_value = mock_db_instance + + save_hackathon(_base_json(), "user123") + + written = mock_transaction.set.call_args[0][1] + assert "deadlines" not in written + + +@patch("services.hackathons_service.clear_cache") +@patch("services.hackathons_service._get_db") +def test_save_hackathon_skips_deadlines_with_unknown_key_but_keeps_rest(mock_get_db, mock_clear_cache): + mock_db_instance, mock_transaction = _mock_db() + mock_get_db.return_value = mock_db_instance + + result = save_hackathon(_base_json(deadlines={"bogus_key": "2026-01-01T15:00:00"}), "user123") + + written = mock_transaction.set.call_args[0][1] + assert "deadlines" not in written + assert getattr(result, "skipped_fields", None) + assert any(s["field"] == "deadlines" for s in result.skipped_fields) + + +# --------------------------------------------------------------------------- +# LOW finding #13: `deadlines: {}` (and `deadlines: null`) on an UPDATE write +# an empty map under set(merge=True) β€” Firestore's merge semantics only +# touch the sub-fields actually present in the map you send, so an empty map +# merges zero keys into the existing `deadlines` map and leaves it entirely +# untouched (a no-op), rather than clearing it. This locks in that actual +# behavior with a test and documents it (see api/submissions/README.md and +# this repo's CLAUDE.md) so "to clear a single deadline send {key: null}; +# sending {} is a no-op" isn't just tribal knowledge. +# --------------------------------------------------------------------------- + +@patch("services.hackathons_service.clear_cache") +@patch("services.hackathons_service._get_db") +def test_save_hackathon_update_empty_deadlines_dict_is_noop(mock_get_db, mock_clear_cache): + mock_db_instance, mock_transaction = _mock_db() + mock_get_db.return_value = mock_db_instance + + save_hackathon(_base_json(id="abc123", deadlines={}), "user123") + + written = mock_transaction.set.call_args[0][1] + # An empty map is what gets sent to Firestore; under merge=True this + # merges zero sub-fields into the existing `deadlines` map, so nothing on + # the stored doc actually changes. + assert written["deadlines"] == {} + assert mock_transaction.set.call_args.kwargs.get("merge") is True + + +@patch("services.hackathons_service.clear_cache") +@patch("services.hackathons_service._get_db") +def test_save_hackathon_update_top_level_null_deadlines_is_also_a_noop(mock_get_db, mock_clear_cache): + """A top-level `deadlines: null` (as opposed to a specific key inside the + object being null) behaves identically to `deadlines: {}` β€” both end up + writing an empty map under merge, since save_hackathon does + `data["deadlines"] or {}` before building the DELETE_FIELD map.""" + mock_db_instance, mock_transaction = _mock_db() + mock_get_db.return_value = mock_db_instance + + save_hackathon(_base_json(id="abc123", deadlines=None), "user123") + + written = mock_transaction.set.call_args[0][1] + assert written["deadlines"] == {} diff --git a/api/peer_votes/__init__.py b/api/peer_votes/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/api/peer_votes/peer_votes_service.py b/api/peer_votes/peer_votes_service.py new file mode 100644 index 0000000..9f5badb --- /dev/null +++ b/api/peer_votes/peer_votes_service.py @@ -0,0 +1,657 @@ +""" +Hackers' Choice β€” an assigned-slate approval vote (Sep 2026). See +docs/plans/team-dashboard-devpost-replacement.md (frontend repo) Part 2.5 for +the product rationale (anti-popularity: exposure-balanced slates, approval +not ranking, no visible tallies) and Part 3 for the wire contract. + +Judging is completely untouched by this module β€” Hackers' Choice is a +SEPARATE peer-vote award, not a judging round. + +Design notes: +- Each eligible voter (isSelected hacker) gets a deterministic slate of + `peer_vote_slate_size` submitted projects, never their own team, built once + and persisted on first GET (so re-opening the page always shows the same + slate). The seed is sha256(f"{event_id}:{propel_id}") so it's stable across + requests without needing to store anything before the first GET. +- Exposure counts (how many times each team has been shown) bias the slate + toward the least-shown projects, so a small early team isn't buried by a + few loud/popular ones. +- Scoring is the Wilson score interval LOWER bound (z=1.96) of + approvals/shown, not a raw approval rate β€” a team shown to 2 people who both + approved should not outrank a team shown to 40 people with a 90% approval + rate. No tallies are ever shown to voters, only to admins (get_results). +""" +import hashlib +import logging +import math +import random +from collections import Counter +from datetime import datetime, timezone + +from firebase_admin import firestore + +from db.db import get_db +from common.utils.firestore_helpers import clear_all_caches +# NOTE: common.utils.slack calls load_dotenv() at import time, which +# populates FIREBASE_CERT_CONFIG (among others) from .env into the process +# environment. common.utils.firebase reads that var at ITS OWN import time +# (module-level, no lazy fallback) β€” importing it before anything has called +# load_dotenv() raises a JSONDecodeError on a real deployment that relies on +# .env. Every existing service that imports both (e.g. api/mentors/ +# mentors_service.py) imports slack first for this reason; keep this order. +from common.utils.slack import send_slack_audit +from common.utils.firebase import get_hackathon_by_event_id +from common.utils.validators import normalize_deadline_iso +from services.teams_service import get_team + +logger = logging.getLogger("myapp") + +PEER_VOTES_COLLECTION = "peer_votes" +AWARD_NAME = "Hackers' Choice" +DEFAULT_SLATE_SIZE = 5 +DEFAULT_MAX_PICKS = 2 + +# Mirror of api.submissions.submissions_service.SUBMITTED_STATUSES β€” a team +# must have a real write-up before it can appear in anyone's slate or get +# voted on. Duplicated (not imported) to keep the two blueprints +# one-directionally independent; the values are the small, stable +# project_submission_status catalog. +SUBMITTED_STATUSES = {"submitted", "late"} + + +def clear_cache() -> None: + """Mirrors api/mentors/mentors_service.py's clear_cache(): bust every + registered cache (including services.teams_service._GET_TEAM_CACHE) plus + the hackathon event cache, since publish_results changes a team's + `awards` array that the event page also renders.""" + clear_all_caches() + try: + from services.hackathons_service import clear_cache as clear_hackathon_caches + clear_hackathon_caches() + except Exception as e: # pragma: no cover - best-effort cache bust + logger.warning("peer_votes clear_cache: hackathon cache clear failed: %s", e) + + +def _ballot_doc_id(event_id, propel_id): + safe_propel_id = (propel_id or "").replace("/", "_") + return f"{event_id}__{safe_propel_id}" + + +def _peer_vote_subdoc(db, hackathon_doc_id, name): + return db.collection("hackathons").document(hackathon_doc_id).collection("peer_vote").document(name) + + +def _settings(event): + """{"enabled", "slate_size", "max_picks", "requires_submission"} β€” reads + constraints.peer_vote_* off the hackathon doc with the Part 3 defaults. + MUST read peer_vote_enabled (a disabled event returns status:"disabled" + from every voter-facing route regardless of anything else). + + LOW finding #12: common.utils.validators.validate_hackathon_data_partial + only enforces max_picks < slate_size when BOTH are present in the SAME + PATCH payload β€” it has no access to the stored doc, so an earlier save + that raised max_picks followed by a later save that lowers slate_size can + leave an inconsistent pair on the hackathon doc. Re-clamp at read time, + here, so every voter-facing route (slate/ballot) sees a consistent pair + regardless of how the doc got there. + """ + constraints = (event or {}).get("constraints") or {} + slate_size = constraints.get("peer_vote_slate_size") or DEFAULT_SLATE_SIZE + max_picks = constraints.get("peer_vote_max_picks") or DEFAULT_MAX_PICKS + max_picks = max(1, min(max_picks, slate_size - 1)) + return { + "enabled": bool(constraints.get("peer_vote_enabled", False)), + "slate_size": slate_size, + "max_picks": max_picks, + "requires_submission": bool(constraints.get("peer_vote_requires_submission", False)), + } + + +def _safe_normalize_deadline(value, tz_name, label): + """None/"" -> None; normalizes a naive or "Z"-suffixed value to an aware + ISO string; an unparseable value is logged and treated as absent rather + than raising (HIGH finding #5 β€” datetime.fromisoformat on a naive stored + string, compared against an aware `now`, raises TypeError; a + "Z"-suffixed string raises ValueError on Python 3.9/3.10 β€” either way an + unhandled crash used to reach the caller as a 500).""" + if not value: + return None + try: + return normalize_deadline_iso(value, tz_name) + except ValueError as e: + logger.warning("compute_voting_window: unparseable %s %r: %s", label, value, e) + return None + + +def compute_voting_window(event, now=None): + """{"state": upcoming|open|closed, "opens_at", "closes_at"}. + + Defaults when unset: opens_at = deadlines.voting_opens, falling back to + late_submission_until, falling back to submission; closes_at = + deadlines.voting_closes, falling back to the event's end_date at + 23:59:59 in the event timezone. No usable opens_at/closes_at at all + (a brand new event with no deadlines configured) -> closed, never open. + + LOW finding #6: the end_date fallback used to blindly append + "T23:59:59" to end_date before normalizing. If end_date already carries a + time component (or an offset), that produced an invalid, double-timed + string ("...T18:00:00T23:59:59"), which failed to parse and silently + fell back to permanently "closed". end_date is now normalized as-is when + it already looks like it has a time, and only gets "T23:59:59" appended + when it's a bare date. + """ + event = event or {} + tz_name = event.get("timezone") or "America/Phoenix" + now_dt = now or datetime.now(timezone.utc) + deadlines = event.get("deadlines") or {} + + opens_raw = deadlines.get("voting_opens") or deadlines.get("late_submission_until") or deadlines.get("submission") + opens_at = _safe_normalize_deadline(opens_raw, tz_name, "voting_opens") + + closes_at = _safe_normalize_deadline(deadlines.get("voting_closes"), tz_name, "voting_closes") + if not closes_at: + end_date = event.get("end_date") + if end_date: + has_time = "T" in end_date or " " in end_date.strip() + raw = end_date if has_time else f"{end_date}T23:59:59" + closes_at = _safe_normalize_deadline(raw, tz_name, "end_date-derived voting_closes") + + if not opens_at or not closes_at: + return {"state": "closed", "opens_at": opens_at, "closes_at": closes_at} + + opens_dt = datetime.fromisoformat(opens_at) + closes_dt = datetime.fromisoformat(closes_at) + if now_dt < opens_dt: + state = "upcoming" + elif now_dt <= closes_dt: + state = "open" + else: + state = "closed" + return {"state": state, "opens_at": opens_at, "closes_at": closes_at} + + +def _voter_eligibility(propel_id, event_id, event): + """(eligible, volunteer|None, reason|None). Eligible = an isSelected + hacker volunteer record for this event; when peer_vote_requires_submission + is on, the voter's own team must also have submitted.""" + from services.volunteers_service import find_volunteer_by_caller_identity + + volunteer = find_volunteer_by_caller_identity(propel_id, event_id, "hacker") + if not volunteer or not volunteer.get("isSelected"): + return False, None, "not_selected_hacker" + + settings = _settings(event) + if settings["requires_submission"]: + own_ids = _own_team_ids(propel_id, event_id) + submitted = False + if own_ids: + db = get_db() + refs = [db.collection("teams").document(tid) for tid in own_ids] + for snap in db.get_all(refs): + if snap.exists and (snap.to_dict() or {}).get("project_submission_status") in SUBMITTED_STATUSES: + submitted = True + break + if not submitted: + return False, volunteer, "own_team_not_submitted" + + return True, volunteer, None + + +def _submitted_teams_for_event(event_id): + """Active, submitted/late teams for the event. Single equality query + + Python filter (no composite index needed).""" + db = get_db() + docs = db.collection("teams").where("hackathon_event_id", "==", event_id).stream() + teams = [] + for doc in docs: + data = doc.to_dict() or {} + if data.get("active") is False: + continue + if data.get("project_submission_status") not in SUBMITTED_STATUSES: + continue + data["id"] = doc.id + teams.append(data) + return teams + + +def _own_team_ids(propel_id, event_id): + from api.teams.teams_service import get_my_teams_by_event_id + + result = get_my_teams_by_event_id(propel_id, event_id) or {} + return {t["id"] for t in result.get("teams", []) if t.get("id")} + + +def build_slate(candidates, exposure, event_id, propel_id, n): + """Deterministic per-voter shuffle (seeded on event_id+propel_id) then a + STABLE sort by current exposure ascending, so the least-shown candidates + win ties in the same order every time this voter is shown a slate. + `candidates` is a list of team dicts with an "id" key; returns a list of + team ids, capped at n.""" + seed = int(hashlib.sha256(f"{event_id}:{propel_id}".encode("utf-8")).hexdigest(), 16) + rng = random.Random(seed) + shuffled = list(candidates) + rng.shuffle(shuffled) + shuffled.sort(key=lambda c: exposure.get(c["id"], 0)) + return [c["id"] for c in shuffled[:n]] + + +def _in_transaction(db, body): + """Runs body(transaction) inside a real Firestore transaction. Tests + monkeypatch this directly (to `lambda db, body: body(FakeTx())`) since a + real @firestore.transactional callable needs a live Firestore client.""" + @firestore.transactional + def _run(transaction): + return body(transaction) + + return _run(db.transaction()) + + +def _tx_get_one(transaction, ref): + """First (only) snapshot from transaction.get(ref) β€” the real + google-cloud-firestore Transaction.get() returns a generator for a + DocumentReference (it delegates to client.get_all()).""" + return next(iter(transaction.get(ref)), None) + + +def _slate_team_view(team): + return { + "team_id": team.get("id"), + "name": team.get("name"), + "project_tagline": team.get("project_tagline"), + "project_thumbnail_url": team.get("project_thumbnail_url"), + "demo_video_url": team.get("demo_video_url"), + "github_links": team.get("github_links") or [], + "users_count": len(team.get("users") or []), + } + + +def _hydrate_slate(team_ids, candidates_by_id=None): + candidates_by_id = candidates_by_id or {} + views = [] + for tid in team_ids: + team = candidates_by_id.get(tid) + if team is None: + team = (get_team(tid) or {}).get("team") or {"id": tid} + views.append(_slate_team_view(team)) + return views + + +def _slate_response_from_ballot(ballot, window, settings, own_team_ids, candidates_by_id=None): + """LOW finding #11: a voided ballot must render status "voided" (with + picks nulled out), never "voted" β€” an admin voiding a ballot shouldn't + leave the voter's own slate page still showing their old picks as live.""" + if ballot.get("voided"): + status = "voided" + picks = None + else: + picks = ballot.get("picks") + status = "voted" if picks else window["state"] + return { + "status": status, + "opens_at": window["opens_at"], + "closes_at": window["closes_at"], + "max_picks": settings["max_picks"], + "slate": _hydrate_slate(ballot.get("slate") or [], candidates_by_id), + "picks": picks, + "own_team_ids": list(own_team_ids), + } + + +def get_slate(propel_id, event_id): + """GET /api/hackathons//peer-vote/slate. + + A ballot doc is persisted the FIRST time a voter's slate is materialized + (inside a transaction, re-checking existence to survive a double-click / + double-request race) and never rebuilt after that β€” re-opening the page + always shows the same slate, and exposure is only incremented once. + """ + event = get_hackathon_by_event_id(event_id) + settings = _settings(event or {}) + if not event or not settings["enabled"]: + return {"status": "disabled"} + + window = compute_voting_window(event) + eligible, _volunteer, reason = _voter_eligibility(propel_id, event_id, event) + if not eligible: + return {"status": "not_eligible", "reason": reason, "opens_at": window["opens_at"], "closes_at": window["closes_at"], "max_picks": settings["max_picks"]} + + db = get_db() + event_doc_id = event.get("id") or event_id + ballot_ref = db.collection(PEER_VOTES_COLLECTION).document(_ballot_doc_id(event_id, propel_id)) + own_team_ids = _own_team_ids(propel_id, event_id) + + existing = ballot_ref.get() + if existing.exists: + return _slate_response_from_ballot(existing.to_dict() or {}, window, settings, own_team_ids) + + if window["state"] == "upcoming": + return {"status": "upcoming", "opens_at": window["opens_at"], "closes_at": window["closes_at"], "max_picks": settings["max_picks"], "own_team_ids": list(own_team_ids)} + if window["state"] == "closed": + return {"status": "closed", "opens_at": window["opens_at"], "closes_at": window["closes_at"], "max_picks": settings["max_picks"], "own_team_ids": list(own_team_ids)} + + candidates = [t for t in _submitted_teams_for_event(event_id) if t["id"] not in own_team_ids] + if len(candidates) < 2: + return { + "status": "open", + "opens_at": window["opens_at"], + "closes_at": window["closes_at"], + "max_picks": settings["max_picks"], + "slate": [], + "own_team_ids": list(own_team_ids), + "reason": "not_enough_submissions", + } + + candidates_by_id = {c["id"]: c for c in candidates} + exposure_ref = _peer_vote_subdoc(db, event_doc_id, "exposure") + + def _persist(transaction): + tx_existing = _tx_get_one(transaction, ballot_ref) + if tx_existing is not None and tx_existing.exists: + return tx_existing.to_dict() or {} + + exposure_snap = _tx_get_one(transaction, exposure_ref) + exposure = (exposure_snap.to_dict() or {}).get("counts", {}) if exposure_snap is not None and exposure_snap.exists else {} + slate_ids = build_slate(candidates, exposure, event_id, propel_id, settings["slate_size"]) + now_iso = datetime.now(timezone.utc).isoformat() + ballot_data = { + "event_id": event_id, + "voter_propel_id": propel_id, + "slate": slate_ids, + "shown_at": now_iso, + "picks": None, + "voted_at": None, + "created_at": now_iso, + "updated_at": now_iso, + "voided": False, + } + transaction.set(ballot_ref, ballot_data) + transaction.set(exposure_ref, {"counts": {tid: firestore.Increment(1) for tid in slate_ids}}, merge=True) + return ballot_data + + ballot_data = _in_transaction(db, _persist) + return _slate_response_from_ballot(ballot_data, window, settings, own_team_ids, candidates_by_id=candidates_by_id) + + +def submit_ballot(propel_id, event_id, picks): + """POST /api/hackathons//peer-vote/ballot. Re-votable until + close β€” a full-doc set() replaces `picks`, keeping the original + `voted_at` (first vote only).""" + event = get_hackathon_by_event_id(event_id) + settings = _settings(event or {}) + if not event or not settings["enabled"]: + return {"error": "peer_vote_disabled"}, 403 + + window = compute_voting_window(event) + eligible, _volunteer, reason = _voter_eligibility(propel_id, event_id, event) + if not eligible: + return {"error": "not_eligible", "reason": reason}, 403 + + db = get_db() + ballot_ref = db.collection(PEER_VOTES_COLLECTION).document(_ballot_doc_id(event_id, propel_id)) + snap = ballot_ref.get() + if not snap.exists: + return {"error": "no_slate"}, 400 + ballot = snap.to_dict() or {} + + if ballot.get("voided"): + return {"error": "ballot_voided"}, 409 + if window["state"] == "closed": + return {"error": "voting_closed"}, 409 + + slate = ballot.get("slate") or [] + max_picks = settings["max_picks"] + if ( + not isinstance(picks, list) + or not picks + or len(picks) > max_picks + or len(set(picks)) != len(picks) + or any(p not in slate for p in picks) + ): + return {"error": "invalid_picks"}, 400 + + # LOW finding #8: the spec calls for a full set() (not a partial + # set(merge=True)) on every ballot write, so the persisted doc is always + # a complete, self-describing record rather than relying on merge + # semantics to preserve fields this write doesn't mention. Build the full + # doc from the existing one (carrying event_id/voter_propel_id/slate/ + # shown_at/created_at forward) and only overwrite the fields that change. + now_iso = datetime.now(timezone.utc).isoformat() + full_doc = dict(ballot) + full_doc["picks"] = picks + full_doc["updated_at"] = now_iso + if not ballot.get("voted_at"): + full_doc["voted_at"] = now_iso + ballot_ref.set(full_doc) + + send_slack_audit( + action="peer_vote_ballot", + message=f"Hackers' Choice ballot recorded for event {event_id}", + payload={"event_id": event_id, "picks": picks}, + ) + return {"success": True, "picks": picks}, 200 + + +def wilson_lower_bound(approvals, shown, z=1.96): + """Wilson score interval LOWER bound for a binomial proportion β€” + approvals/shown, penalized for a small sample. wilson_lower_bound(0,0)==0; + (5,5)β‰ˆ0.566; (1,1)β‰ˆ0.207.""" + if not shown: + return 0.0 + n = float(shown) + p = approvals / n + denom = 1 + (z * z) / n + center = p + (z * z) / (2 * n) + margin = z * math.sqrt((p * (1 - p) + (z * z) / (4 * n)) / n) + return (center - margin) / denom + + +def compute_results(ballots, teams_by_id, exposure): + """Pure. Ranks by Wilson lower bound desc, then raw approvals desc, then + name β€” so a tie only ever breaks toward the more-approved, then + alphabetically (stable, no hidden randomness in the admin view). + + HIGH finding #2: `shown` used to be read straight off the exposure doc, + which counts every slate the team was PERSISTED into (i.e. every voter + who ever opened the vote page), not every ballot that was actually CAST. + A team could be exposure-shown to 10 people but only have 2 real ballots + (both approving), and would still be scored against a denominator of 10 + β€” badly under-stating its Wilson lower bound relative to a team that + happened to get fewer opens but a higher cast-ballot rate. `shown` is now + computed from non-voided ballots that actually have picks (i.e. were + cast), which also means voiding a ballot removes it from both `shown` + and `approvals`. `exposure_shown` keeps the raw persisted-slate count + as a separate, purely informational field. + """ + active = [b for b in ballots if not b.get("voided")] + shown_counts = Counter( + team_id + for ballot in active + if ballot.get("picks") + for team_id in ballot.get("slate", []) + ) + approvals = Counter( + team_id + for ballot in active + for team_id in (ballot.get("picks") or []) + ) + + results = [] + for team_id, team in teams_by_id.items(): + shown = shown_counts.get(team_id, 0) + exposure_shown = exposure.get(team_id, 0) + approved = approvals.get(team_id, 0) + results.append({ + "team_id": team_id, + "name": team.get("name"), + "shown": shown, + "exposure_shown": exposure_shown, + "approvals": approved, + "approval_rate": (approved / shown) if shown else 0.0, + "wilson_lower_bound": wilson_lower_bound(approved, shown), + }) + + results.sort(key=lambda r: (-r["wilson_lower_bound"], -r["approvals"], r["name"] or "")) + for i, r in enumerate(results, start=1): + r["rank"] = i + return results + + +def get_results(event_id): + """GET /api/hackathons//peer-vote/results (admin).""" + event = get_hackathon_by_event_id(event_id) + if not event: + return {"error": "Event not found"}, 404 + + settings = _settings(event) + window = compute_voting_window(event) + event_doc_id = event.get("id") or event_id + + db = get_db() + all_ballots = [d.to_dict() or {} for d in db.collection(PEER_VOTES_COLLECTION).where("event_id", "==", event_id).stream()] + voided_ballots = [b for b in all_ballots if b.get("voided")] + active_ballots = [b for b in all_ballots if not b.get("voided")] + + exposure_snap = _peer_vote_subdoc(db, event_doc_id, "exposure").get() + exposure = (exposure_snap.to_dict() or {}).get("counts", {}) if exposure_snap.exists else {} + + teams_by_id = {t["id"]: t for t in _submitted_teams_for_event(event_id)} + for team_id in exposure.keys(): + if team_id not in teams_by_id: + teams_by_id[team_id] = (get_team(team_id) or {}).get("team") or {"id": team_id} + + results = compute_results(active_ballots, teams_by_id, exposure) + + from services.volunteers_service import get_all_hackers_by_event_id + eligible_estimate = sum(1 for h in get_all_hackers_by_event_id(event_id) if h.get("isSelected")) + + summary_exists = _peer_vote_subdoc(db, event_doc_id, "summary").get().exists + + # LOW finding #11: the admin UI needs a per-voter list to drive `void`, + # but must never see WHO voted for WHAT β€” no names, no emails, and no + # `picks` (only how many). voter_propel_id is already an opaque id, not a + # name/email, so it's safe to surface for the void action's own use. + ballots_detail = [ + { + "voter_propel_id": b.get("voter_propel_id"), + "voted_at": b.get("voted_at"), + "voided": bool(b.get("voided")), + "picks_count": len(b.get("picks") or []), + } + for b in all_ballots + ] + + return { + "ballots": len(active_ballots), + "voided": len(voided_ballots), + "eligible_estimate": eligible_estimate, + "window": window, + "settings": settings, + "published": summary_exists, + "teams": results, + "ballots_detail": ballots_detail, + }, 200 + + +def void_ballot(event_id, voter_propel_id, actor): + """POST /api/hackathons//peer-vote/ballots//void (admin).""" + db = get_db() + ref = db.collection(PEER_VOTES_COLLECTION).document(_ballot_doc_id(event_id, voter_propel_id)) + snap = ref.get() + if not snap.exists: + return {"error": "Ballot not found"}, 404 + + # LOW finding #8: full set(), not a partial merge β€” see submit_ballot's + # comment for why. + ballot = snap.to_dict() or {} + now_iso = datetime.now(timezone.utc).isoformat() + full_doc = dict(ballot) + full_doc["voided"] = True + full_doc["voided_at"] = now_iso + full_doc["voided_by"] = actor + ref.set(full_doc) + send_slack_audit( + action="peer_vote_void", + message=f"Hackers' Choice ballot voided for event {event_id}", + payload={"event_id": event_id, "voter_propel_id": voter_propel_id, "by": actor}, + ) + return {"success": True}, 200 + + +def publish_results(event_id, actor, team_id=None): + """POST /api/hackathons//peer-vote/publish (admin). Idempotent: + re-publishing the same winner never appends AWARD_NAME twice. `team_id` + lets an admin override the computed rank-1 winner (e.g. a tie broken by + judgment); defaults to rank 1.""" + event = get_hackathon_by_event_id(event_id) + if not event: + return {"error": "Event not found"}, 404 + + results_payload, status = get_results(event_id) + if status != 200: + return results_payload, status + + # HIGH finding #1: compute_results emits a row for every SUBMITTED team + # regardless of ballot count, so with zero ballots cast the sort key + # (wilson_lower_bound, approvals) is 0 for every team and the sort falls + # through to team NAME β€” crowning an alphabetically-first team as the + # "winner" of a vote nobody voted in. Refuse to publish when there were + # no ballots at all, OR when the computed rank-1 team has zero approvals + # (covers the case where slates were opened/persisted but nobody + # actually cast a ballot). + ranked = results_payload.get("teams") or [] + if not ranked or results_payload.get("ballots", 0) == 0 or ranked[0].get("approvals", 0) == 0: + return {"error": "no_ballots"}, 409 + + winner_id = team_id or ranked[0]["team_id"] + winner = next((t for t in ranked if t["team_id"] == winner_id), None) + if winner is None: + return {"error": "Winning team not found in results"}, 400 + + db = get_db() + team_ref = db.collection("teams").document(winner_id) + team_snap = team_ref.get() + team_data = (team_snap.to_dict() or {}) if team_snap.exists else {} + awards = list(team_data.get("awards") or []) + if AWARD_NAME not in awards: + awards.append(AWARD_NAME) + team_ref.set({"awards": awards}, merge=True) + + now_iso = datetime.now(timezone.utc).isoformat() + winner_name = winner.get("name") or team_data.get("name") + _peer_vote_subdoc(db, event.get("id") or event_id, "summary").set({ + "winner_team_id": winner_id, + "winner_team_name": winner_name, + "published_at": now_iso, + "published_by": actor, + "ballots": results_payload.get("ballots", 0), + }) + + send_slack_audit( + action="peer_vote_publish", + message=f"Hackers' Choice published for event {event_id}: {winner_name} ({winner_id})", + payload={"event_id": event_id, "winner_team_id": winner_id}, + ) + clear_cache() + + return {"success": True, "winner_team_id": winner_id, "winner_team_name": winner_name}, 200 + + +def get_public_summary(event_id): + """GET /api/hackathons//peer-vote/summary (public).""" + event = get_hackathon_by_event_id(event_id) + if not event: + return {"published": False}, 200 + + db = get_db() + summary_snap = _peer_vote_subdoc(db, event.get("id") or event_id, "summary").get() + if not summary_snap.exists: + return {"published": False}, 200 + + data = summary_snap.to_dict() or {} + return { + "published": True, + "winner_team_id": data.get("winner_team_id"), + "winner_team_name": data.get("winner_team_name"), + "published_at": data.get("published_at"), + "ballots": data.get("ballots"), + }, 200 diff --git a/api/peer_votes/peer_votes_views.py b/api/peer_votes/peer_votes_views.py new file mode 100644 index 0000000..458b122 --- /dev/null +++ b/api/peer_votes/peer_votes_views.py @@ -0,0 +1,86 @@ +""" +Flask routes for Hackers' Choice (the peer-vote award). Voter-facing routes +require login only (eligibility β€” isSelected hacker β€” is enforced in the +service); admin routes additionally check is_admin(auth_user). The public +summary route needs no auth at all. +""" +import logging +from flask import Blueprint, request + +from common.auth import auth, auth_user +from services.hackathon_planning_service import is_admin +from api.peer_votes.peer_votes_service import ( + get_slate, + submit_ballot, + get_results, + void_ballot, + publish_results, + get_public_summary, +) + +logger = logging.getLogger(__name__) +logger.setLevel(logging.INFO) + +bp_name = "api-peer-votes" +bp = Blueprint(bp_name, __name__, url_prefix="/api/hackathons") + + +def _unauthorized(): + return {"error": "Unauthorized"}, 401 + + +def _forbidden(): + return {"error": "Forbidden"}, 403 + + +@bp.route("//peer-vote/slate", methods=["GET"]) +@auth.require_user +def get_slate_api(event_id): + if not (auth_user and auth_user.user_id): + return _unauthorized() + return get_slate(auth_user.user_id, event_id) + + +@bp.route("//peer-vote/ballot", methods=["POST"]) +@auth.require_user +def submit_ballot_api(event_id): + if not (auth_user and auth_user.user_id): + return _unauthorized() + body = request.get_json() or {} + return submit_ballot(auth_user.user_id, event_id, body.get("picks")) + + +@bp.route("//peer-vote/results", methods=["GET"]) +@auth.require_user +def get_results_api(event_id): + if not (auth_user and auth_user.user_id): + return _unauthorized() + if not is_admin(auth_user): + return _forbidden() + return get_results(event_id) + + +@bp.route("//peer-vote/ballots//void", methods=["POST"]) +@auth.require_user +def void_ballot_api(event_id, propel_id): + if not (auth_user and auth_user.user_id): + return _unauthorized() + if not is_admin(auth_user): + return _forbidden() + return void_ballot(event_id, propel_id, auth_user.user_id) + + +@bp.route("//peer-vote/publish", methods=["POST"]) +@auth.require_user +def publish_results_api(event_id): + if not (auth_user and auth_user.user_id): + return _unauthorized() + if not is_admin(auth_user): + return _forbidden() + body = request.get_json() or {} + return publish_results(event_id, auth_user.user_id, team_id=body.get("team_id")) + + +@bp.route("//peer-vote/summary", methods=["GET"]) +def get_summary_api(event_id): + return get_public_summary(event_id) diff --git a/api/peer_votes/tests/__init__.py b/api/peer_votes/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/api/peer_votes/tests/test_peer_votes_service.py b/api/peer_votes/tests/test_peer_votes_service.py new file mode 100644 index 0000000..cad5287 --- /dev/null +++ b/api/peer_votes/tests/test_peer_votes_service.py @@ -0,0 +1,818 @@ +""" +Unit tests for api.peer_votes.peer_votes_service. + +Uses a tiny in-memory fake Firestore (tuple-keyed FakeDb, supporting +collection/document/where/stream/get_all + a FakeTransaction that +_in_transaction is monkeypatched to use) so multi-step flows (slate +persistence, exposure increments, publish) are visible across calls within +one test, the same way real Firestore would behave. +""" +import os +from datetime import datetime, timedelta, timezone +from unittest.mock import MagicMock + +import pytest +from google.cloud.firestore_v1.transforms import Increment + +os.environ.setdefault("ENVIRONMENT", "test") + +import api.peer_votes.peer_votes_service as svc + + +# --------------------------------------------------------------------------- +# Fake Firestore +# --------------------------------------------------------------------------- + +def _resolve_value(existing_value, new_value): + if isinstance(new_value, Increment): + base = existing_value if isinstance(existing_value, (int, float)) else 0 + return base + new_value.value + if isinstance(new_value, dict): + base_dict = existing_value if isinstance(existing_value, dict) else {} + merged = dict(base_dict) + for k, v in new_value.items(): + merged[k] = _resolve_value(base_dict.get(k), v) + return merged + return new_value + + +class FakeSnapshot: + def __init__(self, doc_id, data): + self.id = doc_id + self.exists = data is not None + self._data = data + + def to_dict(self): + return dict(self._data) if self._data is not None else None + + +class FakeDocRef: + def __init__(self, store, key): + self._store = store + self._key = key + + def get(self): + return FakeSnapshot(self._key[-1], self._store.get(self._key)) + + def set(self, data, merge=False): + if merge and self._key in self._store and self._store[self._key] is not None: + existing = self._store[self._key] + for k, v in data.items(): + existing[k] = _resolve_value(existing.get(k), v) + else: + resolved = {} + for k, v in data.items(): + resolved[k] = _resolve_value(None, v) + self._store[self._key] = resolved + + def collection(self, name): + return FakeCollection(self._store, self._key + (name,)) + + +class FakeQuery: + def __init__(self, store, path, filters): + self._store = store + self._path = path + self._filters = filters + + def where(self, field, op, value): + return FakeQuery(self._store, self._path, self._filters + [(field, op, value)]) + + def stream(self): + prefix_len = len(self._path) + 1 + results = [] + for key, data in list(self._store.items()): + if data is None or len(key) != prefix_len or key[:-1] != self._path: + continue + if all(data.get(field) == value for field, op, value in self._filters): + results.append(FakeSnapshot(key[-1], data)) + return results + + +class FakeCollection: + def __init__(self, store, path): + self._store = store + self._path = path + + def document(self, doc_id): + return FakeDocRef(self._store, self._path + (doc_id,)) + + def where(self, field, op, value): + return FakeQuery(self._store, self._path, [(field, op, value)]) + + +class FakeDb: + def __init__(self, store): + self._store = store + + def collection(self, name): + return FakeCollection(self._store, (name,)) + + def get_all(self, refs): + return [ref.get() for ref in refs] + + +class FakeTransaction: + def __init__(self, store): + self._store = store + + def get(self, ref): + return iter([ref.get()]) + + def set(self, ref, data, merge=False): + ref.set(data, merge=merge) + + +@pytest.fixture +def store(): + return {} + + +@pytest.fixture +def wire(monkeypatch, store): + monkeypatch.setattr(svc, "get_db", lambda: FakeDb(store)) + monkeypatch.setattr(svc, "_in_transaction", lambda db, body: body(FakeTransaction(store))) + monkeypatch.setattr(svc, "clear_cache", lambda: None) + monkeypatch.setattr(svc, "send_slack_audit", lambda **kwargs: None) + + def fake_get_team(team_id): + data = store.get(("teams", team_id)) + if data is None: + return {} + return {"team": {**data, "id": team_id}} + + monkeypatch.setattr(svc, "get_team", fake_get_team) + return store + + +def _seed_hackathon(event_id="event-1", doc_id="evtdoc-1", **extra): + return {"id": doc_id, "event_id": event_id, "timezone": "UTC", "deadlines": {}, "constraints": {}, "end_date": "2026-10-11", **extra} + + +# Window relative to "now" so these tests don't rot as the calendar moves β€” +# always currently open, regardless of when the suite runs. +_NOW = datetime.now(timezone.utc) +OPEN_WINDOW = { + "voting_opens": (_NOW - timedelta(days=1)).isoformat(), + "voting_closes": (_NOW + timedelta(days=1)).isoformat(), +} + + +def _seed_team(store, team_id, event_id="event-1", **extra): + store[("teams", team_id)] = {"hackathon_event_id": event_id, "name": team_id, **extra} + + +def _eligible(monkeypatch, isSelected=True): + monkeypatch.setattr( + "services.volunteers_service.find_volunteer_by_caller_identity", + lambda propel, event_id, vtype: {"name": "Hacker", "isSelected": isSelected}, + ) + + +def _own_teams(monkeypatch, team_ids): + monkeypatch.setattr( + "api.teams.teams_service.get_my_teams_by_event_id", + lambda propel, event_id: {"teams": [{"id": tid} for tid in team_ids]}, + ) + + +# --------------------------------------------------------------------------- +# compute_voting_window β€” HIGH finding #5 (naive/"Z"/garbage stored deadline +# strings must not crash) and LOW finding #6 (end_date fallback must not +# double-append a time onto an end_date that already has one). +# --------------------------------------------------------------------------- + +def test_compute_voting_window_naive_voting_opens_is_localized(): + now = datetime(2026, 10, 10, 10, 0, tzinfo=timezone.utc) + event = _seed_hackathon(timezone="UTC", deadlines={ + "voting_opens": "2026-10-10T05:00:00", # naive, no offset + "voting_closes": "2026-10-12T00:00:00+00:00", + }) + window = svc.compute_voting_window(event, now=now) + assert window["state"] == "open" + assert window["opens_at"] == "2026-10-10T05:00:00+00:00" + + +def test_compute_voting_window_z_suffixed_voting_closes_is_normalized(): + now = datetime(2026, 10, 10, 10, 0, tzinfo=timezone.utc) + event = _seed_hackathon(timezone="UTC", deadlines={ + "voting_opens": "2026-10-09T00:00:00+00:00", + "voting_closes": "2026-10-11T00:00:00Z", + }) + window = svc.compute_voting_window(event, now=now) + assert window["state"] == "open" + assert window["closes_at"] == "2026-10-11T00:00:00+00:00" + + +def test_compute_voting_window_garbage_deadline_treated_as_closed(): + event = _seed_hackathon(timezone="UTC", deadlines={ + "voting_opens": "not-a-date", + "voting_closes": "2026-10-12T00:00:00+00:00", + }) + window = svc.compute_voting_window(event) + assert window["state"] == "closed" + assert window["opens_at"] is None + + +def test_compute_voting_window_end_date_only_date_appends_end_of_day(): + now = datetime(2026, 10, 11, 20, 0, tzinfo=timezone.utc) + event = _seed_hackathon(timezone="UTC", end_date="2026-10-11", deadlines={ + "voting_opens": "2026-10-09T00:00:00+00:00", + }) + window = svc.compute_voting_window(event, now=now) + assert window["closes_at"] == "2026-10-11T23:59:59+00:00" + assert window["state"] == "open" + + +def test_compute_voting_window_end_date_with_existing_time_is_not_double_appended(): + """LOW finding #6 regression: end_date already carrying a time used to + get "T23:59:59" appended anyway ("...T18:00:00T23:59:59"), which failed + to parse and silently fell back to permanently closed.""" + now = datetime(2026, 10, 11, 10, 0, tzinfo=timezone.utc) + event = _seed_hackathon(timezone="UTC", end_date="2026-10-11T18:00:00", deadlines={ + "voting_opens": "2026-10-09T00:00:00+00:00", + }) + window = svc.compute_voting_window(event, now=now) + assert window["closes_at"] == "2026-10-11T18:00:00+00:00" + assert window["state"] == "open" + + +# --------------------------------------------------------------------------- +# _settings β€” LOW finding #12: max_picks must never reach a voter-facing +# route >= slate_size, even if the stored doc has an inconsistent pair (the +# validator only checks the relationship when both fields are in the SAME +# PATCH payload). +# --------------------------------------------------------------------------- + +def test_settings_clamps_max_picks_below_stored_slate_size(): + event = _seed_hackathon(constraints={"peer_vote_slate_size": 3, "peer_vote_max_picks": 5}) + settings = svc._settings(event) + assert settings["slate_size"] == 3 + assert settings["max_picks"] == 2 + + +def test_settings_leaves_consistent_pair_untouched(): + event = _seed_hackathon(constraints={"peer_vote_slate_size": 5, "peer_vote_max_picks": 2}) + settings = svc._settings(event) + assert settings["max_picks"] == 2 + + +# --------------------------------------------------------------------------- +# wilson_lower_bound β€” spec values +# --------------------------------------------------------------------------- + +def test_wilson_lower_bound_zero_shown_is_zero(): + assert svc.wilson_lower_bound(0, 0) == 0.0 + + +def test_wilson_lower_bound_five_of_five(): + assert svc.wilson_lower_bound(5, 5) == pytest.approx(0.566, abs=0.001) + + +def test_wilson_lower_bound_one_of_one(): + assert svc.wilson_lower_bound(1, 1) == pytest.approx(0.207, abs=0.001) + + +# --------------------------------------------------------------------------- +# build_slate β€” exposure balance + determinism +# --------------------------------------------------------------------------- + +def test_build_slate_prefers_least_exposed(): + candidates = [{"id": f"t{i}"} for i in range(5)] + exposure = {"t0": 10, "t1": 0, "t2": 5, "t3": 0, "t4": 8} + slate = svc.build_slate(candidates, exposure, "event-1", "propel-1", 2) + assert set(slate) == {"t1", "t3"} + + +def test_build_slate_deterministic_for_same_voter(): + candidates = [{"id": f"t{i}"} for i in range(6)] + a = svc.build_slate(candidates, {}, "event-1", "propel-1", 3) + b = svc.build_slate(candidates, {}, "event-1", "propel-1", 3) + assert a == b + + +def test_build_slate_caps_at_n(): + candidates = [{"id": f"t{i}"} for i in range(10)] + slate = svc.build_slate(candidates, {}, "event-1", "propel-1", 4) + assert len(slate) == 4 + + +# --------------------------------------------------------------------------- +# get_slate β€” disabled/eligibility/window/own-team-exclusion/persistence +# --------------------------------------------------------------------------- + +def test_get_slate_disabled_when_constraint_unset(wire, monkeypatch): + event = _seed_hackathon(event_id="event-1") + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + result = svc.get_slate("propel-1", "event-1") + assert result == {"status": "disabled"} + + +def test_get_slate_disabled_when_event_missing(wire, monkeypatch): + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: None) + result = svc.get_slate("propel-1", "event-1") + assert result == {"status": "disabled"} + + +def test_get_slate_not_eligible_when_not_selected_hacker(wire, monkeypatch): + event = _seed_hackathon(constraints={"peer_vote_enabled": True}, deadlines=OPEN_WINDOW) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + _eligible(monkeypatch, isSelected=False) + result = svc.get_slate("propel-1", "event-1") + assert result["status"] == "not_eligible" + + +def test_get_slate_excludes_own_team_and_unsubmitted_and_inactive(wire, monkeypatch): + event = _seed_hackathon(constraints={"peer_vote_enabled": True, "peer_vote_slate_size": 5}, deadlines=OPEN_WINDOW) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + _eligible(monkeypatch) + _own_teams(monkeypatch, ["own-team"]) + + _seed_team(wire, "own-team", project_submission_status="submitted") + _seed_team(wire, "candidate-a", project_submission_status="submitted") + _seed_team(wire, "candidate-b", project_submission_status="submitted") + _seed_team(wire, "draft-team", project_submission_status="draft") + _seed_team(wire, "inactive-team", project_submission_status="submitted", active=False) + + result = svc.get_slate("propel-1", "event-1") + slate_ids = {t["team_id"] for t in result["slate"]} + assert "own-team" not in slate_ids + assert "draft-team" not in slate_ids + assert "inactive-team" not in slate_ids + assert slate_ids == {"candidate-a", "candidate-b"} + + +def test_get_slate_not_enough_submissions_when_fewer_than_two_candidates(wire, monkeypatch): + event = _seed_hackathon(constraints={"peer_vote_enabled": True}, deadlines=OPEN_WINDOW) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + _eligible(monkeypatch) + _own_teams(monkeypatch, []) + _seed_team(wire, "only-team", project_submission_status="submitted") + + result = svc.get_slate("propel-1", "event-1") + assert result["status"] == "open" + assert result["slate"] == [] + assert result["reason"] == "not_enough_submissions" + # nothing should have been persisted + assert ("peer_votes", "event-1__propel-1") not in wire + + +def test_get_slate_second_call_returns_identical_slate_and_increments_exposure_once(wire, monkeypatch): + event = _seed_hackathon(constraints={"peer_vote_enabled": True, "peer_vote_slate_size": 2}, deadlines=OPEN_WINDOW) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + _eligible(monkeypatch) + _own_teams(monkeypatch, []) + for i in range(4): + _seed_team(wire, f"team-{i}", project_submission_status="submitted") + + first = svc.get_slate("propel-1", "event-1") + second = svc.get_slate("propel-1", "event-1") + + assert first["slate"] == second["slate"] + exposure = wire[("hackathons", "evtdoc-1", "peer_vote", "exposure")]["counts"] + for team in first["slate"]: + assert exposure[team["team_id"]] == 1 + + +def test_get_slate_renders_voided_status_not_voted(wire, monkeypatch): + """LOW finding #11 regression: an admin voiding a ballot must not leave + the voter's own slate page still rendering "voted" with their old picks + β€” it must show status "voided" and null out picks.""" + event = _seed_hackathon(constraints={"peer_vote_enabled": True}, deadlines=OPEN_WINDOW) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + _eligible(monkeypatch) + _own_teams(monkeypatch, []) + wire[("peer_votes", "event-1__propel-1")] = { + "event_id": "event-1", "voter_propel_id": "propel-1", "slate": ["t1", "t2"], + "picks": ["t1"], "voted_at": "2026-01-01T00:00:00+00:00", "voided": True, + } + + result = svc.get_slate("propel-1", "event-1") + assert result["status"] == "voided" + assert result["picks"] is None + + +def test_get_slate_upcoming_and_closed_states(wire, monkeypatch): + _eligible(monkeypatch) + _own_teams(monkeypatch, []) + + upcoming_event = _seed_hackathon(constraints={"peer_vote_enabled": True}, deadlines={ + "voting_opens": "2099-01-01T00:00:00+00:00", "voting_closes": "2099-01-02T00:00:00+00:00", + }) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: upcoming_event) + assert svc.get_slate("propel-1", "event-1")["status"] == "upcoming" + + closed_event = _seed_hackathon(constraints={"peer_vote_enabled": True}, deadlines={ + "voting_opens": "2020-01-01T00:00:00+00:00", "voting_closes": "2020-01-02T00:00:00+00:00", + }) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: closed_event) + assert svc.get_slate("propel-1", "event-1")["status"] == "closed" + + +# --------------------------------------------------------------------------- +# submit_ballot β€” the pick validation matrix +# --------------------------------------------------------------------------- + +def _seed_ballot(store, event_id, propel_id, slate, picks=None, voided=False): + store[("peer_votes", f"{event_id}__{propel_id}")] = { + "event_id": event_id, "voter_propel_id": propel_id, "slate": slate, + "picks": picks, "voted_at": None, "voided": voided, + } + + +def _open_event(**overrides): + base = dict(constraints={"peer_vote_enabled": True, "peer_vote_max_picks": 2}, deadlines={ + "voting_opens": "2020-01-01T00:00:00+00:00", "voting_closes": "2099-01-01T00:00:00+00:00", + }) + base.update(overrides) + return _seed_hackathon(**base) + + +def test_submit_ballot_no_slate_yet(wire, monkeypatch): + event = _open_event() + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + _eligible(monkeypatch) + result, status = svc.submit_ballot("propel-1", "event-1", ["t1"]) + assert status == 400 + assert result["error"] == "no_slate" + + +def test_submit_ballot_rejects_pick_not_in_slate(wire, monkeypatch): + event = _open_event() + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + _eligible(monkeypatch) + _seed_ballot(wire, "event-1", "propel-1", slate=["t1", "t2", "t3"]) + result, status = svc.submit_ballot("propel-1", "event-1", ["t1", "t9"]) + assert status == 400 + assert result["error"] == "invalid_picks" + + +def test_submit_ballot_rejects_too_many_picks(wire, monkeypatch): + event = _open_event() + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + _eligible(monkeypatch) + _seed_ballot(wire, "event-1", "propel-1", slate=["t1", "t2", "t3"]) + result, status = svc.submit_ballot("propel-1", "event-1", ["t1", "t2", "t3"]) + assert status == 400 + + +def test_submit_ballot_rejects_empty_picks(wire, monkeypatch): + event = _open_event() + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + _eligible(monkeypatch) + _seed_ballot(wire, "event-1", "propel-1", slate=["t1", "t2"]) + result, status = svc.submit_ballot("propel-1", "event-1", []) + assert status == 400 + + +def test_submit_ballot_rejects_duplicate_picks(wire, monkeypatch): + event = _open_event() + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + _eligible(monkeypatch) + _seed_ballot(wire, "event-1", "propel-1", slate=["t1", "t2"]) + result, status = svc.submit_ballot("propel-1", "event-1", ["t1", "t1"]) + assert status == 400 + + +def test_submit_ballot_accepts_valid_picks(wire, monkeypatch): + event = _open_event() + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + _eligible(monkeypatch) + _seed_ballot(wire, "event-1", "propel-1", slate=["t1", "t2", "t3"]) + result, status = svc.submit_ballot("propel-1", "event-1", ["t1", "t3"]) + assert status == 200 + assert wire[("peer_votes", "event-1__propel-1")]["picks"] == ["t1", "t3"] + + +def test_submit_ballot_409_when_voided(wire, monkeypatch): + event = _open_event() + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + _eligible(monkeypatch) + _seed_ballot(wire, "event-1", "propel-1", slate=["t1", "t2"], voided=True) + result, status = svc.submit_ballot("propel-1", "event-1", ["t1"]) + assert status == 409 + assert result["error"] == "ballot_voided" + + +def test_submit_ballot_409_when_closed(wire, monkeypatch): + event = _seed_hackathon(constraints={"peer_vote_enabled": True}, deadlines={ + "voting_opens": "2020-01-01T00:00:00+00:00", "voting_closes": "2020-01-02T00:00:00+00:00", + }) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + _eligible(monkeypatch) + _seed_ballot(wire, "event-1", "propel-1", slate=["t1", "t2"]) + result, status = svc.submit_ballot("propel-1", "event-1", ["t1"]) + assert status == 409 + assert result["error"] == "voting_closed" + + +def test_submit_ballot_403_when_disabled(wire, monkeypatch): + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: _seed_hackathon()) + _eligible(monkeypatch) + result, status = svc.submit_ballot("propel-1", "event-1", ["t1"]) + assert status == 403 + assert result["error"] == "peer_vote_disabled" + + +def test_submit_ballot_re_vote_keeps_original_voted_at(wire, monkeypatch): + event = _open_event() + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + _eligible(monkeypatch) + wire[("peer_votes", "event-1__propel-1")] = { + "event_id": "event-1", "slate": ["t1", "t2"], "picks": ["t1"], + "voted_at": "2026-01-01T00:00:00+00:00", "voided": False, + } + svc.submit_ballot("propel-1", "event-1", ["t2"]) + assert wire[("peer_votes", "event-1__propel-1")]["voted_at"] == "2026-01-01T00:00:00+00:00" + assert wire[("peer_votes", "event-1__propel-1")]["picks"] == ["t2"] + + +def test_submit_ballot_writes_full_doc_without_merge(wire, monkeypatch): + """LOW finding #8: the spec calls for a full set() (no merge=True) on + every ballot write. Spy on FakeDocRef.set to assert both that merge is + never passed as True AND that the written doc still carries every field + from the original ballot (event_id/slate/created_at) β€” a bug that + resurrected the old partial-set behavior would either flip merge back to + True, or write a doc missing these fields since a non-merge set() with a + partial dict would have silently dropped them.""" + event = _open_event() + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + _eligible(monkeypatch) + wire[("peer_votes", "event-1__propel-1")] = { + "event_id": "event-1", "voter_propel_id": "propel-1", "slate": ["t1", "t2", "t3"], + "shown_at": "2026-01-01T00:00:00+00:00", "created_at": "2026-01-01T00:00:00+00:00", + "picks": None, "voted_at": None, "voided": False, + } + + calls = [] + original_set = FakeDocRef.set + + def spy_set(self, data, merge=False): + calls.append((dict(data), merge)) + return original_set(self, data, merge=merge) + + monkeypatch.setattr(FakeDocRef, "set", spy_set) + + svc.submit_ballot("propel-1", "event-1", ["t1"]) + + data, merge = calls[-1] + assert merge is False + assert data["slate"] == ["t1", "t2", "t3"] + assert data["event_id"] == "event-1" + assert data["created_at"] == "2026-01-01T00:00:00+00:00" + assert data["picks"] == ["t1"] + + +def test_void_ballot_writes_full_doc_without_merge(wire, monkeypatch): + wire[("peer_votes", "event-1__u1")] = { + "event_id": "event-1", "voter_propel_id": "u1", "slate": ["t1"], + "picks": ["t1"], "created_at": "2026-01-01T00:00:00+00:00", "voided": False, + } + + calls = [] + original_set = FakeDocRef.set + + def spy_set(self, data, merge=False): + calls.append((dict(data), merge)) + return original_set(self, data, merge=merge) + + monkeypatch.setattr(FakeDocRef, "set", spy_set) + + svc.void_ballot("event-1", "u1", "admin-1") + + data, merge = calls[-1] + assert merge is False + assert data["slate"] == ["t1"] + assert data["created_at"] == "2026-01-01T00:00:00+00:00" + assert data["voided"] is True + + +# --------------------------------------------------------------------------- +# compute_results / get_results β€” voided exclusion +# --------------------------------------------------------------------------- + +def test_compute_results_excludes_voided_ballots(): + ballots = [ + {"picks": ["t1"], "voided": False}, + {"picks": ["t1", "t2"], "voided": True}, # excluded entirely + ] + teams_by_id = {"t1": {"name": "Team One"}, "t2": {"name": "Team Two"}} + exposure = {"t1": 2, "t2": 2} + results = svc.compute_results(ballots, teams_by_id, exposure) + by_id = {r["team_id"]: r for r in results} + assert by_id["t1"]["approvals"] == 1 + assert by_id["t2"]["approvals"] == 0 + + +def test_compute_results_shown_reflects_cast_ballots_not_raw_exposure(): + """HIGH finding #2 β€” the reviewer's scenario: team A was persisted into + 10 slates (exposure) but only 2 ballots were actually cast, both + approving it; team B was persisted into only 3 slates but got 1 approving + ballot. Scoring on raw exposure would badly under-rate A's 100%-of-2 + approval rate against a denominator of 10; scoring on cast ballots (as + fixed) ranks A above B. exposure_shown keeps the raw count separately.""" + ballots = [ + {"slate": ["a", "x"], "picks": ["a"], "voided": False}, + {"slate": ["a", "y"], "picks": ["a"], "voided": False}, + {"slate": ["b", "z"], "picks": ["b"], "voided": False}, + ] + teams_by_id = {"a": {"name": "Team A"}, "b": {"name": "Team B"}} + exposure = {"a": 10, "b": 3} + results = svc.compute_results(ballots, teams_by_id, exposure) + by_id = {r["team_id"]: r for r in results} + + assert by_id["a"]["shown"] == 2 + assert by_id["a"]["exposure_shown"] == 10 + assert by_id["b"]["shown"] == 1 + assert by_id["b"]["exposure_shown"] == 3 + # A's rank must beat B's after the fix. + assert by_id["a"]["rank"] < by_id["b"]["rank"] + + +def test_compute_results_voiding_a_ballot_removes_it_from_shown_and_approvals(): + ballots_before = [ + {"slate": ["a"], "picks": ["a"], "voided": False}, + {"slate": ["a"], "picks": ["a"], "voided": False}, + ] + ballots_after_void = [ + {"slate": ["a"], "picks": ["a"], "voided": False}, + {"slate": ["a"], "picks": ["a"], "voided": True}, # this one got voided + ] + teams_by_id = {"a": {"name": "Team A"}} + exposure = {"a": 2} + + before = svc.compute_results(ballots_before, teams_by_id, exposure)[0] + after = svc.compute_results(ballots_after_void, teams_by_id, exposure)[0] + + assert before["shown"] == 2 and before["approvals"] == 2 + assert after["shown"] == 1 and after["approvals"] == 1 + + +def test_get_results_reports_voided_count_separately(wire, monkeypatch): + event = _seed_hackathon(constraints={"peer_vote_enabled": True}) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + monkeypatch.setattr("services.volunteers_service.get_all_hackers_by_event_id", lambda eid: []) + _seed_team(wire, "t1", project_submission_status="submitted") + wire[("peer_votes", "event-1__u1")] = {"event_id": "event-1", "picks": ["t1"], "voided": False} + wire[("peer_votes", "event-1__u2")] = {"event_id": "event-1", "picks": ["t1"], "voided": True} + + result, status = svc.get_results("event-1") + assert status == 200 + assert result["ballots"] == 1 + assert result["voided"] == 1 + + +def test_get_results_includes_ballots_detail_with_no_names_or_picks(wire, monkeypatch): + """LOW finding #11: the admin UI needs a per-voter list to drive `void` + β€” voter_propel_id (an opaque id, not PII) + voted_at + voided + a COUNT + of picks, but never the picks themselves or any name/email.""" + event = _seed_hackathon(constraints={"peer_vote_enabled": True}) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + monkeypatch.setattr("services.volunteers_service.get_all_hackers_by_event_id", lambda eid: []) + _seed_team(wire, "t1", project_submission_status="submitted") + wire[("peer_votes", "event-1__u1")] = { + "event_id": "event-1", "voter_propel_id": "u1", "picks": ["t1"], + "voted_at": "2026-01-01T00:00:00+00:00", "voided": False, + } + wire[("peer_votes", "event-1__u2")] = { + "event_id": "event-1", "voter_propel_id": "u2", "picks": None, + "voted_at": None, "voided": True, + } + + result, status = svc.get_results("event-1") + assert status == 200 + detail_by_voter = {d["voter_propel_id"]: d for d in result["ballots_detail"]} + assert detail_by_voter["u1"] == { + "voter_propel_id": "u1", "voted_at": "2026-01-01T00:00:00+00:00", + "voided": False, "picks_count": 1, + } + assert detail_by_voter["u2"]["voided"] is True + assert detail_by_voter["u2"]["picks_count"] == 0 + for detail in result["ballots_detail"]: + assert "picks" not in detail + assert "name" not in detail + assert "email" not in detail + + +# --------------------------------------------------------------------------- +# publish_results β€” idempotent +# --------------------------------------------------------------------------- + +def test_publish_results_appends_award_once(wire, monkeypatch): + event = _seed_hackathon(constraints={"peer_vote_enabled": True}) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + monkeypatch.setattr("services.volunteers_service.get_all_hackers_by_event_id", lambda eid: []) + _seed_team(wire, "t1", project_submission_status="submitted") + wire[("peer_votes", "event-1__u1")] = {"event_id": "event-1", "picks": ["t1"], "voided": False} + + result1, status1 = svc.publish_results("event-1", "admin-1") + result2, status2 = svc.publish_results("event-1", "admin-1") + + assert status1 == status2 == 200 + assert result1["winner_team_id"] == "t1" + assert wire[("teams", "t1")]["awards"].count("Hackers' Choice") == 1 + + +def test_publish_results_409_with_no_ballots(wire, monkeypatch): + event = _seed_hackathon(constraints={"peer_vote_enabled": True}) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + monkeypatch.setattr("services.volunteers_service.get_all_hackers_by_event_id", lambda eid: []) + result, status = svc.publish_results("event-1", "admin-1") + assert status == 409 + assert result["error"] == "no_ballots" + + +def test_publish_results_409_with_submitted_teams_but_zero_ballots(wire, monkeypatch): + """HIGH finding #1 β€” the actual reviewer scenario: compute_results emits + a row for every SUBMITTED team regardless of ballot count, so with 3 + submitted teams and 0 ballots, every row's wilson_lower_bound/approvals + tie at 0 and the sort falls through to team NAME, crowning an arbitrary + "winner". Must 409 instead, and must not write an award or a summary + doc.""" + event = _seed_hackathon(constraints={"peer_vote_enabled": True}) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + monkeypatch.setattr("services.volunteers_service.get_all_hackers_by_event_id", lambda eid: []) + _seed_team(wire, "aaa-team", project_submission_status="submitted") + _seed_team(wire, "bbb-team", project_submission_status="submitted") + _seed_team(wire, "ccc-team", project_submission_status="submitted") + # No peer_votes docs seeded at all β€” zero ballots cast. + + result, status = svc.publish_results("event-1", "admin-1") + + assert status == 409 + assert result["error"] == "no_ballots" + assert "awards" not in wire.get(("teams", "aaa-team"), {}) + assert ("hackathons", "evtdoc-1", "peer_vote", "summary") not in wire + + +def test_publish_results_409_when_slates_opened_but_nobody_voted(wire, monkeypatch): + """Ballots exist (slates were persisted β€” people opened the vote page) + but nobody actually cast a pick. results["ballots"] is non-zero here, so + this exercises the SECOND half of the HIGH-1 fix: the rank-1 team having + zero approvals.""" + event = _seed_hackathon(constraints={"peer_vote_enabled": True}) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + monkeypatch.setattr("services.volunteers_service.get_all_hackers_by_event_id", lambda eid: []) + _seed_team(wire, "t1", project_submission_status="submitted") + _seed_team(wire, "t2", project_submission_status="submitted") + wire[("peer_votes", "event-1__u1")] = { + "event_id": "event-1", "voter_propel_id": "u1", "slate": ["t1", "t2"], + "picks": None, "voided": False, + } + + result, status = svc.publish_results("event-1", "admin-1") + + assert status == 409 + assert result["error"] == "no_ballots" + + +def test_void_ballot_excludes_from_future_results(wire, monkeypatch): + event = _seed_hackathon(constraints={"peer_vote_enabled": True}) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + monkeypatch.setattr("services.volunteers_service.get_all_hackers_by_event_id", lambda eid: []) + _seed_team(wire, "t1", project_submission_status="submitted") + wire[("peer_votes", "event-1__u1")] = {"event_id": "event-1", "picks": ["t1"], "voided": False} + + void_result, void_status = svc.void_ballot("event-1", "u1", "admin-1") + assert void_status == 200 + + results, status = svc.get_results("event-1") + assert results["voided"] == 1 + assert results["ballots"] == 0 + + +def test_void_ballot_404_for_unknown_voter(wire, monkeypatch): + result, status = svc.void_ballot("event-1", "nope", "admin-1") + assert status == 404 + + +# --------------------------------------------------------------------------- +# get_public_summary β€” gating +# --------------------------------------------------------------------------- + +def test_get_public_summary_unpublished_by_default(wire, monkeypatch): + event = _seed_hackathon(constraints={"peer_vote_enabled": True}) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + result, status = svc.get_public_summary("event-1") + assert status == 200 + assert result == {"published": False} + + +def test_get_public_summary_after_publish(wire, monkeypatch): + event = _seed_hackathon(constraints={"peer_vote_enabled": True}) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event) + monkeypatch.setattr("services.volunteers_service.get_all_hackers_by_event_id", lambda eid: []) + _seed_team(wire, "t1", project_submission_status="submitted") + wire[("peer_votes", "event-1__u1")] = {"event_id": "event-1", "picks": ["t1"], "voided": False} + svc.publish_results("event-1", "admin-1") + + result, status = svc.get_public_summary("event-1") + assert status == 200 + assert result["published"] is True + assert result["winner_team_id"] == "t1" + + +def test_get_public_summary_unknown_event(monkeypatch): + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: None) + result, status = svc.get_public_summary("nope") + assert result == {"published": False} diff --git a/api/submissions/README.md b/api/submissions/README.md new file mode 100644 index 0000000..bcd99af --- /dev/null +++ b/api/submissions/README.md @@ -0,0 +1,139 @@ +# Submissions API + +Team project write-ups + submission deadlines. Introduced Sep 2026 so +ohack.dev's team dashboard can replace DevPost's project page and deadline +mechanics (judging itself is unchanged β€” see `api/judging/`). + +## Base URL +`/api` + +## Ownership + +- `api.submissions.submissions_service` owns SELF-SERVE, deadline-aware + writes to a team's `project_*` fields (and now also the pre-existing + `/team//devpost` and `/team//demo-video` routes, via + `self_serve_team_edit`). `self_serve_team_edit` also busts the + `services.hackathons_service` event cache (via this module's `clear_cache`) + after delegating to `edit_team`, which on its own only clears the generic + per-function caches β€” without this the event page kept showing a stale + DevPost link / demo video for up to 10 minutes after a self-serve save. +- `api.teams.teams_service.edit_team` remains the ADMIN write path (no + deadline gate, no membership check β€” gated at the route by + `volunteer.admin`). An admin can override `project_*` fields through + `PATCH /api/team/edit`, including `project_submission_status` β€” validated + against `draft|submitted|late` (400 on anything else, no write); + `project_tagline`/`project_story` get the same `sanitize_markdown` + treatment as the self-serve `save_project` path; a status *change* stamps + `project_updated_at`. + +Every write here goes through `_authorize_team_write`, which: +1. 404s if the team doesn't exist. +2. 403s `{"error": "not_team_member"}` if the caller isn't on the team and + isn't an admin (`is_admin(auth_user)` from + `services.hackathon_planning_service`). +3. Computes the event's submission window and, unless the caller is an admin + or the route opted out (`enforce_deadline=False`, used only by + mentor-availability), 409s `{"error": "submissions_closed", "deadline", + "late_until", "now"}` once it's closed. + +## Data model + +New optional keys on `teams/{id}` (absent β‡’ legacy team, no dashboard +UI implied): `project_tagline`, `project_story`, `project_built_with`, +`project_links`, `project_thumbnail_url`, `project_images`, +`project_updated_at`, `project_submitted_at`, `project_submission_status` +(`draft|submitted|late`), `mentor_help_wanted` (absent β‡’ treated as `True`). + +New optional key on `hackathons/{id}`: `deadlines` β€” see +`common.utils.validators.validate_deadlines` for the shape and +`services.hackathons_service.save_hackathon` for how a `None` value becomes a +Firestore `DELETE_FIELD` on update (or is simply omitted on create). **To +clear a single deadline, send `{"deadlines": {"": null}}`; sending +`{"deadlines": {}}` (or a top-level `"deadlines": null`) is a no-op** β€” an +empty map merges zero sub-fields into the stored `deadlines` map under +`set(merge=True)`, leaving whatever was already there untouched. + +`compute_submission_window` (here) and `compute_voting_window` +(`api/peer_votes/`) both re-parse stored deadline strings through +`normalize_deadline_iso` before comparing them against `now`, rather than +calling `datetime.fromisoformat` on the raw stored value directly β€” a naive +or `"Z"`-suffixed stored string used to raise (TypeError comparing +naive-vs-aware; `"Z"` isn't accepted by `fromisoformat` until Python 3.11, +and this backend targets 3.9) and surface as an unhandled 500 across +`/project`, `/submit`, `/devpost`, `/demo-video`, `/window`, `/slate`, and +`/ballot`. An unparseable stored value is now logged and treated as absent +(`no_deadline` for submissions, `closed` for voting) instead. + +## Endpoints + +### `POST /api/team//project` +Member (or admin) only. Partial update β€” only keys present in the body are +validated and written. 400 `{"error": "invalid_project", "errors": [...]}` +on a bad field, 403/409 per `_authorize_team_write`. Sets +`project_submission_status = "draft"` on the very first save (never regressed +by a hacker's own write afterward). Returns +`{"success": true, "team": , "window": }`. + +### `POST /api/team//project/submit` +Member (or admin) only. Requires `project_tagline` and `project_story` to +already be saved (400 `{"error": "incomplete", "missing": [...]}` otherwise). +Idempotent β€” resubmitting returns `{"success": true, "already_submitted": +true, "team": ...}` **even after the submission window has fully closed**: +the already-submitted check runs before the deadline gate, not after, so a +team that submitted on time never gets a spurious 409 just by revisiting the +dashboard past close. A team that has *not yet* submitted is still blocked +with 409 `submissions_closed` once the window is fully closed, unless the +caller is an admin. Sets `project_submission_status` to `"submitted"` (window +open/no-deadline) or `"late"` (window in its late-grace period, or an admin +forcing a submission through after full close). + +### `POST /api/team//mentor-availability` +Member (or admin) only. Body `{"open": bool}`. No deadline gate β€” a team can +flip this any time. Signal only; changes no other backend behaviour. + +### `POST /api/team//devpost`, `POST /api/team//demo-video` +(Defined in `api/teams/teams_views.py`, delegate to +`self_serve_team_edit` here.) **Security fix (Part 9 bug #1):** these used to +call `edit_team` directly with no membership check at all β€” any logged-in +user could overwrite any team's DevPost link or demo video. They now run +through the same `_authorize_team_write` gate as everything else in this +module. + +### `GET /api/hackathons//submissions/window` +Public. `{"state": "open"|"late"|"closed"|"no_deadline", "submission", +"late_until", "now", "timezone"}`. Backs the dashboard's deadline strip and +the admin `DeadlinesSection` preview β€” the frontend should treat the server's +`now`/`state` as authoritative rather than deriving state from `deadlines` +client-side, though `deriveVoteWindow`-style client mirroring is fine for a +non-authoritative live countdown between polls. + +## Sanitization + +`project_tagline` / `project_story` are stored as raw markdown. The frontend +renders them with `react-markdown` **without** `rehype-raw`, so any HTML tag +in the stored text is already inert on read β€” `sanitize_markdown` (in +`common.utils.validators`) is defence-in-depth only: it strips a small +denylist of tags (`script|iframe|object|embed|style|link|meta|form|base`), +`on*=` attributes (whitespace- **or** slash-preceded, so `` +is caught too), and neutralizes `javascript:`/`vbscript:`/`data:` targets in +HTML attributes (quoted **or** unquoted) as well as in markdown link/image +syntax (`[text](javascript:...)` β†’ `[text](#)`). The tag-strip pass loops to +a fixpoint so a nested bypass like `ipt>` β€” where a single strip +pass removes the inner `"}, "team-1") + assert errors == [] + assert " for the queue."}, "team-1" + ) + assert errors == [] + assert "List" in clean["project_story"] + + +def test_validate_project_payload_rejects_tagline_over_limit(): + clean, errors = svc.validate_project_payload({"project_tagline": "x" * 141}, "team-1") + assert any(e["field"] == "project_tagline" for e in errors) + assert "project_tagline" not in clean + + +def test_validate_project_payload_rejects_too_many_built_with_tags(): + clean, errors = svc.validate_project_payload({"project_built_with": [f"tag{i}" for i in range(26)]}, "team-1") + assert any(e["field"] == "project_built_with" for e in errors) + + +def test_validate_project_payload_rejects_too_many_links(): + links = [{"label": "l", "url": "https://example.com"} for _ in range(11)] + clean, errors = svc.validate_project_payload({"project_links": links}, "team-1") + assert any(e["field"] == "project_links" for e in errors) + + +def test_validate_project_payload_rejects_http_link_url(): + clean, errors = svc.validate_project_payload( + {"project_links": [{"label": "Repo", "url": "http://example.com"}]}, "team-1" + ) + assert any(e["field"] == "project_links" for e in errors) + + +def test_validate_project_payload_rejects_thumbnail_off_own_cdn(): + clean, errors = svc.validate_project_payload( + {"project_thumbnail_url": "https://evil.example.com/x.png"}, "team-1" + ) + assert any(e["field"] == "project_thumbnail_url" for e in errors) + + +def test_validate_project_payload_trusts_existing_thumbnail_without_reverify(monkeypatch): + url = f"{svc._cdn_server()}/teams/team-1/project/thumb.png" + # If GCS verification were attempted it would raise via this stub. + def boom(path): + raise AssertionError("should not re-verify an already-saved URL") + monkeypatch.setattr("common.utils.cdn.get_blob_metadata", boom) + clean, errors = svc.validate_project_payload( + {"project_thumbnail_url": url}, "team-1", existing={"project_thumbnail_url": url} + ) + assert errors == [] + assert clean["project_thumbnail_url"] == url + + +def test_validate_project_payload_verifies_new_own_cdn_thumbnail(monkeypatch): + url = f"{svc._cdn_server()}/teams/team-1/project/thumb.png" + monkeypatch.setattr( + "common.utils.cdn.get_blob_metadata", + lambda path: {"exists": True, "size": 1000, "content_type": "image/png"}, + ) + clean, errors = svc.validate_project_payload({"project_thumbnail_url": url}, "team-1") + assert errors == [] + assert clean["project_thumbnail_url"] == url + + +def test_validate_project_payload_rejects_missing_upload(monkeypatch): + url = f"{svc._cdn_server()}/teams/team-1/project/thumb.png" + monkeypatch.setattr( + "common.utils.cdn.get_blob_metadata", + lambda path: {"exists": False, "size": None, "content_type": None}, + ) + clean, errors = svc.validate_project_payload({"project_thumbnail_url": url}, "team-1") + assert any(e["reason"] == "upload_not_found" for e in errors) + + +# --------------------------------------------------------------------------- +# save_project β€” auth, deadline gate, draft stamping +# --------------------------------------------------------------------------- + +def test_save_project_403_for_non_member(wire, monkeypatch): + _seed_team(wire) + _member(monkeypatch, is_member=False) + _event(monkeypatch) + result, status = svc.save_project("propel-1", "team-1", {"project_tagline": "hi"}) + assert status == 403 + assert result["error"] == "not_team_member" + + +def test_save_project_409_when_closed(wire, monkeypatch): + _seed_team(wire) + _member(monkeypatch, is_member=True) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: { + "timezone": "UTC", + "deadlines": {"submission": "2026-10-10T15:00:00+00:00"}, + }) + monkeypatch.setattr(svc, "compute_submission_window", lambda event, now=None: {"state": "closed", "submission": "2026-10-10T15:00:00+00:00", "late_until": None, "now": "2026-10-10T20:00:00+00:00", "timezone": "UTC"}) + result, status = svc.save_project("propel-1", "team-1", {"project_tagline": "hi"}) + assert status == 409 + assert result["error"] == "submissions_closed" + assert result["deadline"] == "2026-10-10T15:00:00+00:00" + + +def test_save_project_admin_bypasses_deadline(wire, monkeypatch): + _seed_team(wire) + _member(monkeypatch, is_member=False) # admin bypasses membership too + monkeypatch.setattr(svc, "compute_submission_window", lambda event, now=None: {"state": "closed", "submission": "x", "late_until": None, "now": "y", "timezone": "UTC"}) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {}) + result, status = svc.save_project("admin-propel", "team-1", {"project_tagline": "hi"}, admin=True) + assert status == 200 + assert result["success"] is True + + +def test_save_project_first_save_sets_draft_status(wire, monkeypatch): + _seed_team(wire) + _member(monkeypatch, is_member=True) + _event(monkeypatch) + result, status = svc.save_project("propel-1", "team-1", {"project_tagline": "Hi"}) + assert status == 200 + assert wire["team-1"]["project_submission_status"] == "draft" + assert wire["team-1"]["project_tagline"] == "Hi" + + +def test_save_project_400_on_invalid_payload(wire, monkeypatch): + _seed_team(wire) + _member(monkeypatch, is_member=True) + _event(monkeypatch) + result, status = svc.save_project("propel-1", "team-1", {"project_tagline": "x" * 200}) + assert status == 400 + assert result["error"] == "invalid_project" + + +def test_save_project_after_submit_keeps_status(wire, monkeypatch): + _seed_team(wire, project_submission_status="submitted", project_tagline="T", project_story="S") + _member(monkeypatch, is_member=True) + _event(monkeypatch) + result, status = svc.save_project("propel-1", "team-1", {"project_tagline": "Updated tagline"}) + assert status == 200 + assert wire["team-1"]["project_submission_status"] == "submitted" + assert wire["team-1"]["project_tagline"] == "Updated tagline" + + +# --------------------------------------------------------------------------- +# submit_project +# --------------------------------------------------------------------------- + +def test_submit_project_400_incomplete_without_story(wire, monkeypatch): + _seed_team(wire, project_tagline="Only a tagline") + _member(monkeypatch, is_member=True) + _event(monkeypatch) + result, status = svc.submit_project("propel-1", "team-1") + assert status == 400 + assert result["error"] == "incomplete" + assert "project_story" in result["missing"] + + +def test_submit_project_sets_submitted_when_open(wire, monkeypatch): + _seed_team(wire, project_tagline="T", project_story="S") + _member(monkeypatch, is_member=True) + _event(monkeypatch) + result, status = svc.submit_project("propel-1", "team-1") + assert status == 200 + assert wire["team-1"]["project_submission_status"] == "submitted" + assert "project_submitted_at" in wire["team-1"] + + +def test_submit_project_sets_late_inside_grace_window(wire, monkeypatch): + _seed_team(wire, project_tagline="T", project_story="S") + _member(monkeypatch, is_member=True) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {}) + monkeypatch.setattr(svc, "compute_submission_window", lambda event, now=None: {"state": "late", "submission": "x", "late_until": "y", "now": "z", "timezone": "UTC"}) + result, status = svc.submit_project("propel-1", "team-1") + assert status == 200 + assert wire["team-1"]["project_submission_status"] == "late" + + +def test_submit_project_idempotent_when_already_submitted(wire, monkeypatch): + _seed_team(wire, project_submission_status="submitted", project_tagline="T", project_story="S") + _member(monkeypatch, is_member=True) + _event(monkeypatch) + result, status = svc.submit_project("propel-1", "team-1") + assert status == 200 + assert result["already_submitted"] is True + + +def test_submit_project_idempotent_even_after_deadline_closed(wire, monkeypatch): + """LOW finding #10 regression: a team that submitted on time must still + get the idempotent 200 (not a 409) if they revisit the submit endpoint + after the window has fully closed β€” the already-submitted check has to + run BEFORE the deadline gate.""" + _seed_team(wire, project_submission_status="submitted", project_tagline="T", project_story="S") + _member(monkeypatch, is_member=True) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {}) + monkeypatch.setattr(svc, "compute_submission_window", lambda event, now=None: {"state": "closed", "submission": "x", "late_until": None, "now": "z", "timezone": "UTC"}) + result, status = svc.submit_project("propel-1", "team-1") + assert status == 200 + assert result["already_submitted"] is True + + +def test_submit_project_403_for_non_member(wire, monkeypatch): + _seed_team(wire) + _member(monkeypatch, is_member=False) + _event(monkeypatch) + result, status = svc.submit_project("propel-1", "team-1") + assert status == 403 + + +def test_submit_project_409_when_closed_for_non_admin(wire, monkeypatch): + _seed_team(wire, project_tagline="T", project_story="S") + _member(monkeypatch, is_member=True) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {}) + monkeypatch.setattr(svc, "compute_submission_window", lambda event, now=None: {"state": "closed", "submission": "x", "late_until": None, "now": "z", "timezone": "UTC"}) + result, status = svc.submit_project("propel-1", "team-1") + assert status == 409 + assert result["error"] == "submissions_closed" + + +def test_submit_project_admin_forced_after_close_is_recorded_late(wire, monkeypatch): + _seed_team(wire, project_tagline="T", project_story="S") + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {}) + monkeypatch.setattr(svc, "compute_submission_window", lambda event, now=None: {"state": "closed", "submission": "x", "late_until": None, "now": "z", "timezone": "UTC"}) + result, status = svc.submit_project("admin-propel", "team-1", admin=True) + assert status == 200 + assert wire["team-1"]["project_submission_status"] == "late" + + +# --------------------------------------------------------------------------- +# self_serve_team_edit β€” Part 9 bug #1 (was: any logged-in user could edit +# any team's devpost/demo-video link). +# --------------------------------------------------------------------------- + +def test_self_serve_team_edit_403_for_non_member(wire, monkeypatch): + _seed_team(wire) + _member(monkeypatch, is_member=False) + _event(monkeypatch) + result, status = svc.self_serve_team_edit("propel-1", "team-1", {"devpost_link": "https://devpost.com/x"}) + assert status == 403 + assert result["error"] == "not_team_member" + + +def test_self_serve_team_edit_allows_member(wire, monkeypatch): + _seed_team(wire) + _member(monkeypatch, is_member=True) + _event(monkeypatch) + monkeypatch.setattr( + "api.teams.teams_service.edit_team", + lambda json: {"success": True, "message": "Team updated successfully", "team_id": json["id"]}, + ) + result = svc.self_serve_team_edit("propel-1", "team-1", {"devpost_link": "https://devpost.com/x"}) + assert result["success"] is True + assert "team" in result + + +def test_self_serve_team_edit_409_when_closed(wire, monkeypatch): + _seed_team(wire) + _member(monkeypatch, is_member=True) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {}) + monkeypatch.setattr(svc, "compute_submission_window", lambda event, now=None: {"state": "closed", "submission": "x", "late_until": None, "now": "z", "timezone": "UTC"}) + result, status = svc.self_serve_team_edit("propel-1", "team-1", {"demo_video_url": "https://youtu.be/x"}) + assert status == 409 + + +def test_self_serve_team_edit_also_clears_hackathon_event_cache(monkeypatch, team_store): + """MEDIUM finding #4 regression: self_serve_team_edit delegates the write + to edit_team, which only busts the generic per-function caches β€” it must + ALSO bust services.hackathons_service's own get_single_hackathon_event + cache (via this module's clear_cache()), or the event page shows a stale + DevPost link / demo video for up to 10 minutes. + + Deliberately does not use the `wire` fixture, which stubs svc.clear_cache + to a no-op β€” this test needs the REAL clear_cache() to run so it can + assert on what it calls. + """ + monkeypatch.setattr(svc, "get_db", lambda: FakeDb(team_store)) + monkeypatch.setattr(svc, "send_slack_audit", lambda **kwargs: None) + monkeypatch.setattr(svc, "send_slack", lambda **kwargs: None) + monkeypatch.setattr(svc, "clear_all_caches", lambda: None) + monkeypatch.setattr(svc, "get_team", lambda team_id: {"team": {"id": team_id}}) + + _seed_team(team_store) + _member(monkeypatch, is_member=True) + _event(monkeypatch) + monkeypatch.setattr( + "api.teams.teams_service.edit_team", + lambda json: {"success": True, "message": "Team updated successfully", "team_id": json["id"]}, + ) + hackathon_clear = MagicMock() + monkeypatch.setattr("services.hackathons_service.clear_cache", hackathon_clear) + + svc.self_serve_team_edit("propel-1", "team-1", {"devpost_link": "https://devpost.com/x"}) + + hackathon_clear.assert_called_once() + + +# --------------------------------------------------------------------------- +# set_mentor_help_wanted +# --------------------------------------------------------------------------- + +def test_set_mentor_help_wanted_rejects_non_bool(wire, monkeypatch): + _seed_team(wire) + _member(monkeypatch, is_member=True) + result, status = svc.set_mentor_help_wanted("propel-1", "team-1", "yes") + assert status == 400 + + +def test_set_mentor_help_wanted_no_deadline_gate_even_when_closed(wire, monkeypatch): + _seed_team(wire) + _member(monkeypatch, is_member=True) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {}) + monkeypatch.setattr(svc, "compute_submission_window", lambda event, now=None: {"state": "closed", "submission": "x", "late_until": None, "now": "z", "timezone": "UTC"}) + result, status = svc.set_mentor_help_wanted("propel-1", "team-1", False) + assert status == 200 + assert wire["team-1"]["mentor_help_wanted"] is False + + +def test_set_mentor_help_wanted_403_for_non_member(wire, monkeypatch): + _seed_team(wire) + _member(monkeypatch, is_member=False) + result, status = svc.set_mentor_help_wanted("propel-1", "team-1", True) + assert status == 403 + + +# --------------------------------------------------------------------------- +# get_submission_window_for_event +# --------------------------------------------------------------------------- + +def test_get_submission_window_for_event_404_unknown_event(monkeypatch): + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: None) + result, status = svc.get_submission_window_for_event("nope") + assert status == 404 + + +def test_get_submission_window_for_event_returns_window(monkeypatch): + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {"timezone": "UTC", "deadlines": {}}) + result, status = svc.get_submission_window_for_event("event-1") + assert status == 200 + assert result["state"] == "no_deadline" + + +# --------------------------------------------------------------------------- +# Deadline reminders β€” build_reminder_message / send_deadline_reminders / +# send_due_reminders_for_current_events. A separate, more general fake db is +# used here (needs .where().stream() across multiple team docs, which the +# id-only FakeDb above doesn't support). +# --------------------------------------------------------------------------- + +class _ReminderFakeDocRef: + """Only .set() is exercised β€” send_deadline_reminders reads + reminders_sent off the already-mocked get_hackathon_by_event_id() return + value, never off a Firestore read of this ref.""" + + def __init__(self, store, key): + self._store = store + self._key = key + + def set(self, data, merge=False): + if merge and self._key in self._store: + existing = self._store[self._key] + for k, v in data.items(): + if isinstance(v, dict) and isinstance(existing.get(k), dict): + existing[k] = {**existing[k], **v} + else: + existing[k] = v + else: + self._store[self._key] = dict(data) + + +class _ReminderFakeTeamDoc: + def __init__(self, doc_id, data): + self.id = doc_id + self._data = data + + def to_dict(self): + return dict(self._data) + + +class _ReminderFakeQuery: + def __init__(self, teams, field, value): + self._teams = teams + self._field = field + self._value = value + + def stream(self): + return [ + _ReminderFakeTeamDoc(tid, data) + for tid, data in self._teams.items() + if data.get(self._field) == self._value + ] + + +class _ReminderFakeCollection: + def __init__(self, teams, hackathons, name): + self._teams = teams + self._hackathons = hackathons + self._name = name + + def document(self, doc_id): + assert self._name == "hackathons" + return _ReminderFakeDocRef(self._hackathons, doc_id) + + def where(self, field, op, value): + assert self._name == "teams" + return _ReminderFakeQuery(self._teams, field, value) + + +class _ReminderFakeDb: + def __init__(self, teams, hackathons): + self._teams = teams + self._hackathons = hackathons + + def collection(self, name): + return _ReminderFakeCollection(self._teams, self._hackathons, name) + + +@pytest.fixture +def reminder_wire(monkeypatch): + teams = {} + hackathons = {} + monkeypatch.setattr(svc, "get_db", lambda: _ReminderFakeDb(teams, hackathons)) + monkeypatch.setattr(svc, "clear_cache", lambda: None) + monkeypatch.setattr(svc, "send_slack_audit", lambda **kwargs: None) + sent_messages = [] + monkeypatch.setattr(svc, "send_slack", lambda message, channel: sent_messages.append((channel, message))) + return {"teams": teams, "hackathons": hackathons, "sent": sent_messages} + + +def test_build_reminder_message_none_when_already_submitted(): + team = {"project_submission_status": "submitted"} + assert svc.build_reminder_message(team, {"event_id": "e1"}, "x", 24) is None + + +def test_build_reminder_message_lists_missing_items(): + msg = svc.build_reminder_message({}, {"event_id": "e1"}, "x", 6) + assert "tagline" in msg + assert "demo video" in msg + assert "submit your project" in msg + assert "6 hours left" in msg + + +def test_build_reminder_message_singular_hour_label(): + msg = svc.build_reminder_message({}, {"event_id": "e1"}, "x", 1) + assert "1 hour left" in msg + assert "1 hours" not in msg + + +def test_send_deadline_reminders_404_unknown_event(monkeypatch): + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: None) + result, status = svc.send_deadline_reminders("nope", "submission", 24) + assert status == 404 + + +def test_send_deadline_reminders_409_when_no_deadline_configured(monkeypatch): + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {"deadlines": {}}) + result, status = svc.send_deadline_reminders("event-1", "submission", 24) + assert status == 409 + assert result["error"] == "no_deadline" + + +def test_send_deadline_reminders_rejects_bad_kind_and_hours(monkeypatch): + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {"deadlines": {"submission": "2026-01-01T00:00:00+00:00"}}) + _, status = svc.send_deadline_reminders("event-1", "judging", 24) + assert status == 400 + _, status = svc.send_deadline_reminders("event-1", "submission", 5) + assert status == 400 + + +def test_send_deadline_reminders_notifies_and_records_idempotency_key(reminder_wire, monkeypatch): + now = datetime.now(timezone.utc) + deadline = (now + timedelta(hours=24)).isoformat() + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {"id": "evtdoc-1", "event_id": "event-1", "deadlines": {"submission": deadline}}) + reminder_wire["teams"]["team-1"] = {"hackathon_event_id": "event-1", "slack_channel": "#team-1"} + + result, status = svc.send_deadline_reminders("event-1", "submission", 24) + + assert status == 200 + assert result["notified"] == ["team-1"] + assert reminder_wire["hackathons"]["evtdoc-1"]["reminders_sent"]["submission_24h"]["teams_notified"] == ["team-1"] + + +def test_send_deadline_reminders_skips_team_without_slack_channel(reminder_wire, monkeypatch): + now = datetime.now(timezone.utc) + deadline = (now + timedelta(hours=24)).isoformat() + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {"id": "evtdoc-1", "event_id": "event-1", "deadlines": {"submission": deadline}}) + reminder_wire["teams"]["team-1"] = {"hackathon_event_id": "event-1"} + + result, status = svc.send_deadline_reminders("event-1", "submission", 24) + + assert result["notified"] == [] + assert result["skipped"] == [{"team_id": "team-1", "reason": "no_slack_channel"}] + + +def test_send_deadline_reminders_skips_already_done_team(reminder_wire, monkeypatch): + now = datetime.now(timezone.utc) + deadline = (now + timedelta(hours=24)).isoformat() + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {"id": "evtdoc-1", "event_id": "event-1", "deadlines": {"submission": deadline}}) + reminder_wire["teams"]["team-1"] = {"hackathon_event_id": "event-1", "slack_channel": "#t1", "project_submission_status": "submitted"} + + result, status = svc.send_deadline_reminders("event-1", "submission", 24) + + assert result["notified"] == [] + assert result["skipped"] == [{"team_id": "team-1", "reason": "already_done"}] + + +def test_send_deadline_reminders_409_already_sent_then_force_succeeds(reminder_wire, monkeypatch): + now = datetime.now(timezone.utc) + deadline = (now + timedelta(hours=24)).isoformat() + + def event_with_reminders(): + return { + "id": "evtdoc-1", "event_id": "event-1", "deadlines": {"submission": deadline}, + "reminders_sent": {"submission_24h": {"sent_at": "earlier"}}, + } + + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: event_with_reminders()) + reminder_wire["teams"]["team-1"] = {"hackathon_event_id": "event-1", "slack_channel": "#t1"} + + result, status = svc.send_deadline_reminders("event-1", "submission", 24) + assert status == 409 + assert result["error"] == "already_sent" + + result, status = svc.send_deadline_reminders("event-1", "submission", 24, force=True) + assert status == 200 + assert result["notified"] == ["team-1"] + + +def test_send_deadline_reminders_only_if_due_skips_outside_window(reminder_wire, monkeypatch): + now = datetime.now(timezone.utc) + # Deadline is 10 hours away; a 24h reminder isn't due yet (due window is + # [deadline-24h, deadline)). + deadline = (now + timedelta(hours=10)).isoformat() + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {"id": "evtdoc-1", "event_id": "event-1", "deadlines": {"submission": deadline}}) + reminder_wire["teams"]["team-1"] = {"hackathon_event_id": "event-1", "slack_channel": "#t1"} + + result, status = svc.send_deadline_reminders("event-1", "submission", 1, only_if_due=True) + assert status == 200 + assert result["skipped"] == "not_due" + assert reminder_wire["teams"]["team-1"] # untouched, no reminder recorded + + +def test_send_deadline_reminders_only_if_due_sends_inside_window(reminder_wire, monkeypatch): + # The due window is now exactly one hour wide: [deadline-24h, deadline-23h). + # Center the deadline in that window (23h30m away) so the small delay + # between building `deadline` here and send_deadline_reminders computing + # its own `now_dt` can never push the check outside the window. + now = datetime.now(timezone.utc) + deadline = (now + timedelta(hours=23, minutes=30)).isoformat() + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {"id": "evtdoc-1", "event_id": "event-1", "deadlines": {"submission": deadline}}) + reminder_wire["teams"]["team-1"] = {"hackathon_event_id": "event-1", "slack_channel": "#t1"} + + result, status = svc.send_deadline_reminders("event-1", "submission", 24, only_if_due=True) + assert status == 200 + assert result["notified"] == ["team-1"] + + +def test_send_deadline_reminders_only_if_due_does_not_double_fire_when_close_to_deadline(reminder_wire, monkeypatch): + """LOW finding #9 regression: with the old [deadline-h, deadline) window, + a deadline only 3 hours away would fall inside BOTH the 24h and 6h due + windows on the very first cron tick, sending two reminders at once. Each + tier's due window is now a narrow 1-hour slot, so neither one (whose + slot already elapsed, since the deadline is unusually close) fires.""" + now = datetime.now(timezone.utc) + deadline = (now + timedelta(hours=3)).isoformat() + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {"id": "evtdoc-1", "event_id": "event-1", "deadlines": {"submission": deadline}}) + reminder_wire["teams"]["team-1"] = {"hackathon_event_id": "event-1", "slack_channel": "#t1"} + + result_24h, status_24h = svc.send_deadline_reminders("event-1", "submission", 24, only_if_due=True) + result_6h, status_6h = svc.send_deadline_reminders("event-1", "submission", 6, only_if_due=True) + + assert status_24h == status_6h == 200 + assert result_24h["skipped"] == "not_due" + assert result_6h["skipped"] == "not_due" + + +def test_send_deadline_reminders_simulated_flag_reflects_test_environment(reminder_wire, monkeypatch): + now = datetime.now(timezone.utc) + deadline = (now + timedelta(hours=24)).isoformat() + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {"id": "evtdoc-1", "event_id": "event-1", "deadlines": {"submission": deadline}}) + reminder_wire["teams"]["team-1"] = {"hackathon_event_id": "event-1", "slack_channel": "#t1"} + + result, status = svc.send_deadline_reminders("event-1", "submission", 24) + assert result["simulated"] is True + # ENVIRONMENT=test means _notifications_disabled() is True, so send_slack + # is never actually invoked even though the team is still "notified". + assert reminder_wire["sent"] == [] + + +def test_send_due_reminders_for_current_events_iterates_hours_and_events(reminder_wire, monkeypatch): + now = datetime.now(timezone.utc) + deadline = (now + timedelta(hours=23, minutes=30)).isoformat() + monkeypatch.setattr( + "services.hackathons_service.get_hackathon_list", + lambda kind: {"hackathons": [{"event_id": "event-1"}]}, + ) + monkeypatch.setattr(svc, "get_hackathon_by_event_id", lambda eid: {"id": "evtdoc-1", "event_id": "event-1", "deadlines": {"submission": deadline}}) + reminder_wire["teams"]["team-1"] = {"hackathon_event_id": "event-1", "slack_channel": "#t1"} + + result, status = svc.send_due_reminders_for_current_events() + + assert status == 200 + hours_checked = {r["hours_before"] for r in result["results"]} + assert hours_checked == {24, 6, 1} + # Only the 24h reminder was due (deadline is ~23h30m away); confirm it fired. + fired = [r for r in result["results"] if r["hours_before"] == 24][0] + assert fired["result"]["notified"] == ["team-1"] diff --git a/api/submissions/tests/test_submissions_views.py b/api/submissions/tests/test_submissions_views.py new file mode 100644 index 0000000..29e5837 --- /dev/null +++ b/api/submissions/tests/test_submissions_views.py @@ -0,0 +1,188 @@ +""" +Route-level tests for the submissions blueprint. Copies the signature-check +fixture from api/volunteers/tests/test_volunteers_views.py: propelauth's +decorators don't inject the user into the view, so a `def view(user, ...)` +mismatch only surfaces at real Flask dispatch time, not at import time. +""" +import functools +import importlib +import inspect +import os +import sys +import types +from unittest.mock import MagicMock + +os.environ.setdefault("ENVIRONMENT", "test") + +import pytest +from flask import Flask, g +from werkzeug.local import LocalProxy + +VIEWS_MODULE = "api.submissions.submissions_views" +FAKE_USER = types.SimpleNamespace(user_id="hacker-propel-uuid", email="hacker@example.com") + + +def _passthrough_decorator_factory(*_args, **_kwargs): + def decorator(fn): + @functools.wraps(fn) + def wrapper(*args, **kwargs): + g.propelauth_current_user = FAKE_USER + return fn(*args, **kwargs) + + return wrapper + + return decorator + + +@pytest.fixture +def views(monkeypatch): + stub = types.ModuleType("common.auth") + stub.auth = types.SimpleNamespace( + require_org_member_with_permission=_passthrough_decorator_factory, + require_user=_passthrough_decorator_factory(), + optional_user=_passthrough_decorator_factory(), + ) + stub.auth_user = LocalProxy(lambda: g.propelauth_current_user) + monkeypatch.setitem(sys.modules, "common.auth", stub) + sys.modules.pop(VIEWS_MODULE, None) + module = importlib.import_module(VIEWS_MODULE) + yield module + sys.modules.pop(VIEWS_MODULE, None) + + +@pytest.fixture +def app(views): + flask_app = Flask(__name__) + flask_app.register_blueprint(views.bp) + return flask_app + + +@pytest.fixture +def client(app): + return app.test_client() + + +HEADERS = {"Authorization": "Bearer test"} + + +def test_every_view_signature_matches_its_url_params(app): + mismatches = [] + for rule in app.url_map.iter_rules(): + if rule.endpoint == "static": + continue + view = inspect.unwrap(app.view_functions[rule.endpoint]) + declared = { + name + for name, p in inspect.signature(view).parameters.items() + if p.kind in (p.POSITIONAL_OR_KEYWORD, p.KEYWORD_ONLY) + } + if declared != set(rule.arguments): + mismatches.append((rule.rule, sorted(declared), sorted(rule.arguments))) + assert mismatches == [], f"view params != URL params: {mismatches}" + + +def test_save_project_route_dispatches_with_token_identity(views, client, monkeypatch): + service = MagicMock(return_value=({"success": True}, 200)) + monkeypatch.setattr(views, "save_project", service) + monkeypatch.setattr(views, "is_admin", lambda user: False) + + res = client.post("/api/team/team-1/project", json={"project_tagline": "Hi"}, headers=HEADERS) + + assert res.status_code == 200, res.get_json() + service.assert_called_once_with(FAKE_USER.user_id, "team-1", {"project_tagline": "Hi"}, admin=False) + + +def test_submit_project_route_dispatches(views, client, monkeypatch): + service = MagicMock(return_value=({"success": True, "status": "submitted"}, 200)) + monkeypatch.setattr(views, "submit_project", service) + monkeypatch.setattr(views, "is_admin", lambda user: False) + + res = client.post("/api/team/team-1/project/submit", headers=HEADERS) + + assert res.status_code == 200, res.get_json() + service.assert_called_once_with(FAKE_USER.user_id, "team-1", admin=False) + + +def test_mentor_availability_route_dispatches(views, client, monkeypatch): + service = MagicMock(return_value=({"success": True}, 200)) + monkeypatch.setattr(views, "set_mentor_help_wanted", service) + monkeypatch.setattr(views, "is_admin", lambda user: False) + + res = client.post("/api/team/team-1/mentor-availability", json={"open": False}, headers=HEADERS) + + assert res.status_code == 200, res.get_json() + service.assert_called_once_with(FAKE_USER.user_id, "team-1", False, admin=False) + + +def test_submissions_window_route_is_public_and_dispatches(views, client, monkeypatch): + service = MagicMock(return_value=({"state": "open"}, 200)) + monkeypatch.setattr(views, "get_submission_window_for_event", service) + + res = client.get("/api/hackathons/event-1/submissions/window") + + assert res.status_code == 200, res.get_json() + service.assert_called_once_with("event-1") + + +def test_admin_flag_passed_through_when_org_permission_present(views, client, monkeypatch): + service = MagicMock(return_value=({"success": True}, 200)) + monkeypatch.setattr(views, "save_project", service) + monkeypatch.setattr(views, "is_admin", lambda user: True) + + res = client.post("/api/team/team-1/project", json={}, headers=HEADERS) + + assert res.status_code == 200, res.get_json() + service.assert_called_once_with(FAKE_USER.user_id, "team-1", {}, admin=True) + + +def test_remind_route_403s_without_admin_or_api_key(views, client, monkeypatch): + monkeypatch.setattr(views, "is_admin", lambda user: False) + monkeypatch.setenv("BACKEND_CRON_TOKEN", "secret-token") + + res = client.post("/api/hackathons/event-1/deadlines/remind", json={"hours_before": 24}) + + assert res.status_code == 403 + + +def test_remind_route_allows_admin(views, client, monkeypatch): + monkeypatch.setattr(views, "is_admin", lambda user: True) + service = MagicMock(return_value=({"success": True}, 200)) + monkeypatch.setattr(views, "send_deadline_reminders", service) + + res = client.post("/api/hackathons/event-1/deadlines/remind", json={"hours_before": 24}, headers=HEADERS) + + assert res.status_code == 200, res.get_json() + service.assert_called_once_with("event-1", "submission", 24, only_if_due=False, force=False, actor=FAKE_USER.user_id) + + +def test_remind_route_allows_api_key_without_login(views, client, monkeypatch): + monkeypatch.setattr(views, "is_admin", lambda user: False) + monkeypatch.setenv("BACKEND_CRON_TOKEN", "secret-token") + service = MagicMock(return_value=({"success": True}, 200)) + monkeypatch.setattr(views, "send_deadline_reminders", service) + + res = client.post( + "/api/hackathons/event-1/deadlines/remind", + json={"hours_before": 6, "only_if_due": True}, + headers={"X-Api-Key": "secret-token"}, + ) + + assert res.status_code == 200, res.get_json() + service.assert_called_once_with("event-1", "submission", 6, only_if_due=True, force=False, actor="cron") + + +def test_remind_due_route_requires_api_key(views, client, monkeypatch): + monkeypatch.setenv("BACKEND_CRON_TOKEN", "secret-token") + res = client.post("/api/hackathons/deadlines/remind-due") + assert res.status_code == 403 + + +def test_remind_due_route_dispatches_with_valid_key(views, client, monkeypatch): + monkeypatch.setenv("BACKEND_CRON_TOKEN", "secret-token") + service = MagicMock(return_value=({"success": True, "results": []}, 200)) + monkeypatch.setattr(views, "send_due_reminders_for_current_events", service) + + res = client.post("/api/hackathons/deadlines/remind-due", headers={"X-Api-Key": "secret-token"}) + + assert res.status_code == 200, res.get_json() + service.assert_called_once_with() diff --git a/api/teams/teams_service.py b/api/teams/teams_service.py index 7c53287..514e23a 100644 --- a/api/teams/teams_service.py +++ b/api/teams/teams_service.py @@ -15,11 +15,24 @@ from common.utils.slack import create_slack_channel, invite_user_to_channel, send_slack, send_slack_audit from common.utils.firebase import get_hackathon_by_event_id from common.utils.oauth_providers import extract_slack_user_id, is_oauth_user_id, normalize_slack_user_id +from common.utils.validators import sanitize_markdown -from common.utils.slack import add_bot_to_channel +from common.utils.slack import add_bot_to_channel logger = logging.getLogger("myapp") +# Admin override of a team's project_* write-up fields via PATCH +# /api/team/edit (MEDIUM finding #3 β€” documented in api/submissions/README.md +# "Ownership" as always having been the intended admin path, but edit_team's +# field_mappings never actually carried these fields). Limits/catalog are +# duplicated from api.submissions.submissions_service's PROJECT_LIMITS / +# SUBMITTED_STATUSES (not imported β€” teams_service must never import +# api.submissions, a one-directional dependency documented in that module's +# docstring). +PROJECT_TAGLINE_MAX_LEN = 140 +PROJECT_STORY_MAX_LEN = 20000 +PROJECT_SUBMISSION_STATUSES = {"draft", "submitted", "late"} + # Slack user IDs for OHack admins who are auto-invited to every team channel # and CCed on completion broadcasts. Single source of truth for both call sites. TEAM_COMPLETION_SLACK_ADMINS = [ @@ -294,6 +307,13 @@ def edit_team(json): "admin_notes": "admin_notes", "devpost_link": "devpost_link", "demo_video_url": "demo_video_url", + "project_tagline": "project_tagline", + "project_story": "project_story", + "project_built_with": "project_built_with", + "project_links": "project_links", + "project_thumbnail_url": "project_thumbnail_url", + "project_images": "project_images", + "project_submission_status": "project_submission_status", } # Normalize demo_video_url: trim, cap at 500 chars, empty string => clear @@ -303,6 +323,29 @@ def edit_team(json): trimmed = raw.strip()[:500] json["demo_video_url"] = trimmed if trimmed else None + # project_submission_status must be one of the catalog values β€” a bad + # value 400s rather than writing a status the dashboard/gallery/funnel + # don't know how to render. + if "project_submission_status" in json: + status_val = json.get("project_submission_status") + if status_val not in PROJECT_SUBMISSION_STATUSES: + return { + "message": f"Error: project_submission_status must be one of {sorted(PROJECT_SUBMISSION_STATUSES)}", + "success": False, + }, 400 + if status_val != team_data.get("project_submission_status"): + update_data["project_updated_at"] = datetime.now().isoformat() + + # project_tagline / project_story get the same defence-in-depth + # sanitize_markdown treatment as the self-serve save_project path. + if "project_tagline" in json: + val = json.get("project_tagline") + json["project_tagline"] = sanitize_markdown(val, PROJECT_TAGLINE_MAX_LEN) if isinstance(val, str) and val else None + + if "project_story" in json: + val = json.get("project_story") + json["project_story"] = sanitize_markdown(val, PROJECT_STORY_MAX_LEN) if isinstance(val, str) and val else None + # If this is the first time setting devpost_link, set devpost_link_submitted date if "devpost_link" in json and "devpost_link" not in team_data: update_data["devpost_link_submitted"] = datetime.now().isoformat() diff --git a/api/teams/teams_views.py b/api/teams/teams_views.py index b9595e8..d96b80b 100644 --- a/api/teams/teams_views.py +++ b/api/teams/teams_views.py @@ -78,19 +78,25 @@ def edit_team_api(): @auth.require_user def add_devpost_to_team_api(teamid): """ - Add a Devpost link to a team. - Requires user to be authenticated. + Add a Devpost link to a team. Self-serve β€” the caller must be on the team + (or an admin). Part 9 bug #1 fix: this used to call edit_team directly + with NO membership check, so any logged-in user could overwrite any + team's Devpost link. Routed through submissions.self_serve_team_edit, + which also 409s once the event's submission window has closed for a + non-admin caller. Lazy import: api.teams.teams_service must never import + api.submissions (one-directional dependency). """ logger.info(f"POST /team/{teamid}/devpost called") if auth_user and auth_user.user_id: - # Get the Devpost link from the request logger.info(f"Adding Devpost link to team {teamid}") - devpost_link = request.get_json().get("devpost_link") + devpost_link = (request.get_json() or {}).get("devpost_link") logger.info(f"Devpost link: {devpost_link}") if not devpost_link: return {"error": "Devpost link is required"}, 400 - return edit_team({"id": teamid, "devpost_link": devpost_link}) + from services.hackathon_planning_service import is_admin + from api.submissions.submissions_service import self_serve_team_edit + return self_serve_team_edit(auth_user.user_id, teamid, {"devpost_link": devpost_link}, admin=is_admin(auth_user)) logger.error("Could not obtain user details for POST /team//devpost") return {"error": "Unauthorized"}, 401 @@ -99,8 +105,9 @@ def add_devpost_to_team_api(teamid): @auth.require_user def add_demo_video_to_team_api(teamid): """ - Add or clear a demo video URL for a team. - Requires user to be authenticated (team self-serve, mirrors devpost endpoint). + Add or clear a demo video URL for a team. Self-serve β€” the caller must be + on the team (or an admin); see add_devpost_to_team_api's docstring for the + Part 9 bug #1 context this fixes too. Pass demo_video_url as empty string or null to clear. """ logger.info(f"POST /team/{teamid}/demo-video called") @@ -108,7 +115,10 @@ def add_demo_video_to_team_api(teamid): body = request.get_json() or {} demo_video_url = body.get("demo_video_url", "") # Allow empty string to clear the field; edit_team normalizes to None - return edit_team({"id": teamid, "demo_video_url": demo_video_url}) + + from services.hackathon_planning_service import is_admin + from api.submissions.submissions_service import self_serve_team_edit + return self_serve_team_edit(auth_user.user_id, teamid, {"demo_video_url": demo_video_url}, admin=is_admin(auth_user)) logger.error("Could not obtain user details for POST /team//demo-video") return {"error": "Unauthorized"}, 401 diff --git a/api/teams/tests/__init__.py b/api/teams/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/api/teams/tests/test_edit_team_project_fields.py b/api/teams/tests/test_edit_team_project_fields.py new file mode 100644 index 0000000..ea37ad4 --- /dev/null +++ b/api/teams/tests/test_edit_team_project_fields.py @@ -0,0 +1,95 @@ +""" +Regression tests for MEDIUM finding #3: PATCH /api/team/edit +(api.teams.teams_service.edit_team) is documented (README "Ownership", +CLAUDE.md, submissions_service.py) as the admin override path for a team's +project_* write-up fields, but edit_team's field_mappings never actually +carried them. Covers: all seven project_* fields now flow through +field_mappings, project_submission_status is validated against the +draft/submitted/late catalog (400 on a bad value, no write), tagline/story +get the same sanitize_markdown treatment as the self-serve save_project +path, and a status CHANGE stamps project_updated_at (no stamp when +unchanged). +""" +import os +from unittest.mock import MagicMock + +os.environ.setdefault("ENVIRONMENT", "test") + +import api.teams.teams_service as svc + + +def _wire(monkeypatch, existing_data): + mock_snapshot = MagicMock() + mock_snapshot.to_dict.return_value = dict(existing_data) if existing_data is not None else None + mock_doc = MagicMock() + mock_doc.get.return_value = mock_snapshot + mock_collection = MagicMock() + mock_collection.document.return_value = mock_doc + mock_db = MagicMock() + mock_db.collection.return_value = mock_collection + monkeypatch.setattr(svc, "get_db", lambda: mock_db) + monkeypatch.setattr(svc, "clear_cache", lambda: None) + monkeypatch.setattr(svc, "send_slack_audit", lambda **kwargs: None) + return mock_doc + + +def test_edit_team_persists_project_submission_status(monkeypatch): + mock_doc = _wire(monkeypatch, {"name": "Team A", "project_submission_status": "draft"}) + + result = svc.edit_team({"id": "team-1", "project_submission_status": "submitted"}) + + assert result["success"] is True + written = mock_doc.set.call_args[0][0] + assert written["project_submission_status"] == "submitted" + assert "project_updated_at" in written + + +def test_edit_team_rejects_invalid_submission_status(monkeypatch): + mock_doc = _wire(monkeypatch, {"name": "Team A"}) + + result, status = svc.edit_team({"id": "team-1", "project_submission_status": "bogus"}) + + assert status == 400 + assert result["success"] is False + mock_doc.set.assert_not_called() + + +def test_edit_team_does_not_stamp_project_updated_at_when_status_unchanged(monkeypatch): + mock_doc = _wire(monkeypatch, {"name": "Team A", "project_submission_status": "submitted"}) + + svc.edit_team({"id": "team-1", "project_submission_status": "submitted"}) + + written = mock_doc.set.call_args[0][0] + assert "project_updated_at" not in written + + +def test_edit_team_sanitizes_project_tagline_and_story(monkeypatch): + mock_doc = _wire(monkeypatch, {"name": "Team A"}) + + svc.edit_team({ + "id": "team-1", + "project_tagline": "Hi ", + "project_story": "We used List for the queue.", + }) + + written = mock_doc.set.call_args[0][0] + assert "" in written["project_story"] + + +def test_edit_team_persists_remaining_project_fields(monkeypatch): + mock_doc = _wire(monkeypatch, {"name": "Team A"}) + + svc.edit_team({ + "id": "team-1", + "project_built_with": ["Python", "React"], + "project_links": [{"label": "Repo", "url": "https://github.com/x/y"}], + "project_thumbnail_url": "https://cdn.ohack.dev/teams/team-1/project/thumb.png", + "project_images": ["https://cdn.ohack.dev/teams/team-1/project/1.png"], + }) + + written = mock_doc.set.call_args[0][0] + assert written["project_built_with"] == ["Python", "React"] + assert written["project_links"] == [{"label": "Repo", "url": "https://github.com/x/y"}] + assert written["project_thumbnail_url"] == "https://cdn.ohack.dev/teams/team-1/project/thumb.png" + assert written["project_images"] == ["https://cdn.ohack.dev/teams/team-1/project/1.png"] diff --git a/api/teams/tests/test_teams_devpost_demo_video_views.py b/api/teams/tests/test_teams_devpost_demo_video_views.py new file mode 100644 index 0000000..2f4457d --- /dev/null +++ b/api/teams/tests/test_teams_devpost_demo_video_views.py @@ -0,0 +1,128 @@ +""" +Regression tests for Part 9 bug #1: POST /api/team//devpost and +POST /api/team//demo-video used to call edit_team directly with NO +membership check, so any logged-in user could overwrite any team's Devpost +link or demo video. They now route through +api.submissions.submissions_service.self_serve_team_edit, which enforces +team membership (or admin) and the submission deadline. These tests check +the view-layer wiring; self_serve_team_edit's own gating logic is covered in +api/submissions/tests/test_submissions_service.py. + +Uses the same propelauth-stub pattern as +api/volunteers/tests/test_volunteers_views.py. +""" +import functools +import importlib +import os +import sys +import types +from unittest.mock import MagicMock + +os.environ.setdefault("ENVIRONMENT", "test") + +import pytest +from flask import Flask, g +from werkzeug.local import LocalProxy + +VIEWS_MODULE = "api.teams.teams_views" +FAKE_USER = types.SimpleNamespace(user_id="caller-propel-uuid", email="caller@example.com") + + +def _passthrough_decorator_factory(*_args, **_kwargs): + def decorator(fn): + @functools.wraps(fn) + def wrapper(*args, **kwargs): + g.propelauth_current_user = FAKE_USER + return fn(*args, **kwargs) + + return wrapper + + return decorator + + +@pytest.fixture +def views(monkeypatch): + stub = types.ModuleType("common.auth") + stub.auth = types.SimpleNamespace( + require_org_member_with_permission=_passthrough_decorator_factory, + require_user=_passthrough_decorator_factory(), + optional_user=_passthrough_decorator_factory(), + ) + stub.auth_user = LocalProxy(lambda: g.propelauth_current_user) + monkeypatch.setitem(sys.modules, "common.auth", stub) + sys.modules.pop(VIEWS_MODULE, None) + module = importlib.import_module(VIEWS_MODULE) + yield module + sys.modules.pop(VIEWS_MODULE, None) + + +@pytest.fixture +def app(views): + flask_app = Flask(__name__) + flask_app.register_blueprint(views.bp) + return flask_app + + +@pytest.fixture +def client(app): + return app.test_client() + + +HEADERS = {"Authorization": "Bearer test"} + + +def test_devpost_route_403s_non_member(client, monkeypatch): + """The actual regression: a non-member must be rejected, not silently + allowed to overwrite the team's Devpost link.""" + monkeypatch.setattr("services.hackathon_planning_service.is_admin", lambda user: False) + monkeypatch.setattr( + "api.submissions.submissions_service._authorize_team_write", + lambda propel, team_id, admin=False, enforce_deadline=True: (({"error": "not_team_member"}, 403), None, None, None), + ) + + res = client.post("/api/team/team-1/devpost", json={"devpost_link": "https://devpost.com/x"}, headers=HEADERS) + + assert res.status_code == 403 + assert res.get_json()["error"] == "not_team_member" + + +def test_devpost_route_allows_member(client, monkeypatch): + monkeypatch.setattr("services.hackathon_planning_service.is_admin", lambda user: False) + monkeypatch.setattr( + "api.submissions.submissions_service.self_serve_team_edit", + MagicMock(return_value={"success": True, "team": {"id": "team-1"}}), + ) + + res = client.post("/api/team/team-1/devpost", json={"devpost_link": "https://devpost.com/x"}, headers=HEADERS) + + assert res.status_code == 200 + assert res.get_json()["success"] is True + + +def test_devpost_route_requires_link(client, monkeypatch): + monkeypatch.setattr("services.hackathon_planning_service.is_admin", lambda user: False) + res = client.post("/api/team/team-1/devpost", json={}, headers=HEADERS) + assert res.status_code == 400 + + +def test_demo_video_route_403s_non_member(client, monkeypatch): + monkeypatch.setattr("services.hackathon_planning_service.is_admin", lambda user: False) + monkeypatch.setattr( + "api.submissions.submissions_service.self_serve_team_edit", + lambda propel, team_id, fields, admin=False: ({"error": "not_team_member"}, 403), + ) + + res = client.post("/api/team/team-1/demo-video", json={"demo_video_url": "https://youtu.be/x"}, headers=HEADERS) + + assert res.status_code == 403 + + +def test_demo_video_route_allows_member_and_passes_admin_flag(client, monkeypatch): + monkeypatch.setattr("services.hackathon_planning_service.is_admin", lambda user: True) + service = MagicMock(return_value={"success": True, "team": {"id": "team-1"}}) + monkeypatch.setattr("api.submissions.submissions_service.self_serve_team_edit", service) + + res = client.post("/api/team/team-1/demo-video", json={"demo_video_url": ""}, headers=HEADERS) + + assert res.status_code == 200 + service.assert_called_once_with(FAKE_USER.user_id, "team-1", {"demo_video_url": ""}, admin=True) diff --git a/api/volunteers/tests/test_volunteers_service.py b/api/volunteers/tests/test_volunteers_service.py index 423234e..d209501 100644 --- a/api/volunteers/tests/test_volunteers_service.py +++ b/api/volunteers/tests/test_volunteers_service.py @@ -671,4 +671,38 @@ def test_generate_qr_code_special_characters(): # Assertions assert qr_image_bytes is not None assert isinstance(qr_image_bytes, bytes) - assert len(qr_image_bytes) > 0 \ No newline at end of file + assert len(qr_image_bytes) > 0 + +# --------------------------------------------------------------------------- +# get_volunteer_self_status β€” generic self-check (type=hacker etc.) +# --------------------------------------------------------------------------- +from services.volunteers_service import get_volunteer_self_status # noqa: E402 + + +@patch('services.volunteers_service.find_volunteer_by_caller_identity') +def test_get_volunteer_self_status_selected_hacker(mock_find): + mock_find.return_value = {"name": "Jamie Hacker", "isSelected": True} + result = get_volunteer_self_status(MOCK_USER_ID, MOCK_EVENT_ID, "hacker") + assert result == {"is_hacker": True, "volunteer": {"name": "Jamie Hacker", "isSelected": True}} + mock_find.assert_called_once_with(MOCK_USER_ID, MOCK_EVENT_ID, "hacker") + + +@patch('services.volunteers_service.find_volunteer_by_caller_identity') +def test_get_volunteer_self_status_not_selected_returns_null_volunteer(mock_find): + mock_find.return_value = {"name": "Jamie Hacker", "isSelected": False} + result = get_volunteer_self_status(MOCK_USER_ID, MOCK_EVENT_ID, "hacker") + assert result == {"is_hacker": False, "volunteer": None} + + +@patch('services.volunteers_service.find_volunteer_by_caller_identity') +def test_get_volunteer_self_status_no_doc_returns_false(mock_find): + mock_find.return_value = None + result = get_volunteer_self_status(MOCK_USER_ID, MOCK_EVENT_ID, "hacker") + assert result == {"is_hacker": False, "volunteer": None} + + +@patch('services.volunteers_service.find_volunteer_by_caller_identity') +def test_get_volunteer_self_status_leaks_no_pii_beyond_name(mock_find): + mock_find.return_value = {"name": "Jamie", "isSelected": True, "email": "jamie@example.com", "phone": "555-1234"} + result = get_volunteer_self_status(MOCK_USER_ID, MOCK_EVENT_ID, "hacker") + assert set(result["volunteer"].keys()) == {"name", "isSelected"} diff --git a/api/volunteers/volunteers_views.py b/api/volunteers/volunteers_views.py index adeeb74..1411e7c 100644 --- a/api/volunteers/volunteers_views.py +++ b/api/volunteers/volunteers_views.py @@ -229,18 +229,24 @@ def get_my_volunteer_status_for_event(event_id): """ Lightweight self-check for the calling user against a single event. Default type is 'mentor' (used by the per-team mentor panel to decide - interactive vs. read-only). - Returns { is_mentor: bool, volunteer: {name, email, isSelected, checkInTime?} | null }. + interactive vs. read-only); type=hacker backs the team dashboard's + "am I an approved hacker for this event" gate (findteam/manageteam, + Hackers' Choice eligibility). + Returns { is_mentor: bool, volunteer: {name, email, isSelected, checkInTime?} | null } + for type=mentor, or { is_hacker: bool, volunteer: {name, isSelected} | null } for + type=hacker. """ user = auth_user if not user or not user.user_id: return _error_response("Authentication required", 401) vtype = request.args.get('type', 'mentor') - if vtype != 'mentor': - # Keep the surface narrow for now; extend later if needed. - return _error_response("Only type=mentor is supported", 400) - from api.mentors.mentors_service import get_mentor_self_status - return get_mentor_self_status(user.user_id, event_id) + if vtype == 'mentor': + from api.mentors.mentors_service import get_mentor_self_status + return get_mentor_self_status(user.user_id, event_id) + if vtype == 'hacker': + from services.volunteers_service import get_volunteer_self_status + return get_volunteer_self_status(user.user_id, event_id, 'hacker'), 200 + return _error_response("Only type=mentor or type=hacker is supported", 400) # Sponsor routes @bp.route('/sponsor/application//submit', methods=['POST']) diff --git a/common/utils/github.py b/common/utils/github.py index 27cca3c..84f1c78 100644 --- a/common/utils/github.py +++ b/common/utils/github.py @@ -1,5 +1,7 @@ import os +from collections import Counter +from datetime import datetime, timedelta, timezone from github import Github from dotenv import load_dotenv from github import GithubException @@ -9,6 +11,10 @@ logger.setLevel(logging.DEBUG) load_dotenv() +# Contributor list is capped to keep the team-dashboard "Code activity" card +# small; it's a snapshot of the most recent commit page, not a full history. +MAX_ACTIVITY_CONTRIBUTORS = 8 + def create_github_repo( repository_name, hackathon_event_id, @@ -192,7 +198,98 @@ def validate_github_username(github_username): -def get_all_repos(org_name): +def get_repo_activity(org_name, repo_name): + """ + A team's "Code activity" card (team dashboard, Sep 2026): last commit, + commits in the last 24h, top contributors, open PR count. Deliberately + exactly 3 GitHub API calls (rate-limit budget matters more here than + completeness β€” this is a live-ish snapshot, not a full history): + 1. g.get_repo(f"{org}/{repo}") + 2. repo.get_commits().get_page(0) (first page only, <=100 commits) + 3. repo.get_pulls(state="open").totalCount + + Contributors are derived from that single commit page (a Counter over + each commit's author), not a separate contributors-stats call. + + Raises UnknownObjectException (repo doesn't exist) and + RateLimitExceededException to the caller β€” api.github.github_service + translates those into 404/503. An empty repository (GitHub returns 409 + for get_commits on one) is NOT an error here β€” it's a valid all-zeros + result for a freshly created team repo. + """ + g = Github(os.getenv('GITHUB_TOKEN'), per_page=100) + repo = g.get_repo(f"{org_name}/{repo_name}") + + try: + commits = list(repo.get_commits().get_page(0)) + except GithubException as e: + if e.status == 409: + commits = [] + else: + raise + + open_prs = repo.get_pulls(state="open").totalCount + + now = datetime.now(timezone.utc) + last_24h = 0 + last_commit_at = None + last_commit_message = None + last_author = None + contributor_counts = Counter() + contributor_avatars = {} + + for i, commit in enumerate(commits): + git_commit = getattr(commit, "commit", None) + git_author = getattr(git_commit, "author", None) if git_commit else None + commit_date = getattr(git_author, "date", None) if git_author else None + if commit_date and commit_date.tzinfo is None: + commit_date = commit_date.replace(tzinfo=timezone.utc) + + if i == 0: + last_commit_at = commit_date + last_commit_message = getattr(git_commit, "message", None) if git_commit else None + last_author = ( + (commit.author.login if commit.author else None) + or (getattr(git_author, "name", None) if git_author else None) + ) + + if commit_date and (now - commit_date) <= timedelta(hours=24): + last_24h += 1 + + login = commit.author.login if commit.author else None + name = login or (getattr(git_author, "name", None) if git_author else None) or "Unknown" + avatar = commit.author.avatar_url if commit.author else None + contributor_counts[name] += 1 + if avatar: + contributor_avatars[name] = avatar + + contributors = [ + {"login": name, "avatar_url": contributor_avatars.get(name), "contributions": count} + for name, count in contributor_counts.most_common(MAX_ACTIVITY_CONTRIBUTORS) + ] + + return { + "success": True, + "repo": { + "html_url": repo.html_url, + "default_branch": repo.default_branch, + "pushed_at": repo.pushed_at.isoformat() if getattr(repo, "pushed_at", None) else None, + "open_issues_count": repo.open_issues_count, + "stargazers_count": repo.stargazers_count, + }, + "commits": { + "total_recent": len(commits), + "last_24h": last_24h, + "last_commit_at": last_commit_at.isoformat() if last_commit_at else None, + "last_commit_message": last_commit_message, + "last_author": last_author, + }, + "contributors": contributors, + "open_prs": open_prs, + } + + +def get_all_repos(org_name): g = Github(os.getenv('GITHUB_TOKEN')) org = g.get_organization(org_name) repos = org.get_repos() diff --git a/common/utils/validators.py b/common/utils/validators.py index 6e4d08c..7aa6950 100644 --- a/common/utils/validators.py +++ b/common/utils/validators.py @@ -1,4 +1,5 @@ import re +import unicodedata from copy import deepcopy from urllib.parse import urlparse import logging @@ -15,6 +16,16 @@ "judge_judging_end_time", ) +# Hackathon `deadlines` object (Sep 2026 β€” team dashboard / Hackers' Choice). +# Unknown keys are a validation error (see validate_deadlines) rather than a +# silent skip, so a mistyped key doesn't quietly leave a deadline unset. +DEADLINE_KEYS = {"submission", "late_submission_until", "voting_opens", "voting_closes"} + +# constraints.peer_vote_slate_size / peer_vote_max_picks bounds. Kept in sync +# with DEFAULT_SLATE_SIZE/DEFAULT_MAX_PICKS in api/peer_votes/peer_votes_service.py. +PEER_VOTE_SLATE_SIZE_RANGE = (3, 10) +PEER_VOTE_MAX_PICKS_RANGE = (1, 5) + # 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) @@ -89,6 +100,153 @@ def sanitize_string(input_string, max_length=None): return sanitized +def validate_https_url(url, max_length=2048): + """Return True if `url` is an https:// URL no longer than max_length. + + Used for team project_links[].url and (indirectly, via CDN prefix checks + elsewhere) project image URLs. Deliberately stricter than validate_url, + which also allows http. + """ + if not isinstance(url, str) or not url: + return False + if len(url) > max_length: + return False + try: + parsed = urlparse(url) + except ValueError: + return False + return parsed.scheme == "https" and bool(parsed.netloc) + + +# Tags stripped wholesale (open and close) by sanitize_markdown. Generic `<` +# usage (e.g. "List" in a project story) is intentionally preserved β€” +# only these specific tag names are removed. +_MARKDOWN_STRIP_TAG_NAMES = r"script|iframe|object|embed|style|link|meta|form|base" +_MARKDOWN_TAG_RE = re.compile(rf"]*>", re.IGNORECASE) +# `[\s/]` (not just `\s`) so an attribute glued directly to a self-closing +# slash β€” ``, no space before "onerror" β€” still matches. +_MARKDOWN_ON_ATTR_RE = re.compile(r"""[\s/]on\w+\s*=\s*(?:"[^"]*"|'[^']*'|[^\s>]+)""", re.IGNORECASE) +# Quoted OR unquoted attribute values: `href="javascript:..."` and the +# unquoted `href=javascript:...` (no quotes at all) both get neutralized. +_MARKDOWN_DANGEROUS_HREF_RE = re.compile( + r"""(href|src)\s*=\s*(?:(["'])\s*(?:javascript|vbscript|data):[^"']*\2|(?:javascript|vbscript|data):[^\s>]*)""", + re.IGNORECASE, +) +# Markdown link/image syntax `[text](javascript:...)` / `![alt](data:...)` β€” +# not an HTML attribute, so _MARKDOWN_DANGEROUS_HREF_RE never sees it. +_MARKDOWN_DANGEROUS_MD_LINK_RE = re.compile( + r"""\]\(\s*(?:javascript|vbscript|data):[^)]*\)""", re.IGNORECASE +) +_CONTROL_CHARS_RE = re.compile(r"[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]") + + +def sanitize_markdown(text, max_length): + """Defence-in-depth sanitizer for user-authored markdown (project tagline/story). + + The frontend renders this via react-markdown WITHOUT rehype-raw, so raw + HTML is already inert on read. This still strips a denylist of dangerous + tags/attributes server-side in case the content is ever rendered + elsewhere: script|iframe|object|embed|style|link|meta|form|base tags + (open and close), `on*=` attributes (whitespace- or slash-preceded), + javascript:/vbscript:/data: link or image targets in HTML attributes + (quoted or unquoted) rewritten to "#", and the same targets in markdown + link/image syntax (`[text](javascript:...)`) rewritten to `](#)`. + Generic `<` (e.g. "List", "Map") is preserved. The tag-strip + pass is looped to a fixpoint so a nested bypass like + "ipt>" β€” where stripping the inner " world", 1000) + assert "" not in cleaned + assert "Hello" in cleaned and "world" in cleaned + + +def test_sanitize_markdown_strips_onerror_attribute(): + cleaned = sanitize_markdown('', 1000) + assert "onerror" not in cleaned + + +def test_sanitize_markdown_neutralizes_javascript_href(): + cleaned = sanitize_markdown('click', 1000) + assert "javascript:" not in cleaned + + +def test_sanitize_markdown_preserves_generic_angle_brackets(): + cleaned = sanitize_markdown("We used List and Map internally.", 1000) + assert "List" in cleaned + assert "Map" in cleaned + + +def test_sanitize_markdown_loops_nested_tag_bypass_to_fixpoint(): + """A single non-looped strip pass on "ipt>" removes only the + inner "