Merge lp:~jtv/launchpad/bug-135052 into lp:launchpad

[Jeroen T. Vermeulen]

Catch attempt to set both productseries and distroseries for a POTemplate.

[Jeroen T. Vermeulen (jtv)]

= Bug 135052 =

A failure to validate input to the POTemplate admin form meant that an administrator could attempt to assign a translation template to both a ProductSeries and a DistroSeries. This triggered and assertion failure (and would otherwise have violated a database constraint). Actually this was an old, known bug that we never bothered to fix because it can only affect a handful of people, and even then when they attempt to do something that they know they shouldn't. I'm only fixing it now because we have a zero-oops policy.

In this branch you'll see a validation method for the form added, plus of course a test to exercise it. The solution is so idiomatic and, frankly, bloody obvious that I did not bother having a proper pre-imp call.

Test:
{{{
./bin/test -vv -t xx-potemplate-admin
}}}

No lint.

Jeroen

[Curtis Hovey (sinzui)]

Hi Jeroen.

Thanks for fixing this oops.

review: Approve (code)

=== modified file 'lib/lp/translations/browser/potemplate.py'
--- lib/lp/translations/browser/potemplate.py	2010-01-20 20:36:13 +0000
+++ lib/lp/translations/browser/potemplate.py	2010-01-26 12:48:20 +0000
@@ -556,6 +556,21 @@
     label = 'Administer translation template'
     page_title = "Administer"
 
+    def validate(self, data):
+        distroseries = data['distroseries']
+        productseries = data['productseries']
+        if distroseries is not None and productseries is not None:
+            message = ("Choose a distribution release series or a project "
+                "release series, but not both.")
+        elif distroseries is None and productseries is None:
+            message = ("Choose either a distribution release series or a "
+                "project release series.")
+        else:
+            message = None
+
+        if message is not None:
+            self.addError(message)
+
 
 class POTemplateExportView(BaseExportView):
     """Request downloads of a `POTemplate` and its translations."""

=== modified file 'lib/lp/translations/stories/standalone/xx-potemplate-admin.txt'
--- lib/lp/translations/stories/standalone/xx-potemplate-admin.txt	2009-12-11 11:33:39 +0000
+++ lib/lp/translations/stories/standalone/xx-potemplate-admin.txt	2010-01-26 12:48:20 +0000
@@ -254,3 +254,40 @@
     >>> group_owner_browser.getControl('Change').click()
 
 
+Input Check
+-----------
+
+The template admin form requires either a productseries or a
+distroseries.
+
+    >>> login(ANONYMOUS)
+    >>> product = factory.makeProduct(name='aproduct')
+    >>> template = factory.makePOTemplate(
+    ...     productseries=product.getSeries('trunk'))
+    >>> template_url = canonical_url(template)
+    >>> logout()
+
+    >>> admin_browser.open(template_url + '/+admin')
+
+    >>> admin_browser.getControl(name='field.distroseries').value = [None]
+    >>> admin_browser.getControl(name='field.productseries').value = ''
+    >>> admin_browser.getControl('Change').click()
+    >>> print_errors(admin_browser.contents)
+    There is 1 error.
+    Choose either a distribution release series or a project release series.
+
+It can't be both.
+
+    >>> admin_browser.open(template_url + '/+admin')
+
+    >>> admin_browser.getControl(name='field.distroseries').value = [
+    ...     'ubuntu/hoary']
+    >>> print admin_browser.getControl(name='field.productseries').value
+    aproduct/trunk
+    >>> admin_browser.getControl('Change').click()
+    >>> print_errors(admin_browser.contents)
+    There is 1 error.
+    Choose a distribution release series or a project release series,
+    but not both.
+
+