feat: add step thumbs feedback and /ratings alias routes
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1,12 +1,13 @@
|
|||||||
from uuid import UUID
|
from uuid import UUID
|
||||||
from fastapi import APIRouter, Depends, HTTPException, status
|
from fastapi import APIRouter, Depends, HTTPException, status
|
||||||
from sqlalchemy import select
|
from sqlalchemy import select, func
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
from app.core.database import get_db
|
from app.core.database import get_db
|
||||||
from app.api.deps import get_current_active_user
|
from app.api.deps import get_current_active_user
|
||||||
from app.models import User, Session, SessionRating
|
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"])
|
router = APIRouter(tags=["ratings"])
|
||||||
|
|
||||||
@@ -55,3 +56,69 @@ async def rate_session(
|
|||||||
comment=rating.comment,
|
comment=rating.comment,
|
||||||
created_at=rating.created_at,
|
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)
|
||||||
|
)
|
||||||
|
|||||||
@@ -554,6 +554,42 @@ async def delete_rating(
|
|||||||
return None
|
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])
|
@router.get("/{step_id}/reviews", response_model=list[StepRatingResponse])
|
||||||
async def get_reviews(
|
async def get_reviews(
|
||||||
step_id: UUID,
|
step_id: UUID,
|
||||||
|
|||||||
@@ -125,7 +125,7 @@ class StepRating(Base):
|
|||||||
ForeignKey("users.id", ondelete="CASCADE"),
|
ForeignKey("users.id", ondelete="CASCADE"),
|
||||||
nullable=False
|
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)
|
was_helpful: Mapped[Optional[bool]] = mapped_column(Boolean, nullable=True)
|
||||||
review_text: Mapped[Optional[str]] = mapped_column(String(500), 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)
|
is_verified_use: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False)
|
||||||
|
|||||||
@@ -87,3 +87,129 @@ async def test_rate_incomplete_session(client: AsyncClient, auth_headers: dict,
|
|||||||
headers=auth_headers,
|
headers=auth_headers,
|
||||||
)
|
)
|
||||||
assert response.status_code == 400
|
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
|
||||||
|
|||||||
Reference in New Issue
Block a user