From 937d5ea944605f6a20800eea87bff3d00221a7ef Mon Sep 17 00:00:00 2001 From: Rajesh Kantipudi <44539669+iamrajeshk@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:26:22 +0530 Subject: [PATCH 1/3] Add updatedAt column to workspaces and implement project/member counts retrieval - Introduced updatedAt column in the workspaces table to track updates. - Added get_projects_counts and get_member_counts methods in TaskingProjectRepository and UserRepository respectively. - Updated WorkspaceResponse schema to include projectsCount and membersCount. - Enhanced get_my_workspaces endpoint to return project and member counts. - Added unit tests for new repository methods and updated existing tests for workspace response. --- .../d4e8f1a92b56_add_workspace_updated_at.py | 29 ++++++++++++ api/src/tasking/projects/repository.py | 21 +++++++++ api/src/users/repository.py | 20 ++++++++- api/src/workspaces/repository.py | 2 +- api/src/workspaces/routes.py | 30 ++++++++++++- api/src/workspaces/schemas.py | 16 +++++++ tests/integration/test_workspaces.py | 26 ++++++++--- tests/support/factories.py | 3 +- tests/unit/test_tasking_project_repository.py | 44 +++++++++++++++++++ tests/unit/test_user_repository.py | 42 ++++++++++++++++++ tests/unit/test_workspaces_schemas.py | 27 ++++++++++++ 11 files changed, 251 insertions(+), 9 deletions(-) create mode 100644 alembic_task/versions/d4e8f1a92b56_add_workspace_updated_at.py create mode 100644 tests/unit/test_tasking_project_repository.py create mode 100644 tests/unit/test_user_repository.py diff --git a/alembic_task/versions/d4e8f1a92b56_add_workspace_updated_at.py b/alembic_task/versions/d4e8f1a92b56_add_workspace_updated_at.py new file mode 100644 index 0000000..3216406 --- /dev/null +++ b/alembic_task/versions/d4e8f1a92b56_add_workspace_updated_at.py @@ -0,0 +1,29 @@ +"""Add updatedAt column to workspaces table + +Revision ID: d4e8f1a92b56 +Revises: b3f8a2c91e04 +Create Date: 2026-08-10 00:00:00.000000 + +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "d4e8f1a92b56" +down_revision: Union[str, None] = "b3f8a2c91e04" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # Nullable, no backfill: rows written before this column existed read + # back as None, and WorkspaceResponse.from_workspace (api/src/workspaces/ + # schemas.py) falls back to createdAt for those. New/updated rows get a + # real value from the model's default=/onupdate=datetime.now. + op.add_column("workspaces", sa.Column("updatedAt", sa.DateTime(), nullable=True)) + + +def downgrade() -> None: + op.drop_column("workspaces", "updatedAt") diff --git a/api/src/tasking/projects/repository.py b/api/src/tasking/projects/repository.py index 95875c1..87b0d4a 100644 --- a/api/src/tasking/projects/repository.py +++ b/api/src/tasking/projects/repository.py @@ -486,6 +486,27 @@ async def project_name_exists(self, workspace_id: int, name: str) -> bool: ) return int(result.scalar() or 0) > 0 + async def get_projects_counts(self, workspace_ids: list[int]) -> dict[int, int]: + if not workspace_ids: + return {} + query = ( + select( # pyright: ignore[reportCallIssue] + TaskingProject.workspace_id, + func.count(), # pyright: ignore[reportArgumentType] + ) + .where( + TaskingProject.workspace_id.in_( # pyright: ignore[reportAttributeAccessIssue] + workspace_ids + ) + & TaskingProject.deleted_at.is_( # pyright: ignore[reportAttributeAccessIssue, reportOptionalMemberAccess] + None + ) + ) + .group_by(TaskingProject.workspace_id) + ) + result = await self.session.execute(query) + return {wid: int(c) for wid, c in result.all()} + async def create( self, workspace_id: int, diff --git a/api/src/users/repository.py b/api/src/users/repository.py index d8177fd..5d9730e 100644 --- a/api/src/users/repository.py +++ b/api/src/users/repository.py @@ -1,6 +1,6 @@ from uuid import UUID -from sqlalchemy import delete, select +from sqlalchemy import delete, func, select from sqlalchemy.dialects.postgresql import insert as pg_insert from sqlmodel.ext.asyncio.session import AsyncSession @@ -45,6 +45,24 @@ async def get_privileged_workspace_members( for user, role in result.all() ] + async def get_member_counts(self, workspace_ids: list[int]) -> dict[int, int]: + if not workspace_ids: + return {} + query = ( + select( # pyright: ignore[reportCallIssue] + WorkspaceUserRole.workspace_id, + func.count(), # pyright: ignore[reportArgumentType] + ) + .where( + WorkspaceUserRole.workspace_id.in_( # pyright: ignore[reportAttributeAccessIssue] + workspace_ids + ) + ) + .group_by(WorkspaceUserRole.workspace_id) + ) + result = await self.session.execute(query) + return {wid: int(c) for wid, c in result.all()} + async def get_current_user(self, current_user: UserInfo) -> User: result = await self.session.exec( # pyright: ignore[reportCallIssue] select(User).where( diff --git a/api/src/workspaces/repository.py b/api/src/workspaces/repository.py index 90ac74f..1dc1941 100644 --- a/api/src/workspaces/repository.py +++ b/api/src/workspaces/repository.py @@ -1,4 +1,4 @@ -from sqlalchemy import delete, select, text, update +from sqlalchemy import delete, select, update from sqlalchemy.exc import IntegrityError from sqlmodel.ext.asyncio.session import AsyncSession diff --git a/api/src/workspaces/routes.py b/api/src/workspaces/routes.py index 430509c..1a79644 100644 --- a/api/src/workspaces/routes.py +++ b/api/src/workspaces/routes.py @@ -13,6 +13,7 @@ from api.core.security import UserInfo, evict_user_from_cache, validate_token from api.src.osm.repository import OSMRepository from api.src.osm.routes import get_osm_repo +from api.src.tasking.projects.repository import TaskingProjectRepository from api.src.users.repository import UserRepository from api.src.users.schemas import WorkspaceUserRoleType from api.src.workspaces.repository import WorkspaceRepository @@ -46,6 +47,12 @@ def get_user_repository( return UserRepository(session) +def get_project_repository( + session: AsyncSession = Depends(get_osm_session), +) -> TaskingProjectRepository: + return TaskingProjectRepository(session) + + # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this method properly calls the repository method to fetch the workspace and that the repository method properly fetches the workspace from the database # @test: Test that this method properly handles numeric workspace_id input and invalid values for the same @@ -59,11 +66,32 @@ def get_user_repository( @router.get("/mine", response_model=list[WorkspaceResponse]) async def get_my_workspaces( repository: WorkspaceRepository = Depends(get_workspace_repository), + user_repo: UserRepository = Depends(get_user_repository), + project_repo: TaskingProjectRepository = Depends(get_project_repository), current_user: UserInfo = Depends(validate_token), ) -> list[WorkspaceResponse]: try: workspaces = await repository.getAll(current_user) - return [WorkspaceResponse.from_workspace(ws, current_user) for ws in workspaces] + workspace_ids = [ws.id for ws in workspaces if ws.id is not None] + + # tasking_projects and user_workspace_roles both live in the OSM DB + # (see CLAUDE.md), so both counts come off the osm_session-bound + # repositories, not WorkspaceRepository's task session. + projects_counts = await project_repo.get_projects_counts(workspace_ids) + members_counts = await user_repo.get_member_counts(workspace_ids) + + responses = [] + for ws in workspaces: + assert ws.id is not None # persisted workspace always has an id + responses.append( + WorkspaceResponse.from_workspace( + ws, + current_user, + projects_count=projects_counts.get(ws.id, 0), + members_count=members_counts.get(ws.id, 0), + ) + ) + return responses except Exception as e: logger.error(f"Failed to fetch workspaces: {str(e)}") raise diff --git a/api/src/workspaces/schemas.py b/api/src/workspaces/schemas.py index 9614338..aa2bc0d 100644 --- a/api/src/workspaces/schemas.py +++ b/api/src/workspaces/schemas.py @@ -216,10 +216,13 @@ class WorkspaceResponse(SQLModel): createdAt: datetime createdBy: UUID createdByName: str + updatedAt: datetime externalAppAccess: ExternalAppsDefinitionType kartaViewToken: Optional[str] = None autoFlagReview: bool = False role: str + projectsCount: int = 0 + membersCount: int = 0 # Included in single-workspace GET for mobile app consumption. TODO: remove # this when the app fetches these from dedicated endpoints: longFormQuestDef: Optional[Any] = None @@ -236,6 +239,8 @@ def from_workspace( *, imagery_list_def: Any = None, long_form_quest_def: Any = None, + projects_count: int = 0, + members_count: int = 0, ) -> Self: assert workspace.id is not None # persisted workspace always has an id return cls( @@ -250,10 +255,13 @@ def from_workspace( createdAt=workspace.createdAt, createdBy=workspace.createdBy, createdByName=workspace.createdByName, + updatedAt=workspace.updatedAt or workspace.createdAt, externalAppAccess=workspace.externalAppAccess, kartaViewToken=workspace.kartaViewToken, autoFlagReview=workspace.autoFlagReview, role=user.effective_role(workspace.id), + projectsCount=projects_count, + membersCount=members_count, imageryListDef=imagery_list_def, longFormQuestDef=long_form_quest_def, ) @@ -286,6 +294,14 @@ class Workspace(SQLModel, table=True): createdBy: UUID createdByName: str + # Nullable so that adding this column never requires a data backfill: rows + # written before this column existed simply read back as None, and + # WorkspaceResponse.from_workspace falls back to createdAt for those. + updatedAt: Optional[datetime] = Field( + default=None, + sa_column=Column(nullable=True, default=datetime.now, onupdate=datetime.now), + ) + geometry: Optional[Any] = Field( default=None, sa_column=Column(Geometry("MULTIPOLYGON", srid=4326)) ) diff --git a/tests/integration/test_workspaces.py b/tests/integration/test_workspaces.py index fbd30cb..49a9a79 100644 --- a/tests/integration/test_workspaces.py +++ b/tests/integration/test_workspaces.py @@ -52,13 +52,20 @@ def evictions(monkeypatch): # === GET /mine ============================================================= -async def test_list_my_workspaces(client, login, task_session): +async def test_list_my_workspaces(client, login, task_session, osm_session): login() task_session.queue( fakes.rows( factories.make_workspace(id=1, title="One"), factories.make_workspace(id=2, title="Two"), - ) + ), + ) + # tasking_projects and user_workspace_roles both live in the OSM DB, so + # both batched counts are queued on osm_session, in the order the route + # calls them: projects counts, then member counts. + osm_session.queue( + fakes.rows((1, 4), (2, 2)), # get_projects_counts + fakes.rows((1, 3), (2, 1)), # get_member_counts ) response = await client.get(f"{API}/mine") @@ -67,6 +74,11 @@ async def test_list_my_workspaces(client, login, task_session): body = response.json() assert [w["id"] for w in body] == [1, 2] assert body[0]["role"] == "contributor" + assert body[0]["projectsCount"] == 4 + assert body[0]["membersCount"] == 3 + assert body[1]["projectsCount"] == 2 + assert body[1]["membersCount"] == 1 + assert "updatedAt" in body[0] async def test_list_my_workspaces_empty(client, login, task_session): @@ -86,14 +98,18 @@ async def test_list_my_workspaces_unexpected_error_500( assert response.status_code == 500 -async def test_list_matches_get_by_id(client, login, task_session): +async def test_list_matches_get_by_id(client, login, task_session, osm_session): # The same workspace serialized via /mine and via /{id} agree on shared fields. login( factories.make_user_info(osm_workspace_roles={1: [WorkspaceUserRoleType.LEAD]}) ) task_session.queue( - fakes.rows(factories.make_workspace(id=1, title="Shared")), - fakes.rows(factories.make_workspace(id=1, title="Shared")), + fakes.rows(factories.make_workspace(id=1, title="Shared")), # /mine getAll + fakes.rows(factories.make_workspace(id=1, title="Shared")), # /1 getById + ) + osm_session.queue( + fakes.rows((1, 0)), # get_projects_counts for /mine + fakes.rows((1, 0)), # get_member_counts for /mine ) listed = (await client.get(f"{API}/mine")).json()[0] diff --git a/tests/support/factories.py b/tests/support/factories.py index 9cdd402..480a80a 100644 --- a/tests/support/factories.py +++ b/tests/support/factories.py @@ -70,6 +70,8 @@ def make_workspace( created_by_name: str = "Test User", **extra, ) -> Workspace: + extra.setdefault("createdAt", datetime(2026, 1, 1)) + extra.setdefault("updatedAt", datetime(2026, 1, 1)) return Workspace( id=id, title=title, @@ -77,7 +79,6 @@ def make_workspace( tdeiProjectGroupId=UUID(tdei_project_group_id), createdBy=UUID(created_by), createdByName=created_by_name, - createdAt=datetime(2026, 1, 1), **extra, ) diff --git a/tests/unit/test_tasking_project_repository.py b/tests/unit/test_tasking_project_repository.py new file mode 100644 index 0000000..9f316be --- /dev/null +++ b/tests/unit/test_tasking_project_repository.py @@ -0,0 +1,44 @@ +"""Unit tests for TaskingProjectRepository against a fake session. + +See tests/unit/test_workspace_repository.py for the pattern this follows: +queue the rows the DB "would" return, then assert on the repository's +behavior. TaskingProjectRepository runs on the OSM DB session, since +tasking_projects lives there (see CLAUDE.md). +""" + +from typing import cast + +from sqlmodel.ext.asyncio.session import AsyncSession + +from api.src.tasking.projects.repository import TaskingProjectRepository +from tests.support import fakes + + +def _repo(session: fakes.FakeSession) -> TaskingProjectRepository: + return TaskingProjectRepository(cast(AsyncSession, session)) + + +async def test_get_projects_counts_returns_map(): + session = fakes.FakeSession(fakes.rows((1, 3), (2, 1))) + + result = await _repo(session).get_projects_counts([1, 2]) + + assert result == {1: 3, 2: 1} + + +async def test_get_projects_counts_empty_ids_short_circuits(): + # A queued exception proves the session is never touched for an empty id list. + session = fakes.FakeSession(fakes.raises(RuntimeError("should not query"))) + + result = await _repo(session).get_projects_counts([]) + + assert result == {} + + +async def test_get_projects_counts_omits_ids_with_no_projects(): + session = fakes.FakeSession(fakes.rows((1, 2))) + + result = await _repo(session).get_projects_counts([1, 2]) + + assert result == {1: 2} + assert result.get(2, 0) == 0 diff --git a/tests/unit/test_user_repository.py b/tests/unit/test_user_repository.py new file mode 100644 index 0000000..4a51abb --- /dev/null +++ b/tests/unit/test_user_repository.py @@ -0,0 +1,42 @@ +"""Unit tests for UserRepository against a fake session. + +See tests/unit/test_workspace_repository.py for the pattern this follows: +queue the rows the DB "would" return, then assert on the repository's +behavior. +""" + +from typing import cast + +from sqlmodel.ext.asyncio.session import AsyncSession + +from api.src.users.repository import UserRepository +from tests.support import fakes + + +def _repo(session: fakes.FakeSession) -> UserRepository: + return UserRepository(cast(AsyncSession, session)) + + +async def test_get_member_counts_returns_map(): + session = fakes.FakeSession(fakes.rows((1, 2), (2, 1))) + + result = await _repo(session).get_member_counts([1, 2]) + + assert result == {1: 2, 2: 1} + + +async def test_get_member_counts_empty_ids_short_circuits(): + session = fakes.FakeSession(fakes.raises(RuntimeError("should not query"))) + + result = await _repo(session).get_member_counts([]) + + assert result == {} + + +async def test_get_member_counts_omits_ids_with_no_members(): + session = fakes.FakeSession(fakes.rows((1, 3))) + + result = await _repo(session).get_member_counts([1, 2]) + + assert result == {1: 3} + assert result.get(2, 0) == 0 diff --git a/tests/unit/test_workspaces_schemas.py b/tests/unit/test_workspaces_schemas.py index 15f6266..0d5400c 100644 --- a/tests/unit/test_workspaces_schemas.py +++ b/tests/unit/test_workspaces_schemas.py @@ -61,6 +61,7 @@ def test_workspace_table_schema(): "createdAt", "createdBy", "createdByName", + "updatedAt", "geometry", "externalAppAccess", "kartaViewToken", @@ -138,6 +139,7 @@ def test_workspace_preserves_uuid_and_datetime(): createdBy=pg, createdByName="N", createdAt=created, + updatedAt=created, ) assert ws.tdeiProjectGroupId == pg assert ws.createdAt == created # no truncation @@ -159,6 +161,31 @@ def test_workspace_response_includes_effective_role(): assert resp.title == "Mappy" assert resp.role == WorkspaceUserRoleType.LEAD assert resp.type == WorkspaceType.OSW + assert resp.updatedAt == ws.updatedAt + assert resp.projectsCount == 0 + assert resp.membersCount == 0 + + +def test_workspace_response_includes_counts(): + user = factories.make_user_info() + ws = factories.make_workspace(id=3) + + resp = WorkspaceResponse.from_workspace(ws, user, projects_count=5, members_count=2) + + assert resp.projectsCount == 5 + assert resp.membersCount == 2 + + +def test_workspace_response_updated_at_falls_back_to_created_at(): + # Rows written before the updatedAt column existed read back as None -- + # the response should report createdAt for those rather than null. + user = factories.make_user_info() + created = datetime(2026, 1, 2, 3, 4, 5) + ws = factories.make_workspace(id=4, updatedAt=None, createdAt=created) + + resp = WorkspaceResponse.from_workspace(ws, user) + + assert resp.updatedAt == created def test_workspace_response_passes_through_defs(): From a7113f027205623ebdcce1893cf239b0d0366451 Mon Sep 17 00:00:00 2001 From: Rajesh Kantipudi <44539669+iamrajeshk@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:36:31 +0530 Subject: [PATCH 2/3] Remove unnecessary comments regarding project and member counts retrieval in get_my_workspaces function --- api/src/workspaces/routes.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/api/src/workspaces/routes.py b/api/src/workspaces/routes.py index 1a79644..e13ea1d 100644 --- a/api/src/workspaces/routes.py +++ b/api/src/workspaces/routes.py @@ -74,9 +74,6 @@ async def get_my_workspaces( workspaces = await repository.getAll(current_user) workspace_ids = [ws.id for ws in workspaces if ws.id is not None] - # tasking_projects and user_workspace_roles both live in the OSM DB - # (see CLAUDE.md), so both counts come off the osm_session-bound - # repositories, not WorkspaceRepository's task session. projects_counts = await project_repo.get_projects_counts(workspace_ids) members_counts = await user_repo.get_member_counts(workspace_ids) From 38c47d6d647bcb118f41fcabecd06bd80a123511 Mon Sep 17 00:00:00 2001 From: Rajesh Kantipudi <44539669+iamrajeshk@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:44:56 +0530 Subject: [PATCH 3/3] Refactor query to improve readability by aligning function calls in TaskingProjectRepository and UserRepository --- api/src/tasking/projects/repository.py | 4 ++-- api/src/users/repository.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/src/tasking/projects/repository.py b/api/src/tasking/projects/repository.py index 87b0d4a..017e20a 100644 --- a/api/src/tasking/projects/repository.py +++ b/api/src/tasking/projects/repository.py @@ -491,8 +491,8 @@ async def get_projects_counts(self, workspace_ids: list[int]) -> dict[int, int]: return {} query = ( select( # pyright: ignore[reportCallIssue] - TaskingProject.workspace_id, - func.count(), # pyright: ignore[reportArgumentType] + TaskingProject.workspace_id, # pyright: ignore[reportArgumentType] + func.count(), ) .where( TaskingProject.workspace_id.in_( # pyright: ignore[reportAttributeAccessIssue] diff --git a/api/src/users/repository.py b/api/src/users/repository.py index 5d9730e..7b9c6f1 100644 --- a/api/src/users/repository.py +++ b/api/src/users/repository.py @@ -50,8 +50,8 @@ async def get_member_counts(self, workspace_ids: list[int]) -> dict[int, int]: return {} query = ( select( # pyright: ignore[reportCallIssue] - WorkspaceUserRole.workspace_id, - func.count(), # pyright: ignore[reportArgumentType] + WorkspaceUserRole.workspace_id, # pyright: ignore[reportArgumentType] + func.count(), ) .where( WorkspaceUserRole.workspace_id.in_( # pyright: ignore[reportAttributeAccessIssue]