launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Approve



Diff comments:

> === added file 'lib/lp/registry/scripts/suspendbotaccount.py'
> --- lib/lp/registry/scripts/suspendbotaccount.py	1970-01-01 00:00:00 +0000
> +++ lib/lp/registry/scripts/suspendbotaccount.py	2018-07-12 06:15:20 +0000
> @@ -0,0 +1,44 @@
> +# Copyright 2018 Canonical Ltd.  This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Suspend a bot account."""
> +
> +from zope.component import getUtility
> +
> +from lp.registry.interfaces.person import IPersonSet
> +from lp.services.identity.interfaces.account import AccountStatus
> +from lp.services.scripts.base import (
> +    LaunchpadScript,
> +    LaunchpadScriptFailure,
> +    )
> +from lp.services.webapp import canonical_url
> +
> +
> +class SuspendBotAccountScript(LaunchpadScript):
> +
> +    description = "Suspend a bot account."
> +    output = None
> +
> +    def add_my_options(self):
> +        self.parser.add_option(
> +            '-e', '--email', metavar='ADDR', action='store',
> +            type='string', dest='email', default='',
> +            help='Email address. Defaults to webops+username@xxxxxxxxxxxxx')

I guess this was copied from CreateBotAccountScript; there's no such default here.

Do we know that the IS integration will find it convenient to supply an email address, or should we have an alternative --username option?

> +
> +    def main(self):
> +        emailaddress = unicode(self.options.email)
> +        if not emailaddress:
> +            raise LaunchpadScriptFailure('--email is required')
> +
> +        person = getUtility(IPersonSet).getByEmail(emailaddress)
> +        if person is None:
> +            raise LaunchpadScriptFailure(
> +                'Account with email address {} does not exist'.format(
> +                    emailaddress))
> +
> +        person.account.setStatus(
> +            AccountStatus.SUSPENDED, None,
> +            'Suspended by suspend-bot-account.py')
> +
> +        self.logger.info('Suspended {}'.format(canonical_url(person)))
> +        self.txn.commit()
> 
> === added file 'lib/lp/registry/scripts/tests/test_suspendbotaccount.py'
> --- lib/lp/registry/scripts/tests/test_suspendbotaccount.py	1970-01-01 00:00:00 +0000
> +++ lib/lp/registry/scripts/tests/test_suspendbotaccount.py	2018-07-12 06:15:20 +0000
> @@ -0,0 +1,33 @@
> +# Copyright 2017 Canonical Ltd.  This software is licensed under the

It's 2018.

> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Test the suspend-bot-account script."""
> +
> +__metaclass__ = type
> +
> +from lp.registry.scripts.suspendbotaccount import SuspendBotAccountScript
> +from lp.services.identity.interfaces.account import AccountStatus
> +from lp.services.log.logger import DevNullLogger
> +from lp.testing import TestCaseWithFactory
> +from lp.testing.faketransaction import FakeTransaction
> +from lp.testing.layers import ZopelessDatabaseLayer
> +
> +
> +class TestSuspendBotAccount(TestCaseWithFactory):
> +    """Test `suspend-bot-account`."""
> +
> +    layer = ZopelessDatabaseLayer
> +
> +    def makeScript(self, test_args):
> +        script = SuspendBotAccountScript(test_args=test_args)
> +        script.logger = DevNullLogger()
> +        script.txn = FakeTransaction()
> +        return script
> +
> +    def test_suspendbotaccount(self):
> +        bot = self.factory.makePerson(email='webops+bot@xxxxxxxxxxxxx')
> +        script = self.makeScript(['--email', 'webops+bot@xxxxxxxxxxxxx'])
> +        script.main()
> +        self.assertEqual(AccountStatus.SUSPENDED, bot.account_status)
> +
> +        self.assertEqual(1, script.txn.commit_count)


-- 
https://code.launchpad.net/~wgrant/launchpad/suspend-bot-account/+merge/349392
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.