ASWWU-Web / python_server

The API server for ASWWU Web. This project uses the Tornado web framework to serve and manage data across all ASWWU sites.
https://aswwu.com/server
4 stars 10 forks source link

build_query_parameters does not branch on start or end keys. #170

Open ermsdev opened 4 years ago

ermsdev commented 4 years ago

https://github.com/ASWWU-Web/python_server/blob/c0f3549ca2c0631a87d833a9c7c2594f7b0db7de/src/aswwu/route_handlers/elections.py#L17-L24

This may have been written before we implemented start_before, start_after, end_before, end_after as query parameters. Since we no longer use query parameters start, and end this conditional always branches to the else line, as a result this function returns the same dictionary as its input, with the exception that elements are not represented as lists.

If a start or end parameter is provided, it does branch on the if statement, which causes an exception to be thrown. this is because the strptime function is given a None object from search_criteria. This object was probably intended to be request_arguments instead.

Some considerations and things to investigate before fixing this:

  1. this is not causing bugs currently
  2. despite not modifying the input dictionary to include python datetime objects, the data shows up as a datetime type in the sql database schema.
  3. we should make sure that using a python datetime object instead doesn't break anything
  4. we should make sure we understand how date time comparisons are being performed despite not converting the data to a datetime type in this function.

and lastly, since this is similar to a function we'll be adding in the notifications project, we should find a good place to add a shared function or utility like this, and similar functions.

stacmi commented 4 years ago

just a quick note for this

querying an election ON ASWWU.COM by start_before or end_after works but if you query by start or end you get the same issue of

{
     "status": "strptime() argument 1 must be string, got None"
}

Fixing this would just be a simple refactor

stacmi commented 4 years ago

accidentally closed