diff --git a/backend/app/api/docs/organization/delete.md b/backend/app/api/docs/organization/delete.md index e0841c04a..bcaf2f66f 100644 --- a/backend/app/api/docs/organization/delete.md +++ b/backend/app/api/docs/organization/delete.md @@ -1,3 +1,12 @@ -Delete an organization. +Delete an organization. **Requires superuser access.** -Permanently deletes an organization and all associated data. +Supports two delete modes, selected by an optional request body: + +```json +{ "hard_delete": false } +``` + +- **`hard_delete: false` (default)** — *Soft delete.* The organization is marked **inactive** and all of its projects are deactivated as well. No data is removed, so the organization can be reactivated later. It simply stops appearing in listings and can no longer be used. Omitting the body entirely also performs a soft delete. +- **`hard_delete: true`** — *Permanent delete.* The organization and everything owned by it — projects, collections, documents, credentials, assistants, fine-tunings, conversations, and user-project mappings — are permanently removed. **This cannot be undone.** + +In both modes, user **accounts are never deleted** (a user may belong to other organizations). Any user left without an active project afterwards is marked **inactive** and can no longer log in until they are added to an active project again. diff --git a/backend/app/api/docs/organization/list.md b/backend/app/api/docs/organization/list.md index 0d128ac8e..44fbbda4a 100644 --- a/backend/app/api/docs/organization/list.md +++ b/backend/app/api/docs/organization/list.md @@ -1,3 +1,9 @@ -List all organizations. +List organizations. **Requires superuser access.** -Returns paginated list of all organizations in the system. The response includes a `has_more` field in `metadata` indicating whether additional pages are available. +Returns a paginated list of organizations. The response includes a `has_more` field in `metadata` indicating whether additional pages are available. + +**Query Parameters:** +- `search` (optional): Case-insensitive substring match on the organization **name**. For example, `?search=acme` returns every organization whose name contains "acme". +- `is_active` (optional, default `true`): Filter by active status. Pass `false` to list soft-deleted organizations. +- `skip` (optional, default `0`): Number of records to skip for pagination. +- `limit` (optional, default `100`, max `100`): Maximum number of records to return. diff --git a/backend/app/api/docs/projects/delete.md b/backend/app/api/docs/projects/delete.md index 8afee4da9..f107c8227 100644 --- a/backend/app/api/docs/projects/delete.md +++ b/backend/app/api/docs/projects/delete.md @@ -1,3 +1,12 @@ -Delete a project. +Delete a project. **Requires superuser access.** -Permanently deletes a project and all associated data including documents, collections, and configurations. +Supports two delete modes, selected by an optional request body: + +```json +{ "hard_delete": false } +``` + +- **`hard_delete: false` (default)** — *Soft delete.* The project is marked **inactive**. No data is removed, so the project can be reactivated later. It simply stops appearing in listings and can no longer be used. Omitting the body entirely also performs a soft delete. +- **`hard_delete: true`** — *Permanent delete.* The project and everything owned by it — collections, documents, credentials, assistants, fine-tunings, conversations, and user-project mappings — are permanently removed. **This cannot be undone.** + +In both modes, user **accounts are never deleted** (a user may belong to other projects). Any user left without an active project afterwards is marked **inactive** and can no longer log in until they are added to an active project again. diff --git a/backend/app/api/docs/projects/list.md b/backend/app/api/docs/projects/list.md index 911f7d1dc..c9b18af6b 100644 --- a/backend/app/api/docs/projects/list.md +++ b/backend/app/api/docs/projects/list.md @@ -1,3 +1,9 @@ -List all projects. +List projects across all organizations. **Requires superuser access.** -Returns paginated list of all projects across all organizations. +Returns a paginated list of projects. The response includes a `has_more` field in `metadata` indicating whether additional pages are available. + +**Query Parameters:** +- `search` (optional): Case-insensitive substring match on the project **name**. For example, `?search=onboarding` returns every project whose name contains "onboarding". +- `is_active` (optional, default `true`): Filter by active status. Pass `false` to list soft-deleted projects. +- `skip` (optional, default `0`): Number of records to skip for pagination. +- `limit` (optional, default `100`, max `100`): Maximum number of records to return. diff --git a/backend/app/api/docs/projects/list_by_org.md b/backend/app/api/docs/projects/list_by_org.md index b227312d0..c60d5bf59 100644 --- a/backend/app/api/docs/projects/list_by_org.md +++ b/backend/app/api/docs/projects/list_by_org.md @@ -1,3 +1,7 @@ -List all projects for a given organization. +List projects for a given organization. **Requires superuser access.** -Returns all projects belonging to the specified organization ID. The organization must exist and be active. +Returns the projects belonging to the specified organization ID. The organization must exist and be active. + +**Query Parameters:** +- `search` (optional): Case-insensitive substring match on the project **name**. For example, `?search=onboarding` returns only projects whose name contains "onboarding". +- `is_active` (optional, default `true`): Filter by active status. Pass `false` to list soft-deleted projects (e.g. to selectively reactivate them). diff --git a/backend/app/api/docs/user_project/delete.md b/backend/app/api/docs/user_project/delete.md index 4b765c47f..74a1c4372 100644 --- a/backend/app/api/docs/user_project/delete.md +++ b/backend/app/api/docs/user_project/delete.md @@ -6,4 +6,4 @@ Remove a user from a project. **Requires superuser access.** **Query Parameters:** - `project_id` (required): The ID of the project to remove the user from. -This only removes the user-project mapping — the user account itself is not deleted. You cannot remove yourself from a project. +This only removes the user-project mapping — the user account itself is never deleted. If this was the user's last project, the account is marked **inactive** and can no longer log in until they are added to a project again. You cannot remove yourself from a project. diff --git a/backend/app/api/routes/auth.py b/backend/app/api/routes/auth.py index cc557bfa8..f311c25da 100644 --- a/backend/app/api/routes/auth.py +++ b/backend/app/api/routes/auth.py @@ -114,6 +114,15 @@ def google_auth(session: SessionDep, body: GoogleAuthRequest) -> JSONResponse: available_projects = get_user_accessible_projects(session=session, user_id=user.id) + if not user.is_superuser and not available_projects: + logger.info( + f"[google_auth] User has no accessible projects | user_id: {user.id}" + ) + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="You are not assigned to any active project. Please contact your administrator.", + ) + if len(available_projects) == 1: proj = available_projects[0] logger.info( @@ -389,6 +398,15 @@ def verify_magic_link(session: SessionDep, token: str) -> JSONResponse: # Get user's projects to embed in token available_projects = get_user_accessible_projects(session=session, user_id=user.id) + if not user.is_superuser and not available_projects: + logger.info( + f"[verify_magic_link] User has no accessible projects | user_id: {user.id}" + ) + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="You are not assigned to any active project. Please contact your administrator.", + ) + organization_id = None project_id = None if len(available_projects) == 1: diff --git a/backend/app/api/routes/login.py b/backend/app/api/routes/login.py index 54c1dbeb1..665300ec3 100644 --- a/backend/app/api/routes/login.py +++ b/backend/app/api/routes/login.py @@ -1,7 +1,7 @@ from datetime import timedelta -from typing import Annotated, Any, Optional +from typing import Annotated, Any -from fastapi import APIRouter, Depends, HTTPException, Form +from fastapi import APIRouter, Depends, Form, HTTPException from fastapi.responses import HTMLResponse from fastapi.security import OAuth2PasswordRequestForm @@ -11,6 +11,7 @@ from app.core.config import settings from app.core.security import get_password_hash from app.crud import authenticate, get_user_by_email +from app.crud.auth import get_user_accessible_projects from app.models import Message, NewPassword, Token, UserPublic from app.utils import ( generate_password_reset_token, @@ -26,9 +27,8 @@ def login_access_token( session: SessionDep, form_data: Annotated[OAuth2PasswordRequestForm, Depends()], - token_expiry_minutes: Optional[int] = Form( - default=settings.ACCESS_TOKEN_EXPIRE_MINUTES, ge=1, le=60 * 24 * 360 - ), + token_expiry_minutes: int + | None = Form(default=settings.ACCESS_TOKEN_EXPIRE_MINUTES, ge=1, le=60 * 24 * 360), ) -> Token: """ OAuth2 compatible token login with customizable expiration time. @@ -42,6 +42,14 @@ def login_access_token( elif not user.is_active: raise HTTPException(status_code=400, detail="Inactive user") + if not user.is_superuser and not get_user_accessible_projects( + session=session, user_id=user.id + ): + raise HTTPException( + status_code=403, + detail="You are not assigned to any active project. Please contact your administrator.", + ) + access_token_expires = timedelta(minutes=token_expiry_minutes) return Token( diff --git a/backend/app/api/routes/organization.py b/backend/app/api/routes/organization.py index 40b2941ee..3bb3a74d5 100644 --- a/backend/app/api/routes/organization.py +++ b/backend/app/api/routes/organization.py @@ -1,19 +1,26 @@ import logging -from typing import List from fastapi import APIRouter, Depends, HTTPException, Query from sqlalchemy import func from sqlmodel import select +from app.api.deps import SessionDep +from app.api.permissions import Permission, require_permission +from app.crud.organization import ( + cascade_deactivate_organization, + create_organization, + get_organization_by_id, + get_organization_by_name, + hard_delete_organization, + soft_delete_organization, +) from app.models import ( + DeleteRequest, Organization, OrganizationCreate, - OrganizationUpdate, OrganizationPublic, + OrganizationUpdate, ) -from app.api.deps import SessionDep -from app.api.permissions import Permission, require_permission -from app.crud.organization import create_organization, get_organization_by_id from app.utils import APIResponse, load_description logger = logging.getLogger(__name__) @@ -24,18 +31,30 @@ @router.get( "", dependencies=[Depends(require_permission(Permission.SUPERUSER))], - response_model=APIResponse[List[OrganizationPublic]], + response_model=APIResponse[list[OrganizationPublic]], description=load_description("organization/list.md"), ) def read_organizations( session: SessionDep, skip: int = Query(0, ge=0), limit: int = Query(100, ge=1, le=100), -) -> APIResponse[List[OrganizationPublic]]: - count_statement = select(func.count()).select_from(Organization) + search: str + | None = Query( + None, description="Case-insensitive substring match on the organization name" + ), + is_active: bool = Query( + True, + description="Filter by active status. Pass false to list soft-deleted organizations.", + ), +) -> APIResponse[list[OrganizationPublic]]: + filters = [Organization.is_active.is_(is_active)] + if search and search.strip(): + filters.append(Organization.name.ilike(f"%{search.strip()}%")) + + count_statement = select(func.count()).select_from(Organization).where(*filters) count = session.exec(count_statement).one() - statement = select(Organization).offset(skip).limit(limit) + statement = select(Organization).where(*filters).offset(skip).limit(limit) organizations = session.exec(statement).all() has_more = (skip + limit) < count @@ -69,7 +88,7 @@ def read_organization( Retrieve an organization by ID. """ org = get_organization_by_id(session=session, org_id=org_id) - if org is None: + if org is None or not org.is_active: raise HTTPException(status_code=404, detail="Organization not found") return APIResponse.success_response(org) @@ -89,11 +108,28 @@ def update_organization( raise HTTPException(status_code=404, detail="Organization not found") org_data = org_in.model_dump(exclude_unset=True) - org = org.model_copy(update=org_data) + new_name = org_data.get("name") + if new_name and new_name != org.name: + existing = get_organization_by_name(session=session, name=new_name) + if existing and existing.id != org.id: + raise HTTPException( + status_code=409, + detail="An organization with this name already exists", + ) + + target_active = org_data.get("is_active") + deactivating = target_active is False and org.is_active + + org.sqlmodel_update(org_data) session.add(org) - session.commit() session.flush() + + if deactivating: + cascade_deactivate_organization(session=session, organization=org) + + session.commit() + session.refresh(org) logger.info( f"[update_organization] Organization Updated Successfully | 'org_id': {org.id}" ) @@ -105,17 +141,25 @@ def update_organization( "/{org_id}", dependencies=[Depends(require_permission(Permission.SUPERUSER))], response_model=APIResponse[None], - include_in_schema=False, description=load_description("organization/delete.md"), ) -def delete_organization(session: SessionDep, org_id: int) -> APIResponse[None]: +def delete_organization_endpoint( + session: SessionDep, + org_id: int, + body: DeleteRequest | None = None, +) -> APIResponse[None]: org = get_organization_by_id(session=session, org_id=org_id) if org is None: raise HTTPException(status_code=404, detail="Organization not found") - session.delete(org) - session.commit() + hard_delete = body.hard_delete if body else False + if hard_delete: + hard_delete_organization(session=session, organization=org) + else: + soft_delete_organization(session=session, organization=org) + logger.info( - f"[delete_organization] Organization Deleted Successfully | 'org_id': {org_id}" + f"[delete_organization_endpoint] Organization deleted | 'org_id': {org_id}, " + f"hard_delete: {hard_delete}" ) return APIResponse.success_response(None) diff --git a/backend/app/api/routes/project.py b/backend/app/api/routes/project.py index 62632fba2..9f01e9c3f 100644 --- a/backend/app/api/routes/project.py +++ b/backend/app/api/routes/project.py @@ -1,19 +1,32 @@ import logging -from typing import List from fastapi import APIRouter, Depends, HTTPException, Query from sqlalchemy import func from sqlmodel import select -from app.models import Project, ProjectCreate, ProjectUpdate, ProjectPublic from app.api.deps import SessionDep from app.api.permissions import Permission, require_permission +from app.crud.organization import get_organization_by_id, validate_organization from app.crud.project import ( create_project, get_project_by_id, + get_project_by_name, get_projects_by_organization, + hard_delete_project, + soft_delete_project, +) +from app.crud.user_project import ( + deactivate_users_without_projects, + get_user_ids_for_project, + reactivate_users_with_access, +) +from app.models import ( + DeleteRequest, + Project, + ProjectCreate, + ProjectPublic, + ProjectUpdate, ) -from app.crud.organization import validate_organization from app.utils import APIResponse, load_description logger = logging.getLogger(__name__) @@ -24,18 +37,30 @@ @router.get( "", dependencies=[Depends(require_permission(Permission.SUPERUSER))], - response_model=APIResponse[List[ProjectPublic]], + response_model=APIResponse[list[ProjectPublic]], description=load_description("projects/list.md"), ) def read_projects( session: SessionDep, skip: int = Query(0, ge=0), limit: int = Query(100, ge=1, le=100), + search: str + | None = Query( + None, description="Case-insensitive substring match on the project name" + ), + is_active: bool = Query( + True, + description="Filter by active status. Pass false to list soft-deleted projects.", + ), ): - count_statement = select(func.count()).select_from(Project) + filters = [Project.is_active.is_(is_active)] + if search and search.strip(): + filters.append(Project.name.ilike(f"%{search.strip()}%")) + + count_statement = select(func.count()).select_from(Project).where(*filters) count = session.exec(count_statement).one() - statement = select(Project).offset(skip).limit(limit) + statement = select(Project).where(*filters).offset(skip).limit(limit) projects = session.exec(statement).all() has_more = (skip + limit) < count @@ -65,7 +90,7 @@ def read_project(*, session: SessionDep, project_id: int): Retrieve a project by ID. """ project = get_project_by_id(session=session, project_id=project_id) - if project is None: + if project is None or not project.is_active: raise HTTPException(status_code=404, detail="Project not found") return APIResponse.success_response(project) @@ -83,9 +108,47 @@ def update_project(*, session: SessionDep, project_id: int, project_in: ProjectU raise HTTPException(status_code=404, detail="Project not found") project_data = project_in.model_dump(exclude_unset=True) - project.sqlmodel_update(project_data) + new_name = project_data.get("name") + if new_name and new_name != project.name: + existing = get_project_by_name( + session=session, + project_name=new_name, + organization_id=project.organization_id, + ) + if existing and existing.id != project.id: + raise HTTPException( + status_code=409, + detail="A project with this name already exists in this organization", + ) + + target_active = project_data.get("is_active") + activating = target_active is True and not project.is_active + deactivating = target_active is False and project.is_active + + if activating: + org = get_organization_by_id(session=session, org_id=project.organization_id) + if org is None or not org.is_active: + raise HTTPException( + status_code=400, + detail="Cannot activate a project whose organization is inactive. Reactivate the organization first.", + ) + + project.sqlmodel_update(project_data) session.add(project) + session.flush() + + if activating: + reactivate_users_with_access( + session=session, + user_ids=get_user_ids_for_project(session=session, project_id=project.id), + ) + elif deactivating: + deactivate_users_without_projects( + session=session, + user_ids=get_user_ids_for_project(session=session, project_id=project.id), + ) + session.commit() session.refresh(project) logger.info( @@ -94,36 +157,54 @@ def update_project(*, session: SessionDep, project_id: int, project_in: ProjectU return APIResponse.success_response(project) -# Delete a project @router.delete( "/{project_id}", dependencies=[Depends(require_permission(Permission.SUPERUSER))], - include_in_schema=False, + response_model=APIResponse[None], description=load_description("projects/delete.md"), ) -def delete_project(session: SessionDep, project_id: int): +def delete_project_endpoint( + session: SessionDep, + project_id: int, + body: DeleteRequest | None = None, +): project = get_project_by_id(session=session, project_id=project_id) if project is None: raise HTTPException(status_code=404, detail="Project not found") - session.delete(project) - session.commit() + hard_delete = body.hard_delete if body else False + if hard_delete: + hard_delete_project(session=session, project=project) + else: + soft_delete_project(session=session, project=project) + logger.info( - f"[delete_project] Project deleted successfully | project_id={project_id}" + f"[delete_project_endpoint] Project deleted | project_id={project_id}, " + f"hard_delete: {hard_delete}" ) return APIResponse.success_response(None) -# Get projects by organization @router.get( "/organization/{org_id}", dependencies=[Depends(require_permission(Permission.SUPERUSER))], - response_model=APIResponse[List[ProjectPublic]], + response_model=APIResponse[list[ProjectPublic]], description=load_description("projects/list_by_org.md"), ) def read_projects_by_organization( - session: SessionDep, org_id: int -) -> APIResponse[List[ProjectPublic]]: + session: SessionDep, + org_id: int, + is_active: bool = Query( + True, + description="Filter by active status. Pass false to list soft-deleted projects to reactivate.", + ), + search: str + | None = Query( + None, description="Case-insensitive substring match on the project name" + ), +) -> APIResponse[list[ProjectPublic]]: validate_organization(session=session, org_id=org_id) - projects = get_projects_by_organization(session=session, org_id=org_id) + projects = get_projects_by_organization( + session=session, org_id=org_id, is_active=is_active, search=search + ) return APIResponse.success_response(projects) diff --git a/backend/app/crud/organization.py b/backend/app/crud/organization.py index be7970c5a..60ef7691b 100644 --- a/backend/app/crud/organization.py +++ b/backend/app/crud/organization.py @@ -1,11 +1,11 @@ import logging -from typing import Any, Optional -from datetime import datetime, timezone -from sqlmodel import Session, select + from fastapi import HTTPException +from sqlmodel import Session, select -from app.models import Organization, OrganizationCreate from app.core.util import now +from app.crud.user_project import deactivate_users_without_projects +from app.models import Organization, OrganizationCreate, Project, UserProject logger = logging.getLogger(__name__) @@ -13,6 +13,22 @@ def create_organization( *, session: Session, org_create: OrganizationCreate ) -> Organization: + existing = get_organization_by_name(session=session, name=org_create.name) + if existing: + logger.warning( + f"[create_organization] Organization name already exists | " + f"'org_id': {existing.id}, 'name': {existing.name}, 'is_active': {existing.is_active}" + ) + if existing.is_active: + raise HTTPException( + status_code=409, + detail="An organization with this name already exists", + ) + raise HTTPException( + status_code=409, + detail="An organization with this name already exists but is inactive. Reactivate it instead of creating a new one.", + ) + db_org = Organization.model_validate(org_create) db_org.inserted_at = now() db_org.updated_at = now() @@ -26,16 +42,110 @@ def create_organization( # Get organization by ID -def get_organization_by_id(session: Session, org_id: int) -> Optional[Organization]: +def get_organization_by_id(session: Session, org_id: int) -> Organization | None: statement = select(Organization).where(Organization.id == org_id) return session.exec(statement).first() -def get_organization_by_name(*, session: Session, name: str) -> Optional[Organization]: +def get_organization_by_name(*, session: Session, name: str) -> Organization | None: statement = select(Organization).where(Organization.name == name) return session.exec(statement).first() +def cascade_deactivate_organization( + *, session: Session, organization: Organization +) -> tuple[int, list[int]]: + """ + Mark an organization inactive and cascade the deactivation to all of its + projects, then deactivate any user left without an accessible project. + + Does not commit — the caller is responsible for committing. Returns a tuple + of (number of projects deactivated, list of deactivated user IDs). + """ + org_id = organization.id + + affected_user_ids = session.exec( + select(UserProject.user_id).where(UserProject.organization_id == org_id) + ).all() + + organization.is_active = False + session.add(organization) + + projects = session.exec( + select(Project).where( + Project.organization_id == org_id, + Project.is_active.is_(True), + ) + ).all() + for project in projects: + project.is_active = False + session.add(project) + + session.flush() + + deactivated = deactivate_users_without_projects( + session=session, user_ids=affected_user_ids + ) + return len(projects), deactivated + + +def soft_delete_organization( + *, session: Session, organization: Organization +) -> list[int]: + """ + Soft-delete an organization by marking it inactive. All of its projects are + deactivated as well, so the org and everything under it stop appearing in + listings and can no longer be used. No rows are removed. + + User accounts are never deleted. Any user left without an accessible project + afterwards is marked inactive so they can no longer log in. Returns the list + of deactivated user IDs. + """ + org_id = organization.id + project_count, deactivated = cascade_deactivate_organization( + session=session, organization=organization + ) + session.commit() + logger.info( + f"[soft_delete_organization] Organization soft-deleted | 'org_id': {org_id}, " + f"deactivated_projects: {project_count}, deactivated_users: {len(deactivated)}" + ) + return deactivated + + +def hard_delete_organization( + *, session: Session, organization: Organization +) -> list[int]: + """ + Permanently delete an organization and everything owned by it — projects, + collections, documents, credentials, assistants, fine-tunings, conversations, + and user-project mappings all cascade away. This cannot be undone. + + User *accounts* are still never deleted (a user may belong to other orgs): + any user left without an accessible project after the cascade is marked + inactive instead. Returns the list of deactivated user IDs. + """ + org_id = organization.id + + # Capture affected users before the cascade removes their mappings. + affected_user_ids = session.exec( + select(UserProject.user_id).where(UserProject.organization_id == org_id) + ).all() + + session.delete(organization) + session.flush() + + deactivated = deactivate_users_without_projects( + session=session, user_ids=affected_user_ids + ) + session.commit() + logger.info( + f"[hard_delete_organization] Organization permanently deleted | " + f"'org_id': {org_id}, deactivated_users: {len(deactivated)}" + ) + return deactivated + + # Validate if organization exists and is active def validate_organization(session: Session, org_id: int) -> Organization: """ diff --git a/backend/app/crud/project.py b/backend/app/crud/project.py index 2f73982f7..be47e24cf 100644 --- a/backend/app/crud/project.py +++ b/backend/app/crud/project.py @@ -1,10 +1,11 @@ import logging -from typing import List, Optional -from sqlmodel import Session, select + from fastapi import HTTPException +from sqlmodel import Session, select from app.core.util import now -from app.models import Project, ProjectCreate, Organization +from app.crud.user_project import deactivate_users_without_projects +from app.models import Project, ProjectCreate, UserProject logger = logging.getLogger(__name__) @@ -17,9 +18,17 @@ def create_project(*, session: Session, project_create: ProjectCreate) -> Projec ) if project: logger.warning( - f"[create_project] Project already exists | 'project_id': {project.id}, 'name': {project.name}" + f"[create_project] Project name already exists | 'project_id': {project.id}, " + f"'name': {project.name}, 'is_active': {project.is_active}" + ) + if project.is_active: + raise HTTPException( + 409, "A project with this name already exists in this organization" + ) + raise HTTPException( + 409, + "A project with this name already exists in this organization but is inactive. Reactivate it instead of creating a new one.", ) - raise HTTPException(409, "Project already exists") db_project = Project.model_validate(project_create) db_project.inserted_at = now() @@ -33,24 +42,104 @@ def create_project(*, session: Session, project_create: ProjectCreate) -> Projec return db_project -def get_project_by_id(*, session: Session, project_id: int) -> Optional[Project]: +def get_project_by_id(*, session: Session, project_id: int) -> Project | None: return session.get(Project, project_id) def get_project_by_name( *, session: Session, project_name: str, organization_id: int -) -> Optional[Project]: +) -> Project | None: statement = select(Project).where( Project.name == project_name, Project.organization_id == organization_id ) return session.exec(statement).first() -def get_projects_by_organization(*, session: Session, org_id: int) -> List[Project]: +def get_projects_by_organization( + *, + session: Session, + org_id: int, + is_active: bool | None = True, + search: str | None = None, +) -> list[Project]: + """ + Return projects for an organization. + + By default only active projects are returned. Pass ``is_active=False`` to + list soft-deleted projects (e.g. to selectively reactivate them), or + ``is_active=None`` to return every project regardless of status. Pass + ``search`` for a case-insensitive substring match on the project name. + """ statement = select(Project).where(Project.organization_id == org_id) + if is_active is not None: + statement = statement.where(Project.is_active.is_(is_active)) + if search and search.strip(): + statement = statement.where(Project.name.ilike(f"%{search.strip()}%")) return session.exec(statement).all() +def soft_delete_project(*, session: Session, project: Project) -> list[int]: + """ + Soft-delete a project by marking it inactive, so it stops appearing in + listings and can no longer be used. No rows are removed. + + User accounts are never deleted. Any user left without an accessible project + afterwards is marked inactive so they can no longer log in. Returns the list + of deactivated user IDs. + """ + project_id = project.id + + affected_user_ids = session.exec( + select(UserProject.user_id).where(UserProject.project_id == project_id) + ).all() + + # Soft-delete the project. + project.is_active = False + session.add(project) + session.flush() + + deactivated = deactivate_users_without_projects( + session=session, user_ids=affected_user_ids + ) + session.commit() + logger.info( + f"[soft_delete_project] Project soft-deleted | 'project_id': {project_id}, " + f"deactivated_users: {len(deactivated)}" + ) + return deactivated + + +def hard_delete_project(*, session: Session, project: Project) -> list[int]: + """ + Permanently delete a project and everything owned by it — collections, + documents, credentials, assistants, fine-tunings, conversations, and + user-project mappings all cascade away. This cannot be undone. + + User *accounts* are still never deleted (a user may belong to other + projects): any user left without an accessible project after the cascade is + marked inactive instead. Returns the list of deactivated user IDs. + """ + project_id = project.id + + # Capture affected users before the cascade removes their mappings. + affected_user_ids = session.exec( + select(UserProject.user_id).where(UserProject.project_id == project_id) + ).all() + + session.delete(project) + session.flush() + + deactivated = deactivate_users_without_projects( + session=session, user_ids=affected_user_ids + ) + session.commit() + logger.info( + f"[hard_delete_project] Project permanently deleted | " + f"'project_id': {project_id}, deactivated_users: {len(deactivated)}" + ) + return deactivated + + def validate_project(session: Session, project_id: int) -> Project: """ Ensures that an project exists and is active. diff --git a/backend/app/crud/user_project.py b/backend/app/crud/user_project.py index fec57c01d..3a69d3519 100644 --- a/backend/app/crud/user_project.py +++ b/backend/app/crud/user_project.py @@ -1,10 +1,11 @@ import logging import secrets -from typing import Sequence +from collections.abc import Iterable, Sequence from sqlmodel import Session, and_, select from app.core.security import get_password_hash +from app.crud.auth import get_user_accessible_projects from app.models import ( User, UserProject, @@ -94,12 +95,85 @@ def add_user_to_project( return user, "added" +def deactivate_users_without_projects( + *, session: Session, user_ids: Iterable[int] +) -> list[int]: + """ + Mark users inactive when they no longer belong to any *active* project. + + A user counts as having access only through a mapping to an active project + in an active organization (see ``get_user_accessible_projects``), so this + correctly handles soft-deleted orgs/projects whose mappings still exist. + + Users are never deleted: superusers are left untouched, and any user that + still has at least one accessible project keeps their active status. A user + left without any accessible project can no longer log in until they are + added to one again. Returns the list of user IDs that were deactivated. + """ + deactivated: list[int] = [] + for user_id in set(user_ids): + if get_user_accessible_projects(session=session, user_id=user_id): + continue + + user = session.get(User, user_id) + if user and not user.is_superuser and user.is_active: + user.is_active = False + session.add(user) + deactivated.append(user_id) + + if deactivated: + session.flush() + logger.info( + f"[deactivate_users_without_projects] Users deactivated | user_ids: {deactivated}" + ) + return deactivated + + +def reactivate_users_with_access( + *, session: Session, user_ids: Iterable[int] +) -> list[int]: + """ + Re-activate users who regained access to an active project. + + The inverse of ``deactivate_users_without_projects``: when an org or project + is reactivated, users that had been deactivated (because they had lost all + access) are made active again so they can log in. Their project mappings are + never removed by a soft delete, so access is restored automatically once the + org/project is active again. Superusers and already-active users are left + untouched. Returns the list of user IDs that were reactivated. + """ + reactivated: list[int] = [] + for user_id in set(user_ids): + user = session.get(User, user_id) + if user and not user.is_active and not user.is_superuser: + if get_user_accessible_projects(session=session, user_id=user_id): + user.is_active = True + session.add(user) + reactivated.append(user_id) + + if reactivated: + session.flush() + logger.info( + f"[reactivate_users_with_access] Users reactivated | user_ids: {reactivated}" + ) + return reactivated + + +def get_user_ids_for_project(*, session: Session, project_id: int) -> list[int]: + """Return the IDs of all users mapped to a project.""" + return list( + session.exec( + select(UserProject.user_id).where(UserProject.project_id == project_id) + ).all() + ) + + def remove_user_from_project( *, session: Session, user_id: int, project_id: int ) -> bool: """ Remove a user from a project. If this was their last project, - deactivate the user account. + deactivate the user account (the user is never deleted). Returns True if removed, False if not found. """ @@ -118,16 +192,7 @@ def remove_user_from_project( session.delete(user_project) session.flush() - # Check if user has any remaining projects - remaining = session.exec( - select(UserProject.id).where(UserProject.user_id == user_id).limit(1) - ).first() - - if not remaining: - user = session.get(User, user_id) - if user and not user.is_superuser: - session.delete(user) - session.flush() + deactivate_users_without_projects(session=session, user_ids=[user_id]) return True diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index 4f4bc5af0..f604d6175 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -136,7 +136,7 @@ LLMJobImmediatePublic, LLMJobPublic, ) -from .message import Message +from .message import DeleteRequest, Message from .model_config import ( ModelConfig, ModelConfigBase, diff --git a/backend/app/models/message.py b/backend/app/models/message.py index e0aadfeaa..1d7f4f5e1 100644 --- a/backend/app/models/message.py +++ b/backend/app/models/message.py @@ -1,6 +1,17 @@ -from sqlmodel import SQLModel +from sqlmodel import Field, SQLModel # Generic message class Message(SQLModel): message: str + + +class DeleteRequest(SQLModel): + hard_delete: bool = Field( + default=False, + description=( + "When true, permanently delete the record and all of its related " + "data. When false (the default), perform a soft delete by marking " + "the record inactive." + ), + ) diff --git a/backend/app/tests/api/routes/test_login.py b/backend/app/tests/api/routes/test_login.py index 67ce7feb1..95d5ca5e8 100644 --- a/backend/app/tests/api/routes/test_login.py +++ b/backend/app/tests/api/routes/test_login.py @@ -5,6 +5,7 @@ from app.core.security import verify_password from app.crud import create_user from app.models import UserCreate +from app.tests.utils.test_data import add_user_to_test_project from app.tests.utils.user import user_authentication_headers from app.tests.utils.utils import random_email, random_lower_string from app.utils import generate_password_reset_token @@ -67,6 +68,8 @@ def test_reset_password(client: TestClient, db: Session) -> None: is_superuser=False, ) user = create_user(session=db, user_create=user_create) + # Login requires an active project membership. + add_user_to_test_project(db, user) token = generate_password_reset_token(email=email) headers = user_authentication_headers(client=client, email=email, password=password) data = {"new_password": new_password, "token": token} diff --git a/backend/app/tests/api/routes/test_org.py b/backend/app/tests/api/routes/test_org.py index 7daae61be..1ae748b1d 100644 --- a/backend/app/tests/api/routes/test_org.py +++ b/backend/app/tests/api/routes/test_org.py @@ -3,9 +3,9 @@ from sqlmodel import Session from app.core.config import settings -from app.models import Organization -from app.main import app from app.crud.organization import get_organization_by_id +from app.main import app +from app.models import Organization from app.tests.utils.test_data import create_test_organization client = TestClient(app) @@ -74,23 +74,50 @@ def test_update_organization( assert updated_org["is_active"] == update_data["is_active"] -# Test deleting an organization +# Test soft deleting an organization (default) def test_delete_organization( db: Session, test_organization: Organization, superuser_token_headers: dict[str, str], ) -> None: + org_id = test_organization.id response = client.delete( - f"{settings.API_V1_STR}/organizations/{test_organization.id}", + f"{settings.API_V1_STR}/organizations/{org_id}", headers=superuser_token_headers, ) assert response.status_code == 200 + + # Soft-deleted orgs are hidden from the API... response = client.get( - f"{settings.API_V1_STR}/organizations/{test_organization.id}", + f"{settings.API_V1_STR}/organizations/{org_id}", headers=superuser_token_headers, ) assert response.status_code == 404 + db.expire_all() + org = get_organization_by_id(session=db, org_id=org_id) + assert org is not None + assert org.is_active is False + + +def test_hard_delete_organization( + db: Session, + test_organization: Organization, + superuser_token_headers: dict[str, str], +) -> None: + org_id = test_organization.id + response = client.request( + "DELETE", + f"{settings.API_V1_STR}/organizations/{org_id}", + json={"hard_delete": True}, + headers=superuser_token_headers, + ) + assert response.status_code == 200 + + # The row is permanently gone. + db.expire_all() + assert get_organization_by_id(session=db, org_id=org_id) is None + # Test pagination has_more metadata def test_read_organizations_has_more( diff --git a/backend/app/tests/api/routes/test_project.py b/backend/app/tests/api/routes/test_project.py index 487bec70f..4f0f9c8ad 100644 --- a/backend/app/tests/api/routes/test_project.py +++ b/backend/app/tests/api/routes/test_project.py @@ -2,11 +2,12 @@ from fastapi.testclient import TestClient from sqlmodel import Session -from app.main import app from app.core.config import settings -from app.models import Project, ProjectCreate +from app.crud.project import create_project, get_project_by_id +from app.main import app +from app.models import Organization, Project, ProjectCreate from app.tests.utils.test_data import create_test_organization, create_test_project - +from app.tests.utils.utils import random_lower_string client = TestClient(app) @@ -16,6 +17,21 @@ def test_project(db: Session) -> Project: return create_test_project(db) +def _make_project( + db: Session, org: Organization, name: str, is_active: bool +) -> Project: + """Create a project with a given name and active status under an org.""" + return create_project( + session=db, + project_create=ProjectCreate( + name=name, + description="Test project", + is_active=is_active, + organization_id=org.id, + ), + ) + + # Test creating a project def test_create_new_project( db: Session, superuser_token_headers: dict[str, str] @@ -86,6 +102,57 @@ def test_read_projects_has_more( assert response_data["metadata"]["has_more"] is False +# Test the search param filters projects by name (defaults to active only) +def test_read_projects_search( + db: Session, superuser_token_headers: dict[str, str] +) -> None: + org = create_test_organization(db) + prefix = random_lower_string() + _make_project(db, org, f"{prefix}-alpha", is_active=True) + _make_project(db, org, f"{prefix}-beta", is_active=True) + _make_project(db, org, "unrelated-name", is_active=True) + + response = client.get( + f"{settings.API_V1_STR}/projects?search={prefix}", + headers=superuser_token_headers, + ) + assert response.status_code == 200 + names = [p["name"] for p in response.json()["data"]] + assert f"{prefix}-alpha" in names + assert f"{prefix}-beta" in names + assert "unrelated-name" not in names + + +# Test the is_active param toggles active vs inactive projects, together with search +def test_read_projects_active_inactive_filter( + db: Session, superuser_token_headers: dict[str, str] +) -> None: + org = create_test_organization(db) + prefix = random_lower_string() + _make_project(db, org, f"{prefix}-active", is_active=True) + _make_project(db, org, f"{prefix}-inactive", is_active=False) + + # Default (is_active omitted) returns only active matches. + response = client.get( + f"{settings.API_V1_STR}/projects?search={prefix}", + headers=superuser_token_headers, + ) + assert response.status_code == 200 + names = [p["name"] for p in response.json()["data"]] + assert f"{prefix}-active" in names + assert f"{prefix}-inactive" not in names + + # is_active=false returns only inactive matches. + response = client.get( + f"{settings.API_V1_STR}/projects?search={prefix}&is_active=false", + headers=superuser_token_headers, + ) + assert response.status_code == 200 + names = [p["name"] for p in response.json()["data"]] + assert f"{prefix}-inactive" in names + assert f"{prefix}-active" not in names + + # Test updating a project def test_update_project( db: Session, test_project: Project, superuser_token_headers: dict[str, str] @@ -106,21 +173,45 @@ def test_update_project( assert updated_project["is_active"] == update_data["is_active"] -# Test deleting a project +# Test soft deleting a project (default) def test_delete_project( db: Session, test_project: Project, superuser_token_headers: dict[str, str] ) -> None: + project_id = test_project.id response = client.delete( - f"{settings.API_V1_STR}/projects/{test_project.id}", + f"{settings.API_V1_STR}/projects/{project_id}", headers=superuser_token_headers, ) assert response.status_code == 200 + response = client.get( - f"{settings.API_V1_STR}/projects/{test_project.id}", + f"{settings.API_V1_STR}/projects/{project_id}", headers=superuser_token_headers, ) assert response.status_code == 404 + db.expire_all() + project = get_project_by_id(session=db, project_id=project_id) + assert project is not None + assert project.is_active is False + + +def test_hard_delete_project( + db: Session, test_project: Project, superuser_token_headers: dict[str, str] +) -> None: + project_id = test_project.id + response = client.request( + "DELETE", + f"{settings.API_V1_STR}/projects/{project_id}", + json={"hard_delete": True}, + headers=superuser_token_headers, + ) + assert response.status_code == 200 + + # The row is permanently gone. + db.expire_all() + assert get_project_by_id(session=db, project_id=project_id) is None + # Test retrieving projects by organization def test_read_projects_by_organization( @@ -139,6 +230,70 @@ def test_read_projects_by_organization( assert any(p["id"] == project.id for p in response_data["data"]) +# Test the is_active param toggles active/inactive projects for an organization +def test_read_projects_by_organization_active_inactive( + db: Session, superuser_token_headers: dict[str, str] +) -> None: + org = create_test_organization(db) + active = _make_project(db, org, f"{random_lower_string()}-active", is_active=True) + inactive = _make_project( + db, org, f"{random_lower_string()}-inactive", is_active=False + ) + + # Default returns only active projects. + response = client.get( + f"{settings.API_V1_STR}/projects/organization/{org.id}", + headers=superuser_token_headers, + ) + assert response.status_code == 200 + ids = [p["id"] for p in response.json()["data"]] + assert active.id in ids + assert inactive.id not in ids + + # is_active=false returns only inactive projects. + response = client.get( + f"{settings.API_V1_STR}/projects/organization/{org.id}?is_active=false", + headers=superuser_token_headers, + ) + assert response.status_code == 200 + ids = [p["id"] for p in response.json()["data"]] + assert inactive.id in ids + assert active.id not in ids + + +# Test search combined with is_active for the org-scoped project list +def test_read_projects_by_organization_search( + db: Session, superuser_token_headers: dict[str, str] +) -> None: + org = create_test_organization(db) + prefix = random_lower_string() + active = _make_project(db, org, f"{prefix}-active", is_active=True) + inactive = _make_project(db, org, f"{prefix}-inactive", is_active=False) + _make_project(db, org, "other-project", is_active=True) + + # Search active projects by name. + response = client.get( + f"{settings.API_V1_STR}/projects/organization/{org.id}?search={prefix}", + headers=superuser_token_headers, + ) + assert response.status_code == 200 + names = [p["name"] for p in response.json()["data"]] + assert active.name in names + assert "other-project" not in names + assert inactive.name not in names + + # Search inactive projects by name. + response = client.get( + f"{settings.API_V1_STR}/projects/organization/{org.id}" + f"?search={prefix}&is_active=false", + headers=superuser_token_headers, + ) + assert response.status_code == 200 + names = [p["name"] for p in response.json()["data"]] + assert inactive.name in names + assert active.name not in names + + # Test retrieving projects by non-existent organization def test_read_projects_by_organization_not_found( db: Session, superuser_token_headers: dict[str, str] diff --git a/backend/app/tests/api/routes/test_users.py b/backend/app/tests/api/routes/test_users.py index 4b1e2113f..12712af17 100644 --- a/backend/app/tests/api/routes/test_users.py +++ b/backend/app/tests/api/routes/test_users.py @@ -1,5 +1,3 @@ -from unittest.mock import patch - from fastapi.testclient import TestClient from sqlmodel import Session, select @@ -7,7 +5,8 @@ from app.core.config import settings from app.core.security import verify_password from app.models import User, UserCreate -from app.tests.utils.utils import random_email, random_lower_string, get_non_existent_id +from app.tests.utils.test_data import add_user_to_test_project +from app.tests.utils.utils import get_non_existent_id, random_email, random_lower_string def test_get_users_superuser_me( @@ -57,6 +56,8 @@ def test_get_existing_user_current_user(client: TestClient, db: Session) -> None user_in = UserCreate(email=username, password=password) user = crud.create_user(session=db, user_create=user_in) user_id = user.id + # Login requires an active project membership. + add_user_to_test_project(db, user) login_data = { "username": username, @@ -372,6 +373,8 @@ def test_delete_user_me(client: TestClient, db: Session) -> None: user_in = UserCreate(email=username, password=password) user = crud.create_user(session=db, user_create=user_in) user_id = user.id + # Login requires an active project membership. + add_user_to_test_project(db, user) login_data = { "username": username, diff --git a/backend/app/tests/api/test_auth.py b/backend/app/tests/api/test_auth.py index b94782a98..04eaf015c 100644 --- a/backend/app/tests/api/test_auth.py +++ b/backend/app/tests/api/test_auth.py @@ -13,6 +13,7 @@ verify_magic_link_token, ) from app.tests.utils.auth import TestAuthContext +from app.tests.utils.test_data import add_user_to_test_project from app.tests.utils.user import create_random_user GOOGLE_AUTH_URL = f"{settings.API_V1_STR}/auth/google" @@ -100,6 +101,8 @@ def test_google_auth_activates_inactive_user( ): """Test that inactive user is activated on first Google login.""" user = create_random_user(db) + # Login requires an active project membership. + add_user_to_test_project(db, user) user.is_active = False db.add(user) db.commit() @@ -118,10 +121,10 @@ def test_google_auth_activates_inactive_user( @patch("app.api.routes.auth.id_token.verify_oauth2_token") @patch("app.api.routes.auth.settings") - def test_google_auth_success_no_projects( + def test_google_auth_no_projects_forbidden( self, mock_settings, mock_verify, db: Session, client: TestClient ): - """Test successful login for user with no projects.""" + """A non-superuser with no active project cannot log in via Google.""" user = create_random_user(db) mock_settings.GOOGLE_CLIENT_ID = "test-client-id" @@ -133,15 +136,8 @@ def test_google_auth_success_no_projects( mock_verify.return_value = _mock_idinfo(user.email) resp = client.post(GOOGLE_AUTH_URL, json={"token": "fake"}) - assert resp.status_code == 200 - - body = resp.json() - assert body["success"] is True - data = body["data"] - assert "access_token" in data - assert data["requires_project_selection"] is False - assert data["available_projects"] == [] - assert "access_token" in resp.cookies + assert resp.status_code == 403 + assert "not assigned to any active project" in resp.json()["error"] @patch("app.api.routes.auth.id_token.verify_oauth2_token") @patch("app.api.routes.auth.settings") @@ -454,6 +450,8 @@ def test_verify_user_not_found(self, client: TestClient): def test_verify_activates_inactive_user(self, db: Session, client: TestClient): """Test magic link verify activates inactive user.""" user = create_random_user(db) + # Login requires an active project membership. + add_user_to_test_project(db, user) user.is_active = False db.add(user) db.commit() @@ -469,6 +467,8 @@ def test_verify_activates_inactive_user(self, db: Session, client: TestClient): def test_verify_success(self, db: Session, client: TestClient): """Test successful magic link verification logs user in.""" user = create_random_user(db) + # Login requires an active project membership. + add_user_to_test_project(db, user) token = generate_magic_link_token(email=user.email) resp = client.get(f"{MAGIC_LINK_VERIFY_URL}?token={token}") diff --git a/backend/app/tests/api/test_deps.py b/backend/app/tests/api/test_deps.py index ddb09cb2e..881df45e8 100644 --- a/backend/app/tests/api/test_deps.py +++ b/backend/app/tests/api/test_deps.py @@ -1,20 +1,20 @@ from unittest.mock import MagicMock import pytest -from sqlmodel import Session from fastapi import HTTPException from fastapi.testclient import TestClient +from sqlmodel import Session from app.api.deps import get_auth_context +from app.core.config import settings +from app.core.security import create_access_token, create_refresh_token from app.models import ( - User, AuthContext, + User, ) from app.tests.utils.auth import TestAuthContext +from app.tests.utils.test_data import add_user_to_test_project, create_test_api_key from app.tests.utils.user import authentication_token_from_email, create_random_user -from app.core.config import settings -from app.core.security import create_access_token, create_refresh_token -from app.tests.utils.test_data import create_test_api_key def _mock_request(cookies: dict | None = None) -> MagicMock: @@ -127,6 +127,8 @@ def test_get_auth_context_with_inactive_user_via_token( ) -> None: """Test authentication fails when token belongs to inactive user""" user = create_random_user(db) + # Login requires an active project membership. + add_user_to_test_project(db, user) token_headers = authentication_token_from_email( client=client, email=user.email, db=db ) diff --git a/backend/app/tests/crud/test_org.py b/backend/app/tests/crud/test_org.py index db5bb2f2c..503fc76d0 100644 --- a/backend/app/tests/crud/test_org.py +++ b/backend/app/tests/crud/test_org.py @@ -8,8 +8,8 @@ validate_organization, ) from app.models import Organization, OrganizationCreate -from app.tests.utils.utils import random_lower_string, get_non_existent_id from app.tests.utils.test_data import create_test_organization +from app.tests.utils.utils import get_non_existent_id, random_lower_string def test_create_organization(db: Session) -> None: @@ -23,6 +23,28 @@ def test_create_organization(db: Session) -> None: assert organization.is_active is True +def test_create_organization_duplicate_active_name(db: Session) -> None: + """Creating an org whose name is already taken by an active org is rejected.""" + name = random_lower_string() + create_organization(session=db, org_create=OrganizationCreate(name=name)) + + with pytest.raises(HTTPException, match="already exists"): + create_organization(session=db, org_create=OrganizationCreate(name=name)) + + +def test_create_organization_duplicate_inactive_name(db: Session) -> None: + """Recreating a soft-deleted org name is rejected with a reactivate hint.""" + name = random_lower_string() + org = create_organization(session=db, org_create=OrganizationCreate(name=name)) + + org.is_active = False + db.add(org) + db.flush() + + with pytest.raises(HTTPException, match="inactive"): + create_organization(session=db, org_create=OrganizationCreate(name=name)) + + def test_get_organization_by_id(db: Session) -> None: """Test retrieving an organization by ID.""" organization = create_test_organization(db) diff --git a/backend/app/tests/crud/test_project.py b/backend/app/tests/crud/test_project.py index 9698b4662..d96b5fd77 100644 --- a/backend/app/tests/crud/test_project.py +++ b/backend/app/tests/crud/test_project.py @@ -1,8 +1,7 @@ import pytest -from sqlmodel import Session from fastapi import HTTPException +from sqlmodel import Session -from app.models import Project, ProjectCreate from app.crud.project import ( create_project, get_project_by_id, @@ -10,8 +9,9 @@ get_projects_by_organization, validate_project, ) -from app.tests.utils.utils import random_lower_string, get_non_existent_id -from app.tests.utils.test_data import create_test_project, create_test_organization +from app.models import Project, ProjectCreate +from app.tests.utils.test_data import create_test_organization, create_test_project +from app.tests.utils.utils import get_non_existent_id, random_lower_string def test_create_project(db: Session) -> None: @@ -45,8 +45,28 @@ def test_create_project_duplicate_name(db: Session) -> None: is_active=True, organization_id=organization.id, ) + create_project(session=db, project_create=project_data) + with pytest.raises(HTTPException, match="already exists in this organization"): + create_project(session=db, project_create=project_data) + + +def test_create_project_duplicate_inactive_name(db: Session) -> None: + """Recreating a soft-deleted project name is rejected with a reactivate hint.""" + organization = create_test_organization(db) + + project_data = ProjectCreate( + name=random_lower_string(), + description="Test description", + is_active=True, + organization_id=organization.id, + ) project = create_project(session=db, project_create=project_data) - with pytest.raises(HTTPException, match="Project already exists"): + + project.is_active = False + db.add(project) + db.flush() + + with pytest.raises(HTTPException, match="inactive"): create_project(session=db, project_create=project_data) diff --git a/backend/app/tests/crud/test_user_project.py b/backend/app/tests/crud/test_user_project.py index 917686e6b..65b592951 100644 --- a/backend/app/tests/crud/test_user_project.py +++ b/backend/app/tests/crud/test_user_project.py @@ -1,4 +1,3 @@ -import pytest from sqlmodel import Session from app.crud.user_project import ( @@ -162,8 +161,8 @@ def test_remove_nonexistent_mapping_returns_false(self, db: Session): removed = remove_user_from_project(session=db, user_id=99999, project_id=99999) assert removed is False - def test_remove_last_project_deletes_user(self, db: Session): - """Test removing user from their last project deletes the user.""" + def test_remove_last_project_deactivates_user(self, db: Session): + """Test removing user from their last project deactivates (not deletes) the user.""" project = create_test_project(db) email = random_email() @@ -174,16 +173,23 @@ def test_remove_last_project_deletes_user(self, db: Session): project_id=project.id, ) user_id = user.id + # Simulate an activated user before they lose their last project. + user.is_active = True + db.add(user) + db.flush() remove_user_from_project(session=db, user_id=user_id, project_id=project.id) - assert db.get(User, user_id) is None + deactivated_user = db.get(User, user_id) + assert deactivated_user is not None + assert deactivated_user.is_active is False def test_remove_last_project_preserves_superuser(self, db: Session): - """Test superuser is not deleted when removed from last project.""" + """Test superuser is not deactivated when removed from last project.""" project = create_test_project(db) user = create_random_user(db) user.is_superuser = True + user.is_active = True db.add(user) db.flush() @@ -197,7 +203,9 @@ def test_remove_last_project_preserves_superuser(self, db: Session): remove_user_from_project(session=db, user_id=user.id, project_id=project.id) - assert db.get(User, user.id) is not None + preserved_user = db.get(User, user.id) + assert preserved_user is not None + assert preserved_user.is_active is True class TestGetUserProjects: diff --git a/backend/app/tests/utils/test_data.py b/backend/app/tests/utils/test_data.py index b7b4320ae..2538b5380 100644 --- a/backend/app/tests/utils/test_data.py +++ b/backend/app/tests/utils/test_data.py @@ -1,43 +1,45 @@ from sqlmodel import Session +from app.core.providers import Provider +from app.crud import ( + APIKeyCrud, + create_fine_tuning_job, + create_model_evaluation, + create_organization, + create_project, + set_creds_for_org, +) +from app.crud.config import ConfigCrud, ConfigVersionCrud from app.models import ( - Organization, - Project, APIKeyCreateResponse, - Credential, - OrganizationCreate, - ProjectCreate, + Config, ConfigBlob, + ConfigCreate, + ConfigVersion, + ConfigVersionUpdate, + Credential, CredsCreate, - FineTuningJobCreate, + EvaluationDataset, FineTuning, + FineTuningJobCreate, ModelEvaluation, ModelEvaluationBase, ModelEvaluationStatus, - Config, - ConfigCreate, - ConfigVersion, - ConfigVersionUpdate, - EvaluationDataset, + Organization, + OrganizationCreate, + Project, + ProjectCreate, + User, + UserProject, ) from app.models.config.config import ConfigTag -from app.models.llm import KaapiLLMParams, KaapiCompletionConfig, NativeCompletionConfig -from app.crud import ( - create_organization, - create_project, - set_creds_for_org, - create_fine_tuning_job, - create_model_evaluation, - APIKeyCrud, -) -from app.crud.config import ConfigCrud, ConfigVersionCrud -from app.core.providers import Provider +from app.models.llm import KaapiCompletionConfig, NativeCompletionConfig from app.tests.utils.user import create_random_user from app.tests.utils.utils import ( - random_lower_string, generate_random_string, get_document, get_project, + random_lower_string, ) @@ -71,6 +73,25 @@ def create_test_project(db: Session) -> Project: return create_project(session=db, project_create=project_in) +def add_user_to_test_project(db: Session, user: User) -> Project: + """ + Map a user to a fresh active organization + project. + + Login requires at least one active project, so tests that create a bare + user and then log in must give the user a project first. + """ + project = create_test_project(db) + db.add( + UserProject( + user_id=user.id, + organization_id=project.organization_id, + project_id=project.id, + ) + ) + db.commit() + return project + + def test_credential_data(db: Session) -> CredsCreate: """ Returns credential data for a test project in the form of a CredsCreate schema. @@ -324,7 +345,8 @@ def create_test_version( """ if config_blob is None: # Fetch the latest version to maintain type consistency - from sqlmodel import select, and_ + from sqlmodel import and_, select + from app.models import ConfigVersion stmt = (