-
Notifications
You must be signed in to change notification settings - Fork 1
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
Fix/list all events in previous events view #100
base: master
Are you sure you want to change the base?
Changes from 4 commits
9be0086
9811b48
dbd584e
f4d105d
4d6f5ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
from datetime import datetime, timedelta | ||
from datetime import datetime, timedelta, timezone | ||
import os | ||
import shutil | ||
from fastapi import APIRouter, Response, Request, HTTPException, Depends, BackgroundTasks | ||
from fastapi import APIRouter, Response, Request, HTTPException, Depends, BackgroundTasks, Query | ||
from fastapi.datastructures import UploadFile | ||
from fastapi.param_functions import File | ||
from pydantic import ValidationError | ||
|
@@ -93,9 +93,28 @@ def get_upcoming_events(request: Request, token: AccessTokenPayload = Depends(op | |
return [Event.model_validate(event) for event in upcoming_events] | ||
|
||
|
||
@router.get('/past-events/count') | ||
def get_past_events_count(request: Request, token: AccessTokenPayload = Depends(optional_authentication)): | ||
"""Get count of past events""" | ||
db = get_database(request) | ||
|
||
# Get current UTC datetime | ||
current_datetime = datetime.now(timezone.utc) | ||
|
||
# Apply search filter according to role | ||
search_filter = {"date": {"$lt": current_datetime}} | ||
if token and token.role != Role.admin: | ||
search_filter["public"] = True | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure this is leads to a valid mongodb query. You can copy the search filter logic from the |
||
# Count the number of documents that match the filter | ||
count = db.events.count_documents(search_filter) | ||
return {"count": count} | ||
|
||
@router.get('/past-events') | ||
def get_past_events(request: Request, token: AccessTokenPayload = Depends(optional_authentication)): | ||
""" Get last 10 events that have passed """ | ||
def get_past_events(request: Request, token: AccessTokenPayload = Depends(optional_authentication), | ||
skip: int = Query(0, ge=0), | ||
limit: int = Query(10,ge=1,le=50)): | ||
""" Get last events that have passed """ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like I commented on the frontend, another design here that might be simpler is to simply use a hardcoded limit, instead of allowing clients to supply it in the query parameters. If you choose to simplify it, remember to do it both on the api and the frontend. |
||
# TODO: Expand endpoint to accept custom amount? | ||
db = get_database(request) | ||
|
||
|
@@ -110,11 +129,12 @@ def get_past_events(request: Request, token: AccessTokenPayload = Depends(option | |
if token and token.role == Role.admin: | ||
search_filter = {'date': {"$lt": date}} | ||
|
||
# Get the last 10 events | ||
# Get the last events | ||
pipeline = [ | ||
{"$match": search_filter}, | ||
{"$sort": {"date": -1}}, | ||
{"$limit": 10} | ||
{"$skip": skip}, | ||
{"$limit": limit}, | ||
] | ||
|
||
res = db.events.aggregate(pipeline) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This endpoint uses
timezone.utc
, but none of the others do. This might lead to wrong results here