Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

[1.0.X] Fixed #2698 -- Fixed deleting in the presence of custom manag…

…ers.

A custom manager on a related object that filtered away objects would prevent
those objects being deleted via the relation. This is now fixed.

Backport of r10104 from trunk.

git-svn-id: http://code.djangoproject.com/svn/django/branches/releases/1.0.X@10106 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit ebfe7faaa3efef952d092b332b79f182ea5f45d1 1 parent 083a720
Malcolm Tredinnick authored March 20, 2009
12  django/db/models/base.py
@@ -437,7 +437,17 @@ def _collect_sub_objects(self, seen_objs, parent=None, nullable=False):
437 437
                 else:
438 438
                     sub_obj._collect_sub_objects(seen_objs, self.__class__, related.field.null)
439 439
             else:
440  
-                for sub_obj in getattr(self, rel_opts_name).all():
  440
+                # To make sure we can access all elements, we can't use the
  441
+                # normal manager on the related object. So we work directly
  442
+                # with the descriptor object.
  443
+                for cls in self.__class__.mro():
  444
+                    if rel_opts_name in cls.__dict__:
  445
+                        rel_descriptor = cls.__dict__[rel_opts_name]
  446
+                        break
  447
+                else:
  448
+                    raise AssertionError("Should never get here.")
  449
+                delete_qs = rel_descriptor.delete_manager(self).all()
  450
+                for sub_obj in delete_qs:
441 451
                     sub_obj._collect_sub_objects(seen_objs, self.__class__, related.field.null)
442 452
 
443 453
         # Handle any ancestors (for the model-inheritance case). We do this by
44  django/db/models/fields/related.py
@@ -181,7 +181,7 @@ def __get__(self, instance, instance_type=None):
@@ -289,13 +289,36 @@ def __get__(self, instance, instance_type=None):
@@ -345,17 +368,6 @@ def clear(self):
52  tests/regressiontests/custom_managers_regress/models.py
@@ -11,9 +11,27 @@ class RestrictedManager(models.Manager):
11 11
     def get_query_set(self):
12 12
         return super(RestrictedManager, self).get_query_set().filter(is_public=True)
13 13
 
14  
-class RestrictedClass(models.Model):
  14
+class RelatedModel(models.Model):
  15
+    name = models.CharField(max_length=50)
  16
+
  17
+    def __unicode__(self):
  18
+        return self.name
  19
+
  20
+class RestrictedModel(models.Model):
  21
+    name = models.CharField(max_length=50)
  22
+    is_public = models.BooleanField(default=False)
  23
+    related = models.ForeignKey(RelatedModel)
  24
+
  25
+    objects = RestrictedManager()
  26
+    plain_manager = models.Manager()
  27
+
  28
+    def __unicode__(self):
  29
+        return self.name
  30
+
  31
+class OneToOneRestrictedModel(models.Model):
15 32
     name = models.CharField(max_length=50)
16 33
     is_public = models.BooleanField(default=False)
  34
+    related = models.OneToOneField(RelatedModel)
17 35
 
18 36
     objects = RestrictedManager()
19 37
     plain_manager = models.Manager()
@@ -24,15 +42,41 @@ def __unicode__(self):
24 42
 __test__ = {"tests": """
25 43
 Even though the default manager filters out some records, we must still be able
26 44
 to save (particularly, save by updating existing records) those filtered
27  
-instances. This is a regression test for #FIXME.
28  
->>> obj = RestrictedClass.objects.create(name="hidden")
  45
+instances. This is a regression test for #8990, #9527
  46
+>>> related = RelatedModel.objects.create(name="xyzzy")
  47
+>>> obj = RestrictedModel.objects.create(name="hidden", related=related)
29 48
 >>> obj.name = "still hidden"
30 49
 >>> obj.save()
31 50
 
32 51
 # If the hidden object wasn't seen during the save process, there would now be
33 52
 # two objects in the database.
34  
->>> RestrictedClass.plain_manager.count()
  53
+>>> RestrictedModel.plain_manager.count()
35 54
 1
36 55
 
  56
+Deleting related objects should also not be distracted by a restricted manager
  57
+on the related object. This is a regression test for #2698.
  58
+>>> RestrictedModel.plain_manager.all().delete()
  59
+>>> for name, public in (('one', True), ('two', False), ('three', False)):
  60
+...     _ = RestrictedModel.objects.create(name=name, is_public=public, related=related)
  61
+
  62
+# Reload the RelatedModel instance, just to avoid any instance artifacts.
  63
+>>> obj = RelatedModel.objects.get(name="xyzzy")
  64
+>>> obj.delete()
  65
+
  66
+# All of the RestrictedModel instances should have been deleted, since they
  67
+# *all* pointed to the RelatedModel. If the default manager is used, only the
  68
+# public one will be deleted.
  69
+>>> RestrictedModel.plain_manager.all()
  70
+[]
  71
+
  72
+# The same test case as the last one, but for one-to-one models, which are
  73
+# implemented slightly different internally, so it's a different code path.
  74
+>>> obj = RelatedModel.objects.create(name="xyzzy")
  75
+>>> _ = OneToOneRestrictedModel.objects.create(name="foo", is_public=False, related=obj)
  76
+>>> obj = RelatedModel.objects.get(name="xyzzy")
  77
+>>> obj.delete()
  78
+>>> OneToOneRestrictedModel.plain_manager.all()
  79
+[]
  80
+
37 81
 """
38 82
 }

0 notes on commit ebfe7fa

Please sign in to comment.
Something went wrong with that request. Please try again.