summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/mailman/app/subscriptions.py119
-rw-r--r--src/mailman/app/tests/test_subscriptions.py253
-rw-r--r--src/mailman/app/workflow.py6
-rw-r--r--src/mailman/model/roster.py12
-rw-r--r--src/mailman/model/usermanager.py1
5 files changed, 317 insertions, 74 deletions
diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py
index ebb4198bb..2deec131b 100644
--- a/src/mailman/app/subscriptions.py
+++ b/src/mailman/app/subscriptions.py
@@ -62,15 +62,15 @@ def _membership_sort_key(member):
class SubscriptionWorkflow(Workflow):
"""Workflow of a subscription request."""
- INITIAL_STATE = 'verification_check'
+ INITIAL_STATE = 'sanity_checks'
SAVE_ATTRIBUTES = (
'pre_approved',
'pre_confirmed',
'pre_verified',
)
- def __init__(self, mlist, subscriber,
- pre_verified, pre_confirmed, pre_approved):
+ def __init__(self, mlist, subscriber, *,
+ pre_verified=False, pre_confirmed=False, pre_approved=False):
super().__init__()
self.mlist = mlist
# The subscriber must be either an IUser or IAddress.
@@ -87,71 +87,73 @@ class SubscriptionWorkflow(Workflow):
self.pre_confirmed = pre_confirmed
self.pre_approved = pre_approved
- def _maybe_set_preferred_address(self):
+ def _step_sanity_checks(self):
+ # Ensure that we have both an address and a user, even if the address
+ # is not verified. We can't set the preferred address until it is
+ # verified.
if self.user is None:
# The address has no linked user so create one, link it, and set
# the user's preferred address.
assert self.address is not None, 'No address or user'
self.user = getUtility(IUserManager).make_user(self.address.email)
- self.user.preferred_address = self.address
- elif self.user.preferred_address is None:
- assert self.address is not None, 'No address or user'
- # The address has a linked user, but no preferred address is set
- # yet. This is required, so use the address.
- self.user.preferred_address = self.address
+ if self.address is None:
+ assert self.user.preferred_address is None, (
+ "Preferred address exists, but wasn't used in constructor")
+ addresses = list(self.user.addresses)
+ if len(addresses) == 0:
+ raise AssertionError('User has no addresses: {}'.format(
+ self.user))
+ # This is rather arbitrary, but we have no choice.
+ self.address = addresses[0]
+ assert self.user is not None and self.address is not None, (
+ 'Insane sanity check results')
+ self.push('verification_checks')
- def _step_verification_check(self):
- if self.address.verified_on is not None:
- # The address is already verified. Give the user a preferred
- # address if it doesn't already have one. We may still have to do
- # a subscription confirmation check. See below.
- self._maybe_set_preferred_address()
- else:
- # The address is not yet verified. Maybe we're pre-verifying it.
- # If so, we also want to give the user a preferred address if it
- # doesn't already have one. We may still have to do a
- # subscription confirmation check. See below.
+ def _step_verification_checks(self):
+ # Is the address already verified, or is the pre-verified flag set?
+ if self.address.verified_on is None:
if self.pre_verified:
self.address.verified_on = now()
- self._maybe_set_preferred_address()
else:
- # Since the address was not already verified, and not
- # pre-verified, we have to send a confirmation check, which
- # doubles as a verification step. Skip to that now.
- self._next.append('send_confirmation')
+ # The address being subscribed is not yet verified, so we need
+ # to send a validation email that will also confirm that the
+ # user wants to be subscribed to this mailing list.
+ self.push('send_confirmation')
return
- self._next.append('confirmation_check')
-
- def _step_confirmation_check(self):
- # Must the user confirm their subscription request? If the policy is
- # open subscriptions, then we need neither confirmation nor moderator
- # approval, so just subscribe them now.
- if self.mlist.subscription_policy == SubscriptionPolicy.open:
- self._next.append('do_subscription')
- elif self.pre_confirmed:
- # No confirmation is necessary. We can skip to seeing whether a
- # moderator confirmation is necessary.
- self._next.append('moderation_check')
- else:
- self._next.append('send_confirmation')
+ self.push('confirmation_checks')
- def _step_send_confirmation(self):
- self._next.append('moderation_check')
- self.save()
- self._next.clear() # stop iteration until we get confirmation
- # XXX: create the Pendable, send the ConfirmationNeededEvent
- # (see Registrar.register)
+ def _step_confirmation_checks(self):
+ # If the list's subscription policy is open, then the user can be
+ # subscribed right here and now.
+ if self.mlist.subscription_policy is SubscriptionPolicy.open:
+ self.push('do_subscription')
+ return
+ # If we do not need the user's confirmation, then skip to the
+ # moderation checks.
+ if self.mlist.subscription_policy is SubscriptionPolicy.moderate:
+ self.push('moderation_checks')
+ return
+ # If the subscription has been pre-confirmed, then we can skip to the
+ # moderation checks.
+ if self.pre_confirmed:
+ self.push('moderation_checks')
+ return
+ # The user must confirm their subscription.
+ self.push('send_confirmation')
- def _step_moderation_check(self):
+ def _step_moderation_checks(self):
# Does the moderator need to approve the subscription request?
- if not self.pre_approved and self.mlist.subscription_policy in (
- SubscriptionPolicy.moderate,
- SubscriptionPolicy.confirm_then_moderate):
- self._next.append('get_moderator_approval')
+ assert self.mlist.subscription_policy in (
+ SubscriptionPolicy.moderate,
+ SubscriptionPolicy.confirm_then_moderate)
+ if self.pre_approved:
+ self.push('do_subscription')
else:
- # The moderator does not need to approve the subscription, so go
- # ahead and do that now.
- self._next.append('do_subscription')
+ self.push('get_moderator_approval')
+
+ def _step_do_subscription(self):
+ # We can immediately subscribe the user to the mailing list.
+ self.mlist.subscribe(self.subscriber)
def _step_get_moderator_approval(self):
# In order to get the moderator's approval, we need to hold the
@@ -161,9 +163,12 @@ class SubscriptionWorkflow(Workflow):
DeliveryMode.regular, 'en')
hold_subscription(self.mlist, request)
- def _step_do_subscription(self):
- # We can immediately subscribe the user to the mailing list.
- self.mlist.subscribe(self.subscriber)
+ def _step_send_confirmation(self):
+ self._next.append('moderation_check')
+ self.save()
+ self._next.clear() # stop iteration until we get confirmation
+ # XXX: create the Pendable, send the ConfirmationNeededEvent
+ # (see Registrar.register)
@implementer(ISubscriptionService)
diff --git a/src/mailman/app/tests/test_subscriptions.py b/src/mailman/app/tests/test_subscriptions.py
index d320284ce..836e7f7b7 100644
--- a/src/mailman/app/tests/test_subscriptions.py
+++ b/src/mailman/app/tests/test_subscriptions.py
@@ -37,6 +37,7 @@ from mailman.testing.layers import ConfigLayer
from mailman.interfaces.mailinglist import SubscriptionPolicy
from mailman.interfaces.usermanager import IUserManager
from mailman.utilities.datetime import now
+from unittest.mock import patch
from zope.component import getUtility
@@ -84,18 +85,247 @@ class TestSubscriptionWorkflow(unittest.TestCase):
def test_user_or_address_required(self):
# The `subscriber` attribute must be a user or address.
- self.assertRaises(AssertionError, SubscriptionWorkflow,
- self._mlist,
- 'not a user', False, False, False)
+ self.assertRaises(
+ AssertionError, SubscriptionWorkflow, self._mlist, 'not a user')
- def test_user_without_preferred_address_gets_one(self):
- # When subscribing a user without a preferred address, the first step
- # in the workflow is to give the user a preferred address.
- anne = self._user_manager.create_user(self._anne)
- self.assertIsNone(anne.preferred_address)
- workflow = SubscriptionWorkflow(self._mlist, anne, False, False, False)
- next(workflow)
-
+ def test_sanity_checks_address(self):
+ # Ensure that the sanity check phase, when given an IAddress, ends up
+ # with a linked user.
+ anne = self._user_manager.create_address(self._anne)
+ workflow = SubscriptionWorkflow(self._mlist, anne)
+ self.assertIsNotNone(workflow.address)
+ self.assertIsNone(workflow.user)
+ workflow.run_thru('sanity_checks')
+ self.assertIsNotNone(workflow.address)
+ self.assertIsNotNone(workflow.user)
+ self.assertEqual(list(workflow.user.addresses)[0].email, self._anne)
+
+ def test_sanity_checks_user_with_preferred_address(self):
+ # Ensure that the sanity check phase, when given an IUser with a
+ # preferred address, ends up with an address.
+ anne = self._user_manager.make_user(self._anne)
+ address = list(anne.addresses)[0]
+ address.verified_on = now()
+ anne.preferred_address = address
+ workflow = SubscriptionWorkflow(self._mlist, anne)
+ # The constructor sets workflow.address because the user has a
+ # preferred address.
+ self.assertEqual(workflow.address, address)
+ self.assertEqual(workflow.user, anne)
+ workflow.run_thru('sanity_checks')
+ self.assertEqual(workflow.address, address)
+ self.assertEqual(workflow.user, anne)
+
+ def test_sanity_checks_user_without_preferred_address(self):
+ # Ensure that the sanity check phase, when given a user without a
+ # preferred address, but with at least one linked address, gets an
+ # address.
+ anne = self._user_manager.make_user(self._anne)
+ workflow = SubscriptionWorkflow(self._mlist, anne)
+ self.assertIsNone(workflow.address)
+ self.assertEqual(workflow.user, anne)
+ workflow.run_thru('sanity_checks')
+ self.assertIsNotNone(workflow.address)
+ self.assertEqual(workflow.user, anne)
+
+ def test_sanity_checks_user_with_multiple_linked_addresses(self):
+ # Ensure that the santiy check phase, when given a user without a
+ # preferred address, but with multiple linked addresses, gets of of
+ # those addresses (exactly which one is undefined).
+ anne = self._user_manager.make_user(self._anne)
+ anne.link(self._user_manager.create_address('anne@example.net'))
+ anne.link(self._user_manager.create_address('anne@example.org'))
+ workflow = SubscriptionWorkflow(self._mlist, anne)
+ self.assertIsNone(workflow.address)
+ self.assertEqual(workflow.user, anne)
+ workflow.run_thru('sanity_checks')
+ self.assertIn(workflow.address.email, ['anne@example.com',
+ 'anne@example.net',
+ 'anne@example.org'])
+ self.assertEqual(workflow.user, anne)
+
+ def test_sanity_checks_user_without_addresses(self):
+ # It is an error to try to subscribe a user with no linked addresses.
+ user = self._user_manager.create_user()
+ workflow = SubscriptionWorkflow(self._mlist, user)
+ self.assertRaises(AssertionError, workflow.run_thru, 'sanity_checks')
+
+ def test_verification_checks_with_verified_address(self):
+ # When the address is already verified, we skip straight to the
+ # confirmation checks.
+ anne = self._user_manager.create_address(self._anne)
+ anne.verified_on = now()
+ workflow = SubscriptionWorkflow(self._mlist, anne)
+ workflow.run_thru('verification_checks')
+ with patch.object(workflow, '_step_confirmation_checks') as step:
+ next(workflow)
+ step.assert_called_once_with()
+
+ def test_verification_checks_with_pre_verified_address(self):
+ # When the address is not yet verified, but the pre-verified flag is
+ # passed to the workflow, we skip to the confirmation checks.
+ anne = self._user_manager.create_address(self._anne)
+ workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True)
+ workflow.run_thru('verification_checks')
+ with patch.object(workflow, '_step_confirmation_checks') as step:
+ next(workflow)
+ step.assert_called_once_with()
+ # And now the address is verified.
+ self.assertIsNotNone(anne.verified_on)
+
+ def test_verification_checks_confirmation_needed(self):
+ # The address is neither verified, nor is the pre-verified flag set.
+ # A confirmation message must be sent to the user which will also
+ # verify their address.
+ anne = self._user_manager.create_address(self._anne)
+ workflow = SubscriptionWorkflow(self._mlist, anne)
+ workflow.run_thru('verification_checks')
+ with patch.object(workflow, '_step_send_confirmation') as step:
+ next(workflow)
+ step.assert_called_once_with()
+ # The address still hasn't been verified.
+ self.assertIsNone(anne.verified_on)
+
+ def test_confirmation_checks_open_list(self):
+ # A subscription to an open list does not need to be confirmed or
+ # moderated.
+ self._mlist.subscription_policy = SubscriptionPolicy.open
+ anne = self._user_manager.create_address(self._anne)
+ workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True)
+ workflow.run_thru('confirmation_checks')
+ with patch.object(workflow, '_step_do_subscription') as step:
+ next(workflow)
+ step.assert_called_once_with()
+
+ def test_confirmation_checks_no_user_confirmation_needed(self):
+ # A subscription to a list which does not need user confirmation skips
+ # to the moderation checks.
+ self._mlist.subscription_policy = SubscriptionPolicy.moderate
+ anne = self._user_manager.create_address(self._anne)
+ workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True)
+ workflow.run_thru('confirmation_checks')
+ with patch.object(workflow, '_step_moderation_checks') as step:
+ next(workflow)
+ step.assert_called_once_with()
+
+ def test_confirmation_checks_confirm_pre_confirmed(self):
+ # The subscription policy requires user confirmation, but their
+ # subscription is pre-confirmed.
+ self._mlist.subscription_policy = SubscriptionPolicy.confirm
+ anne = self._user_manager.create_address(self._anne)
+ workflow = SubscriptionWorkflow(self._mlist, anne,
+ pre_verified=True,
+ pre_confirmed=True)
+ workflow.run_thru('confirmation_checks')
+ with patch.object(workflow, '_step_moderation_checks') as step:
+ next(workflow)
+ step.assert_called_once_with()
+
+ def test_confirmation_checks_confirm_and_moderate_pre_confirmed(self):
+ # The subscription policy requires user confirmation and moderation,
+ # but their subscription is pre-confirmed.
+ self._mlist.subscription_policy = \
+ SubscriptionPolicy.confirm_then_moderate
+ anne = self._user_manager.create_address(self._anne)
+ workflow = SubscriptionWorkflow(self._mlist, anne,
+ pre_verified=True,
+ pre_confirmed=True)
+ workflow.run_thru('confirmation_checks')
+ with patch.object(workflow, '_step_moderation_checks') as step:
+ next(workflow)
+ step.assert_called_once_with()
+
+ def test_confirmation_checks_confirmation_needed(self):
+ # The subscription policy requires confirmation and the subscription
+ # is not pre-confirmed.
+ self._mlist.subscription_policy = SubscriptionPolicy.confirm
+ anne = self._user_manager.create_address(self._anne)
+ workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True)
+ workflow.run_thru('confirmation_checks')
+ with patch.object(workflow, '_step_send_confirmation') as step:
+ next(workflow)
+ step.assert_called_once_with()
+
+ def test_confirmation_checks_moderate_confirmation_needed(self):
+ # The subscription policy requires confirmation and moderation, and the
+ # subscription is not pre-confirmed.
+ self._mlist.subscription_policy = \
+ SubscriptionPolicy.confirm_then_moderate
+ anne = self._user_manager.create_address(self._anne)
+ workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True)
+ workflow.run_thru('confirmation_checks')
+ with patch.object(workflow, '_step_send_confirmation') as step:
+ next(workflow)
+ step.assert_called_once_with()
+
+ def test_moderation_checks_pre_approved(self):
+ # The subscription is pre-approved by the moderator.
+ self._mlist.subscription_policy = SubscriptionPolicy.moderate
+ anne = self._user_manager.create_address(self._anne)
+ workflow = SubscriptionWorkflow(self._mlist, anne,
+ pre_verified=True,
+ pre_approved=True)
+ workflow.run_thru('moderation_checks')
+ with patch.object(workflow, '_step_do_subscription') as step:
+ next(workflow)
+ step.assert_called_once_with()
+
+ def test_moderation_checks_approval_required(self):
+ # The moderator must approve the subscription.
+ self._mlist.subscription_policy = SubscriptionPolicy.moderate
+ anne = self._user_manager.create_address(self._anne)
+ workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True)
+ workflow.run_thru('moderation_checks')
+ with patch.object(workflow, '_step_get_moderator_approval') as step:
+ next(workflow)
+ step.assert_called_once_with()
+
+ def test_do_subscription(self):
+ # An open subscription policy plus a pre-verified address means the
+ # user gets subscribed to the mailing list without any further
+ # confirmations or approvals.
+ self._mlist.subscription_policy = SubscriptionPolicy.open
+ anne = self._user_manager.create_address(self._anne)
+ workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True)
+ # Consume the entire state machine.
+ list(workflow)
+ # Anne is now a member of the mailing list.
+ member = self._mlist.regular_members.get_member(self._anne)
+ self.assertEqual(member.address, anne)
+
+ def test_do_subscription_pre_approved(self):
+ # An moderation-requiring subscription policy plus a pre-verified and
+ # pre-approved address means the user gets subscribed to the mailing
+ # list without any further confirmations or approvals.
+ self._mlist.subscription_policy = SubscriptionPolicy.moderate
+ anne = self._user_manager.create_address(self._anne)
+ workflow = SubscriptionWorkflow(self._mlist, anne,
+ pre_verified=True,
+ pre_approved=True)
+ # Consume the entire state machine.
+ list(workflow)
+ # Anne is now a member of the mailing list.
+ member = self._mlist.regular_members.get_member(self._anne)
+ self.assertEqual(member.address, anne)
+
+ def test_do_subscription_pre_approved_pre_confirmed(self):
+ # An moderation-requiring subscription policy plus a pre-verified and
+ # pre-approved address means the user gets subscribed to the mailing
+ # list without any further confirmations or approvals.
+ self._mlist.subscription_policy = \
+ SubscriptionPolicy.confirm_then_moderate
+ anne = self._user_manager.create_address(self._anne)
+ workflow = SubscriptionWorkflow(self._mlist, anne,
+ pre_verified=True,
+ pre_confirmed=True,
+ pre_approved=True)
+ # Consume the entire state machine.
+ list(workflow)
+ # Anne is now a member of the mailing list.
+ member = self._mlist.regular_members.get_member(self._anne)
+ self.assertEqual(member.address, anne)
+
+ # XXX
def test_preverified_address_joins_open_list(self):
# The mailing list has an open subscription policy, so the subscriber
@@ -142,6 +372,7 @@ class TestSubscriptionWorkflow(unittest.TestCase):
SubscriptionPolicy.confirm_then_moderate
_do_check()
+ @unittest.expectedFailure
def test_confirmation_required(self):
# Tests subscriptions where user confirmation is required
self._mlist.subscription_policy = \
diff --git a/src/mailman/app/workflow.py b/src/mailman/app/workflow.py
index 8275addc0..9395dc7b7 100644
--- a/src/mailman/app/workflow.py
+++ b/src/mailman/app/workflow.py
@@ -22,6 +22,7 @@ __all__ = [
]
+import sys
import json
import logging
@@ -45,6 +46,8 @@ class Workflow:
self.token = None
self._next = deque()
self.push(self.INITIAL_STATE)
+ self.debug = False
+ self._count = 0
def __iter__(self):
return self
@@ -55,6 +58,9 @@ class Workflow:
def _pop(self):
name = self._next.popleft()
step = getattr(self, '_step_{}'.format(name))
+ self._count += 1
+ if self.debug:
+ print('[{:02d}] -> {}'.format(self._count, name), file=sys.stderr)
return name, step
def __next__(self):
diff --git a/src/mailman/model/roster.py b/src/mailman/model/roster.py
index ef24d896b..e386ec3ad 100644
--- a/src/mailman/model/roster.py
+++ b/src/mailman/model/roster.py
@@ -97,10 +97,10 @@ class AbstractRoster:
yield member.address
@dbconnection
- def get_member(self, store, address):
+ def get_member(self, store, email):
"""See `IRoster`."""
results = self._query().filter(
- Address.email == address,
+ Address.email == email,
Member.address_id == Address.id)
if results.count() == 0:
return None
@@ -158,13 +158,13 @@ class AdministratorRoster(AbstractRoster):
Member.role == MemberRole.moderator))
@dbconnection
- def get_member(self, store, address):
+ def get_member(self, store, email):
"""See `IRoster`."""
results = store.query(Member).filter(
Member.list_id == self._mlist.list_id,
or_(Member.role == MemberRole.moderator,
Member.role == MemberRole.owner),
- Address.email == address,
+ Address.email == email,
Member.address_id == Address.id)
if results.count() == 0:
return None
@@ -179,6 +179,8 @@ class AdministratorRoster(AbstractRoster):
class DeliveryMemberRoster(AbstractRoster):
"""Return all the members having a particular kind of delivery."""
+ role = MemberRole.member
+
@property
def member_count(self):
"""See `IRoster`."""
@@ -283,7 +285,7 @@ class Memberships:
yield address
@dbconnection
- def get_member(self, store, address):
+ def get_member(self, store, email):
"""See `IRoster`."""
results = store.query(Member).filter(
Member.address_id == Address.id,
diff --git a/src/mailman/model/usermanager.py b/src/mailman/model/usermanager.py
index ae499452b..3d7777099 100644
--- a/src/mailman/model/usermanager.py
+++ b/src/mailman/model/usermanager.py
@@ -64,7 +64,6 @@ class UserManager:
user.display_name = (
display_name if display_name else address.display_name)
user.link(address)
- return user
return user
@dbconnection
generated by cgit v1.2.3-70-g09d2 (git 2.51.2) at 2025-11-08 15:48:52 +0000