launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/daily-build-disabled-archive into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #845846 in Launchpad itself: "recipe request daily build fails when the archive is disabled"
  https://bugs.launchpad.net/launchpad/+bug/845846

For more details, see:
https://code.launchpad.net/~abentley/launchpad/daily-build-disabled-archive/+merge/75239

= Summary =
Fix bug #845846: recipe request daily build fails when the archive is disabled

== Proposed fix ==
We can't build when an archive is disabled, but we can provide a better user experience:
- Do not show the link for a daily build if the archive is disabled.
- If a daily build is requested and the archive is disabled, handle it as a user error.

== Pre-implementation notes ==
None

== Implementation details ==
None

== Tests ==
bin/test test_sourcepackagerecipe -t test_request_daily_builds_disabled_archive -t test_request_daily_builds_button_ppa_disabled

== Demo and Q/A ==
Create a recipe, set up daily builds, navigate to the request-daily-build page.

Disable the archive.

Request a build.

You should get a sane error message.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/browser/sourcepackagerecipe.py
  lib/lp/code/browser/tests/test_sourcepackagerecipe.py
-- 
https://code.launchpad.net/~abentley/launchpad/daily-build-disabled-archive/+merge/75239
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/daily-build-disabled-archive into lp:launchpad.

=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py	2011-08-31 19:45:02 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py	2011-09-13 18:41:32 +0000
@@ -114,6 +114,7 @@
 from lp.services.features import getFeatureFlag
 from lp.services.fields import PersonChoice
 from lp.services.propertycache import cachedproperty
+from lp.soyuz.interfaces.archive import ArchiveDisabled
 from lp.soyuz.model.archive import Archive
 
 
@@ -196,8 +197,8 @@
         """Provide a link for requesting a daily build of a recipe."""
         recipe = self.context
         ppa = recipe.daily_build_archive
-        if (ppa is None or not recipe.build_daily or not recipe.is_stale
-                or not recipe.distroseries):
+        if (ppa is None or not ppa.enabled or not recipe.build_daily or not
+            recipe.is_stale or not recipe.distroseries):
             show_request_build = False
         else:
             has_upload = ppa.checkArchivePermission(recipe.owner)
@@ -544,7 +545,7 @@
         recipe = self.context
         try:
             builds = recipe.performDailyBuild()
-        except TooManyBuilds, e:
+        except (TooManyBuilds, ArchiveDisabled) as e:
             self.request.response.addErrorNotification(str(e))
             self.next_url = canonical_url(recipe)
             return

=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2011-08-31 20:27:28 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2011-09-13 18:41:32 +0000
@@ -1317,6 +1317,21 @@
         build_button = find_tag_by_id(browser.contents, 'field.actions.build')
         self.assertIs(None, build_button)
 
+    def test_request_daily_builds_button_ppa_disabled(self):
+        # Recipes whose daily build ppa is disabled do not have a build now
+        # link.
+        distroseries = self.factory.makeSourcePackageRecipeDistroseries()
+        daily_build_archive = self.factory.makeArchive(
+                distribution=distroseries.distribution, owner=self.user)
+        with person_logged_in(self.user):
+            daily_build_archive.disable()
+        recipe = self.factory.makeSourcePackageRecipe(
+            owner=self.chef, daily_build_archive=daily_build_archive,
+            is_stale=True, build_daily=True)
+        browser = self.getViewBrowser(recipe)
+        build_button = find_tag_by_id(browser.contents, 'field.actions.build')
+        self.assertIs(None, build_button)
+
     def test_request_daily_builds_ajax_link_not_rendered(self):
         # The Build now link should not be rendered without javascript.
         recipe = self.factory.makeSourcePackageRecipe(
@@ -1362,6 +1377,20 @@
             "for distroseries ubuntu warty",
             harness.view.request.notifications[0].message)
 
+    def test_request_daily_builds_disabled_archive(self):
+        # Requesting a daily build from a disabled archive is a user error.
+        recipe = self.factory.makeSourcePackageRecipe(
+            owner=self.chef, daily_build_archive=self.ppa,
+            name=u'julia', is_stale=True, build_daily=True)
+        harness = LaunchpadFormHarness(
+            recipe, SourcePackageRecipeRequestDailyBuildView)
+        with person_logged_in(self.ppa.owner):
+            self.ppa.disable()
+        harness.submit('build', {})
+        self.assertEqual(
+            "Secret PPA is disabled.",
+            harness.view.request.notifications[0].message)
+
     def test_request_builds_page(self):
         """Ensure the +request-builds page is sane."""
         recipe = self.makeRecipe()