launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:bugfix-remove-ocirecipe-pushrules into launchpad:master.

Commit message:
Removing associated push rules when deleting an OCI recipe

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1899078 in Launchpad itself: "OCI: Can't delete OCI recipe (internal error)"
  https://bugs.launchpad.net/launchpad/+bug/1899078

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/392055
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:bugfix-remove-ocirecipe-pushrules into launchpad:master.

diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
index 15fa5be..fd21dde 100644
--- a/lib/lp/oci/model/ocirecipe.py
+++ b/lib/lp/oci/model/ocirecipe.py
@@ -240,6 +240,8 @@ class OCIRecipe(Storm, WebhookTargetMixin):
         store.find(Job, Job.id.is_in(affected_jobs)).remove()
         builds = store.find(OCIRecipeBuild, OCIRecipeBuild.recipe == self)
         builds.remove()
+        for push_rule in self.push_rules:
+            push_rule.destroySelf()
         getUtility(IWebhookSet).delete(self.webhooks)
         store.remove(self)
         store.find(
diff --git a/lib/lp/oci/tests/test_ocirecipe.py b/lib/lp/oci/tests/test_ocirecipe.py
index 76997a7..a0b1c59 100644
--- a/lib/lp/oci/tests/test_ocirecipe.py
+++ b/lib/lp/oci/tests/test_ocirecipe.py
@@ -32,7 +32,10 @@ from zope.security.proxy import removeSecurityProxy
 
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.interfaces.processor import IProcessorSet
-from lp.oci.interfaces.ocipushrule import OCIPushRuleAlreadyExists
+from lp.oci.interfaces.ocipushrule import (
+    IOCIPushRuleSet,
+    OCIPushRuleAlreadyExists,
+    )
 from lp.oci.interfaces.ocirecipe import (
     CannotModifyOCIRecipeProcessor,
     DuplicateOCIRecipeName,
@@ -385,11 +388,16 @@ class TestOCIRecipe(OCIConfigHelperMixin, TestCaseWithFactory):
                     for payload_matcher in payload_matchers]))
 
     def test_destroySelf(self):
+        self.setConfig()
         oci_recipe = self.factory.makeOCIRecipe()
-        build_ids = []
-        for x in range(3):
-            build_ids.append(
-                self.factory.makeOCIRecipeBuild(recipe=oci_recipe).id)
+        # Create associated builds:
+        build_ids = [
+            self.factory.makeOCIRecipeBuild(recipe=oci_recipe).id
+            for _ in range(3)]
+        # Create associated push rules:
+        push_rule_ids = [
+            self.factory.makeOCIPushRule(recipe=oci_recipe).id
+            for i in range(3)]
 
         with person_logged_in(oci_recipe.owner):
             oci_recipe.destroySelf()
@@ -397,6 +405,9 @@ class TestOCIRecipe(OCIConfigHelperMixin, TestCaseWithFactory):
 
         for build_id in build_ids:
             self.assertIsNone(getUtility(IOCIRecipeBuildSet).getByID(build_id))
+        for push_rule_id in push_rule_ids:
+            self.assertIsNone(
+                getUtility(IOCIPushRuleSet).getByID(push_rule_id))
 
     def test_related_webhooks_deleted(self):
         owner = self.factory.makePerson()