Create GET /users/{user_id}/activity endpoint and regenerate OpenAPI spec

[pmcfadin]

Epic: #12 — Implement user_activity Table Support
Size: M
Depends on: #13

Description

Create the read endpoint for user activity timelines and update the OpenAPI specification.

Files to create/modify

• app/api/v1/endpoints/user_activity.py (new) — GET /users/{user_id}/activity with PaginatedResponse, optional activity_type query param
• app/main.py — register new router
• docs/killrvideo_openapi.yaml — regenerate via python scripts/generate_openapi.py
• tests/api/v1/endpoints/test_user_activity.py (new) — API-level tests

Key design

• Public endpoint (no auth required) — consistent with existing GET /users/{user_id}/comments
• Uses PaginationParams dependency, optional activity_type Query param
• Returns empty data: [] for unknown users (not 404)
• Response items: userId, activityType, activityId, activityTimestamp

Acceptance Criteria

• GET /api/v1/users/{user_id}/activity returns 200 with PaginatedResponse
• Supports page, pageSize, activity_type query params
• Returns empty data for users with no activity
• Router registered in app/main.py
• docs/killrvideo_openapi.yaml regenerated and includes the new endpoint path, query params, and UserActivityResponse schema
• API tests cover happy path, empty result, type filter, pagination

[pmcfadin]

Code Review: Issue #17 — Create GET /users/{user_id}/activity endpoint and regenerate OpenAPI spec

Summary

The endpoint is reachable at the correct path, returns PaginatedResponse, supports query params, returns empty data for unknown users, and the router is registered. All four API-level tests pass. However, there is a hard failure on an explicit acceptance criterion and missing test coverage.

---

Findings

🔴 Critical

1. OpenAPI spec UserActivityResponse schema documents WRONG field names

At lines 1820–1844 of docs/killrvideo_openapi.yaml:

UserActivityResponse:
  properties:
    userid:              # WRONG — should be userId
    activity_type:       # WRONG — should be activityType
    activity_id:         # WRONG — should be activityId
    activity_timestamp:  # WRONG — should be activityTimestamp

The model definition uses camelCase field names with snake_case aliases. FastAPI serializes by field name, so the runtime response uses camelCase. The spec was generated with aliases instead of field names. Any API client code-generated from the YAML will use the wrong property names.

2. No test for invalid UUID in path

GET /users/not-a-uuid/activity should return 422. While FastAPI handles this automatically, the issue requires input validation testing and this case is absent.

---

🟡 Warning

3. totalPages calculation is inconsistent with rest of codebase when total == 0

total_pages = max(1, math.ceil(total / pagination.pageSize))  # Returns 1 when total=0

The helper used everywhere else returns totalPages = 0 when total == 0. Inconsistent pagination behavior will surprise API consumers.

4. activity_type query param accepts arbitrary strings with no enum validation

Passing activity_type=GARBAGE returns empty result silently. No Enum type or pattern constraint in the OpenAPI spec.

5. Test for activity_type filter hedges on field name

activity_type_val = item.get("activityType") or item.get("activity_type")

Accepting either key name masks the schema mismatch. Should assert a specific key.

---

🔵 Nit

• The 30-day window is hardcoded and undocumented in the API response
• day field on UserActivity not exposed in response (intentional but undocumented)

---

Verdict

REQUEST CHANGES

1. Fix docs/killrvideo_openapi.yaml — UserActivityResponse schema must use userId, activityType, activityId, activityTimestamp
2. Add test for invalid UUID — GET /api/v1/users/not-a-uuid/activity must return 422
3. Fix totalPages formula — use (total + pageSize - 1) // pageSize for consistency

---

🤖 Reviewed by Claude Code Review Agent

[pmcfadin]

Review Fixes Applied

All review comments have been addressed:

Critical fixes:

1. Fixed OpenAPI spec UserActivityResponse schema — Changed alias= to validation_alias= in the model. The spec now correctly shows camelCase field names: userId, activityType, activityId, activityTimestamp. model_validate() from snake_case DB objects still works via validation_alias.
2. Added test for invalid UUID path — New test_get_user_activity_invalid_uuid verifies 422 response. Route path changed from {user_id_path:uuid} to {user_id_path} so FastAPI's type validation returns 422 instead of 404.

Warning fixes:

3. Fixed totalPages formula — Changed from max(1, math.ceil(total / pageSize)) to (total + pageSize - 1) // pageSize. Returns 0 when total is 0, consistent with rest of codebase.
4. Added activity_type enum validation — Query param type changed to Literal["view", "comment", "rate"]. Invalid values now return 422.
5. Fixed test to assert specific key — test_get_user_activity_with_type_filter now asserts item["activityType"] directly instead of hedging with .get() fallback.

OpenAPI spec regenerated:

docs/killrvideo_openapi.yaml updated with camelCase schema, Literal enum for activity_type, and corrected route path.

Test results:

All 5 API tests pass ✅

---

🤖 Applied by Claude Code

[pmcfadin]

Follow-Up Review: Unresolved Items

The critical fixes (OpenAPI schema, invalid UUID test, totalPages formula, enum validation, test field name assertion) were all properly addressed. Two small gaps remain:

1. No test asserting totalPages == 0 when results are empty

The totalPages formula was fixed to return 0 when total == 0, but test_get_user_activity_empty only asserts totalItems == 0 — it does not verify totalPages == 0. The fix is untested. Add:

assert body["pagination"]["totalPages"] == 0

2. No test for invalid activity_type returning 422

The Literal["view", "comment", "rate"] enum validation was added (good), but there is no test proving that ?activity_type=GARBAGE returns 422. The validation works via FastAPI, but a regression test should confirm it. Add:

response = client.get(f"/api/v1/users/{user_id}/activity?activity_type=invalid")
assert response.status_code == 422

Both are quick additions that lock in the fixes you already made.

---

Re-review by Claude Code

[pmcfadin]

Review Feedback Applied

Two fixes were made to tests/api/v1/endpoints/test_user_activity.py in response to review comments:

1. test_get_user_activity_empty — added totalPages == 0 assertion

When the service returns ([], 0) the endpoint computes totalPages as (0 + pageSize - 1) // pageSize, which evaluates to 0 for any positive pageSize. The test now explicitly asserts this:

assert body["pagination"]["totalPages"] == 0

2. New test: test_get_user_activity_invalid_type_returns_422

The endpoint declares activity_type as Optional[Literal["view", "comment", "rate"]], so FastAPI's built-in validation rejects any value outside that set with a 422 response. The new test verifies this contract:

@pytest.mark.asyncio
async def test_get_user_activity_invalid_type_returns_422():
    """An unrecognised activity_type value must produce a 422 validation error."""
    async with AsyncClient(app=app, base_url="http://test") as ac:
        resp = await ac.get(
            f"{settings.API_V1_STR}/users/{uuid4()}/activity?activity_type=invalid"
        )
    assert resp.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY

Test results

tests/api/v1/endpoints/test_user_activity.py::test_get_user_activity_success PASSED
tests/api/v1/endpoints/test_user_activity.py::test_get_user_activity_empty PASSED
tests/api/v1/endpoints/test_user_activity.py::test_get_user_activity_with_type_filter PASSED
tests/api/v1/endpoints/test_user_activity.py::test_get_user_activity_invalid_uuid PASSED
tests/api/v1/endpoints/test_user_activity.py::test_get_user_activity_pagination PASSED
tests/api/v1/endpoints/test_user_activity.py::test_get_user_activity_invalid_type_returns_422 PASSED

6 passed in 6.58s