launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/improve-savane-url-parsing into lp:launchpad.

Commit message:
Parse a few more possible Savane URL formats.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #197250 in Launchpad itself: "bug watches on savannah have to use exact urls, but savannah uses many itself"
  https://bugs.launchpad.net/launchpad/+bug/197250

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/improve-savane-url-parsing/+merge/358610
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/improve-savane-url-parsing into lp:launchpad.

=== modified file 'lib/lp/bugs/model/bugwatch.py'
--- lib/lp/bugs/model/bugwatch.py	2016-07-04 17:11:29 +0000
+++ lib/lp/bugs/model/bugwatch.py	2018-11-11 23:14:12 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -613,12 +613,20 @@
 
     def parseSavaneURL(self, scheme, host, path, query):
         """Extract Savane base URL and bug ID."""
-        # Savane bugs URLs are in the form /bugs/?<bug-id>, so we
-        # exclude any path that isn't '/bugs/'. We also exclude query
-        # string that have a length of more or less than one, since in
-        # such cases we'd be taking a guess at the bug ID, which would
-        # probably be wrong.
-        if path != '/bugs/' or len(query) != 1:
+        # We're only interested in URLs that look like they come from a
+        # Savane bugtracker. We currently accept URL paths /bugs/ or
+        # /bugs/index.php, and accept query strings that are just the bug ID
+        # or that have an item_id parameter containing the bug ID.
+        if path not in ('/bugs/', '/bugs/index.php'):
+            return None
+        if len(query) == 1 and query.values()[0] is None:
+            # The query string is just a bare ID.
+            remote_bug = query.keys()[0]
+        elif 'item_id' in query:
+            remote_bug = query['item_id']
+        else:
+            return None
+        if not remote_bug.isdigit():
             return None
 
         # There's only one global Savannah bugtracker registered with
@@ -628,13 +636,6 @@
             urlsplit(alias)[1] for alias in savannah_tracker.aliases]
         savannah_hosts.append(urlsplit(savannah_tracker.baseurl)[1])
 
-        # The remote bug is actually a key in the query dict rather than
-        # a value, so we simply use the first and only key we come
-        # across as a best-effort guess.
-        remote_bug = query.popitem()[0]
-        if remote_bug is None or not remote_bug.isdigit():
-            return None
-
         if host in savannah_hosts:
             return savannah_tracker.baseurl, remote_bug
         else:

=== modified file 'lib/lp/bugs/tests/test_bugwatch.py'
--- lib/lp/bugs/tests/test_bugwatch.py	2018-01-02 10:54:31 +0000
+++ lib/lp/bugs/tests/test_bugwatch.py	2018-11-11 23:14:12 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for BugWatchSet."""
@@ -143,6 +143,15 @@
             'bug_id': '22003',
             'already_registered': True,
             }),
+        ('SavannahWithParameters', {
+            'bugtracker_type': BugTrackerType.SAVANE,
+            'bug_url': (
+                'http://savannah.gnu.org/bugs/index.php'
+                '?func=detailitem&item_id=22003'),
+            'base_url': 'http://savannah.gnu.org/',
+            'bug_id': '22003',
+            'already_registered': True,
+            }),
         ('Savane', {
             'bugtracker_type': BugTrackerType.SAVANE,
             'bug_url': 'http://savane.example.com/bugs/?12345',