Opened 2 months ago

Closed 2 months 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 (4)

comment:1 Changed 2 months ago 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.

comment:2 Changed 2 months ago by heba1108

Owner: changed from nobody to heba1108
Status: newassigned

comment:3 Changed 2 months ago by Tim Graham

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 Changed 2 months ago by Tim Graham

Resolution: wontfix
Status: assignedclosed

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

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