Adds call to API client for charm search

Adds call to API client for charm search

-Adds a method, search, to CharmworldAPI0, to make the search API call.
-Adds a test for it.

Nothing in this is wired into anything; it's used by a subsequent branch that
is doing the sidebar search view.

https://code.launchpad.net/~jcsackett/juju-gui/search-api/+merge/158929

(do not edit description out of merge proposal)

[j.c.sackett, 2013-04-15 14:07:22]

Please take a look.

[jeff.pihach, 2013-04-15 14:13:56]

LGTM with trivial and cleanup in the test.

https://codereview.appspot.com/8612045/diff/1/app/store/charm.js
File app/store/charm.js (right):

https://codereview.appspot.com/8612045/diff/1/app/store/charm.js#newcode208
app/store/charm.js:208: * @param {String} text the search text.
Missing a couple params.

https://codereview.appspot.com/8612045/diff/1/test/test_charm_store.js
File test/test_charm_store.js (right):

https://codereview.appspot.com/8612045/diff/1/test/test_charm_store.js#newcod...
test/test_charm_store.js:286: api = new Y.juju.Charmworld0({
should be destroyed?

[rharding, 2013-04-15 14:24:36]

LGTM with a couple of notes on naming/var setup.

https://codereview.appspot.com/8612045/diff/1/app/store/charm.js
File app/store/charm.js (right):

https://codereview.appspot.com/8612045/diff/1/app/store/charm.js#newcode208
app/store/charm.js:208: * @param {String} text the search text.
can we standardize on term for the search text? We'll use that in the query
params for the search and it'd be cool to keep it in sync.

https://codereview.appspot.com/8612045/diff/1/test/test_charm_store.js
File test/test_charm_store.js (right):

https://codereview.appspot.com/8612045/diff/1/test/test_charm_store.js#newcod...
test/test_charm_store.js:286: api = new Y.juju.Charmworld0({
multi-line var should be on their own and not grouped. You can move the url up
here though to declare it in the function scope.

https://codereview.appspot.com/8612045/diff/1/test/test_charm_store.js#newcod...
test/test_charm_store.js:299: datasource.realSendRequest =
datasource.sendRequest;
I think you can just set this datasource on the Charmworld0 instance and it'll
use it.

[j.c.sackett, 2013-04-15 15:00:24]

Replies here, and changes coming.

https://codereview.appspot.com/8612045/diff/1/app/store/charm.js
File app/store/charm.js (right):

https://codereview.appspot.com/8612045/diff/1/app/store/charm.js#newcode208
app/store/charm.js:208: * @param {String} text the search text.
On 2013/04/15 14:24:36, rharding wrote:
> can we standardize on term for the search text? We'll use that in the query
> params for the search and it'd be cool to keep it in sync.

Right now we're actually using "text", unless I misunderstand you're suggestion.

http://staging.jujucharms.com/api/0/charms?text=apache2

https://codereview.appspot.com/8612045/diff/1/app/store/charm.js#newcode208
app/store/charm.js:208: * @param {String} text the search text.
On 2013/04/15 14:13:56, jeff.pihach wrote:
> Missing a couple params.

Fixed.

https://codereview.appspot.com/8612045/diff/1/test/test_charm_store.js
File test/test_charm_store.js (right):

https://codereview.appspot.com/8612045/diff/1/test/test_charm_store.js#newcod...
test/test_charm_store.js:286: api = new Y.juju.Charmworld0({
On 2013/04/15 14:13:56, jeff.pihach wrote:
> should be destroyed?

There are other tests creating 'api' and not destroying it. I have however added
destroy for api in this test, as it passes with it. We may want to clean up the
other tests, but I think that's out of scope for this, since it's a bit more
than a driveby fix.

https://codereview.appspot.com/8612045/diff/1/test/test_charm_store.js#newcod...
test/test_charm_store.js:286: api = new Y.juju.Charmworld0({
On 2013/04/15 14:24:36, rharding wrote:
> multi-line var should be on their own and not grouped. You can move the url up
> here though to declare it in the function scope.

Done.

https://codereview.appspot.com/8612045/diff/1/test/test_charm_store.js#newcod...
test/test_charm_store.js:299: datasource.realSendRequest =
datasource.sendRequest;
On 2013/04/15 14:24:36, rharding wrote:
> I think you can just set this datasource on the Charmworld0 instance and it'll
> use it. 

Not sure I follow what you mean; I do set it as the datasource on the instance a
few lines down; do you mean reorder declaration and create the instance after
I've created this, and set it in the declaration?

[j.c.sackett, 2013-04-15 15:58:14]

*** Submitted:

Adds call to API client for charm search

-Adds a method, search, to CharmworldAPI0, to make the search API call.
-Adds a test for it.

Nothing in this is wired into anything; it's used by a subsequent branch that
is doing the sidebar search view.

R=jeff.pihach, rharding
CC=
https://codereview.appspot.com/8612045