launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Richard Harding has proposed merging lp:~rharding/launchpad/link_tags_894726 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #894726 in Launchpad itself: "Tags not linked on bug listing"
  https://bugs.launchpad.net/launchpad/+bug/894726

For more details, see:
https://code.launchpad.net/~rharding/launchpad/link_tags_894726/+merge/86045

= Summary =
Fixes qa-bad state by bypassing an issue with PyStache and it's handling of an empty list/falsey logic.

== Implementation ==
Fixes qa-bad in MP https://code.launchpad.net/~rharding/launchpad/link_tags_894726/+merge/85673.

The tags on the model can be an empty list. We want to display None if the list is empty, or a series of links if there are tags. This worked correctly when it hit the JS mustache engine, but pystache would display both the true and false values. Causing you to get a ui like:

Tags: uitrivial NoneNone

There was also a lack of space in between the tags, so the model was adjusted to add a "has_tags" dict key that is not a list. pystache can check this for true/false correctly and display None if there are indeed no tags.

A hard coded space was placed after each link to correct for the spacing issue as well.

== QA ==
This can only be reviewed in Firefox since Chrome runs the JS engine when the history is popped, hiding the bug from Chrome users. It can only be qa'd on the initial page load, when the pystache engine has rendered the bug listing. You must load a buglisting page and check the tags display. It should either be:

Tags: None
-or-
Tags: $link $link2 ...
-- 
https://code.launchpad.net/~rharding/launchpad/link_tags_894726/+merge/86045
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/link_tags_894726 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-12-15 21:16:41 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-12-16 14:09:28 +0000
@@ -2231,10 +2231,7 @@
             self.bugtask.target,
             view_name="+bugs")
 
-        def build_tag_url(tag):
-            """Generate a url for the tag based on the current request ctx."""
-
-        return {
+        flattened = {
             'age': age,
             'assignee': assignee,
             'bug_url': canonical_url(self.bugtask),
@@ -2255,6 +2252,13 @@
             'title': self.bug.title,
             }
 
+        # This is a total hack, but pystache will run both truth/false values
+        # for an empty list for some reason, and it "works" if it's just a flag
+        # like this. We need this value for the mustache template to be able
+        # to tell that there are no tags without looking at the list.
+        flattened['has_tags'] = True if len(flattened['tags']) else False
+        return flattened
+
 
 class BugListingBatchNavigator(TableBatchNavigator):
     """A specialised batch navigator to load smartly extra bug information."""

=== modified file 'lib/lp/bugs/templates/buglisting.mustache'
--- lib/lp/bugs/templates/buglisting.mustache	2011-12-15 18:55:46 +0000
+++ lib/lp/bugs/templates/buglisting.mustache	2011-12-16 14:09:28 +0000
@@ -61,10 +61,10 @@
             {{/show_datecreated}}
             {{#show_tag}}
                 <span class="field">Tags:
+                  {{^has_tags}}None{{/has_tags}}
                   {{#tags}}
-                    <a href="{{url}}" class="tag">{{tag}}</a>
+                     <a href="{{url}}" class="tag">{{tag}}</a>&#160;
                   {{/tags}}
-                  {{^tags}}None{{/tags}}
                 </span>
             {{/show_tag}}
         </div>