Admin search behavior changed from 1.2.5 to 1.3

27 views
Skip to first unread message

Ryan K

unread,
Apr 20, 2011, 11:42:04 AM4/20/11
to Django developers
Today I discovered behavior similar to that originally reported in
#15819 (http://code.djangoproject.com/ticket/15819). I've updated it
with a simple way to reproduce the issue. Could anyone confirm this
behavior? It's nothing major but it does seem that the admin search
behavior changed from 1.2.5 to 1.3.

Would it be possible to add a boolean, say 'search_distinct', to
ModelAdmin so distinct() is called after a search here:
http://code.djangoproject.com/browser/django/trunk/django/contrib/admin/views/main.py#L271
?

I haven't yet contributed to the project but would like to if this is
indeed something that should be fixed.

Best,
Ryan

Carl Meyer

unread,
Apr 20, 2011, 3:02:38 PM4/20/11
to django-d...@googlegroups.com
Hi Ryan,

On 04/20/2011 10:42 AM, Ryan K wrote:
> Today I discovered behavior similar to that originally reported in
> #15819 (http://code.djangoproject.com/ticket/15819). I've updated it
> with a simple way to reproduce the issue. Could anyone confirm this
> behavior? It's nothing major but it does seem that the admin search
> behavior changed from 1.2.5 to 1.3.

I can reproduce the issue (and confirm that it is a regression in 1.3),
and I've accepted the ticket.

> Would it be possible to add a boolean, say 'search_distinct', to
> ModelAdmin so distinct() is called after a search here:
> http://code.djangoproject.com/browser/django/trunk/django/contrib/admin/views/main.py#L271
> ?

Is there any reason why someone would _want_ duplicate search results? I
don't think so, so there is no reason for a ModelAdmin option here - it
should just be fixed.

> I haven't yet contributed to the project but would like to if this is
> indeed something that should be fixed.

That'd be great! Patch should be against SVN trunk, and include a test
that fails before the fix and passes afterwards, in
tests/regressiontests/admin_views/tests.py. Feel free to hit me up on
IRC (carljm) or here if you have questions working on the patch. I made
some additional comments on the ticket. Thanks!

Carl

Ryan K

unread,
Apr 21, 2011, 7:19:16 AM4/21/11
to Django developers
On Apr 20, 8:02 pm, Carl Meyer <c...@oddbird.net> wrote:#

> Feel free to hit me up on
> IRC (carljm) or here if you have questions working on the patch. I made
> some additional comments on the ticket. Thanks!

Thanks for checking over the ticket so quickly! I'm going to take up
your offer for help (or anyone for that matter).

I've updated #15819 (http://code.djangoproject.com/ticket/15819) with
a patch and tests.

Basically, in addition to testing if the field is a ManyToMany
relationship, it also checks for a relationship where unique might not
be true (e.g. a ForeignKey(model, unique=False)). Also, I didn't put
the tests in the suggested location because it seemed more appropriate
in tests/regressiontests/admin_changelist/test.py.

I don't know if the test is good enough so more eyes and hopefully
constructive criticism would be much appreciated! Here is the essence
of the addition:

isinstance(field, models.related.RelatedObject) and not
field.field.unique)

I've also added some additional comments on the bottom of the ticket
regarding the use of select_related() in get_query_set.

Thanks for all your help.

Ryan
Reply all
Reply to author
Forward
0 new messages