You are viewing an historical archive of past issues. Please report new issues to the appropriate project issue tracker on GitHub.

Feature request #734: Replace confirm-then-delete pattern with delete-then-undo

Kind feature request
Product wincent.dev
When Created , updated
Status open
Reporter Greg Hurrell
Tags no tags

Description

When doing a destructive operation like deleting a record there are two possible workflows:

  • Provide a delete link and then throw up a confirmation dialog with JavaScript to prevent accidental deletions
  • Provide a delete link with no confirmation dialog, but do offer an "undo" link to correct accidental deletions

When deleting spam tickets I am finding that the former workflow isn't as user-friendly and efficient as it could be. If there are 10 spam tickets then you have to do 20 operations (click 10 delete links and dismiss 10 confirmation dialogs).

A much nicer way to work would just be to offer delete-with-the-option-of-undo; then you're looking at just 10 clicks under the same situation. If the volume of spam increases too much then I'll look at implementing mass deletion, but I think that this interface improvement is worth doing anyway.

See also: http://www.alistapart.com/articles/neveruseawarning

Comments

  1. Greg Hurrell

    A further note, to minimize the risk of the change I'll be making it to the admin issues interface first. If it works there then I'll generalize it elsewhere.

  2. Greg Hurrell

    Interesting viewpoint about implementation:

    The argument there is "archive deleted records (somewhere else) rather than marking them as deleted (in-place in the table)".

  3. Greg Hurrell

    I was thinking about this earlier today in relation to the "spam" field that I currently have on a lot of models.

    It turns out that this has been an unnecessary complication ever since it was implemented. Basically, I never mark stuff as spam; I always just delete it. So all of those queries where I have to add a spam == false condition could have been made simpler.

    If you want to do some sort of bayesian training on spain records then you don't really need to keep those records around in their original form anyway. In short: keep the "Spam" button, but get rid of the "spam" attribute in the database tables. Training as spam would analyse the message and update the corpus, and then the spam record would be deleted.

    So the point is, if all those queries were made unnecessarily complicated by the spam == false check, then perhaps I should forget about using this "deleted_at" column in the database because it will complicate all those queries in exactly the same way.

    It would be better to have a separate table (even tables, one for each kind of record) to hold "deleted" records. Sure, it would be more complicated in some ways (more tables, more classes), but it would make the "99% code path", the one which gets used almost exclusively, a lot more simple. For basic, non-deleted records (ie. 99% of all use), you can just forget entirely about the notion of "deleted_at".

  4. Greg Hurrell

    I've made a new ticket, ticket #1264, to track the removal of the "spam" field.

  5. Greg Hurrell

    I currently have two models with a "deleted_at" field: "emails" and "users". So whatever I decide to do, will have to take those existing arrangements into account.

  6. anonymous

    (Posting anonymously from another computer seeing as I don't have my login details with me.)

    I'm thinking that I do want to go ahead with this after all, for the reasons stated in ticket #1587. In short, it allows us to elegantly handle the no-JavaScript case, and is a nicer workflow anyway.

    The basic idea is to show in the flash, "Record successfully deleted ([link to undo])".

    The undo link would just be a post back to the update action with the deleted_at field set to nil. The update action updates and then redirects to the show action, with a flash saying "Record successfully updated".

    Will need some kind of helper method to produce the "undo" links in a comfortable fashion.

    Will still be tricky in terms of adapting the other queries to use WHERE deleted_at IS NULL, but I think it's most likely worth it in the overall scheme of things.

  7. anonymous

    (Me again.)

    One of the arguments against using this "mark as deleted" model is that the database can fill up with crud.

    In reality, it's just a simple mechanism for providing a short-term "undo-ability".

    There is nothing stopping us from allow the admin to really delete (as in destroy, remove from the database) records that are "in the Trash" (marked as deleted). Best place for this is probably in the admin-only interface.

  8. Greg Hurrell

    In terms of query complexity, the comment model will actually be the most complex because it already has two boolean fields that must be checked on pretty much every query (the public and awaiting_moderation flags).

    For the initial implementation, then, will probably start with a more simple model, like the Post, Article or Tweet models, seeing as they at least don't have the awaiting_moderation flag (nor author/user fields either). It will therefore be simpler to modify those queries.

    As for the whole object graph question, most (all?) of the "commentable" models use :dependent => :destroy. So I guess if we mark a parent object like a port or an article as deleted, the simplest thing will be to leave the comments untouched. Any attempt to access an individual one will end up yielding a 404. When we actually proceed to really destroy the parent model (ie. "empty the recycle bin") then we'll destroy the dependent comments.

Add a comment

Comments are now closed for this issue.

Menu