Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#26917 closed Bug (fixed)

Disabled ModelChoiceFields crash in Django 1.10

Reported by: karyon Owned by: Ryan Schave
Component: Forms Version: 1.10
Severity: Release blocker Keywords:
Cc: Claude Paroz Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

i have several tests that fail when trying to submit a form that has disabled fields in 1.10, which worked in 1.9, and also works when making it disabled=False. example stacktrace:

Traceback (most recent call last):
  File "/vagrant/evap/staff/tests/test_forms.py", line 105, in test_single_result_form_saves_participant_and_voter_count
    self.assertTrue(form.is_valid())
  File "/vagrant/src/django/django/forms/forms.py", line 161, in is_valid
    return self.is_bound and not self.errors
  File "/vagrant/src/django/django/forms/forms.py", line 153, in errors
    self.full_clean()
  File "/vagrant/src/django/django/forms/forms.py", line 364, in full_clean
    self._post_clean()
  File "/vagrant/src/django/django/forms/models.py", line 401, in _post_clean
    self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude)
  File "/vagrant/src/django/django/forms/models.py", line 60, in construct_instance
    f.save_form_data(instance, cleaned_data[f.name])
  File "/vagrant/src/django/django/db/models/fields/__init__.py", line 867, in save_form_data
    setattr(instance, self.name, data)
  File "/vagrant/src/django/django/db/models/fields/related_descriptors.py", line 203, in __set__
    self.field.remote_field.model._meta.object_name,
ValueError: Cannot assign "1": "Course.semester" must be a "Semester" instance.

code for the form is here (it's about the semester attribute, and ignore the two methods, this also fails when removing those) and test is here.

this regressed in db19619545dd99a1d2502c72974d79eca33acff7.

Attachments (1)

Refs__26917___Added_test_to_duplicate_problem___Added_test_to_duplicate_problem_in_which_a.patch (1.2 KB) - added by Ryan Schave 3 years ago.
Added unit test to duplicate problem

Download all attachments as: .zip

Change History (9)

comment:1 Changed 3 years ago by karyon

Component: UncategorizedForms

comment:2 Changed 3 years ago by Tim Graham

Severity: NormalRelease blocker
Summary: Disabled fields don't validate anymore in 1.10Disabled ModelChoiceFields crash in Django 1.10
Triage Stage: UnreviewedAccepted

comment:3 Changed 3 years ago by Ryan Schave

Owner: changed from nobody to Ryan Schave
Status: newassigned

Changed 3 years ago by Ryan Schave

Added unit test to duplicate problem

comment:4 Changed 3 years ago by Tim Graham

Cc: Claude Paroz added

Claude, I'm not sure how to proceed here. It seems that the assumption about "Initial values are supposed to be clean" isn't correct for foreign keys. They're converted to primary key in model_to_dict and (previously) back to model instances in clean(). Now that latter conversion is skipped.

comment:5 Changed 3 years ago by Claude Paroz

If we don't find a better solution, we could partially revert that faulty part of the patch, like I did when backporting to 1.9 (#25532). But then a disabled JSONField will be non-functional. It may be less critical, though.

comment:6 Changed 3 years ago by Tim Graham

Has patch: set

PR to partially revert and add the regression test from Ryan.

I created #26949 for the issue about disabled JSONField's crashing.

comment:7 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In a5f85d89:

Fixed #26917 -- Fixed crash in disabled ModelChoiceFields.

Partially reverted refs #25532 to fix a regression in Django 1.10.
This reintroduces a crash for disabled forms.JSONField (refs #26949),
however, that issue is also present on Django 1.9.

Thanks Ryan Schave for the test.

comment:8 Changed 3 years ago by Tim Graham <timograham@…>

In 3744fc1:

[1.10.x] Fixed #26917 -- Fixed crash in disabled ModelChoiceFields.

Partially reverted refs #25532 to fix a regression in Django 1.10.
This reintroduces a crash for disabled forms.JSONField (refs #26949),
however, that issue is also present on Django 1.9.

Thanks Ryan Schave for the test.

Backport of a5f85d891b51d7ceb4f9e422e3e4f5c741062288 from master

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