Re: [Django] #13902: When using a ManyToMany in list_filter, results may apper more than once

[Django]

#13902: When using a ManyToMany in list_filter, results may apper more than once
+---
   Reporter:  rasca | Owner:  rasca 
  
 Status:  new   | Milestone:  1.3   
  
  Component:  django.contrib.admin  |   Version:  SVN   
  
 Resolution:|  Keywords:  admin 
change_list list_filter manytomany
   Triage Stage:  Ready for checkin | Has patch:  1 
  
Needs documentation:  0 |   Needs tests:  0 
  
Patch needs improvement:  1 |  
+---
Changes (by julien):

  * stage:  Accepted => Ready for checkin


Comment:

 The patch looks good and tests are now passing. Just one small suggestion:
 I think that "use_distinct" (or "do_distinct") would be a better variable
 name than "is_distinct" ;-)

-- 
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] #13902: When using a ManyToMany in list_filter, results may apper more than once

[Django]

#13902: When using a ManyToMany in list_filter, results may apper more than once
---+
  Reporter:  rasca | Owner:  rasca  
 
Status:  new   | Milestone:  1.3
 
 Component:  django.contrib.admin  |   Version:  SVN
 
Resolution:|  Keywords:  admin change_list 
list_filter manytomany
 Stage:  Accepted  | Has_patch:  1  
 
Needs_docs:  0 |   Needs_tests:  0  
 
Needs_better_patch:  1 |  
---+
Comment (by rasca):

 I've just uploaded a new patch, not sure if it's correct though.

 The problem with the test fail was that the model inherited from another
 concrete model and the get_field() returned FieldDoesNotExist. This are
 the models from the test:

 {{{
 class Title(models.Model):
 pass

 class TitleTranslation(models.Model):
 title = models.ForeignKey(Title)
 text = models.CharField(max_length=100)

 class Recommender(Title):
 pass

 class Recommendation(Title):
 recommender = models.ForeignKey(Recommender)

 class RecommendationAdmin(admin.ModelAdmin):
 search_fields = ('titletranslation__text',
 'recommender__titletranslation__text',)

 }}}


 What I now did is to use get_field_by_name() and perform and extra check
 to see if it's a RelatedObject, in this case
 `isinstance(get_field_by_name('titletranslation')[0],
 models.related.RelatedObject)`

 I'm not familiarized with this part of Django, so I have no idea if what I
 did is correct. If it's not, can you give me some pointers please?

-- 
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] #13902: When using a ManyToMany in list_filter, results may apper more than once

[Django]

#13902: When using a ManyToMany in list_filter, results may apper more than once
---+
  Reporter:  rasca | Owner:  rasca  
 
Status:  new   | Milestone:  1.3
 
 Component:  django.contrib.admin  |   Version:  SVN
 
Resolution:|  Keywords:  admin change_list 
list_filter manytomany
 Stage:  Accepted  | Has_patch:  1  
 
Needs_docs:  0 |   Needs_tests:  0  
 
Needs_better_patch:  1 |  
---+
Changes (by ramiro):

  * needs_better_patch:  0 => 1

Comment:

 The modification to the search checks in the patch is making `admin_views`
 tests to fail, these tests are related to `search_fields` option values
 that specify a reverse FK accesor:
 {{{
 ==
 ERROR: test_search_on_sibling_models
 (regressiontests.admin_views.tests.AdminSearchTest)
 Check that a search that mentions sibling models
 --
 Traceback (most recent call last):
   File "django/upstream/tests/regressiontests/admin_views/tests.py", line
 1491, in test_search_on_sibling_models
 response =
 self.client.get('/test_admin/admin/admin_views/recommendation/?q=bar')
   File "django/upstream/django/test/client.py", line 441, in get
 response = super(Client, self).get(path, data=data, **extra)
   File "django/upstream/django/test/client.py", line 225, in get
 return self.request(**r)
   File "django/upstream/django/core/handlers/base.py", line 111, in
 get_response
 response = callback(request, *callback_args, **callback_kwargs)
   File "django/upstream/django/contrib/admin/options.py", line 298, in
 wrapper
 return self.admin_site.admin_view(view)(*args, **kwargs)
   File "django/upstream/django/utils/decorators.py", line 93, in
 _wrapped_view
 response = view_func(request, *args, **kwargs)
   File "django/upstream/django/views/decorators/cache.py", line 79, in
 _wrapped_view_func
 response = view_func(request, *args, **kwargs)
   File "django/upstream/django/contrib/admin/sites.py", line 190, in inner
 return view(request, *args, **kwargs)
   File "django/upstream/django/utils/decorators.py", line 28, in _wrapper
 return bound_func(*args, **kwargs)
   File "django/upstream/django/utils/decorators.py", line 93, in
 _wrapped_view
 response = view_func(request, *args, **kwargs)
   File "django/upstream/django/utils/decorators.py", line 24, in
 bound_func
 return func(self, *args2, **kwargs2)
   File "django/upstream/django/contrib/admin/options.py", line 1034, in
 changelist_view
 self.list_select_related, self.list_per_page, self.list_editable,
 self)
   File "django/upstream/django/contrib/admin/views/main.py", line 66, in
 __init__
 self.query_set = self.get_query_set()
   File "django/upstream/django/contrib/admin/views/main.py", line 261, in
 get_query_set
 f = self.lookup_opts.get_field(field_name.split('__',1)[0])
   File "django/upstream/django/db/models/options.py", line 279, in
 get_field
 raise FieldDoesNotExist('%s has no field named %r' %
 (self.object_name, name))
 FieldDoesNotExist: Recommendation has no field named 'titletranslation'
 }}}

-- 
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] #13902: When using a ManyToMany in list_filter, results may apper more than once

[Django]

#13902: When using a ManyToMany in list_filter, results may apper more than once
---+
  Reporter:  rasca | Owner:  rasca  
 
Status:  new   | Milestone:  1.3
 
 Component:  django.contrib.admin  |   Version:  SVN
 
Resolution:|  Keywords:  admin change_list 
list_filter manytomany
 Stage:  Accepted  | Has_patch:  1  
 
Needs_docs:  0 |   Needs_tests:  0  
 
Needs_better_patch:  0 |  
---+
Changes (by rasca):

  * milestone:  => 1.3

Comment:

 Uploaded new patch, taking into consideration Alex's note.

-- 
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] #13902: When using a ManyToMany in list_filter, results may apper more than once

[Django]

#13902: When using a ManyToMany in list_filter, results may apper more than once
---+
  Reporter:  rasca | Owner:  rasca  
 
Status:  new   | Milestone: 
 
 Component:  django.contrib.admin  |   Version:  SVN
 
Resolution:|  Keywords:  admin change_list 
list_filter manytomany
 Stage:  Accepted  | Has_patch:  1  
 
Needs_docs:  0 |   Needs_tests:  0  
 
Needs_better_patch:  0 |  
---+
Changes (by CarstenF):

 * cc: CarstenF (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-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] #13902: When using a ManyToMany in list_filter, results may apper more than once

[Django]

#13902: When using a ManyToMany in list_filter, results may apper more than once
---+
  Reporter:  rasca | Owner:  rasca  
 
Status:  new   | Milestone: 
 
 Component:  django.contrib.admin  |   Version:  SVN
 
Resolution:|  Keywords:  admin change_list 
list_filter manytomany
 Stage:  Accepted  | Has_patch:  1  
 
Needs_docs:  0 |   Needs_tests:  0  
 
Needs_better_patch:  0 |  
---+
Comment (by Alex):

 I'd be -1 on using distinct() everywhere, there are DB engines that don't
 support distinct, and list filter should work on them.  Also it seems it
 could use the same logic as here:
 
http://code.djangoproject.com/browser/django/trunk/django/contrib/admin/views/main.py#L208
 if there's a MultipleRel it can be duplicate, just call it then.

-- 
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] #13902: When using a ManyToMany in list_filter, results may apper more than once

[Django]

#13902: When using a ManyToMany in list_filter, results may apper more than once
---+
  Reporter:  rasca | Owner:  rasca  
 
Status:  new   | Milestone: 
 
 Component:  django.contrib.admin  |   Version:  SVN
 
Resolution:|  Keywords:  admin change_list 
list_filter manytomany
 Stage:  Accepted  | Has_patch:  1  
 
Needs_docs:  0 |   Needs_tests:  0  
 
Needs_better_patch:  0 |  
---+
Changes (by russellm):

  * stage:  Unreviewed => Accepted

Comment:

 On the whole, this looks good. Don't worry about the diff display problem
 -- this is a known issue with Trac; it has something to do with patches
 that have "No newline at end of file" lines in them. This doesn't actually
 pose a problem applying the patch, but Trac won't display the patch at
 all. Don't get too hung up on this problem.

 As for the patch: a couple of fairly minor fixes:

  * Constants are in ALLCAPS, variables shouldn't be. Use is_distinct, not
 DISTINCT.
  * Rather than using unittest.TestCase and manually tearing down the test,
 it will be faster to use Django's TestCase and use the rollback facilities
 that it provides.

 ... and one bigish issue:

 I'm not convinced that finding more than one '!__' in a list filter is
 enough of a qualifier. For example, !__exact is an implied comparison so
 multiple_m2m__id=1 will only have 1 !__, but will be prone to duplicates.

 That said, I'm wondering if there's an argument to be made that distinct()
 should always be applied. There will be a inefficiency in the query if we
 always apply distinct(), but the cases where it isn't required should be
 able to optimize out the distinct() at the SQL level.

-- 
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] #13902: When using a ManyToMany in list_filter, results may apper more than once

[Django]

#13902: When using a ManyToMany in list_filter, results may apper more than once
---+
  Reporter:  rasca | Owner:  rasca  
 
Status:  new   | Milestone: 
 
 Component:  django.contrib.admin  |   Version:  SVN
 
Resolution:|  Keywords:  admin change_list 
list_filter manytomany
 Stage:  Unreviewed| Has_patch:  1  
 
Needs_docs:  0 |   Needs_tests:  0  
 
Needs_better_patch:  0 |  
---+
Changes (by rasca):

  * needs_better_patch:  => 0
  * needs_tests:  => 0
  * version:  1.2 => SVN
  * owner:  nobody => rasca
  * needs_docs:  => 0
  * has_patch:  0 => 1

Comment:

 Patch 13902_distinct_in_changelist.git.diff fixes the problem (git
 format).

 I don't know why the diff isn't properly showing in trac.
 I compared it to
 http://code.djangoproject.com/attachment/ticket/10918/to_field-10918.git.diff
 and it has the same format.

 This is my first code contribution to Django, so please tell me if there's
 anything I can do better. Thanks!

-- 
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.

[Django] #13902: When using a ManyToMany in list_filter, results may apper more than once

[Django]

#13902: When using a ManyToMany in list_filter, results may apper more than once
--+-
 Reporter:  rasca |   Owner:  nobody
   Status:  new   |   Milestone:
Component:  django.contrib.admin  | Version:  1.2   
 Keywords:  admin change_list list_filter manytomany  |   Stage:  Unreviewed
Has_patch:  0 |  
--+-
 Suppose you have a ManyToMany relationship between models A and B through
 a model C.
 If you have more than one entry of C relating the same A and B instances,
 and in the admin in the model A change_list you are list_filtering by the
 relationship with B, and filter by the B instance that has more than 1
 relation to the same A instance, the A instance appears as many times as
 there are relationships in the list, when it should appear only once.

 I'm not sure where to add the .distinct() in
 admin.views.ChangeList.get_query_set(). We could add it when it returns
 the final queryset but I'm not sure if that's the most efficient way to do
 it. We could check to see if it's needed.
 There's also another .distinct() before but that one only applies to the
 search field (in case it's needed). Maybe combine these two?

-- 
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.