Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions pecha_api/verse_of_day/verse_of_day_enums.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
from enum import Enum


class SortOrder(str, Enum):
ASC = "asc"
DESC = "desc"
22 changes: 16 additions & 6 deletions pecha_api/verse_of_day/verse_of_day_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from .verse_of_day_model import VerseOfDay
from .verse_metadata_model import VerseMetadata
from .verse_of_day_enums import SortOrder
from pecha_api.plans.groups.groups_models import AuthorGroupMetadata


Expand All @@ -30,21 +31,30 @@ def get_verses_of_day_list(
db: Session,
group_id: Optional[UUID] = None,
filter_date: Optional[date] = None,
search: Optional[str] = None,
sort_order: SortOrder = SortOrder.DESC,
skip: int = 0,
limit: int = 100
) -> tuple[List[VerseOfDay], int]:
"""Get list of verses with pagination."""
"""Get list of verses with search, sorting, and pagination."""
query = db.query(VerseOfDay).options(joinedload(VerseOfDay.verse_metadata))

if group_id is not None:
query = query.filter(VerseOfDay.group_id == group_id)

if filter_date is not None:
query = query.filter(VerseOfDay.date == filter_date)


if search:
query = query.filter(
VerseOfDay.verse_metadata.any(VerseMetadata.verse.ilike(f"%{search}%"))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Wildcard Search Matches Everything

When search contains SQL LIKE metacharacters such as % or _, the new ilike pattern treats them as wildcards instead of literal text. A CMS request like search=% or search=_ can therefore return nearly every verse rather than verses containing those characters, so users get broad false-positive results from the list endpoint.

)

total = query.count()
verses = query.order_by(VerseOfDay.date.desc()).offset(skip).limit(limit).all()


order_by = VerseOfDay.date.asc() if sort_order == SortOrder.ASC else VerseOfDay.date.desc()
verses = query.order_by(order_by).offset(skip).limit(limit).all()

return verses, total


Expand Down
7 changes: 5 additions & 2 deletions pecha_api/verse_of_day/verse_of_day_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
GroupInfoDTO
)
from .verse_of_day_model import VerseOfDay
from .verse_of_day_enums import SortOrder
from ..uploads.S3_utils import generate_presigned_access_url
from ..config import get

Expand Down Expand Up @@ -148,12 +149,14 @@ def get_verses_of_day_list_service(
group_id: Optional[UUID] = None,
filter_date: Optional[date] = None,
lang: Optional[str] = None,
search: Optional[str] = None,
sort_order: SortOrder = SortOrder.DESC,
skip: int = 0,
limit: int = 100
) -> VerseOfDayListResponse:
"""Get list of verses with pagination."""
"""Get list of verses with search, sorting, and pagination."""
with SessionLocal() as db:
verses, total = get_verses_of_day_list(db, group_id=group_id, filter_date=filter_date, skip=skip, limit=limit)
verses, total = get_verses_of_day_list(db, group_id=group_id, filter_date=filter_date, search=search, sort_order=sort_order, skip=skip, limit=limit)

verse_dtos = []
for verse in verses:
Expand Down
5 changes: 4 additions & 1 deletion pecha_api/verse_of_day/verse_of_day_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from starlette import status

from .verse_of_day_response_models import VerseOfDayPublicResponse, VerseOfDayListResponse, CreateVerseOfDayRequest, UpdateVerseOfDayRequest, VerseOfDayDTO
from .verse_of_day_enums import SortOrder
from .verse_of_day_service import get_verse_of_day, get_verses_of_day_list_service, get_verse_of_day_by_id_service, get_verse_of_day_today_service, create_verse_of_day_service, update_verse_of_day_service, delete_verse_of_day_service
from pecha_api.users.users_service import validate_and_extract_user_details

Expand Down Expand Up @@ -74,11 +75,13 @@ def cms_get_verse_of_day_endpoint(
group_id: Annotated[Optional[UUID], Query(description="Filter by group ID")] = None,
date: Annotated[Optional[date], Query(description="Filter by date (YYYY-MM-DD)")] = None,
lang: Annotated[Optional[str], Query(description="Filter by language (en, bo, zh, hi, ne, mn). Returns all languages if not specified.")] = None,
search: Annotated[Optional[str], Query(description="Free-text search over verse content (any language)")] = None,
sort_order: Annotated[SortOrder, Query(description="Sort by date: asc (oldest first) or desc (newest first)")] = SortOrder.DESC,
skip: Annotated[int, Query(description="Number of records to skip", ge=0)] = 0,
limit: Annotated[int, Query(description="Maximum number of records to return", ge=1, le=100)] = 100,
):
validate_and_extract_user_details(credentials.credentials)
return get_verses_of_day_list_service(group_id=group_id, filter_date=date, lang=lang, skip=skip, limit=limit)
return get_verses_of_day_list_service(group_id=group_id, filter_date=date, lang=lang, search=search, sort_order=sort_order, skip=skip, limit=limit)


@cms_verse_of_day_router.get(
Expand Down
2 changes: 1 addition & 1 deletion tests/plans/public/test_plan_public_service.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import pytest
from uuid import uuid4
from types import SimpleNamespace
from unittest.mock import patch, MagicMock, Mock, AsyncMock
from unittest.mock import patch, MagicMock, Mock, AsyncMock, call
from datetime import date as DateType, datetime, timedelta, timezone
from fastapi import HTTPException
from starlette import status
Expand Down
29 changes: 29 additions & 0 deletions tests/verse_of_day/test_verse_of_day_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
CreateVerseOfDayRequest,
UpdateVerseOfDayRequest,
)
from pecha_api.verse_of_day.verse_of_day_enums import SortOrder
from fastapi import HTTPException


Expand Down Expand Up @@ -813,6 +814,8 @@ async def test_get_verses_of_day_list_service_success(sample_verse_list, mock_db
mock_db_session.__enter__.return_value,
group_id=None,
filter_date=None,
search=None,
sort_order=SortOrder.DESC,
skip=0,
limit=100
)
Expand All @@ -833,6 +836,8 @@ async def test_get_verses_of_day_list_service_with_pagination(sample_verse_list,
mock_db_session.__enter__.return_value,
group_id=None,
filter_date=None,
search=None,
sort_order=SortOrder.DESC,
skip=5,
limit=20
)
Expand All @@ -855,6 +860,8 @@ async def test_get_verses_of_day_list_service_with_group_id_filter(sample_verse_
mock_db_session.__enter__.return_value,
group_id=group_id,
filter_date=None,
search=None,
sort_order=SortOrder.DESC,
skip=0,
limit=100
)
Expand All @@ -876,6 +883,28 @@ async def test_get_verses_of_day_list_service_with_date_filter(sample_verse_list
mock_db_session.__enter__.return_value,
group_id=None,
filter_date=filter_date,
search=None,
sort_order=SortOrder.DESC,
skip=0,
limit=100
)


@pytest.mark.asyncio
async def test_get_verses_of_day_list_service_with_search_and_sort_order(sample_verse_list, mock_db_session):
"""Test that search and sort_order are forwarded to the repository."""
with patch("pecha_api.verse_of_day.verse_of_day_service.SessionLocal", return_value=mock_db_session), \
patch("pecha_api.verse_of_day.verse_of_day_service.get_verses_of_day_list", return_value=(sample_verse_list, 2)) as mock_repo:

result = get_verses_of_day_list_service(search="compassion", sort_order=SortOrder.ASC)

assert isinstance(result, VerseOfDayListResponse)
mock_repo.assert_called_once_with(
mock_db_session.__enter__.return_value,
group_id=None,
filter_date=None,
search="compassion",
sort_order=SortOrder.ASC,
skip=0,
limit=100
)
Expand Down
41 changes: 38 additions & 3 deletions tests/verse_of_day/test_verse_of_day_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
VerseOfDayListResponse,
GroupInfoDTO,
)
from pecha_api.verse_of_day.verse_of_day_enums import SortOrder


client = TestClient(api)
Expand Down Expand Up @@ -819,7 +820,7 @@ async def test_cms_get_verse_of_day_list_success(sample_verse_list_response):
assert len(data["verses"]) == 1

mock_validate.assert_called_once_with("valid-token")
mock_service.assert_called_once_with(group_id=None, filter_date=None, lang=None, skip=0, limit=100)
mock_service.assert_called_once_with(group_id=None, filter_date=None, lang=None, search=None, sort_order=SortOrder.DESC, skip=0, limit=100)


@pytest.mark.asyncio
Expand All @@ -838,7 +839,7 @@ async def test_cms_get_verse_of_day_list_with_pagination(sample_verse_list_respo

assert response.status_code == status.HTTP_200_OK
mock_validate.assert_called_once_with("valid-token")
mock_service.assert_called_once_with(group_id=None, filter_date=None, lang=None, skip=10, limit=20)
mock_service.assert_called_once_with(group_id=None, filter_date=None, lang=None, search=None, sort_order=SortOrder.DESC, skip=10, limit=20)


@pytest.mark.asyncio
Expand All @@ -859,7 +860,41 @@ async def test_cms_get_verse_of_day_list_with_filters(sample_verse_list_response

assert response.status_code == status.HTTP_200_OK
mock_validate.assert_called_once_with("valid-token")
mock_service.assert_called_once_with(group_id=group_id, filter_date=filter_date, lang="en", skip=0, limit=100)
mock_service.assert_called_once_with(group_id=group_id, filter_date=filter_date, lang="en", search=None, sort_order=SortOrder.DESC, skip=0, limit=100)


@pytest.mark.asyncio
async def test_cms_get_verse_of_day_list_with_search_and_sort_order(sample_verse_list_response):
"""Test retrieval with search and ascending sort order."""
mock_user = MagicMock()
mock_user.email = "test@example.com"

with patch("pecha_api.verse_of_day.verse_of_day_views.validate_and_extract_user_details", return_value=mock_user) as mock_validate, \
patch("pecha_api.verse_of_day.verse_of_day_views.get_verses_of_day_list_service", return_value=sample_verse_list_response) as mock_service:

response = client.get(
"/cms/verse-of-day?search=compassion&sort_order=asc",
headers={"Authorization": "Bearer valid-token"}
)

assert response.status_code == status.HTTP_200_OK
mock_validate.assert_called_once_with("valid-token")
mock_service.assert_called_once_with(group_id=None, filter_date=None, lang=None, search="compassion", sort_order=SortOrder.ASC, skip=0, limit=100)


@pytest.mark.asyncio
async def test_cms_get_verse_of_day_list_invalid_sort_order():
"""Test that an invalid sort_order value is rejected."""
mock_user = MagicMock()
mock_user.email = "test@example.com"

with patch("pecha_api.verse_of_day.verse_of_day_views.validate_and_extract_user_details", return_value=mock_user):
response = client.get(
"/cms/verse-of-day?sort_order=sideways",
headers={"Authorization": "Bearer valid-token"}
)

assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY


@pytest.mark.asyncio
Expand Down
Loading