[Patchew-devel] [PATCH] rest: POST for message endpoint
Fam Zheng
famz at redhat.com
Mon May 7 14:09:55 UTC 2018
On Mon, 05/07 11:03, Shubham Jain wrote:
> On Mon, May 7, 2018 at 4:25 PM Paolo Bonzini <pbonzini at redhat.com> wrote:
>
> > On 05/05/2018 09:09, Shubham Jain wrote:
> > > This commit allows to create/POST message from browser
> > > - Add "create" method to MessageManager so that it calls save_mbox()
> > > - Add getter and setter for mbox
> > > - Rename the existing "mbox" field to e.g. "mbox_blob"
> > > - Fix nested writable serializer issue
> >
> > This already looks pretty good, but:
> >
> > - there are no test cases
> >
>
> I was thinking of doing this when we have POST for both json and text/plain
> format.
>
> >
> > - we should use it as an exercise in posting a series composed of
> > multiple changes
> >
>
> I still don't know how to send the multiple patches in a single patch.
You can use git-publish
$ sudo dnf install git-publish
$ git checkout master -b foo-feature
$ # add sub-feature A and commit
$ # add sub-feature B and commit
$ # add sub-feature C and commit
$ # continue until the whole feature is done...
$ git publish
Here you'll be brought to an editor to compose the cover letter. The first line
is the subject, followed by a blank line, then the cover letter body. Similar
to writing commit messages.
Once that is done, select "send" (a) from the interactive menu.
Fam
>
> > - I have a few small comments below on the code.
> >
> > If you feel more confident sending everything as a single patch first,
> > and still without tests (that is with only the code comments addressed),
> > that's fine for me.
> >
> > Now, on to the code.
> >
> > > ---
> > > api/models.py | 39 +++++++++++++++++++++++++++++++--------
> > > api/rest.py | 20 +++++++++++---------
> > > 2 files changed, 42 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/api/models.py b/api/models.py
> > > index 504f2c7..43b1037 100644
> > > --- a/api/models.py
> > > +++ b/api/models.py
> > > @@ -247,6 +247,24 @@ class MessageManager(models.Manager):
> > > self.delete_subthread(r)
> > > msg.delete()
> > >
> > > + def create(self,project, **validated_data):
> > > + mbox = validated_data.pop('mbox')
> > > + m = MboxMessage(mbox)
> > > + msg = Message(**validated_data)
> > > + msg.in_reply_to = m.get_in_reply_to() or ""
> >
> > Setting in_reply_to here is the right thing to do for how the code
> > behaves right now. However, the real problem is that it's missing in
> > MessageSerializer! Can you add it, and here do:
> >
> > if 'in_reply_to' not in validated_data:
> > msg.in_reply_to = m.get_in_reply_to() or ""
> >
> > ?
> >
> Yeah, this makes more sense.
>
> > > + msg.stripped_subject = m.get_subject(strip_tags=True)
> > > + msg.version = m.get_version()
> > > + msg.prefixes = m.get_prefixes()
> > > + msg.is_series_head = m.is_series_head()
> > > + msg.is_patch = m.is_patch()
> > > + msg.patch_num = m.get_num()[0]
> > > + msg.project = project
> > > + msg.mbox = mbox
> > > + msg.save_mbox(mbox)
> > > + msg.save()
> > > + emit_event("MessageAdded", message=msg)
> > > + return msg
> > > +
> > > def add_message_from_mbox(self, mbox, user, project_name=None):
> > >
> > > def find_message_projects(m):
> > > @@ -321,20 +339,25 @@ class Message(models.Model):
> > >
> > > objects = MessageManager()
> > >
> > > - def save_mbox(self, mbox):
> > > - save_blob(mbox, self.message_id)
> > > + def save_mbox(self, mbox_blob):
> > > + save_blob(mbox_blob, self.message_id)
> > >
> > > def get_mbox_obj(self):
> > > self.get_mbox()
> > > return self._mbox_obj
> > >
> > > def get_mbox(self):
> > > - if hasattr(self, "mbox"):
> > > - return self.mbox
> > > - self.mbox = load_blob(self.message_id)
> > > - self._mbox_obj = MboxMessage(self.mbox)
> > > - return self.mbox
> > > -
> > > + if hasattr(self, "mbox_blob"):
> > > + return self.mbox_blob
> > > + self.mbox_blob = load_blob(self.message_id)
> > > + self._mbox_obj = MboxMessage(self.mbox_blob)
> > > + return self.mbox_blob
> > > +
> > > + mbox = property(get_mbox)
> > > + @mbox.setter
> > > + def mbox(self,value):
> > > + self.mbox_blob = value
> > > +
> > > def get_num(self):
> > > assert self.is_patch or self.is_series_head
> > > cur, total = 1, 1
> > > diff --git a/api/rest.py b/api/rest.py
> > > index fc10b46..7c38d39 100644
> > > --- a/api/rest.py
> > > +++ b/api/rest.py
> > > @@ -140,9 +140,13 @@ class
> > BaseMessageSerializer(serializers.ModelSerializer):
> > > fields = ('resource_uri', 'message_id', 'subject', 'date',
> > 'sender', 'recipients')
> > >
> > > resource_uri = HyperlinkedMessageField(view_name='messages-detail')
> > > -
> > > recipients = AddressSerializer(many=True)
> > > - sender = AddressSerializer()
> > > + sender = AddressSerializer()
> >
> > You're adding whitespace at the end of the line here.
> >
> > > +
> > > + def create(self, validated_data):
> > > + validated_data['recipients'] =
> > self.fields['recipients'].create(validated_data['recipients'])
> > > + validated_data['sender'] =
> > self.fields['sender'].create(validated_data['sender'])
> > > + return Message.objects.create(project=self.context['project'],
> > **validated_data)
> > >
> > > # a message_id is *not* unique, so we can only list
> > > class BaseMessageViewSet(mixins.ListModelMixin,
> > viewsets.GenericViewSet):
> > > @@ -156,7 +160,9 @@ class BaseMessageViewSet(mixins.ListModelMixin,
> > viewsets.GenericViewSet):
> > > class ProjectMessagesViewSetMixin(mixins.RetrieveModelMixin):
> > > def get_queryset(self):
> > > return self.queryset.filter(project=self.kwargs['projects_pk'])
> > > -
> > > + def get_serializer_context(self):
> > > + return {'project':
> > Project.objects.get(id=self.kwargs['projects_pk']), 'request': self.request}
> >
> > Good! Out of curiosity what do you actually need 'request' for?
> >
> > Request is used by dispatch_module_hook is BaseMessageSerializer
>
> > > # Series
> > >
> > > class ReplySerializer(BaseMessageSerializer):
> > > @@ -287,10 +293,7 @@ class MessageSerializer(BaseMessageSerializer):
> > > class Meta:
> > > model = Message
> > > fields = BaseMessageSerializer.Meta.fields + ('mbox', )
> > > -
> > > - def get_mbox(self, obj):
> > > - return obj.get_mbox()
> > > - mbox = SerializerMethodField()
> > > + mbox = JSONField()
> >
> > Can this be a CharField() instead?
> >
> Okay.
>
> > >
> > > def get_fields(self):
> > > fields = super(MessageSerializer, self).get_fields()
> > > @@ -312,9 +315,8 @@ class StaticTextRenderer(renderers.BaseRenderer):
> > > return data
> > >
> > > class MessagesViewSet(ProjectMessagesViewSetMixin,
> > > - BaseMessageViewSet):
> > > + BaseMessageViewSet, mixins.CreateModelMixin):
> > > serializer_class = MessageSerializer
> > > -
> > > @detail_route(renderer_classes=[StaticTextRenderer])
> > > def mbox(self, request, *args, **kwargs):
> > > message = self.get_object()
> > >
> >
> >
> _______________________________________________
> Patchew-devel mailing list
> Patchew-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/patchew-devel
More information about the Patchew-devel
mailing list