Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#34119 closed Bug (fixed)

ModelForm fields with callable defaults don't correctly propagate default values

Reported by: Benjamin Rigaud Owned by: David Sanders
Component: Forms Version: 4.1
Severity: Normal Keywords: modelform
Cc: David Sanders Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When creating an object via the admin, if an inline contains an ArrayField in error, the validation will be bypassed (and the inline dismissed) if we submit the form a second time (without modification).

  • go to /admin/my_app/thing/add/
  • type anything in plop
  • submit -> it shows an error on the inline
  • submit again -> no errors, plop become unfilled
# models.py

class Thing(models.Model):
    pass


class RelatedModel(models.Model):
    thing = models.ForeignKey(Thing, on_delete=models.CASCADE)

    plop = ArrayField(
        models.CharField(max_length=42),
        default=list,
    )


# admin.py

class RelatedModelForm(forms.ModelForm):
    def clean(self):
        raise ValidationError("whatever")


class RelatedModelInline(admin.TabularInline):
    form = RelatedModelForm
    model = RelatedModel
    extra = 1


@admin.register(Thing)
class ThingAdmin(admin.ModelAdmin):
    inlines = [
        RelatedModelInline
    ]

It seems related to the hidden input containing the initial value:

<input type="hidden" name="initial-relatedmodel_set-0-plop" value="test" id="initial-relatedmodel_set-0-id_relatedmodel_set-0-plop">

I can fix the issue locally by forcing show_hidden_initial=False on the field (in the form init)

Attachments (2)

Screenshot from 2022-10-25 13-28-33.png (19.6 KB ) - added by Benjamin Rigaud 2 years ago.
First submit
Screenshot from 2022-10-25 13-29-00.png (27.4 KB ) - added by Benjamin Rigaud 2 years ago.
Second submit

Download all attachments as: .zip

Change History (13)

by Benjamin Rigaud, 2 years ago

First submit

by Benjamin Rigaud, 2 years ago

Second submit

comment:1 by Mariusz Felisiak, 2 years ago

Can you reproduce this issue with Django 4.1? (or with the current main branch). Django 3.2 is in extended support so it doesn't receive bugfixes anymore (except security patches).

in reply to:  1 comment:2 by Benjamin Rigaud, 2 years ago

Replying to Mariusz Felisiak:

Can you reproduce this issue with Django 4.1? (or with the current main branch). Django 3.2 is in extended support so it doesn't receive bugfixes anymore (except security patches).

Same issue with Django 4.1.2

comment:3 by David Sanders, 2 years ago

This is because the inline form is considered to be changed on the first submission but not on the second submission.

Fields with callable defaults have a hidden widget which then used to detect if it's changed – a mechanism implemented long ago to preserve changing defaults between submissions like timezone.now.

I think the problem here is that the hidden widget is being populated with the submitted data instead of the result of the callable default, thus making the form think it's unchanged. I've submitted a PR to prevent these hidden widgets from being overridden by submitted values.

Last edited 2 years ago by David Sanders (previous) (diff)

comment:4 by David Sanders, 2 years ago

Cc: David Sanders added
Component: UncategorizedForms
Has patch: set
Owner: changed from nobody to David Sanders
Patch needs improvement: set
Status: newassigned
Triage Stage: UnreviewedAccepted
Version: 3.24.1

comment:5 by David Sanders, 2 years ago

For reference here's the original ticket implementing the logic causing behaviour: #7975

Version 0, edited 2 years ago by David Sanders (next)

comment:6 by David Sanders, 2 years ago

Keywords: modelform added; admin arrayfield forms removed
Patch needs improvement: unset
Summary: Admin: ArrayField in inlines are not properly validatedModelForm fields with callable defaults don't correctly propagate default values
Triage Stage: AcceptedReady for checkin

comment:7 by Mariusz Felisiak, 2 years ago

Triage Stage: Ready for checkinAccepted

You shouldn't mark your own PRs as "Ready for Checkin".

comment:8 by Mariusz Felisiak, 2 years ago

#33871 was a duplicate for JSONField.

comment:9 by Carlton Gibson, 2 years ago

Triage Stage: AcceptedReady for checkin

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In 25904db9:

Fixed #34119 -- Prevented callable default hidden widget value from being overridden.

Thanks to Benjamin Rigaud for the report.

comment:11 by GitHub <noreply@…>, 2 years ago

In 60a7bd8:

Refs #34119 -- Skipped test_callable_default_hidden_widget_value_not_overridden when JSONField is not supported.

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