Merge lp:~cjwatson/launchpad/close-account-message-owner into lp:launchpad

[Colin Watson]

Skip Message.owner references in close-account.

These are created if (e.g.) the user had commented on a question at some point. There's no need to break those links.

[Natalia Bidart (nataliabidart)]

13:37 < nessita> cjwatson, question about your branch: the commit says "skip owner references..." but the code adds them, so I'm not sure I understand what is going on
13:39 < cjwatson> nessita: It adds them to a set called "skip" :)
13:41 < cjwatson>| close-account's job is to do as full an erasure of an account as is possible (to satisfy legal demands etc.) while not breaking referential integrity. There are way too many FKs on Person.id to be realistic to decide what to do about all of them up-front, so the purpose of the "skip" set is to be a list of FKs that we've determined are OK to keep around even after an account has been wiped ...
13:41 < cjwatson>| ... and replaced with a minimal placeholder.
13:43 < nessita> cjwatson, I understand, thanks. And the tests, do they have a machinery where by adding an owner there are asserts that will ensure is not there?
13:45 < cjwatson> nessita: Indeed. With only the test changes, one gets https://paste.ubuntu.com/p/DcKHgpfRPn/
13:45 < cjwatson>| I recently refactored close-account to be super-paranoid about what happens if any unhandled FKs are left over
13:46 < cjwatson>| So we are forced to make some kind of decision about them
13:46 < nessita> cjwatson, great, it makes sense and I think it looks good

review: Approve

=== modified file 'lib/lp/registry/scripts/closeaccount.py'
--- lib/lp/registry/scripts/closeaccount.py	2018-12-03 11:34:32 +0000
+++ lib/lp/registry/scripts/closeaccount.py	2018-12-04 13:15:36 +0000
@@ -100,6 +100,7 @@
         ('gitrule', 'creator'),
         ('gitrulegrant', 'grantor'),
         ('gitsubscription', 'subscribed_by'),
+        ('message', 'owner'),
         ('messageapproval', 'disposed_by'),
         ('messageapproval', 'posted_by'),
         ('packagecopyrequest', 'requester'),

=== modified file 'lib/lp/registry/scripts/tests/test_closeaccount.py'
--- lib/lp/registry/scripts/tests/test_closeaccount.py	2018-11-29 18:44:25 +0000
+++ lib/lp/registry/scripts/tests/test_closeaccount.py	2018-12-04 13:15:36 +0000
@@ -206,6 +206,7 @@
                 QuestionStatus.OPEN, QuestionStatus.NEEDSINFO,
                 QuestionStatus.ANSWERED):
             question = self.factory.makeQuestion(owner=person)
+            question.addComment(person, "comment")
             removeSecurityProxy(question).status = status
             questions.append(question)
         script = self.makeScript([nativeString(person.name)])
@@ -227,6 +228,7 @@
                 QuestionStatus.SOLVED, QuestionStatus.EXPIRED,
                 QuestionStatus.INVALID):
             question = self.factory.makeQuestion(owner=person)
+            question.addComment(person, "comment")
             removeSecurityProxy(question).status = status
             questions[status] = question
         script = self.makeScript([nativeString(person.name)])