Merge lp:~sinzui/charmworld/sane-categories into lp:~juju-jitsu/charmworld/trunk

[Curtis Hovey]

Do not include bogus categories in charm data.

The GUI is mis-rendering the icons for charms with invalid categories

RULES

    Pre-implementation: rick
    * Proof does not enforce the 6 valid categories because it wants to
      maintain backward compatibility. The API though, does need to enforce
      the categories to discourage mistakes and community fragmentation.
    * The Charm model object is passing the categories to the API and pages.
      It ensures categories is a list.
      * It must also ensure the categories are one or more of the official
        categories.

    Extra credit
    * Update the charm page to list the categories after the summary.

QA

    * Visit http://staging.jujucharms.com/~nextrevision/precise/openvpn/json
    * Verify the categories property does not contain "application".
    * Visit http://staging.jujucharms.com/~nextrevision/precise/openvpn
    * Verify the categories property shown, but there are no categories.

[Aaron Bentley (abentley)]

review: Approve

[lp:~sinzui/charmworld/sane-categories]

301. By Curtis Hovey on 2013-07-08

Added test to confirm that the API does not include invalid categories.

=== modified file 'charmworld/models.py'
--- charmworld/models.py	2013-07-04 16:30:39 +0000
+++ charmworld/models.py	2013-07-08 20:05:31 +0000
@@ -252,6 +252,15 @@
         'is_featured': False,
     }
 
+    OFFICIAL_CATEGORIES = [
+        'applications',
+        'app-servers',
+        'cache-proxy',
+        'databases',
+        'file-servers',
+        'misc',
+    ]
+
     def __init__(self, charm_data):
         """Init the charm from a charm representation.
 
@@ -491,7 +500,10 @@
 
         The categories come from the charm's metadata.
         """
-        return self._representation['categories']
+        return [
+            category for category in self._representation['categories']
+            if category in self.OFFICIAL_CATEGORIES
+        ]
 
     @property
     def maintainer(self):

=== modified file 'charmworld/static/css/theme.less'
--- charmworld/static/css/theme.less	2013-01-21 14:14:49 +0000
+++ charmworld/static/css/theme.less	2013-07-08 20:05:31 +0000
@@ -70,3 +70,7 @@
 .left-align {
     text-align: left
 }
+
+.field .subordinate {
+    margin-bottom: 0;
+}

=== modified file 'charmworld/templates/charm.pt'
--- charmworld/templates/charm.pt	2013-06-25 08:32:00 +0000
+++ charmworld/templates/charm.pt	2013-07-08 20:05:31 +0000
@@ -39,6 +39,13 @@
             ${charm.summary}
           </div>
 
+          <div class="field">
+            <b>categories</b><br/>
+            <ul class="subordinate">
+              <li tal:repeat="category charm.categories">${category}</li>
+            </ul>
+          </div>
+
           <div class="field" tal:condition="charm.store_data.get('errors', False)">
             <b>Charm Store</b><br/>
             <em>juju deploy ${charm.store_data['address']}</em>

=== modified file 'charmworld/tests/test_models.py'
--- charmworld/tests/test_models.py	2013-07-01 14:45:02 +0000
+++ charmworld/tests/test_models.py	2013-07-08 20:05:31 +0000
@@ -355,11 +355,22 @@
         categories = ['cache-proxy', 'app-servers']
         charm_data = factory.get_charm_json(categories=categories)
         charm = Charm(charm_data)
-        self.assertIs(categories, charm.categories)
+        self.assertEqual(categories, charm.categories)
         # The default is an empty list.
         charm = Charm({})
         self.assertEqual([], charm.categories)
 
+    def test_categories_are_official(self):
+        # Unofficial categories are filtered out of the list.
+        categories = ['application']
+        charm_data = factory.get_charm_json(categories=categories)
+        charm = Charm(charm_data)
+        self.assertEqual(
+            ['applications', 'app-servers', 'cache-proxy', 'databases',
+             'file-servers', 'misc'],
+            charm.OFFICIAL_CATEGORIES)
+        self.assertEqual([], charm.categories)
+
     def test_maintainer(self):
         # The maintainer property is a list of category names.
         maintainer = ['Jane <[e-mail]>', 'Ian <[e-mail]>']

=== modified file 'charmworld/views/tests/test_api.py'
--- charmworld/views/tests/test_api.py	2013-07-02 12:28:28 +0000
+++ charmworld/views/tests/test_api.py	2013-07-08 20:05:31 +0000
@@ -702,6 +702,11 @@
         formatted = self.api_class._format_charm(Charm(charm))
         self.assertFalse(formatted['is_approved'])
 
+    def test_format_invalid_category(self):
+        charm = factory.get_charm_json(categories=['applications', 'bong'])
+        formatted = self.api_class._format_charm(Charm(charm))
+        self.assertEqual(['applications'], formatted['categories'])
+
     def test_charm(self):
         """Retrieving a charm works."""
         self.use_index_client()