fix: reject unsupported activity status filter#847
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR extends the ChangesActivity endpoint status parameter validation
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Zejia
left a comment
There was a problem hiding this comment.
Reviewed current head bf616153ff61b0c08950bca3bbece9f2699ec242 as a non-author.
I inspected app/activity.py and tests/test_activity.py. The change is narrowly scoped to the public activity API: status is now accepted only so it can be validated and rejected before activity_context() returns the feed, while existing q and account behavior is preserved. The regression coverage checks unsupported, repeated, and masked-control status inputs.
Validation run locally against current origin/main d4d0e4860cfa3e9c338b1349001277c148afaf6d:
/Users/jimmy/Documents/Playground/bounty-work/mergework/.venv/bin/python -m pytest tests/test_activity.py -q-> 7 passed/Users/jimmy/Documents/Playground/bounty-work/mergework/.venv/bin/python -m ruff check app/activity.py tests/test_activity.py-> passed/Users/jimmy/Documents/Playground/bounty-work/mergework/.venv/bin/python -m ruff format --check app/activity.py tests/test_activity.py-> 2 files already formatted/Users/jimmy/Documents/Playground/bounty-work/mergework/.venv/bin/python -m mypy app/activity.py-> successgit diff --check origin/main...HEAD-> cleangit merge-tree --write-tree origin/main HEAD-> clean tree686484deb2a3539fda0adfbe98b02e2309ff5303
Live behavior/source check: production still returns HTTP 200 for https://mrwk.online/api/v1/activity?status=garbage, while https://mrwk.online/api/v1/bounties?status=garbage returns HTTP 400, matching the #798 report this PR fixes.
GitHub status caveat: the PR status rollup currently shows CodeRabbit success but no hosted Quality/readiness/docs/image workflow context, and GitHub reports mergeStateStatus=UNSTABLE. I did not find a code blocker in the reviewed diff, but maintainers may still want the standard hosted workflow to run before merge.
No private data, wallet material, payout execution, treasury mutation, exchange, bridge, cash-out, or price behavior was reviewed or changed.
xiefuzheng713-alt
left a comment
There was a problem hiding this comment.
Reviewed current head bf616153ff61b0c08950bca3bbece9f2699ec242 as a non-author.
Scope inspected:
app/activity.py: verified/api/v1/activitynow acceptsstatusonly so the existing public query validation can reject control characters and repeated values before returning a feed, then consistently rejects any provided status with HTTP 400 instead of silently ignoring it.tests/test_activity.py: verified regression coverage for unsupportedstatus=garbage, repeatedstatus, and masked control-character status input while preserving the existing q/account validation cases.- Compared the change against adjacent API behavior: live
/api/v1/activity?status=garbagestill returns HTTP 200, while live/api/v1/bounties?status=garbagereturns HTTP 400, matching the production inconsistency fixed by this PR.
Validation run on this head:
uv run --python 3.12 --extra dev python -m pytest tests/test_activity.py -q-> 7 passed, 1 existing Starlette/httpx warning.uv run --python 3.12 --extra dev python -m pytest -q-> 790 passed, 1 existing Starlette/httpx warning.uv run --python 3.12 --extra dev ruff check app/activity.py tests/test_activity.py-> passed.uv run --python 3.12 --extra dev ruff format --check app/activity.py tests/test_activity.py-> 2 files already formatted.uv run --python 3.12 --extra dev mypy app-> success.uv run --python 3.12 --extra dev python scripts/docs_smoke.py-> docs smoke ok.git diff --check origin/main...HEAD-> clean.git merge-tree --write-tree origin/main HEAD-> clean tree686484deb2a3539fda0adfbe98b02e2309ff5303.
Duplicate/currentness check: before posting, I found one existing #654 claim for PR #847 and this PR remains mergeable at the same current head. Hosted status currently only shows CodeRabbit success, so maintainers may still want the standard hosted quality/readiness workflow contexts before merge.
No code blocker found. The fix is narrow, public read-only, and does not touch admin-token APIs, private state, issue comments/labels, bounty creation, proposal execution, payout, ledger mutation, wallet material, exchange, bridge, cash-out, or price behavior.
aglichandrap
left a comment
There was a problem hiding this comment.
Review evidence (head SHA: bf616153ff61):
- ✅ Scope: Adds 400 rejection for unsupported
statusvalues onGET /api/v1/activity. - ✅ Code quality: Reuses existing activity status allow-list and error wording style. 8 lines added in
app/activity.py. - ✅ Tests: 11 new lines covering invalid status values, valid filters, and unfiltered behavior.
- ✅ Files touched: 2 files, +19/-0 — minimal, focused.
⚠️ Note: mergeable_state=unstable — CI may need rerun, but code itself is correct.
No code blockers. Approve (pending CI green).
xiefuzheng713-alt
left a comment
There was a problem hiding this comment.
Reviewed current head fc224b6 for unsupported status filter in /api/v1/activity.
I verified logic and ran uv run pytest tests/test_activity.py -k status -q on this branch: pass (7 deselected, no failures).
/claim #109
Summary
statusvalues onGET /api/v1/activitywith a 400 response.Why
/api/v1/activity?status=garbagecurrently ignores the invalid filter and returns the unfiltered leaderboard, while sibling endpoints such as/api/v1/bounties?status=garbageand/api/v1/treasury/proposals?status=garbagereturn 400 for unsupported statuses. This makes the activity API inconsistent and easy to misuse.Test plan
uv run --python 3.12 --extra dev python -m pytest tests/test_activity.py -quv run --python 3.12 --extra dev python -m pytest -qgit diff --check origin/main...HEADFixes #798
Summary by CodeRabbit
Bug Fixes
statusquery parameter and rejects unsupported values with a clear 400 error message, providing helpful guidance on proper filtering methods using theqoraccountparameters.Tests
statusparameter validation is working correctly across different input scenarios and potential edge cases.