Merge lp:~jcsackett/juju-gui/search-routing-and-view into lp:juju-gui/experimental

[j.c.sackett]

Adds search functionality

This adds text search from the search widget
-Search widget propogates search text to browser subapp
-Subapp routes data to search view
-Search view does query from text
-Search view can render search results

As a driveby, this moves repeated apiFailure code out to an extension.

There is an issue outstanding:
-The querystring can be eaten by the routing code in some cases. A follow up
branch is in progress for this problem.

https://codereview.appspot.com/8910043/

[j.c.sackett (jcsackett)]

Reviewers: mp+160110_code.launchpad.net,

Message:
Please take a look.

Description:
Adds search functionality

This adds text search from the search widget
-Search widget propogates search text to browser subapp
-Subapp routes data to search view
-Search view does query from text
-Search view can render search results

As a driveby, this moves repeated apiFailure code out to a utility
function in
Y.juju.browser.views.utils

There are two issues outstanding:
-Fullscreen should work, but doesn't because of rendering problems with
the
fullscreen view (being taken care of by Huw)
-The querystring can be eaten by the routing code in some cases. A
follow up
branch is in progress for this problem.

https://code.launchpad.net/~jcsackett/juju-gui/search-routing-and-view/+merge/160110

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/8910043/

Affected files:
   A [revision details]
   M app/modules-debug.js
   M app/subapps/browser/browser.js
   A app/subapps/browser/templates/search.handlebars
   M app/subapps/browser/views/charm.js
   M app/subapps/browser/views/editorial.js
   A app/subapps/browser/views/search.js
   A app/subapps/browser/views/utils.js
   M app/subapps/browser/views/view.js
   M app/templates/browser-search.handlebars
   M app/widgets/charm-search.js
   M test/index.html
   M test/test_browser_app.js
   M test/test_browser_charm_details.js
   A test/test_browser_search_view.js
   M test/test_browser_search_widget.js
   A test/test_browser_view_utils.js

[Richard Harding (rharding)]

LGTM With some comments

- I really think the apiFailure as extension makes a lot of sense and
will be cleaner.

- I think that renderSearchResults, while longer clears up some
confusion we had as search is two parts. The widget/input, and the
rendering view of results.

- We also need to make sure to note that we need a way to 'undo' search.
There needs to be a way to reset back to the default views and get
editorial content. I had thought an empty search input valueChange would
do it, but it's been brought up that the search term being empty doesn't
mean 'do no search'.

https://codereview.appspot.com/8910043/diff/1/app/modules-debug.js
File app/modules-debug.js (right):

https://codereview.appspot.com/8910043/diff/1/app/modules-debug.js#newcode310
app/modules-debug.js:310: fullpath:
'/juju-ui/subapps/browser/views/utils.js'
Can we add this to the existing view utils? Maybe it's worthwhile to
sync this code up as long as it doesn't collide with anything.

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/browser.js
File app/subapps/browser/browser.js (right):

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/browser.js#newcode209
app/subapps/browser/browser.js:209: if (params.id && params.id !==
'search') {
I wonder if there's some way to make this work like a python web app and
hit only the first route. Then we'd not need to worry about the /search
and /*id collision since we've gone this route.

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/templates/search.handlebars
File app/subapps/browser/templates/search.handlebars (right):

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/templates/search.handlebars#newcode2
app/subapps/browser/templates/search.handlebars:2: <h3>{{ count }}
results found</h3>
check the css used in the other headings and try to match up. Huw's got
a class='section-title' we should make sure to reuse and help him out a
step.

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/views/charm.js
File app/subapps/browser/views/charm.js (right):

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/views/charm.js#newcode71
app/subapps/browser/views/charm.js:71:
Y.juju.browser.views.utils.apiFailure(data, request, this);
Maybe this is a diff bug or something, but this might be great as an
extension you can stick on the view so that the right scope is there and
you can tack it onto any view.

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/views/editorial.js
File app/subapps/browser/views/editorial.js (right):

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/views/editorial.js#newcode149
app/subapps/browser/views/editorial.js:149: /*
typo?

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/views/search.js
File app/subapps/browser/views/search.js (right):

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/views/search.js#newcode33
app/subapps/browser/views/search.js:33: results.map(function(charm) {
does it make sense to use a charm-container here with a larger default
count?

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/views/search.js#newcod...

[Jeff Pihach (hatch)]

LGTM - I'd like to see that apiFailure util moved into an extension as
it's not really a utility method because it requires the view instance
to be passed in

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/browser.js
File app/subapps/browser/browser.js (right):

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/browser.js#newcode209
app/subapps/browser/browser.js:209: if (params.id && params.id !==
'search') {
That could be why the *id is undocumented as it brings with it this
issue :)

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/browser.js#newcode371
app/subapps/browser/browser.js:371: if (this._viewState.viewmode ===
'fullscreen') {
because this is a route callback the req object will have the param for
:viewmode which you should probably use here.

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/views/utils.js
File app/subapps/browser/views/utils.js (right):

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/views/utils.js#newcode25
app/subapps/browser/views/utils.js:25: ns.apiFailure = function(data,
request, view) {
This should probably be an extension to the views because it isn't
really a utility method as it requires the view to function.

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/views/view.js
File app/subapps/browser/views/view.js (right):

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/views/view.js#newcode173
app/subapps/browser/views/view.js:173:
Y.juju.browser.views.utils.apiFaiure(data, request, this);
see comment about view extension in utils

https://codereview.appspot.com/8910043/diff/1/test/test_browser_search_widget.js
File test/test_browser_search_widget.js (right):

https://codereview.appspot.com/8910043/diff/1/test/test_browser_search_widget.js#newcode64
test/test_browser_search_widget.js:64: it('should supports clearing
search string', function() {
should support clearning
small typo.

https://codereview.appspot.com/8910043/

[Curtis Hovey (sinzui)]

Hi Jon. Thank you for the branch. I have a question about a test.

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/templates/search.handlebars
File app/subapps/browser/templates/search.handlebars (right):

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/templates/search.handlebars#newcode3
app/subapps/browser/templates/search.handlebars:3: <div
class="search-results"></div>
the div is not subordinate to the heading, it does not need to be
indented.

https://codereview.appspot.com/8910043/diff/1/test/test_browser_search_view.js
File test/test_browser_search_view.js (right):

https://codereview.appspot.com/8910043/diff/1/test/test_browser_search_view.js#newcode71
test/test_browser_search_view.js:71: });
Do we need a query string test for the cause when there isn't one? I see
renderSearch in app/subapps/browser/browser.js will perform the search
with with {text: ''}.

https://codereview.appspot.com/8910043/

[lp:~jcsackett/juju-gui/search-routing-and-view]

575. By j.c.sackett on 2013-04-22

Merged trunk.

576. By j.c.sackett on 2013-04-22

Made the apiFailure part of an extension and put in main view utils.

577. By j.c.sackett on 2013-04-22

Fixed search template.

578. By j.c.sackett on 2013-04-22

Removed the apifailure stuff from mainview; it's easily shared via extension on views that need it.

579. By j.c.sackett on 2013-04-22

Fixed typos and formatting.

580. By j.c.sackett on 2013-04-22

Fixes from review.

581. By j.c.sackett on 2013-04-22

Fixed test string from review.

582. By j.c.sackett on 2013-04-22

Remaining changes from review.

[j.c.sackett (jcsackett)]

Comments addressed; changes coming in submission.

https://codereview.appspot.com/8910043/diff/1/app/modules-debug.js
File app/modules-debug.js (right):

https://codereview.appspot.com/8910043/diff/1/app/modules-debug.js#newcode310
app/modules-debug.js:310: fullpath:
'/juju-ui/subapps/browser/views/utils.js'
On 2013/04/22 14:08:15, rharding wrote:
> Can we add this to the existing view utils? Maybe it's worthwhile to
sync this
> code up as long as it doesn't collide with anything.

Done.

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/browser.js
File app/subapps/browser/browser.js (right):

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/browser.js#newcode209
app/subapps/browser/browser.js:209: if (params.id && params.id !==
'search') {
On 2013/04/22 14:20:55, jeff.pihach wrote:
> That could be why the *id is undocumented as it brings with it this
issue :)

I've got a follow up branch to deal with the two outstanding issues
referenced in this proposal; I'll investigate this in that branch.

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/browser.js#newcode371
app/subapps/browser/browser.js:371: if (this._viewState.viewmode ===
'fullscreen') {
On 2013/04/22 14:20:55, jeff.pihach wrote:
> because this is a route callback the req object will have the param
for
> :viewmode which you should probably use here.
Done, thanks.

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/templates/search.handlebars
File app/subapps/browser/templates/search.handlebars (right):

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/templates/search.handlebars#newcode2
app/subapps/browser/templates/search.handlebars:2: <h3>{{ count }}
results found</h3>
On 2013/04/22 14:08:15, rharding wrote:
> check the css used in the other headings and try to match up. Huw's
got a
> class='section-title' we should make sure to reuse and help him out a
step.

Done, thanks.

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/templates/search.handlebars#newcode3
app/subapps/browser/templates/search.handlebars:3: <div
class="search-results"></div>
On 2013/04/22 14:33:07, curtis wrote:
> the div is not subordinate to the heading, it does not need to be
indented.

Fixed, thanks

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/views/charm.js
File app/subapps/browser/views/charm.js (right):

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/views/charm.js#newcode71
app/subapps/browser/views/charm.js:71:
Y.juju.browser.views.utils.apiFailure(data, request, this);
On 2013/04/22 14:08:15, rharding wrote:
> Maybe this is a diff bug or something, but this might be great as an
extension
> you can stick on the view so that the right scope is there and you can
tack it
> onto any view.

Done.

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/views/editorial.js
File app/subapps/browser/views/editorial.js (right):

https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/views/editorial.js#newcode149
app/subapps/browser/views/editorial.js:149: /*
On 2013/04/22 14:08:15, rharding wrote:
> typo?
Fixed.

https://codereview.appspot.com/8910043/d...

[lp:~jcsackett/juju-gui/search-routing-and-view]

583. By j.c.sackett on 2013-04-22

Testfix.

[Curtis Hovey (sinzui)]

LGTM, thanks.

https://codereview.appspot.com/8910043/

[lp:~jcsackett/juju-gui/search-routing-and-view]

584. By j.c.sackett on 2013-04-22

Revert a typo, silencing lint.

585. By j.c.sackett on 2013-04-23

Merged trunk.

586. By j.c.sackett on 2013-04-23

Removed stray debugger statement.

587. By j.c.sackett on 2013-04-23

Comments.

[j.c.sackett (jcsackett)]

*** Submitted:

Adds search functionality

This adds text search from the search widget
-Search widget propogates search text to browser subapp
-Subapp routes data to search view
-Search view does query from text
-Search view can render search results

As a driveby, this moves repeated apiFailure code out to an extension.

There is an issue outstanding:
-The querystring can be eaten by the routing code in some cases. A
follow up
branch is in progress for this problem.

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

https://codereview.appspot.com/8910043/

=== modified file 'app/modules-debug.js'
--- app/modules-debug.js	2013-04-13 00:18:21 +0000
+++ app/modules-debug.js	2013-04-23 13:47:38 +0000
@@ -288,6 +288,10 @@
           fullpath: '/juju-ui/subapps/browser/views/fullscreen.js'
         },
 
+        'subapp-browser-searchview': {
+          fullpath: '/juju-ui/subapps/browser/views/search.js'
+        },
+
         'subapp-browser-sidebar': {
           fullpath: '/juju-ui/subapps/browser/views/sidebar.js',
           requires: [
@@ -298,8 +302,7 @@
         },
 
         'subapp-browser-editorial': {
-          fullpath: '/juju-ui/subapps/browser/views/editorial.js',
-          requires: ['subapp-browser-sidebar']
+          fullpath: '/juju-ui/subapps/browser/views/editorial.js'
         },
 
         //Browser Models

=== modified file 'app/subapps/browser/browser.js'
--- app/subapps/browser/browser.js	2013-04-22 17:11:25 +0000
+++ app/subapps/browser/browser.js	2013-04-23 13:47:38 +0000
@@ -59,9 +59,14 @@
       if (this._viewState.charmID) {
         urlParts.push(this._viewState.charmID);
       }
-
-      // Always end on a /
-      return urlParts.join('/');
+      var url = urlParts.join('/');
+      if (this._viewState.querystring) {
+        url = Y.Lang.sub('{ url }?{ qs }', {
+          url: url,
+          qs: this._viewState.querystring
+        });
+      }
+      return url;
     },
 
     /**
@@ -197,7 +202,7 @@
       this._viewState.viewmode = params.viewmode;
 
       // Check for a charm id in the request.
-      if (params.id) {
+      if (params.id && params.id !== 'search') {
         this._viewState.charmID = params.id;
       } else {
         this._viewState.charmID = null;
@@ -264,9 +269,9 @@
     },
 
     /**
-     * Render the sidebar view of a specific charm to the client.
+     * Render the charm details view
      *
-     * @method sidebarCharm
+     * @method renderCharmDetails
      * @param {Request} req current request object.
      * @param {Response} res current response object.
      * @param {function} next callable for the next route in the chain.
@@ -314,7 +319,6 @@
     renderEditorial: function(req, res, next) {
       // If loading the interesting content then it's not a search going on.
       var container = this.get('container'),
-          editorialContainer,
           extraCfg = {};
 
       if (this._viewState.viewmode === 'fullscreen') {
@@ -343,11 +347,35 @@
     },
 
     /**
-      Place holder for a method to render out search so we can test url parsing
-
-    */
+     * Render search results
+     *
+     * @method renderSearchResults
+     * @param {Request} req current request object.
+     * @param {Response} res current response object.
+     * @param {function} next callable for the next route in the chain.
+     */
     renderSearchResults: function(req, res, next) {
-      console.log('rendered search results.');
+      var container = this.get('container'),
+          extraCfg = {},
+          query;
+      if (this._viewState.querystring) {
+        query = Y.QueryString.parse(this._viewState.querystring);
+      } else {
+        // If there's no querystring, we need a default "empty" search.
+        query = {text: ''};
+      }
+
+      if (req.params.viewmode === 'fullscreen') {
+        extraCfg.renderTo = container.one('.bws-view-data');
+        extraCfg.isFullscreen = true;
+      } else {
+        extraCfg.renderTo = container.one('.bws-content');
+      }
+      extraCfg.text = query.text;
+      this._search = new Y.juju.browser.views.BrowserSearchView(
+          this._getViewCfg(extraCfg));
+      this._search.render();
+      this._search.addTarget(this);
     },
 
     /**
@@ -539,10 +567,12 @@
   requires: [
     'juju-charm-store',
     'juju-models',
+    'querystring-parse',
     'sub-app',
     'subapp-browser-charmview',
     'subapp-browser-editorial',
     'subapp-browser-fullscreen',
+    'subapp-browser-searchview',
     'subapp-browser-sidebar'
   ]
 });

=== added file 'app/subapps/browser/templates/search.handlebars'
--- app/subapps/browser/templates/search.handlebars	1970-01-01 00:00:00 +0000
+++ app/subapps/browser/templates/search.handlebars	2013-04-23 13:47:38 +0000
@@ -0,0 +1,4 @@
+<div id="bws-search">
+  <h3 class="section-title">{{ count }} results found</h3>
+  <div class="search-results"></div>
+</div>

=== modified file 'app/subapps/browser/views/charm.js'
--- app/subapps/browser/views/charm.js	2013-04-19 21:01:13 +0000
+++ app/subapps/browser/views/charm.js	2013-04-23 13:47:38 +0000
@@ -18,7 +18,8 @@
    */
   ns.BrowserCharmView = Y.Base.create('browser-view-charmview', Y.View, [
     widgets.browser.IndicatorManager,
-    Y.Event.EventTracker
+    Y.Event.EventTracker,
+    views.utils.apiFailingView
   ], {
 
     template: views.Templates.browser_charm,
@@ -77,19 +78,7 @@
      *
      */
     apiFailure: function(data, request) {
-      var message;
-      if (data && data.type) {
-        message = 'Charm API error of type: ' + data.type;
-      } else {
-        message = 'Charm API server did not respond';
-      }
-      this.get('db').notifications.add(
-          new models.Notification({
-            title: 'Failed to load sidebar content.',
-            message: message,
-            level: 'error'
-          })
-      );
+      this._apiFailure(data, request, 'Failed to load charm details.');
     },
 
     /**

=== modified file 'app/subapps/browser/views/editorial.js'
--- app/subapps/browser/views/editorial.js	2013-04-19 17:09:07 +0000
+++ app/subapps/browser/views/editorial.js	2013-04-23 13:47:38 +0000
@@ -22,7 +22,9 @@
    * @extends {juju.browser.views.Editorial}
    *
    */
-  ns.EditorialView = Y.Base.create('browser-view-sidebar', Y.View, [], {
+  ns.EditorialView = Y.Base.create('browser-view-sidebar', Y.View, [
+    views.utils.apiFailingView
+  ], {
     template: views.Templates.editorial,
 
     events: {
@@ -51,6 +53,18 @@
     },
 
     /**
+     * Generates a message to the user based on a bad api call.
+     *
+     * @method apiFailure
+     * @param {Object} data the json decoded response text.
+     * @param {Object} request the original io_request object for debugging.
+     *
+     */
+    apiFailure: function(data, request) {
+      this._apiFailure(data, request, 'Failed to load sidebar content.');
+    },
+
+    /**
      * General YUI initializer.
      *
      * @method initializer
@@ -131,21 +145,7 @@
           ];
         },
 
-        'failure': function(data, request) {
-          var message;
-          if (data && data.type) {
-            message = 'Charm API error of type: ' + data.type;
-          } else {
-            message = 'Charm API server did not respond';
-          }
-          this.get('db').notifications.add(
-              new models.Notification({
-                title: 'Failed to load landing page content.',
-                message: message,
-                level: 'error'
-              })
-          );
-        }
+        'failure': this.apiFailure
       }, this);
     },
 
@@ -172,33 +172,28 @@
        * @attribute isFullscreen
        * @default false
        * @type {Boolean}
-       *
        */
       isFullscreen: {
         value: false
       },
+
       /**
        * What is the container node we should render our container into?
        *
        * @attribute renderTo
        * @default undefined
        * @type {Node}
-       *
        */
-      renderTo: {
+      renderTo: {},
 
-      },
       /**
        * The Charmworld0 Api store instance for loading content.
        *
        * @attribute store
        * @default undefined
        * @type {Charmworld0}
-       *
        */
-      store: {
-
-      }
+      store: {}
     }
   });
 
@@ -210,6 +205,7 @@
     'juju-charm-store',
     'juju-models',
     'juju-templates',
+    'juju-view-utils',
     'view'
   ]
 });

=== added file 'app/subapps/browser/views/search.js'
--- app/subapps/browser/views/search.js	1970-01-01 00:00:00 +0000
+++ app/subapps/browser/views/search.js	2013-04-23 13:47:38 +0000
@@ -0,0 +1,110 @@
+'use strict';
+
+
+/**
+ * Provides searching functionality for the charm browser.
+ *
+ * @namespace juju
+ * @module browser
+ * @submodule views
+ */
+YUI.add('subapp-browser-searchview', function(Y) {
+  var ns = Y.namespace('juju.browser.views'),
+      views = Y.namespace('juju.views'),
+      widgets = Y.namespace('juju.widgets'),
+      models = Y.namespace('juju.models');
+
+  ns.BrowserSearchView = Y.Base.create('browser-view-searchview', Y.View, [
+    views.utils.apiFailingView
+  ], {
+    template: views.Templates.search,
+    /**
+     * Renders the search results from the the store query.
+     *
+     * @method _renderSearchResults
+     * @param {Y.Node} container Optional container to render results to.
+     */
+    _renderSearchResults: function(results) {
+      var target = this.get('renderTo'),
+          tpl = this.template({count: results.size()}),
+          tplNode = Y.Node.create(tpl),
+          container = tplNode.one('.search-results');
+
+      results.map(function(charm) {
+        var ct = new widgets.browser.CharmToken(charm.getAttrs());
+        ct.render(container);
+      });
+      target.setHTML(tplNode);
+    },
+
+    /**
+     * Generates a message to the user based on a bad api call.
+     *
+     * @method apiFailure
+     * @param {Object} data the json decoded response text.
+     * @param {Object} request the original io_request object for debugging.
+     *
+     */
+    apiFailure: function(data, request) {
+      this._apiFailure(data, request, 'Failed to load search results.');
+    },
+
+    /**
+     * Renders the searchview, rendering search results for the view's search
+     * text.
+     *
+     * @method render
+     */
+    render: function() {
+      var text = this.get('text');
+      this.get('store').search(text, {
+        'success': function(data) {
+          var results = this.get('store').resultsToCharmlist(data.result);
+          this._renderSearchResults(results);
+        },
+        'failure': this.apiFailure
+      }, this);
+    }
+  }, {
+    ATTRS: {
+      /**
+       * The container node the view is rendering to.
+       *
+       * @attribute renderTo
+       * @default undefined
+       * @type {Y.Node}
+       */
+      renderTo: {},
+
+      /**
+       * An instance of the Charmworld API object to hit for any data that
+       * needs fetching.
+       *
+       * @attribute store
+       * @default undefined
+       * @type {Charmworld0}
+       *
+       */
+      store: {},
+
+      /**
+       * The text being searched on
+       *
+       * @attribute text
+       * @default ''
+       * @type {String}
+       */
+      text: {}
+    }
+  });
+
+}, '0.1.0', {
+  requires: [
+    'base-build',
+    'browser-charm-token',
+    'browser-overlay-indicator',
+    'event-tracker',
+    'juju-view-utils',
+    'view'
+  ]
+});

=== modified file 'app/subapps/browser/views/view.js'
--- app/subapps/browser/views/view.js	2013-04-16 23:39:54 +0000
+++ app/subapps/browser/views/view.js	2013-04-23 13:47:38 +0000
@@ -107,7 +107,17 @@
      *
      */
     _searchChanged: function(ev) {
-      console.log('search changed.');
+      // NB: This is temporary; eventually filtering will include categories,
+      // and the Filter object will handle qs generation. But it's an unwieldy
+      // url to parse while we only support text search.
+      var qs = Y.QueryString.stringify({text: ev.details[0]});
+      var change = {
+        search: true,
+        querystring: qs
+      };
+      this.fire('viewNavigate', {
+        change: change
+      });
     },
 
     /**
@@ -151,31 +161,6 @@
     },
 
     /**
-     * Shared method to generate a message to the user based on a bad api
-     * call.
-     *
-     * @method apiFailure
-     * @param {Object} data the json decoded response text.
-     * @param {Object} request the original io_request object for debugging.
-     *
-     */
-    apiFailure: function(data, request) {
-      var message;
-      if (data && data.type) {
-        message = 'Charm API error of type: ' + data.type;
-      } else {
-        message = 'Charm API server did not respond';
-      }
-      this.get('db').notifications.add(
-          new models.Notification({
-            title: 'Failed to load sidebar content.',
-            message: message,
-            level: 'error'
-          })
-      );
-    },
-
-    /**
      * Destroy this view and clear from the dom world.
      *
      * @method destructor
@@ -269,6 +254,7 @@
     'event-tracker',
     'juju-charm-store',
     'juju-models',
+    'querystring-stringify',
     'view'
   ]
 });

=== modified file 'app/templates/browser-search.handlebars'
--- app/templates/browser-search.handlebars	2013-04-16 21:58:19 +0000
+++ app/templates/browser-search.handlebars	2013-04-23 13:47:38 +0000
@@ -6,7 +6,7 @@
     <div class="bws-searchbox">
         <form>
             <input type="search" name="bws-search"
-                value="{{ term }}"
+                value="{{ text }}"
                 placeholder="Search&hellip;"/>
         </form>
     </div>

=== modified file 'app/views/utils.js'
--- app/views/utils.js	2013-04-15 15:45:18 +0000
+++ app/views/utils.js	2013-04-23 13:47:38 +0000
@@ -1,15 +1,16 @@
 'use strict';
 
+
 /**
  * The view utils.
  *
  * @module views
  * @submodule views.utils
  */
-
 YUI.add('juju-view-utils', function(Y) {
 
   var views = Y.namespace('juju.views'),
+      models = Y.namespace('juju.models'),
       utils = Y.namespace('juju.views.utils');
 
   /*jshint bitwise: false*/
@@ -1288,13 +1289,59 @@
     }
   });
 
+  /**
+   * Extension for views to provide an apiFailure method.
+   *
+   * @class apiFailure
+   */
+  utils.apiFailingView = function() {
+    this._initAPIFailingView();
+  };
+  utils.apiFailingView.prototype = {
+    /**
+     * Constructor
+     *
+     * @method _initAPIFailingView
+     */
+    _initAPIFailingView: function() {},
+
+    /**
+     * Shared method to generate a message to the user based on a bad api
+     * call from a view.
+     *
+     * @method apiFailure
+     * @param {Object} data the json decoded response text.
+     * @param {Object} request the original io_request object for debugging.
+     * @param {String} title the title for the generated notification.
+     */
+    _apiFailure: function(data, request, title) {
+      var message;
+      if (data && data.type) {
+        message = 'Charm API error of type: ' + data.type;
+      } else {
+        message = 'Charm API server did not respond';
+      }
+      if (!title) {
+        title = 'Unidentified API failure';
+      }
+      this.get('db').notifications.add(
+          new models.Notification({
+            title: title,
+            message: message,
+            level: 'error'
+          })
+      );
+    }
+  };
 }, '0.1.0', {
-  requires: ['base-build',
-             'handlebars',
-             'node',
-             'view',
-             'panel',
-             'json-stringify',
-             'gallery-markdown',
-             'datatype-date-format']
+  requires: [
+    'base-build',
+    'handlebars',
+    'node',
+    'view',
+    'panel',
+    'json-stringify',
+    'gallery-markdown',
+    'datatype-date-format'
+  ]
 });

=== modified file 'app/widgets/charm-search.js'
--- app/widgets/charm-search.js	2013-04-09 17:00:45 +0000
+++ app/widgets/charm-search.js	2013-04-23 13:47:38 +0000
@@ -34,6 +34,17 @@
     TEMPLATE: templates['browser-search'],
 
     /**
+     * Halt page reload from form submit and let the app know we have a new
+     * search.
+     *
+     * @method _handleSubmit
+     * @param {Event} ev the submit event.
+     */
+    _handleSubmit: function(ev) {
+      ev.halt();
+      this.fire(this.EVT_UPDATE_SEARCH, this.get('text'));
+    },
+    /**
      * Expose to the outside world that we've got a request to go fullscreen.
      *
      * @method _toggleFullScreen
@@ -76,6 +87,10 @@
           container.one('.toggle-fullscreen').on(
               'click', this._toggleFullScreen, this)
       );
+      this.addEvent(
+          container.one('form').on(
+              'submit', this._handleSubmit, this)
+      );
 
       // Note that the search could be updated either from our internal input
       // control, or it could come from someone outside of the widget asking
@@ -84,6 +99,7 @@
       var input = container.one('input');
       this.addEvent(
           input.on('valueChange', function(ev) {
+            this.set('text', ev.newVal);
             this.fire(this.EVT_SEARCH_CHANGED);
           }, this)
       );
@@ -155,13 +171,13 @@
       },
 
       /**
-       * @attribute term
-       * @default undefined
+       * The search text.
+       *
+       * @attribute text
+       * @default ''
        * @type {String}
-       *
        */
-      term: {}
-
+      text: {}
     }
   });
 

=== modified file 'test/index.html'
--- test/index.html	2013-04-10 11:57:11 +0000
+++ test/index.html	2013-04-23 13:47:38 +0000
@@ -30,36 +30,38 @@
 
   <!-- Tests (Alphabetical)-->
 
+  <script src="test_app_hotkeys.js"></script>
   <script src="test_app.js"></script>
-  <script src="test_app_hotkeys.js"></script>
   <script src="test_application_notifications.js"></script>
   <script src="test_browser_app.js"></script>
   <script src="test_browser_charm_details.js"></script>
   <script src="test_browser_models.js"></script>
+  <script src="test_browser_search_view.js"></script>
+  <script src="test_browser_search_widget.js"></script>
+  <script src="test_browser_view_utils.js"></script>
   <script src="test_charm_collection_view.js"></script>
   <script src="test_charm_configuration.js"></script>
   <script src="test_charm_container.js"></script>
   <script src="test_charm_panel.js"></script>
+  <script src="test_charm_store.js"></script>
   <script src="test_charm_token.js"></script>
-  <script src="test_browser_search_widget.js"></script>
-  <script src="test_charm_store.js"></script>
   <script src="test_charm_view.js"></script>
   <script src="test_console.js"></script>
   <script src="test_d3_components.js"></script>
   <script src="test_endpoints.js"></script>
+  <script src="test_env_go.js"></script>
+  <script src="test_environment_view.js"></script>
   <script src="test_env.js"></script>
+  <script src="test_env_python.js"></script>
   <script src="test_event_tracker.js"></script>
-  <script src="test_env_go.js"></script>
-  <script src="test_env_python.js"></script>
-  <script src="test_environment_view.js"></script>
   <script src="test_fakebackend.js"></script>
-  <script src="test_overlay_indicator.js"></script>
   <script src="test_landscape.js"></script>
   <script src="test_login.js"></script>
+  <script src="test_model_handlers.js"></script>
   <script src="test_model.js"></script>
-  <script src="test_model_handlers.js"></script>
   <script src="test_notifications.js"></script>
   <script src="test_notifier_widget.js"></script>
+  <script src="test_overlay_indicator.js"></script>
   <script src="test_panzoom.js"></script>
   <script src="test_prettify.js"></script>
   <script src="test_routing.js"></script>

=== modified file 'test/test_browser_charm_details.js'
--- test/test_browser_charm_details.js	2013-04-22 15:40:54 +0000
+++ test/test_browser_charm_details.js	2013-04-23 13:47:38 +0000
@@ -46,7 +46,6 @@
       delete window.juju_config;
     });
 
-    // Ensure the search results are rendered inside the container.
     it('should be able to locate a readme file', function() {
       view = new CharmView({
         charm: new models.BrowserCharm({

=== added file 'test/test_browser_search_view.js'
--- test/test_browser_search_view.js	1970-01-01 00:00:00 +0000
+++ test/test_browser_search_view.js	2013-04-23 13:47:38 +0000
@@ -0,0 +1,78 @@
+'use strict';
+
+
+describe('search view', function() {
+  var apiURL,
+      container,
+      view,
+      Y;
+
+  before(function(done) {
+    Y = YUI(GlobalConfig).use(
+        'json',
+        'juju-charm-store',
+        'node',
+        'subapp-browser-searchview',
+        function(Y) {
+          done();
+        });
+  });
+
+  beforeEach(function() {
+    // Mock out a dummy location for the Store used in view instances.
+    window.juju_config = {charmworldURL: 'http://localhost'};
+    container = Y.Node.create('<div id="container"></div>');
+    Y.one('body').append(container);
+    view = new Y.juju.browser.views.BrowserSearchView({text: 'foo'});
+    //
+    // Create monkeypatched store to verify right method is called.
+    apiURL = '';
+    var fakeStore = new Y.juju.Charmworld0({});
+    var sampleData = {
+      result: [{
+        id: 'foo/bar-2',
+        name: 'bar',
+        description: 'some charm named bar'
+      }]
+    };
+    fakeStore.set('datasource', {
+      sendRequest: function(params) {
+        // Stubbing the server callback value
+        apiURL = params.request;
+        params.callback.success({
+          response: {
+            results: [{
+              responseText: Y.JSON.stringify(sampleData)
+            }]
+          }
+        });
+      }
+    });
+    view.set('store', fakeStore);
+    view.set('renderTo', container);
+  });
+
+  afterEach(function() {
+    delete window.juju_config;
+    view.destroy();
+    container.remove(true);
+  });
+
+  it('exists', function() {
+    assert.isObject(view);
+  });
+
+  it('renders correctly', function() {
+    view.render();
+    assert.equal('charms?text=foo', apiURL);
+    assert.equal(1, Y.all('.yui3-charmtoken').size());
+    var charmText = Y.one('.yui3-charmtoken').one('.title').get('text');
+    assert.equal(charmText.replace(/\s+/g, ''), 'bar');
+  });
+
+  it('handles empty text for search', function() {
+    view.set('text', '');
+    view.render();
+    assert.equal('charms?text=', apiURL);
+  });
+});

=== modified file 'test/test_browser_search_widget.js'
--- test/test_browser_search_widget.js	2013-03-25 19:12:45 +0000
+++ test/test_browser_search_widget.js	2013-04-23 13:47:38 +0000
@@ -44,6 +44,7 @@
       // now trigger the event and make sure that it fired to our custom
       // watcher outside the widget.
       triggered.should.eql(true);
+      assert.equal('test', this.get('text'));
       done();
     });
 
@@ -60,20 +61,9 @@
     container.one('input').get('value').should.eql('test');
   });
 
-  it('should supports clearing search string', function() {
-    var search = new Search({
-      term: 'test'
-    });
-    search.render(container);
-    container.one('input').get('value').should.eql('test');
-
-    search.clearSearch();
-    container.one('input').get('value').should.eql('');
-  });
-
-  it('should supports clearing search string', function() {
-    var search = new Search({
-      term: 'test'
+  it('should support clearing search string', function() {
+    var search = new Search({
+      text: 'test'
     });
     search.render(container);
     container.one('input').get('value').should.eql('test');
@@ -109,5 +99,4 @@
     toggle.simulate('click');
     triggered.should.eql(true);
   });
-
 });