View Issue Details

IDProjectCategoryView StatusLast Update
0015029MMW 5Syncpublic2022-05-06 16:12
Reporterrusty Assigned To 
PriorityurgentSeverityminorReproducibilityalways
Status closedResolutionreopened 
Product Version5.0 
Target Version5.0Fixed in Version5.0 
Summary0015029: Deletion of library tracks stored also to the cloud
DescriptionIn #14272 c iv) I'd written:
"Delete content from external locations
Note: for Delete operations, we'll have to modify the confirmation dialog slightly:
- For local content change options wording to:
 ( ) Remove from the library only
 (o) Remove from the library and delete
- For cloud-only content (that is R/W), retain the dialog but eliminate the options entirely since 'Remove from the library only' isn't useful because sync operations will cause the deleted track to re-appear."


The current implementation (build 2118) is that if the user attempts to delete a track stored only to Google drive
--> MM gives the user the option to delete the track from the library, but not from the drive.

This is the inverse of what was intended, however, in retrospect, it probably makes sense for cloud-only content to include _both_ deletion options since there can be cases in which the user will sync to the cloud but elect to not add new tracks to the library.
TagsNo tags attached.
Fixed in build2214

Relationships

related to 0015133 closedLudek Downloading from cloud issues 
related to 0015023 closedLudek Ability to show to which remote locations track is stored 
related to 0015577 closedLudek File IO operation: MM5 Deletes files directly not thru Recycle Bin 
related to 0019020 closedLudek Track deletion fails when deleting mix of local and cloud tracks 

Activities

Ludek

2018-08-10 21:34

developer   ~0050907

Last edited: 2018-08-10 21:35

In my case it shows this:
https://www.dropbox.com/s/5x8u2u375a5vdsu/Screenshot%202018-08-10%2023.25.25.png?dl=0
per item 2c here 0014272:0049795

i.e.
( ) Delete local copy
( ) Delete remote copy
(o) Remove from the library and delete (note: this deletes all copies)

Nevertheless I see that if I select the same track via [Music > All tracks] then there is only the option to delete it from the library and not from the remote location(s), this is probably how you have tested, right?
The problem is that in [Music > All Tracks] there can be track with multiple remote copies, e.g. in both 'Google Drive' and 'Drop box' at the same time, in that case this dialog would be useless, any suggestion how to solve this?

rusty

2018-10-12 03:00

administrator   ~0051303

Here's a suggested change that resolves cases:
a) of the originally described problem (i.e. if the user deletes a track stored to a local and remote location and the track is selected from a node other than the 'Folders' or 'Devices & Services' nodes (which represent content that may or may not be in the library and without library metadata))
b) where the track is stored to multiple remote locations (as raised by Ludek)
c) where the tracks have different storage/sync relationships (e.g. some are [hdd][cloud] , and others are [link][cloud], or [download][cloud])
This would supercede the proposal in 2c)iv) at #14272:

Are you sure that you want to remove <x file(s)>? Please choose how to proceed.
[ ] Remove x from the library [<HD / Network location1>]
. [ ] Delete y from [<drive:>]
. [ ] Delete z from [//<Network share>]
[ ] Remove a from the library [<Cloud location2>]
. [ ] Delete b from cache
. [ ] Delete c from [<Cloud location X>]
[OK] [[Cancel]]

Note:
- child options selectable only if the parent is selected
- in cases where an item is synced or scanned from a remote location it's possible that the track or link will just be resynced on the next sync operation. I'm not sure if there's anything we can do about that?
- the dialog would present the relevant options depending on the types of content being deleted.e.g each of the scenarios below would have slighly different deletion options presented
1) [hdd][cloud] - x and y, z,... (synced)
2) [download][cloud] - y (cached)
3) [link][cloud] - y (linked)
4) [cloud] - y (not in library)
5) [hdd] - x (in library)
6) [link][youtube] - youtube (linked)
7) [youtube] - youtube (not in library)
 
Test note: verify that Device & Services subnodes don't display library metadata and that deletion works as expected for these subnodes as well.

Ludek

2018-10-12 07:53

developer   ~0051305

Last edited: 2018-10-12 07:57

I am afraid this makes the dialogs unnecessarily complicated. Especially when you are deleting a track from a library playlist where we have currently choices to delete from
(o) From the playlist only
( ) Delete from the playlist and the library
( ) Delete from the playlist, Library, Computer

If we would add the other choices to delete from [<Cloud location2>], [<Cloud location X>] per proposals above then the dialog would become even more complicated than the proposed dialog above is :-(
i.e. it is complicated for us to implement and also complicated for users to go through the dialogs and to understand it.

I guess whether we shouldn't rather simplify and leave the dialogs similar as is now, but just add a checkbox like:
[ ] Delete from cloud locations
By checking the checkbox user can do a "hard" delete -- to delete the track from whatever cloud location.
And in the cases when user wants to delete it only from a particular cloud location then he still can -- by going sub to Devices&Services ... or sub to Music > Location > [<Cloud location2>] > ... and delete it there.

Thoughts?

rusty

2018-11-26 19:11

administrator   ~0051621

OK, so something like this?

Track including Track from Location / folder nodes:
(o) Remove from the library only
( ) Remove from the library and delete
 . . [x] Local copy (ies)
 . . [ ] Remote copy (ies)
 
Track from Playlist / Now Playing:
(o) Remove from Now Playing only
( ) Remove from Now Playing and the library
( ) Remove from the library and delete
 . . [x] Local copy (ies)
 . . [ ] Remote copy (ies)

Ludek

2018-12-18 17:41

developer   ~0051820

Last edited: 2018-12-18 17:43

To simplify (and based on assumption that there will be mostly only single cloud location used by users) I used the original dialog ( https://www.dropbox.com/s/5x8u2u375a5vdsu/Screenshot%202018-08-10%2023.25.25.png?dl=0 per item 2c here 0014272:0049795).

i.e. for the cloud tracks in library it takes the first cloud location and shows the dialog for the first cloud location (where the track resides).
Another reason for this is that using just 'Remote copy(ies)' would hardly tell something to user(s). i.e. it would be very unclear from which remote locations the track is going to be deleted.

Fixed in 2142.

rusty

2019-03-08 22:02

administrator   ~0052923

Re-opened re. item 14 from 0015133:
As described there, there's some confusion regarding this dialog--it's unclear whether the content is being deleted from MM and what the term 'library' means in this context.
https://www.dropbox.com/s/ek2al16ai1ubc7m/removal%20dbox%20unclear.png?dl=0

report at: https://www.mediamonkey.com/forum/viewtopic.php?f=30&t=93804

rusty

2019-03-11 17:33

administrator   ~0052934

Based on the user feedback, I'd suggest going back to the earlier suggestion at 0015029:0051621 :

Track including Track from Location / folder nodes:
(o) Remove from the database
( ) Remove from the database and delete
 . . [x] Local copy
 . . [ ] Remote copy
 
Track from Playlist / Now Playing:
(o) Remove from Now Playing only
( ) Remove from Now Playing and the database
( ) Remove from the database and delete
 . . [x] Local copy
 . . [ ] Remote copy

This:
- replaces 'library' with database to make eliminate confusion about 'library' in this context
- clarifies that deletion of the actual tracks also deletes the db entry
- supports the usecases that the user was asking for

note:
- 'Remove from the database and delete' should only remove the database entry for the deleted copy. Hopefully this isn't confusing. Alternative and perhaps clearer wording would be 'Delete and remove from the database', but I prefer the former.
- We should update the regular deletion dialogs to use the same terminology

Ludek

2019-03-11 18:45

developer   ~0052937

Last edited: 2019-03-11 18:49

But your approach is missing the way how it currently works (and what the user is using).
i.e. deleting the database entry only when both copies are deleted.

Currently (by using dialog https://www.dropbox.com/s/ek2al16ai1ubc7m/removal%20dbox%20unclear.png?dl=0) :
either:
- local copy is deleted and database entry retained (for the remote copy)
- remote copy is deleted and database entry retained (for the local copy)
- both copies are deleted (and database entry deleted)

So I think that current approach is good, but it needs wording clarification (to be clear that the database entry is retained whenever a copy still exists)

I think that deletion of database entry when remote copy still exists (as suggested by you above) won't work for these reasons:
a) subsequent scan of cloud storage/service re-adds the database entry to MM library (for the remote copy)
b) user use-case of deleting certain local copies (to free local disc space) and keep the entries "streamed", i.e. using remote copy for playback etc.

rusty

2019-03-12 16:08

administrator   ~0052942

Re. the assertion that we shouldn't allow DB-only deletions. I'd originally thought so as well, but then I was thinking: Why should this be the case for online files but not local files? i.e. if file scanning is enabled, and the user deletes a track from the DB, then the file scan will subsequently add it back. So I included this option based on the user's feedback that there may be cases we haven't envisioned where that would be desirable (e.g. if the user wants to just back up tracks online but not use MM for them).

If we're sure that such a case will be rare, we can go with:

Track including Track from Location / folder nodes:

Are you sure you want to delete xxxx?
( ) Delete local copy and remove it from the database
( ) Delete remote copy and remove it from the database
(o) Delete all copies and remove them from the database

Ludek

2019-03-12 17:07

developer   ~0052943

But, with your approach, how can one delete local copy with retaining the database entry at the same time (for the remote copy)?
i.e. it disallows the currently allowed use-case of deleting certain local copies (to free local disc space) and keeping the entries "streamed", i.e. using remote copy for playback etc.

Also, I don't understand the choice "Delete remote copy and remove it from the database", what is the point of keeping local copy but deleting it from the database? This does not make much sense.

rusty

2019-03-13 05:53

administrator   ~0052947

The wording is intended for the current functionality:
( ) Delete local copy and remove it from the database -- 'it' refers to the local copy (i.e. MM retains the remote instance)
( ) Delete remote copy and remove it from the database -- 'it' refers to the remote copy ( i.e. MM retains the local instance)
I thought that this was clear by virtue of the difference in wording for the third option '...remove _them_ from the database' (i.e. the first two options only remove the local or remote instance, whereas only the third removes both instances).

If this wasn't clear, then we can try the following:
( ) Delete local copy and update the database
( ) Delete remote copy and update the database
(o) Delete all copies and remove from the database

Ludek

2019-03-15 09:05

developer   ~0052973

Last edited: 2019-03-15 09:05

ok, sounds good, fixed in 2167

rusty

2019-03-20 21:02

administrator   ~0053040

I haven't tested 2167, but in 2166, the implementation wasn't adapted to Playlist and Now Playing views. i.e. when attempting to delete a track stored to multiple locations:
- from a Playlist, the new dialog which allows deletion of different copies appeared (i.e. there was no way to remove a track from the playlist)
- from Now Playing, the original dialog (which doesn't allow the user to choose which location to delete from) appeared.

Something along the following lines should work:
(o) Remove from Now Playing only // (o) Remove from the Playlist only
( ) Remove from Now Playing and // (o) Remove from the Playlist and
. . [ ] Delete local copy and update the database
. . [ ] Delete remote copy and update the database
. . [x] Delete all copies and remove from the database

(note: at least one of the 3 delete options must be checked if the second radio button is enabled)

Ludek

2019-03-27 17:25

developer   ~0053113

Fixed in 2168

rusty

2019-04-11 03:31

administrator   ~0053179

Summary:
Tested 2168 and much as I hate to re-open this, it seems that the originally suggested approach didn't adequately cover all cases:
a) Deletion of content stored locally and remotely
b) Deletion of content some stored locally, some remotely, and some both (not covered)
c) Deletion of only remote content (see below)
d) Deletion of only local content

After coming up with the suggestions described below, and considering that even these approaches are limited in the sense that they don't allow control over specific locations (e.g. gdrive vs google music), I wonder whether the entire idea of giving the user the ability to manage content from different sources differently depending on content location is too complex. Perhaps a simpler approach would be to have a de-deduplication view that (optionally) shows _all_ track instances, and thereby give users the ability to choose which instances to delete. What do you think?


Details:
1) Re. 0015029:0052947 deletion of content stored locally and remotely (case a), the 3 options are displayed correctly:
( ) Delete local copy and update the database
( ) Delete remote copy and update the database
(o) Delete all copies and remove from the database
 but they are prefaced with:
"Are you sure you want to remove 1 files from the "Google Drive?". This is confusing because the options don't match up to the question. It should probably be changed to something like: "Are you sure you want to delete x track(s) ?"


2) If the user selects some track(s) that are local only and some that are local and remote (case b), MM shows:
Are you sure you want to delete x track(s)?
( ) Delete from Library only
(o) Delete from Library and Computer

This can be problematic if the user has only a single local track among a group of tracks that is otherwise local and in the cloud.

An approach that could deal more effectively with this case would be:

Are you sure you want to delete x track(s)?
Local content:
[ ] Remove from the database
. . [ ] Delete and update the database [selecting this auto-selects both options]

Remote content:
[ ] Delete and update the database


3) For case c) (deletion of remote-only content), MM currently displays the options from case a) (which don't really apply)!
Would it be preferable to just show:

Are you sure you want to delete x track(s) (remote)?


4) For deletion of local content only (case d), if we go with the approach suggested in 2) we should just update the strings so that the same strings are used:

Are you sure you want to delete x track(s) (local) ?

( ) Remove from the database
(o) Delete and update the database


5) For Deletion of content stored locally and remotely (case a), if we go with the approach suggested in 2) then we should probably use the same approach in this case. i.e.:

Are you sure you want to delete x tracks?
Local content:
[ ] Remove from the database
. . [ ] Delete and update the database [selecting this auto-selects both options]

Remote content:
[ ] Delete and update the database


6) If we agree on the above, then 4 variants for deletion from a Playlist / now Playing would be needed. Below is shown a couple of options for case a/b since these are the most complicated and since since the other cases flow from this:

Option I (Use the current implementation and only change 'copy'-->'content' for consistency, BUT the approach doesn't support DB-only deletion of local content):
(o) Remove from Now Playing only // (o) Remove from the Playlist only
( ) Remove from Now Playing and // (o) Remove from the Playlist and
. . [ ] Delete local content and update the database
. . [ ] Delete remote content and update the database
. . [x] Delete all content and remove from the database

Option II (based on the implementation suggested in 2) above. I think that this is the better approach.:
(o) Remove from Now Playing only // (o) Remove from the Playlist only
( ) Remove from Now Playing and // (o) Remove from the Playlist and
. . Local content:
. . [ ] Remove from the database
. . . . [ ] Delete and update the database [selecting this auto-selects both options]
. . Remote content:
. . [ ] Delete and update the database

Ludek

2019-05-06 09:55

developer   ~0053425

Last edited: 2019-05-06 10:07

Re the cases:
a) Deletion of content stored locally and remotely
b) Deletion of content some stored locally, some remotely, and some both (not covered)
c) Deletion of only remote content (see below)
d) Deletion of only local content

What about to join a&b into one? i.e. whenever there is a mix of local and remote tracks then the current dialog for the case a) is shown
The only minor downside is that for the local content there is just the option to delete both from the library and the disc, but I believe that the upside of being clearer is more important. If one wants to delete local content from the library only then he can still do so by selecting local only tracks group.
=> Unified in 2174

c) Fixed in 2174

d) left as is (to prevent from confusion for MM4 users not planning to use remote locations), I believe that MM4 wording is still clearer in case of local content only.
Maybe just the word "Library" should by replaced by "database" in all cases ?

Ludek

2019-11-19 12:54

developer   ~0055401

Last edited: 2019-11-19 12:55

c) this does not work fine for tracks that has remote content only, but are also scanned in the MM library.
For such a tracks user may want to delete only the MM5 library link (without deleting the remote copy), this is not possible currently: https://www.mediamonkey.com/forum/viewtopic.php?f=30&t=95413&p=463116#p463116
Solution is to use the same dialog as for cases a&b in such a cases.

=> Fixed in 2214

peke

2020-08-05 00:00

developer   ~0059233

Verified 2262

I do not see any further issues. Please close after review.

peke

2020-12-23 02:15

developer   ~0061056

Re verified 2291

No regression found, no additional such reports located. Closing.

peke

2022-05-06 16:12

developer   ~0068025

Verified 2621

No regressions. THX for pointers.