Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Dr-Blank opened this issue Apr 17, 2023 · 10 comments
Open
Labels
enhancement New feature or request

Comments

@Dr-Blank
Copy link
Contributor

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.

@Dr-Blank Dr-Blank added the enhancement New feature or request label Apr 17, 2023
@advplyr
Copy link
Owner

advplyr commented Apr 17, 2023

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
Copy link
Owner

advplyr commented Apr 17, 2023

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

@Dr-Blank
Copy link
Contributor Author

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

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
Copy link

dedors commented Oct 25, 2023

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
Copy link
Owner

advplyr commented Dec 2, 2023

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
Copy link
Contributor Author

Dr-Blank commented Dec 3, 2023

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
Copy link
Owner

advplyr commented Dec 3, 2023

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
Copy link

dedors commented Dec 3, 2023

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
Copy link

KaiStarkk commented Dec 17, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants