[Patchew-devel] [PATCH v2] serializer: Add custom serialzer for recipients and sender

Shubham Jain posted 1 patch 5 years, 11 months ago
Failed in applying to current master (apply log)
api/rest.py        | 36 +++++++++++++++++++-----------------
tests/test_rest.py | 17 +++++++++++++++++
2 files changed, 36 insertions(+), 17 deletions(-)
[Patchew-devel] [PATCH v2] serializer: Add custom serialzer for recipients and sender
Posted by Shubham Jain 5 years, 11 months ago
- Earlier, recipients and sender were read only serializers and could not be used for POST/PUT/PATCH requests. Change it to custome serializer for POST request at api/v1/messages/
- Add test corresponding to it
---
 api/rest.py        | 36 +++++++++++++++++++-----------------
 tests/test_rest.py | 17 +++++++++++++++++
 2 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/api/rest.py b/api/rest.py
index e71bf0b..5610844 100644
--- a/api/rest.py
+++ b/api/rest.py
@@ -19,7 +19,7 @@ from .search import SearchEngine
 from rest_framework import (permissions, serializers, viewsets, filters,
     mixins, generics, renderers)
 from rest_framework.decorators import detail_route
-from rest_framework.fields import SerializerMethodField, CharField, JSONField
+from rest_framework.fields import SerializerMethodField, CharField, JSONField, EmailField
 from rest_framework.relations import HyperlinkedIdentityField
 from rest_framework.response import Response
 import rest_framework
@@ -118,6 +118,22 @@ class HyperlinkedMessageField(HyperlinkedIdentityField):
         kwargs = {'projects_pk': obj.project_id, self.lookup_field: obj.message_id}
         return self.reverse(view_name, kwargs=kwargs, request=request, format=format)
 
+class AddressSerializer(serializers.Serializer):
+    name = CharField(required=False)
+    address = EmailField()
+    def to_representation(self,obj):
+        if obj[0] != obj[1]:
+            return {"name":obj[0],"address":obj[1]}
+        else:
+            return {"address":obj[1]}
+
+    def create(self,validated_data):
+        try:
+            return [validated_data['name'],validated_data['address']]
+        except:
+            return [validated_data['address'],validated_data['address']]
+
+
 class BaseMessageSerializer(serializers.ModelSerializer):
     class Meta:
         model = Message
@@ -125,21 +141,8 @@ class BaseMessageSerializer(serializers.ModelSerializer):
 
     resource_uri = HyperlinkedMessageField(view_name='messages-detail')
 
-    recipients = SerializerMethodField()
-    sender = SerializerMethodField()
-
-    def format_name_addr(self, name, addr):
-        d = {}
-        if name != addr:
-            d['name'] = name
-        d['address'] = addr
-        return d
-
-    def get_recipients(self, obj):
-        return [self.format_name_addr(*x) for x in obj.recipients]
-
-    def get_sender(self, obj):
-        return self.format_name_addr(*obj.sender)
+    recipients = AddressSerializer(many=True)
+    sender = AddressSerializer()
 
 # a message_id is *not* unique, so we can only list
 class BaseMessageViewSet(mixins.ListModelMixin, viewsets.GenericViewSet):
@@ -280,7 +283,6 @@ class ProjectSeriesViewSet(ProjectMessagesViewSetMixin,
 # Messages
 
 # TODO: add POST endpoint connected to email plugin?
-
 class MessageSerializer(BaseMessageSerializer):
     class Meta:
         model = Message
diff --git a/tests/test_rest.py b/tests/test_rest.py
index d1fc80f..21bc2b9 100755
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -16,6 +16,8 @@ from django.contrib.auth.models import User
 sys.path.append(os.path.dirname(__file__))
 from patchewtest import PatchewTestCase, main
 from api.models import Message
+from api.rest import AddressSerializer
+from collections import OrderedDict
 
 class RestTest(PatchewTestCase):
     def setUp(self):
@@ -270,6 +272,21 @@ class RestTest(PatchewTestCase):
         resp = self.client.get(message + 'mbox/')
         self.assertEqual(resp.data, Message.objects.all()[0].get_mbox())
 
+    def test_address_serializer(self):
+        data1 = {"name":"Shubham", "address":"shubhamjain7495@gmail.com"}
+        serializer1 = AddressSerializer(data = data1)
+        valid1 = serializer1.is_valid()
+        valid_data1 = serializer1.validated_data
+        data2 = {"name":123, "address":"shubhamjain7495@gmail.com"}
+        serializer2 = AddressSerializer(data = data2)
+        valid2 = serializer2.is_valid()
+        valid_data2 = serializer2.validated_data
+
+        self.assertEqual(valid1,True)
+        self.assertEqual(valid_data1,OrderedDict([('name', 'Shubham'), ('address', 'shubhamjain7495@gmail.com')]))
+        self.assertEqual(valid2,True)
+        self.assertEqual(valid_data2,OrderedDict([('name', '123'), ('address', 'shubhamjain7495@gmail.com')]))
+
     def test_message_replies(self):
         series = self.apply_and_retrieve('0004-multiple-patch-reviewed.mbox.gz',
                                          self.p.id, '1469192015-16487-1-git-send-email-berrange@redhat.com')
-- 
2.14.3 (Apple Git-98)

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH v2] serializer: Add custom serialzer for recipients and sender
Posted by Paolo Bonzini 5 years, 11 months ago
On 30/04/2018 12:26, Shubham Jain wrote:
> - Earlier, recipients and sender were read only serializers and could not be used for POST/PUT/PATCH requests. Change it to custome serializer for POST request at api/v1/messages/
> - Add test corresponding to it

Thanks, this looks good.  I'm running tests and should push it soon if
they pass.

The only comment I have is that usually a space is left after "," and
":" characters, so that the code looks a bit less crammed.

The next step is changing the handling of mbox as mentioned in the
GitHub issue.

Paolo

>  api/rest.py        | 36 +++++++++++++++++++-----------------
>  tests/test_rest.py | 17 +++++++++++++++++
>  2 files changed, 36 insertions(+), 17 deletions(-)
> 
> diff --git a/api/rest.py b/api/rest.py
> index e71bf0b..5610844 100644
> --- a/api/rest.py
> +++ b/api/rest.py
> @@ -19,7 +19,7 @@ from .search import SearchEngine
>  from rest_framework import (permissions, serializers, viewsets, filters,
>      mixins, generics, renderers)
>  from rest_framework.decorators import detail_route
> -from rest_framework.fields import SerializerMethodField, CharField, JSONField
> +from rest_framework.fields import SerializerMethodField, CharField, JSONField, EmailField
>  from rest_framework.relations import HyperlinkedIdentityField
>  from rest_framework.response import Response
>  import rest_framework
> @@ -118,6 +118,22 @@ class HyperlinkedMessageField(HyperlinkedIdentityField):
>          kwargs = {'projects_pk': obj.project_id, self.lookup_field: obj.message_id}
>          return self.reverse(view_name, kwargs=kwargs, request=request, format=format)
>  
> +class AddressSerializer(serializers.Serializer):
> +    name = CharField(required=False)
> +    address = EmailField()
> +    def to_representation(self,obj):
> +        if obj[0] != obj[1]:
> +            return {"name":obj[0],"address":obj[1]}
> +        else:
> +            return {"address":obj[1]}
> +
> +    def create(self,validated_data):
> +        try:
> +            return [validated_data['name'],validated_data['address']]
> +        except:
> +            return [validated_data['address'],validated_data['address']]
> +
> +
>  class BaseMessageSerializer(serializers.ModelSerializer):
>      class Meta:
>          model = Message
> @@ -125,21 +141,8 @@ class BaseMessageSerializer(serializers.ModelSerializer):
>  
>      resource_uri = HyperlinkedMessageField(view_name='messages-detail')
>  
> -    recipients = SerializerMethodField()
> -    sender = SerializerMethodField()
> -
> -    def format_name_addr(self, name, addr):
> -        d = {}
> -        if name != addr:
> -            d['name'] = name
> -        d['address'] = addr
> -        return d
> -
> -    def get_recipients(self, obj):
> -        return [self.format_name_addr(*x) for x in obj.recipients]
> -
> -    def get_sender(self, obj):
> -        return self.format_name_addr(*obj.sender)
> +    recipients = AddressSerializer(many=True)
> +    sender = AddressSerializer()
>  
>  # a message_id is *not* unique, so we can only list
>  class BaseMessageViewSet(mixins.ListModelMixin, viewsets.GenericViewSet):
> @@ -280,7 +283,6 @@ class ProjectSeriesViewSet(ProjectMessagesViewSetMixin,
>  # Messages
>  
>  # TODO: add POST endpoint connected to email plugin?
> -
>  class MessageSerializer(BaseMessageSerializer):
>      class Meta:
>          model = Message
> diff --git a/tests/test_rest.py b/tests/test_rest.py
> index d1fc80f..21bc2b9 100755
> --- a/tests/test_rest.py
> +++ b/tests/test_rest.py
> @@ -16,6 +16,8 @@ from django.contrib.auth.models import User
>  sys.path.append(os.path.dirname(__file__))
>  from patchewtest import PatchewTestCase, main
>  from api.models import Message
> +from api.rest import AddressSerializer
> +from collections import OrderedDict
>  
>  class RestTest(PatchewTestCase):
>      def setUp(self):
> @@ -270,6 +272,21 @@ class RestTest(PatchewTestCase):
>          resp = self.client.get(message + 'mbox/')
>          self.assertEqual(resp.data, Message.objects.all()[0].get_mbox())
>  
> +    def test_address_serializer(self):
> +        data1 = {"name":"Shubham", "address":"shubhamjain7495@gmail.com"}
> +        serializer1 = AddressSerializer(data = data1)
> +        valid1 = serializer1.is_valid()
> +        valid_data1 = serializer1.validated_data
> +        data2 = {"name":123, "address":"shubhamjain7495@gmail.com"}
> +        serializer2 = AddressSerializer(data = data2)
> +        valid2 = serializer2.is_valid()
> +        valid_data2 = serializer2.validated_data
> +
> +        self.assertEqual(valid1,True)
> +        self.assertEqual(valid_data1,OrderedDict([('name', 'Shubham'), ('address', 'shubhamjain7495@gmail.com')]))
> +        self.assertEqual(valid2,True)
> +        self.assertEqual(valid_data2,OrderedDict([('name', '123'), ('address', 'shubhamjain7495@gmail.com')]))
> +
>      def test_message_replies(self):
>          series = self.apply_and_retrieve('0004-multiple-patch-reviewed.mbox.gz',
>                                           self.p.id, '1469192015-16487-1-git-send-email-berrange@redhat.com')
> 

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH v2] serializer: Add custom serialzer for recipients and sender
Posted by Shubham Jain 5 years, 11 months ago
I've found the error. Apparently In BaseMessageSerializer, sender and
recipient are in the form of  OrderedDict([('name', 'Andrey Smirnov'),
('address', 'andrew.smirnov@gmail.com')]) instead of ['Andrey Smirnov','
andrew.smirnov@gmail.com']. I'm not sure what is happening here. Is create
of AddressSerializer not getting called?

On Mon, Apr 30, 2018 at 4:59 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 30/04/2018 12:26, Shubham Jain wrote:
> > - Earlier, recipients and sender were read only serializers and could
> not be used for POST/PUT/PATCH requests. Change it to custome serializer
> for POST request at api/v1/messages/
> > - Add test corresponding to it
>
> Thanks, this looks good.  I'm running tests and should push it soon if
> they pass.
>
> The only comment I have is that usually a space is left after "," and
> ":" characters, so that the code looks a bit less crammed.
>
> The next step is changing the handling of mbox as mentioned in the
> GitHub issue.
>
> Paolo
>
> >  api/rest.py        | 36 +++++++++++++++++++-----------------
> >  tests/test_rest.py | 17 +++++++++++++++++
> >  2 files changed, 36 insertions(+), 17 deletions(-)
> >
> > diff --git a/api/rest.py b/api/rest.py
> > index e71bf0b..5610844 100644
> > --- a/api/rest.py
> > +++ b/api/rest.py
> > @@ -19,7 +19,7 @@ from .search import SearchEngine
> >  from rest_framework import (permissions, serializers, viewsets, filters,
> >      mixins, generics, renderers)
> >  from rest_framework.decorators import detail_route
> > -from rest_framework.fields import SerializerMethodField, CharField,
> JSONField
> > +from rest_framework.fields import SerializerMethodField, CharField,
> JSONField, EmailField
> >  from rest_framework.relations import HyperlinkedIdentityField
> >  from rest_framework.response import Response
> >  import rest_framework
> > @@ -118,6 +118,22 @@ class
> HyperlinkedMessageField(HyperlinkedIdentityField):
> >          kwargs = {'projects_pk': obj.project_id, self.lookup_field:
> obj.message_id}
> >          return self.reverse(view_name, kwargs=kwargs, request=request,
> format=format)
> >
> > +class AddressSerializer(serializers.Serializer):
> > +    name = CharField(required=False)
> > +    address = EmailField()
> > +    def to_representation(self,obj):
> > +        if obj[0] != obj[1]:
> > +            return {"name":obj[0],"address":obj[1]}
> > +        else:
> > +            return {"address":obj[1]}
> > +
> > +    def create(self,validated_data):
> > +        try:
> > +            return [validated_data['name'],validated_data['address']]
> > +        except:
> > +            return [validated_data['address'],validated_data['address']]
> > +
> > +
> >  class BaseMessageSerializer(serializers.ModelSerializer):
> >      class Meta:
> >          model = Message
> > @@ -125,21 +141,8 @@ class
> BaseMessageSerializer(serializers.ModelSerializer):
> >
> >      resource_uri = HyperlinkedMessageField(view_name='messages-detail')
> >
> > -    recipients = SerializerMethodField()
> > -    sender = SerializerMethodField()
> > -
> > -    def format_name_addr(self, name, addr):
> > -        d = {}
> > -        if name != addr:
> > -            d['name'] = name
> > -        d['address'] = addr
> > -        return d
> > -
> > -    def get_recipients(self, obj):
> > -        return [self.format_name_addr(*x) for x in obj.recipients]
> > -
> > -    def get_sender(self, obj):
> > -        return self.format_name_addr(*obj.sender)
> > +    recipients = AddressSerializer(many=True)
> > +    sender = AddressSerializer()
> >
> >  # a message_id is *not* unique, so we can only list
> >  class BaseMessageViewSet(mixins.ListModelMixin,
> viewsets.GenericViewSet):
> > @@ -280,7 +283,6 @@ class
> ProjectSeriesViewSet(ProjectMessagesViewSetMixin,
> >  # Messages
> >
> >  # TODO: add POST endpoint connected to email plugin?
> > -
> >  class MessageSerializer(BaseMessageSerializer):
> >      class Meta:
> >          model = Message
> > diff --git a/tests/test_rest.py b/tests/test_rest.py
> > index d1fc80f..21bc2b9 100755
> > --- a/tests/test_rest.py
> > +++ b/tests/test_rest.py
> > @@ -16,6 +16,8 @@ from django.contrib.auth.models import User
> >  sys.path.append(os.path.dirname(__file__))
> >  from patchewtest import PatchewTestCase, main
> >  from api.models import Message
> > +from api.rest import AddressSerializer
> > +from collections import OrderedDict
> >
> >  class RestTest(PatchewTestCase):
> >      def setUp(self):
> > @@ -270,6 +272,21 @@ class RestTest(PatchewTestCase):
> >          resp = self.client.get(message + 'mbox/')
> >          self.assertEqual(resp.data, Message.objects.all()[0].get_mbox())
> >
> > +    def test_address_serializer(self):
> > +        data1 = {"name":"Shubham", "address":"shubhamjain7495@gmail.com
> "}
> > +        serializer1 = AddressSerializer(data = data1)
> > +        valid1 = serializer1.is_valid()
> > +        valid_data1 = serializer1.validated_data
> > +        data2 = {"name":123, "address":"shubhamjain7495@gmail.com"}
> > +        serializer2 = AddressSerializer(data = data2)
> > +        valid2 = serializer2.is_valid()
> > +        valid_data2 = serializer2.validated_data
> > +
> > +        self.assertEqual(valid1,True)
> > +        self.assertEqual(valid_data1,OrderedDict([('name', 'Shubham'),
> ('address', 'shubhamjain7495@gmail.com')]))
> > +        self.assertEqual(valid2,True)
> > +        self.assertEqual(valid_data2,OrderedDict([('name', '123'),
> ('address', 'shubhamjain7495@gmail.com')]))
> > +
> >      def test_message_replies(self):
> >          series =
> self.apply_and_retrieve('0004-multiple-patch-reviewed.mbox.gz',
> >                                           self.p.id, '
> 1469192015-16487-1-git-send-email-berrange@redhat.com')
> >
>
>
_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH v2] serializer: Add custom serialzer for recipients and sender
Posted by Paolo Bonzini 5 years, 11 months ago

----- Original Message -----
> From: "Shubham Jain" <shubhamjain7495@gmail.com>
> To: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: patchew-devel@redhat.com
> Sent: Friday, May 4, 2018 8:47:44 AM
> Subject: Re: [Patchew-devel] [PATCH v2] serializer: Add custom serialzer for recipients and sender
> 
> I've found the error. Apparently In BaseMessageSerializer, sender and
> recipient are in the form of  OrderedDict([('name', 'Andrey Smirnov'),
> ('address', 'andrew.smirnov@gmail.com')]) instead of ['Andrey Smirnov','
> andrew.smirnov@gmail.com']. I'm not sure what is happening here. Is create
> of AddressSerializer not getting called?

I guess this is the "nested writable serializer" issue, and then you
have to call "create" yourself.  Following the example in the DRF
manual, it would be something like

    def create(self, validated_data):
        validated_data['sender'] = self.sender.create(**validated_data['sender'])
        ... same for recipients ...
        # Override get_serializer_context in ProjectMessagesViewSetMixin,
        # so that the project is passed down to the serializer
        # (see https://stackoverflow.com/questions/31038742/)
        # The new create method on MessageManager (as discussed on IRC)
        # saves the mbox and creates the object
        return Message.objects.create(project=self.context['project'],
                                      **validated_data)

Which of the tasks in "Convert `import` to REST style POST"
(https://github.com/patchew-project/patchew/issues/75) are you working
on now?

Thanks,

Paolo

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH v2] serializer: Add custom serialzer for recipients and sender
Posted by Shubham Jain 5 years, 11 months ago
I was working on the create Mixin part. But I'll first work on this
serialiser part and as well as setters and getters of mbox (according to
the previous review)

On Fri, May 4, 2018 at 1:02 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

>
>
> ----- Original Message -----
> > From: "Shubham Jain" <shubhamjain7495@gmail.com>
> > To: "Paolo Bonzini" <pbonzini@redhat.com>
> > Cc: patchew-devel@redhat.com
> > Sent: Friday, May 4, 2018 8:47:44 AM
> > Subject: Re: [Patchew-devel] [PATCH v2] serializer: Add custom serialzer
> for recipients and sender
> >
> > I've found the error. Apparently In BaseMessageSerializer, sender and
> > recipient are in the form of  OrderedDict([('name', 'Andrey Smirnov'),
> > ('address', 'andrew.smirnov@gmail.com')]) instead of ['Andrey Smirnov','
> > andrew.smirnov@gmail.com']. I'm not sure what is happening here. Is
> create
> > of AddressSerializer not getting called?
>
> I guess this is the "nested writable serializer" issue, and then you
> have to call "create" yourself.  Following the example in the DRF
> manual, it would be something like
>
>     def create(self, validated_data):
>         validated_data['sender'] =
> self.sender.create(**validated_data['sender'])
>         ... same for recipients ...
>         # Override get_serializer_context in ProjectMessagesViewSetMixin,
>         # so that the project is passed down to the serializer
>         # (see https://stackoverflow.com/questions/31038742/)
>         # The new create method on MessageManager (as discussed on IRC)
>         # saves the mbox and creates the object
>         return Message.objects.create(project=self.context['project'],
>                                       **validated_data)
>
> Which of the tasks in "Convert `import` to REST style POST"
> (https://github.com/patchew-project/patchew/issues/75) are you working
> on now?
>
> Thanks,
>
> Paolo
>
_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH v2] serializer: Add custom serialzer for recipients and sender
Posted by Paolo Bonzini 5 years, 11 months ago

----- Original Message -----
> From: "Shubham Jain" <shubhamjain7495@gmail.com>
> To: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: patchew-devel@redhat.com
> Sent: Friday, May 4, 2018 11:13:22 AM
> Subject: Re: [Patchew-devel] [PATCH v2] serializer: Add custom serialzer for recipients and sender
> 
> I was working on the create Mixin part. But I'll first work on this
> serialiser part and as well as setters and getters of mbox (according to
> the previous review)

Yeah, they are all related.  Note that "Add create method to MessageManager" is a subtask
of the CreateMixin, so technically it comes first. :)

Paolo

> On Fri, May 4, 2018 at 1:02 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> >
> >
> > ----- Original Message -----
> > > From: "Shubham Jain" <shubhamjain7495@gmail.com>
> > > To: "Paolo Bonzini" <pbonzini@redhat.com>
> > > Cc: patchew-devel@redhat.com
> > > Sent: Friday, May 4, 2018 8:47:44 AM
> > > Subject: Re: [Patchew-devel] [PATCH v2] serializer: Add custom serialzer
> > for recipients and sender
> > >
> > > I've found the error. Apparently In BaseMessageSerializer, sender and
> > > recipient are in the form of  OrderedDict([('name', 'Andrey Smirnov'),
> > > ('address', 'andrew.smirnov@gmail.com')]) instead of ['Andrey Smirnov','
> > > andrew.smirnov@gmail.com']. I'm not sure what is happening here. Is
> > create
> > > of AddressSerializer not getting called?
> >
> > I guess this is the "nested writable serializer" issue, and then you
> > have to call "create" yourself.  Following the example in the DRF
> > manual, it would be something like
> >
> >     def create(self, validated_data):
> >         validated_data['sender'] =
> > self.sender.create(**validated_data['sender'])
> >         ... same for recipients ...
> >         # Override get_serializer_context in ProjectMessagesViewSetMixin,
> >         # so that the project is passed down to the serializer
> >         # (see https://stackoverflow.com/questions/31038742/)
> >         # The new create method on MessageManager (as discussed on IRC)
> >         # saves the mbox and creates the object
> >         return Message.objects.create(project=self.context['project'],
> >                                       **validated_data)
> >
> > Which of the tasks in "Convert `import` to REST style POST"
> > (https://github.com/patchew-project/patchew/issues/75) are you working
> > on now?
> >
> > Thanks,
> >
> > Paolo
> >
> 

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel