Merge lp:~benji/launchpad/bug-810113 into lp:launchpad

[Benji York]

[r=bac][bug=810113] if multiple ws.op fields are provided, ignore them when generating the page ID

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

[Brad Crittenden (bac)]

Looks good save for a couple of typos:

mutliple
alltogether -> altogether

review: Approve (code)

[Brad Crittenden (bac)]

Your review would've been much faster if it weren't for the insane music.

[Benji York (benji)]

On Wed, Aug 31, 2011 at 5:03 PM, Brad Crittenden <email address hidden> wrote:
> Review: Approve code
> Looks good save for a couple of typos:

Fixed. Thanks.
--
Benji York

[Francis J. Lacoste (flacoste)]

> Review music: http://www.youtube.com/watch?v=0O2aH4XLbto

Seriously!!! That put a smile on my face. Thanks for that!

=== 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 21:06:26 +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 21:06:26 +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 multiple 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 altogether.  (It used to generate an
+        # error, see bug 810113).
+        self.view.request.form_values['ws.op'] = ['one', 'another']
+        self.assertEqual(self.makePageID(), 'FakeContext:FakeView')