Code

Opened 3 years ago

Closed 3 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: UI/UX:

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@…> 3 years ago.
advertisor2.png (103.4 KB) - added by Stan 3 years ago.
Advertisor without office
bug_15355.tgz (9.9 KB) - added by Stan 3 years ago.
Django project with fixtures to illustrate the bug

Download all attachments as: .zip

Change History (6)

comment:1 Changed 3 years ago by ramiro

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed

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.

Changed 3 years ago by Stanislas Guerra <stanislas.guerra@…>

comment:2 Changed 3 years ago by Stan

  • Resolution invalid deleted
  • Status changed from closed to reopened

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.

Changed 3 years ago by Stan

Advertisor without office

Changed 3 years ago by Stan

Django project with fixtures to illustrate the bug

comment:3 Changed 3 years ago by russellm

  • Resolution set to invalid
  • Status changed from reopened to closed

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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.