Test: Nominee endpoint #127

jbriones1 · 2026-01-02T07:01:03Z

depends on #126

Added basic integration tests for the Nominees endpoints
TODO: Once the checks for election officers is completed, create the rest of the tests for election officers

* refactored how the election officers are looked up * changed the Nominee put back to a patch * made election tests work with the new testing setup

p-north · 2026-01-17T06:24:28Z

src/blog/crud.py

+    query = query.where(BlogPosts.title == title)
+
+    # should return the one entry with an unique title
+    post = await db_session.scalar(query)


nit pick: might wanna check if post is none

It looks like the whole Blog module is a mess. It showed up here since I ran a formatter over everything. Blogs will be addressed in a future update.

p-north · 2026-01-17T06:26:37Z

src/blog/crud.py

+    query = query.where(BlogPosts.post_tags in tags).where(BlogPosts.last_edited).order_by(BlogPosts.last_edited.desc())
+    # .all() should return a list of all the posts


slight syntax issue for sqlalchemy. BlogPosts.post_tags in tags is invalid syntax

Correction: query = query.where(BlogPosts.post_tags.contains(tags)).order_by(BlogPosts.last_edited.desc())

It looks like the whole Blog module is a mess. It showed up here since I ran a formatter over everything. Blogs will be addressed in a future update.

p-north · 2026-01-17T06:35:05Z

src/candidates/urls.py

+            detail="candidate is already registered for that position",
+        )
+
+    registration.update_from_params(body)


might want to await this

Nothing asynchronous about this function.

def update_from_params(self, params: CandidateUpdate): update_data = params.model_dump(exclude_unset=True) for k, v in update_data.items(): setattr(self, k, v)

The commit on line 176 should update the candidate at this point.

Based off this: https://docs.sqlalchemy.org/en/21/tutorial/orm_data_manipulation.html#updating-orm-objects-using-the-unit-of-work-pattern

p-north

Sorry for the delay, just been trying to balance work and other things! Hopefully its better now.

Everything looks good. Just some minor nitpicks here and there.

src/nominees/urls.py

p-north · 2026-02-07T19:16:41Z

src/officers/models.py

@@ -12,26 +16,105 @@
    "phone_number",
    "github_username",
    "google_drive_email",
-    "photo_url"
+    "photo_url",
 }


I know this was already here from before, but maybe if it's not being used, we can get rid of it?

We'd be losing everything from that field when you do the database migration.

src/officers/models.py

p-north · 2026-02-07T19:20:19Z

src/officers/models.py

+    start_date: date | None = None
+    end_date: date | None = None


-might want to validate the start_date and end_date but its not a big deal.

src/officers/models.py

p-north · 2026-02-07T19:26:12Z

src/scripts/migrate_from_about_officers.py

-                sqlalchemy
-                .select(OfficerInfo)
-                .where(OfficerInfo.computing_id == new_officer_info.computing_id)
+                sqlalchemy.select(OfficerInfoDB).where(OfficerInfoDB.computing_id == new_officer_info.computing_id)


I think you might be missing the sqlalchemy import.

It's on line 8

p-north · 2026-02-07T19:27:16Z

src/scripts/migrate_from_about_officers.py

        password=DB_PASSWORD,
        database="postgres",
-        host="sfucsss.org", # NOTE: this should point to the old sfucsss.org server (made initially by jace)
+        host="sfucsss.org",  # NOTE: this should point to the old sfucsss.org server (made initially by jace)


I know this was here before, but maybe we shouldn't hard code this?

We shouldn't, but this script was for migrating before we took down the old site.

p-north · 2026-02-07T19:32:45Z

tests/integration/conftest.py

+
+
+@pytest_asyncio.fixture(scope="module", loop_scope="session")
 async def client() -> AsyncGenerator[Any, None]:


missing import for AsyncGenerator, Any

Lines 3 and 4

p-north · 2026-02-07T19:35:06Z

tests/integration/test_nominees.py

+async def test__admin_delete_nominee(admin_client: AsyncClient):
+    response = await admin_client.delete(f"/nominee/{TEST_NOMINEE['computing_id']}")
+    assert response.status_code == HTTPStatus.OK
+    assert response.json()["success"]


This is asserting thruthiness not correctness.
Consider:
assert response.json() == {"success": True}

It works fine when running the test. I changed the response to False and it correctly catches the assert. What's wrong with this?

I prefer Jon's approach since it's more stable we decide to add extra fields and allow us to do more granular tests on the fields separately.

Though I would like to add is True or is False to be more explicit

assert response.json()["success"] is True # more fields in the future rather than simple equality assert response.json()["remaining_nominiees"], "There should be more nominees"

jbriones1

Addressed some comments

jbriones1 added 30 commits December 16, 2025 19:01

chore(pyproject): update install steps and change to using uv

c8ead80

fix: python version pinned to 3.11

e6f1072

chore(gitignore): update gitignore with uv items

0d4ce6c

chore(pyright): update pyright config to use version 3.11

b7c305f

chore(format): ignore alembic migrations

c94aa06

chore(format): ran the formatter over the entire project

a721e4f

fix(ci): fix script to not rely on requirements.txt

df0f490

fix(ci): split test and dev dependencies again, update action script

01cb9f4

fix(ci): removed requirements.txt requirement

8fe4e74

fix(uv): update uv.lock to current state

5ab91f4

refactor(permissions): rewrote permissions to integrate admin types

f571eb3

fix: type errors in officers crud

74bc701

refactor(permissions): restructured most endpoints to use Depends

4873317

fix(tests): whole test suite can now be run

6c880e1

fix: missing awaits on permission check calls

268fd97

fix(officers): tests now all work

69469e1

fix: move conftest.py to only work on integrations tests

6e39610

wip: add GET all registrations

3eeaebc

wip(nominee): add create endpoint

150f08c

wip: add GET all nominees

32a3ee0

wip: add DELETE nominee_info

8492ff0

fix(Elections): duplicate operation id in nominees URLs

3fa406a

fix: patch to put

95ad68e

fix: election tests now work

a751e67

* refactored how the election officers are looked up * changed the Nominee put back to a patch * made election tests work with the new testing setup

fix(tests): add missing param types

56f3585

refactor: split election tests into multiple functions

f8824f7

fix(tests): split admin elections tests

96ca71c

fix(permissions): fixed some permission tests to be more efficient

a34a3c4

refactor: update Nominee model names

e95efd9

fix(elections): fix all elections tests

0f2a339

jbriones1 added 20 commits December 27, 2025 02:37

refactor: rename OfficerUpdate to OfficerInfoUpdate

6293336

refactor: change csss_email to a computed field

91941f9

chore(officers): delete old types

846446c

fix: remove csss_email

49a3c9b

fix: check that end_date is null for current officers

68a5eba

fix: tests failed after the check for current admin changed

2b44419

refactor(elections): Election to ElectionDB

68758d1

fix: election models now use timezone naive datetimes

a9ffd2f

fix: made all timestamps timezone aware, UTC stored now

5331eb9

fix(blogs): blogs are now timezone aware

12bd99f

chore(tables): rename the rest of the DB tables

141a9ae

chore: ruff fix unsorted imports

8f585f1

refactor(nominee): update nominee models and PATCH request

1f1c8f0

refactor(candidates): renamed registrations to candidates

5f3cdb6

fix(alembic): registration renamed to candidates

17c0c67

fix(nominee): update POST to use create model

43a0cf6

fix(candidates): missed some renames

bf9fe64

fix(candidates): updating a candidate with the same position failed

1a6ac99

feat(nominee-tests): created base tests

3f39439

fix(tests): databases now persist for an entire test module (file)

e7c9a2b

jbriones1 requested a review from p-north January 2, 2026 07:01

jbriones1 changed the title ~~Feature: Nominee tests~~ Test: Nominee endpoint Jan 2, 2026

jbriones1 marked this pull request as ready for review January 3, 2026 21:25

p-north reviewed Jan 17, 2026

View reviewed changes

jbriones1 requested a review from p-north January 25, 2026 01:03

p-north reviewed Feb 7, 2026

View reviewed changes

fix: address some code comments

da86d6b

jbriones1 commented Feb 9, 2026

View reviewed changes

		query = query.where(BlogPosts.post_tags in tags).where(BlogPosts.last_edited).order_by(BlogPosts.last_edited.desc())
		# .all() should return a list of all the posts



		@pytest_asyncio.fixture(scope="module", loop_scope="session")
		async def client() -> AsyncGenerator[Any, None]:

Test: Nominee endpoint #127

Are you sure you want to change the base?

Test: Nominee endpoint #127

Uh oh!

Conversation

jbriones1 commented Jan 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

p-north left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jbriones1 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

p-north left a comment •

edited

Loading