Re: [Django] #11716: Various methods in django.db.models.fields don't wrap ValueErrors and allow them to escape
Django
them to escape
-------------------------------------+-------------------------------------
Reporter: Leo | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Leo):
* status: assigned => new
* owner: Leo =>
Comment:
Not sure if this is still an issue but I'm not going to be able to do an
iteration of the patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/11716#comment:13>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
them to escape
-------------------------------------+-------------------------------------
Reporter: Leo | Owner:
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: => albertoconnor
* status: new => assigned
* cc: albertoconnor (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/11716#comment:14>
Django
them to escape
-------------------------------------+-------------------------------------
Reporter: Leo | Owner:
| albertoconnor
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by albertoconnor):
I have been able to verify that the issue isn't reproduced in master
locally using the following models.py and form.py
{{{
# models.py
from django.db import models
class Bar(models.Model):
name = models.TextField()
class Foo(models.Model):
bar = models.ForeignKey(Bar, blank=True, null=True)
}}}
{{{
# forms.py
from django import forms
from .models import Foo, Bar
class FooForm(forms.ModelForm):
def __init__(self, *args, **kwargs):
super(FooForm, self).__init__(*args, **kwargs)
bars = list(Bar.objects.order_by('name'))
self.fields['bar'].choices = [('---','---')] + [(b.id, b.name) for
b in bars]
class Meta:
model = Foo
fields = ['bar']
}}}
If I run the following
{{{
>>> form = FooForm(dict(bar='---'))
>>> form.is_valid()
False
>>> form.errors
{'bar': [u'Select a valid choice. That choice is not one of the available
choices.']}
}}}
If I pass in something else like 'f' for bar it fails in the same way. The
interesting thing about '---' is it is actually a choice which was
inserted in the choices option for the field.
I believe the next step to write a test which tests this case so we can
make sure there isn't a regression. The test will also test this case
through CI. There seem to be tests for the IntegerField case already so I
will write a test for the ModelChoiceField case first thing tomorrow
morning.
--
Ticket URL: <https://code.djangoproject.com/ticket/11716#comment:15>
Django
them to escape
-------------------------------------+-------------------------------------
Reporter: Leo | Owner:
| albertoconnor
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by timgraham):
albertoconnor, just wanted to suggest to use git bisect to find the commit
where this was fixed to see what tests were added there (to prevent adding
duplicate tests).
--
Ticket URL: <https://code.djangoproject.com/ticket/11716#comment:16>
Django
them to escape
-------------------------------------+-------------------------------------
Reporter: Leo | Owner:
| albertoconnor
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by albertoconnor):
Thanks tim, will do.
--
Ticket URL: <https://code.djangoproject.com/ticket/11716#comment:17>
Django
them to escape
-------------------------------------+-------------------------------------
Reporter: Leo | Owner:
| albertoconnor
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
After bisecting I discovered commit
9e04c3b7444ba136efa03896a67e46d2e7045e28 which fixes the issue and does
add a test which seems tests the issue raised in this bug.
--
Ticket URL: <https://code.djangoproject.com/ticket/11716#comment:18>
Django
them to escape
-------------------------------------+-------------------------------------
Reporter: Leo | Owner:
| albertoconnor
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Leo):
Thank you albertoconnor for putting this bug to rest!
--
Ticket URL: <https://code.djangoproject.com/ticket/11716#comment:19>