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: 4 additions & 2 deletions api/src/workspaces/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,13 @@ async def getAll(
select(Workspace).where(
Workspace.tdeiProjectGroupId.in_(current_user.getProjectGroupIds()) # type: ignore[attr-defined]
)
# The list response sends none of these, so don't load them: the
# geometry can be a large multipolygon, and the quest and imagery
# The list response (WorkspaceSummaryResponse) sends none of these,
# so don't load them: the geometry can be a large multipolygon, the
# TDEI metadata a whole dataset record, and the quest and imagery
# definitions are otherwise joined in for every workspace.
.options(
defer(Workspace.geometry), # pyright: ignore[reportArgumentType]
defer(Workspace.tdeiMetadata), # pyright: ignore[reportArgumentType]
noload(
Workspace.longFormQuestDef # pyright: ignore[reportArgumentType]
),
Expand Down
9 changes: 5 additions & 4 deletions api/src/workspaces/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
WorkspaceNameCheckResponse,
WorkspacePatch,
WorkspaceResponse,
WorkspaceSummaryResponse,
)

# Set up logger for this module
Expand Down Expand Up @@ -108,7 +109,7 @@ async def save_uploaded_dataset(workspace_id: int, upload: UploadFile) -> Path:
# @test: Test that this method properly handles numeric workspace_id input and invalid values for the same
# @test: Test that this method properly checks permissions to see if the user has access to the workspace and if they don't, it doesn't appear in the list
# @test: Test that this method properly handles the case where the workspace does not exist and doesn't include it
# @test: Test that this method properly handles inputs that match the schema in WorkspaceResponse
# @test: Test that this method properly handles inputs that match the schema in WorkspaceSummaryResponse, and that no item carries tdeiMetadata or kartaViewToken
# @test: Test that this method's results match the values of the fetch-by-workspace-id method below; all workspaces in this list are retrievable via that method
# @test: Test that gig_only=true returns only OSW workspaces with external app access, and that lat, lon and radius are accepted and ignored

Expand All @@ -118,7 +119,7 @@ async def save_uploaded_dataset(workspace_id: int, upload: UploadFile) -> Path:
# The mobile apps also send lat, lon and radius. Those are deliberately not
# applied: the apps get every workspace in the user's project groups, wherever
# it is. FastAPI ignores undeclared query parameters, so they need no handling.
@router.get("/mine", response_model=list[WorkspaceResponse])
@router.get("/mine", response_model=list[WorkspaceSummaryResponse])
async def get_my_workspaces(
gig_only: bool = Query(
False,
Expand All @@ -129,7 +130,7 @@ async def get_my_workspaces(
user_repo: UserRepository = Depends(get_user_repository),
project_repo: TaskingProjectRepository = Depends(get_project_repository),
current_user: UserInfo = Depends(validate_token),
) -> list[WorkspaceResponse]:
) -> list[WorkspaceSummaryResponse]:
try:
workspaces = await repository.getAll(current_user, gig_only=gig_only)
workspace_ids = [ws.id for ws in workspaces if ws.id is not None]
Expand All @@ -141,7 +142,7 @@ async def get_my_workspaces(
for ws in workspaces:
assert ws.id is not None # persisted workspace always has an id
responses.append(
WorkspaceResponse.from_workspace(
WorkspaceSummaryResponse.from_workspace(
ws,
current_user,
projects_count=projects_counts.get(ws.id, 0),
Expand Down
83 changes: 61 additions & 22 deletions api/src/workspaces/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,10 +268,13 @@ class ImagerySettingsPatch(SQLModel):
definition: Optional[list[Any]] = None


class WorkspaceResponse(SQLModel):
class WorkspaceSummaryResponse(SQLModel):
"""
Workspace serialized for API responses. Includes the effective role for the
user making the request.
A workspace as listed by ``GET /workspaces/mine``. Includes the effective
role for the user making the request. Leaves out what only the
single-workspace GET needs (see `WorkspaceResponse`), above all
``tdeiMetadata``, which for a workspace created from a TDEI dataset holds
the whole dataset record and made the list large.
"""

id: int
Expand All @@ -281,62 +284,98 @@ class WorkspaceResponse(SQLModel):
tdeiProjectGroupId: UUID
tdeiRecordId: Optional[UUID] = None
tdeiServiceId: Optional[UUID] = None
tdeiMetadata: Optional[Any] = None
createdAt: datetime
createdBy: UUID
createdByName: str
updatedAt: datetime
externalAppAccess: ExternalAppsDefinitionType
kartaViewToken: Optional[str] = None
autoFlagReview: bool = False
overrideConflicts: 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
imageryListDef: Optional[Any] = None
importStatus: Optional[str] = None

# @test: Test that this class properly serializes the workspace data for API responses, including the effective role for the user making the request
# @test: Test that the values are populated in this class match the expected values from the database and that the relationships are correctly serialized
# @test: Test that this class serializes the listed fields, including the effective role for the user making the request, and has no tdeiMetadata, kartaViewToken, longFormQuestDef or imageryListDef

@classmethod
def from_workspace(
def _summary_fields(
cls,
workspace: "Workspace",
user: "UserInfo",
*,
imagery_list_def: Any = None,
long_form_quest_def: Any = None,
projects_count: int = 0,
members_count: int = 0,
) -> Self:
projects_count: int,
members_count: int,
) -> dict[str, Any]:
assert workspace.id is not None # persisted workspace always has an id
return cls(
return dict(
id=workspace.id,
type=workspace.type,
title=workspace.title,
description=workspace.description,
tdeiProjectGroupId=workspace.tdeiProjectGroupId,
tdeiRecordId=workspace.tdeiRecordId,
tdeiServiceId=workspace.tdeiServiceId,
tdeiMetadata=workspace.tdeiMetadata,
createdAt=workspace.createdAt,
createdBy=workspace.createdBy,
createdByName=workspace.createdByName,
updatedAt=workspace.updatedAt or workspace.createdAt,
externalAppAccess=workspace.externalAppAccess,
kartaViewToken=workspace.kartaViewToken,
autoFlagReview=workspace.autoFlagReview,
overrideConflicts=workspace.overrideConflicts,
role=user.effective_role(workspace.id),
projectsCount=projects_count,
membersCount=members_count,
importStatus=workspace.importStatus,
)

@classmethod
def from_workspace(
cls,
workspace: "Workspace",
user: "UserInfo",
*,
projects_count: int = 0,
members_count: int = 0,
) -> Self:
return cls(
**cls._summary_fields(workspace, user, projects_count, members_count)
)


class WorkspaceResponse(WorkspaceSummaryResponse):
"""
Workspace serialized for the single-workspace GET: everything in the list,
plus the TDEI metadata, the KartaView token and the mobile apps'
definitions.
"""

tdeiMetadata: Optional[Any] = None
kartaViewToken: Optional[str] = None
# Included in single-workspace GET for mobile app consumption. TODO: remove
# this when the app fetches these from dedicated endpoints:
longFormQuestDef: Optional[Any] = None
imageryListDef: Optional[Any] = None

# @test: Test that this class properly serializes the workspace data for API responses, including the effective role for the user making the request
# @test: Test that the values are populated in this class match the expected values from the database and that the relationships are correctly serialized

@classmethod
def from_workspace(
cls,
workspace: "Workspace",
user: "UserInfo",
*,
imagery_list_def: Any = None,
long_form_quest_def: Any = None,
projects_count: int = 0,
members_count: int = 0,
) -> Self:
return cls(
**cls._summary_fields(workspace, user, projects_count, members_count),
tdeiMetadata=workspace.tdeiMetadata,
kartaViewToken=workspace.kartaViewToken,
imageryListDef=imagery_list_def,
longFormQuestDef=long_form_quest_def,
importStatus=workspace.importStatus,
)


Expand Down
36 changes: 36 additions & 0 deletions tests/integration/test_workspaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,42 @@ async def test_list_matches_get_by_id(client, login, task_session, osm_session):
assert {k: listed[k] for k in shared} == {k: fetched[k] for k in shared}


async def test_list_leaves_out_what_only_get_by_id_sends(
client, login, task_session, osm_session
):
# tdeiMetadata can be a whole TDEI dataset record, and kartaViewToken a
# credential: the list leaves both out, along with the mobile apps'
# definitions; the single-workspace GET still sends them.
login(
factories.make_user_info(osm_workspace_roles={1: [WorkspaceUserRoleType.LEAD]})
)

def workspace():
return factories.make_workspace(
id=1, tdeiMetadata={"metadata": {"big": True}}, kartaViewToken="secret"
)

task_session.queue(
fakes.rows(workspace()), # /mine getAll
fakes.rows(workspace()), # /1 getById
)
osm_session.queue(fakes.rows(), fakes.rows())

listed = (await client.get(f"{API}/mine")).json()[0]
fetched = (await client.get(f"{API}/1")).json()

only_in_get = {
"tdeiMetadata",
"kartaViewToken",
"longFormQuestDef",
"imageryListDef",
}
assert only_in_get.isdisjoint(listed)
assert only_in_get <= fetched.keys()
assert fetched["tdeiMetadata"] == {"metadata": {"big": True}}
assert fetched["kartaViewToken"] == "secret"


# === GET /{id} =============================================================


Expand Down
1 change: 1 addition & 0 deletions tests/unit/test_workspace_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ async def test_get_all_skips_geometry_and_definition_joins(user):

sql = session.statements[-1]
assert "geometry" not in sql
assert '"tdeiMetadata"' not in sql
assert "JOIN" not in sql.upper()


Expand Down