Members and nonmembers moderation action fallback

Members and nonmember's moderation action should be None by default, and
in that case the moderation rule should fallback to the mailing list's
default action.

Fixes: #189

3 files changed, 72 insertions, 11 deletions

diff --git a/src/mailman/rules/docs/moderation.rst b/src/mailman/rules/docs/moderation.rst
index f5ceec29a..56602189b 100644
--- a/src/mailman/rules/docs/moderation.rst
+++ b/src/mailman/rules/docs/moderation.rst
@@ -2,9 +2,10 @@
 Moderation
 ==========
 
-All members and nonmembers have a moderation 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.
+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.
 
 Two separate rules check for member and nonmember moderation.  Member
 moderation happens early in the built-in chain, while nonmember moderation
@@ -30,7 +31,7 @@ postings are not moderated.
              as MemberRole.member>
 
     >>> member = mlist.members.get_member('aperson@example.com')
-    >>> print(member.moderation_action)
+    >>> print(mlist.default_member_action)
     Action.defer
 
 Because Anne is not moderated, the member moderation rule does not match.
@@ -44,9 +45,9 @@ Because Anne is not moderated, the member moderation rule does not match.
     >>> member_rule.check(mlist, member_msg, {})
     False
 
-Once the member's moderation action is set to something other than `defer`,
-the rule matches.  Also, the message metadata has a few extra pieces of
-information for the eventual moderation chain.
+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.
 
     >>> from mailman.interfaces.action import Action
     >>> member.moderation_action = Action.hold
@@ -78,7 +79,7 @@ Bart, who is not a member of the mailing list, sends a message to the list.
              as MemberRole.nonmember>
 
     >>> nonmember = mlist.nonmembers.get_member('bperson@example.com')
-    >>> print(nonmember.moderation_action)
+    >>> print(mlist.default_nonmember_action)
     Action.hold
 
 When Bart is registered as a nonmember of the list, his moderation action is
diff --git a/src/mailman/rules/moderation.py b/src/mailman/rules/moderation.py
index fdecd1d5f..9c89cf0b2 100644
--- a/src/mailman/rules/moderation.py
+++ b/src/mailman/rules/moderation.py
@@ -42,8 +42,10 @@ class MemberModeration:
         """See `IRule`."""
         for sender in msg.senders:
             member = mlist.members.get_member(sender)
-            action = (None if member is None
-                      else member.moderation_action)
+            if member is None:
+                return False
+            action = (member.moderation_action
+                      or mlist.default_member_action)
             if action is Action.defer:
                 # The regular moderation rules apply.
                 return False
@@ -112,7 +114,8 @@ class NonmemberModeration:
                         _record_action(msgdata, action, sender,
                                        reason.format(action))
                         return True
-            action = nonmember.moderation_action
+            action = (nonmember.moderation_action
+                      or mlist.default_nonmember_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 df1658792..3e80aeeb5 100644
--- a/src/mailman/rules/tests/test_moderation.py
+++ b/src/mailman/rules/tests/test_moderation.py
@@ -144,3 +144,60 @@ A message body.
             self.assertEqual(
                 msgdata['moderation_action'], action_name,
                 'Wrong action for {}: {}'.format(address, action_name))
+
+    def test_nonmember_fallback_to_list_defaults(self):
+        # https://gitlab.com/mailman/mailman/issues/189
+        self._mlist.default_nonmember_action = Action.hold
+        user_manager = getUtility(IUserManager)
+        user_manager.create_address('anne@example.com')
+        rule = moderation.NonmemberModeration()
+        msg = mfs("""\
+From: anne@example.com
+To: test@example.com
+Subject: A test message
+Message-ID: <ant>
+MIME-Version: 1.0
+
+A message body.
+""")
+        # 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._mlist.default_nonmember_action = Action.discard
+        msg.replace_header('Message-ID', '<ant2>')
+        # This time, the message should be discarded
+        result = rule.check(self._mlist, msg, msgdata)
+        self.assertTrue(result)
+        self.assertEqual(msgdata.get('moderation_action'), 'discard')
+
+    def test_member_fallback_to_list_defaults(self):
+        # https://gitlab.com/mailman/mailman/issues/189
+        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)
+        rule = moderation.MemberModeration()
+        msg = mfs("""\
+From: anne@example.com
+To: test@example.com
+Subject: A test message
+Message-ID: <ant>
+MIME-Version: 1.0
+
+A message body.
+""")
+        # First, the message should get held
+        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
+        self._mlist.default_member_action = Action.hold
+        msg.replace_header('Message-ID', '<ant2>')
+        # This time, the message should be discarded
+        result = rule.check(self._mlist, msg, msgdata)
+        self.assertTrue(result)
+        self.assertEqual(msgdata.get('moderation_action'), 'hold')