Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#14880 closed (fixed)

raw_id_fields in admin does not work when limit_choices_to dictionary has value=False

Reported by: smallming@… Owned by: Luke Plant
Component: contrib.admin Version: 1.2
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The related-lookup popup does not properly filter the queryset when the limit_choices_to dictionary has value=False. The models.py and admin.py demonstrate the problem.


To reproduce:

  1. Using the following models.py and admin.py, do
    Publisher(name='local', overseas=False).save()
    Publisher(name='overseas', overseas=True).save()
    
  2. Inside admin interface > add book, select related-lookup link to display popup. Only publisher "overseas" is available. Should display publisher "local" only.


Work-around: setting limit_choices_to={'overseas': 0} works, but this work-around is less intuitive than a fix.


models.py

from django.db import models

class Publisher(models.Model):
    name        = models.CharField(max_length=20)
    overseas    = models.BooleanField()

class Book(models.Model):
    title           = models.CharField(max_length=20)
    local_publisher = models.ForeignKey(Publisher, limit_choices_to={'overseas': False})

admin.py

from testproj.testapp.models import *
from django.contrib import admin

class BookAdmin(admin.ModelAdmin):
    raw_id_fields = ['local_publisher',]

class PublisherAdmin(admin.ModelAdmin):
    list_display = ['name', 'overseas']

admin.site.register(Book, BookAdmin)
admin.site.register(Publisher, PublisherAdmin)

Attachments (4)

ticket_14880_tests_1.2.3.diff (3.2 KB ) - added by Luke Plant 13 years ago.
Tests - patch against 1.2.3
ticket_14880_tests_1.2.4.diff (3.1 KB ) - added by Luke Plant 13 years ago.
Tests - patch against 1.2.4
ticket_14880_tests_1.2.X.diff (3.2 KB ) - added by Luke Plant 13 years ago.
Tests - patch against 1.2.X
ticket_14880_tests_trunk.diff (3.2 KB ) - added by Luke Plant 13 years ago.
Tests - patch against trunk

Download all attachments as: .zip

Change History (15)

comment:1 by Julien Phalip, 13 years ago

I tried your example, but when clicking the hourglass to select a publisher, the popup crashes with an error: SuspiciousOperation: Filtering by overseas not allowed. What version of Django are you using?

in reply to:  1 comment:2 by smallming@…, 13 years ago

Replying to julien:

I tried your example, but when clicking the hourglass to select a publisher, the popup crashes with an error: SuspiciousOperation: Filtering by overseas not allowed. What version of Django are you using?

I'm using 1.2.1. Tested using manage.py runserver.

comment:3 by Russell Keith-Magee, 13 years ago

milestone: 1.3
Triage Stage: UnreviewedAccepted

Confirmed that this worked in 1.2.3, but doesn't in 1.2.4 or trunk. This makes it a release blocker for 1.3.

comment:4 by Russell Keith-Magee, 13 years ago

Keywords: blocker regression added

comment:5 by Luke Plant, 13 years ago

I don't think this was ever working. I have tried with a real life project, and with tests, and I get failure on 1.2.3. On 1.2.4 I get the suspicious operation thing. The original reporter was using 1.2.1 apparently, so this doesn't look like a regression.

I will attach patches for the tests - against 1.2.3, 1.2.4, most recent 1.2.X and most recent trunk, all slightly different.

by Luke Plant, 13 years ago

Tests - patch against 1.2.3

by Luke Plant, 13 years ago

Tests - patch against 1.2.4

by Luke Plant, 13 years ago

Tests - patch against 1.2.X

by Luke Plant, 13 years ago

Tests - patch against trunk

comment:6 by smallming, 13 years ago

I will like to add a line to admin.py in the initial bug report so it properly reflects the bug and "skip" SuspiciousOperation exception. Reason below.

class PublisherAdmin(admin.ModelAdmin):
    list_display = ['name', 'overseas']
    list_filter = ['overseas'] # Added for illustration

I was tracing the bug using 1.2.4, and found that the SuspiciousOperation is due to the lack of corresponding
ModelAdmin.list_filter specified. Adding list_filter = ['overseas'] to PublisherAdmin will lead to the original bug reported (at least in 1.2.4).

Perhaps a separate bug can be filed under "raw_id_fields in admin raise SuspiciousOperation when limit_choices_to is used without corresponding ModelAdmin.list_filter".

comment:7 by Ramiro Morales, 13 years ago

The SuspiciousOperation error you see in 1.2.4 is because of a problem in the security fix that triggered its release, it was fixed in r15140 and it is also already fixed in trunk. We can ignore the noise introduced by that.

Regarding he original report of this ticket, I can trace back this problem showing itself as far as Django 1.1 (and even 1.0, but there bugs like #9561 introduce additional noise)(tested with sqlite3).

In such environment, clicking in the magnifying glass icon opens a popup where only the overseas publisher is shown. Later, when trying to save the PublishedBook instance, validation rejects it so it seems there the filter overseas=False filter/validation is working in that stage.

comment:8 by Luke Plant, 13 years ago

Keywords: blocker regression removed
milestone: 1.3

OK, so this isn't a regression, we can remove the blocker/regression keywords. It would still be nice to be fixed for 1.3, but we don't have to.

For those looking to fix it, there are probably two routes:

  1. Change the generated query string to use '0' instead of 'False', since '0' does work.
  2. Fix the code that reads the query string to treat 'False' as boolean False (but obviously only for boolean fields).

I have no idea at the moment which is the 'right' way to fix it, one of them is likely to be cleaner that the other.

comment:9 by Luke Plant, 13 years ago

Owner: changed from nobody to Luke Plant
Status: newassigned

I will look at this, as it is related to the fix in #15103

comment:10 by Luke Plant, 13 years ago

Resolution: fixed
Status: assignedclosed

(In [15348]) Fixed #14880 - raw_id_fields in admin does not work when limit_choices_to dictionary has value=False

Thanks to smallming for the report.

comment:11 by Luke Plant, 13 years ago

(In [15351]) [1.2.X] Fixed #14880 - raw_id_fields in admin does not work when limit_choices_to dictionary has value=False

Thanks to smallming for the report.

Backport of [15348] from trunk.

Note: See TracTickets for help on using tickets.
Back to Top