Opened 7 years ago

Closed 7 years ago

Last modified 3 years ago

#28619 closed New feature (wontfix)

Handle ValidationError in model saving in admin change views

Reported by: Daniel Hahler Owned by: heba1108
Component: contrib.admin Version: 1.11
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Change History (5)

comment:1 by Simon Charette, 7 years ago

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.

comment:2 by heba1108, 7 years ago

Owner: changed from nobody to heba1108
Status: newassigned

comment:3 by Tim Graham, 7 years ago

Summary: Handle ValidationError in change viewsHandle ValidationError in model saving in admin change views

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.

comment:4 by Tim Graham, 7 years ago

Resolution: wontfix
Status: assignedclosed

Feel free to start a discussion on the DevelopersMailingList to form a consensus about this -- closing as wontfix for now.

in reply to:  1 comment:5 by Jerome Leclanche, 3 years ago

Replying to 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().

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