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 743310 - Updates to Smart Playlist functionality in 3.15.4
Updates to Smart Playlist functionality in 3.15.4
Status: RESOLVED FIXED
Product: gnome-music
Classification: Applications
Component: general
3.15.x
Other other
: Normal normal
: 3.14
Assigned To: gnome-music-maint
gnome-music-maint
Depends on:
Blocks:
 
 
Reported: 2015-01-21 18:24 UTC by Maia
Modified: 2015-02-10 10:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
added 'recently added' playlist, filtered out never-played songs from 'recently played' (3.96 KB, patch)
2015-01-21 18:26 UTC, Maia
reviewed Details | Review
update all static playlists whenever media added to library (673 bytes, patch)
2015-01-21 18:27 UTC, Maia
needs-work Details | Review
static playlists now update from player.py at time of scrobble rather than from playlists.update_playcount (1.51 KB, patch)
2015-01-21 18:27 UTC, Maia
accepted-commit_now Details | Review
static playlists updated on program start (i.e. at time of fetch/create) (835 bytes, patch)
2015-01-21 18:27 UTC, Maia
accepted-commit_now Details | Review
update all static playlists whenever media added to library (755 bytes, patch)
2015-01-23 22:26 UTC, Maia
none Details | Review

Description Maia 2015-01-21 18:24:34 UTC
Here are a handful of patches for the latest update to GNOME Music (3.15.4, merged 1/19/15), accomplishing the following:

- adding 'recently added' playlist; modifying 'recently_played' query so that it doesn't pick up never-played songs (i.e. songs with playcount=null). Also, minor comment fixes for consistency.
- updating all static playlists whenever media added to library (via view._on_changes_pending)
- updating static playlists from player.py at time of scrobble rather than from playlists.update_playcount--there was a problem here where, depending on the order that playlists.update_playcount and playlists.update_last_played got called in, changes to a song's last_played value might not be reflected in the playlist update.
- updating static playlists on program start--i.e. at the time they are fetched/created by playlists.fetch_or_create_static_playlists.
Comment 1 Maia 2015-01-21 18:26:58 UTC
Created attachment 295123 [details] [review]
added 'recently added' playlist, filtered out never-played songs from 'recently played'
Comment 2 Maia 2015-01-21 18:27:02 UTC
Created attachment 295124 [details] [review]
update all static playlists whenever media added to library
Comment 3 Maia 2015-01-21 18:27:06 UTC
Created attachment 295125 [details] [review]
static playlists now update from player.py at time of scrobble rather than from playlists.update_playcount
Comment 4 Maia 2015-01-21 18:27:10 UTC
Created attachment 295126 [details] [review]
static playlists updated on program start (i.e. at time of fetch/create)
Comment 5 Vadim Rutkovsky 2015-01-22 08:49:27 UTC
Review of attachment 295123 [details] [review]:

Looks good to me
Comment 6 Vadim Rutkovsky 2015-01-22 10:28:06 UTC
Review of attachment 295124 [details] [review]:

::: gnomemusic/view.py
@@ +141,3 @@
     @log
     def _on_changes_pending(self, data=None):
+        playlists.update_all_static_playlists()

Since its a base class for others (Albums, Views etc.) we should make sure base function is also called from classes inheriting it:

  ViewContainer._on_changes_pending(self, None)


However I'm not sure if its worth to update playlists from a base class - this clearly belongs to Playlists class in view.py - this class is also loaded while another view is active.
Comment 7 Vadim Rutkovsky 2015-01-22 10:28:49 UTC
Review of attachment 295125 [details] [review]:

Looks fine
Comment 8 Vadim Rutkovsky 2015-01-22 10:29:12 UTC
Review of attachment 295126 [details] [review]:

+1
Comment 10 Maia 2015-01-23 22:26:08 UTC
Created attachment 295307 [details] [review]
update all static playlists whenever media added to library

(revision of earlier patch -- this one now updates static playlists from Playlists container (view.Playlists._on_changes_pending) rather than from ViewContainer.)
Comment 11 Maia 2015-01-23 22:32:13 UTC
(In reply to comment #6)

Okay, revised that patch so that playlist updating is called from the Playlists container rather than the ViewContainer superclass. See comment #10.
> Review of attachment 295124 [details] [review]:
> 
> ::: gnomemusic/view.py
> @@ +141,3 @@
>      @log
>      def _on_changes_pending(self, data=None):
> +        playlists.update_all_static_playlists()
> 
> Since its a base class for others (Albums, Views etc.) we should make sure base
> function is also called from classes inheriting it:
> 
>   ViewContainer._on_changes_pending(self, None)
> 
> 
> However I'm not sure if its worth to update playlists from a base class - this
> clearly belongs to Playlists class in view.py - this class is also loaded while
> another view is active.
Comment 12 Vadim Rutkovsky 2015-02-10 10:21:57 UTC
(In reply to Maia from comment #10)
> Created attachment 295307 [details] [review] [review]
> update all static playlists whenever media added to library

Pushed it as https://git.gnome.org/browse/gnome-music/commit/?id=1cdaa3a, thanks