fix(officers): tests now all work

Author: jbriones1

src/officers/crud.py

@@ -325,6 +325,10 @@ async def create_multiple_officers(db_session: database.DBSession, new_officers:
     # Flush gets the generated IDs, but does not commit
     await db_session.flush()
 
+    # Refresh each term to ensure all attributes are loaded for Pydantic validation
+    for term in new_officer_terms:
+        await db_session.refresh(term)
+
     return new_officer_terms
 
 

src/officers/models.py

@@ -19,6 +19,7 @@
 
 # Officer Info Models
 class OfficerInfo(BaseModel):
+    model_config = ConfigDict(from_attributes=True)
     computing_id: str = Field(..., max_length=COMPUTING_ID_LEN)
     legal_name: str = Field(..., max_length=OFFICER_LEGAL_NAME_MAX)
     phone_number: str | None = None
@@ -57,8 +58,8 @@ class OfficerTerm(OfficerTermCreate):
 class OfficerTermUpdate(BaseModel):
     """Request body to patch an Officer Term"""
 
-    computing_id: str | None = Field(..., max_length=COMPUTING_ID_LEN)
-    position: str | None = Field(..., max_length=128)
+    computing_id: str | None = Field(None, max_length=COMPUTING_ID_LEN)
+    position: str | None = Field(None, max_length=128)
     start_date: date | None = None
     end_date: date | None = None
     nickname: str | None = Field(None, max_length=128)
@@ -77,6 +78,9 @@ class OfficerBase(BaseModel):
     position: OfficerPositionEnum
     start_date: date
     end_date: date | None = None
+    nickname: str | None = None
+    biography: str | None = None
+    csss_email: str | None = None
 
 
 class OfficerPublic(OfficerBase):
@@ -85,9 +89,6 @@ class OfficerPublic(OfficerBase):
     """
 
     is_active: bool
-    nickname: str | None = None
-    biography: str | None = None
-    csss_email: str | None = None
 
 
 class OfficerPrivate(OfficerPublic):
@@ -105,7 +106,20 @@ class OfficerPrivate(OfficerPublic):
     photo_url: str | None = None
 
 
-class OfficerCreate(OfficerPrivate):
+class OfficerCreate(OfficerBase):
+    """
+    Parameters when creating a new Officer
+    """
+
+    computing_id: str
+
+    discord_id: str | None = None
+    discord_name: str | None = None
+    discord_nickname: str | None = None
+    phone_number: str | None = None
+    github_username: str | None = None
+    google_drive_email: str | None = None
+    photo_url: str | None = None
     favourite_course_0: str | None = Field(None, max_length=64)
     favourite_course_1: str | None = Field(None, max_length=64)
     favourite_pl_0: str | None = Field(None, max_length=64)

src/officers/tables.py

@@ -22,7 +22,7 @@
 )
 from database import Base
 from officers.constants import OFFICER_LEGAL_NAME_MAX, OFFICER_POSITION_MAX, OfficerPositionEnum
-from officers.models import OfficerInfo, OfficerTermUpdate, OfficerUpdate
+from officers.models import OfficerInfo, OfficerTerm, OfficerTermUpdate, OfficerUpdate
 
 
 # A row represents an assignment of a person to a position.
@@ -55,20 +55,7 @@ class OfficerTermDB(Base):
     __table_args__ = (UniqueConstraint("computing_id", "position", "start_date"),)  # This needs a comma to work
 
     def serializable_dict(self) -> dict:
-        return {
-            "id": self.id,
-            "computing_id": self.computing_id,
-            "position": self.position,
-            "start_date": self.start_date.isoformat() if self.start_date is not None else None,
-            "end_date": self.end_date.isoformat() if self.end_date is not None else None,
-            "nickname": self.nickname,
-            "favourite_course_0": self.favourite_course_0,
-            "favourite_course_1": self.favourite_course_1,
-            "favourite_pl_0": self.favourite_pl_0,
-            "favourite_pl_1": self.favourite_pl_1,
-            "biography": self.biography,
-            "photo_url": self.photo_url,
-        }
+        return OfficerTerm.model_validate(self).model_dump(mode="json")
 
     def update_from_params(self, params: OfficerTermUpdate, admin_update: bool = True):
         if admin_update:

src/officers/urls.py

@@ -5,7 +5,7 @@
 import database
 import officers.crud
 import utils
-from dependencies import LoggedInUser, perm_admin
+from dependencies import LoggedInUser, SessionUser, perm_admin
 from officers.constants import OfficerPositionEnum
 from officers.models import (
     OfficerCreate,
@@ -16,7 +16,6 @@
     OfficerTermUpdate,
     OfficerUpdate,
 )
-from officers.tables import OfficerInfoDB
 from permission.types import OfficerPrivateInfo
 from utils.permissions import is_user_website_admin, verify_update
 from utils.shared_models import DetailModel, SuccessResponse
@@ -112,21 +111,18 @@ async def all_officers(
     operation_id="get_officer_terms_by_id",
 )
 async def get_officer_terms(
-    request: Request, db_session: database.DBSession, computing_id: str, include_future_terms: bool = False
+    user_id: SessionUser, db_session: database.DBSession, computing_id: str, include_future_terms: bool = False
 ):
     if include_future_terms:
-        await verify_update(request, db_session, computing_id)
-        # _, session_computing_id = await get_user(request, db_session)
-        # if computing_id != session_computing_id and not is_user_website_admin(session_computing_id, db_session):
-        #     raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="not authorized")
+        await verify_update(user_id, db_session, computing_id)
 
     # all term info is public, so anyone can get any of it
     officer_terms = await officers.crud.get_officer_terms(db_session, computing_id, include_future_terms)
     return JSONResponse([OfficerTerm.model_validate(term).model_dump(mode="json") for term in officer_terms])
 
 
 @router.get(
-    "/info/{computing_id:str}",
+    "/info/{computing_id}",
     description="Get officer info for the current user, if they've ever been an exec. Only admins can get info about another user.",
     response_model=OfficerInfo,
     responses={403: {"description": "not authorized to view author user info", "model": DetailModel}},
@@ -163,9 +159,10 @@ async def create_officer_term(
     officer_list: list[OfficerCreate],
 ):
     new_terms = await officers.crud.create_multiple_officers(db_session, officer_list)
+    content = [term.serializable_dict() for term in new_terms]
 
     await db_session.commit()
-    return JSONResponse([OfficerTerm.model_validate(term).model_dump(mode="json") for term in new_terms])
+    return JSONResponse(content)
 
 
 @router.patch(
@@ -183,12 +180,12 @@ async def create_officer_term(
     operation_id="update_officer_info",
 )
 async def update_officer_info(
-    request: Request,
+    user_id: SessionUser,
     db_session: database.DBSession,
     computing_id: str,
     officer_info_upload: OfficerUpdate,
 ):
-    await verify_update(request, db_session, computing_id)
+    await verify_update(user_id, db_session, computing_id)
 
     old_officer_info = await officers.crud.get_officer_info_or_raise(db_session, computing_id)
     old_officer_info.update_from_params(officer_info_upload)
@@ -221,8 +218,9 @@ async def update_officer_term(db_session: database.DBSession, term_id: int, body
 
     old_officer_term = await officers.crud.get_officer_term_by_id_or_raise(db_session, term_id)
 
-    if utils.is_past_term(old_officer_term):
-        raise HTTPException(status_code=403, detail="you may not update past terms")
+    # TODO: Enable this check if we allow non-website admins to change their information
+    # if utils.is_past_term(old_officer_term):
+    #     raise HTTPException(status_code=403, detail="you may not update past terms")
 
     new_data = body.model_dump(exclude_unset=True)