[Amarok] aa4c4f5: Enable filtering by labels for the sql collections

[Maximilian Kossick]

On Thu, Feb 25, 2010 at 10:35 AM, Nikolaj Hald Nielsen
<nhnfreespirit at gmail.com> wrote:
>> Unless I'm very mistaken this approach does not work correctly. Due to
>> the m:n relation of tracks to labels this will not work when querying
>> for multiple labels, e.g. it will deliver incorrect results when
>> querying for "label:A AND label:B" or various other combinations. And
>> I should now, given that I had to deal with the bug reports in the A1
>> days. In my opionion we will need various subqueries to make it work
>> correctly, which was not possible at all with sqlite.
>
> Unfortunately it seems you are right. "OR" seems to work fine, but
> "AND" does not. The thing is, if this is not the way to implement it,
> then how to do so is beyond me (I found making just this relatively
> small change in the SqlQuerymaker quite intimidating). This is in my
> opinion a pretty big issue, as that basically leaves one person, you,
> who knows how to make this work correctly, and having any parts of the
> code dependent on one person is always a bad idea.

OR is the default, so that's not much of a surprise. But throw in a
couple of NOTs in addition to AND as well (or whatever the filter
syntax for NOT is) and it gets really interesting.

>> Personally I vote for reverting this feature because it is incomplete
>> and doing it properly in a future release instead of releasing it now.
>> Other opinions?
>
> Hmm... I'm not sure. At least I can now find all tracks that I
> actually tagged with something, even if more advanced queries do not
> work. Without this, we might as well remove the label feature
> completely as it is basically useless (it is just another comment
> field)

The current label feature needs a complete rewrite anyway (it only
works for SqlCollection. Labelling magnatune/ampache/media device
track are required too imo if possible at all). I do not think adding
a feature for which we know it is incomplete annd  will receive bug
reports is a good idea. Does not make a very good impression  in my
opinion. Till yesterday querying for labels was just a wishlist item
to which we would implement eventually. Now it's a feature for which
we did not take the time to get right.

>> Oh, and why is there no unit test for a new feature in SqlCollection?
>> the test infrastructure is all there, so it's really easy to add that.
>> Can we start using a TDD approach please?
>
> I need someone to actually show me how to work with the test system.
> Last time I tried running a test, I got 5 pages of XML output and gave
> up on trying to figure out what that actually meant.

Read http://doc.trolltech.com/4.5/qtestlib-manual.html

Set KDE4_BUILD_TESTS = ON in cmake
Set KDE4_TEST_OUTPUT = plaintext in cmake (this is the default,
setting this to xml is only required on the CI server to make use of
the output)

run "make test" (or use ctest directly, e.g. "ctest -VV")

enjoy

Cheers,
Max