Permalink
Browse files

Fixed #18491 -- deleting a proxy doesn't show warning about cascade d…

…eletes
  • Loading branch information...
1 parent 83ecb7b commit 2b48fcc607010065c0f8107baf669dd41b164f3c @HonzaKral HonzaKral committed Feb 23, 2013
View
3 django/contrib/admin/util.py
@@ -154,6 +154,9 @@ def collect(self, objs, source_attr=None, **kwargs):
if source_attr:
self.add_edge(getattr(obj, source_attr), obj)
else:
+ if obj._meta.proxy:
+ # Take concrete model's instance to avoid mismatch in edges
+ obj = obj._meta.concrete_model(pk=obj.pk)
self.add_edge(None, obj)
try:
return super(NestedObjects, self).collect(objs, source_attr=source_attr, **kwargs)
View
2 setup.py
@@ -1,4 +1,4 @@
-from distutils.core import setup
+from setuptools import setup
@claudep
Django member
claudep added a line comment Feb 23, 2013

I don't think this was intended in the patch, was it?

@HonzaKral
Django member
HonzaKral added a line comment Feb 23, 2013

crap, you are absolutely correct, my fault, revert is coming

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
from distutils.command.install_data import install_data
from distutils.command.install import INSTALL_SCHEMES
from distutils.sysconfig import get_python_lib
View
24 tests/modeltests/proxy_models/fixtures/myhorses.json
@@ -0,0 +1,24 @@
+[
+ {
+ "pk": 100,
+ "model": "proxy_models.BaseUser",
+ "fields": {
+ "name": "Django Pony"
+ }
+ },
+ {
+ "pk": 100,
+ "model": "proxy_models.TrackerUser",
+ "fields": {
+ "status": "emperor"
+ }
+ },
+ {
+ "pk": 100,
+ "model": "proxy_models.Issue",
+ "fields": {
+ "summary": "Pony's Issue",
+ "assignee": 100
+ }
+ }
+]
View
28 tests/modeltests/proxy_models/tests.py
@@ -2,6 +2,7 @@
import copy
from django.conf import settings
+from django.contrib import admin
from django.contrib.contenttypes.models import ContentType
from django.core import management
from django.core.exceptions import FieldError
@@ -14,7 +15,7 @@
from .models import (MyPerson, Person, StatusPerson, LowerStatusPerson,
MyPersonProxy, Abstract, OtherPerson, User, UserProxy, UserProxyProxy,
Country, State, StateProxy, TrackerUser, BaseUser, Bug, ProxyTrackerUser,
- Improvement, ProxyProxyBug, ProxyBug, ProxyImprovement)
+ Improvement, ProxyProxyBug, ProxyBug, ProxyImprovement, Issue)
class ProxyModelTests(TestCase):
@@ -360,3 +361,28 @@ def test_proxy_load_from_fixture(self):
management.call_command('loaddata', 'mypeople.json', verbosity=0, commit=False)
p = MyPerson.objects.get(pk=100)
self.assertEqual(p.name, 'Elvis Presley')
+
+
+class ProxyModelAdminTests(TestCase):
+ def setUp(self):
+ management.call_command('loaddata', 'myhorses.json', verbosity=0,
+ commit=False)
@uruz
uruz added a line comment Feb 24, 2013

I am not sure, is there some reason not to declare fixture in a usual way of fixtures = ['myhorses'] ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ def tearDown(self):
+ TrackerUser.objects.all().delete()
+ Issue.objects.all().delete()
@claudep
Django member
claudep added a line comment Feb 23, 2013

What is the rationale behind this tearDown method?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ def test_cascade_delete_proxy_model_admin_warning(self):
+ """
+ Test if admin gives warning about cascade deleting models referenced
+ to concrete model by deleting proxy object.
+ """
+ tracker_user = TrackerUser.objects.all()[0]
+ base_user = BaseUser.objects.all()[0]
+ issue = Issue.objects.all()[0]
+ with self.assertNumQueries(7):
+ collector = admin.util.NestedObjects('default')
+ collector.collect(ProxyTrackerUser.objects.all())
+ self.assertTrue(tracker_user in collector.edges.get(None, ()))
+ self.assertTrue(base_user in collector.edges.get(None, ()))
+ self.assertTrue(issue in collector.edges.get(tracker_user, ()))

0 comments on commit 2b48fcc

Please sign in to comment.