MusicBottle / MusicBottle

Bottlin' musicmetadata to put on display since 2-0-12!
GNU General Public License v2.0
10 stars 7 forks source link

Add a simple search view function #35

Closed machakux closed 11 years ago

Freso commented 11 years ago

I added a few specific comments mostly regarding code style. However, I note that the Travis test is also failing - which also seems to be all coding style errors. Feel free to try and catch my on IRC if any of the automated test messages are difficult to understand. :)

Freso commented 11 years ago

Travis still complains about coding style: https://travis-ci.org/MusicBottle/MusicBottle/jobs/6412750#L219

Freso commented 11 years ago

I've just tried to play around with a small bit.

  1. What is the JSON in the debug section? It doesn't look like the search results - the JSON for the top search result? (I could look at the code, but...)
  2. Searching for "4 slags sort" in release groups gives me.. nothing. Well, I get JSON data about the release group, but no link. Or even just a "4 slags sort" unclickable name.
  3. FreeDB is still there as a search option, and we really don't care about that. (I think @CallerNo6 already mentioned this in a comment somewhere.)

Other than that, looking great for a start. I'd like to see some tests too. (I'm sloowly increasing test coverage (or trying to anyway), so I'd be very happy if new code had tests to both ensure that it works as intended, and that future changes don't break it (unintentionally, anyway).)

Edit: Oh, and better URLs! This can probably come later, so no worries, but /search/?query=4+slags+sort&type=release-group? Really? Something like /search/release-group/4+slags+sort or even just /search/release-group/?query=4+slags+sort I'd prefer a lot more.

machakux commented 11 years ago
  1. The debug section displays a Python object (dictionary) of results obtained by loading/deserializing JSON response from web service plus one additional field (type) to define results type.
  2. I have added unclickable name listing for release-groups, recordings and labels in search results template. SHA: b0b1cf13f42ea3d3851b3a3204bbba22ca85a6cd
  3. I am sorry I didn't remove FreeDB search option from search form, but now its removed SHA: b0b1cf13f42ea3d3851b3a3204bbba22ca85a6cd

I didn't add more entity views so as to make search results clickable because I think I may add too much non-reviewed code for a start. Also when you have time you can try to check out the mock-up at https://github.com/machakux/MusicBottleMockUp (just try to install and runserver within similiar/same enviroment as MusicBottle based on MusicBottle with one additional dependancy (beautifulsoup4) and configured to run on port 19049. Probably it has one or few acceptable implementations).

URLs patterns for search can be modified later, For now I was trying to follow the MusicBrainz.org URL scheme.