list_filter not available if model contains OneToOneField

[Daniel Roseman]

If a model is related to via a OneToOneField (even if it's not used in the admin list), the list_filter block doesn't appear on the admin list page.

The offending code appears to be in django.contrib.admin.views.main

571    def get_filters(self, request):
572        filter_specs = []
573        if self.lookup_opts.admin.list_filter and not self.opts.one_to_one_field:
574            filter_fields = [self.lookup_opts.get_field(field_name) \
575                              for field_name in self.lookup_opts.admin.list_filter]

Commenting out "and not self.opts.one_to_one_field" in line 573 reinstates the functionality and appears to have no ill effects. I don't know what it is supposed to be testing for, though, so don't know if it breaks something somewhere else.

[remco@…]

Any changes concerning this ticket? I still get the feeling that though for some relationships OneToOneField is conceptually nicer to use, one is better off using a ForeignKey field within django.

[James Bennett]

Please don't modify ticket status anonymously, and if you close a ticket leave a note explaining the rationale.

[alex@…]

We have a absolutely duplicate ticket:2718 on the same issue with same patch suggested.
I hope nobody will be angry on me, I closed ticket:2718 and kept this one as being "earlier" :)

This is very trivial fix and I have to maintain it on all my Django powered severs. I am voting to push this change into
main tree.

[bram.dejong+django@…]

I experience the same trouble, and independently found the same fix... When is this planned to be included in trunk?

[Simon G. <dev@…>]

I'll move this to ready to checkin once someone works up a patch.

--Simon

[Simon G. <dev@…>]

...and some tests would be nice too.

[alex@…]

trivial patch

[alex@…]

I've attached the trivial patch which I am using last months on all django installs.

[Jeremy Dunck <jdunck@…>]

I've tried to work up a test, but have run into trouble due to tight coupling.

I think it may be possible using the web client, but a simple model-based regression test doesn't seem possible. It looks like no other tests require the web client, so I'm reluctant to add the first one.

I think the trivial patch given is wrong, in that it misses the point. I think the real problem is that one_to_one_rel is being contributed to the related class, e.g. "place" when it ought to be contributed to the referencing class (e.g. restaurant). I think one_to_one_field is basically just supposed to be a well-known name, like self.pk.

I'll attach a different patch in a moment, but note the patch solved two problems for me: this one, and the fact that limit_choices_to on Restaurant was causing the *Place* change list to be filtered based on the limit_choices_to kwargs.

This pointed me in what I think is the right direction: one_to_one_field should be contributed to restaurant, not place.

All existing tests still pass with the new patch.

[Jeremy Dunck <jdunck@…>]

Contribute to restaurant, not place.

[Ilya Semenov <semenov@…>]

+1 on this patch, just run into the same issue and did the same fix. I see absolutely no rationale behind this limitation (list_filter disabled if there's at least one OneToOneField).

[sgt.hulka@…]

What needs to be done to push this into trunk?

[Jos@…]

I've run into the same problem, waiting for this to be pushed to trunk.

[anonymous]

Hmm.. not sure why I was so eager for tests. I've moved this on, and thanks for reminding me :)

[Malcolm Tredinnick]

Something's not right about this patch. With current trunk ([5983]) and the following two models:

from django.db import models

class Place(models.Model):
    name = models.CharField(max_length=50)

    def __unicode__(self):
        return u"%s the place" % self.name

class Restaurant(models.Model):
    place = models.OneToOneField(Place)
    serves_hot_dogs = models.BooleanField()
    serves_pizza = models.BooleanField()

    class Admin:
        list_display = ('place', 'serves_hot_dogs', 'serves_pizza')
        list_filter = ('serves_hot_dogs', 'serves_pizza')

    def __unicode__(self):
        return u"%s the restaurant" % self.place.name

the list filtering displays and works correctly without the patch and is broken when the patch is applied. Current behaviour looks to be correct for this particular example case.

On the surface, Jeremy's logic in comment 11 looks correct. In practice, something is going wrong.

[Peter Baumgartner]

list_filter appears to be working in current svn with OneToOneField. Jeremy's patch no longer breaks Malcolm's example above.

Neither svn nor Jeremy's patch fix the other issue with limit_choices_to.

[anonymous]

Filed ticket #5497 for limit_choices_to bug

[jos@…]

I still see this problem in svn (revision 6463)
It can be recreated with the following model:

from django.db import models

class System(models.Model):
    name = models.CharField(maxlength=255)

    class Admin:
        list_filter = ( 'name',  )

class SystemDeploy(models.Model):
    system=models.OneToOneField(System)
    currentversion = models.IntegerField()

When system=models.OneToOneField(System) is commented the filters show up.

The initial patch created by alex works for me

[Jeremy Dunck]

FWIW, I've stopped using OneToOneField in favor of ForeignKey(unique=True, related_name='<some singular>_one').

OneToOneField is to be dropped some day, if I remember the mail thread correctly (can't find it now).

I think it's a bug that ForiegnKey(unique=True) results in a related queryset rather than a single object descriptor. But that's another matter.

For now, this works (omitted as before):

class Restaurant(models.Model):
    place = models.ForeignKey(Place, unique=True, related_name='restaurant_one')
    ....
...
>>> p=Place.objects.all()[0]
>>> r = p.restaurant_one.all()[0]

A bit ugly, but not buggy.

[Ilya Semenov]

I don't think the 1-to-1 relation will be dropped away. It's one of the fundamental database concepts, and officially replacing it with unique-1-t-m would be just plain wrong (since in many times that's like replacing O(1) complexity to O(N)).

Still hoping for the bugfix to be applied. I'm also wondering why the 1-to-1 problem was not arised during September Sprint - personally, I forgot about it, using the proposed patch and "empty" records instead of 1-to-1 NULLs for several months.

[Jeremy Dunck]

@semenov:

The fact that it didn't rise to attention in the Sept sprint suggests that not that many people (particularly core committers) are using OneToOne.

As for 1-1 being cheaper than 1-m, in practical application, accessing the descriptor of a 1-1 field goes and fetches the related record by it's primary key, as does a FK descriptor access. Even if you're talking about querying across the table, it usually goes something like Related.objects.get(o2osomeattribute='x'), which IIRC, Django doesn't munge into a subquery, but rather uses a join anyway. (Arguably another bug.)

The fact is that 1-1 is an edge case in lots of code, and fixing the bugs isn't very easy. It needs a champion for it to live, I think.

(I'm not a committer, but my tea-reading skills are fairly good.)

[remco@…]

"The semantics of one-to-one relationships will be changing soon, so we don’t recommend you use them. If that doesn’t scare you away, keep reading."

-- ​http://www.djangoproject.com/documentation/model-api/#one-to-one-relationships

@jdunck: I don't know exactly how the 1-1 not being part of the sprint would suggest that not too many people are using it, as I can equally well state that maybe there was a lot of other stuff to improve and change during the sprint. But I agree with semenov that 1-1 relationships is a fundamental part of defining relationships in data models. So I for one hope this issue get's fixed over time.

[Ilya Semenov]

As for 1-1 being cheaper than 1-m, in practical application, accessing the descriptor of a 1-1 field goes and fetches the related record by it's primary key, as does a FK descriptor access

That is only correct in the current (broken) implementation, where a 1-to-1 relation effectively can not have null=True (well you can describe it as NULL, but the ORM code would break on fetching, and I didn't manage to find a simple way to fix that). Once the 1-to-1 is (hopefully) fixed to allow NULL values, it becomes zero time to fetch a missing 1-to-1 value, and a separate database lookup to fetch a missing unique-1-to-m value.

Besides of that, it degrades the code readability to have a collection rather than a value where it just doesn't make sense. For example, consider User and Profile models:

if user.profile:
 phone = user.profile.phone
else:
 phone = DEFAULT_PHONE

The 1-to-m approach would lead to unneeded overhead (in terms of both effeciency and the code clarity):

if user.profile_set.count():
 phone = user.profile_set.get().phone
 # and you always pray it doesn't crash with an AssertionError for 2+ objects
else:
 phone = DEFAULT_PHONE

[alex@…]

This discussion needs to be moved to django-developers, because otherwise we add too much unrelated theoretical information to ticket.

[alex@…]

​http://groups.google.com/group/django-developers/browse_thread/thread/62367e3b7b4c199d/add914e5b25a55f0#add914e5b25a55f0

[Tyson Clugg]

I've just experienced similar problems in trunk (now that the ModelInheritance branch has been merged).

I noticed that the OneToOneField.init has kwargseditable=False hard coded, which breaks how we were using OneToOneFields. The patch I'm submitting restores OneToOneField behaviour to pre ModelInheritance status by checking if the field has parent_link=True before setting editable=False.

[Tyson Clugg]

Patch to allow editing OneToOneFields

[Tyson Clugg]

My last comment and the patch I submitted do not belong on this ticket - I should have created a new ticket (the patch applies to a new defect). The new ticket (#7947) has been created, please disregard the patch and comment of mine above.

[anonymous]

Because multi-table inheritance is achieved using implicit OneToOneFields, any non-abstract parent model classes will not have the list_filter functionality:

class Place(models.Model):
    name = models.CharField(max_length=50)
    city = models.CharField(max_length=50)

class Restaurant(Place):
    serves_pizza = models.BooleanField()

In the example above, Restaurant will have filter functionality, but Place won't!

This issue needs to be resolved!!!

[Brian Rosner]

I am going to commit the original patch. There has been a ton of refactoring since the queryset-refactor branch landed completely overhualing OneToOneFields. I have tested the first patch and it works. I would rather close this as fixed with that change (to me it doesn't seem correct to limit the filtering on that condition). Then if any new bugs surface lets clean those up in new tickets.

[Brian Rosner]

(In [8388]) Removed some checks for Model._meta.one_to_one_field to prevent list_filter and the show_result_count in search_form.html. Fixes #2145.

[Brian Rosner]

For the record, that should have read "that prevented" and not "to prevent" :)

[Jacob]

Milestone 1.0 deleted