From 98ad8526672e24063d59f52f999ff81d7370934d Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sun, 28 Sep 2025 15:07:50 -0700 Subject: [PATCH 01/32] wip: add webmaster data to test DB --- src/load_test_db.py | 71 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/src/load_test_db.py b/src/load_test_db.py index 9d470a3..725195f 100644 --- a/src/load_test_db.py +++ b/src/load_test_db.py @@ -298,6 +298,76 @@ async def load_sysadmin(db_session: AsyncSession): )) await db_session.commit() +WEBMASTER_COMPUTING_ID = "jbriones" +async def load_webmaster(db_session: AsyncSession): + # put your computing id here for testing purposes + print(f"loading new webmaster '{WEBMASTER_COMPUTING_ID}'") + await create_user_session(db_session, f"temp_id_{WEBMASTER_COMPUTING_ID}", WEBMASTER_COMPUTING_ID) + await create_new_officer_info(db_session, OfficerInfo( + legal_name="Jon Andre Briones", + discord_id=None, + discord_name=None, + discord_nickname=None, + + computing_id=WEBMASTER_COMPUTING_ID, + phone_number=None, + github_username=None, + google_drive_email=None, + )) + await create_new_officer_term(db_session, OfficerTerm( + computing_id=WEBMASTER_COMPUTING_ID, + + position=OfficerPositionEnum.FIRST_YEAR_REPRESENTATIVE, + start_date=date.today() - timedelta(days=(365*3)), + end_date=date.today() - timedelta(days=(365*2)), + + nickname="Jon Andre Briones", + favourite_course_0="CMPT 379", + favourite_course_1="CMPT 371", + + favourite_pl_0="TypeScript", + favourite_pl_1="C#", + + biography="o hey fellow kids \n\n\n I can newline", + photo_url=None, + )) + await create_new_officer_term(db_session, OfficerTerm( + computing_id=WEBMASTER_COMPUTING_ID, + + position=OfficerPositionEnum.WEBMASTER, + start_date=date.today() - timedelta(days=365), + end_date=None, + + nickname="G2", + favourite_course_0="CMPT 379", + favourite_course_1="CMPT 295", + + favourite_pl_0="Rust", + favourite_pl_1="C", + + biography="The systems are good o7", + photo_url=None, + )) + # a future term + await create_new_officer_term(db_session, OfficerTerm( + computing_id=WEBMASTER_COMPUTING_ID, + + position=OfficerPositionEnum.DIRECTOR_OF_ARCHIVES, + start_date=date.today() + timedelta(days=365*1), + end_date=date.today() + timedelta(days=365*2), + + nickname="G3", + favourite_course_0="MACM 102", + favourite_course_1="CMPT 127", + + favourite_pl_0="C%", + favourite_pl_1="C$$", + + biography="o hey fellow kids \n\n\n I will can newline .... !!", + photo_url=None, + )) + await db_session.commit() + async def load_test_elections_data(db_session: AsyncSession): print("loading election data...") await create_election(db_session, Election( @@ -385,6 +455,7 @@ async def async_main(sessionmanager): await load_test_auth_data(db_session) await load_test_officers_data(db_session) await load_sysadmin(db_session) + await load_webmaster(db_session) await load_test_elections_data(db_session) await load_test_election_nominee_application_data(db_session) From 2ae6aea6d03fdeba52c4b2959e7615bf9719b0d2 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sun, 28 Sep 2025 15:57:01 -0700 Subject: [PATCH 02/32] wip: add officer models and update GET current officer docs --- src/elections/urls.py | 12 ++++++------ src/officers/models.py | 41 +++++++++++++++++++++++++++++++++++++++++ src/officers/urls.py | 22 +++++++++++----------- 3 files changed, 58 insertions(+), 17 deletions(-) create mode 100644 src/officers/models.py diff --git a/src/elections/urls.py b/src/elections/urls.py index caae581..a235bd2 100644 --- a/src/elections/urls.py +++ b/src/elections/urls.py @@ -25,7 +25,7 @@ tags=["election"], ) -async def get_user_permissions( +async def get_election_permissions( request: Request, db_session: database.DBSession, ) -> tuple[bool, str | None, str | None]: @@ -96,7 +96,7 @@ async def list_elections( request: Request, db_session: database.DBSession, ): - is_admin, _, _ = await get_user_permissions(request, db_session) + is_admin, _, _ = await get_election_permissions(request, db_session) election_list = await elections.crud.get_all_elections(db_session) if election_list is None or len(election_list) == 0: raise HTTPException( @@ -145,7 +145,7 @@ async def get_election( detail=f"election with slug {slugified_name} does not exist" ) - is_valid_user, _, _ = await get_user_permissions(request, db_session) + is_valid_user, _, _ = await get_election_permissions(request, db_session) if current_time >= election.datetime_start_voting or is_valid_user: election_json = election.private_details(current_time) @@ -233,7 +233,7 @@ async def create_election( available_positions ) - is_valid_user, _, _ = await get_user_permissions(request, db_session) + is_valid_user, _, _ = await get_election_permissions(request, db_session) if not is_valid_user: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, @@ -293,7 +293,7 @@ async def update_election( db_session: database.DBSession, election_name: str, ): - is_valid_user, _, _ = await get_user_permissions(request, db_session) + is_valid_user, _, _ = await get_election_permissions(request, db_session) if not is_valid_user: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, @@ -349,7 +349,7 @@ async def delete_election( election_name: str ): slugified_name = slugify(election_name) - is_valid_user, _, _ = await get_user_permissions(request, db_session) + is_valid_user, _, _ = await get_election_permissions(request, db_session) if not is_valid_user: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, diff --git a/src/officers/models.py b/src/officers/models.py new file mode 100644 index 0000000..c7913fc --- /dev/null +++ b/src/officers/models.py @@ -0,0 +1,41 @@ +from datetime import datetime + +from pydantic import BaseModel + +from officers.constants import OfficerPositionEnum + + +class BaseOfficerModel(BaseModel): + # TODO (#71): compute this using SFU's API & remove from being uploaded + legal_name: str + position: OfficerPositionEnum + start_date: datetime + end_date: str | None = None + csss_email: str + +class PublicOfficerResponse(BaseOfficerModel): + """ + Response when fetching public officer data + """ + is_active: bool + nickname: str | None = None + discord_name: str | None = None + discord_nickname: int | None = None + biography: str | None = None + +class PrivateOfficerResponse(PublicOfficerResponse): + """ + Response when fetching private officer data + """ + computing_id: str + phone_number: str | None = None + github_username: str | None = None + google_drive_email: str | None = None + +class OfficerTermParams(BaseModel): + """ + Create a new officer term + """ + computing_id: str + position: OfficerPositionEnum + start_date: str diff --git a/src/officers/urls.py b/src/officers/urls.py index 8ce70f7..9acfe81 100755 --- a/src/officers/urls.py +++ b/src/officers/urls.py @@ -1,5 +1,3 @@ -import logging - from fastapi import APIRouter, Body, HTTPException, Request from fastapi.responses import JSONResponse, PlainTextResponse @@ -7,13 +5,13 @@ import database import officers.crud import utils +from elections.urls import get_election_permissions +from officers.models import PrivateOfficerResponse, PublicOfficerResponse from officers.tables import OfficerInfo, OfficerTerm from officers.types import InitialOfficerInfo, OfficerInfoUpload, OfficerTermUpload from permission.types import OfficerPrivateInfo, WebsiteAdmin from utils.urls import logged_in_or_raise -_logger = logging.getLogger(__name__) - router = APIRouter( prefix="/officers", tags=["officers"], @@ -25,32 +23,34 @@ async def _has_officer_private_info_access( request: Request, db_session: database.DBSession -) -> tuple[None | str, None | str, bool]: +) -> tuple[bool, str | None,]: """determine if the user has access to private officer info""" session_id = request.cookies.get("session_id", None) if session_id is None: - return None, None, False + return False, None computing_id = await auth.crud.get_computing_id(db_session, session_id) if computing_id is None: - return session_id, None, False + return False, None has_private_access = await OfficerPrivateInfo.has_permission(db_session, computing_id) - return session_id, computing_id, has_private_access + return has_private_access, computing_id # ---------------------------------------- # # 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.", + description="Get information about the current officers. With no authorization, only get basic info.", + response_model=list[PrivateOfficerResponse] | list[PublicOfficerResponse], + operation_id="get_current_officers" ) async def current_officers( # the request headers request: Request, db_session: database.DBSession, ): - _, _, has_private_access = await _has_officer_private_info_access(request, db_session) + has_private_access, _ = await _has_officer_private_info_access(request, db_session) current_officers = await officers.crud.current_officers(db_session, has_private_access) return JSONResponse({ position: [ @@ -71,7 +71,7 @@ async def all_officers( # and may only be accessed by that officer and executives. All other officer terms are public. include_future_terms: bool = False, ): - _, computing_id, has_private_access = await _has_officer_private_info_access(request, db_session) + has_private_access, computing_id = await _has_officer_private_info_access(request, db_session) if include_future_terms: is_website_admin = (computing_id is not None) and (await WebsiteAdmin.has_permission(db_session, computing_id)) if not is_website_admin: From 14cef3503c088da08ec0ff0ab5d72220ca112f77 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sun, 28 Sep 2025 16:32:26 -0700 Subject: [PATCH 03/32] wip: get current officers done --- src/officers/models.py | 17 ++++++++++++----- src/officers/tables.py | 3 +++ src/officers/urls.py | 19 +++++++++++++------ 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/officers/models.py b/src/officers/models.py index c7913fc..33d55a5 100644 --- a/src/officers/models.py +++ b/src/officers/models.py @@ -11,7 +11,6 @@ class BaseOfficerModel(BaseModel): position: OfficerPositionEnum start_date: datetime end_date: str | None = None - csss_email: str class PublicOfficerResponse(BaseOfficerModel): """ @@ -22,6 +21,7 @@ class PublicOfficerResponse(BaseOfficerModel): discord_name: str | None = None discord_nickname: int | None = None biography: str | None = None + csss_email: str class PrivateOfficerResponse(PublicOfficerResponse): """ @@ -32,10 +32,17 @@ class PrivateOfficerResponse(PublicOfficerResponse): github_username: str | None = None google_drive_email: str | None = None -class OfficerTermParams(BaseModel): +class OfficerTermCreate(BaseOfficerModel): """ - Create a new officer term + Create a new Officer term """ computing_id: str - position: OfficerPositionEnum - start_date: str + +class OfficerTermUpdate(BaseModel): + """ + Update an Officer Term + """ + legal_name: str | None = None + position: OfficerPositionEnum | None = None + start_date: datetime | None = None + end_date: datetime | None = None diff --git a/src/officers/tables.py b/src/officers/tables.py index 23575a0..9bb201e 100644 --- a/src/officers/tables.py +++ b/src/officers/tables.py @@ -8,6 +8,7 @@ Integer, String, Text, + UniqueConstraint, ) from sqlalchemy.orm import Mapped, mapped_column @@ -50,6 +51,8 @@ class OfficerTerm(Base): biography: Mapped[str] = mapped_column(Text, nullable=True) photo_url: Mapped[str] = mapped_column(Text, nullable=True) # some urls get big, best to let it be a string + __table_args__ = (UniqueConstraint("computing_id", "position", "start_date"),) # This needs a comma to work + def serializable_dict(self) -> dict: return { "id": self.id, diff --git a/src/officers/urls.py b/src/officers/urls.py index 9acfe81..79471c0 100755 --- a/src/officers/urls.py +++ b/src/officers/urls.py @@ -46,7 +46,6 @@ async def _has_officer_private_info_access( operation_id="get_current_officers" ) async def current_officers( - # the request headers request: Request, db_session: database.DBSession, ): @@ -62,7 +61,9 @@ async def current_officers( @router.get( "/all", - description="Information for all execs from all exec terms" + description="Information for all execs from all exec terms", + response_model=list[PrivateOfficerResponse] | list[PublicOfficerResponse], + operation_id="get_all_officers" ) async def all_officers( request: Request, @@ -78,10 +79,16 @@ async def all_officers( raise HTTPException(status_code=401, detail="only website admins can view all executive terms that have not started yet") all_officers = await officers.crud.all_officers(db_session, has_private_access, include_future_terms) - return JSONResponse([ - officer_data.serializable_dict() - for officer_data in all_officers - ]) + if has_private_access: + return JSONResponse([ + PrivateOfficerResponse.model_validate(officer_data) + for officer_data in all_officers + ]) + else: + return JSONResponse([ + PublicOfficerResponse.model_validate(officer_data) + for officer_data in all_officers + ]) @router.get( "/terms/{computing_id}", From af9e46492341e72fdcf079e52f7859c334b68aad Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sun, 28 Sep 2025 17:16:20 -0700 Subject: [PATCH 04/32] feat: Migration for Elections and Officers - closes #71: Added unique constraint Officer Term - Changed TIMESTAMP to DATETIME on Elections table --- src/alembic/env.py | 2 + ...f_add_unique_constraint_officer_term_71.py | 52 +++++++++++++++++++ src/elections/tables.py | 6 +-- src/nominees/tables.py | 10 ++-- src/officers/tables.py | 5 +- 5 files changed, 64 insertions(+), 11 deletions(-) create mode 100644 src/alembic/versions/6b58a293a7df_add_unique_constraint_officer_term_71.py diff --git a/src/alembic/env.py b/src/alembic/env.py index b3338bf..c7ac8ab 100644 --- a/src/alembic/env.py +++ b/src/alembic/env.py @@ -9,7 +9,9 @@ import blog.tables import database import elections.tables +import nominees.tables import officers.tables +import registrations.tables from alembic import context # this is the Alembic Config object, which provides diff --git a/src/alembic/versions/6b58a293a7df_add_unique_constraint_officer_term_71.py b/src/alembic/versions/6b58a293a7df_add_unique_constraint_officer_term_71.py new file mode 100644 index 0000000..4a526be --- /dev/null +++ b/src/alembic/versions/6b58a293a7df_add_unique_constraint_officer_term_71.py @@ -0,0 +1,52 @@ +"""add_unique_constraint_officer_term_71 + +Revision ID: 6b58a293a7df +Revises: 243190df5588 +Create Date: 2025-09-28 17:13:51.024959 + +""" +from collections.abc import Sequence + +import sqlalchemy as sa + +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "6b58a293a7df" +down_revision: str | None = "243190df5588" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column("election", "type", + existing_type=sa.VARCHAR(length=64), + nullable=False) + op.alter_column("officer_term", "start_date", + existing_type=sa.DATE(), + type_=sa.DateTime(), + existing_nullable=False) + op.alter_column("officer_term", "end_date", + existing_type=sa.DATE(), + type_=sa.DateTime(), + existing_nullable=True) + op.create_unique_constraint(op.f("uq_officer_term_computing_id"), "officer_term", ["computing_id", "position", "start_date"]) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint(op.f("uq_officer_term_computing_id"), "officer_term", type_="unique") + op.alter_column("officer_term", "end_date", + existing_type=sa.DateTime(), + type_=sa.DATE(), + existing_nullable=True) + op.alter_column("officer_term", "start_date", + existing_type=sa.DateTime(), + type_=sa.DATE(), + existing_nullable=False) + op.alter_column("election", "type", + existing_type=sa.VARCHAR(length=64), + nullable=True) + # ### end Alembic commands ### diff --git a/src/elections/tables.py b/src/elections/tables.py index 1d9709c..ea3776b 100644 --- a/src/elections/tables.py +++ b/src/elections/tables.py @@ -25,9 +25,9 @@ class Election(Base): slug: Mapped[str] = mapped_column(String(MAX_ELECTION_SLUG), primary_key=True) name: Mapped[str] = mapped_column(String(MAX_ELECTION_NAME), nullable=False) type: Mapped[ElectionTypeEnum] = mapped_column(String(64), default=ElectionTypeEnum.GENERAL) - datetime_start_nominations: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) - datetime_start_voting: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) - datetime_end_voting: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) + datetime_start_nominations: Mapped[datetime] = mapped_column(DateTime, nullable=False) + datetime_start_voting: Mapped[datetime] = mapped_column(DateTime, nullable=False) + datetime_end_voting: Mapped[datetime] = mapped_column(DateTime, nullable=False) # a comma-separated string of positions which must be elements of OfficerPosition # By giving it the type `StringList`, the database entry will automatically be marshalled to the correct form diff --git a/src/nominees/tables.py b/src/nominees/tables.py index f2203bf..23dff17 100644 --- a/src/nominees/tables.py +++ b/src/nominees/tables.py @@ -14,11 +14,11 @@ class NomineeInfo(Base): __tablename__ = "election_nominee_info" computing_id: Mapped[str] = mapped_column(String(COMPUTING_ID_LEN), primary_key=True) - full_name: Mapped[str] = mapped_column(String(64), nullable=False) - linked_in: Mapped[str] = mapped_column(String(128)) - instagram: Mapped[str] = mapped_column(String(128)) - email: Mapped[str] = mapped_column(String(64)) - discord_username: Mapped[str] = mapped_column(String(DISCORD_NICKNAME_LEN)) + full_name: Mapped[str] = mapped_column(String(64)) + linked_in: Mapped[str] = mapped_column(String(128), nullable=True) + instagram: Mapped[str] = mapped_column(String(128), nullable=True) + email: Mapped[str] = mapped_column(String(64), nullable=True) + discord_username: Mapped[str] = mapped_column(String(DISCORD_NICKNAME_LEN), nullable=True) def to_update_dict(self) -> dict: return { diff --git a/src/officers/tables.py b/src/officers/tables.py index 9bb201e..16aa555 100644 --- a/src/officers/tables.py +++ b/src/officers/tables.py @@ -28,7 +28,6 @@ class OfficerTerm(Base): __tablename__ = "officer_term" - # TODO (#98): create a unique constraint for (computing_id, position, start_date). id: Mapped[str] = mapped_column(Integer, primary_key=True, autoincrement=True) computing_id: Mapped[str] = mapped_column( @@ -38,9 +37,9 @@ class OfficerTerm(Base): ) position: Mapped[OfficerPositionEnum] = mapped_column(String(128), nullable=False) - start_date: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) + start_date: Mapped[datetime] = mapped_column(DateTime, nullable=False) # end_date is only not-specified for positions that don't have a length (ie. webmaster) - end_date: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=True) + end_date: Mapped[datetime] = mapped_column(DateTime, nullable=True) nickname: Mapped[str] = mapped_column(String(128), nullable=True) favourite_course_0: Mapped[str] = mapped_column(String(64), nullable=True) From c3130b01b48e8cfbdae7178912be27ee55c3b0b2 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sun, 28 Sep 2025 17:26:59 -0700 Subject: [PATCH 05/32] fix: update election type max length to 32 --- .../6b58a293a7df_add_unique_constraint_officer_term_71.py | 2 +- src/elections/tables.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/alembic/versions/6b58a293a7df_add_unique_constraint_officer_term_71.py b/src/alembic/versions/6b58a293a7df_add_unique_constraint_officer_term_71.py index 4a526be..884a497 100644 --- a/src/alembic/versions/6b58a293a7df_add_unique_constraint_officer_term_71.py +++ b/src/alembic/versions/6b58a293a7df_add_unique_constraint_officer_term_71.py @@ -21,7 +21,7 @@ def upgrade() -> None: # ### commands auto generated by Alembic - please adjust! ### op.alter_column("election", "type", - existing_type=sa.VARCHAR(length=64), + existing_type=sa.VARCHAR(length=32), nullable=False) op.alter_column("officer_term", "start_date", existing_type=sa.DATE(), diff --git a/src/elections/tables.py b/src/elections/tables.py index ea3776b..1300943 100644 --- a/src/elections/tables.py +++ b/src/elections/tables.py @@ -24,7 +24,7 @@ class Election(Base): # Slugs are unique identifiers slug: Mapped[str] = mapped_column(String(MAX_ELECTION_SLUG), primary_key=True) name: Mapped[str] = mapped_column(String(MAX_ELECTION_NAME), nullable=False) - type: Mapped[ElectionTypeEnum] = mapped_column(String(64), default=ElectionTypeEnum.GENERAL) + type: Mapped[ElectionTypeEnum] = mapped_column(String(32), default=ElectionTypeEnum.GENERAL) datetime_start_nominations: Mapped[datetime] = mapped_column(DateTime, nullable=False) datetime_start_voting: Mapped[datetime] = mapped_column(DateTime, nullable=False) datetime_end_voting: Mapped[datetime] = mapped_column(DateTime, nullable=False) From c554946f249a324dcf6a09c592a6bdc8217f265e Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sun, 28 Sep 2025 18:02:21 -0700 Subject: [PATCH 06/32] fix: elections and officers datetime to DATE --- ...f_add_unique_constraint_officer_term_71.py | 52 ------------------ ...41c_elections_officers_datetime_to_date.py | 54 +++++++++++++++++++ src/elections/tables.py | 7 +-- src/officers/tables.py | 5 +- 4 files changed, 61 insertions(+), 57 deletions(-) delete mode 100644 src/alembic/versions/6b58a293a7df_add_unique_constraint_officer_term_71.py create mode 100644 src/alembic/versions/876041e5b41c_elections_officers_datetime_to_date.py diff --git a/src/alembic/versions/6b58a293a7df_add_unique_constraint_officer_term_71.py b/src/alembic/versions/6b58a293a7df_add_unique_constraint_officer_term_71.py deleted file mode 100644 index 884a497..0000000 --- a/src/alembic/versions/6b58a293a7df_add_unique_constraint_officer_term_71.py +++ /dev/null @@ -1,52 +0,0 @@ -"""add_unique_constraint_officer_term_71 - -Revision ID: 6b58a293a7df -Revises: 243190df5588 -Create Date: 2025-09-28 17:13:51.024959 - -""" -from collections.abc import Sequence - -import sqlalchemy as sa - -from alembic import op - -# revision identifiers, used by Alembic. -revision: str = "6b58a293a7df" -down_revision: str | None = "243190df5588" -branch_labels: str | Sequence[str] | None = None -depends_on: str | Sequence[str] | None = None - - -def upgrade() -> None: - # ### commands auto generated by Alembic - please adjust! ### - op.alter_column("election", "type", - existing_type=sa.VARCHAR(length=32), - nullable=False) - op.alter_column("officer_term", "start_date", - existing_type=sa.DATE(), - type_=sa.DateTime(), - existing_nullable=False) - op.alter_column("officer_term", "end_date", - existing_type=sa.DATE(), - type_=sa.DateTime(), - existing_nullable=True) - op.create_unique_constraint(op.f("uq_officer_term_computing_id"), "officer_term", ["computing_id", "position", "start_date"]) - # ### end Alembic commands ### - - -def downgrade() -> None: - # ### commands auto generated by Alembic - please adjust! ### - op.drop_constraint(op.f("uq_officer_term_computing_id"), "officer_term", type_="unique") - op.alter_column("officer_term", "end_date", - existing_type=sa.DateTime(), - type_=sa.DATE(), - existing_nullable=True) - op.alter_column("officer_term", "start_date", - existing_type=sa.DateTime(), - type_=sa.DATE(), - existing_nullable=False) - op.alter_column("election", "type", - existing_type=sa.VARCHAR(length=64), - nullable=True) - # ### end Alembic commands ### diff --git a/src/alembic/versions/876041e5b41c_elections_officers_datetime_to_date.py b/src/alembic/versions/876041e5b41c_elections_officers_datetime_to_date.py new file mode 100644 index 0000000..5bad3af --- /dev/null +++ b/src/alembic/versions/876041e5b41c_elections_officers_datetime_to_date.py @@ -0,0 +1,54 @@ +"""elections-officers-datetime-to-date + +Revision ID: 876041e5b41c +Revises: 243190df5588 +Create Date: 2025-09-28 18:01:03.913302 + +""" +from collections.abc import Sequence +from typing import Union + +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "876041e5b41c" +down_revision: str | None = "243190df5588" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column("election", "datetime_start_nominations", + existing_type=postgresql.TIMESTAMP(), + type_=sa.Date(), + existing_nullable=False) + op.alter_column("election", "datetime_start_voting", + existing_type=postgresql.TIMESTAMP(), + type_=sa.Date(), + existing_nullable=False) + op.alter_column("election", "datetime_end_voting", + existing_type=postgresql.TIMESTAMP(), + type_=sa.Date(), + existing_nullable=False) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column("election", "datetime_end_voting", + existing_type=sa.Date(), + type_=postgresql.TIMESTAMP(), + existing_nullable=False) + op.alter_column("election", "datetime_start_voting", + existing_type=sa.Date(), + type_=postgresql.TIMESTAMP(), + existing_nullable=False) + op.alter_column("election", "datetime_start_nominations", + existing_type=sa.Date(), + type_=postgresql.TIMESTAMP(), + existing_nullable=False) + # ### end Alembic commands ### diff --git a/src/elections/tables.py b/src/elections/tables.py index 1300943..0f8d5fb 100644 --- a/src/elections/tables.py +++ b/src/elections/tables.py @@ -1,6 +1,7 @@ from datetime import datetime from sqlalchemy import ( + Date, DateTime, String, ) @@ -25,9 +26,9 @@ class Election(Base): slug: Mapped[str] = mapped_column(String(MAX_ELECTION_SLUG), primary_key=True) name: Mapped[str] = mapped_column(String(MAX_ELECTION_NAME), nullable=False) type: Mapped[ElectionTypeEnum] = mapped_column(String(32), default=ElectionTypeEnum.GENERAL) - datetime_start_nominations: Mapped[datetime] = mapped_column(DateTime, nullable=False) - datetime_start_voting: Mapped[datetime] = mapped_column(DateTime, nullable=False) - datetime_end_voting: Mapped[datetime] = mapped_column(DateTime, nullable=False) + datetime_start_nominations: Mapped[datetime] = mapped_column(Date, nullable=False) + datetime_start_voting: Mapped[datetime] = mapped_column(Date, nullable=False) + datetime_end_voting: Mapped[datetime] = mapped_column(Date, nullable=False) # a comma-separated string of positions which must be elements of OfficerPosition # By giving it the type `StringList`, the database entry will automatically be marshalled to the correct form diff --git a/src/officers/tables.py b/src/officers/tables.py index 16aa555..6d7373e 100644 --- a/src/officers/tables.py +++ b/src/officers/tables.py @@ -3,6 +3,7 @@ from datetime import datetime from sqlalchemy import ( + Date, DateTime, ForeignKey, Integer, @@ -37,9 +38,9 @@ class OfficerTerm(Base): ) position: Mapped[OfficerPositionEnum] = mapped_column(String(128), nullable=False) - start_date: Mapped[datetime] = mapped_column(DateTime, nullable=False) + start_date: Mapped[datetime] = mapped_column(Date, nullable=False) # end_date is only not-specified for positions that don't have a length (ie. webmaster) - end_date: Mapped[datetime] = mapped_column(DateTime, nullable=True) + end_date: Mapped[datetime] = mapped_column(Date, nullable=True) nickname: Mapped[str] = mapped_column(String(128), nullable=True) favourite_course_0: Mapped[str] = mapped_column(String(64), nullable=True) From 1a5a3c1b4a69b4c293279bae793ec78e4aa54036 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sun, 28 Sep 2025 18:04:32 -0700 Subject: [PATCH 07/32] fix: update officer term constraint migration --- ...dda5c_update_officer_term_constraint_71.py | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 src/alembic/versions/a5c42bcdda5c_update_officer_term_constraint_71.py diff --git a/src/alembic/versions/a5c42bcdda5c_update_officer_term_constraint_71.py b/src/alembic/versions/a5c42bcdda5c_update_officer_term_constraint_71.py new file mode 100644 index 0000000..ac535a1 --- /dev/null +++ b/src/alembic/versions/a5c42bcdda5c_update_officer_term_constraint_71.py @@ -0,0 +1,38 @@ +"""update_officer_term_constraint_71 + +Revision ID: a5c42bcdda5c +Revises: 876041e5b41c +Create Date: 2025-09-28 18:03:54.856781 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = 'a5c42bcdda5c' +down_revision: Union[str, None] = '876041e5b41c' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column('election', 'type', + existing_type=sa.VARCHAR(length=64), + type_=sa.String(length=32), + nullable=False) + op.create_unique_constraint(op.f('uq_officer_term_computing_id'), 'officer_term', ['computing_id', 'position', 'start_date']) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint(op.f('uq_officer_term_computing_id'), 'officer_term', type_='unique') + op.alter_column('election', 'type', + existing_type=sa.String(length=32), + type_=sa.VARCHAR(length=64), + nullable=True) + # ### end Alembic commands ### From 777948cf1c57d35dbca89c268a746a4b2de75d36 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sun, 28 Sep 2025 18:07:03 -0700 Subject: [PATCH 08/32] fix: fix lint errors in constraint update --- ...dda5c_update_officer_term_constraint_71.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/alembic/versions/a5c42bcdda5c_update_officer_term_constraint_71.py b/src/alembic/versions/a5c42bcdda5c_update_officer_term_constraint_71.py index ac535a1..b15244e 100644 --- a/src/alembic/versions/a5c42bcdda5c_update_officer_term_constraint_71.py +++ b/src/alembic/versions/a5c42bcdda5c_update_officer_term_constraint_71.py @@ -5,33 +5,33 @@ Create Date: 2025-09-28 18:03:54.856781 """ -from typing import Sequence, Union +from collections.abc import Sequence -from alembic import op import sqlalchemy as sa +from alembic import op # revision identifiers, used by Alembic. -revision: str = 'a5c42bcdda5c' -down_revision: Union[str, None] = '876041e5b41c' -branch_labels: Union[str, Sequence[str], None] = None -depends_on: Union[str, Sequence[str], None] = None +revision: str = "a5c42bcdda5c" +down_revision: str | None = "876041e5b41c" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None def upgrade() -> None: # ### commands auto generated by Alembic - please adjust! ### - op.alter_column('election', 'type', + op.alter_column("election", "type", existing_type=sa.VARCHAR(length=64), type_=sa.String(length=32), nullable=False) - op.create_unique_constraint(op.f('uq_officer_term_computing_id'), 'officer_term', ['computing_id', 'position', 'start_date']) + op.create_unique_constraint(op.f("uq_officer_term_computing_id"), "officer_term", ["computing_id", "position", "start_date"]) # ### end Alembic commands ### def downgrade() -> None: # ### commands auto generated by Alembic - please adjust! ### - op.drop_constraint(op.f('uq_officer_term_computing_id'), 'officer_term', type_='unique') - op.alter_column('election', 'type', + op.drop_constraint(op.f("uq_officer_term_computing_id"), "officer_term", type_="unique") + op.alter_column("election", "type", existing_type=sa.String(length=32), type_=sa.VARCHAR(length=64), nullable=True) From ce28f1563f16a4b9651ef889e8da14f6f88e229d Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sun, 28 Sep 2025 18:17:05 -0700 Subject: [PATCH 09/32] wip: removed some unused imports and updated some constraints --- src/elections/tables.py | 1 - src/officers/constants.py | 1 + src/officers/models.py | 3 ++- src/officers/tables.py | 5 ++--- src/officers/urls.py | 1 - 5 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/elections/tables.py b/src/elections/tables.py index 0f8d5fb..ac6c05c 100644 --- a/src/elections/tables.py +++ b/src/elections/tables.py @@ -2,7 +2,6 @@ from sqlalchemy import ( Date, - DateTime, String, ) from sqlalchemy.orm import Mapped, mapped_column diff --git a/src/officers/constants.py b/src/officers/constants.py index 9784829..8c4ff53 100644 --- a/src/officers/constants.py +++ b/src/officers/constants.py @@ -1,5 +1,6 @@ from enum import StrEnum +OFFICER_POSITION_MAX_LENGTH = 128 class OfficerPositionEnum(StrEnum): PRESIDENT = "president" diff --git a/src/officers/models.py b/src/officers/models.py index 33d55a5..42b7e25 100644 --- a/src/officers/models.py +++ b/src/officers/models.py @@ -18,8 +18,9 @@ class PublicOfficerResponse(BaseOfficerModel): """ is_active: bool nickname: str | None = None + discord_id: str | None = None discord_name: str | None = None - discord_nickname: int | None = None + discord_nickname: str | None = None biography: str | None = None csss_email: str diff --git a/src/officers/tables.py b/src/officers/tables.py index 6d7373e..f886ca3 100644 --- a/src/officers/tables.py +++ b/src/officers/tables.py @@ -4,7 +4,6 @@ from sqlalchemy import ( Date, - DateTime, ForeignKey, Integer, String, @@ -21,7 +20,7 @@ GITHUB_USERNAME_LEN, ) from database import Base -from officers.constants import OfficerPositionEnum +from officers.constants import OFFICER_POSITION_MAX_LENGTH, OfficerPositionEnum # A row represents an assignment of a person to a position. @@ -37,7 +36,7 @@ class OfficerTerm(Base): nullable=False, ) - position: Mapped[OfficerPositionEnum] = mapped_column(String(128), nullable=False) + position: Mapped[OfficerPositionEnum] = mapped_column(String(OFFICER_POSITION_MAX_LENGTH), nullable=False) start_date: Mapped[datetime] = mapped_column(Date, nullable=False) # end_date is only not-specified for positions that don't have a length (ie. webmaster) end_date: Mapped[datetime] = mapped_column(Date, nullable=True) diff --git a/src/officers/urls.py b/src/officers/urls.py index 79471c0..b651a88 100755 --- a/src/officers/urls.py +++ b/src/officers/urls.py @@ -5,7 +5,6 @@ import database import officers.crud import utils -from elections.urls import get_election_permissions from officers.models import PrivateOfficerResponse, PublicOfficerResponse from officers.tables import OfficerInfo, OfficerTerm from officers.types import InitialOfficerInfo, OfficerInfoUpload, OfficerTermUpload From ab736bef082efe17b98c9e0f21113ade3e81fd3f Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sun, 28 Sep 2025 18:20:14 -0700 Subject: [PATCH 10/32] wip: update GET all officers --- src/elections/urls.py | 2 +- src/officers/urls.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/elections/urls.py b/src/elections/urls.py index a235bd2..f5f9421 100644 --- a/src/elections/urls.py +++ b/src/elections/urls.py @@ -88,7 +88,7 @@ def _raise_if_bad_election_data( description="Returns a list of all election & their status", response_model=list[ElectionResponse], responses={ - 404: { "description": "No election found" } + 404: { "description": "No election found", "model": DetailModel } }, operation_id="get_all_elections" ) diff --git a/src/officers/urls.py b/src/officers/urls.py index b651a88..a2d3013 100755 --- a/src/officers/urls.py +++ b/src/officers/urls.py @@ -9,6 +9,7 @@ from officers.tables import OfficerInfo, OfficerTerm from officers.types import InitialOfficerInfo, OfficerInfoUpload, OfficerTermUpload from permission.types import OfficerPrivateInfo, WebsiteAdmin +from utils.shared_models import DetailModel from utils.urls import logged_in_or_raise router = APIRouter( @@ -62,6 +63,9 @@ async def current_officers( "/all", description="Information for all execs from all exec terms", response_model=list[PrivateOfficerResponse] | list[PublicOfficerResponse], + responses={ + 401: { "description": "not authorized to view private info", "model": DetailModel } + }, operation_id="get_all_officers" ) async def all_officers( From 223f1e5adff373dee2dab56de41864cf9577b461 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sun, 28 Sep 2025 19:30:29 -0700 Subject: [PATCH 11/32] wip: update Officer models --- src/officers/constants.py | 4 +++- src/officers/models.py | 42 +++++++++++++++++++-------------------- src/officers/tables.py | 12 +++++------ src/officers/urls.py | 23 ++++++++++----------- 4 files changed, 41 insertions(+), 40 deletions(-) diff --git a/src/officers/constants.py b/src/officers/constants.py index 8c4ff53..59aaf9c 100644 --- a/src/officers/constants.py +++ b/src/officers/constants.py @@ -1,6 +1,8 @@ from enum import StrEnum -OFFICER_POSITION_MAX_LENGTH = 128 +# OFFICER FIELD CONSTRAINTS +OFFICER_POSITION_MAX = 128 +OFFICER_LEGAL_NAME_MAX = 128 class OfficerPositionEnum(StrEnum): PRESIDENT = "president" diff --git a/src/officers/models.py b/src/officers/models.py index 42b7e25..33ea1e3 100644 --- a/src/officers/models.py +++ b/src/officers/models.py @@ -1,18 +1,18 @@ -from datetime import datetime +from datetime import date -from pydantic import BaseModel +from pydantic import BaseModel, Field -from officers.constants import OfficerPositionEnum +from officers.constants import OFFICER_LEGAL_NAME_MAX, OfficerPositionEnum -class BaseOfficerModel(BaseModel): +class OfficerBaseModel(BaseModel): # TODO (#71): compute this using SFU's API & remove from being uploaded - legal_name: str + legal_name: str = Field(..., max_length=OFFICER_LEGAL_NAME_MAX) position: OfficerPositionEnum - start_date: datetime - end_date: str | None = None + start_date: date + end_date: date | None = None -class PublicOfficerResponse(BaseOfficerModel): +class PublicOfficerResponse(OfficerBaseModel): """ Response when fetching public officer data """ @@ -33,17 +33,17 @@ class PrivateOfficerResponse(PublicOfficerResponse): github_username: str | None = None google_drive_email: str | None = None -class OfficerTermCreate(BaseOfficerModel): - """ - Create a new Officer term - """ +class OfficerTermBaseModel(BaseModel): computing_id: str - -class OfficerTermUpdate(BaseModel): - """ - Update an Officer Term - """ - legal_name: str | None = None - position: OfficerPositionEnum | None = None - start_date: datetime | None = None - end_date: datetime | None = None + position: OfficerPositionEnum + start_date: date + +class OfficerTermResponse(OfficerTermBaseModel): + id: int + end_date: date | None = None + favourite_course_0: str | None = None + favourite_course_1: str | None = None + favourite_pl_0: str | None = None + favourite_pl_1: str | None = None + biography: str | None = None + photo_url: str | None = None diff --git a/src/officers/tables.py b/src/officers/tables.py index f886ca3..271fecc 100644 --- a/src/officers/tables.py +++ b/src/officers/tables.py @@ -1,6 +1,6 @@ from __future__ import annotations -from datetime import datetime +from datetime import date, datetime from sqlalchemy import ( Date, @@ -20,7 +20,7 @@ GITHUB_USERNAME_LEN, ) from database import Base -from officers.constants import OFFICER_POSITION_MAX_LENGTH, OfficerPositionEnum +from officers.constants import OFFICER_LEGAL_NAME_MAX, OFFICER_POSITION_MAX, OfficerPositionEnum # A row represents an assignment of a person to a position. @@ -36,10 +36,10 @@ class OfficerTerm(Base): nullable=False, ) - position: Mapped[OfficerPositionEnum] = mapped_column(String(OFFICER_POSITION_MAX_LENGTH), nullable=False) - start_date: Mapped[datetime] = mapped_column(Date, nullable=False) + position: Mapped[OfficerPositionEnum] = mapped_column(String(OFFICER_POSITION_MAX), nullable=False) + start_date: Mapped[date] = mapped_column(Date, nullable=False) # end_date is only not-specified for positions that don't have a length (ie. webmaster) - end_date: Mapped[datetime] = mapped_column(Date, nullable=True) + end_date: Mapped[date] = mapped_column(Date, nullable=True) nickname: Mapped[str] = mapped_column(String(128), nullable=True) favourite_course_0: Mapped[str] = mapped_column(String(64), nullable=True) @@ -113,7 +113,7 @@ class OfficerInfo(Base): ) # TODO (#71): we'll need to use SFU's API to get the legal name for users - legal_name: Mapped[str] = mapped_column(String(128), nullable=False) # some people have long names, you never know + legal_name: Mapped[str] = mapped_column(String(OFFICER_LEGAL_NAME_MAX), nullable=False) # some people have long names, you never know phone_number: Mapped[str] = mapped_column(String(24), nullable=True) # TODO (#99): add unique constraints to discord_id (stops users from stealing the username of someone else) diff --git a/src/officers/urls.py b/src/officers/urls.py index a2d3013..2b72f14 100755 --- a/src/officers/urls.py +++ b/src/officers/urls.py @@ -5,7 +5,7 @@ import database import officers.crud import utils -from officers.models import PrivateOfficerResponse, PublicOfficerResponse +from officers.models import OfficerTermResponse, PrivateOfficerResponse, PublicOfficerResponse from officers.tables import OfficerInfo, OfficerTerm from officers.types import InitialOfficerInfo, OfficerInfoUpload, OfficerTermUpload from permission.types import OfficerPrivateInfo, WebsiteAdmin @@ -82,16 +82,10 @@ async def all_officers( raise HTTPException(status_code=401, detail="only website admins can view all executive terms that have not started yet") all_officers = await officers.crud.all_officers(db_session, has_private_access, include_future_terms) - if has_private_access: - return JSONResponse([ - PrivateOfficerResponse.model_validate(officer_data) - for officer_data in all_officers - ]) - else: - return JSONResponse([ - PublicOfficerResponse.model_validate(officer_data) - for officer_data in all_officers - ]) + return JSONResponse([ + officer_data.serializable_dict() + for officer_data in all_officers + ]) @router.get( "/terms/{computing_id}", @@ -99,6 +93,11 @@ async def all_officers( Get term info for an executive. All term info is public for all past or active terms. Future terms can only be accessed by website admins. """, + response_model=list[OfficerTermResponse], + responses={ + 401: { "description": "not authorized to view private info", "model": DetailModel } + }, + operation_id="get_all_officers" ) async def get_officer_terms( request: Request, @@ -118,7 +117,7 @@ async def get_officer_terms( include_future_terms ) return JSONResponse([ - term.serializable_dict() for term in officer_terms + OfficerTermResponse.model_validate(term) for term in officer_terms ]) @router.get( From b59fffdb1b1bb0e9b44c611e2764bb4c224fe62d Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sun, 28 Sep 2025 20:50:54 -0700 Subject: [PATCH 12/32] fix: site users couldn't have NULL log in time --- ...c458d1ddd_site_user_timestamps_nullable.py | 40 +++++++++++++++++++ src/auth/tables.py | 15 ++++--- src/officers/crud.py | 4 +- 3 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 src/alembic/versions/0a2c458d1ddd_site_user_timestamps_nullable.py diff --git a/src/alembic/versions/0a2c458d1ddd_site_user_timestamps_nullable.py b/src/alembic/versions/0a2c458d1ddd_site_user_timestamps_nullable.py new file mode 100644 index 0000000..6121feb --- /dev/null +++ b/src/alembic/versions/0a2c458d1ddd_site_user_timestamps_nullable.py @@ -0,0 +1,40 @@ +"""site_user_timestamps_nullable + +Revision ID: 0a2c458d1ddd +Revises: a5c42bcdda5c +Create Date: 2025-09-28 20:52:02.486734 + +""" +from collections.abc import Sequence + +from sqlalchemy.dialects import postgresql + +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "0a2c458d1ddd" +down_revision: str | None = "a5c42bcdda5c" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column("site_user", "first_logged_in", + existing_type=postgresql.TIMESTAMP(), + nullable=True) + op.alter_column("site_user", "last_logged_in", + existing_type=postgresql.TIMESTAMP(), + nullable=True) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column("site_user", "last_logged_in", + existing_type=postgresql.TIMESTAMP(), + nullable=False) + op.alter_column("site_user", "first_logged_in", + existing_type=postgresql.TIMESTAMP(), + nullable=False) + # ### end Alembic commands ### diff --git a/src/auth/tables.py b/src/auth/tables.py index b6ffe07..b2471cb 100644 --- a/src/auth/tables.py +++ b/src/auth/tables.py @@ -37,16 +37,21 @@ class SiteUser(Base): ) # first and last time logged into the CSSS API - first_logged_in: Mapped[datetime] = mapped_column(DateTime, nullable=False, server_default=func.now()) - last_logged_in: Mapped[datetime] = mapped_column(DateTime, nullable=False, server_default=func.now()) + first_logged_in: Mapped[datetime | None] = mapped_column(DateTime, nullable=True) + last_logged_in: Mapped[datetime | None] = mapped_column(DateTime, nullable=True) # optional user information for display purposes profile_picture_url: Mapped[str | None] = mapped_column(Text, nullable=True) def serialize(self) -> dict[str, str | int | bool | None]: - return { + + res = { "computing_id": self.computing_id, - "first_logged_in": self.first_logged_in.isoformat(), - "last_logged_in": self.last_logged_in.isoformat(), "profile_picture_url": self.profile_picture_url } + if self.first_logged_in is not None: + res["first_logged_in"] = self.first_logged_in.isoformat() + if self.last_logged_in is not None: + res["last_logged_in"] = self.last_logged_in.isoformat() + + return res diff --git a/src/officers/crud.py b/src/officers/crud.py index a2e1b6f..2886c1e 100644 --- a/src/officers/crud.py +++ b/src/officers/crud.py @@ -161,8 +161,8 @@ async def create_new_officer_info( # if computing_id has not been created as a site_user yet, add them db_session.add(auth.tables.SiteUser( computing_id=new_officer_info.computing_id, - first_logged_in=datetime.now(), - last_logged_in=datetime.now() + first_logged_in=None, + last_logged_in=None )) existing_officer_info = await db_session.scalar( From d817743f9c5e5389f4b819afcf81cc99cfae5789 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sun, 28 Sep 2025 20:54:03 -0700 Subject: [PATCH 13/32] wip: add endpoint information for officer terms and officer info POST --- src/nominees/urls.py | 6 +-- src/officers/crud.py | 13 ++++++ src/officers/models.py | 35 ++++++++++++++-- src/officers/tables.py | 6 +++ src/officers/urls.py | 85 +++++++++++++++++++++------------------ src/registrations/urls.py | 8 ++-- src/utils/urls.py | 30 ++++++++++---- 7 files changed, 127 insertions(+), 56 deletions(-) diff --git a/src/nominees/urls.py b/src/nominees/urls.py index 15d8136..f1b6d9d 100644 --- a/src/nominees/urls.py +++ b/src/nominees/urls.py @@ -8,7 +8,7 @@ NomineeInfoUpdateParams, ) from nominees.tables import NomineeInfo -from utils.urls import admin_or_raise +from utils.urls import AdminTypeEnum, admin_or_raise router = APIRouter( prefix="/nominee", @@ -30,7 +30,7 @@ async def get_nominee_info( computing_id: str ): # Putting this one behind the admin wall since it has contact information - await admin_or_raise(request, db_session) + await admin_or_raise(request, db_session, AdminTypeEnum.Election) nominee_info = await nominees.crud.get_nominee_info(db_session, computing_id) if nominee_info is None: raise HTTPException( @@ -56,7 +56,7 @@ async def provide_nominee_info( computing_id: str ): # TODO: There needs to be a lot more validation here. - await admin_or_raise(request, db_session) + await admin_or_raise(request, db_session, AdminTypeEnum.Election) updated_data = {} # Only update fields that were provided diff --git a/src/officers/crud.py b/src/officers/crud.py index 2886c1e..1fca088 100644 --- a/src/officers/crud.py +++ b/src/officers/crud.py @@ -98,6 +98,19 @@ async def get_officer_info_or_raise(db_session: database.DBSession, computing_id raise HTTPException(status_code=404, detail=f"officer_info for computing_id={computing_id} does not exist yet") return officer_term +async def get_new_officer_info_or_raise(db_session: database.DBSession, computing_id: str) -> OfficerInfo: + """ + This check is for after a create/update + """ + officer_term = await db_session.scalar( + sqlalchemy + .select(OfficerInfo) + .where(OfficerInfo.computing_id == computing_id) + ) + if officer_term is None: + raise HTTPException(status_code=500, detail=f"failed to fetch {computing_id} after update") + return officer_term + async def get_officer_terms( db_session: database.DBSession, computing_id: str, diff --git a/src/officers/models.py b/src/officers/models.py index 33ea1e3..9d0459d 100644 --- a/src/officers/models.py +++ b/src/officers/models.py @@ -5,14 +5,14 @@ from officers.constants import OFFICER_LEGAL_NAME_MAX, OfficerPositionEnum -class OfficerBaseModel(BaseModel): +class OfficerInfoBaseModel(BaseModel): # TODO (#71): compute this using SFU's API & remove from being uploaded legal_name: str = Field(..., max_length=OFFICER_LEGAL_NAME_MAX) position: OfficerPositionEnum start_date: date end_date: date | None = None -class PublicOfficerResponse(OfficerBaseModel): +class PublicOfficerInfoResponse(OfficerInfoBaseModel): """ Response when fetching public officer data """ @@ -24,7 +24,7 @@ class PublicOfficerResponse(OfficerBaseModel): biography: str | None = None csss_email: str -class PrivateOfficerResponse(PublicOfficerResponse): +class PrivateOfficerInfoResponse(PublicOfficerInfoResponse): """ Response when fetching private officer data """ @@ -33,11 +33,40 @@ class PrivateOfficerResponse(PublicOfficerResponse): github_username: str | None = None google_drive_email: str | None = None +class OfficerSelfUpdate(BaseModel): + """ + Used when an Officer is updating their own information + """ + nickname: str | None = None + discord_id: str | None = None + discord_name: str | None = None + discord_nickname: str | None = None + biography: str | None = None + phone_number: str | None = None + github_username: str | None = None + google_drive_email: str | None = None + +class OfficerUpdate(OfficerSelfUpdate): + """ + Used when an admin is updating an Officer's info + """ + legal_name: str | None = Field(None, max_length=OFFICER_LEGAL_NAME_MAX) + position: OfficerPositionEnum | None = None + start_date: date | None = None + end_date: date | None = None + class OfficerTermBaseModel(BaseModel): computing_id: str position: OfficerPositionEnum start_date: date +class OfficerTermCreate(OfficerTermBaseModel): + """ + Params to create a new Officer Term + """ + legal_name: str + + class OfficerTermResponse(OfficerTermBaseModel): id: int end_date: date | None = None diff --git a/src/officers/tables.py b/src/officers/tables.py index 271fecc..bdcac3d 100644 --- a/src/officers/tables.py +++ b/src/officers/tables.py @@ -21,6 +21,7 @@ ) from database import Base from officers.constants import OFFICER_LEGAL_NAME_MAX, OFFICER_POSITION_MAX, OfficerPositionEnum +from officers.models import OfficerSelfUpdate, OfficerUpdate # A row represents an assignment of a person to a position. @@ -149,6 +150,11 @@ def serializable_dict(self) -> dict: "google_drive_email": self.google_drive_email, } + def update_from_params(self, params: OfficerUpdate | OfficerSelfUpdate): + update_data = params.model_dump(exclude_unset=True) + for k, v in update_data.items(): + setattr(update_data, k, v) + def is_filled_in(self): return ( self.computing_id is not None diff --git a/src/officers/urls.py b/src/officers/urls.py index 2b72f14..30ed42f 100755 --- a/src/officers/urls.py +++ b/src/officers/urls.py @@ -5,12 +5,19 @@ import database import officers.crud import utils -from officers.models import OfficerTermResponse, PrivateOfficerResponse, PublicOfficerResponse +from officers.models import ( + OfficerSelfUpdate, + OfficerTermCreate, + OfficerTermResponse, + OfficerUpdate, + PrivateOfficerInfoResponse, + PublicOfficerInfoResponse, +) from officers.tables import OfficerInfo, OfficerTerm from officers.types import InitialOfficerInfo, OfficerInfoUpload, OfficerTermUpload from permission.types import OfficerPrivateInfo, WebsiteAdmin -from utils.shared_models import DetailModel -from utils.urls import logged_in_or_raise +from utils.shared_models import DetailModel, SuccessResponse +from utils.urls import admin_or_raise, is_website_admin, logged_in_or_raise router = APIRouter( prefix="/officers", @@ -42,7 +49,7 @@ async def _has_officer_private_info_access( @router.get( "/current", description="Get information about the current officers. With no authorization, only get basic info.", - response_model=list[PrivateOfficerResponse] | list[PublicOfficerResponse], + response_model=list[PrivateOfficerInfoResponse] | list[PublicOfficerInfoResponse], operation_id="get_current_officers" ) async def current_officers( @@ -62,9 +69,9 @@ async def current_officers( @router.get( "/all", description="Information for all execs from all exec terms", - response_model=list[PrivateOfficerResponse] | list[PublicOfficerResponse], + response_model=list[PrivateOfficerInfoResponse] | list[PublicOfficerInfoResponse], responses={ - 401: { "description": "not authorized to view private info", "model": DetailModel } + 403: { "description": "not authorized to view private info", "model": DetailModel } }, operation_id="get_all_officers" ) @@ -97,7 +104,7 @@ async def all_officers( responses={ 401: { "description": "not authorized to view private info", "model": DetailModel } }, - operation_id="get_all_officers" + operation_id="get_officer_terms_by_id" ) async def get_officer_terms( request: Request, @@ -121,8 +128,13 @@ async def get_officer_terms( ]) @router.get( - "/info/{computing_id}", + "/info/{computing_id:str}", description="Get officer info for the current user, if they've ever been an exec. Only admins can get info about another user.", + response_model=PrivateOfficerInfoResponse, + responses={ + 403: { "description": "not authorized to view author user info", "model": DetailModel } + }, + operation_id="get_officer_info_by_id" ) async def get_officer_info( request: Request, @@ -145,20 +157,19 @@ async def get_officer_info( 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. """, + response_model=SuccessResponse, + responses={ + 403: { "description": "must be a website admin", "model": DetailModel }, + 500: { "model": DetailModel }, + }, + operation_id="create_officer_term" ) async def new_officer_term( request: Request, db_session: database.DBSession, - officer_info_list: list[InitialOfficerInfo] = Body(), # noqa: B008 + officer_info_list: list[OfficerTermCreate], ): - """ - If the current computing_id is not already an officer, officer_info will be created for them. - """ - for officer_info in officer_info_list: - officer_info.valid_or_raise() - - _, session_computing_id = await logged_in_or_raise(request, db_session) - await WebsiteAdmin.has_permission_or_raise(db_session, session_computing_id) + await admin_or_raise(request, db_session) for officer_info in officer_info_list: # if user with officer_info.computing_id has never logged into the website before, @@ -166,7 +177,7 @@ async def new_officer_term( await officers.crud.create_new_officer_info(db_session, OfficerInfo( computing_id = officer_info.computing_id, # TODO (#71): use sfu api to get legal name from officer_info.computing_id - legal_name = "default name", + legal_name = officer_info.legal_name, phone_number = None, discord_id = None, @@ -183,47 +194,43 @@ async def new_officer_term( )) await db_session.commit() - return PlainTextResponse("ok") + return JSONResponse({ "success": True }) @router.patch( - "/info/{computing_id}", + "/info/{computing_id:str}", description=""" After election, 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. - """ + """, + response_model=OfficerPrivateInfo, + responses={ + 403: { "description": "must be a website admin", "model": DetailModel }, + 500: { "description": "failed to fetch after update", "model": DetailModel }, + }, + operation_id="update_officer_info" ) async def update_info( request: Request, db_session: database.DBSession, computing_id: str, - officer_info_upload: OfficerInfoUpload = Body() # noqa: B008 + officer_info_upload: OfficerUpdate | OfficerSelfUpdate ): - officer_info_upload.valid_or_raise() - _, session_computing_id = await logged_in_or_raise(request, db_session) + is_site_admin, _, session_computing_id = await is_website_admin(request, db_session) - if computing_id != session_computing_id: - await WebsiteAdmin.has_permission_or_raise( - db_session, session_computing_id, - errmsg="must have website admin permissions to update another user" - ) + if computing_id != session_computing_id and not is_site_admin: + raise HTTPException(status_code=403, detail="you may not update other officers") old_officer_info = await officers.crud.get_officer_info_or_raise(db_session, computing_id) - validation_failures, corrected_officer_info = await officer_info_upload.validate(computing_id, old_officer_info) + old_officer_info.update_from_params(officer_info_upload) + await officers.crud.update_officer_info(db_session, old_officer_info) # TODO (#27): log all important changes just to a .log file & persist them for a few years - success = await officers.crud.update_officer_info(db_session, corrected_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.get_officer_info_or_raise(db_session, computing_id) - return JSONResponse({ - "officer_info": updated_officer_info.serializable_dict(), - "validation_failures": validation_failures, - }) + updated_officer_info = await officers.crud.get_new_officer_info_or_raise(db_session, computing_id) + return JSONResponse(updated_officer_info) @router.patch( "/term/{term_id}", diff --git a/src/registrations/urls.py b/src/registrations/urls.py index 1fe552f..c71c656 100644 --- a/src/registrations/urls.py +++ b/src/registrations/urls.py @@ -18,7 +18,7 @@ ) from registrations.tables import NomineeApplication from utils.shared_models import DetailModel, SuccessResponse -from utils.urls import admin_or_raise, logged_in_or_raise, slugify +from utils.urls import AdminTypeEnum, admin_or_raise, logged_in_or_raise, slugify router = APIRouter( prefix="/registration", @@ -74,7 +74,7 @@ async def register_in_election( body: NomineeApplicationParams, election_name: str ): - await admin_or_raise(request, db_session) + await admin_or_raise(request, db_session, AdminTypeEnum.Election) if body.position not in OfficerPositionEnum: raise HTTPException( @@ -157,7 +157,7 @@ async def update_registration( computing_id: str, position: OfficerPositionEnum ): - await admin_or_raise(request, db_session) + await admin_or_raise(request, db_session, AdminTypeEnum.Election) if body.position not in OfficerPositionEnum: raise HTTPException( @@ -221,7 +221,7 @@ async def delete_registration( position: OfficerPositionEnum, computing_id: str ): - await admin_or_raise(request, db_session) + await admin_or_raise(request, db_session, AdminTypeEnum.Election) if position not in OfficerPositionEnum: raise HTTPException( diff --git a/src/utils/urls.py b/src/utils/urls.py index 0313300..6d4c391 100644 --- a/src/utils/urls.py +++ b/src/utils/urls.py @@ -1,4 +1,5 @@ import re +from enum import Enum from fastapi import HTTPException, Request, status @@ -8,6 +9,11 @@ from permission.types import ElectionOfficer, WebsiteAdmin +class AdminTypeEnum(Enum): + Full = 1 + Election = 2 + + # TODO: move other utils into this module def slugify(text: str) -> str: """Creates a unique slug based on text passed in. Assumes non-unicode text.""" @@ -49,7 +55,8 @@ async def get_current_user(request: Request, db_session: database.DBSession) -> return session_id, session_computing_id -async def admin_or_raise(request: Request, db_session: database.DBSession) -> tuple[str, str]: +# TODO: Add an election admin version that checks the election attempting to be modified as well +async def admin_or_raise(request: Request, db_session: database.DBSession, admintype: AdminTypeEnum = AdminTypeEnum.Full) -> tuple[str, str]: session_id, computing_id = await get_current_user(request, db_session) if not session_id or not computing_id: raise HTTPException( @@ -58,11 +65,20 @@ async def admin_or_raise(request: Request, db_session: database.DBSession) -> tu ) # where valid means election officer or website admin - if (await ElectionOfficer.has_permission(db_session, computing_id)) or (await WebsiteAdmin.has_permission(db_session, computing_id)): + if (await WebsiteAdmin.has_permission(db_session, computing_id)) or (admintype is AdminTypeEnum.Election and await ElectionOfficer.has_permission(db_session, computing_id)): return session_id, computing_id - else: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="must be an admin" - ) + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="must be an admin" + ) + +async def is_website_admin(request: Request, db_session: database.DBSession) -> tuple[bool, str | None, str | None]: + session_id, computing_id = await get_current_user(request, db_session) + if session_id is None or computing_id is None: + return False, session_id, computing_id + + if (await WebsiteAdmin.has_permission(db_session, computing_id)): + return True, session_id, computing_id + + return False, session_id, computing_id From 75f6644e47066ea18e46dd2b907f3219710e6d37 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sun, 28 Sep 2025 21:33:47 -0700 Subject: [PATCH 14/32] wip: PATCH officer term and officer info done --- src/officers/crud.py | 7 +++-- src/officers/models.py | 18 ++++++++++-- src/officers/tables.py | 11 ++++++- src/officers/urls.py | 67 ++++++++++++++++-------------------------- 4 files changed, 56 insertions(+), 47 deletions(-) diff --git a/src/officers/crud.py b/src/officers/crud.py index 1fca088..8a38a93 100644 --- a/src/officers/crud.py +++ b/src/officers/crud.py @@ -155,14 +155,17 @@ async def current_officer_positions(db_session: database.DBSession, computing_id officer_term_list = await get_active_officer_terms(db_session, computing_id) return [term.position for term in officer_term_list] -async def get_officer_term_by_id(db_session: database.DBSession, term_id: int) -> OfficerTerm: +async def get_officer_term_by_id_or_raise(db_session: database.DBSession, term_id: int, is_new: bool = False) -> OfficerTerm: officer_term = await db_session.scalar( sqlalchemy .select(OfficerTerm) .where(OfficerTerm.id == term_id) ) if officer_term is None: - raise HTTPException(status_code=400, detail=f"Could not find officer_term with id={term_id}") + if is_new: + raise HTTPException(status_code=500, detail=f"could not find new officer_term with id={term_id}") + else: + raise HTTPException(status_code=400, detail=f"could not find officer_term with id={term_id}") return officer_term async def create_new_officer_info( diff --git a/src/officers/models.py b/src/officers/models.py index 9d0459d..fcfb2ba 100644 --- a/src/officers/models.py +++ b/src/officers/models.py @@ -66,8 +66,7 @@ class OfficerTermCreate(OfficerTermBaseModel): """ legal_name: str - -class OfficerTermResponse(OfficerTermBaseModel): +class OfficerTermResponse(OfficerTermCreate): id: int end_date: date | None = None favourite_course_0: str | None = None @@ -76,3 +75,18 @@ class OfficerTermResponse(OfficerTermBaseModel): favourite_pl_1: str | None = None biography: str | None = None photo_url: str | None = None + + +class OfficerTermUpdate(BaseModel): + nickname: str | None = None + favourite_course_0: str | None = None + favourite_course_1: str | None = None + favourite_pl_0: str | None = None + favourite_pl_1: str | None = None + biography: str | None = None + + # Admin only + position: OfficerPositionEnum | None = None + start_date: date | None = None + end_date: date | None = None + photo_url: str | None = None # Block this, just in case diff --git a/src/officers/tables.py b/src/officers/tables.py index bdcac3d..8394bde 100644 --- a/src/officers/tables.py +++ b/src/officers/tables.py @@ -21,7 +21,7 @@ ) from database import Base from officers.constants import OFFICER_LEGAL_NAME_MAX, OFFICER_POSITION_MAX, OfficerPositionEnum -from officers.models import OfficerSelfUpdate, OfficerUpdate +from officers.models import OfficerSelfUpdate, OfficerTermUpdate, OfficerUpdate # A row represents an assignment of a person to a position. @@ -71,6 +71,15 @@ def serializable_dict(self) -> dict: "photo_url": self.photo_url, } + def update_from_params(self, params: OfficerTermUpdate, self_update: bool = True): + if self_update: + update_data = params.model_dump(exclude_unset=True, exclude={"position", "start_date", "end_date", "photo_url"}) + else: + update_data = params.model_dump(exclude_unset=True) + for k, v in update_data.items(): + setattr(update_data, k, v) + + def is_filled_in(self): return ( # photo & end_date don't have to be uploaded for the term to be "filled" diff --git a/src/officers/urls.py b/src/officers/urls.py index 30ed42f..f5bf6e4 100755 --- a/src/officers/urls.py +++ b/src/officers/urls.py @@ -1,5 +1,5 @@ -from fastapi import APIRouter, Body, HTTPException, Request -from fastapi.responses import JSONResponse, PlainTextResponse +from fastapi import APIRouter, HTTPException, Request +from fastapi.responses import JSONResponse import auth.crud import database @@ -9,12 +9,12 @@ OfficerSelfUpdate, OfficerTermCreate, OfficerTermResponse, + OfficerTermUpdate, OfficerUpdate, PrivateOfficerInfoResponse, PublicOfficerInfoResponse, ) from officers.tables import OfficerInfo, OfficerTerm -from officers.types import InitialOfficerInfo, OfficerInfoUpload, OfficerTermUpload from permission.types import OfficerPrivateInfo, WebsiteAdmin from utils.shared_models import DetailModel, SuccessResponse from utils.urls import admin_or_raise, is_website_admin, logged_in_or_raise @@ -203,7 +203,7 @@ async def new_officer_term( 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. """, - response_model=OfficerPrivateInfo, + response_model=PrivateOfficerInfoResponse, responses={ 403: { "description": "must be a website admin", "model": DetailModel }, 500: { "description": "failed to fetch after update", "model": DetailModel }, @@ -234,59 +234,42 @@ async def update_info( @router.patch( "/term/{term_id}", - description="" + description="Update the information for an Officer's term", + response_model=OfficerTermResponse, + responses={ + 403: { "description": "must be a website admin", "model": DetailModel }, + 500: { "description": "failed to fetch after update", "model": DetailModel }, + }, + operation_id="update_officer_term" ) async def update_term( request: Request, db_session: database.DBSession, term_id: int, - officer_term_upload: OfficerTermUpload = Body(), # noqa: B008 + body: OfficerTermUpdate ): """ A website admin may change the position & term length however they wish. """ - officer_term_upload.valid_or_raise() - _, session_computing_id = await logged_in_or_raise(request, db_session) + is_site_admin, _, session_computing_id = await is_website_admin(request, db_session) - old_officer_term = await officers.crud.get_officer_term_by_id(db_session, term_id) - if old_officer_term.computing_id != session_computing_id: - await WebsiteAdmin.has_permission_or_raise( - db_session, session_computing_id, - errmsg="must have website admin permissions to update another user" - ) - elif utils.is_past_term(old_officer_term): - await WebsiteAdmin.has_permission_or_raise( - db_session, session_computing_id, - errmsg="only website admins can update past terms" - ) + old_officer_term = await officers.crud.get_officer_term_by_id_or_raise(db_session, term_id) + if not is_site_admin: + if old_officer_term.computing_id != session_computing_id: + raise HTTPException(status_code=403, detail="you may not update other officer terms") - if ( - officer_term_upload.computing_id != old_officer_term.computing_id - or 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 - ): - await WebsiteAdmin.has_permission_or_raise( - db_session, session_computing_id, - errmsg="only admins can write new versions of position, start_date, and end_date" - ) + if utils.is_past_term(old_officer_term): + raise HTTPException(status_code=403, detail="you may not update past terms") + + old_officer_term.update_from_params(body) # TODO (#27): log all important changes to a .log file - success = await officers.crud.update_officer_term( - db_session, - officer_term_upload.to_officer_term(term_id) - ) - if not success: - raise HTTPException(status_code=400, detail="the associated officer_term does not exist yet, please create it first") + await officers.crud.update_officer_term(db_session, old_officer_term) await db_session.commit() - new_officer_term = await officers.crud.get_officer_term_by_id(db_session, term_id) - return JSONResponse({ - "officer_term": new_officer_term.serializable_dict(), - # none for now, but may be added if frontend requests - "validation_failures": [], - }) + new_officer_term = await officers.crud.get_officer_term_by_id_or_raise(db_session, term_id) + return JSONResponse(new_officer_term) @router.delete( "/term/{term_id}", @@ -303,7 +286,7 @@ async def remove_officer( errmsg="must have website admin permissions to remove a term" ) - deleted_officer_term = await officers.crud.get_officer_term_by_id(db_session, term_id) + deleted_officer_term = await officers.crud.get_officer_term_by_id_or_raise(db_session, term_id) # TODO (#27): log all important changes to a .log file From c9b561b9ce18f6dac35b16fc9c0c61003e5ab77d Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sun, 28 Sep 2025 22:03:48 -0700 Subject: [PATCH 15/32] wip: DELETE officer term works --- src/officers/crud.py | 2 +- src/officers/urls.py | 28 +++++++++++++--------------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/officers/crud.py b/src/officers/crud.py index 8a38a93..036ad7b 100644 --- a/src/officers/crud.py +++ b/src/officers/crud.py @@ -165,7 +165,7 @@ async def get_officer_term_by_id_or_raise(db_session: database.DBSession, term_i if is_new: raise HTTPException(status_code=500, detail=f"could not find new officer_term with id={term_id}") else: - raise HTTPException(status_code=400, detail=f"could not find officer_term with id={term_id}") + raise HTTPException(status_code=404, detail=f"could not find officer_term with id={term_id}") return officer_term async def create_new_officer_info( diff --git a/src/officers/urls.py b/src/officers/urls.py index f5bf6e4..27a3668 100755 --- a/src/officers/urls.py +++ b/src/officers/urls.py @@ -233,14 +233,14 @@ async def update_info( return JSONResponse(updated_officer_info) @router.patch( - "/term/{term_id}", + "/term/{term_id:int}", description="Update the information for an Officer's term", response_model=OfficerTermResponse, responses={ 403: { "description": "must be a website admin", "model": DetailModel }, 500: { "description": "failed to fetch after update", "model": DetailModel }, }, - operation_id="update_officer_term" + operation_id="update_officer_term_by_id" ) async def update_term( request: Request, @@ -272,28 +272,26 @@ async def update_term( return JSONResponse(new_officer_term) @router.delete( - "/term/{term_id}", + "/term/{term_id:int}", description="Remove the specified officer term. Only website admins can run this endpoint. BE CAREFUL WITH THIS!", + response_model=SuccessResponse, + responses={ + 401: { "description": "must be logged in", "model": DetailModel }, + 403: { "description": "must be a website admin", "model": DetailModel }, + 500: { "description": "server error", "model": DetailModel }, + }, + operation_id="delete_officer_term_by_id" ) -async def remove_officer( +async def remove_officer_term( request: Request, db_session: database.DBSession, term_id: int, ): - _, session_computing_id = await logged_in_or_raise(request, db_session) - await WebsiteAdmin.has_permission_or_raise( - db_session, session_computing_id, - errmsg="must have website admin permissions to remove a term" - ) - - deleted_officer_term = await officers.crud.get_officer_term_by_id_or_raise(db_session, term_id) + await admin_or_raise(request, db_session) # TODO (#27): log all important changes to a .log file - # TODO (#100): return whether the deletion succeeded or not await officers.crud.delete_officer_term_by_id(db_session, term_id) await db_session.commit() - return JSONResponse({ - "officer_term": deleted_officer_term.serializable_dict(), - }) + return SuccessResponse(success=True) From 87bf0d57c1660de52736afd53bd9b932f0242289 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sun, 28 Sep 2025 22:24:48 -0700 Subject: [PATCH 16/32] fix: return to timezone unaware datetimes --- src/elections/tables.py | 10 +++++----- src/elections/urls.py | 8 ++++---- src/registrations/urls.py | 6 +++--- tests/integration/test_elections.py | 18 +++++++++--------- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/elections/tables.py b/src/elections/tables.py index ac6c05c..270631a 100644 --- a/src/elections/tables.py +++ b/src/elections/tables.py @@ -1,7 +1,7 @@ -from datetime import datetime +from datetime import date, datetime from sqlalchemy import ( - Date, + DateTime, String, ) from sqlalchemy.orm import Mapped, mapped_column @@ -25,9 +25,9 @@ class Election(Base): slug: Mapped[str] = mapped_column(String(MAX_ELECTION_SLUG), primary_key=True) name: Mapped[str] = mapped_column(String(MAX_ELECTION_NAME), nullable=False) type: Mapped[ElectionTypeEnum] = mapped_column(String(32), default=ElectionTypeEnum.GENERAL) - datetime_start_nominations: Mapped[datetime] = mapped_column(Date, nullable=False) - datetime_start_voting: Mapped[datetime] = mapped_column(Date, nullable=False) - datetime_end_voting: Mapped[datetime] = mapped_column(Date, nullable=False) + datetime_start_nominations: Mapped[datetime] = mapped_column(DateTime, nullable=False) + datetime_start_voting: Mapped[datetime] = mapped_column(DateTime, nullable=False) + datetime_end_voting: Mapped[datetime] = mapped_column(DateTime, nullable=False) # a comma-separated string of positions which must be elements of OfficerPosition # By giving it the type `StringList`, the database entry will automatically be marshalled to the correct form diff --git a/src/elections/urls.py b/src/elections/urls.py index f5f9421..3dbbf39 100644 --- a/src/elections/urls.py +++ b/src/elections/urls.py @@ -104,7 +104,7 @@ async def list_elections( detail="no election found" ) - current_time = datetime.datetime.now(tz=datetime.UTC) + current_time = datetime.datetime.now() if is_admin: election_metadata_list = [ election.private_details(current_time) @@ -136,7 +136,7 @@ async def get_election( db_session: database.DBSession, election_name: str ): - current_time = datetime.datetime.now(tz=datetime.UTC) + current_time = datetime.datetime.now() slugified_name = slugify(election_name) election = await elections.crud.get_election(db_session, slugified_name) if election is None: @@ -218,7 +218,7 @@ async def create_election( available_positions = body.available_positions slugified_name = slugify(body.name) - current_time = datetime.datetime.now(tz=datetime.UTC) + current_time = datetime.datetime.now() start_nominations = datetime.datetime.fromisoformat(body.datetime_start_nominations) start_voting = datetime.datetime.fromisoformat(body.datetime_start_voting) end_voting = datetime.datetime.fromisoformat(body.datetime_end_voting) @@ -332,7 +332,7 @@ async def update_election( election = await elections.crud.get_election(db_session, slugified_name) if election is None: raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="couldn't find updated election") - return JSONResponse(election.private_details(datetime.datetime.now(tz=datetime.UTC))) + return JSONResponse(election.private_details(datetime.datetime.now())) @router.delete( "/{election_name:str}", diff --git a/src/registrations/urls.py b/src/registrations/urls.py index c71c656..f723c4f 100644 --- a/src/registrations/urls.py +++ b/src/registrations/urls.py @@ -105,7 +105,7 @@ async def register_in_election( detail=f"{body.position} is not available to register for in this election" ) - if election.status(datetime.datetime.now(tz=datetime.UTC)) != ElectionStatusEnum.NOMINATIONS: + if election.status(datetime.datetime.now()) != ElectionStatusEnum.NOMINATIONS: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail="registrations can only be made during the nomination period" @@ -174,7 +174,7 @@ async def update_registration( ) # self updates can only be done during nomination period. Officer updates can be done whenever - if election.status(datetime.datetime.now(tz=datetime.UTC)) != ElectionStatusEnum.NOMINATIONS: + if election.status(datetime.datetime.now()) != ElectionStatusEnum.NOMINATIONS: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail="speeches can only be updated during the nomination period" @@ -237,7 +237,7 @@ async def delete_registration( detail=f"election with slug {slugified_name} does not exist" ) - if election.status(datetime.datetime.now(tz=datetime.UTC)) != ElectionStatusEnum.NOMINATIONS: + if election.status(datetime.datetime.now()) != ElectionStatusEnum.NOMINATIONS: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail="registration can only be revoked during the nomination period" diff --git a/tests/integration/test_elections.py b/tests/integration/test_elections.py index b79220f..2949b99 100644 --- a/tests/integration/test_elections.py +++ b/tests/integration/test_elections.py @@ -198,9 +198,9 @@ async def test_endpoints_admin(client, database_setup): response = await client.post("/election", json={ "name": "testElection4", "type": "general_election", - "datetime_start_nominations": (datetime.datetime.now(tz=datetime.UTC) - timedelta(days=1)).isoformat(), - "datetime_start_voting": (datetime.datetime.now(tz=datetime.UTC) + timedelta(days=7)).isoformat(), - "datetime_end_voting": (datetime.datetime.now(tz=datetime.UTC) + timedelta(days=14)).isoformat(), + "datetime_start_nominations": (datetime.datetime.now() - timedelta(days=1)).isoformat(), + "datetime_start_voting": (datetime.datetime.now() + timedelta(days=7)).isoformat(), + "datetime_end_voting": (datetime.datetime.now() + timedelta(days=14)).isoformat(), "available_positions": ["president", "treasurer"], "survey_link": "https://youtu.be/dQw4w9WgXcQ?si=kZROi2tu-43MXPM5" }) @@ -209,9 +209,9 @@ async def test_endpoints_admin(client, database_setup): response = await client.post("/election", json={ "name": "byElection4", "type": "by_election", - "datetime_start_nominations": (datetime.datetime.now(tz=datetime.UTC) - timedelta(days=1)).isoformat(), - "datetime_start_voting": (datetime.datetime.now(tz=datetime.UTC) + timedelta(days=7)).isoformat(), - "datetime_end_voting": (datetime.datetime.now(tz=datetime.UTC) + timedelta(days=14)).isoformat(), + "datetime_start_nominations": (datetime.datetime.now() - timedelta(days=1)).isoformat(), + "datetime_start_voting": (datetime.datetime.now() + timedelta(days=7)).isoformat(), + "datetime_end_voting": (datetime.datetime.now() + timedelta(days=14)).isoformat(), "survey_link": "https://youtu.be/dQw4w9WgXcQ?si=kZROi2tu-43MXPM5" }) assert response.status_code == 200 @@ -276,9 +276,9 @@ async def test_endpoints_admin(client, database_setup): # update the above election response = await client.patch("/election/testElection4", json={ "election_type": "general_election", - "datetime_start_nominations": (datetime.datetime.now(tz=datetime.UTC) - timedelta(days=1)).isoformat(), - "datetime_start_voting": (datetime.datetime.now(tz=datetime.UTC) + timedelta(days=7)).isoformat(), - "datetime_end_voting": (datetime.datetime.now(tz=datetime.UTC) + timedelta(days=14)).isoformat(), + "datetime_start_nominations": (datetime.datetime.now() - timedelta(days=1)).isoformat(), + "datetime_start_voting": (datetime.datetime.now() + timedelta(days=7)).isoformat(), + "datetime_end_voting": (datetime.datetime.now() + timedelta(days=14)).isoformat(), "available_positions": ["president", "vice-president", "treasurer"], # update this "survey_link": "https://youtu.be/dQw4w9WgXcQ?si=kZROi2tu-43MXPM5" }) From 0892540687f061d900dc23e54124bbfc7c854624 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Mon, 29 Sep 2025 01:00:33 -0700 Subject: [PATCH 17/32] fix: election timestamps back to datetime --- ...717bd88d06_elections_timestamp_datetime.py | 52 +++++++++++++++++++ src/load_test_db.py | 18 ------- 2 files changed, 52 insertions(+), 18 deletions(-) create mode 100644 src/alembic/versions/0c717bd88d06_elections_timestamp_datetime.py diff --git a/src/alembic/versions/0c717bd88d06_elections_timestamp_datetime.py b/src/alembic/versions/0c717bd88d06_elections_timestamp_datetime.py new file mode 100644 index 0000000..7c9a38a --- /dev/null +++ b/src/alembic/versions/0c717bd88d06_elections_timestamp_datetime.py @@ -0,0 +1,52 @@ +"""elections_timestamp_datetime + +Revision ID: 0c717bd88d06 +Revises: 0a2c458d1ddd +Create Date: 2025-09-28 22:25:28.864945 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = '0c717bd88d06' +down_revision: Union[str, None] = '0a2c458d1ddd' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column('election', 'datetime_start_nominations', + existing_type=sa.DATE(), + type_=sa.DateTime(), + existing_nullable=False) + op.alter_column('election', 'datetime_start_voting', + existing_type=sa.DATE(), + type_=sa.DateTime(), + existing_nullable=False) + op.alter_column('election', 'datetime_end_voting', + existing_type=sa.DATE(), + type_=sa.DateTime(), + existing_nullable=False) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column('election', 'datetime_end_voting', + existing_type=sa.DateTime(), + type_=sa.DATE(), + existing_nullable=False) + op.alter_column('election', 'datetime_start_voting', + existing_type=sa.DateTime(), + type_=sa.DATE(), + existing_nullable=False) + op.alter_column('election', 'datetime_start_nominations', + existing_type=sa.DateTime(), + type_=sa.DATE(), + existing_nullable=False) + # ### end Alembic commands ### diff --git a/src/load_test_db.py b/src/load_test_db.py index 725195f..e59f3a1 100644 --- a/src/load_test_db.py +++ b/src/load_test_db.py @@ -348,24 +348,6 @@ async def load_webmaster(db_session: AsyncSession): biography="The systems are good o7", photo_url=None, )) - # a future term - await create_new_officer_term(db_session, OfficerTerm( - computing_id=WEBMASTER_COMPUTING_ID, - - position=OfficerPositionEnum.DIRECTOR_OF_ARCHIVES, - start_date=date.today() + timedelta(days=365*1), - end_date=date.today() + timedelta(days=365*2), - - nickname="G3", - favourite_course_0="MACM 102", - favourite_course_1="CMPT 127", - - favourite_pl_0="C%", - favourite_pl_1="C$$", - - biography="o hey fellow kids \n\n\n I will can newline .... !!", - photo_url=None, - )) await db_session.commit() async def load_test_elections_data(db_session: AsyncSession): From 422617b0737b862119163c0a8ef98c7634dc3402 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Mon, 29 Sep 2025 01:01:13 -0700 Subject: [PATCH 18/32] wip: GET all officers dynamically shows fields --- src/officers/crud.py | 55 +++++++++++++++++------------- src/officers/models.py | 16 ++++----- src/officers/tables.py | 2 +- src/officers/urls.py | 28 ++++++++++----- src/utils/__init__.py | 3 +- tests/integration/test_officers.py | 46 ++++++++++++------------- 6 files changed, 83 insertions(+), 67 deletions(-) diff --git a/src/officers/crud.py b/src/officers/crud.py index 036ad7b..acd480f 100644 --- a/src/officers/crud.py +++ b/src/officers/crud.py @@ -1,7 +1,9 @@ +from collections.abc import Sequence from datetime import datetime import sqlalchemy from fastapi import HTTPException +from sqlalchemy.ext.asyncio import AsyncSession import auth.crud import auth.tables @@ -9,6 +11,7 @@ import utils from data import semesters from officers.constants import OfficerPosition +from officers.models import OfficerInfoResponse, OfficerTermResponse from officers.tables import OfficerInfo, OfficerTerm from officers.types import ( OfficerData, @@ -54,39 +57,43 @@ async def current_officers( return officer_data async def all_officers( - db_session: database.DBSession, - include_private_data: bool, + db_session: AsyncSession, include_future_terms: bool -) -> list[OfficerData]: +) -> list[OfficerInfoResponse]: """ This could be a lot of data, so be careful """ # NOTE: paginate data if needed - query = ( - sqlalchemy - .select(OfficerTerm) - # Ordered recent first - .order_by(OfficerTerm.start_date.desc()) - ) + query = (sqlalchemy.select(OfficerTerm, OfficerInfo) + .join(OfficerInfo, OfficerTerm.computing_id == OfficerInfo.computing_id) + .order_by(OfficerTerm.start_date.desc()) + ) + if not include_future_terms: query = utils.has_started_term(query) + result: Sequence[sqlalchemy.Row[tuple[OfficerTerm, OfficerInfo]]] = (await db_session.execute(query)).all() + officer_list = [] + for term, officer in result: + officer_list.append(OfficerInfoResponse( + legal_name = officer.legal_name, + is_active = utils.is_active_term(term), + position = term.position, + start_date = term.start_date, + end_date = term.end_date, + biography = term.biography, + csss_email = OfficerPosition.to_email(term.position), - officer_data_list = [] - officer_terms = (await db_session.scalars(query)).all() - for term in officer_terms: - officer_info = await db_session.scalar( - sqlalchemy - .select(OfficerInfo) - .where(OfficerInfo.computing_id == term.computing_id) - ) - officer_data_list += [OfficerData.from_data( - term, - officer_info, - include_private_data, - utils.is_active_term(term) - )] + discord_id = officer.discord_id, + discord_name = officer.discord_name, + discord_nickname = officer.discord_nickname, + computing_id = officer.computing_id, + phone_number = officer.phone_number, + github_username = officer.github_username, + google_drive_email = officer.google_drive_email, + photo_url = term.photo_url + )) - return officer_data_list + return officer_list async def get_officer_info_or_raise(db_session: database.DBSession, computing_id: str) -> OfficerInfo: officer_term = await db_session.scalar( diff --git a/src/officers/models.py b/src/officers/models.py index fcfb2ba..3381ff0 100644 --- a/src/officers/models.py +++ b/src/officers/models.py @@ -12,26 +12,24 @@ class OfficerInfoBaseModel(BaseModel): start_date: date end_date: date | None = None -class PublicOfficerInfoResponse(OfficerInfoBaseModel): +class OfficerInfoResponse(OfficerInfoBaseModel): """ Response when fetching public officer data """ is_active: bool nickname: str | None = None + biography: str | None = None + csss_email: str | None = None + + # Private data discord_id: str | None = None discord_name: str | None = None discord_nickname: str | None = None - biography: str | None = None - csss_email: str - -class PrivateOfficerInfoResponse(PublicOfficerInfoResponse): - """ - Response when fetching private officer data - """ - computing_id: str + computing_id: str | None = None phone_number: str | None = None github_username: str | None = None google_drive_email: str | None = None + photo_url: str | None = None class OfficerSelfUpdate(BaseModel): """ diff --git a/src/officers/tables.py b/src/officers/tables.py index 8394bde..071fc3b 100644 --- a/src/officers/tables.py +++ b/src/officers/tables.py @@ -33,7 +33,7 @@ class OfficerTerm(Base): computing_id: Mapped[str] = mapped_column( String(COMPUTING_ID_LEN), - ForeignKey("site_user.computing_id"), + ForeignKey("officer_info.computing_id"), nullable=False, ) diff --git a/src/officers/urls.py b/src/officers/urls.py index 27a3668..522ec44 100755 --- a/src/officers/urls.py +++ b/src/officers/urls.py @@ -6,13 +6,12 @@ import officers.crud import utils from officers.models import ( + OfficerInfoResponse, OfficerSelfUpdate, OfficerTermCreate, OfficerTermResponse, OfficerTermUpdate, OfficerUpdate, - PrivateOfficerInfoResponse, - PublicOfficerInfoResponse, ) from officers.tables import OfficerInfo, OfficerTerm from permission.types import OfficerPrivateInfo, WebsiteAdmin @@ -49,7 +48,7 @@ async def _has_officer_private_info_access( @router.get( "/current", description="Get information about the current officers. With no authorization, only get basic info.", - response_model=list[PrivateOfficerInfoResponse] | list[PublicOfficerInfoResponse], + response_model=list[OfficerInfoResponse], operation_id="get_current_officers" ) async def current_officers( @@ -69,7 +68,7 @@ async def current_officers( @router.get( "/all", description="Information for all execs from all exec terms", - response_model=list[PrivateOfficerInfoResponse] | list[PublicOfficerInfoResponse], + response_model=list[OfficerInfoResponse], responses={ 403: { "description": "not authorized to view private info", "model": DetailModel } }, @@ -88,9 +87,20 @@ async def all_officers( if not is_website_admin: raise HTTPException(status_code=401, detail="only website admins can view all executive terms that have not started yet") - all_officers = await officers.crud.all_officers(db_session, has_private_access, include_future_terms) - return JSONResponse([ - officer_data.serializable_dict() + all_officers = await officers.crud.all_officers(db_session, include_future_terms) + exclude = { + "discord_id", + "discord_name", + "discord_nickname", + "computing_id", + "phone_number", + "github_username", + "google_drive_email", + "photo_url" + } if not has_private_access else {} + + return JSONResponse(content=[ + officer_data.model_dump(exclude=exclude, mode="json") for officer_data in all_officers ]) @@ -130,7 +140,7 @@ async def get_officer_terms( @router.get( "/info/{computing_id:str}", description="Get officer info for the current user, if they've ever been an exec. Only admins can get info about another user.", - response_model=PrivateOfficerInfoResponse, + response_model=OfficerInfoResponse, responses={ 403: { "description": "not authorized to view author user info", "model": DetailModel } }, @@ -203,7 +213,7 @@ async def new_officer_term( 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. """, - response_model=PrivateOfficerInfoResponse, + response_model=OfficerInfoResponse, responses={ 403: { "description": "must be a website admin", "model": DetailModel }, 500: { "description": "failed to fetch after update", "model": DetailModel }, diff --git a/src/utils/__init__.py b/src/utils/__init__.py index c52bd4c..6baa9c5 100644 --- a/src/utils/__init__.py +++ b/src/utils/__init__.py @@ -1,5 +1,6 @@ import re from datetime import date, datetime +from typing import Tuple from sqlalchemy import Select @@ -34,7 +35,7 @@ def is_active_officer(query: Select) -> Select: ) ) -def has_started_term(query: Select) -> bool: +def has_started_term(query: Select) -> Select[tuple[OfficerTerm]]: return query.where( OfficerTerm.start_date <= date.today() ) diff --git a/tests/integration/test_officers.py b/tests/integration/test_officers.py index dd8ba0c..172fe68 100644 --- a/tests/integration/test_officers.py +++ b/tests/integration/test_officers.py @@ -5,12 +5,12 @@ import pytest from httpx import ASGITransport, AsyncClient -import load_test_db -from auth.crud import create_user_session -from database import SQLALCHEMY_TEST_DATABASE_URL, DatabaseSessionManager -from main import app -from officers.constants import OfficerPosition -from officers.crud import all_officers, current_officers, get_active_officer_terms +from src import load_test_db +from src.auth.crud import create_user_session +from src.database import SQLALCHEMY_TEST_DATABASE_URL, DatabaseSessionManager +from src.main import app +from src.officers.constants import OfficerPosition, OfficerPositionEnum +from src.officers.crud import all_officers, current_officers, get_active_officer_terms # TODO: setup a database on the CI machine & run this as a unit test then (since # this isn't really an integration test) @@ -51,7 +51,7 @@ async def test__read_execs(database_setup): abc11_officer_terms = await get_active_officer_terms(db_session, "abc11") assert len(abc11_officer_terms) == 1 assert abc11_officer_terms[0].computing_id == "abc11" - assert abc11_officer_terms[0].position == OfficerPosition.EXECUTIVE_AT_LARGE + assert abc11_officer_terms[0].position == OfficerPositionEnum.EXECUTIVE_AT_LARGE assert abc11_officer_terms[0].start_date is not None assert abc11_officer_terms[0].nickname == "the holy A" assert abc11_officer_terms[0].favourite_course_0 == "CMPT 361" @@ -59,18 +59,18 @@ async def test__read_execs(database_setup): current_exec_team = await current_officers(db_session, include_private=False) assert current_exec_team is not None - assert len(current_exec_team.keys()) == 4 - assert next(iter(current_exec_team.keys())) == OfficerPosition.EXECUTIVE_AT_LARGE + assert len(current_exec_team.keys()) == 5 + assert next(iter(current_exec_team.keys())) == OfficerPositionEnum.EXECUTIVE_AT_LARGE assert next(iter(current_exec_team.values()))[0].favourite_course_0 == "CMPT 361" - assert next(iter(current_exec_team.values()))[0].csss_email == OfficerPosition.to_email(OfficerPosition.EXECUTIVE_AT_LARGE) + assert next(iter(current_exec_team.values()))[0].csss_email == OfficerPosition.to_email(OfficerPositionEnum.EXECUTIVE_AT_LARGE) assert next(iter(current_exec_team.values()))[0].private_data is None current_exec_team = await current_officers(db_session, include_private=True) assert current_exec_team is not None - assert len(current_exec_team) == 4 - assert next(iter(current_exec_team.keys())) == OfficerPosition.EXECUTIVE_AT_LARGE + assert len(current_exec_team) == 5 + assert next(iter(current_exec_team.keys())) == OfficerPositionEnum.EXECUTIVE_AT_LARGE assert next(iter(current_exec_team.values()))[0].favourite_course_0 == "CMPT 361" - assert next(iter(current_exec_team.values()))[0].csss_email == OfficerPosition.to_email(OfficerPosition.EXECUTIVE_AT_LARGE) + assert next(iter(current_exec_team.values()))[0].csss_email == OfficerPosition.to_email(OfficerPositionEnum.EXECUTIVE_AT_LARGE) assert next(iter(current_exec_team.values()))[0].private_data is not None assert next(iter(current_exec_team.values()))[0].private_data.computing_id == "abc11" @@ -91,7 +91,7 @@ async def test__endpoints(client, database_setup): response = await client.get("/officers/current") assert response.status_code == 200 assert response.json() != {} - assert len(response.json().values()) == 4 + assert len(response.json().values()) == 5 assert not response.json()["executive at large"][0]["private_data"] response = await client.get("/officers/all?include_future_terms=false") @@ -124,7 +124,7 @@ async def test__endpoints(client, database_setup): response = await client.post("officers/term", content=json.dumps([{ "computing_id": "ehbc12", - "position": OfficerPosition.DIRECTOR_OF_MULTIMEDIA, + "position": OfficerPositionEnum.DIRECTOR_OF_MULTIMEDIA, "start_date": "2025-12-29" }])) assert response.status_code == 401 @@ -147,7 +147,7 @@ async def test__endpoints(client, database_setup): response = await client.patch("officers/term/1", content=json.dumps({ "computing_id": "abc11", - "position": OfficerPosition.VICE_PRESIDENT, + "position": OfficerPositionEnum.VICE_PRESIDENT, "start_date": (date.today() - timedelta(days=365)).isoformat(), "end_date": (date.today() - timedelta(days=1)).isoformat(), @@ -177,15 +177,15 @@ async def test__endpoints_admin(client, database_setup): response = await client.get("/officers/current") assert response.status_code == 200 assert response.json() != {} - assert len(response.json().values()) == 4 + assert len(response.json().values()) == 5 assert response.json()["executive at large"][0]["private_data"] response = await client.get("/officers/all?include_future_terms=true") assert response.status_code == 200 assert response.json() != [] - print(len(response.json())) - assert len(response.json()) == 7 - assert response.json()[1]["private_data"]["phone_number"] == "1234567890" + assert len(response.json()) == 9 + print(response.json()[1]) + assert response.json()[1]["phone_number"] == "1234567890" response = await client.get(f"/officers/terms/{load_test_db.SYSADMIN_COMPUTING_ID}?include_future_terms=false") assert response.status_code == 200 @@ -213,7 +213,7 @@ async def test__endpoints_admin(client, database_setup): response = await client.post("officers/term", content=json.dumps([{ "computing_id": "ehbc12", - "position": OfficerPosition.DIRECTOR_OF_MULTIMEDIA, + "position": OfficerPositionEnum.DIRECTOR_OF_MULTIMEDIA, "start_date": "2025-12-29" }])) assert response.status_code == 200 @@ -245,7 +245,7 @@ async def test__endpoints_admin(client, database_setup): response = await client.patch("officers/term/1", content=json.dumps({ "computing_id": "abc11", - "position": OfficerPosition.TREASURER, + "position": OfficerPositionEnum.TREASURER, "start_date": (date.today() - timedelta(days=365)).isoformat(), "end_date": (date.today() - timedelta(days=1)).isoformat(), "nickname": "1", @@ -260,7 +260,7 @@ async def test__endpoints_admin(client, database_setup): response = await client.get("/officers/terms/abc11?include_future_terms=true") assert response.status_code == 200 assert response.json() != [] - assert response.json()[1]["position"] == OfficerPosition.TREASURER + assert response.json()[1]["position"] == OfficerPositionEnum.TREASURER assert response.json()[0]["favourite_course_0"] != "2" assert response.json()[1]["biography"] == "hello o77" From 30785b96e9ee0dfbecc0647991cf59c32e792c1b Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Mon, 29 Sep 2025 01:03:00 -0700 Subject: [PATCH 19/32] fix: lint issues --- ...717bd88d06_elections_timestamp_datetime.py | 25 ++++++++++--------- src/utils/__init__.py | 1 - 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/alembic/versions/0c717bd88d06_elections_timestamp_datetime.py b/src/alembic/versions/0c717bd88d06_elections_timestamp_datetime.py index 7c9a38a..e8e1b2f 100644 --- a/src/alembic/versions/0c717bd88d06_elections_timestamp_datetime.py +++ b/src/alembic/versions/0c717bd88d06_elections_timestamp_datetime.py @@ -5,30 +5,31 @@ Create Date: 2025-09-28 22:25:28.864945 """ -from typing import Sequence, Union +from collections.abc import Sequence +from typing import Union -from alembic import op import sqlalchemy as sa +from alembic import op # revision identifiers, used by Alembic. -revision: str = '0c717bd88d06' -down_revision: Union[str, None] = '0a2c458d1ddd' -branch_labels: Union[str, Sequence[str], None] = None -depends_on: Union[str, Sequence[str], None] = None +revision: str = "0c717bd88d06" +down_revision: str | None = "0a2c458d1ddd" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None def upgrade() -> None: # ### commands auto generated by Alembic - please adjust! ### - op.alter_column('election', 'datetime_start_nominations', + op.alter_column("election", "datetime_start_nominations", existing_type=sa.DATE(), type_=sa.DateTime(), existing_nullable=False) - op.alter_column('election', 'datetime_start_voting', + op.alter_column("election", "datetime_start_voting", existing_type=sa.DATE(), type_=sa.DateTime(), existing_nullable=False) - op.alter_column('election', 'datetime_end_voting', + op.alter_column("election", "datetime_end_voting", existing_type=sa.DATE(), type_=sa.DateTime(), existing_nullable=False) @@ -37,15 +38,15 @@ def upgrade() -> None: def downgrade() -> None: # ### commands auto generated by Alembic - please adjust! ### - op.alter_column('election', 'datetime_end_voting', + op.alter_column("election", "datetime_end_voting", existing_type=sa.DateTime(), type_=sa.DATE(), existing_nullable=False) - op.alter_column('election', 'datetime_start_voting', + op.alter_column("election", "datetime_start_voting", existing_type=sa.DateTime(), type_=sa.DATE(), existing_nullable=False) - op.alter_column('election', 'datetime_start_nominations', + op.alter_column("election", "datetime_start_nominations", existing_type=sa.DateTime(), type_=sa.DATE(), existing_nullable=False) diff --git a/src/utils/__init__.py b/src/utils/__init__.py index 6baa9c5..62c95b6 100644 --- a/src/utils/__init__.py +++ b/src/utils/__init__.py @@ -1,6 +1,5 @@ import re from datetime import date, datetime -from typing import Tuple from sqlalchemy import Select From 8c7f4beba4fb771bc6c5285df8b8780ae18fc09a Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Mon, 29 Sep 2025 01:38:34 -0700 Subject: [PATCH 20/32] wip: get current officers works --- src/officers/crud.py | 61 +++++++++++++++++++++--------------------- src/officers/models.py | 10 +++++++ src/officers/tables.py | 2 +- src/officers/urls.py | 28 +++++++------------ 4 files changed, 51 insertions(+), 50 deletions(-) diff --git a/src/officers/crud.py b/src/officers/crud.py index acd480f..b6d61b1 100644 --- a/src/officers/crud.py +++ b/src/officers/crud.py @@ -1,5 +1,5 @@ from collections.abc import Sequence -from datetime import datetime +from datetime import date, datetime import sqlalchemy from fastapi import HTTPException @@ -11,50 +11,49 @@ import utils from data import semesters from officers.constants import OfficerPosition -from officers.models import OfficerInfoResponse, OfficerTermResponse +from officers.models import OfficerInfoResponse from officers.tables import OfficerInfo, OfficerTerm -from officers.types import ( - OfficerData, -) # NOTE: this module should not do any data validation; that should be done in the urls.py or higher layer async def current_officers( db_session: database.DBSession, - include_private: bool -) -> dict[str, list[OfficerData]]: +) -> list[OfficerInfoResponse]: """ Get info about officers that are active. Go through all active & complete officer terms. Returns a mapping between officer position and officer terms """ - query = ( - sqlalchemy - .select(OfficerTerm) - .order_by(OfficerTerm.start_date.desc()) - ) - query = utils.is_active_officer(query) + curr_time = date.today() + query = (sqlalchemy.select(OfficerTerm, OfficerInfo) + .join(OfficerInfo, OfficerTerm.computing_id == OfficerInfo.computing_id) + .where((OfficerTerm.start_date <= curr_time) & (OfficerTerm.end_date >= curr_time)) + .order_by(OfficerTerm.start_date.desc()) + ) - officer_terms = (await db_session.scalars(query)).all() - officer_data = {} - for term in officer_terms: - officer_info_query = ( - sqlalchemy - .select(OfficerInfo) - .where(OfficerInfo.computing_id == term.computing_id) - ) - officer_info = (await db_session.scalars(officer_info_query)).first() - if officer_info is None: - # TODO (#93): make sure there are daily checks that this data actually exists - continue - elif term.position not in officer_data: - officer_data[term.position] = [] + result: Sequence[sqlalchemy.Row[tuple[OfficerTerm, OfficerInfo]]] = (await db_session.execute(query)).all() + officer_list = [] + for term, officer in result: + officer_list.append(OfficerInfoResponse( + legal_name = officer.legal_name, + is_active = True, + position = term.position, + start_date = term.start_date, + end_date = term.end_date, + biography = term.biography, + csss_email = OfficerPosition.to_email(term.position), - officer_data[term.position] += [ - OfficerData.from_data(term, officer_info, include_private, is_active=True) - ] + discord_id = officer.discord_id, + discord_name = officer.discord_name, + discord_nickname = officer.discord_nickname, + computing_id = officer.computing_id, + phone_number = officer.phone_number, + github_username = officer.github_username, + google_drive_email = officer.google_drive_email, + photo_url = term.photo_url + )) - return officer_data + return officer_list async def all_officers( db_session: AsyncSession, diff --git a/src/officers/models.py b/src/officers/models.py index 3381ff0..1559216 100644 --- a/src/officers/models.py +++ b/src/officers/models.py @@ -4,6 +4,16 @@ from officers.constants import OFFICER_LEGAL_NAME_MAX, OfficerPositionEnum +OFFICER_PRIVATE_INFO = { + "discord_id", + "discord_name", + "discord_nickname", + "computing_id", + "phone_number", + "github_username", + "google_drive_email", + "photo_url" +} class OfficerInfoBaseModel(BaseModel): # TODO (#71): compute this using SFU's API & remove from being uploaded diff --git a/src/officers/tables.py b/src/officers/tables.py index 071fc3b..8394bde 100644 --- a/src/officers/tables.py +++ b/src/officers/tables.py @@ -33,7 +33,7 @@ class OfficerTerm(Base): computing_id: Mapped[str] = mapped_column( String(COMPUTING_ID_LEN), - ForeignKey("officer_info.computing_id"), + ForeignKey("site_user.computing_id"), nullable=False, ) diff --git a/src/officers/urls.py b/src/officers/urls.py index 522ec44..8bb7950 100755 --- a/src/officers/urls.py +++ b/src/officers/urls.py @@ -6,6 +6,7 @@ import officers.crud import utils from officers.models import ( + OFFICER_PRIVATE_INFO, OfficerInfoResponse, OfficerSelfUpdate, OfficerTermCreate, @@ -56,14 +57,14 @@ async def current_officers( db_session: database.DBSession, ): has_private_access, _ = await _has_officer_private_info_access(request, db_session) - current_officers = await officers.crud.current_officers(db_session, has_private_access) - return JSONResponse({ - position: [ - officer_data.serializable_dict() - for officer_data in officer_data_list - ] - for position, officer_data_list in current_officers.items() - }) + + curr_officers = await officers.crud.current_officers(db_session) + exclude = OFFICER_PRIVATE_INFO if not has_private_access else {} + + return JSONResponse(content=[ + officer_data.model_dump(exclude=exclude, mode="json") + for officer_data in curr_officers + ]) @router.get( "/all", @@ -88,16 +89,7 @@ async def all_officers( raise HTTPException(status_code=401, detail="only website admins can view all executive terms that have not started yet") all_officers = await officers.crud.all_officers(db_session, include_future_terms) - exclude = { - "discord_id", - "discord_name", - "discord_nickname", - "computing_id", - "phone_number", - "github_username", - "google_drive_email", - "photo_url" - } if not has_private_access else {} + exclude = OFFICER_PRIVATE_INFO if not has_private_access else {} return JSONResponse(content=[ officer_data.model_dump(exclude=exclude, mode="json") From c9673d2d1a420463bddcfe0ec4a52efeec1ebab1 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Tue, 30 Sep 2025 08:17:23 -0700 Subject: [PATCH 21/32] test: Suppress SQLAlchemy logs and marked tests WIP - Discord, GitHub, and Google tests marked WIP - Suppressed SQLAlchemy logs when tests are running - Added a global fixture to set up the database (wip) --- pyproject.toml | 3 ++- src/conftest.py | 11 +++++++++++ tests/{integration => wip}/test_discord.py | 0 tests/{integration => wip}/test_github.py | 0 tests/{integration => wip}/test_google.py | 0 5 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 src/conftest.py rename tests/{integration => wip}/test_discord.py (100%) rename tests/{integration => wip}/test_github.py (100%) rename tests/{integration => wip}/test_google.py (100%) diff --git a/pyproject.toml b/pyproject.toml index 79e80cf..bc02a5d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,7 +12,8 @@ log_cli = true log_cli_level = "INFO" testpaths = [ "tests", -] + ] +norecursedirs = "tests/wip" [tool.ruff] line-length = 120 diff --git a/src/conftest.py b/src/conftest.py new file mode 100644 index 0000000..3528464 --- /dev/null +++ b/src/conftest.py @@ -0,0 +1,11 @@ +# Configuration of Pytest + +import logging + +import pytest + + +@pytest.fixture(scope="session", autouse=True) +def suppress_sqlalchemy_logs(): + # Suppress SQLAlchemy logs while testing + logging.getLogger("sqlalchemy.engine").setLevel(logging.WARNING) diff --git a/tests/integration/test_discord.py b/tests/wip/test_discord.py similarity index 100% rename from tests/integration/test_discord.py rename to tests/wip/test_discord.py diff --git a/tests/integration/test_github.py b/tests/wip/test_github.py similarity index 100% rename from tests/integration/test_github.py rename to tests/wip/test_github.py diff --git a/tests/integration/test_google.py b/tests/wip/test_google.py similarity index 100% rename from tests/integration/test_google.py rename to tests/wip/test_google.py From f3ac5ac05acc699175a051f78a96e1b8a3214346 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Tue, 30 Sep 2025 08:27:03 -0700 Subject: [PATCH 22/32] fix: suppress asyncio warning --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index bc02a5d..d2d3a39 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,6 +14,7 @@ testpaths = [ "tests", ] norecursedirs = "tests/wip" +asyncio_mode = "auto" [tool.ruff] line-length = 120 From f39c994fb1aac47dfe6a588003b1514ff61b6055 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Tue, 30 Sep 2025 15:25:44 -0700 Subject: [PATCH 23/32] wip: refactored the unit test fixtures to the configuration file --- pyproject.toml | 1 + src/conftest.py | 11 --- src/cron/daily.py | 4 +- src/database.py | 7 +- src/officers/tables.py | 4 +- src/officers/urls.py | 18 +++-- tests/conftest.py | 50 ++++++++++++ tests/integration/test_officers.py | 122 ++++++++++++----------------- tests/{ => wip}/unit/test_dates.py | 0 9 files changed, 116 insertions(+), 101 deletions(-) delete mode 100644 src/conftest.py create mode 100644 tests/conftest.py rename tests/{ => wip}/unit/test_dates.py (100%) diff --git a/pyproject.toml b/pyproject.toml index d2d3a39..ddd3fd5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,6 +15,7 @@ testpaths = [ ] norecursedirs = "tests/wip" asyncio_mode = "auto" +asyncio_default_fixture_loop_scope = "function" [tool.ruff] line-length = 120 diff --git a/src/conftest.py b/src/conftest.py deleted file mode 100644 index 3528464..0000000 --- a/src/conftest.py +++ /dev/null @@ -1,11 +0,0 @@ -# Configuration of Pytest - -import logging - -import pytest - - -@pytest.fixture(scope="session", autouse=True) -def suppress_sqlalchemy_logs(): - # Suppress SQLAlchemy logs while testing - logging.getLogger("sqlalchemy.engine").setLevel(logging.WARNING) diff --git a/src/cron/daily.py b/src/cron/daily.py index 00615d1..37e949b 100644 --- a/src/cron/daily.py +++ b/src/cron/daily.py @@ -6,7 +6,7 @@ import github import google_api import utils -from database import _db_session +from database import get_db_session from officers.crud import all_officers, get_user_by_username _logger = logging.getLogger(__name__) @@ -55,7 +55,7 @@ async def update_github_permissions(db_session): _logger.info("updated github permissions") async def update_permissions(): - db_session = _db_session() + db_session = get_db_session() update_google_permissions(db_session) db_session.commit() diff --git a/src/database.py b/src/database.py index 2e872bd..470c7ca 100644 --- a/src/database.py +++ b/src/database.py @@ -102,7 +102,7 @@ def setup_database(): # TODO: where is sys.stdout piped to? I want all these to go to a specific logs folder sessionmanager = DatabaseSessionManager( SQLALCHEMY_TEST_DATABASE_URL if os.environ.get("LOCAL") else SQLALCHEMY_DATABASE_URL, - { "echo": True }, + { "echo": False }, ) @contextlib.asynccontextmanager @@ -116,10 +116,9 @@ async def lifespan(app: FastAPI): await sessionmanager.close() -async def _db_session(): +async def get_db_session(): async with sessionmanager.session() as session: yield session -# TODO: what does this do again? -DBSession = Annotated[AsyncSession, Depends(_db_session)] +DBSession = Annotated[AsyncSession, Depends(get_db_session)] diff --git a/src/officers/tables.py b/src/officers/tables.py index 8394bde..07a1d80 100644 --- a/src/officers/tables.py +++ b/src/officers/tables.py @@ -77,7 +77,7 @@ def update_from_params(self, params: OfficerTermUpdate, self_update: bool = True else: update_data = params.model_dump(exclude_unset=True) for k, v in update_data.items(): - setattr(update_data, k, v) + setattr(self, k, v) def is_filled_in(self): @@ -162,7 +162,7 @@ def serializable_dict(self) -> dict: def update_from_params(self, params: OfficerUpdate | OfficerSelfUpdate): update_data = params.model_dump(exclude_unset=True) for k, v in update_data.items(): - setattr(update_data, k, v) + setattr(self, k, v) def is_filled_in(self): return ( diff --git a/src/officers/urls.py b/src/officers/urls.py index 8bb7950..79fc5a3 100755 --- a/src/officers/urls.py +++ b/src/officers/urls.py @@ -5,6 +5,7 @@ import database import officers.crud import utils +from officers.constants import OfficerPositionEnum from officers.models import ( OFFICER_PRIVATE_INFO, OfficerInfoResponse, @@ -49,7 +50,7 @@ async def _has_officer_private_info_access( @router.get( "/current", description="Get information about the current officers. With no authorization, only get basic info.", - response_model=list[OfficerInfoResponse], + response_model=dict[OfficerPositionEnum, OfficerInfoResponse], operation_id="get_current_officers" ) async def current_officers( @@ -61,10 +62,11 @@ async def current_officers( curr_officers = await officers.crud.current_officers(db_session) exclude = OFFICER_PRIVATE_INFO if not has_private_access else {} - return JSONResponse(content=[ - officer_data.model_dump(exclude=exclude, mode="json") - for officer_data in curr_officers - ]) + res = {} + for officer in curr_officers: + res[officer.position] = officer.model_dump(exclude=exclude, mode="json") + + return JSONResponse(res) @router.get( "/all", @@ -126,7 +128,7 @@ async def get_officer_terms( include_future_terms ) return JSONResponse([ - OfficerTermResponse.model_validate(term) for term in officer_terms + term.serializable_dict() for term in officer_terms ]) @router.get( @@ -232,7 +234,7 @@ async def update_info( await db_session.commit() updated_officer_info = await officers.crud.get_new_officer_info_or_raise(db_session, computing_id) - return JSONResponse(updated_officer_info) + return JSONResponse(updated_officer_info.serializable_dict()) @router.patch( "/term/{term_id:int}", @@ -271,7 +273,7 @@ async def update_term( await db_session.commit() new_officer_term = await officers.crud.get_officer_term_by_id_or_raise(db_session, term_id) - return JSONResponse(new_officer_term) + return JSONResponse(new_officer_term.serializable_dict()) @router.delete( "/term/{term_id:int}", diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..41f780f --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,50 @@ +# Configuration of Pytest +import logging +from collections.abc import AsyncGenerator +from typing import Any + +import pytest +import pytest_asyncio +from httpx import ASGITransport, AsyncClient + +from src import load_test_db +from src.auth.crud import create_user_session +from src.database import SQLALCHEMY_DATABASE_URL, SQLALCHEMY_TEST_DATABASE_URL, DatabaseSessionManager +from src.main import app + + +# This might be able to be moved to `package` scope as long as I inject it to every test function +@pytest.fixture(scope="session") +def suppress_sqlalchemy_logs(): + logging.getLogger("sqlalchemy.engine").setLevel(logging.WARNING) + yield + logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) + + +@pytest.fixture(scope="function", autouse=True) +async def client() -> AsyncGenerator[Any, None]: + # base_url is just a random placeholder url + # ASGITransport is just telling the async client to pass all requests to app + # `async with` syntax used so that the connecton will automatically be closed once done + async with AsyncClient(transport=ASGITransport(app), base_url="http://test") as client: + yield client + +# Will need to change this to session scope if performance becomes an issue for tests +@pytest_asyncio.fixture(scope="function") +async def database_setup(): + # reset the database again, just in case + print("Resetting DB...") + sessionmanager = DatabaseSessionManager(SQLALCHEMY_TEST_DATABASE_URL, {"echo": False}, check_db=False) + # this resets the contents of the database to be whatever is from `load_test_db.py` + await load_test_db.async_main(sessionmanager) + print("Done setting up!") + yield sessionmanager + await sessionmanager.close() + +@pytest_asyncio.fixture(scope="function") +async def admin_session(database_setup): + session_id = "temp_id_" + load_test_db.SYSADMIN_COMPUTING_ID + async with database_setup.session() as db_session: + await create_user_session(db_session, session_id, load_test_db.SYSADMIN_COMPUTING_ID) + + diff --git a/tests/integration/test_officers.py b/tests/integration/test_officers.py index 172fe68..410d7e9 100644 --- a/tests/integration/test_officers.py +++ b/tests/integration/test_officers.py @@ -3,48 +3,20 @@ from datetime import date, timedelta import pytest -from httpx import ASGITransport, AsyncClient from src import load_test_db -from src.auth.crud import create_user_session -from src.database import SQLALCHEMY_TEST_DATABASE_URL, DatabaseSessionManager -from src.main import app -from src.officers.constants import OfficerPosition, OfficerPositionEnum +from src.officers.constants import OfficerPositionEnum from src.officers.crud import all_officers, current_officers, get_active_officer_terms # TODO: setup a database on the CI machine & run this as a unit test then (since # this isn't really an integration test) -@pytest.fixture(scope="session") -def anyio_backend(): - return "asyncio" - -@pytest.fixture(scope="session") -async def client(): - # base_url is just a random placeholder url - # ASGITransport is just telling the async client to pass all requests to app - async with AsyncClient(transport=ASGITransport(app), base_url="http://test") as client: - yield client - -# run this again for every function -@pytest.fixture(scope="function") -async def database_setup(): - # reset the database again, just in case - print("Resetting DB...") - sessionmanager = DatabaseSessionManager(SQLALCHEMY_TEST_DATABASE_URL, {"echo": False}, check_db=False) - await DatabaseSessionManager.test_connection(SQLALCHEMY_TEST_DATABASE_URL) - # this resets the contents of the database to be whatever is from `load_test_db.py` - await load_test_db.async_main(sessionmanager) - print("Done setting up!") - - return sessionmanager - # TODO: switch to mark.anyio @pytest.mark.asyncio async def test__read_execs(database_setup): - sessionmanager = await database_setup - async with sessionmanager.session() as db_session: + async with database_setup.session() as db_session: # test that reads from the database succeeded as expected + print(type(db_session)) assert (await get_active_officer_terms(db_session, "blarg")) == [] assert (await get_active_officer_terms(db_session, "abc22")) != [] @@ -57,42 +29,38 @@ async def test__read_execs(database_setup): assert abc11_officer_terms[0].favourite_course_0 == "CMPT 361" assert abc11_officer_terms[0].biography == "Hi! I'm person A and I want school to be over ; _ ;" - current_exec_team = await current_officers(db_session, include_private=False) + current_exec_team = await current_officers(db_session) assert current_exec_team is not None - assert len(current_exec_team.keys()) == 5 - assert next(iter(current_exec_team.keys())) == OfficerPositionEnum.EXECUTIVE_AT_LARGE - assert next(iter(current_exec_team.values()))[0].favourite_course_0 == "CMPT 361" - assert next(iter(current_exec_team.values()))[0].csss_email == OfficerPosition.to_email(OfficerPositionEnum.EXECUTIVE_AT_LARGE) - assert next(iter(current_exec_team.values()))[0].private_data is None + assert len(current_exec_team) == 3 + # assert next(iter(current_exec_team)) == OfficerPositionEnum.EXECUTIVE_AT_LARGE + # assert next(iter(current_exec_team))["favourite_course_0"] == "CMPT 361" + # assert next(iter(current_exec_team.values()))[0].csss_email == OfficerPosition.to_email(OfficerPositionEnum.EXECUTIVE_AT_LARGE) + # assert next(iter(current_exec_team.values()))[0].private_data is None - current_exec_team = await current_officers(db_session, include_private=True) + current_exec_team = await current_officers(db_session) assert current_exec_team is not None - assert len(current_exec_team) == 5 - assert next(iter(current_exec_team.keys())) == OfficerPositionEnum.EXECUTIVE_AT_LARGE - assert next(iter(current_exec_team.values()))[0].favourite_course_0 == "CMPT 361" - assert next(iter(current_exec_team.values()))[0].csss_email == OfficerPosition.to_email(OfficerPositionEnum.EXECUTIVE_AT_LARGE) - assert next(iter(current_exec_team.values()))[0].private_data is not None - assert next(iter(current_exec_team.values()))[0].private_data.computing_id == "abc11" - - all_terms = await all_officers(db_session, include_private_data=True, include_future_terms=False) - assert len(all_terms) == 6 + assert len(current_exec_team) == 3 + # assert next(iter(current_exec_team.keys())) == OfficerPositionEnum.EXECUTIVE_AT_LARGE + # assert next(iter(current_exec_team.values()))[0].favourite_course_0 == "CMPT 361" + # assert next(iter(current_exec_team.values()))[0].csss_email == OfficerPosition.to_email(OfficerPositionEnum.EXECUTIVE_AT_LARGE) + # assert next(iter(current_exec_team.values()))[0].private_data is not None + # assert next(iter(current_exec_team.values()))[0].private_data.computing_id == "abc11" + + all_terms = await all_officers(db_session, include_future_terms=False) + assert len(all_terms) == 8 - all_terms = await all_officers(db_session, include_private_data=True, include_future_terms=True) - assert len(all_terms) == 7 #async def test__update_execs(database_setup): # # TODO: the second time an update_officer_info call occurs, the user should be updated with info # pass -@pytest.mark.anyio -async def test__endpoints(client, database_setup): - # `database_setup` resets & loads the test database - +@pytest.mark.asyncio +async def test__endpoints(client): response = await client.get("/officers/current") assert response.status_code == 200 assert response.json() != {} - assert len(response.json().values()) == 5 - assert not response.json()["executive at large"][0]["private_data"] + assert len(response.json().values()) == 3 + assert not response.json()["executive at large"]["computing_id"] response = await client.get("/officers/all?include_future_terms=false") assert response.status_code == 200 @@ -164,27 +132,23 @@ async def test__endpoints(client, database_setup): response = await client.delete("officers/term/1") assert response.status_code == 401 -@pytest.mark.anyio -async def test__endpoints_admin(client, database_setup): +@pytest.mark.asyncio +async def test__endpoints_admin(client, database_setup, admin_session): # login as website admin session_id = "temp_id_" + load_test_db.SYSADMIN_COMPUTING_ID - async with database_setup.session() as db_session: - await create_user_session(db_session, session_id, load_test_db.SYSADMIN_COMPUTING_ID) client.cookies = { "session_id": session_id } # test that more info is given if logged in & with access to it response = await client.get("/officers/current") assert response.status_code == 200 - assert response.json() != {} - assert len(response.json().values()) == 5 - assert response.json()["executive at large"][0]["private_data"] + curr_officers = response.json() + assert len(curr_officers) == 3 + assert curr_officers["executive at large"]["computing_id"] is not None response = await client.get("/officers/all?include_future_terms=true") assert response.status_code == 200 - assert response.json() != [] assert len(response.json()) == 9 - print(response.json()[1]) assert response.json()[1]["phone_number"] == "1234567890" response = await client.get(f"/officers/terms/{load_test_db.SYSADMIN_COMPUTING_ID}?include_future_terms=false") @@ -214,7 +178,8 @@ async def test__endpoints_admin(client, database_setup): response = await client.post("officers/term", content=json.dumps([{ "computing_id": "ehbc12", "position": OfficerPositionEnum.DIRECTOR_OF_MULTIMEDIA, - "start_date": "2025-12-29" + "start_date": "2025-12-29", + "legal_name": "Eh Bc" }])) assert response.status_code == 200 @@ -231,8 +196,12 @@ async def test__endpoints_admin(client, database_setup): "google_drive_email": "person_a@gmail.com", })) assert response.status_code == 200 - assert response.json()["officer_info"] != {} - assert len(response.json()["validation_failures"]) == 3 + resJson = response.json() + assert resJson["legal_name"] == "Person A2" + assert resJson["phone_number"] == "12345asdab67890" + assert resJson["discord_name"] == "person_a_yeah" + assert resJson["github_username"] == "person_a" + assert resJson["google_drive_email"] == "person_a@gmail.com" response = await client.patch("officers/info/aaabbbc", content=json.dumps({ "legal_name": "Person AABBCC", @@ -244,7 +213,6 @@ async def test__endpoints_admin(client, database_setup): assert response.status_code == 404 response = await client.patch("officers/term/1", content=json.dumps({ - "computing_id": "abc11", "position": OfficerPositionEnum.TREASURER, "start_date": (date.today() - timedelta(days=365)).isoformat(), "end_date": (date.today() - timedelta(days=1)).isoformat(), @@ -259,14 +227,20 @@ async def test__endpoints_admin(client, database_setup): response = await client.get("/officers/terms/abc11?include_future_terms=true") assert response.status_code == 200 - assert response.json() != [] - assert response.json()[1]["position"] == OfficerPositionEnum.TREASURER - assert response.json()[0]["favourite_course_0"] != "2" - assert response.json()[1]["biography"] == "hello o77" + resJson = response.json() + assert resJson[1]["position"] == OfficerPositionEnum.TREASURER + assert resJson[1]["start_date"] == (date.today() - timedelta(days=365)).isoformat() + assert resJson[1]["end_date"] == (date.today() - timedelta(days=1)).isoformat() + assert resJson[1]["nickname"] != "1" + assert resJson[1]["favourite_course_0"] != "2" + assert resJson[1]["favourite_course_1"] != "3" + assert resJson[1]["favourite_pl_0"] != "4" + assert resJson[1]["favourite_pl_1"] != "5" + assert resJson[1]["biography"] == "hello o77" async with database_setup.session() as db_session: - all_terms = await all_officers(db_session, include_private_data=True, include_future_terms=True) - assert len(all_terms) == 8 + all_terms = await all_officers(db_session, include_future_terms=True) + assert len(all_terms) == 10 response = await client.delete("officers/term/1") assert response.status_code == 200 diff --git a/tests/unit/test_dates.py b/tests/wip/unit/test_dates.py similarity index 100% rename from tests/unit/test_dates.py rename to tests/wip/unit/test_dates.py From f5c87965aa9d7b366f57ccd2dfcb54955f12f3a9 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Tue, 30 Sep 2025 16:20:06 -0700 Subject: [PATCH 24/32] fix: change database set up to once per module --- tests/conftest.py | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 41f780f..eb69b41 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,5 @@ # Configuration of Pytest +import asyncio import logging from collections.abc import AsyncGenerator from typing import Any @@ -9,7 +10,7 @@ from src import load_test_db from src.auth.crud import create_user_session -from src.database import SQLALCHEMY_DATABASE_URL, SQLALCHEMY_TEST_DATABASE_URL, DatabaseSessionManager +from src.database import SQLALCHEMY_TEST_DATABASE_URL, DatabaseSessionManager from src.main import app @@ -20,8 +21,14 @@ def suppress_sqlalchemy_logs(): yield logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) +@pytest.fixture(scope="session") +def event_loop(request): + loop = asyncio.get_event_loop_policy().new_event_loop() + yield loop + loop.close() + -@pytest.fixture(scope="function", autouse=True) +@pytest.fixture(scope="function") async def client() -> AsyncGenerator[Any, None]: # base_url is just a random placeholder url # ASGITransport is just telling the async client to pass all requests to app @@ -29,8 +36,7 @@ async def client() -> AsyncGenerator[Any, None]: async with AsyncClient(transport=ASGITransport(app), base_url="http://test") as client: yield client -# Will need to change this to session scope if performance becomes an issue for tests -@pytest_asyncio.fixture(scope="function") +@pytest_asyncio.fixture(scope="module") async def database_setup(): # reset the database again, just in case print("Resetting DB...") @@ -41,10 +47,24 @@ async def database_setup(): yield sessionmanager await sessionmanager.close() +@pytest_asyncio.fixture(scope="function") +async def db_transaction(database_setup): + async with database_setup.session() as session: + try: + await session.begin() + yield session + finally: + await session.rollback() + +@pytest_asyncio.fixture(scope="function") +async def db_session(db_transaction): + yield db_transaction + @pytest_asyncio.fixture(scope="function") async def admin_session(database_setup): session_id = "temp_id_" + load_test_db.SYSADMIN_COMPUTING_ID async with database_setup.session() as db_session: await create_user_session(db_session, session_id, load_test_db.SYSADMIN_COMPUTING_ID) + yield From c93ea35943876f8308908f02e8c72a990a299d00 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Tue, 30 Sep 2025 17:08:51 -0700 Subject: [PATCH 25/32] fix: unit tests working with only one set up --- tests/conftest.py | 50 ++++------ tests/integration/test_officers.py | 152 ++++++++++++++++++----------- 2 files changed, 114 insertions(+), 88 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index eb69b41..df141be 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,7 +9,7 @@ from httpx import ASGITransport, AsyncClient from src import load_test_db -from src.auth.crud import create_user_session +from src.auth.crud import create_user_session, remove_user_session from src.database import SQLALCHEMY_TEST_DATABASE_URL, DatabaseSessionManager from src.main import app @@ -21,22 +21,7 @@ def suppress_sqlalchemy_logs(): yield logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) -@pytest.fixture(scope="session") -def event_loop(request): - loop = asyncio.get_event_loop_policy().new_event_loop() - yield loop - loop.close() - - -@pytest.fixture(scope="function") -async def client() -> AsyncGenerator[Any, None]: - # base_url is just a random placeholder url - # ASGITransport is just telling the async client to pass all requests to app - # `async with` syntax used so that the connecton will automatically be closed once done - async with AsyncClient(transport=ASGITransport(app), base_url="http://test") as client: - yield client - -@pytest_asyncio.fixture(scope="module") +@pytest_asyncio.fixture(scope="session", loop_scope="session") async def database_setup(): # reset the database again, just in case print("Resetting DB...") @@ -47,24 +32,25 @@ async def database_setup(): yield sessionmanager await sessionmanager.close() -@pytest_asyncio.fixture(scope="function") -async def db_transaction(database_setup): - async with database_setup.session() as session: - try: - await session.begin() - yield session - finally: - await session.rollback() +@pytest_asyncio.fixture(scope="function", loop_scope="session") +async def client() -> AsyncGenerator[Any, None]: + # base_url is just a random placeholder url + # ASGITransport is just telling the async client to pass all requests to app + # `async with` syntax used so that the connecton will automatically be closed once done + async with AsyncClient(transport=ASGITransport(app), base_url="http://test") as client: + yield client -@pytest_asyncio.fixture(scope="function") -async def db_session(db_transaction): - yield db_transaction +@pytest_asyncio.fixture(scope="function", loop_scope="session") +async def db_session(database_setup): + async with database_setup.session() as session: + yield session -@pytest_asyncio.fixture(scope="function") +@pytest_asyncio.fixture(scope="function", loop_scope="session") async def admin_session(database_setup): session_id = "temp_id_" + load_test_db.SYSADMIN_COMPUTING_ID - async with database_setup.session() as db_session: - await create_user_session(db_session, session_id, load_test_db.SYSADMIN_COMPUTING_ID) - yield + async with database_setup.session() as session: + await create_user_session(session, session_id, load_test_db.SYSADMIN_COMPUTING_ID) + yield + await remove_user_session(session, session_id) diff --git a/tests/integration/test_officers.py b/tests/integration/test_officers.py index 410d7e9..bdba8f0 100644 --- a/tests/integration/test_officers.py +++ b/tests/integration/test_officers.py @@ -3,6 +3,7 @@ from datetime import date, timedelta import pytest +from httpx import AsyncClient from src import load_test_db from src.officers.constants import OfficerPositionEnum @@ -11,76 +12,108 @@ # TODO: setup a database on the CI machine & run this as a unit test then (since # this isn't really an integration test) -# TODO: switch to mark.anyio -@pytest.mark.asyncio -async def test__read_execs(database_setup): - async with database_setup.session() as db_session: - # test that reads from the database succeeded as expected - print(type(db_session)) - assert (await get_active_officer_terms(db_session, "blarg")) == [] - assert (await get_active_officer_terms(db_session, "abc22")) != [] - - abc11_officer_terms = await get_active_officer_terms(db_session, "abc11") - assert len(abc11_officer_terms) == 1 - assert abc11_officer_terms[0].computing_id == "abc11" - assert abc11_officer_terms[0].position == OfficerPositionEnum.EXECUTIVE_AT_LARGE - assert abc11_officer_terms[0].start_date is not None - assert abc11_officer_terms[0].nickname == "the holy A" - assert abc11_officer_terms[0].favourite_course_0 == "CMPT 361" - assert abc11_officer_terms[0].biography == "Hi! I'm person A and I want school to be over ; _ ;" - - current_exec_team = await current_officers(db_session) - assert current_exec_team is not None - assert len(current_exec_team) == 3 - # assert next(iter(current_exec_team)) == OfficerPositionEnum.EXECUTIVE_AT_LARGE - # assert next(iter(current_exec_team))["favourite_course_0"] == "CMPT 361" - # assert next(iter(current_exec_team.values()))[0].csss_email == OfficerPosition.to_email(OfficerPositionEnum.EXECUTIVE_AT_LARGE) - # assert next(iter(current_exec_team.values()))[0].private_data is None - - current_exec_team = await current_officers(db_session) - assert current_exec_team is not None - assert len(current_exec_team) == 3 - # assert next(iter(current_exec_team.keys())) == OfficerPositionEnum.EXECUTIVE_AT_LARGE - # assert next(iter(current_exec_team.values()))[0].favourite_course_0 == "CMPT 361" - # assert next(iter(current_exec_team.values()))[0].csss_email == OfficerPosition.to_email(OfficerPositionEnum.EXECUTIVE_AT_LARGE) - # assert next(iter(current_exec_team.values()))[0].private_data is not None - # assert next(iter(current_exec_team.values()))[0].private_data.computing_id == "abc11" - - all_terms = await all_officers(db_session, include_future_terms=False) - assert len(all_terms) == 8 +pytestmark = pytest.mark.asyncio(loop_scope="session") + +async def test__read_execs(db_session): + # test that reads from the database succeeded as expected + print(type(db_session)) + assert (await get_active_officer_terms(db_session, "blarg")) == [] + assert (await get_active_officer_terms(db_session, "abc22")) != [] + + abc11_officer_terms = await get_active_officer_terms(db_session, "abc11") + assert len(abc11_officer_terms) == 1 + assert abc11_officer_terms[0].computing_id == "abc11" + assert abc11_officer_terms[0].position == OfficerPositionEnum.EXECUTIVE_AT_LARGE + assert abc11_officer_terms[0].start_date is not None + assert abc11_officer_terms[0].nickname == "the holy A" + assert abc11_officer_terms[0].favourite_course_0 == "CMPT 361" + assert abc11_officer_terms[0].biography == "Hi! I'm person A and I want school to be over ; _ ;" + + current_exec_team = await current_officers(db_session) + assert current_exec_team is not None + assert len(current_exec_team) == 3 + # assert next(iter(current_exec_team)) == OfficerPositionEnum.EXECUTIVE_AT_LARGE + # assert next(iter(current_exec_team))["favourite_course_0"] == "CMPT 361" + # assert next(iter(current_exec_team.values()))[0].csss_email == OfficerPosition.to_email(OfficerPositionEnum.EXECUTIVE_AT_LARGE) + # assert next(iter(current_exec_team.values()))[0].private_data is None + + current_exec_team = await current_officers(db_session) + assert current_exec_team is not None + assert len(current_exec_team) == 3 + # assert next(iter(current_exec_team.keys())) == OfficerPositionEnum.EXECUTIVE_AT_LARGE + # assert next(iter(current_exec_team.values()))[0].favourite_course_0 == "CMPT 361" + # assert next(iter(current_exec_team.values()))[0].csss_email == OfficerPosition.to_email(OfficerPositionEnum.EXECUTIVE_AT_LARGE) + # assert next(iter(current_exec_team.values()))[0].private_data is not None + # assert next(iter(current_exec_team.values()))[0].private_data.computing_id == "abc11" + + all_terms = await all_officers(db_session, include_future_terms=False) + assert len(all_terms) == 8 #async def test__update_execs(database_setup): # # TODO: the second time an update_officer_info call occurs, the user should be updated with info # pass -@pytest.mark.asyncio -async def test__endpoints(client): +async def test__get_officers(client): + # private data shoudn't be leaked + print(f"[DEBUG] Loop ID in {__name__}: {id(asyncio.get_running_loop())}") response = await client.get("/officers/current") assert response.status_code == 200 assert response.json() != {} assert len(response.json().values()) == 3 - assert not response.json()["executive at large"]["computing_id"] + assert "computing_id" not in response.json()[OfficerPositionEnum.EXECUTIVE_AT_LARGE] + assert "discord_id" not in response.json()[OfficerPositionEnum.EXECUTIVE_AT_LARGE] + assert "discord_name" not in response.json()[OfficerPositionEnum.EXECUTIVE_AT_LARGE] + assert "discord_nickname" not in response.json()[OfficerPositionEnum.EXECUTIVE_AT_LARGE] + assert "phone_number" not in response.json()[OfficerPositionEnum.EXECUTIVE_AT_LARGE] + assert "github_username" not in response.json()[OfficerPositionEnum.EXECUTIVE_AT_LARGE] + assert "google_drive_email" not in response.json()[OfficerPositionEnum.EXECUTIVE_AT_LARGE] + assert "photo_url" not in response.json()[OfficerPositionEnum.EXECUTIVE_AT_LARGE] + + assert "computing_id" not in response.json()[OfficerPositionEnum.DIRECTOR_OF_ARCHIVES] + assert "discord_id" not in response.json()[OfficerPositionEnum.DIRECTOR_OF_ARCHIVES] + assert "discord_name" not in response.json()[OfficerPositionEnum.DIRECTOR_OF_ARCHIVES] + assert "discord_nickname" not in response.json()[OfficerPositionEnum.DIRECTOR_OF_ARCHIVES] + assert "phone_number" not in response.json()[OfficerPositionEnum.DIRECTOR_OF_ARCHIVES] + assert "github_username" not in response.json()[OfficerPositionEnum.DIRECTOR_OF_ARCHIVES] + assert "google_drive_email" not in response.json()[OfficerPositionEnum.DIRECTOR_OF_ARCHIVES] + assert "photo_url" not in response.json()[OfficerPositionEnum.DIRECTOR_OF_ARCHIVES] + + assert "computing_id" not in response.json()[OfficerPositionEnum.PRESIDENT] + assert "discord_id" not in response.json()[OfficerPositionEnum.PRESIDENT] + assert "discord_name" not in response.json()[OfficerPositionEnum.PRESIDENT] + assert "discord_nickname" not in response.json()[OfficerPositionEnum.PRESIDENT] + assert "phone_number" not in response.json()[OfficerPositionEnum.PRESIDENT] + assert "github_username" not in response.json()[OfficerPositionEnum.PRESIDENT] + assert "google_drive_email" not in response.json()[OfficerPositionEnum.PRESIDENT] + assert "photo_url" not in response.json()[OfficerPositionEnum.PRESIDENT] response = await client.get("/officers/all?include_future_terms=false") assert response.status_code == 200 assert response.json() != [] - assert len(response.json()) == 6 - assert response.json()[0]["private_data"] is None + assert len(response.json()) == 8 + assert "computing_id" not in response.json()[0] + assert "discord_id" not in response.json()[0] + assert "discord_name" not in response.json()[0] + assert "discord_nickname" not in response.json()[0] + assert "phone_number" not in response.json()[0] + assert "github_username" not in response.json()[0] + assert "google_drive_email" not in response.json()[0] + assert "photo_url" not in response.json()[0] response = await client.get("/officers/all?include_future_terms=true") assert response.status_code == 401 +async def test__get_officer_terms(client: AsyncClient): response = await client.get(f"/officers/terms/{load_test_db.SYSADMIN_COMPUTING_ID}?include_future_terms=false") assert response.status_code == 200 - assert response.json() != [] assert len(response.json()) == 2 assert response.json()[0]["nickname"] == "G2" assert response.json()[1]["nickname"] == "G1" response = await client.get("/officers/terms/balargho?include_future_terms=false") assert response.status_code == 200 - assert response.json() == [] + assert len(response.json()) == 0 response = await client.get("/officers/terms/abc11?include_future_terms=true") assert response.status_code == 401 @@ -90,28 +123,35 @@ async def test__endpoints(client): response = await client.get(f"/officers/info/{load_test_db.SYSADMIN_COMPUTING_ID}") assert response.status_code == 401 - response = await client.post("officers/term", content=json.dumps([{ +async def test__post_officer_terms(client: AsyncClient): + # Only admins can create new terms + response = await client.post("officers/term", json=[{ "computing_id": "ehbc12", "position": OfficerPositionEnum.DIRECTOR_OF_MULTIMEDIA, - "start_date": "2025-12-29" - }])) + "start_date": "2025-12-29", + "legal_name": "Eh Bc" + }]) assert response.status_code == 401 - response = await client.post("officers/term", content=json.dumps([{ + # Position must be one of the enum positions + response = await client.post("officers/term", json=[{ "computing_id": "ehbc12", "position": "balargho", - "start_date": "2025-12-29" - }])) - assert response.status_code == 400 + "start_date": "2025-12-29", + "legal_name": "Eh Bc" + }]) + assert response.status_code == 422 - response = await client.patch("officers/info/abc11", content=json.dumps({ +async def test__patch_officer_terms(client: AsyncClient): + # Only admins can update new terms + response = await client.patch("officers/info/abc11", json={ "legal_name": "fancy name", "phone_number": None, "discord_name": None, "github_username": None, "google_drive_email": None, - })) - assert response.status_code == 401 + }) + assert response.status_code == 403 response = await client.patch("officers/term/1", content=json.dumps({ "computing_id": "abc11", @@ -127,12 +167,12 @@ async def test__endpoints(client): "favourite_pl_1": "5", "biography": "hello" })) - assert response.status_code == 401 + assert response.status_code == 403 response = await client.delete("officers/term/1") assert response.status_code == 401 -@pytest.mark.asyncio +@pytest.mark.skip async def test__endpoints_admin(client, database_setup, admin_session): # login as website admin session_id = "temp_id_" + load_test_db.SYSADMIN_COMPUTING_ID From 86435f8552f42eea9bcb76b1b68cb47681b4dc0d Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Tue, 30 Sep 2025 17:32:21 -0700 Subject: [PATCH 26/32] fix: event loop fixed --- tests/conftest.py | 11 ++++++----- tests/integration/test_officers.py | 16 ++++------------ 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index df141be..bad6ae8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -21,7 +21,7 @@ def suppress_sqlalchemy_logs(): yield logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) -@pytest_asyncio.fixture(scope="session", loop_scope="session") +@pytest_asyncio.fixture(scope="module", loop_scope="session") async def database_setup(): # reset the database again, just in case print("Resetting DB...") @@ -32,7 +32,7 @@ async def database_setup(): yield sessionmanager await sessionmanager.close() -@pytest_asyncio.fixture(scope="function", loop_scope="session") +@pytest_asyncio.fixture(scope="session", loop_scope="session") async def client() -> AsyncGenerator[Any, None]: # base_url is just a random placeholder url # ASGITransport is just telling the async client to pass all requests to app @@ -45,12 +45,13 @@ async def db_session(database_setup): async with database_setup.session() as session: yield session -@pytest_asyncio.fixture(scope="function", loop_scope="session") -async def admin_session(database_setup): +@pytest_asyncio.fixture(scope="session", loop_scope="session") +async def admin_client(database_setup, client): session_id = "temp_id_" + load_test_db.SYSADMIN_COMPUTING_ID + client.cookies = { "session_id": session_id } async with database_setup.session() as session: await create_user_session(session, session_id, load_test_db.SYSADMIN_COMPUTING_ID) - yield + yield client await remove_user_session(session, session_id) diff --git a/tests/integration/test_officers.py b/tests/integration/test_officers.py index bdba8f0..ed0b94b 100644 --- a/tests/integration/test_officers.py +++ b/tests/integration/test_officers.py @@ -1,4 +1,3 @@ -import asyncio # NOTE: don't comment this out; it's required import json from datetime import date, timedelta @@ -16,7 +15,6 @@ async def test__read_execs(db_session): # test that reads from the database succeeded as expected - print(type(db_session)) assert (await get_active_officer_terms(db_session, "blarg")) == [] assert (await get_active_officer_terms(db_session, "abc22")) != [] @@ -55,8 +53,7 @@ async def test__read_execs(db_session): # pass async def test__get_officers(client): - # private data shoudn't be leaked - print(f"[DEBUG] Loop ID in {__name__}: {id(asyncio.get_running_loop())}") + # private data shouldn't be leaked response = await client.get("/officers/current") assert response.status_code == 200 assert response.json() != {} @@ -172,20 +169,15 @@ async def test__patch_officer_terms(client: AsyncClient): response = await client.delete("officers/term/1") assert response.status_code == 401 -@pytest.mark.skip -async def test__endpoints_admin(client, database_setup, admin_session): - # login as website admin - session_id = "temp_id_" + load_test_db.SYSADMIN_COMPUTING_ID - - client.cookies = { "session_id": session_id } - +async def test__get_current_officers_admin(admin_client): # test that more info is given if logged in & with access to it - response = await client.get("/officers/current") + response = await admin_client.get("/officers/current") assert response.status_code == 200 curr_officers = response.json() assert len(curr_officers) == 3 assert curr_officers["executive at large"]["computing_id"] is not None +async def test__get_all_officers_admin(client): response = await client.get("/officers/all?include_future_terms=true") assert response.status_code == 200 assert len(response.json()) == 9 From 8e17f27dadad7ef1e6cbb2f7361f0758176b7863 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Tue, 30 Sep 2025 17:35:03 -0700 Subject: [PATCH 27/32] wip: add admin_client --- tests/conftest.py | 4 ++-- tests/integration/test_officers.py | 37 +++++++++++++++--------------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index bad6ae8..d662267 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -21,7 +21,7 @@ def suppress_sqlalchemy_logs(): yield logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) -@pytest_asyncio.fixture(scope="module", loop_scope="session") +@pytest_asyncio.fixture(scope="session", loop_scope="session") async def database_setup(): # reset the database again, just in case print("Resetting DB...") @@ -45,7 +45,7 @@ async def db_session(database_setup): async with database_setup.session() as session: yield session -@pytest_asyncio.fixture(scope="session", loop_scope="session") +@pytest_asyncio.fixture(scope="module", loop_scope="session") async def admin_client(database_setup, client): session_id = "temp_id_" + load_test_db.SYSADMIN_COMPUTING_ID client.cookies = { "session_id": session_id } diff --git a/tests/integration/test_officers.py b/tests/integration/test_officers.py index ed0b94b..1f10539 100644 --- a/tests/integration/test_officers.py +++ b/tests/integration/test_officers.py @@ -177,37 +177,38 @@ async def test__get_current_officers_admin(admin_client): assert len(curr_officers) == 3 assert curr_officers["executive at large"]["computing_id"] is not None -async def test__get_all_officers_admin(client): - response = await client.get("/officers/all?include_future_terms=true") +async def test__get_all_officers_admin(admin_client): + response = await admin_client.get("/officers/all?include_future_terms=true") assert response.status_code == 200 assert len(response.json()) == 9 assert response.json()[1]["phone_number"] == "1234567890" - response = await client.get(f"/officers/terms/{load_test_db.SYSADMIN_COMPUTING_ID}?include_future_terms=false") +async def test__get_officer_term(admin_client): + response = await admin_client.get(f"/officers/terms/{load_test_db.SYSADMIN_COMPUTING_ID}?include_future_terms=false") assert response.status_code == 200 assert response.json() != [] assert len(response.json()) == 2 - response = await client.get(f"/officers/terms/{load_test_db.SYSADMIN_COMPUTING_ID}?include_future_terms=true") + response = await admin_client.get(f"/officers/terms/{load_test_db.SYSADMIN_COMPUTING_ID}?include_future_terms=true") assert response.status_code == 200 assert response.json() != [] assert len(response.json()) == 3 - response = await client.get("/officers/info/abc11") + response = await admin_client.get("/officers/info/abc11") assert response.status_code == 200 assert response.json() != {} assert response.json()["legal_name"] == "Person A" - response = await client.get(f"/officers/info/{load_test_db.SYSADMIN_COMPUTING_ID}") + response = await admin_client.get(f"/officers/info/{load_test_db.SYSADMIN_COMPUTING_ID}") assert response.status_code == 200 assert response.json() != {} - response = await client.get("/officers/info/balargho") + response = await admin_client.get("/officers/info/balargho") assert response.status_code == 404 - response = await client.get("/officers/terms/ehbc12?include_future_terms=true") + response = await admin_client.get("/officers/terms/ehbc12?include_future_terms=true") assert response.status_code == 200 assert response.json() == [] - response = await client.post("officers/term", content=json.dumps([{ + response = await admin_client.post("officers/term", content=json.dumps([{ "computing_id": "ehbc12", "position": OfficerPositionEnum.DIRECTOR_OF_MULTIMEDIA, "start_date": "2025-12-29", @@ -215,12 +216,12 @@ async def test__get_all_officers_admin(client): }])) assert response.status_code == 200 - response = await client.get("/officers/terms/ehbc12?include_future_terms=true") + response = await admin_client.get("/officers/terms/ehbc12?include_future_terms=true") assert response.status_code == 200 assert response.json() != [] assert len(response.json()) == 1 - response = await client.patch("officers/info/abc11", content=json.dumps({ + response = await admin_client.patch("officers/info/abc11", content=json.dumps({ "legal_name": "Person A2", "phone_number": "12345asdab67890", "discord_name": "person_a_yeah", @@ -235,7 +236,7 @@ async def test__get_all_officers_admin(client): assert resJson["github_username"] == "person_a" assert resJson["google_drive_email"] == "person_a@gmail.com" - response = await client.patch("officers/info/aaabbbc", content=json.dumps({ + response = await admin_client.patch("officers/info/aaabbbc", content=json.dumps({ "legal_name": "Person AABBCC", "phone_number": "1234567890", "discord_name": None, @@ -244,7 +245,7 @@ async def test__get_all_officers_admin(client): })) assert response.status_code == 404 - response = await client.patch("officers/term/1", content=json.dumps({ + response = await admin_client.patch("officers/term/1", content=json.dumps({ "position": OfficerPositionEnum.TREASURER, "start_date": (date.today() - timedelta(days=365)).isoformat(), "end_date": (date.today() - timedelta(days=1)).isoformat(), @@ -257,7 +258,7 @@ async def test__get_all_officers_admin(client): })) assert response.status_code == 200 - response = await client.get("/officers/terms/abc11?include_future_terms=true") + response = await admin_client.get("/officers/terms/abc11?include_future_terms=true") assert response.status_code == 200 resJson = response.json() assert resJson[1]["position"] == OfficerPositionEnum.TREASURER @@ -274,13 +275,13 @@ async def test__get_all_officers_admin(client): all_terms = await all_officers(db_session, include_future_terms=True) assert len(all_terms) == 10 - response = await client.delete("officers/term/1") + response = await admin_client.delete("officers/term/1") assert response.status_code == 200 - response = await client.delete("officers/term/2") + response = await admin_client.delete("officers/term/2") assert response.status_code == 200 - response = await client.delete("officers/term/3") + response = await admin_client.delete("officers/term/3") assert response.status_code == 200 - response = await client.delete("officers/term/4") + response = await admin_client.delete("officers/term/4") assert response.status_code == 200 async with database_setup.session() as db_session: From 15020658880e027f23b5e36b1c42016e51196d44 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Tue, 30 Sep 2025 18:58:28 -0700 Subject: [PATCH 28/32] fix: unit tests split up and all pass --- src/officers/crud.py | 2 +- src/officers/models.py | 2 +- src/officers/tables.py | 8 ++-- src/officers/urls.py | 10 ++-- tests/integration/test_elections.py | 1 - tests/integration/test_officers.py | 74 ++++++++++++++++++----------- 6 files changed, 58 insertions(+), 39 deletions(-) diff --git a/src/officers/crud.py b/src/officers/crud.py index b6d61b1..d9fd332 100644 --- a/src/officers/crud.py +++ b/src/officers/crud.py @@ -11,7 +11,7 @@ import utils from data import semesters from officers.constants import OfficerPosition -from officers.models import OfficerInfoResponse +from officers.models import OfficerInfoResponse, OfficerTermCreate from officers.tables import OfficerInfo, OfficerTerm # NOTE: this module should not do any data validation; that should be done in the urls.py or higher layer diff --git a/src/officers/models.py b/src/officers/models.py index 1559216..67c1b2c 100644 --- a/src/officers/models.py +++ b/src/officers/models.py @@ -1,6 +1,6 @@ from datetime import date -from pydantic import BaseModel, Field +from pydantic import BaseModel, ConfigDict, Field from officers.constants import OFFICER_LEGAL_NAME_MAX, OfficerPositionEnum diff --git a/src/officers/tables.py b/src/officers/tables.py index 07a1d80..15567eb 100644 --- a/src/officers/tables.py +++ b/src/officers/tables.py @@ -71,11 +71,11 @@ def serializable_dict(self) -> dict: "photo_url": self.photo_url, } - def update_from_params(self, params: OfficerTermUpdate, self_update: bool = True): - if self_update: - update_data = params.model_dump(exclude_unset=True, exclude={"position", "start_date", "end_date", "photo_url"}) - else: + def update_from_params(self, params: OfficerTermUpdate, admin_update: bool = True): + if admin_update: update_data = params.model_dump(exclude_unset=True) + else: + update_data = params.model_dump(exclude_unset=True, exclude={"position", "start_date", "end_date", "photo_url"}) for k, v in update_data.items(): setattr(self, k, v) diff --git a/src/officers/urls.py b/src/officers/urls.py index 79fc5a3..f045982 100755 --- a/src/officers/urls.py +++ b/src/officers/urls.py @@ -168,7 +168,7 @@ async def get_officer_info( }, operation_id="create_officer_term" ) -async def new_officer_term( +async def create_officer_term( request: Request, db_session: database.DBSession, officer_info_list: list[OfficerTermCreate], @@ -214,7 +214,7 @@ async def new_officer_term( }, operation_id="update_officer_info" ) -async def update_info( +async def update_officer_info( request: Request, db_session: database.DBSession, computing_id: str, @@ -246,7 +246,7 @@ async def update_info( }, operation_id="update_officer_term_by_id" ) -async def update_term( +async def update_officer_term( request: Request, db_session: database.DBSession, term_id: int, @@ -265,7 +265,7 @@ async def update_term( if utils.is_past_term(old_officer_term): raise HTTPException(status_code=403, detail="you may not update past terms") - old_officer_term.update_from_params(body) + old_officer_term.update_from_params(body, is_site_admin) # TODO (#27): log all important changes to a .log file await officers.crud.update_officer_term(db_session, old_officer_term) @@ -286,7 +286,7 @@ async def update_term( }, operation_id="delete_officer_term_by_id" ) -async def remove_officer_term( +async def delete_officer_term( request: Request, db_session: database.DBSession, term_id: int, diff --git a/tests/integration/test_elections.py b/tests/integration/test_elections.py index 2949b99..bf402a9 100644 --- a/tests/integration/test_elections.py +++ b/tests/integration/test_elections.py @@ -61,7 +61,6 @@ async def test_read_elections(database_setup): election_false = await get_election(db_session, "this-not-a-election") assert election_false is None - # Test getting specific election election = await get_election(db_session, "test-election-1") assert election is not None diff --git a/tests/integration/test_officers.py b/tests/integration/test_officers.py index 1f10539..d2c43b7 100644 --- a/tests/integration/test_officers.py +++ b/tests/integration/test_officers.py @@ -139,7 +139,7 @@ async def test__post_officer_terms(client: AsyncClient): }]) assert response.status_code == 422 -async def test__patch_officer_terms(client: AsyncClient): +async def test__patch_officer_term(client: AsyncClient): # Only admins can update new terms response = await client.patch("officers/info/abc11", json={ "legal_name": "fancy name", @@ -183,7 +183,7 @@ async def test__get_all_officers_admin(admin_client): assert len(response.json()) == 9 assert response.json()[1]["phone_number"] == "1234567890" -async def test__get_officer_term(admin_client): +async def test__get_officer_term_admin(admin_client): response = await admin_client.get(f"/officers/terms/{load_test_db.SYSADMIN_COMPUTING_ID}?include_future_terms=false") assert response.status_code == 200 assert response.json() != [] @@ -194,6 +194,11 @@ async def test__get_officer_term(admin_client): assert response.json() != [] assert len(response.json()) == 3 + response = await admin_client.get("/officers/terms/ehbc12?include_future_terms=true") + assert response.status_code == 200 + assert response.json() == [] + +async def test__get_officer_info_admin(admin_client): response = await admin_client.get("/officers/info/abc11") assert response.status_code == 200 assert response.json() != {} @@ -204,16 +209,13 @@ async def test__get_officer_term(admin_client): response = await admin_client.get("/officers/info/balargho") assert response.status_code == 404 - response = await admin_client.get("/officers/terms/ehbc12?include_future_terms=true") - assert response.status_code == 200 - assert response.json() == [] - - response = await admin_client.post("officers/term", content=json.dumps([{ +async def test__post_officer_term_admin(admin_client): + response = await admin_client.post("officers/term", json=[{ "computing_id": "ehbc12", "position": OfficerPositionEnum.DIRECTOR_OF_MULTIMEDIA, "start_date": "2025-12-29", "legal_name": "Eh Bc" - }])) + }]) assert response.status_code == 200 response = await admin_client.get("/officers/terms/ehbc12?include_future_terms=true") @@ -221,6 +223,7 @@ async def test__get_officer_term(admin_client): assert response.json() != [] assert len(response.json()) == 1 +async def test__patch_officer_info_admin(admin_client): response = await admin_client.patch("officers/info/abc11", content=json.dumps({ "legal_name": "Person A2", "phone_number": "12345asdab67890", @@ -245,7 +248,9 @@ async def test__get_officer_term(admin_client): })) assert response.status_code == 404 - response = await admin_client.patch("officers/term/1", content=json.dumps({ +async def test__patch_officer_term_admin(admin_client): + target_id = 1 + response = await admin_client.patch(f"officers/term/{target_id}", json={ "position": OfficerPositionEnum.TREASURER, "start_date": (date.today() - timedelta(days=365)).isoformat(), "end_date": (date.today() - timedelta(days=1)).isoformat(), @@ -255,25 +260,41 @@ async def test__get_officer_term(admin_client): "favourite_pl_0": "4", "favourite_pl_1": "5", "biography": "hello o77" - })) + }) assert response.status_code == 200 response = await admin_client.get("/officers/terms/abc11?include_future_terms=true") assert response.status_code == 200 - resJson = response.json() - assert resJson[1]["position"] == OfficerPositionEnum.TREASURER - assert resJson[1]["start_date"] == (date.today() - timedelta(days=365)).isoformat() - assert resJson[1]["end_date"] == (date.today() - timedelta(days=1)).isoformat() - assert resJson[1]["nickname"] != "1" - assert resJson[1]["favourite_course_0"] != "2" - assert resJson[1]["favourite_course_1"] != "3" - assert resJson[1]["favourite_pl_0"] != "4" - assert resJson[1]["favourite_pl_1"] != "5" - assert resJson[1]["biography"] == "hello o77" - - async with database_setup.session() as db_session: - all_terms = await all_officers(db_session, include_future_terms=True) - assert len(all_terms) == 10 + modifiedTerm = next((item for item in response.json() if item["id"] == target_id), None) + print(modifiedTerm) + assert modifiedTerm is not None + assert modifiedTerm["position"] == OfficerPositionEnum.TREASURER + assert modifiedTerm["start_date"] == (date.today() - timedelta(days=365)).isoformat() + assert modifiedTerm["end_date"] == (date.today() - timedelta(days=1)).isoformat() + assert modifiedTerm["nickname"] == "1" + assert modifiedTerm["favourite_course_0"] == "2" + assert modifiedTerm["favourite_course_1"] == "3" + assert modifiedTerm["favourite_pl_0"] == "4" + assert modifiedTerm["favourite_pl_1"] == "5" + assert modifiedTerm["biography"] == "hello o77" + + # other one shouldn't be modified + assert response.status_code == 200 + modifiedTerm = next((item for item in response.json() if item["id"] == target_id + 1), None) + print(modifiedTerm) + assert modifiedTerm is not None + assert modifiedTerm["position"] == OfficerPositionEnum.EXECUTIVE_AT_LARGE + assert modifiedTerm["start_date"] != (date.today() - timedelta(days=365)).isoformat() + assert modifiedTerm["end_date"] != (date.today() - timedelta(days=1)).isoformat() + assert modifiedTerm["nickname"] != "1" + assert modifiedTerm["favourite_course_0"] != "2" + assert modifiedTerm["favourite_course_1"] != "3" + assert modifiedTerm["favourite_pl_0"] != "4" + assert modifiedTerm["favourite_pl_1"] != "5" + assert modifiedTerm["biography"] != "hello o77" + + response = await admin_client.get("officers/all?include_future_terms=True") + assert len(response.json()) == 10 response = await admin_client.delete("officers/term/1") assert response.status_code == 200 @@ -284,6 +305,5 @@ async def test__get_officer_term(admin_client): response = await admin_client.delete("officers/term/4") assert response.status_code == 200 - async with database_setup.session() as db_session: - all_terms = await all_officers(db_session, include_private_data=True, include_future_terms=True) - assert len(all_terms) == (8 - 4) + response = await admin_client.get("officers/all?include_future_terms=True") + assert len(response.json()) == 6 From 5c3c3f923428fb6ac25f3f5978b68ed1de7a362a Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Tue, 30 Sep 2025 19:20:04 -0700 Subject: [PATCH 29/32] fix: github workflow update to pass CI --- .github/workflows/pytest_unit.yml | 1 + tests/conftest.py | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pytest_unit.yml b/.github/workflows/pytest_unit.yml index b48479d..88fd22d 100644 --- a/.github/workflows/pytest_unit.yml +++ b/.github/workflows/pytest_unit.yml @@ -30,5 +30,6 @@ jobs: - name: Run unit tests run: | + export PYTHONPATH=src source ./venv/bin/activate pytest ./tests/unit -v diff --git a/tests/conftest.py b/tests/conftest.py index d662267..88c8230 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,4 @@ # Configuration of Pytest -import asyncio import logging from collections.abc import AsyncGenerator from typing import Any From 422c99d4e2484265e2e9988c5c3753fee0d73318 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Tue, 30 Sep 2025 19:27:42 -0700 Subject: [PATCH 30/32] fix(tests): update GitHub actions and re-enabled unit tests --- .github/workflows/pytest_unit.yml | 5 +--- pyproject.toml | 2 +- tests/wip/unit/test_dates.py | 39 ------------------------------- 3 files changed, 2 insertions(+), 44 deletions(-) delete mode 100644 tests/wip/unit/test_dates.py diff --git a/.github/workflows/pytest_unit.yml b/.github/workflows/pytest_unit.yml index 88fd22d..6f75618 100644 --- a/.github/workflows/pytest_unit.yml +++ b/.github/workflows/pytest_unit.yml @@ -29,7 +29,4 @@ jobs: pip install -r requirements.txt - name: Run unit tests - run: | - export PYTHONPATH=src - source ./venv/bin/activate - pytest ./tests/unit -v + run: PYTHONPATH=src ./venv/bin/python -m pytest ./tests/unit -v diff --git a/pyproject.toml b/pyproject.toml index ddd3fd5..ff7761a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,7 +7,7 @@ requires-python = ">= 3.11" # older versions untested, but we use new features o Homepage = "https://api.sfucsss.org/" [tool.pytest.ini_options] -pythonpath = "./src/" +pythonpath = ["src"] log_cli = true log_cli_level = "INFO" testpaths = [ diff --git a/tests/wip/unit/test_dates.py b/tests/wip/unit/test_dates.py deleted file mode 100644 index 06cb391..0000000 --- a/tests/wip/unit/test_dates.py +++ /dev/null @@ -1,39 +0,0 @@ -from datetime import date, datetime - -from data.semesters import current_semester, current_semester_start, step_semesters - - -def test_semesters(): - start1 = current_semester_start(date(year=2022, month=11, day=1)) - start2 = current_semester_start(date(year=1022, month=7, day=30)) - start3 = current_semester_start(date(year=2322, month=1, day=12)) - - assert start1.month == 9 - assert start2.month == 5 - assert start3.month == 1 - - assert step_semesters(start1, -3).month == 9 - assert step_semesters(start1, -3).year == start1.year - 1 - - assert step_semesters(start1, -2).month == 1 - assert step_semesters(start1, -2).year == start1.year - - assert step_semesters(start1, -1).month == 5 - assert step_semesters(start1, -1).year == start1.year - - assert step_semesters(start1, 0).month == 9 - assert step_semesters(start1, 0).year == start1.year - - assert step_semesters(start1, 1).month == 1 - assert step_semesters(start1, 1).year == start1.year + 1 - - assert step_semesters(start1, 2).month == 5 - assert step_semesters(start1, 2).year == start1.year + 1 - - assert step_semesters(start1, 3).month == 9 - assert step_semesters(start1, 3).year == start1.year + 1 - - assert step_semesters(start3, -4).month == 9 - assert step_semesters(start3, -4).year == start3.year - 2 - - assert str(current_semester(start1)) == "fall" From 7262d5b695ee6ed217e3bd896383361eaf887cb7 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Tue, 30 Sep 2025 19:36:45 -0700 Subject: [PATCH 31/32] fix(tests): move conftest to integration folder, moved unit tests --- tests/{ => integration}/conftest.py | 0 tests/unit/test_dates.py | 39 +++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) rename tests/{ => integration}/conftest.py (100%) create mode 100644 tests/unit/test_dates.py diff --git a/tests/conftest.py b/tests/integration/conftest.py similarity index 100% rename from tests/conftest.py rename to tests/integration/conftest.py diff --git a/tests/unit/test_dates.py b/tests/unit/test_dates.py new file mode 100644 index 0000000..06cb391 --- /dev/null +++ b/tests/unit/test_dates.py @@ -0,0 +1,39 @@ +from datetime import date, datetime + +from data.semesters import current_semester, current_semester_start, step_semesters + + +def test_semesters(): + start1 = current_semester_start(date(year=2022, month=11, day=1)) + start2 = current_semester_start(date(year=1022, month=7, day=30)) + start3 = current_semester_start(date(year=2322, month=1, day=12)) + + assert start1.month == 9 + assert start2.month == 5 + assert start3.month == 1 + + assert step_semesters(start1, -3).month == 9 + assert step_semesters(start1, -3).year == start1.year - 1 + + assert step_semesters(start1, -2).month == 1 + assert step_semesters(start1, -2).year == start1.year + + assert step_semesters(start1, -1).month == 5 + assert step_semesters(start1, -1).year == start1.year + + assert step_semesters(start1, 0).month == 9 + assert step_semesters(start1, 0).year == start1.year + + assert step_semesters(start1, 1).month == 1 + assert step_semesters(start1, 1).year == start1.year + 1 + + assert step_semesters(start1, 2).month == 5 + assert step_semesters(start1, 2).year == start1.year + 1 + + assert step_semesters(start1, 3).month == 9 + assert step_semesters(start1, 3).year == start1.year + 1 + + assert step_semesters(start3, -4).month == 9 + assert step_semesters(start3, -4).year == start3.year - 2 + + assert str(current_semester(start1)) == "fall" From a65f832a90efeafee84343196dbdfd647d1d30b0 Mon Sep 17 00:00:00 2001 From: Jon Andre Briones Date: Sat, 25 Oct 2025 11:35:20 -0700 Subject: [PATCH 32/32] fix: address code review --- src/officers/urls.py | 7 ++++--- src/permission/types.py | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/officers/urls.py b/src/officers/urls.py index f045982..b74e839 100755 --- a/src/officers/urls.py +++ b/src/officers/urls.py @@ -106,7 +106,8 @@ async def all_officers( """, response_model=list[OfficerTermResponse], responses={ - 401: { "description": "not authorized to view private info", "model": DetailModel } + 401: { "description": "not logged in", "model": DetailModel }, + 403: { "description": "not authorized to view private info", "model": DetailModel } }, operation_id="get_officer_terms_by_id" ) @@ -201,7 +202,7 @@ async def create_officer_term( return JSONResponse({ "success": True }) @router.patch( - "/info/{computing_id:str}", + "/info/{computing_id}", description=""" After election, officer computing ids are input into our system. If you have been elected as a new officer, you may authenticate with SFU CAS, @@ -237,7 +238,7 @@ async def update_officer_info( return JSONResponse(updated_officer_info.serializable_dict()) @router.patch( - "/term/{term_id:int}", + "/term/{term_id}", description="Update the information for an Officer's term", response_model=OfficerTermResponse, responses={ diff --git a/src/permission/types.py b/src/permission/types.py index 2a1acf6..f0a3765 100644 --- a/src/permission/types.py +++ b/src/permission/types.py @@ -76,5 +76,5 @@ async def has_permission_or_raise( errmsg:str = "must have website admin permissions" ) -> bool: if not await WebsiteAdmin.has_permission(db_session, computing_id): - raise HTTPException(status_code=401, detail=errmsg) + raise HTTPException(status_code=403, detail=errmsg) return True