Merge lp:~stevenk/launchpad/link-bug-tags-correctly into lp:launchpad

[Steve Kowalik]

[r=wallyworld][bug=911189] Make certain bug tag links are properly encoded.

Fiddle with BugTaskView.{,un}official_tags so they return a dict of tag: url, and then use the url in the TAL. Also add a test to make sure that unofficial_tags returns what we expect, and that the TAL is using the data in the right way.

Fixed up two imports which were in a very odd place.

[Ian Booth (wallyworld)]

Looks good. I assume there are no other places where those view properties are used that need porting.

review: Approve

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2012-01-17 14:14:49 +0000
+++ lib/lp/bugs/browser/bugtask.py	2012-01-20 00:17:28 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """IBugTask-related browser views."""
@@ -1038,15 +1038,25 @@
     def official_tags(self):
         """The list of official tags for this bug."""
         target_official_tags = set(self.context.bug.official_tags)
-        return [tag for tag in self.context.bug.tags
-                if tag in target_official_tags]
+        links = []
+        for tag in self.context.bug.tags:
+            if tag in target_official_tags:
+                links.append((tag, '%s?field.tag=%s' % (
+                    canonical_url(self.context.target, view_name='+bugs',
+                        force_local_path=True), urllib.quote(tag))))
+        return links
 
     @property
     def unofficial_tags(self):
         """The list of unofficial tags for this bug."""
         target_official_tags = set(self.context.bug.official_tags)
-        return [tag for tag in self.context.bug.tags
-                if tag not in target_official_tags]
+        links = []
+        for tag in self.context.bug.tags:
+            if tag not in target_official_tags:
+                links.append((tag, '%s?field.tag=%s' % (
+                    canonical_url(self.context.target, view_name='+bugs',
+                        force_local_path=True), urllib.quote(tag))))
+        return links
 
     @property
     def available_official_tags_js(self):

=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py	2012-01-18 12:04:16 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py	2012-01-20 00:17:28 +0000
@@ -1,9 +1,5 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
-from BeautifulSoup import BeautifulSoup
-
-from lp.registry.interfaces.person import PersonVisibility
-
 
 __metaclass__ = type
 
@@ -16,6 +12,7 @@
 import simplejson
 import urllib
 
+from BeautifulSoup import BeautifulSoup
 from lazr.lifecycle.event import ObjectModifiedEvent
 from lazr.restful.interfaces import IJSONRequestCache
 from lazr.lifecycle.snapshot import Snapshot
@@ -35,6 +32,7 @@
 from zope.interface import providedBy
 from zope.security.proxy import removeSecurityProxy
 
+from lp.registry.interfaces.person import PersonVisibility
 from lp.services.config import config
 from lp.services.database.constants import UTC_NOW
 from lp.testing import (
@@ -254,6 +252,20 @@
             self.assertEqual(1, len(view.errors))
             self.assertEqual(product, bug.default_bugtask.target)
 
+    def test_bugtag_urls_are_encoded(self):
+        # The link to bug tags are encoded to protect against special chars.
+        product = self.factory.makeProduct(name='foobar')
+        bug = self.factory.makeBug(product=product, tags=['depends-on+987'])
+        getUtility(ILaunchBag).add(bug.default_bugtask)
+        view = create_initialized_view(bug.default_bugtask, name=u'+index')
+        expected = [(u'depends-on+987',
+            u'/foobar/+bugs?field.tag=depends-on%2B987')]
+        self.assertEqual(expected, view.unofficial_tags)
+        browser = self.getUserBrowser(canonical_url(bug), bug.owner)
+        self.assertIn(
+            'href="/foobar/+bugs?field.tag=depends-on%2B987"',
+            browser.contents)
+
 
 class TestBugTasksAndNominationsView(TestCaseWithFactory):
 

=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt	2011-12-08 20:31:53 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt	2012-01-20 00:17:28 +0000
@@ -146,13 +146,13 @@
             Tags:
             <span id="tag-list">
               <a tal:repeat="tag view/official_tags"
-                 tal:content="tag"
+                 tal:content="python: tag[0]"
                  class="official-tag"
-                 tal:attributes="href string:${context/target/fmt:url}/+bugs?field.tag=${tag}">tag</a>
+                 tal:attributes="href python: tag[1]">tag</a>
               <a tal:repeat="tag view/unofficial_tags"
-                 tal:content="tag"
+                 tal:content="python: tag[0]"
                  class="unofficial-tag"
-                 tal:attributes="href string:${context/target/fmt:url}/+bugs?field.tag=${tag}">tag</a>
+                 tal:attributes="href python: tag[1]">tag</a>
             </span>
             <form id="tags-form" style="display: inline">
                 <input type="text" id="tag-input" class="hidden" style="width: 35em;" />