chore: Reduce number of queries by prefetching foreign key objects on comment

[abhiabhi94]

• the fetched objects are user, reaction and flag.

[abhiabhi94]

The performance gain will increase with the number of comments. For example, on my local database, when viewing the API localhost:8000/api/comments/model_name=post&model_id=1&app_name=post, the number of queries went down from 143 to 109 when having a comment count of 17 on the database.

Stats courtesy django-debug-toolbar.

[abhiabhi94]

Hey @Radi85, how are you? just wanted to check whether everything is okay with you.

in case you don't have the time to review patches, maybe you can remove the review required restriction from the settings. This will allow me to merge pull-requests and continue the development of this project. It will also likely help me in making future releases.

[Radi85]

Hey @abhiabhi94,
I'm good thanks for asking. I have recently moved to another city and I'm busy with that. I will review your PRs ASAP and will be hopefully active as usual very soon.

[abhiabhi94]

Hey @Radi85, why was this removed in 877d4e6

diff --git a/comment/api/serializers.py b/comment/api/serializers.py 
index 5d5b29c..bc319dd 100644
--- a/comment/api/serializers.py
+++ b/comment/api/serializers.py

-    def __init__(self, *args, **kwargs):
-        user = kwargs['context']['request'].user
-        self.email_service = None
-        if user.is_authenticated or not settings.COMMENT_ALLOW_ANONYMOUS:
-            del self.fields['email']
-
-        super().__init__(*args, **kwargs)

Now, i see email field is present in the serializer even when an authenticated user tries to add a comment or react, etc., to a comment. Was this was removed inadvertently?

[Radi85]

Now, i see email field is present in the serializer even when an authenticated user tries to add a comment or react, etc., to a comment. Was this was removed inadvertently?

It was removed intentionally since the email validation won't happen if the user is authenticated. With the current code we can create comment using curl. I am not sure why the validation is occurring while using djangorestframework UI.

curl -X POST 'http://127.0.0.1:8001/api/comments/create/?app_name=post&model_name=post&model_id=1' -d '{"content": "test"}' -H "Content-Type: application/json" -u test:test

Response:

{"id":187,"user":{"username":"test","email":"test@test.com","id":3,"profile":{"display_name":null,"birth_date":null,"image":"/media/pic/default.png"}},"email":"test@test.com","content":"test","parent":null,"posted":"2021-07-20T06:06:19.429350Z","edited":"2021-07-20T06:06:30.284091Z","reply_count":0,"replies":[],"urlhash":"comment-ncoppzay"}

In the ideal world, the api won't be consumed using the restframework UI.

[abhiabhi94]

It was removed intentionally since the email validation won't happen if the user is authenticated

The code in contention isn't doing validation. It is just removing the field email in case the user is authenticated and the appropriate setting is turned on.

[Radi85]

The code in contention isn't doing validation. It is just removing the field email in case the user is authenticated and the appropriate setting is turned on.

For the restframework UI, when the email is presented on the serializer, the validation is happening. That means the email was deleted from the init method to skip the validation.

The above is not apply when using curl or fetch lib so INHO we don't need to delete the email field from the serializer.

[abhiabhi94]

I am not sure why the validation is occurring while using djangorestframework UI.

It is occurring because we have a method validate_email in the serializer and since we explicitly declare the field email in Meta.fields, that method is getting called. It is the one throwing the error.

When using something like curl for making a request, i will try to investigate more on this and inform.

In an ideal scenario, we probably shouldn't have to manually call the validate_email method, but there were some issue that was happening in the past. I will if that can be fixed.

[Radi85]

It is occurring because we have a method validate_email in the serializer and since we explicitly declare the field email in Meta.fields, that method is getting called. It is the one throwing the error.

As I said, this is happening when using the restframework UI but it does NOT when using external library unless we explicitly call validate_email

[abhiabhi94]

The issue is that if the API is being used to create a form for frontend development using the serializers passed from the API. They will have an extra email field.

[Radi85]

Thanks for your time :)