CRUD Endpoint POST response is a list

[ocervell]

On making a POST request to a CRUD endpoint, the response is something like:

[{'id': 4}]

IMHO this should be a dict following good REST practices and not a list since we cannot post multiple objects to the CRUD Endpoint. The client should be able to run something like:

>>> response = requests.post(crud_endpoint_url, json=data).json()
>>> response['id']
# created object id

The problem stems from this line: https://github.com/piccolo-orm/piccolo_api/blob/master/piccolo_api/crud/endpoints.py#L819

Actually row.save().run() seems to always return a list with one object ... assuming this is the case, we could strip the object out of the list and simply return the dict:

response = await row.save().run()
item = response[0]
json = dump_json(item)
return CustomJSONResponse(json, status_code=201)

[mastercoms]

Also it returns 201 instead of 200 as in the OpenAPI schema (for FastAPIWrapper), the schema should be fixed or if it can't, change to 200

[sinisaos]

@mastercoms I think HTTP 201 CREATED is a good response status code because the request was successful and led to the creation of a new resource. Sorry if I'm wrong.

[dantownsend]

I think what's going on is we're returning a 201 from PiccoloCRUD, but FastAPIWrapper has the default of 200. They should be consistent really - it should be a quick fix.

https://github.com/piccolo-orm/piccolo_api/blob/40d431693602da33460d10b4b2e7257065f8bcf6/piccolo_api/fastapi/endpoints.py#L393

[sinisaos]

@dantownsend I don't think we should change anything because both PiccoloCRUD and FastAPIWrapper ,in my case, return correct 201 on POST request? Sorry if I don't understand correctly.

[dantownsend]

@sinisaos Sorry, I wasn't clear - they both return 201, but in the Swagger docs it probably say 200.

It's because we don't tell FastAPI what status code we're returning.

[sinisaos]

@dantownsend Aha, now I understand! Here we can add status_code=status.HTTP_201_CREATED, to add_api_route of POST method and it works.

Should we do it for all methods?

[dantownsend]

Should we do it for all methods?

@sinisaos yeah, at least for any which don't return 200

[sinisaos]

@dantownsend If you OK with this changes

fastapi_app.add_api_route(
    path=root_url,
    endpoint=post,
    response_model=self.ModelOut,
    status_code=status.HTTP_201_CREATED, # new line
    methods=["POST"],
    **fastapi_kwargs.get_kwargs("post"),
)

I can try to do this tomorrow for all the methods to have the same status codes in the Swagger docs as in the PiccoloCRUD.

[dantownsend]

@sinisaos Yes, that looks right.