[Pywikibot-commits] [Gerrit] pywikibot/core[master]: [IMPR] Rewrite Page.botMayEdit method

jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/599271 ) Change subject: [IMPR] Rewrite Page.botMayEdit method ...................................................................... [IMPR] Rewrite Page.botMayEdit method - introduce two new family methods get_edit_restricted_templates and get_archived_page_templates which return edit_restricted_templates and archived_page_templates for a given site.code. Return an empty tuple if no key is found. These methods are available as Site method too. - use the new methods by botMayEdit - never return True inside the botMayEdit loop because other restricting templates may follow and the result should not be depend to the order; continue looping instead and enable multiple bots/nobots templates to be used - show a warning if bots/nobots template has more than 1 parameter - show an error message if nobots has a named parameter and decline edit then - show a warning if bots has no named parameter or empty parameter values and continue looping - use subTest for tests and simplify them - split tests in 3 parts to avoid missing/wrong self.page._templates assignment Bug: T253709 Change-Id: I035ed06fe400e59b2239f73d3c1780ec3647212f --- M pywikibot/family.py M pywikibot/page/__init__.py M tests/page_tests.py 3 files changed, 196 insertions(+), 99 deletions(-) Approvals: Matěj Suchánek: Looks good to me, approved jenkins-bot: Verified diff --git a/pywikibot/family.py b/pywikibot/family.py index 9fca4b7..c1a4f9b 100644 --- a/pywikibot/family.py +++ b/pywikibot/family.py @@ -1108,6 +1108,14 @@ """DEPRECATED: Build list of category redirect templates.""" self._get_cr_templates(code, fallback) + def get_edit_restricted_templates(self, code): + """Return tuple of edit restricted templates.""" + return self.edit_restricted_templates.get(code, ()) + + def get_archived_page_templates(self, code): + """Return tuple of archived page templates.""" + return self.archived_page_templates.get(code, ()) + def disambig(self, code, fallback='_default'): """Return list of disambiguation templates.""" if code in self.disambiguationTemplates: diff --git a/pywikibot/page/__init__.py b/pywikibot/page/__init__.py index 9d46cc3..77dacef 100644 --- a/pywikibot/page/__init__.py +++ b/pywikibot/page/__init__.py @@ -1180,14 +1180,12 @@ @rtype: bool """ - # TODO: move this to Site object? - - # FIXME: templatesWithParams is defined in Page only. if not hasattr(self, 'templatesWithParams'): return True if config.ignore_bot_templates: # Check the "master ignore switch" return True + username = self.site.user() try: templates = self.templatesWithParams() @@ -1197,48 +1195,85 @@ return True # go through all templates and look for any restriction - # multiple bots/nobots templates are allowed - restrictions = self.site.family.edit_restricted_templates.get( - self.site.code) + restrictions = set(self.site.get_edit_restricted_templates()) + # also add archive templates for non-archive bots if pywikibot.calledModuleName() != 'archivebot': - archived = self.site.family.archived_page_templates.get( - self.site.code) - if restrictions and archived: - restrictions += archived - elif archived: - restrictions = archived + restrictions.update(self.site.get_archived_page_templates()) + # multiple bots/nobots templates are allowed for template, params in templates: title = template.title(with_ns=False) - if restrictions: - if title in restrictions: - return False + + if title in restrictions: + return False + + if title not in ('Bots', 'Nobots'): + continue + + try: + key, sep, value = params[0].partition('=') + except IndexError: + key, sep, value = '', '', '' + names = set() + else: + if not sep: + key, value = value, key + key = key.strip() + names = {name.strip() for name in value.split(',')} + + if len(params) > 1: + pywikibot.warning( + '{{%s|%s}} has more than 1 parameter; taking the first.' + % (title.lower(), '|'.join(params))) + if title == 'Nobots': if not params: return False - else: - bots = [bot.strip() for bot in params[0].split(',')] - if 'all' in bots or pywikibot.calledModuleName() in bots \ - or username in bots: - return False - elif title == 'Bots': - if not params: - return True - else: - (ttype, bots) = [part.strip() for part - in params[0].split('=', 1)] - bots = [bot.strip() for bot in bots.split(',')] - if ttype == 'allow': - return 'all' in bots or username in bots - if ttype == 'deny': - return not ('all' in bots or username in bots) - if ttype == 'allowscript': - return ('all' in bots - or pywikibot.calledModuleName() in bots) - if ttype == 'denyscript': - return not ('all' in bots - or pywikibot.calledModuleName() in bots) + + if key: + pywikibot.error( + '%s parameter for {{nobots}} is not allowed. ' + 'Edit declined' % key) + return False + + if 'all' in names \ + or pywikibot.calledModuleName() in names \ + or username in names: + return False + + if title == 'Bots': + if value and not key: + pywikibot.warning( + '{{bots|%s}} is not valid. Ignoring.' % value) + continue + + if key and not value: + pywikibot.warning( + '{{bots|%s=}} is not valid. Ignoring.' % key) + continue + + if key == 'allow' and not ('all' in names + or username in names): + return False + + if key == 'deny' and ('all' in names or username in names): + return False + + if key == 'allowscript' \ + and not ('all' in names + or pywikibot.calledModuleName() in names): + return False + + if key == 'denyscript' \ + and ('all' in names + or pywikibot.calledModuleName() in names): + return False + + if key: # ignore unrecognized keys with a warning + pywikibot.warning( + '{{bots|%s}} is not valid. Ignoring.' % params[0]) + # no restricting template found return True diff --git a/tests/page_tests.py b/tests/page_tests.py index 90a4323..09eb93c 100644 --- a/tests/page_tests.py +++ b/tests/page_tests.py @@ -793,98 +793,152 @@ cached = True user = True - @mock.patch.object(config, 'ignore_bot_templates', False) - def test_bot_may_edit_general(self): - """Test that bot is allowed to edit.""" - site = self.get_site() - user = site.user() - - page = pywikibot.Page(site, 'not_existent_page_for_pywikibot_tests') - if page.exists(): + def setUp(self): + """Setup test.""" + super(TestPageBotMayEdit, self).setUp() + self.page = pywikibot.Page(self.site, + 'not_existent_page_for_pywikibot_tests') + if self.page.exists(): self.skipTest( 'Page {} exists! Change page name in tests/page_tests.py' - .format(page.title())) + .format(self.page.title())) + + @mock.patch.object(config, 'ignore_bot_templates', False) + def test_bot_may_edit_nobots(self): + """Test with {{nobots}} that bot is allowed to edit.""" + self.page._templates = [pywikibot.Page(self.site, 'Template:Nobots')] + user = self.site.user() # Ban all compliant bots (shortcut). - page.text = '{{nobots}}' - page._templates = [pywikibot.Page(site, 'Template:Nobots')] - self.assertFalse(page.botMayEdit()) + self.page.text = '{{nobots}}' + with self.subTest(template=self.page.text, user=user): + self.assertFalse(self.page.botMayEdit()) # Ban all compliant bots not in the list, syntax for de wp. - page.text = '{{nobots|HagermanBot,Werdnabot}}' - self.assertTrue(page.botMayEdit(), - '{}: {} but user={}' - .format(page.text, page.botMayEdit(), user)) + self.page.text = '{{nobots|HagermanBot,Werdnabot}}' + with self.subTest(template=self.page.text, user=user): + self.assertTrue(self.page.botMayEdit()) # Ban all compliant bots not in the list, syntax for de wp. - page.text = '{{nobots|%s, HagermanBot,Werdnabot}}' % user - self.assertFalse(page.botMayEdit(), - '{}: {} but user={}' - .format(page.text, page.botMayEdit(), user)) + self.page.text = '{{nobots|%s, HagermanBot,Werdnabot}}' % user + with self.subTest(template=self.page.text, user=user): + self.assertFalse(self.page.botMayEdit()) # Ban all bots, syntax for de wp. - page.text = '{{nobots|all}}' - self.assertFalse(page.botMayEdit(), - '{}: {} but user={}' - .format(page.text, page.botMayEdit(), user)) + self.page.text = '{{nobots|all}}' + with self.subTest(template=self.page.text, user=user): + self.assertFalse(self.page.botMayEdit()) + + # Decline wrong nobots parameter + self.page.text = '{{nobots|allow=%s}}' % user + with self.subTest(template=self.page.text, user=user): + self.assertFalse(self.page.botMayEdit()) + + # Decline wrong nobots parameter + self.page.text = '{{nobots|deny=%s}}' % user + with self.subTest(template=self.page.text, user=user): + self.assertFalse(self.page.botMayEdit()) + + # Decline wrong nobots parameter + self.page.text = '{{nobots|decline=%s}}' % user + with self.subTest(template=self.page.text, user=user): + self.assertFalse(self.page.botMayEdit()) + + # Decline empty keyword parameter with nobots + self.page.text = '{{nobots|with_empty_parameter=}}' + with self.subTest(template=self.page.text, user=user): + self.assertFalse(self.page.botMayEdit()) + + # Ignore second parameter + self.page.text = '{{nobots|%s|MyBot}}' % user + with self.subTest(template=self.page.text, user=user): + self.assertFalse(self.page.botMayEdit()) + + # Ignore second parameter + self.page.text = '{{nobots|MyBot|%s}}' % user + with self.subTest(template=self.page.text, user=user): + self.assertTrue(self.page.botMayEdit()) + + @mock.patch.object(config, 'ignore_bot_templates', False) + def test_bot_may_edit_bots(self): + """Test with {{bots}} that bot is allowed to edit.""" + self.page._templates = [pywikibot.Page(self.site, 'Template:Bots')] + user = self.site.user() # Allow all bots (shortcut). - page.text = '{{bots}}' - page._templates = [pywikibot.Page(site, 'Template:Bots')] - self.assertTrue(page.botMayEdit()) + self.page.text = '{{bots}}' + with self.subTest(template=self.page.text, user=user): + self.assertTrue(self.page.botMayEdit()) # Ban all compliant bots not in the list. - page.text = '{{bots|allow=HagermanBot,Werdnabot}}' - self.assertFalse(page.botMayEdit(), - '{}: {} but user={}' - .format(page.text, page.botMayEdit(), user)) + self.page.text = '{{bots|allow=HagermanBot,Werdnabot}}' + with self.subTest(template=self.page.text, user=user): + self.assertFalse(self.page.botMayEdit()) # Ban all compliant bots in the list. - page.text = '{{bots|deny=HagermanBot,Werdnabot}}' - self.assertTrue(page.botMayEdit(), - '{}: {} but user={}' - .format(page.text, page.botMayEdit(), user)) + self.page.text = '{{bots|deny=HagermanBot,Werdnabot}}' + with self.subTest(template=self.page.text, user=user): + self.assertTrue(self.page.botMayEdit()) # Ban all compliant bots not in the list. - page.text = '{{bots|allow=%s, HagermanBot}}' % user - self.assertTrue(page.botMayEdit(), - '{}: {} but user={}' - .format(page.text, page.botMayEdit(), user)) + self.page.text = '{{bots|allow=%s, HagermanBot}}' % user + with self.subTest(template=self.page.text, user=user): + self.assertTrue(self.page.botMayEdit()) # Ban all compliant bots in the list. - page.text = '{{bots|deny=%s, HagermanBot}}' % user - self.assertFalse(page.botMayEdit(), - '{}: {} but user={}' - .format(page.text, page.botMayEdit(), user)) + self.page.text = '{{bots|deny=%s, HagermanBot}}' % user + with self.subTest(template=self.page.text, user=user): + self.assertFalse(self.page.botMayEdit()) # Allow all bots. - page.text = '{{bots|allow=all}}' - self.assertTrue(page.botMayEdit(), - '{}: {} but user={}' - .format(page.text, page.botMayEdit(), user)) + self.page.text = '{{bots|allow=all}}' + with self.subTest(template=self.page.text, user=user): + self.assertTrue(self.page.botMayEdit()) # Ban all compliant bots. - page.text = '{{bots|allow=none}}' - self.assertFalse(page.botMayEdit(), - '{}: {} but user={}' - .format(page.text, page.botMayEdit(), user)) + self.page.text = '{{bots|allow=none}}' + with self.subTest(template=self.page.text, user=user): + self.assertFalse(self.page.botMayEdit()) # Ban all compliant bots. - page.text = '{{bots|deny=all}}' - self.assertFalse(page.botMayEdit(), - '{}: {} but user={}' - .format(page.text, page.botMayEdit(), user)) + self.page.text = '{{bots|deny=all}}' + with self.subTest(template=self.page.text, user=user): + self.assertFalse(self.page.botMayEdit()) # Allow all bots. - page.text = '{{bots|deny=none}}' - self.assertTrue(page.botMayEdit(), - '{}: {} but user={}' - .format(page.text, page.botMayEdit(), user)) + self.page.text = '{{bots|deny=none}}' + with self.subTest(template=self.page.text, user=user): + self.assertTrue(self.page.botMayEdit()) + + # Ignore missing named parameter. + self.page.text = '{{bots|%s}}' % user + with self.subTest(template=self.page.text, user=user): + self.assertTrue(self.page.botMayEdit()) + + # Ignore empty keyword parameter with bots + for param in ('allow', 'deny', 'empty_parameter'): + self.page.text = '{{bots|%s=}}' % param + with self.subTest(template=self.page.text, user=user, param=param): + self.assertTrue(self.page.botMayEdit()) + + # Ignore unknown keyword parameter with bots + self.page.text = '{{bots|with=unknown_parameter}}' + with self.subTest(template=self.page.text, user=user): + self.assertTrue(self.page.botMayEdit()) + + # Ignore unknown empty parameter keyword with bots + self.page.text = '{{bots|with_empty_parameter=}}' + with self.subTest(template=self.page.text, user=user): + self.assertTrue(self.page.botMayEdit()) + + @mock.patch.object(config, 'ignore_bot_templates', False) + def test_bot_may_edit_inuse(self): + """Test with {{inuse}} that bot is allowed to edit.""" + self.page._templates = [pywikibot.Page(self.site, 'Template:In use')] # Ban all users including bots. - page.text = '{{in use}}' - page._templates = [pywikibot.Page(site, 'Template:In use')] - self.assertFalse(page.botMayEdit()) + self.page.text = '{{in use}}' + self.assertFalse(self.page.botMayEdit()) class TestPageHistory(DefaultSiteTestCase): -- To view, visit https://gerrit.wikimedia.org/r/599271 To unsubscribe, or for help writing mail filters, visit https://gerrit.wikimedia.org/r/settings Gerrit-Project: pywikibot/core Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I035ed06fe400e59b2239f73d3c1780ec3647212f Gerrit-Change-Number: 599271 Gerrit-PatchSet: 15 Gerrit-Owner: Xqt <info(a)gno.de> Gerrit-Reviewer: Dvorapa <dvorapa(a)seznam.cz> Gerrit-Reviewer: JJMC89 <JJMC89.Wikimedia(a)gmail.com> Gerrit-Reviewer: Matěj Suchánek <matejsuchanek97(a)gmail.com> Gerrit-Reviewer: Russell Blau <russblau(a)imapmail.org> Gerrit-Reviewer: Xqt <info(a)gno.de> Gerrit-Reviewer: jenkins-bot (75)