Handle ValidationError in model saving in admin change views

[Daniel Hahler]

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.

[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.

[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.

[Tim Graham]

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

[Jerome Leclanche]

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