Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
98 changes: 83 additions & 15 deletions backend/app/api/donors.py
Original file line number Diff line number Diff line change
@@ -1,23 +1,91 @@
"""Starter endpoints for the Donors API.
"""Donors REST API.

This module contains a minimal placeholder to extend and implement.
Refer to `docs/STARTER_BACKEND_GUIDE.md` for implementation
examples and guidance on using `AsyncSession`, models, and schemas.
Full CRUD implementation for the Donors resource.
"""

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

from ..config import get_settings
from ..database import get_db
from ..schemas import DonorCreate, DonorUpdate, Donor as DonorSchema
from ..services import donor_service

router = APIRouter()


@router.get("")
async def list_donors():
"""List donors.
@router.get("", response_model=list[DonorSchema])
async def list_donors(db: AsyncSession = Depends(get_db)):
"""List all donors."""
return await donor_service.list_donors(db)


@router.post("", response_model=DonorSchema, status_code=status.HTTP_201_CREATED)
async def create_donor(donor: DonorCreate, db: AsyncSession = Depends(get_db)):
"""Create a new donor."""
try:
return await donor_service.create_donor(db, donor)
except Exception as e:
if get_settings().DEBUG:
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail=str(e),
) from e
raise


@router.get("/{donor_id}", response_model=DonorSchema, status_code=status.HTTP_200_OK)
async def get_donor(donor_id: str, db: AsyncSession = Depends(get_db)):
"""Get a single donor by ID."""
donor = await donor_service.get_donor(db, donor_id)
if not donor:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail="Donor not found",
)
return donor


@router.put("/{donor_id}", response_model=DonorSchema, status_code=status.HTTP_200_OK)
async def update_donor(
donor_id: str, payload: DonorUpdate, db: AsyncSession = Depends(get_db)
):
"""Update a donor."""
try:
donor = await donor_service.update_donor(db, donor_id, payload)
if not donor:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail="Donor not found",
)
return donor
except HTTPException:
raise
except Exception as e:
if get_settings().DEBUG:
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail=str(e),
) from e
raise


Placeholder implementation. See `docs/STARTER_BACKEND_GUIDE.md` (repo root)
for details on implementing this handler.
"""
return Response(
content="Not implemented — see docs/STARTER_BACKEND_GUIDE.md",
status_code=status.HTTP_501_NOT_IMPLEMENTED,
)
@router.delete("/{donor_id}", status_code=status.HTTP_204_NO_CONTENT)
async def delete_donor(donor_id: str, db: AsyncSession = Depends(get_db)):
"""Delete a donor."""
try:
deleted = await donor_service.delete_donor(db, donor_id)
if not deleted:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail="Donor not found",
)
except HTTPException:
raise
except Exception as e:
if get_settings().DEBUG:
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail=str(e),
) from e
raise
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 donors as donor_service

__all__ = ["donor_service"]
70 changes: 70 additions & 0 deletions backend/app/services/donors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
"""Donor services module.

Contains business logic for donor-related operations.
"""

from sqlalchemy import select
from sqlalchemy.ext.asyncio import AsyncSession
from ..models import Donor
from ..schemas import DonorCreate, DonorUpdate


async def list_donors(db: AsyncSession) -> list[Donor]:
"""List all donors."""
result = await db.execute(select(Donor).order_by(Donor.name))
return list(result.scalars().all())


async def create_donor(db: AsyncSession, donor: DonorCreate) -> Donor:
"""Create a new donor."""
try:
db_donor = Donor(**donor.model_dump())
db.add(db_donor)
await db.commit()
await db.refresh(db_donor)
return db_donor
except Exception:
await db.rollback()
raise


async def get_donor(db: AsyncSession, donor_id: str) -> Donor | None:
"""Get a single donor by ID."""
result = await db.execute(select(Donor).where(Donor.id == donor_id))
return result.scalar_one_or_none()


async def update_donor(
db: AsyncSession, donor_id: str, payload: DonorUpdate
) -> Donor | None:
"""Update a donor."""
donor = await get_donor(db, donor_id)
if not donor:
return None

try:
# Only update fields that were actually provided in the request
update_data = payload.model_dump(exclude_unset=True)
for key, value in update_data.items():
setattr(donor, key, value)

await db.commit()
await db.refresh(donor)
return donor
except Exception:
await db.rollback()
raise


async def delete_donor(db: AsyncSession, donor_id: str) -> bool:
"""Delete a donor."""
donor = await get_donor(db, donor_id)
if not donor:
return False
try:
await db.delete(donor)

Choose a reason for hiding this comment

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

Deleting a donor can fail here if that donor is still referenced by furniture.donor_id. In that case this will raise a foreign key error and the endpoint returns a 500 instead of a controlled client response.

Should either block deletion with a 4xx (for example 409) when linked furniture exists, or explicitly handle those dependent records.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a good point, we might want to think about whether furniture should still exist if the donor is deleted. Is it safe to assume that if a donor is deleted, we should also delete all furniture associated with that donor?

To me, it doesn’t make much sense to throw an error just because a furniture item references a donor that’s being deleted. A bulk delete might make more sense here, otherwise someone would have to delete all the furniture before deleting the donor?

@kenzysoror thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

mmm really good catch! i spoke with the PMs and it seems we actually want to allow furniture to exist without a donor (e.g., if HAFB adds existing inventory into the platform when they first adopt it).

it turns out that we don't currently foresee any use cases where HAFB would need to delete a donor from the system. so for now, and given the above, we'd want deleting a donor to simply nullify the donor field within the associated furniture and keep the furniture intact.

updating the furniture schema to reflect this can be another PR, and i believe this delete logic should work as-is once that's done.

does this make sense @nuthanan06 @petersenmatthew ?

await db.commit()
return True
except Exception:
await db.rollback()
raise