Skip to content

Commit

Permalink
refactor: create organisation with proper log and schema to generate …
Browse files Browse the repository at this point in the history
…form fields
  • Loading branch information
Sujanadh committed Nov 14, 2024
1 parent 9e5baa6 commit 5876566
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 26 deletions.
47 changes: 27 additions & 20 deletions src/backend/app/db/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
from loguru import logger as log
from psycopg import Connection
from psycopg.rows import class_row
from psycopg.errors import UniqueViolation
from pydantic import AwareDatetime, BaseModel, Field, ValidationInfo
from pydantic.functional_validators import field_validator

Expand Down Expand Up @@ -352,6 +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 ({id}) not found.")
return db_org

@classmethod
Expand Down Expand Up @@ -394,11 +397,6 @@ async def create(
# Set requesting user to the org owner
org_in.created_by = user_id

if not ignore_conflict and await 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())
Expand All @@ -411,25 +409,34 @@ async def create(
{'ON CONFLICT ("name") DO NOTHING' if ignore_conflict else ''}
RETURNING *;
"""

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 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))

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

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:
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: {msg}")
log.error(f"User ({user_id}) failed organisation creation: {e}")
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

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
) from e

@classmethod
async def upload_logo(
Expand Down
2 changes: 0 additions & 2 deletions src/backend/app/organisations/organisation_deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,6 @@ async def get_organisation(
pass
db_org = await DbOrganisation.one(db, id)

if db_org is None:
raise KeyError(f"Organisation ({id}) not found.")
except KeyError as e:
raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail=str(e)) from e

Expand Down
3 changes: 2 additions & 1 deletion src/backend/app/organisations/organisation_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
OrganisationIn,
OrganisationOut,
OrganisationUpdate,
parse_organisation_input,
)

router = APIRouter(
Expand Down Expand Up @@ -92,7 +93,7 @@ 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.
Expand Down
40 changes: 37 additions & 3 deletions src/backend/app/organisations/organisation_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@

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.central.central_schemas import ODKCentralIn
from app.db.enums import OrganisationType, CommunityType
from app.db.models import DbOrganisation, slugify


class OrganisationInBase(ODKCentralIn, DbOrganisation):
"""Base model for project insert / update (validators).
Expand All @@ -51,6 +51,40 @@ class OrganisationIn(OrganisationInBase):
# Name is mandatory
name: str

def parse_organisation_input(
name: str = Form(...),
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)
) -> OrganisationIn:
"""
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 = OrganisationIn(
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 OrganisationUpdate(OrganisationInBase):
"""Edit an org from user input."""
Expand Down

0 comments on commit 5876566

Please sign in to comment.