Inline delete not prompting cascade delete warning

[Yeago]

Models and admin can befound here: ​http://dpaste.com/hold/254060/

In this situation, no cascading delete warning is given when deleting questions from questionnaires in admin, resulting in the loss of all answer objects associated with the question by an FK.

Setting milestone due to my perceived severity. Please undo if necessary.

[Carl Meyer]

There's no obvious fix here; the normal dedicated cascade-delete warning page can't be used in the case of deleting an inline, because deleting an inline isn't all you're doing: you may be editing the main model and other inlines, adding new inlines... trying to preserve all that state across an extra request to the warning page would be quite fragile.

Two possibilities I can think of: either a documentation note that this is simply the risk you take if you use inlines for a model with an FK pointing to it (not the common case anyway), or prepopulate the page with counts of dependent objects for each inline and use that data to provide some kind of JS warning. Not very fond of the latter option: it doesn't scale well to pathological cases with trees of multiple dependent models; also JS warnings are irritating and people are likely to click OK before they think anyway.

So on the whole I think I'd lean towards just documenting this, unless someone has a brilliant solution I've overlooked. Setting to DDN.

[Adrian Holovaty]

carljm explained the difficulty well. I don't see an elegant code fix here, unfortunately, which makes me believe documenting it clearly is the best way to go. Who can write up some docs?

[Yeago]

What if the delete button wasn't an element of the form but rather a button?

[Carl Meyer]

Replying to subsume:

What if the delete button wasn't an element of the form but rather a button?

Then we trade this problem for an alternative that's just as bad: potential loss of all the work you may have put into editing other fields on the page before you click that Delete button.

Accepting and setting component to Documentation.

[Yeago]

Sorry, that seems like an easy fix that allows us to stamp 'TOLDYASO' on situations where serious data loss can occur. Pressing a back-button usually will save your work within a form. I don't think users are often inclined to click links away from forms they are editing and its rather easy to make it obviously not part of the form. We could even have it open in a new window if that's really a huge problem.

[Alex Gaynor]

Possibly solution: turn it into a button, have the button do a popup raw-id-fields style and let it prompt fro deleting, then on completion it can remove the item from the inline.

[Yeago]

or a 'do you wish to navigate away from this page' alert... or... or... or...

[Ubercore]

None of the options are perfect, agreed, but I'd rather see a solution that minimizes data loss. Better to document that a user may lose changed data on the admin form than document that a user may lose an entire table's worth of data to a cascading delete. A failure on the user's part to heed the warning is far less catastrophic.

[Russell Keith-Magee]

IMHO, I don't think documentation alone is the right solution here. We're talking about potentially catastrophic data loss that can occur fairly easily. We don't get a free pass by putting a line in the docs that explains, after the fact, why your entire database was just deleted. Even if we assume people completely read and comprehend the docs, it's trivial for someone who didn't set up the inline to add a foreign key to a model and get massive data loss as a result.

I don't know that I have a good UI solution, but one option at a programmatic level would be to make the behavior specifically opt-in. As an admin validation activity, we are able to inspect whether an inline model has foreign keys that would be affected by a cascading delete. We could set the default behavior to raise a validation error and refuse to allow the admin to start. We could then provide an an opt-in option on the inline that requires you to explicitly nominate that "yes, it's ok for these foreign keys to be cascade deleted as inlines".

This makes inlines safe-by-default, it forces the user explicitly opt-in to any data loss that may occur, and it provides a good reason for people to hunt down the relevant documentation for an explanation of the problem -- first time they see the error, they're going to go looking for the docs on the option for their inline.

[Gabriel Hurley]

The opt-in solution might be a decent stopgap measure, though it still doesn't solve the "naive end-user" problem once the developer has opted in.

In the long-term, this does seem like a problem of an implicit behavior which ought to be explicitly user-controlled. Offering an idea somewhat like Alex's, stated in terms of the OP's models:

You click to delete the Question from the Questionnaire, and you are presented a dialog (or intermediate page) with the following options...

1. delete the associated Answers,
2. re-assign the associated Answers to another Question,
3. set their FKs to null (if they are nullable),

It doesn't seem like either the backend code or front-end UI should be terribly complex. This seems to offer a maximum amount of user-control and a minimum amount of potentially catastrophic behavior happening automatically.

Either way, I agree that docs alone aren't the answer here.

[Carl Meyer]

I hadn't thought of the admin validation warning or overridable error; that seems like a good idea. Serves the same purpose as docs, but much more effective. Particularly if we can land #7539, which would allow another route around the warning; just make your FK cascade-set-null (or cascade-set-default) instead of cascade-delete.

If a developer explicitly wants a cascade-deleting inline, they can implement any of the end-user warning options proposed here outside Django via ModelAdmin customization. A really good generic solution for that warning could be built-in; my concern there is Hippocratic ("do no harm"). In other words, IMO the bar for including such a warning is that it have zero usability regression for the inline-deletion UI in the common case (no dependent models, no warning needed), not introduce confusing inconsistencies where some inlines have one deletion UI and others another, and generally be consistent with how the rest of the admin UI works. I don't think any of the warning solutions proposed thus far have a very good chance of meeting those criteria -- there are many devils lurking in the details; I can go into more detail on what I think some of them are in a mailing-list thread if there's interest -- but in any case I'd be happy to be proven wrong, preferably in code.

[Julien Phalip]

Related issue: #13106.

[Jacob]

Milestone 1.3 deleted

[Aymeric Augustin]

Django now has configurable cascade behavior (#7539). Moving back to accepted, based on the latest comments by Carl and Gabriel.

[ris]

Yeah, we just got bitten by this yesterday. Staff member managed to delete quite a bit of data without warning.

[Tim Graham]

The dpaste that contained the models in the report are gone, but it looks to me like this is a duplicate of #12382.