[Patchew-devel] [PATCH v3] rest: add support for series DELETE in the REST API, and a corresponding unit test

[Paolo Bonzini]

On 11/04/2018 11:50, Shubham Jain wrote:
> 
>     Use the commit message to write information about your decisions
>     regarding the patch.  This includes decisions that you think are wrong,
>     but you had to make anyway because you were stuck!
> 
> Noted. Will keep in mind in the future.  
> 
>     >     As a next step, perhaps you can implement POST for /api/v1/messages?
>     >     That should accept a text/plain object; can you find the corresponding
>     >     legacy API endpoint?
> 
> It would be a JSON object right? The elements we are extracting from the
> mbox in add_message_from_mbox() would be provided as JSON?
> For example, instead of         msgid = m.get_message_id() it should be
> something like this msgid = request.data['message_id']? 

It can be both.

We could accept both application/json and text/plain in the REST API.
In fact, all four combinations make sense:

- /api/v1/messages + application/json: import message, looking at
recipients to find all applicable projects

- /api/v1/projects/{id}/messages + application/json: import message to
specific project

- /api/v1/messages + text/plain: import message from mbox, looking at
recipients to find all applicable projects

- /api/v1/projects/{id}/messages + text/plain: import message from mbox
to specific project

The third is simply what matches the legacy import endpoint.  text/plain
also the simplest to implement, because the Python code to create
Messages is more suited to text/plain import.

However, you can attack the problem both ways:

- first write the endpoint to only accept application/json; add code to
MboxMessage to return a JSON representation just like what the REST API
would use; use the new MboxMessage to handle text/plain in the endpoint.

- first write the endpoint to only accept text/plain; then add code to
MboxMessage to return a JSON representation just like what the REST API
would use; finally refactor the endpoint to support application/json too.

I was suggesting the second mostly because the DRF side of it is a bit
simpler.  In particular some fields are read-only and should not be
processed by write requests (POST/PUT/PATCH).  If you want to write the
API to first accept application/json, that's certainly okay!  Keep us
updated on your progress.

(A small hint that just came to mind: the MboxMessage->JSON conversion
logically comes second, but it can come in handy to write tests: if you
have that converter, your tests for the application/json REST API can
reuse the existing .gz files.  So perhaps it's better to write that
converter first, together with the associated unit tests).

Thanks,

Paolo