Merge ~andrey-fedoseev/launchpad:close-account into launchpad:master

[Andrey Fedoseev]

close-account script: skip references related to inactive products

announcement.registrant
milestonetag.created_by
productrelease.owner
productreleasefile.uploader

[Colin Watson (cjwatson)]

review: Approve

diff --git a/lib/lp/registry/scripts/closeaccount.py b/lib/lp/registry/scripts/closeaccount.py
index c4e59fd..708bd5b 100644
--- a/lib/lp/registry/scripts/closeaccount.py
+++ b/lib/lp/registry/scripts/closeaccount.py
@@ -8,9 +8,11 @@ __all__ = [
     "CloseAccountScript",
 ]
 
+from typing import List, Tuple
+
 import six
 from storm.exceptions import IntegrityError
-from storm.expr import LeftJoin, Lower, Or
+from storm.expr import And, LeftJoin, Lower, Or
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
@@ -19,8 +21,12 @@ from lp.answers.model.question import Question
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.bugs.model.bugtask import BugTask
 from lp.registry.interfaces.person import PersonCreationRationale
+from lp.registry.model.announcement import Announcement
+from lp.registry.model.milestone import Milestone
+from lp.registry.model.milestonetag import MilestoneTag
 from lp.registry.model.person import Person, PersonSettings
 from lp.registry.model.product import Product
+from lp.registry.model.productrelease import ProductRelease, ProductReleaseFile
 from lp.registry.model.productseries import ProductSeries
 from lp.services.database import postgresql
 from lp.services.database.constants import DEFAULT
@@ -419,7 +425,7 @@ def close_account(username, log):
             "Can't delete non-trivial PPAs for user %s" % person_name
         )
 
-    has_references = False
+    reference_counts = []  # type: List[Tuple[str, int]]
 
     # Check for active related projects, and skip inactive ones.
     for col in "bug_supervisor", "driver", "owner":
@@ -434,11 +440,7 @@ def close_account(username, log):
         )
         count = result.get_one()[0]
         if count:
-            log.error(
-                "User %s is still referenced by %d product.%s values"
-                % (person_name, count, col)
-            )
-            has_references = True
+            reference_counts.append(("product.{}".format(col), count))
         skip.add(("product", col))
     for col in "driver", "owner":
         count = store.find(
@@ -448,13 +450,65 @@ def close_account(username, log):
             getattr(ProductSeries, col) == person,
         ).count()
         if count:
-            log.error(
-                "User %s is still referenced by %d productseries.%s values"
-                % (person_name, count, col)
-            )
-            has_references = True
+            reference_counts.append(("productseries.{}".format(col), count))
         skip.add(("productseries", col))
 
+    # Check announcements, skipping the ones
+    # that are related to inactive products.
+    count = store.find(
+        Announcement,
+        Or(
+            And(Announcement.product == Product.id, Product.active),
+            Announcement.product == None,
+        ),
+        Announcement.registrant == person,
+    ).count()
+    if count:
+        reference_counts.append(("announcement.registrant", count))
+    skip.add(("announcement", "registrant"))
+
+    # Check MilestoneTags, skipping the ones
+    # that are related to inactive products / product series.
+    count = store.find(
+        MilestoneTag,
+        MilestoneTag.milestone_id == Milestone.id,
+        Or(
+            And(Milestone.product == Product.id, Product.active),
+            Milestone.product == None,
+        ),
+        MilestoneTag.created_by_id == person.id,
+    ).count()
+    if count:
+        reference_counts.append(("milestonetag.created_by", count))
+    skip.add(("milestonetag", "created_by"))
+
+    # Check ProductReleases, skipping the ones
+    # that are related to inactive products / product series.
+    count = store.find(
+        ProductRelease,
+        ProductRelease.milestone == Milestone.id,
+        Milestone.product == Product.id,
+        Product.active,
+        ProductRelease.owner == person.id,
+    ).count()
+    if count:
+        reference_counts.append(("productrelease.owner", count))
+    skip.add(("productrelease", "owner"))
+
+    # Check ProductReleases, skipping the ones
+    # that are related to inactive products / product series.
+    count = store.find(
+        ProductReleaseFile,
+        ProductReleaseFile.productrelease == ProductRelease.id,
+        ProductRelease.milestone == Milestone.id,
+        Milestone.product == Product.id,
+        Product.active,
+        ProductReleaseFile.uploader == person.id,
+    ).count()
+    if count:
+        reference_counts.append(("productreleasefile.uploader", count))
+    skip.add(("productreleasefile", "uploader"))
+
     # Closing the account will only work if all references have been handled
     # by this point.  If not, it's safer to bail out.  It's OK if this
     # doesn't work in all conceivable situations, since some of them may
@@ -474,12 +528,14 @@ def close_account(username, log):
         )
         count = result.get_one()[0]
         if count:
+            reference_counts.append(("{}.{}".format(src_tab, src_col), count))
+
+    if reference_counts:
+        for reference, count in reference_counts:
             log.error(
-                "User %s is still referenced by %d %s.%s values"
-                % (person_name, count, src_tab, src_col)
+                "User %s is still referenced by %d %s values"
+                % (person_name, count, reference)
             )
-            has_references = True
-    if has_references:
         raise LaunchpadScriptFailure(
             "User %s is still referenced" % person_name
         )
diff --git a/lib/lp/registry/scripts/tests/test_closeaccount.py b/lib/lp/registry/scripts/tests/test_closeaccount.py
index 348b747..b78ff60 100644
--- a/lib/lp/registry/scripts/tests/test_closeaccount.py
+++ b/lib/lp/registry/scripts/tests/test_closeaccount.py
@@ -2,7 +2,9 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test the close-account script."""
+from datetime import datetime
 
+import pytz
 import transaction
 from storm.store import Store
 from testtools.matchers import (
@@ -25,6 +27,7 @@ from lp.code.interfaces.codeimportjob import ICodeImportJobWorkflow
 from lp.code.tests.helpers import GitHostingFixture
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.teammembership import ITeamMembershipSet
+from lp.registry.model.productrelease import ProductRelease
 from lp.registry.scripts.closeaccount import CloseAccountScript
 from lp.scripts.garbo import PopulateLatestPersonSourcePackageReleaseCache
 from lp.services.database.interfaces import IStore
@@ -1005,3 +1008,169 @@ class TestCloseAccount(TestCaseWithFactory):
         self.assertRemoved(account_id, person_id)
         self.assertIsNone(store.get(Archive, ppa_id))
         self.assertEqual(other_ppa, store.get(Archive, other_ppa_id))
+
+    def test_skips_announcements_in_deactivated_products(self):
+        person = self.factory.makePerson()
+        person_id = person.id
+        account_id = person.account.id
+
+        product = self.factory.makeProduct()
+        product.announce(person, "announcement")
+
+        script = self.makeScript([person.name])
+        with dbuser("launchpad"):
+            self.assertRaisesWithContent(
+                LaunchpadScriptFailure,
+                "User %s is still referenced" % person.name,
+                self.runScript,
+                script,
+            )
+        self.assertNotRemoved(account_id, person_id)
+
+        product.active = False
+        with dbuser("launchpad"):
+            self.runScript(script)
+
+        self.assertRemoved(account_id, person_id)
+
+    def test_non_product_announcements_are_not_skipped(self):
+        person = self.factory.makePerson()
+        person_id = person.id
+        account_id = person.account.id
+
+        distro = self.factory.makeDistribution()
+        distro.announce(person, "announcement")
+
+        script = self.makeScript([person.name])
+        with dbuser("launchpad"):
+            self.assertRaisesWithContent(
+                LaunchpadScriptFailure,
+                "User %s is still referenced" % person.name,
+                self.runScript,
+                script,
+            )
+        self.assertNotRemoved(account_id, person_id)
+
+    def test_skip_milestone_tags_from_inactive_products(self):
+
+        active_product = self.factory.makeProduct()
+        inactive_product = self.factory.makeProduct()
+        inactive_product.active = False
+
+        active_product_series = self.factory.makeProductSeries(
+            product=active_product
+        )
+        inactive_product_series = self.factory.makeProductSeries(
+            product=inactive_product
+        )
+
+        for milestone_target, expected_to_be_removed in (
+            ({"product": active_product}, False),
+            ({"product": inactive_product}, True),
+            ({"productseries": active_product_series}, False),
+            ({"productseries": inactive_product_series}, True),
+            ({"distribution": self.factory.makeDistribution()}, False),
+            ({"distroseries": self.factory.makeDistroSeries()}, False),
+        ):
+            person = self.factory.makePerson()
+            person_id = person.id
+            account_id = person.account.id
+
+            milestone = self.factory.makeMilestone(**milestone_target)
+            milestone.setTags(["tag"], person)
+            script = self.makeScript([person.name])
+            with dbuser("launchpad"):
+                if not expected_to_be_removed:
+                    self.assertRaisesWithContent(
+                        LaunchpadScriptFailure,
+                        "User %s is still referenced" % person.name,
+                        self.runScript,
+                        script,
+                    )
+                    self.assertNotRemoved(account_id, person_id)
+                else:
+                    self.runScript(script)
+                    self.assertRemoved(account_id, person_id)
+
+    def test_skip_product_releases_from_inactive_products(self):
+
+        active_product = self.factory.makeProduct()
+        inactive_product = self.factory.makeProduct()
+        inactive_product.active = False
+
+        active_product_series = self.factory.makeProductSeries(
+            product=active_product
+        )
+        inactive_product_series = self.factory.makeProductSeries(
+            product=inactive_product
+        )
+
+        for milestone_target, expected_to_be_removed in (
+            ({"product": active_product}, False),
+            ({"product": inactive_product}, True),
+            ({"productseries": active_product_series}, False),
+            ({"productseries": inactive_product_series}, True),
+        ):
+            person = self.factory.makePerson()
+            person_id = person.id
+            account_id = person.account.id
+
+            milestone = self.factory.makeMilestone(**milestone_target)
+            milestone.createProductRelease(person, datetime.now(pytz.UTC))
+            script = self.makeScript([person.name])
+            with dbuser("launchpad"):
+                if not expected_to_be_removed:
+                    self.assertRaisesWithContent(
+                        LaunchpadScriptFailure,
+                        "User %s is still referenced" % person.name,
+                        self.runScript,
+                        script,
+                    )
+                    self.assertNotRemoved(account_id, person_id)
+                else:
+                    self.runScript(script)
+                    self.assertRemoved(account_id, person_id)
+
+    def test_skip_product_releases_files_from_inactive_products(self):
+
+        active_product = self.factory.makeProduct()
+        inactive_product = self.factory.makeProduct()
+        inactive_product.active = False
+
+        active_product_series = self.factory.makeProductSeries(
+            product=active_product
+        )
+        inactive_product_series = self.factory.makeProductSeries(
+            product=inactive_product
+        )
+
+        for milestone_target, expected_to_be_removed in (
+            ({"product": active_product}, False),
+            ({"product": inactive_product}, True),
+            ({"productseries": active_product_series}, False),
+            ({"productseries": inactive_product_series}, True),
+        ):
+            person = self.factory.makePerson()
+            person_id = person.id
+            account_id = person.account.id
+
+            milestone = self.factory.makeMilestone(**milestone_target)
+            product_release = milestone.createProductRelease(
+                milestone.product.owner, datetime.now(pytz.UTC)
+            )  # type: ProductRelease
+            product_release.addReleaseFile(
+                "test.txt", b"test", "text/plain", person
+            )
+            script = self.makeScript([person.name])
+            with dbuser("launchpad"):
+                if not expected_to_be_removed:
+                    self.assertRaisesWithContent(
+                        LaunchpadScriptFailure,
+                        "User %s is still referenced" % person.name,
+                        self.runScript,
+                        script,
+                    )
+                    self.assertNotRemoved(account_id, person_id)
+                else:
+                    self.runScript(script)
+                    self.assertRemoved(account_id, person_id)