Honor activity feed limit#752
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)
📝 WalkthroughWalkthroughActivity endpoints accept a bounded integer ChangesActivity Feed Limit Parameter
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Pushed follow-up commit Updated local validation:
|
|
Note for maintainers: after opening this I noticed #751 predates it and covers the same live #656 activity-limit report. Please treat this PR as an alternate/supplement rather than the first implementation. The extra pieces here are: documenting the |
|
Review evidence for MergeWork review bounty #654. Reviewed current head What I checked:
Overlap check: this PR is an alternate/supplement to #751 for the same live activity-limit report. Since #751 predates this one, the maintainer decision looks like either (a) merge #751 as primary and cherry-pick the extra response-field/docs/link-preservation coverage from this PR, or (b) close this PR as duplicate if #751 already covers the desired acceptance surface. No private data, credentials, wallet material, payout execution, ledger mutation, exchange, bridge, cash-out, price claims, or fabricated payout claims used. |
Jorel97
left a comment
There was a problem hiding this comment.
Reviewed current head a145a9a4d872a989d12472c0974b78dfe34f41fd as a non-author.
No blocking issue found. This PR is technically sound as an alternate/supplement to #751: it validates limit on both /api/v1/activity and /activity, applies the cap consistently to contributors, pending_payouts, and recent, preserves full-match totals, keeps the HTML JSON link aligned with non-default limits, and documents the response shape. Maintainers should still choose how to handle the overlap with #751 because #751 predates this PR and covers the same live #656 activity-limit report; from a code-quality perspective this branch is merge-ready if that overlap is acceptable.
Evidence checked:
- Inspected
app/activity.py,app/serializers.py,app/templates/activity.html,docs/api-examples.md, and the focused tests. - Confirmed the FastAPI
limitparameter is bounded to 1..200 with default 100 on both API and page routes. - Confirmed
activity_to_dict()preserves full totals while slicing only returned row arrays. - Confirmed the activity page JSON link includes the active
qand non-defaultlimitparameters. - Confirmed the docs describe the default/range and clarify that totals remain full-match totals.
- Confirmed GitHub reports
mergeStateStatus: CLEAN, hosted Quality/readiness/docs/image is successful, and CodeRabbit is successful.
Validation run locally on this head:
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest -q-> 591 passed, 1 existing Starlette/httpx warning.- Focused pytest for activity/docs/serializer coverage -> 8 passed, 1 warning.
ruff check app/activity.py app/serializers.py tests/test_activity.py tests/test_activity_routes.py tests/test_docs_public_urls.py tests/test_serializers.py-> passed.ruff format --check app/activity.py app/serializers.py tests/test_activity.py tests/test_activity_routes.py tests/test_docs_public_urls.py tests/test_serializers.py-> 6 files already formatted.python -m mypy app/activity.py app/serializers.py-> success.python scripts/docs_smoke.py-> docs smoke ok.git diff --check origin/main...HEAD-> clean.git merge-tree --write-tree origin/main HEAD-> clean tree0ba46788307584519ef0c3a817b3be2930680a97.
No private data, credentials, wallet material, production mutation, payout/proof/ledger execution, exchange, bridge, cash-out, price behavior, or fabricated payout claims were used or changed.
Summary
limiton/api/v1/activityand/activityas 1..200 with default 100contributors,pending_payouts, andrecentwhile preserving full-match totalsWhy
A live report in #656 showed
/api/v1/activity?limit=1,limit=2, and invalidlimit=notanintreturning 200 with the full contributor list. This makes API polling unexpectedly unbounded and hides invalid client input.The older closed #495 only capped
recentand was closed unmerged after a previous bounty pool was exhausted; this PR addresses the currently reported behavior, including contributor and pending payout arrays.Refs #655. Related report: #656.
Validation
.venv/bin/python -m pytest tests/test_activity.py tests/test_docs_public_urls.py::test_api_examples_document_activity_response_shape -q-> 6 passed, 1 warning.venv/bin/ruff check app/activity.py app/serializers.py tests/test_activity.py tests/test_docs_public_urls.py-> passed.venv/bin/ruff format --check app/activity.py app/serializers.py tests/test_activity.py tests/test_docs_public_urls.py-> passed.venv/bin/mypy app/activity.py app/serializers.py-> passed.venv/bin/python scripts/docs_smoke.py-> passedgit diff --check-> passedgit merge-tree --write-tree origin/main HEAD-> no merge conflictSummary by CodeRabbit