From ef9eb822bccec4aa3693d662dfc02ca2fa0fa953 Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Tue, 20 Aug 2024 01:11:30 -0700
Subject: [PATCH 01/29] take in new officer terms in a single transaction

---
 src/officers/urls.py | 50 +++++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/src/officers/urls.py b/src/officers/urls.py
index 4abb874..033852c 100755
--- a/src/officers/urls.py
+++ b/src/officers/urls.py
@@ -1,5 +1,6 @@
 import logging
-from datetime import datetime
+from dataclasses import dataclass
+from datetime import date, datetime
 
 import auth.crud
 import database
@@ -92,6 +93,12 @@ async def get_officer_terms(
     officer_terms = await officers.crud.officer_terms(db_session, computing_id, max_terms, hide_filled_in=True)
     return JSONResponse([term.serializable_dict() for term in officer_terms])
 
+@dataclass
+class InitialOfficerInfo:
+    computing_id: str
+    position: str
+    start_date: date
+
 @router.post(
     "/new_term",
     description="Only the sysadmin, president, or DoA can submit this request. It will usually be the DoA. Updates the system with a new officer, and enables the user to login to the system to input their information.",
@@ -99,33 +106,34 @@ async def get_officer_terms(
 async def new_officer_term(
     request: Request,
     db_session: database.DBSession,
-    computing_id: str = Body(), # request body
-    position: str = Body(),
-    start_date: str = Body(), # basic iso date format
+    # TODO: minimize number of transactions, so we can fail more easily
+    officer_info_list: list[InitialOfficerInfo] = Body(),  # noqa: B008
 ):
     """
     If the current computing_id is not already an officer, officer_info will be created for them.
     """
-    if len(computing_id) > COMPUTING_ID_MAX:
-        raise HTTPException(status_code=400, detail=f"computing_id={computing_id} is too large")
-    elif position not in OfficerPosition.__members__.values():
-        raise HTTPException(status_code=400, detail=f"invalid position={position}")
-    elif not is_iso_format(start_date):
-        raise HTTPException(status_code=400, detail=f"start_date={start_date} must be a valid iso date")
+    for officer_info in officer_info_list:
+        if len(officer_info.computing_id) > COMPUTING_ID_MAX:
+            raise HTTPException(status_code=400, detail=f"computing_id={officer_info.computing_id} is too large")
+        elif officer_info.position not in OfficerPosition.__members__.values():
+            raise HTTPException(status_code=400, detail=f"invalid position={officer_info.position}")
+        elif not is_iso_format(officer_info.start_date):
+            raise HTTPException(status_code=400, detail=f"start_date={officer_info.start_date} must be a valid iso date")
 
     WebsiteAdmin.validate_request(db_session, request)
 
-    officers.crud.create_new_officer_info(db_session, OfficerInfoData(
-        computing_id = computing_id,
-    ))
-    success = officers.crud.create_new_officer_term(db_session, OfficerTermData(
-        computing_id = computing_id,
-        position = position,
-        # TODO: remove the hours & seconds (etc.) from start_date
-        start_date = start_date,
-    ))
-    if not success:
-        raise HTTPException(status_code=400, detail="Officer term already exists, no changes made")
+    for officer_info in officer_info_list:
+        officers.crud.create_new_officer_info(db_session, OfficerInfoData(
+            computing_id = officer_info.computing_id,
+        ))
+        success = officers.crud.create_new_officer_term(db_session, OfficerTermData(
+            computing_id = officer_info.computing_id,
+            position = officer_info.position,
+            # TODO: remove the hours & seconds (etc.) from start_date
+            start_date = officer_info.start_date,
+        ))
+        if not success:
+            raise HTTPException(status_code=400, detail="Officer term already exists, no changes made")
 
     await db_session.commit()
     return PlainTextResponse("ok")

From 73d8c4e68c6d2981ab22568f1e21f35b4b921aa7 Mon Sep 17 00:00:00 2001
From: EarthenSky <24978329+EarthenSky@users.noreply.github.com>
Date: Wed, 21 Aug 2024 17:30:43 -0700
Subject: [PATCH 02/29] add daily cron file for updating officer permissions

---
 config/cron.sh       |  6 ++++++
 src/cron/daily.py    | 39 +++++++++++++++++++++++++++++++++++++++
 src/officers/crud.py |  4 +++-
 3 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100644 config/cron.sh
 create mode 100644 src/cron/daily.py

diff --git a/config/cron.sh b/config/cron.sh
new file mode 100644
index 0000000..0660237
--- /dev/null
+++ b/config/cron.sh
@@ -0,0 +1,6 @@
+# This script adds commands to the current crontab
+
+# run the daily script at 1am every morning
+# TODO: make sure timezone is PST
+crontab -l | { cat; echo "0 1 * * * /home/csss-site/csss-site-backend/src/cron/daily.py"; } | crontab -
+
diff --git a/src/cron/daily.py b/src/cron/daily.py
new file mode 100644
index 0000000..9501688
--- /dev/null
+++ b/src/cron/daily.py
@@ -0,0 +1,39 @@
+"""This module gets called by cron every day"""
+
+import asyncio
+import logging
+
+from database import _db_session
+from officers.crud import officer_terms
+
+_logger = logging.getLogger(__name__)
+
+async def update_permissions():
+    db_session = _db_session()
+
+    # TODO: get current github permissions
+
+    # TODO: get current google drive permissions
+
+    one_year_ago = datetime.today() - timedelta(days=365)
+
+    # TODO: for performance, only include officers with recent end-date (1 yr)
+    all_officer_terms = await all_officer_terms(db_session)
+    for term in all_officer_terms:
+        if utils.is_active(term):
+            # TODO: if google drive permissions is not active, update them
+            # TODO: if github permissions is not active, update them
+            pass
+        elif utils.end_date <= one_year_ago:
+            # ignore old executives
+            continue
+        else:
+            # TODO: if google drive permissions are active, remove them
+            # TODO: if github permissions are active, remove them
+            pass 
+
+    _logger.info("Complete permissions update")
+
+if __name__ == "__main__":
+    asyncio.run(update_permissions())
+
diff --git a/src/officers/crud.py b/src/officers/crud.py
index 9fe91df..6c22281 100644
--- a/src/officers/crud.py
+++ b/src/officers/crud.py
@@ -35,6 +35,7 @@ async def current_officer_position(db_session: database.DBSession, computing_id:
     """
     Returns None if the user is not currently an officer
     """
+
     query = sqlalchemy.select(OfficerTerm)
     query = query.where(OfficerTerm.computing_id == computing_id)
     query = utils.is_active_officer(query)
@@ -153,7 +154,8 @@ async def all_officer_terms(
             OfficerInfo.computing_id == term.computing_id
         )
         officer_info = await db_session.scalar(officer_info_query)
-
+        
+        # TODO: remove is_active from the database
         is_active = (term.end_date is None) or (datetime.today() <= term.end_date)
         officer_data_list += [OfficerData.from_data(term, officer_info, include_private, is_active)]
 

From 29083cbe0d2443ed30f8bf881cc30370008336a0 Mon Sep 17 00:00:00 2001
From: EarthenSky <24978329+EarthenSky@users.noreply.github.com>
Date: Wed, 21 Aug 2024 18:01:27 -0700
Subject: [PATCH 03/29] update daily

---
 src/cron/daily.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/cron/daily.py b/src/cron/daily.py
index 9501688..e8b4b00 100644
--- a/src/cron/daily.py
+++ b/src/cron/daily.py
@@ -6,14 +6,16 @@
 from database import _db_session
 from officers.crud import officer_terms
 
+import github
+import google
+
 _logger = logging.getLogger(__name__)
 
 async def update_permissions():
     db_session = _db_session()
 
-    # TODO: get current github permissions
-
-    # TODO: get current google drive permissions
+    google_permissions = google.current_permissions()
+    github_permissions = github.current_permissions()
 
     one_year_ago = datetime.today() - timedelta(days=365)
 

From 5741b66510513a12b6e2b63ce8c143ea62a6c616 Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Wed, 21 Aug 2024 21:25:52 -0700
Subject: [PATCH 04/29] fix legal name bug

---
 src/database.py       |  3 ++-
 src/officers/types.py |  2 +-
 src/officers/urls.py  | 11 ++++++-----
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/database.py b/src/database.py
index c05eaff..12aac10 100644
--- a/src/database.py
+++ b/src/database.py
@@ -66,7 +66,8 @@ async def session(self) -> AsyncIterator[AsyncSession]:
         if self._sessionmaker is None:
             raise Exception("DatabaseSessionManager is not initialized")
 
-        session = self._sessionmaker()
+        # autoflush off means that flush will not be triggered....
+        session = self._sessionmaker(autoflush=False)
         try:
             yield session
         except Exception:
diff --git a/src/officers/types.py b/src/officers/types.py
index 26510a6..093cab7 100644
--- a/src/officers/types.py
+++ b/src/officers/types.py
@@ -16,7 +16,7 @@
 class OfficerInfoData:
     computing_id: str
 
-    legal_name: None | str = None
+    legal_name: str
     discord_id: None | str = None # TODO: do we need this to get info about a person
     discord_name: None | str = None
     discord_nickname: None | str = None
diff --git a/src/officers/urls.py b/src/officers/urls.py
index 033852c..3b871e0 100755
--- a/src/officers/urls.py
+++ b/src/officers/urls.py
@@ -115,18 +115,19 @@ async def new_officer_term(
     for officer_info in officer_info_list:
         if len(officer_info.computing_id) > COMPUTING_ID_MAX:
             raise HTTPException(status_code=400, detail=f"computing_id={officer_info.computing_id} is too large")
-        elif officer_info.position not in OfficerPosition.__members__.values():
+        elif officer_info.position not in OfficerPosition.position_values():
             raise HTTPException(status_code=400, detail=f"invalid position={officer_info.position}")
-        elif not is_iso_format(officer_info.start_date):
-            raise HTTPException(status_code=400, detail=f"start_date={officer_info.start_date} must be a valid iso date")
 
     WebsiteAdmin.validate_request(db_session, request)
 
     for officer_info in officer_info_list:
-        officers.crud.create_new_officer_info(db_session, OfficerInfoData(
+        # TODO: fix a bug with this stuff & test inserting & viewing mutliple executives
+        await officers.crud.create_new_officer_info(db_session, OfficerInfoData(
+            # TODO: use sfu api to get legal name
+            legal_name = "default name",
             computing_id = officer_info.computing_id,
         ))
-        success = officers.crud.create_new_officer_term(db_session, OfficerTermData(
+        success = await officers.crud.create_new_officer_term(db_session, OfficerTermData(
             computing_id = officer_info.computing_id,
             position = officer_info.position,
             # TODO: remove the hours & seconds (etc.) from start_date

From 407993c44991b9fe0e21cd7caba0b5dca2c25726 Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Wed, 21 Aug 2024 21:27:42 -0700
Subject: [PATCH 05/29] don't touch autoflush

---
 src/database.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/database.py b/src/database.py
index 12aac10..c05eaff 100644
--- a/src/database.py
+++ b/src/database.py
@@ -66,8 +66,7 @@ async def session(self) -> AsyncIterator[AsyncSession]:
         if self._sessionmaker is None:
             raise Exception("DatabaseSessionManager is not initialized")
 
-        # autoflush off means that flush will not be triggered....
-        session = self._sessionmaker(autoflush=False)
+        session = self._sessionmaker()
         try:
             yield session
         except Exception:

From 5e645eb08640f7653e759204e83693e5fdaef624 Mon Sep 17 00:00:00 2001
From: EarthenSky <24978329+EarthenSky@users.noreply.github.com>
Date: Thu, 22 Aug 2024 17:05:40 -0700
Subject: [PATCH 06/29] add google module

---
 src/google/__init__.py | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 src/google/__init__.py

diff --git a/src/google/__init__.py b/src/google/__init__.py
new file mode 100644
index 0000000..e69de29

From e70a981c9b69860cbb99c045d4ecb697b4a4feb3 Mon Sep 17 00:00:00 2001
From: EarthenSky <24978329+EarthenSky@users.noreply.github.com>
Date: Thu, 22 Aug 2024 17:10:55 -0700
Subject: [PATCH 07/29] small reformatting of github module

---
 src/github/github.py | 89 +++++++++++++++++++++++++-------------------
 1 file changed, 51 insertions(+), 38 deletions(-)

diff --git a/src/github/github.py b/src/github/github.py
index 15ae943..b2dbfbe 100644
--- a/src/github/github.py
+++ b/src/github/github.py
@@ -23,8 +23,8 @@ class GithubTeam:
     slug: str
 
 async def _github_request_get(
-        url: str,
-        token: str
+    url: str,
+    token: str
 ) -> Response | None:
     result = requests.get(
         url,
@@ -42,9 +42,9 @@ async def _github_request_get(
     return result
 
 async def _github_request_post(
-        url: str,
-        token: str,
-        post_data: Any
+    url: str,
+    token: str,
+    post_data: Any
 ) -> Response | None:
     result = requests.post(
         url,
@@ -63,15 +63,16 @@ async def _github_request_post(
     return result
 
 async def _github_request_delete(
-        url: str,
-        token: str
+    url: str,
+    token: str
 ) -> Response | None:
     result = requests.delete(
         url,
-        headers={
+        headers = {
             "Accept": "application/vnd.github+json",
             "Authorization": f"Bearer {token}",
-            "X-GitHub-Api-Version": "2022-11-28"}
+            "X-GitHub-Api-Version": "2022-11-28"
+        }
     )
     rate_limit_remaining = int(result.headers["x-ratelimit-remaining"])
     if rate_limit_remaining < 50:
@@ -81,9 +82,9 @@ async def _github_request_delete(
     return result
 
 async def _github_request_put(
-        url: str,
-        token: str,
-        put_data: Any
+    url: str,
+    token: str,
+    put_data: Any
 ) -> Response | None:
     result = requests.put(
         url,
@@ -101,7 +102,7 @@ async def _github_request_put(
     return result
 
 async def get_user_by_username(
-        username: str
+    username: str
 ) -> GithubUser | None:
     """
         Takes in a Github username and returns an instance of GithubUser.
@@ -137,9 +138,9 @@ async def get_user_by_id(
         return GithubUser(result_json["login"], result_json["id"], result_json["name"])
 
 async def add_user_to_org(
-        org: str = github_org_name,
-        uid: str | None = None,
-        email: str | None = None
+    org: str = github_org_name,
+    uid: str | None = None,
+    email: str | None = None
 ) -> None:
     """
         Takes one of either uid or email. Fails if provided both.
@@ -151,57 +152,68 @@ async def add_user_to_org(
         raise ValueError("cannot populate both uid and email")
     # Arbitrarily prefer uid
     elif uid is not None:
-        result = await _github_request_post(f"https://api.github.com/orgs/{org}/invitations",
-                               os.environ.get("GITHUB_TOKEN"),
-                               dumps({"invitee_id":uid, "role":"direct_member"}))
+        result = await _github_request_post(
+            f"https://api.github.com/orgs/{org}/invitations",
+            os.environ.get("GITHUB_TOKEN"),
+            dumps({"invitee_id":uid, "role":"direct_member"})
+        )
     elif email is not None:
-        result = await _github_request_post(f"https://api.github.com/orgs/{org}/invitations",
-                               os.environ.get("GITHUB_TOKEN"),
-                               dumps({"email":email, "role":"direct_member"}))
-    result_json = result.json()
+        result = await _github_request_post(
+            f"https://api.github.com/orgs/{org}/invitations",
+            os.environ.get("GITHUB_TOKEN"),
+            dumps({"email":email, "role":"direct_member"})
+        )
+
     # Logging here potentially?
     if result.status_code != 201:
+        result_json = result.json()
         raise Exception(
             f"Status code {result.status_code} returned when attempting to add user to org: "
             f"{result_json['message']}: {[error['message'] for error in result_json['errors']]}"
         )
 
 async def delete_user_from_org(
-        username: str,
-        org: str = github_org_name
+    username: str,
+    org: str = github_org_name
 ) -> None:
     if username is None:
         raise Exception("Username cannot be empty")
-    result = await _github_request_delete(f"https://api.github.com/orgs/{org}/memberships/{username}",
-                                          os.environ.get("GITHUB_TOKEN"))
+    result = await _github_request_delete(
+        f"https://api.github.com/orgs/{org}/memberships/{username}",
+        os.environ.get("GITHUB_TOKEN")
+    )
+
     # Logging here potentially?
     if result.status_code != 204:
         raise Exception(f"Status code {result.status_code} returned when attempting to delete user {username} from organization {org}")
 
 async def get_teams(
-        org: str = github_org_name
+    org: str = github_org_name
 ) -> list[str]:
     result = await _github_request_get(f"https://api.github.com/orgs/{org}/teams", os.environ.get("GITHUB_TOKEN"))
     json_result = result.json()
     return [GithubTeam(team["id"], team["url"], team["name"], team["slug"]) for team in json_result]
 
 async def add_user_to_team(
-        username: str,
-        slug: str,
-        org: str = github_org_name
+    username: str,
+    slug: str,
+    org: str = github_org_name
 ) -> None:
-    result = await _github_request_put(f"https://api.github.com/orgs/{org}/teams/{slug}/memberships/{username}",
-                                        os.environ.get("GITHUB_TOKEN"),
-                                        dumps({"role":"member"}))
-    result_json = result.json()
+    result = await _github_request_put(
+        f"https://api.github.com/orgs/{org}/teams/{slug}/memberships/{username}",
+        os.environ.get("GITHUB_TOKEN"),
+        dumps({"role":"member"}),
+    )
+
     # Logging here potentially?
     if result.status_code != 200:
+        result_json = result.json()
         raise Exception(f"Status code {result.status_code} returned when attempting to add user to team: {result_json['message']}")
 
 async def remove_user_from_team(
-        username: str,
-        slug: str,
-        org: str = github_org_name
+    username: str,
+    slug: str,
+    org: str = github_org_name
 ) -> None:
     result = await _github_request_delete(
         f"https://api.github.com/orgs/{org}/teams/{slug}/memberships/{username}",
@@ -209,3 +221,4 @@ async def remove_user_from_team(
     )
     if result.status_code != 204:
         raise Exception(f"Status code {result.status_code} returned when attempting to delete user {username} from team {slug}")
+

From 5ec3b949255b6b7a32591c11df549ab0b25485a8 Mon Sep 17 00:00:00 2001
From: EarthenSky <24978329+EarthenSky@users.noreply.github.com>
Date: Thu, 22 Aug 2024 17:37:21 -0700
Subject: [PATCH 08/29] update github module & add email function

---
 src/admin/email.py                     | 24 ++++++++++++++++++
 src/github/__init__.py                 | 34 ++++++++++++++++++++++++++
 src/github/{github.py => internals.py} | 16 +-----------
 src/github/types.py                    | 25 +++++++++++++++++++
 4 files changed, 84 insertions(+), 15 deletions(-)
 create mode 100644 src/admin/email.py
 create mode 100644 src/github/__init__.py
 rename src/github/{github.py => internals.py} (96%)
 create mode 100644 src/github/types.py

diff --git a/src/admin/email.py b/src/admin/email.py
new file mode 100644
index 0000000..c1987b2
--- /dev/null
+++ b/src/admin/email.py
@@ -0,0 +1,24 @@
+import os
+import smtplib
+
+# TODO: set this up
+GMAIL_PASSWORD = os.environ['GMAIL_PASSWORD']
+GMAIL_ADDRESS = "csss-site@gmail.com"
+
+# TODO: look into sending emails from an sfu maillist (this might be painful)
+def send_email(
+    recipient_address: str,
+    subject: str,
+    contents: str,
+):
+    mail = smtplib.SMTP('smtp.gmail.com', 587)
+    mail.ehlo()
+    mail.starttls()
+    mail.login(GMAIL_ADDRESS, GMAIL_PASSWORD)
+
+    header = f"To: {recipient_address}\nFrom: {GMAIL_USERNAME}\nSubject: {subject}"
+    content = header + content
+
+    mail.sendmail(GMAIL_ADDRESS, recipient_address, content)
+    mail.quit()
+
diff --git a/src/github/__init__.py b/src/github/__init__.py
new file mode 100644
index 0000000..646cd54
--- /dev/null
+++ b/src/github/__init__.py
@@ -0,0 +1,34 @@
+# TODO: does this allow importing anything from the module?
+
+from github.internals import 
+from admin.email import send_email
+
+# TODO: move this to github.constants.py
+GITHUB_TEAMS = {
+    "doa" : "auto",
+    "election_officer": "auto",
+    "officers": "auto",
+    "w3_committee": "manual",
+    "wall_e": "manual",
+}
+
+# TODO: move these functions to github.public.py
+
+def current_permissions() -> list[GithubUserPermissions]:
+    person_list = []
+
+    # this function should return a list of members that have permisisons
+    
+    # get info for each person in an auto github team
+
+    # log warning if there are any unknown teams
+
+    # log error & email if there are any missing teams
+    # send_email("csss-sysadmin@sfu.ca", "ERROR: Missing Team", "...") 
+    pass
+
+def invite_user(github_username: str):
+    # invite this user to the github organization
+    pass
+
+
diff --git a/src/github/github.py b/src/github/internals.py
similarity index 96%
rename from src/github/github.py
rename to src/github/internals.py
index b2dbfbe..8892446 100644
--- a/src/github/github.py
+++ b/src/github/internals.py
@@ -1,5 +1,4 @@
 import os
-from dataclasses import dataclass
 from json import dumps
 from typing import Any
 
@@ -7,20 +6,7 @@
 from constants import github_org_name
 from requests import Response
 
-
-@dataclass
-class GithubUser:
-    username: str
-    id: int
-    name: str
-
-@dataclass
-class GithubTeam:
-    id: int
-    url: str
-    name: str
-    # slugs are the space-free special names that github likes to use
-    slug: str
+from github.types import GithubUser, GithubTeam
 
 async def _github_request_get(
     url: str,
diff --git a/src/github/types.py b/src/github/types.py
new file mode 100644
index 0000000..8db3e5a
--- /dev/null
+++ b/src/github/types.py
@@ -0,0 +1,25 @@
+from dataclasses import dataclass
+
+@dataclass
+class GithubUser:
+    username: str
+    id: int
+    name: str
+
+@dataclass
+class GithubTeam:
+    id: int
+    url: str
+    name: str
+    # slugs are the space-free special names that github likes to use
+    slug: str
+
+@dataclass
+class GithubUserPermissions:
+    # this class should store all the possible permissions a user might have
+
+    # used to connect the user to their officer info
+    username: str
+
+    # which github teams they're in
+    teams: list[str]

From f61167f8c3449b4d8844254694a25f6eb000ee35 Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Thu, 22 Aug 2024 20:07:38 -0700
Subject: [PATCH 09/29] add /auth/info endpoint

---
 src/auth/crud.py | 30 ++++++++++++++++++++++++++++--
 src/auth/urls.py | 23 +++++++++++++++++++++++
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/src/auth/crud.py b/src/auth/crud.py
index bfad678..2d12e1f 100644
--- a/src/auth/crud.py
+++ b/src/auth/crud.py
@@ -7,7 +7,7 @@
 from sqlalchemy.ext.asyncio import AsyncSession
 
 
-async def create_user_session(db_session: AsyncSession, session_id: str, computing_id: str) -> None:
+async def create_user_session(db_session: AsyncSession, session_id: str, computing_id: str):
     """
     Updates the past user session if one exists, so no duplicate sessions can ever occur.
 
@@ -84,9 +84,35 @@ async def get_computing_id(db_session: AsyncSession, session_id: str) -> str | N
 
 
 # remove all out of date user sessions
-async def task_clean_expired_user_sessions(db_session: AsyncSession) -> None:
+async def task_clean_expired_user_sessions(db_session: AsyncSession):
     one_day_ago = datetime.now() - timedelta(days=0.5)
 
     query = sqlalchemy.delete(UserSession).where(UserSession.issue_time < one_day_ago)
     await db_session.execute(query)
     await db_session.commit()
+
+
+async def user_info(db_session: AsyncSession, session_id: str) -> None | dict:
+    query = (
+        sqlalchemy
+        .select(UserSession)
+        .where(UserSession.session_id == session_id)
+    )
+    user_session = await db_session.scalar(query)
+    if user_session is None:
+        return None
+
+    query = (
+        sqlalchemy
+        .select(SiteUser)
+        .where(SiteUser.computing_id == user_session.computing_id)
+    )
+    user = await db_session.scalar(query)
+    if user is None:
+        return None
+
+    return {
+        "computing_id": user_session.computing_id,
+        "first_logged_in": user.first_logged_in.isoformat(),
+        "last_logged_in": user.last_logged_in.isoformat()
+    }
diff --git a/src/auth/urls.py b/src/auth/urls.py
index 15fef99..dba2157 100644
--- a/src/auth/urls.py
+++ b/src/auth/urls.py
@@ -74,6 +74,7 @@ async def login_user(
         return response
 
 
+# TODO: deprecate this when possible
 @router.get(
     "/check",
     description="Check if the current user is logged in based on session_id from cookies",
@@ -93,6 +94,28 @@ async def check_authentication(
     return JSONResponse(response_dict)
 
 
+@router.get(
+    "/info",
+    description="Get info about the current user. Only accessible by that user",
+)
+async def get_info(
+    request: Request,
+    db_session: database.DBSession,
+):
+    """
+    Currently this endpoint only returns the info stored in tables in the auth module.
+    """
+    session_id = request.cookies.get("session_id", None)
+    if session_id is None:
+        raise HTTPException(status_code=401, detail="User must be authenticated to get their info")
+
+    user_info = await crud.user_info(db_session, session_id)
+    if user_info is None:
+        raise HTTPException(status_code=500, detail="Could not find user with session_id")
+
+    return JSONResponse(user_info)
+
+
 @router.post(
     "/logout",
     description="Logs out the current user by invalidating the session_id cookie",

From 3d29dca2753102f855ea8e4bb5f6a0b2f12267a7 Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Thu, 22 Aug 2024 22:16:17 -0700
Subject: [PATCH 10/29] treat error correctly

---
 src/auth/urls.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/auth/urls.py b/src/auth/urls.py
index dba2157..9e6dbcb 100644
--- a/src/auth/urls.py
+++ b/src/auth/urls.py
@@ -111,7 +111,7 @@ async def get_info(
 
     user_info = await crud.user_info(db_session, session_id)
     if user_info is None:
-        raise HTTPException(status_code=500, detail="Could not find user with session_id")
+        raise HTTPException(status_code=401, detail="Could not find user with session_id, please log in")
 
     return JSONResponse(user_info)
 

From 19e3a9f501614568cdbeaadd2742b3f22cfd4855 Mon Sep 17 00:00:00 2001
From: EarthenSky <24978329+EarthenSky@users.noreply.github.com>
Date: Fri, 23 Aug 2024 09:20:44 -0700
Subject: [PATCH 11/29] add temp file for holding server secrets; todo: look
 into better methods?

---
 config/env.sh | 10 ++++++++++
 1 file changed, 10 insertions(+)
 create mode 100644 config/env.sh

diff --git a/config/env.sh b/config/env.sh
new file mode 100644
index 0000000..41eab07
--- /dev/null
+++ b/config/env.sh
@@ -0,0 +1,10 @@
+# TODO: only fill out this file in production
+export GMAIL_USERNAME = "todo"
+export GMAIL_PASSWORD = "todo"
+export GOOGLE_DRIVE_TOKEN = "todo"
+export GITHUB_TOKEN = "todo"
+export DISCORD_TOKEN = "todo"
+
+export CSSS_GUILD_ID = "todo"
+export SFU_API_TOKEN = "todo"
+

From 1c6ae8285b32eec38378ca5f9484469aa90cced1 Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Fri, 23 Aug 2024 22:49:41 -0700
Subject: [PATCH 12/29] add /officers/my_info endpoint

---
 src/officers/crud.py |  9 +++++++++
 src/officers/urls.py | 22 ++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/src/officers/crud.py b/src/officers/crud.py
index 9fe91df..7dda781 100644
--- a/src/officers/crud.py
+++ b/src/officers/crud.py
@@ -46,6 +46,15 @@ async def current_officer_position(db_session: database.DBSession, computing_id:
     else:
         return officer_term.position
 
+async def officer_info(db_session: database.DBSession, computing_id: str) -> OfficerInfoData | None:
+    query = (
+        sqlalchemy
+        .select(OfficerInfo)
+        .where(OfficerInfo.computing_id == computing_id)
+    )
+    officer_term = await db_session.scalar(query)
+    return officer_term
+
 async def officer_terms(
     db_session: database.DBSession,
     computing_id: str,
diff --git a/src/officers/urls.py b/src/officers/urls.py
index 34a96e6..9e40788 100755
--- a/src/officers/urls.py
+++ b/src/officers/urls.py
@@ -97,6 +97,28 @@ async def get_officer_terms(
     officer_terms = await officers.crud.officer_terms(db_session, computing_id, max_terms, hide_filled_in=True)
     return JSONResponse([term.serializable_dict() for term in officer_terms])
 
+@router.get(
+    "/my_info",
+    description="Get officer info for the current user, if they've ever been an exec.",
+)
+async def get_officer_info(
+    request: Request,
+    db_session: database.DBSession,
+):
+    session_id = request.cookies.get("session_id", None)
+    if session_id is None:
+        raise HTTPException(status_code=401)
+
+    computing_id = await auth.crud.get_computing_id(db_session, session_id)
+    if computing_id is None:
+        raise HTTPException(status_code=401)
+
+    officer_info = await officers.crud.officer_info(db_session, computing_id)
+    if officer_info is None:
+        raise HTTPException(status_code=404, detail="user has no officer info")
+
+    return JSONResponse(officer_info)
+
 @dataclass
 class InitialOfficerInfo:
     computing_id: str

From 428d8bc0ce784a3708903f4b449aa93594934514 Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Fri, 23 Aug 2024 23:10:59 -0700
Subject: [PATCH 13/29] update database.py

---
 src/database.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/database.py b/src/database.py
index 34ac004..c1abf41 100644
--- a/src/database.py
+++ b/src/database.py
@@ -84,6 +84,7 @@ async def session(self) -> AsyncIterator[AsyncSession]:
 
 
 if os.environ.get("DB_PORT") is not None:
+    # using a remote (or docker) database
     db_port = os.environ.get("DB_PORT")
     SQLALCHEMY_DATABASE_URL = f"postgresql+asyncpg://localhost:{db_port}/main"
     SQLALCHEMY_TEST_DATABASE_URL = f"postgresql+asyncpg://localhost:{db_port}/test"
@@ -91,16 +92,17 @@ async def session(self) -> AsyncIterator[AsyncSession]:
     SQLALCHEMY_DATABASE_URL = "postgresql+asyncpg:///main"
     SQLALCHEMY_TEST_DATABASE_URL = "postgresql+asyncpg:///test"
 
+
 # also TODO: make this nicer, using a class to hold state...
 # and use this in load_test_db for the test db as well?
 def setup_database():
     global sessionmanager
 
     # TODO: where is sys.stdout piped to? I want all these to go to a specific logs folder
-    if os.environ.get("LOCAL"):
-        sessionmanager = DatabaseSessionManager(SQLALCHEMY_TEST_DATABASE_URL, {"echo": True})
-    else:
-        sessionmanager = DatabaseSessionManager(SQLALCHEMY_DATABASE_URL, {"echo": True})
+    sessionmanager = DatabaseSessionManager(
+        SQLALCHEMY_TEST_DATABASE_URL if os.environ.get("LOCAL") else SQLALCHEMY_DATABASE_URL,
+        { "echo": True },
+    )
 
 @contextlib.asynccontextmanager
 async def lifespan(app: FastAPI):

From a5883275a43dfd42cf5c48793595aae99a38d7bc Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Fri, 23 Aug 2024 23:13:53 -0700
Subject: [PATCH 14/29] fix bug adding serializable dict function

---
 src/officers/tables.py | 14 ++++++++++++++
 src/officers/urls.py   |  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/officers/tables.py b/src/officers/tables.py
index 0733937..070cbb0 100644
--- a/src/officers/tables.py
+++ b/src/officers/tables.py
@@ -178,3 +178,17 @@ def update_dict(is_filled_in: bool, officer_info_data: OfficerInfoData) -> dict:
             "github_username": officer_info_data.github_username,
             "google_drive_email": officer_info_data.google_drive_email,
         }
+
+    def serializable_dict(self) -> dict:
+        return {
+            "legal_name": self.legal_name,
+            "discord_id": self.discord_id,
+            "discord_name": self.discord_name,
+            "discord_nickname": self.discord_nickname,
+
+            "computing_id": self.computing_id,
+            "phone_number": self.phone_number,
+            "github_username": self.github_username,
+
+            "google_drive_email": self.google_drive_email,
+        }
diff --git a/src/officers/urls.py b/src/officers/urls.py
index 9e40788..570b652 100755
--- a/src/officers/urls.py
+++ b/src/officers/urls.py
@@ -117,7 +117,7 @@ async def get_officer_info(
     if officer_info is None:
         raise HTTPException(status_code=404, detail="user has no officer info")
 
-    return JSONResponse(officer_info)
+    return JSONResponse(officer_info.serializable_dict())
 
 @dataclass
 class InitialOfficerInfo:

From 71950a134564aab889dfb670a11a5db7446bb8a1 Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Sat, 24 Aug 2024 00:33:08 -0700
Subject: [PATCH 15/29] reorder columns

---
 src/officers/tables.py | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/officers/tables.py b/src/officers/tables.py
index 070cbb0..08b00fb 100644
--- a/src/officers/tables.py
+++ b/src/officers/tables.py
@@ -116,9 +116,17 @@ def serializable_dict(self) -> dict:
 class OfficerInfo(Base):
     __tablename__ = "officer_info"
 
+    computing_id = Column(
+        String(COMPUTING_ID_LEN),
+        ForeignKey("user_session.computing_id"),
+        primary_key=True,
+    )
+
     is_filled_in = Column(Boolean, nullable=False)
+
     # TODO: we'll need to use SFU's API to get the legal name for users
     legal_name = Column(String(128), nullable=False)  # some people have long names, you never know
+    phone_number = Column(String(24))
 
     # a null discord id would mean you don't have discord
     discord_id = Column(String(DISCORD_ID_LEN))
@@ -126,19 +134,12 @@ class OfficerInfo(Base):
     # this is their nickname in the csss server
     discord_nickname = Column(String(DISCORD_NICKNAME_LEN))
 
-    # private info will be added last
-    computing_id = Column(
-        String(COMPUTING_ID_LEN),
-        ForeignKey("user_session.computing_id"),
-        primary_key=True,
-    )
-    phone_number = Column(String(24))
-    github_username = Column(String(GITHUB_USERNAME_LEN))
-
     # Technically 320 is the most common max-size for emails, but we'll use 256 instead,
     # since it's reasonably large (input validate this too)
     google_drive_email = Column(String(256))
 
+    github_username = Column(String(GITHUB_USERNAME_LEN))
+
     # NOTE: not sure if we'll need this, depending on implementation
     # TODO: get this data on the fly when requested, but rate limit users
     # to something like 1/s 100/hour

From 3f59f60d453ce28a130d64f316d444660bf0ae88 Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Sat, 24 Aug 2024 19:52:38 -0700
Subject: [PATCH 16/29] remove is_valid from database & upload subset of data
 through /officers/update_info endpoint

---
 .../166f3772fce7_auth_officer_init.py         |   2 -
 src/load_test_db.py                           |  21 ++-
 src/officers/crud.py                          |  36 ++--
 src/officers/tables.py                        | 158 +++++++-----------
 src/officers/types.py                         | 100 +++++++++--
 src/officers/urls.py                          |  32 +++-
 src/utils.py                                  |  18 +-
 7 files changed, 203 insertions(+), 164 deletions(-)

diff --git a/src/alembic/versions/166f3772fce7_auth_officer_init.py b/src/alembic/versions/166f3772fce7_auth_officer_init.py
index 03d828c..7e1a345 100644
--- a/src/alembic/versions/166f3772fce7_auth_officer_init.py
+++ b/src/alembic/versions/166f3772fce7_auth_officer_init.py
@@ -39,7 +39,6 @@ def upgrade() -> None:
         "officer_term",
         sa.Column("id", sa.Integer(), primary_key=True, autoincrement=True),
         sa.Column("computing_id", sa.String(length=32), sa.ForeignKey("site_user.computing_id"), nullable=False),
-        sa.Column("is_filled_in", sa.Boolean(), nullable=False),
         sa.Column("position", sa.String(length=128), nullable=False),
         sa.Column("start_date", sa.DateTime(), nullable=False),
         sa.Column("end_date", sa.DateTime(), nullable=True),
@@ -53,7 +52,6 @@ def upgrade() -> None:
     )
     op.create_table(
         "officer_info",
-        sa.Column("is_filled_in", sa.Boolean(), nullable=False),
         sa.Column("legal_name", sa.String(length=128), nullable=False),
         sa.Column("discord_id", sa.String(length=18), nullable=True),
         sa.Column("discord_name", sa.String(length=32), nullable=True),
diff --git a/src/load_test_db.py b/src/load_test_db.py
index b0a5e5c..dc2d63f 100644
--- a/src/load_test_db.py
+++ b/src/load_test_db.py
@@ -10,7 +10,8 @@
 from database import SQLALCHEMY_TEST_DATABASE_URL, Base, DatabaseSessionManager
 from officers.constants import OfficerPosition
 from officers.crud import create_new_officer_info, create_new_officer_term, update_officer_info, update_officer_term
-from officers.types import OfficerInfoData, OfficerTermData
+from officers.tables import OfficerInfo
+from officers.types import OfficerInfoUpload, OfficerTermData
 from sqlalchemy.ext.asyncio import AsyncSession
 
 
@@ -68,7 +69,7 @@ async def load_test_officers_data(db_session: AsyncSession):
 
     print("add officer info")
     # this person has uploaded all of their info
-    await create_new_officer_info(db_session, OfficerInfoData(
+    await create_new_officer_info(db_session, OfficerInfo(
         legal_name="Person A",
         discord_id=str(88_1234_7182_4877_1111),
         discord_name="person_a_yeah",
@@ -80,19 +81,21 @@ async def load_test_officers_data(db_session: AsyncSession):
         google_drive_email="person_a@gmail.com",
     ))
     # this person has not joined the CSSS discord, so their discord name & nickname could not be found
-    await create_new_officer_info(db_session, OfficerInfoData(
+    await create_new_officer_info(db_session, OfficerInfo(
+        computing_id="abc22",
+
         legal_name="Person B",
+        phone_number="1112223333",
+
         discord_id=str(88_1234_7182_4877_2222),
         discord_name=None,
         discord_nickname=None,
 
-        computing_id="abc22",
-        phone_number="1112223333",
-        github_username="person_b",
         google_drive_email="person_b@gmail.com",
+        github_username="person_b",
     ))
     # this person has uploaded the minimal amount of information
-    await create_new_officer_info(db_session, OfficerInfoData(
+    await create_new_officer_info(db_session, OfficerInfo(
         legal_name="Person C",
         discord_id=None,
         discord_name=None,
@@ -176,7 +179,7 @@ async def load_test_officers_data(db_session: AsyncSession):
     ))
     await db_session.commit()
 
-    await update_officer_info(db_session, OfficerInfoData(
+    await update_officer_info(db_session, OfficerInfo(
         legal_name="Person C ----",
         discord_id=None,
         discord_name=None,
@@ -212,7 +215,7 @@ async def load_sysadmin(db_session: AsyncSession):
     # put your computing id here for testing purposes
     SYSADMIN_COMPUTING_ID = "gsa92"
 
-    await create_new_officer_info(db_session, OfficerInfoData(
+    await create_new_officer_info(db_session, OfficerInfo(
         legal_name="Gabe Schulz",
         discord_id=None,
         discord_name=None,
diff --git a/src/officers/crud.py b/src/officers/crud.py
index 7dda781..350e276 100644
--- a/src/officers/crud.py
+++ b/src/officers/crud.py
@@ -11,7 +11,7 @@
 from officers.tables import OfficerInfo, OfficerTerm
 from officers.types import (
     OfficerData,
-    OfficerInfoData,
+    OfficerInfoUpload,
     OfficerPrivateData,
     OfficerTermData,
 )
@@ -46,7 +46,7 @@ async def current_officer_position(db_session: database.DBSession, computing_id:
     else:
         return officer_term.position
 
-async def officer_info(db_session: database.DBSession, computing_id: str) -> OfficerInfoData | None:
+async def officer_info(db_session: database.DBSession, computing_id: str) -> OfficerInfo | None:
     query = (
         sqlalchemy
         .select(OfficerInfo)
@@ -151,7 +151,7 @@ async def all_officer_terms(
     """
     query = sqlalchemy.select(OfficerTerm)
     if view_only_filled_in:
-        query = query.where(OfficerTerm.is_filled_in)
+        query = OfficerTerm.sql_is_filled_in(query)
     query = query.order_by(OfficerTerm.start_date.desc())
     officer_terms = (await db_session.scalars(query)).all()
 
@@ -168,41 +168,38 @@ async def all_officer_terms(
 
     return officer_data_list
 
-async def create_new_officer_info(db_session: database.DBSession, officer_info_data: OfficerInfoData) -> bool:
+async def create_new_officer_info(db_session: database.DBSession, new_officer_info: OfficerInfo) -> bool:
     """
     Return False if the officer already exists
     """
     query = sqlalchemy.select(OfficerInfo)
-    query = query.where(OfficerInfo.computing_id == officer_info_data.computing_id)
-    officer_info = await db_session.scalar(query)
-    if officer_info is not None:
+    query = query.where(OfficerInfo.computing_id == officer_info.computing_id)
+    stored_officer_info = await db_session.scalar(query)
+    if stored_officer_info is not None:
         return False
 
-    is_filled_in = officer_info_data.is_filled_in()
-
-    new_user_session = OfficerInfo.from_data(is_filled_in, officer_info_data)
-    db_session.add(new_user_session)
+    db_session.add(new_officer_info)
     return True
 
-async def update_officer_info(db_session: database.DBSession, officer_info_data: OfficerInfoData) -> bool:
+async def update_officer_info(db_session: database.DBSession, new_officer_info: OfficerInfo) -> bool:
     """
     Return False if the officer doesn't exist yet
     """
     query = sqlalchemy.select(OfficerInfo)
-    query = query.where(OfficerInfo.computing_id == officer_info_data.computing_id)
+    query = query.where(OfficerInfo.computing_id == new_officer_info.computing_id)
     officer_info = await db_session.scalar(query)
     if officer_info is None:
         return False
 
-    is_filled_in = officer_info_data.is_filled_in()
+    # TODO: how to detect an entry insert error?
     query = (
         sqlalchemy
         .update(OfficerInfo)
         .where(OfficerInfo.computing_id == officer_info.computing_id)
-        .values(OfficerInfo.update_dict(is_filled_in, officer_info_data))
+        .values(new_officer_info.to_update_dict())
     )
-
     await db_session.execute(query)
+
     return True
 
 async def create_new_officer_term(
@@ -218,9 +215,7 @@ async def create_new_officer_term(
         # if an entry with this (computing_id, position, start_date) already exists, do nothing
         return False
 
-    is_filled_in = officer_term_data.is_filled_in()
-
-    db_session.add(OfficerTerm.from_data(is_filled_in, officer_term_data))
+    db_session.add(officer_term_data.to_officer_term())
     return True
 
 async def update_officer_term(
@@ -245,14 +240,13 @@ async def update_officer_term(
     if officer_term is None:
         return False
 
-    is_filled_in = officer_term_data.is_filled_in()
     query = (
         sqlalchemy
         .update(OfficerTerm)
         .where(OfficerTerm.computing_id == officer_term_data.computing_id)
         .where(OfficerTerm.position == officer_term_data.position)
         .where(OfficerTerm.start_date == officer_term_data.start_date)
-        .values(OfficerTerm.update_dict(is_filled_in, officer_term_data))
+        .values(officer_term_data.to_update_dict())
     )
 
     await db_session.execute(query)
diff --git a/src/officers/tables.py b/src/officers/tables.py
index 08b00fb..21cc0bd 100644
--- a/src/officers/tables.py
+++ b/src/officers/tables.py
@@ -10,17 +10,17 @@
 )
 from database import Base
 from sqlalchemy import (
-    Boolean,
+    # Boolean,
     Column,
     DateTime,
     ForeignKey,
     Integer,
+    Select,
     String,
     Text,
+    and_,
 )
 
-from officers.types import OfficerInfoData, OfficerTermData
-
 
 # A row represents an assignment of a person to a position.
 # An officer with multiple positions, such as Frosh Chair & DoE, is broken up into multiple assignments.
@@ -34,70 +34,26 @@ class OfficerTerm(Base):
         nullable=False,
     )
 
-    # a record will only be set as publically visible if sufficient data has been given
-    is_filled_in = Column(Boolean, nullable=False)
 
     position = Column(String(128), nullable=False)
     start_date = Column(DateTime, nullable=False)
     # end_date is only not-specified for positions that don't have a length (ie. webmaster)
-    end_date = Column(DateTime)
+    end_date = Column(DateTime, nullable=True)
 
-    nickname = Column(String(128))
-    favourite_course_0 = Column(String(32))
-    favourite_course_1 = Column(String(32))
+    nickname = Column(String(128), nullable=True)
+    favourite_course_0 = Column(String(32), nullable=True)
+    favourite_course_1 = Column(String(32), nullable=True)
     # programming language
-    favourite_pl_0 = Column(String(32))
-    favourite_pl_1 = Column(String(32))
-    biography = Column(Text)
-    photo_url = Column(Text)  # some urls get big, best to let it be a string
-
-    @staticmethod
-    def from_data(is_filled_in: bool, officer_term_data: OfficerTermData) -> OfficerTerm:
-        return OfficerTerm(
-            computing_id = officer_term_data.computing_id,
-            is_filled_in = is_filled_in,
-
-            position = officer_term_data.position,
-            start_date = officer_term_data.start_date,
-            end_date = officer_term_data.end_date,
-
-            nickname = officer_term_data.nickname,
-            favourite_course_0 = officer_term_data.favourite_course_0,
-            favourite_course_1 = officer_term_data.favourite_course_1,
-            favourite_pl_0 = officer_term_data.favourite_pl_0,
-            favourite_pl_1 = officer_term_data.favourite_pl_1,
-            biography = officer_term_data.biography,
-            photo_url = officer_term_data.photo_url,
-        )
-
-    @staticmethod
-    def update_dict(is_filled_in: bool, officer_term_data: OfficerTermData) -> dict:
-        # cannot update:
-        # - computing_id
-        # - start_date
-        # - position
-        return {
-            "is_filled_in": is_filled_in,
-
-            "end_date": officer_term_data.end_date,
-            "nickname": officer_term_data.nickname,
-
-            "favourite_course_0": officer_term_data.favourite_course_0,
-            "favourite_course_1": officer_term_data.favourite_course_1,
-            "favourite_pl_0": officer_term_data.favourite_pl_0,
-            "favourite_pl_1": officer_term_data.favourite_pl_1,
-
-            "biography": officer_term_data.biography,
-            "photo_url": officer_term_data.photo_url,
-        }
+    favourite_pl_0 = Column(String(32), nullable=True)
+    favourite_pl_1 = Column(String(32), nullable=True)
+    biography = Column(Text, nullable=True)
+    photo_url = Column(Text, nullable=True)  # some urls get big, best to let it be a string
 
     def serializable_dict(self) -> dict:
         return {
             "id": self.id,
             "computing_id": self.computing_id,
 
-            "is_filled_in": self.is_filled_in,
-
             "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,
@@ -111,6 +67,37 @@ def serializable_dict(self) -> dict:
             "photo_url": self.photo_url,
         }
 
+    # a record will only be publically visible if sufficient data has been given
+    def is_filled_in(self):
+        return (
+            # photo & end_date don't have to be uploaded for the term to be "filled"
+            # NOTE: this definition might have to be updated
+            self.computing_id is not None
+            and self.start_date is not None
+            and self.nickname is not None
+            and self.favourite_course_0 is not None
+            and self.favourite_course_1 is not None
+            and self.favourite_pl_0 is not None
+            and self.favourite_pl_1 is not None
+            and self.biography is not None
+        )
+
+    @staticmethod
+    def sql_is_filled_in(query: Select) -> Select:
+        """Should be identical to self.is_filled_in()"""
+        return query.where(
+            and_(
+                OfficerTerm.computing_id is not None,
+                OfficerTerm.start_date is not None,
+                OfficerTerm.nickname is not None,
+                OfficerTerm.favourite_course_0 is not None,
+                OfficerTerm.favourite_course_1 is not None,
+                OfficerTerm.favourite_pl_0 is not None,
+                OfficerTerm.favourite_pl_1 is not None,
+                OfficerTerm.biography is not None,
+            )
+        )
+
 # this table contains information that we only need a most up-to-date version of, and
 # don't need to keep a history of. However, it also can't be easily updated.
 class OfficerInfo(Base):
@@ -122,64 +109,27 @@ class OfficerInfo(Base):
         primary_key=True,
     )
 
-    is_filled_in = Column(Boolean, nullable=False)
-
     # TODO: we'll need to use SFU's API to get the legal name for users
     legal_name = Column(String(128), nullable=False)  # some people have long names, you never know
-    phone_number = Column(String(24))
+    phone_number = Column(String(24), nullable=True)
 
     # a null discord id would mean you don't have discord
-    discord_id = Column(String(DISCORD_ID_LEN))
-    discord_name = Column(String(DISCORD_NAME_LEN))
+    discord_id = Column(String(DISCORD_ID_LEN), nullable=True)
+    discord_name = Column(String(DISCORD_NAME_LEN), nullable=True)
     # this is their nickname in the csss server
-    discord_nickname = Column(String(DISCORD_NICKNAME_LEN))
+    discord_nickname = Column(String(DISCORD_NICKNAME_LEN), nullable=True)
 
     # Technically 320 is the most common max-size for emails, but we'll use 256 instead,
     # since it's reasonably large (input validate this too)
-    google_drive_email = Column(String(256))
+    google_drive_email = Column(String(256), nullable=True)
 
-    github_username = Column(String(GITHUB_USERNAME_LEN))
+    github_username = Column(String(GITHUB_USERNAME_LEN), nullable=True)
 
     # NOTE: not sure if we'll need this, depending on implementation
     # TODO: get this data on the fly when requested, but rate limit users
     # to something like 1/s 100/hour
     # has_signed_into_bitwarden = Column(Boolean)
 
-    @staticmethod
-    def from_data(is_filled_in: bool, officer_info_data: OfficerInfoData) -> OfficerTerm:
-        return OfficerInfo(
-            is_filled_in = is_filled_in,
-            legal_name = officer_info_data.legal_name,
-
-            discord_id = officer_info_data.discord_id,
-            discord_name = officer_info_data.discord_name,
-            discord_nickname = officer_info_data.discord_nickname,
-
-            computing_id = officer_info_data.computing_id,
-            phone_number = officer_info_data.phone_number,
-            github_username = officer_info_data.github_username,
-
-            google_drive_email = officer_info_data.google_drive_email,
-        )
-
-    @staticmethod
-    def update_dict(is_filled_in: bool, officer_info_data: OfficerInfoData) -> dict:
-        # should only NOT contain the pkey (computing_id)
-        return {
-            "is_filled_in": is_filled_in,
-
-            # TODO: if the API call to SFU's api to get legal name fails, we want to fail & not insert the entry.
-            # for now, we should insert a default value
-            "legal_name": "default name" if officer_info_data.legal_name is None else officer_info_data.legal_name,
-            "discord_id": officer_info_data.discord_id,
-            "discord_name": officer_info_data.discord_name,
-            "discord_nickname": officer_info_data.discord_nickname,
-
-            "phone_number": officer_info_data.phone_number,
-            "github_username": officer_info_data.github_username,
-            "google_drive_email": officer_info_data.google_drive_email,
-        }
-
     def serializable_dict(self) -> dict:
         return {
             "legal_name": self.legal_name,
@@ -193,3 +143,15 @@ def serializable_dict(self) -> dict:
 
             "google_drive_email": self.google_drive_email,
         }
+
+    def is_filled_in(self):
+        return (
+            self.computing_id is not None
+            and self.legal_name is not None
+            and self.phone_number is not None
+            and self.discord_id is not None
+            and self.discord_name is not None
+            and self.discord_nickname is not None
+            and self.google_drive_email is not None
+            and self.github_username is not None
+        )
diff --git a/src/officers/types.py b/src/officers/types.py
index 55fcfd5..c4661e5 100644
--- a/src/officers/types.py
+++ b/src/officers/types.py
@@ -1,17 +1,17 @@
 from __future__ import annotations
 
-from dataclasses import asdict, dataclass, fields
+from dataclasses import asdict, dataclass
 from datetime import date, datetime
 
 from constants import COMPUTING_ID_MAX
 from fastapi import HTTPException
 
-import officers.tables
 from officers.constants import OfficerPosition
+from officers.tables import OfficerInfo, OfficerTerm
 
 
 @dataclass
-class OfficerInfoData:
+class OfficerInfoDownload:
     computing_id: str
 
     legal_name: str
@@ -33,13 +33,54 @@ def validate(self) -> None | HTTPException:
         else:
             return None
 
-    def is_filled_in(self):
-        for field in fields(self):
-            if getattr(self, field.name) is None:
-                return False
+@dataclass
+class OfficerInfoUpload:
+    computing_id: str
 
-        return True
+    # TODO: compute this using SFU's API; if unable, use a default value
+    legal_name: str
+    phone_number: None | str = None
+    discord_name: None | str = None
+    github_username: None | str = None
+    google_drive_email: None | str = None
 
+    def validate(self) -> None | HTTPException:
+        if len(self.computing_id) > COMPUTING_ID_MAX:
+            return HTTPException(status_code=400, detail=f"computing_id={self.computing_id} is too large")
+        elif self.legal_name is not None and self.legal_name == "":
+            return HTTPException(status_code=400, detail="legal name must not be empty")
+        # TODO: more checks
+        else:
+            return None
+
+    def to_officer_info(self, discord_id: str | None, discord_nickname: str | None) -> OfficerInfo:
+        return OfficerInfo(
+            computing_id = self.computing_id,
+            legal_name = self.legal_name,
+
+            discord_id = discord_id,
+            discord_name = self.discord_name,
+            discord_nickname = discord_nickname,
+
+            phone_number = self.phone_number,
+            github_username = self.github_username,
+            google_drive_email = self.google_drive_email,
+        )
+
+    def to_update_dict(self) -> dict:
+        return {
+            # TODO: if the API call to SFU's api to get legal name fails, we want to fail & not insert the entry.
+            # for now, we should insert a default value
+            "legal_name": "default name" if self.legal_name is None else self.legal_name,
+
+            "discord_id": self.discord_id,
+            "discord_name": self.discord_name,
+            "discord_nickname": self.discord_nickname,
+
+            "phone_number": self.phone_number,
+            "github_username": self.github_username,
+            "google_drive_email": self.google_drive_email,
+        }
 
 @dataclass
 class OfficerTermData:
@@ -72,16 +113,41 @@ def validate(self) -> None | HTTPException:
         else:
             return None
 
-    def is_filled_in(self):
-        for field in fields(self):
-            if field.name == "photo_url" or field.name == "end_date":
-                # photo & end_date don't have to be uploaded for the term to be "filled"
-                # NOTE: this definition might have to be updated
-                continue
-            elif getattr(self, field.name) is None:
-                return False
+    def to_officer_term(self) -> OfficerTerm:
+        return OfficerTerm(
+            computing_id = self.computing_id,
+
+            position = self.position,
+            start_date = self.start_date,
+            end_date = self.end_date,
+
+            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 True
+    def update_dict(self) -> dict:
+        # TODO: control this by term_id & allow the others to be updated
+        # cannot update:
+        # - computing_id
+        # - start_date
+        # - position
+        return {
+            "end_date": self.end_date,
+            "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,
+        }
 
 # -------------------------------------------- #
 
diff --git a/src/officers/urls.py b/src/officers/urls.py
index 570b652..b57dfd5 100755
--- a/src/officers/urls.py
+++ b/src/officers/urls.py
@@ -12,7 +12,8 @@
 
 import officers.crud
 from officers.constants import OfficerPosition
-from officers.types import OfficerInfoData, OfficerTermData
+from officers.tables import OfficerInfo
+from officers.types import OfficerInfoUpload, OfficerTermData
 
 _logger = logging.getLogger(__name__)
 
@@ -147,11 +148,15 @@ async def new_officer_term(
 
     for officer_info in officer_info_list:
         # TODO: fix a bug with this stuff & test inserting & viewing mutliple executives
-        await officers.crud.create_new_officer_info(db_session, OfficerInfoData(
-            # TODO: use sfu api to get legal name
-            legal_name = "default name",
-            computing_id = officer_info.computing_id,
-        ))
+        await officers.crud.create_new_officer_info(
+            db_session,
+            OfficerInfoUpload(
+                # TODO: use sfu api to get legal name
+                legal_name = "default name",
+                computing_id = officer_info.computing_id,
+            ).to_officer_info(None, None),
+        )
+        # TODO: update create_new_officer_term to be the same as create_new_officer_info
         success = await officers.crud.create_new_officer_term(db_session, OfficerTermData(
             computing_id = officer_info.computing_id,
             position = officer_info.position,
@@ -172,10 +177,11 @@ async def new_officer_term(
         "then input your information & the valid token for us. Admins may update this info."
     ),
 )
+# TODO: computing_id in path
 async def update_info(
     request: Request,
     db_session: database.DBSession,
-    officer_info: OfficerInfoData = Body(), # noqa: B008
+    officer_info: OfficerInfoUpload = Body(), # noqa: B008
 ):
     http_exception = officer_info.validate()
     if http_exception is not None:
@@ -195,13 +201,23 @@ async def update_info(
 
     # TODO: log all important changes just to a .log file
 
+    # TODO: do validation checking, return the updated info data
+
     success = await officers.crud.update_officer_info(db_session, officer_info)
     if not success:
         raise HTTPException(status_code=400, detail="officer_info does not exist yet, please create the officer info entry first")
 
+    updated_officer_info = await officers.crud.officer_info(db_session, officer_info.computing_id)
+    if updated_officer_info is None:
+        raise HTTPException(status_code=500, detail="failed to get officer info?! something is very wrong...")
+
     await db_session.commit()
-    return PlainTextResponse("ok")
 
+    return JSONResponse(updated_officer_info.serializable_dict())
+
+# TODO: access term by term_id
+# TODO: only allow access if the user is admin or if the id is their term
+# TODO: don't allow a user to change who owns the term if they're not an admin.
 @router.post(
     "/update_term",
 )
diff --git a/src/utils.py b/src/utils.py
index 8f2b8cb..c9ff18e 100644
--- a/src/utils.py
+++ b/src/utils.py
@@ -17,14 +17,14 @@ def is_iso_format(date_str: str) -> bool:
 
 def is_active_officer(query: Select) -> Select:
     # TODO: assert this constraint at the SQL level, so that we don't even have to check it?
-    return query.where(
-        and_(
-            OfficerTerm.is_filled_in,
-            or_(
-                # executives without a specified end_date are considered active
-                OfficerTerm.end_date.is_(None),
-                # check that today's timestamp is before (smaller than) the term's end date
-                datetime.today() <= OfficerTerm.end_date
-            )
+    query = query.where(
+        or_(
+            # executives without a specified end_date are considered active
+            OfficerTerm.end_date.is_(None),
+            # check that today's timestamp is before (smaller than) the term's end date
+            datetime.today() <= OfficerTerm.end_date
         )
     )
+    return OfficerTerm.sql_is_filled_in(query)
+
+

From eb031c43b513db246b3bdb488b8ab84c093b0ee7 Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Sun, 25 Aug 2024 00:47:34 -0700
Subject: [PATCH 17/29] add utils, clean discord module, and start update_info
 endpoint validation of input

---
 src/discord/discord.py | 43 +++++++++++++--------------------
 src/officers/crud.py   |  3 +--
 src/officers/tables.py |  3 ++-
 src/officers/types.py  | 33 +++-----------------------
 src/officers/urls.py   | 54 ++++++++++++++++++++++++++++--------------
 src/utils.py           |  6 ++++-
 6 files changed, 63 insertions(+), 79 deletions(-)

diff --git a/src/discord/discord.py b/src/discord/discord.py
index 4b5c51f..e123789 100644
--- a/src/discord/discord.py
+++ b/src/discord/discord.py
@@ -13,6 +13,8 @@
 ADMINISTRATOR = 0b1000
 VIEW_CHANNEL = 0b0010_0000_0000
 
+TOKEN = os.environ.get("DISCORD_TOKEN")
+
 @dataclass
 class User:
     id: str
@@ -138,9 +140,8 @@ async def get_channel(
     cid: str,
     gid: str = guild_id
 ) -> Channel | None:
-    token = os.environ.get("TOKEN")
     url = f"https://discord.com/api/v10/guilds/{gid}/channels"
-    result = await _discord_request(url, token)
+    result = await _discord_request(url, TOKEN)
 
     result_json = result.json()
     channel = next((channel for channel in result_json if channel["id"] == cid), None)
@@ -152,9 +153,8 @@ async def get_channel(
 async def get_all_channels(
     gid: str = guild_id
 ) -> list[str]:
-    token = os.environ.get("TOKEN")
     url = f"https://discord.com/api/v10/guilds/{gid}/channels"
-    result = await _discord_request(url, token)
+    result = await _discord_request(url, TOKEN)
 
     result_json = result.json()
     channels = [channel for channel in result_json if channel["type"] != DISCORD_CATEGORY_ID]
@@ -162,7 +162,6 @@ async def get_all_channels(
 
     return channel_names
 
-
 async def get_role_name_by_id(
     rid: str,
     gid: str = guild_id
@@ -174,9 +173,8 @@ async def get_role_by_id(
     rid: str,
     gid: str = guild_id
 ) -> dict | None:
-    token = os.environ.get("TOKEN")
     url = f"https://discord.com/api/v10/guilds/{gid}/roles"
-    result = await _discord_request(url, token)
+    result = await _discord_request(url, TOKEN)
 
     result_json = result.json()
     return next((role for role in result_json if role["id"] == rid), None)
@@ -185,23 +183,20 @@ async def get_user_roles(
     uid: str,
     gid: str = guild_id
 ) -> list[str]:
-    token = os.environ.get("TOKEN")
     url = f"https://discord.com/api/v10/guilds/{gid}/members/{uid}"
-    result = await _discord_request(url, token)
+    result = await _discord_request(url, TOKEN)
 
     result_json = result.json()
     return result_json["roles"]
 
-
 async def get_all_roles(
     gid: str = guild_id
 ) ->  dict[str, list[str]]:
     """
     Grabs all roles in a given guild.
     """
-    token = os.environ.get("TOKEN")
     url = f"https://discord.com/api/v10/guilds/{gid}/roles"
-    result = await _discord_request(url, token)
+    result = await _discord_request(url, TOKEN)
 
     result_json = result.json()
     roles = [([role["id"], [role["name"], role["permissions"]]]) for role in result_json]
@@ -211,10 +206,9 @@ async def get_guild_members_with_role(
     rid: str,
     gid: str = guild_id
 ) -> list[GuildMember]:
-    token = os.environ.get("TOKEN")
     # base case
     url = f"https://discord.com/api/v10/guilds/{gid}/members?limit=1000"
-    result = await _discord_request(url, token)
+    result = await _discord_request(url, TOKEN)
 
     result_json = result.json()
 
@@ -233,7 +227,7 @@ async def get_guild_members_with_role(
 
     while True:
         url = f"https://discord.com/api/v10/guilds/{gid}/members?limit=1000&after={last_uid}"
-        result = await _discord_request(url, token)
+        result = await _discord_request(url, TOKEN)
 
         result_json = result.json()
 
@@ -249,10 +243,9 @@ async def get_guild_members_with_role(
 async def get_guild_members(
     gid: str = guild_id
 ) -> list[GuildMember]:
-    token = os.environ.get("TOKEN")
     # base case
     url = f"https://discord.com/api/v10/guilds/{gid}/members?limit=1000"
-    result = await _discord_request(url, token)
+    result = await _discord_request(url, TOKEN)
 
     result_json = result.json()
     users = [GuildMember(User(user["user"]["id"], user["user"]["username"], user["user"]["discriminator"], user["user"]["global_name"], user["user"]["avatar"]), user["roles"]) for user in result_json]
@@ -260,7 +253,7 @@ async def get_guild_members(
 
     while True:
         url = f"https://discord.com/api/v10/guilds/{gid}/members?limit=1000&after={last_uid}"
-        result = await _discord_request(url, token)
+        result = await _discord_request(url, TOKEN)
 
         result_json = result.json()
 
@@ -275,9 +268,8 @@ async def get_guild_members(
 async def get_categories(
     gid: str = guild_id
 ) -> list[str]:
-    token = os.environ.get("TOKEN")
     url = f"https://discord.com/api/v10/guilds/{gid}/channels"
-    result = await _discord_request(url, token)
+    result = await _discord_request(url, TOKEN)
 
     result_json = result.json()
     return [category["name"] for category in result_json if category["type"] == DISCORD_CATEGORY_ID]
@@ -286,9 +278,8 @@ async def get_channels_by_category_name(
     category_name: str,
     gid: str = guild_id
 ) -> list[Channel]:
-    token = os.environ.get("TOKEN")
     url = f"https://discord.com/api/v10/guilds/{gid}/channels"
-    result = await _discord_request(url, token)
+    result = await _discord_request(url, TOKEN)
 
     result_json = result.json()
     # TODO: edge case if there exist duplicate category names, see get_channels_by_category_id()
@@ -315,9 +306,8 @@ async def get_channels_by_category_id(
     cid: str,
     gid: str = guild_id
 ) -> list[Channel]:
-    token = os.environ.get("TOKEN")
     url = f"https://discord.com/api/v10/guilds/{gid}/channels"
-    result = await _discord_request(url, token)
+    result = await _discord_request(url, TOKEN)
 
     result_json = result.json()
     channels = [
@@ -336,10 +326,9 @@ async def get_channels_by_category_id(
 async def search_user(
     user: str,
     gid: str = guild_id
-) -> User:
-    token = os.environ.get("TOKEN")
+) -> User | None:
     url = f"https://discord.com/api/v10/guilds/{gid}/members/search?query={user}"
-    result = await _discord_request(url, token)
+    result = await _discord_request(url, TOKEN)
     json = result.json()
 
     if len(json) == 0:
diff --git a/src/officers/crud.py b/src/officers/crud.py
index 350e276..5497dea 100644
--- a/src/officers/crud.py
+++ b/src/officers/crud.py
@@ -11,8 +11,7 @@
 from officers.tables import OfficerInfo, OfficerTerm
 from officers.types import (
     OfficerData,
-    OfficerInfoUpload,
-    OfficerPrivateData,
+    #OfficerPrivateData,
     OfficerTermData,
 )
 
diff --git a/src/officers/tables.py b/src/officers/tables.py
index 21cc0bd..f0e3fe7 100644
--- a/src/officers/tables.py
+++ b/src/officers/tables.py
@@ -34,7 +34,6 @@ class OfficerTerm(Base):
         nullable=False,
     )
 
-
     position = Column(String(128), nullable=False)
     start_date = Column(DateTime, nullable=False)
     # end_date is only not-specified for positions that don't have a length (ie. webmaster)
@@ -132,6 +131,8 @@ class OfficerInfo(Base):
 
     def serializable_dict(self) -> dict:
         return {
+            "is_filled_in": self.is_filled_in(),
+
             "legal_name": self.legal_name,
             "discord_id": self.discord_id,
             "discord_name": self.discord_name,
diff --git a/src/officers/types.py b/src/officers/types.py
index c4661e5..7546991 100644
--- a/src/officers/types.py
+++ b/src/officers/types.py
@@ -10,33 +10,8 @@
 from officers.tables import OfficerInfo, OfficerTerm
 
 
-@dataclass
-class OfficerInfoDownload:
-    computing_id: str
-
-    legal_name: str
-    discord_id: None | str = None # TODO: do we need this to get info about a person
-
-    discord_name: None | str = None
-    discord_nickname: None | str = None
-
-    phone_number: None | str = None
-    github_username: None | str = None
-    google_drive_email: None | str = None
-
-    def validate(self) -> None | HTTPException:
-        if len(self.computing_id) > COMPUTING_ID_MAX:
-            return HTTPException(status_code=400, detail=f"computing_id={self.computing_id} is too large")
-        elif self.legal_name is not None and self.legal_name == "":
-            return HTTPException(status_code=400, detail="legal name must not be empty")
-        # TODO: more checks
-        else:
-            return None
-
 @dataclass
 class OfficerInfoUpload:
-    computing_id: str
-
     # TODO: compute this using SFU's API; if unable, use a default value
     legal_name: str
     phone_number: None | str = None
@@ -45,17 +20,15 @@ class OfficerInfoUpload:
     google_drive_email: None | str = None
 
     def validate(self) -> None | HTTPException:
-        if len(self.computing_id) > COMPUTING_ID_MAX:
-            return HTTPException(status_code=400, detail=f"computing_id={self.computing_id} is too large")
-        elif self.legal_name is not None and self.legal_name == "":
+        if self.legal_name is not None and self.legal_name == "":
             return HTTPException(status_code=400, detail="legal name must not be empty")
         # TODO: more checks
         else:
             return None
 
-    def to_officer_info(self, discord_id: str | None, discord_nickname: str | None) -> OfficerInfo:
+    def to_officer_info(self, computing_id: str, discord_id: str | None, discord_nickname: str | None) -> OfficerInfo:
         return OfficerInfo(
-            computing_id = self.computing_id,
+            computing_id = computing_id,
             legal_name = self.legal_name,
 
             discord_id = discord_id,
diff --git a/src/officers/urls.py b/src/officers/urls.py
index b57dfd5..7a18e2a 100755
--- a/src/officers/urls.py
+++ b/src/officers/urls.py
@@ -4,11 +4,13 @@
 
 import auth.crud
 import database
+import sqlalchemy
+import utils
 from constants import COMPUTING_ID_MAX
+from discord import discord
 from fastapi import APIRouter, Body, HTTPException, Request
 from fastapi.responses import JSONResponse, PlainTextResponse
 from permission.types import OfficerPrivateInfo, WebsiteAdmin
-from utils import is_iso_format
 
 import officers.crud
 from officers.constants import OfficerPosition
@@ -150,11 +152,11 @@ async def new_officer_term(
         # TODO: fix a bug with this stuff & test inserting & viewing mutliple executives
         await officers.crud.create_new_officer_info(
             db_session,
+            # TODO: do I need this object atm?
             OfficerInfoUpload(
                 # TODO: use sfu api to get legal name
                 legal_name = "default name",
-                computing_id = officer_info.computing_id,
-            ).to_officer_info(None, None),
+            ).to_officer_info(officer_info.computing_id, None, None),
         )
         # TODO: update create_new_officer_term to be the same as create_new_officer_info
         success = await officers.crud.create_new_officer_term(db_session, OfficerTermData(
@@ -170,20 +172,21 @@ async def new_officer_term(
     return PlainTextResponse("ok")
 
 @router.post(
-    "/update_info",
+    "/{computing_id}/update_info",
     description=(
         "After elections, officer computing ids are input into our system. "
         "If you have been elected as a new officer, you may authenticate with SFU CAS, "
         "then input your information & the valid token for us. Admins may update this info."
     ),
 )
-# TODO: computing_id in path
+# TODO: computing_id in all paths
 async def update_info(
     request: Request,
     db_session: database.DBSession,
-    officer_info: OfficerInfoUpload = Body(), # noqa: B008
+    computing_id: str,
+    new_officer_info: OfficerInfoUpload = Body(), # noqa: B008
 ):
-    http_exception = officer_info.validate()
+    http_exception = new_officer_info.validate()
     if http_exception is not None:
         raise http_exception
 
@@ -193,7 +196,7 @@ async def update_info(
 
     session_computing_id = await auth.crud.get_computing_id(db_session, session_id)
     if (
-        officer_info.computing_id != session_computing_id
+        computing_id != session_computing_id
         and not await WebsiteAdmin.has_permission(db_session, session_computing_id)
     ):
         # the current user can only input the info for another user if they have permissions
@@ -201,13 +204,35 @@ async def update_info(
 
     # TODO: log all important changes just to a .log file
 
-    # TODO: do validation checking, return the updated info data
+    # TODO: turn this into a function first
+    # get officer info
+    query = sqlalchemy.select(OfficerInfo)
+    query = query.where(OfficerInfo.computing_id == new_officer_info.computing_id)
+    officer_info = await db_session.scalar(query)
+    if officer_info is None:
+        raise HTTPException(status_code=400, detail="officer_info does not exist yet, please create the officer info entry first")
+
+    # TODO: 1. validate phone-number
+    if not utils.is_valid_phone_number(new_officer_info.phone_number):
+        new_officer_info.phone_number = officer_info.phone_number
+
+    # TODO: use this API
+    discord_user = discord.search_user()
+    # TODO: 2.0 validate (find) discord-name in csss discord
+    # TODO: 2.1 use discord-name to find discord-id
+    # TODO: 2.3 use discord-name to find discord-nickname
+
+    # TODO: 3. validate google-email using google module
 
-    success = await officers.crud.update_officer_info(db_session, officer_info)
+    success = await officers.crud.update_officer_info(db_session, new_officer_info.to_officer_info(
+        computing_id=computing_id,
+        discord_id=None,
+        discord_nickname=None,
+    ))
     if not success:
         raise HTTPException(status_code=400, detail="officer_info does not exist yet, please create the officer info entry first")
 
-    updated_officer_info = await officers.crud.officer_info(db_session, officer_info.computing_id)
+    updated_officer_info = await officers.crud.officer_info(db_session, computing_id)
     if updated_officer_info is None:
         raise HTTPException(status_code=500, detail="failed to get officer info?! something is very wrong...")
 
@@ -258,13 +283,6 @@ async def update_term(
 async def raise_error():
     raise ValueError("This is an error, you're welcome")
 
-@router.get(
-    "/my_info",
-    description="Get info about whether you are still an executive or not / what your position is.",
-)
-async def my_info():
-    return {}
-
 @router.post(
     "/remove",
     description="Only the sysadmin, president, or DoA can submit this request. It will usually be the DoA. Removes the officer from the system entirely. BE CAREFUL WITH THIS OPTION aaaaaaaaaaaaaaaaaa.",
diff --git a/src/utils.py b/src/utils.py
index c9ff18e..5b56970 100644
--- a/src/utils.py
+++ b/src/utils.py
@@ -27,4 +27,8 @@ def is_active_officer(query: Select) -> Select:
     )
     return OfficerTerm.sql_is_filled_in(query)
 
-
+def is_valid_phone_number(phone_number: str) -> bool:
+    return (
+        len(phone_number) == 10
+        and phone_number.isnumeric()
+    )

From ed02b602b276ea95b78bb89386bd1b706170a200 Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Thu, 29 Aug 2024 22:35:11 -0700
Subject: [PATCH 18/29] clean discord api & add checking for it in /update_info

---
 src/constants.py       |  5 ++-
 src/discord/discord.py | 74 ++++++++++++++++++++++++++++--------------
 src/officers/urls.py   | 41 ++++++++++++++---------
 3 files changed, 80 insertions(+), 40 deletions(-)

diff --git a/src/constants.py b/src/constants.py
index 1da1153..af24019 100644
--- a/src/constants.py
+++ b/src/constants.py
@@ -1,9 +1,12 @@
 import os
 
 root_ip_address = "http://localhost:8080" if os.environ.get("LOCAL") == "true" else "https://api.sfucsss.org"
-guild_id = "1260652618875797504" if os.environ.get("LOCAL") == "true" else "228761314644852736"
 github_org_name = "CSSS-Test-Organization" if os.environ.get("LOCAL") == "true" else "CSSS"
 
+W3_GUILD_ID = "1260652618875797504"
+CSSS_GUILD_ID = "228761314644852736"
+ACTIVE_GUILD_ID = W3_GUILD_ID if os.environ.get("LOCAL") == "true" else CSSS_GUILD_ID
+
 SESSION_ID_LEN = 512
 # technically a max of 8 digits https://www.sfu.ca/computing/about/support/tips/sfu-userid.html
 COMPUTING_ID_LEN = 32
diff --git a/src/discord/discord.py b/src/discord/discord.py
index e123789..334b99f 100644
--- a/src/discord/discord.py
+++ b/src/discord/discord.py
@@ -3,7 +3,7 @@
 from time import sleep
 
 import requests
-from constants import guild_id
+from constants import ACTIVE_GUILD_ID
 from requests import Response
 
 # ----------------------- #
@@ -13,15 +13,19 @@
 ADMINISTRATOR = 0b1000
 VIEW_CHANNEL = 0b0010_0000_0000
 
+# this is the "Application ID"
 TOKEN = os.environ.get("DISCORD_TOKEN")
 
 @dataclass
 class User:
     id: str
+    # this is the normal username
     username: str
     # Discriminators are what used to be the #xxxx after a discord username. Accounts which haven't
     # migrated over yet have them still.
+    # For accounts that don't have one, it's '0'
     discriminator: str
+    # this is the server-nickname
     global_name: str | None = None
     avatar: str | None = None
 
@@ -65,7 +69,7 @@ async def _discord_request(
 async def get_channel_members(
     cid: str,
     # TODO: hardcode guild_id (remove it as argument) if we ever refactor this module
-    gid: str = guild_id
+    gid: str = ACTIVE_GUILD_ID
 ) -> list[GuildMember]:
     """
     Returns empty list if invalid channel id is provided.
@@ -95,8 +99,8 @@ async def get_channel_members(
     assert role_everyone is not None
     base_permission = role_everyone["permissions"]
 
-    users = await get_guild_members(guild_id)
-    roles = await get_all_roles(guild_id)
+    users = await get_guild_members(gid)
+    roles = await get_all_roles(gid)
 
     users_with_access = []
     # note string conversion to int
@@ -138,7 +142,7 @@ async def get_channel_members(
 
 async def get_channel(
     cid: str,
-    gid: str = guild_id
+    gid: str = ACTIVE_GUILD_ID
 ) -> Channel | None:
     url = f"https://discord.com/api/v10/guilds/{gid}/channels"
     result = await _discord_request(url, TOKEN)
@@ -151,7 +155,7 @@ async def get_channel(
         return Channel(channel["id"], channel["type"], channel["guild_id"], channel["name"], channel["permission_overwrites"])
 
 async def get_all_channels(
-    gid: str = guild_id
+    gid: str = ACTIVE_GUILD_ID
 ) -> list[str]:
     url = f"https://discord.com/api/v10/guilds/{gid}/channels"
     result = await _discord_request(url, TOKEN)
@@ -164,14 +168,14 @@ async def get_all_channels(
 
 async def get_role_name_by_id(
     rid: str,
-    gid: str = guild_id
+    gid: str = ACTIVE_GUILD_ID
 ) -> str:
     roles = await get_all_roles(gid)
     return roles[rid][0]
 
 async def get_role_by_id(
     rid: str,
-    gid: str = guild_id
+    gid: str = ACTIVE_GUILD_ID
 ) -> dict | None:
     url = f"https://discord.com/api/v10/guilds/{gid}/roles"
     result = await _discord_request(url, TOKEN)
@@ -181,7 +185,7 @@ async def get_role_by_id(
 
 async def get_user_roles(
     uid: str,
-    gid: str = guild_id
+    gid: str = ACTIVE_GUILD_ID
 ) -> list[str]:
     url = f"https://discord.com/api/v10/guilds/{gid}/members/{uid}"
     result = await _discord_request(url, TOKEN)
@@ -190,7 +194,7 @@ async def get_user_roles(
     return result_json["roles"]
 
 async def get_all_roles(
-    gid: str = guild_id
+    gid: str = ACTIVE_GUILD_ID
 ) ->  dict[str, list[str]]:
     """
     Grabs all roles in a given guild.
@@ -204,7 +208,7 @@ async def get_all_roles(
 
 async def get_guild_members_with_role(
     rid: str,
-    gid: str = guild_id
+    gid: str = ACTIVE_GUILD_ID
 ) -> list[GuildMember]:
     # base case
     url = f"https://discord.com/api/v10/guilds/{gid}/members?limit=1000"
@@ -241,7 +245,7 @@ async def get_guild_members_with_role(
         last_uid = res[-1].user.id
 
 async def get_guild_members(
-    gid: str = guild_id
+    gid: str = ACTIVE_GUILD_ID
 ) -> list[GuildMember]:
     # base case
     url = f"https://discord.com/api/v10/guilds/{gid}/members?limit=1000"
@@ -266,7 +270,7 @@ async def get_guild_members(
         last_uid = res[-1].user.id
 
 async def get_categories(
-    gid: str = guild_id
+    gid: str = ACTIVE_GUILD_ID
 ) -> list[str]:
     url = f"https://discord.com/api/v10/guilds/{gid}/channels"
     result = await _discord_request(url, TOKEN)
@@ -276,7 +280,7 @@ async def get_categories(
 
 async def get_channels_by_category_name(
     category_name: str,
-    gid: str = guild_id
+    gid: str = ACTIVE_GUILD_ID
 ) -> list[Channel]:
     url = f"https://discord.com/api/v10/guilds/{gid}/channels"
     result = await _discord_request(url, TOKEN)
@@ -304,7 +308,7 @@ async def get_channels_by_category_name(
 
 async def get_channels_by_category_id(
     cid: str,
-    gid: str = guild_id
+    gid: str = ACTIVE_GUILD_ID
 ) -> list[Channel]:
     url = f"https://discord.com/api/v10/guilds/{gid}/channels"
     result = await _discord_request(url, TOKEN)
@@ -324,14 +328,36 @@ async def get_channels_by_category_id(
     return channels
 
 async def search_user(
-    user: str,
-    gid: str = guild_id
-) -> User | None:
-    url = f"https://discord.com/api/v10/guilds/{gid}/members/search?query={user}"
+    starts_with: str,
+    gid: str = ACTIVE_GUILD_ID
+) -> list[User]:
+    """
+    Returns a list of User objects "whose username or nickname starts with a provided string"
+    """
+    url = f"https://discord.com/api/v10/guilds/{gid}/members/search?query={starts_with}"
     result = await _discord_request(url, TOKEN)
-    json = result.json()
+    return [
+        User(
+            entry["user"]["id"],
+            entry["user"]["username"],
+            entry["user"]["discriminator"],
+            entry["user"]["global_name"],
+            entry["user"]["avatar"]
+        ) for entry in result.json()
+    ]
 
-    if len(json) == 0:
-        return None
-    json = json[0]["user"]
-    return User(json["id"], json["username"], json["discriminator"], json["global_name"], json["avatar"])
+async def search_username(
+    username_starts_with: str,
+    gid: str = ACTIVE_GUILD_ID
+) -> list[User]:
+    """
+    Returns a list of User objects whose username starts with a provided string.
+
+    Will not return a user with a non-zero descriminator -> these users must update their discord version!
+    """
+    user_list = search_user(username_starts_with, gid)
+    return [
+        user for user in user_list
+        if user.username.startswith(username_starts_with)
+        and user.discriminator != "0"
+    ]
diff --git a/src/officers/urls.py b/src/officers/urls.py
index 7a18e2a..c1e2d96 100755
--- a/src/officers/urls.py
+++ b/src/officers/urls.py
@@ -184,9 +184,9 @@ async def update_info(
     request: Request,
     db_session: database.DBSession,
     computing_id: str,
-    new_officer_info: OfficerInfoUpload = Body(), # noqa: B008
+    officer_info_upload: OfficerInfoUpload = Body(), # noqa: B008
 ):
-    http_exception = new_officer_info.validate()
+    http_exception = officer_info_upload.validate()
     if http_exception is not None:
         raise http_exception
 
@@ -207,28 +207,39 @@ async def update_info(
     # TODO: turn this into a function first
     # get officer info
     query = sqlalchemy.select(OfficerInfo)
-    query = query.where(OfficerInfo.computing_id == new_officer_info.computing_id)
+    query = query.where(OfficerInfo.computing_id == officer_info_upload.computing_id)
     officer_info = await db_session.scalar(query)
     if officer_info is None:
         raise HTTPException(status_code=400, detail="officer_info does not exist yet, please create the officer info entry first")
 
-    # TODO: 1. validate phone-number
-    if not utils.is_valid_phone_number(new_officer_info.phone_number):
+    new_officer_info = officer_info_upload.to_officer_info(
+        computing_id=computing_id,
+        discord_id=None,
+        discord_nickname=None,
+    )
+
+    if not utils.is_valid_phone_number(officer_info_upload.phone_number):
         new_officer_info.phone_number = officer_info.phone_number
 
-    # TODO: use this API
-    discord_user = discord.search_user()
-    # TODO: 2.0 validate (find) discord-name in csss discord
-    # TODO: 2.1 use discord-name to find discord-id
-    # TODO: 2.3 use discord-name to find discord-nickname
+    # TODO: turn this into a function
+    validation_failures = []
+
+    discord_user = discord.search_username(officer_info_upload.discord_name)
+    if discord_user == []:
+        validation_failures += [f"unable to find discord user with the name {officer_info_upload.discord_name}"]
+        new_officer_info.discord_name = officer_info.discord_name
+    elif len(discord_user) >= 1:
+        validation_failures += [f"too many discord users start with {officer_info_upload.discord_name}"]
+        new_officer_info.discord_name = officer_info.discord_name
+    else:
+        new_officer_info.discord_id = discord_user.id
+        new_officer_info.discord_name = discord_user.username
+        # TODO: what value does the nickname have before it's set?
+        new_officer_info.discord_nickname = discord_user.global_name
 
     # TODO: 3. validate google-email using google module
 
-    success = await officers.crud.update_officer_info(db_session, new_officer_info.to_officer_info(
-        computing_id=computing_id,
-        discord_id=None,
-        discord_nickname=None,
-    ))
+    success = await officers.crud.update_officer_info(db_session, )
     if not success:
         raise HTTPException(status_code=400, detail="officer_info does not exist yet, please create the officer info entry first")
 

From 6823b731f85f3b7d824264845b7295288453b018 Mon Sep 17 00:00:00 2001
From: EarthenSky <24978329+EarthenSky@users.noreply.github.com>
Date: Fri, 30 Aug 2024 14:07:35 -0700
Subject: [PATCH 19/29] add function for getting current github permissions

---
 config/{env.sh => export_secrets.sh} |  0
 src/github/__init__.py               | 65 +++++++++++++++++++----
 src/github/internals.py              | 78 ++++++++++++++++++++--------
 src/github/types.py                  |  2 +-
 src/officers/tables.py               |  1 +
 5 files changed, 112 insertions(+), 34 deletions(-)
 rename config/{env.sh => export_secrets.sh} (100%)

diff --git a/config/env.sh b/config/export_secrets.sh
similarity index 100%
rename from config/env.sh
rename to config/export_secrets.sh
diff --git a/src/github/__init__.py b/src/github/__init__.py
index 646cd54..7af7591 100644
--- a/src/github/__init__.py
+++ b/src/github/__init__.py
@@ -1,34 +1,79 @@
 # TODO: does this allow importing anything from the module?
 
-from github.internals import 
+from github.internals import list_members, list_teams 
 from admin.email import send_email
 
+# Rules:
+# - all past officers will be members of the github org
+# - all past officers will be put in past_officers team
+# - all current officers will be put in the officers team
+# - 
+
+
 # TODO: move this to github.constants.py
 GITHUB_TEAMS = {
-    "doa" : "auto",
+    "doa": "auto",
     "election_officer": "auto",
+
     "officers": "auto",
+    # TODO: create the past_officers team
+    "past_officers": "auto",
+
     "w3_committee": "manual",
     "wall_e": "manual",
 }
+AUTO_GITHUB_TEAMS = [
+    team
+    for (name, kind) in GITHUB_TEAMS.items()
+    if kind == "auto"
+]
+
 
 # TODO: move these functions to github.public.py
 
 def current_permissions() -> list[GithubUserPermissions]:
-    person_list = []
+    """
+    return a list of members in the organization (org) & their permissions
+    """
 
-    # this function should return a list of members that have permisisons
-    
-    # get info for each person in an auto github team
+    member_list = list_members()
+    member_name_list = { member.name for member in member_list }
 
-    # log warning if there are any unknown teams
+    team_list = []
+    for team in list_teams():
+        if team.name not in GITHUB_TEAMS.keys():
+            _logger.warning(f"Found unexpected github team {team.name}")
+            continue
+        elif GITHUB_TEAMS[team.name] == "manual":
+            continue
 
-    # log error & email if there are any missing teams
-    # send_email("csss-sysadmin@sfu.ca", "ERROR: Missing Team", "...") 
-    pass
+        team_list += [team]
+
+    team_name_list = [team.name for team in team_list]
+    for team_name in AUTO_GITHUB_TEAMS:
+        if team_name not in team_name_list:
+            # TODO: send email for all errors & warnings 
+            # send_email("csss-sysadmin@sfu.ca", "ERROR: Missing Team", "...") 
+            _logger.error(f"Could not find 'auto' team {team_name} in organization")
+
+    user_permissions = {
+        user.username: GithubUserPermissions(user.username, [])
+        for user in member_list
+    }
+    for team in team_list: 
+        team_members = list_team_members(team.slug)
+        for member in team_members:
+            if member.name not in member_name_list:
+                _logger.warning(f"Found unexpected team_member={member.name} not in the organization")
+                continue
+            user_permissions[member.username].teams += [team.name]
+
+    return user_permissions.values()
 
 def invite_user(github_username: str):
     # invite this user to the github organization
     pass
 
+def add_to_team(github_username: str):
+    pass
 
diff --git a/src/github/internals.py b/src/github/internals.py
index 8892446..18f30df 100644
--- a/src/github/internals.py
+++ b/src/github/internals.py
@@ -3,11 +3,13 @@
 from typing import Any
 
 import requests
-from constants import github_org_name
+from constants import github_org_name as GITHUB_ORG_NAME
 from requests import Response
 
 from github.types import GithubUser, GithubTeam
 
+GITHUB_TOKEN = os.environ.get("GITHUB_TOKEN")
+
 async def _github_request_get(
     url: str,
     token: str
@@ -97,7 +99,7 @@ async def get_user_by_username(
     """
     result = await _github_request_get(
         f"https://api.github.com/users/{username}",
-        os.environ.get("GITHUB_TOKEN"),
+        GITHUB_TOKEN,
     )
     result_json = result.json()
     if result_json["status"] == "404":
@@ -115,8 +117,9 @@ async def get_user_by_id(
     """
     result = await _github_request_get(
         f"https://api.github.com/user/{uid}",
-        os.environ.get("GITHUB_TOKEN"),
+        GITHUB_TOKEN,
     )
+    # TODO: should we check the actual status of the response?
     result_json = result.json()
     if result_json["status"] == "404":
         return None
@@ -124,7 +127,7 @@ async def get_user_by_id(
         return GithubUser(result_json["login"], result_json["id"], result_json["name"])
 
 async def add_user_to_org(
-    org: str = github_org_name,
+    org: str = GITHUB_ORG_NAME,
     uid: str | None = None,
     email: str | None = None
 ) -> None:
@@ -140,13 +143,13 @@ async def add_user_to_org(
     elif uid is not None:
         result = await _github_request_post(
             f"https://api.github.com/orgs/{org}/invitations",
-            os.environ.get("GITHUB_TOKEN"),
+            GITHUB_TOKEN,
             dumps({"invitee_id":uid, "role":"direct_member"})
         )
     elif email is not None:
         result = await _github_request_post(
             f"https://api.github.com/orgs/{org}/invitations",
-            os.environ.get("GITHUB_TOKEN"),
+            GITHUB_TOKEN,
             dumps({"email":email, "role":"direct_member"})
         )
 
@@ -160,34 +163,48 @@ async def add_user_to_org(
 
 async def delete_user_from_org(
     username: str,
-    org: str = github_org_name
+    org: str = GITHUB_ORG_NAME
 ) -> None:
     if username is None:
         raise Exception("Username cannot be empty")
     result = await _github_request_delete(
-        f"https://api.github.com/orgs/{org}/memberships/{username}",
-        os.environ.get("GITHUB_TOKEN")
+        f"https://api.github.com/orgs/{org}/memberships/{username}", GITHUB_TOKEN
     )
 
     # Logging here potentially?
     if result.status_code != 204:
         raise Exception(f"Status code {result.status_code} returned when attempting to delete user {username} from organization {org}")
 
-async def get_teams(
-    org: str = github_org_name
+async def list_teams(
+    org: str = GITHUB_ORG_NAME
 ) -> list[str]:
-    result = await _github_request_get(f"https://api.github.com/orgs/{org}/teams", os.environ.get("GITHUB_TOKEN"))
-    json_result = result.json()
-    return [GithubTeam(team["id"], team["url"], team["name"], team["slug"]) for team in json_result]
+    result = await _github_request_get(f"https://api.github.com/orgs/{org}/teams", GITHUB_TOKEN)
+    return [
+        GithubTeam(team["id"], team["url"], team["name"], team["slug"])
+        for team in result.json()
+    ]
+
+async def list_team_members(
+    team_slug: str,
+    org: str = GITHUB_ORG_NAME
+):
+    result = await _github_request_get(
+        f"https://api.github.com/orgs/{org}/teams/{team_slug}/members",
+        GITHUB_TOKEN
+    )
+    return [
+        GithubUser(user["login"], user["id"], user["name"])
+        for user in result.json()
+    ]
 
 async def add_user_to_team(
     username: str,
-    slug: str,
-    org: str = github_org_name
+    team_slug: str,
+    org: str = GITHUB_ORG_NAME
 ) -> None:
     result = await _github_request_put(
-        f"https://api.github.com/orgs/{org}/teams/{slug}/memberships/{username}",
-        os.environ.get("GITHUB_TOKEN"),
+        f"https://api.github.com/orgs/{org}/teams/{team_slug}/memberships/{username}",
+        GITHUB_TOKEN,
         dumps({"role":"member"}),
     )
 
@@ -198,13 +215,28 @@ async def add_user_to_team(
 
 async def remove_user_from_team(
     username: str,
-    slug: str,
-    org: str = github_org_name
+    team_slug: str,
+    org: str = GITHUB_ORG_NAME
 ) -> None:
     result = await _github_request_delete(
-        f"https://api.github.com/orgs/{org}/teams/{slug}/memberships/{username}",
-        os.environ.get("GITHUB_TOKEN"),
+        f"https://api.github.com/orgs/{org}/teams/{team_slug}/memberships/{username}",
+        GITHUB_TOKEN,
     )
     if result.status_code != 204:
-        raise Exception(f"Status code {result.status_code} returned when attempting to delete user {username} from team {slug}")
+        raise Exception(f"Status code {result.status_code} returned when attempting to delete user {username} from team {team_slug}")
+
+async def list_members(
+    org: str = GITHUB_ORG_NAME
+) -> list[GithubUser]:
+    result = await _github_request_get(
+        f"https://api.github.com/orgs/{org}/members",
+        GITHUB_TOKEN
+    )
+
+    # TODO: check for errors
+
+    return [
+        GithubUser(user["login"], user["id"], user["name"])
+        for user in result.json()
+    ] 
 
diff --git a/src/github/types.py b/src/github/types.py
index 8db3e5a..9d1e09e 100644
--- a/src/github/types.py
+++ b/src/github/types.py
@@ -18,7 +18,7 @@ class GithubTeam:
 class GithubUserPermissions:
     # this class should store all the possible permissions a user might have
 
-    # used to connect the user to their officer info
+    # unique name used to connect the user to their officer info
     username: str
 
     # which github teams they're in
diff --git a/src/officers/tables.py b/src/officers/tables.py
index 9fe45a9..fd929e4 100644
--- a/src/officers/tables.py
+++ b/src/officers/tables.py
@@ -130,6 +130,7 @@ class OfficerInfo(Base):
         primary_key=True,
     )
     phone_number = Column(String(24))
+    # TODO: add unique constraint to this (stops users from stealing the username of someone else)
     github_username = Column(String(GITHUB_USERNAME_LEN))
 
     # A comma separated list of emails

From cb0a22974c076c7e1c2fdea12710f3906003eb6e Mon Sep 17 00:00:00 2001
From: EarthenSky <24978329+EarthenSky@users.noreply.github.com>
Date: Fri, 30 Aug 2024 15:00:15 -0700
Subject: [PATCH 20/29] add function to update github permissions

---
 src/cron/daily.py      | 77 ++++++++++++++++++++++++++++++++++--------
 src/github/__init__.py | 23 ++++++++-----
 2 files changed, 78 insertions(+), 22 deletions(-)

diff --git a/src/cron/daily.py b/src/cron/daily.py
index e8b4b00..5616db0 100644
--- a/src/cron/daily.py
+++ b/src/cron/daily.py
@@ -5,36 +5,85 @@
 
 from database import _db_session
 from officers.crud import officer_terms
+from officers.constants import OfficerPosition
 
 import github
 import google
 
 _logger = logging.getLogger(__name__)
 
-async def update_permissions():
-    db_session = _db_session()
-
+async def update_google_permissions(db_session):
     google_permissions = google.current_permissions()
-    github_permissions = github.current_permissions()
-
-    one_year_ago = datetime.today() - timedelta(days=365)
+    #one_year_ago = datetime.today() - timedelta(days=365)
 
     # TODO: for performance, only include officers with recent end-date (1 yr)
+    # but measure performance first
     all_officer_terms = await all_officer_terms(db_session)
     for term in all_officer_terms:
         if utils.is_active(term):
-            # TODO: if google drive permissions is not active, update them
-            # TODO: if github permissions is not active, update them
+            # TODO: if google drive permission is not active, update them
             pass
-        elif utils.end_date <= one_year_ago:
-            # ignore old executives
-            continue
         else:
             # TODO: if google drive permissions are active, remove them
-            # TODO: if github permissions are active, remove them
-            pass 
+            pass
+
+    _logger.info("updated google permissions")
+
+async def update_github_permissions(db_session):
+    github_permissions = github.current_permissions()
+    #one_year_ago = datetime.today() - timedelta(days=365)
+
+    # TODO: for performance, only include officers with recent end-date (1 yr)
+    # but measure performance first
+    all_officer_terms = await all_officer_terms(db_session)
+    for term in all_officer_terms:
+        if term.username not in github_permissions:
+            # will wait another day until giving a person their required permissions
+            # TODO: setup a hook or something?
+            github.invite_user(term.username)
+            continue
+
+        if utils.is_active(term):
+            # move all active officers to their respective teams
+            if term.position == OfficerPosition.DIRECTOR_OF_ARCHIVES:
+                github.set_user_teams(
+                    term.username,
+                    github_permissions[term.username].teams,
+                    ["doa", "officers"]
+                )
+            elif term.position == OfficerPosition.ELECTION_OFFICER:
+                github.set_user_teams(
+                    term.username,
+                    github_permissions[term.username].teams,
+                    ["election_officer", "officers"]
+                )
+            else:
+                github.set_user_teams(
+                    term.username,
+                    github_permissions[term.username].teams,
+                    ["officers"]
+                )
+
+        else:
+            # move all inactive officers to the past_officers github organization
+            github.set_user_teams(
+                term.username,
+                github_permissions[term.username].teams,
+                ["past_officers"]
+            )
+
+    _logger.info("updated github permissions")
+
+async def update_permissions():
+    db_session = _db_session()
+
+    update google_permissions(db_session)
+    db_session.commit()
+
+    update_github_permissions(db_session)
+    db_session.commit()
 
-    _logger.info("Complete permissions update")
+    _logger.info("all permissions updated")
 
 if __name__ == "__main__":
     asyncio.run(update_permissions())
diff --git a/src/github/__init__.py b/src/github/__init__.py
index 7af7591..2bb0434 100644
--- a/src/github/__init__.py
+++ b/src/github/__init__.py
@@ -28,10 +28,9 @@
     if kind == "auto"
 ]
 
-
 # TODO: move these functions to github.public.py
 
-def current_permissions() -> list[GithubUserPermissions]:
+def current_permissions() -> dict[str, GithubUserPermissions]:
     """
     return a list of members in the organization (org) & their permissions
     """
@@ -64,16 +63,24 @@ def current_permissions() -> list[GithubUserPermissions]:
         team_members = list_team_members(team.slug)
         for member in team_members:
             if member.name not in member_name_list:
-                _logger.warning(f"Found unexpected team_member={member.name} not in the organization")
+                _logger.warning(f"Found unexpected team_member={member.name} in team_slug={team.slug} not in the organization")
                 continue
-            user_permissions[member.username].teams += [team.name]
+            user_permissions[member.username].teams += [team.slug]
+
+    return user_permissions
+
+def set_user_teams(username: str, old_teams: list[str], new_teams: list[str]):
+    for team_slug in old_teams:
+        if team_slug not in new_teams:
+            remove_user_from_team(term.username, team_slug)
 
-    return user_permissions.values()
+    for team_slug in new_teams:
+        if team_slug not in old_teams:
+            # TODO: what happens when adding a user to a team who is not part of the github org yet?
+            add_user_to_team(term.username, team_slug)
 
 def invite_user(github_username: str):
     # invite this user to the github organization
-    pass
-
-def add_to_team(github_username: str):
+    # TODO: is an invited user considered a member of the organization?
     pass
 

From 5537b19661fcafab846f60e91e71d018d5b61a28 Mon Sep 17 00:00:00 2001
From: EarthenSky <24978329+EarthenSky@users.noreply.github.com>
Date: Fri, 30 Aug 2024 16:03:46 -0700
Subject: [PATCH 21/29] add invite endpoint & simplify implementation

---
 src/cron/daily.py       | 47 ++++++++++++-----------------------------
 src/github/__init__.py  | 21 +++++++++++++++---
 src/github/internals.py | 43 +++++++++++++++----------------------
 3 files changed, 49 insertions(+), 62 deletions(-)

diff --git a/src/cron/daily.py b/src/cron/daily.py
index 5616db0..f085114 100644
--- a/src/cron/daily.py
+++ b/src/cron/daily.py
@@ -5,7 +5,6 @@
 
 from database import _db_session
 from officers.crud import officer_terms
-from officers.constants import OfficerPosition
 
 import github
 import google
@@ -30,46 +29,28 @@ async def update_google_permissions(db_session):
     _logger.info("updated google permissions")
 
 async def update_github_permissions(db_session):
-    github_permissions = github.current_permissions()
-    #one_year_ago = datetime.today() - timedelta(days=365)
+    github_permissions, team_id_map = github.all_permissions()
 
-    # TODO: for performance, only include officers with recent end-date (1 yr)
-    # but measure performance first
     all_officer_terms = await all_officer_terms(db_session)
     for term in all_officer_terms:
-        if term.username not in github_permissions:
-            # will wait another day until giving a person their required permissions
-            # TODO: setup a hook or something?
-            github.invite_user(term.username)
-            continue
-
-        if utils.is_active(term):
+        new_teams = (
             # move all active officers to their respective teams
-            if term.position == OfficerPosition.DIRECTOR_OF_ARCHIVES:
-                github.set_user_teams(
-                    term.username,
-                    github_permissions[term.username].teams,
-                    ["doa", "officers"]
-                )
-            elif term.position == OfficerPosition.ELECTION_OFFICER:
-                github.set_user_teams(
-                    term.username,
-                    github_permissions[term.username].teams,
-                    ["election_officer", "officers"]
-                )
-            else:
-                github.set_user_teams(
-                    term.username,
-                    github_permissions[term.username].teams,
-                    ["officers"]
-                )
-
-        else:
+            github.officer_teams(term.position)
+            if utils.is_active(term)
             # move all inactive officers to the past_officers github organization
+            else ["past_officers"]
+        )
+        if term.username not in github_permissions:
+            user = get_user_by_username(term.username)
+            github.invite_user(
+                user.id,
+                [team_id_map[team] for team in new_teams], 
+            )
+        else:
             github.set_user_teams(
                 term.username,
                 github_permissions[term.username].teams,
-                ["past_officers"]
+                new_teams
             )
 
     _logger.info("updated github permissions")
diff --git a/src/github/__init__.py b/src/github/__init__.py
index 2bb0434..fda5809 100644
--- a/src/github/__init__.py
+++ b/src/github/__init__.py
@@ -3,6 +3,8 @@
 from github.internals import list_members, list_teams 
 from admin.email import send_email
 
+from officers.constants import OfficerPosition
+
 # Rules:
 # - all past officers will be members of the github org
 # - all past officers will be put in past_officers team
@@ -28,9 +30,17 @@
     if kind == "auto"
 ]
 
+def officer_teams(position: str) -> list[str]:
+    if position == OfficerPosition.DIRECTOR_OF_ARCHIVES:
+        return ["doa", "officers"]
+    elif position == OfficerPosition.ELECTIONS_OFFICER:
+        return ["election_officer", "officers"]
+    else:
+        return ["officers"]
+
 # TODO: move these functions to github.public.py
 
-def current_permissions() -> dict[str, GithubUserPermissions]:
+def all_permissions() -> dict[str, GithubUserPermissions]:
     """
     return a list of members in the organization (org) & their permissions
     """
@@ -67,7 +77,12 @@ def current_permissions() -> dict[str, GithubUserPermissions]:
                 continue
             user_permissions[member.username].teams += [team.slug]
 
-    return user_permissions
+    # create a mapping between team name & team id, for use in creating invitations
+    team_id_map = {}
+    for team in team_list:
+        team_id_map[team.slug] = team.id
+
+    return user_permissions, team_id_map
 
 def set_user_teams(username: str, old_teams: list[str], new_teams: list[str]):
     for team_slug in old_teams:
@@ -79,7 +94,7 @@ def set_user_teams(username: str, old_teams: list[str], new_teams: list[str]):
             # TODO: what happens when adding a user to a team who is not part of the github org yet?
             add_user_to_team(term.username, team_slug)
 
-def invite_user(github_username: str):
+def invite_user(github_username: str, teams: str):
     # invite this user to the github organization
     # TODO: is an invited user considered a member of the organization?
     pass
diff --git a/src/github/internals.py b/src/github/internals.py
index 18f30df..8c7ecd6 100644
--- a/src/github/internals.py
+++ b/src/github/internals.py
@@ -126,38 +126,29 @@ async def get_user_by_id(
     else:
         return GithubUser(result_json["login"], result_json["id"], result_json["name"])
 
-async def add_user_to_org(
+# TODO: if needed, add support for getting user by email
+
+async def invite_user(
+    uid: str,
+    team_id_list: Optional[list[str]] = None,
     org: str = GITHUB_ORG_NAME,
-    uid: str | None = None,
-    email: str | None = None
 ) -> None:
-    """
-        Takes one of either uid or email. Fails if provided both.
-    """
-    result = None
-    if uid is None and email is None:
-        raise ValueError("uid and username cannot both be empty")
-    elif uid is not None and email is not None:
-        raise ValueError("cannot populate both uid and email")
-    # Arbitrarily prefer uid
-    elif uid is not None:
-        result = await _github_request_post(
-            f"https://api.github.com/orgs/{org}/invitations",
-            GITHUB_TOKEN,
-            dumps({"invitee_id":uid, "role":"direct_member"})
-        )
-    elif email is not None:
-        result = await _github_request_post(
-            f"https://api.github.com/orgs/{org}/invitations",
-            GITHUB_TOKEN,
-            dumps({"email":email, "role":"direct_member"})
-        )
+    """Invites the user & gives them access to the supplied teams"""
+    # TODO: how long until the invite goes out of date?
+    if team_id_list is None:
+        team_id_list = []
+
+    result = await _github_request_post(
+        f"https://api.github.com/orgs/{org}/invitations",
+        GITHUB_TOKEN,
+        dumps({"invitee_id":uid, "role":"direct_member", "team_ids":team_id_list})
+    )
 
     # Logging here potentially?
     if result.status_code != 201:
         result_json = result.json()
         raise Exception(
-            f"Status code {result.status_code} returned when attempting to add user to org: "
+            f"Status code {result.status_code} returned when attempting to invite user: "
             f"{result_json['message']}: {[error['message'] for error in result_json['errors']]}"
         )
 
@@ -166,7 +157,7 @@ async def delete_user_from_org(
     org: str = GITHUB_ORG_NAME
 ) -> None:
     if username is None:
-        raise Exception("Username cannot be empty")
+        raise ValueError("Username cannot be empty")
     result = await _github_request_delete(
         f"https://api.github.com/orgs/{org}/memberships/{username}", GITHUB_TOKEN
     )

From b9e47de18e7cc316620e543bfe287c94791ae7ac Mon Sep 17 00:00:00 2001
From: Gabe <24978329+EarthenSky@users.noreply.github.com>
Date: Sat, 31 Aug 2024 21:50:33 -0700
Subject: [PATCH 22/29] Add Google Drive API (#81)

* add basic google drive api usage

* finish off lightweight google api module
---
 .gitignore                       |  4 ++
 requirements.txt                 |  1 +
 src/google_api/__init__.py       |  0
 src/google_api/constants.py      | 34 ++++++++++++++++
 src/google_api/internals.py      | 68 ++++++++++++++++++++++++++++++++
 src/officers/types.py            |  4 +-
 src/utils.py                     |  4 ++
 tests/integration/test_google.py | 25 ++++++++++++
 8 files changed, 138 insertions(+), 2 deletions(-)
 create mode 100644 src/google_api/__init__.py
 create mode 100644 src/google_api/constants.py
 create mode 100644 src/google_api/internals.py
 create mode 100644 tests/integration/test_google.py

diff --git a/.gitignore b/.gitignore
index e869bd6..12a75c6 100755
--- a/.gitignore
+++ b/.gitignore
@@ -1,6 +1,10 @@
+# main
 src/run/
 logs/
 
+# google drive api
+google_key.json
+
 # Python - Byte-compiled / optimized / DLL files
 __pycache__/
 *.py[cod]
diff --git a/requirements.txt b/requirements.txt
index 1a2989f..7b350cf 100755
--- a/requirements.txt
+++ b/requirements.txt
@@ -5,6 +5,7 @@ uvicorn[standard]==0.27.1
 sqlalchemy==2.0.27
 asyncpg==0.29.0
 alembic==1.13.1
+google-api-python-client==2.143.0
 
 # minor
 pyOpenSSL==24.0.0 # for generating cryptographically secure random numbers
diff --git a/src/google_api/__init__.py b/src/google_api/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/src/google_api/constants.py b/src/google_api/constants.py
new file mode 100644
index 0000000..f547817
--- /dev/null
+++ b/src/google_api/constants.py
@@ -0,0 +1,34 @@
+import os
+import pathlib
+
+# this google account runs the google workspace for executives
+GOOGLE_WORKSPACE_ACCOUNT = "csss@sfucsss.org"
+
+# any officer from the past 5 semesters has access to these
+# TODO: ask the pres if we still want these rules, or not
+FIVE_SEM_OFFICER_ACCESS = [
+    "CSSS@SFU",
+]
+
+EXECUTIVE_ACCESS = [
+    "CSSS Gallery",
+    "CSSS@SFU",
+    "Deep-Exec",
+    "Exec_Photos",
+    "Private Gallery",
+]
+
+# scopes are like permissions to google
+GOOGLE_API_SCOPES = [
+    # google drive permission
+    "https://www.googleapis.com/auth/drive"
+]
+
+# TODO: make this into an enum, or something
+GOOGLE_DRIVE_PERMISSION_ROLES = [
+    "organizer",
+    "fileOrganizer",
+]
+
+_this_file_path = pathlib.Path(__file__).parent.resolve()
+SERVICE_ACCOUNT_KEY_PATH = str((_this_file_path / "../../google_key.json").resolve())
diff --git a/src/google_api/internals.py b/src/google_api/internals.py
new file mode 100644
index 0000000..2246076
--- /dev/null
+++ b/src/google_api/internals.py
@@ -0,0 +1,68 @@
+# google workspace (shared drives) + google drive api
+
+from google.oauth2 import service_account
+from googleapiclient.discovery import build
+
+from google_api.constants import GOOGLE_API_SCOPES, GOOGLE_WORKSPACE_ACCOUNT, SERVICE_ACCOUNT_KEY_PATH
+
+# TODO: understand how these work
+credentials = service_account.Credentials.from_service_account_file(
+    filename=SERVICE_ACCOUNT_KEY_PATH,
+    scopes=GOOGLE_API_SCOPES
+)
+delegated_credentials = credentials.with_subject(GOOGLE_WORKSPACE_ACCOUNT)
+service = build("drive", "v3", credentials=delegated_credentials)
+
+def _list_shared_drives() -> list:
+    return (
+        service
+        .drives()
+        .list(
+            #pageSize = 50,
+            #q = "name contains 'CSSS'",
+            #useDomainAdminAccess = True,
+        )
+        .execute()
+    )
+
+def list_drive_permissions(drive_id: str) -> list:
+    return (
+        service
+        .permissions()
+        .list(
+            fileId = drive_id,
+            # important to find the shared drive
+            supportsAllDrives = True,
+            fields = "*",
+        )
+        .execute()
+    )
+
+def create_drive_permission(drive_id: str, permission: dict):
+    return (
+        service
+        .permissions()
+        .create(
+            fileId = drive_id,
+
+            # TODO: update message
+            emailMessage = "You were just given permission to an SFU CSSS shared google drive!",
+            sendNotificationEmail = True,
+            supportsAllDrives = True,
+
+            body=permission,
+        )
+        .execute()
+    )
+
+def delete_drive_permission(drive_id: str, permission_id: str):
+    return (
+        service
+        .permissions()
+        .delete(
+            fileId = drive_id,
+            permissionId = permission_id,
+            supportsAllDrives = True,
+        )
+        .execute()
+    )
diff --git a/src/officers/types.py b/src/officers/types.py
index 7546991..12a2784 100644
--- a/src/officers/types.py
+++ b/src/officers/types.py
@@ -166,8 +166,8 @@ def serializable_dict(self):
 
     @staticmethod
     def from_data(
-        term: officers.tables.OfficerTerm,
-        officer_info: officers.tables.OfficerInfo,
+        term: OfficerTerm,
+        officer_info: OfficerInfo,
         include_private: bool,
         is_active: bool,
     ) -> OfficerData:
diff --git a/src/utils.py b/src/utils.py
index 5b56970..39758e1 100644
--- a/src/utils.py
+++ b/src/utils.py
@@ -1,3 +1,4 @@
+import re
 from datetime import datetime
 
 from officers.tables import OfficerInfo, OfficerTerm
@@ -32,3 +33,6 @@ def is_valid_phone_number(phone_number: str) -> bool:
         len(phone_number) == 10
         and phone_number.isnumeric()
     )
+
+def is_valid_email(email: str):
+    return not re.match(r"^[^@]+@[^@]+\.[a-zA-Z]*$", email)
diff --git a/tests/integration/test_google.py b/tests/integration/test_google.py
new file mode 100644
index 0000000..5b41c59
--- /dev/null
+++ b/tests/integration/test_google.py
@@ -0,0 +1,25 @@
+from google_api import internals
+
+
+def test__list_drives():
+    """should not fail"""
+    drive_list = internals._list_shared_drives()
+    print(drive_list)
+
+    drive_id = drive_list["drives"][0]["id"]
+    print(drive_id)
+
+    permissions = internals.list_drive_permissions(drive_id)
+    print(permissions)
+
+    # NOTE: this will raise an exception if the email address is a non-google account
+    """
+    internals.create_drive_permission(
+        drive_id,
+        {
+            "type": "user",
+            "emailAddress": "tester_123591735013000019@gmail2.ca",
+            "role": "fileOrganizer",
+        }
+    )
+    """

From 711b90e7882c7f6893a7babbace237663660e009 Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Sun, 1 Sep 2024 15:23:05 -0700
Subject: [PATCH 23/29] fix bugs, add support for using discord API in user
 login page

---
 src/discord/discord.py            | 42 ++++++++++++++++----
 src/officers/crud.py              |  6 ++-
 src/officers/tables.py            | 16 ++++++++
 src/officers/types.py             | 15 -------
 src/officers/urls.py              | 66 +++++++++++++++++++++----------
 src/utils.py                      |  3 +-
 tests/integration/test_discord.py |  9 +++++
 tests/integration/test_google.py  |  1 +
 8 files changed, 112 insertions(+), 46 deletions(-)
 create mode 100644 tests/integration/test_discord.py

diff --git a/src/discord/discord.py b/src/discord/discord.py
index 334b99f..ffcd998 100644
--- a/src/discord/discord.py
+++ b/src/discord/discord.py
@@ -251,8 +251,21 @@ async def get_guild_members(
     url = f"https://discord.com/api/v10/guilds/{gid}/members?limit=1000"
     result = await _discord_request(url, TOKEN)
 
-    result_json = result.json()
-    users = [GuildMember(User(user["user"]["id"], user["user"]["username"], user["user"]["discriminator"], user["user"]["global_name"], user["user"]["avatar"]), user["roles"]) for user in result_json]
+    if result.status_code != 200:
+        raise Exception(f"Got unexpected error result: {result.json()}")
+
+    users = [
+        GuildMember(
+            User(
+                user["user"]["id"],
+                user["user"]["username"],
+                user["user"]["discriminator"],
+                user["user"]["global_name"],
+                user["user"]["avatar"],
+            ),
+            user["roles"]
+        ) for user in result.json()
+    ]
     last_uid = users[-1].user.id
 
     while True:
@@ -260,11 +273,21 @@ async def get_guild_members(
         result = await _discord_request(url, TOKEN)
 
         result_json = result.json()
-
         if len(result_json) == 0:
             return users
 
-        res = [GuildMember(User(user["user"]["id"], user["user"]["username"], user["user"]["discriminator"], user["user"]["global_name"], user["user"]["avatar"]), user["roles"]) for user in result_json]
+        res = [
+            GuildMember(
+                User(
+                    user["user"]["id"],
+                    user["user"]["username"],
+                    user["user"]["discriminator"],
+                    user["user"]["global_name"],
+                    user["user"]["avatar"],
+                ),
+                user["roles"]
+            ) for user in result_json
+        ]
         users += res
 
         last_uid = res[-1].user.id
@@ -329,12 +352,17 @@ async def get_channels_by_category_id(
 
 async def search_user(
     starts_with: str,
+    limit: int = 1,
     gid: str = ACTIVE_GUILD_ID
 ) -> list[User]:
     """
     Returns a list of User objects "whose username or nickname starts with a provided string"
     """
-    url = f"https://discord.com/api/v10/guilds/{gid}/members/search?query={starts_with}"
+    if starts_with == "":
+        raise ValueError("starts_with must be non-empty string; use get_guild_members instead if desired.")
+
+    url = f"https://discord.com/api/v10/guilds/{gid}/members/search?query={starts_with}&limit={limit}"
+
     result = await _discord_request(url, TOKEN)
     return [
         User(
@@ -355,9 +383,9 @@ async def search_username(
 
     Will not return a user with a non-zero descriminator -> these users must update their discord version!
     """
-    user_list = search_user(username_starts_with, gid)
+    user_list = await search_user(username_starts_with, 2, gid)
     return [
         user for user in user_list
         if user.username.startswith(username_starts_with)
-        and user.discriminator != "0"
+        and user.discriminator == "0"
     ]
diff --git a/src/officers/crud.py b/src/officers/crud.py
index 5497dea..379e46e 100644
--- a/src/officers/crud.py
+++ b/src/officers/crud.py
@@ -190,7 +190,8 @@ async def update_officer_info(db_session: database.DBSession, new_officer_info:
     if officer_info is None:
         return False
 
-    # TODO: how to detect an entry insert error?
+    # TODO: how to detect an entry insert error? For example, what happens if
+    # we try to set our discord id to be the same as another executive's?
     query = (
         sqlalchemy
         .update(OfficerInfo)
@@ -245,7 +246,8 @@ async def update_officer_term(
         .where(OfficerTerm.computing_id == officer_term_data.computing_id)
         .where(OfficerTerm.position == officer_term_data.position)
         .where(OfficerTerm.start_date == officer_term_data.start_date)
-        .values(officer_term_data.to_update_dict())
+        # TODO: fix this by passing params to to_officer_info
+        .values(officer_term_data.to_officer_info().to_update_dict())
     )
 
     await db_session.execute(query)
diff --git a/src/officers/tables.py b/src/officers/tables.py
index f0e3fe7..1ae5e5f 100644
--- a/src/officers/tables.py
+++ b/src/officers/tables.py
@@ -156,3 +156,19 @@ def is_filled_in(self):
             and self.google_drive_email is not None
             and self.github_username is not None
         )
+
+    def to_update_dict(self) -> dict:
+        return {
+            # TODO: if the API call to SFU's api to get legal name fails, we want to fail & not insert the entry.
+            # for now, we should insert a default value
+            "legal_name": "default name" if self.legal_name is None else self.legal_name,
+
+            "discord_id": self.discord_id,
+            "discord_name": self.discord_name,
+            "discord_nickname": self.discord_nickname,
+
+            "phone_number": self.phone_number,
+            "github_username": self.github_username,
+            "google_drive_email": self.google_drive_email,
+        }
+
diff --git a/src/officers/types.py b/src/officers/types.py
index 12a2784..d6bd649 100644
--- a/src/officers/types.py
+++ b/src/officers/types.py
@@ -40,21 +40,6 @@ def to_officer_info(self, computing_id: str, discord_id: str | None, discord_nic
             google_drive_email = self.google_drive_email,
         )
 
-    def to_update_dict(self) -> dict:
-        return {
-            # TODO: if the API call to SFU's api to get legal name fails, we want to fail & not insert the entry.
-            # for now, we should insert a default value
-            "legal_name": "default name" if self.legal_name is None else self.legal_name,
-
-            "discord_id": self.discord_id,
-            "discord_name": self.discord_name,
-            "discord_nickname": self.discord_nickname,
-
-            "phone_number": self.phone_number,
-            "github_username": self.github_username,
-            "google_drive_email": self.google_drive_email,
-        }
-
 @dataclass
 class OfficerTermData:
     computing_id: str
diff --git a/src/officers/urls.py b/src/officers/urls.py
index c1e2d96..be4e910 100755
--- a/src/officers/urls.py
+++ b/src/officers/urls.py
@@ -207,7 +207,7 @@ async def update_info(
     # TODO: turn this into a function first
     # get officer info
     query = sqlalchemy.select(OfficerInfo)
-    query = query.where(OfficerInfo.computing_id == officer_info_upload.computing_id)
+    query = query.where(OfficerInfo.computing_id == computing_id)
     officer_info = await db_session.scalar(query)
     if officer_info is None:
         raise HTTPException(status_code=400, detail="officer_info does not exist yet, please create the officer info entry first")
@@ -218,38 +218,64 @@ async def update_info(
         discord_nickname=None,
     )
 
-    if not utils.is_valid_phone_number(officer_info_upload.phone_number):
-        new_officer_info.phone_number = officer_info.phone_number
-
     # TODO: turn this into a function
     validation_failures = []
 
-    discord_user = discord.search_username(officer_info_upload.discord_name)
-    if discord_user == []:
-        validation_failures += [f"unable to find discord user with the name {officer_info_upload.discord_name}"]
-        new_officer_info.discord_name = officer_info.discord_name
-    elif len(discord_user) >= 1:
-        validation_failures += [f"too many discord users start with {officer_info_upload.discord_name}"]
-        new_officer_info.discord_name = officer_info.discord_name
+    if not utils.is_valid_phone_number(officer_info_upload.phone_number):
+        validation_failures += [f"invalid phone number {officer_info_upload.phone_number}"]
+        new_officer_info.phone_number = officer_info.phone_number
+
+    if officer_info_upload.discord_name is None or officer_info_upload.discord_name == "":
+        new_officer_info.discord_name = None
+        new_officer_info.discord_id = None
+        new_officer_info.discord_nickname = None
     else:
-        new_officer_info.discord_id = discord_user.id
-        new_officer_info.discord_name = discord_user.username
-        # TODO: what value does the nickname have before it's set?
-        new_officer_info.discord_nickname = discord_user.global_name
+        discord_user_list = await discord.search_username(officer_info_upload.discord_name)
+        if discord_user_list == []:
+            validation_failures += [f"unable to find discord user with the name {officer_info_upload.discord_name}"]
+            new_officer_info.discord_name = officer_info.discord_name
+            new_officer_info.discord_id = officer_info.discord_id
+            new_officer_info.discord_nickname = officer_info.discord_nickname
+        elif len(discord_user_list) > 1:
+            validation_failures += [f"too many discord users start with {officer_info_upload.discord_name}"]
+            new_officer_info.discord_name = officer_info.discord_name
+            new_officer_info.discord_id = officer_info.discord_id
+            new_officer_info.discord_nickname = officer_info.discord_nickname
+        else:
+            discord_user = discord_user_list[0]
+            new_officer_info.discord_name = discord_user.username
+            new_officer_info.discord_id = discord_user.id
+            new_officer_info.discord_nickname = (
+                discord_user.global_name
+                if discord_user.global_name is not None
+                else discord_user.username
+            )
 
-    # TODO: 3. validate google-email using google module
+    # TODO: validate google-email using google module, by trying to assign the user to a permission or something
+    if not utils.is_valid_email(officer_info_upload.google_drive_email):
+        validation_failures += [f"invalid email format {officer_info_upload.google_drive_email}"]
+        new_officer_info.google_drive_email = officer_info.google_drive_email
 
-    success = await officers.crud.update_officer_info(db_session, )
+    # TODO: validate github user
+
+    # TODO: invite github user
+
+    # TODO: detect if changing github user & remove old one
+
+    success = await officers.crud.update_officer_info(db_session, new_officer_info)
     if not success:
         raise HTTPException(status_code=400, detail="officer_info does not exist yet, please create the officer info entry first")
 
+    await db_session.commit()
+
     updated_officer_info = await officers.crud.officer_info(db_session, computing_id)
     if updated_officer_info is None:
         raise HTTPException(status_code=500, detail="failed to get officer info?! something is very wrong...")
 
-    await db_session.commit()
-
-    return JSONResponse(updated_officer_info.serializable_dict())
+    return JSONResponse({
+        "updated_officer_info": updated_officer_info.serializable_dict(),
+        "validation_failures": validation_failures,
+    })
 
 # TODO: access term by term_id
 # TODO: only allow access if the user is admin or if the id is their term
diff --git a/src/utils.py b/src/utils.py
index 39758e1..7b7a762 100644
--- a/src/utils.py
+++ b/src/utils.py
@@ -15,7 +15,6 @@ def is_iso_format(date_str: str) -> bool:
     except ValueError:
         return False
 
-
 def is_active_officer(query: Select) -> Select:
     # TODO: assert this constraint at the SQL level, so that we don't even have to check it?
     query = query.where(
@@ -35,4 +34,4 @@ def is_valid_phone_number(phone_number: str) -> bool:
     )
 
 def is_valid_email(email: str):
-    return not re.match(r"^[^@]+@[^@]+\.[a-zA-Z]*$", email)
+    return re.match(r"^[^@]+@[^@]+\.[a-zA-Z]*$", email)
diff --git a/tests/integration/test_discord.py b/tests/integration/test_discord.py
new file mode 100644
index 0000000..31f69a0
--- /dev/null
+++ b/tests/integration/test_discord.py
@@ -0,0 +1,9 @@
+import pytest
+from discord import discord
+
+
+# NOTE: must perform setup as described in the csss-site-backend wiki
+@pytest.mark.asyncio
+async def test__list_users():
+    guild_members = await discord.get_guild_members()
+    print(guild_members)
diff --git a/tests/integration/test_google.py b/tests/integration/test_google.py
index 5b41c59..c479aad 100644
--- a/tests/integration/test_google.py
+++ b/tests/integration/test_google.py
@@ -1,6 +1,7 @@
 from google_api import internals
 
 
+# NOTE: must perform setup as described in the csss-site-backend wiki
 def test__list_drives():
     """should not fail"""
     drive_list = internals._list_shared_drives()

From 14ea691fa71bf64cf63602a1f6a9104944f5a0ea Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Sun, 1 Sep 2024 15:33:38 -0700
Subject: [PATCH 24/29] get more discord users, in case of nickname collisions

---
 src/discord/discord.py                       |  3 ++-
 src/officers/urls.py                         |  5 +----
 src/scripts/load_officer_data_from_old_db.py | 11 +++++++++++
 3 files changed, 14 insertions(+), 5 deletions(-)
 create mode 100644 src/scripts/load_officer_data_from_old_db.py

diff --git a/src/discord/discord.py b/src/discord/discord.py
index ffcd998..b2ebba2 100644
--- a/src/discord/discord.py
+++ b/src/discord/discord.py
@@ -383,7 +383,8 @@ async def search_username(
 
     Will not return a user with a non-zero descriminator -> these users must update their discord version!
     """
-    user_list = await search_user(username_starts_with, 2, gid)
+    # if there are more than 100 users with the same nickname as the "username_starts_with" string, this may fail
+    user_list = await search_user(username_starts_with, 99, gid)
     return [
         user for user in user_list
         if user.username.startswith(username_starts_with)
diff --git a/src/officers/urls.py b/src/officers/urls.py
index be4e910..e5780a9 100755
--- a/src/officers/urls.py
+++ b/src/officers/urls.py
@@ -257,10 +257,8 @@ async def update_info(
         new_officer_info.google_drive_email = officer_info.google_drive_email
 
     # TODO: validate github user
-
     # TODO: invite github user
-
-    # TODO: detect if changing github user & remove old one
+    # TODO: detect if changing github username & uninvite old user
 
     success = await officers.crud.update_officer_info(db_session, new_officer_info)
     if not success:
@@ -313,7 +311,6 @@ async def update_term(
     await db_session.commit()
     return PlainTextResponse("ok")
 
-
 """
 # TODO: test this error later
 @router.get("/please_error", description="Raises an error & should send an email to the sysadmin")
diff --git a/src/scripts/load_officer_data_from_old_db.py b/src/scripts/load_officer_data_from_old_db.py
new file mode 100644
index 0000000..21e992f
--- /dev/null
+++ b/src/scripts/load_officer_data_from_old_db.py
@@ -0,0 +1,11 @@
+# This file loads officers from the old csss-site's database
+# see: https://github.com/CSSS/csss-site
+
+# This file is a combination of code and manual steps to be performed.
+
+DB_NAME = "csss-site-db-old"
+DB_PORT = 5117
+
+# 1. make a local copy of the db as a docker container, as named above
+
+# TODO: write code to read & write between the databases

From 916d0d44575c58aa6744fff6c0ce5d518686b994 Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Sun, 1 Sep 2024 16:45:31 -0700
Subject: [PATCH 25/29] tests, formatting, use github API to validate username

---
 src/admin/email.py               |  4 ++--
 src/github/__init__.py           | 24 +++++++++----------
 src/github/internals.py          | 40 ++++++++++++++++++--------------
 src/github/types.py              |  1 +
 src/officers/urls.py             |  7 +++++-
 tests/integration/test_github.py | 25 ++++++++++++++++++++
 6 files changed, 69 insertions(+), 32 deletions(-)
 create mode 100644 tests/integration/test_github.py

diff --git a/src/admin/email.py b/src/admin/email.py
index c1987b2..88a5b9f 100644
--- a/src/admin/email.py
+++ b/src/admin/email.py
@@ -2,7 +2,7 @@
 import smtplib
 
 # TODO: set this up
-GMAIL_PASSWORD = os.environ['GMAIL_PASSWORD']
+GMAIL_PASSWORD = os.environ.get("GMAIL_PASSWORD")
 GMAIL_ADDRESS = "csss-site@gmail.com"
 
 # TODO: look into sending emails from an sfu maillist (this might be painful)
@@ -11,7 +11,7 @@ def send_email(
     subject: str,
     contents: str,
 ):
-    mail = smtplib.SMTP('smtp.gmail.com', 587)
+    mail = smtplib.SMTP("smtp.gmail.com", 587)
     mail.ehlo()
     mail.starttls()
     mail.login(GMAIL_ADDRESS, GMAIL_PASSWORD)
diff --git a/src/github/__init__.py b/src/github/__init__.py
index fda5809..67fb7bc 100644
--- a/src/github/__init__.py
+++ b/src/github/__init__.py
@@ -1,16 +1,18 @@
 # TODO: does this allow importing anything from the module?
+import logging
 
-from github.internals import list_members, list_teams 
-from admin.email import send_email
-
+#from admin.email import send_email
 from officers.constants import OfficerPosition
 
+from github.internals import add_user_to_team, list_members, list_team_members, list_teams, remove_user_from_team
+from github.types import GithubUserPermissions
+
 # Rules:
 # - all past officers will be members of the github org
 # - all past officers will be put in past_officers team
 # - all current officers will be put in the officers team
-# - 
 
+_logger = logging.getLogger(__name__)
 
 # TODO: move this to github.constants.py
 GITHUB_TEAMS = {
@@ -25,7 +27,7 @@
     "wall_e": "manual",
 }
 AUTO_GITHUB_TEAMS = [
-    team
+    name
     for (name, kind) in GITHUB_TEAMS.items()
     if kind == "auto"
 ]
@@ -44,7 +46,6 @@ def all_permissions() -> dict[str, GithubUserPermissions]:
     """
     return a list of members in the organization (org) & their permissions
     """
-
     member_list = list_members()
     member_name_list = { member.name for member in member_list }
 
@@ -55,21 +56,20 @@ def all_permissions() -> dict[str, GithubUserPermissions]:
             continue
         elif GITHUB_TEAMS[team.name] == "manual":
             continue
-
         team_list += [team]
 
     team_name_list = [team.name for team in team_list]
     for team_name in AUTO_GITHUB_TEAMS:
         if team_name not in team_name_list:
-            # TODO: send email for all errors & warnings 
-            # send_email("csss-sysadmin@sfu.ca", "ERROR: Missing Team", "...") 
+            # TODO: send email for all errors & warnings
+            # send_email("csss-sysadmin@sfu.ca", "ERROR: Missing Team", "...")
             _logger.error(f"Could not find 'auto' team {team_name} in organization")
 
     user_permissions = {
         user.username: GithubUserPermissions(user.username, [])
         for user in member_list
     }
-    for team in team_list: 
+    for team in team_list:
         team_members = list_team_members(team.slug)
         for member in team_members:
             if member.name not in member_name_list:
@@ -87,12 +87,12 @@ def all_permissions() -> dict[str, GithubUserPermissions]:
 def set_user_teams(username: str, old_teams: list[str], new_teams: list[str]):
     for team_slug in old_teams:
         if team_slug not in new_teams:
-            remove_user_from_team(term.username, team_slug)
+            remove_user_from_team(username, team_slug)
 
     for team_slug in new_teams:
         if team_slug not in old_teams:
             # TODO: what happens when adding a user to a team who is not part of the github org yet?
-            add_user_to_team(term.username, team_slug)
+            add_user_to_team(username, team_slug)
 
 def invite_user(github_username: str, teams: str):
     # invite this user to the github organization
diff --git a/src/github/internals.py b/src/github/internals.py
index 8c7ecd6..da1b2a9 100644
--- a/src/github/internals.py
+++ b/src/github/internals.py
@@ -6,10 +6,13 @@
 from constants import github_org_name as GITHUB_ORG_NAME
 from requests import Response
 
-from github.types import GithubUser, GithubTeam
+from github.types import GithubTeam, GithubUser
 
 GITHUB_TOKEN = os.environ.get("GITHUB_TOKEN")
 
+# TODO: go through this module & make sure that all functions check for response.status_code
+# being invalid as specified by the API endpoints
+
 async def _github_request_get(
     url: str,
     token: str
@@ -101,11 +104,11 @@ async def get_user_by_username(
         f"https://api.github.com/users/{username}",
         GITHUB_TOKEN,
     )
-    result_json = result.json()
-    if result_json["status"] == "404":
+    if result.status_code == 404:
         return None
-    else:
-        return GithubUser(result_json["login"], result_json["id"], result_json["name"])
+
+    result_json = result.json()
+    return GithubUser(result_json["login"], result_json["id"], result_json["name"])
 
 async def get_user_by_id(
     uid: str
@@ -119,18 +122,17 @@ async def get_user_by_id(
         f"https://api.github.com/user/{uid}",
         GITHUB_TOKEN,
     )
-    # TODO: should we check the actual status of the response?
-    result_json = result.json()
-    if result_json["status"] == "404":
+    if result.status == 404:
         return None
-    else:
-        return GithubUser(result_json["login"], result_json["id"], result_json["name"])
+
+    result_json = result.json()
+    return GithubUser(result_json["login"], result_json["id"], result_json["name"])
 
 # TODO: if needed, add support for getting user by email
 
 async def invite_user(
     uid: str,
-    team_id_list: Optional[list[str]] = None,
+    team_id_list: list[str] | None = None,
     org: str = GITHUB_ORG_NAME,
 ) -> None:
     """Invites the user & gives them access to the supplied teams"""
@@ -153,11 +155,12 @@ async def invite_user(
         )
 
 async def delete_user_from_org(
-    username: str,
+    username: str | None,
     org: str = GITHUB_ORG_NAME
 ) -> None:
     if username is None:
         raise ValueError("Username cannot be empty")
+
     result = await _github_request_delete(
         f"https://api.github.com/orgs/{org}/memberships/{username}", GITHUB_TOKEN
     )
@@ -217,17 +220,20 @@ async def remove_user_from_team(
         raise Exception(f"Status code {result.status_code} returned when attempting to delete user {username} from team {team_slug}")
 
 async def list_members(
-    org: str = GITHUB_ORG_NAME
+    org: str = GITHUB_ORG_NAME,
+    page_number: int = 1,
+    page_size: int = 99,
 ) -> list[GithubUser]:
     result = await _github_request_get(
-        f"https://api.github.com/orgs/{org}/members",
+        f"https://api.github.com/orgs/{org}/members?per_page=99",
         GITHUB_TOKEN
     )
 
-    # TODO: check for errors
+    if result.status_code != 200:
+        raise Exception(f"Got result with status_code={result.status_code}, and contents={result.text}")
 
     return [
-        GithubUser(user["login"], user["id"], user["name"])
+        (user["login"], user["id"])
         for user in result.json()
-    ] 
+    ]
 
diff --git a/src/github/types.py b/src/github/types.py
index 9d1e09e..9e5f48a 100644
--- a/src/github/types.py
+++ b/src/github/types.py
@@ -1,5 +1,6 @@
 from dataclasses import dataclass
 
+
 @dataclass
 class GithubUser:
     username: str
diff --git a/src/officers/urls.py b/src/officers/urls.py
index e5780a9..ae6ba3b 100755
--- a/src/officers/urls.py
+++ b/src/officers/urls.py
@@ -4,6 +4,7 @@
 
 import auth.crud
 import database
+import github
 import sqlalchemy
 import utils
 from constants import COMPUTING_ID_MAX
@@ -256,7 +257,11 @@ async def update_info(
         validation_failures += [f"invalid email format {officer_info_upload.google_drive_email}"]
         new_officer_info.google_drive_email = officer_info.google_drive_email
 
-    # TODO: validate github user
+    # validate github user is real
+    if await github.internals.get_user_by_username(officer_info_upload.github_username) is None:
+        validation_failures += [f"invalid github username {officer_info_upload.github_username}"]
+        new_officer_info.github_username = officer_info.github_username
+
     # TODO: invite github user
     # TODO: detect if changing github username & uninvite old user
 
diff --git a/tests/integration/test_github.py b/tests/integration/test_github.py
new file mode 100644
index 0000000..9484af9
--- /dev/null
+++ b/tests/integration/test_github.py
@@ -0,0 +1,25 @@
+import github.internals
+import pytest
+
+# NOTE: must export API key to use github api (mostly...)
+
+@pytest.mark.asyncio
+async def test__list_users():
+    member_list = await github.internals.list_members()
+    print(member_list)
+
+@pytest.mark.asyncio
+async def test__get_user_by_name():
+    user = await github.internals.get_user_by_username("EarthenSky")
+    print(user)
+
+    user2 = await github.internals.get_user_by_username("jamieklo")
+    print(user2)
+
+    user3 = await github.internals.get_user_by_username("")
+    assert user3 is None
+    print(user3)
+
+    user4 = await github.internals.get_user_by_username("asfgkahdgOO_OPPEdkdhghk57777777777")
+    assert user4 is None
+    print(user4)

From f71952648b4a36ac539f5ae1022328b2774a756f Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Mon, 2 Sep 2024 01:26:52 -0700
Subject: [PATCH 26/29] implement & refactor officer term

---
 src/github/internals.py |  4 +-
 src/load_test_db.py     | 16 +++----
 src/officers/crud.py    | 57 ++++++++++++++-----------
 src/officers/types.py   | 12 +++---
 src/officers/urls.py    | 92 ++++++++++++++++++++++++-----------------
 5 files changed, 102 insertions(+), 79 deletions(-)

diff --git a/src/github/internals.py b/src/github/internals.py
index da1b2a9..43c7e60 100644
--- a/src/github/internals.py
+++ b/src/github/internals.py
@@ -130,6 +130,7 @@ async def get_user_by_id(
 
 # TODO: if needed, add support for getting user by email
 
+# TODO: can we revoke access before an invite is accepeted?
 async def invite_user(
     uid: str,
     team_id_list: list[str] | None = None,
@@ -225,7 +226,7 @@ async def list_members(
     page_size: int = 99,
 ) -> list[GithubUser]:
     result = await _github_request_get(
-        f"https://api.github.com/orgs/{org}/members?per_page=99",
+        f"https://api.github.com/orgs/{org}/members?per_page={page_size}&page={page_number}",
         GITHUB_TOKEN
     )
 
@@ -236,4 +237,3 @@ async def list_members(
         (user["login"], user["id"])
         for user in result.json()
     ]
-
diff --git a/src/load_test_db.py b/src/load_test_db.py
index dc2d63f..c784428 100644
--- a/src/load_test_db.py
+++ b/src/load_test_db.py
@@ -10,8 +10,7 @@
 from database import SQLALCHEMY_TEST_DATABASE_URL, Base, DatabaseSessionManager
 from officers.constants import OfficerPosition
 from officers.crud import create_new_officer_info, create_new_officer_term, update_officer_info, update_officer_term
-from officers.tables import OfficerInfo
-from officers.types import OfficerInfoUpload, OfficerTermData
+from officers.tables import OfficerInfo, OfficerTerm
 from sqlalchemy.ext.asyncio import AsyncSession
 
 
@@ -108,7 +107,8 @@ async def load_test_officers_data(db_session: AsyncSession):
     ))
     await db_session.commit()
 
-    await create_new_officer_term(db_session, OfficerTermData(
+    # TODO: will id autoincrement?
+    await create_new_officer_term(db_session, OfficerTerm(
         computing_id="abc11",
 
         position=OfficerPosition.VICE_PRESIDENT,
@@ -125,7 +125,7 @@ async def load_test_officers_data(db_session: AsyncSession):
         biography="Hi! I'm person A and I do lots of cool things! :)",
         photo_url=None, # TODO: this should be replaced with a default image
     ))
-    await create_new_officer_term(db_session, OfficerTermData(
+    await create_new_officer_term(db_session, OfficerTerm(
         computing_id="abc11",
 
         position=OfficerPosition.EXECUTIVE_AT_LARGE,
@@ -142,7 +142,7 @@ async def load_test_officers_data(db_session: AsyncSession):
         biography="Hi! I'm person A and I want school to be over ; _ ;",
         photo_url=None, # TODO: this should be replaced with a default image
     ))
-    await create_new_officer_term(db_session, OfficerTermData(
+    await create_new_officer_term(db_session, OfficerTerm(
         computing_id="abc33",
 
         position=OfficerPosition.PRESIDENT,
@@ -160,7 +160,7 @@ async def load_test_officers_data(db_session: AsyncSession):
         photo_url=None, # TODO: this should be replaced with a default image
     ))
     # this officer term is not fully filled in
-    await create_new_officer_term(db_session, OfficerTermData(
+    await create_new_officer_term(db_session, OfficerTerm(
         computing_id="abc22",
 
         position=OfficerPosition.DIRECTOR_OF_ARCHIVES,
@@ -191,7 +191,7 @@ async def load_test_officers_data(db_session: AsyncSession):
         github_username=None,
         google_drive_email=None,
     ))
-    await update_officer_term(db_session, OfficerTermData(
+    await update_officer_term(db_session, OfficerTerm(
         computing_id="abc33",
 
         position=OfficerPosition.PRESIDENT,
@@ -226,7 +226,7 @@ async def load_sysadmin(db_session: AsyncSession):
         github_username=None,
         google_drive_email=None,
     ))
-    await create_new_officer_term(db_session, OfficerTermData(
+    await create_new_officer_term(db_session, OfficerTerm(
         computing_id=SYSADMIN_COMPUTING_ID,
 
         position=OfficerPosition.SYSTEM_ADMINISTRATOR,
diff --git a/src/officers/crud.py b/src/officers/crud.py
index 09e74c0..13ab444 100644
--- a/src/officers/crud.py
+++ b/src/officers/crud.py
@@ -6,13 +6,12 @@
 import sqlalchemy
 import utils
 from auth.tables import SiteUser
+from fastapi import HTTPException
 
 from officers.constants import OfficerPosition
 from officers.tables import OfficerInfo, OfficerTerm
 from officers.types import (
     OfficerData,
-    #OfficerPrivateData,
-    OfficerTermData,
 )
 
 _logger = logging.getLogger(__name__)
@@ -46,13 +45,26 @@ async def current_officer_position(db_session: database.DBSession, computing_id:
     else:
         return officer_term.position
 
-async def officer_info(db_session: database.DBSession, computing_id: str) -> OfficerInfo | None:
+async def officer_info(db_session: database.DBSession, computing_id: str) -> OfficerInfo:
     query = (
         sqlalchemy
         .select(OfficerInfo)
         .where(OfficerInfo.computing_id == computing_id)
     )
     officer_term = await db_session.scalar(query)
+    if officer_term is None:
+        raise HTTPException(status_code=400, detail=f"officer_info for computing_id={computing_id} does not exist yet")
+    return officer_term
+
+async def officer_term(db_session: database.DBSession, term_id: int) -> OfficerTerm:
+    query = (
+        sqlalchemy
+        .select(OfficerTerm)
+        .where(OfficerTerm.id == term_id)
+    )
+    officer_term = await db_session.scalar(query)
+    if officer_term is None:
+        raise HTTPException(status_code=400, detail=f"Could not find officer_term with id={term_id}")
     return officer_term
 
 async def officer_terms(
@@ -162,8 +174,7 @@ async def all_officer_terms(
             OfficerInfo.computing_id == term.computing_id
         )
         officer_info = await db_session.scalar(officer_info_query)
-        
-        # TODO: remove is_active from the database
+
         is_active = (term.end_date is None) or (datetime.today() <= term.end_date)
         officer_data_list += [OfficerData.from_data(term, officer_info, include_private, is_active)]
 
@@ -206,37 +217,37 @@ async def update_officer_info(db_session: database.DBSession, new_officer_info:
 
 async def create_new_officer_term(
     db_session: database.DBSession,
-    officer_term_data: OfficerTermData
+    new_officer_term: OfficerTerm
 ) -> bool:
-    query = sqlalchemy.select(OfficerTerm)
-    query = query.where(OfficerTerm.computing_id == officer_term_data.computing_id)
-    query = query.where(OfficerTerm.start_date == officer_term_data.start_date)
-    query = query.where(OfficerTerm.position == officer_term_data.position)
+    """
+    query = (
+        sqlalchemy
+        .select(OfficerTerm)
+        .where(OfficerTerm.computing_id == officer_term_data.computing_id)
+        .where(OfficerTerm.start_date == officer_term_data.start_date)
+        .where(OfficerTerm.position == officer_term_data.position)
+    )
     officer_data = await db_session.scalar(query)
     if officer_data is not None:
         # if an entry with this (computing_id, position, start_date) already exists, do nothing
         return False
-
-    db_session.add(officer_term_data.to_officer_term())
+    """
+    db_session.add(new_officer_term)
     return True
 
 async def update_officer_term(
     db_session: database.DBSession,
-    officer_term_data: OfficerTermData,
+    new_officer_term: OfficerTerm,
 ):
     """
-    If there's an existing entry with the same computing_id, start_date, and position,
-    update the data of that term.
+    Update based on the term id.
 
     Returns false if the above entry does not exist.
     """
-    # TODO: we should move towards using the term_id, so that the start_date can be updated if needed?
     query = (
         sqlalchemy
         .select(OfficerTerm)
-        .where(OfficerTerm.computing_id == officer_term_data.computing_id)
-        .where(OfficerTerm.position == officer_term_data.position)
-        .where(OfficerTerm.start_date == officer_term_data.start_date)
+        .where(OfficerTerm.id == new_officer_term.id)
     )
     officer_term = await db_session.scalar(query)
     if officer_term is None:
@@ -245,13 +256,9 @@ async def update_officer_term(
     query = (
         sqlalchemy
         .update(OfficerTerm)
-        .where(OfficerTerm.computing_id == officer_term_data.computing_id)
-        .where(OfficerTerm.position == officer_term_data.position)
-        .where(OfficerTerm.start_date == officer_term_data.start_date)
-        # TODO: fix this by passing params to to_officer_info
-        .values(officer_term_data.to_officer_info().to_update_dict())
+        .where(OfficerTerm.id == new_officer_term.id)
+        .values(new_officer_term.to_update_dict())
     )
-
     await db_session.execute(query)
     return True
 
diff --git a/src/officers/types.py b/src/officers/types.py
index d6bd649..1d9fc1d 100644
--- a/src/officers/types.py
+++ b/src/officers/types.py
@@ -41,13 +41,13 @@ def to_officer_info(self, computing_id: str, discord_id: str | None, discord_nic
         )
 
 @dataclass
-class OfficerTermData:
-    computing_id: str
-
+class OfficerTermUpload:
+    # only admins can change:
     position: str
     start_date: date
     end_date: None | date = None
 
+    # officer should change
     nickname: None | str = None
     favourite_course_0: None | str = None
     favourite_course_1: None | str = None
@@ -71,9 +71,11 @@ def validate(self) -> None | HTTPException:
         else:
             return None
 
-    def to_officer_term(self) -> OfficerTerm:
+    def to_officer_term(self, computing_id:str) -> OfficerTerm:
+        # TODO: many positions have a length; if the length is defined, fill it in right here
+        # (end date is 1st of month, 12 months after start date's month).
         return OfficerTerm(
-            computing_id = self.computing_id,
+            computing_id = computing_id,
 
             position = self.position,
             start_date = self.start_date,
diff --git a/src/officers/urls.py b/src/officers/urls.py
index ae6ba3b..e07531c 100755
--- a/src/officers/urls.py
+++ b/src/officers/urls.py
@@ -15,8 +15,8 @@
 
 import officers.crud
 from officers.constants import OfficerPosition
-from officers.tables import OfficerInfo
-from officers.types import OfficerInfoUpload, OfficerTermData
+from officers.tables import OfficerInfo, OfficerTerm
+from officers.types import OfficerInfoUpload, OfficerTermUpload
 
 _logger = logging.getLogger(__name__)
 
@@ -25,6 +25,7 @@
     tags=["officers"],
 )
 
+# TODO: combine the following two 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.",
@@ -101,6 +102,7 @@ async def get_officer_terms(
     officer_terms = await officers.crud.officer_terms(db_session, computing_id, max_terms, hide_filled_in=True)
     return JSONResponse([term.serializable_dict() for term in officer_terms])
 
+# TODO: make this into getting info for any computing_id?
 @router.get(
     "/my_info",
     description="Get officer info for the current user, if they've ever been an exec.",
@@ -130,7 +132,7 @@ class InitialOfficerInfo:
     start_date: date
 
 @router.post(
-    "/new_term",
+    "/term",
     description="Only the sysadmin, president, or DoA can submit this request. It will usually be the DoA. Updates the system with a new officer, and enables the user to login to the system to input their information.",
 )
 async def new_officer_term(
@@ -160,20 +162,20 @@ async def new_officer_term(
             ).to_officer_info(officer_info.computing_id, None, None),
         )
         # TODO: update create_new_officer_term to be the same as create_new_officer_info
-        success = await officers.crud.create_new_officer_term(db_session, OfficerTermData(
+        success = await officers.crud.create_new_officer_term(db_session, OfficerTermUpload(
             computing_id = officer_info.computing_id,
             position = officer_info.position,
             # TODO: remove the hours & seconds (etc.) from start_date
             start_date = officer_info.start_date,
-        ))
+        ).to_officer_term())
         if not success:
             raise HTTPException(status_code=400, detail="Officer term already exists, no changes made")
 
     await db_session.commit()
     return PlainTextResponse("ok")
 
-@router.post(
-    "/{computing_id}/update_info",
+@router.patch(
+    "/info/{computing_id}",
     description=(
         "After elections, officer computing ids are input into our system. "
         "If you have been elected as a new officer, you may authenticate with SFU CAS, "
@@ -205,14 +207,7 @@ async def update_info(
 
     # TODO: log all important changes just to a .log file
 
-    # TODO: turn this into a function first
-    # get officer info
-    query = sqlalchemy.select(OfficerInfo)
-    query = query.where(OfficerInfo.computing_id == computing_id)
-    officer_info = await db_session.scalar(query)
-    if officer_info is None:
-        raise HTTPException(status_code=400, detail="officer_info does not exist yet, please create the officer info entry first")
-
+    old_officer_info = await officers.crud.officer_info(db_session, computing_id)
     new_officer_info = officer_info_upload.to_officer_info(
         computing_id=computing_id,
         discord_id=None,
@@ -224,7 +219,7 @@ async def update_info(
 
     if not utils.is_valid_phone_number(officer_info_upload.phone_number):
         validation_failures += [f"invalid phone number {officer_info_upload.phone_number}"]
-        new_officer_info.phone_number = officer_info.phone_number
+        new_officer_info.phone_number = old_officer_info.phone_number
 
     if officer_info_upload.discord_name is None or officer_info_upload.discord_name == "":
         new_officer_info.discord_name = None
@@ -234,14 +229,14 @@ async def update_info(
         discord_user_list = await discord.search_username(officer_info_upload.discord_name)
         if discord_user_list == []:
             validation_failures += [f"unable to find discord user with the name {officer_info_upload.discord_name}"]
-            new_officer_info.discord_name = officer_info.discord_name
-            new_officer_info.discord_id = officer_info.discord_id
-            new_officer_info.discord_nickname = officer_info.discord_nickname
+            new_officer_info.discord_name = old_officer_info.discord_name
+            new_officer_info.discord_id = old_officer_info.discord_id
+            new_officer_info.discord_nickname = old_officer_info.discord_nickname
         elif len(discord_user_list) > 1:
             validation_failures += [f"too many discord users start with {officer_info_upload.discord_name}"]
-            new_officer_info.discord_name = officer_info.discord_name
-            new_officer_info.discord_id = officer_info.discord_id
-            new_officer_info.discord_nickname = officer_info.discord_nickname
+            new_officer_info.discord_name = old_officer_info.discord_name
+            new_officer_info.discord_id = old_officer_info.discord_id
+            new_officer_info.discord_nickname = old_officer_info.discord_nickname
         else:
             discord_user = discord_user_list[0]
             new_officer_info.discord_name = discord_user.username
@@ -255,12 +250,12 @@ async def update_info(
     # TODO: validate google-email using google module, by trying to assign the user to a permission or something
     if not utils.is_valid_email(officer_info_upload.google_drive_email):
         validation_failures += [f"invalid email format {officer_info_upload.google_drive_email}"]
-        new_officer_info.google_drive_email = officer_info.google_drive_email
+        new_officer_info.google_drive_email = old_officer_info.google_drive_email
 
     # validate github user is real
     if await github.internals.get_user_by_username(officer_info_upload.github_username) is None:
         validation_failures += [f"invalid github username {officer_info_upload.github_username}"]
-        new_officer_info.github_username = officer_info.github_username
+        new_officer_info.github_username = old_officer_info.github_username
 
     # TODO: invite github user
     # TODO: detect if changing github username & uninvite old user
@@ -272,49 +267,68 @@ async def update_info(
     await db_session.commit()
 
     updated_officer_info = await officers.crud.officer_info(db_session, computing_id)
-    if updated_officer_info is None:
-        raise HTTPException(status_code=500, detail="failed to get officer info?! something is very wrong...")
-
     return JSONResponse({
         "updated_officer_info": updated_officer_info.serializable_dict(),
         "validation_failures": validation_failures,
     })
 
-# TODO: access term by term_id
-# TODO: only allow access if the user is admin or if the id is their term
-# TODO: don't allow a user to change who owns the term if they're not an admin.
-@router.post(
-    "/update_term",
+@router.patch(
+    "/term/{term_id}",
 )
 async def update_term(
     request: Request,
     db_session: database.DBSession,
-    officer_term: OfficerTermData = Body(), # noqa: B008
+    term_id: int,
+    officer_term_upload: OfficerTermUpload = Body(), # noqa: B008
 ):
-    http_exception = officer_term.validate()
+    http_exception = officer_term_upload.validate()
     if http_exception is not None:
         raise http_exception
 
+    # Refactor all of these gets & raises into small functions
     session_id = request.cookies.get("session_id", None)
     if session_id is None:
         raise HTTPException(status_code=401, detail="must be logged in")
 
     session_computing_id = await auth.crud.get_computing_id(db_session, session_id)
     if (
-        officer_term.computing_id != session_computing_id
+        officer_term_upload.computing_id != session_computing_id
         and not await WebsiteAdmin.has_permission(db_session, session_computing_id)
     ):
         # the current user can only input the info for another user if they have permissions
         raise HTTPException(status_code=401, detail="must have website admin permissions to update another user")
 
-    # TODO: log all important changes just to a .log file
+    old_officer_info = await officers.crud.officer_term(db_session, term_id)
+
+    # NOTE: Only admins can write new versions of position, start_date, and end_date.
+    if (
+        (
+            officer_term_upload.position != old_officer_info.position
+            or officer_term_upload.start_date != old_officer_info.start_date
+            or officer_term_upload.end_date != old_officer_info.end_date
+        )
+        and not await WebsiteAdmin.has_permission(db_session, session_computing_id)
+    ):
+        raise HTTPException(status_code=401, detail="Non-admins cannot modify position, start_date, or end_date.")
 
-    success = await officers.crud.update_officer_term(db_session, officer_term)
+    # NOTE: An officer can change their own data for terms that are ongoing.
+    if officer_term_upload.position not in OfficerPosition.position_list():
+        raise HTTPException(status_code=400, detail=f"invalid new position={officer_term_upload.position}")
+    elif officer_term_upload.end_date is not None and officer_term_upload.start_date > officer_term_upload.end_date:
+        raise HTTPException(status_code=400, detail="end_date must be after start_date")
+
+    # TODO: log all important changes just to a .log file
+    success = await officers.crud.update_officer_term(db_session, old_officer_info)
     if not success:
-        raise HTTPException(status_code=400, detail="the associated officer_term does not exist yet, please create the associated officer term")
+        raise HTTPException(status_code=400, detail="the associated officer_term does not exist yet, please create it first")
 
     await db_session.commit()
-    return PlainTextResponse("ok")
+
+    new_officer_term = await officers.crud.officer_term(db_session, term_id)
+    return JSONResponse({
+        "updated_officer_term": new_officer_term.serializable_dict(),
+        "validation_failures": [], # none for now, but may be important later
+    })
 
 """
 # TODO: test this error later

From 78a12297735d65566fa7a5b0962a55db2ced1da3 Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Mon, 2 Sep 2024 01:33:43 -0700
Subject: [PATCH 27/29] get info about any user by computing_id

---
 src/officers/urls.py | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/officers/urls.py b/src/officers/urls.py
index e07531c..f5e3dbc 100755
--- a/src/officers/urls.py
+++ b/src/officers/urls.py
@@ -102,23 +102,31 @@ async def get_officer_terms(
     officer_terms = await officers.crud.officer_terms(db_session, computing_id, max_terms, hide_filled_in=True)
     return JSONResponse([term.serializable_dict() for term in officer_terms])
 
-# TODO: make this into getting info for any computing_id?
 @router.get(
-    "/my_info",
+    "/info/{computing_id}",
     description="Get officer info for the current user, if they've ever been an exec.",
 )
 async def get_officer_info(
     request: Request,
     db_session: database.DBSession,
+    computing_id: str,
 ):
     session_id = request.cookies.get("session_id", None)
     if session_id is None:
         raise HTTPException(status_code=401)
 
-    computing_id = await auth.crud.get_computing_id(db_session, session_id)
-    if computing_id is None:
+    session_computing_id = await auth.crud.get_computing_id(db_session, session_id)
+    if session_computing_id is None:
         raise HTTPException(status_code=401)
 
+    session_computing_id = await auth.crud.get_computing_id(db_session, session_id)
+    if (
+        computing_id != session_computing_id
+        and not await WebsiteAdmin.has_permission(db_session, session_computing_id)
+    ):
+        # the current user can only input the info for another user if they have permissions
+        raise HTTPException(status_code=401, detail="must have website admin permissions to get officer info about another user")
+
     officer_info = await officers.crud.officer_info(db_session, computing_id)
     if officer_info is None:
         raise HTTPException(status_code=404, detail="user has no officer info")
@@ -291,6 +299,9 @@ async def update_term(
         raise HTTPException(status_code=401, detail="must be logged in")
 
     session_computing_id = await auth.crud.get_computing_id(db_session, session_id)
+    if session_computing_id is None:
+        raise HTTPException(status_code=401)
+
     if (
         officer_term_upload.computing_id != session_computing_id
         and not await WebsiteAdmin.has_permission(db_session, session_computing_id)

From 35a9e1ac341cc3902dce5ee6f32100b4f3ebab51 Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Mon, 2 Sep 2024 12:20:58 -0700
Subject: [PATCH 28/29] finalize officer term api endpoints

---
 src/officers/crud.py   | 27 +++++++--------------------
 src/officers/tables.py | 19 +++++++++++++++++++
 src/officers/types.py  | 41 ++++++++++-------------------------------
 src/officers/urls.py   | 30 ++++++++++++------------------
 4 files changed, 48 insertions(+), 69 deletions(-)

diff --git a/src/officers/crud.py b/src/officers/crud.py
index 13ab444..682359a 100644
--- a/src/officers/crud.py
+++ b/src/officers/crud.py
@@ -67,6 +67,7 @@ async def officer_term(db_session: database.DBSession, term_id: int) -> OfficerT
         raise HTTPException(status_code=400, detail=f"Could not find officer_term with id={term_id}")
     return officer_term
 
+# TODO: change to "get_officer_terms" naming convention (& all functions in this module)
 async def officer_terms(
     db_session: database.DBSession,
     computing_id: str,
@@ -193,6 +194,12 @@ async def create_new_officer_info(db_session: database.DBSession, new_officer_in
     db_session.add(new_officer_info)
     return True
 
+async def create_new_officer_term(
+    db_session: database.DBSession,
+    new_officer_term: OfficerTerm
+):
+    db_session.add(new_officer_term)
+
 async def update_officer_info(db_session: database.DBSession, new_officer_info: OfficerInfo) -> bool:
     """
     Return False if the officer doesn't exist yet
@@ -215,26 +222,6 @@ async def update_officer_info(db_session: database.DBSession, new_officer_info:
 
     return True
 
-async def create_new_officer_term(
-    db_session: database.DBSession,
-    new_officer_term: OfficerTerm
-) -> bool:
-    """
-    query = (
-        sqlalchemy
-        .select(OfficerTerm)
-        .where(OfficerTerm.computing_id == officer_term_data.computing_id)
-        .where(OfficerTerm.start_date == officer_term_data.start_date)
-        .where(OfficerTerm.position == officer_term_data.position)
-    )
-    officer_data = await db_session.scalar(query)
-    if officer_data is not None:
-        # if an entry with this (computing_id, position, start_date) already exists, do nothing
-        return False
-    """
-    db_session.add(new_officer_term)
-    return True
-
 async def update_officer_term(
     db_session: database.DBSession,
     new_officer_term: OfficerTerm,
diff --git a/src/officers/tables.py b/src/officers/tables.py
index 61dfc86..d5ab995 100644
--- a/src/officers/tables.py
+++ b/src/officers/tables.py
@@ -97,6 +97,25 @@ def sql_is_filled_in(query: Select) -> Select:
             )
         )
 
+
+    def to_update_dict(self) -> dict:
+        return {
+            # TODO: do we want computing_id to be changeable?
+            # "computing_id": self.computing_id,
+
+            "position": self.position,
+            "start_date": self.start_date,
+            "end_date": self.end_date,
+
+            "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,
+        }
+
 # this table contains information that we only need a most up-to-date version of, and
 # don't need to keep a history of. However, it also can't be easily updated.
 class OfficerInfo(Base):
diff --git a/src/officers/types.py b/src/officers/types.py
index 1d9fc1d..5b8dc07 100644
--- a/src/officers/types.py
+++ b/src/officers/types.py
@@ -59,22 +59,20 @@ class OfficerTermUpload:
     # NOTE: changing the name of this variable without changing all instances is breaking
     photo_url: None | str = None
 
-    def validate(self) -> None | HTTPException:
-        if len(self.computing_id) > COMPUTING_ID_MAX:
-            return HTTPException(status_code=400, detail=f"computing_id={self.computing_id} is too large")
-        elif self.position not in OfficerPosition.position_list():
-            raise HTTPException(status_code=400, detail=f"invalid position={self.position}")
-        # TODO: more checks
-        # TODO: how to check this one? make sure date is date & not datetime?
-        #elif not is_iso_format(self.start_date):
-        #    raise HTTPException(status_code=400, detail=f"start_date={self.start_date} must be a valid iso date")
-        else:
-            return None
+    def validate(self):
+        """input validation"""
+        # NOTE: An officer can change their own data for terms that are ongoing.
+        if self.position not in OfficerPosition.position_list():
+            raise HTTPException(status_code=400, detail=f"invalid new position={self.position}")
+        elif self.end_date is not None and self.start_date > self.end_date:
+            raise HTTPException(status_code=400, detail="end_date must be after start_date")
+
 
-    def to_officer_term(self, computing_id:str) -> OfficerTerm:
+    def to_officer_term(self, term_id: str, computing_id:str) -> OfficerTerm:
         # TODO: many positions have a length; if the length is defined, fill it in right here
         # (end date is 1st of month, 12 months after start date's month).
         return OfficerTerm(
+            id = term_id,
             computing_id = computing_id,
 
             position = self.position,
@@ -90,25 +88,6 @@ def to_officer_term(self, computing_id:str) -> OfficerTerm:
             photo_url = self.photo_url,
         )
 
-    def update_dict(self) -> dict:
-        # TODO: control this by term_id & allow the others to be updated
-        # cannot update:
-        # - computing_id
-        # - start_date
-        # - position
-        return {
-            "end_date": self.end_date,
-            "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,
-        }
-
 # -------------------------------------------- #
 
 @dataclass
diff --git a/src/officers/urls.py b/src/officers/urls.py
index f5e3dbc..30dee1a 100755
--- a/src/officers/urls.py
+++ b/src/officers/urls.py
@@ -94,7 +94,7 @@ async def get_officer_terms(
     db_session: database.DBSession,
     computing_id: str,
     # the maximum number of terms to return, in chronological order
-    max_terms: None | int = 1,
+    max_terms: int | None = None,
     # TODO: implement the following
     # view_only_filled_in: bool = True,
 ):
@@ -190,7 +190,6 @@ async def new_officer_term(
         "then input your information & the valid token for us. Admins may update this info."
     ),
 )
-# TODO: computing_id in all paths
 async def update_info(
     request: Request,
     db_session: database.DBSession,
@@ -289,9 +288,7 @@ async def update_term(
     term_id: int,
     officer_term_upload: OfficerTermUpload = Body(), # noqa: B008
 ):
-    http_exception = officer_term_upload.validate()
-    if http_exception is not None:
-        raise http_exception
+    officer_term_upload.validate()
 
     # Refactor all of these gets & raises into small functions
     session_id = request.cookies.get("session_id", None)
@@ -302,34 +299,31 @@ async def update_term(
     if session_computing_id is None:
         raise HTTPException(status_code=401)
 
+    old_officer_term = await officers.crud.officer_term(db_session, term_id)
+
     if (
-        officer_term_upload.computing_id != session_computing_id
+        old_officer_term.computing_id != session_computing_id
         and not await WebsiteAdmin.has_permission(db_session, session_computing_id)
     ):
         # the current user can only input the info for another user if they have permissions
         raise HTTPException(status_code=401, detail="must have website admin permissions to update another user")
 
-    old_officer_info = await officers.crud.officer_term(db_session, term_id)
-
     # NOTE: Only admins can write new versions of position, start_date, and end_date.
     if (
         (
-            officer_term_upload.position != old_officer_info.position
-            or officer_term_upload.start_date != old_officer_info.start_date
-            or officer_term_upload.end_date != old_officer_info.end_date
+            officer_term_upload.position != old_officer_term.position
+            or officer_term_upload.start_date != old_officer_term.start_date
+            or officer_term_upload.end_date != old_officer_term.end_date
         )
         and not await WebsiteAdmin.has_permission(db_session, session_computing_id)
     ):
         raise HTTPException(status_code=401, detail="Non-admins cannot modify position, start_date, or end_date.")
 
-    # NOTE: An officer can change their own data for terms that are ongoing.
-    if officer_term_upload.position not in OfficerPosition.position_list():
-        raise HTTPException(status_code=400, detail=f"invalid new position={officer_term_upload.position}")
-    elif officer_term_upload.end_date is not None and officer_term_upload.start_date > officer_term_upload.end_date:
-        raise HTTPException(status_code=400, detail="end_date must be after start_date")
-
     # TODO: log all important changes just to a .log file
-    success = await officers.crud.update_officer_term(db_session, old_officer_info)
+    success = await officers.crud.update_officer_term(
+        db_session,
+        officer_term_upload.to_officer_term(term_id, old_officer_term.computing_id)
+    )
     if not success:
         raise HTTPException(status_code=400, detail="the associated officer_term does not exist yet, please create it first")
 

From 453ba38ce8dd779a5812c28b4d6b7be71861a6d3 Mon Sep 17 00:00:00 2001
From: Gabe WSL Debian <24978329+EarthenSky@users.noreply.github.com>
Date: Mon, 2 Sep 2024 12:26:09 -0700
Subject: [PATCH 29/29] appease linter

---
 src/admin/email.py      | 3 ++-
 src/constants.py        | 2 +-
 src/cron/daily.py       | 4 ++--
 src/github/internals.py | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/admin/email.py b/src/admin/email.py
index 88a5b9f..646512a 100644
--- a/src/admin/email.py
+++ b/src/admin/email.py
@@ -4,12 +4,13 @@
 # TODO: set this up
 GMAIL_PASSWORD = os.environ.get("GMAIL_PASSWORD")
 GMAIL_ADDRESS = "csss-site@gmail.com"
+GMAIL_USERNAME = ""
 
 # TODO: look into sending emails from an sfu maillist (this might be painful)
 def send_email(
     recipient_address: str,
     subject: str,
-    contents: str,
+    content: str,
 ):
     mail = smtplib.SMTP("smtp.gmail.com", 587)
     mail.ehlo()
diff --git a/src/constants.py b/src/constants.py
index af24019..60915da 100644
--- a/src/constants.py
+++ b/src/constants.py
@@ -1,7 +1,7 @@
 import os
 
 root_ip_address = "http://localhost:8080" if os.environ.get("LOCAL") == "true" else "https://api.sfucsss.org"
-github_org_name = "CSSS-Test-Organization" if os.environ.get("LOCAL") == "true" else "CSSS"
+GITHUB_ORG_NAME = "CSSS-Test-Organization" if os.environ.get("LOCAL") == "true" else "CSSS"
 
 W3_GUILD_ID = "1260652618875797504"
 CSSS_GUILD_ID = "228761314644852736"
diff --git a/src/cron/daily.py b/src/cron/daily.py
index b0f3292..40e2d0c 100644
--- a/src/cron/daily.py
+++ b/src/cron/daily.py
@@ -13,8 +13,8 @@
 
 async def update_google_permissions(db_session):
     # TODO: implement this function
-    google_permissions = google_api.all_permissions()
-    #one_year_ago = datetime.today() - timedelta(days=365)
+    # google_permissions = google_api.all_permissions()
+    # one_year_ago = datetime.today() - timedelta(days=365)
 
     # TODO: for performance, only include officers with recent end-date (1 yr)
     # but measure performance first
diff --git a/src/github/internals.py b/src/github/internals.py
index 43c7e60..15c1768 100644
--- a/src/github/internals.py
+++ b/src/github/internals.py
@@ -3,7 +3,7 @@
 from typing import Any
 
 import requests
-from constants import github_org_name as GITHUB_ORG_NAME
+from constants import GITHUB_ORG_NAME
 from requests import Response
 
 from github.types import GithubTeam, GithubUser