[Django] #28619: Handle ValidationError in change views
Django
-----------------------------------------+------------------------
Reporter: Daniel Hahler | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
Currently ValidationErrors raised in `save_model` and `save_related` are
not handled: the form is considered to be valid, and the error wil
typically bubble up, causing a 500.
A patch/PR is available at: https://github.com/django/django/pull/9113 -
currently no tests are failing because of it.
Some background:
`save_model` typically calls the model's `save` method, but with apps like
django-fsm this might be a custom transition etc, and it is easier to
ensure some state in there (e.g. that a certain field is non-empty when
transitioning the state), especially when the admin is not the main
interface to those transitions.
As for django-fsm-admin (admin integration for django-fsm), it is possible
to hook into the form's `is_valid`, but it would still require an extra
method/callback on the model then: https://github.com/gadventures/django-
fsm-admin/pull/74.
--
Ticket URL: <https://code.djangoproject.com/ticket/28619>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Daniel Hahler | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version: 1.11
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
Raising `ValidationError` during model save isn't a pattern used anywhere
else in the code base. I'm not sure the admin should be different in this
regard.
Ideally there would be a normalized API to raise constraint violation
exceptions during `save()` which could wrap existing database level
violations errors (e.g. `IntegrityError`) which `django-fsm` could rely on
but that's not the case now.
--
Ticket URL: <https://code.djangoproject.com/ticket/28619#comment:1>
Django
Reporter: Daniel Hahler | Owner: heba1108
Type: New feature | Status: assigned
Component: contrib.admin | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by heba1108):
* status: new => assigned
* owner: nobody => heba1108
--
Ticket URL: <https://code.djangoproject.com/ticket/28619#comment:2>
Django
-------------------------------+--------------------------------------
Reporter: Daniel Hahler | Owner: heba1108
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
Jon proposed Simon's idea in ticket:28405#comment:2. As I replied there,
I'm not sure if it's feasible. I don't think it makes sense to make the
proposed change to the admin in isolation. If I remember correctly,
someone proposed making a similar change to generic views (can't find the
ticket right now). I believe I closed it as wontfix since, as Simon said,
raising `ValidationError` during `model.save()` isn't a pattern Django
encourages.
--
Ticket URL: <https://code.djangoproject.com/ticket/28619#comment:3>
Django
-------------------------------+--------------------------------------
Reporter: Daniel Hahler | Owner: heba1108
Component: contrib.admin | Version: 1.11
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: assigned => closed
* resolution: => wontfix
Comment:
Feel free to start a discussion on the DevelopersMailingList to form a
consensus about this -- closing as wontfix for now.
--
Ticket URL: <https://code.djangoproject.com/ticket/28619#comment:4>
Django
-------------------------------+--------------------------------------
Reporter: Daniel Hahler | Owner: heba1108
Type: New feature | Status: closed
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jerome Leclanche):
Replying to [comment:1 Simon Charette]:
> Raising `ValidationError` during model save isn't a pattern used
anywhere else in the code base. I'm not sure the admin should be different
in this regard.
Actually, it kind of is. ModelForm's `_post_clean` hook usage is a hack
specifically there to work around the inability to raise ValidationError
during save.
In the library I'm working on, an object is created in an upstream API
once the model has been validated locally. When this happens, the upstream
API does its own field validation. The only way to properly raise form
errors in that scenario was to hook into `_post_clean()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/28619#comment:5>