Opened 13 years ago

Closed 13 years ago

#15355 closed (invalid)

Choices on ForeignKey in Inlines not limited to the model constraints

Reported by: Stan Owned by: nobody
Component: contrib.admin Version: 1.3-beta
Severity: Keywords: inline foreignkey admin
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I think the following behavior in the Admin is a bug. The following (simplified) model allow you to add Emails to an Advertiser for some of its Offices.

Everything take place in the Advertiser Change page :

  1. Add/delete offices via StackedInlines ;
  2. add/delete emails via TabularInlines.
# models.py

class Address(models.Model):
    ...
    class Meta:
        abstract = True

class Advertisor(Address):
    ...

class AdvertisorOffice(Address):
    advertisor = models.ForeignKey(Advertisor)
    ...

class EmailAdvertisor(models.Model):
    advertisor = models.ForeignKey(Advertisor)
    address = models.ForeignKey(Advertisor, blank=True, null=True)
    support = models.ForeignKey(Support, blank=True, null=True)
    ...


# admin.py

class AdvertisorAdmin(admin.ModelAdmin):
    ...
    inlines = [
        AdvertisorOfficeInline,
        EmailAdvertisorInline,
        ...
    ]
admin.site.register(models.Advertisor, AdvertisorAdmin)

class AdvertisorOfficeInline(admin.StackedInline):
    model = models.AdvertisorOffice

class EmailAdvertisorInline(admin.TabularInline):
    model = models.EmailAdvertisor
    extra = 1
    ...

The thing is the select field to choose an Office in the Email inlines is not limited to the Offices of the current Advertisor. The full table is present in the choices and I have to make some Ajax to set the proper ones because overriding the queryset in the init() of a custom form is not possible, the advertisor_id not being present in the initial form attribute for some reasons.
Another side-effect is the impossibility to benefit of the overriding of the queryset method to make some select_related. My current workaround being a custom form and field.queryset redefined in the init :

class EmailAdvertisorForm(forms.ModelForm):
    class Meta:
        model = models.EmailAdvertisor
    def __init__(self, *args, **kwargs):
        super(EmailAdvertisorForm, self).__init__(*args, **kwargs)
        # We cannot use none() here despite the Ajax reset if we want a valid form.
        offices = AdvertisorOffice.objects.all().select_related('advertisor')
        if self.instance.pk and self.instance.advertisor:
            offices = self.instance.advertisor.officeadvertisor_set.all()
        self.fields["address"].queryset = offices

Kindly,

Stan.

Attachments (3)

advertisor1.png (125.2 KB ) - added by Stanislas Guerra <stanislas.guerra@…> 13 years ago.
advertisor2.png (103.4 KB ) - added by Stan 13 years ago.
Advertisor without office
bug_15355.tgz (9.9 KB ) - added by Stan 13 years ago.
Django project with fixtures to illustrate the bug

Download all attachments as: .zip

Change History (6)

comment:1 by Ramiro Morales, 13 years ago

Resolution: invalid
Status: newclosed

There is no direct relation between your EmailAdvertisorInline and AdvertisorOffice models so there is no way to get a selecte field there.

As posted, the code seems to be have been stripped from some helpful fields (at least one field in Address or Advertisor would do) and still have an unneeded extra field (EmailAdvertisor.support), also select_related options are needed in the EmailAdvertisorInline FKs. Also, ordering of the !Inlines and ModelAdmin definitions don't give a working admin setup.

Reopen if you can refactor your sample code to demonstrate the issue you want to report, and please try to really to extract/reduce it to a working case so somebody can triage the ticket without having to guess and spend time trying to adapt it to pass validation.

by Stanislas Guerra <stanislas.guerra@…>, 13 years ago

Attachment: advertisor1.png added

comment:2 by Stan, 13 years ago

Resolution: invalid
Status: closedreopened

Many thanks Ramiro for reviewing the bug.

There is an error in the models I have posted above : the second FK (address) in EmailAdvertisorInline is for AdvertisorOffice.
I have attached a small Django project with fixtures (admin:admin) and some screenshots to illustrate the case.

In advertisor1.png you can see an advertisor having 2 offices. In advertisor2.png, in the Email's inlines, the Advertisor1's offices are present.

Here the working models.py :

from django.db import models
from django.contrib.sites.models import Site, SiteManager


class Support(Site):
    short_name = models.CharField(max_length=10)
    logo = models.ImageField('logo', upload_to='images/supports', blank=True)
    objects = SiteManager()


class Address(models.Model):
    name = models.CharField(max_length=200, blank=True)
    memo = models.CharField(max_length=50)
    short_name = models.CharField(max_length=200, null=True, blank=True)
    address = models.CharField(max_length=200, blank=True)

    class Meta:
        abstract = True

    def __unicode__(self):
        return '%s' % self.memo


class Advertisor(Address):
    euid = models.IntegerField(blank=True, null=True)
    login = models.CharField(max_length=30, unique=True)
    password = models.CharField(max_length=30, blank=True)


class AdvertisorOffice(Address):
    name_selected = models.BooleanField('')
    short_name_selected = models.BooleanField('')
    address_selected = models.BooleanField('')
    advertisor = models.ForeignKey(Advertisor)


TYPE_EMAIL_CHOICES = (
    ('BAT', 'BAT'),
)


class EmailAdvertisor(models.Model):
    advertisor = models.ForeignKey(Advertisor)
    address = models.ForeignKey(AdvertisorOffice, blank=True, null=True)
    support = models.ForeignKey(Support, blank=True, null=True)
    email_type = models.CharField('type', max_length=10, choices=TYPE_EMAIL_CHOICES, default='BAT')
    email = models.EmailField()

And the admin.py :

from django.contrib import admin
from bug_15355.myapp import models


class AdvertisorOfficeInline(admin.StackedInline):
    model = models.AdvertisorOffice
    extra = 0
    can_delete = False
    fieldsets = (
        (None, {
                'fields': (('memo',), 
                           ('name_selected', 'name'),
                           ('short_name_selected', 'short_name'),
                           ('address_selected', 'address')),
                'description': 'Select to override the advertisor data.'
                }),
        )


class EmailAdvertisorInline(admin.TabularInline):
    model = models.EmailAdvertisor
    extra = 1

class AdvertisorAdmin(admin.ModelAdmin):
    fieldsets = (
        (None, {
            'fields': (('memo',), 
                       ('name', 'short_name',),
                       'address'),
            }),
        ('Authentification', {
            'fields': ('login', 'password')
            }),
        )
    inlines = [
        AdvertisorOfficeInline,
        EmailAdvertisorInline,
    ]


admin.site.register(models.Support)
admin.site.register(models.Advertisor, AdvertisorAdmin)
 

Thanks.

ps : If you need manpower to investigate, just let me know.

by Stan, 13 years ago

Attachment: advertisor2.png added

Advertisor without office

by Stan, 13 years ago

Attachment: bug_15355.tgz added

Django project with fixtures to illustrate the bug

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

Resolution: invalid
Status: reopenedclosed

The fact that the address of an EmailAdvertisor needs to be restricted to a subset is a business decision. It isn't something that can be automatically assumed from the models you have provided. Mere existence of a foreign key isn't enough to imply that a relation should be constrained.

Encoding this business rule is something that limit_choices_to is intended to address.

So, as far as I can make out, this isn't a bug.

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