[Patchew-devel] [PATCH v2] rest: add api/v1/messages endpoint (POST support)

[Paolo Bonzini]

On 16/05/2018 03:31, Fam Zheng wrote:
> On Tue, 05/15 14:08, Paolo Bonzini wrote:
>> On 15/05/2018 14:02, Shubham Jain wrote:
>>> +    def create(self, request, *args, **kwargs):
>>> +        projects = [p for p in Project.objects.all() if p.recognizes(MboxMessage(self.request.data['mbox']))]
> 
> It is better to create MboxMessage object once to avoid repeating the mbox
> parse:
> 
>              m = MboxMessage(self.request.data['mbox'])
>              projects = [p for p in Project.objects.all() if p.recognizes(m)]

Also the MboxMessage probably should be created from self.request.data,
without doing more parsing of the mbox.

Alternatively, p.recognizes() could take an array of recipients rather
than an MboxMessage.

> 
>>> +        if 'importers' not in self.request.user.groups.all():
>>> +            projects = set(projects) & set([p for p in Project.objects.all() if p.maintained_by(self.request.user)])
>>
>> This can be just "projects = (p for p in projects if
>> p.maintained_by(self.request_user))".  I'll make the change and commit it.
>>
>> Fam, what do you think should Admin accounts do?  Import only to the
>> projects they maintain, or to all of them?
> 
> I think all is better. The projects to import can be explicitly limited if they want.

Ok, thanks!  Shubham is going to add more tests which I'm sure will
uncover some bugs, :) so let's do this change as well.

I've updated the checklist at
https://github.com/patchew-project/patchew/issues/75.

Thanks,

Paolo