hotosm / fmtm

Field Mapping Tasking Manager - coordinated field mapping.
https://fmtm.hotosm.org/
GNU Affero General Public License v3.0
48 stars 46 forks source link

Unable to Create or Delete Organisation #1866

Open Sujanadh opened 6 days ago

Sujanadh commented 6 days ago

Describe the bug Unable to create an organisation and delete it.

To Reproduce

create

  1. Go to manage organisations
  2. Fill up the form to create organisation
  3. Submit
  4. See the error"Failed to create organisation"

delete Try deleting organisation using endpoint from API docs.

Expected behavior Should be able to create and delete organisation.

Sujanadh commented 6 days ago

@spwoodcock I noticed that when creating an organisation, every field except the logo is sent as a query instead of a body from the UI.

spwoodcock commented 6 days ago

Good observation!

I would say that's generally bad practice on a POST endpoint.

Since you can't attach a file to a JSON body payload (unless you base64 encode it or something), the only good option is a multipart form upload.

I think we had it set up with query params, but I maybe updated the backend to properly take a multipart form and forgot the frontend?

At least that's the ideal scenario. Query params to still work if necessary

Sujanadh commented 6 days ago

What I meant is Frontend is sending data in form but the backend expects everything as query parameters except the logo. All the fields from pydantic model which is DbOrganisation beneath OrganisationIn is by default used as query parameters when they are used as Depends() in endpoint.


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(),
    logo: Optional[UploadFile] = File(None),
) -> OrganisationOut: 
spwoodcock commented 5 days ago

Good point!

There have since been changes to FastAPI thankfully to support what we need: https://github.com/fastapi/fastapi/pull/12129

The docs https://fastapi.tiangolo.com/tutorial/request-form-models/

We can define the Pydantic model including the file now, then Annotate it as form data 😁

spwoodcock commented 5 days ago

And we are using FastAPI 0.115.x now so should have this functionality available (added 0.113.x) 👍

Sujanadh commented 5 days ago

Actually, I looked into this documentation and implemented it. I wanted to have form data with individual fields as we used to do by explicitly defining form in pydantic models. But for multipart/form-data, we need to pass data like: image

Instead of :

image

spwoodcock commented 5 days ago

Ah fair enough! Sometimes the OpenAPI docs page generated by FastAPI isn't ideal! It's a bit of a pain to look at and not ideal documentation for a frontend dev or testing.

But as long as we can use the endpoint in the correct way from the frontend, then it's all good 😄