launchpad-reviewers team mailing list archive

[Nigel Babu <nigelbabu@xxxxxxxxx>]

Nigel Babu has proposed merging lp:~nigelbabu/launchpad/4595-upgrade-bug-linking into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~nigelbabu/launchpad/4595-upgrade-bug-linking/+merge/71575

Distinguish between valid and invalid bugs when autolinkifying bugs
-- 
https://code.launchpad.net/~nigelbabu/launchpad/4595-upgrade-bug-linking/+merge/71575
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~nigelbabu/launchpad/4595-upgrade-bug-linking into lp:launchpad.

=== modified file 'lib/lp/app/browser/linkchecker.py'
--- lib/lp/app/browser/linkchecker.py	2010-11-12 18:31:05 +0000
+++ lib/lp/app/browser/linkchecker.py	2011-08-21 07:46:30 +0000
@@ -11,6 +11,9 @@
 import simplejson
 from zope.component import getUtility
 
+from canonical.launchpad.searchbuilder import any
+from canonical.launchpad.webapp import LaunchpadView
+
 from lp.app.errors import NotFoundError
 from lp.code.errors import (
     CannotHaveLinkedBranch,
@@ -19,10 +22,12 @@
     NoSuchBranch,
     )
 from lp.code.interfaces.branchlookup import IBranchLookup
+from lp.bugs.interfaces.bugtask import BugTaskSearchParams, IBugTaskSet
+from lp.registry.interfaces.person import IPerson
 from lp.registry.interfaces.product import InvalidProductName
 
 
-class LinkCheckerAPI:
+class LinkCheckerAPI(LaunchpadView):
     """Validates Launchpad shortcut links.
 
     This class provides the endpoint of an Ajax call to .../+check-links.
@@ -47,6 +52,7 @@
         # Each link type has it's own validation method.
         self.link_checkers = dict(
             branch_links=self.check_branch_links,
+            bug_links=self.check_bug_links,
         )
 
     def __call__(self):
@@ -78,6 +84,21 @@
                 invalid_links[link] = self._error_message(e)
         return invalid_links
 
+    def check_bug_links(self, links):
+        """Checks if links of the form /bugs/100"""
+        invalid_links = {}
+        user = self.user
+        bugs = [int(link[len('/bugs/'):]) for link in links]
+        if bugs:
+            params = BugTaskSearchParams(
+                user=user, status=None,
+                bug=any(*bugs))
+            bug_ids = getUtility(IBugTaskSet).searchBugIds(params)
+            invalid = set(bugs) - set(bug_ids)
+            for bug in invalid:
+                invalid_links['/bugs/' + str(bug)] = "Bug %s cannot be found" % bug
+        return invalid_links
+         
     def _error_message(self, ex):
         if hasattr(ex, 'display_message'):
             return ex.display_message

=== modified file 'lib/lp/app/browser/stringformatter.py'
--- lib/lp/app/browser/stringformatter.py	2011-07-12 10:02:51 +0000
+++ lib/lp/app/browser/stringformatter.py	2011-08-21 07:46:30 +0000
@@ -283,7 +283,7 @@
         # linkify to the general bug url.
         url = '/bugs/%s' % bugnum
         # The text will have already been cgi escaped.
-        return '<a href="%s">%s</a>%s' % (url, text, trailers)
+        return '<a href="%s" class="bug-link">%s</a>%s' % (url, text, trailers)
 
     @staticmethod
     def _handle_parens_in_trailers(url, trailers):

=== modified file 'lib/lp/app/browser/tests/test_linkchecker.py'
--- lib/lp/app/browser/tests/test_linkchecker.py	2010-11-11 23:40:36 +0000
+++ lib/lp/app/browser/tests/test_linkchecker.py	2011-08-21 07:46:30 +0000
@@ -10,6 +10,7 @@
 import simplejson
 from zope.security.proxy import removeSecurityProxy
 
+from canonical.launchpad.webapp import canonical_url
 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.app.browser.linkchecker import LinkCheckerAPI
@@ -28,7 +29,7 @@
         self.assertEqual(len(invalid_links), len(links_to_check))
         self.assertEqual(set(invalid_links), set(links_to_check))
 
-    def make_valid_links(self):
+    def make_valid_branch_links(self):
         branch = self.factory.makeProductBranch()
         valid_branch_url = self.BRANCH_URL_TEMPLATE % branch.unique_name
         product = self.factory.makeProduct()
@@ -41,24 +42,41 @@
             valid_product_url,
         ]
 
-    def make_invalid_links(self):
+    def make_invalid_branch_links(self):
         return [
             self.BRANCH_URL_TEMPLATE % 'foo',
             self.BRANCH_URL_TEMPLATE % 'bar',
             ]
 
-    def invoke_branch_link_checker(
-        self, valid_branch_urls=None, invalid_branch_urls=None):
+    def make_valid_bug_links(self):
+        bug = self.factory.makeBug()
+        return ['/bugs/%d' % (bug.id)]
+
+    def make_invalid_bug_links(self):
+        bug = self.factory.makeBug(private=True)
+        return ['/bugs/%d' % (bug.id)]
+
+    def invoke_link_checker(
+        self, valid_branch_urls=None, invalid_branch_urls=None,
+        valid_bug_urls=None, invalid_bug_urls=None):
         if valid_branch_urls is None:
             valid_branch_urls = {}
         if invalid_branch_urls is None:
             invalid_branch_urls = {}
+        if valid_bug_urls is None:
+            valid_bug_urls = {}
+        if invalid_bug_urls is None:
+            invalid_bug_urls = {}
 
         branch_urls = list(valid_branch_urls)
         branch_urls.extend(invalid_branch_urls)
         shuffle(branch_urls)
+        
+        bug_urls = list(valid_bug_urls)
+        bug_urls.extend(invalid_bug_urls)
+        shuffle(bug_urls)
 
-        links_to_check = dict(branch_links=branch_urls)
+        links_to_check = dict(branch_links=branch_urls, bug_links=bug_urls)
         link_json = simplejson.dumps(links_to_check)
 
         request = LaunchpadTestRequest(link_hrefs=link_json)
@@ -74,17 +92,23 @@
         link_dict = simplejson.loads(result_json)
         self.assertEqual(link_dict, {})
 
-    def test_only_valid_branchlinks(self):
-        branch_urls = self.make_valid_links()
-        self.invoke_branch_link_checker(valid_branch_urls=branch_urls)
-
-    def test_only_invalid_branchlinks(self):
-        branch_urls = self.make_invalid_links()
-        self.invoke_branch_link_checker(invalid_branch_urls=branch_urls)
-
-    def test_valid_and_invald_branchlinks(self):
-        valid_branch_urls = self.make_valid_links()
-        invalid_branch_urls = self.make_invalid_links()
-        self.invoke_branch_link_checker(
+    def test_only_valid_links(self):
+        branch_urls = self.make_valid_branch_links()
+        bug_urls = self.make_valid_bug_links()
+        self.invoke_link_checker(valid_branch_urls=branch_urls, valid_bug_urls=bug_urls)
+
+    def test_only_invalid_links(self):
+        branch_urls = self.make_invalid_branch_links()
+        bug_urls = self.make_invalid_bug_links()
+        self.invoke_link_checker(invalid_branch_urls=branch_urls, invalid_bug_urls=bug_urls)
+
+    def test_valid_and_invald_links(self):
+        valid_branch_urls = self.make_valid_branch_links()
+        invalid_branch_urls = self.make_invalid_branch_links()
+        valid_bug_urls = self.make_valid_bug_links()
+        invalid_bug_urls = self.make_invalid_bug_links()
+        self.invoke_link_checker(
             valid_branch_urls=valid_branch_urls,
-            invalid_branch_urls=invalid_branch_urls)
+            invalid_branch_urls=invalid_branch_urls,
+            valid_bug_urls=valid_bug_urls,
+            invalid_bug_urls=invalid_bug_urls)

=== modified file 'lib/lp/app/javascript/lp-links.js'
--- lib/lp/app/javascript/lp-links.js	2011-02-24 00:23:04 +0000
+++ lib/lp/app/javascript/lp-links.js	2011-08-21 07:46:30 +0000
@@ -58,6 +58,7 @@
         // We get all the links with defined css classes.
         // At the moment, we just handle branch links, but in future...
         harvest_links(links_to_check, 'branch-short-link', 'branch_links');
+        harvest_links(links_to_check, 'bug-link', 'bug_links');
 
         // Do we have anything to do?
         if( Y.Object.size(links_to_check) == 0 ) {
@@ -85,6 +86,8 @@
                     // ATM, we just handle branch links, but in future...
                     process_invalid_links(link_info, 'branch-short-link',
                             'branch_links');
+                    process_invalid_links(link_info, 'bug-link',
+                            'bug_links');
                 }
             }
         }