-
Notifications
You must be signed in to change notification settings - Fork 0
CRUD operations for Donors (DEV-39) #16
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.
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
Changes from all commits
5b89464
c4dd8f4
26e0509
bceb163
a1369bd
56d4213
a659f35
19b769a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,112 @@ | ||
| """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, Response, status | ||
| from sqlalchemy.ext.asyncio import AsyncSession | ||
|
|
||
| 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, | ||
| responses={400: {"description": "Integrity error (e.g., duplicate donor email)"}}, | ||
| ) | ||
| 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 ValueError as e: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=str(e), | ||
| ) from e | ||
|
|
||
|
|
||
| @router.get( | ||
| "/{donor_id}", | ||
| response_model=DonorSchema, | ||
| status_code=status.HTTP_200_OK, | ||
| responses={404: {"description": "Donor not found"}}, | ||
| ) | ||
| 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, | ||
| responses={ | ||
| 400: {"description": "Integrity error (e.g., duplicate donor email)"}, | ||
| 404: {"description": "Donor not found"}, | ||
| }, | ||
| ) | ||
| async def update_donor( | ||
| donor_id: str, payload: DonorUpdate, db: AsyncSession = Depends(get_db) | ||
| ): | ||
| """Update a donor.""" | ||
| 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", | ||
| ) | ||
|
|
||
| try: | ||
| return await donor_service.update_donor(db, donor, payload) | ||
| except ValueError as e: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=str(e), | ||
| ) from e | ||
|
|
||
|
|
||
| @router.delete( | ||
| "/{donor_id}", | ||
| status_code=status.HTTP_204_NO_CONTENT, | ||
| responses={ | ||
| 400: { | ||
| "description": "Integrity error (e.g., donor has linked furniture records)" | ||
| }, | ||
| 404: {"description": "Donor not found"}, | ||
| }, | ||
| ) | ||
| async def delete_donor(donor_id: str, db: AsyncSession = Depends(get_db)): | ||
| """Delete a donor.""" | ||
| 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", | ||
| ) | ||
|
|
||
| try: | ||
| await donor_service.delete_donor(db, donor) | ||
| except ValueError as e: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=str(e), | ||
| ) from e | ||
|
|
||
| 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, | ||
| ) | ||
| return Response(status_code=status.HTTP_204_NO_CONTENT) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,9 @@ | ||
| """Services package. | ||
| Business logic layer; API routes depend on services, not the reverse. | ||
| """ | ||
|
|
||
| from . import clients as clients_service | ||
|
|
||
| __all__ = ["clients_service"] | ||
| """Services package. | ||
| Business logic layer; API routes depend on services, not the reverse. | ||
| """ | ||
|
|
||
| from . import donors as donor_service | ||
| from . import clients as clients_service | ||
|
|
||
| __all__ = ["donor_service", "clients_service"] |
| 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.exc import IntegrityError | ||
| 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 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 IntegrityError as e: | ||
| await db.rollback() | ||
| raise ValueError(f"Unable to create donor: {str(e.orig)}") from e | ||
| except Exception: | ||
nuthanan06 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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: Donor, payload: DonorUpdate) -> Donor: | ||
| """Update a donor.""" | ||
| 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() | ||
nuthanan06 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| await db.refresh(donor) | ||
| return donor | ||
| except IntegrityError as e: | ||
| await db.rollback() | ||
| raise ValueError(f"Unable to update donor: {str(e.orig)}") from e | ||
| except Exception: | ||
| await db.rollback() | ||
| raise | ||
|
|
||
|
|
||
| async def delete_donor(db: AsyncSession, donor: Donor) -> None: | ||
| """Delete a donor.""" | ||
| try: | ||
| await db.delete(donor) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Should either block deletion with a 4xx (for example 409) when linked furniture exists, or explicitly handle those dependent records.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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() | ||
| except IntegrityError as e: | ||
| await db.rollback() | ||
| raise ValueError(f"Unable to delete donor: {str(e.orig)}") from e | ||
| except Exception: | ||
| await db.rollback() | ||
| raise | ||
Uh oh!
There was an error while loading. Please reload this page.