GNU Mailman

Merge lp:~abompard/mailman/moderation_reasons into lp:mailman

  1. moderation_reasons
  2. Merge into 3.0
Proposed by Aurélien Bompard
Status: Merged
Merge reported by: Barry Warsaw
Merged at revision: not available
Proposed branch: lp:~abompard/mailman/moderation_reasons
Merge into: lp:mailman
Diff against target: 293 lines (+115/-19)
8 files modified
src/mailman/app/docs/chains.rst (+2/-2)
src/mailman/chains/hold.py (+7/-6)
src/mailman/chains/tests/test_hold.py (+51/-2)
src/mailman/rules/docs/moderation.rst (+9/-6)
src/mailman/rules/moderation.py (+8/-0)
src/mailman/rules/tests/test_moderation.py (+35/-0)
src/mailman/templates/en/postauth.txt (+2/-2)
src/mailman/templates/en/postheld.txt (+1/-1)
To merge this branch: bzr merge lp:~abompard/mailman/moderation_reasons
Reviewer Review Type Date Requested Status
Barry Warsaw Approve
Review via email: mp+216392@code.launchpad.net

Description of the change

Currently moderation reasons are always 'XXX'. This branch fixes that with a message that comes from the moderation rules that matched.

To post a comment you must log in.
Revision history for this message
Barry Warsaw (barry) wrote :

Thanks for the contribution. I've adapted this to the current git trunk and will apply over there.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/mailman/app/docs/chains.rst'
2--- src/mailman/app/docs/chains.rst 2012-03-07 23:45:09 +0000
3+++ src/mailman/app/docs/chains.rst 2014-04-17 20:19:21 +0000
4@@ -132,7 +132,7 @@
5 List: test@example.com
6 From: aperson@example.com
7 Subject: My first post
8- Reason: XXX
9+ Reasons: (n/a)
10 <BLANKLINE>
11 At your convenience, visit:
12 <BLANKLINE>
13@@ -189,7 +189,7 @@
14 <BLANKLINE>
15 The reason it is being held:
16 <BLANKLINE>
17- XXX
18+ (n/a)
19 <BLANKLINE>
20 Either the message will get posted to the list, or you will receive
21 notification of the moderator's decision. If you would like to cancel
22
23=== modified file 'src/mailman/chains/hold.py'
24--- src/mailman/chains/hold.py 2014-01-01 14:59:42 +0000
25+++ src/mailman/chains/hold.py 2014-04-17 20:19:21 +0000
26@@ -26,6 +26,7 @@
27
28
29 import logging
30+import re
31
32 from email.mime.message import MIMEMessage
33 from email.mime.text import MIMEText
34@@ -138,7 +139,6 @@
35 if rule_misses:
36 msg['X-Mailman-Rule-Misses'] = SEMISPACE.join(rule_misses)
37 # Hold the message by adding it to the list's request database.
38- # XXX How to calculate the reason?
39 request_id = hold_message(mlist, msg, msgdata, None)
40 # Calculate a confirmation token to send to the author of the
41 # message.
42@@ -158,11 +158,13 @@
43 original_subject = _('(no subject)')
44 else:
45 original_subject = oneline(original_subject, charset)
46+ reasons = msgdata.get("moderation_reasons", [_("(n/a)")])
47 substitutions = dict(
48 listname = mlist.fqdn_listname,
49 subject = original_subject,
50 sender = msg.sender,
51- reason = 'XXX', #reason,
52+ reasons = "\r\n".join(
53+ [' ' + wrap(_(r), column=66) for r in reasons]),
54 confirmurl = '{0}/{1}'.format(mlist.script_url('confirm'), token),
55 admindb_url = mlist.script_url('admindb'),
56 )
57@@ -209,7 +211,8 @@
58 charset = language.charset
59 # We need to regenerate or re-translate a few values in the
60 # substitution dictionary.
61- #d['reason'] = _(reason) # XXX reason
62+ substitutions["reasons"] = wrap(
63+ ", ".join([_(r) for r in reasons]), column=55)
64 substitutions['subject'] = original_subject
65 # craft the admin notification message and deliver it
66 subject = _(
67@@ -240,9 +243,7 @@
68 nmsg.attach(MIMEMessage(dmsg))
69 nmsg.send(mlist, **dict(tomoderators=True))
70 # Log the held message
71- # XXX reason
72- reason = 'n/a'
73 log.info('HOLD: %s post from %s held, message-id=%s: %s',
74 mlist.fqdn_listname, msg.sender,
75- msg.get('message-id', 'n/a'), reason)
76+ msg.get('message-id', 'n/a'), " ; ".join(reasons))
77 notify(HoldEvent(mlist, msg, msgdata, self))
78
79=== modified file 'src/mailman/chains/tests/test_hold.py'
80--- src/mailman/chains/tests/test_hold.py 2014-01-01 14:59:42 +0000
81+++ src/mailman/chains/tests/test_hold.py 2014-04-17 20:19:21 +0000
82@@ -29,10 +29,14 @@
83 from zope.component import getUtility
84
85 from mailman.app.lifecycle import create_list
86-from mailman.chains.hold import autorespond_to_sender
87+from mailman.chains.hold import autorespond_to_sender, HoldChain
88+from mailman.email.message import Message
89 from mailman.interfaces.autorespond import IAutoResponseSet, Response
90 from mailman.interfaces.usermanager import IUserManager
91-from mailman.testing.helpers import configuration, get_queue_messages
92+from mailman.testing.helpers import (
93+ configuration,
94+ get_queue_messages,
95+ specialized_message_from_string as mfs)
96 from mailman.testing.layers import ConfigLayer
97
98
99@@ -89,3 +93,48 @@
100
101 If you believe this message is in error, or if you have any questions,
102 please contact the list owner at test-owner@example.com.""")
103+
104+
105+
106
107+class TestHoldChain(unittest.TestCase):
108+ """Test the hold chain code."""
109+
110+ layer = ConfigLayer
111+
112+ def setUp(self):
113+ self._mlist = create_list('test@example.com')
114+
115+ def test_hold_chain(self):
116+ msg = mfs("""\
117+From: anne@example.com
118+To: test@example.com
119+Subject: A message
120+Message-ID: <ant>
121+MIME-Version: 1.0
122+
123+A message body.
124+""")
125+ metadata = {
126+ "moderation_reasons": [
127+ "TEST-REASON-1",
128+ "TEST-REASON-2",
129+ ],
130+ }
131+ chain = HoldChain()
132+ chain._process(self._mlist, msg, metadata)
133+ messages = get_queue_messages('virgin')
134+ self.assertEqual(len(messages), 2)
135+ sent_msgs = {}
136+ for msg_bag in messages:
137+ if msg_bag.msg["To"] == "test-owner@example.com":
138+ sent_msgs["owner"] = msg_bag.msg
139+ elif msg_bag.msg["To"] == "anne@example.com":
140+ sent_msgs["sender"] = msg_bag.msg
141+ else:
142+ self.fail("Unexpected message: %s" % msg_bag.msg)
143+ self.assertTrue("TEST-REASON-1, TEST-REASON-2" in
144+ sent_msgs["owner"].as_string())
145+ self.assertTrue(" TEST-REASON-1" in
146+ sent_msgs["sender"].as_string())
147+ self.assertTrue(" TEST-REASON-2" in
148+ sent_msgs["sender"].as_string())
149
150=== modified file 'src/mailman/rules/docs/moderation.rst'
151--- src/mailman/rules/docs/moderation.rst 2013-06-19 02:43:40 +0000
152+++ src/mailman/rules/docs/moderation.rst 2014-04-17 20:19:21 +0000
153@@ -51,8 +51,9 @@
154 >>> member_rule.check(mlist, member_msg, msgdata)
155 True
156 >>> dump_msgdata(msgdata)
157- moderation_action: hold
158- moderation_sender: aperson@example.com
159+ moderation_action : hold
160+ moderation_reasons: [u'The message comes from a moderated member']
161+ moderation_sender : aperson@example.com
162
163
164 Nonmembers
165@@ -87,8 +88,9 @@
166 >>> nonmember_rule.check(mlist, nonmember_msg, msgdata)
167 True
168 >>> dump_msgdata(msgdata)
169- moderation_action: hold
170- moderation_sender: bperson@example.com
171+ moderation_action : hold
172+ moderation_reasons: [u'The message is not from a list member']
173+ moderation_sender : bperson@example.com
174
175 Of course, the nonmember action can be set to defer the decision, in which
176 case the rule does not match.
177@@ -140,8 +142,9 @@
178 >>> nonmember_rule.check(mlist, msg, msgdata)
179 True
180 >>> dump_msgdata(msgdata)
181- moderation_action: hold
182- moderation_sender: cperson@example.com
183+ moderation_action : hold
184+ moderation_reasons: [u'The message is not from a list member']
185+ moderation_sender : cperson@example.com
186
187 >>> dump_list(mlist.members.members, key=memberkey)
188 <Member: Anne Person <aperson@example.com>
189
190=== modified file 'src/mailman/rules/moderation.py'
191--- src/mailman/rules/moderation.py 2014-03-15 19:13:46 +0000
192+++ src/mailman/rules/moderation.py 2014-04-17 20:19:21 +0000
193@@ -59,6 +59,10 @@
194 # stored in the pending request table.
195 msgdata['moderation_action'] = action.name
196 msgdata['moderation_sender'] = sender
197+ if 'moderation_reasons' not in msgdata:
198+ msgdata['moderation_reasons'] = []
199+ msgdata['moderation_reasons'].append(
200+ _('The message comes from a moderated member'))
201 return True
202 # The sender is not a member so this rule does not match.
203 return False
204@@ -104,6 +108,10 @@
205 # stored in the pending request table.
206 msgdata['moderation_action'] = action.name
207 msgdata['moderation_sender'] = sender
208+ if 'moderation_reasons' not in msgdata:
209+ msgdata['moderation_reasons'] = []
210+ msgdata['moderation_reasons'].append(
211+ _('The message is not from a list member'))
212 return True
213 # The sender must be a member, so this rule does not match.
214 return False
215
216=== modified file 'src/mailman/rules/tests/test_moderation.py'
217--- src/mailman/rules/tests/test_moderation.py 2014-03-15 19:13:46 +0000
218+++ src/mailman/rules/tests/test_moderation.py 2014-04-17 20:19:21 +0000
219@@ -28,6 +28,7 @@
220 import unittest
221
222 from mailman.app.lifecycle import create_list
223+from mailman.interfaces.action import Action
224 from mailman.interfaces.member import MemberRole
225 from mailman.interfaces.usermanager import IUserManager
226 from mailman.rules import moderation
227@@ -76,3 +77,37 @@
228 # Bill is not a member.
229 bill_member = self._mlist.members.get_member('bill@example.com')
230 self.assertIsNone(bill_member)
231+
232+ def test_moderation_reason(self):
233+ user_manager = getUtility(IUserManager)
234+ anne = user_manager.create_address('anne@example.com')
235+ msg = mfs("""\
236+From: anne@example.com
237+To: test@example.com
238+Subject: A test message
239+Message-ID: <ant>
240+MIME-Version: 1.0
241+
242+A message body.
243+""")
244+ # Anne is in the message's senders list.
245+ self.assertIn('anne@example.com', msg.senders)
246+ # Now run the rule
247+ rule = moderation.NonmemberModeration()
248+ msgdata = {}
249+ result = rule.check(self._mlist, msg, msgdata)
250+ self.assertTrue(result, 'NonmemberModeration rule should hit')
251+ # The reason for moderation should be in the msgdata
252+ self.assertTrue("moderation_reasons" in msgdata)
253+ self.assertEqual(len(msgdata["moderation_reasons"]), 1)
254+ # Now make Anne a moderated member
255+ anne_member = self._mlist.subscribe(anne, MemberRole.member)
256+ anne_member.moderation_action = Action.hold
257+ # ...and run the rule again
258+ rule = moderation.MemberModeration()
259+ msgdata = {}
260+ result = rule.check(self._mlist, msg, msgdata)
261+ self.assertTrue(result, 'MemberModeration rule should hit')
262+ # The reason for moderation should be in the msgdata
263+ self.assertTrue("moderation_reasons" in msgdata)
264+ self.assertEqual(len(msgdata["moderation_reasons"]), 1)
265
266=== modified file 'src/mailman/templates/en/postauth.txt'
267--- src/mailman/templates/en/postauth.txt 2008-09-29 06:44:19 +0000
268+++ src/mailman/templates/en/postauth.txt 2014-04-17 20:19:21 +0000
269@@ -4,10 +4,10 @@
270 List: $listname
271 From: $sender
272 Subject: $subject
273- Reason: $reason
274+ Reasons: $reasons
275
276 At your convenience, visit:
277
278 $admindb_url
279-
280+
281 to approve or deny the request.
282
283=== modified file 'src/mailman/templates/en/postheld.txt'
284--- src/mailman/templates/en/postheld.txt 2008-09-29 06:44:19 +0000
285+++ src/mailman/templates/en/postheld.txt 2014-04-17 20:19:21 +0000
286@@ -6,7 +6,7 @@
287
288 The reason it is being held:
289
290- $reason
291+$reasons
292
293 Either the message will get posted to the list, or you will receive
294 notification of the moderator's decision. If you would like to cancel