Skip to content

Commit

Permalink
Fixed django#373 - Added CompositePrimaryKey.
Browse files Browse the repository at this point in the history
  • Loading branch information
csirmazbendeguz committed Aug 21, 2024
1 parent e30684e commit 22c80e7
Show file tree
Hide file tree
Showing 8 changed files with 168 additions and 11 deletions.
13 changes: 8 additions & 5 deletions django/contrib/admin/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -1808,7 +1808,7 @@ def _get_obj_does_not_exist_redirect(self, request, opts, object_id):
"""
msg = _("%(name)s with ID “%(key)s” doesn’t exist. Perhaps it was deleted?") % {
"name": opts.verbose_name,
"key": unquote(object_id),
"key": self.unquote(object_id),
}
self.message_user(request, msg, messages.WARNING)
url = reverse("admin:index", current_app=self.admin_site.name)
Expand Down Expand Up @@ -1840,7 +1840,7 @@ def _changeform_view(self, request, object_id, form_url, extra_context):
obj = None

else:
obj = self.get_object(request, unquote(object_id), to_field)
obj = self.get_object(request, self.unquote(object_id), to_field)

if request.method == "POST":
if not self.has_change_permission(request, obj):
Expand Down Expand Up @@ -2196,7 +2196,7 @@ def _delete_view(self, request, object_id, extra_context):
"The field %s cannot be referenced." % to_field
)

obj = self.get_object(request, unquote(object_id), to_field)
obj = self.get_object(request, self.unquote(object_id), to_field)

if not self.has_delete_permission(request, obj):
raise PermissionDenied
Expand Down Expand Up @@ -2258,7 +2258,7 @@ def history_view(self, request, object_id, extra_context=None):

# First check if the user can see this history.
model = self.model
obj = self.get_object(request, unquote(object_id))
obj = self.get_object(request, self.unquote(object_id))
if obj is None:
return self._get_obj_does_not_exist_redirect(
request, model._meta, object_id
Expand All @@ -2271,7 +2271,7 @@ def history_view(self, request, object_id, extra_context=None):
app_label = self.opts.app_label
action_list = (
LogEntry.objects.filter(
object_id=unquote(object_id),
object_id=self.unquote(object_id),
content_type=get_content_type_for_model(model),
)
.select_related()
Expand Down Expand Up @@ -2362,6 +2362,9 @@ def user_deleted_form(request, obj, formset, index, inline):
inline_instances.append(inline)
return formsets, inline_instances

def unquote(self, pk):
return unquote(pk, is_composite=self.opts.is_composite_pk())


class InlineModelAdmin(BaseModelAdmin):
"""
Expand Down
15 changes: 12 additions & 3 deletions django/contrib/admin/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
QUOTE_MAP = {i: "_%02X" % i for i in b'":/_#?;@&=+$,"[]<>%\n\\'}
UNQUOTE_MAP = {v: chr(k) for k, v in QUOTE_MAP.items()}
UNQUOTE_RE = _lazy_re_compile("_(?:%s)" % "|".join([x[1:] for x in UNQUOTE_MAP]))
PK_SEP = ","


class FieldIsAForeignKeyColumnName(Exception):
Expand Down Expand Up @@ -91,12 +92,20 @@ def quote(s):
Similar to urllib.parse.quote(), except that the quoting is slightly
different so that it doesn't get automatically unquoted by the web browser.
"""
return s.translate(QUOTE_MAP) if isinstance(s, str) else s
if isinstance(s, str):
return s.translate(QUOTE_MAP)
elif isinstance(s, tuple):
return PK_SEP.join(str(quote(f)) for f in s)
else:
return s


def unquote(s):
def unquote(s, is_composite=False):
"""Undo the effects of quote()."""
return UNQUOTE_RE.sub(lambda m: UNQUOTE_MAP[m[0]], s)
if is_composite:
return tuple(unquote(f) for f in s.split(PK_SEP))
else:
return UNQUOTE_RE.sub(lambda m: UNQUOTE_MAP[m[0]], s)


def flatten(fields):
Expand Down
3 changes: 1 addition & 2 deletions django/contrib/auth/admin.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from django.conf import settings
from django.contrib import admin, messages
from django.contrib.admin.options import IS_POPUP_VAR
from django.contrib.admin.utils import unquote
from django.contrib.auth import update_session_auth_hash
from django.contrib.auth.forms import (
AdminPasswordChangeForm,
Expand Down Expand Up @@ -153,7 +152,7 @@ def _add_view(self, request, form_url="", extra_context=None):

@sensitive_post_parameters_m
def user_change_password(self, request, id, form_url=""):
user = self.get_object(request, unquote(id))
user = self.get_object(request, self.unquote(id))
if not self.has_change_permission(request, user):
raise PermissionDenied
if user is None:
Expand Down
5 changes: 5 additions & 0 deletions django/db/models/expressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1374,6 +1374,11 @@ def relabeled_clone(self, relabels):
def resolve_expression(self, *args, **kwargs):
return self

@staticmethod
def db_converter(value, *_):
assert isinstance(value, list)
return (tuple(value),)


class Ref(Expression):
"""
Expand Down
34 changes: 33 additions & 1 deletion tests/admin_utils/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
label_for_field,
lookup_field,
quote,
unquote,
)
from django.core.validators import EMPTY_VALUES
from django.db import DEFAULT_DB_ALIAS, models
Expand Down Expand Up @@ -436,7 +437,38 @@ def test_flatten_fieldsets(self):
)

def test_quote(self):
self.assertEqual(quote("something\nor\nother"), "something_0Aor_0Aother")
test_cases = (
("something\nor\nother", "something_0Aor_0Aother"),
("f,o,o", "f_2Co_2Co"),
("b-a-r", "b-a-r"),
((), ""),
((1, 2), "1,2"),
((3, "f,o,o"), "3,f_2Co_2Co"),
((4, "b-a-r"), "4,b-a-r"),
)

for s, expected in test_cases:
with self.subTest(s=s, expected=expected):
self.assertEqual(quote(s), expected)

def test_unquote(self):
test_cases = (
("something_0Aor_0Aother", False, "something\nor\nother"),
("f_2Co_2Co", False, "f,o,o"),
("b-a-r", False, "b-a-r"),
("", False, ""),
("", True, ("",)),
("1,2,3", False, "1,2,3"),
("1,2,3", True, ("1", "2", "3")),
("3,f_2Co_2Co", False, "3,f,o,o"),
("3,f_2Co_2Co", True, ("3", "f,o,o")),
("4,b-a-r", False, "4,b-a-r"),
("4,b-a-r", True, ("4", "b-a-r")),
)

for s, is_composite, expected in test_cases:
with self.subTest(s=s, is_composite=is_composite, expected=expected):
self.assertEqual(unquote(s, is_composite=is_composite), expected)

def test_build_q_object_from_lookup_parameters(self):
parameters = {
Expand Down
4 changes: 4 additions & 0 deletions tests/admin_views/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@
FieldOverridePost,
FilteredManager,
FooAccount,
FooBarCompositePK,
FooCompositePK,
FoodDelivery,
FunkyTag,
Gadget,
Expand Down Expand Up @@ -1199,6 +1201,8 @@ class CamelCaseAdmin(admin.ModelAdmin):
search_fields=["name"],
)
site.register(ModelWithStringPrimaryKey)
site.register(FooBarCompositePK)
site.register(FooCompositePK)
site.register(Color)
site.register(Thing, ThingAdmin)
site.register(Actor)
Expand Down
11 changes: 11 additions & 0 deletions tests/admin_views/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,17 @@ def get_absolute_url(self):
return "/dummy/%s/" % self.string_pk


class FooBarCompositePK(models.Model):
pk = models.CompositePrimaryKey("foo", "bar")
foo = models.CharField(max_length=10)
bar = models.CharField(max_length=10)


class FooCompositePK(models.Model):
pk = models.CompositePrimaryKey("foo")
foo = models.CharField(max_length=10)


class Color(models.Model):
value = models.CharField(max_length=10)
warm = models.BooleanField(default=False)
Expand Down
94 changes: 94 additions & 0 deletions tests/admin_views/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@
FieldOverridePost,
FilteredManager,
FooAccount,
FooBarCompositePK,
FooCompositePK,
FoodDelivery,
FunkyTag,
Gallery,
Expand Down Expand Up @@ -4050,6 +4052,98 @@ def test_redirect_on_add_view_continue_button(self):
self.assertIn("/123_2Fhistory/", response.headers["location"]) # PK is quoted


@override_settings(ROOT_URLCONF="admin_views.urls")
class AdminViewCompositePKTests(TestCase):
FOOBAR = "foobarcompositepk"
FOO = "foocompositepk"
CHANGE_VIEW = "admin:admin_views_%s_change"
HISTORY_VIEW = "admin:admin_views_%s_history"
DELETE_VIEW = "admin:admin_views_%s_delete"

@classmethod
def setUpTestData(cls):
cls.foobar = FooBarCompositePK.objects.create(foo="f,o,o", bar="b-a-r")
cls.foo = FooCompositePK.objects.create(foo="f,o,o")
cls.superuser = User.objects.create_superuser(
username="super", password="secret", email="super@example.com"
)

def setUp(self):
self.client.force_login(self.superuser)

def test_foobar_history_view(self):
viewname = self.HISTORY_VIEW % (self.FOOBAR,)
url = reverse(viewname, args=(quote(self.foobar.pk),))
response = self.client.get(url)
self.assertContains(response, escape(self.foobar.pk))

def test_foobar_history_view_redirects_if_does_not_exist(self):
viewname = self.HISTORY_VIEW % (self.FOOBAR,)
url = reverse(viewname, args=("1,2,3",))
response = self.client.get(url)
self.assertEqual(response.status_code, 302)

def test_foo_history_view(self):
viewname = self.HISTORY_VIEW % (self.FOO,)
url = reverse(viewname, args=(quote(self.foo.pk),))
response = self.client.get(url)
self.assertContains(response, escape(self.foo.pk))

def test_foo_history_view_redirects_if_does_not_exist(self):
viewname = self.HISTORY_VIEW % (self.FOO,)
url = reverse(viewname, args=("1,2",))
response = self.client.get(url)
self.assertEqual(response.status_code, 302)

def test_foobar_change_view(self):
viewname = self.CHANGE_VIEW % (self.FOOBAR,)
url = reverse(viewname, args=(quote(self.foobar.pk),))
response = self.client.get(url)
self.assertContains(response, escape(self.foobar.pk))

def test_foobar_change_view_redirects_if_does_not_exist(self):
viewname = self.CHANGE_VIEW % (self.FOOBAR,)
url = reverse(viewname, args=("f,o,o",))
response = self.client.get(url)
self.assertEqual(response.status_code, 302)

def test_foo_change_view(self):
viewname = self.CHANGE_VIEW % (self.FOO,)
url = reverse(viewname, args=(quote(self.foo.pk),))
response = self.client.get(url)
self.assertContains(response, escape(self.foo.pk))

def test_foo_change_view_redirects_if_does_not_exist(self):
viewname = self.CHANGE_VIEW % (self.FOO,)
url = reverse(viewname, args=("f,o,o",))
response = self.client.get(url)
self.assertEqual(response.status_code, 302)

def test_foobar_delete_view(self):
viewname = self.DELETE_VIEW % (self.FOOBAR,)
url = reverse(viewname, args=(quote(self.foobar.pk),))
response = self.client.get(url)
self.assertContains(response, escape(self.foobar.pk))

def test_foobar_delete_view_redirects_if_does_not_exist(self):
viewname = self.DELETE_VIEW % (self.FOOBAR,)
url = reverse(viewname, args=("1,2",))
response = self.client.get(url)
self.assertEqual(response.status_code, 302)

def test_foo_delete_view(self):
viewname = self.DELETE_VIEW % (self.FOO,)
url = reverse(viewname, args=(quote(self.foo.pk),))
response = self.client.get(url)
self.assertContains(response, escape(self.foo.pk))

def test_foo_delete_view_redirects_if_does_not_exist(self):
viewname = self.DELETE_VIEW % (self.FOO,)
url = reverse(viewname, args=("123",))
response = self.client.get(url)
self.assertEqual(response.status_code, 302)


@override_settings(ROOT_URLCONF="admin_views.urls")
class SecureViewTests(TestCase):
"""
Expand Down

0 comments on commit 22c80e7

Please sign in to comment.