Merge lp:~edwin-grubbs/launchpad/bug-526001-edit-packaging-oops into lp:launchpad

Proposed by Edwin Grubbs
Status: Merged
Approved by: Brad Crittenden
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~edwin-grubbs/launchpad/bug-526001-edit-packaging-oops
Merge into: lp:launchpad
Diff against target: 134 lines (+91/-9)
2 files modified
lib/lp/registry/browser/sourcepackage.py (+16/-9)
lib/lp/registry/tests/test_sourcepackage.py (+75/-0)
To merge this branch: bzr merge lp:~edwin-grubbs/launchpad/bug-526001-edit-packaging-oops
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+19997@code.launchpad.net

Commit message

Fix oops when changing the sourcepackage's productseries if the current series is obsolete.

To post a comment you must log in.
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Summary
-------

Fix bug where an exception is raised if you try to change the
productseries for a sourcepackage, when the current productseries
is obsolete.

Implementation details
----------------------

If the current context.productseries is not in the series_list variable,
add it. The code that converts the series_list into SimpleTerm objects
was moved below that if-statement.

Tests
-----

./bin/test -vv -t 'xx-distribution-packages.txt|xx-also-affects-upstream-default-values.txt|sourcepackage-views.txt|xx-sourcepackage-packaging.txt'

Demo and Q/A
------------

* Open https://launchpad.dev/ubuntu/warty/+source/mozilla-firefox/+edit-packaging
    * Set the product series.
* Open the productseries page that you used.
    * Change the status to OBSOLETE.
* Open https://launchpad.dev/ubuntu/warty/+source/mozilla-firefox/+edit-packaging
    * The current project should already be entered, so just click "Continue".
        * The obsolete series should show up as a radio button option
          instead of raising an exception.

Revision history for this message
Brad Crittenden (bac) wrote :

Edwin as we discussed on IRC I think we need a test showing the successful working when the productseries is obsolete. Otherwise it looks good.

review: Needs Fixing (code)
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :
Download full text (4.0 KiB)

Hi Brad,

Thanks for the review. Here are the added tests.

=== modified file 'lib/lp/registry/tests/test_sourcepackage.py'
--- lib/lp/registry/tests/test_sourcepackage.py 2009-12-13 11:55:40 +0000
+++ lib/lp/registry/tests/test_sourcepackage.py 2010-02-24 03:06:12 +0000
@@ -22,6 +22,7 @@
 from lp.code.interfaces.seriessourcepackagebranch import (
     IMakeOfficialBranchLinks)
 from lp.testing import TestCaseWithFactory
+from lp.testing.views import create_initialized_view
 from canonical.testing.layers import DatabaseFunctionalLayer

@@ -238,5 +239,79 @@
             Unauthorized, sourcepackage.setBranch, pocket, branch, registrant)

+class TestSourcePackageViews(TestCaseWithFactory):
+ """Tests for source package view classes."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ TestCaseWithFactory.setUp(self)
+ self.owner = self.factory.makePerson()
+ self.product = self.factory.makeProduct(
+ name='bonkers', displayname='Bonkers', owner=self.owner)
+
+ self.obsolete_productseries = self.factory.makeProductSeries(
+ name='obsolete', product=self.product)
+ self.obsolete_productseries.status = SeriesStatus.OBSOLETE
+
+ self.dev_productseries = self.factory.makeProductSeries(
+ name='current', product=self.product)
+ self.dev_productseries.status = SeriesStatus.DEVELOPMENT
+
+ self.distribution = self.factory.makeDistribution(
+ name='youbuntu', displayname='Youbuntu', owner=self.owner)
+ self.distroseries = self.factory.makeDistroRelease(name='busy',
+ distribution=self.distribution)
+ self.sourcepackagename = self.factory.makeSourcePackageName(
+ name='bonkers')
+ self.package = self.factory.makeSourcePackage(
+ sourcepackagename=self.sourcepackagename,
+ distroseries=self.distroseries)
+
+ def test_editpackaging_obsolete_series_in_vocabulary(self):
+ # The sourcepackage's current product series is included in
+ # the vocabulary even if it is obsolete.
+ self.package.setPackaging(self.obsolete_productseries, self.owner)
+ form = {
+ 'field.product': 'bonkers',
+ 'field.actions.continue': 'Continue',
+ 'field.__visited_steps__': 'sourcepackage_change_upstream_step1',
+ }
+ view = create_initialized_view(
+ self.package, name='+edit-packaging', form=form,
+ principal=self.owner)
+ self.assertEqual([], view.view.errors)
+ self.assertEqual(
+ self.obsolete_productseries,
+ view.view.form_fields['productseries'].field.default,
+ "The form's default productseries must be the current one.")
+ options = [term.token
+ for term in view.view.widgets['productseries'].vocabulary]
+ self.assertEqual(
+ ['trunk', 'current', 'obsolete'], options,
+ "The obsolete series must be in the vocabulary.")
+
+ def test_editpackaging_obsolete_series_not_in_vocabulary(self):
+ # Obsolete productseries are normally not in the vocabulary.
+ form = {
+ ...

Read more...

Revision history for this message
Brad Crittenden (bac) wrote :

Thanks for adding the test.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/registry/browser/sourcepackage.py'
--- lib/lp/registry/browser/sourcepackage.py 2010-02-18 20:39:01 +0000
+++ lib/lp/registry/browser/sourcepackage.py 2010-02-24 17:24:21 +0000
@@ -212,6 +212,22 @@
212 series for series in self.product.series212 series for series in self.product.series
213 if series.status != SeriesStatus.OBSOLETE213 if series.status != SeriesStatus.OBSOLETE
214 ]214 ]
215
216 # If the product is not being changed, then the current
217 # productseries can be the default choice. Otherwise,
218 # it will not exist in the vocabulary.
219 if (self.context.productseries is not None
220 and self.context.productseries.product == self.product):
221 series_default = self.context.productseries
222 # This only happens for obsolete series, since they aren't
223 # added to the vocabulary normally.
224 if series_default not in series_list:
225 series_list.append(series_default)
226 else:
227 series_default = None
228
229 # Put the development focus at the top of the list and create
230 # the vocabulary.
215 dev_focus = self.product.development_focus231 dev_focus = self.product.development_focus
216 if dev_focus in series_list:232 if dev_focus in series_list:
217 series_list.remove(dev_focus)233 series_list.remove(dev_focus)
@@ -223,15 +239,6 @@
223 dev_focus, dev_focus.name, "%s (Recommended)" % dev_focus.name)239 dev_focus, dev_focus.name, "%s (Recommended)" % dev_focus.name)
224 vocab_terms.insert(0, dev_focus_term)240 vocab_terms.insert(0, dev_focus_term)
225241
226 # If the product is not being changed, then the current
227 # productseries can be the default choice. Otherwise,
228 # it will not exist in the vocabulary.
229 if (self.context.productseries is not None
230 and self.context.productseries.product == self.product):
231 series_default = self.context.productseries
232 else:
233 series_default = None
234
235 productseries_choice = Choice(242 productseries_choice = Choice(
236 __name__='productseries',243 __name__='productseries',
237 title=_("Series"),244 title=_("Series"),
238245
=== modified file 'lib/lp/registry/tests/test_sourcepackage.py'
--- lib/lp/registry/tests/test_sourcepackage.py 2009-12-13 11:55:40 +0000
+++ lib/lp/registry/tests/test_sourcepackage.py 2010-02-24 17:24:21 +0000
@@ -22,6 +22,7 @@
22from lp.code.interfaces.seriessourcepackagebranch import (22from lp.code.interfaces.seriessourcepackagebranch import (
23 IMakeOfficialBranchLinks)23 IMakeOfficialBranchLinks)
24from lp.testing import TestCaseWithFactory24from lp.testing import TestCaseWithFactory
25from lp.testing.views import create_initialized_view
25from canonical.testing.layers import DatabaseFunctionalLayer26from canonical.testing.layers import DatabaseFunctionalLayer
2627
2728
@@ -238,5 +239,79 @@
238 Unauthorized, sourcepackage.setBranch, pocket, branch, registrant)239 Unauthorized, sourcepackage.setBranch, pocket, branch, registrant)
239240
240241
242class TestSourcePackageViews(TestCaseWithFactory):
243 """Tests for source package view classes."""
244
245 layer = DatabaseFunctionalLayer
246
247 def setUp(self):
248 TestCaseWithFactory.setUp(self)
249 self.owner = self.factory.makePerson()
250 self.product = self.factory.makeProduct(
251 name='bonkers', displayname='Bonkers', owner=self.owner)
252
253 self.obsolete_productseries = self.factory.makeProductSeries(
254 name='obsolete', product=self.product)
255 self.obsolete_productseries.status = SeriesStatus.OBSOLETE
256
257 self.dev_productseries = self.factory.makeProductSeries(
258 name='current', product=self.product)
259 self.dev_productseries.status = SeriesStatus.DEVELOPMENT
260
261 self.distribution = self.factory.makeDistribution(
262 name='youbuntu', displayname='Youbuntu', owner=self.owner)
263 self.distroseries = self.factory.makeDistroRelease(name='busy',
264 distribution=self.distribution)
265 self.sourcepackagename = self.factory.makeSourcePackageName(
266 name='bonkers')
267 self.package = self.factory.makeSourcePackage(
268 sourcepackagename=self.sourcepackagename,
269 distroseries=self.distroseries)
270
271 def test_editpackaging_obsolete_series_in_vocabulary(self):
272 # The sourcepackage's current product series is included in
273 # the vocabulary even if it is obsolete.
274 self.package.setPackaging(self.obsolete_productseries, self.owner)
275 form = {
276 'field.product': 'bonkers',
277 'field.actions.continue': 'Continue',
278 'field.__visited_steps__': 'sourcepackage_change_upstream_step1',
279 }
280 view = create_initialized_view(
281 self.package, name='+edit-packaging', form=form,
282 principal=self.owner)
283 self.assertEqual([], view.view.errors)
284 self.assertEqual(
285 self.obsolete_productseries,
286 view.view.form_fields['productseries'].field.default,
287 "The form's default productseries must be the current one.")
288 options = [term.token
289 for term in view.view.widgets['productseries'].vocabulary]
290 self.assertEqual(
291 ['trunk', 'current', 'obsolete'], options,
292 "The obsolete series must be in the vocabulary.")
293
294 def test_editpackaging_obsolete_series_not_in_vocabulary(self):
295 # Obsolete productseries are normally not in the vocabulary.
296 form = {
297 'field.product': 'bonkers',
298 'field.actions.continue': 'Continue',
299 'field.__visited_steps__': 'sourcepackage_change_upstream_step1',
300 }
301 view = create_initialized_view(
302 self.package, name='+edit-packaging', form=form,
303 principal=self.owner)
304 self.assertEqual([], view.view.errors)
305 self.assertEqual(
306 None,
307 view.view.form_fields['productseries'].field.default,
308 "The form's default productseries must be None.")
309 options = [term.token
310 for term in view.view.widgets['productseries'].vocabulary]
311 self.assertEqual(
312 ['trunk', 'current'], options,
313 "The obsolete series must NOT be in the vocabulary.")
314
315
241def test_suite():316def test_suite():
242 return unittest.TestLoader().loadTestsFromName(__name__)317 return unittest.TestLoader().loadTestsFromName(__name__)