launchpad-reviewers team mailing list archive

[Cristian Gonzalez <mp+410354@xxxxxxxxxxxxxxxxxx>]

Cristian Gonzalez has proposed merging ~cristiangsp/launchpad:closeaccount-skip-pofile-owner into launchpad:master.

Commit message:
Skip POFile.owner reference when running the close account script

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cristiangsp/launchpad/+git/launchpad/+merge/410354

It seems that the POFile.owner reference exists just for auditing purposes and there are not special privileges attached to it therefore it should be ok to include it in the skip list.

Regarding the unit test added, I have decided to set the ownership using "pofile.owner = person" as the owner parameter offered by the makePOFile is setting the POTemplate owner and not the POFile owner. Also other tests use this way too so probably not worht digging deeper on this now.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cristiangsp/launchpad:closeaccount-skip-pofile-owner into launchpad:master.

diff --git a/lib/lp/registry/scripts/closeaccount.py b/lib/lp/registry/scripts/closeaccount.py
index 419f99c..f488b78 100644
--- a/lib/lp/registry/scripts/closeaccount.py
+++ b/lib/lp/registry/scripts/closeaccount.py
@@ -142,6 +142,7 @@ def close_account(username, log):
         ('persontransferjob', 'minor_person'),
         ('poexportrequest', 'person'),
         ('pofile', 'lasttranslator'),
+        ('pofile', 'owner'),
         ('pofiletranslator', 'person'),
         ('product', 'registrant'),
         ('question', 'answerer'),
diff --git a/lib/lp/registry/scripts/tests/test_closeaccount.py b/lib/lp/registry/scripts/tests/test_closeaccount.py
index b90a255..fca56e5 100644
--- a/lib/lp/registry/scripts/tests/test_closeaccount.py
+++ b/lib/lp/registry/scripts/tests/test_closeaccount.py
@@ -407,6 +407,18 @@ class TestCloseAccount(TestCaseWithFactory):
             getUtility(IPOFileTranslatorSet).getForPersonPOFile(
                 person, pofile))
 
+    def test_skips_po_file_owners(self):
+        person = self.factory.makePerson()
+        pofile = self.factory.makePOFile()
+        pofile.owner = person
+        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)
+
+
     def test_handles_archive_subscriptions_and_tokens(self):
         person = self.factory.makePerson()
         ppa = self.factory.makeArchive(private=True)