After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 743884 - Protect smart playlists
Protect smart playlists
Status: RESOLVED FIXED
Product: gnome-music
Classification: Applications
Component: general
3.15.x
Other All
: Normal minor
: 3.16
Assigned To: Maia
gnome-music-maint
Depends on:
Blocks:
 
 
Reported: 2015-02-02 19:43 UTC by Maia
Modified: 2015-02-28 15:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
disable deletion of smart playlists (3.55 KB, patch)
2015-02-20 18:45 UTC, Maia
accepted-commit_now Details | Review
disable deletion of smart playlists (3.66 KB, patch)
2015-02-23 17:42 UTC, Maia
none Details | Review
disable deletion of smart playlists (3.66 KB, patch)
2015-02-23 17:48 UTC, Maia
none Details | Review
disable deletion of smart playlists (3.66 KB, patch)
2015-02-23 17:49 UTC, Maia
none Details | Review
disable deletion of smart playlists (3.66 KB, patch)
2015-02-24 16:49 UTC, Maia
none Details | Review
disable deletion of smart playlists (3.63 KB, patch)
2015-02-24 17:00 UTC, Maia
none Details | Review

Description Maia 2015-02-02 19:43:48 UTC
Smart playlists should be "protected" from renaming and deletion. Additional thought: should names of smart playlists be 'reserved' (so user can't name a second playlist, say, "Recently Played")?
Comment 1 Vadim Rutkovsky 2015-02-03 10:48:58 UTC
(In reply to comment #0)
> Smart playlists should be "protected" from renaming and deletion.
Agreed on that, that should be pretty easy since we store playlist id for smart playlists.

> Additional
> thought: should names of smart playlists be 'reserved' (so user can't name a
> second playlist, say, "Recently Played")?

Not sure about that, I don't mind the duplicates there. Although we should make sure that smart playlists are on top of the list
Comment 2 Maia 2015-02-20 18:45:15 UTC
Created attachment 297455 [details] [review]
disable deletion of smart playlists
Comment 3 Vadim Rutkovsky 2015-02-23 10:37:17 UTC
Review of attachment 297455 [details] [review]:

LGTM, will update the patch with comment below and push it to master

::: gnomemusic/view.py
@@ +1185,3 @@
+    def current_playlist_is_protected(self):
+        current_playlist_id = self.current_playlist.get_id()
+        if current_playlist_id in StaticPlaylists.get_protected_ids():

Nitpick:
simplier way would be "return current_playlist_id in StaticPlaylists.get_protected_ids()"
Comment 4 Vadim Rutkovsky 2015-02-23 12:51:09 UTC
(In reply to Vadim Rutkovsky from comment #3)
> Review of attachment 297455 [details] [review] [review]:
> 
> LGTM, will update the patch with comment below and push it to master

Actually we can't push it now - we're in Strings Freeze, so adding new strings is prohibited (unless we really want to).

How about we simply set 'Delete' menu as disabled and push this change for 3.18?
Comment 5 Maia 2015-02-23 17:42:30 UTC
Created attachment 297692 [details] [review]
disable deletion of smart playlists

(revised -- disables delete button rather than popping up a "can't delete this playlist" notification")
Comment 6 Maia 2015-02-23 17:48:20 UTC
Created attachment 297694 [details] [review]
disable deletion of smart playlists

(revised -- disables delete button rather than popping up a "can't delete this playlist" notification")
Comment 7 Maia 2015-02-23 17:49:35 UTC
Created attachment 297695 [details] [review]
disable deletion of smart playlists

(revised -- disables delete button rather than popping up a "can't delete this playlist" notification")
Comment 8 Maia 2015-02-24 16:49:32 UTC
Created attachment 297783 [details] [review]
disable deletion of smart playlists

(revised -- disables delete button rather than popping up a "can't delete this playlist" notification")
Comment 9 Maia 2015-02-24 17:00:06 UTC
Created attachment 297784 [details] [review]
disable deletion of smart playlists

(revised -- disables delete button rather than popping up a "can't delete this playlist" notification")
Comment 10 Vadim Rutkovsky 2015-02-28 15:34:14 UTC
Thanks, pushed as https://git.gnome.org/browse/gnome-music/commit/?id=910eaad