From 6630e71809f16617c3c62223be3de278b7df2a26 Mon Sep 17 00:00:00 2001 From: chihlasm Date: Mon, 16 Feb 2026 00:25:17 -0500 Subject: [PATCH] feat: add step thumbs feedback and /ratings alias routes Co-Authored-By: Claude Opus 4.6 --- backend/app/api/endpoints/ratings.py | 71 ++++++++++++++- backend/app/api/endpoints/steps.py | 36 ++++++++ backend/app/models/step_library.py | 2 +- backend/tests/test_ratings.py | 126 +++++++++++++++++++++++++++ 4 files changed, 232 insertions(+), 3 deletions(-) diff --git a/backend/app/api/endpoints/ratings.py b/backend/app/api/endpoints/ratings.py index e192b637..23f07054 100644 --- a/backend/app/api/endpoints/ratings.py +++ b/backend/app/api/endpoints/ratings.py @@ -1,12 +1,13 @@ from uuid import UUID from fastapi import APIRouter, Depends, HTTPException, status -from sqlalchemy import select +from sqlalchemy import select, func from sqlalchemy.ext.asyncio import AsyncSession from app.core.database import get_db from app.api.deps import get_current_active_user from app.models import User, Session, SessionRating -from app.schemas.analytics import SessionRatingCreate, SessionRatingResponse +from app.models.step_library import StepLibrary, StepRating +from app.schemas.analytics import SessionRatingCreate, SessionRatingResponse, StepFeedbackCreate router = APIRouter(tags=["ratings"]) @@ -55,3 +56,69 @@ async def rate_session( comment=rating.comment, created_at=rating.created_at, ) + + +@router.post("/steps/{step_id}/feedback", status_code=status.HTTP_201_CREATED) +async def submit_step_feedback( + step_id: UUID, + data: StepFeedbackCreate, + db: AsyncSession = Depends(get_db), + current_user: User = Depends(get_current_active_user), +): + """Submit thumbs up/down feedback for a step used in a session.""" + # Verify step exists + result = await db.execute(select(StepLibrary).where(StepLibrary.id == step_id)) + step = result.scalar_one_or_none() + if not step: + raise HTTPException(status_code=404, detail="Step not found") + + session_uuid = UUID(data.session_id) + + # Check for existing feedback for this step+user+session + existing_result = await db.execute( + select(StepRating).where( + StepRating.step_id == step_id, + StepRating.user_id == current_user.id, + StepRating.session_id == session_uuid, + ) + ) + existing = existing_result.scalar_one_or_none() + + if existing: + existing.was_helpful = data.was_helpful + resp_status = "updated" + else: + new_rating = StepRating( + step_id=step_id, + user_id=current_user.id, + session_id=session_uuid, + was_helpful=data.was_helpful, + # rating is nullable now — thumbs-only mode + ) + db.add(new_rating) + resp_status = "created" + + # Update aggregates on step_library + await _update_step_helpful_counts(db, step_id) + await db.commit() + + return {"step_id": str(step_id), "was_helpful": data.was_helpful, "status": resp_status} + + +async def _update_step_helpful_counts(db: AsyncSession, step_id: UUID): + """Recalculate helpful_yes and helpful_no on step_library.""" + yes_q = await db.execute( + select(func.count()).select_from(StepRating).where( + StepRating.step_id == step_id, StepRating.was_helpful == True + ) + ) + no_q = await db.execute( + select(func.count()).select_from(StepRating).where( + StepRating.step_id == step_id, StepRating.was_helpful == False + ) + ) + await db.execute( + StepLibrary.__table__.update() + .where(StepLibrary.id == step_id) + .values(helpful_yes=yes_q.scalar() or 0, helpful_no=no_q.scalar() or 0) + ) diff --git a/backend/app/api/endpoints/steps.py b/backend/app/api/endpoints/steps.py index 2e5d1c61..b188cf8d 100644 --- a/backend/app/api/endpoints/steps.py +++ b/backend/app/api/endpoints/steps.py @@ -554,6 +554,42 @@ async def delete_rating( return None +# Backward-compatible /ratings alias routes +@router.post("/{step_id}/ratings", response_model=StepRatingResponse, status_code=201, + include_in_schema=False) +async def rate_step_alias( + step_id: UUID, + rating_data: StepRatingCreate, + db: AsyncSession = Depends(get_db), + current_user: User = Depends(get_current_active_user) +): + """Alias for POST /{step_id}/rate.""" + return await rate_step(step_id, rating_data, db, current_user) + + +@router.put("/{step_id}/ratings", response_model=StepRatingResponse, + include_in_schema=False) +async def update_rating_alias( + step_id: UUID, + rating_data: StepRatingUpdate, + db: AsyncSession = Depends(get_db), + current_user: User = Depends(get_current_active_user) +): + """Alias for PUT /{step_id}/rate.""" + return await update_rating(step_id, rating_data, db, current_user) + + +@router.delete("/{step_id}/ratings", status_code=204, + include_in_schema=False) +async def delete_rating_alias( + step_id: UUID, + db: AsyncSession = Depends(get_db), + current_user: User = Depends(get_current_active_user) +): + """Alias for DELETE /{step_id}/rate.""" + return await delete_rating(step_id, db, current_user) + + @router.get("/{step_id}/reviews", response_model=list[StepRatingResponse]) async def get_reviews( step_id: UUID, diff --git a/backend/app/models/step_library.py b/backend/app/models/step_library.py index ae2b319b..a3f23488 100644 --- a/backend/app/models/step_library.py +++ b/backend/app/models/step_library.py @@ -125,7 +125,7 @@ class StepRating(Base): ForeignKey("users.id", ondelete="CASCADE"), nullable=False ) - rating: Mapped[int] = mapped_column(Integer, nullable=False) + rating: Mapped[Optional[int]] = mapped_column(Integer, nullable=True) was_helpful: Mapped[Optional[bool]] = mapped_column(Boolean, nullable=True) review_text: Mapped[Optional[str]] = mapped_column(String(500), nullable=True) is_verified_use: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) diff --git a/backend/tests/test_ratings.py b/backend/tests/test_ratings.py index d623bd0d..8f6d3f66 100644 --- a/backend/tests/test_ratings.py +++ b/backend/tests/test_ratings.py @@ -87,3 +87,129 @@ async def test_rate_incomplete_session(client: AsyncClient, auth_headers: dict, headers=auth_headers, ) assert response.status_code == 400 + + +# --- Step Feedback Tests --- + +@pytest.fixture +async def test_step(client: AsyncClient, auth_headers: dict): + """Create a step in the step library.""" + response = await client.post( + "/api/v1/steps", + json={ + "title": "Test Step", + "step_type": "action", + "content": { + "instructions": "Run the diagnostic command", + "commands": [{"label": "Echo test", "command": "echo hello"}] + }, + }, + headers=auth_headers, + ) + assert response.status_code == 201, f"Failed to create step: {response.text}" + return response.json() + + +async def test_step_feedback_thumbs_up(client: AsyncClient, auth_headers: dict, test_step: dict, test_session: dict): + """Submit thumbs-up feedback for a step.""" + response = await client.post( + f"/api/v1/steps/{test_step['id']}/feedback", + json={"session_id": test_session["id"], "was_helpful": True}, + headers=auth_headers, + ) + assert response.status_code == 201 + data = response.json() + assert data["was_helpful"] is True + assert data["status"] == "created" + + +async def test_step_feedback_thumbs_down(client: AsyncClient, auth_headers: dict, test_step: dict, test_session: dict): + """Submit thumbs-down feedback for a step.""" + response = await client.post( + f"/api/v1/steps/{test_step['id']}/feedback", + json={"session_id": test_session["id"], "was_helpful": False}, + headers=auth_headers, + ) + assert response.status_code == 201 + data = response.json() + assert data["was_helpful"] is False + assert data["status"] == "created" + + +async def test_step_feedback_toggle(client: AsyncClient, auth_headers: dict, test_step: dict, test_session: dict): + """Submitting again for same step+session updates the rating.""" + await client.post( + f"/api/v1/steps/{test_step['id']}/feedback", + json={"session_id": test_session["id"], "was_helpful": True}, + headers=auth_headers, + ) + response = await client.post( + f"/api/v1/steps/{test_step['id']}/feedback", + json={"session_id": test_session["id"], "was_helpful": False}, + headers=auth_headers, + ) + assert response.status_code == 201 + data = response.json() + assert data["was_helpful"] is False + assert data["status"] == "updated" + + +async def test_step_feedback_not_found(client: AsyncClient, auth_headers: dict, test_session: dict): + """Feedback on non-existent step returns 404.""" + import uuid + fake_id = str(uuid.uuid4()) + response = await client.post( + f"/api/v1/steps/{fake_id}/feedback", + json={"session_id": test_session["id"], "was_helpful": True}, + headers=auth_headers, + ) + assert response.status_code == 404 + + +# --- /ratings Alias Route Tests --- + +async def test_ratings_alias_post(client: AsyncClient, auth_headers: dict, test_step: dict): + """POST /steps/{id}/ratings works as alias for /rate.""" + response = await client.post( + f"/api/v1/steps/{test_step['id']}/ratings", + json={"rating": 4, "was_helpful": True}, + headers=auth_headers, + ) + assert response.status_code == 201 + data = response.json() + assert data["rating"] == 4 + + +async def test_ratings_alias_put(client: AsyncClient, auth_headers: dict, test_step: dict): + """PUT /steps/{id}/ratings works as alias for /rate.""" + # First create a rating + await client.post( + f"/api/v1/steps/{test_step['id']}/rate", + json={"rating": 3, "was_helpful": True}, + headers=auth_headers, + ) + # Update via alias + response = await client.put( + f"/api/v1/steps/{test_step['id']}/ratings", + json={"rating": 5}, + headers=auth_headers, + ) + assert response.status_code == 200 + data = response.json() + assert data["rating"] == 5 + + +async def test_ratings_alias_delete(client: AsyncClient, auth_headers: dict, test_step: dict): + """DELETE /steps/{id}/ratings works as alias for /rate.""" + # First create a rating + await client.post( + f"/api/v1/steps/{test_step['id']}/rate", + json={"rating": 4, "was_helpful": True}, + headers=auth_headers, + ) + # Delete via alias + response = await client.delete( + f"/api/v1/steps/{test_step['id']}/ratings", + headers=auth_headers, + ) + assert response.status_code == 204