Skip to content

Honor activity feed limit#752

Closed
attaboy11 wants to merge 2 commits into
ramimbo:mainfrom
attaboy11:attaboy11/activity-limit-validation
Closed

Honor activity feed limit#752
attaboy11 wants to merge 2 commits into
ramimbo:mainfrom
attaboy11:attaboy11/activity-limit-validation

Conversation

@attaboy11
Copy link
Copy Markdown
Contributor

@attaboy11 attaboy11 commented Jun 1, 2026

Summary

  • validate limit on /api/v1/activity and /activity as 1..200 with default 100
  • apply the limit to contributors, pending_payouts, and recent while preserving full-match totals
  • keep the activity page JSON link aligned with the current search and non-default limit
  • document the limit parameter and response field

Why

A live report in #656 showed /api/v1/activity?limit=1, limit=2, and invalid limit=notanint returning 200 with the full contributor list. This makes API polling unexpectedly unbounded and hides invalid client input.

The older closed #495 only capped recent and 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 -> passed
  • git diff --check -> passed
  • git merge-tree --write-tree origin/main HEAD -> no merge conflict

Summary by CodeRabbit

  • New Features
    • Activity endpoints (API and HTML) accept a limit query parameter to cap returned rows; defaults to 100 and is capped at 200. HTML links now reflect the current query and limit.
  • Documentation
    • API docs updated with examples and notes on default, valid range, and how totals vs. returned rows behave.
  • Tests
    • Added/expanded tests to cover limit validation, payload truncation, and link rendering.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 1, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 76d4511b-6a8c-4e49-8eec-1a501fd1960e

📥 Commits

Reviewing files that changed from the base of the PR and between f462f15 and a145a9a.

📒 Files selected for processing (2)
  • tests/test_activity_routes.py
  • tests/test_serializers.py

📝 Walkthrough

Walkthrough

Activity endpoints accept a bounded integer limit (default 100, max 200) that caps returned activity arrays; serializers slice result lists by limit, route handlers validate/forward it, the HTML template uses a constructed API URL including limit, and docs/tests updated accordingly.

Changes

Activity Feed Limit Parameter

Layer / File(s) Summary
Serialization with limit constraint
app/serializers.py
activity_to_dict accepts a keyword-only limit parameter and applies it to slice contributors, pending_payouts, and recent arrays instead of hard-coded bounds.
Route handlers and limit validation
app/activity.py
Add ACTIVITY_LIMIT_DEFAULT and ACTIVITY_LIMIT_MAX constants. Introduce _activity_api_url helper to construct /api/v1/activity URLs with optional q and limit. Update activity_context to accept and forward limit. Both JSON (/api/v1/activity) and HTML (/activity) route handlers validate limit as a bounded integer and pass it through; HTML handler sets context["api_results_url"] for template.
Template link update
app/templates/activity.html
The "View JSON activity" link now uses the api_results_url context variable, reflecting the current q and limit parameters instead of constructing the URL inline.
Documentation example update
docs/api-examples.md
API example updated to include limit=25 and text documenting default, allowed range, and how limit caps returned row arrays while totals remain complete.
Activity endpoint tests
tests/test_activity.py
Tests added/extended to exercise valid limit values, invalid/non-integer and out-of-range rejects, limit+q behavior, truncated arrays in JSON responses, and HTML rendering including limit in the JSON link.
Supporting tests adjustments
tests/test_docs_public_urls.py, tests/test_activity_routes.py, tests/test_serializers.py
Update assertions to expect limit present in example URLs, parsed docs, empty-feed activity_context, and serializer outputs (default 100 or example 25).
  • Possibly related PRs:
    • ramimbo/mergework#611: Also implements and validates a limit parameter for list-style endpoints and adds tests for truncation and bounds.
🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Honor activity feed limit' directly reflects the main change: adding and enforcing limit validation on activity endpoints.
Description check ✅ Passed The description follows the template with all major sections completed: Summary, Evidence (Why), and Validation (Test Evidence) with passing test runs and tool checks documented.
Linked Issues check ✅ Passed Changes fully implement the #495 objectives: limit parameter validation (1–200, default 100), capping contributors/pending_payouts/recent arrays while preserving totals, JSON link alignment, and test/docs coverage.
Out of Scope Changes check ✅ Passed All changes are directly scoped to limit parameter validation, application, and documentation; no unrelated modifications detected.
Mergework Public Artifact Hygiene ✅ Passed PR contains only technical pagination changes with accurate MRWK description; no investment, price, off-ramp, fabricated payout, or security claims detected in docs or PR text.
Bounty Pr Focus ✅ Passed PR adds limit parameter to activity endpoints with validation (1–200, default 100), applies to response arrays while preserving totals, includes docs and tests with no scope creep.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@attaboy11
Copy link
Copy Markdown
Contributor Author

Pushed follow-up commit a145a9a after the hosted full-suite run caught two exact-payload tests that needed the new limit: 100 field.

Updated local validation:

  • .venv/bin/python -m pytest -q -> 591 passed, 1 warning
  • targeted activity/docs serializer tests -> 15 passed, 1 warning
  • targeted Ruff check/format, mypy app/activity.py app/serializers.py, docs smoke, and git diff --check passed.

@attaboy11
Copy link
Copy Markdown
Contributor Author

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 limit parameter/response field, returning limit in the activity payload, preserving non-default limit in the activity page JSON link, and covering the default payload shape in the full test suite. Hosted CI and CodeRabbit are green on a145a9a. Happy for this to be closed as duplicate if #751 is the preferred path.

@MyTH-zyxeon
Copy link
Copy Markdown

Review evidence for MergeWork review bounty #654.

Reviewed current head a145a9a4d872a989d12472c0974b78dfe34f41fd.

What I checked:

  • Files touched: app/activity.py, app/serializers.py, app/templates/activity.html, docs/api-examples.md, tests/test_activity.py, tests/test_activity_routes.py, tests/test_docs_public_urls.py, tests/test_serializers.py.
  • GitHub reports mergeStateStatus: CLEAN.
  • Hosted checks on the reviewed head are green: Quality, readiness, docs, and image checks = SUCCESS, CodeRabbit = SUCCESS.
  • The implementation adds bounded limit validation (1..200, default 100) to both JSON and HTML activity routes, returns limit in the serialized activity payload, slices contributors, pending_payouts, and recent, and keeps totals full-set.
  • The activity page now reuses api_results_url, which preserves the non-default limit in the "View JSON activity" link.
  • Tests/docs cover valid limit, q + limit, non-integer/too-low/too-high rejection, default payload shape, and docs examples.

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.

Copy link
Copy Markdown
Contributor

@Jorel97 Jorel97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 limit parameter 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 q and non-default limit parameters.
  • 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 tree 0ba46788307584519ef0c3a817b3be2930680a97.

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.

@ramimbo
Copy link
Copy Markdown
Owner

ramimbo commented Jun 1, 2026

Thanks. I am closing this as the later overlapping activity-limit implementation: PR #751 predates it and covers the same live activity limit report. The #655 small-fix round is now filled/paid, so this PR should not be treated as an available #655 claim.

@ramimbo ramimbo closed this Jun 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants