diff options
Diffstat (limited to 'src/mailman')
| -rw-r--r-- | src/mailman/chains/docs/moderation.rst | 35 | ||||
| -rw-r--r-- | src/mailman/database/alembic/versions/7b254d88f122_members_and_list_moderation_action.py | 28 | ||||
| -rw-r--r-- | src/mailman/database/tests/test_migrations.py | 41 | ||||
| -rw-r--r-- | src/mailman/docs/NEWS.rst | 4 | ||||
| -rw-r--r-- | src/mailman/model/member.py | 2 | ||||
| -rw-r--r-- | src/mailman/rest/docs/membership.rst | 8 | ||||
| -rw-r--r-- | src/mailman/rest/members.py | 2 | ||||
| -rw-r--r-- | src/mailman/rules/docs/moderation.rst | 41 | ||||
| -rw-r--r-- | src/mailman/rules/moderation.py | 10 | ||||
| -rw-r--r-- | src/mailman/rules/tests/test_moderation.py | 26 | ||||
| -rw-r--r-- | src/mailman/utilities/importer.py | 2 | ||||
| -rw-r--r-- | src/mailman/utilities/tests/test_import.py | 2 |
12 files changed, 122 insertions, 79 deletions
diff --git a/src/mailman/chains/docs/moderation.rst b/src/mailman/chains/docs/moderation.rst index 2415b8f4b..880f9b63c 100644 --- a/src/mailman/chains/docs/moderation.rst +++ b/src/mailman/chains/docs/moderation.rst @@ -14,10 +14,14 @@ such as seeing if the message has a matching `Approved:` header, or if the emergency flag has been set on the mailing list, or whether a mail loop has been detected. -After those, the moderation action for the sender is checked. Members -generally have a `defer` action, meaning the normal moderation checks are -done, but it is also common for first-time posters to have a `hold` action, -meaning that their messages are held for moderator approval for a while. +Mailing lists have a default moderation action, one for members and another +for nonmembers. If a member's moderation action is ``None``, then the member +moderation check falls back to the appropriate list default. + +A moderation action of `defer` means that no explicit moderation check is +performed and the rest of the rule chain processing proceeds as normal. But +it is also common for first-time posters to have a `hold` action, meaning that +their messages are held for moderator approval for a while. Nonmembers almost always have a `hold` action, though some mailing lists may choose to set this default action to `discard`, meaning their posts would be @@ -31,14 +35,12 @@ Posts by list members are moderated if the member's moderation action is not deferred. The default setting for the moderation action of new members is determined by the mailing list's settings. By default, a mailing list is not set to moderate new member postings. -:: >>> print(mlist.default_member_action) Action.defer In order to find out whether the message is held or accepted, we can subscribe -to Zope events that are triggered on each case. -:: +to internal events that are triggered on each case. >>> from mailman.interfaces.chain import ChainEvent >>> def on_chain(event): @@ -53,17 +55,16 @@ to Zope events that are triggered on each case. ... for miss in event.msgdata.get('rule_misses', []): ... print(' ', miss) -The user Anne will be a list member. A list member's default moderation action -is ``None``, meaning that it will use the mailing list's -``default_member_action``. +Anne is a list member with moderation action of ``None`` so that moderation +will fall back to the mailing list's ``default_member_action``. >>> from mailman.testing.helpers import subscribe >>> member = subscribe(mlist, 'Anne', email='anne@example.com') >>> member <Member: Anne Person <anne@example.com> on test@example.com as MemberRole.member> - >>> print(member.moderation_action is None) - True + >>> print(member.moderation_action) + None Anne's post to the mailing list runs through the incoming runner's default built-in chain. No rules hit and so the message is accepted. @@ -126,7 +127,7 @@ moderator approval. emergency loop -The list's member moderation action can also be set to `discard`... +Anne's moderation action can also be set to `discard`... :: >>> member.moderation_action = Action.discard @@ -179,9 +180,9 @@ The list's member moderation action can also be set to `discard`... Nonmembers ========== -Registered nonmembers are handled very similarly to members, the main -difference being that the mailing list's default moderation action is -different. This is how the incoming runner adds sender addresses as nonmembers. +Registered nonmembers are handled very similarly to members, except that a +different list default setting is used when moderating nonmemberds. This is +how the incoming runner adds sender addresses as nonmembers. >>> from zope.component import getUtility >>> from mailman.interfaces.usermanager import IUserManager @@ -219,7 +220,7 @@ moderator approval. >>> nonmember <Member: bart@example.com on test@example.com as MemberRole.nonmember> -A nonmember's default moderation action is ``None``, meaning that it will use +When a nonmember's default moderation action is ``None``, the rule will use the mailing list's ``default_nonmember_action``. >>> print(nonmember.moderation_action) diff --git a/src/mailman/database/alembic/versions/7b254d88f122_members_and_list_moderation_action.py b/src/mailman/database/alembic/versions/7b254d88f122_members_and_list_moderation_action.py index 09aedc640..c393528d9 100644 --- a/src/mailman/database/alembic/versions/7b254d88f122_members_and_list_moderation_action.py +++ b/src/mailman/database/alembic/versions/7b254d88f122_members_and_list_moderation_action.py @@ -1,13 +1,12 @@ -"""Members and list moderation action +"""Members and list moderation action. Revision ID: 7b254d88f122 Revises: d4fbb4fd34ca Create Date: 2016-02-10 11:31:04.233619 -This is a data-only migration. If a member has the same moderation action as +This is a data-only migration. If a member has the same moderation action as the mailing list's default, then set its moderation action to None and use the fallback to the list's default. - """ @@ -19,7 +18,7 @@ from mailman.interfaces.action import Action from mailman.interfaces.member import MemberRole -# revision identifiers, used by Alembic. +# Revision identifiers, used by Alembic. revision = '7b254d88f122' down_revision = 'd4fbb4fd34ca' @@ -49,21 +48,24 @@ members_query = member_table.select().where(sa.or_( )) +# list-id -> {property-name -> action} +# +# where property-name will be either default_member_action or +# default_nonmember_action. DEFAULT_ACTION_CACHE = {} +MISSING = object() def _get_default_action(connection, member): list_id = member['list_id'] - propname = 'default_{}_action'.format(member['role'].name) - try: - action = DEFAULT_ACTION_CACHE[list_id][propname] - except KeyError: + property_name = 'default_{}_action'.format(member['role'].name) + list_mapping = DEFAULT_ACTION_CACHE.setdefault(list_id, {}) + action = list_mapping.get(property_name, MISSING) + if action is MISSING: mailing_list = connection.execute(mailinglist_table.select().where( mailinglist_table.c.list_id == list_id)).fetchone() - action = mailing_list[propname] - if list_id not in DEFAULT_ACTION_CACHE: - DEFAULT_ACTION_CACHE[list_id] = {} - DEFAULT_ACTION_CACHE[list_id][propname] = action + action = mailing_list[property_name] + list_mapping[property_name] = action return action @@ -72,7 +74,7 @@ def upgrade(): for member in connection.execute(members_query).fetchall(): default_action = _get_default_action(connection, member) # If the (non)member's moderation action is the same as the mailing - # list's default, then set it to None. The moderation rule will + # list's default, then set it to None. The moderation rule will # fallback to the list's default. if member['moderation_action'] == default_action: connection.execute(member_table.update().where( diff --git a/src/mailman/database/tests/test_migrations.py b/src/mailman/database/tests/test_migrations.py index bc394149b..e1ab3f90f 100644 --- a/src/mailman/database/tests/test_migrations.py +++ b/src/mailman/database/tests/test_migrations.py @@ -19,8 +19,8 @@ import os import unittest -import alembic.command import sqlalchemy as sa +import alembic.command from mailman.app.lifecycle import create_list from mailman.config import config @@ -28,7 +28,12 @@ from mailman.database.alembic import alembic_cfg from mailman.database.helpers import exists_in_db from mailman.database.model import Model from mailman.database.transaction import transaction +from mailman.database.types import Enum +from mailman.interfaces.action import Action +from mailman.interfaces.member import MemberRole +from mailman.interfaces.usermanager import IUserManager from mailman.testing.layers import ConfigLayer +from zope.component import getUtility class TestMigrations(unittest.TestCase): @@ -123,7 +128,9 @@ class TestMigrations(unittest.TestCase): sa.sql.column('value', sa.Unicode), sa.sql.column('pended_id', sa.Integer), ) - def get_from_db(): # flake8: noqa + + # https://github.com/PyCQA/pycodestyle/issues/28 + def get_from_db(): # noqa results = {} for i in range(1, 6): query = sa.sql.select( @@ -220,19 +227,13 @@ class TestMigrations(unittest.TestCase): self.assertTrue(os.path.exists(bee.data_path)) def test_7b254d88f122_moderation_action(self): - from mailman.database.types import Enum - from mailman.interfaces.action import Action - from mailman.interfaces.member import MemberRole - from mailman.interfaces.usermanager import IUserManager - from zope.component import getUtility - mailinglist_table = sa.sql.table( + mailinglist_table = sa.sql.table( # noqa 'mailinglist', sa.sql.column('id', sa.Integer), sa.sql.column('list_id', sa.Unicode), sa.sql.column('default_member_action', Enum(Action)), sa.sql.column('default_nonmember_action', Enum(Action)), ) - member_table = sa.sql.table( 'member', sa.sql.column('id', sa.Integer), @@ -243,16 +244,18 @@ class TestMigrations(unittest.TestCase): ) user_manager = getUtility(IUserManager) with transaction(): - # Start at the previous revision + # Start at the previous revision. alembic.command.downgrade(alembic_cfg, 'd4fbb4fd34ca') # Create a mailing list through the standard API. ant = create_list('ant@example.com') - # Create members + # Create some members. anne = user_manager.create_address('anne@example.com') bart = user_manager.create_address('bart@example.com') cris = user_manager.create_address('cris@example.com') dana = user_manager.create_address('dana@example.com') - config.db.store.flush() # to get the last auto-increment id. + # Flush the database to get the last auto-increment id. + config.db.store.flush() + # Assign some moderation actions to the members created above. config.db.store.execute(member_table.insert().values([ {'address_id': anne.id, 'role': MemberRole.owner, 'list_id': ant.list_id, 'moderation_action': Action.accept}, @@ -263,7 +266,16 @@ class TestMigrations(unittest.TestCase): {'address_id': dana.id, 'role': MemberRole.nonmember, 'list_id': ant.list_id, 'moderation_action': Action.hold}, ])) - # Upgrade and check the moderation_action. + # Cris and Dana have actions which match the list default action for + # members and nonmembers respectively. + self.assertEqual( + ant.members.get_member('cris@example.com').moderation_action, + ant.default_member_action) + self.assertEqual( + ant.nonmembers.get_member('dana@example.com').moderation_action, + ant.default_nonmember_action) + # Upgrade and check the moderation_actions. Cris's and Dana's + # actions have been set to None to fall back to the list defaults. alembic.command.upgrade(alembic_cfg, '7b254d88f122') members = config.db.store.execute(sa.select([ member_table.c.address_id, member_table.c.moderation_action, @@ -274,7 +286,8 @@ class TestMigrations(unittest.TestCase): (cris.id, None), (dana.id, None), ]) - # Downgrade and check. + # Downgrade and check that Cris's and Dana's actions have been set + # explicitly. alembic.command.downgrade(alembic_cfg, 'd4fbb4fd34ca') members = config.db.store.execute(sa.select([ member_table.c.address_id, member_table.c.moderation_action, diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 95e01019e..301221ce1 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -126,6 +126,10 @@ Message handling non-member posts instead of always holding them for moderator review. Given by Aurélien Bompard. (Closes #163) * Bounces can now contain rejection messages. Given by Aurélien Bompard. + * The `moderation_action` for members and nonmember can now be ``None`` which + signals falling back to the appropriate list default action, + e.g. `default_member_action` and `default_nonmember_action`. Given by + Aurélien Bompard. (Closes #189) REST ---- diff --git a/src/mailman/model/member.py b/src/mailman/model/member.py index a6555c0fe..26c36b305 100644 --- a/src/mailman/model/member.py +++ b/src/mailman/model/member.py @@ -78,7 +78,7 @@ class Member(Model): self.moderation_action = Action.accept else: assert role in (MemberRole.member, MemberRole.nonmember), ( - 'Invalid MemberRole: {0}'.format(role)) + 'Invalid MemberRole: {}'.format(role)) self.moderation_action = None def __repr__(self): diff --git a/src/mailman/rest/docs/membership.rst b/src/mailman/rest/docs/membership.rst index ec8765d68..24065ba07 100644 --- a/src/mailman/rest/docs/membership.rst +++ b/src/mailman/rest/docs/membership.rst @@ -926,8 +926,8 @@ Moderating a member =================== The moderation action for a member can be changed by PATCH'ing the -`moderation_action` attribute. -:: +`moderation_action` attribute. When the member action falls back to the list +default, there is no such attribute in the resource. >>> dump_json('http://localhost:9001/3.0/members/10') address: http://localhost:9001/3.0/addresses/hperson@example.com @@ -940,6 +940,10 @@ The moderation action for a member can be changed by PATCH'ing the self_link: http://localhost:9001/3.0/members/10 user: http://localhost:9001/3.0/users/7 +Patching the moderation action both changes it for the given user, and adds +the attribute to the member's resource. +:: + >>> dump_json('http://localhost:9001/3.0/members/10', { ... 'moderation_action': 'hold', ... }, method='PATCH') diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py index c842ccd7d..b9461fdaa 100644 --- a/src/mailman/rest/members.py +++ b/src/mailman/rest/members.py @@ -64,7 +64,7 @@ class _MemberBase(CollectionMixin): role=role, self_link=self.api.path_to('members/{}'.format(member_id)), ) - # Add the moderation action if there is one. + # Add the moderation action if overriding the list's default. if member.moderation_action is not None: response['moderation_action'] = member.moderation_action # Add the user link if there is one. diff --git a/src/mailman/rules/docs/moderation.rst b/src/mailman/rules/docs/moderation.rst index 56602189b..d1cc5fd67 100644 --- a/src/mailman/rules/docs/moderation.rst +++ b/src/mailman/rules/docs/moderation.rst @@ -3,9 +3,9 @@ Moderation ========== All members and nonmembers have a moderation action, which defaults to the -list's default action. When the action is not `defer`, the `moderation` rule -flags the message as needing moderation. This might be to automatically -accept, discard, reject, or hold the message. +appropriate list's default action. When the action is not `defer`, the +`moderation` rule flags the message as needing moderation. This might be to +automatically accept, discard, reject, or hold the message. Two separate rules check for member and nonmember moderation. Member moderation happens early in the built-in chain, while nonmember moderation @@ -22,15 +22,19 @@ Member moderation member-moderation Anne, a mailing list member, sends a message to the mailing list. Her -postings are not moderated. -:: +moderation action is not set. >>> from mailman.testing.helpers import subscribe - >>> subscribe(mlist, 'Anne') + >>> member = subscribe(mlist, 'Anne') + >>> member <Member: Anne Person <aperson@example.com> on test@example.com as MemberRole.member> + >>> print(member.moderation_action) + None + +Because the list's default member action is set to `defer`, Anne's posting is +not moderated. - >>> member = mlist.members.get_member('aperson@example.com') >>> print(mlist.default_member_action) Action.defer @@ -46,8 +50,9 @@ Because Anne is not moderated, the member moderation rule does not match. False Once the member's moderation action is set to something other than `defer` or -``None``, the rule matches. Also, the message metadata has a few extra pieces -of information for the eventual moderation chain. +``None`` (given the list's current default member moderation action), the rule +matches. Also, the message metadata has a few extra pieces of information for +the eventual moderation chain. >>> from mailman.interfaces.action import Action >>> member.moderation_action = Action.hold @@ -71,20 +76,26 @@ postings are held for moderator approval. nonmember-moderation Bart, who is not a member of the mailing list, sends a message to the list. -:: +He has no explicit nonmember moderation action. >>> from mailman.interfaces.member import MemberRole - >>> subscribe(mlist, 'Bart', MemberRole.nonmember) + >>> nonmember = subscribe(mlist, 'Bart', MemberRole.nonmember) + >>> nonmember <Member: Bart Person <bperson@example.com> on test@example.com as MemberRole.nonmember> + >>> print(nonmember.moderation_action) + None + +The list's default nonmember moderation action is to hold postings by +nonmembers. - >>> nonmember = mlist.nonmembers.get_member('bperson@example.com') >>> print(mlist.default_nonmember_action) Action.hold -When Bart is registered as a nonmember of the list, his moderation action is -set to hold by default. Thus the rule matches and the message metadata again -carries some useful information. +Since Bart is registered as a nonmember of the list, and his moderation action +is set to None, the action falls back to the list's default nonmember +moderation action, which is to hold the post for moderator approval. Thus the +rule matches and the message metadata again carries some useful information. >>> nonmember_msg = message_from_string("""\ ... From: bperson@example.com diff --git a/src/mailman/rules/moderation.py b/src/mailman/rules/moderation.py index 9c89cf0b2..d3dc07a8f 100644 --- a/src/mailman/rules/moderation.py +++ b/src/mailman/rules/moderation.py @@ -44,8 +44,9 @@ class MemberModeration: member = mlist.members.get_member(sender) if member is None: return False - action = (member.moderation_action - or mlist.default_member_action) + action = (mlist.default_member_action + if member.moderation_action is None + else member.moderation_action) if action is Action.defer: # The regular moderation rules apply. return False @@ -114,8 +115,9 @@ class NonmemberModeration: _record_action(msgdata, action, sender, reason.format(action)) return True - action = (nonmember.moderation_action - or mlist.default_nonmember_action) + action = (mlist.default_nonmember_action + if nonmember.moderation_action is None + else nonmember.moderation_action) if action is Action.defer: # The regular moderation rules apply. return False diff --git a/src/mailman/rules/tests/test_moderation.py b/src/mailman/rules/tests/test_moderation.py index 3e80aeeb5..05bae5e1b 100644 --- a/src/mailman/rules/tests/test_moderation.py +++ b/src/mailman/rules/tests/test_moderation.py @@ -160,15 +160,19 @@ MIME-Version: 1.0 A message body. """) - # First, the message should get held + # First, the message should get held. msgdata = {} result = rule.check(self._mlist, msg, msgdata) self.assertTrue(result) - self.assertEqual(msgdata.get('moderation_action'), 'hold') - # Then the list's default nonmember action is changed + self.assertEqual(msgdata['moderation_action'], 'hold') + # As a side-effect, Anne has been added as a nonmember with a + # moderation action that falls back to the list's default. + anne = self._mlist.nonmembers.get_member('anne@example.com') + self.assertIsNone(anne.moderation_action) + # Then the list's default nonmember action is changed. self._mlist.default_nonmember_action = Action.discard - msg.replace_header('Message-ID', '<ant2>') - # This time, the message should be discarded + msg.replace_header('Message-ID', '<bee>') + # This time, the message should be discarded. result = rule.check(self._mlist, msg, msgdata) self.assertTrue(result) self.assertEqual(msgdata.get('moderation_action'), 'discard') @@ -178,7 +182,9 @@ A message body. self._mlist.default_member_action = Action.accept user_manager = getUtility(IUserManager) anne = user_manager.create_address('anne@example.com') - self._mlist.subscribe(anne, MemberRole.member) + member = self._mlist.subscribe(anne, MemberRole.member) + # Anne's moderation rule falls back to the list default. + self.assertIsNone(member.moderation_action) rule = moderation.MemberModeration() msg = mfs("""\ From: anne@example.com @@ -189,15 +195,15 @@ MIME-Version: 1.0 A message body. """) - # First, the message should get held + # First, the message gets accepted. msgdata = {} result = rule.check(self._mlist, msg, msgdata) self.assertTrue(result) self.assertEqual(msgdata.get('moderation_action'), 'accept') - # Then the list's default nonmember action is changed + # Then the list's default member action is changed. self._mlist.default_member_action = Action.hold - msg.replace_header('Message-ID', '<ant2>') - # This time, the message should be discarded + msg.replace_header('Message-ID', '<bee>') + # This time, the message is held. result = rule.check(self._mlist, msg, msgdata) self.assertTrue(result) self.assertEqual(msgdata.get('moderation_action'), 'hold') diff --git a/src/mailman/utilities/importer.py b/src/mailman/utilities/importer.py index 0d511b2c4..1f350205f 100644 --- a/src/mailman/utilities/importer.py +++ b/src/mailman/utilities/importer.py @@ -575,7 +575,7 @@ def import_roster(mlist, config_dict, members, role, action=None): # The member is moderated. Check the member_moderation_action # option to know which action should be taken. action = member_moderation_action_mapping( - config_dict.get("member_moderation_action")) + config_dict.get('member_moderation_action')) else: # Member is not moderated: defer is the best option, as # discussed on merge request 100. diff --git a/src/mailman/utilities/tests/test_import.py b/src/mailman/utilities/tests/test_import.py index f55debde5..b2d48891b 100644 --- a/src/mailman/utilities/tests/test_import.py +++ b/src/mailman/utilities/tests/test_import.py @@ -1112,7 +1112,7 @@ class TestPreferencesImport(unittest.TestCase): self._do_test(128, dict(moderation_action=Action.discard)) def test_no_moderate(self): - # If option flag Moderate is not set, action is defer. + # If the option flag Moderate is not set, the action is defer. # See: https://gitlab.com/mailman/mailman/merge_requests/100 self._pckdict['member_moderation_action'] = 1 # reject self._do_test(0, dict(moderation_action=Action.defer)) |
