Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  carljm
Status:  new   | Milestone:  1.2   
 Component:  django.contrib.admin  |   Version:  SVN   
Resolution:|  Keywords:
 Stage:  Accepted  | Has_patch:  1 
Needs_docs:  0 |   Needs_tests:  0 
Needs_better_patch:  0 |  
---+
Comment (by carljm):

 I spent some time at the sprints today working on merging NestedObjects
 and CollectedObjects. I don't have working merged code yet, but my feeling
 so far is that it's not worth it. The two collections accept the same
 .add() API, but the needed semantics in each case are quite specific and
 quite different.

 CollectedObjects cares only about dependencies between model classes, not
 actual model objects, and it doesn't care about nullable relations (the
 delete is still cascaded across the nullable FK, but because its nullable
 it doesn't affect the order of deletions). Aside from the nullable FKs, it
 needs to build a complete depgraph (leaving open the possibility of cyclic
 deps), and then just bail in the cyclic case.

 NestedObjects, OTOH, needs to build a depgraph of individual objects, with
 nullable relations treated no differently from non-nullable (either way
 it's a parent relationship for the nested list). It doesn't even care
 about having a complete depgraph, just "as much as possible without
 including a given object twice," which also ensures no cyclic deps.

 Given these differing semantics, the merged collection class ends up
 looking very much like two separate collections internally anyway, and I
 think there's a net loss in clarity of purpose.

 If you feel strongly about at least seeing a working merged version for
 comparison before moving ahead, comment to that effect and I'll continue
 working on it.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  carljm
Status:  new   | Milestone:  1.2   
 Component:  django.contrib.admin  |   Version:  SVN   
Resolution:|  Keywords:
 Stage:  Accepted  | Has_patch:  1 
Needs_docs:  0 |   Needs_tests:  0 
Needs_better_patch:  0 |  
---+
Comment (by carljm):

 Thanks for the review, very helpful.

 Replying to [comment:23 russellm]:
 >  * I got a test failure running the
 admin_util.NestedObjectsTests.test_siblings test (got [0, [2, 1]],
 expected [0, [1, 2]]). The problem appears to be that NestedObjects uses a
 set to collect objects, which therefore doesn't give a guaranteed result
 order. One fix is to use a list instead if a set; the other fix is to
 modify the test so it isn't order dependent. I'm fairly certain just using
 a list is all that is required - I couldn't see anything obvious that was
 relying on the fact that sets were unique or easy to look up.

 Right, I'll just use a list. Somehow I thought the set was to prevent
 duplicates, but I check for already-seen first, so that's not an issue.

 >  * I'm not wild about the inner _format_callback function in
 get_deleted_objects(). admin_site, perms_needed, and levels_to_root are
 all variables from a scope outside the function itself, which is a bit of
 a code smell. I'd be much happier if _format_callback was a standalone
 function in its own right that took these extra values as kwargs that were
 passed in by nested (i.e., nested takes any extra kwargs and passes them
 down the the callback)

 One man's closure is another's code smell, I guess ;-) Just don't tell the
 Lispers & Javascripters. I think the closure style is simpler and more
 elegant here than adding **kwargs all over the place, but I'm not tied to
 it; I'll use your suggestion instead.

 >  * The one test case that I can see that is obviously missing is
 inheritance. On paper, this is probably caught by deleting OneToOneFields,
 but there is enough special handling for inheritance that it's worth
 having a specific test case for it (suggestion: SuperVillain subclassing
 Villain, create a SuperVillain; what happens if you delete the villain? if
 you delete the supervillain?)

 I was trying to strike a balance of thoroughly testing the admin
 functionality without redundantly testing Model._collect_sub_objects,
 which is tested elsewhere; since my code has no special handling for
 inheritance I thought this fell into the "redundant" category. But I guess
 that same argument could apply to the multiple-fkey tests; I'll add
 inheritance tests.

 >  * The template change makes me mildly nervous. Ideally, I'd prefer that
 this change wasn't necessary, as it will break any existing templates in
 the field that are constructed against the context that has historically
 existed - even if that just means introducing a dummy top-level list entry
 so the old template can iterate over a single element.

 Ok. I'll use a dummy top-level list wrapper to keep the template context
 backwards-compatible, but I'll at least fix the template misspelling of
 "deletable," since that's internal to the short loop ;)

 >  * The comments on the add() method for NestedObjects makes a point of
 highlighting that the model, pk and parent_model arguments aren't actually
 used. I accept that they are completely redundant as they can be derived
 from the obj and parent_obj arguments. However, I'm a little nervous

 Was there more here that got cut off regarding the nature of your
 nervousness? If I do merge CollectedObjects and NestedObjects, I would at
 least be tempted to remove redundancy from the .add() API, but this would
 require giving CollectedObjects knowledge about model internals
 (specifically model.pk), which currently it avoids. Do you have a clear
 preference between the simpler API (object, parent), which requires the
 collection class to know it's collecting model instances, vs. the current
 redundant API (obj_class, obj_pk, obj, parent_class, parent_pk), where the
 redundancy is needed because NestedObjects ultimately needs access to the
 full instance?

 >  * Is there any reason that NestedObjects can't be merged into
 CollectedObjects? It seems to me like there is a lot of duplication
 between the two classes, and we would be better served by extending
 CollectedObjects to provide the nested() functionality rather than have a
 second collection class.

 These last two issues are a result of me trying to touch django.db.* as
 little as possible. I certainly thought of merging CollectedObjects and
 NestedObjects; if you like the idea, I'll give it a shot. I'm a

Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  carljm
Status:  new   | Milestone:  1.2   
 Component:  django.contrib.admin  |   Version:  SVN   
Resolution:|  Keywords:
 Stage:  Accepted  | Has_patch:  1 
Needs_docs:  0 |   Needs_tests:  0 
Needs_better_patch:  0 |  
---+
Comment (by russellm):

 The general approach here looks really good. Some review comments:

  * I got a test failure running the
 admin_util.NestedObjectsTests.test_siblings test (got [0, [2, 1]],
 expected [0, [1, 2]]). The problem appears to be that NestedObjects uses a
 set to collect objects, which therefore doesn't give a guaranteed result
 order. One fix is to use a list instead if a set; the other fix is to
 modify the test so it isn't order dependent. I'm fairly certain just using
 a list is all that is required - I couldn't see anything obvious that was
 relying on the fact that sets were unique or easy to look up.

  * I'm not wild about the inner _format_callback function in
 get_deleted_objects(). admin_site, perms_needed, and levels_to_root are
 all variables from a scope outside the function itself, which is a bit of
 a code smell. I'd be much happier if _format_callback was a standalone
 function in its own right that took these extra values as kwargs that were
 passed in by nested (i.e., nested takes any extra kwargs and passes them
 down the the callback)

  * The one test case that I can see that is obviously missing is
 inheritance. On paper, this is probably caught by deleting OneToOneFields,
 but there is enough special handling for inheritance that it's worth
 having a specific test case for it (suggestion: SuperVillain subclassing
 Villain, create a SuperVillain; what happens if you delete the villain? if
 you delete the supervillain?)

  * The template change makes me mildly nervous. Ideally, I'd prefer that
 this change wasn't necessary, as it will break any existing templates in
 the field that are constructed against the context that has historically
 existed - even if that just means introducing a dummy top-level list entry
 so the old template can iterate over a single element.

  * The comments on the add() method for NestedObjects makes a point of
 highlighting that the model, pk and parent_model arguments aren't actually
 used. I accept that they are completely redundant as they can be derived
 from the obj and parent_obj arguments. However, I'm a little nervous

  * Is there any reason that NestedObjects can't be merged into
 CollectedObjects? It seems to me like there is a lot of duplication
 between the two classes, and we would be better served by extending
 CollectedObjects to provide the nested() functionality rather than have a
 second collection class.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  carljm
Status:  new   | Milestone:  1.2   
 Component:  django.contrib.admin  |   Version:  SVN   
Resolution:|  Keywords:
 Stage:  Accepted  | Has_patch:  1 
Needs_docs:  0 |   Needs_tests:  0 
Needs_better_patch:  0 |  
---+
Comment (by carljm):

 The latest patch here includes special-case code to fix #12025. If the fix
 for #12953 moves generic-relation handling up to
 Model._collect_sub_objects(), it should be possible to get rid of the
 special-case here.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  carljm
Status:  new   | Milestone:  1.2   
 Component:  django.contrib.admin  |   Version:  SVN   
Resolution:|  Keywords:
 Stage:  Accepted  | Has_patch:  1 
Needs_docs:  0 |   Needs_tests:  0 
Needs_better_patch:  0 |  
---+
Comment (by carljm):

 Oh, now I see #12025: apparently the previous code didn't notify on
 GenericRelations either. Seems like it wouldn't hurt to knock that off at
 the same time, but maybe it's better to keep the issues separate.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  carljm
Status:  new   | Milestone:  1.2   
 Component:  django.contrib.admin  |   Version:  SVN   
Resolution:|  Keywords:
 Stage:  Accepted  | Has_patch:  1 
Needs_docs:  0 |   Needs_tests:  0 
Needs_better_patch:  0 |  
---+
Changes (by carljm):

  * keywords:  nfa-someday =>

Comment:

 Just realized from working on #12721 that this patch would not notify the
 deletion of related objects via a GenericRelation. Those objects are not
 collected as part of _collect_sub_objects, they are deleted far deeper in
 the internals, in DeleteQuery.delete_batch_related. I'm not sure why that
 needs to be the case (seems a bit ugly), but haven't looked at it closely;
 in any case I guess changing that is too invasive for the scope of this
 bug? So given that, the only reasonable solution I can see is to duplicate
 the generic-related-finding code from delete_batch_related in the admin
 routine. Reintroduces a bit of duplication, but only for GenericRelation,
 so much less than was there previously.

 I'll work on that unless anyone pops up with a better approach.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  carljm 
Status:  new   | Milestone:  1.2
 Component:  django.contrib.admin  |   Version:  SVN
Resolution:|  Keywords:  nfa-someday
 Stage:  Accepted  | Has_patch:  1  
Needs_docs:  0 |   Needs_tests:  0  
Needs_better_patch:  0 |  
---+
Changes (by carljm):

  * needs_better_patch:  1 => 0
  * milestone:  => 1.2

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  carljm 
Status:  new   | Milestone: 
 Component:  django.contrib.admin  |   Version:  SVN
Resolution:|  Keywords:  nfa-someday
 Stage:  Accepted  | Has_patch:  1  
Needs_docs:  0 |   Needs_tests:  0  
Needs_better_patch:  1 |  
---+
Comment (by carljm):

 Discovered two more differences between this patch and current behavior.

 1. Currently the list of "related objects to be deleted" includes entries
 for "one or more foo in bar: my_bar", where foo is a M2M field on the bar
 model pointing to the object about to be deleted. With the patch, m2m
 intermediary relationship objects are only listed if you have an explicit
 "through" model, in which case each "through" object is listed separately.
 IMO, an m2m without an explicit through model is conceptually just a
 relationship; the fact that an entry in a table will be deleted is just a
 relationship implementation detail that doesn't require notification. If
 the previous behavior is preferred, however, I am willing to add it to the
 patch.

 2. In general, this patch only ever lists a given object once, even if
 there are multiple relationship paths to it. Current behavior can
 potentially list the same object many times if there are several paths to
 it.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  carljm 
Status:  new   | Milestone: 
 Component:  django.contrib.admin  |   Version:  SVN
Resolution:|  Keywords:  nfa-someday
 Stage:  Accepted  | Has_patch:  1  
Needs_docs:  0 |   Needs_tests:  0  
Needs_better_patch:  1 |  
---+
Comment (by carljm):

 One additional note: there was a minor change I had to make to the
 CollectedObject.add() signature; no way this approach could work without
 it. The change is minimally intrusive: there's test coverage of
 CollectedObject and _collect_sub_objects in modeltests/delete, and all
 those tests pass with no change.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  carljm 
Status:  new   | Milestone: 
 Component:  django.contrib.admin  |   Version:  SVN
Resolution:|  Keywords:  nfa-someday
 Stage:  Accepted  | Has_patch:  1  
Needs_docs:  0 |   Needs_tests:  0  
Needs_better_patch:  1 |  
---+
Comment (by carljm):

 I ended up taking a slightly different approach thanks to feedback from
 Alex Gaynor on my above comment. I built a collection object that can be
 passed into Model._collect_sub_objects (implementing the interface it
 expects from its collector), but then can spit out the collected objects
 in the nested list form that the delete confirmation view needs. The patch
 now fixes both this bug and #11296 (passes all three tests I mentioned
 previously), and removes the algorithm duplication so there shouldn't be
 able to be discrepancies between the displayed objects and the deleted
 ones.

 Some areas where I'm aware this changes behavior:

 - If an inherited model is deleted, the parent model is now explicitly
 displayed as well, whereas before it wasn't.

 - The previous code appeared to try to use url reversing for the links to
 changeform for each object, but was broken in such a way that the
 reversing always failed, causing a fallback to the old method. Reversing
 now works if the admin urls are included in the new style. This doesn't
 cause a user-visible change, but does cause the generated links to now go
 through the iri_to_uri encoding that reverse() performs. This required one
 small test change.

 I know the admin is already coupled to internal details of various things
 in Django, but I'm slightly uncomfortable introducing yet another one with
 the use of Model._collect_sub_objects, which is technically marked as a
 private API. I do think it's reasonable that the public model API would
 provide some way for client code to discover in advance what related
 objects would be affected by a delete() call. Should this patch should
 also promote _collect_sub_objects to a semi-public (but not documented)
 API, presumably just by removing the underscore? Or even document it?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  carljm 
Status:  new   | Milestone: 
 Component:  django.contrib.admin  |   Version:  SVN
Resolution:|  Keywords:  nfa-someday
 Stage:  Accepted  | Has_patch:  1  
Needs_docs:  0 |   Needs_tests:  0  
Needs_better_patch:  1 |  
---+
Comment (by carljm):

 The attached patch is based on nicklane's 6191.patch, updated to apply to
 trunk, and with a number of new tests. Just uploading for the tests; I
 don't think the fix is ready for trunk.

 Notably (among other things), the new tests test that:

 1. If a model has two foreign keys to it from a single related model, both
 are followed (the original issue here, passes after the patch).

 2. A given object is only displayed once in the list, even if it can be
 found through two different paths (passes before the patch, fails after
 it).

 3. A cyclic dependency still only lists each object once, not repeatedly
 until an arbitrary max recursion is hit (this is #11296, currently fails
 both before and after).

 IMO, kmtracey is right: the real fix here is to re-use the same code that
 is actually used to collect the objects for deletion (that's
 Model._collect_sub_objects). Otherwise we're likely to continue to see
 recurring problems with mismatches between the two duplicate algorithms.
 It's tricky, because get_deleted_objects generates a nested data structure
 for display, where Model._collect_sub_objects generates a flat one. I
 think it's possible by adding a callback hook to
 Model._collect_sub_objects, so get_deleted_objects can be notified each
 time _collect_sub_objects recurses. This is the route I'm planning to
 pursue, and I think I should be able to get all three of the above cases
 to pass (fixing both this and #11296), and eliminate the duplicate logic.
 Comments welcome.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  carljm 
Status:  new   | Milestone: 
 Component:  django.contrib.admin  |   Version:  SVN
Resolution:|  Keywords:  nfa-someday
 Stage:  Accepted  | Has_patch:  1  
Needs_docs:  0 |   Needs_tests:  0  
Needs_better_patch:  1 |  
---+
Changes (by carljm):

  * owner:  nobody => carljm

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  nobody 
Status:  new   | Milestone: 
 Component:  django.contrib.admin  |   Version:  SVN
Resolution:|  Keywords:  nfa-someday
 Stage:  Accepted  | Has_patch:  1  
Needs_docs:  0 |   Needs_tests:  0  
Needs_better_patch:  1 |  
---+
Changes (by shie...@msrl.com):

 * cc: mshie...@google.com (added)

Comment:

 I've attached the patch I wrote when reporting this as #11821.

 I think this problem is fairly serious, so if this patch can't be
 accepted, let me know what needs to change.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  nobody 
Status:  new   | Milestone: 
 Component:  django.contrib.admin  |   Version:  SVN
Resolution:|  Keywords:  nfa-someday
 Stage:  Accepted  | Has_patch:  1  
Needs_docs:  0 |   Needs_tests:  0  
Needs_better_patch:  1 |  
---+
Comment (by kmtracey):

 #11821 reported this again and has a patch.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  nobody 
Status:  new   | Milestone: 
 Component:  django.contrib.admin  |   Version:  SVN
Resolution:|  Keywords:  nfa-someday
 Stage:  Accepted  | Has_patch:  1  
Needs_docs:  0 |   Needs_tests:  0  
Needs_better_patch:  1 |  
---+
Changes (by daemondazz):

 * cc: daemondazz (added)

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  nobody 
Status:  new   | Milestone: 
 Component:  django.contrib.admin  |   Version:  SVN
Resolution:|  Keywords:  nfa-someday
 Stage:  Accepted  | Has_patch:  1  
Needs_docs:  0 |   Needs_tests:  0  
Needs_better_patch:  1 |  
---+
Changes (by carljm):

 * cc: carljm (added)

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #6191: Admin delete view doesn't show all items in some circumstances

[Django]

#6191: Admin delete view doesn't show all items in some circumstances
---+
  Reporter:  nicklane  | Owner:  nobody 
Status:  new   | Milestone: 
 Component:  django.contrib.admin  |   Version:  SVN
Resolution:|  Keywords:  nfa-someday
 Stage:  Accepted  | Has_patch:  1  
Needs_docs:  0 |   Needs_tests:  0  
Needs_better_patch:  1 |  
---+
Changes (by kmtracey):

  * needs_better_patch:  0 => 1

Comment:

 As of the fix for #7276, there's at least one more case where the admin
 delete confirmation page doesn't show all the objects that are actually
 going to be deleted, which highlights the perils of having two separate
 code paths: one for calculating the to-be-deleted objects for display and
 the 2nd that does the actual deletion.  It would really be better if the
 admin for-display part could somehow make use of the underlying ORM
 routines that are going to govern what actually gets deleted, but that's
 not an easy fix.

 Also the current patch will display an object twice -- if for example A is
 both the team_leader and contact_person for a project, that project will
 get displayed twice on the confirmation page for deleting A.  That's a bit
 confusing (though I'm not entirely sure it can't also happen with current
 code -- I find that admin routine a bit hard to get my head around).

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---