launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05247
[Merge] lp:~abentley/launchpad/mustache-bugs into lp:launchpad
Aaron Bentley has proposed merging lp:~abentley/launchpad/mustache-bugs into lp:launchpad with lp:~abentley/launchpad/support-mustache as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #874595 in Launchpad itself: "bug listings need to follow new style"
https://bugs.launchpad.net/launchpad/+bug/874595
For more details, see:
https://code.launchpad.net/~abentley/launchpad/mustache-bugs/+merge/79441
= 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
--
https://code.launchpad.net/~abentley/launchpad/mustache-bugs/+merge/79441
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/mustache-bugs into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2011-10-12 04:22:25 +0000
+++ lib/lp/bugs/browser/bugtask.py 2011-10-14 20:34:28 +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
@@ -265,6 +267,7 @@
from lp.registry.interfaces.sourcepackage import ISourcePackage
from lp.registry.model.personroles import PersonRoles
from lp.registry.vocabularies import MilestoneVocabulary
+from lp.services.features import getFeatureFlag
from lp.services.fields import PersonChoice
from lp.services.propertycache import (
cachedproperty,
@@ -2136,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."""
@@ -2177,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"""
@@ -2435,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-14 20:34:28 +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">'
+ ' </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-14 20:34:28 +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-14 20:34:28 +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-14 20:34:28 +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-14 20:34:28 +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-14 20:34:28 +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-14 20:34:28 +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>
Follow ups
