Merge lp:~bac/charmworld/bundle-icon-path into lp:~juju-jitsu/charmworld/trunk

[Brad Crittenden]

Fix path to bundle icon. Use default icon if the bundle does not supply one.

In the current design, all bundles are to use the default bundle icon. However, there is a bug that does not generate the correct URL and the icon shows up as broken in charmworld, witness:

http://staging.jujucharms.com/~benji/bundle/wiki/wiki

This branch fixes that branch. It also removes the condition in icon_path so that the path to the icon is returned whether the icon exists or not, which then gets redirected to the default icon.

A test was incorrectly reinforcing the wrong URL and has been fixed.

[Benji York (benji)]

The branch looks good. Two small observations:

The function call on line 15 of the diff would probably all fit on one line.

"id" on line 35 (which you didn't write), should be capitalized -- unless we're getting all Freudian here.

review: Approve

[Juju Gui Bot (juju-gui-bot)]

review: Approve (continuous-integration)

[Juju Gui Bot (juju-gui-bot)]

FAILED: Autolanding.
Unapproved changes made after approval.
http://162.213.35.27:8080/job/charmworld-autoland-lxc/33/
Executed test runs:

review: Needs Fixing (continuous-integration)

[Benji York (benji)]

Approved again.

review: Approve

[Juju Gui Bot (juju-gui-bot)]

review: Approve (continuous-integration)

=== modified file 'charmworld/views/bundles.py'
--- charmworld/views/bundles.py	2013-09-30 16:00:47 +0000
+++ charmworld/views/bundles.py	2013-10-10 20:29:55 +0000
@@ -75,13 +75,8 @@
                 raise
 
     def icon_path(self, request):
-        path = None
-        if self.get_file(request.db, ICON_FN) is not None:
-            api_prefix = request.route_path(
-                'api_3',
-                endpoint='bundle',
-            )
-            path = '/'.join([api_prefix, self.id, ICON_FN])
+        api_prefix = request.route_path('api_3', endpoint='bundle')
+        path = '/'.join([api_prefix, self.id, 'file', ICON_FN])
         return path
 
     def readme_info(self, db):

=== modified file 'charmworld/views/charms.py'
--- charmworld/views/charms.py	2013-09-04 14:03:52 +0000
+++ charmworld/views/charms.py	2013-10-10 20:29:55 +0000
@@ -122,9 +122,9 @@
     icon_path = None
     if quote_key('icon.svg') in charm.files.keys():
         remainder = API3._get_api_id(charm) + '/file/icon.svg'
-        # If we let the route_url do the work it'll escape the ~   in the charm
-        # id for the owner. We generate the base of Thee route via route_url,
-        # but then manually tack on the known 'good' reminder for the path to
+        # If we let the route_url do the work it'll escape the ~ in the charm
+        # ID for the owner. We generate the base of the route via route_url,
+        # but then manually tack on the known 'good' remainder for the path to
         # complete.
         icon_path = request.route_path(
             'api_2_single',

=== modified file 'charmworld/views/tests/test_bundles.py'
--- charmworld/views/tests/test_bundles.py	2013-09-09 13:45:45 +0000
+++ charmworld/views/tests/test_bundles.py	2013-10-10 20:29:55 +0000
@@ -87,7 +87,7 @@
         request = self.getRequest()
         bundle = BundleDetail(data)
         bundle.get_file = lambda s, x: True
-        expected = '/'.join(['/api/3/bundle', bundle.id, 'icon.svg'])
+        expected = '/'.join(['/api/3/bundle', bundle.id, 'file', 'icon.svg'])
         self.assertEqual(expected, bundle.icon_path(request))
 
     def test_readme(self):
@@ -175,6 +175,7 @@
         self.assertEqual(None, found)
 
     def test_personal_bundle(self):
+        self.enable_routes()
         basket = 'ball'
         rev = '4'
         bundle, bundle_data = factory.makeBundle(