launchpad-reviewers team mailing list archive

[Benji York <benji@xxxxxxxxxxxxx>]

Benji York has proposed merging lp:~benji/launchpad/bug-810113 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #810113 in Launchpad itself: "TypeError constructing page id: cannot concatenate 'str' and 'list' objects"
  https://bugs.launchpad.net/launchpad/+bug/810113

For more details, see:
https://code.launchpad.net/~benji/launchpad/bug-810113/+merge/73591

This branch fixes a very simple bug, the page ID mechanism wasn't able to cope with multiple ws.op fields (which are represented as lists instead of strings).

Lint: the lint report is clean

Test: bin/test -c -m canonical.launchpad.webapp.tests -t TestPageIdCorners

QA: load a URL like this: https://api.launchpad.net/1.0/checkbox?ws.op=1&ws.op=2

Review music: http://www.youtube.com/watch?v=0O2aH4XLbto
-- 
https://code.launchpad.net/~benji/launchpad/bug-810113/+merge/73591
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~benji/launchpad/bug-810113 into lp:launchpad.

=== modified file 'lib/canonical/launchpad/webapp/servers.py'
--- lib/canonical/launchpad/webapp/servers.py	2011-08-16 19:08:01 +0000
+++ lib/canonical/launchpad/webapp/servers.py	2011-08-31 20:55:34 +0000
@@ -1180,7 +1180,7 @@
                 pageid += ':' + collection_identifier
         op = (view.request.get('ws.op')
             or view.request.query_string_params.get('ws.op'))
-        if op:
+        if op and isinstance(op, basestring):
             pageid += ':' + op
         return pageid
 

=== modified file 'lib/canonical/launchpad/webapp/tests/test_pageid.py'
--- lib/canonical/launchpad/webapp/tests/test_pageid.py	2010-08-27 16:01:05 +0000
+++ lib/canonical/launchpad/webapp/tests/test_pageid.py	2011-08-31 20:55:34 +0000
@@ -99,3 +99,24 @@
         self.assertEqual(
             self.makePageID(),
             'FakeContext:FakeCollectionResourceView:#milestone-page-resource')
+
+
+class TestPageIdCorners(TestCase):
+    """Ensure that the page ID generation handles corner cases well."""
+
+    def setUp(self):
+        super(TestPageIdCorners, self).setUp()
+        self.publication = WebServicePublication(db=None)
+        self.view = FakeView()
+        self.context = FakeContext()
+
+    def makePageID(self):
+        return self.publication.constructPageID(self.view, self.context)
+
+    def test_pageid_with_multiple_op_fields(self):
+        # The publisher will combine mutliple form values with the same name
+        # into a list.  If those values are for "ws.op", the page ID mechanism
+        # should just ignore the op alltogether.  (It used to generate an
+        # error, see bug 810113).
+        self.view.request.form_values['ws.op'] = ['one', 'another']
+        self.assertEqual(self.makePageID(), 'FakeContext:FakeView')