[Redesign] Delete from Server Button Cleanup #1098
Conversation
…creen. refactor delete from server check into a provider.
| @@ -24,8 +24,9 @@ import 'jellyfin_api_helper.dart'; | |||
|
|
|||
| part 'downloads_service_backend.g.dart'; | |||
|
|
|||
| const FINAMP_BASE_DOWNLOAD_DIRECTORY = | |||
| "songs"; //!!! don't ever change this without implementing a migration, it will break existing downloads | |||
| // This is used during migration from the legacy hive download storage and cannot be changed without mitigations | |||
There was a problem hiding this comment.
Isn't this generally used for the downloads system still, not just during migrations? The previous warning message seemed a bit more accurate.
There was a problem hiding this comment.
maybe a mix of both? Add a clear warning, but keep the newly-added details about what it's used for?
There was a problem hiding this comment.
It's used to calculate paths for newly downloaded items, but then that gets included in the saved path string, so changing it shouldn't break old downloads, it just creates a new folder for newly added ones.
|
Thanks for adding the fixes! The whole riverpod provider stuff still looks a bit convoluted to me (not your code, just the architecture in general); I can understand the code, but wouldn't have a clue how to properly write them from scratch. |
|
I wasn't familiar with riverpod, or flutter as a whole, before this. I didn't find anything particularly special, just read the docs and did some experimentation. There's still a lot I don't understand as well - a lot of my use cases are just repeats of the same pattern where I wrap a future in a non-future provider to give it a default value. Also, whatever I was doing with the theme providers hasn't really turned out too well so far. |
…ly take BaseItemDtos instead of using the wrapper class. This requires changing the BaseItemDto class so that instances with the same id are considered equal, but I don't believe the default behaviour of checking if the objects are the same instance was used.
I've cleaned up the logic for the delete from server button on album/playlist screens and moved it all under DownloadButton. We now properly show the lock button on incidental downloads. I've refactored the canDeleteFromServer check into a provider while doing so to make usage cleaner and reduce repeated requests.
Also, I noticed we seemed to be using the 'tracks' directory in the download migration code, so I fixed that.