Add rename and remove for a workspace's placeholder partner
Settings is gaining a partner display, which hides the add-form once a
partner exists. Without these routes a typo'd partner name would be
permanently unfixable, since no endpoint could change or drop one.
PATCH /workspace/{id}/parents/{uuid} renames; DELETE soft-deletes.
Soft rather than hard delete because PersonalData holds PII and the
repository is a SoftDeleteRepository specifically so erasure stays
auditable.
Removal refuses (422) when the member still has leave wishes, planned
leave, parental-leave rows or vacation days, naming the blockers, so a
removal can never orphan plan data. It also refuses for the caller's own
profile, and for a member who has their own login -- the invite/redeem
flow re-points a real user's personal_data into the inviter's workspace,
so soft-deleting that row would silently break their login.
Errors carry message_key + data rather than prose, per the no-Swedish-on
-the-wire rule; NotFoundError/ValidationError gained the same optional
shape DubbeldagError already had, defaulting empty so existing call
sites are unaffected.
Also fixes seed_demo.py constructing WorkspaceService with a stale
3-arg signature, which would have crashed at runtime -- invisible to
pytest since it is a standalone script.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RXMhc9nT6sNz1SL4j8dznL
This commit is contained in:
@@ -237,8 +237,22 @@ def get_workspace_service(
|
||||
ws_repo: WorkspaceRepositoryDep,
|
||||
inv_repo: WorkspaceInvitationRepositoryDep,
|
||||
pd_repo: PersonalDataRepositoryDep,
|
||||
user_repo: UserRepositoryDep,
|
||||
leave_wish_repo: LeaveWishRepositoryDep,
|
||||
planned_leave_repo: PlannedLeaveRepositoryDep,
|
||||
parental_leave_repo: ParentalLeaveRepositoryDep,
|
||||
vacation_days_repo: VacationDaysRepositoryDep,
|
||||
) -> WorkspaceService:
|
||||
return WorkspaceService(ws_repo, inv_repo, pd_repo)
|
||||
return WorkspaceService(
|
||||
ws_repo,
|
||||
inv_repo,
|
||||
pd_repo,
|
||||
user_repo,
|
||||
leave_wish_repo,
|
||||
planned_leave_repo,
|
||||
parental_leave_repo,
|
||||
vacation_days_repo,
|
||||
)
|
||||
|
||||
|
||||
WorkspaceServiceDep = Annotated[WorkspaceService, Depends(get_workspace_service)]
|
||||
|
||||
@@ -47,6 +47,11 @@ class PersonalDataResponse(BaseModel):
|
||||
|
||||
|
||||
class PersonalDataCreate(BaseModel):
|
||||
"""Request body for both adding a placeholder parent (POST) and renaming
|
||||
|
||||
one (PATCH) — both take exactly one field, so one schema serves both.
|
||||
"""
|
||||
|
||||
display_name: str = Field(min_length=1, max_length=100)
|
||||
|
||||
|
||||
|
||||
@@ -5,6 +5,7 @@ from uuid import UUID
|
||||
from fastapi import APIRouter, HTTPException, status
|
||||
|
||||
from app.dependencies import CurrentUserDep, WorkspaceServiceDep
|
||||
from app.entities.personal_data import PersonalData
|
||||
from app.models.workspace import (
|
||||
InvitationResponse,
|
||||
InviteResponse,
|
||||
@@ -20,6 +21,35 @@ from app.utils.errors import InvitationError, NotFoundError, ValidationError
|
||||
router = APIRouter(prefix="/api/workspace", tags=["workspace"])
|
||||
|
||||
|
||||
def _to_response(pd: PersonalData) -> PersonalDataResponse:
|
||||
return PersonalDataResponse(
|
||||
uuid=pd.uuid,
|
||||
workspace_id=pd.workspace_id,
|
||||
name=pd.name,
|
||||
salary_type=pd.salary_type,
|
||||
employment_status=pd.employment_status,
|
||||
created_at=pd.created_at,
|
||||
)
|
||||
|
||||
|
||||
def _to_http_error(exc: NotFoundError | ValidationError) -> HTTPException:
|
||||
"""Map a NotFoundError/ValidationError to an HTTPException.
|
||||
|
||||
Both carry an optional `message_key` + `data` pair (§3.5); when present it
|
||||
becomes the structured `detail` so the frontend never has to parse an
|
||||
English sentence. Falls back to the plain message for the (rare) error
|
||||
raised without one.
|
||||
"""
|
||||
code = (
|
||||
status.HTTP_404_NOT_FOUND
|
||||
if isinstance(exc, NotFoundError)
|
||||
else status.HTTP_422_UNPROCESSABLE_ENTITY
|
||||
)
|
||||
if exc.message_key:
|
||||
return HTTPException(code, detail={"message_key": exc.message_key, "data": exc.data})
|
||||
return HTTPException(code, detail=str(exc))
|
||||
|
||||
|
||||
@router.post("", response_model=WorkspaceResponse, status_code=status.HTTP_201_CREATED)
|
||||
async def create_workspace(
|
||||
payload: WorkspaceCreate,
|
||||
@@ -71,17 +101,7 @@ async def list_members(
|
||||
_current_user: CurrentUserDep,
|
||||
) -> list[PersonalDataResponse]:
|
||||
members = await service.list_members(workspace_id)
|
||||
return [
|
||||
PersonalDataResponse(
|
||||
uuid=m.uuid,
|
||||
workspace_id=m.workspace_id,
|
||||
name=m.name,
|
||||
salary_type=m.salary_type,
|
||||
employment_status=m.employment_status,
|
||||
created_at=m.created_at,
|
||||
)
|
||||
for m in members
|
||||
]
|
||||
return [_to_response(m) for m in members]
|
||||
|
||||
|
||||
@router.post(
|
||||
@@ -97,16 +117,45 @@ async def add_placeholder_parent(
|
||||
) -> PersonalDataResponse:
|
||||
try:
|
||||
pd = await service.add_placeholder_parent(workspace_id, payload.display_name)
|
||||
except NotFoundError as exc:
|
||||
raise HTTPException(status.HTTP_404_NOT_FOUND, str(exc)) from exc
|
||||
return PersonalDataResponse(
|
||||
uuid=pd.uuid,
|
||||
workspace_id=pd.workspace_id,
|
||||
name=pd.name,
|
||||
salary_type=pd.salary_type,
|
||||
employment_status=pd.employment_status,
|
||||
created_at=pd.created_at,
|
||||
)
|
||||
except (NotFoundError, ValidationError) as exc:
|
||||
raise _to_http_error(exc) from exc
|
||||
return _to_response(pd)
|
||||
|
||||
|
||||
@router.patch(
|
||||
"/{workspace_id}/parents/{personal_data_uuid}",
|
||||
response_model=PersonalDataResponse,
|
||||
)
|
||||
async def rename_parent(
|
||||
workspace_id: UUID,
|
||||
personal_data_uuid: UUID,
|
||||
payload: PersonalDataCreate,
|
||||
service: WorkspaceServiceDep,
|
||||
_current_user: CurrentUserDep,
|
||||
) -> PersonalDataResponse:
|
||||
try:
|
||||
pd = await service.rename_member(workspace_id, personal_data_uuid, payload.display_name)
|
||||
except (NotFoundError, ValidationError) as exc:
|
||||
raise _to_http_error(exc) from exc
|
||||
return _to_response(pd)
|
||||
|
||||
|
||||
@router.delete(
|
||||
"/{workspace_id}/parents/{personal_data_uuid}",
|
||||
status_code=status.HTTP_204_NO_CONTENT,
|
||||
)
|
||||
async def remove_parent(
|
||||
workspace_id: UUID,
|
||||
personal_data_uuid: UUID,
|
||||
service: WorkspaceServiceDep,
|
||||
current_user: CurrentUserDep,
|
||||
) -> None:
|
||||
try:
|
||||
await service.remove_member(
|
||||
workspace_id, personal_data_uuid, current_user.personal_data_uuid
|
||||
)
|
||||
except (NotFoundError, ValidationError) as exc:
|
||||
raise _to_http_error(exc) from exc
|
||||
|
||||
|
||||
@router.post(
|
||||
|
||||
@@ -17,7 +17,12 @@ from uuid import UUID, uuid4
|
||||
from app.entities.personal_data import PersonalData
|
||||
from app.entities.workspace import Workspace
|
||||
from app.entities.workspace_invitation import WorkspaceInvitation
|
||||
from app.repositories.leave_wish_repository import LeaveWishRepository
|
||||
from app.repositories.parental_leave_repository import ParentalLeaveRepository
|
||||
from app.repositories.personal_data_repository import PersonalDataRepository
|
||||
from app.repositories.planned_leave_repository import PlannedLeaveRepository
|
||||
from app.repositories.user_repository import UserRepository
|
||||
from app.repositories.vacation_days_repository import VacationDaysRepository
|
||||
from app.repositories.workspace_repository import (
|
||||
WorkspaceInvitationRepository,
|
||||
WorkspaceRepository,
|
||||
@@ -33,10 +38,20 @@ class WorkspaceService:
|
||||
ws_repo: WorkspaceRepository,
|
||||
inv_repo: WorkspaceInvitationRepository,
|
||||
pd_repo: PersonalDataRepository,
|
||||
user_repo: UserRepository,
|
||||
leave_wish_repo: LeaveWishRepository,
|
||||
planned_leave_repo: PlannedLeaveRepository,
|
||||
parental_leave_repo: ParentalLeaveRepository,
|
||||
vacation_days_repo: VacationDaysRepository,
|
||||
) -> None:
|
||||
self._ws = ws_repo
|
||||
self._inv = inv_repo
|
||||
self._pd = pd_repo
|
||||
self._user = user_repo
|
||||
self._leave_wish = leave_wish_repo
|
||||
self._planned_leave = planned_leave_repo
|
||||
self._parental_leave = parental_leave_repo
|
||||
self._vacation_days = vacation_days_repo
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Workspace CRUD
|
||||
@@ -49,7 +64,11 @@ class WorkspaceService:
|
||||
async def get_workspace(self, workspace_id: UUID) -> Workspace:
|
||||
ws = await self._ws.get_by_id(workspace_id)
|
||||
if ws is None:
|
||||
raise NotFoundError(f"Workspace {workspace_id} not found")
|
||||
raise NotFoundError(
|
||||
f"Workspace {workspace_id} not found",
|
||||
message_key="workspace.notFound",
|
||||
data={"workspace_id": str(workspace_id)},
|
||||
)
|
||||
return ws
|
||||
|
||||
async def update_workspace(
|
||||
@@ -80,6 +99,7 @@ class WorkspaceService:
|
||||
|
||||
async def add_placeholder_parent(self, workspace_id: UUID, display_name: str) -> PersonalData:
|
||||
"""Create a parent profile without a login (solo planning for a partner)."""
|
||||
_validate_display_name(display_name)
|
||||
await self.get_workspace(workspace_id) # verify workspace exists
|
||||
pd = PersonalData(
|
||||
uuid=uuid4(),
|
||||
@@ -89,6 +109,123 @@ class WorkspaceService:
|
||||
)
|
||||
return await self._pd.create(pd)
|
||||
|
||||
async def rename_member(
|
||||
self, workspace_id: UUID, personal_data_uuid: UUID, display_name: str
|
||||
) -> PersonalData:
|
||||
"""Rename a workspace member's display name.
|
||||
|
||||
Allowed for the placeholder partner and for the logged-in user's own
|
||||
profile alike — there is no other endpoint that can correct a typo'd
|
||||
name (the settings profile-update path only covers salary_type /
|
||||
employment_status / min_vacation_days_per_year), so this is the one
|
||||
place a name gets fixed.
|
||||
"""
|
||||
_validate_display_name(display_name)
|
||||
pd = await self._get_member(workspace_id, personal_data_uuid)
|
||||
pd.name = display_name
|
||||
return await self._pd.update(personal_data_uuid, pd)
|
||||
|
||||
async def remove_member(
|
||||
self,
|
||||
workspace_id: UUID,
|
||||
personal_data_uuid: UUID,
|
||||
requesting_personal_data_uuid: UUID,
|
||||
) -> None:
|
||||
"""Soft-delete a placeholder partner from a workspace.
|
||||
|
||||
Refuses (ValidationError, 422 at the router) when:
|
||||
- the target is the caller's own profile,
|
||||
- the target has its own login (it is not actually a "placeholder"
|
||||
— e.g. a real partner who redeemed an invite into this workspace),
|
||||
- the target still has dependent plan data (leave wishes, planned
|
||||
leave days, parental-leave rows, or vacation-days rows) that would
|
||||
be orphaned.
|
||||
"""
|
||||
await self._get_member(workspace_id, personal_data_uuid) # 404 if not a member
|
||||
|
||||
if personal_data_uuid == requesting_personal_data_uuid:
|
||||
raise ValidationError(
|
||||
"Cannot remove your own profile from the workspace",
|
||||
message_key="workspace.parents.remove.self",
|
||||
data={},
|
||||
)
|
||||
|
||||
if await self._has_login(personal_data_uuid):
|
||||
raise ValidationError(
|
||||
"Cannot remove a member that has their own login",
|
||||
message_key="workspace.parents.remove.hasLogin",
|
||||
data={},
|
||||
)
|
||||
|
||||
blockers = await self._dependent_data_blockers(personal_data_uuid)
|
||||
if blockers:
|
||||
raise ValidationError(
|
||||
f"Cannot remove member with existing {', '.join(blockers)}",
|
||||
message_key="workspace.parents.remove.hasDependentData",
|
||||
data={"blockers": blockers},
|
||||
)
|
||||
|
||||
await self._pd.soft_delete(personal_data_uuid)
|
||||
|
||||
async def _get_member(self, workspace_id: UUID, personal_data_uuid: UUID) -> PersonalData:
|
||||
"""Return the member, raising NotFoundError if the workspace or membership is missing.
|
||||
|
||||
Deliberately checks membership via `list_by_workspace` rather than a bare
|
||||
`pd_repo.get_by_id` — a `personal_data_uuid` that exists but belongs to a
|
||||
*different* workspace must 404 here, not succeed.
|
||||
"""
|
||||
ws = await self._ws.get_by_id(workspace_id)
|
||||
if ws is None:
|
||||
raise NotFoundError(
|
||||
f"Workspace {workspace_id} not found",
|
||||
message_key="workspace.notFound",
|
||||
data={"workspace_id": str(workspace_id)},
|
||||
)
|
||||
members = await self._pd.list_by_workspace(workspace_id)
|
||||
for member in members:
|
||||
if member.uuid == personal_data_uuid:
|
||||
return member
|
||||
raise NotFoundError(
|
||||
f"PersonalData {personal_data_uuid} is not a member of workspace {workspace_id}",
|
||||
message_key="workspace.parents.notFound",
|
||||
data={"personal_data_uuid": str(personal_data_uuid)},
|
||||
)
|
||||
|
||||
async def _has_login(self, personal_data_uuid: UUID) -> bool:
|
||||
"""True if a User account is linked to this personal_data_uuid.
|
||||
|
||||
A genuine placeholder (created via `add_placeholder_parent`) never has
|
||||
one; a partner who registered and redeemed an invite into the same
|
||||
workspace does — removing them here would silently break their login.
|
||||
"""
|
||||
users = await self._user.list()
|
||||
return any(u.personal_data_uuid == personal_data_uuid for u in users)
|
||||
|
||||
async def _dependent_data_blockers(self, personal_data_uuid: UUID) -> list[str]:
|
||||
"""Return the kinds of plan data that would be orphaned by removing this member."""
|
||||
blockers: list[str] = []
|
||||
|
||||
# LeaveWishRepository.list() only recognizes workspace_id/child_id
|
||||
# filters today — passing parent_personal_data_uuid would be silently
|
||||
# ignored rather than applied (returning every wish in every
|
||||
# workspace unfiltered). Fetch unfiltered and filter in Python
|
||||
# instead, so an unsupported filter can never be mistaken for "no
|
||||
# wishes for this parent".
|
||||
all_wishes = await self._leave_wish.list()
|
||||
if any(w.parent_personal_data_uuid == personal_data_uuid for w in all_wishes):
|
||||
blockers.append("leave_wishes")
|
||||
|
||||
if await self._planned_leave.list(parent_personal_data_uuid=personal_data_uuid):
|
||||
blockers.append("planned_leave")
|
||||
|
||||
if await self._parental_leave.list(personal_data_uuid=personal_data_uuid):
|
||||
blockers.append("parental_leave")
|
||||
|
||||
if await self._vacation_days.list(personal_data_uuid=personal_data_uuid):
|
||||
blockers.append("vacation_days")
|
||||
|
||||
return blockers
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Invitation flow
|
||||
# ------------------------------------------------------------------
|
||||
@@ -152,3 +289,16 @@ class WorkspaceService:
|
||||
|
||||
def _sha256(value: str) -> str:
|
||||
return hashlib.sha256(value.encode()).hexdigest()
|
||||
|
||||
|
||||
def _validate_display_name(display_name: str) -> None:
|
||||
"""Shared validation for add_placeholder_parent and rename_member so the
|
||||
two paths agree on what counts as a valid name (Pydantic's `min_length=1`
|
||||
on `display_name` does not reject a whitespace-only string).
|
||||
"""
|
||||
if not display_name.strip():
|
||||
raise ValidationError(
|
||||
"display_name must not be empty or whitespace-only",
|
||||
message_key="workspace.parents.invalidName",
|
||||
data={},
|
||||
)
|
||||
|
||||
@@ -10,11 +10,41 @@ class DomainError(Exception):
|
||||
|
||||
|
||||
class NotFoundError(DomainError):
|
||||
"""Raised when an entity cannot be found by its identifier."""
|
||||
"""Raised when an entity cannot be found by its identifier.
|
||||
|
||||
May optionally carry a `message_key` + `data` pair (§3.5) so a router can
|
||||
surface a structured, i18n-ready refusal instead of a hardcoded string —
|
||||
see `DubbeldagError` for the established shape. Existing call sites that
|
||||
only pass `message` are unaffected: `message_key` defaults to `""`.
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
message: str,
|
||||
message_key: str = "",
|
||||
data: dict[str, object] | None = None,
|
||||
) -> None:
|
||||
super().__init__(message)
|
||||
self.message_key = message_key
|
||||
self.data: dict[str, object] = data if data is not None else {}
|
||||
|
||||
|
||||
class ValidationError(DomainError):
|
||||
"""Raised when input fails a business rule (not just schema validation)."""
|
||||
"""Raised when input fails a business rule (not just schema validation).
|
||||
|
||||
May optionally carry a `message_key` + `data` pair (§3.5); see
|
||||
`NotFoundError` above for the same shape and rationale.
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
message: str,
|
||||
message_key: str = "",
|
||||
data: dict[str, object] | None = None,
|
||||
) -> None:
|
||||
super().__init__(message)
|
||||
self.message_key = message_key
|
||||
self.data: dict[str, object] = data if data is not None else {}
|
||||
|
||||
|
||||
class InsufficientDaysError(DomainError):
|
||||
|
||||
Reference in New Issue
Block a user