Code

Opened 2 years ago

Last modified 21 months ago

#18357 new Bug

BaseGenericInlineFormSet in Django Admin does not set generic foreign key fields when constructing form

Reported by: buettgenbach@… Owned by: nobody
Component: contrib.contenttypes Version: 1.3
Severity: Normal 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

For a BaseInlineFormSet in _construct_form the fk value is set in order to do validation.

    def _construct_form(self, i, **kwargs):
        ...
        # Set the fk value here so that the form can do it's validation.
        setattr(form.instance, self.fk.get_attname(), self.instance.pk)
        return form

Something similar is missing for BaseGenericInlineFormSet. Actually it has been removed in https://github.com/django/django/commit/856a39e841080abc34e8ae3bb2b20f780c33762d#L0R329.
As a consequence during model based cleaning the generic foreign key attribute cannot be accessed/cleaned because it is not set. Assume the following fictional code:

class Poll(models.Model):
    question = models.CharField(max_length=200)
    pub_date = models.DateTimeField('date published')

class GenericChoice(models.Model):
    content_type = models.ForeignKey(ContentType)
    content_id = models.PositiveIntegerField()
    content = generic.GenericForeignKey('content_type', 'content_id')
    choice = models.CharField(max_length=200)
    votes = models.IntegerField()
    
    def clean(self):
        if self.votes > 10 and self.content.question == 'Don't allow more than 10 votes':
            raise ValidationError('Test')

This results in an NoneType error since self.content is None.
With something like this in BaseGenericInlineFormSet it could be fixed:

    def _construct_form(self, i, **kwargs):
        # Avoid a circular import.
        from django.contrib.contenttypes.models import ContentType
        form = super(BaseGenericInlineFormSet, self)._construct_form(i, **kwargs)
#        if self.save_as_new:
#            # Remove the key from the form's data, we are only creating new instances.
#            form.data[form.add_prefix(self.ct_fk_field.name)] = None
#            form.data[form.add_prefix(self.ct_field.name)] = None

        # Set the GenericForeignKey value here so that the form can do its validation.
        setattr(form.instance, self.ct_fk_field.attname, self.instance.pk)
        setattr(form.instance, self.ct_field.attname, ContentType.objects.get_for_model(self.instance).pk)
        return form

But this would only fix those cases where the parent model already exists (Admin edit). If the parent does not exist (for example when creating a new Poll with some GenricChoice inline sets) the generic foreign key field cannot be accessed since content_id is not set (and the _cache_content is not set, too). Using a simple BaseInlineFormSet this is achieved by setting the foreing key object through the InlineForeignKeyField which is instantiated with the foreign key object during add_fields in BaseInlineFormSet.

Attachments (0)

Change History (1)

comment:1 Changed 21 months ago by lrekucki

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


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

 
Note: See TracTickets for help on using tickets.