launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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

Commit message:
Add a dry-run mode to close-account.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/close-account-dry-run/+merge/366853

I keep finding myself hacking something like this in when testing account closures on dogfood, so it'd be better to have it as a proper option.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/close-account-dry-run into lp:launchpad.

=== modified file 'lib/lp/registry/scripts/closeaccount.py'
--- lib/lp/registry/scripts/closeaccount.py	2019-04-23 09:26:52 +0000
+++ lib/lp/registry/scripts/closeaccount.py	2019-05-02 17:53:49 +0000
@@ -410,6 +410,12 @@
         "Close a person's account, deleting as much personal information "
         "as possible.")
 
+    def add_my_options(self):
+        """See `LaunchpadScript`."""
+        self.parser.add_option(
+            "-n", "--dry-run", default=False, action="store_true",
+            help="Do not commit changes.")
+
     def main(self):
         if not self.args:
             raise LaunchpadScriptFailure(
@@ -421,5 +427,10 @@
             except Exception:
                 self.txn.abort()
                 raise
-        self.logger.debug("Committing changes")
-        self.txn.commit()
+
+        if self.options.dry_run:
+            self.logger.debug("Dry run, so not committing changes")
+            self.txn.abort()
+        else:
+            self.logger.debug("Committing changes")
+            self.txn.commit()

=== modified file 'lib/lp/registry/scripts/tests/test_closeaccount.py'
--- lib/lp/registry/scripts/tests/test_closeaccount.py	2019-04-23 09:26:52 +0000
+++ lib/lp/registry/scripts/tests/test_closeaccount.py	2019-05-02 17:53:49 +0000
@@ -180,6 +180,15 @@
             script.logger.getLogBuffer())
         self.assertNotRemoved(account_id, person_id)
 
+    def test_dry_run(self):
+        person, person_id, account_id = self.makePopulatedUser()
+        script = self.makeScript(['--dry-run', six.ensure_str(person.name)])
+        with dbuser('launchpad'):
+            self.runScript(script)
+        self.assertIn(
+            'Dry run, so not committing changes', script.logger.getLogBuffer())
+        self.assertNotRemoved(account_id, person_id)
+
     def test_single_by_name(self):
         person, person_id, account_id = self.makePopulatedUser()
         script = self.makeScript([six.ensure_str(person.name)])