1 files changed, 79 insertions, 149 deletions

diff --git a/src/mailman/workflows/common.py b/src/mailman/workflows/common.py
index d6c372b1e..8593d0997 100644
--- a/src/mailman/workflows/common.py
+++ b/src/mailman/workflows/common.py
@@ -58,6 +58,11 @@ class WhichSubscriber(Enum):
     user = 2
 
 
+class WhichWorkflow(Enum):
+    subscription = 1
+    unsubscription = 2
+
+
 @implementer(IPendable)
 class PendableSubscription(dict):
     PEND_TYPE = 'subscription'
@@ -130,6 +135,16 @@ class SubscriptionWorkflowCommon(Workflow):
     def token_owner_key(self, value):
         self.token_owner = TokenOwner(value)
 
+    def _restore_subscriber(self):
+        # Restore a little extra state that can't be stored in the database
+        # (because the order of setattr() on restore is indeterminate), then
+        # continue with the confirmation/verification step.
+        if self.which is WhichSubscriber.address:
+            self.subscriber = self.address
+        else:
+            assert self.which is WhichSubscriber.user
+            self.subscriber = self.user
+
     def _set_token(self, token_owner):
         assert isinstance(token_owner, TokenOwner)
         pendings = getUtility(IPendings)
@@ -163,6 +178,10 @@ class SubscriptionWorkflowCommon(Workflow):
 
 class SubscriptionBase(SubscriptionWorkflowCommon):
 
+    def __init__(self, mlist, subscriber):
+        super().__init__(mlist, subscriber)
+        self._workflow = WhichWorkflow.subscription
+
     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
@@ -217,27 +236,49 @@ class SubscriptionBase(SubscriptionWorkflowCommon):
             'Unexpected active token at end of subscription workflow')
 
 
+class UnsubscriptionBase(SubscriptionWorkflowCommon):
+
+    def __init__(self, mlist, subscriber):
+        super().__init__(mlist, subscriber)
+        if IAddress.providedBy(subscriber) or IUser.providedBy(subscriber):
+            self.member = self.mlist.regular_members.get_member(
+                self.address.email)
+        self._workflow = WhichWorkflow.unsubscription
+
+    def _step_subscription_checks(self):
+        assert self.mlist.is_subscribed(self.subscriber)
+
+    def _step_do_unsubscription(self):
+        try:
+            delete_member(self.mlist, self.address.email)
+        except NotAMemberError:
+            # The member has already been unsubscribed.
+            pass
+        self.member = None
+        assert self.token is None and self.token_owner is TokenOwner.no_one, (
+            'Unexpected active token at end of subscription workflow')
+
+
 class RequestMixin:
 
     def _step_send_confirmation(self):
         self._set_token(TokenOwner.subscriber)
         self.push('do_confirm_verify')
         self.save()
+        if self._workflow is WhichWorkflow.subscription:
+            event_class = SubscriptionConfirmationNeededEvent
+        else:
+            event_class = UnsubscriptionConfirmationNeededEvent
+
         # Triggering this event causes the confirmation message to be sent.
-        notify(SubscriptionConfirmationNeededEvent(
-            self.mlist, self.token, self.address.email))
+        notify(event_class(
+                self.mlist, self.token, self.address.email))
         # Now we wait for the confirmation.
         raise StopIteration
 
     def _step_do_confirm_verify(self):
-        # Restore a little extra state that can't be stored in the database
-        # (because the order of setattr() on restore is indeterminate), then
-        # continue with the confirmation/verification step.
-        if self.which is WhichSubscriber.address:
-            self.subscriber = self.address
-        else:
-            assert self.which is WhichSubscriber.user
-            self.subscriber = self.user
+        # Restore a little extra state that can't be stored in the database.
+        self._restore_subscriber()
         # Reset the token so it can't be used in a replay attack.
         self._set_token(TokenOwner.no_one)
         # The user has confirmed their subscription request, and also verified
@@ -249,6 +290,10 @@ class RequestMixin:
         self.verified = True
         self.confirmed = True
 
+        if self._workflow is WhichWorkflow.unsubscription:
+            self.member = self.mlist.regular_members.get_member(
+                self.address.email)
+
 
 class VerificationMixin(RequestMixin):
 
@@ -265,7 +310,6 @@ class VerificationMixin(RequestMixin):
                 # 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
 
 
 class ConfirmationMixin(RequestMixin):
@@ -275,11 +319,10 @@ class ConfirmationMixin(RequestMixin):
 
     def _step_confirmation_checks(self):
         # If the subscription has been pre-confirmed, then we can skip the
-        # confirmation check can be skipped.
-        if self.confirmed:
-            return
-        # The user must confirm their subscription.
-        self.push('send_confirmation')
+        # confirmation check.
+        if not self.confirmed:
+            # The user must confirm their subscription.
+            self.push('send_confirmation')
 
 
 class ModerationMixin:
@@ -288,146 +331,36 @@ class ModerationMixin:
         self.approved = pre_approved
 
     def _step_moderation_checks(self):
-        # Does the moderator need to approve the subscription request?
-        if self.approved:
-            return
-        # A moderator must confirm the subscription.
-        self.push('get_moderator_approval')
+        # Does the moderator need to approve the request?
+        if not self.approved:
+            self.push('get_moderator_approval')
 
     def _step_get_moderator_approval(self):
         # Here's the next step in the workflow, assuming the moderator
-        # approves of the subscription.  If they don't, the workflow and
-        # subscription request will just be thrown away.
+        # approves of the request.  If they don't, the workflow and
+        # request will just be thrown away.
         self._set_token(TokenOwner.moderator)
-        self.push('subscribe_from_restored')
-        self.save()
-        log.info('{}: held subscription request from {}'.format(
-            self.mlist.fqdn_listname, self.address.email))
-        # Possibly send a notification to the list moderators.
-        if self.mlist.admin_immed_notify:
-            subject = _(
-                'New subscription request to $self.mlist.display_name '
-                'from $self.address.email')
-            username = formataddr(
-                (self.subscriber.display_name, self.address.email))
-            template = getUtility(ITemplateLoader).get(
-                'list:admin:action:subscribe', self.mlist)
-            text = wrap(expand(template, self.mlist, dict(
-                member=username,
-                )))
-            # This message should appear to come from the <list>-owner so as
-            # to avoid any useless bounce processing.
-            msg = UserNotification(
-                self.mlist.owner_address, self.mlist.owner_address,
-                subject, text, self.mlist.preferred_language)
-            msg.send(self.mlist)
-        # The workflow must stop running here.
-        raise StopIteration
-
-    def _step_subscribe_from_restored(self):
-        # Prevent replay attacks.
-        self._set_token(TokenOwner.no_one)
-        # Restore a little extra state that can't be stored in the database
-        # (because the order of setattr() on restore is indeterminate), then
-        # subscribe the user.
-        if self.which is WhichSubscriber.address:
-            self.subscriber = self.address
-        else:
-            assert self.which is WhichSubscriber.user
-            self.subscriber = self.user
-
-
-class UnsubscriptionBase(SubscriptionWorkflowCommon):
-
-    def __init__(self, mlist, subscriber):
-        super().__init__(mlist, subscriber)
-        if IAddress.providedBy(subscriber) or IUser.providedBy(subscriber):
-            self.member = self.mlist.regular_members.get_member(
-                self.address.email)
-
-    def _step_subscription_checks(self):
-        assert self.mlist.is_subscribed(self.subscriber)
-
-    def _step_do_unsubscription(self):
-        try:
-            delete_member(self.mlist, self.address.email)
-        except NotAMemberError:
-            # The member has already been unsubscribed.
-            pass
-        self.member = None
-        assert self.token is None and self.token_owner is TokenOwner.no_one, (
-            'Unexpected active token at end of subscription workflow')
-
-
-class UnRequestMixin:
-
-    def _step_send_confirmation(self):
-        self._set_token(TokenOwner.subscriber)
-        self.push('do_confirm_verify')
+        self.push('restore')
         self.save()
-        notify(UnsubscriptionConfirmationNeededEvent(
-            self.mlist, self.token, self.address.email))
-        raise StopIteration
 
-    def _step_do_confirm_verify(self):
-        # Restore a little extra state that can't be stored in the database
-        # (because the order of setattr() on restore is indeterminate), then
-        # continue with the confirmation/verification step.
-        if self.which is WhichSubscriber.address:
-            self.subscriber = self.address
+        if self._workflow is WhichWorkflow.subscription:
+            workflow_name = 'subscription'
+            template_name = 'list:admin:action:subscribe'
         else:
-            assert self.which is WhichSubscriber.user
-            self.subscriber = self.user
-        # Reset the token so it can't be used in a replay attack.
-        self._set_token(TokenOwner.no_one)
-        # Restore the member object.
-        self.member = self.mlist.regular_members.get_member(self.address.email)
-        # It's possible the member was already unsubscribed while we were
-        # waiting for the confirmation.
-        if self.member is None:
-            return
-        # The user has confirmed their unsubscription request
-        self.confirmed = True
-
-
-class UnConfirmationMixin(UnRequestMixin):
-
-    def __init__(self, pre_confirmed=False):
-        self.confirmed = pre_confirmed
-
-    def _step_confirmation_checks(self):
-        # If the unsubscription has been pre-confirmed, then we can skip the
-        # confirmation check can be skipped.
-        if self.confirmed:
-            return
-        # The user must confirm their unsubscription.
-        self.push('send_confirmation')
-
+            workflow_name = 'unsubscription'
+            template_name = 'list:admin:action:unsubscribe'
 
-class UnModerationMixin(UnRequestMixin):
-
-    def __init__(self, pre_approved=False):
-        self.approved = pre_approved
-
-    def _step_moderation_checks(self):
-        # Does the moderator need to approve the unsubscription request?
-        if not self.approved:
-            self.push('get_moderator_approval')
-
-    def _step_get_moderator_approval(self):
-        self._set_token(TokenOwner.moderator)
-        self.push('unsubscribe_from_restored')
-        self.save()
-        log.info('{}: held unsubscription request from {}'.format(
-            self.mlist.fqdn_listname, self.address.email))
+        log.info('{}: held {} request from {}'.format(
+            self.mlist.fqdn_listname, workflow_name, self.address.email))
+        # Possibly send a notification to the list moderators.
         if self.mlist.admin_immed_notify:
             subject = _(
-                'New unsubscription request to $self.mlist.display_name '
+                'New $workflow_name request to $self.mlist.display_name '
                 'from $self.address.email')
             username = formataddr(
                 (self.subscriber.display_name, self.address.email))
             template = getUtility(ITemplateLoader).get(
-                'list:admin:action:unsubscribe', self.mlist)
+                template_name, self.mlist)
             text = wrap(expand(template, self.mlist, dict(
                 member=username,
                 )))
@@ -437,14 +370,11 @@ class UnModerationMixin(UnRequestMixin):
                 self.mlist.owner_address, self.mlist.owner_address,
                 subject, text, self.mlist.preferred_language)
             msg.send(self.mlist)
-        # The workflow must stop running here
+        # The workflow must stop running here.
         raise StopIteration
 
-    def _step_unsubscribe_from_restored(self):
+    def _step_restore(self):
         # Prevent replay attacks.
         self._set_token(TokenOwner.no_one)
-        if self.which is WhichSubscriber.address:
-            self.subscriber = self.address
-        else:
-            assert self.which is WhichSubscriber.user
-            self.subscriber = self.user
+        # Restore a little extra state that can't be stored in the database.
+        self._restore_subscriber()