Bug 9242 - Some album favorites created on controller are incorrect
: Some album favorites created on controller are incorrect
Status: CLOSED FIXED
Product: SB Controller
Classification: Unclassified
Component: UI
: unspecified
: PC Linux (other)
: -- normal with 1 vote (vote)
: 7.2
Assigned To: Ben Klaas
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-08-21 08:13 UTC by Nope
Modified: 2008-12-15 12:06 UTC (History)
3 users (show)

See Also:
Category: ---


Attachments
proposed patch to URI encode favorites url and ignore case and articles (2.14 KB, patch)
2008-08-21 12:37 UTC, Ben Klaas
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nope 2008-08-21 08:13:25 UTC
If I add a favorite for an album from the duet controller (latest firmware), such as for an album titled "The Very Best of Julie London" the favorite will be defined as "db:album.titlesearch=The Very Best of Julie London" which, when you attempt to play will fail as if it cant find the album (also the album art doesn't show up on the actual favorite, just the default no artwork image)

but if you add the very same album as a favorite from SqueezeCenter (7.1), it is added as "db:album.titlesearch=VERY%20BEST%20OF%20JULIE%20LONDON" and works perfectly.

This doesn't happen with every album favorite added from the controller, but certainly a lot of them.

I can provide additional album titles for which this occurs if needed.

Hugely annoying, especially when you are trying to use the duet system entirely from the controller.

Also, please see this thread in the forums for at least 1 other person *probably* experiencing the same issue: 
http://forums.slimdevices.com/showthread.php?t=51020
Comment 1 James Richardson 2008-08-21 09:16:18 UTC
Ben: Can you have a look at this, please prioritize accordingly.
Comment 2 Chris Owens 2008-08-21 09:45:59 UTC
Ben to attach the patch for review before committing.
Comment 3 Ben Klaas 2008-08-21 12:37:10 UTC
Created attachment 3858 [details]
proposed patch to URI encode favorites url and ignore case and articles

this should closely if not perfectly mimic what is currently done with the webUI.

I've fully reproduced the bug (man, nasty...can't believe that didn't come up before), and this patch appears to fully fix it.
Comment 4 Ben Klaas 2008-08-21 12:40:24 UTC
cc:ing Andy for comment on the patch. Andy, this was originally opened against SN, which I believe Queries.pm has nothing to do with...is there also a problem on that side of the fence?


also, something to consider for the future. Came across the comment below in Queries.pm, which judging by its snarky tone must have been written by me a while back. 

# the favorites url to be sent to jive is the album title here
# album id would be (much) better, but that would screw up the favorite on a rescan
# title is a really stupid thing to use, since there's no assurance it's unique

Perhaps when we get around to implementing "new schema", we can start identifying favorites with a less faulty way than db.titlesearch=?
Comment 5 Nope 2008-08-21 12:51:13 UTC
Tried the patch, seems to work perfectly.

Thanks a lot guys!
Comment 6 Ben Klaas 2008-08-21 12:51:51 UTC
forget what I asked about SN-- this bug was never opened against SN...was mixed up with another bug.

on the patch, using ignoreCaseArticles() is not going to work for a band like this--
http://www.last.fm/music/The+The
don't think that's a deal breaker, but it'll be nice if we can move to using a unique id for favorite urls in the future.
Comment 7 Ben Klaas 2008-08-21 14:01:45 UTC
fixed on a private branch, change 22834 (will get merged with public 7.2 next week)

QA, to verify this you should Add to Favorites from the controller for a bunch of various things, and then verify that you can go to Favorites->That Favorite and successfully play it. I recommend:

1. An artist that begins with "The"
2. An album that begins with "The"
3. An album that starts with a number
4. An album/artist/track that contains chars outside of A-Z and a-z. I'd recommend something with punctuation in the title.
5. An album/artist track that meets none of the above 4 items

Comment 8 James Richardson 2008-12-15 12:06:20 UTC
This bug has been fixed in the 7.3.0 release version of SqueezeCenter!

Please download the new version from http://www.slimdevices.com/su_downloads.html if you haven't already.  

If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look.