Merge lp:~abompard/mailman/subpolicy into lp:~barry/mailman/subpolicy-2

Proposed by Aurélien Bompard
Status: Rejected
Rejected by: Barry Warsaw
Proposed branch: lp:~abompard/mailman/subpolicy
Merge into: lp:~barry/mailman/subpolicy-2
Diff against target: 62 lines (+15/-3)
4 files modified
src/mailman/app/subscriptions.py (+1/-1)
src/mailman/app/tests/test_subscriptions.py (+0/-1)
src/mailman/model/tests/test_workflow.py (+10/-0)
src/mailman/model/workflow.py (+4/-1)
To merge this branch: bzr merge lp:~abompard/mailman/subpolicy
Reviewer Review Type Date Requested Status
Barry Warsaw Disapprove
Review via email: mp+255904@code.launchpad.net

Description of the change

A couple of minor changes. The rest looks good, but I have questions on the "self.token" value that we need to generate for the SubscriptionWorkflow class.

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

Thanks, but I think this isn't relevant to the current state of the branch.

review: Disapprove

Unmerged revisions

7329. By Aurelien Bompard <email address hidden>

Make sure stored state is deleted after restoring

7328. By Aurelien Bompard <email address hidden>

Fix a failing test

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/mailman/app/subscriptions.py'
2--- src/mailman/app/subscriptions.py 2015-04-10 02:44:42 +0000
3+++ src/mailman/app/subscriptions.py 2015-04-10 23:32:59 +0000
4@@ -164,7 +164,7 @@
5 hold_subscription(self.mlist, request)
6
7 def _step_send_confirmation(self):
8- self._next.append('moderation_check')
9+ self._next.append('moderation_checks')
10 self.save()
11 self._next.clear() # stop iteration until we get confirmation
12 # XXX: create the Pendable, send the ConfirmationNeededEvent
13
14=== modified file 'src/mailman/app/tests/test_subscriptions.py'
15--- src/mailman/app/tests/test_subscriptions.py 2015-04-10 02:44:42 +0000
16+++ src/mailman/app/tests/test_subscriptions.py 2015-04-10 23:32:59 +0000
17@@ -372,7 +372,6 @@
18 SubscriptionPolicy.confirm_then_moderate
19 _do_check()
20
21- @unittest.expectedFailure
22 def test_confirmation_required(self):
23 # Tests subscriptions where user confirmation is required
24 self._mlist.subscription_policy = \
25
26=== modified file 'src/mailman/model/tests/test_workflow.py'
27--- src/mailman/model/tests/test_workflow.py 2015-03-29 21:14:09 +0000
28+++ src/mailman/model/tests/test_workflow.py 2015-04-10 23:32:59 +0000
29@@ -24,6 +24,7 @@
30
31 import unittest
32
33+from mailman.config import config
34 from mailman.interfaces.workflow import IWorkflowStateManager
35 from mailman.testing.layers import ConfigLayer
36 from zope.component import getUtility
37@@ -108,3 +109,12 @@
38 self._manager.save(name, key)
39 workflow = self._manager.restore('ewe', 'fly')
40 self.assertIsNone(workflow)
41+
42+ def test_restore_cleanup(self):
43+ # After restoring, stale data must be deleted from the database
44+ name = 'ant'
45+ key = 'bee'
46+ self._manager.save(name, key)
47+ self.assertIsNotNone(self._manager.restore(name, key))
48+ config.db.store.flush() # there's some caching involved
49+ self.assertIsNone(self._manager.restore(name, key))
50
51=== modified file 'src/mailman/model/workflow.py'
52--- src/mailman/model/workflow.py 2015-03-29 21:14:09 +0000
53+++ src/mailman/model/workflow.py 2015-04-10 23:32:59 +0000
54@@ -62,4 +62,7 @@
55 @dbconnection
56 def restore(self, store, name, key):
57 """See `IWorkflowStateManager`."""
58- return store.query(WorkflowState).get((name, key))
59+ state = store.query(WorkflowState).get((name, key))
60+ if state is not None:
61+ store.delete(state)
62+ return state

Subscribers

People subscribed via source and target branches