Uh oh!
There was an error while loading. Please reload this page.
fix(gradescope): staging serves the whole feature at a doubled prefix; restore the working route - #565
Conversation
…t a doubled prefix `82787ca` replaced the finished Gradescope integration with a `TODO(wire-me)` scaffold and left its tests behind. Nobody saw it because `ruff` fails before `pytest` in the backend job, so the suite has not run on staging since 2026-07-10. It is broken three independent ways: 1. The router declares `APIRouter(prefix="/api/gradescope")` and main.py includes it with the same prefix, so every endpoint is mounted at `/api/gradescope/api/gradescope/...`. The whole feature 404s on staging. 2. The scaffold calls `table(...).select(...).eq(...).execute()`, but this repo's `db.connection.table()` is PostgREST-style — `select(columns, filters=...)`, `update(data, filters)`, `delete(filters)`. There is no `.eq()` and no `.execute()`, so every handler would AttributeError even if it were reachable. 3. No rate limiting and no `/credentials/bu-sso` endpoint, both of which tests/test_gradescope.py requires. The tests were right; the route was wrong. Restores routes/gradescope.py and services/gradescope_service.py from main — the version that matches this branch's own test file AND its db/connection.py, and that production runs. models/gradescope.py and services/gradescope_sync_service.py are left in place. Also clears the two lint errors 82787ca left, which are what gate the suite: the unused pydantic.Field import, and GradescopeConnect's exhaustive-deps error (useCallback keyed on userId, which additionally fixes switching user without unmounting keeping the previous account's state).
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reached
Next review available in:14 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Uh oh!
There was an error while loading. Please reload this page.
The
Backend (pytest)job has not actually run onstagingsince 2026-07-10ruffruns beforepytestin the backend job, and82787caleft an unused-import error behind — so the suite has been skipped, not passing, ever since. Turning the lint step green (see below) makes it run again, and it comes back red on 5 tests intests/test_gradescope.py.What
82787caactually shippedThat commit replaced the finished Gradescope integration with a
TODO(wire-me)scaffold (649 lines ofroutes/gradescope.py, mostly deletions; 413 more fromservices/gradescope_service.py) and did not updatetests/test_gradescope.py. The scaffold is broken three independent ways:1. Every endpoint is unreachable. The router declares its own prefix and
main.pyadds the same one:Dumping the live route table confirms it:
The whole feature 404s on staging today. Some tests "passed" only because they assert
404.2. It calls a database client that does not exist. The scaffold uses supabase-py chaining:
but this repo's
db/connection.pyis PostgREST-style —select(columns, filters=...),update(data, filters),delete(filters),upsert(data, on_conflict). There is no.eq()and no.execute(), so every handler wouldAttributeErroreven if it were reachable.3. No rate limiting and no
/credentials/bu-sso, both of whichtests/test_gradescope.pyrequires (10/5min on courses and sync, 3/10min on bu-sso, per-user).The fix
The tests were right; the route was wrong. This restores
routes/gradescope.pyandservices/gradescope_service.pyfrommain— the version that matches this branch's own test file, matches this branch'sdb/connection.py, and is what production runs.models/gradescope.pyandservices/gradescope_sync_service.pyfrom82787caare left in place.Also clears the two lint errors that gate the suite:
models/gradescope.py— unusedpydantic.Fieldimport.settings/GradescopeConnect.tsx—react-hooks/exhaustive-depserror, fixed withuseCallbackkeyed onuserIdrather than a suppression. That also fixes a real bug the empty dep array hid: switching user without unmounting kept the previous account's Gradescope state.Verification
ruff check .→ clean (was 1 error)npx eslinton the component → clean (was 1 error);npx tsc --noEmitcleantests/test_gradescope.py: 25 passed (was 5 failed / 20 passed)/api/gradescope/...prefix, includingcredentials/bu-ssoContext
stagingis 239 commits behindmainand carries exactly one commitmaindoes not —82787ca, the one above. Worth deciding separately whether staging should simply be brought up tomainrather than maintained as a fork; this PR only repairs the breakage. It also unblocks #535, whose backend job fails on these same 5 tests.