cal/paper-dynasty-database
1
0
Fork 0
Code Issues 7 Pull Requests 4 Actions Packages Projects Releases Wiki Activity

feat: add limit/pagination to notifications endpoint (#140) #150

Merged
cal merged 4 commits from issue/140-feat-add-limit-pagination-to-notifications-endpoin into main 2026-03-24 12:13:07 +00:00
Conversation 0 Commits 4 Files Changed 1 +94 -43
1 changed files with 94 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

137
app/routers_v2/notifications.py
View File

@ -9,10 +9,7 @@ from ..db_engine import Notification, model_to_dict, fn, DoesNotExist
from ..dependencies import oauth2_scheme, valid_token
router = APIRouter(
prefix='/api/v2/notifs',
tags=['notifs']
)
router = APIRouter(prefix="/api/v2/notifs", tags=["notifs"])
class NotifModel(pydantic.BaseModel):
@ -21,19 +18,30 @@ class NotifModel(pydantic.BaseModel):
desc: Optional[str] = None
field_name: str
message: str
about: Optional[str] = 'blank'
about: Optional[str] = "blank"
ack: Optional[bool] = False
@router.get('')
@router.get("")
async def get_notifs(
created_after: Optional[int] = None, title: Optional[str] = None, desc: Optional[str] = None,
field_name: Optional[str] = None, in_desc: Optional[str] = None, about: Optional[str] = None,
ack: Optional[bool] = None, csv: Optional[bool] = None):
created_after: Optional[int] = None,
title: Optional[str] = None,
desc: Optional[str] = None,
field_name: Optional[str] = None,
in_desc: Optional[str] = None,
about: Optional[str] = None,
ack: Optional[bool] = None,
csv: Optional[bool] = None,
Claude commented 2026-03-24 04:17:17 +00:00
Review
Copy Link

Negative limit values (e.g. ?limit=-1) survive min(-1, 500) unchanged and will cause a PostgreSQL error on LIMIT -1. Consider: limit = max(0, min(limit, 500))

Negative `limit` values (e.g. `?limit=-1`) survive `min(-1, 500)` unchanged and will cause a PostgreSQL error on `LIMIT -1`. Consider: `limit = max(0, min(limit, 500))`
limit: Optional[int] = 100,
):
if limit is not None:
limit = max(0, min(limit, 500))
all_notif = Notification.select().order_by(Notification.id)
if all_notif.count() == 0:
raise HTTPException(status_code=404, detail=f'There are no notifications to filter')
raise HTTPException(
status_code=404, detail="There are no notifications to filter"
)
if created_after is not None:
# Convert milliseconds timestamp to datetime for PostgreSQL comparison
@ -46,62 +54,90 @@ async def get_notifs(
if field_name is not None:
all_notif = all_notif.where(Notification.field_name == field_name)
if in_desc is not None:
all_notif = all_notif.where(fn.Lower(Notification.desc).contains(in_desc.lower()))
all_notif = all_notif.where(
fn.Lower(Notification.desc).contains(in_desc.lower())
)
if about is not None:
all_notif = all_notif.where(Notification.about == about)
if ack is not None:
all_notif = all_notif.where(Notification.ack == ack)
total_count = all_notif.count()
if limit is not None:
all_notif = all_notif.limit(limit)
if csv:
data_list = [['id', 'created', 'title', 'desc', 'field_name', 'message', 'about', 'ack']]
data_list = [
["id", "created", "title", "desc", "field_name", "message", "about", "ack"]
]
for line in all_notif:
data_list.append([
line.id, line.created, line.title, line.desc, line.field_name, line.message, line.about, line.ack
])
data_list.append(
[
line.id,
line.created,
line.title,
line.desc,
line.field_name,
line.message,
line.about,
line.ack,
]
)
return_val = DataFrame(data_list).to_csv(header=False, index=False)
cal commented 2026-03-24 12:10:32 +00:00
Review
Copy Link

After all_notif = all_notif.limit(limit) is applied, this .count() call wraps the limited queryset in a subquery — so it returns the count of limited results, not the total matching count. Capture total_count = all_notif.count() before applying .limit() and use that here to preserve the existing API contract.

After `all_notif = all_notif.limit(limit)` is applied, this `.count()` call wraps the limited queryset in a subquery — so it returns the count of limited results, not the total matching count. Capture `total_count = all_notif.count()` before applying `.limit()` and use that here to preserve the existing API contract.
return Response(content=return_val, media_type='text/csv')
return Response(content=return_val, media_type="text/csv")
else:
return_val = {'count': all_notif.count(), 'notifs': []}
return_val = {"count": total_count, "notifs": []}
for x in all_notif:
return_val['notifs'].append(model_to_dict(x))
return_val["notifs"].append(model_to_dict(x))
return return_val
@router.get('/{notif_id}')
@router.get("/{notif_id}")
async def get_one_notif(notif_id, csv: Optional[bool] = None):
try:
this_notif = Notification.get_by_id(notif_id)
except DoesNotExist:
raise HTTPException(status_code=404, detail=f'No notification found with id {notif_id}')
raise HTTPException(
status_code=404, detail=f"No notification found with id {notif_id}"
)
if csv:
data_list = [
['id', 'created', 'title', 'desc', 'field_name', 'message', 'about', 'ack'],
[this_notif.id, this_notif.created, this_notif.title, this_notif.desc, this_notif.field_name,
this_notif.message, this_notif.about, this_notif.ack]
["id", "created", "title", "desc", "field_name", "message", "about", "ack"],
[
this_notif.id,
this_notif.created,
this_notif.title,
this_notif.desc,
this_notif.field_name,
this_notif.message,
this_notif.about,
this_notif.ack,
],
]
return_val = DataFrame(data_list).to_csv(header=False, index=False)
return Response(content=return_val, media_type='text/csv')
return Response(content=return_val, media_type="text/csv")
else:
return_val = model_to_dict(this_notif)
return return_val
@router.post('')
@router.post("")
async def post_notif(notif: NotifModel, token: str = Depends(oauth2_scheme)):
if not valid_token(token):
logging.warning('Bad Token: [REDACTED]')
logging.warning("Bad Token: [REDACTED]")
raise HTTPException(
status_code=401,
detail='You are not authorized to post notifications. This event has been logged.'
detail="You are not authorized to post notifications. This event has been logged.",
)
logging.info(f'new notif: {notif}')
logging.info(f"new notif: {notif}")
this_notif = Notification(
created=datetime.fromtimestamp(notif.created / 1000),
title=notif.title,
@ -118,25 +154,34 @@ async def post_notif(notif: NotifModel, token: str = Depends(oauth2_scheme)):
else:
raise HTTPException(
status_code=418,
detail='Well slap my ass and call me a teapot; I could not save that notification'
detail="Well slap my ass and call me a teapot; I could not save that notification",
)
@router.patch('/{notif_id}')
@router.patch("/{notif_id}")
async def patch_notif(
notif_id, created: Optional[int] = None, title: Optional[str] = None, desc: Optional[str] = None,
field_name: Optional[str] = None, message: Optional[str] = None, about: Optional[str] = None,
ack: Optional[bool] = None, token: str = Depends(oauth2_scheme)):
notif_id,
created: Optional[int] = None,
title: Optional[str] = None,
desc: Optional[str] = None,
field_name: Optional[str] = None,
message: Optional[str] = None,
about: Optional[str] = None,
ack: Optional[bool] = None,
token: str = Depends(oauth2_scheme),
):
if not valid_token(token):
logging.warning('Bad Token: [REDACTED]')
logging.warning("Bad Token: [REDACTED]")
raise HTTPException(
status_code=401,
detail='You are not authorized to patch notifications. This event has been logged.'
detail="You are not authorized to patch notifications. This event has been logged.",
)
try:
this_notif = Notification.get_by_id(notif_id)
except DoesNotExist:
raise HTTPException(status_code=404, detail=f'No notification found with id {notif_id}')
raise HTTPException(
status_code=404, detail=f"No notification found with id {notif_id}"
)
if title is not None:
this_notif.title = title
@ -159,26 +204,32 @@ async def patch_notif(
else:
raise HTTPException(
status_code=418,
detail='Well slap my ass and call me a teapot; I could not save that rarity'
detail="Well slap my ass and call me a teapot; I could not save that rarity",
)
@router.delete('/{notif_id}')
@router.delete("/{notif_id}")
async def delete_notif(notif_id, token: str = Depends(oauth2_scheme)):
if not valid_token(token):
logging.warning('Bad Token: [REDACTED]')
logging.warning("Bad Token: [REDACTED]")
raise HTTPException(
status_code=401,
detail='You are not authorized to delete notifications. This event has been logged.'
detail="You are not authorized to delete notifications. This event has been logged.",
)
try:
this_notif = Notification.get_by_id(notif_id)
except DoesNotExist:
raise HTTPException(status_code=404, detail=f'No notification found with id {notif_id}')
raise HTTPException(
status_code=404, detail=f"No notification found with id {notif_id}"
)
count = this_notif.delete_instance()
if count == 1:
raise HTTPException(status_code=200, detail=f'Notification {notif_id} has been deleted')
raise HTTPException(
status_code=200, detail=f"Notification {notif_id} has been deleted"
)
else:
raise HTTPException(status_code=500, detail=f'Notification {notif_id} was not deleted')
raise HTTPException(
status_code=500, detail=f"Notification {notif_id} was not deleted"
)