launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/handle-disabled-archives-recipe into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #715325 recipe is forbidden when it lists a deleted archive
  https://bugs.launchpad.net/bugs/715325

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/handle-disabled-archives-recipe/+merge/50095

Filter both completed builds and pending builds to not return builds that have built or are building in an archive that is disabled. I've added a view test as well as test of getBuilds() and getPendingBuilds(). I also cleaned up a little lint.
-- 
https://code.launchpad.net/~stevenk/launchpad/handle-disabled-archives-recipe/+merge/50095
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/handle-disabled-archives-recipe into lp:launchpad.

=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2011-02-15 11:31:56 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2011-02-17 05:04:58 +0000
@@ -45,7 +45,10 @@
     )
 from lp.code.interfaces.sourcepackagerecipe import MINIMAL_RECIPE_TEXT
 from lp.code.tests.helpers import recipe_parser_newest_version
-from lp.registry.interfaces.person import TeamSubscriptionPolicy
+from lp.registry.interfaces.person import (
+    IPersonSet,
+    TeamSubscriptionPolicy,
+    )
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.propertycache import clear_property_cache
 from lp.soyuz.model.processor import ProcessorFamily
@@ -58,6 +61,7 @@
     time_counter,
     )
 from lp.testing.factory import remove_security_proxy_and_shout_at_engineer
+from lp.testing.sampledata import NO_PRIVILEGE_EMAIL
 from lp.testing.views import create_initialized_view
 
 
@@ -405,7 +409,7 @@
         # The error for a recipe with invalid instruction parameters should
         # include instruction usage.
         branch = self.factory.makeBranch(name='veggies')
-        package_branch = self.factory.makeBranch(name='packaging')
+        self.factory.makeBranch(name='packaging')
 
         browser = self.createRecipe(
             dedent('''\
@@ -1157,7 +1161,7 @@
             archive=self.ppa, sourcepackagename=package_name,
             distroseries=self.squirrel, source_package_recipe_build=build,
             version='0+r42')
-        spph = self.factory.makeSourcePackagePublishingHistory(
+        self.factory.makeSourcePackagePublishingHistory(
             sourcepackagerelease=source_package_release, archive=self.ppa,
             distroseries=self.squirrel)
         builder = self.factory.makeBuilder()
@@ -1188,7 +1192,7 @@
             archive=self.ppa, sourcepackagename=package_name,
             distroseries=self.squirrel, source_package_recipe_build=build,
             version='0+r42')
-        spph = self.factory.makeSourcePackagePublishingHistory(
+        self.factory.makeSourcePackagePublishingHistory(
             sourcepackagerelease=source_package_release, archive=self.ppa,
             distroseries=self.squirrel)
         builder = self.factory.makeBuilder()
@@ -1439,6 +1443,22 @@
             "upload packages into the daily build PPA (PPA for Team2)",
             messages[-1])
 
+    def test_view_with_disabled_archive(self):
+        # When a PPA is disabled, it is only viewable to the owner. This
+        # case is handled with the view not showing builds into a disabled
+        # archive, rather than giving an Unauthorized error to the user.
+        no_priv = getUtility(IPersonSet).getByEmail(NO_PRIVILEGE_EMAIL)
+        recipe = self.factory.makeSourcePackageRecipe(build_daily=True)
+        recipe.requestBuild(
+            recipe.daily_build_archive, recipe.owner, self.squirrel,
+            PackagePublishingPocket.RELEASE)
+        with person_logged_in(recipe.owner):
+            recipe.daily_build_archive.disable()
+        browser = self.getUserBrowser(canonical_url(recipe), user=no_priv)
+        self.assertIn(
+            "This recipe has not been built yet.",
+            extract_text(find_main_content(browser.contents)))
+
 
 class TestSourcePackageRecipeBuildView(BrowserTestCase):
     """Test behaviour of SourcePackageReciptBuildView."""

=== modified file 'lib/lp/code/model/sourcepackagerecipe.py'
--- lib/lp/code/model/sourcepackagerecipe.py	2011-02-03 03:39:14 +0000
+++ lib/lp/code/model/sourcepackagerecipe.py	2011-02-17 05:04:58 +0000
@@ -67,6 +67,7 @@
 from lp.registry.model.distroseries import DistroSeries
 from lp.services.database.stormexpr import Greatest
 from lp.soyuz.interfaces.archive import IArchiveSet
+from lp.soyuz.model.archive import Archive
 
 
 def get_buildable_distroseries_set(user):
@@ -291,6 +292,8 @@
             SourcePackageRecipeBuild.recipe==self,
             SourcePackageRecipeBuild.package_build_id == PackageBuild.id,
             PackageBuild.build_farm_job_id == BuildFarmJob.id,
+            And(PackageBuild.archive_id == Archive.id,
+                Archive._enabled == True),
             where_clause)
         result.order_by(order_by)
         return result

=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipe.py	2011-02-13 22:07:27 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py	2011-02-17 05:04:58 +0000
@@ -616,6 +616,17 @@
         self.assertEqual([build], list(recipe.getBuilds()))
         self.assertEqual([], list(recipe.getPendingBuilds()))
 
+    def test_getBuilds_disabled_archive(self):
+        # Builds into an disabled archive aren't returned
+        archive = self.factory.makeArchive()
+        recipe = self.factory.makeSourcePackageRecipe()
+        build = self.factory.makeSourcePackageRecipeBuild(
+            recipe=recipe, archive=archive)
+        with person_logged_in(archive.owner):
+            archive.disable()
+        self.assertEqual([], list(recipe.getBuilds()))
+        self.assertEqual([], list(recipe.getPendingBuilds()))
+
 
 class TestRecipeBranchRoundTripping(TestCaseWithFactory):