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

allow to make comments for non-abstract models by COMMENTS_FOR_CONCRETE_MODEL setting #177

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions django_comments/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,20 +105,31 @@ class CommentDetailsForm(CommentSecurityForm):
comment = forms.CharField(label=_('Comment'), widget=forms.Textarea,
max_length=COMMENT_MAX_LENGTH)

def get_comment_object(self, site_id=None):
def get_comment_object(self, site_id=None, for_concrete_model=True):
"""
Return a new (unsaved) comment object based on the information in this
form. Assumes that the form is already validated and will throw a
ValueError if not.

Does not set any of the fields that would come from a Request object
(i.e. ``user`` or ``ip_address``).

:param for_concrete_model: To change behavior of a model specifically for `django-comments` app like
overriding `_get_pk_val()` to use non-pk UUID field as key for comments in URL.

Django by default returns non-proxy ancestor when determining content type of proxy model.
With `COMMENTS_FOR_CONCRETE_MODEL=False` the `django-comments` app will pass `for_concrete_model=False`
argument when determining content type via `ContentType.get_for_model()` method.
"""
if not self.is_valid():
raise ValueError("get_comment_object may only be called on valid forms")

CommentModel = self.get_comment_model()
new = CommentModel(**self.get_comment_create_data(site_id=site_id))
try:
create_data = self.get_comment_create_data(site_id=site_id, for_concrete_model=for_concrete_model)
except TypeError: # Fallback for overrides that doesn't count with more attributes
create_data = self.get_comment_create_data(site_id=site_id)
new = CommentModel(**create_data)
new = self.check_for_duplicate_comment(new)

return new
Expand All @@ -131,14 +142,17 @@ def get_comment_model(self):
"""
return get_model()

def get_comment_create_data(self, site_id=None):
def get_comment_create_data(self, site_id=None, for_concrete_model=True):
"""
Returns the dict of data to be used to create a comment. Subclasses in
custom comment apps that override get_comment_model can override this
method to add extra fields onto a custom comment model.
"""
return dict(
content_type=ContentType.objects.get_for_model(self.target_object),
content_type=ContentType.objects.get_for_model(
self.target_object,
for_concrete_model=for_concrete_model,
),
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if the new setting is the preferred way 🤔 What do you think about adding the for_concrete_model=True keyword argument to the get_comment_object() and get_comment_create_data()?

object_pk=force_str(self.target_object._get_pk_val()),
user_name=self.cleaned_data["name"],
user_email=self.cleaned_data["email"],
Expand Down
12 changes: 12 additions & 0 deletions tests/testapp/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"""

from django.db import models
import uuid


class Author(models.Model):
Expand All @@ -15,13 +16,24 @@ def __str__(self):


class Article(models.Model):
uuid = models.UUIDField(editable=False, null=True)
author = models.ForeignKey(Author, on_delete=models.CASCADE)
headline = models.CharField(max_length=100)

def __str__(self):
return self.headline


class UUIDArticle(Article):
""" Override _get_pk_val to use UUID as PK """

def _get_pk_val(self, meta=None):
return self.uuid

class Meta:
proxy = True


class Entry(models.Model):
title = models.CharField(max_length=250)
body = models.TextField()
Expand Down
37 changes: 36 additions & 1 deletion tests/testapp/tests/test_comment_form.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import time

from django.conf import settings
from django.contrib.contenttypes.models import ContentType
from django.contrib.sites.models import Site

from django_comments.forms import CommentForm
from django_comments.models import Comment

from . import CommentTestCase
from testapp.models import Article
from testapp.models import UUIDArticle, Article
from django.test.utils import override_settings


class CommentFormTests(CommentTestCase):
Expand Down Expand Up @@ -75,6 +77,39 @@ def testGetCommentObject(self):
c = f.get_comment_object(site_id=self.site_2.id)
self.assertEqual(c.site_id, self.site_2.id)

def testGetCommentCreateData(self):
"""
Test that get_comment_create_data() returns
content_type for Article even if the proxy model UUIDArticle
is set to CommentForm.
"""
a = UUIDArticle.objects.get(pk=1)
d = self.getValidData(a)
d["comment"] = "testGetCommentObject with a site"
f = CommentForm(UUIDArticle.objects.get(pk=1), data=d)
self.assertTrue(f.is_valid())
c = f.get_comment_create_data(site_id=self.site_2.id)
self.assertEqual(c["content_type"], ContentType.objects.get_for_model(Article, for_concrete_model=False))

o = f.get_comment_object(site_id=self.site_2.id)
self.assertEqual(o.content_type, ContentType.objects.get_for_model(Article, for_concrete_model=False))

def testGetCommentCreateDataConcreteModel(self):
"""
Test that get_comment_create_data() returns
content_type for UUIDArticle if COMMENTS_FOR_CONCRETE_MODEL is False.
"""
a = UUIDArticle.objects.get(pk=1)
d = self.getValidData(a)
d["comment"] = "testGetCommentObject with a site"
f = CommentForm(UUIDArticle.objects.get(pk=1), data=d)
self.assertTrue(f.is_valid())
c = f.get_comment_create_data(site_id=self.site_2.id, for_concrete_model=False)
self.assertEqual(c["content_type"], ContentType.objects.get_for_model(UUIDArticle, for_concrete_model=False))

o = f.get_comment_object(site_id=self.site_2.id, for_concrete_model=False)
self.assertEqual(o.content_type, ContentType.objects.get_for_model(UUIDArticle, for_concrete_model=False))

def testProfanities(self):
"""Test COMMENTS_ALLOW_PROFANITIES and PROFANITIES_LIST settings"""
a = Article.objects.get(pk=1)
Expand Down