Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatic Inclusion of @Property Fields in ModelSerializer #9414

Closed
wants to merge 2 commits into from

Conversation

irshadirshu0722
Copy link

Overview

Backend developers often face the challenge of manually adding @Property fields to serializers using SerializerMethodField. This process can be repetitive and error-prone. To simplify and expedite the inclusion of @Property fields, we've introduced a new feature in the ModelSerializer that allows you to automatically include these fields with minimal configuration.

Feature Description

With this new feature, you can easily include @Property fields in your ModelSerializer by setting the property_fields attribute in the serializer's Meta class. You have the option to include all @Property fields or manually specify which ones to include.

Usage

  1. Include All @Property Fields:
    Set property_fields to all in the Meta class to automatically include all @Property fields.

    class MyModelSerializer(ModelSerializer):
         class Meta:
              model = MyModel
              fields = '_all_'
              property_fields = '_all_'
    
  2. Manually Specify @Property Fields:
    Provide a list or tuple of @Property field names to include only specific fields.

    class MyModelSerializer(ModelSerializer):
         class Meta:
              model = MyModel
              fields = '_all_'
              property_fields = ('property_field1', 'property_field2')
    

Implementation

The feature is implemented in the get_fields method of the ModelSerializer. Here’s the relevant code:

        def get_fields(self):
            ...
         property_fields = getattr(self.Meta, 'property_fields', ())
         if property_fields:
             if property_fields == '_all_':
                 for attr_name in dir(model):
                     attr = getattr(model, attr_name)
                     if isinstance(attr, property):
                         fields[attr_name] = serializers.ReadOnlyField()
             elif isinstance(property_fields, list) or isinstance(property_fields, tuple):
                 for attr_name in property_fields:
                     attr = getattr(model, attr_name, None)
                     if not attr:
                         raise ValueError(f"{attr_name} doesn't exist in {model} model properties")
                     if isinstance(attr, property):
                         fields[attr_name] = serializers.ReadOnlyField()
             else:
                 raise ValueError(
                     "Please select the appropriate value for property_fields in the serializer. "
                     "Use '_all_' to include all property fields, or provide a tuple or list to manually specify the property fields you want to include."
                 )
         ...

Benefits

  • Ease of Use: No need to manually define each @Property field using SerializerMethodField.
  • Time-Saving: Quickly include all or specific @Property fields in the serializer.
  • Error Reduction: Reduces the risk of typos and other errors when adding @Property fields manually.

#Conclusion
This feature significantly enhances the efficiency of working with @Property fields in Django REST Framework serializers. By setting a single attribute in the Meta class, developers can now easily include these fields, making their code cleaner and more maintainable.

Feel free to use this feature in your projects and contribute any improvements or suggestions. Happy coding!

@irshadirshu0722
Copy link
Author

@cclauss Could you please review this pull request?

Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DRF is net negetive in introducing new API surface, unless it is needed for Python or Django compatibility. so I am not sure if the other maintainers will accept this change. it would be nice you could share some example of true use cases that are used by most of the devs? it might be a good fit for some DRF extensions IMHO if the use case is very widely used.

@irshadirshu0722
Copy link
Author

irshadirshu0722 commented May 29, 2024

@cclauss @auvipy Yes sure , I can share the exact use case and how this feature reduce the line of code below the example of that

Consider i have model in django for storing complaint of users and the model look like this

   class Complaint(models.Model):
     PROBLEM_RATE_CHOICES = [(str(i), str(i)) for i in range(1, 11)]
     STATUS_CHOICES = [('pending','Pending'),('rejected','Rejected'),('approved','Approved')]
     ward = models.IntegerField()
     name = models.CharField(max_length=100)
     user = models.ForeignKey(User,on_delete=models.SET_NULL,related_name='complaints',null=True)
     subject = models.CharField(max_length=100)
     description = models.TextField()
     landmark = models.CharField(max_length=500)
     problem_rate = models.CharField(max_length=10,choices=PROBLEM_RATE_CHOICES)
     audio = CloudinaryField(resource_type='auto',folder='complaints/audio',null=True,blank=True)
     status = models.CharField(max_length=20,choices=STATUS_CHOICES,default='pending')
     _update_at = models.DateTimeField(null=True)
     _complaint_at = models.DateTimeField(auto_now_add=True,null=True)
     @property
     def update_at(self):
       if(self._update_at):
         return self._update_at.strftime("%d %b %Y") 
       else:
         return "---"
     @property
     def complaint_at(self):
       return self._complaint_at.strftime("%d %b %Y") 
     @property
     def audio_url(self):
         return self.audio.url

by analyzing this model schema , we can see that there are three 3 property fields exist . To get these property fields in representation we can do using these following method .note these are the current django framework feature

   class ComplaintSerializer(serializers.ModelSerializer):
         class Meta:
           model = Complaint
           fields = [
             'ward', 'name', 'user', 'subject', 'description', 
            'landmark', 'problem_rate', 'audio_url', 'status', 
            'update_at', 'complaint_at'
          ]

by using the above method we will get the property fields in representation .

if we are planning to use fields = "all" , writing fields name of model in serializer's fields variables it something wired if there are more fields in model . so with fields = 'all' we can do it by following method

   class ComplaintSerializer(serializers.ModelSerializer):
          audio_url = serializers.SerializerMethodField(read_only=True)
          complaint_at = serializers.SerializerMethodField(read_only=True)
          update_at = serializers.SerializerMethodField(read_only=True)
          class Meta:
                model = Complaint
                fields = '__all__'
         def get_audio_url(self,obj):
               return obj.audio_url
         def get_complaint_at(self,obj):
               return obj.complaint_at
         def get_update_at(self,obj):
               return obj.update_at

here we have to add that manually using serializer method field

by analyzing the two method we can see that adding the property fields taking more lines of code . To tackle this problem with adding property fields with single line we have to use new feature which i added . Using that feature we can add the property fields very easily and only 1 line is needed and it helps to reduce code line , complexity and reduce the error

Now see the difference between doing the same job with new feature

   class ComplaintSerializer(serializers.ModelSerializer):
         class Meta:
           model = Complaint
           fields = '__all__'
           property_fields = '__all__'   #or can add that manually using () or list like fields = ('audio_url',) 

Now you can see the change and effect of new feature in the scenario

I faced this issue when i building project and i asked about this issue to my friends developers they also respond me positively

I hope it clear well.

i am sure that it definitely help whole the developers 💯

Thank you

@irshadirshu0722
Copy link
Author

@auvipy can you please review it

@browniebroke
Copy link
Contributor

In my opinion, using __all__ is a bit of an anti-pattern. While it's nice for quick prototyping, but it's a dangerous hook where one risks to expose more than they intend to. So as a user I'm -1 on this.

@irshadirshu0722
Copy link
Author

In my opinion, using __all__ is a bit of an anti-pattern. While it's nice for quick prototyping, but it's a dangerous hook where one risks to expose more than they intend to. So as a user I'm -1 on this.

But we can use tuples or list to mention the property fields rather than all

This feature actually reducing the code line

Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to close this as design not accepted. you are welcome to contribute on other issues which do not need to expose new API surface, unless otherwise needed by new django or python versions.

@auvipy auvipy closed this Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants