remove comments, make names more consistent

Author: EarthenSky

src/cron/daily.py

@@ -7,7 +7,7 @@
 import google_api
 import utils
 from database import _db_session
-from officers.crud import all_officer_data, get_user_by_username
+from officers.crud import all_officers, get_user_by_username
 
 _logger = logging.getLogger(__name__)
 
@@ -18,7 +18,7 @@ async def update_google_permissions(db_session):
 
     # TODO: for performance, only include officers with recent end-date (1 yr)
     # but measure performance first
-    for term in await all_officer_data(db_session):
+    for term in await all_officers(db_session):
         if utils.is_active(term):
             # TODO: if google drive permission is not active, update them
             pass
@@ -31,7 +31,7 @@ async def update_google_permissions(db_session):
 async def update_github_permissions(db_session):
     github_permissions, team_id_map = github.all_permissions()
 
-    for term in await all_officer_data(db_session):
+    for term in await all_officers(db_session):
         new_teams = (
             # move all active officers to their respective teams
             github.officer_teams(term.position)

src/officers/crud.py

@@ -90,7 +90,10 @@ async def get_officer_terms(
 
     return (await db_session.scalars(query)).all()
 
-async def current_executive_team(db_session: database.DBSession, include_private: bool) -> dict[str, list[OfficerData]]:
+async def current_officers(
+    db_session: database.DBSession,
+    include_private: bool
+) -> dict[str, list[OfficerData]]:
     """
     Get info about officers that are active. Go through all active & complete officer terms.
 
@@ -104,17 +107,8 @@ async def current_executive_team(db_session: database.DBSession, include_private
     query = utils.is_active_officer(query)
 
     officer_terms = (await db_session.scalars(query)).all()
-    ##num_officers = {}
     officer_data = {}
     for term in officer_terms:
-        """
-        if term.position not in OfficerPosition.position_list():
-            _logger.warning(
-                f"Unknown OfficerTerm.position={term.position} in database. Ignoring in request."
-            )
-            continue
-        """
-
         officer_info_query = (
             sqlalchemy
             .select(OfficerInfo)
@@ -124,43 +118,16 @@ async def current_executive_team(db_session: database.DBSession, include_private
         if officer_info is None:
             # TODO: make sure there are daily checks that this data actually exists
             continue
-
-        if term.position not in officer_data:
-            ##num_officers[term.position] = 0
+        elif term.position not in officer_data:
             officer_data[term.position] = []
 
-        """
-        num_officers[term.position] += 1
-        # TODO: move this to a daily cronjob
-        if num_officers[term.position] > OfficerPosition.num_active(term.position):
-            # If there are more active positions than expected, log it to a file
-            _logger.warning(
-                f"There are more active {term.position} positions in the OfficerTerm than expected "
-                f"({num_officers[term.position]} > {OfficerPosition.num_active(term.position)})"
-            )
-        """
-        officer_data[term.position] += [OfficerData.from_data(term, officer_info, include_private, is_active=True)]
+        officer_data[term.position] += [
+            OfficerData.from_data(term, officer_info, include_private, is_active=True)
+        ]
 
-    """
-    # validate & warn if there are any data issues
-    # TODO: decide whether we should enforce empty instances or force the frontend to deal with it
-    for position in OfficerPosition.expected_positions():
-        if position not in officer_data:
-            _logger.warning(
-                f"Expected position={position} in response current_executive_team."
-            )
-        elif (
-            OfficerPosition.num_active(position) is not None
-            and len(officer_data[position]) != OfficerPosition.num_active(position)
-        ):
-            _logger.warning(
-                f"Unexpected number of {position} entries "
-                f"({len(officer_data[position])} entries) in current_executive_team response."
-            )
-    """
     return officer_data
 
-async def all_officer_data(
+async def all_officers(
     db_session: database.DBSession,
     include_private_data: bool,
     view_not_started_officer_terms: bool,

src/officers/urls.py

@@ -25,6 +25,9 @@
     tags=["officers"],
 )
 
+# ---------------------------------------- #
+# checks
+
 async def has_officer_private_info_access(
     request: Request,
     db_session: database.DBSession,
@@ -41,6 +44,9 @@ async def has_officer_private_info_access(
     has_private_access = await OfficerPrivateInfo.has_permission(db_session, computing_id)
     return session_id, computing_id, has_private_access
 
+# ---------------------------------------- #
+# endpoints
+
 @router.get(
     "/current",
     description="Get information about all the officers. More information is given if you're authenticated & have access to private executive data.",
@@ -51,12 +57,11 @@ async def current_officers(
     db_session: database.DBSession,
 ):
     _, _, has_private_access = await has_officer_private_info_access(request, db_session)
-
-    current_executives = await officers.crud.current_executive_team(db_session, has_private_access)
+    current_officers = await officers.crud.current_officers(db_session, has_private_access)
     json_current_executives = {
         position: [
             officer_data.serializable_dict() for officer_data in officer_data_list
-        ] for position, officer_data_list in current_executives.items()
+        ] for position, officer_data_list in current_officers.items()
     }
 
     return JSONResponse(json_current_executives)
@@ -78,10 +83,10 @@ async def all_officers(
         if not is_website_admin:
             raise HTTPException(status_code=401, detail="only website admins can view all executive terms that have not started yet")
 
-    all_officer_data = await officers.crud.all_officer_data(db_session, has_private_access, not view_not_started_officer_terms)
+    all_officers = await officers.crud.all_officers(db_session, has_private_access, not view_not_started_officer_terms)
     return JSONResponse([
         officer_data.serializable_dict()
-        for officer_data in all_officer_data
+        for officer_data in all_officers
     ])
 
 @router.get(

tests/integration/test_officers.py

@@ -5,7 +5,7 @@
 import load_test_db
 from database import SQLALCHEMY_TEST_DATABASE_URL, DatabaseSessionManager
 from officers.constants import OfficerPosition
-from officers.crud import all_officer_data, current_executive_team, most_recent_officer_term
+from officers.crud import all_officers, current_officers, most_recent_officer_term
 
 # TODO: setup a database on the CI machine & run this as a unit test then (since
 # this isn't really an integration test)
@@ -39,15 +39,15 @@ async def test__read_execs(database_setup):
         assert abc11_officer_term.favourite_course_0 == "CMPT 361"
         assert abc11_officer_term.biography == "Hi! I'm person A and I want school to be over ; _ ;"
 
-        current_exec_team = await current_executive_team(db_session, include_private=False)
+        current_exec_team = await current_officers(db_session, include_private=False)
         assert current_exec_team is not None
         assert len(current_exec_team.keys()) == 1
         assert next(iter(current_exec_team.keys())) == OfficerPosition.PRESIDENT
         assert next(iter(current_exec_team.values()))[0].favourite_course_0 == "CMPT 999"
         assert next(iter(current_exec_team.values()))[0].csss_email == OfficerPosition.President.to_email()
         assert next(iter(current_exec_team.values()))[0].private_data is None
 
-        current_exec_team = await current_executive_team(db_session, include_private=True)
+        current_exec_team = await current_officers(db_session, include_private=True)
         assert current_exec_team is not None
         assert len(current_exec_team) == 1
         assert next(iter(current_exec_team.keys())) == OfficerPosition.PRESIDENT
@@ -56,7 +56,7 @@ async def test__read_execs(database_setup):
         assert next(iter(current_exec_team.values()))[0].private_data is not None
         assert next(iter(current_exec_team.values()))[0].private_data.computing_id == "abc33"
 
-        all_terms = await all_officer_data(db_session, include_private=True)
+        all_terms = await all_officers(db_session, include_private=True)
         assert len(all_terms) == 3