Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First Elections PR #63

Open
wants to merge 34 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
d67b000
Added elections model to alembic/env.py
DerpyWasHere Aug 9, 2024
905170b
Added election tables revision to alembic
DerpyWasHere Aug 9, 2024
7cb496c
Initial elections model implementation
DerpyWasHere Aug 9, 2024
8b06dda
Redid revision, added position column to NomineeAplication, renamed N…
DerpyWasHere Aug 10, 2024
b5103ca
Added ElectionOfficer class, created has_permission() to check whethe…
DerpyWasHere Aug 12, 2024
b579e08
Changed date to be nonnull, officer_id in alembic revision
DerpyWasHere Aug 13, 2024
c89e659
Removed unique constraint on officer_id
DerpyWasHere Aug 13, 2024
117611a
Removed unique constraint on officer_id, removed nullability from date
DerpyWasHere Aug 13, 2024
3aded13
properly handled session validation on create_elections
DerpyWasHere Aug 29, 2024
fc8def5
created create_election stub
DerpyWasHere Aug 29, 2024
d9b8e52
added elections router to main
DerpyWasHere Aug 29, 2024
e239b9b
Working commit containing fns to create, delete, and update elections
DerpyWasHere Sep 27, 2024
8bf5dcb
Merge branch 'main' into dev-issue-25
EarthenSky Oct 3, 2024
06fe816
switch from models to tables
EarthenSky Oct 3, 2024
82685ef
fix small import bug
EarthenSky Oct 3, 2024
0c299d2
fix past alembic migration
EarthenSky Oct 3, 2024
5a2b886
Removed enum in urls.py, satisfied linter for tables.py
DerpyWasHere Jan 13, 2025
5faad35
Changed old ElectionTypes enum into a string array
DerpyWasHere Jan 13, 2025
7b465b5
Changed query in get_election() to be more SQL-like.
DerpyWasHere Jan 13, 2025
44d60f3
Changed list comprehension in create_election() to just a 'if not in'…
DerpyWasHere Jan 13, 2025
40eedd7
Made change referenced in pr 63 wrt committing transactions in get_el…
DerpyWasHere Jan 13, 2025
e2bb4db
Removed commits from crud.py and added commits to endpoints in urls.py
DerpyWasHere Jan 13, 2025
50302b1
Changed occurrences of websurvey to survey_link to match that websurv…
DerpyWasHere Jan 13, 2025
82ccc24
Changed election parameters from a list to a dedicated dataclass, ref…
DerpyWasHere Jan 13, 2025
a36eef6
Changed parameter orders to be consistent with other crud functions i…
DerpyWasHere Jan 13, 2025
057e405
Merge branch 'main' into dev-issue-25
DerpyWasHere Jan 13, 2025
ff2951c
Appeased linter
DerpyWasHere Jan 13, 2025
8d0e267
Reintroduced elections router into main.py
DerpyWasHere Jan 13, 2025
40041ad
update down revision to be blog posts
EarthenSky Jan 13, 2025
853038d
Added lost default param in elections table migration
DerpyWasHere Jan 13, 2025
2dee83a
Changed discord id length in election_nominee table from 18 to 32.
DerpyWasHere Jan 13, 2025
945fb29
Changed date -> start_date in elections table
DerpyWasHere Jan 13, 2025
57f4b2a
Changed date -> start_datetime, start_date -> start_datetime, end_dat…
DerpyWasHere Jan 13, 2025
1c61134
Changed date -> start_datetime, end_date -> end_datetime
DerpyWasHere Jan 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/alembic/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import auth.tables
import blog.tables
import database
import elections.tables
import officers.tables
from alembic import context

Expand Down
58 changes: 58 additions & 0 deletions src/alembic/versions/243190df5588_create_election_tables.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
"""create election tables

Revision ID: 243190df5588
Revises: 43f71e4bd6fc
Create Date: 2024-08-10 08:32:54.037614

"""
from collections.abc import Sequence
from typing import Union

import sqlalchemy as sa

from alembic import op

# revision identifiers, used by Alembic.
revision: str = "243190df5588"
down_revision: str | None = "2a6ea95342dc"
branch_labels: str | Sequence[str] | None = None
depends_on: str | Sequence[str] | None = None


def upgrade() -> None:
op.create_table("election",
sa.Column("slug", sa.String(length=32), nullable=False),
sa.Column("name", sa.String(length=32), nullable=False),
sa.Column("officer_id", sa.String(length=32), nullable=False),
sa.Column("type", sa.String(length=64), nullable=True, default="general_election"),
sa.Column("date", sa.DateTime(), nullable=False),
sa.Column("end_date", sa.DateTime(), nullable=True),
sa.Column("survey_link", sa.String(length=300), nullable=True),
sa.PrimaryKeyConstraint("slug")
)
op.create_table("election_nominee",
sa.Column("computing_id", sa.String(length=32), nullable=False),
sa.Column("full_name", sa.String(length=64), nullable=False),
sa.Column("facebook", sa.String(length=64), nullable=True),
sa.Column("instagram", sa.String(length=64), nullable=True),
sa.Column("email", sa.String(length=64), nullable=True),
sa.Column("discord", sa.String(length=32), nullable=True),
sa.Column("discord_id", sa.String(length=18), nullable=True),
sa.Column("discord_username", sa.String(length=32), nullable=True),
sa.PrimaryKeyConstraint("computing_id")
)
op.create_table("nominee_application",
sa.Column("computing_id", sa.String(length=32), nullable=False),
sa.Column("nominee_election", sa.String(length=32), nullable=False),
sa.Column("speech", sa.Text(), nullable=True),
sa.Column("position", sa.String(length=64), nullable=False),
sa.ForeignKeyConstraint(["computing_id"], ["election_nominee.computing_id"], ),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is there a comma and the end of the inside of the function call? Any idea?

sa.ForeignKeyConstraint(["nominee_election"], ["election.slug"], ),
sa.PrimaryKeyConstraint("computing_id", "nominee_election")
)


def downgrade() -> None:
op.drop_table("nominee_application")
op.drop_table("election_nominee")
op.drop_table("election")
82 changes: 82 additions & 0 deletions src/elections/crud.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import logging
from dataclasses import dataclass
from datetime import datetime

import sqlalchemy
from sqlalchemy.ext.asyncio import AsyncSession

import database
from elections.tables import Election
from officers.constants import OfficerPosition
from officers.tables import OfficerInfo, OfficerTerm


@dataclass
class ElectionParameters:
"""
Dataclass encompassing the data that can go into an Election.
"""
slug: str
name: str
officer_id: str
type: str
date: datetime
end_date: datetime
survey_link: str


_logger = logging.getLogger(__name__)

async def get_election(db_session: AsyncSession, election_slug: str) -> Election | None:
query = (
sqlalchemy
.select(Election)
.where(Election.slug == election_slug)
)
result = await db_session.scalar(query)
return result

async def create_election(db_session: AsyncSession, params: ElectionParameters) -> None:
"""
Creates a new election with given parameters.
Does not validate if an election _already_ exists
"""
election = Election(slug=params.slug,
name=params.name,
officer_id=params.officer_id,
type=params.type,
date=params.date,
end_date=params.end_date,
survey_link=params.survey_link)
db_session.add(election)

async def delete_election(db_session: AsyncSession, slug: str) -> None:
"""
Deletes a given election by its slug.
Does not validate if an election exists
"""
query = sqlalchemy.delete(Election).where(Election.slug == slug)
await db_session.execute(query)

async def update_election(db_session: AsyncSession, params: ElectionParameters) -> None:
"""
Updates an election with the provided parameters.
Take care as this will replace values with None if not populated.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not true? You have an if-statement that doesn't update the values if null? (null == None)

You _cannot_ change the name or slug, you should instead delete and create a new election.
Does not validate if an election _already_ exists
"""

election = (await db_session.execute(sqlalchemy.select(Election).filter_by(slug=params.slug))).scalar_one()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be consistent, let's only use the SQL syntax

Suggested change
election = (await db_session.execute(sqlalchemy.select(Election).filter_by(slug=params.slug))).scalar_one()
election = await db_session.scalar(sqlalchemy.select(Election).where(Election.slug == params.slug))


if params.date is not None:
election.date = params.date
if params.type is not None:
election.type = params.type
if params.end_date is not None:
election.end_date = params.end_date
if params.survey_link is not None:
election.survey_link = params.survey_link

query = sqlalchemy.update(Election).where(Election.slug == params.slug).values(election)
await db_session.execute(query)

59 changes: 59 additions & 0 deletions src/elections/tables.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
from datetime import datetime

from sqlalchemy import (
Column,
DateTime,
ForeignKey,
PrimaryKeyConstraint,
String,
Text,
)

from constants import (
COMPUTING_ID_LEN,
DISCORD_ID_LEN,
DISCORD_NAME_LEN,
DISCORD_NICKNAME_LEN,
)
from database import Base

election_types = ["general_election", "by_election", "council_rep_election"]

# Each row represents an instance of an
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

each row represents an instance of a... ?

class Election(Base):
__tablename__ = "election"

# Slugs are unique identifiers
slug = Column(String(32), primary_key=True)
name = Column(String(32), nullable=False)
officer_id = Column(String(COMPUTING_ID_LEN), nullable=False)
type = Column(String(64), default="general_election")
date = Column(DateTime, nullable=False)
EarthenSky marked this conversation as resolved.
Show resolved Hide resolved
end_date = Column(DateTime)
survey_link = Column(String(300))

# Each row represents a nominee of a given election
class Nominee(Base):
__tablename__ = "election_nominee"

# Previously named sfuid
computing_id = Column(String(COMPUTING_ID_LEN), primary_key=True)
full_name = Column(String(64), nullable=False)
facebook = Column(String(64))
instagram = Column(String(64))
email = Column(String(64))
discord = Column(String(DISCORD_NAME_LEN))
discord_id = Column(String(DISCORD_ID_LEN))
EarthenSky marked this conversation as resolved.
Show resolved Hide resolved
discord_username = Column(String(DISCORD_NICKNAME_LEN))
EarthenSky marked this conversation as resolved.
Show resolved Hide resolved

class NomineeApplication(Base):
__tablename__ = "nominee_application"

computing_id = Column(ForeignKey("election_nominee.computing_id"), primary_key=True)
nominee_election = Column(ForeignKey("election.slug"), primary_key=True)
speech = Column(Text)
position = Column(String(64), nullable=False)

__table_args__ = (
PrimaryKeyConstraint(computing_id, nominee_election),
)
160 changes: 160 additions & 0 deletions src/elections/urls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
import base64
import logging
import os
import re
import urllib.parse
from datetime import datetime

import requests # TODO: make this async
import xmltodict
from crud import ElectionParameters
from fastapi import APIRouter, BackgroundTasks, FastAPI, HTTPException, Request, status
from fastapi.exceptions import RequestValidationError
from tables import election_types

import auth
import auth.crud
import database
import elections
from constants import root_ip_address
from permission import types

_logger = logging.getLogger(__name__)

router = APIRouter(
prefix="/elections",
tags=["elections"],
)

def _slugify(
text: str
) -> str:
"""
Creates a unique slug based on text passed in. Assumes non-unicode text.
"""
return re.sub(r"[\W_]+", "-", text)

async def _validate_user(
db_session: database.DBSession,
session_id: str
) -> dict:
computing_id = await auth.crud.get_computing_id(db_session, session_id)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

computing_id may be None. This should be handled in has_permission or here (preferably here, since that's how I do it for other functions)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, why does validate user return dict, but you compare it against a bool later?

# Assuming now user is validated
result = await types.ElectionOfficer.has_permission(db_session, computing_id)
return result

@router.get(
"/create_election",
description="Creates an election and places it in the database",
)
async def create_election(
request: Request,
db_session: database.DBSession,
name: str,
election_type: str,
date: datetime | None = None,
end_date: datetime | None = None,
survey_link: str | None = None
):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

end_date and date should be changed to end_datetime and start_datetime respectively

"""
aaa
"""
session_id = request.cookies.get("session_id", None)
user_auth = await _validate_user(db_session, session_id)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async def logged_in_or_raise(

I made a function which may have the same behaviour as _validate_user; maybe you can reuse the code?

if user_auth is False:
# let's workshop how we actually wanna handle this
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="You do not have permission to access this resource",
headers={"WWW-Authenticate": "Basic"},
)
Comment on lines +66 to +70
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

raise HTTPException(status_code=401, detail="only website admins can view all executive terms that have not started yet")

you can simplify exceptions a little bit


# Default start time should be now unless specified otherwise
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default start time should not be now! That would show all the election speeches immediately! (Since I assume that's what it means). Can we allow an election to have a null (None) start time? This way, the election officer can decide when they want the election to start.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OR, we could require the start time to be specified on creation

if date is None:
date = datetime.now()

if election_type not in election_types:
raise RequestValidationError()

params = ElectionParameters(
_slugify(name),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since slug is used as a unique identifier, what if we have a duplicate slug here ("My election" vs "my election", etc...)? We should have a check to make sure that slug does not currently exist, so we don't accidentally overwrite a past election.

name,
await auth.crud.get_computing_id(db_session, session_id),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we reuse the computing_id from above? Here's how I do it in the officers module

_, session_computing_id = await logged_in_or_raise(request, db_session)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note about naming: I had to have a distinction between the computing_id of the current session vs the computing_id of the person being looked up (you must be admin to lookup info about others, but you can about yourself)

election_type,
date,
end_date,
survey_link
)

await elections.crud.create_election(params, db_session)
await db_session.commit()

# TODO: create a suitable json response
return {}

@router.get(
"/delete_election",
description="Deletes an election from the database"
)
Comment on lines +95 to +98
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@router.get(
"/delete_election",
description="Deletes an election from the database"
)
@router.delete(
"/election/{slug:str}",
description="Deletes an election from the database"
)

async def delete_election(
request: Request,
db_session: database.DBSession,
slug: str
):
session_id = request.cookies.get("session_id", None)
user_auth = await _validate_user(db_session, session_id)
if user_auth is False:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if user_auth is False:
if not user_auth:

# let's workshop how we actually wanna handle this
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="You do not have permission to access this resource",
headers={"WWW-Authenticate": "Basic"},
)

if slug is not None:
await elections.crud.delete_election(slug, db_session)
await db_session.commit()
Comment on lines +114 to +116
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks weird; what should the behaviour be if slug is None? iirc fastapi will not even allow it to be null, since you've set the type as str. (it will return a 422 error iirc)


@router.get(
"/update_election",
description="""Updates an election in the database.
Note that this does not allow you to change the _name_ of an election as this would generate a new slug."""
)
Comment on lines +118 to +122
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@router.get(
"/update_election",
description="""Updates an election in the database.
Note that this does not allow you to change the _name_ of an election as this would generate a new slug."""
)
@router.patch(
"/election/{slug:str}",
description="""Updates an election in the database.
Note that this does not allow you to change the _name_ of an election as this would generate a new slug."""
)

async def update_election(
request: Request,
db_session: database.DBSession,
slug: str,
name: str,
election_type: str,
date: datetime | None = None,
end_date: datetime | None = None,
survey_link: str | None = None
):
session_id = request.cookies.get("session_id", None)
user_auth = await _validate_user(db_session, session_id)
if user_auth is False:
# let's workshop how we actually wanna handle this
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="You do not have permission to access this resource",
headers={"WWW-Authenticate": "Basic"},
)
if slug is not None:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slug is not allowed to be none here, since fastapi actually enforces input types (iirc) -> it's worth a simple test anyways as a santity check

params = ElectionParameters(
_slugify(name),
name,
await auth.crud.get_computing_id(db_session, session_id),
election_type,
date,
end_date,
survey_link
)
await elections.crud.update_election(params, db_session)
await db_session.commit()


@router.get(
"/test"
)
async def test():
return {"error": "lol"}
Loading
Loading