Review Request 111991: Better handling of non-writeable UMS collections

[Konrad Zemek]

> On Aug. 10, 2013, 7:26 p.m., Mark Kretschmann wrote:
> > I'm not so sure about the warning that is shown when an unwritable UMS collection is loaded. First of all it seems to me this would belong on a higher level, as other collections could be read-only as well. Second, I'm not sure a warning is called for. It's not a critical situation. And last, doesn't it suffice to show an error when the actual writing failed?
> 
> Frank Meerkoetter wrote:
>     There won't be an error message when the actual writing fails, as the user is not offered certain operations (move/copy to/organize) for read only collections. :-)
>     
>     I am not sure what would be a good solution. I have to admit that I added the warning as an afterthought after having fixed the isWriteable() for UMS. The reason was that i found it highly intransparent why certain operations are not longer offered.
>     
>     The question is how to make it transparent to the user why move/copy/org. are disabled for a certain UMS collection?
>     
>     Perhaps the solution is to leave isWriteable() as it was (always return true for UMS) and to generate error messages when operations fail?
>     
>

I think that leaving isWritable() that returns true despite the fact that collection is read-only is hacky at best, and dead wrong at worst. If we ever find ourselves needing to do something like that, we have a sure sign of a faulty design.

That being said, I completely agree with Mark about it belonging on the higher level. It may be nice to get a visual hint that a collection is read-only, but it should do so for every collection type, not only for UMS.

FWIW, I would instantly grasp the concept if the relevant actions were added but disabled (as currently not being added at all). Maybe add a tooltip on hover saying that the collection is read-only, and I'm sure everything would be clear enough. Just something to consider, I'm not necessarily convinced it's the best way.


- Konrad


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


On Aug. 10, 2013, 4:44 p.m., Frank Meerkoetter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111991/
> -----------------------------------------------------------
> 
> (Updated Aug. 10, 2013, 4:44 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> If an UMS-collection is non-writeable (think freshly created ext4 on an USB-stick) a copy/move to
> collection will silently fail. It can only be seen in the debug logs on the console.
> 
> The reason is that the UMS-collection is not actually checking if its writeable. Its just assuming
> to be always writeable.
> 
> My change consists of two parts:
> 1. I check if the UMS-collection is actually writeable (mount point and if set the "music folder").
> 2. To give the user a hint what is going on I added a warning message, informing the user that
>    this collection is read-only (otherwise he might be confused why certain operations (move/copy/organize)
>    are missing.
> 
> 
> Diffs
> -----
> 
>   src/core-impl/collections/umscollection/UmsCollection.h 749ff81 
>   src/core-impl/collections/umscollection/UmsCollection.cpp 028966e 
>   src/core-impl/collections/umscollection/UmsCollectionLocation.cpp 1cd1ba8 
> 
> Diff: http://git.reviewboard.kde.org/r/111991/diff/
> 
> 
> Testing
> -------
> 
> I have tested with an USB-Stick containing an ext4 filesystem. First i had the base-dir owned by root.root (non-writeable), next i made it writeable and verified that its correctly detected as such. Next i created a "Music" folder and configured it for this USB-Stick. I checked that it was detected as writeable. Next i made the "Music" folder non-writeable and checked that amarok also detected that.
> 
> 
> Thanks,
> 
> Frank Meerkoetter
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130810/5b9df6b0/attachment.html>