Merge pull request #56 from alimkhann/feature/views-trending-sort

Feature/views trending sort

Author: alimkhann

commitly-backend/alembic/versions/20241115_add_generated_roadmaps.py

@@ -1,7 +1,7 @@
 """add generated roadmaps table
 
 Revision ID: 20241115_add_generated_roadmaps
-Revises: 20241114_add_github_credentials
+Revises: 20241114_add_github_creds
 Create Date: 2025-11-15 07:40:00.000000
 """
 
@@ -10,7 +10,7 @@
 
 # revision identifiers, used by Alembic.
 revision = "20241115_add_generated_roadmaps"
-down_revision = "20241114_add_github_credentials"
+down_revision = "20241114_add_github_creds"
 branch_labels = None
 depends_on = None
 

commitly-backend/alembic/versions/20241116_add_roadmap_view_tracker.py

@@ -0,0 +1,51 @@
+"""add roadmap view tracker table
+
+Revision ID: add_view_tracker
+Revises: 20241116_add_roadmap_ratings_table
+Create Date: 2024-11-16 14:00:00.000000
+
+"""
+
+from typing import Sequence, Union
+
+from alembic import op
+import sqlalchemy as sa
+
+# revision identifiers, used by Alembic.
+revision: str = "add_view_tracker"
+down_revision: Union[str, None] = "20241116_add_roadmap_ratings"
+branch_labels: Union[str, Sequence[str], None] = None
+depends_on: Union[str, Sequence[str], None] = None
+
+
+def upgrade() -> None:
+    """Create roadmap_view_tracker table for anti-spam view counting."""
+    op.create_table(
+        "roadmap_view_tracker",
+        sa.Column("id", sa.Integer(), nullable=False),
+        sa.Column("repo_full_name", sa.String(length=255), nullable=False),
+        sa.Column("user_id", sa.String(length=255), nullable=False),
+        sa.Column(
+            "viewed_at",
+            sa.DateTime(timezone=True),
+            server_default=sa.text("now()"),
+            nullable=False,
+        ),
+        sa.PrimaryKeyConstraint("id"),
+        sa.UniqueConstraint(
+            "repo_full_name", "user_id", name="uq_roadmap_view_tracker_repo_user"
+        ),
+    )
+    op.create_index(
+        "idx_roadmap_view_tracker_viewed_at",
+        "roadmap_view_tracker",
+        ["viewed_at"],
+    )
+
+
+def downgrade() -> None:
+    """Drop roadmap_view_tracker table."""
+    op.drop_index(
+        "idx_roadmap_view_tracker_viewed_at", table_name="roadmap_view_tracker"
+    )
+    op.drop_table("roadmap_view_tracker")

commitly-backend/app/api/roadmap.py

@@ -1,9 +1,11 @@
 from __future__ import annotations
 
-from fastapi import APIRouter, Depends, Response, status
+from typing import Annotated, Optional
+
+from fastapi import APIRouter, Depends, HTTPException, Response, status
 from sqlalchemy.orm import Session
 
-from app.core.auth import ClerkClaims, require_clerk_auth
+from app.core.auth import ClerkClaims, optional_clerk_auth, require_clerk_auth
 from app.core.database import get_db
 from app.models.roadmap import (
     RatingRequest,
@@ -13,6 +15,7 @@
     RoadmapResponse,
     UserRepoStateResponse,
 )
+from app.services.roadmap_repository import SortOption
 from app.services.roadmap_service import RoadmapService, build_roadmap_service
 
 router = APIRouter()
@@ -22,13 +25,38 @@ def get_roadmap_service(session: Session = Depends(get_db)) -> RoadmapService:
     return build_roadmap_service(session)
 
 
+def get_user_id(claims: ClerkClaims) -> str:
+    """Extract user ID from ClerkClaims, raising error if missing."""
+    user_id = claims.get("sub")
+    if not user_id:
+        raise HTTPException(
+            status_code=status.HTTP_401_UNAUTHORIZED,
+            detail="User ID not found in authentication claims",
+        )
+    return user_id
+
+
 @router.get("/catalog", response_model=RoadmapCatalogPage)
 async def list_roadmaps(
     page: int = 1,
     page_size: int = 20,
+    sort: Annotated[
+        SortOption,
+        "Sort option: newest, most_viewed, most_synced, highest_rated, trending",
+    ] = "newest",
     service: RoadmapService = Depends(get_roadmap_service),
 ) -> RoadmapCatalogPage:
-    return await service.list_catalog(page, page_size)
+    """
+    List roadmaps with pagination and sorting.
+
+    Sort options:
+    - newest: Order by updated_at DESC (default)
+    - most_viewed: Order by view_count DESC
+    - most_synced: Order by sync_count DESC
+    - highest_rated: Order by average rating DESC
+    - trending: Order by trending score (combines views, syncs, and ratings)
+    """
+    return await service.list_catalog(page, page_size, sort)
 
 
 @router.get("/cached/{owner}/{repo}", response_model=RoadmapResponse)
@@ -47,10 +75,11 @@ async def generate_roadmap(
     service: RoadmapService = Depends(get_roadmap_service),
     current_user: ClerkClaims = Depends(require_clerk_auth),
 ) -> RoadmapResponse:
+    user_id = get_user_id(current_user)
     return await service.generate(
         repo_url=str(payload.repo_url),
         force_refresh=payload.force_refresh,
-        actor_id=current_user["sub"],
+        actor_id=user_id,
     )
 
 
@@ -59,7 +88,7 @@ async def list_pinned_roadmaps(
     current_user: ClerkClaims = Depends(require_clerk_auth),
     service: RoadmapService = Depends(get_roadmap_service),
 ) -> list[RoadmapResponse]:
-    return await service.list_user_pins(current_user["sub"])
+    return await service.list_user_pins(get_user_id(current_user))
 
 
 @router.delete("/pins/{owner}/{repo}", status_code=status.HTTP_204_NO_CONTENT)
@@ -69,7 +98,7 @@ async def unpin_roadmap(
     current_user: ClerkClaims = Depends(require_clerk_auth),
     service: RoadmapService = Depends(get_roadmap_service),
 ) -> Response:
-    await service.unpin_repo(current_user["sub"], f"{owner}/{repo}")
+    await service.unpin_repo(get_user_id(current_user), f"{owner}/{repo}")
     return Response(status_code=status.HTTP_204_NO_CONTENT)
 
 
@@ -78,7 +107,7 @@ async def list_user_repositories(
     current_user: ClerkClaims = Depends(require_clerk_auth),
     service: RoadmapService = Depends(get_roadmap_service),
 ) -> list[UserRepoStateResponse]:
-    return await service.list_user_repos(current_user["sub"])
+    return await service.list_user_repos(get_user_id(current_user))
 
 
 @router.post("/sync/{owner}/{repo}", response_model=UserRepoStateResponse)
@@ -88,7 +117,7 @@ async def sync_repository(
     current_user: ClerkClaims = Depends(require_clerk_auth),
     service: RoadmapService = Depends(get_roadmap_service),
 ) -> UserRepoStateResponse:
-    return await service.sync_repo(owner, repo, current_user["sub"])
+    return await service.sync_repo(owner, repo, get_user_id(current_user))
 
 
 @router.delete("/sync/{owner}/{repo}", status_code=status.HTTP_204_NO_CONTENT)
@@ -98,7 +127,7 @@ async def desync_repository(
     current_user: ClerkClaims = Depends(require_clerk_auth),
     service: RoadmapService = Depends(get_roadmap_service),
 ) -> Response:
-    await service.desync_repo(owner, repo, current_user["sub"])
+    await service.desync_repo(owner, repo, get_user_id(current_user))
     return Response(status_code=status.HTTP_204_NO_CONTENT)
 
 
@@ -109,7 +138,7 @@ async def archive_repository(
     current_user: ClerkClaims = Depends(require_clerk_auth),
     service: RoadmapService = Depends(get_roadmap_service),
 ) -> UserRepoStateResponse:
-    return await service.archive_repo(owner, repo, current_user["sub"])
+    return await service.archive_repo(owner, repo, get_user_id(current_user))
 
 
 @router.post("/unarchive/{owner}/{repo}", response_model=UserRepoStateResponse)
@@ -119,15 +148,15 @@ async def unarchive_repository(
     current_user: ClerkClaims = Depends(require_clerk_auth),
     service: RoadmapService = Depends(get_roadmap_service),
 ) -> UserRepoStateResponse:
-    return await service.unarchive_repo(owner, repo, current_user["sub"])
+    return await service.unarchive_repo(owner, repo, get_user_id(current_user))
 
 
 @router.get("/archived", response_model=list[UserRepoStateResponse])
 async def list_archived_repositories(
     current_user: ClerkClaims = Depends(require_clerk_auth),
     service: RoadmapService = Depends(get_roadmap_service),
 ) -> list[UserRepoStateResponse]:
-    return await service.list_archived_repos(current_user["sub"])
+    return await service.list_archived_repos(get_user_id(current_user))
 
 
 @router.post("/{owner}/{repo}/rating", response_model=RatingResponse)
@@ -138,7 +167,7 @@ async def set_repository_rating(
     current_user: ClerkClaims = Depends(require_clerk_auth),
     service: RoadmapService = Depends(get_roadmap_service),
 ) -> RatingResponse:
-    return service.set_rating(current_user["sub"], owner, repo, payload.rating)
+    return service.set_rating(get_user_id(current_user), owner, repo, payload.rating)
 
 
 @router.get("/{owner}/{repo}/rating", response_model=RatingResponse | None)
@@ -148,4 +177,22 @@ async def get_repository_rating(
     current_user: ClerkClaims = Depends(require_clerk_auth),
     service: RoadmapService = Depends(get_roadmap_service),
 ) -> RatingResponse | None:
-    return service.get_user_rating(current_user["sub"], owner, repo)
+    return service.get_user_rating(get_user_id(current_user), owner, repo)
+
+
+@router.post("/{owner}/{repo}/view", status_code=status.HTTP_204_NO_CONTENT)
+async def record_roadmap_view(
+    owner: str,
+    repo: str,
+    current_user: Optional[ClerkClaims] = Depends(optional_clerk_auth),
+    service: RoadmapService = Depends(get_roadmap_service),
+) -> Response:
+    """
+    Record a view of a roadmap timeline.
+
+    Authentication is optional. If authenticated, implements anti-spam logic
+    (only one view per user per 24-hour window). Anonymous views are always counted.
+    """
+    user_id = current_user.get("sub") if current_user else None
+    await service.record_roadmap_view(f"{owner}/{repo}", user_id)
+    return Response(status_code=status.HTTP_204_NO_CONTENT)

commitly-backend/app/core/auth.py

@@ -211,6 +211,29 @@ def require_clerk_auth(request: Request) -> ClerkClaims:
     return claims
 
 
+def optional_clerk_auth(request: Request) -> Optional[ClerkClaims]:
+    """
+    Optional Clerk authentication dependency.
+
+    Returns ClerkClaims if a valid token is present, otherwise None.
+    Does not raise exceptions for missing or invalid tokens.
+
+    This is useful for endpoints that should work for both authenticated
+    and unauthenticated users.
+    """
+    cached: Optional[ClerkClaims] = getattr(request.state, "clerk_claims", None)
+    if cached:
+        return cached
+
+    try:
+        token = _get_bearer_token(request)
+        claims = verify_clerk_token(token)
+        request.state.clerk_claims = claims
+        return claims
+    except (MissingBearerToken, InvalidClerkToken):
+        return None
+
+
 class ClerkAuthMiddleware(BaseHTTPMiddleware):
     """Pre-decodes Clerk tokens so downstream dependencies can reuse them."""
 

commitly-backend/app/models/roadmap.py

@@ -130,6 +130,24 @@ class RoadmapRating(Base):
     )
 
 
+class RoadmapViewTracker(Base):
+    """Tracks views to prevent spam and implement anti-spam logic."""
+
+    __tablename__ = "roadmap_view_tracker"
+    __table_args__ = (
+        UniqueConstraint(
+            "repo_full_name", "user_id", name="uq_roadmap_view_tracker_repo_user"
+        ),
+    )
+
+    id: Mapped[int] = mapped_column(Integer, primary_key=True)
+    repo_full_name: Mapped[str] = mapped_column(String(255), nullable=False, index=True)
+    user_id: Mapped[str] = mapped_column(String(255), nullable=False, index=True)
+    viewed_at: Mapped[datetime] = mapped_column(
+        DateTime(timezone=True), server_default=func.now(), nullable=False
+    )
+
+
 class RoadmapRequest(BaseModel):
     repo_url: AnyHttpUrl = Field(description="GitHub repository URL")
     force_refresh: bool = Field(