launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/close-account-product-owner into lp:launchpad.

Commit message:
Skip references from inactive projects in close-account.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/close-account-product-owner/+merge/366294

Since projects can't be deleted as such, we need a way to indicate that we've got rid of them sufficiently for the purpose of closing an account.  Marking the project inactive is what we do when we're asked to remove projects, so that's good enough here too.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/close-account-product-owner into lp:launchpad.

=== modified file 'lib/lp/registry/scripts/closeaccount.py'
--- lib/lp/registry/scripts/closeaccount.py	2019-03-21 17:37:19 +0000
+++ lib/lp/registry/scripts/closeaccount.py	2019-04-18 21:58:29 +0000
@@ -24,6 +24,8 @@
     Person,
     PersonSettings,
     )
+from lp.registry.model.product import Product
+from lp.registry.model.productseries import ProductSeries
 from lp.services.database import postgresql
 from lp.services.database.constants import DEFAULT
 from lp.services.database.interfaces import IMasterStore
@@ -119,6 +121,7 @@
         ('poexportrequest', 'person'),
         ('pofile', 'lasttranslator'),
         ('pofiletranslator', 'person'),
+        ('product', 'registrant'),
         ('question', 'answerer'),
         ('questionreopening', 'answerer'),
         ('questionreopening', 'reopener'),
@@ -343,11 +346,39 @@
     table_notification('HWSubmission')
     store.find(HWSubmission, HWSubmission.ownerID == person.id).remove()
 
+    has_references = False
+
+    # Check for active related projects, and skip inactive ones.
+    for col in 'bug_supervisor', 'driver', 'owner':
+        # Raw SQL because otherwise using Product._owner while displaying it
+        # as Product.owner is too fiddly.
+        result = store.execute("""
+            SELECT COUNT(*) FROM product WHERE active AND %(col)s = ?
+            """ % {'col': col},
+            (person.id,))
+        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
+        skip.add(('product', col))
+    for col in 'driver', 'owner':
+        count = store.find(
+            ProductSeries,
+            ProductSeries.product == Product.id, Product.active,
+            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
+        skip.add(('productseries', col))
+
     # 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
     # require careful thought and decisions by a human administrator.
-    has_references = False
     for src_tab, src_col, ref_tab, ref_col, updact, delact in references:
         if (src_tab, src_col) in skip:
             continue

=== modified file 'lib/lp/registry/scripts/tests/test_closeaccount.py'
--- lib/lp/registry/scripts/tests/test_closeaccount.py	2019-03-21 17:37:19 +0000
+++ lib/lp/registry/scripts/tests/test_closeaccount.py	2019-04-18 21:58:29 +0000
@@ -448,3 +448,15 @@
             BugSummary.viewed_by_id.is_in([person.id, other_person.id])))
         self.assertThat(summaries, MatchesSetwise(
             MatchesStructure.byEquality(viewed_by=other_person)))
+
+    def test_skips_inactive_product_owner(self):
+        person = self.factory.makePerson()
+        product = self.factory.makeProduct(owner=person)
+        product.active = False
+        person_id = person.id
+        account_id = person.account.id
+        script = self.makeScript([six.ensure_str(person.name)])
+        with dbuser('launchpad'):
+            self.runScript(script)
+        self.assertRemoved(account_id, person_id)
+        self.assertEqual(person, product.owner)