Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#25942 closed Bug (fixed)

TypedChoiceField.has_changed behaviour has changed between 1.8 and 1.9

Reported by: Arthur Vuillard Owned by: nobody
Component: Forms Version: 1.9
Severity: Release blocker Keywords:
Cc: 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

We've met a strange bug with Django 1.9 on TypedChoiceField.has_changed.

We encountered the bug when using a TypedChoiceField in a form of a formset. When we have an empty extra form, and we submit all the form, the submitted form is not validated due to some "changes" made to the TypedChoiceFields.

The excepted behavior would be to save the formset data without treating that extra form.

I finally found that the method has_changed of a field has been modified from 1.8, and calls self._coerce for the initial value. By removing it, my use case now works like expected.

I attach to this ticket a unit test that reproduces my case.

Attachments (1)

test.patch (722 bytes) - added by Arthur Vuillard 4 years ago.
Unit test

Download all attachments as: .zip

Change History (9)

Changed 4 years ago by Arthur Vuillard

Attachment: test.patch added

Unit test

comment:1 Changed 4 years ago by Tim Graham

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

I haven't completely verified the report, but bisected the change to 8714403614c4dfa37e806db4a6708b8a91a827a4.

comment:2 Changed 4 years ago by Claude Paroz

It would help to know a bit more about the use case, notably because your test didn't set the coerce argument. Could you provide a little more context, also about the form classes involved?

comment:3 Changed 4 years ago by Arthur Vuillard

In fact, in my case, the coerce is used using the field's to_python method as following (my mistake, sorry) :

from django.db import models
from django.forms import TypedChoiceField

CHOICES = [
    ('choice1', 'Choice 1'),
    ('choice2', 'Choice 2'),
    ('choice3', 'Choice 3')
]

my_choices = models.CharField(
    max_length=7,
    choices=CHOICES,
    null=True,
    blank=True,
    default=None,
)


kwargs = {
    'label': 'Method',
    'choices': [
        ('', '---------'),
        ('choice1', 'Choice 1'),
        ('choice2', 'Choice 2'),
        ('choice3', 'Choice 3')
    ],
    'initial': None,
    'empty_value': None,
    'required': False,
    'help_text': '',
    'coerce': my_choices.to_python,
}

tcf = TypedChoiceField(**kwargs)
# the following line did work in django 1.8
assert not tcf.has_changed(None, '')

Overall, I detected that when using my model in a formset having an extra=1. When submitting without touching anything, the form is treated as a changed form due to this changed field (where it should be ignored as nothing was changed).

comment:4 Changed 4 years ago by Claude Paroz

Has patch: set

That's an issue with CharField and null=True (which is discouraged). However, I think the PR fixes this.

comment:5 Changed 4 years ago by Arthur Vuillard

I can confirm that the PR fixes the bug on my code. Thanks for that !

comment:6 Changed 4 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:7 Changed 4 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: newclosed

In d91cc25:

Fixed #25942 -- Fixed TypedChoiceField.has_changed with nullable field

This fixes a regression introduced by 871440361.

comment:8 Changed 4 years ago by Claude Paroz <claude@…>

In ff077cd:

[1.9.x] Fixed #25942 -- Fixed TypedChoiceField.has_changed with nullable field

This fixes a regression introduced by 871440361.
Backport of d91cc25a2a from master.

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