Fix create and delete organisation

src/backend/app/db/models.py

@@ -32,6 +32,7 @@
 from fastapi import HTTPException, UploadFile
 from loguru import logger as log
 from psycopg import Connection
+from psycopg.errors import UniqueViolation
 from psycopg.rows import class_row
 from pydantic import AwareDatetime, BaseModel, Field, ValidationInfo
 from pydantic.functional_validators import field_validator
@@ -352,9 +353,8 @@ async def one(cls, db: Connection, org_identifier: int | str) -> Self:
                 )
                 db_org = await cur.fetchone()
 
-        if db_org is None:
-            raise KeyError(f"Organisation ({org_identifier}) not found.")
-
+            if db_org is None:
+                raise KeyError(f"Organisation ({org_identifier}) not found.")
         return db_org
 
     @classmethod
@@ -397,11 +397,6 @@ async def create(
         # Set requesting user to the org owner
         org_in.created_by = user_id
 
-        if not ignore_conflict and cls.one(db, org_in.name):
-            msg = f"Organisation named ({org_in.name}) already exists!"
-            log.warning(f"User ({user_id}) failed organisation creation: {msg}")
-            raise HTTPException(status_code=HTTPStatus.CONFLICT, detail=msg)
-
         model_dump = dump_and_check_model(org_in)
         columns = ", ".join(model_dump.keys())
         value_placeholders = ", ".join(f"%({key})s" for key in model_dump.keys())
@@ -415,24 +410,33 @@ async def create(
             RETURNING *;
         """
 
-        async with db.cursor(row_factory=class_row(cls)) as cur:
-            await cur.execute(sql, model_dump)
-            new_org = await cur.fetchone()
+        try:
+            async with db.cursor(row_factory=class_row(cls)) as cur:
+                await cur.execute(sql, model_dump)
+                new_org = await cur.fetchone()
 
-        if new_org is None and not ignore_conflict:
-            msg = f"Unknown SQL error for data: {model_dump}"
-            log.error(f"User ({user_id}) failed organisation creation: {msg}")
-            raise HTTPException(
-                status_code=HTTPStatus.INTERNAL_SERVER_ERROR, detail=msg
-            )
+            if new_org and new_org.logo is None and new_logo:
+                from app.organisations.organisation_schemas import OrganisationUpdate
 
-        if new_org and new_org.logo is None and new_logo:
-            from app.organisations.organisation_schemas import OrganisationUpdate
+                logo_url = await cls.upload_logo(new_org.id, new_logo)
+                await cls.update(db, new_org.id, OrganisationUpdate(logo=logo_url))
 
-            logo_url = await cls.upload_logo(new_org.id, new_logo)
-            await cls.update(db, new_org.id, OrganisationUpdate(logo=logo_url))
+            return new_org
+        except UniqueViolation as e:
+            # Return conflict error when organisation name already exists
+            log.error(f"Organisation named ({org_in.name}) already exists!")
+            raise HTTPException(
+                status_code=HTTPStatus.CONFLICT,
+                detail=f"Organisation named ({org_in.name}) already exists!",
+            ) from e
 
-        return new_org
+        except Exception as e:
+            # Log errors only for actual failures (e.g., DB errors)
+            msg = f"Unknown SQL error for data: {model_dump}"
+            log.error(f"User ({user_id}) failed organisation creation: {e}")
+            raise HTTPException(
+                status_code=HTTPStatus.INTERNAL_SERVER_ERROR, detail=msg
+            ) from e
 
     @classmethod
     async def upload_logo(
@@ -519,7 +523,11 @@ async def delete(cls, db: Connection, org_id: int) -> bool:
                 DELETE FROM projects
                 WHERE organisation_id = %(org_id)s
                 RETURNING organisation_id
-            ), deleted_org AS (
+            ), deleted_org_managers AS (
+                DELETE FROM organisation_managers
+                WHERE organisation_id = %(org_id)s
+                RETURNING organisation_id
+            ),deleted_org AS (
                 DELETE FROM organisations
                 WHERE id = %(org_id)s
                 RETURNING id
@@ -536,6 +544,7 @@ async def delete(cls, db: Connection, org_id: int) -> bool:
                 settings.S3_BUCKET_NAME,
                 f"/{org_id}/",
             )
+            return True
 
     @classmethod
     async def unapproved(cls, db: Connection) -> Optional[list[Self]]:

src/backend/app/organisations/organisation_crud.py

@@ -19,12 +19,13 @@
 
 from fastapi import UploadFile
 from psycopg import Connection
+from psycopg.rows import class_row
 
 from app.auth.auth_schemas import AuthUser
 from app.config import settings
 from app.db.enums import MappingLevel, UserRole
 from app.db.models import DbOrganisation, DbOrganisationManagers, DbUser
-from app.organisations.organisation_schemas import OrganisationIn
+from app.organisations.organisation_schemas import OrganisationIn, OrganisationOut
 from app.users.user_schemas import UserIn
 
 
@@ -97,14 +98,12 @@ async def get_my_organisations(
     Returns:
         list[dict]: A list of organisation objects to be serialised.
     """
-    user_id = current_user.id
-
     sql = """
         SELECT DISTINCT org.*
         FROM organisations org
         JOIN organisation_managers managers
             ON managers.organisation_id = org.id
-        WHERE managers.user_id = :user_id
+        WHERE managers.user_id = %(user_id)s
 
         UNION
 
@@ -114,7 +113,6 @@ async def get_my_organisations(
             ON project.organisation_id = org.id
         WHERE project.author_id = %(user_id)s;
     """
-    async with db.cursor() as cur:
-        await cur.execute(sql, {"user_id": user_id})
-        orgs = cur.fetchall()
-    return orgs
+    async with db.cursor(row_factory=class_row(OrganisationOut)) as cur:
+        await cur.execute(sql, {"user_id": current_user.id})
+        return await cur.fetchall()

src/backend/app/organisations/organisation_routes.py

@@ -23,6 +23,7 @@
     APIRouter,
     Depends,
     File,
+    HTTPException,
     Response,
     UploadFile,
 )
@@ -41,6 +42,7 @@
     OrganisationIn,
     OrganisationOut,
     OrganisationUpdate,
+    parse_organisation_input,
 )
 
 router = APIRouter(
@@ -91,27 +93,32 @@ async def create_organisation(
     db: Annotated[Connection, Depends(db_conn)],
     current_user: Annotated[AuthUser, Depends(login_required)],
     # Depends required below to allow logo upload
-    org_in: OrganisationIn = Depends(),
+    org_in: OrganisationIn = Depends(parse_organisation_input),
     logo: Optional[UploadFile] = File(None),
 ) -> OrganisationOut:
     """Create an organisation with the given details.
 
     Either a logo can be uploaded, or a link to the logo provided
     in the Organisation JSON ('logo': 'https://your.link.to.logo.png').
     """
-    return DbOrganisation.create(db, org_in, current_user.id, logo)
+    if org_in.name is None:
+        raise HTTPException(
+            status_code=HTTPStatus.BAD_REQUEST,
+            detail="The `name` is required to create an organisation.",
+        )
+    return await DbOrganisation.create(db, org_in, current_user.id, logo)
 
 
 @router.patch("/{org_id}/", response_model=OrganisationOut)
 async def update_organisation(
     db: Annotated[Connection, Depends(db_conn)],
     org_user_dict: Annotated[AuthUser, Depends(org_admin)],
-    new_values: OrganisationUpdate = Depends(),
+    new_values: OrganisationUpdate = Depends(parse_organisation_input),
     logo: UploadFile = File(None),
 ):
     """Partial update for an existing organisation."""
     org_id = org_user_dict.get("org").id
-    return DbOrganisation.update(db, org_id, new_values, logo)
+    return await DbOrganisation.update(db, org_id, new_values, logo)
 
 
 @router.delete("/{org_id}")
@@ -121,18 +128,14 @@ async def delete_org(
 ):
     """Delete an organisation."""
     org = org_user_dict.get("org")
-    deleted_org_id = await DbOrganisation.delete(db, org.id)
-
-    if not deleted_org_id:
-        return Response(
+    org_deleted = await DbOrganisation.delete(db, org.id)
+    if not org_deleted:
+        log.error(f"Failed deleting org ({org.name}).")
+        raise HTTPException(
             status_code=HTTPStatus.UNPROCESSABLE_ENTITY,
-            details=f"Failed deleting org {(org.name)}.",
+            detail=f"Failed deleting org ({org.name}).",
         )
-
-    return Response(
-        status_code=HTTPStatus.NO_CONTENT,
-        details=f"Deleted org {(org.deleted_org_id)}.",
-    )
+    return Response(status_code=HTTPStatus.NO_CONTENT)
 
 
 @router.delete("/unapproved/{org_id}")
@@ -148,11 +151,17 @@ async def delete_unapproved_org(
     will also check if the organisation is approved and error if it's not.
     This is an ADMIN-only endpoint for deleting unapproved orgs.
     """
-    await DbOrganisation.delete(db, org_id)
-    return Response(
-        status_code=HTTPStatus.NO_CONTENT,
-        detail=f"Deleted org ({org_id}).",
-    )
+    org_deleted = await DbOrganisation.delete(db, org_id)
+
+    if not org_deleted:
+        log.error(f"Failed deleting org ({org_id}).")
+        raise HTTPException(
+            status_code=HTTPStatus.UNPROCESSABLE_ENTITY,
+            detail=f"Failed deleting org ({org_id}).",
+        )
+
+    log.info(f"Successfully deleted org ({org_id}).")
+    return Response(status_code=HTTPStatus.NO_CONTENT)
 
 
 @router.post("/approve/", response_model=OrganisationOut)

src/backend/app/organisations/organisation_schemas.py

@@ -19,11 +19,12 @@
 
 from typing import Annotated, Optional, Self
 
+from fastapi import Form
 from pydantic import BaseModel, Field
 from pydantic.functional_validators import model_validator
 
 from app.central.central_schemas import ODKCentralIn
-from app.db.enums import OrganisationType
+from app.db.enums import CommunityType, OrganisationType
 from app.db.models import DbOrganisation, slugify
 
 
@@ -59,6 +60,41 @@ class OrganisationUpdate(OrganisationInBase):
     name: Optional[str] = None
 
 
+def parse_organisation_input(
+    name: Optional[str] = Form(None),
+    slug: Optional[str] = Form(None),
+    created_by: Optional[int] = Form(None),
+    community_type: CommunityType = Form(None),
+    type: OrganisationType = Form(None, alias="type"),
+    odk_central_url: Optional[str] = Form(None),
+    odk_central_user: Optional[str] = Form(None),
+    odk_central_password: Optional[str] = Form(None),
+) -> OrganisationUpdate:
+    """Parse organisation input data from a FastAPI Form.
+
+    The organisation fields are passed as keyword arguments. The
+    ODKCentralIn model is used to parse the ODK credential fields, and
+    the OrganisationIn model is used to parse the organisation fields.
+
+    The parsed data is returned as an OrganisationIn instance, with the
+    ODKCentralIn fields merged in.
+    """
+    odk_central_data = ODKCentralIn(
+        odk_central_url=odk_central_url,
+        odk_central_user=odk_central_user,
+        odk_central_password=odk_central_password,
+    )
+    org_data = OrganisationUpdate(
+        name=name,
+        slug=slug,
+        created_by=created_by,
+        community_type=community_type,
+        type=type,
+        **odk_central_data.dict(exclude_unset=True),
+    )
+    return org_data
+
+
 class OrganisationOut(BaseModel):
     """Organisation to display to user."""