Review Request 112023: Disable move/copy actions for non-writeable UMS collections.

[Frank Meerkoetter]

> On Aug. 12, 2013, 4:57 p.m., Matěj Laitl wrote:
> > src/core/collections/Collection.h, lines 125-135
> > <http://git.reviewboard.kde.org/r/112023/diff/1/?file=178093#file178093line125>
> >
> >     Hmm, I don't like the combination of 2 bools to signify 3 possible values (because you introduce one invalid combination).
> >     
> >     What about making it:
> >     Enum WritableType {
> >         Writable,
> >         TemporarilyNotWritable,
> >         NotWritable // permanently
> >     }
> >     WritableType writableType() const;
> 
> Frank Meerkoetter wrote:
>     Sure, i'll update the patch
> 
> Frank Meerkoetter wrote:
>     I have a question about your proposal.
>     
>     Your intention is to replace isWriteable() and isWriteableType() with writeableType() which returns an enum?
> 
> Matěj Laitl wrote:
>     Yes, exactly. It might be a good idea to change it also in CollectionLocation so that Collection::writableType() remains just a convenience shortcut.

Hi, did you see my updated patch?


- Frank


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112023/#review37599
-----------------------------------------------------------


On Aug. 25, 2013, 9:09 p.m., Frank Meerkoetter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112023/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2013, 9:09 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> The fix in https://git.reviewboard.kde.org/r/111991/ fixes isWriteable() for UMS collections. This has the effect that if an UMS-collection is actually non-writeable the copy/move operations will be missing from the TreeView/Filebrowser context menus. There is no clue to the user what is going on.
> 
> This patch is introducing a new method to the Collection interface so we can differentiate between collections that are non-writeable by principle and collections that just happen to be non-writeable right now (for what ever reason).
> 
> I use this changed collection interface to disable move/copy actions (as opposed to hide) for non-writeable UMS-colllections. This should give the user at least a hint what is going on.
> 
> Unfortunately i haven't found a way to display a tooltip ("Currently readonly"). The action is offering an setTooltip() method but now effect was observed.
> 
> 
> Diffs
> -----
> 
>   src/browsers/CollectionTreeItemModel.cpp 1ac4463 
>   src/browsers/CollectionTreeView.cpp 6fad164 
>   src/browsers/filebrowser/FileView.cpp ad200eb 
>   src/core-impl/collections/db/sql/SqlCollectionLocation.h da1b2e5 
>   src/core-impl/collections/db/sql/SqlCollectionLocation.cpp 3b55f66 
>   src/core-impl/collections/ipodcollection/IpodCollection.h 14fdd40 
>   src/core-impl/collections/ipodcollection/IpodCollection.cpp 36738d7 
>   src/core-impl/collections/ipodcollection/IpodCollectionLocation.h cc27e19 
>   src/core-impl/collections/ipodcollection/IpodCollectionLocation.cpp f8105f9 
>   src/core-impl/collections/ipodcollection/IpodMeta.cpp 6beca0a 
>   src/core-impl/collections/ipodcollection/IpodPlaylist.cpp 61c60ba 
>   src/core-impl/collections/ipodcollection/IpodPlaylistProvider.cpp dcef4c2 
>   src/core-impl/collections/mediadevicecollection/MediaDeviceCollectionLocation.h e40529f 
>   src/core-impl/collections/mediadevicecollection/MediaDeviceCollectionLocation.cpp 603ceff 
>   src/core-impl/collections/mediadevicecollection/MediaDeviceMeta.cpp 4a419d5 
>   src/core-impl/collections/nepomukcollection/NepomukCollection.h c5d7a13 
>   src/core-impl/collections/nepomukcollection/NepomukCollection.cpp 77aef8e 
>   src/core-impl/collections/playdarcollection/PlaydarCollection.h 1a7ec89 
>   src/core-impl/collections/playdarcollection/PlaydarCollection.cpp 5c2349d 
>   src/core-impl/collections/support/FileCollectionLocation.h 9bd303e 
>   src/core-impl/collections/support/FileCollectionLocation.cpp 4b9b4b0 
>   src/core-impl/collections/support/TrashCollectionLocation.h 239a977 
>   src/core-impl/collections/support/TrashCollectionLocation.cpp 61c2e49 
>   src/core-impl/collections/umscollection/UmsCollection.cpp a73fd34 
>   src/core-impl/collections/umscollection/UmsCollectionLocation.h 6cfe4df 
>   src/core-impl/collections/umscollection/UmsCollectionLocation.cpp cef90d7 
>   src/core/collections/Collection.h 94d24d7 
>   src/core/collections/Collection.cpp 01a2e8c 
>   src/core/collections/CollectionLocation.h 7a1ed1e 
>   src/core/collections/CollectionLocation.cpp 2385570 
>   src/services/ServiceCollectionLocation.h cea3b84 
>   src/services/ServiceCollectionLocation.cpp d1cb0d8 
>   src/services/mp3tunes/Mp3tunesService.cpp ed2b006 
>   src/services/mp3tunes/Mp3tunesServiceCollectionLocation.h 2b06cb4 
>   src/services/mp3tunes/Mp3tunesServiceCollectionLocation.cpp 6aaa66d 
>   src/synchronization/MasterSlaveSynchronizationJob.cpp 1979701 
>   src/synchronization/OneWaySynchronizationJob.cpp 99f504c 
>   src/synchronization/UnionJob.cpp 8602044 
>   tests/core-impl/collections/db/sql/TestSqlCollectionLocation.cpp ab41db3 
>   tests/core/collections/CollectionLocationTest.cpp 720fa6a 
>   tests/core/collections/TestCollection.cpp 72a8b3b 
>   tests/synchronization/TestMasterSlaveSynchronizationJob.cpp b8b59c4 
>   tests/synchronization/TestOneWaySynchronizationJob.cpp ce7a2b5 
>   tests/synchronization/TestUnionJob.cpp 2343577 
> 
> Diff: http://git.reviewboard.kde.org/r/112023/diff/
> 
> 
> Testing
> -------
> 
> I have tested with an USB-stick that is writeable and another one that is non-writeable. I have also observed that for the FileBrowser the "Local collection" is still shown as active.
>  
> 
> 
> Thanks,
> 
> Frank Meerkoetter
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130901/7ead3c7a/attachment.html>