Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
82 changes: 74 additions & 8 deletions backend/app/api/admins.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,81 @@
Endpoints for Admin users (Supabase Auth; link via supabase_user_id).
"""

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

from ..database import get_db
from ..schemas import Admin as AdminSchema
from ..schemas import AdminCreate, AdminUpdate
from ..services import admins

router = APIRouter()


@router.get("")
async def list_admins():
"""List admins. Placeholder — implement with Admin model and schemas."""
return Response(
content="Not implemented — see backend/STARTER_BACKEND_GUIDE.md",
status_code=status.HTTP_501_NOT_IMPLEMENTED,
)
@router.get("", response_model=list[AdminSchema], status_code=status.HTTP_200_OK)
Copy link

Choose a reason for hiding this comment

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

status_code=status.HTTP_200_OK is default, so this is a bit redundant. also applies to GET!

async def list_admins(db: AsyncSession = Depends(get_db)):
"""List all admins."""
return await admins.list_admins(db)


@router.post("", response_model=AdminSchema, status_code=status.HTTP_201_CREATED)
async def create_admin(
payload: AdminCreate,
db: AsyncSession = Depends(get_db),
):
"""Create a new admin."""
return await admins.create_admin(db, payload)


@router.get("/{id}", response_model=AdminSchema, status_code=status.HTTP_200_OK)
async def get_admin(
id: str,
Copy link

Choose a reason for hiding this comment

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

id is a Python built-in function; prob shouldn't be using it as a parameter name. Consider admin_id instead? Also applies to PUT and DELETE endpoints!

db: AsyncSession = Depends(get_db),
):
"""Get a single admin."""
admin = await admins.get_admin(db, id)

if not admin:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail="Admin not found",
)

return admin


@router.put("/{id}", response_model=AdminSchema, status_code=status.HTTP_200_OK)
async def update_admin(
id: str,
payload: AdminUpdate,
db: AsyncSession = Depends(get_db),
):
"""Update an admin."""
admin = await admins.get_admin(db, id)
Copy link

Choose a reason for hiding this comment

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

This 404 block is repeated for GET, PUT, DELETE. Consider a helper function to avoid this repetition


if not admin:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail="Admin not found",
)

return await admins.update_admin(db, admin, payload)


@router.delete("/{id}", status_code=status.HTTP_204_NO_CONTENT)
async def delete_admin(
id: str,
db: AsyncSession = Depends(get_db),
):
"""Delete an admin."""
admin = await admins.get_admin(db, id)

if not admin:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail="Admin not found",
)

await admins.delete_admin(db, admin)

return Response(status_code=status.HTTP_204_NO_CONTENT)
Copy link

Choose a reason for hiding this comment

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

Decorator alr sets status_code=status.HTTP_204_NO_CONTENT, so explicit return is perhaps redundant as well.

3 changes: 2 additions & 1 deletion backend/app/api/agencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@

from ..database import get_db
from ..models import Agency
from ..schemas import AgencyCreate, AgencyUpdate, Agency as AgencySchema
from ..schemas import Agency as AgencySchema
from ..schemas import AgencyCreate, AgencyUpdate

router = APIRouter()

Expand Down
3 changes: 2 additions & 1 deletion backend/app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
"""

from functools import lru_cache
from pydantic_settings import BaseSettings
from typing import List

from pydantic_settings import BaseSettings


class Settings(BaseSettings):
"""Application settings."""
Expand Down
2 changes: 1 addition & 1 deletion backend/app/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
@see https://docs.sqlalchemy.org/en/20/orm/
"""

from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine, AsyncSession
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine
from sqlalchemy.orm import declarative_base

from .config import get_settings
Expand Down
5 changes: 3 additions & 2 deletions backend/app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@
dependencies configured.
"""

from contextlib import asynccontextmanager

from fastapi import FastAPI
from fastapi.middleware.cors import CORSMiddleware
from contextlib import asynccontextmanager

from .config import get_settings
from .api import router as api_router
from .config import get_settings

settings = get_settings()

Expand Down
11 changes: 1 addition & 10 deletions backend/app/models/__init__.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
"""Models package initialization."""

from .base import (
Admin,
Agency,
Agent,
Client,
Donor,
Furniture,
Referral,
Route,
)
from .base import Admin, Agency, Agent, Client, Donor, Furniture, Referral, Route

__all__ = [
"Admin",
Expand Down
8 changes: 8 additions & 0 deletions backend/app/services/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
"""Services package.

Business logic layer; API routes depend on services, not the reverse.
"""

from . import admins as admins_service

__all__ = ["admins_service"]
49 changes: 49 additions & 0 deletions backend/app/services/admins.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
"""Admin service layer.

Handles database operations for Admins.
"""

from sqlalchemy import select
from sqlalchemy.ext.asyncio import AsyncSession

from ..models import Admin
from ..schemas import AdminCreate, AdminUpdate


async def list_admins(db: AsyncSession):
Copy link

Choose a reason for hiding this comment

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

All functions are missing return types. Return types makes it easier to understand and safer to use without needing to inspect the implementation!

"""Return all admins ordered by first name."""
result = await db.execute(select(Admin).order_by(Admin.first_name))
return list(result.scalars().all())
Copy link

Choose a reason for hiding this comment

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

Suggested change
return list(result.scalars().all())
return result.scalars().all()

Already returns a list! Saw this in one of Kenzys comments LOL



async def create_admin(db: AsyncSession, payload: AdminCreate):
"""Create a new admin."""
db_admin = Admin(**payload.model_dump())
db.add(db_admin)
await db.commit()
Copy link
Member

Choose a reason for hiding this comment

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

create_admin, update_admin, and delete_admin don't currently handle IntegrityError

if a database constraint fails (e.g., a foreign key or uniqueness constraint), await db.commit() will raise an IntegrityError and the API will return a 500 instead of a controlled client error

let's catch these and convert them into a ValueError after rolling back the transaction! something like

try:
      await db.commit() 
except IntegrityError as e:
      await db.rollback()
      raise ValueError(f"Unable to create admin: {str(e.orig)}") from e

but for each of create_admin, update_admin, and delete_admin

#16 for reference

await db.refresh(db_admin)
return db_admin


async def get_admin(db: AsyncSession, admin_id: str):
"""Return a single admin by ID or None."""
result = await db.execute(select(Admin).where(Admin.id == admin_id))
return result.scalar_one_or_none()


async def update_admin(db: AsyncSession, admin: Admin, payload: AdminUpdate):
"""Update an admin."""
data = payload.model_dump(exclude_unset=True)
Copy link
Member

Choose a reason for hiding this comment

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

potentially add a check like

if not data:
    raise ValueError("No update fields were provided.")

to avoid committing without modifying anything (#17 for reference)


for key, value in data.items():
setattr(admin, key, value)

await db.commit()
await db.refresh(admin)
return admin


async def delete_admin(db: AsyncSession, admin: Admin):
"""Delete an admin."""
await db.delete(admin)
await db.commit()
2 changes: 1 addition & 1 deletion backend/migrations/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@

# Import application settings and models so alembic can autogenerate
try:
from app.config import get_settings
from app import database # ensures Base is defined
from app.config import get_settings
from app.models import base # register all models with Base.metadata
except Exception: # pragma: no cover - keep safe for CI
raise RuntimeError(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
"""

from alembic import op
import sqlalchemy as sa
from alembic import op

revision = "20260215_redesign"
down_revision = "20260215_hafb"
Expand Down
2 changes: 1 addition & 1 deletion backend/migrations/versions/20260215_hafb_tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

"""

from alembic import op
import sqlalchemy as sa
from alembic import op

revision = "20260215_hafb"
down_revision = "2adf14c14d9c"
Expand Down
2 changes: 1 addition & 1 deletion backend/migrations/versions/2adf14c14d9c_initial.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
"""

from alembic import op
import sqlalchemy as sa
from alembic import op

# revision identifiers, used by Alembic.
revision = "2adf14c14d9c"
Expand Down
4 changes: 2 additions & 2 deletions backend/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
"""

import os
import uvicorn
from dotenv import load_dotenv

import uvicorn
from app.main import create_app
from dotenv import load_dotenv

if __name__ == "__main__":
load_dotenv()
Expand Down
2 changes: 1 addition & 1 deletion backend/setup.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
from setuptools import setup, find_packages
from setuptools import find_packages, setup

setup(name="app", packages=find_packages())
1 change: 0 additions & 1 deletion backend/tests/functional/conftest.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import pytest

from app import create_app


Expand Down
3 changes: 2 additions & 1 deletion e2e-tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import inflection
import os

import inflection
import pytest
import requests
from dotenv import load_dotenv
Expand Down
1 change: 0 additions & 1 deletion e2e-tests/test_auth.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import inflection
import requests

from test_user import delete_user


Expand Down
1 change: 0 additions & 1 deletion e2e-tests/test_auth_gql.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import inflection
import requests

from test_user_gql import delete_user


Expand Down
1 change: 0 additions & 1 deletion frontend/app/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export default function HomePage() {
</div>
</div>


<div className="flex flex-wrap gap-4">
<a
href="http://localhost:8000/docs"
Expand Down
Loading