Bug 258792 - Collection browser filter doesn't find tracks that were never played
Summary: Collection browser filter doesn't find tracks that were never played
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: Collection Browser (show other bugs)
Version: 2.4-GIT
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: 2.4.0
Assignee: Amarok Developers
URL:
Keywords:
: 258935 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-12-04 13:07 UTC by Benedikt Waldvogel
Modified: 2010-12-08 13:11 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.4


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Benedikt Waldvogel 2010-12-04 13:07:00 UTC
Version:           2.4-GIT (using KDE 4.4.5) 
OS:                Linux

I try to filter all tracks in the collection browser that were never played (i.e. new tracks). I'm using the search string "playcount:0" but the browser shows zero tracks. I've also tried "playcount:<1" and "playcount:<=0" but none of them work either.

Reproducible: Always

Steps to Reproduce:
1) Add some tracks that were never played (i.e. playcount=0)
2) Use the search string "playcount:0" in the collection browser

Actual Results:  
No results

Expected Results:  
List of all tracks that were never played
Comment 1 Benedikt Waldvogel 2010-12-04 13:08:13 UTC
The filter did work in v2.3.0. I'm currently doing a git bisect to find the commit that broke the behavior.
Comment 2 Benedikt Waldvogel 2010-12-05 17:31:59 UTC
I was finally able to finish bisect.
Here's the result:

56334026740a26e3f667ed5ff8812d014d648c77 is the first bad commit
commit 56334026740a26e3f667ed5ff8812d014d648c77
Author: Ralf Engels <ralf.engels@nokia.com>
Date:   Thu Oct 28 14:07:24 2010 +0200

    Fix auto tests
    - fix several broken tests
    - fix sql tests build so that it doesn't rebuild the sqlcollection classes
    - remove several unused functions from Amarok.cpp and the tests
Comment 3 Myriam Schweingruber 2010-12-05 17:46:33 UTC
Confirmed with latest git.
Comment 4 Benedikt Waldvogel 2010-12-05 18:51:08 UTC
This is a workaround:

-playcount:>0
Comment 5 Benedikt Waldvogel 2010-12-05 19:08:49 UTC
I actually found the reason. Tracks that were never played seem to have a playcount of NULL in the database, so the query won't find it because of this change:

@@ -538,16 +570,8 @@ SqlQueryMaker::addNumberFilter( qint64 value, qint64 filter, QueryMaker::NumberC
             break;
     }
 
-    if( (filter == 0 && compare == QueryMaker::Equals)
-     || (filter <  0 && compare == QueryMaker::GreaterThan)
-     || (filter >  0 && compare == QueryMaker::LessThan) )
-    {
-        d->queryFilter += QString( " %1 (%2 %3 %4 or %2 is null)" ).arg( andOr(), nameForValue( value ), comparison, QString::number( filter ) );
-    }
-    else
-    {
-        d->queryFilter += QString( " %1 %2 %3 %4 " ).arg( andOr(), nameForValue( value ), comparison, QString::number( filter ) );
-    }
+    // note: a NULL value in the database means undefined and not 0!
+    d->queryFilter += QString( " %1 %2 %3 %4 " ).arg( andOr(), nameForValue( value ), comparison, QString::number( filter ) );


Reverting exactly that change fixes this bug.
Comment 6 Myriam Schweingruber 2010-12-05 19:31:17 UTC
*** Bug 258935 has been marked as a duplicate of this bug. ***
Comment 7 Benedikt Waldvogel 2010-12-05 22:31:17 UTC
I've two ideas for unit tests that could prevent such bugs in the future:

1) playcount:0 and -playcount:>0 should exactly yield the same results
2) playcount:0 + playcount:>0 should equal all tracks in the database
Comment 8 Ralf Engels 2010-12-06 12:02:55 UTC
Hi Benedikt,
we had this kind of logic before and it fails with the "lastplayed" date.
There we had lastplayed:<12345 also picking up never played songs.

However the fix is quite simple. A song that was just added to the collection obviously was never played (unless it has an embedded playcount).

No problem to fix this.
Comment 9 Myriam Schweingruber 2010-12-08 13:11:17 UTC
Fixed by this commit:

commit 11161fed3fb5a2b60b090cdd9907d56fda45af76
branch master
Author: Ralf Engels <ralf-engels@gmx.de>
Date:   Tue Dec 7 20:41:53 2010 +0100

   Playcount and rating is never written as NULL. Bug:258792
   This is in addition to the previous fix that set the colums to NOT NULL for playcount.
   Rating was already NOT NULL, so this fix only helps when new tracks are scanned.