search

django / django-contrib-comments

BSD 3-Clause "New" or "Revised" License
619 stars 197 forks source link

Multiple Db hits with authenticated users #48

Closed kakai248 closed 8 years ago

kakai248 commented 9 years ago

I've noticed that for each comment of an authenticated user, it will hit the db once to get the user.

We need a select_related('user') to stop this behavior.

I'm not very experienced with Django however, it seems that putting it in the get_queryset method of the BaseCommentNode class will fix the issue.

claudep commented 9 years ago

That might be a nice improvement.

ivanvenosdel commented 8 years ago

Implementing this is really the only way to solve this as the old solution won't work in Django 1.9.

The old way involved creating a custom comments app and then adding a related manager to your custom model that would do as you describe. Here is a broken (in 1.9) example.

Unfortunately this is no longer possible as 1.9 prevents importing models from another app (even indirectly) before all app models have been loaded.

In practice this means that contrib-comments custom app approach no longer works because of the way it has you serving up your model (which inherits from another model) in __init__.py. As that will get called before models are ready.

Edit: Updated to describe why contrib-comments can no longer allow for custom model. Edit2: A work around is still possible. Just not as easy as it used to be. I had to inherit from the abstract comment model instead of creating a proxy of the concrete version for reasons that I don't understand. https://github.com/WimpyAnalytics/django-andablog/blob/master/demo/democomments/models.py

claudep commented 8 years ago

It would be nice if some of you could test the patch.