fix(Officers): merged the public and private models

Author: jbriones1

src/officers/crud.py

@@ -10,15 +10,13 @@
 from auth.tables import SiteUserDB
 from data import semesters
 from officers.constants import OfficerPosition, OfficerPositionEnum
-from officers.models import OfficerCreate, OfficerPrivate, OfficerPublic
+from officers.models import Officer, OfficerCreate
 from officers.tables import OfficerInfoDB, OfficerTermDB
 
 # NOTE: this module should not do any data validation; that should be done in the urls.py or higher layer
 
 
-async def current_officers(
-    db_session: database.DBSession, include_private: bool = False
-) -> list[OfficerPrivate] | list[OfficerPublic]:
+async def current_officers(db_session: database.DBSession, include_private: bool = False) -> list[Officer]:
     """
     Get info about officers that are active. Go through all active & complete officer terms.
     """
@@ -35,7 +33,7 @@ async def current_officers(
     if include_private:
         for term, officer in result:
             officer_list.append(
-                OfficerPrivate(
+                Officer(
                     legal_name=officer.legal_name,
                     is_active=True,
                     position=term.position,
@@ -55,17 +53,7 @@ async def current_officers(
             )
     else:
         for term, officer in result:
-            officer_list.append(
-                OfficerPublic(
-                    legal_name=officer.legal_name,
-                    is_active=True,
-                    position=term.position,
-                    start_date=term.start_date,
-                    end_date=term.end_date,
-                    biography=term.biography,
-                    csss_email=OfficerPosition.to_email(term.position),
-                )
-            )
+            officer_list.append(Officer.public_fields(term, officer))
 
     return officer_list
 
@@ -96,7 +84,7 @@ async def get_current_terms_by_position(
 
 async def get_all_officers(
     db_session: AsyncSession, include_future_terms: bool, include_private: bool
-) -> list[OfficerPrivate] | list[OfficerPublic]:
+) -> list[Officer]:
     """
     This could be a lot of data, so be careful
     """
@@ -114,7 +102,7 @@ async def get_all_officers(
     if include_private:
         for term, officer in result:
             officer_list.append(
-                OfficerPrivate(
+                Officer(
                     legal_name=officer.legal_name,
                     is_active=utils.is_active_term(term),
                     position=term.position,
@@ -134,17 +122,7 @@ async def get_all_officers(
             )
     else:
         for term, officer in result:
-            officer_list.append(
-                OfficerPublic(
-                    legal_name=officer.legal_name,
-                    is_active=utils.is_active_term(term),
-                    position=term.position,
-                    start_date=term.start_date,
-                    end_date=term.end_date,
-                    biography=term.biography,
-                    csss_email=OfficerPosition.to_email(term.position),
-                )
-            )
+            officer_list.append(Officer.public_fields(term, officer))
 
     return officer_list
 

src/officers/models.py

@@ -1,9 +1,11 @@
 from datetime import date
+from typing import Self
 
 from pydantic import BaseModel, ConfigDict, Field
 
 from constants import COMPUTING_ID_LEN
-from officers.constants import OFFICER_LEGAL_NAME_MAX, OfficerPositionEnum
+from officers.constants import OFFICER_LEGAL_NAME_MAX, OfficerPosition, OfficerPositionEnum
+from officers.tables import OfficerInfoDB, OfficerTermDB
 
 OFFICER_PRIVATE_INFO = {
     "discord_id",
@@ -83,23 +85,26 @@ class OfficerBase(BaseModel):
     csss_email: str | None = None
 
 
-class OfficerPublic(OfficerBase):
-    """
-    Response when fetching public officer data
-    """
+class Officer(OfficerBase):
+    @classmethod
+    def public_fields(cls, term: OfficerTermDB, info: OfficerInfoDB) -> Self:
+        return cls(
+            legal_name=info.legal_name,
+            is_active=True,
+            position=term.position,
+            start_date=term.start_date,
+            end_date=term.end_date,
+            biography=term.biography,
+            csss_email=OfficerPosition.to_email(term.position),
+        )
 
     is_active: bool
 
-
-class OfficerPrivate(OfficerPublic):
-    """
-    Response when fetching private officer data
-    """
-
+    # Private Info
     discord_id: str | None = None
     discord_name: str | None = None
     discord_nickname: str | None = None
-    computing_id: str
+    computing_id: str | None = None
     phone_number: str | None = None
     github_username: str | None = None
     google_drive_email: str | None = None

src/officers/tables.py

@@ -22,7 +22,6 @@
 )
 from database import Base
 from officers.constants import OFFICER_LEGAL_NAME_MAX, OFFICER_POSITION_MAX, OfficerPositionEnum
-from officers.models import OfficerInfo, OfficerTerm, OfficerTermUpdate, OfficerUpdate
 
 
 # A row represents an assignment of a person to a position.
@@ -54,19 +53,6 @@ class OfficerTermDB(Base):
 
     __table_args__ = (UniqueConstraint("computing_id", "position", "start_date"),)  # This needs a comma to work
 
-    def serializable_dict(self) -> dict:
-        return OfficerTerm.model_validate(self).model_dump(mode="json")
-
-    def update_from_params(self, params: OfficerTermUpdate, admin_update: bool = True):
-        if admin_update:
-            update_data = params.model_dump(exclude_unset=True)
-        else:
-            update_data = params.model_dump(
-                exclude_unset=True, exclude={"position", "start_date", "end_date", "photo_url"}
-            )
-        for k, v in update_data.items():
-            setattr(self, k, v)
-
     def is_filled_in(self):
         return (
             # photo & end_date don't have to be uploaded for the term to be "filled"
@@ -131,9 +117,6 @@ class OfficerInfoDB(Base):
     # TODO (#22): add support for giving executives bitwarden access automagically
     # has_signed_into_bitwarden: Mapped[str] = mapped_column(Boolean)
 
-    def serializable_dict(self) -> dict[str, Any]:
-        return OfficerInfo.model_validate(self).model_dump(mode="json")
-
     def update_from_params(self, params: OfficerUpdate | OfficerUpdate):
         update_data = params.model_dump(exclude_unset=True)
         for k, v in update_data.items():

src/officers/urls.py

@@ -6,10 +6,9 @@
 import officers.crud
 from dependencies import LoggedInUser, SessionUser, perm_admin
 from officers.models import (
+    Officer,
     OfficerCreate,
     OfficerInfo,
-    OfficerPrivate,
-    OfficerPublic,
     OfficerTerm,
     OfficerTermUpdate,
     OfficerUpdate,
@@ -54,7 +53,7 @@ async def _has_officer_private_info_access(
 @router.get(
     "/current",
     description="Get information about the current officers. With no authorization, only get basic info.",
-    response_model=list[OfficerPrivate] | list[OfficerPublic],
+    response_model=list[Officer],
     operation_id="get_current_officers",
 )
 async def current_officers(
@@ -65,15 +64,13 @@ async def current_officers(
 
     curr_officers = await officers.crud.current_officers(db_session, has_private_access)
 
-    res = [o.model_dump(mode="json") for o in curr_officers]
-
-    return JSONResponse(res)
+    return JSONResponse([o.model_dump(mode="json", exclude_unset=True) for o in curr_officers])
 
 
 @router.get(
     "/all",
     description="Information for all execs from all exec terms",
-    response_model=list[OfficerPrivate] | list[OfficerPublic],
+    response_model=list[Officer],
     responses={403: {"description": "not authorized", "model": DetailModel}},
     operation_id="get_all_officers",
 )
@@ -90,7 +87,7 @@ async def all_officers(
 
     all_officers = await officers.crud.get_all_officers(db_session, include_future_terms, has_private_access)
 
-    return JSONResponse([officer_data.model_dump(mode="json") for officer_data in all_officers])
+    return JSONResponse([officer_data.model_dump(mode="json", exclude_unset=True) for officer_data in all_officers])
 
 
 @router.get(
@@ -114,7 +111,9 @@ async def get_officer_terms(
 
     # 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])
+    return JSONResponse(
+        [OfficerTerm.model_validate(term).model_dump(mode="json", exclude_unset=True) for term in officer_terms]
+    )
 
 
 @router.get(
@@ -133,7 +132,7 @@ async def get_officer_info(
         raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="not authorized")
 
     officer_info = await officers.crud.get_officer_info_or_raise(db_session, computing_id)
-    return JSONResponse(officer_info.serializable_dict())
+    return JSONResponse(OfficerInfo.model_validate(officer_info).model_dump(mode="json", exclude_unset=True))
 
 
 @router.post(
@@ -155,7 +154,7 @@ 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]
+    content = [OfficerTerm.model_validate(term).model_dump(mode="json", exclude_unset=True) for term in new_terms]
 
     await db_session.commit()
     return JSONResponse(content)
@@ -184,15 +183,17 @@ async def update_officer_info(
     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)
+    update_data = officer_info_upload.model_dump(exclude_unset=True)
+    for k, v in update_data.items():
+        setattr(old_officer_info, k, v)
     await officers.crud.update_officer_info(db_session, old_officer_info)
 
     # TODO (#27): log all important changes just to a .log file & persist them for a few years
 
     await db_session.commit()
 
     updated_officer_info = await officers.crud.get_new_officer_info_or_raise(db_session, computing_id)
-    return JSONResponse(updated_officer_info.serializable_dict())
+    return JSONResponse(OfficerInfo.model_validate(updated_officer_info).model_dump(mode="json", exclude_unset=True))
 
 
 @router.patch(
@@ -229,7 +230,7 @@ async def update_officer_term(db_session: database.DBSession, term_id: int, body
     await db_session.commit()
     await db_session.refresh(old_officer_term)
 
-    return JSONResponse(OfficerTerm.model_validate(old_officer_term).model_dump(mode="json"))
+    return JSONResponse(OfficerTerm.model_validate(old_officer_term).model_dump(mode="json", exclude_unset=True))
 
 
 @router.delete(