diff --git a/.gitignore b/.gitignore index 1d84a4ad..146f98fb 100644 --- a/.gitignore +++ b/.gitignore @@ -61,3 +61,4 @@ Thumbs.db .playwright-mcp/* host/client_app/.playwright-cli/* .superpowers/ +.qa/ diff --git a/docs/superpowers/plans/2026-06-21-admin-user-crud.md b/docs/superpowers/plans/2026-06-21-admin-user-crud.md new file mode 100644 index 00000000..d0d9ceac --- /dev/null +++ b/docs/superpowers/plans/2026-06-21-admin-user-crud.md @@ -0,0 +1,1788 @@ +# Admin User CRUD Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let an admin create, edit, and delete users directly from the `/users/admin` page (today it can only invite). + +**Architecture:** Additive surface on the existing `users` module admin slice — no new module, no new tables, no migration. The over-cap `admin/service.py` is split into `admin/queries.py` (read/helpers, `class _UserServiceBase`) + `admin/service.py` (writes, `class UserService(_UserServiceBase)`), keeping the `from users.admin.service import UserService` import path unchanged. Three new service methods + three new REST endpoints + one new Inertia view route + new/updated React pages. + +**Tech Stack:** Python 3.12, FastAPI, SQLModel, fastapi-users, Inertia.js, React 19, Tailwind 4, shadcn/ui, pytest (`anyio`). + +## Global Constraints + +- **300-line cap** on every `.py` / `.ts` / `.tsx` file (CI: `scripts/check_file_size.py`). Split by responsibility if approaching it. +- **SQLModel only** for schemas/DTOs (plain `SQLModel` subclass) — never Pydantic `BaseModel`. +- **Service code never calls `session.commit()`** — use `await self._db.flush()`; the per-request session auto-commits on pending writes. +- **Events are `@dataclass` subclasses of `simple_module_core.events.Event`**, defined in `contracts/events.py`. +- **Admin pages use plain English strings** (no `useT()` / i18n) — matches existing `Invite.tsx` / `Edit.tsx`. +- **New endpoints stay under the existing `users.manage` permission** (the `admin_router` dependency already enforces it). +- **Route ordering:** the new view route `GET /admin/create` MUST be declared before `GET /admin/{user_id}` (FastAPI matches in declaration order), same as `/admin/invite`. +- **Verification after each backend task:** `uv run pytest modules/users/tests/ -v`. Final gate: `make test-py`, `make lint`, `make doctor`. + +--- + +## File Structure + +| File | Responsibility | +|---|---| +| `modules/users/users/admin/queries.py` | NEW — `_UserServiceBase`: read/query/helper methods (moved verbatim from `service.py`). | +| `modules/users/users/admin/service.py` | `UserService(_UserServiceBase)`: write/command methods + new `create_user` / `update_details` / `delete_user`. | +| `modules/users/users/contracts/schemas.py` | + `UserAdminCreate`, `UserDetailsUpdate`. | +| `modules/users/users/contracts/events.py` | + `UserCreated`, `UserDeleted`. | +| `modules/users/users/exceptions.py` | + `EmailAlreadyExistsError`. | +| `modules/users/users/admin/api.py` | + `POST ""`, `PATCH "/{user_id}"`, `DELETE "/{user_id}"`. | +| `modules/users/users/admin/views.py` | + `GET /admin/create` Inertia page route. | +| `modules/users/users/pages/Users/Create.tsx` | NEW — create-user form. | +| `modules/users/users/pages/Users/Index.tsx` | + "Create user" button next to "Invite member". | +| `modules/users/users/pages/Users/Edit.tsx` | + render `DetailsCard` + `DangerZone`. | +| `modules/users/users/pages/Users/components/DetailsCard.tsx` | NEW — edit email + full name. | +| `modules/users/users/pages/Users/components/DangerZone.tsx` | NEW — delete with confirm dialog. | +| `modules/users/tests/test_service_admin.py` | + create/update/delete service tests. | +| `modules/users/tests/test_api_admin.py` | + create/update/delete API tests. | +| `modules/users/tests/test_views_admin.py` | + create-page view test. | + +--- + +## Task 1: Split `admin/service.py` into queries + service (pure refactor) + +No behavior change. Move read/helper methods to a new `_UserServiceBase` in `queries.py`; leave write methods on `UserService`, which now subclasses the base. Existing import path and every caller/test stay unchanged. + +**Files:** +- Create: `modules/users/users/admin/queries.py` +- Modify: `modules/users/users/admin/service.py` (full rewrite — same behavior) +- Test: existing `modules/users/tests/test_service_admin.py`, `test_api_admin.py`, `test_views_admin.py` (unchanged — they must still pass) + +**Interfaces:** +- Produces: `class _UserServiceBase` (in `queries.py`) with `__init__(self, db, user_manager)`, `_resolve_roles`, `to_list_item`, `list_roles`, `_get_user_with_roles`, `_require_user`, `list_users`, `count_user_states`, `get_with_roles`, `get_list_item`. And `class UserService(_UserServiceBase)` (in `service.py`) with `invite`, `disable`, `enable`, `mark_verified`, `set_roles`, `generate_reset_link`. + +- [ ] **Step 1: Create `admin/queries.py` with the read/helper base class** + +```python +"""Read/query helpers for the admin UserService (split from service.py).""" + +from __future__ import annotations + +import uuid + +from sqlalchemy import func, or_, select +from sqlalchemy.ext.asyncio import AsyncSession +from sqlalchemy.orm import selectinload + +from users.contracts.schemas import RoleListItem, UserListItem +from users.exceptions import UserNotFoundError +from users.manager import UserManager +from users.models import Role, User, UserRole + + +class _UserServiceBase: + def __init__( + self, + db: AsyncSession, + user_manager: UserManager, + ) -> None: + self._db = db + self._manager = user_manager + + # ── Helpers ───────────────────────────────────────────────── + + async def _resolve_roles(self, role_names: list[str]) -> list[Role]: + """Return Role ORM objects matching the given names.""" + if not role_names: + return [] + result = await self._db.execute(select(Role).where(Role.name.in_(role_names))) + return list(result.scalars().all()) + + def to_list_item(self, user: User) -> UserListItem: + """Build the DTO from a User with roles already eager-loaded.""" + return UserListItem( + id=user.id, + email=user.email, + full_name=user.full_name, + is_active=user.is_active, + is_verified=user.is_verified, + disabled_at=user.disabled_at, + last_login_at=user.last_login_at, + created_at=user.created_at, + roles=[r.name for r in user.roles], + ) + + async def list_roles(self) -> list[RoleListItem]: + stmt = ( + select(Role, func.count(UserRole.user_id)) + .outerjoin(UserRole, UserRole.role_id == Role.id) + .group_by(Role.id) + .order_by(Role.name) + ) + result = await self._db.execute(stmt) + return [ + RoleListItem( + id=role.id, + name=role.name, + description=role.description, + user_count=user_count, + ) + for role, user_count in result.all() + ] + + async def _get_user_with_roles(self, user_id: uuid.UUID) -> User | None: + result = await self._db.execute( + select(User).where(User.id == user_id).options(selectinload(User.roles)) + ) + return result.scalar_one_or_none() + + async def _require_user(self, user_id: uuid.UUID) -> User: + """Fetch a user with roles eager-loaded, or raise UserNotFoundError.""" + user = await self._get_user_with_roles(user_id) + if user is None: + raise UserNotFoundError(user_id) + return user + + # ── Queries ────────────────────────────────────────────────── + + async def list_users( + self, + *, + page: int = 1, + per_page: int = 20, + search: str | None = None, + status: str | None = None, + role_name: str | None = None, + verified: str | None = None, + sort: str = "email", + order: str = "asc", + ) -> tuple[list[UserListItem], int]: + """Returns (items, total_count). last_login_at sort always uses NULLS LAST.""" + stmt = select(User).options(selectinload(User.roles)) + count_stmt = select(func.count()).select_from(User) + + conditions = [] + + if search: + pattern = f"%{search}%" + conditions.append( + or_( + User.email.ilike(pattern), + User.full_name.ilike(pattern), + ) + ) + + if status == "active": + conditions.append(User.is_active.is_(True)) + elif status == "disabled": + conditions.append(User.is_active.is_(False)) + + if verified == "yes": + conditions.append(User.is_verified.is_(True)) + elif verified == "no": + conditions.append(User.is_verified.is_(False)) + + if role_name is not None: + subq = ( + select(UserRole.user_id) + .join(Role, Role.id == UserRole.role_id) + .where(Role.name == role_name) + ) + conditions.append(User.id.in_(subq)) + + for cond in conditions: + stmt = stmt.where(cond) + count_stmt = count_stmt.where(cond) + + total = (await self._db.execute(count_stmt)).scalar_one() + + sort_col_map = { + "email": User.email, + "last_login_at": User.last_login_at, + "created_at": User.created_at, + } + sort_col = sort_col_map.get(sort, User.email) + + if sort == "last_login_at": + order_clause = ( + sort_col.desc().nulls_last() # type: ignore[union-attr] + if order == "desc" + else sort_col.asc().nulls_last() # type: ignore[union-attr] + ) + else: + order_clause = ( + sort_col.desc() if order == "desc" else sort_col.asc() # type: ignore[union-attr] + ) + + stmt = stmt.order_by(order_clause).offset((page - 1) * per_page).limit(per_page) + rows = (await self._db.execute(stmt)).scalars().all() + + items = [self.to_list_item(u) for u in rows] + return items, total + + async def count_user_states(self) -> dict[str, int]: + """Workspace-wide counts unaffected by list filters/pagination — + feeds the dashboard cards on /users/admin so they don't reflect + the current page slice.""" + active_q = select(func.count()).select_from(User).where(User.is_active.is_(True)) + unverified_q = ( + select(func.count()) + .select_from(User) + .where(User.is_active.is_(True), User.is_verified.is_(False)) + ) + active = (await self._db.execute(active_q)).scalar_one() + unverified = (await self._db.execute(unverified_q)).scalar_one() + return {"active": int(active), "unverified": int(unverified)} + + async def get_with_roles(self, user_id: uuid.UUID) -> User | None: + return await self._get_user_with_roles(user_id) + + async def get_list_item(self, user_id: uuid.UUID) -> UserListItem: + user = await self._require_user(user_id) + return self.to_list_item(user) +``` + +- [ ] **Step 2: Rewrite `admin/service.py` to subclass the base, keeping only write methods** + +```python +"""UserService — admin write operations (reads live in queries.py).""" + +from __future__ import annotations + +import secrets +import uuid +from datetime import UTC, datetime + +from sqlalchemy import delete + +from users.admin.queries import _UserServiceBase +from users.contracts.schemas import UserCreate +from users.models import User, UserRole + + +class UserService(_UserServiceBase): + async def invite( + self, + email: str, + full_name: str | None, + role_names: list[str], + *, + invited_by: User | None = None, + ) -> tuple[User, str]: + """Creates unverified user + random unusable password, assigns roles, + mints a verification token. Returns (user, token).""" + password = secrets.token_urlsafe(32) + user_create = UserCreate( + email=email, + password=password, + full_name=full_name, + is_active=True, + is_verified=False, + ) + user = await self._manager.create(user_create, safe=False) + + # Assign roles + roles = await self._resolve_roles(role_names) + invited_by_str = str(invited_by.id) if invited_by else None + for role in roles: + self._db.add( + UserRole( + user_id=user.id, + role_id=role.id, + assigned_by=invited_by_str, + ) + ) + if roles: + await self._db.flush() + await self._db.refresh(user, attribute_names=["roles"]) + + token = await self._manager.generate_verification_token(user) + return user, token + + async def disable(self, user_id: uuid.UUID) -> User: + user = await self._require_user(user_id) + user.disabled_at = datetime.now(UTC) + user.is_active = False + await self._db.flush() + return user + + async def enable(self, user_id: uuid.UUID) -> User: + user = await self._require_user(user_id) + user.disabled_at = None + user.is_active = True + await self._db.flush() + return user + + async def mark_verified(self, user_id: uuid.UUID) -> User: + user = await self._require_user(user_id) + if not user.is_verified: + user.is_verified = True + await self._db.flush() + return user + + async def set_roles( + self, + user_id: uuid.UUID, + role_names: list[str], + *, + assigned_by: str | None = None, + ) -> User: + user = await self._require_user(user_id) + + # Delete all existing role assignments for this user + await self._db.execute(delete(UserRole).where(UserRole.user_id == user_id)) + + # Insert new role assignments + roles = await self._resolve_roles(role_names) + for role in roles: + self._db.add( + UserRole( + user_id=user_id, + role_id=role.id, + assigned_by=assigned_by, + ) + ) + + await self._db.flush() + await self._db.refresh(user, attribute_names=["roles"]) + return user + + async def generate_reset_link(self, user_id: uuid.UUID, base_url: str) -> str: + """Build an admin-copyable password-reset URL. No email side-effect.""" + user = await self._require_user(user_id) + + token = await self._manager.generate_reset_password_token(user) + return f"{base_url.rstrip('/')}/users/reset-password?token={token}" +``` + +- [ ] **Step 3: Run the existing admin test suites to verify the refactor is behavior-preserving** + +Run: `uv run pytest modules/users/tests/test_service_admin.py modules/users/tests/test_api_admin.py modules/users/tests/test_views_admin.py modules/users/tests/test_invite_flow.py -v` +Expected: PASS (all existing tests green — no behavior changed). + +- [ ] **Step 4: Verify both files are under the line cap** + +Run: `uv run python scripts/check_file_size.py modules/users/users/admin/queries.py modules/users/users/admin/service.py` +Expected: no violations (queries.py ≈ 175 lines, service.py ≈ 130 lines). + +- [ ] **Step 5: Commit** + +```bash +git add modules/users/users/admin/queries.py modules/users/users/admin/service.py +git commit -m "refactor(users): split admin UserService into queries + service" +``` + +--- + +## Task 2: Backend — create user (active + verified) + +**Files:** +- Modify: `modules/users/users/contracts/schemas.py` (+ `UserAdminCreate`) +- Modify: `modules/users/users/contracts/events.py` (+ `UserCreated`) +- Modify: `modules/users/users/admin/service.py` (+ `create_user`) +- Modify: `modules/users/users/admin/api.py` (+ `POST ""`) +- Test: `modules/users/tests/test_service_admin.py`, `modules/users/tests/test_api_admin.py` + +**Interfaces:** +- Consumes: `_UserServiceBase._resolve_roles`, `.to_list_item`; `UserManager.create`. +- Produces: + - `class UserAdminCreate(SQLModel)`: `email: EmailStr`, `password: str`, `full_name: str | None = None`, `role_names: list[str] = []`. + - `class UserCreated(Event)`: `user_id: uuid.UUID`, `email: str`, `created_by: str | None`. + - `UserService.create_user(self, email: str, password: str, full_name: str | None, role_names: list[str], *, created_by: str | None) -> User`. + - Endpoint `POST /api/users/admin` → `UserListItem`, 201. + +- [ ] **Step 1: Write the failing service test** + +Append to `modules/users/tests/test_service_admin.py`: + +```python +# --------------------------------------------------------------------------- +# create_user +# --------------------------------------------------------------------------- + + +@pytest.mark.anyio +async def test_create_user_active_verified_with_roles(users_app): + """create_user makes an active+verified user and assigns roles.""" + async with users_app.state.sm.db.session_factory() as session: + svc = _build_service(session, users_app) + user = await svc.create_user( + email="created@example.com", + password="SecurePass1!", + full_name="Created User", + role_names=["user"], + created_by="admin-id", + ) + assert user.is_active is True + assert user.is_verified is True + assert user.email == "created@example.com" + assert user.full_name == "Created User" + assert [r.name for r in user.roles] == ["user"] + + +@pytest.mark.anyio +async def test_create_user_weak_password_rejected(users_app): + from fastapi_users import exceptions as fa_exc + + async with users_app.state.sm.db.session_factory() as session: + svc = _build_service(session, users_app) + with pytest.raises(fa_exc.InvalidPasswordException): + await svc.create_user( + email="weak@example.com", + password="short", + full_name=None, + role_names=[], + created_by=None, + ) + + +@pytest.mark.anyio +async def test_create_user_duplicate_email_rejected(users_app): + from fastapi_users import exceptions as fa_exc + + async with users_app.state.sm.db.session_factory() as session: + svc = _build_service(session, users_app) + await svc.create_user( + email="dup@example.com", + password="SecurePass1!", + full_name=None, + role_names=[], + created_by=None, + ) + await session.flush() + with pytest.raises(fa_exc.UserAlreadyExists): + await svc.create_user( + email="dup@example.com", + password="SecurePass1!", + full_name=None, + role_names=[], + created_by=None, + ) +``` + +- [ ] **Step 2: Run the service test to verify it fails** + +Run: `uv run pytest modules/users/tests/test_service_admin.py -k create_user -v` +Expected: FAIL with `AttributeError: 'UserService' object has no attribute 'create_user'`. + +- [ ] **Step 3: Add `UserAdminCreate` schema** + +In `modules/users/users/contracts/schemas.py`, after the `UserInvite` class (around line 49), add: + +```python +class UserAdminCreate(SQLModel): + email: EmailStr + password: str + full_name: str | None = None + role_names: list[str] = [] +``` + +- [ ] **Step 4: Add `UserCreated` event** + +In `modules/users/users/contracts/events.py`, after the `UserInvited` class, add: + +```python +@dataclass +class UserCreated(Event): + user_id: uuid.UUID + email: str + created_by: str | None +``` + +- [ ] **Step 5: Add `create_user` to `UserService`** + +In `modules/users/users/admin/service.py`, the schemas import already pulls in `UserCreate`; add this method to the `UserService` class (place it before `invite`): + +```python + async def create_user( + self, + email: str, + password: str, + full_name: str | None, + role_names: list[str], + *, + created_by: str | None, + ) -> User: + """Create an active+verified user with an admin-set password. + + Reuses ``manager.create`` for the password policy + email-uniqueness + check. ``is_verified=True`` means ``on_after_register`` does not fire a + verification email (and with no request, no event is published here — + the endpoint publishes ``UserCreated``).""" + user_create = UserCreate( + email=email, + password=password, + full_name=full_name, + is_active=True, + is_verified=True, + is_superuser=False, + ) + user = await self._manager.create(user_create, safe=False) + + roles = await self._resolve_roles(role_names) + for role in roles: + self._db.add( + UserRole( + user_id=user.id, + role_id=role.id, + assigned_by=created_by, + ) + ) + if roles: + await self._db.flush() + await self._db.refresh(user, attribute_names=["roles"]) + return user +``` + +- [ ] **Step 6: Run the service test to verify it passes** + +Run: `uv run pytest modules/users/tests/test_service_admin.py -k create_user -v` +Expected: PASS (3 tests). + +- [ ] **Step 7: Write the failing API test** + +Append to `modules/users/tests/test_api_admin.py`: + +```python +# --------------------------------------------------------------------------- +# Admin create +# --------------------------------------------------------------------------- + + +class TestAdminCreate: + @pytest.mark.anyio + async def test_create_returns_201(self, admin_client): + resp = await admin_client.post( + "/api/users/admin", + json={ + "email": "newuser@example.com", + "password": "SecurePass1!", + "full_name": "New User", + "role_names": ["user"], + }, + ) + assert resp.status_code == 201 + body = resp.json() + assert body["email"] == "newuser@example.com" + assert body["is_active"] is True + assert body["is_verified"] is True + assert body["roles"] == ["user"] + + @pytest.mark.anyio + async def test_create_duplicate_returns_409(self, admin_client, users_db): + await _make_user(users_db, email="taken@example.com") + resp = await admin_client.post( + "/api/users/admin", + json={"email": "taken@example.com", "password": "SecurePass1!"}, + ) + assert resp.status_code == 409 + + @pytest.mark.anyio + async def test_create_weak_password_returns_400(self, admin_client): + resp = await admin_client.post( + "/api/users/admin", + json={"email": "weakpw@example.com", "password": "short"}, + ) + assert resp.status_code == 400 + assert "8 characters" in resp.json()["detail"] + + @pytest.mark.anyio + async def test_create_without_auth_is_rejected(self, anon_client): + resp = await anon_client.post( + "/api/users/admin", + json={"email": "hacker@example.com", "password": "SecurePass1!"}, + follow_redirects=False, + ) + assert resp.status_code == 401 + assert resp.json() == {"detail": "Not authenticated"} +``` + +- [ ] **Step 8: Run the API test to verify it fails** + +Run: `uv run pytest modules/users/tests/test_api_admin.py::TestAdminCreate -v` +Expected: FAIL — `POST /api/users/admin` returns 405 (Method Not Allowed) because the endpoint doesn't exist yet. + +- [ ] **Step 9: Add the `POST ""` endpoint** + +In `modules/users/users/admin/api.py`: + +Add the fastapi-users exceptions import (after `from fastapi import status as http_status`): + +```python +from fastapi_users import exceptions as fa_exceptions +``` + +Change the events import to include `UserCreated`: + +```python +from users.contracts.events import RoleAssigned, UserCreated, UserDisabled, UserInvited +``` + +Change the schemas import to include `UserAdminCreate`: + +```python +from users.contracts.schemas import ( + PasswordResetLink, + RoleAssignment, + UserAdminCreate, + UserInvite, + UserListItem, +) +``` + +Then add the endpoint immediately after `admin_invite_user` (before `admin_disable_user`): + +```python +@admin_router.post( + "", + response_model=UserListItem, + status_code=http_status.HTTP_201_CREATED, +) +async def admin_create_user( + data: UserAdminCreate, + request: Request, + bus: EventBus = Depends(get_event_bus), + service: UserService = Depends(get_user_service), +): + """Create an active+verified user with an admin-set password.""" + creator = getattr(request.state, "user", None) + created_by = creator.id if creator else None + try: + user = await service.create_user( + data.email, + data.password, + data.full_name, + data.role_names, + created_by=created_by, + ) + except fa_exceptions.UserAlreadyExists: + raise HTTPException( + status_code=409, + detail="A user with this email already exists.", + ) from None + except fa_exceptions.InvalidPasswordException as exc: + raise HTTPException(status_code=400, detail=exc.reason) from None + await bus.publish( + UserCreated(user_id=user.id, email=user.email, created_by=created_by) + ) + return service.to_list_item(user) +``` + +- [ ] **Step 10: Run the API test to verify it passes** + +Run: `uv run pytest modules/users/tests/test_api_admin.py::TestAdminCreate -v` +Expected: PASS (4 tests). + +- [ ] **Step 11: Commit** + +```bash +git add modules/users/users/contracts/schemas.py modules/users/users/contracts/events.py \ + modules/users/users/admin/service.py modules/users/users/admin/api.py \ + modules/users/tests/test_service_admin.py modules/users/tests/test_api_admin.py +git commit -m "feat(users): admin create-user endpoint (active+verified)" +``` + +--- + +## Task 3: Backend — edit user details (email + full name) + +**Files:** +- Modify: `modules/users/users/contracts/schemas.py` (+ `UserDetailsUpdate`) +- Modify: `modules/users/users/exceptions.py` (+ `EmailAlreadyExistsError`) +- Modify: `modules/users/users/admin/service.py` (+ `update_details`) +- Modify: `modules/users/users/admin/api.py` (+ `PATCH "/{user_id}"`) +- Test: `modules/users/tests/test_service_admin.py`, `modules/users/tests/test_api_admin.py` + +**Interfaces:** +- Consumes: `_UserServiceBase._require_user`, `.to_list_item`. +- Produces: + - `class UserDetailsUpdate(SQLModel)`: `email: EmailStr`, `full_name: str | None = None`. + - `class EmailAlreadyExistsError(Exception)`: `__init__(self, email: str)`, attribute `.email`. + - `UserService.update_details(self, user_id: uuid.UUID, email: str, full_name: str | None) -> User`. + - Endpoint `PATCH /api/users/admin/{user_id}` → `UserListItem`, 200. + +- [ ] **Step 1: Write the failing service test** + +Append to `modules/users/tests/test_service_admin.py`: + +```python +# --------------------------------------------------------------------------- +# update_details +# --------------------------------------------------------------------------- + + +@pytest.mark.anyio +async def test_update_details_changes_email_and_name(users_app): + from test_api_admin import _make_user + + async with users_app.state.sm.db.session_factory() as session: + user = await _make_user(session, email="old@example.com") + svc = _build_service(session, users_app) + updated = await svc.update_details( + user.id, email="new@example.com", full_name="New Name" + ) + assert updated.email == "new@example.com" + assert updated.full_name == "New Name" + + +@pytest.mark.anyio +async def test_update_details_duplicate_email_rejected(users_app): + from test_api_admin import _make_user + from users.exceptions import EmailAlreadyExistsError + + async with users_app.state.sm.db.session_factory() as session: + await _make_user(session, email="a@example.com") + target = await _make_user(session, email="b@example.com") + svc = _build_service(session, users_app) + with pytest.raises(EmailAlreadyExistsError): + await svc.update_details(target.id, email="a@example.com", full_name=None) + + +@pytest.mark.anyio +async def test_update_details_same_email_is_allowed(users_app): + from test_api_admin import _make_user + + async with users_app.state.sm.db.session_factory() as session: + user = await _make_user(session, email="keep@example.com") + svc = _build_service(session, users_app) + updated = await svc.update_details( + user.id, email="keep@example.com", full_name="Renamed" + ) + assert updated.email == "keep@example.com" + assert updated.full_name == "Renamed" +``` + +- [ ] **Step 2: Run the service test to verify it fails** + +Run: `uv run pytest modules/users/tests/test_service_admin.py -k update_details -v` +Expected: FAIL with `AttributeError: 'UserService' object has no attribute 'update_details'`. + +- [ ] **Step 3: Add `EmailAlreadyExistsError`** + +In `modules/users/users/exceptions.py`, after `UserNotFoundError`, add: + +```python +class EmailAlreadyExistsError(Exception): + """Raised when updating a user to an email already owned by another user.""" + + def __init__(self, email: str) -> None: + super().__init__(f"Email {email} already in use") + self.email = email +``` + +- [ ] **Step 4: Add `UserDetailsUpdate` schema** + +In `modules/users/users/contracts/schemas.py`, after `UserAdminCreate`, add: + +```python +class UserDetailsUpdate(SQLModel): + email: EmailStr + full_name: str | None = None +``` + +- [ ] **Step 5: Add `update_details` to `UserService`** + +In `modules/users/users/admin/service.py`: + +Change the sqlalchemy import (currently `from sqlalchemy import delete`) to add `func` and `select`: + +```python +from sqlalchemy import delete, func, select +``` + +Add the `EmailAlreadyExistsError` import (this file currently imports nothing from `users.exceptions`): + +```python +from users.exceptions import EmailAlreadyExistsError +``` + +Add the method to `UserService` (place after `create_user`): + +```python + async def update_details( + self, + user_id: uuid.UUID, + email: str, + full_name: str | None, + ) -> User: + """Update a user's email + full name. Raises UserNotFoundError if the + user is missing, EmailAlreadyExistsError if the new email is taken by + another user.""" + user = await self._require_user(user_id) + if email.lower() != user.email.lower(): + clash = await self._db.execute( + select(User.id).where( + func.lower(User.email) == email.lower(), + User.id != user_id, + ) + ) + if clash.scalar_one_or_none() is not None: + raise EmailAlreadyExistsError(email) + user.email = email + user.full_name = full_name + await self._db.flush() + return user +``` + +- [ ] **Step 6: Run the service test to verify it passes** + +Run: `uv run pytest modules/users/tests/test_service_admin.py -k update_details -v` +Expected: PASS (3 tests). + +- [ ] **Step 7: Write the failing API test** + +Append to `modules/users/tests/test_api_admin.py`: + +```python +# --------------------------------------------------------------------------- +# Admin update details +# --------------------------------------------------------------------------- + + +class TestAdminUpdate: + @pytest.mark.anyio + async def test_update_changes_email_and_name(self, admin_client, users_db): + user = await _make_user(users_db, email="before@example.com") + resp = await admin_client.patch( + f"/api/users/admin/{user.id}", + json={"email": "after@example.com", "full_name": "After Name"}, + ) + assert resp.status_code == 200 + body = resp.json() + assert body["email"] == "after@example.com" + assert body["full_name"] == "After Name" + + @pytest.mark.anyio + async def test_update_duplicate_email_returns_409(self, admin_client, users_db): + await _make_user(users_db, email="exists@example.com") + target = await _make_user(users_db, email="target@example.com") + resp = await admin_client.patch( + f"/api/users/admin/{target.id}", + json={"email": "exists@example.com"}, + ) + assert resp.status_code == 409 + + @pytest.mark.anyio + async def test_update_nonexistent_returns_404(self, admin_client): + resp = await admin_client.patch( + f"/api/users/admin/{uuid.uuid4()}", + json={"email": "ghost@example.com"}, + ) + assert resp.status_code == 404 + + @pytest.mark.anyio + async def test_update_without_auth_is_rejected(self, anon_client): + resp = await anon_client.patch( + f"/api/users/admin/{uuid.uuid4()}", + json={"email": "x@example.com"}, + follow_redirects=False, + ) + assert resp.status_code == 401 +``` + +- [ ] **Step 8: Run the API test to verify it fails** + +Run: `uv run pytest modules/users/tests/test_api_admin.py::TestAdminUpdate -v` +Expected: FAIL — `PATCH /api/users/admin/{id}` returns 405 (endpoint doesn't exist). + +- [ ] **Step 9: Add the `PATCH "/{user_id}"` endpoint** + +In `modules/users/users/admin/api.py`: + +Add `EmailAlreadyExistsError` to the exceptions import: + +```python +from users.exceptions import EmailAlreadyExistsError, UserNotFoundError +``` + +Add `UserDetailsUpdate` to the schemas import block: + +```python +from users.contracts.schemas import ( + PasswordResetLink, + RoleAssignment, + UserAdminCreate, + UserDetailsUpdate, + UserInvite, + UserListItem, +) +``` + +Add the endpoint after `admin_create_user` (before `admin_disable_user`): + +```python +@admin_router.patch("/{user_id}", response_model=UserListItem) +async def admin_update_user( + user_id: uuid.UUID, + data: UserDetailsUpdate, + service: UserService = Depends(get_user_service), +): + """Update a user's email and full name.""" + try: + user = await service.update_details(user_id, data.email, data.full_name) + except UserNotFoundError: + raise HTTPException(status_code=404, detail="User not found") from None + except EmailAlreadyExistsError: + raise HTTPException( + status_code=409, + detail="A user with this email already exists.", + ) from None + return service.to_list_item(user) +``` + +- [ ] **Step 10: Run the API test to verify it passes** + +Run: `uv run pytest modules/users/tests/test_api_admin.py::TestAdminUpdate -v` +Expected: PASS (4 tests). + +- [ ] **Step 11: Commit** + +```bash +git add modules/users/users/contracts/schemas.py modules/users/users/exceptions.py \ + modules/users/users/admin/service.py modules/users/users/admin/api.py \ + modules/users/tests/test_service_admin.py modules/users/tests/test_api_admin.py +git commit -m "feat(users): admin edit-user-details endpoint" +``` + +--- + +## Task 4: Backend — delete user (hard delete, self-delete guarded) + +**Files:** +- Modify: `modules/users/users/contracts/events.py` (+ `UserDeleted`) +- Modify: `modules/users/users/admin/service.py` (+ `delete_user`) +- Modify: `modules/users/users/admin/api.py` (+ `DELETE "/{user_id}"`) +- Test: `modules/users/tests/test_service_admin.py`, `modules/users/tests/test_api_admin.py` + +**Interfaces:** +- Consumes: `_UserServiceBase._require_user`. +- Produces: + - `class UserDeleted(Event)`: `user_id: uuid.UUID`. + - `UserService.delete_user(self, user_id: uuid.UUID) -> None`. + - Endpoint `DELETE /api/users/admin/{user_id}` → 204; 400 on self-delete; 404 missing. + +- [ ] **Step 1: Write the failing service test** + +Append to `modules/users/tests/test_service_admin.py`: + +```python +# --------------------------------------------------------------------------- +# delete_user +# --------------------------------------------------------------------------- + + +@pytest.mark.anyio +async def test_delete_user_removes_user_and_roles(users_app): + from sqlalchemy import select + from test_api_admin import _make_user + from users.models import User, UserRole + + async with users_app.state.sm.db.session_factory() as session: + user = await _make_user(session, email="todelete@example.com", role_names=["admin"]) + svc = _build_service(session, users_app) + await svc.delete_user(user.id) + await session.flush() + + remaining = ( + await session.execute(select(User).where(User.id == user.id)) + ).scalar_one_or_none() + assert remaining is None + roles = ( + (await session.execute(select(UserRole).where(UserRole.user_id == user.id))) + .scalars() + .all() + ) + assert roles == [] + + +@pytest.mark.anyio +async def test_delete_user_nonexistent_raises(users_app): + from users.exceptions import UserNotFoundError + + async with users_app.state.sm.db.session_factory() as session: + svc = _build_service(session, users_app) + with pytest.raises(UserNotFoundError): + await svc.delete_user(uuid.uuid4()) +``` + +- [ ] **Step 2: Run the service test to verify it fails** + +Run: `uv run pytest modules/users/tests/test_service_admin.py -k delete_user -v` +Expected: FAIL with `AttributeError: 'UserService' object has no attribute 'delete_user'`. + +- [ ] **Step 3: Add `UserDeleted` event** + +In `modules/users/users/contracts/events.py`, after `UserCreated`, add: + +```python +@dataclass +class UserDeleted(Event): + user_id: uuid.UUID +``` + +- [ ] **Step 4: Add `delete_user` to `UserService`** + +In `modules/users/users/admin/service.py`: + +Extend the models import to cover every child table: + +```python +from users.models import OAuthAccount, RefreshToken, User, UserAccessToken, UserRole +``` + +Add the method to `UserService` (place after `update_details`): + +```python + async def delete_user(self, user_id: uuid.UUID) -> None: + """Hard-delete a user and its dependent rows. + + Child rows are deleted explicitly (not via FK cascade) so the result is + identical on Postgres and SQLite — SQLite only enforces FK cascade when + the per-connection ``foreign_keys`` pragma is on, which we don't rely + on. RefreshToken has no DB cascade at all, so it must be cleared here.""" + user = await self._require_user(user_id) + for model in (UserRole, UserAccessToken, OAuthAccount, RefreshToken): + await self._db.execute(delete(model).where(model.user_id == user_id)) + await self._db.delete(user) + await self._db.flush() +``` + +- [ ] **Step 5: Run the service test to verify it passes** + +Run: `uv run pytest modules/users/tests/test_service_admin.py -k delete_user -v` +Expected: PASS (2 tests). + +- [ ] **Step 6: Write the failing API test** + +Append to `modules/users/tests/test_api_admin.py`: + +```python +# --------------------------------------------------------------------------- +# Admin delete +# --------------------------------------------------------------------------- + + +class TestAdminDelete: + @pytest.mark.anyio + async def test_delete_returns_204(self, admin_client, users_db): + user = await _make_user(users_db, email="deleteme@example.com") + resp = await admin_client.delete(f"/api/users/admin/{user.id}") + assert resp.status_code == 204 + + @pytest.mark.anyio + async def test_delete_nonexistent_returns_404(self, admin_client): + resp = await admin_client.delete(f"/api/users/admin/{uuid.uuid4()}") + assert resp.status_code == 404 + + @pytest.mark.anyio + async def test_delete_self_returns_400(self, admin_client, users_app): + from sqlalchemy import select + + async with users_app.state.sm.db.session_factory() as session: + admin = ( + await session.execute( + select(User).where(User.email == "admin@example.com") + ) + ).scalar_one() + resp = await admin_client.delete(f"/api/users/admin/{admin.id}") + assert resp.status_code == 400 + + @pytest.mark.anyio + async def test_delete_without_auth_is_rejected(self, anon_client): + resp = await anon_client.delete( + f"/api/users/admin/{uuid.uuid4()}", + follow_redirects=False, + ) + assert resp.status_code == 401 +``` + +- [ ] **Step 7: Run the API test to verify it fails** + +Run: `uv run pytest modules/users/tests/test_api_admin.py::TestAdminDelete -v` +Expected: FAIL — `DELETE /api/users/admin/{id}` returns 405 (endpoint doesn't exist). + +- [ ] **Step 8: Add the `DELETE "/{user_id}"` endpoint** + +In `modules/users/users/admin/api.py`: + +Add `Response` to the fastapi import: + +```python +from fastapi import APIRouter, Depends, HTTPException, Request, Response +``` + +Add `UserDeleted` to the events import: + +```python +from users.contracts.events import ( + RoleAssigned, + UserCreated, + UserDeleted, + UserDisabled, + UserInvited, +) +``` + +Add the endpoint after `admin_update_user` (before `admin_disable_user`): + +```python +@admin_router.delete("/{user_id}", status_code=http_status.HTTP_204_NO_CONTENT) +async def admin_delete_user( + user_id: uuid.UUID, + request: Request, + bus: EventBus = Depends(get_event_bus), + service: UserService = Depends(get_user_service), +): + """Hard-delete a user. An admin cannot delete their own account.""" + actor = getattr(request.state, "user", None) + if actor is not None and str(user_id) == actor.id: + raise HTTPException( + status_code=400, + detail="You cannot delete your own account.", + ) + try: + await service.delete_user(user_id) + except UserNotFoundError: + raise HTTPException(status_code=404, detail="User not found") from None + await bus.publish(UserDeleted(user_id=user_id)) + return Response(status_code=http_status.HTTP_204_NO_CONTENT) +``` + +- [ ] **Step 9: Run the API test to verify it passes** + +Run: `uv run pytest modules/users/tests/test_api_admin.py::TestAdminDelete -v` +Expected: PASS (4 tests). + +- [ ] **Step 10: Run the whole admin API + service suite to confirm no route collisions** + +Run: `uv run pytest modules/users/tests/test_api_admin.py modules/users/tests/test_service_admin.py modules/users/tests/test_api_admin_filters.py -v` +Expected: PASS (existing + new tests; `PATCH/DELETE /{user_id}` do not shadow `/{user_id}/disable` etc.). + +- [ ] **Step 11: Commit** + +```bash +git add modules/users/users/contracts/events.py modules/users/users/admin/service.py \ + modules/users/users/admin/api.py \ + modules/users/tests/test_service_admin.py modules/users/tests/test_api_admin.py +git commit -m "feat(users): admin delete-user endpoint (hard delete, self-delete guarded)" +``` + +--- + +## Task 5: View route — `GET /admin/create` + +**Files:** +- Modify: `modules/users/users/admin/views.py` (+ create-page route + page constant) +- Test: `modules/users/tests/test_views_admin.py` + +**Interfaces:** +- Consumes: `_roles_payload(app)` (existing helper). +- Produces: view route `GET /users/admin/create` rendering Inertia component `Users/Users/Create` with prop `roles`. + +- [ ] **Step 1: Write the failing view test** + +Append to `modules/users/tests/test_views_admin.py`: + +```python +# --------------------------------------------------------------------------- +# Admin create page +# --------------------------------------------------------------------------- + + +class TestAdminCreatePage: + @pytest.mark.anyio + async def test_create_page_renders_with_roles(self, admin_client): + resp = await admin_client.get( + "/users/admin/create", + headers={"X-Inertia": "true", "Accept": "application/json"}, + ) + assert resp.status_code == 200 + data = resp.json() + assert data["component"] == "Users/Users/Create" + assert "roles" in data["props"] + + @pytest.mark.anyio + async def test_create_page_requires_auth(self, anon_client): + resp = await anon_client.get("/users/admin/create", follow_redirects=False) + assert resp.status_code == 302 +``` + +- [ ] **Step 2: Run the view test to verify it fails** + +Run: `uv run pytest modules/users/tests/test_views_admin.py::TestAdminCreatePage -v` +Expected: FAIL — `test_create_page_renders_with_roles` hits the edit route (the `/admin/{user_id}` route catches `create` as a user_id and 404s). Confirms route ordering matters. + +- [ ] **Step 3: Add the create-page route (before the `/admin/{user_id}` route)** + +In `modules/users/users/admin/views.py`: + +Add the page constant after `_PAGE_ADMIN_INVITE`: + +```python +_PAGE_ADMIN_CREATE = "Users/Users/Create" +``` + +Insert this route between `admin_invite_page` and `admin_edit_page` (it MUST come before `admin_edit_page`, which owns `/admin/{user_id}`): + +```python +@router.get( + "/admin/create", + response_model=None, + dependencies=[Depends(RequiresPermission(PERM_USERS_MANAGE))], +) +async def admin_create_page( + request: Request, + inertia: InertiaDep, +) -> InertiaResponse: + return await inertia.render( + _PAGE_ADMIN_CREATE, + { + "roles": await _roles_payload(request.app), + }, + ) +``` + +- [ ] **Step 4: Run the view test to verify it passes** + +Run: `uv run pytest modules/users/tests/test_views_admin.py::TestAdminCreatePage -v` +Expected: PASS (2 tests). + +- [ ] **Step 5: Commit** + +```bash +git add modules/users/users/admin/views.py modules/users/tests/test_views_admin.py +git commit -m "feat(users): admin create-user view route" +``` + +--- + +## Task 6: Frontend — Create page + "Create user" button + +**Files:** +- Create: `modules/users/users/pages/Users/Create.tsx` +- Modify: `modules/users/users/pages/Users/Index.tsx` (actions: add "Create user") + +**Interfaces:** +- Consumes: view route `GET /users/admin/create` (props `roles`), endpoint `POST /api/users/admin`. + +- [ ] **Step 1: Create `Create.tsx`** + +Create `modules/users/users/pages/Users/Create.tsx`: + +```tsx +import { Link, router, usePage } from '@inertiajs/react'; +import { PageShell } from '@simple-module-py/ui/components/PageShell'; +import { Button } from '@simple-module-py/ui/components/ui/button'; +import { Card, CardContent } from '@simple-module-py/ui/components/ui/card'; +import { Input } from '@simple-module-py/ui/components/ui/input'; +import { Label } from '@simple-module-py/ui/components/ui/label'; +import { AuthenticatedLayout } from '@simple-module-py/ui/layouts/AuthenticatedLayout'; +import { Lock, Mail, UserPlus } from 'lucide-react'; +import { useState } from 'react'; +import { toast } from 'sonner'; + +interface Role { + id: string; + name: string; +} + +interface Props { + roles: Role[]; +} + +function Create() { + const { roles } = usePage<{ props: Props }>().props as unknown as Props; + + const [email, setEmail] = useState(''); + const [fullName, setFullName] = useState(''); + const [password, setPassword] = useState(''); + const [selectedRoles, setSelectedRoles] = useState([]); + const [error, setError] = useState(null); + const [loading, setLoading] = useState(false); + + const toggleRole = (roleName: string) => { + setSelectedRoles((prev) => + prev.includes(roleName) ? prev.filter((r) => r !== roleName) : [...prev, roleName], + ); + }; + + const handleSubmit = (e: React.FormEvent) => { + e.preventDefault(); + setError(null); + setLoading(true); + fetch('/api/users/admin', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + email, + password, + full_name: fullName || null, + role_names: selectedRoles, + }), + }) + .then(async (res) => { + if (res.ok) { + toast.success('User created'); + router.visit('/users/admin'); + } else { + const data = await res.json().catch(() => ({})); + setError(typeof data?.detail === 'string' ? data.detail : 'Failed to create user'); + } + }) + .catch(() => setError('An error occurred. Please try again.')) + .finally(() => setLoading(false)); + }; + + return ( + + Back to Users + + } + > + + +
+
+ +
+ + setEmail(e.target.value)} + placeholder="teammate@example.com" + required + autoComplete="off" + className="pl-9" + /> +
+
+ +
+ + setFullName(e.target.value)} + placeholder="Jane Doe" + /> +
+ +
+ +
+ + setPassword(e.target.value)} + placeholder="At least 8 characters" + required + autoComplete="new-password" + className="pl-9" + /> +
+

+ Must be at least 8 characters and not all numbers. +

+
+ + {roles.length > 0 && ( +
+ +
+ {roles.map((role) => { + const active = selectedRoles.includes(role.name); + return ( + + ); + })} +
+
+ )} + + {error &&

{error}

} + +
+ + +
+
+
+
+
+ ); +} + +Create.layout = (page: React.ReactNode) => {page}; +export default Create; +``` + +- [ ] **Step 2: Add the "Create user" button to `Index.tsx`** + +In `modules/users/users/pages/Users/Index.tsx`, replace the `actions` prop of `PageShell` (currently a single Button linking to `/users/admin/invite`) with two buttons: + +```tsx + actions={ +
+ + +
+ } +``` + +(`Plus` and `Mail` are already imported in `Index.tsx`.) + +- [ ] **Step 3: Regenerate module page manifests so Vite/tsc pick up the new page** + +Run: `make gen-pages` +Expected: `host/client_app/modules.generated.ts` now references `Users/Create`. + +- [ ] **Step 4: Typecheck + lint the frontend changes** + +Run: `npx biome check modules/users/users/pages/Users/Create.tsx modules/users/users/pages/Users/Index.tsx` +Expected: no errors. Then run the per-workspace TS check (`make lint` runs the full sweep) — Expected: no `tsc` errors. + +- [ ] **Step 5: Verify both files are under the line cap** + +Run: `uv run python scripts/check_file_size.py modules/users/users/pages/Users/Create.tsx modules/users/users/pages/Users/Index.tsx` +Expected: no violations (Create.tsx ≈ 200 lines, Index.tsx ≈ 275 lines). + +- [ ] **Step 6: Commit** + +```bash +git add modules/users/users/pages/Users/Create.tsx modules/users/users/pages/Users/Index.tsx \ + host/client_app/modules.generated.ts host/client_app/modules.manifest.json \ + host/client_app/modules.generated.css +git commit -m "feat(users): create-user page + Create button on admin index" +``` + +--- + +## Task 7: Frontend — DetailsCard + DangerZone wired into Edit + +**Files:** +- Create: `modules/users/users/pages/Users/components/DetailsCard.tsx` +- Create: `modules/users/users/pages/Users/components/DangerZone.tsx` +- Modify: `modules/users/users/pages/Users/Edit.tsx` (render both; pass current-user id) + +**Interfaces:** +- Consumes: endpoints `PATCH /api/users/admin/{id}`, `DELETE /api/users/admin/{id}`; Inertia shared prop `auth.user.id` (string). +- Produces: + - `DetailsCard({ user }: { user: { id: string; email: string; full_name: string | null } })`. + - `DangerZone({ userId, email, isSelf }: { userId: string; email: string; isSelf: boolean })`. + +- [ ] **Step 1: Create `components/DetailsCard.tsx`** + +Create `modules/users/users/pages/Users/components/DetailsCard.tsx`: + +```tsx +import { SectionTitle } from '@simple-module-py/ui/components/SectionTitle'; +import { Button } from '@simple-module-py/ui/components/ui/button'; +import { Card, CardContent } from '@simple-module-py/ui/components/ui/card'; +import { Input } from '@simple-module-py/ui/components/ui/input'; +import { Label } from '@simple-module-py/ui/components/ui/label'; +import { useState } from 'react'; +import { toast } from 'sonner'; + +interface Props { + user: { id: string; email: string; full_name: string | null }; +} + +export function DetailsCard({ user }: Props) { + const [email, setEmail] = useState(user.email); + const [fullName, setFullName] = useState(user.full_name ?? ''); + const [saving, setSaving] = useState(false); + const [error, setError] = useState(null); + + const handleSave = () => { + setSaving(true); + setError(null); + fetch(`/api/users/admin/${user.id}`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email, full_name: fullName || null }), + }) + .then(async (res) => { + if (res.ok) { + toast.success('Details updated'); + } else { + const data = await res.json().catch(() => ({})); + setError(typeof data?.detail === 'string' ? data.detail : 'Failed to update details'); + } + }) + .catch(() => setError('An error occurred')) + .finally(() => setSaving(false)); + }; + + return ( + + + + {saving ? 'Saving…' : 'Save details'} + + } + > + Details + +
+
+ + setEmail(e.target.value)} + autoComplete="off" + /> +
+
+ + setFullName(e.target.value)} + placeholder="Jane Doe" + /> +
+
+ {error &&

{error}

} +
+
+ ); +} +``` + +- [ ] **Step 2: Create `components/DangerZone.tsx`** + +Create `modules/users/users/pages/Users/components/DangerZone.tsx`: + +```tsx +import { router } from '@inertiajs/react'; +import { SectionTitle } from '@simple-module-py/ui/components/SectionTitle'; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, + AlertDialogTrigger, +} from '@simple-module-py/ui/components/ui/alert-dialog'; +import { Button } from '@simple-module-py/ui/components/ui/button'; +import { Card, CardContent } from '@simple-module-py/ui/components/ui/card'; +import { Trash2 } from 'lucide-react'; +import { useState } from 'react'; +import { toast } from 'sonner'; + +interface Props { + userId: string; + email: string; + isSelf: boolean; +} + +export function DangerZone({ userId, email, isSelf }: Props) { + const [deleting, setDeleting] = useState(false); + + const handleDelete = () => { + setDeleting(true); + fetch(`/api/users/admin/${userId}`, { method: 'DELETE' }) + .then(async (res) => { + if (res.ok) { + toast.success('User deleted'); + router.visit('/users/admin'); + } else { + const data = await res.json().catch(() => ({})); + toast.error(typeof data?.detail === 'string' ? data.detail : 'Failed to delete user'); + } + }) + .catch(() => toast.error('An error occurred')) + .finally(() => setDeleting(false)); + }; + + return ( + + + Danger zone + {isSelf ? ( +

You cannot delete your own account.

+ ) : ( +
+

+ Permanently delete this user. This cannot be undone. +

+ + + + + + + Delete {email}? + + This permanently removes the account and all of its access. This action cannot + be undone. + + + + Cancel + Delete + + + +
+ )} +
+
+ ); +} +``` + +- [ ] **Step 3: Wire `DetailsCard` + `DangerZone` into `Edit.tsx`** + +In `modules/users/users/pages/Users/Edit.tsx`: + +Add the two component imports next to the existing `AccountStatusCard` import: + +```tsx +import { AccountStatusCard } from './components/AccountStatusCard'; +import { DangerZone } from './components/DangerZone'; +import { DetailsCard } from './components/DetailsCard'; +``` + +Extend the `Props` interface to read the current user from the shared `auth` prop: + +```tsx +interface Props { + user: UserListItem; + roles: Role[]; + has_permissions_module: boolean; + auth?: { user?: { id?: string } }; +} +``` + +Change the `usePage` destructure and compute `isSelf`: + +```tsx + const { user, roles, has_permissions_module, auth } = usePage<{ props: Props }>() + .props as unknown as Props; + const isSelf = auth?.user?.id === user.id; +``` + +Render `DetailsCard` as the first child inside `
` (immediately before the Metadata ``): + +```tsx +
+ + + + {/* ...existing Metadata card unchanged... */} +``` + +Render `DangerZone` as the last child inside that grid, immediately before its closing `
` (after the Roles `
`): + +```tsx + +
+``` + +- [ ] **Step 4: Typecheck + lint the frontend changes** + +Run: `npx biome check modules/users/users/pages/Users/components/DetailsCard.tsx modules/users/users/pages/Users/components/DangerZone.tsx modules/users/users/pages/Users/Edit.tsx` +Expected: no errors. Then run `make lint` (full sweep) — Expected: no `tsc` errors. + +- [ ] **Step 5: Verify all three files are under the line cap** + +Run: `uv run python scripts/check_file_size.py modules/users/users/pages/Users/components/DetailsCard.tsx modules/users/users/pages/Users/components/DangerZone.tsx modules/users/users/pages/Users/Edit.tsx` +Expected: no violations (DetailsCard ≈ 95, DangerZone ≈ 90, Edit.tsx ≈ 245). + +- [ ] **Step 6: Commit** + +```bash +git add modules/users/users/pages/Users/components/DetailsCard.tsx \ + modules/users/users/pages/Users/components/DangerZone.tsx \ + modules/users/users/pages/Users/Edit.tsx +git commit -m "feat(users): edit-details + delete UI on admin Edit page" +``` + +--- + +## Task 8: Full verification gate + +**Files:** none changed — this is the final cross-cutting check. + +- [ ] **Step 1: Run the full Python test suite** + +Run: `make test-py` +Expected: PASS (all users tests + framework tests green). + +- [ ] **Step 2: Run the full lint sweep (Ruff/ty/Biome/tsc + 300-line cap)** + +Run: `make lint` +Expected: PASS — no formatting, type, or file-size violations across the new/changed files. + +- [ ] **Step 3: Run module diagnostics** + +Run: `make doctor` +Expected: no NEW `SM0xx` warnings. In particular, confirm no new `SM003` (orphan page) for `Users/Create` — it is rendered by the `admin_create_page` view route — and no `SM018` (the new pages use `fetch`, not Inertia `router.post` to `/api/*`). + +- [ ] **Step 4: Manual smoke (optional, requires `make dev`)** + +With `make dev` running and logged in as an admin: +1. `/users/admin` → "Create user" → fill email/password → submit → new user appears in list. +2. Log out, log in as the new user → confirms active+verified (login succeeds, no verification wall). +3. Open a user's Edit page → change email + name → Save details → toast, value persists on reload. +4. Edit page → Danger zone → Delete user → confirm → user gone from list. +5. Open your own admin account's Edit page → Danger zone shows "You cannot delete your own account." + +--- + +## Self-Review (completed by plan author) + +- **Spec coverage:** create (Task 2), edit details (Task 3), delete + self-delete guard (Task 4), view route (Task 5), Create page + Index button (Task 6), DetailsCard + DangerZone + Edit wiring (Task 7), full verification (Task 8). Service split that the spec calls out (Task 1). New schemas `UserAdminCreate`/`UserDetailsUpdate`, events `UserCreated`/`UserDeleted`, exception `EmailAlreadyExistsError` — all present. Out-of-scope items (direct password-set on existing user, last-admin lockout, verification-on-email-change) intentionally not implemented. +- **Type consistency:** `create_user(email, password, full_name, role_names, *, created_by)`, `update_details(user_id, email, full_name)`, `delete_user(user_id)` are referenced identically in service, API, and tests. `EmailAlreadyExistsError(email)` raised in service, caught in API. `DangerZone` props `{userId, email, isSelf}` and `DetailsCard` props `{user:{id,email,full_name}}` match their Edit.tsx call sites. `request.state.user.id` is a string, so the self-delete guard compares `str(user_id) == actor.id`. +- **No placeholders:** every code step contains complete code; every run step has an exact command and expected result. diff --git a/docs/superpowers/specs/2026-06-19-admin-user-crud-design.md b/docs/superpowers/specs/2026-06-19-admin-user-crud-design.md new file mode 100644 index 00000000..d8f48f33 --- /dev/null +++ b/docs/superpowers/specs/2026-06-19-admin-user-crud-design.md @@ -0,0 +1,219 @@ +# Admin user CRUD — create, edit, delete from the admin page + +**Date:** 2026-06-19 +**Module:** `users` +**Status:** Proposed + +## Problem + +The admin Users page (`/users/admin`) can only *invite* users (email a one-time +link so the recipient sets their own password). Admins cannot create a user +directly, edit a user's email or name, or delete a user. The goal is for an +admin to perform the full set of account-management operations from the UI. + +## Scope + +Add three operations, exposed under the existing `users.manage` permission: + +1. **Create user** — admin enters email, full name, password, and roles. The + user is created **active and verified** and can log in immediately. No email + is sent; the admin shares credentials out-of-band. (Invite stays as-is for + the email-link flow.) +2. **Edit details** — admin can change a user's **email** and **full name**. +3. **Delete user** — hard-delete a user account, with a confirmation step and a + guard preventing an admin from deleting their own account. + +Already present and unchanged: list/search/filter/sort, invite, disable/enable, +mark-verified, set-roles, copy-reset-link, manage-permissions. + +### Out of scope (noted so it's a deliberate choice, not an omission) + +- **Directly setting a password on an existing user.** The existing + "copy reset link" action already covers admin-driven password changes; adding + a second path is redundant. Create sets an initial password; edits to an + existing account's password go through the reset link. +- **Last-admin lockout protection.** Disable already lacks this guard, so adding + it only for delete would be inconsistent. Self-delete is guarded (the common + footgun); broader lockout protection is a separate concern. +- Changing email does **not** reset verification status. The admin is trusted; + they can toggle verification separately if needed. + +## Architecture + +No new module, no new tables, no migration. This is additive surface on the +existing `users` admin slice: + +``` +modules/users/users/ +├── admin/ +│ ├── api.py # + POST "", PATCH "/{id}", DELETE "/{id}" +│ ├── views.py # + GET "/admin/create" (Inertia page route) +│ ├── queries.py # NEW — read/query methods (split from service.py) +│ └── service.py # write/command methods + NEW create/update/delete +├── contracts/ +│ ├── schemas.py # + UserAdminCreate, UserDetailsUpdate +│ └── events.py # + UserCreated, UserDeleted +└── pages/Users/ + ├── Create.tsx # NEW — create-user form + ├── Index.tsx # + "Create user" button next to "Invite member" + ├── Edit.tsx # + wires DetailsCard + DangerZone + └── components/ + ├── DetailsCard.tsx # NEW — edit email + full name + └── DangerZone.tsx # NEW — delete with confirm dialog +``` + +### Why split `service.py` + +`admin/service.py` is at 272 / 300 lines. The three new methods would push it +over the CI line cap. Split by responsibility: + +- `admin/queries.py` — `class _UserServiceBase`: `__init__`, `_resolve_roles`, + `to_list_item`, `_get_user_with_roles`, `_require_user`, `list_users`, + `count_user_states`, `list_roles`, `get_with_roles`, `get_list_item`. +- `admin/service.py` — `class UserService(_UserServiceBase)`: the write/command + methods (`invite`, `disable`, `enable`, `mark_verified`, `set_roles`, + `generate_reset_link`) plus the three new ones below. + +`UserService` keeps its name and import path (`from users.admin.service import +UserService`), so `get_user_service` and every existing caller and test are +unchanged. + +## Components + +### 1. Schemas (`contracts/schemas.py`) + +```python +class UserAdminCreate(SQLModel): + email: EmailStr + password: str + full_name: str | None = None + role_names: list[str] = [] + +class UserDetailsUpdate(SQLModel): + email: EmailStr + full_name: str | None = None +``` + +### 2. Events (`contracts/events.py`) + +```python +@dataclass +class UserCreated(Event): + user_id: uuid.UUID + email: str + created_by: str | None + +@dataclass +class UserDeleted(Event): + user_id: uuid.UUID +``` + +### 3. Service methods (`admin/service.py`) + +**`create_user(email, password, full_name, role_names, *, created_by) -> User`** +- Build `UserCreate(email, password, full_name, is_active=True, + is_verified=True, is_superuser=False)` and call + `self._manager.create(user_create, safe=False)`. This runs the password policy + (`validate_password`) and email-uniqueness check for free, and — because + `is_verified=True` — does **not** trigger a verification email. +- Assign roles exactly as `invite` does (resolve names → insert `UserRole` rows + with `assigned_by=created_by`, flush, refresh `roles`). +- Return the user. (No mailer call.) + +**`update_details(user_id, email, full_name) -> User`** +- `_require_user(user_id)`. +- If `email` differs (case-insensitive) from the current email, check no other + user owns it: `select(User).where(func.lower(User.email) == email.lower(), + User.id != user_id)`. If found, raise `EmailAlreadyExistsError` (new, in + `exceptions.py`). +- Update `email` and `full_name`, flush, return user. + +**`delete_user(user_id) -> None`** +- `_require_user(user_id)` (404 if missing). +- Explicitly delete dependent rows so behavior is identical on Postgres and + SQLite (SQLite FK cascade depends on a pragma; don't rely on it): + `delete(UserRole)`, `delete(UserAccessToken)`, `delete(OAuthAccount)`, + `delete(RefreshToken)` all `where(... .user_id == user_id)`. +- Then `await self._db.delete(user)` and flush. + +### 4. API endpoints (`admin/api.py`, all under `users.manage`) + +| Method & path | Handler | Behavior | +|---|---|---| +| `POST /api/users/admin` | `admin_create_user` | Create. `UserAlreadyExists` → 409; `InvalidPasswordException` → 400 (detail = reason). On success publish `UserCreated`, return `UserListItem` (201). | +| `PATCH /api/users/admin/{id}` | `admin_update_user` | Edit details. `UserNotFoundError` → 404; `EmailAlreadyExistsError` → 409. Returns `UserListItem`. | +| `DELETE /api/users/admin/{id}` | `admin_delete_user` | Delete. If `id == request.state.user.id` → 400 "You cannot delete your own account." `UserNotFoundError` → 404. On success publish `UserDeleted`, return 204. | + +Route note: `POST ""` and `PATCH "/{id}"` / `DELETE "/{id}"` don't collide with +the existing `/{id}/disable` etc. The new view route `GET /admin/create` must be +declared **before** `GET /admin/{user_id}` (FastAPI matches in order), same as +the existing `/admin/invite`. + +### 5. View route (`admin/views.py`) + +`GET /admin/create` renders `Users/Users/Create` with `{"roles": +await _roles_payload(request.app)}` — mirrors `admin_invite_page`. + +### 6. Frontend + +- **`Create.tsx`** — modeled on `Invite.tsx`: email, full name, **password** + (required; helper text notes the ≥8-char rule), and role chips. Submits + `POST /api/users/admin`; on success toast + `router.visit('/users/admin')`. + Surfaces the server `detail` string on 400/409 (weak password / email taken). +- **`Index.tsx`** — add a primary **"Create user"** button (`Link` to + `/users/admin/create`) alongside the existing outline **"Invite member"**. +- **`DetailsCard.tsx`** — email + full-name inputs with a Save button; + `PATCH /api/users/admin/{id}`; toast on success, shows server `detail` on 409. +- **`DangerZone.tsx`** — "Delete user" button opening a confirm step + (AlertDialog if available in the ui package, else `window.confirm`); + `DELETE /api/users/admin/{id}`; on success toast + `router.visit('/users/admin')`. + Not rendered for the admin's own row is handled server-side, but the button + may also be hidden client-side when `user.id` is the current user (nice-to-have). +- **`Edit.tsx`** — render `DetailsCard` (top) and `DangerZone` (bottom), wiring + to keep the file under the line cap. + +## Data flow (create) + +1. Admin submits Create form → `POST /api/users/admin` (JSON). +2. `RequiresPermission(users.manage)` gate → `admin_create_user`. +3. `service.create_user(...)` → `manager.create` (hash + policy + uniqueness) → + role rows → flush. Per-request session auto-commits (pending writes). +4. Publish `UserCreated`. Return `UserListItem` (201). +5. Front end toasts and redirects to the list, where the new user appears. + +## Error handling + +- Weak password → 400, `detail` = the policy reason; shown inline on the form. +- Duplicate email (create or edit) → 409, `detail` = "A user with this email + already exists."; shown inline. +- Missing user (edit/delete) → 404. +- Self-delete → 400 with a clear message; the delete button is also hidden for + the current user client-side. +- All mutations require `users.manage`; unauthenticated `/api/*` → 401 (existing + middleware behavior). + +## Testing + +Mirror existing `tests/test_service_admin.py`, `test_api_admin.py`, +`test_views_admin.py` patterns (`users_app`, `authenticated_client`). + +- **Service** (`test_service_admin.py` additions): + - `create_user` creates an active+verified user, hashes the password, assigns + roles, and is rejected for a weak password / duplicate email. + - `update_details` updates email + name; rejects an email owned by another user. + - `delete_user` removes the user and its `UserRole` / token / oauth rows; + raises `UserNotFoundError` for a missing id. +- **API** (`test_api_admin.py` additions): + - `POST /admin` 201 + body; 409 on duplicate; 400 on weak password; 401/403 + without permission. + - `PATCH /admin/{id}` 200; 409 on duplicate email; 404 missing. + - `DELETE /admin/{id}` 204; 404 missing; **400 when deleting self**. +- **Views** (`test_views_admin.py` addition): `GET /admin/create` renders the + `Create` page with roles in props. + +## Verification + +- `make test-py` (users suite green), `make lint` (Ruff/ty/Biome/tsc + 300-line + cap all pass), `make doctor` (no new SM00x/SM01x warnings). +- Manual: create a user and log in as them; edit an email; delete a user and + confirm they're gone; confirm self-delete is blocked. diff --git a/modules/users/tests/test_api_admin_crud.py b/modules/users/tests/test_api_admin_crud.py new file mode 100644 index 00000000..5793d0d0 --- /dev/null +++ b/modules/users/tests/test_api_admin_crud.py @@ -0,0 +1,163 @@ +"""Admin REST CRUD tests: create / update / delete.""" + +from __future__ import annotations + +import uuid + +import pytest +from sqlalchemy import select +from test_api_admin import _make_user +from users.models import User, UserRole + +# --------------------------------------------------------------------------- +# Admin create +# --------------------------------------------------------------------------- + + +class TestAdminCreate: + @pytest.mark.anyio + async def test_create_returns_201(self, admin_client): + resp = await admin_client.post( + "/api/users/admin", + json={ + "email": "newuser@example.com", + "password": "SecurePass1!", + "full_name": "New User", + "role_names": ["user"], + }, + ) + assert resp.status_code == 201 + body = resp.json() + assert body["email"] == "newuser@example.com" + assert body["is_active"] is True + assert body["is_verified"] is True + assert body["roles"] == ["user"] + + @pytest.mark.anyio + async def test_create_duplicate_returns_409(self, admin_client, users_db): + await _make_user(users_db, email="taken@example.com") + resp = await admin_client.post( + "/api/users/admin", + json={"email": "taken@example.com", "password": "SecurePass1!"}, + ) + assert resp.status_code == 409 + assert "already exists" in resp.json()["detail"] + + @pytest.mark.anyio + async def test_create_weak_password_returns_400(self, admin_client): + resp = await admin_client.post( + "/api/users/admin", + json={"email": "weakpw@example.com", "password": "short"}, + ) + assert resp.status_code == 400 + assert "8 characters" in resp.json()["detail"] + + @pytest.mark.anyio + async def test_create_without_auth_is_rejected(self, anon_client): + resp = await anon_client.post( + "/api/users/admin", + json={"email": "hacker@example.com", "password": "SecurePass1!"}, + follow_redirects=False, + ) + assert resp.status_code == 401 + assert resp.json() == {"detail": "Not authenticated"} + + +# --------------------------------------------------------------------------- +# Admin update details +# --------------------------------------------------------------------------- + + +class TestAdminUpdate: + @pytest.mark.anyio + async def test_update_changes_email_and_name(self, admin_client, users_db): + user = await _make_user(users_db, email="before@example.com") + resp = await admin_client.patch( + f"/api/users/admin/{user.id}", + json={"email": "after@example.com", "full_name": "After Name"}, + ) + assert resp.status_code == 200 + body = resp.json() + assert body["email"] == "after@example.com" + assert body["full_name"] == "After Name" + + @pytest.mark.anyio + async def test_update_duplicate_email_returns_409(self, admin_client, users_db): + await _make_user(users_db, email="exists@example.com") + target = await _make_user(users_db, email="target@example.com") + resp = await admin_client.patch( + f"/api/users/admin/{target.id}", + json={"email": "exists@example.com"}, + ) + assert resp.status_code == 409 + + @pytest.mark.anyio + async def test_update_nonexistent_returns_404(self, admin_client): + resp = await admin_client.patch( + f"/api/users/admin/{uuid.uuid4()}", + json={"email": "ghost@example.com"}, + ) + assert resp.status_code == 404 + + @pytest.mark.anyio + async def test_update_without_auth_is_rejected(self, anon_client): + resp = await anon_client.patch( + f"/api/users/admin/{uuid.uuid4()}", + json={"email": "x@example.com"}, + follow_redirects=False, + ) + assert resp.status_code == 401 + + +# --------------------------------------------------------------------------- +# Admin delete +# --------------------------------------------------------------------------- + + +class TestAdminDelete: + @pytest.mark.anyio + async def test_delete_returns_204(self, admin_client, users_db): + user = await _make_user(users_db, email="deleteme@example.com") + resp = await admin_client.delete(f"/api/users/admin/{user.id}") + assert resp.status_code == 204 + + @pytest.mark.anyio + async def test_delete_user_with_role_returns_204(self, admin_client, users_db): + """Regression: deleting a user that HAS a role must not 500. + + The delete runs in the request session, which eager-loads the user's + ``roles`` relationship. Bulk-deleting the ``users_user_role`` rows the + ORM is tracking raised ``StaleDataError`` on flush. The roleless + ``test_delete_returns_204`` never exercised this path.""" + user = await _make_user(users_db, email="hasrole@example.com", role_names=["admin"]) + resp = await admin_client.delete(f"/api/users/admin/{user.id}") + assert resp.status_code == 204 + # The role association row is gone too (no orphan). + rows = ( + (await users_db.execute(select(UserRole).where(UserRole.user_id == user.id))) + .scalars() + .all() + ) + assert rows == [] + + @pytest.mark.anyio + async def test_delete_nonexistent_returns_404(self, admin_client): + resp = await admin_client.delete(f"/api/users/admin/{uuid.uuid4()}") + assert resp.status_code == 404 + + @pytest.mark.anyio + async def test_delete_self_returns_400(self, admin_client, users_app): + async with users_app.state.sm.db.session_factory() as session: + admin = ( + await session.execute(select(User).where(User.email == "admin@example.com")) + ).scalar_one() + resp = await admin_client.delete(f"/api/users/admin/{admin.id}") + assert resp.status_code == 400 + + @pytest.mark.anyio + async def test_delete_without_auth_is_rejected(self, anon_client): + resp = await anon_client.delete( + f"/api/users/admin/{uuid.uuid4()}", + follow_redirects=False, + ) + assert resp.status_code == 401 diff --git a/modules/users/tests/test_negative_authz.py b/modules/users/tests/test_negative_authz.py index 76c22100..71197b74 100644 --- a/modules/users/tests/test_negative_authz.py +++ b/modules/users/tests/test_negative_authz.py @@ -27,6 +27,13 @@ ("PUT", f"/api/users/admin/{_FAKE_ID}/roles", {"role_names": []}), ("PATCH", f"/api/users/admin/{_FAKE_ID}/verify", None), ("POST", f"/api/users/admin/{_FAKE_ID}/reset-password-link", None), + ( + "POST", + "/api/users/admin", + {"email": "x@y.test", "password": "SecurePass1!", "role_names": []}, + ), + ("PATCH", f"/api/users/admin/{_FAKE_ID}", {"email": "x@y.test"}), + ("DELETE", f"/api/users/admin/{_FAKE_ID}", None), # permissions — root GET lists registered groups (PERM_VIEW) ("GET", "/api/permissions/", None), ("GET", f"/api/permissions/roles/{_FAKE_ID}", None), diff --git a/modules/users/tests/test_service_admin_crud.py b/modules/users/tests/test_service_admin_crud.py new file mode 100644 index 00000000..6e0cd02e --- /dev/null +++ b/modules/users/tests/test_service_admin_crud.py @@ -0,0 +1,172 @@ +"""UserService write-op tests: create / update / delete.""" + +from __future__ import annotations + +import uuid + +import pytest +from test_service_admin import _build_service + +# --------------------------------------------------------------------------- +# create_user +# --------------------------------------------------------------------------- + + +@pytest.mark.anyio +async def test_create_user_active_verified_with_roles(users_app): + """create_user makes an active+verified user and assigns roles.""" + async with users_app.state.sm.db.session_factory() as session: + svc = _build_service(session, users_app) + user = await svc.create_user( + email="created@example.com", + password="SecurePass1!", + full_name="Created User", + role_names=["user"], + created_by="admin-id", + ) + assert user.is_active is True + assert user.is_verified is True + assert user.email == "created@example.com" + assert user.full_name == "Created User" + assert [r.name for r in user.roles] == ["user"] + + +@pytest.mark.anyio +async def test_create_user_weak_password_rejected(users_app): + from fastapi_users import exceptions as fa_exc + + async with users_app.state.sm.db.session_factory() as session: + svc = _build_service(session, users_app) + with pytest.raises(fa_exc.InvalidPasswordException): + await svc.create_user( + email="weak@example.com", + password="short", + full_name=None, + role_names=[], + created_by=None, + ) + + +@pytest.mark.anyio +async def test_create_user_duplicate_email_rejected(users_app): + from fastapi_users import exceptions as fa_exc + + async with users_app.state.sm.db.session_factory() as session: + svc = _build_service(session, users_app) + await svc.create_user( + email="dup@example.com", + password="SecurePass1!", + full_name=None, + role_names=[], + created_by=None, + ) + await session.flush() + with pytest.raises(fa_exc.UserAlreadyExists): + await svc.create_user( + email="dup@example.com", + password="SecurePass1!", + full_name=None, + role_names=[], + created_by=None, + ) + + +# --------------------------------------------------------------------------- +# update_details +# --------------------------------------------------------------------------- + + +@pytest.mark.anyio +async def test_update_details_changes_email_and_name(users_app): + from test_api_admin import _make_user + + async with users_app.state.sm.db.session_factory() as session: + user = await _make_user(session, email="old@example.com") + svc = _build_service(session, users_app) + updated = await svc.update_details(user.id, email="new@example.com", full_name="New Name") + assert updated.email == "new@example.com" + assert updated.full_name == "New Name" + + +@pytest.mark.anyio +async def test_update_details_duplicate_email_rejected(users_app): + from test_api_admin import _make_user + from users.exceptions import EmailAlreadyExistsError + + async with users_app.state.sm.db.session_factory() as session: + await _make_user(session, email="a@example.com") + target = await _make_user(session, email="b@example.com") + svc = _build_service(session, users_app) + with pytest.raises(EmailAlreadyExistsError): + await svc.update_details(target.id, email="a@example.com", full_name=None) + + +@pytest.mark.anyio +async def test_update_details_same_email_is_allowed(users_app): + from test_api_admin import _make_user + + async with users_app.state.sm.db.session_factory() as session: + user = await _make_user(session, email="keep@example.com") + svc = _build_service(session, users_app) + updated = await svc.update_details(user.id, email="keep@example.com", full_name="Renamed") + assert updated.email == "keep@example.com" + assert updated.full_name == "Renamed" + + +# --------------------------------------------------------------------------- +# delete_user +# --------------------------------------------------------------------------- + + +@pytest.mark.anyio +async def test_delete_user_removes_user_and_all_child_rows(users_app): + from datetime import UTC, datetime, timedelta + + from sqlalchemy import select + from test_api_admin import _make_user + from users.models import ( + OAuthAccount, + RefreshToken, + User, + UserAccessToken, + UserRole, + ) + + async with users_app.state.sm.db.session_factory() as session: + user = await _make_user(session, email="todelete@example.com", role_names=["admin"]) + session.add(UserAccessToken(token=f"tok-{user.id.hex}", user_id=user.id)) + session.add( + OAuthAccount( + user_id=user.id, + oauth_name="google", + access_token="x", + account_id="acct-1", + account_email="todelete@example.com", + ) + ) + session.add(RefreshToken(user_id=user.id, expires_at=datetime.now(UTC) + timedelta(days=1))) + await session.flush() + + svc = _build_service(session, users_app) + await svc.delete_user(user.id) + + assert ( + await session.execute(select(User).where(User.id == user.id)) + ).scalar_one_or_none() is None + for model in (UserRole, UserAccessToken, OAuthAccount, RefreshToken): + rows = ( + (await session.execute(select(model).where(model.user_id == user.id))) + .scalars() + .all() + ) + assert rows == [], f"{model.__name__} rows not cleaned up" + + +@pytest.mark.anyio +async def test_delete_user_nonexistent_raises(users_app): + from users.exceptions import UserNotFoundError + + async with users_app.state.sm.db.session_factory() as session: + svc = _build_service(session, users_app) + with pytest.raises(UserNotFoundError): + await svc.delete_user(uuid.uuid4()) diff --git a/modules/users/tests/test_views_admin.py b/modules/users/tests/test_views_admin.py index 4ed61d19..82391419 100644 --- a/modules/users/tests/test_views_admin.py +++ b/modules/users/tests/test_views_admin.py @@ -126,3 +126,26 @@ async def test_flag_false_when_not_installed(self, admin_client, users_app, user assert resp.status_code == 200 props = resp.json()["props"] assert props["has_permissions_module"] is False + + +# --------------------------------------------------------------------------- +# Admin create page +# --------------------------------------------------------------------------- + + +class TestAdminCreatePage: + @pytest.mark.anyio + async def test_create_page_renders_with_roles(self, admin_client): + resp = await admin_client.get( + "/users/admin/create", + headers={"X-Inertia": "true", "Accept": "application/json"}, + ) + assert resp.status_code == 200 + data = resp.json() + assert data["component"] == "Users/Users/Create" + assert "roles" in data["props"] + + @pytest.mark.anyio + async def test_create_page_requires_auth(self, anon_client): + resp = await anon_client.get("/users/admin/create", follow_redirects=False) + assert resp.status_code == 302 diff --git a/modules/users/users/admin/api.py b/modules/users/users/admin/api.py index 71f1ff39..42daaa59 100644 --- a/modules/users/users/admin/api.py +++ b/modules/users/users/admin/api.py @@ -4,22 +4,31 @@ import uuid -from fastapi import APIRouter, Depends, HTTPException, Request +from fastapi import APIRouter, Depends, HTTPException, Request, Response from fastapi import status as http_status +from fastapi_users import exceptions as fa_exceptions from simple_module_core.events import EventBus from simple_module_hosting.permissions import RequiresPermission from users.admin.service import UserService from users.constants import PERM_USERS_MANAGE, sanitize_list_filters -from users.contracts.events import RoleAssigned, UserDisabled, UserInvited +from users.contracts.events import ( + RoleAssigned, + UserCreated, + UserDeleted, + UserDisabled, + UserInvited, +) from users.contracts.schemas import ( PasswordResetLink, RoleAssignment, + UserAdminCreate, + UserDetailsUpdate, UserInvite, UserListItem, ) from users.deps import get_event_bus, get_mailer, get_user_service -from users.exceptions import UserNotFoundError +from users.exceptions import EmailAlreadyExistsError, UserNotFoundError admin_router = APIRouter( prefix="/admin", @@ -84,6 +93,80 @@ async def admin_invite_user( return service.to_list_item(user) +@admin_router.post( + "", + response_model=UserListItem, + status_code=http_status.HTTP_201_CREATED, +) +async def admin_create_user( + data: UserAdminCreate, + request: Request, + bus: EventBus = Depends(get_event_bus), + service: UserService = Depends(get_user_service), +): + """Create an active+verified user with an admin-set password.""" + creator = getattr(request.state, "user", None) + created_by = str(creator.id) if creator else None + try: + user = await service.create_user( + data.email, + data.password, + data.full_name, + data.role_names, + created_by=created_by, + ) + except fa_exceptions.UserAlreadyExists: + raise HTTPException( + status_code=409, + detail="A user with this email already exists.", + ) from None + except fa_exceptions.InvalidPasswordException as exc: + raise HTTPException(status_code=400, detail=exc.reason) from None + await bus.publish(UserCreated(user_id=user.id, email=user.email, created_by=created_by)) + return service.to_list_item(user) + + +@admin_router.patch("/{user_id}", response_model=UserListItem) +async def admin_update_user( + user_id: uuid.UUID, + data: UserDetailsUpdate, + service: UserService = Depends(get_user_service), +): + """Update a user's email and full name.""" + try: + user = await service.update_details(user_id, data.email, data.full_name) + except UserNotFoundError: + raise HTTPException(status_code=404, detail="User not found") from None + except EmailAlreadyExistsError: + raise HTTPException( + status_code=409, + detail="A user with this email already exists.", + ) from None + return service.to_list_item(user) + + +@admin_router.delete("/{user_id}", status_code=http_status.HTTP_204_NO_CONTENT) +async def admin_delete_user( + user_id: uuid.UUID, + request: Request, + bus: EventBus = Depends(get_event_bus), + service: UserService = Depends(get_user_service), +): + """Hard-delete a user. An admin cannot delete their own account.""" + actor = getattr(request.state, "user", None) + if actor is not None and str(user_id) == actor.id: + raise HTTPException( + status_code=400, + detail="You cannot delete your own account.", + ) + try: + await service.delete_user(user_id) + except UserNotFoundError: + raise HTTPException(status_code=404, detail="User not found") from None + await bus.publish(UserDeleted(user_id=user_id)) + return Response(status_code=http_status.HTTP_204_NO_CONTENT) + + @admin_router.patch("/{user_id}/disable", response_model=UserListItem) async def admin_disable_user( user_id: uuid.UUID, diff --git a/modules/users/users/admin/queries.py b/modules/users/users/admin/queries.py new file mode 100644 index 00000000..8d05aeac --- /dev/null +++ b/modules/users/users/admin/queries.py @@ -0,0 +1,176 @@ +"""Read/query helpers for the admin UserService (split from service.py).""" + +from __future__ import annotations + +import uuid + +from sqlalchemy import func, or_, select +from sqlalchemy.ext.asyncio import AsyncSession +from sqlalchemy.orm import selectinload + +from users.contracts.schemas import RoleListItem, UserListItem +from users.exceptions import UserNotFoundError +from users.manager import UserManager +from users.models import Role, User, UserRole + + +class _UserServiceBase: + def __init__( + self, + db: AsyncSession, + user_manager: UserManager, + ) -> None: + self._db = db + self._manager = user_manager + + # ── Helpers ───────────────────────────────────────────────── + + async def _resolve_roles(self, role_names: list[str]) -> list[Role]: + """Return Role ORM objects matching the given names.""" + if not role_names: + return [] + result = await self._db.execute(select(Role).where(Role.name.in_(role_names))) + return list(result.scalars().all()) + + def to_list_item(self, user: User) -> UserListItem: + """Build the DTO from a User with roles already eager-loaded.""" + return UserListItem( + id=user.id, + email=user.email, + full_name=user.full_name, + is_active=user.is_active, + is_verified=user.is_verified, + disabled_at=user.disabled_at, + last_login_at=user.last_login_at, + created_at=user.created_at, + roles=[r.name for r in user.roles], + ) + + async def list_roles(self) -> list[RoleListItem]: + stmt = ( + select(Role, func.count(UserRole.user_id)) + .outerjoin(UserRole, UserRole.role_id == Role.id) + .group_by(Role.id) + .order_by(Role.name) + ) + result = await self._db.execute(stmt) + return [ + RoleListItem( + id=role.id, + name=role.name, + description=role.description, + user_count=user_count, + ) + for role, user_count in result.all() + ] + + async def _get_user_with_roles(self, user_id: uuid.UUID) -> User | None: + result = await self._db.execute( + select(User).where(User.id == user_id).options(selectinload(User.roles)) + ) + return result.scalar_one_or_none() + + async def _require_user(self, user_id: uuid.UUID) -> User: + """Fetch a user with roles eager-loaded, or raise UserNotFoundError.""" + user = await self._get_user_with_roles(user_id) + if user is None: + raise UserNotFoundError(user_id) + return user + + # ── Queries ────────────────────────────────────────────────── + + async def list_users( + self, + *, + page: int = 1, + per_page: int = 20, + search: str | None = None, + status: str | None = None, + role_name: str | None = None, + verified: str | None = None, + sort: str = "email", + order: str = "asc", + ) -> tuple[list[UserListItem], int]: + """Returns (items, total_count). last_login_at sort always uses NULLS LAST.""" + stmt = select(User).options(selectinload(User.roles)) + count_stmt = select(func.count()).select_from(User) + + conditions = [] + + if search: + pattern = f"%{search}%" + conditions.append( + or_( + User.email.ilike(pattern), + User.full_name.ilike(pattern), + ) + ) + + if status == "active": + conditions.append(User.is_active.is_(True)) + elif status == "disabled": + conditions.append(User.is_active.is_(False)) + + if verified == "yes": + conditions.append(User.is_verified.is_(True)) + elif verified == "no": + conditions.append(User.is_verified.is_(False)) + + if role_name is not None: + subq = ( + select(UserRole.user_id) + .join(Role, Role.id == UserRole.role_id) + .where(Role.name == role_name) + ) + conditions.append(User.id.in_(subq)) + + for cond in conditions: + stmt = stmt.where(cond) + count_stmt = count_stmt.where(cond) + + total = (await self._db.execute(count_stmt)).scalar_one() + + sort_col_map = { + "email": User.email, + "last_login_at": User.last_login_at, + "created_at": User.created_at, + } + sort_col = sort_col_map.get(sort, User.email) + + if sort == "last_login_at": + order_clause = ( + sort_col.desc().nulls_last() # type: ignore[union-attr] + if order == "desc" + else sort_col.asc().nulls_last() # type: ignore[union-attr] + ) + else: + order_clause = ( + sort_col.desc() if order == "desc" else sort_col.asc() # type: ignore[union-attr] + ) + + stmt = stmt.order_by(order_clause).offset((page - 1) * per_page).limit(per_page) + rows = (await self._db.execute(stmt)).scalars().all() + + items = [self.to_list_item(u) for u in rows] + return items, total + + async def count_user_states(self) -> dict[str, int]: + """Workspace-wide counts unaffected by list filters/pagination — + feeds the dashboard cards on /users/admin so they don't reflect + the current page slice.""" + active_q = select(func.count()).select_from(User).where(User.is_active.is_(True)) + unverified_q = ( + select(func.count()) + .select_from(User) + .where(User.is_active.is_(True), User.is_verified.is_(False)) + ) + active = (await self._db.execute(active_q)).scalar_one() + unverified = (await self._db.execute(unverified_q)).scalar_one() + return {"active": int(active), "unverified": int(unverified)} + + async def get_with_roles(self, user_id: uuid.UUID) -> User | None: + return await self._get_user_with_roles(user_id) + + async def get_list_item(self, user_id: uuid.UUID) -> UserListItem: + user = await self._require_user(user_id) + return self.to_list_item(user) diff --git a/modules/users/users/admin/service.py b/modules/users/users/admin/service.py index ea947d93..1cb27a59 100644 --- a/modules/users/users/admin/service.py +++ b/modules/users/users/admin/service.py @@ -1,4 +1,4 @@ -"""UserService — admin operations delegating to the DB and UserManager.""" +"""UserService — admin write operations (reads live in queries.py).""" from __future__ import annotations @@ -6,169 +6,117 @@ import uuid from datetime import UTC, datetime -from sqlalchemy import delete, func, or_, select -from sqlalchemy.ext.asyncio import AsyncSession -from sqlalchemy.orm import selectinload +from sqlalchemy import delete, func, select +from sqlalchemy.orm import noload -from users.contracts.schemas import RoleListItem, UserCreate, UserListItem -from users.exceptions import UserNotFoundError -from users.manager import UserManager -from users.models import Role, User, UserRole +from users.admin.queries import _UserServiceBase +from users.contracts.schemas import UserCreate +from users.exceptions import EmailAlreadyExistsError, UserNotFoundError +from users.models import OAuthAccount, RefreshToken, User, UserAccessToken, UserRole -class UserService: - def __init__( +class UserService(_UserServiceBase): + async def create_user( self, - db: AsyncSession, - user_manager: UserManager, - ) -> None: - self._db = db - self._manager = user_manager - - # ── Helpers ───────────────────────────────────────────────── - - async def _resolve_roles(self, role_names: list[str]) -> list[Role]: - """Return Role ORM objects matching the given names.""" - if not role_names: - return [] - result = await self._db.execute(select(Role).where(Role.name.in_(role_names))) - return list(result.scalars().all()) + email: str, + password: str, + full_name: str | None, + role_names: list[str], + *, + created_by: str | None, + ) -> User: + """Create an active+verified user with an admin-set password. - def to_list_item(self, user: User) -> UserListItem: - """Build the DTO from a User with roles already eager-loaded.""" - return UserListItem( - id=user.id, - email=user.email, - full_name=user.full_name, - is_active=user.is_active, - is_verified=user.is_verified, - disabled_at=user.disabled_at, - last_login_at=user.last_login_at, - created_at=user.created_at, - roles=[r.name for r in user.roles], + Reuses ``manager.create`` for the password policy + email-uniqueness + check. ``is_verified=True`` means ``on_after_register`` does not fire a + verification email (and with no request, no event is published here — + the endpoint publishes ``UserCreated``).""" + user_create = UserCreate( + email=email, + password=password, + full_name=full_name, + is_active=True, + is_verified=True, + is_superuser=False, ) + user = await self._manager.create(user_create, safe=False) - async def list_roles(self) -> list[RoleListItem]: - stmt = ( - select(Role, func.count(UserRole.user_id)) - .outerjoin(UserRole, UserRole.role_id == Role.id) - .group_by(Role.id) - .order_by(Role.name) - ) - result = await self._db.execute(stmt) - return [ - RoleListItem( - id=role.id, - name=role.name, - description=role.description, - user_count=user_count, + roles = await self._resolve_roles(role_names) + for role in roles: + self._db.add( + UserRole( + user_id=user.id, + role_id=role.id, + assigned_by=created_by, + ) ) - for role, user_count in result.all() - ] - - async def _get_user_with_roles(self, user_id: uuid.UUID) -> User | None: - result = await self._db.execute( - select(User).where(User.id == user_id).options(selectinload(User.roles)) - ) - return result.scalar_one_or_none() - - async def _require_user(self, user_id: uuid.UUID) -> User: - """Fetch a user with roles eager-loaded, or raise UserNotFoundError.""" - user = await self._get_user_with_roles(user_id) - if user is None: - raise UserNotFoundError(user_id) + if roles: + await self._db.flush() + # User.roles is lazy="noload": selectinload only populates a fresh + # fetch, not an identity-map hit. Expunge first to force a reload. + user_id = user.id + self._db.expunge(user) + loaded = await self._get_user_with_roles(user_id) + if loaded is None: # impossible: row was just flushed in this txn + raise RuntimeError(f"User {user_id} vanished immediately after create") + return loaded return user - # ── Public API ─────────────────────────────────────────────── - - async def list_users( + async def update_details( self, - *, - page: int = 1, - per_page: int = 20, - search: str | None = None, - status: str | None = None, - role_name: str | None = None, - verified: str | None = None, - sort: str = "email", - order: str = "asc", - ) -> tuple[list[UserListItem], int]: - """Returns (items, total_count). last_login_at sort always uses NULLS LAST.""" - stmt = select(User).options(selectinload(User.roles)) - count_stmt = select(func.count()).select_from(User) - - conditions = [] - - if search: - pattern = f"%{search}%" - conditions.append( - or_( - User.email.ilike(pattern), - User.full_name.ilike(pattern), + user_id: uuid.UUID, + email: str, + full_name: str | None, + ) -> User: + """Update a user's email + full name. Raises UserNotFoundError if the + user is missing, EmailAlreadyExistsError if the new email is taken by + another user.""" + user = await self._require_user(user_id) + if email.lower() != user.email.lower(): + clash = await self._db.execute( + select(User.id).where( + func.lower(User.email) == email.lower(), + User.id != user_id, ) ) + if clash.scalar_one_or_none() is not None: + raise EmailAlreadyExistsError(email) + user.email = email + user.full_name = full_name + await self._db.flush() + return user - if status == "active": - conditions.append(User.is_active.is_(True)) - elif status == "disabled": - conditions.append(User.is_active.is_(False)) - - if verified == "yes": - conditions.append(User.is_verified.is_(True)) - elif verified == "no": - conditions.append(User.is_verified.is_(False)) - - if role_name is not None: - subq = ( - select(UserRole.user_id) - .join(Role, Role.id == UserRole.role_id) - .where(Role.name == role_name) - ) - conditions.append(User.id.in_(subq)) - - for cond in conditions: - stmt = stmt.where(cond) - count_stmt = count_stmt.where(cond) - - total = (await self._db.execute(count_stmt)).scalar_one() - - sort_col_map = { - "email": User.email, - "last_login_at": User.last_login_at, - "created_at": User.created_at, - } - sort_col = sort_col_map.get(sort, User.email) - - if sort == "last_login_at": - order_clause = ( - sort_col.desc().nulls_last() # type: ignore[union-attr] - if order == "desc" - else sort_col.asc().nulls_last() # type: ignore[union-attr] - ) - else: - order_clause = ( - sort_col.desc() if order == "desc" else sort_col.asc() # type: ignore[union-attr] - ) - - stmt = stmt.order_by(order_clause).offset((page - 1) * per_page).limit(per_page) - rows = (await self._db.execute(stmt)).scalars().all() - - items = [self.to_list_item(u) for u in rows] - return items, total - - async def count_user_states(self) -> dict[str, int]: - """Workspace-wide counts unaffected by list filters/pagination — - feeds the dashboard cards on /users/admin so they don't reflect - the current page slice.""" - active_q = select(func.count()).select_from(User).where(User.is_active.is_(True)) - unverified_q = ( - select(func.count()) - .select_from(User) - .where(User.is_active.is_(True), User.is_verified.is_(False)) + async def delete_user(self, user_id: uuid.UUID) -> None: + """Hard-delete a user and all of its dependent rows. + + Every child table is cleared with an explicit bulk ``DELETE`` so the + result is deterministic and identical on Postgres and SQLite (SQLite + only honours FK cascade with a per-connection pragma we don't set, and + ``RefreshToken`` has no DB cascade at all). + + The user is loaded with ``roles`` and ``oauth_accounts`` forced to + ``noload``. ``noload`` returns an empty collection *without* emitting a + query, which matters twice: (1) the unit of work isn't tracking the + ``users_user_role`` / ``oauth_account`` rows, so our bulk deletes don't + race the ORM and trigger ``StaleDataError`` on flush (the original bug — + it only bit a real request session deleting a user that actually had a + role); (2) ``session.delete(user)`` won't trigger an implicit async + lazy-load of the ``delete-orphan`` ``oauth_accounts`` collection + mid-flush. ``session.delete(user)`` is what flags the session as + written so ``get_db`` commits — a bulk ``DELETE`` of the user alone + would be rolled back as a read-only request.""" + result = await self._db.execute( + select(User) + .where(User.id == user_id) + .options(noload(User.roles), noload(User.oauth_accounts)) ) - active = (await self._db.execute(active_q)).scalar_one() - unverified = (await self._db.execute(unverified_q)).scalar_one() - return {"active": int(active), "unverified": int(unverified)} + user = result.scalar_one_or_none() + if user is None: + raise UserNotFoundError(user_id) + for model in (UserRole, UserAccessToken, OAuthAccount, RefreshToken): + await self._db.execute(delete(model).where(model.user_id == user_id)) + await self._db.delete(user) + await self._db.flush() async def invite( self, @@ -262,10 +210,3 @@ async def generate_reset_link(self, user_id: uuid.UUID, base_url: str) -> str: token = await self._manager.generate_reset_password_token(user) return f"{base_url.rstrip('/')}/users/reset-password?token={token}" - - async def get_with_roles(self, user_id: uuid.UUID) -> User | None: - return await self._get_user_with_roles(user_id) - - async def get_list_item(self, user_id: uuid.UUID) -> UserListItem: - user = await self._require_user(user_id) - return self.to_list_item(user) diff --git a/modules/users/users/admin/views.py b/modules/users/users/admin/views.py index af9b38a7..5b070c5d 100644 --- a/modules/users/users/admin/views.py +++ b/modules/users/users/admin/views.py @@ -19,6 +19,7 @@ _PAGE_ADMIN_INDEX = "Users/Users/Index" _PAGE_ADMIN_INVITE = "Users/Users/Invite" +_PAGE_ADMIN_CREATE = "Users/Users/Create" _PAGE_ADMIN_EDIT = "Users/Users/Edit" @@ -96,6 +97,23 @@ async def admin_invite_page( ) +@router.get( + "/admin/create", + response_model=None, + dependencies=[Depends(RequiresPermission(PERM_USERS_MANAGE))], +) +async def admin_create_page( + request: Request, + inertia: InertiaDep, +) -> InertiaResponse: + return await inertia.render( + _PAGE_ADMIN_CREATE, + { + "roles": await _roles_payload(request.app), + }, + ) + + @router.get( "/admin/{user_id}", response_model=None, diff --git a/modules/users/users/contracts/events.py b/modules/users/users/contracts/events.py index f683e982..4dc4d379 100644 --- a/modules/users/users/contracts/events.py +++ b/modules/users/users/contracts/events.py @@ -21,6 +21,18 @@ class UserInvited(Event): invited_by: str | None +@dataclass +class UserCreated(Event): + user_id: uuid.UUID + email: str + created_by: str | None + + +@dataclass +class UserDeleted(Event): + user_id: uuid.UUID + + @dataclass class UserDisabled(Event): user_id: uuid.UUID diff --git a/modules/users/users/contracts/schemas.py b/modules/users/users/contracts/schemas.py index 90b8affb..732145a2 100644 --- a/modules/users/users/contracts/schemas.py +++ b/modules/users/users/contracts/schemas.py @@ -49,6 +49,18 @@ class UserInvite(SQLModel): role_names: list[str] = [] +class UserAdminCreate(SQLModel): + email: EmailStr + password: str + full_name: str | None = None + role_names: list[str] = [] + + +class UserDetailsUpdate(SQLModel): + email: EmailStr + full_name: str | None = None + + class UserListItem(SQLModel): id: uuid.UUID email: EmailStr diff --git a/modules/users/users/exceptions.py b/modules/users/users/exceptions.py index 653452ea..e45e94ee 100644 --- a/modules/users/users/exceptions.py +++ b/modules/users/users/exceptions.py @@ -16,3 +16,11 @@ class UserNotFoundError(Exception): def __init__(self, user_id: uuid.UUID) -> None: super().__init__(f"User {user_id} not found") self.user_id = user_id + + +class EmailAlreadyExistsError(Exception): + """Raised when updating a user to an email already owned by another user.""" + + def __init__(self, email: str) -> None: + super().__init__(f"Email {email} already in use") + self.email = email diff --git a/modules/users/users/pages/Users/Create.tsx b/modules/users/users/pages/Users/Create.tsx new file mode 100644 index 00000000..42e1b184 --- /dev/null +++ b/modules/users/users/pages/Users/Create.tsx @@ -0,0 +1,175 @@ +import { Link, router, usePage } from '@inertiajs/react'; +import { PageShell } from '@simple-module-py/ui/components/PageShell'; +import { Button } from '@simple-module-py/ui/components/ui/button'; +import { Card, CardContent } from '@simple-module-py/ui/components/ui/card'; +import { Input } from '@simple-module-py/ui/components/ui/input'; +import { Label } from '@simple-module-py/ui/components/ui/label'; +import { AuthenticatedLayout } from '@simple-module-py/ui/layouts/AuthenticatedLayout'; +import { Lock, Mail, UserPlus } from 'lucide-react'; +import { useState } from 'react'; +import { toast } from 'sonner'; + +interface Role { + id: string; + name: string; +} + +interface Props { + roles: Role[]; +} + +function Create() { + const { roles } = usePage<{ props: Props }>().props as unknown as Props; + + const [email, setEmail] = useState(''); + const [fullName, setFullName] = useState(''); + const [password, setPassword] = useState(''); + const [selectedRoles, setSelectedRoles] = useState([]); + const [error, setError] = useState(null); + const [loading, setLoading] = useState(false); + + const toggleRole = (roleName: string) => { + setSelectedRoles((prev) => + prev.includes(roleName) ? prev.filter((r) => r !== roleName) : [...prev, roleName], + ); + }; + + const handleSubmit = (e: React.FormEvent) => { + e.preventDefault(); + setError(null); + setLoading(true); + fetch('/api/users/admin', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + email, + password, + full_name: fullName || null, + role_names: selectedRoles, + }), + }) + .then(async (res) => { + if (res.ok) { + toast.success('User created'); + router.visit('/users/admin'); + } else { + const data = await res.json().catch(() => ({})); + setError(typeof data?.detail === 'string' ? data.detail : 'Failed to create user'); + } + }) + .catch(() => setError('An error occurred. Please try again.')) + .finally(() => setLoading(false)); + }; + + return ( + + Back to Users + + } + > + + +
+
+ +
+ + setEmail(e.target.value)} + placeholder="teammate@example.com" + required + autoComplete="off" + className="pl-9" + /> +
+
+ +
+ + setFullName(e.target.value)} + placeholder="Jane Doe" + /> +
+ +
+ +
+ + setPassword(e.target.value)} + placeholder="At least 8 characters" + required + autoComplete="new-password" + className="pl-9" + /> +
+

+ Must be at least 8 characters and not all numbers. +

+
+ + {roles.length > 0 && ( +
+ +
+ {roles.map((role) => { + const active = selectedRoles.includes(role.name); + return ( + + ); + })} +
+
+ )} + + {error &&

{error}

} + +
+ + +
+
+
+
+
+ ); +} + +Create.layout = (page: React.ReactNode) => {page}; +export default Create; diff --git a/modules/users/users/pages/Users/Edit.tsx b/modules/users/users/pages/Users/Edit.tsx index 38ff4a8e..01df14cd 100644 --- a/modules/users/users/pages/Users/Edit.tsx +++ b/modules/users/users/pages/Users/Edit.tsx @@ -9,6 +9,8 @@ import { ShieldCheck } from 'lucide-react'; import { useState } from 'react'; import { toast } from 'sonner'; import { AccountStatusCard } from './components/AccountStatusCard'; +import { DangerZone } from './components/DangerZone'; +import { DetailsCard } from './components/DetailsCard'; interface UserListItem { id: string; @@ -31,6 +33,7 @@ interface Props { user: UserListItem; roles: Role[]; has_permissions_module: boolean; + auth?: { user?: { id?: string } }; } function fmt(dt: string | null): string { @@ -39,8 +42,9 @@ function fmt(dt: string | null): string { } function Edit() { - const { user, roles, has_permissions_module } = usePage<{ props: Props }>() + const { user, roles, has_permissions_module, auth } = usePage<{ props: Props }>() .props as unknown as Props; + const isSelf = auth?.user?.id === user.id; const [isActive, setIsActive] = useState(user.is_active); const [isVerified, setIsVerified] = useState(user.is_verified); @@ -129,6 +133,11 @@ function Edit() { } >
+ + Metadata @@ -226,6 +235,8 @@ function Edit() { )} + +
); diff --git a/modules/users/users/pages/Users/Index.tsx b/modules/users/users/pages/Users/Index.tsx index be07709c..dc30b4c8 100644 --- a/modules/users/users/pages/Users/Index.tsx +++ b/modules/users/users/pages/Users/Index.tsx @@ -128,12 +128,20 @@ function Index() { title="Users" description="People with access to this workspace. Invites use the configured mailer." actions={ - +
+ + +
} >
diff --git a/modules/users/users/pages/Users/components/DangerZone.tsx b/modules/users/users/pages/Users/components/DangerZone.tsx new file mode 100644 index 00000000..1d0940eb --- /dev/null +++ b/modules/users/users/pages/Users/components/DangerZone.tsx @@ -0,0 +1,85 @@ +import { router } from '@inertiajs/react'; +import { SectionTitle } from '@simple-module-py/ui/components/SectionTitle'; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, + AlertDialogTrigger, +} from '@simple-module-py/ui/components/ui/alert-dialog'; +import { Button } from '@simple-module-py/ui/components/ui/button'; +import { Card, CardContent } from '@simple-module-py/ui/components/ui/card'; +import { Trash2 } from 'lucide-react'; +import { useState } from 'react'; +import { toast } from 'sonner'; + +interface Props { + userId: string; + email: string; + isSelf: boolean; +} + +export function DangerZone({ userId, email, isSelf }: Props) { + const [deleting, setDeleting] = useState(false); + + const handleDelete = () => { + setDeleting(true); + fetch(`/api/users/admin/${userId}`, { method: 'DELETE' }) + .then(async (res) => { + if (res.ok) { + toast.success('User deleted'); + router.visit('/users/admin'); + return; // navigating away — leave `deleting` set + } + const data = await res.json().catch(() => ({})); + toast.error(typeof data?.detail === 'string' ? data.detail : 'Failed to delete user'); + setDeleting(false); + }) + .catch(() => { + toast.error('An error occurred'); + setDeleting(false); + }); + }; + + return ( + + + Danger zone + {isSelf ? ( +

You cannot delete your own account.

+ ) : ( +
+

+ Permanently delete this user. This cannot be undone. +

+ + + + + + + Delete {email}? + + This permanently removes the account and all of its access. This action cannot + be undone. + + + + Cancel + Delete + + + +
+ )} +
+
+ ); +} diff --git a/modules/users/users/pages/Users/components/DetailsCard.tsx b/modules/users/users/pages/Users/components/DetailsCard.tsx new file mode 100644 index 00000000..bd1de671 --- /dev/null +++ b/modules/users/users/pages/Users/components/DetailsCard.tsx @@ -0,0 +1,81 @@ +import { SectionTitle } from '@simple-module-py/ui/components/SectionTitle'; +import { Button } from '@simple-module-py/ui/components/ui/button'; +import { Card, CardContent } from '@simple-module-py/ui/components/ui/card'; +import { Input } from '@simple-module-py/ui/components/ui/input'; +import { Label } from '@simple-module-py/ui/components/ui/label'; +import { useState } from 'react'; +import { toast } from 'sonner'; + +interface Props { + user: { id: string; email: string; full_name: string | null }; +} + +export function DetailsCard({ user }: Props) { + const [email, setEmail] = useState(user.email); + const [fullName, setFullName] = useState(user.full_name ?? ''); + const [saving, setSaving] = useState(false); + const [error, setError] = useState(null); + + const handleSave = () => { + setSaving(true); + setError(null); + fetch(`/api/users/admin/${user.id}`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email, full_name: fullName || null }), + }) + .then(async (res) => { + if (res.ok) { + toast.success('Details updated'); + } else { + const data = await res.json().catch(() => ({})); + setError(typeof data?.detail === 'string' ? data.detail : 'Failed to update details'); + } + }) + .catch(() => setError('An error occurred')) + .finally(() => setSaving(false)); + }; + + return ( + + + + {saving ? 'Saving…' : 'Save details'} + + } + > + Details + +
+
+ + setEmail(e.target.value)} + autoComplete="off" + /> +
+
+ + setFullName(e.target.value)} + placeholder="Jane Doe" + /> +
+
+ {error &&

{error}

} +
+
+ ); +}