Merge lp:~jcsackett/juju-gui/handle-new-charm-charmids into lp:juju-gui/experimental

[j.c.sackett]

Fixes mishandling of new charm IDs

routeDirectCharmID assumed that a charm id would only have 2 parts, series and
charm. New charms have series and charm, but also the ~ identity. This patched
the method to check for 2 parts, or three parts starting with '~'.

https://codereview.appspot.com/10914044/

[j.c.sackett (jcsackett)]

Reviewers: mp+172907_code.launchpad.net,

Message:
Please take a look.

Description:
Fixes mishandling of new charm IDs

routeDirectCharmID assumed that a charm id would only have 2 parts,
series and
charm. New charms have series and charm, but also the ~ identity. This
patched
the method to check for 2 parts, or three parts starting with '~'.

https://code.launchpad.net/~jcsackett/juju-gui/handle-new-charm-charmids/+merge/172907

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M app/subapps/browser/browser.js
   M test/test_browser_app.js

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: test/test_browser_app.js
=== modified file 'test/test_browser_app.js'
--- test/test_browser_app.js 2013-07-02 17:10:25 +0000
+++ test/test_browser_app.js 2013-07-03 20:30:06 +0000
@@ -280,7 +280,7 @@
        assert.equal(req.params, undefined);
      });

- it('/charm/id routes to a sidebar view correcetly', function() {
+ it('/charm/id routes to the default view correctly', function() {
        app = new browser.Browser();
        // Stub out the sidebar so we don't render anything.
        app.sidebar = function() {};
@@ -294,6 +294,17 @@
        assert.equal(req.params.id, 'precise/mysql-10');
      });

+ it('/charm/id handles routes for new charms correctly', function() {
+ app = new browser.Browser();
+ // Stub out the sidebar so we don't render anything.
+ app.sidebar = function() {};
+ var req = {
+ path: '~foo/precise/mysql-10/'
+ };
+ app.routeDirectCharmId(req, null, next);
+ assert.equal(req.params.id, '~foo/precise/mysql-10');
+ });
+
      it('does not add sidebar to urls that do not require it', function() {
        app = new browser.Browser();

Index: app/subapps/browser/browser.js
=== modified file 'app/subapps/browser/browser.js'
--- app/subapps/browser/browser.js 2013-06-27 18:47:57 +0000
+++ app/subapps/browser/browser.js 2013-07-03 20:46:33 +0000
@@ -765,7 +765,8 @@
        var idBits =
req.path.replace(/^\//, '').replace(/\/$/, '').split('/'),
            id = null;

- if (idBits.length === 2) {
+ if ((idBits.length === 3 && idBits[0][0] === '~') || // new charms
+ (idBits.length === 2)) { // reviewed
charms
          id = this._stripViewMode(req.path);
        }
        if (!id) {

[Curtis Hovey (sinzui)]

LGTM. Thanks.

https://codereview.appspot.com/10914044/

[Gary Poster (gary)]

LGTM. Thank you!

https://codereview.appspot.com/10914044/

[lp:~jcsackett/juju-gui/handle-new-charm-charmids]

797. By j.c.sackett on 2013-07-03

Merged in trunk.

[j.c.sackett (jcsackett)]

*** Submitted:

Fixes mishandling of new charm IDs

routeDirectCharmID assumed that a charm id would only have 2 parts,
series and
charm. New charms have series and charm, but also the ~ identity. This
patched
the method to check for 2 parts, or three parts starting with '~'.

R=curtis, gary.poster
CC=
https://codereview.appspot.com/10914044

https://codereview.appspot.com/10914044/

=== modified file 'app/subapps/browser/browser.js'
--- app/subapps/browser/browser.js	2013-07-02 14:58:58 +0000
+++ app/subapps/browser/browser.js	2013-07-03 21:19:25 +0000
@@ -768,7 +768,8 @@
       var idBits = req.path.replace(/^\//, '').replace(/\/$/, '').split('/'),
           id = null;
 
-      if (idBits.length === 2) {
+      if ((idBits.length === 3 && idBits[0][0] === '~') || // new charms
+          (idBits.length === 2)) {                         // reviewed charms
         id = this._stripViewMode(req.path);
       }
       if (!id) {

=== modified file 'test/test_browser_app.js'
--- test/test_browser_app.js	2013-07-03 17:35:21 +0000
+++ test/test_browser_app.js	2013-07-03 21:19:25 +0000
@@ -294,7 +294,7 @@
       assert.equal(req.params, undefined);
     });
 
-    it('/charm/id routes to a sidebar view correcetly', function() {
+    it('/charm/id routes to the default view correctly', function() {
       app = new browser.Browser();
       // Stub out the sidebar so we don't render anything.
       app.sidebar = function() {};
@@ -308,6 +308,17 @@
       assert.equal(req.params.id, 'precise/mysql-10');
     });
 
+    it('/charm/id handles routes for new charms correctly', function() {
+      app = new browser.Browser();
+      // Stub out the sidebar so we don't render anything.
+      app.sidebar = function() {};
+      var req = {
+        path: '~foo/precise/mysql-10/'
+      };
+      app.routeDirectCharmId(req, null, next);
+      assert.equal(req.params.id, '~foo/precise/mysql-10');
+    });
+
     it('does not add sidebar to urls that do not require it', function() {
       app = new browser.Browser();