Merge lp:~abentley/launchpad/mustache-bugs into lp:launchpad

[Aaron Bentley]

Implement new bug listing style on server-side and client-side.

= Summary =
Fix bug 874595: bug listings need to follow new style

== Proposed fix ==
Use Mustache to render new bugs client-side and server-side.
Screenshot: http://people.canonical.com/~abentley/mustache-listings.png

== Pre-implementation notes ==
Discussed with deryck

== Implementation details ==
This is hidden behind a feature flag (bugs.dynamic_bug_listings.enabled). No change to behaviour or performance is expected while the flag is off.

The Mustache template is provided as a separate file, which is loaded by the view class, and provided in JSON form as LP.mustache_listings.

The data to be rendered is provided as LP.cache.mustache_model.

== Tests ==
bin/test -t buglisting -t test_bugtask

== Demo and Q/A ==
Enable the flag. Go to +bugs for a given project or source package. You should see a display similar to the screenshot above.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/javascript/tests/test_buglisting.html
  lib/lp/bugs/javascript/buglisting.js
  versions.cfg
  setup.py
  utilities/find-changed-files.sh
  lib/lp/bugs/templates/buglisting-default.pt
  lib/lp/bugs/browser/tests/test_bugtask.py
  lib/lp/bugs/browser/bugtask.py
  lib/lp/bugs/templates/buglisting.mustache
  utilities/sourcedeps.conf
  utilities/sourcedeps.cache
  lib/lp/bugs/templates/bugs-listing-table-without-navlinks.pt
  lib/lp/bugs/javascript/tests/test_buglisting.js

[Aaron Bentley (abentley)]

This is for https://dev.launchpad.net/Projects/CustomBugListings

[Benji York (benji)]

Overall this branch looks good. I look forward to using mustache.

I suspect you want to remove the "<em>Server-side mustache</em>" and
"<em>Client-side mustache</em>" bits as well as making the client/server
rendering conditional on the feature flag.

review: Approve (code)

[Aaron Bentley (abentley)]

The rendering is already conditional on the feature flag. When this feature is more complete, we will want to switch completely to one rendering, but doing that now would prevent us from QAing either the client-side or the server-side renderings.

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-10-19 14:54:07 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-10-20 19:36:35 +0000
@@ -53,6 +53,7 @@
     log,
     )
 from operator import attrgetter
+import os.path
 import re
 import transaction
 import urllib
@@ -74,6 +75,7 @@
     )
 from lazr.restful.utils import smartquote
 from lazr.uri import URI
+import pystache
 from pytz import utc
 from simplejson import dumps
 from z3c.pt.pagetemplate import ViewPageTemplateFile
@@ -2137,6 +2139,25 @@
         """Returns the bug heat flames HTML."""
         return bugtask_heat_html(self.bugtask, target=self.target_context)
 
+    @property
+    def model(self):
+        """Provide flattened data about bugtask for simple templaters."""
+        badges = getAdapter(self.bugtask, IPathAdapter, 'image').badges()
+        target_image = getAdapter(self.target, IPathAdapter, 'image')
+        return {
+            'importance': self.importance.title,
+            'importance_class': 'importance' + self.importance.name,
+            'status': self.status.title,
+            'status_class': 'status' + self.status.name,
+            'title': self.bug.title,
+            'id': self.bug.id,
+            'bug_url': canonical_url(self.bugtask),
+            'bugtarget': self.bugtargetdisplayname,
+            'bugtarget_css': target_image.sprite_css(),
+            'bug_heat_html': self.bug_heat_html,
+            'badges': badges,
+            }
+
 
 class BugListingBatchNavigator(TableBatchNavigator):
     """A specialised batch navigator to load smartly extra bug information."""
@@ -2178,6 +2199,26 @@
         """Return a decorated list of visible bug tasks."""
         return [self._getListingItem(bugtask) for bugtask in self.batch]
 
+    @cachedproperty
+    def mustache_template(self):
+        template_path = os.path.join(
+            config.root, 'lib/lp/bugs/templates/buglisting.mustache')
+        with open(template_path) as template_file:
+            return template_file.read()
+
+    @property
+    def mustache_listings(self):
+        return 'LP.mustache_listings = %s;' % dumps(self.mustache_template)
+
+    @property
+    def mustache(self):
+        return pystache.render(self.mustache_template, self.model)
+
+    @property
+    def model(self):
+        bugtasks = [bugtask.model for bugtask in self.getBugListingItems()]
+        return {'bugtasks': bugtasks}
+
 
 class NominatedBugReviewAction(EnumeratedType):
     """Enumeration for nomination review actions"""
@@ -2436,6 +2477,9 @@
 
         expose_structural_subscription_data_to_js(
             self.context, self.request, self.user)
+        if getFeatureFlag('bugs.dynamic_bug_listings.enabled'):
+            cache = IJSONRequestCache(self.request)
+            cache.objects['mustache_model'] = self.search().model
 
     @property
     def columns_to_show(self):

=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py	2011-10-05 18:02:45 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py	2011-10-20 19:36:35 +0000
@@ -3,13 +3,20 @@
 
 __metaclass__ = type
 
+from contextlib import contextmanager
 from datetime import datetime
+import re
 
 from lazr.lifecycle.event import ObjectModifiedEvent
+from lazr.restful.interfaces import IJSONRequestCache
 from lazr.lifecycle.snapshot import Snapshot
 from pytz import UTC
+import soupmatchers
 from storm.store import Store
-from testtools.matchers import LessThan
+from testtools.matchers import (
+    LessThan,
+    Not,
+    )
 import transaction
 from zope.component import (
     getMultiAdapter,
@@ -38,7 +45,9 @@
 from lp.bugs.browser.bugtask import (
     BugActivityItem,
     BugTaskEditView,
+    BugTaskListingItem,
     BugTasksAndNominationsView,
+    BugTaskSearchListingView,
     )
 from lp.bugs.interfaces.bugactivity import IBugActivitySet
 from lp.bugs.interfaces.bugnomination import IBugNomination
@@ -54,8 +63,11 @@
 from lp.services.propertycache import get_property_cache
 from lp.soyuz.interfaces.component import IComponentSet
 from lp.testing import (
+    BrowserTestCase,
     celebrity_logged_in,
+    feature_flags,
     person_logged_in,
+    set_feature_flag,
     TestCaseWithFactory,
     )
 from lp.testing._webservice import QueryCollector
@@ -1194,3 +1206,126 @@
         self._assertThatUnbatchedAndBatchedActivityMatch(
             unbatched_view.activity_and_comments[4:],
             batched_view.activity_and_comments)
+
+
+def make_bug_task_listing_item(factory):
+    owner = factory.makePerson()
+    bug = factory.makeBug(
+        owner=owner, private=True, security_related=True)
+    bugtask = factory.makeBugTask(bug)
+    bug_task_set = getUtility(IBugTaskSet)
+    bug_badge_properties = bug_task_set.getBugTaskBadgeProperties(
+        [bugtask])
+    badge_property = bug_badge_properties[bugtask]
+    return owner, BugTaskListingItem(
+        bugtask,
+        badge_property['has_branch'],
+        badge_property['has_specification'],
+        badge_property['has_patch'],
+        target_context=bugtask.target)
+
+
+class TestBugTaskSearchListingView(BrowserTestCase):
+
+    layer = DatabaseFunctionalLayer
+
+    server_listing = soupmatchers.Tag(
+        'Server', 'em', text='Server-side mustache')
+
+    client_listing = soupmatchers.Tag(
+        'client-listing', True, attrs={'id': 'client-listing'})
+
+    def makeView(self, bugtask=None):
+        request = LaunchpadTestRequest()
+        if bugtask is None:
+            bugtask = self.factory.makeBugTask()
+        view = BugTaskSearchListingView(bugtask.target, request)
+        view.initialize()
+        return view
+
+    @contextmanager
+    def dynamic_listings(self):
+        with feature_flags():
+            set_feature_flag(u'bugs.dynamic_bug_listings.enabled', u'on')
+            yield
+
+    def test_mustache_model_missing_if_no_flag(self):
+        """The IJSONRequestCache should contain mustache_model."""
+        view = self.makeView()
+        cache = IJSONRequestCache(view.request)
+        self.assertIs(None, cache.objects.get('mustache_model'))
+
+    def test_mustache_model_in_json(self):
+        """The IJSONRequestCache should contain mustache_model.
+
+        mustache_model should contain bugtasks, the BugTaskListingItem.model
+        for each BugTask.
+        """
+        owner, item = make_bug_task_listing_item(self.factory)
+        self.useContext(person_logged_in(owner))
+        with self.dynamic_listings():
+            view = self.makeView(item.bugtask)
+        cache = IJSONRequestCache(view.request)
+        bugtasks = cache.objects['mustache_model']['bugtasks']
+        self.assertEqual(1, len(bugtasks))
+        self.assertEqual(item.model, bugtasks[0])
+
+    def getBugtaskBrowser(self):
+        bugtask = self.factory.makeBugTask()
+        with person_logged_in(bugtask.target.owner):
+            bugtask.target.official_malone = True
+        browser = self.getViewBrowser(
+            bugtask.target, '+bugs', rootsite='bugs')
+        return bugtask, browser
+
+    def assertHTML(self, browser, *tags, **kwargs):
+        matcher = soupmatchers.HTMLContains(*tags)
+        if kwargs.get('invert', False):
+            matcher = Not(matcher)
+        self.assertThat(browser.contents, matcher)
+
+    @staticmethod
+    def getBugNumberTag(bug_task):
+        """Bug numbers with a leading hash are unique to new rendering."""
+        bug_number_re = re.compile(r'\#%d' % bug_task.bug.id)
+        return soupmatchers.Tag('bug_number', 'td', text=bug_number_re)
+
+    def test_mustache_rendering_missing_if_no_flag(self):
+        """If the flag is missing, then no mustache features appear."""
+        bug_task, browser = self.getBugtaskBrowser()
+        number_tag = self.getBugNumberTag(bug_task)
+        self.assertHTML(browser, number_tag, invert=True)
+        self.assertHTML(browser, self.client_listing, invert=True)
+        self.assertHTML(browser, self.server_listing, invert=True)
+
+    def test_mustache_rendering(self):
+        """If the flag is present, then all mustache features appear."""
+        with self.dynamic_listings():
+            bug_task, browser = self.getBugtaskBrowser()
+        bug_number = self.getBugNumberTag(bug_task)
+        self.assertHTML(
+            browser, self.client_listing, self.server_listing, bug_number)
+
+
+class TestBugTaskListingItem(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_model(self):
+        """Model contains expected fields with expected values."""
+        owner, item = make_bug_task_listing_item(self.factory)
+        with person_logged_in(owner):
+            model = item.model
+            self.assertEqual('Undecided', model['importance'])
+            self.assertEqual('importanceUNDECIDED', model['importance_class'])
+            self.assertEqual('New', model['status'])
+            self.assertEqual('statusNEW', model['status_class'])
+            self.assertEqual(item.bug.title, model['title'])
+            self.assertEqual(item.bug.id, model['id'])
+            self.assertEqual(canonical_url(item.bugtask), model['bug_url'])
+            self.assertEqual(item.bugtargetdisplayname, model['bugtarget'])
+            self.assertEqual('sprite product', model['bugtarget_css'])
+            self.assertEqual(item.bug_heat_html, model['bug_heat_html'])
+            self.assertEqual(
+                '<span alt="private" title="Private" class="sprite private">'
+                '&nbsp;</span>', model['badges'])

=== added file 'lib/lp/bugs/javascript/buglisting.js'
--- lib/lp/bugs/javascript/buglisting.js	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/javascript/buglisting.js	2011-10-20 19:36:35 +0000
@@ -0,0 +1,23 @@
+/* Copyright 2011 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * Client-side rendering of bug listings.
+ *
+ * @module bugs
+ * @submodule buglisting
+ */
+
+YUI.add('lp.bugs.buglisting', function(Y) {
+
+var namespace = Y.namespace('lp.bugs.buglisting');
+
+namespace.rendertable = function(){
+    client_listing = Y.one('#client-listing');
+    if (client_listing === null){
+        return;
+    }
+    var txt = Mustache.to_html(LP.mustache_listings, LP.cache.mustache_model);
+    client_listing.set('innerHTML', txt);
+};
+
+}, "0.1", {"requires": ["node"]});

=== added file 'lib/lp/bugs/javascript/tests/test_buglisting.html'
--- lib/lp/bugs/javascript/tests/test_buglisting.html	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/javascript/tests/test_buglisting.html	2011-10-20 19:36:35 +0000
@@ -0,0 +1,32 @@
+<html>
+  <head>
+  <title>Bug task listing</title>
+
+  <!-- YUI and test setup -->
+  <script type="text/javascript"
+          src="../../../../canonical/launchpad/icing/yui/yui/yui.js">
+  </script>
+  <link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
+  <script type="text/javascript"
+          src="../../../app/javascript/testing/testrunner.js"></script>
+
+    <script type="text/javascript"
+      src="../../../contrib/javascript/mustache.js"></script>
+
+    <!-- The module under test -->
+    <script type="text/javascript"
+      src="../buglisting.js"></script>
+
+    <!-- The test suite -->
+    <script type="text/javascript"
+      src="test_buglisting.js"></script>
+
+    <!-- Pretty up the sample html -->
+    <style type="text/css">
+      div#sample {margin:15px; width:200px; border:1px solid #999; padding:10px;}
+    </style>
+  </head>
+  <body class="yui3-skin-sam">
+      <div id="fixture"></div>
+  </body>
+</html>

=== added file 'lib/lp/bugs/javascript/tests/test_buglisting.js'
--- lib/lp/bugs/javascript/tests/test_buglisting.js	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/javascript/tests/test_buglisting.js	2011-10-20 19:36:35 +0000
@@ -0,0 +1,58 @@
+YUI({
+    base: '../../../../canonical/launchpad/icing/yui/',
+    filter: 'raw', combine: false, fetchCSS: false
+    }).use('test', 'console', 'lp.bugs.buglisting', function(Y) {
+
+var suite = new Y.Test.Suite("lp.bugs.buglisting Tests");
+var module = Y.lp.bugs.buglisting;
+
+/**
+ * Test is_notification_level_shown() for a given set of
+ * conditions.
+ */
+suite.add(new Y.Test.Case({
+    name: 'rendertable',
+
+    setUp: function () {
+        this.MY_NAME = "ME";
+        window.LP = { links: { me: "/~" + this.MY_NAME } };
+    },
+
+    tearDown: function() {
+        delete window.LP;
+        this.set_fixture('');
+    },
+
+    set_fixture: function(value) {
+        var fixture = Y.one('#fixture');
+        fixture.set('innerHTML', value);
+    },
+    test_rendertable_no_client_listing: function() {
+        module.rendertable();
+    },
+    test_rendertable: function() {
+        this.set_fixture('<div id="client-listing"></div>');
+        window.LP.cache = {
+            mustache_model: {
+                foo: 'bar'
+            }
+        };
+        window.LP.mustache_listings = "{{foo}}";
+        module.rendertable();
+        Y.Assert.areEqual('bar', Y.one('#client-listing').get('innerHTML'));
+    }
+}));
+
+var handle_complete = function(data) {
+    window.status = '::::' + JSON.stringify(data);
+    };
+Y.Test.Runner.on('complete', handle_complete);
+Y.Test.Runner.add(suite);
+
+var console = new Y.Console({newestOnTop: false});
+console.render('#log');
+
+Y.on('domready', function() {
+    Y.Test.Runner.run();
+});
+});

=== modified file 'lib/lp/bugs/templates/buglisting-default.pt'
--- lib/lp/bugs/templates/buglisting-default.pt	2011-06-22 14:09:43 +0000
+++ lib/lp/bugs/templates/buglisting-default.pt	2011-10-20 19:36:35 +0000
@@ -18,10 +18,12 @@
     </tal:comment>
     <script type="text/javascript"
       tal:condition="not: view/shouldShowAdvancedForm">
-    LPS.use('lp.registry.structural_subscription', function(Y) {
+    LPS.use('lp.registry.structural_subscription', 'lp.bugs.buglisting',
+            function(Y) {
         Y.on('domready', function() {
             Y.lp.registry.structural_subscription.setup(
                 {content_box: "#structural-subscription-content-box"});
+            Y.lp.bugs.buglisting.rendertable()
         });
     });
   </script>

=== added file 'lib/lp/bugs/templates/buglisting.mustache'
--- lib/lp/bugs/templates/buglisting.mustache	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/templates/buglisting.mustache	2011-10-20 19:36:35 +0000
@@ -0,0 +1,25 @@
+<table class="listing narrow-listing">
+<tbody>
+{{#bugtasks}}
+    <tr>
+        <td class={{importance_class}}>
+            {{importance}}
+        </td>
+        <td>
+            #{{id}} <a href="{{bug_url}}">{{title}}</a>
+        </td>
+        <td align="right">
+            {{{badges}}}{{{bug_heat_html}}}
+        </td>
+    </tr>
+    <tr >
+        <td class={{status_class}} style="border-style: none">
+            {{status}}
+        </td>
+        <td colspan="2" style="border-style: none">
+            <span class="{{bugtarget_css}}">{{bugtarget}}</span>
+        </td>
+    </tr>
+{{/bugtasks}}
+</tbody>
+</table>

=== modified file 'lib/lp/bugs/templates/bugs-listing-table-without-navlinks.pt'
--- lib/lp/bugs/templates/bugs-listing-table-without-navlinks.pt	2011-05-27 18:10:50 +0000
+++ lib/lp/bugs/templates/bugs-listing-table-without-navlinks.pt	2011-10-20 19:36:35 +0000
@@ -76,5 +76,12 @@
       </tr>
     </tbody>
   </table>
+  <tal:mustache condition="request/features/bugs.dynamic_bug_listings.enabled">
+  <em>Server-side mustache</em>
+  <span tal:replace="structure context/mustache" />
+  <em>Client-side mustache</em>
+  <span id="client-listing" />
+  <script tal:content="structure context/mustache_listings" />
+  </tal:mustache>
 </tal:results>
 </tal:root>