launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/launchpad/initseries-for-one-night-only-bug-793620 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #793620 in Launchpad itself: "The +initseries page should display an error message if accessed once a series is initialized."
  https://bugs.launchpad.net/launchpad/+bug/793620

For more details, see:
https://code.launchpad.net/~allenap/launchpad/initseries-for-one-night-only-bug-793620/+merge/63756

Show a message on +initseries, and prevent attempts to use the page,
when the distroseries is in the process of being derived or when the
distroseries has already been derived.
-- 
https://code.launchpad.net/~allenap/launchpad/initseries-for-one-night-only-bug-793620/+merge/63756
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/initseries-for-one-night-only-bug-793620 into lp:launchpad.

=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py	2011-06-03 00:13:41 +0000
+++ lib/lp/registry/browser/distroseries.py	2011-06-07 19:53:29 +0000
@@ -647,6 +647,29 @@
         return getFeatureFlag("soyuz.derived_series_ui.enabled") is not None
 
     @property
+    def show_derivation_not_yet_available(self):
+        return not self.is_derived_series_feature_enabled
+
+    @property
+    def show_derivation_form(self):
+        return (
+            self.is_derived_series_feature_enabled and
+            not self.context.is_derived_series and
+            not self.context.is_initialising)
+
+    @property
+    def show_already_derived_message(self):
+        return (
+            self.is_derived_series_feature_enabled and
+            self.context.is_derived_series)
+
+    @property
+    def show_already_initializing_message(self):
+        return (
+            self.is_derived_series_feature_enabled and
+            self.context.is_initialising)
+
+    @property
     def next_url(self):
         return canonical_url(self.context)
 

=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
--- lib/lp/registry/browser/tests/test_distroseries.py	2011-06-02 18:36:43 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py	2011-06-07 19:53:29 +0000
@@ -40,11 +40,11 @@
     LaunchpadFunctionalLayer,
     LaunchpadZopelessLayer,
     )
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.archivepublisher.debversion import Version
-from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.registry.browser.distroseries import (
+    HIGHER_VERSION_THAN_PARENT,
     IGNORED,
-    HIGHER_VERSION_THAN_PARENT,
     NON_IGNORED,
     RESOLVED,
     )
@@ -73,8 +73,8 @@
 from lp.soyuz.interfaces.sourcepackageformat import (
     ISourcePackageFormatSelectionSet,
     )
+from lp.soyuz.model import distroseriesdifferencejob
 from lp.soyuz.model.archivepermission import ArchivePermission
-from lp.soyuz.model import distroseriesdifferencejob
 from lp.testing import (
     anonymous_logged_in,
     celebrity_logged_in,
@@ -455,6 +455,41 @@
                 u"javascript-disabled",
                 message.get("class").split())
 
+    def test_form_hidden_when_distroseries_is_initialized(self):
+        # The form is hidden when the feature flag is set but the series has
+        # already been derived.
+        distroseries = self.factory.makeDistroSeries()
+        self.factory.makeDistroSeriesParent(derived_series=distroseries)
+        view = create_initialized_view(distroseries, "+initseries")
+        with feature_flags():
+            set_feature_flag(u"soyuz.derived_series_ui.enabled", u"true")
+            root = html.fromstring(view())
+            self.assertEqual(
+                [], root.cssselect("#initseries-form-container"))
+            # Instead an explanatory message is shown.
+            [message] = root.cssselect("p.error.message")
+            self.assertEqual(
+                u"This series has already been derived.",
+                message.text.strip())
+
+    def test_form_hidden_when_distroseries_is_being_initialized(self):
+        # The form is hidden when the feature flag is set but the series has
+        # already been derived.
+        distroseries = self.factory.makeDistroSeries()
+        getUtility(IInitialiseDistroSeriesJobSource).create(
+            distroseries, [self.factory.makeDistroSeries().id])
+        view = create_initialized_view(distroseries, "+initseries")
+        with feature_flags():
+            set_feature_flag(u"soyuz.derived_series_ui.enabled", u"true")
+            root = html.fromstring(view())
+            self.assertEqual(
+                [], root.cssselect("#initseries-form-container"))
+            # Instead an explanatory message is shown.
+            [message] = root.cssselect("p.error.message")
+            self.assertEqual(
+                u"This series is already being initialized.",
+                message.text.strip())
+
 
 class DistroSeriesDifferenceMixin:
     """A helper class for testing differences pages"""

=== modified file 'lib/lp/registry/templates/distroseries-initialize.pt'
--- lib/lp/registry/templates/distroseries-initialize.pt	2011-05-30 15:17:11 +0000
+++ lib/lp/registry/templates/distroseries-initialize.pt	2011-06-07 19:53:29 +0000
@@ -12,13 +12,13 @@
   </metal:head-epilogue>
   <body>
     <div metal:fill-slot="main">
-      <tal:enabled condition="view/is_derived_series_feature_enabled">
+
+      <tal:enabled condition="view/show_derivation_form">
       <div class="top-portlet">
         This page allows you to initialize a distribution series.
         <a href="/+help/init-series-title-help.html"
            target="help" class="sprite maybe">&nbsp;
           <span class="invisible-link">Initialization help</span></a>
-
       </div>
       <p class="error message javascript-disabled">
         Javascript is required to use this page. Please enable
@@ -39,7 +39,8 @@
         many thousands of packages is likely to take hours to complete.
       </p>
       </tal:enabled>
-      <tal:disabled condition="not:view/is_derived_series_feature_enabled">
+
+      <tal:disabled condition="view/show_derivation_not_yet_available">
       <p class="error message">
         The Derivative Distributions feature is under development
         and is not yet generally available. You can read more about
@@ -48,6 +49,20 @@
         page</a>.
       </p>
       </tal:disabled>
+
+      <tal:already-derived condition="view/show_already_derived_message">
+      <p class="error message">
+        This series has already been derived.
+      </p>
+      </tal:already-derived>
+
+      <tal:already-initializing
+          condition="view/show_already_initializing_message">
+      <p class="error message">
+        This series is already being initialized.
+      </p>
+      </tal:already-initializing>
+
     </div>
   </body>
 </html>