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

add interchangable model keys support #188

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
7 changes: 4 additions & 3 deletions django_comments/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from django.utils.translation import pgettext_lazy, ngettext, gettext, gettext_lazy as _

from . import get_model
from .utils import get_key_value

COMMENT_MAX_LENGTH = getattr(settings, 'COMMENT_MAX_LENGTH', 3000)
DEFAULT_COMMENTS_TIMEOUT = getattr(settings, 'COMMENTS_TIMEOUT', (2 * 60 * 60)) # 2h
Expand Down Expand Up @@ -65,7 +66,7 @@ def generate_security_data(self):
timestamp = int(time.time())
security_dict = {
'content_type': str(self.target_object._meta),
'object_pk': str(self.target_object._get_pk_val()),
'object_pk': get_key_value(self.target_object),
'timestamp': str(timestamp),
'security_hash': self.initial_security_hash(timestamp),
}
Expand All @@ -79,7 +80,7 @@ def initial_security_hash(self, timestamp):

initial_security_dict = {
'content_type': str(self.target_object._meta),
'object_pk': str(self.target_object._get_pk_val()),
'object_pk': get_key_value(self.target_object),
'timestamp': str(timestamp),
}
return self.generate_security_hash(**initial_security_dict)
Expand Down Expand Up @@ -139,7 +140,7 @@ def get_comment_create_data(self, site_id=None):
"""
return dict(
content_type=ContentType.objects.get_for_model(self.target_object),
object_pk=force_str(self.target_object._get_pk_val()),
object_pk=get_key_value(self.target_object),
user_name=self.cleaned_data["name"],
user_email=self.cleaned_data["email"],
user_url=self.cleaned_data["url"],
Expand Down
31 changes: 31 additions & 0 deletions django_comments/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
from django.conf import settings
from django.utils.encoding import force_str


def get_key(model):
"""
Get key of the model.

By default returns 'pk', but if COMMENTS_ID_OVERRIDES is defined,
returns the key defined by user.
"""
COMMENTS_ID_OVERRIDES = getattr(settings, 'COMMENTS_ID_OVERRIDES', {})
class_identifier = f"{model._meta.app_label}.{model.__name__}"
if class_identifier in COMMENTS_ID_OVERRIDES:
return COMMENTS_ID_OVERRIDES[class_identifier]
else:
return 'pk'


def get_key_value(target_object):
"""
Get key of the model.

By default returns 'pk', but if COMMENTS_ID_OVERRIDES is defined,
returns the key defined by user.
"""
key = get_key(target_object.__class__)
if key == 'pk':
return force_str(target_object._get_pk_val())
else:
return force_str(getattr(target_object, key))
3 changes: 2 additions & 1 deletion django_comments/views/comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import django_comments
from django_comments import signals
from django_comments.views.utils import next_redirect, confirmation_view
from django_comments.utils import get_key


class CommentPostBadRequest(http.HttpResponseBadRequest):
Expand Down Expand Up @@ -51,7 +52,7 @@ def post_comment(request, next=None, using=None):
return CommentPostBadRequest("Missing content_type or object_pk field.")
try:
model = apps.get_model(*ctype.split(".", 1))
target = model._default_manager.using(using).get(pk=object_pk)
target = model._default_manager.using(using).get(**{get_key(model): object_pk})
except (LookupError, TypeError):
return CommentPostBadRequest(
"Invalid content_type value: %r" % escape(ctype))
Expand Down
17 changes: 17 additions & 0 deletions docs/settings.txt
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,20 @@ COMMENT_TIMEOUT

The maximum comment form timeout in seconds. The default value is
``2 * 60 * 60`` (2 hours).


COMMENTS_ID_OVERRIDES
----------------------

.. setting:: COMMENTS_ID_OVERRIDES

A dictionary of ``{"app_label": "id_field"}`` pairs that override the
PK used for referencing comment objects.

If you want to disguise the plain IDs of your referenced model used by the comment form, you can
use uuid field as an ID for the model. You don't have to change the model to use different PK.
For example::

COMMENTS_ID_OVERRIDES = {
"myapp.MyModel": "uuid",
}
2 changes: 2 additions & 0 deletions tests/testapp/fixtures/comment_tests.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"pk" : 1,
"fields" : {
"author" : 1,
"uuid" : "336384ea-b04f-4a3a-a06a-1f25a8048f8f",
"headline" : "Man Bites Dog"
}
},
Expand All @@ -35,6 +36,7 @@
"pk" : 2,
"fields" : {
"author" : 2,
"uuid" : "d77c5d7d-1b0d-467b-814b-96697bd9a686",
"headline" : "Dog Bites Man"
}
},
Expand Down
1 change: 1 addition & 0 deletions tests/testapp/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ 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)

Expand Down
93 changes: 92 additions & 1 deletion tests/testapp/tests/test_comment_form.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
import time
from datetime import datetime

from django.conf import settings
from django.contrib.contenttypes.models import ContentType
from django.contrib.sites.models import Site
from django.test.utils import override_settings
from freezegun import freeze_time
from testapp.models import Article

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

from . import CommentTestCase
from testapp.models import Article

CT = ContentType.objects.get_for_model

class CommentFormTests(CommentTestCase):

Expand Down Expand Up @@ -75,6 +80,92 @@ def testGetCommentObject(self):
c = f.get_comment_object(site_id=self.site_2.id)
self.assertEqual(c.site_id, self.site_2.id)

@freeze_time("2012-01-14 13:21:34")
def test_get_comment_create_data_uuid(self):
"""
The get_comment_create_data() method returns
uuid field as object_pk if overriden by settings
"""
a = Article.objects.get(pk=1)
d = self.getValidData(a)
d["comment"] = "testGetCommentObject with a site"
f = CommentForm(Article.objects.get(pk=1), data=d)
self.assertTrue(f.is_valid())
with override_settings(
COMMENTS_ID_OVERRIDES={
"testapp.Article": "uuid",
}
):
c = f.get_comment_create_data(site_id=self.site_2.id)
self.assertDictEqual(
c,
{
"comment": "testGetCommentObject with a site",
"content_type": CT(Article),
"is_public": True,
"is_removed": False,
"object_pk": "336384ea-b04f-4a3a-a06a-1f25a8048f8f", # uuid is returned
"site_id": 2,
"submit_date": datetime(2012, 1, 14, 13, 21, 34),
"user_email": "[email protected]",
"user_name": "Jim Bob",
"user_url": "",
},
)
c = f.get_comment_create_data(site_id=self.site_2.id)
self.assertDictEqual(
c,
{
"comment": "testGetCommentObject with a site",
"content_type": CT(Article),
"is_public": True,
"is_removed": False,
"object_pk": "1", # pk is returned as object_pk
"site_id": 2,
"submit_date": datetime(2012, 1, 14, 13, 21, 34),
"user_email": "[email protected]",
"user_name": "Jim Bob",
"user_url": "",
},
)

@freeze_time("2012-01-14 13:21:34")
def test_generate_security_data_uuid(self):
"""
The generate_security_data() method returns
uuid field as object_pk if overriden by settings
"""
a = Article.objects.get(pk=1)
d = self.getValidData(a)
d["comment"] = "testGetCommentObject with a site"
f = CommentForm(Article.objects.get(pk=1), data=d)
self.assertTrue(f.is_valid())
with override_settings(
COMMENTS_ID_OVERRIDES={
"testapp.Article": "uuid",
}
):
c = f.generate_security_data()
self.assertDictEqual(
c,
{
"content_type": "testapp.article",
"object_pk": "336384ea-b04f-4a3a-a06a-1f25a8048f8f",
"security_hash": "b89ebc7c1c6ed757991fa06027405aecdf8d51f1",
"timestamp": "1326547294",
},
)
c = f.generate_security_data()
self.assertDictEqual(
c,
{
"content_type": "testapp.article",
"object_pk": "1",
"security_hash": "2f4a55f47e58b22791d4d26f3b1a2e594302fb61",
"timestamp": "1326547294",
},
)

def testProfanities(self):
"""Test COMMENTS_ALLOW_PROFANITIES and PROFANITIES_LIST settings"""
a = Article.objects.get(pk=1)
Expand Down
23 changes: 23 additions & 0 deletions tests/testapp/tests/test_comment_views.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from django.conf import settings
from django.contrib.auth.models import User
from django.test.utils import override_settings

from django_comments import signals
from django_comments.abstracts import COMMENT_MAX_LENGTH
Expand Down Expand Up @@ -333,6 +334,28 @@ def testCommentPostRedirectWithInvalidIntegerPK(self):
response = self.client.get(broken_location)
self.assertEqual(response.status_code, 200)

@override_settings(
COMMENTS_ID_OVERRIDES={
"testapp.Article": "uuid",
}
)
def testCommentPostWithUUID(self):
"""
Tests that attempting to retrieve the location specified in the
post redirect, after adding some invalid data to the expected
querystring it ends with, doesn't cause a server error.
"""
a = Article.objects.get(pk=1)
data = self.getValidData(a)
data["comment"] = "This is another comment"
self.assertEqual(data["object_pk"], "336384ea-b04f-4a3a-a06a-1f25a8048f8f")
response = self.client.post("/post/", data)
self.assertEqual(response.status_code, 302)
self.assertEqual(Comment.objects.count(), 1)
self.assertEqual(
Comment.objects.first().object_pk, "336384ea-b04f-4a3a-a06a-1f25a8048f8f"
)

def testCommentNextWithQueryStringAndAnchor(self):
"""
The `next` key needs to handle already having an anchor. Refs #13411.
Expand Down
1 change: 1 addition & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ deps=
django-40: Django>=4.0a1,<4.1
django-41: Django>=4.1a1,<4.2
django-main: https://github.com/django/django/archive/main.tar.gz
freezegun