Delete from server button

[flloschy]

A delete button to remove tracks form the servers filesystem/library
Request from #758

[Chaphasilor]

No description provided.

🧐

[flloschy]

Im bad at writing these things :d

[Chaphasilor]

@flloschy take a look at metadata_provider.dart, and how it's used in the menu:

https://github.com/flloschy/finamp/blob/f5da742a56fe9c9dadd5413239271dce5c21b041/lib/components/AlbumScreen/song_menu.dart#L224-L230

This is the best spot to implement the deletability checks.

As for the download button itself, you've re-implemented the local part of it, which you can find here:
https://github.com/flloschy/finamp/blob/f5da742a56fe9c9dadd5413239271dce5c21b041/lib/components/AlbumScreen/song_menu.dart#L426-L486

The delete button only shows up once you've actually downloaded a track, naturally. So try doing that first. A track can only be deleted if it was downloaded directly, and not as part of an album. If it is part of a downloaded album/artist/playlist, the menu will only show the "lock" button, which will separately mark the track as required for downloading. This means that even if the album is deleted, the track will stay downloaded until explicitly deleted.
It's currently not possible to exclude certain tracks/items from existing downloads (like downloading an entire album except for one track), so deleting a local track is only possible if it was downloaded directly.
So the confirmation dialog only needs to contain the alternative button if the track can actually be deleted from the device. You can take a look how that check is done.

Deleting a track from the download system then works like this:

var item = DownloadStub.fromItem(
    type: DownloadItemType.song, item: widget.item);
unawaited(downloadsService.deleteDownload(stub: item));

[Chaphasilor]

Also, you could try to (conditionally) extend the existing confirmation dialog with additional buttons instead of creating a new widget. That would also help staying consistent with the background colors and such ^^

[flloschy]

Thats kinda what I did in lib/components/confirm_prompt_dialog_with_actions.dart
but its a sketchy workaround because a ListView broke the rendering so i made the buttons wide and now they dont order them selfs vertically if the window is wide enough so I decided to do the second menu option instead

[flloschy]

Ohhhhhhhhh there is already a delete from device button which only shows up when a track is separately downloaded... So that makes things easier easier for me xd

[flloschy]

I updated the already existing delete button to adapt to the 2 deletion types: server and client

If only one of the two is possible its reflected in the button icon and label, and I added the confirm dialog which was missing previously

If its both the name is more generic with just "Delete", when pressed the user will be ask if they want to delete from Server, Client or both, when clicked it will ask one more time to verify the deletion

So I think thats all. If there is nothing more major i will add the translations, clean up a bit and format

[Chaphasilor]

Let's keep the server delete separate please. It shouldn't be as common as deleting a download. The prompt was meant as a way to say "hey, are you sure you don't just want to remove the download?" instead of "here are your options, choose whichever you like".

I even thought about a separate setting that controls if deleting from the server is possible at all, which defaults to false.

So just leave the existing button like it was, and add a separate buttom for deleting from the server :)

[F-4Dev]

I added a delete-button to the context menu of the album and playlist. @flloschy and I thought it was a good idea to put this in the same PR.
The page doesn't get refreshed after deletion at the moment. @Chaphasilor I might need your help for that.
Also need to add a confirm dialog.

[Chaphasilor]

TIL you can collaborate on a PR xD
@F-4Dev maybe it would be better to only have the delete option when actually looking at the album/playlist, instead of in the list of all albums. To refresh, take a look at the search functionality, I think that does something similar (i.e. when closing the search bar).
I'm also not sure if deleting playlists should be included here, because that work's a bit different, right? Playlist improvements (reordering, renaming, etc.) are also planned for the future, so we could do that in one go.

[flloschy]

I gave F4 permission to push to my fork thats why he could contribute haha

Thought since this uses similar code it fits in here perfectly :)

[F-4Dev]

@Chaphasilor I think having the delete option there is pretty useful. If you want to delete multiple Album/Playlists you don't have tap on them first and then delete them. Maybe you could also create an batch/multi select feature in the future. Personally I think deleting should be possible, both in the list of all albums and at the album screen.
From what I have seen (and it's not that much "^^) I also think that the code is pretty similar. Deleting a playlist and deleting an album uses the same api _jellyfinApiHelper.deleteItem(widget.album.id);

Also Playlists renaming and reordering etc would be awesome :)

[Chaphasilor]

Okay, if it really uses the same endpoint then let's include this in the PR. Do you know if this will only delete the playlist, or also the tracks within the playlist (e.g. if the playlist was automatically created from a directory + m3u file on the file system)?
Also, if I'm not mistaken you only added the delete option to the long-press menu on the album list, but not on the single album screen. So please go ahead and add this :)

[Chaphasilor]

@flloschy @F-4Dev if this is ready for review and merging, please mark it as such! :)

[flloschy]

@F-4Dev has a problem with his part where the delete button only appears the second time the context menu opens after changing the settings. The confirm dialog is missing, reloading of album/playlist view after deleting and a way to delete albums/playlists in their respective "full screen" view.

I believe my additions are done as far as i can tell, though id like to remove some duplicate code after everything seams to be ready.

I could also help out a bit more but im scared of merge issues when we both work on something in parallel xd

[Chaphasilor]

@flloschy if you have time to spare you could also open a new PR and work on a separate issue there! 😁

[flloschy]

Anything particular in mind?

[Chaphasilor]

Nothing particular, pick your poison.
There are enough bugs to fix, but it would also help if a few more screens would get a fresh coat of paint. Settings could be a good place to start, especially since you've already worked on those before. You can find some recent mockups in the #design channel on discord...

[Chaphasilor]

Since we're getting close to finalizing this, please fix the merge conflicts when pushing the other commits :)

[flloschy]

Okay so, final big thing to do (i believe) is the reload when deleting from the album full screen view (works with the context menu inside the list itself).
I tried a few different ways but i cant get it to work...

[Chaphasilor]

Reloading the list of albums, right?

What about refreshing the album when deleting a single track, does that work?

[flloschy]

Yes reloading works when deleting a single item inside the list, not the full screen view

[Chaphasilor]

okay, it took a while to finish this one up, but everything looks good now! this will be included in the next beta release.
if you could give this (the redesign branch) another test before that, that would be nice, just to make sure I resolved the merge conflicts correctly :)

[Chaphasilor]

Thanks to both of you for contributing this! <3

[flloschy]

Umm

[Chaphasilor]

You'll need to clear storage first, since the Hive IDs werw shuffled around since the last version you had installed :)

[flloschy]

Ohh yeah makes sense!
Do you happen to know the storage location on linux? ^^

[Chaphasilor]

The equivalent of %appdata%, probably somewhere in the home directory? But no idea really

[flloschy]

found it, its $HOME$/.local/share/com.unicornsonlsd.finamp.

[flloschy]

Appears to work correctly
Well.. I cant delete an Album but I figure thats because I enabled read only? The error suggests its an server problem so probably just me
Playlist deletion works fine so everything should still be neato burrito