Skip to content

[InProgress] DEV #54

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Saurabh254 wants to merge 2 commits into from
Closed

[InProgress] DEV #54

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
39dcaef
fix warning and add get user by id endpoint
Saurabh254 Feb 5, 2026
77ce6a4
feat: configure supabase and Added logging setup
Saurabh254 Feb 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions server/pin_sphere/comments/schemas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,3 @@ class SlimCommentResponse(CommentBase):

class CommentResponse(SlimCommentResponse):
replies: List["SlimCommentResponse"] = []

class Config:
orm_mode = True
17 changes: 16 additions & 1 deletion server/pin_sphere/users/endpoint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import Literal
from uuid import UUID

from fastapi import APIRouter, Depends, HTTPException
from fastapi import APIRouter, Depends, HTTPException, Path
from sqlalchemy.ext.asyncio import AsyncSession
from starlette import status

Expand Down Expand Up @@ -83,6 +84,20 @@ async def check_username_availability(
return {"message": "Username is available"}


@router.get(
"/{user_id}",

Copilot AI Feb 5, 2026

Copy link

Choose a reason for hiding this comment

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

This endpoint path "/{user_id}" conflicts with the existing "/{username}" GET endpoint on line 178. FastAPI will route requests to the first matching pattern, which means this new endpoint will intercept all requests intended for the username endpoint. When a client requests "/users/john", FastAPI will try to parse "john" as a UUID for this endpoint instead of treating it as a username. Consider using a more specific path like "/by-id/{user_id}" or "/user-id/{user_id}" to avoid this ambiguity.

Suggested change
"/{user_id}",
"/by-id/{user_id}",

Copilot uses AI. Check for mistakes.
response_model=schemas.UserResponse,
summary="Delete user account",
description="Delete a user account by username. Returns the deleted user information.",
Comment on lines +90 to +91

Copilot AI Feb 5, 2026

Copy link

Choose a reason for hiding this comment

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

The summary and description are incorrect for this endpoint. The endpoint is a GET operation that retrieves a user by ID, not a DELETE operation. The summary should be something like "Get user by ID" and the description should indicate that it retrieves user information by their unique user ID.

Suggested change
summary="Delete user account",
description="Delete a user account by username. Returns the deleted user information.",
summary="Get user by ID",
description="Retrieve user information by their unique user ID.",

Copilot uses AI. Check for mistakes.
tags=["Account Operations"],
dependencies=[Depends(auth.get_current_user)],
)
async def get_user_by_user_id(
user_id: UUID = Path(), db: AsyncSession = Depends(get_async_session)
):
return await service.get_user_by_user_id(user_id, db)

Copilot AI Feb 5, 2026

Copy link

Choose a reason for hiding this comment

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

This endpoint lacks error handling when the user is not found. The function can return None (as specified in the service function return type), but there's no check to raise a 404 HTTPException when the user doesn't exist. Other similar endpoints in this file (like delete_account on line 109, read_user on line 184) properly check for None and raise appropriate exceptions. This should follow the same pattern.

Suggested change
return await service.get_user_by_user_id(user_id, db)
user = await service.get_user_by_user_id(user_id, db)
if not user:
raise HTTPException(status_code=404, detail="User not found")
return user

Copilot uses AI. Check for mistakes.


@router.delete(
"/{username}",
response_model=schemas.UserResponse,
Expand Down
6 changes: 1 addition & 5 deletions server/pin_sphere/users/filters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Literal, Optional
from typing import Optional

import pytz
from pydantic import BaseModel, Field
Expand Down Expand Up @@ -66,10 +66,6 @@ class PrivacySecuritySettingsFilter(BaseModel):

# ---------- Main Filter Wrapper ----------
class SettingsFilter(BaseModel):
settings_type: Literal[
"general", "notification", "appearance", "privacy_and_security"
]

general: Optional[GeneralSettingsFilter] = None
notification: Optional[NotificationSettingsFilter] = None
appearance: Optional[AppearanceSettingsFilter] = None
Expand Down
9 changes: 8 additions & 1 deletion server/pin_sphere/users/service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from sqlalchemy.exc import NoResultFound
from sqlalchemy.ext.asyncio import AsyncSession

from core import storage
from core import models, storage
from core.authflow.service import hash_password
from core.models import User
from core.types import FileContentType
Expand All @@ -25,6 +25,13 @@ async def get_user(db: AsyncSession, username: str):
return None


async def get_user_by_user_id(
user_id: uuid.UUID, db: AsyncSession
Comment on lines +28 to +29

Copilot AI Feb 5, 2026

Copy link

Choose a reason for hiding this comment

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

The parameter order is inconsistent with other service functions in this file. Other functions follow the pattern of putting the database session (db) as the first parameter (see get_user, get_user_by_email, get_user_by_username). This function should use get_user_by_user_id(db: AsyncSession, user_id: uuid.UUID) for consistency with established conventions in this file.

Copilot uses AI. Check for mistakes.
) -> models.User | None:
query = select(models.User).filter(models.User.id == str(user_id))
Comment on lines +28 to +31

Copilot AI Feb 5, 2026

Copy link

Choose a reason for hiding this comment

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

There's an inconsistency in the model reference. This function uses models.User while other functions in the same file (like get_user, get_user_by_email, get_user_by_username) use the directly imported User from core.models. Since User is already imported on line 11, consider using User directly instead of models.User for consistency with the rest of the codebase.

Copilot uses AI. Check for mistakes.

Copilot AI Feb 5, 2026

Copy link

Choose a reason for hiding this comment

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

The User model's id field is defined as UUID type in the database (see core/database/base_model.py:34), but this query converts the UUID to a string for comparison. This conversion may not be necessary depending on the database driver's UUID handling. Other functions in this file (like get_user, get_user_by_email) don't perform string conversion when filtering. Consider checking if the database driver handles UUID comparisons directly, and if so, use models.User.id == user_id instead for consistency and potential performance improvement.

Suggested change
query = select(models.User).filter(models.User.id == str(user_id))
query = select(models.User).filter(models.User.id == user_id)

Copilot uses AI. Check for mistakes.
return await db.scalar(query)


# Service to fetch a users by email
async def get_user_by_email(db: AsyncSession, email: EmailStr):
try:
Expand Down
Loading