Code review comment for lp:~adiroiban/launchpad/bug-462013-try-2

Hey,

Thanks for the review.
Have no mercy reviewing my branches. It is your project and you should not accept bad code.
Don't worry, I will not take offence.

Here is the latest diff.

=== modified file 'lib/lp/translations/browser/product.py'
--- lib/lp/translations/browser/product.py 2010-01-28 14:23:10 +0000
+++ lib/lp/translations/browser/product.py 2010-01-29 16:08:54 +0000
@@ -116,13 +116,10 @@
         """Return series which are not yet set up for translations.

         The list is sorted in alphabetically order and obsolete series
- will be excluded.
+ are excluded.
         """

- all_active_series = set(
- [serie for serie in self.context.series if (
- serie.status != SeriesStatus.OBSOLETE)])
- translatable = set(self.context.translatable_series)
- return sorted(
- all_active_series - translatable,
- key=lambda series: series.name)
+ translatable = self.context.translatable_series
+ return [series for series in self.context.series if (
+ series.status != SeriesStatus.OBSOLETE and
+ series not in translatable)]

=== modified file 'lib/lp/translations/browser/tests/test_product_view.py'
--- lib/lp/translations/browser/tests/test_product_view.py 2010-01-28 14:23:10 +0000
+++ lib/lp/translations/browser/tests/test_product_view.py 2010-01-29 16:11:36 +0000
@@ -7,9 +7,9 @@

 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 from canonical.testing import LaunchpadZopelessLayer
+from lp.registry.interfaces.series import SeriesStatus
 from lp.translations.browser.product import ProductView
 from lp.testing import TestCaseWithFactory
-from lp.registry.interfaces.series import SeriesStatus

 class TestProduct(TestCaseWithFactory):
@@ -17,9 +17,6 @@

     layer = LaunchpadZopelessLayer

- def setUp(self):
- TestCaseWithFactory.setUp(self)
-
     def test_primary_translatable_with_package_link(self):
         # Create a product that uses translations.
         product = self.factory.makeProduct()
@@ -75,24 +72,17 @@
             product=product, name='evo-future')
         series_future.status = SeriesStatus.FUTURE

- # 'untranslatable_series' is a cached property, this is why we
- # check it after adding all series.
- self.assertTrue(series_experimental in view.untranslatable_series)
- self.assertTrue(series_development in view.untranslatable_series)
- self.assertTrue(series_frozen in view.untranslatable_series)
- self.assertTrue(series_current in view.untranslatable_series)
- self.assertTrue(series_supported in view.untranslatable_series)
- self.assertTrue(series_future in view.untranslatable_series)
-
- # Obsolete series are not included
- self.assertFalse(series_obsolete in view.untranslatable_series)
-
- # Series are listed in alphabetical order, 'evo-current'
- # is firsts, while 'trunk' is last.
- self.assertTrue(
- series_current == view.untranslatable_series[0])
- self.assertTrue(
- series_trunk == view.untranslatable_series[-1])
+ # The series are returned in alphabetical order and do not
+ # include obsolete series.
+ series_names = [series.name for series in view.untranslatable_series]
+ self.assertEqual([
+ u'evo-current',
+ u'evo-development',
+ u'evo-experimental',
+ u'evo-frozen',
+ u'evo-future',
+ u'evo-supported',
+ u'trunk'], series_names)

 def test_suite():

=== modified file 'lib/lp/translations/stories/productseries/xx-productseries-translations.txt'
--- lib/lp/translations/stories/productseries/xx-productseries-translations.txt 2010-01-28 14:23:10 +0000
+++ lib/lp/translations/stories/productseries/xx-productseries-translations.txt 2010-01-29 16:04:34 +0000
@@ -271,7 +271,7 @@

 Setting up translations for series
-------------------------------------
+----------------------------------

 When visiting product translations main page, project developers sees
 status for current series configured for translations.