advplyr / audiobookshelf

Self-hosted audiobook and podcast server
https://audiobookshelf.org
GNU General Public License v3.0
6.93k stars 489 forks source link

UI: Disable Delete functionality server-wide and make it OFF by Default #1689

Open Dr-Blank opened 1 year ago

Dr-Blank commented 1 year ago

Describe the feature/enhancement

Deleting files from the drive cannot be undone and hence making it on by default is extremely dangerous, as one could trigger it by mistake (items with issues section, especially) or in a hurry.

I believe that this functionality should be turned off by default and have a setting to disable it entirely, server-wide.

Making it off by default requires the user to take one extra step of clicking the check mark, which will confirm the intention of the user.

And having a server wide disable would just be an added layer of protection and a QOL feature.

advplyr commented 1 year ago

I think this could go either way. This was a highly requested feature which is why it was implemented.

I was under the impression that most users expect the "delete" option would be deleting from the file system. Abs now works the same as Jellyfin in regards to delete except in Abs the permission you would have to give a user is called "Can Delete".

Also, in Abs you have the option to only remove the item from the database for those rare cases you might want to re-scan the item in fresh. That shouldn't really be necessary but the scanner still needs some work and might not pick up every change on re-scans.

I think the warning message in the release notes may be making this seem like a scary change but from my perspective it is just making it closer to what users expect after using other media servers.

That being said, I'm open to updating this feature. Currently podcast episodes have a feature where you can cap the number of episodes you keep so that auto-deletes episodes. That would be the only thing I can think of interfering with the idea of globally disabling file system deletes.

I'm curious what everyone else thinks.

advplyr commented 1 year ago

Slight correction. There technically was no "delete" option before. It was intentionally called "remove" and was in a different place.

Dr-Blank commented 1 year ago

That was a good design choice, the distinction between remove and delete. And I understand the use of the word delete instead of remove to describe this action as I definitely did not expect the remove option to delete my files before this update.

That being said, the icon still remains the same for both, and since this is media server, I would expect it to deal with media server items and not interact directly with my file system, especially delete my files by default when I click on the trashcan icon. But then you are right, we have a re-scan option then why keep a remove option that does the same thing as re-scan with just extra steps.

maybe just make deletion option library specific if not server wide to not affect with podcasts auto delete image

image The tooltip needs an update btw.

Plex has this option of disabling file deletion, and when enabled, only owners could delete the files. image

last-available-username commented 1 year ago

random user opinion: I was troubled by the option at first, but I grew to be okay with it.

That being said, it would be nice to have a global or library specific option to swap the default action from "delete with checkbox to only remove" to "remove with checkbox to actually delete".

dedors commented 1 year ago

It felt wrong to me that I had to do an extra step to do the less dangerous action. I personally would change the default. Maybe make it remember - by default or by an other checkbox?

advplyr commented 1 year ago

Updated in v2.6.0 to persist locally whatever option you choose. If you uncheck the hard delete option it will stay unchecked for future prompts in that browser.

I still think that having an option to only delete the item from the database is an unusual and potentially confusing thing for users. For a while Abs didn't allow hard deletions at all but removing from the db was still an option.

Dr-Blank commented 1 year ago

I still think that having an option to only delete the item from the database is an unusual and potentially confusing thing for users. For a while Abs didn't allow hard deletions at all but removing from the db was still an option.

Totally get what you're saying here! When you're rocking ABS as a media server, you want it to be like Plex or Jellyfin, serving up all your media goodness with cover art, metadata, the whole shebang. Deleting stuff should default to keeping your actual files intact, right? I mean, those files might be used elsewhere or just need to chill for posterity. Having a global safety net would be sweet, you know?

But then, switch gears to using ABS as an audiobook manager, and suddenly you're all about tweaking files – combining them, embedding metadata, the whole deal. Deleting stuff here means something else entirely – you're in control for library maintenance and organization.

So, pressing delete has different context depending on how you roll with this software. Giving users some config options for file management, deletion prefs, maybe even a recycling bin would make it more organized as a software.

advplyr commented 1 year ago

When you're rocking ABS as a media server, you want it to be like Plex or Jellyfin, serving up all your media goodness with cover art, metadata, the whole shebang. Deleting stuff should default to keeping your actual files intact, right? I mean, those files might be used elsewhere or just need to chill for posterity. Having a global safety net would be sweet, you know?

When deleting in both Plex and Jellyfin it is deleting the files. As far as I know they don't have an option to remove the media from the database only. In Jellyfin if you make your library read-only then it will remove from the database and not the file system.

As far as I can tell for all other media servers deleting media is deleting actual files unless you have your files read-only

dedors commented 1 year ago

As far as I can tell for all other media servers deleting media is deleting actual files unless you have your files read-only

Kodi has a follow-up dialog after removing it from the database, asking if it should also be deleted from the filesystem. TinyMediaManager defaults to removing it from the database when using the keyboard. To delete, you have to use Ctrl-Shift. Sonarr/Radarr defaults to database only. When checking the box, you get a red warning that it will delete from filesystem. With MediaMonkey, you can choose, and it will default to the last option used.

I guess it just depends on what you're used to. I manage my files mostly on explorer level.

When sorting all the stuff into abs, I had to delete a few times from the database (not correctly updating changed metadata, weird order of importing, mass renamed cover.jpg to not get back side).

KaiStarkk commented 11 months ago

Image of current behaviour for future readers. The setting persists per browser as mentioned by owner above.

image

Seems closeable to me. Although I agree defaulting off would be preferrable. This seems to be the conventional behaviour for other software "out there in the world" (citation needed). Even ephemeral stuff like torrent managers. A warning symbol wouldn't go astray either:

image

ZLoth commented 5 months ago

The problem with having the "Delete from file system" is that it is sticky, and follows what was set when you last used the command. I'm running Audiobookshelf as a Docker container in a *nix environment, and if a file gets deleted, it is gone forever with almost no change of recovery. (That's why I have backups!)