jenkins-bot merged this change.

View Change

Approvals: Matěj Suchánek: Looks good to me, approved jenkins-bot: Verified
[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(-)

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 change 599271. To unsubscribe, or for help writing mail filters, visit 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@gno.de>
Gerrit-Reviewer: Dvorapa <dvorapa@seznam.cz>
Gerrit-Reviewer: JJMC89 <JJMC89.Wikimedia@gmail.com>
Gerrit-Reviewer: Matěj Suchánek <matejsuchanek97@gmail.com>
Gerrit-Reviewer: Russell Blau <russblau@imapmail.org>
Gerrit-Reviewer: Xqt <info@gno.de>
Gerrit-Reviewer: jenkins-bot (75)