5 files changed, 82 insertions, 4 deletions

diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py
index a0c0c8059..ba62e9f52 100644
--- a/src/mailman/app/subscriptions.py
+++ b/src/mailman/app/subscriptions.py
@@ -59,7 +59,7 @@ class SubscriptionManager:
         workflow_type = pendable.get('type')
         if workflow_type in {PendableSubscription.PEND_TYPE,
                              PendableUnsubscription.PEND_TYPE}:
-            workflow = (self._mlist.subscription_policy
+            workflow = (self._mlist.subscription_policy  # pragma: nocover
                         if workflow_type == PendableSubscription.PEND_TYPE
                         else self._mlist.unsubscription_policy)(self._mlist)
         else:
diff --git a/src/mailman/model/tests/test_mailinglist.py b/src/mailman/model/tests/test_mailinglist.py
index 72232cb53..a402c6dc4 100644
--- a/src/mailman/model/tests/test_mailinglist.py
+++ b/src/mailman/model/tests/test_mailinglist.py
@@ -28,6 +28,8 @@ from mailman.interfaces.mailinglist import (
 from mailman.interfaces.member import (
     AlreadySubscribedError, MemberRole, MissingPreferredAddressError)
 from mailman.interfaces.usermanager import IUserManager
+from mailman.interfaces.workflows import (
+    ISubscriptionWorkflow, IUnsubscriptionWorkflow)
 from mailman.testing.helpers import (
     configuration, get_queue_messages, set_preferred)
 from mailman.testing.layers import ConfigLayer
@@ -408,3 +410,25 @@ class TestHeaderMatch(unittest.TestCase):
         header_matches = IHeaderMatchList(self._mlist)
         with self.assertRaises(IndexError):
             del header_matches[0]
+
+    def test_subscription_policy(self):
+        for workflow_class in config.workflows:
+            if ISubscriptionWorkflow.implementedBy(workflow_class):
+                self._mlist.subscription_policy = workflow_class.name
+                self.assertEqual(self._mlist.subscription_policy,
+                                 workflow_class)
+
+    def test_subscription_policy_invalid(self):
+        with self.assertRaises(ValueError):
+            self._mlist.subscription_policy = 'not-a-subscription-policy'
+
+    def test_unsubscription_policy(self):
+        for workflow_class in config.workflows:
+            if IUnsubscriptionWorkflow.implementedBy(workflow_class):
+                self._mlist.unsubscription_policy = workflow_class.name
+                self.assertEqual(self._mlist.unsubscription_policy,
+                                 workflow_class)
+
+    def test_unsubscription_policy_invalid(self):
+        with self.assertRaises(ValueError):
+            self._mlist.unsubscription_policy = 'not-an-unsubscription-policy'
diff --git a/src/mailman/rest/tests/test_validator.py b/src/mailman/rest/tests/test_validator.py
index 0d197032e..95855d170 100644
--- a/src/mailman/rest/tests/test_validator.py
+++ b/src/mailman/rest/tests/test_validator.py
@@ -22,9 +22,14 @@ import unittest
 from mailman.core.api import API30, API31
 from mailman.interfaces.action import Action
 from mailman.interfaces.usermanager import IUserManager
+from mailman.interfaces.workflows import (ISubscriptionWorkflow,
+                                          IUnsubscriptionWorkflow)
 from mailman.rest.validator import (
-    enum_validator, list_of_strings_validator, subscriber_validator)
+    enum_validator, list_of_strings_validator, policy_validator,
+    subscriber_validator)
 from mailman.testing.layers import RESTLayer
+from mailman.workflows.subscription import ConfirmSubscriptionPolicy
+from mailman.workflows.unsubscription import ConfirmUnsubscriptionPolicy
 from zope.component import getUtility
 
 
@@ -91,3 +96,40 @@ class TestValidators(unittest.TestCase):
 
     def test_enum_validator_blank(self):
         self.assertEqual(enum_validator(Action, allow_blank=True)(''), None)
+
+    def test_policy_validator_wrong_policy_class(self):
+        self.assertRaises(ValueError, policy_validator, None)
+
+    def test_policy_validator_sub_name(self):
+        self.assertEqual(policy_validator(ISubscriptionWorkflow)(
+                ConfirmSubscriptionPolicy.name), ConfirmSubscriptionPolicy)
+
+    def test_policy_validator_sub_class(self):
+        self.assertEqual(policy_validator(ISubscriptionWorkflow)(
+                ConfirmSubscriptionPolicy), ConfirmSubscriptionPolicy)
+
+    def test_policy_validator_sub_backward_compat(self):
+        self.assertEqual(policy_validator(ISubscriptionWorkflow)('confirm'),
+                         ConfirmSubscriptionPolicy)
+
+    def test_policy_validator_sub_wrong_policy(self):
+        validator = policy_validator(ISubscriptionWorkflow)
+        with self.assertRaises(ValueError):
+            validator('not a subscription policy')
+
+    def test_policy_validator_unsub_name(self):
+        self.assertEqual(policy_validator(IUnsubscriptionWorkflow)(
+                ConfirmUnsubscriptionPolicy.name), ConfirmUnsubscriptionPolicy)
+
+    def test_policy_validator_unsub_class(self):
+        self.assertEqual(policy_validator(IUnsubscriptionWorkflow)(
+                ConfirmUnsubscriptionPolicy), ConfirmUnsubscriptionPolicy)
+
+    def test_policy_validator_unsub_backward_compat(self):
+        self.assertEqual(policy_validator(IUnsubscriptionWorkflow)('confirm'),
+                         ConfirmUnsubscriptionPolicy)
+
+    def test_policy_validator_unsub_wrong_policy(self):
+        validator = policy_validator(IUnsubscriptionWorkflow)
+        with self.assertRaises(ValueError):
+            validator('not an unsubscription policy')
diff --git a/src/mailman/workflows/common.py b/src/mailman/workflows/common.py
index 1f63d3d1d..c250785c2 100644
--- a/src/mailman/workflows/common.py
+++ b/src/mailman/workflows/common.py
@@ -211,7 +211,7 @@ class SubscriptionBase(SubscriptionWorkflowCommon):
             # verified by a full coverage run, but diffcov for some reason
             # claims that the test added in the branch that added this code
             # does not cover the change.  That seems like a bug in diffcov.
-            raise AlreadySubscribedError(           # pragma: no cover
+            raise AlreadySubscribedError(           # pragma: nocover
                 self.mlist.fqdn_listname,
                 self.address.email,
                 MemberRole.member)
diff --git a/src/mailman/workflows/tests/test_subscriptions.py b/src/mailman/workflows/tests/test_subscriptions.py
index 23487ab1c..65569691b 100644
--- a/src/mailman/workflows/tests/test_subscriptions.py
+++ b/src/mailman/workflows/tests/test_subscriptions.py
@@ -24,7 +24,9 @@ from mailman.app.lifecycle import create_list
 from mailman.interfaces.bans import IBanManager
 from mailman.interfaces.member import MemberRole, MembershipIsBannedError
 from mailman.interfaces.pending import IPendings
-from mailman.interfaces.subscriptions import TokenOwner
+from mailman.interfaces.subscriptions import (
+    SubscriptionPendingError,
+    TokenOwner)
 from mailman.interfaces.usermanager import IUserManager
 from mailman.testing.helpers import (
     LogFileMark, get_queue_messages, set_preferred)
@@ -157,6 +159,16 @@ class TestSubscriptionWorkflow(unittest.TestCase):
         workflow = ConfirmSubscriptionPolicy(self._mlist, anne)
         self.assertRaises(MembershipIsBannedError, list, workflow)
 
+    def test_sanity_checks_already_requested(self):
+        # An exception is raised if there is already a subscription request.
+        anne = self._user_manager.create_address(self._anne)
+        workflow = ConfirmSubscriptionPolicy(self._mlist, anne)
+        list(workflow)
+        other_workflow = ConfirmSubscriptionPolicy(self._mlist, anne)
+        self.assertRaises(SubscriptionPendingError, list, other_workflow)
+        # The original workflow token is still in the database.
+        self._expected_pendings_count = 1
+
     def test_verification_checks_with_verified_address(self):
         # When the address is already verified, we skip straight to the
         # confirmation checks.