diff --git a/backend/app/repositories/filter_validator.py b/backend/app/repositories/filter_validator.py new file mode 100644 index 0000000..bf276dd --- /dev/null +++ b/backend/app/repositories/filter_validator.py @@ -0,0 +1,24 @@ +"""Validates repository list() filters to catch typos and unsupported filter names. + +This helper raises ValueError when a caller passes a filter name the repository +doesn't support, rather than silently ignoring it (which would return unfiltered data). +""" + + +def validate_filters(provided_filters: dict[str, object], supported_filters: set[str]) -> None: + """Raise ValueError if any provided filter is not supported. + + Args: + provided_filters: dict of filters passed to list() + supported_filters: set of filter names the repository recognizes + + Raises: + ValueError: if any provided_filters key is not in supported_filters + """ + unsupported = set(provided_filters.keys()) - supported_filters + if unsupported: + unsupported_list = sorted(unsupported) + supported_list = sorted(supported_filters) + raise ValueError( + f"Unsupported filter(s): {unsupported_list}. This repository supports: {supported_list}" + ) diff --git a/backend/app/repositories_impl/postgres/vacation_days_repository.py b/backend/app/repositories_impl/postgres/vacation_days_repository.py index bc5570a..663eb27 100644 --- a/backend/app/repositories_impl/postgres/vacation_days_repository.py +++ b/backend/app/repositories_impl/postgres/vacation_days_repository.py @@ -7,6 +7,7 @@ All methods raise NotImplementedError until then. from uuid import UUID from app.entities.vacation_days import VacationDays +from app.repositories.filter_validator import validate_filters from app.repositories.vacation_days_repository import VacationDaysRepository @@ -27,6 +28,7 @@ class PostgresVacationDaysRepository(VacationDaysRepository): raise NotImplementedError("Postgres repo not yet implemented") async def list(self, **filters: object) -> list[VacationDays]: + validate_filters(filters, {"workspace_id", "personal_data_uuid", "year"}) raise NotImplementedError("Postgres repo not yet implemented") async def get_by_parent_year(self, personal_data_uuid: UUID, year: int) -> VacationDays | None: diff --git a/backend/app/repositories_impl/sqlite/child_repository.py b/backend/app/repositories_impl/sqlite/child_repository.py index e548bb3..89f94b9 100644 --- a/backend/app/repositories_impl/sqlite/child_repository.py +++ b/backend/app/repositories_impl/sqlite/child_repository.py @@ -7,6 +7,7 @@ import aiosqlite from app.entities.child import Child from app.repositories.child_repository import ChildRepository +from app.repositories.filter_validator import validate_filters from app.utils.errors import NotFoundError @@ -87,6 +88,7 @@ class SQLiteChildRepository(ChildRepository): raise NotFoundError(f"Child {id} not found") async def list(self, **filters: object) -> list[Child]: + validate_filters(filters, {"workspace_id"}) clauses: list[str] = [] params: list[object] = [] if "workspace_id" in filters and filters["workspace_id"] is not None: diff --git a/backend/app/repositories_impl/sqlite/day_override_repository.py b/backend/app/repositories_impl/sqlite/day_override_repository.py index d669abf..64e4741 100644 --- a/backend/app/repositories_impl/sqlite/day_override_repository.py +++ b/backend/app/repositories_impl/sqlite/day_override_repository.py @@ -12,6 +12,7 @@ import aiosqlite from app.entities.day_override import DayOverride from app.repositories.day_override_repository import DayOverrideRepository +from app.repositories.filter_validator import validate_filters from app.utils.errors import NotFoundError @@ -91,6 +92,7 @@ class SQLiteDayOverrideRepository(DayOverrideRepository): raise NotFoundError(f"DayOverride {id} not found") async def list(self, **filters: object) -> list[DayOverride]: + validate_filters(filters, {"workspace_id"}) clauses: list[str] = [] params: list[object] = [] for column in ("workspace_id",): diff --git a/backend/app/repositories_impl/sqlite/leave_wish_repository.py b/backend/app/repositories_impl/sqlite/leave_wish_repository.py index 1c3a40f..8a5ce25 100644 --- a/backend/app/repositories_impl/sqlite/leave_wish_repository.py +++ b/backend/app/repositories_impl/sqlite/leave_wish_repository.py @@ -10,6 +10,7 @@ from uuid import UUID import aiosqlite from app.entities.leave_wish import LeaveWish +from app.repositories.filter_validator import validate_filters from app.repositories.leave_wish_repository import LeaveWishRepository from app.utils.errors import NotFoundError @@ -154,6 +155,7 @@ class SQLiteLeaveWishRepository(LeaveWishRepository): raise NotFoundError(f"LeaveWish {id} not found") async def list(self, **filters: object) -> list[LeaveWish]: + validate_filters(filters, {"workspace_id", "child_id"}) clauses: list[str] = [] params: list[object] = [] for column in ("workspace_id", "child_id"): diff --git a/backend/app/repositories_impl/sqlite/parental_leave_repository.py b/backend/app/repositories_impl/sqlite/parental_leave_repository.py index e3a09d2..cdb854d 100644 --- a/backend/app/repositories_impl/sqlite/parental_leave_repository.py +++ b/backend/app/repositories_impl/sqlite/parental_leave_repository.py @@ -7,6 +7,7 @@ from uuid import UUID import aiosqlite from app.entities.parental_leave import ParentalLeaveFetch, TierData +from app.repositories.filter_validator import validate_filters from app.repositories.parental_leave_repository import ParentalLeaveRepository from app.utils.errors import NotFoundError @@ -165,6 +166,7 @@ class SQLiteParentalLeaveRepository(ParentalLeaveRepository): raise NotFoundError(f"ParentalLeaveFetch {id} not found") async def list(self, **filters: object) -> list[ParentalLeaveFetch]: + validate_filters(filters, {"personal_data_uuid", "workspace_id", "child_id"}) clauses: list[str] = [] params: list[object] = [] for column in ("personal_data_uuid", "workspace_id", "child_id"): diff --git a/backend/app/repositories_impl/sqlite/personal_data_repository.py b/backend/app/repositories_impl/sqlite/personal_data_repository.py index 4e6069f..95dd770 100644 --- a/backend/app/repositories_impl/sqlite/personal_data_repository.py +++ b/backend/app/repositories_impl/sqlite/personal_data_repository.py @@ -10,6 +10,7 @@ from uuid import UUID import aiosqlite from app.entities.personal_data import PersonalData +from app.repositories.filter_validator import validate_filters from app.repositories.personal_data_repository import PersonalDataRepository from app.utils.errors import NotFoundError @@ -108,14 +109,19 @@ class SQLitePersonalDataRepository(PersonalDataRepository): raise NotFoundError(f"PersonalData {id} not found") async def list(self, **filters: object) -> list[PersonalData]: - clauses = ["deleted_at IS NULL"] + validate_filters(filters, {"workspace_id", "include_deleted"}) + clauses: list[str] = [] params: list[object] = [] + include_deleted = filters.get("include_deleted", False) + if not include_deleted: + clauses.append("deleted_at IS NULL") for column in ("workspace_id",): if column in filters and filters[column] is not None: clauses.append(f"{column} = ?") params.append(str(filters[column])) - where = "WHERE " + " AND ".join(clauses) - cursor = await self._conn.execute(f"SELECT * FROM personal_data {where}", params) + where = "WHERE " + " AND ".join(clauses) if clauses else "" + query = f"SELECT * FROM personal_data {where}" if where else "SELECT * FROM personal_data" + cursor = await self._conn.execute(query, params) rows = await cursor.fetchall() return [self._row_to_entity(r) for r in rows] diff --git a/backend/app/repositories_impl/sqlite/planned_leave_repository.py b/backend/app/repositories_impl/sqlite/planned_leave_repository.py index 7150734..e8a2a93 100644 --- a/backend/app/repositories_impl/sqlite/planned_leave_repository.py +++ b/backend/app/repositories_impl/sqlite/planned_leave_repository.py @@ -6,6 +6,7 @@ from uuid import UUID import aiosqlite from app.entities.planned_leave import PlannedLeaveDay +from app.repositories.filter_validator import validate_filters from app.repositories.planned_leave_repository import PlannedLeaveRepository from app.utils.errors import NotFoundError @@ -85,6 +86,7 @@ class SQLitePlannedLeaveRepository(PlannedLeaveRepository): raise NotFoundError(f"PlannedLeaveDay {id} not found") async def list(self, **filters: object) -> list[PlannedLeaveDay]: + validate_filters(filters, {"workspace_id", "child_id", "parent_personal_data_uuid"}) clauses: list[str] = [] params: list[object] = [] for column in ("workspace_id", "child_id", "parent_personal_data_uuid"): diff --git a/backend/app/repositories_impl/sqlite/user_repository.py b/backend/app/repositories_impl/sqlite/user_repository.py index 3ebf3fb..4430134 100644 --- a/backend/app/repositories_impl/sqlite/user_repository.py +++ b/backend/app/repositories_impl/sqlite/user_repository.py @@ -6,6 +6,7 @@ from uuid import UUID import aiosqlite from app.entities.user import User +from app.repositories.filter_validator import validate_filters from app.repositories.user_repository import UserRepository from app.utils.errors import NotFoundError @@ -72,6 +73,7 @@ class SQLiteUserRepository(UserRepository): raise NotFoundError(f"User {id} not found") async def list(self, **filters: object) -> list[User]: + validate_filters(filters, set()) cursor = await self._conn.execute('SELECT * FROM "user"') rows = await cursor.fetchall() return [self._row_to_entity(r) for r in rows] diff --git a/backend/app/repositories_impl/sqlite/vacation_days_repository.py b/backend/app/repositories_impl/sqlite/vacation_days_repository.py index 72b280c..050e588 100644 --- a/backend/app/repositories_impl/sqlite/vacation_days_repository.py +++ b/backend/app/repositories_impl/sqlite/vacation_days_repository.py @@ -13,6 +13,7 @@ from uuid import UUID import aiosqlite from app.entities.vacation_days import VacationDays +from app.repositories.filter_validator import validate_filters from app.repositories.vacation_days_repository import VacationDaysRepository from app.utils.errors import NotFoundError @@ -96,6 +97,7 @@ class SQLiteVacationDaysRepository(VacationDaysRepository): raise NotFoundError(f"VacationDays {id} not found") async def list(self, **filters: object) -> list[VacationDays]: + validate_filters(filters, {"workspace_id", "personal_data_uuid", "year"}) clauses: list[str] = [] params: list[object] = [] for column in ("workspace_id", "personal_data_uuid", "year"): diff --git a/backend/app/repositories_impl/sqlite/workspace_repository.py b/backend/app/repositories_impl/sqlite/workspace_repository.py index 199b455..d8aab90 100644 --- a/backend/app/repositories_impl/sqlite/workspace_repository.py +++ b/backend/app/repositories_impl/sqlite/workspace_repository.py @@ -7,6 +7,7 @@ import aiosqlite from app.entities.workspace import Workspace from app.entities.workspace_invitation import WorkspaceInvitation +from app.repositories.filter_validator import validate_filters from app.repositories.workspace_repository import ( WorkspaceInvitationRepository, WorkspaceRepository, @@ -70,6 +71,7 @@ class SQLiteWorkspaceRepository(WorkspaceRepository): raise NotFoundError(f"Workspace {id} not found") async def list(self, **filters: object) -> list[Workspace]: + validate_filters(filters, set()) cursor = await self._conn.execute("SELECT * FROM workspace") rows = await cursor.fetchall() return [self._row_to_entity(r) for r in rows] @@ -157,6 +159,7 @@ class SQLiteWorkspaceInvitationRepository(WorkspaceInvitationRepository): raise NotFoundError(f"WorkspaceInvitation {id} not found") async def list(self, **filters: object) -> list[WorkspaceInvitation]: + validate_filters(filters, {"workspace_id", "status"}) clauses: list[str] = [] params: list[object] = [] if "workspace_id" in filters and filters["workspace_id"] is not None: diff --git a/backend/tests/unit/test_leave_wish_repository.py b/backend/tests/unit/test_leave_wish_repository.py index 2d705ca..e812fc1 100644 --- a/backend/tests/unit/test_leave_wish_repository.py +++ b/backend/tests/unit/test_leave_wish_repository.py @@ -287,6 +287,47 @@ class TestSQLiteLeaveWishRepository: assert tied.id in ids assert family_wide.id not in ids + async def test_list_rejects_unsupported_filter( + self, + leave_wish_repo: SQLiteLeaveWishRepository, + seeded_ids: SeededIds, + ) -> None: + """Passing an unsupported filter (like a typo) must raise ValueError, not return all rows.""" + with pytest.raises(ValueError, match="parent_personal_data_uuid"): + await leave_wish_repo.list(parent_personal_data_uuid=seeded_ids.personal_data_uuid) + + # Verify the error message lists what filters ARE supported + with pytest.raises(ValueError, match="child_id.*workspace_id"): + await leave_wish_repo.list(parent_personal_data_uuid=seeded_ids.personal_data_uuid) + + async def test_list_with_supported_filter_still_works( + self, + leave_wish_repo: SQLiteLeaveWishRepository, + seeded_ids: SeededIds, + ) -> None: + """Verify filter validation doesn't break existing filtering behavior.""" + ws = seeded_ids.workspace_id + created_1 = await leave_wish_repo.create(_make(workspace_id=ws, name="W1")) + created_2 = await leave_wish_repo.create(_make(workspace_id=ws, name="W2")) + + rows = await leave_wish_repo.list(workspace_id=ws) + assert len(rows) >= 2 + ids = {r.id for r in rows} + assert created_1.id in ids + assert created_2.id in ids + + async def test_list_rejects_multiple_unsupported_filters( + self, + leave_wish_repo: SQLiteLeaveWishRepository, + seeded_ids: SeededIds, + ) -> None: + """Error message should list all unsupported filters.""" + with pytest.raises(ValueError, match="parent_personal_data_uuid.*typo_filter"): + await leave_wish_repo.list( + parent_personal_data_uuid=seeded_ids.personal_data_uuid, + typo_filter="value", + ) + class TestLeaveWishService: """Test LeaveWishService business logic (validation).""" diff --git a/backend/tests/unit/test_personal_data_repository.py b/backend/tests/unit/test_personal_data_repository.py index 1120674..8fcb3a2 100644 --- a/backend/tests/unit/test_personal_data_repository.py +++ b/backend/tests/unit/test_personal_data_repository.py @@ -9,6 +9,8 @@ this file targets the new column's round trip specifically. from datetime import datetime from uuid import uuid4 +import pytest + from app.entities.personal_data import PersonalData from app.entities.workspace import Workspace from app.repositories_impl.sqlite.personal_data_repository import SQLitePersonalDataRepository @@ -75,3 +77,43 @@ class TestPersonalDataMinVacationDays: fetched = await personal_data_repo.get_by_id(created.uuid) assert fetched is not None assert fetched.min_vacation_days_per_year == 15 + + async def test_list_rejects_unsupported_filter( + self, + personal_data_repo: SQLitePersonalDataRepository, + ) -> None: + """Passing an unsupported filter must raise ValueError.""" + with pytest.raises(ValueError, match="invalid_column"): + await personal_data_repo.list(invalid_column="value") + + async def test_list_accepts_include_deleted_filter( + self, + personal_data_repo: SQLitePersonalDataRepository, + workspace_repo: SQLiteWorkspaceRepository, + ) -> None: + """The SoftDeleteRepository contract requires list() to accept include_deleted=True.""" + ws = await workspace_repo.create( + Workspace(id=uuid4(), name="WS", created_at=datetime.now()) + ) + pd = await personal_data_repo.create( + PersonalData(uuid=uuid4(), workspace_id=ws.id, name="Parent", created_at=datetime.now()) + ) + + # By default, deleted items should not be listed + await personal_data_repo.soft_delete(pd.uuid) + rows = await personal_data_repo.list(workspace_id=ws.id) + ids = {r.uuid for r in rows} + assert pd.uuid not in ids + + # With include_deleted=True, deleted items should be included + rows_with_deleted = await personal_data_repo.list(workspace_id=ws.id, include_deleted=True) + ids_with_deleted = {r.uuid for r in rows_with_deleted} + assert pd.uuid in ids_with_deleted + + async def test_list_error_message_lists_supported_filters( + self, + personal_data_repo: SQLitePersonalDataRepository, + ) -> None: + """Error message should list supported filters.""" + with pytest.raises(ValueError, match="include_deleted.*workspace_id"): + await personal_data_repo.list(bad_filter="value") diff --git a/backend/tests/unit/test_vacation_days_repository.py b/backend/tests/unit/test_vacation_days_repository.py index 630473d..00d0e3c 100644 --- a/backend/tests/unit/test_vacation_days_repository.py +++ b/backend/tests/unit/test_vacation_days_repository.py @@ -134,6 +134,22 @@ class TestSQLiteVacationDaysRepository: assert len(rows) == 2 assert [r.year for r in rows] == [2026, 2025] # ordered year DESC + async def test_list_rejects_unsupported_filter( + self, + vacation_repo: SQLiteVacationDaysRepository, + ) -> None: + """Passing an unsupported filter must raise ValueError.""" + with pytest.raises(ValueError, match="invalid_filter"): + await vacation_repo.list(invalid_filter="value") + + async def test_list_error_message_lists_supported_filters( + self, + vacation_repo: SQLiteVacationDaysRepository, + ) -> None: + """Error message should list all supported filters (sorted).""" + with pytest.raises(ValueError, match="personal_data_uuid.*workspace_id.*year"): + await vacation_repo.list(bad_filter="value") + class TestVacationDaysService: async def test_upsert_creates_then_updates(