Bug 214761

Summary: Sorting playlist by "Last Played" sorts alphabetically instead of numerically
Product: [Applications] amarok Reporter: Sean Madsen <seanmadsen>
Component: PlaylistAssignee: Amarok Developers <amarok-bugs-dist>
Status: RESOLVED FIXED    
Severity: normal CC: teo
Priority: NOR    
Version: 2.2.0   
Target Milestone: 2.2.2   
Platform: Ubuntu   
OS: Linux   
Latest Commit: Version Fixed In:

Description Sean Madsen 2009-11-16 06:22:45 UTC 
Version:           2.2.0 (using KDE 4.3.2)
OS:                Linux
Installed from:    Ubuntu Packages

Steps to reproduce:

1. Populate the playlist with some tracks that have a variety of values in the "Last Played" field
2. Use a playlist layout that shows the "Last Played" field for each track individually. 
3. Sort the playlist by "Last Played"
4. Expect that the tracks will be ordered by the amount of time since they were played last
5. Observe that the tracks are ordered alphabetically by the "Last Played" field

For example...

I'd like to see:
Within the last minute
46 minutes ago
3 weeks ago
2 months ago
one month ago

But instead I see:
2 months ago
46 minutes ago
5 weeks ago
one month ago
Within the last minute

I should also mention that sorting by the "Track number" does, in fact, produce the behavior I expect.
Comment 1 Myriam Schweingruber 2009-12-17 18:03:09 UTC 
Is this still valid with any of the current Amarok versions, be this 2.2.1, 2.2.2beta or 2.2-git?
Comment 2 Teo Mrnjavac 2009-12-20 13:29:12 UTC 
commit 52355fcb4d1a3b07d325a0981a21b4a5adba6376
Author: Teo Mrnjavac <teo@getamarok.com>
Date:   Sun Dec 20 13:24:53 2009 +0100

    Handle playlist sorting by time since last played as a special case.
    
    BUG:214761

diff --git a/src/playlist/proxymodels/SortAlgorithms.cpp b/src/playlist/proxymodels/SortAlgorithms.cpp
index fe4fe09..23e9f1d 100644
--- a/src/playlist/proxymodels/SortAlgorithms.cpp
+++ b/src/playlist/proxymodels/SortAlgorithms.cpp
@@ -16,7 +16,7 @@
 
 #include "SortAlgorithms.h"
 
-#include "Debug.h"
+#include "AbstractModel.h"
 
 namespace Playlist
 {
@@ -32,7 +32,26 @@ multilevelLessThan::operator()( int rowA, int rowB)
             return static_cast<bool>( qrand() % 2 );
         QVariant dataA = m_sourceProxy->index( rowA, currentCategory ).data();  //FIXME: are you sure you need to do comparisons on sourceProxy indexes?
         QVariant dataB = m_sourceProxy->index( rowB, currentCategory ).data();  //or better, are you sure those rowA and rowB don't need a rowToSource around them?
-        if( m_scheme.level( i ).isString() )
+
+        //Handle "Last Played" as a special case because the time since last played is not
+        //reported as an int in the data columns.
+        //Also, the verdicts are inverted because I answer to the question about the time
+        //since the track was played by comparing the absolute time when the track was last
+        //played.
+        if( m_scheme.level( i ).category() == Playlist::LastPlayed )
+        {
+            Meta::TrackPtr trackA = dynamic_cast< AbstractModel * >( m_sourceProxy )->trackAt( rowA );
+            Meta::TrackPtr trackB = dynamic_cast< AbstractModel * >( m_sourceProxy )->trackAt( rowB );
+            if( trackA->lastPlayed() < trackB->lastPlayed() )
+                verdict = 0;
+            else if( trackA->lastPlayed() > trackB->lastPlayed() )
+                verdict = 1;
+            else
+                verdict = 2;
+        }
+
+        //And now the comparison logic for ordinary columns.
+        else if( m_scheme.level( i ).isString() )
         {
             if( dataA.toString().toLower() < dataB.toString().toLower() )
                 verdict = 1;
diff --git a/src/playlist/proxymodels/SortAlgorithms.h b/src/playlist/proxymodels/SortAlgorithms.h
index 53bcf76..d47b92d 100644
--- a/src/playlist/proxymodels/SortAlgorithms.h
+++ b/src/playlist/proxymodels/SortAlgorithms.h
@@ -35,7 +35,7 @@ struct multilevelLessThan
 {
     /**
      * Constructor.
-     * @param sourceProxy a pointer to the FilteProxy instance.
+     * @param sourceProxy a pointer to the underlying proxy instance.
      * @param scheme the sorting scheme that needs to be applied.
      */
     multilevelLessThan( QAbstractItemModel *sourceProxy, const SortScheme &scheme )