Re: [Django] #15126: Misleading error in ModelForm

[Django]

#15126: Misleading error in ModelForm
-------------------------------------+-------------------------------------
Reporter: ingo@… | Owner: nobody
Type: Bug | Status: new
Milestone: | Component: Forms
Version: 1.2 | Severity: Normal
Resolution: | Keywords: modelform fields
Triage Stage: Accepted | attributeerror widget subset
Needs documentation: 0 | Has patch: 1
Patch needs improvement: 1 | Needs tests: 0
UI/UX: 0 | Easy pickings: 0
-------------------------------------+-------------------------------------
Changes (by ptone):

* needs_better_patch: 0 => 1
* ui_ux: => 0
* easy: => 0

Comment:

I think the test for str is reasonable.

I think this would be better implemented in the __init__ of
ModelFormOptions

also the test should use assertRaises to actually test that the assertion
is raised on instantiation, rather than fail after a class is defined,
which doesn't actually test your code.

A couple minor points:

* I believe you don't need to test for None, isinstance should suffice

* the error should be that the option should be a iterable of strings, not
a single string - list is too specific here

* You don't need to determine the type of the errant value, as you only
raise the exception for str type anyway.

--
Ticket URL: <https://code.djangoproject.com/ticket/15126#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#15126: Misleading error in ModelForm
-------------------------------------+-------------------------------------
Reporter: ingo@… | Owner: nobody
Type: Bug | Status: new

Component: Forms | Version: 1.2
Severity: Normal | Resolution:
Keywords: modelform fields | Triage Stage: Accepted
attributeerror widget subset | Needs documentation: 0

Has patch: 1 | Patch needs improvement: 0

Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Changes (by bmispelon):

* cc: bmispelon@… (added)
* needs_better_patch: 1 => 0

Comment:

I fixed up the proposed patch a little and brought it up to date,
adressing some of the points made above.

I opted not to put the validation inside the `ModelFormOptions.__init__`,
so that I would have access to the ModelForm's class in order to provide a
more helpful message.

Here's the corresponding pull request:
https://github.com/django/django/pull/1000

I added two extra tests (on top of the two new ones for this particular
feature):

* Make sure that a `FieldError` is raised when passing an inexisting field
to `Meta.fields`.
* Make sure that no errors are raised when passing an inexisting field to
`Meta.exclude`.

--
Ticket URL: <https://code.djangoproject.com/ticket/15126#comment:6>

[Django]

#15126: Misleading error in ModelForm
-------------------------------------+-------------------------------------
Reporter: ingo@… | Owner: nobody

Type: Bug | Status: closed
Component: Forms | Version: 1.2
Severity: Normal | Resolution: fixed

Keywords: modelform fields | Triage Stage: Accepted
attributeerror widget subset | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Changes (by Baptiste Mispelon <bmispelon@…>):

* status: new => closed
* resolution: => fixed

Comment:

In [changeset:"f9dc1379b874f80694a80e95f0f266a3d42f7368"]:
{{{
#!CommitTicketReference repository=""
revision="f9dc1379b874f80694a80e95f0f266a3d42f7368"
Fix #15126: Better error message when passing invalid options to
ModelForm.Meta.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/15126#comment:7>

[Django]

#15126: Misleading error in ModelForm
-------------------------------------+-------------------------------------
Reporter: ingo@… | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 1.2
Severity: Normal | Resolution: fixed
Keywords: modelform fields | Triage Stage: Accepted
attributeerror widget subset | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by Jannis Leidel <jannis@…>):

In [changeset:"b5c0b3d1d9328e67baa9c7cc98ac38be3d94a0ec"]:
{{{
#!CommitTicketReference repository=""
revision="b5c0b3d1d9328e67baa9c7cc98ac38be3d94a0ec"
Merge pull request #1000 from bmispelon/ticket-15126

Fix #15126: Better error message when passing invalid options to

ModelFo...
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/15126#comment:8>

[Django]

#15126: Misleading error in ModelForm
-------------------------------------+-------------------------------------
Reporter: ingo@… | Owner: nobody

Type: Uncategorized | Status: closed
Component: contrib.admin | Version: master

Severity: Normal | Resolution: fixed
Keywords: modelform fields | Triage Stage:

attributeerror widget subset | Unreviewed
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 1
-------------------------------------+-------------------------------------
Changes (by ogpcludi <sample@…>):

* needs_better_patch: 0 => 1

* component: Forms => contrib.admin
* needs_tests: 0 => 1
* version: 1.2 => master
* easy: 0 => 1
* needs_docs: 0 => 1
* ui_ux: 0 => 1
* type: Bug => Uncategorized
* stage: Accepted => Unreviewed

Comment:

1

--
Ticket URL: <https://code.djangoproject.com/ticket/15126#comment:9>

[Django]

#15126: Misleading error in ModelForm
-------------------------------------+-------------------------------------
Reporter: ingo@… | Owner: nobody
Type: Uncategorized | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: modelform fields | Triage Stage:
attributeerror widget subset | Unreviewed
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 1
-------------------------------------+-------------------------------------

Comment (by ogpcludi <sample@…>):

1

--
Ticket URL: <https://code.djangoproject.com/ticket/15126#comment:10>

[Django]

#15126: Misleading error in ModelForm
-------------------------------------+-------------------------------------
Reporter: ingo@… | Owner: nobody
Type: Uncategorized | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: modelform fields | Triage Stage:
attributeerror widget subset | Unreviewed
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 1
-------------------------------------+-------------------------------------

Comment (by ogpcludi <sample@…>):

1

--
Ticket URL: <https://code.djangoproject.com/ticket/15126#comment:11>