diff options
21 files changed, 96 insertions, 358 deletions
diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index c7aaef43d..26c3b1d17 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -24,6 +24,7 @@ from datetime import timedelta from email.utils import formataddr from enum import Enum from mailman import public +from mailman.app.membership import delete_member from mailman.app.workflow import Workflow from mailman.core.i18n import _ from mailman.database.transaction import flush @@ -32,7 +33,7 @@ from mailman.interfaces.address import IAddress from mailman.interfaces.bans import IBanManager from mailman.interfaces.listmanager import ListDeletingEvent from mailman.interfaces.mailinglist import SubscriptionPolicy -from mailman.interfaces.member import MembershipIsBannedError +from mailman.interfaces.member import MembershipIsBannedError, NotAMemberError from mailman.interfaces.pending import IPendable, IPendings from mailman.interfaces.workflowmanager import ConfirmationNeededEvent from mailman.interfaces.subscriptions import ( @@ -335,6 +336,7 @@ class SubscriptionWorkflow(Workflow): self.push(next_step) +@public class UnSubscriptionWorkflow(Workflow): """Workflow of a un-subscription request.""" @@ -445,17 +447,14 @@ class UnSubscriptionWorkflow(Workflow): self.push('do_unsubscription') return # If we don't need the user's confirmation, then skip to the moderation - # checks + # checks. if self.mlist.unsubscription_policy is SubscriptionPolicy.moderate: self.push('moderation_checks') return - + # If the request is pre-confirmed, then the user can unsubscribe right + # now. if self.pre_confirmed: - next_step = ('moderation_checks' - if self.mlist.subscription_policy is - SubscriptionPolicy.confirm_then_moderate # noqa - else 'do_subscription') - self.push(next_step) + self.push('do_unsubscription') return # The user must confirm their un-subsbcription. self.push('send_confirmation') @@ -469,11 +468,11 @@ class UnSubscriptionWorkflow(Workflow): raise StopIteration def _step_moderation_checks(self): - # Does the moderator need to approve the unsubscription request. + # Does the moderator need to approve the unsubscription request? assert self.mlist.unsubscription_policy in ( SubscriptionPolicy.moderate, SubscriptionPolicy.confirm_then_moderate, - ), self.mlist.unsubscription_policy + ), self.mlist.unsubscription_policy if self.pre_approved: self.push('do_unsubscription') else: @@ -491,11 +490,11 @@ class UnSubscriptionWorkflow(Workflow): 'from $self.address.email') username = formataddr( (self.subscriber.display_name, self.address.email)) - text = make('unsubauth.txt', - mailing_list=self.mlist, - username=username, - listname=self.mlist.fqdn_listname, - ) + template = getUtility(ITemplateLoader).get( + 'list:admin:action:unsubscribe', 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( @@ -506,26 +505,34 @@ class UnSubscriptionWorkflow(Workflow): 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 - # Reset the token so it can't be used in a replay attack. + # Reset the token so it can't be used in a replay attack. self._set_token(TokenOwner.no_one) + # The user has confirmed their unsubscription request next_step = ('moderation_checks' if self.mlist.unsubscription_policy in ( SubscriptionPolicy.moderate, SubscriptionPolicy.confirm_then_moderate, ) else 'do_unsubscription') - self.push('do_unsubscription') + self.push(next_step) def _step_do_unsubscription(self): - delete_member(self.mlist, self.address.email) + try: + delete_member(self.mlist, self.address.email) + except NotAMemberError: + # The member has already been unsubscribed. + pass self.member = None # This workflow is done so throw away any associated state. - getUtility(IWorkflowStateManager).restore(self.name, self.token) + getUtility(IWorkflowStateManager).restore(self.name, self.token) def _step_unsubscribe_from_restored(self): # Prevent replay attacks. @@ -533,7 +540,7 @@ class UnSubscriptionWorkflow(Workflow): if self.which is WhichSubscriber.address: self.subscriber = self.address else: - assert self.which is WhichSubsriber.user + assert self.which is WhichSubscriber.user self.subscriber = self.user self.push('do_unsubscription') @@ -569,7 +576,7 @@ class SubscriptionWorkflowManager(BaseSubscriptionManager): def register(self, subscriber=None, *, pre_verified=False, pre_confirmed=False, pre_approved=False): - """See `IWorkflowManager`.""" + """See `ISubscriptionManager`.""" workflow = SubscriptionWorkflow( self._mlist, subscriber, pre_verified=pre_verified, diff --git a/src/mailman/app/tests/test_moderation.py b/src/mailman/app/tests/test_moderation.py index a7fc9e1b4..9217ef4f5 100644 --- a/src/mailman/app/tests/test_moderation.py +++ b/src/mailman/app/tests/test_moderation.py @@ -24,7 +24,7 @@ from mailman.app.moderator import ( handle_message, handle_unsubscription, hold_message, hold_unsubscription) from mailman.interfaces.action import Action from mailman.interfaces.messages import IMessageStore -from mailman.interfaces.workflowmanager import IWorkflowManager +from mailman.interfaces.subscriptions import ISubscriptionManager from mailman.interfaces.requests import IListRequests from mailman.interfaces.usermanager import IUserManager from mailman.runners.incoming import IncomingRunner @@ -154,7 +154,7 @@ class TestUnsubscription(unittest.TestCase): def setUp(self): self._mlist = create_list('test@example.com') self._registrar = getAdapter( - self._mlist, IWorkflowManager, name='subscribe') + self._mlist, ISubscriptionManager, name='subscribe') def test_unsubscribe_defer(self): # When unsubscriptions must be approved by the moderator, but the diff --git a/src/mailman/app/tests/test_unsubscriptions.py b/src/mailman/app/tests/test_unsubscriptions.py index 9c7b27b87..c4f36e78a 100644 --- a/src/mailman/app/tests/test_unsubscriptions.py +++ b/src/mailman/app/tests/test_unsubscriptions.py @@ -23,14 +23,11 @@ import unittest from contextlib import suppress from mailman.app.lifecycle import create_list from mailman.app.unsubscriptions import UnSubscriptionWorkflow -from mailman.interfaces.bans import IBanManager from mailman.interfaces.mailinglist import SubscriptionPolicy -from mailman.interfaces.member import MembershipIsBannedError from mailman.interfaces.pending import IPendings from mailman.interfaces.subscriptions import TokenOwner from mailman.interfaces.usermanager import IUserManager -from mailman.testing.helpers import ( - LogFileMark, get_queue_messages, set_preferred) +from mailman.testing.helpers import LogFileMark, get_queue_messages from mailman.testing.layers import ConfigLayer from mailman.utilities.datetime import now from unittest.mock import patch @@ -38,7 +35,6 @@ from zope.component import getUtility class TestUnSubscriptionWorkflow(unittest.TestCase): - layer = ConfigLayer maxDiff = None @@ -62,8 +58,9 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): self.assertIsNone(workflow.member) def test_pended_data(self): - # Test there is a Pendable object associated with a held un-subscription - # request and it has some valid data associated with it. + # Test there is a Pendable object associated with a held + # un-subscription request and it has some valid data associated with + # it. workflow = UnSubscriptionWorkflow(self._mlist, self.anne) with suppress(StopIteration): workflow.run_thru('send_confirmation') @@ -115,8 +112,8 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): # un-subscription is pre-confirmed. Since moderation is not reuqired, # the user will be immediately un-subscribed. self._mlist.unsubscription_policy = SubscriptionPolicy.confirm - workflow = UnSubscriptionWorkflow(self._mlist, self.anne, - pre_confirmed=True) + workflow = UnSubscriptionWorkflow( + self._mlist, self.anne, pre_confirmed=True) workflow.run_thru('confirmation_checks') with patch.object(workflow, '_step_do_unsubscription') as step: next(workflow) @@ -128,8 +125,8 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): # check will be performed. self._mlist.unsubscription_policy = ( SubscriptionPolicy.confirm_then_moderate) - workflow = UnSubscriptionWorkflow(self._mlist, self.anne, - pre_confirmed=True) + workflow = UnSubscriptionWorkflow( + self._mlist, self.anne, pre_confirmed=True) workflow.run_thru('confirmation_checks') with patch.object(workflow, '_step_do_unsubscription') as step: next(workflow) @@ -173,9 +170,9 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): self.assertIsNone(member) def test_do_unsubscription_pre_approved(self): - # A moderation-requiring subscription policy plus a pre-approved address - # means the user gets un-subscribed from the mailing list without any - # further confirmation or approvals. + # A moderation-requiring subscription policy plus a pre-approved + # address means the user gets un-subscribed from the mailing list + # without any further confirmation or approvals. self._mlist.unsubscription_policy = SubscriptionPolicy.moderate workflow = UnSubscriptionWorkflow(self._mlist, self.anne, pre_approved=True) @@ -199,7 +196,7 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): list(workflow) member = self._mlist.regular_members.get_member(self._anne) self.assertIsNone(member) - # No further token is needed. + # No further token is needed. self.assertIsNone(workflow.token) self.assertEqual(workflow.token_owner, TokenOwner.no_one) @@ -233,8 +230,8 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): # point the workflow is saved. Once the moderator approves, the # workflow resumes and the user is un-subscribed. self._mlist.unsubscription_policy = SubscriptionPolicy.moderate - workflow = UnSubscriptionWorkflow(self._mlist, self.anne, - pre_confirmed=True) + workflow = UnSubscriptionWorkflow( + self._mlist, self.anne, pre_confirmed=True) # Run the entire workflow. list(workflow) # The user is currently subscribed to the mailing list. @@ -264,22 +261,22 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): # logged. mark = LogFileMark('mailman.subscribe') self._mlist.unsubscription_policy = SubscriptionPolicy.moderate - workflow = UnSubscriptionWorkflow(self._mlist, self.anne, - pre_confirmed=True) + workflow = UnSubscriptionWorkflow( + self._mlist, self.anne, pre_confirmed=True) # Run the entire workflow. list(workflow) self.assertIn( - 'test@example.com: held unsubscription request from anne@example.com', - mark.readline() - ) + 'test@example.com: held unsubscription request from anne@example.com', + mark.readline() + ) def test_get_moderator_approval_notifies_moderators(self): # When the un-subscription is held for moderator approval, and the list # is so configured, a notification is sent to the list moderators. self._mlist.admin_immed_notify = True self._mlist.unsubscription_policy = SubscriptionPolicy.moderate - workflow = UnSubscriptionWorkflow(self._mlist, self.anne, - pre_confirmed=True) + workflow = UnSubscriptionWorkflow( + self._mlist, self.anne, pre_confirmed=True) # Consume the entire state machine. list(workflow) items = get_queue_messages('virgin', expected_count=1) @@ -302,8 +299,8 @@ request approval: # moderators. self._mlist.admin_immed_notify = False self._mlist.unsubscription_policy = SubscriptionPolicy.moderate - workflow = UnSubscriptionWorkflow(self._mlist, self.anne, - pre_confirmed=True) + workflow = UnSubscriptionWorkflow( + self._mlist, self.anne, pre_confirmed=True) # Consume the entire state machine. list(workflow) get_queue_messages('virgin', expected_count=0) diff --git a/src/mailman/app/tests/test_workflowmanager.py b/src/mailman/app/tests/test_workflowmanager.py index a6c5f92ca..047291838 100644 --- a/src/mailman/app/tests/test_workflowmanager.py +++ b/src/mailman/app/tests/test_workflowmanager.py @@ -18,14 +18,12 @@ """Test email address registration.""" import unittest -import pdb from mailman.app.lifecycle import create_list from mailman.interfaces.mailinglist import SubscriptionPolicy from mailman.interfaces.member import MemberRole from mailman.interfaces.pending import IPendings -from mailman.interfaces.workflowmanager import IWorkflowManager -from mailman.interfaces.subscriptions import TokenOwner +from mailman.interfaces.subscriptions import ISubscriptionManager, TokenOwner from mailman.interfaces.usermanager import IUserManager from mailman.testing.helpers import get_queue_messages from mailman.testing.layers import ConfigLayer @@ -41,7 +39,7 @@ class TestRegistrar(unittest.TestCase): def setUp(self): self._mlist = create_list('ant@example.com') self._registrar = getAdapter( - self._mlist, IWorkflowManager, name='subscribe') + self._mlist, ISubscriptionManager, name='subscribe') self._pendings = getUtility(IPendings) self._anne = getUtility(IUserManager).create_address( 'anne@example.com') diff --git a/src/mailman/app/unsubscriptions.py b/src/mailman/app/unsubscriptions.py deleted file mode 100644 index 1a88b121f..000000000 --- a/src/mailman/app/unsubscriptions.py +++ /dev/null @@ -1,255 +0,0 @@ -# Copyright (C) 2016 by the Free Software Foundation, Inc. -# -# This file is part of GNU Mailman. -# -# GNU Mailman is free software: you can redistribute it and/or modify it under -# the terms of the GNU General Public License as published by the Free -# Software Foundation, either version 3 of the License, or (at your option) -# any later version. -# -# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -# more details. -# -# You should have received a copy of the GNU General Public License along with -# GNU Mailman. If not, see <http://www.gnu.org/licenses/>. - -"""Handle un-subscriptions.""" - -import uuid -import logging - -from datetime import timedelta -from email.utils import formataddr -from mailman import public -from mailman.app.membership import delete_member -from mailman.app.subscriptions import WhichSubscriber -from mailman.app.workflow import Workflow -from mailman.core.i18n import _ -from mailman.email.message import UserNotification -from mailman.interfaces.address import IAddress -from mailman.interfaces.mailinglist import SubscriptionPolicy -from mailman.interfaces.workflowmanager import ConfirmationNeededEvent -from mailman.interfaces.user import IUser -from mailman.interfaces.pending import IPendings, IPendable -from mailman.interfaces.subscriptions import TokenOwner -from mailman.interfaces.usermanager import IUserManager -from mailman.interfaces.workflow import IWorkflowStateManager -from mailman.utilities.datetime import now -from mailman.utilities.i18n import make -from zope.component import getUtility -from zope.event import notify -from zope.interface import implementer - - -log = logging.getLogger('mailman.subscribe') - - -@implementer(IPendable) -class Pendable(dict): - PEND_TYPE = 'unsubscription' - - -class UnSubscriptionWorkflow(Workflow): - """Workflow of a un-subscription request - """ - - INITIAL_STATE = 'subscription_checks' - SAVE_ATTRIBUTES = ( - 'pre_approved', - 'pre_confirmed', - 'address_key', - 'user_key', - 'subscriber_key', - 'token_owner_key', - ) - - def __init__(self, mlist, subscriber=None, *, - pre_approved=False, pre_confirmed = False): - super().__init__() - self.mlist = mlist - self.address = None - self.user = None - self.which = None - self._set_token(TokenOwner.no_one) - # `subscriber` should be an implementer of IAddress. - if IAddress.providedBy(subscriber): - self.address = subscriber - self.user = self.address.user - self.which = WhichSubscriber.address - self.member = self.mlist.regular_members.get_member( - self.address.email) - elif IUser.providedBy(subscriber): - self.address = subscriber.preferred_address - self.user = subscriber - self.which = WhichSubscriber.address - self.member = self.mlist.regular_members.get_member( - self.address.email) - self.subscriber = subscriber - self.pre_confirmed = pre_confirmed - self.pre_approved = pre_approved - - @property - def user_key(self): - # For save. - return self.user.user_id.hex - - @user_key.setter - def user_key(self, hex_key): - # For restore. - uid = uuid.UUID(hex_key) - self.user = getUtility(IUserManager).get_user_by_id(uid) - assert self.user is not None - - @property - def address_key(self): - # For save. - return self.address.email - - @address_key.setter - def address_key(self, email): - # For restore. - self.address = getUtility(IUserManager).get_address(email) - assert self.address is not None - - @property - def subscriber_key(self): - return self.which.value - - @subscriber_key.setter - def subscriber_key(self, key): - self.which = WhichSubscriber(key) - - @property - def token_owner_key(self): - return self.token_owner.value - - @token_owner_key.setter - def token_owner_key(self, value): - self.token_owner = TokenOwner(value) - - def _set_token(self, token_owner): - assert isinstance(token_owner, TokenOwner) - pendings = getUtility(IPendings) - # Clear out the previous pending token if there is one. - if self.token is not None: - pendings.confirm(self.token) - # Create a new token to prevent replay attacks. It seems like this - # would produce the same token, but it won't because the pending adds a - # bit of randomization. - self.token_owner = token_owner - if token_owner is TokenOwner.no_one: - self.token = None - return - pendable = Pendable( - list_id=self.mlist.list_id, - email=self.address.email, - display_name=self.address.display_name, - when=now().replace(microsecond=0).isoformat(), - token_owner=token_owner.name, - ) - self.token = pendings.add(pendable, timedelta(days=3650)) - - def _step_subscription_checks(self): - assert self.mlist.is_subscribed(self.subscriber) - self.push('confirmation_checks') - - def _step_confirmation_checks(self): - # If list's unsubscription policy is open, the user can unsubscribe - # right now. - if self.mlist.unsubscription_policy is SubscriptionPolicy.open: - self.push('do_unsubscription') - return - # If we don't need the user's confirmation, then skip to the moderation - # checks - if self.mlist.unsubscription_policy is SubscriptionPolicy.moderate: - self.push('moderation_checks') - return - - if self.pre_confirmed: - next_step = ('moderation_checks' - if self.mlist.subscription_policy is - SubscriptionPolicy.confirm_then_moderate # noqa - else 'do_subscription') - self.push(next_step) - return - # The user must confirm their un-subsbcription. - self.push('send_confirmation') - - def _step_send_confirmation(self): - self._set_token(TokenOwner.subscriber) - self.push('do_confirm_verify') - self.save() - notify(ConfirmationNeededEvent( - self.mlist, self.token, self.address.email)) - raise StopIteration - - def _step_moderation_checks(self): - # Does the moderator need to approve the unsubscription request. - assert self.mlist.unsubscription_policy in ( - SubscriptionPolicy.moderate, - SubscriptionPolicy.confirm_then_moderate, - ), self.mlist.unsubscription_policy - if self.pre_approved: - self.push('do_unsubscription') - else: - 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)) - if self.mlist.admin_immed_notify: - subject = _( - 'New unsubscription request to $self.mlist.display_name ' - 'from $self.address.email') - username = formataddr( - (self.subscriber.display_name, self.address.email)) - text = make('unsubauth.txt', - mailing_list=self.mlist, - username=username, - listname=self.mlist.fqdn_listname, - ) - # 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, tomoderators=True) - # The workflow must stop running here - raise StopIteration - - def _step_do_confirm_verify(self): - if self.which is WhichSubscriber.address: - self.subscriber = self.address - 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) - next_step = ('moderation_checks' - if self.mlist.unsubscription_policy in ( - SubscriptionPolicy.moderate, - SubscriptionPolicy.confirm_then_moderate, - ) - else 'do_unsubscription') - self.push('do_unsubscription') - - def _step_do_unsubscription(self): - delete_member(self.mlist, self.address.email) - self.member = None - # This workflow is done so throw away any associated state. - getUtility(IWorkflowStateManager).restore(self.name, self.token) - - def _step_unsubscribe_from_restored(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 WhichSubsriber.user - self.subscriber = self.user - self.push('do_unsubscription') diff --git a/src/mailman/commands/eml_membership.py b/src/mailman/commands/eml_membership.py index af391fd5b..fad1cbe50 100644 --- a/src/mailman/commands/eml_membership.py +++ b/src/mailman/commands/eml_membership.py @@ -25,7 +25,7 @@ from mailman.interfaces.member import DeliveryMode, MemberRole from mailman.interfaces.workflowmanager import IWorkflowManager from mailman.interfaces.subscriptions import ISubscriptionService from mailman.interfaces.usermanager import IUserManager -from zope.component import getUtility, getAdapter +from zope.component import getAdapter, getUtility from zope.interface import implementer @@ -101,8 +101,8 @@ used. print(_('$person is already a member'), file=results) return ContinueProcessing.yes subscriber = match_subscriber(email, display_name) - getAdapter(mlist, - IWorkflowManager, name='subscribe').register(subscriber) + getAdapter( + mlist, IWorkflowManager, name='subscribe').register(subscriber) print(_('Confirmation email sent to $person'), file=results) return ContinueProcessing.yes @@ -187,8 +187,8 @@ You may be asked to confirm your request.""") '$self.name: $email is not a member of $mlist.fqdn_listname'), file=results) return ContinueProcessing.no - getAdapter(mlist, - IWorkflowManager, name='unsubscribe').register(user_address) + getAdapter( + mlist, IWorkflowManager, name='unsubscribe').register(user_address) # member.unsubscribe() person = formataddr((user.display_name, email)) # noqa print(_('Confirmation email sent to $person to leave' diff --git a/src/mailman/commands/tests/test_confirm.py b/src/mailman/commands/tests/test_confirm.py index d7414fbfa..797edcad5 100644 --- a/src/mailman/commands/tests/test_confirm.py +++ b/src/mailman/commands/tests/test_confirm.py @@ -30,7 +30,7 @@ from mailman.interfaces.usermanager import IUserManager from mailman.runners.command import CommandRunner, Results from mailman.testing.helpers import get_queue_messages, make_testable_runner from mailman.testing.layers import ConfigLayer -from zope.component import getUtility, getAdapter +from zope.component import getAdapter, getUtility class TestConfirm(unittest.TestCase): diff --git a/src/mailman/config/configure.zcml b/src/mailman/config/configure.zcml index 153a8b9bd..98cfa1232 100644 --- a/src/mailman/config/configure.zcml +++ b/src/mailman/config/configure.zcml @@ -48,16 +48,16 @@ <adapter for="mailman.interfaces.mailinglist.IMailingList" - provides="mailman.interfaces.workflowmanager.IWorkflowManager" + provides="mailman.interfaces.subscriptions.ISubscriptionManager" factory="mailman.app.workflowmanager.SubscriptionWorkflowManager" - name='subscribe' - /> + name='subscribe' + /> <adapter for="mailman.interfaces.mailinglist.IMailingList" - provides="mailman.interfaces.workflowmanager.IWorkflowManager" + provides="mailman.interfaces.subscriptions.ISubscriptionManager" factory="mailman.app.workflowmanager.UnsubscriptionWorkflowManager" - name='unsubscribe' + name='unsubscribe' /> <utility diff --git a/src/mailman/database/alembic/versions/448a93984c35_unsubscription_workflow.py b/src/mailman/database/alembic/versions/448a93984c35_unsubscription_workflow.py index 51be76051..64fcdffcd 100644 --- a/src/mailman/database/alembic/versions/448a93984c35_unsubscription_workflow.py +++ b/src/mailman/database/alembic/versions/448a93984c35_unsubscription_workflow.py @@ -3,32 +3,34 @@ Revision ID: 448a93984c35 Revises: 7b254d88f122 Create Date: 2016-06-02 14:34:24.154723 - """ -# revision identifiers, used by Alembic. -revision = '448a93984c35' -down_revision = '7b254d88f122' - import sqlalchemy as sa from alembic import op -from mailman.database.helpers import is_sqlite, exists_in_db +from mailman.database.helpers import exists_in_db from mailman.database.types import Enum from mailman.interfaces.mailinglist import SubscriptionPolicy + +# revision identifiers, used by Alembic. +revision = '448a93984c35' +down_revision = '7b254d88f122' + + def upgrade(): if not exists_in_db(op.get_bind(), 'mailinglist', 'unsubscription_policy'): - # SQLite may not have removed it when downgrading. - op.add_column('mailinglist', sa.Column( - 'unsubscription_policy', Enum(SubscriptionPolicy), nullable=True)) - + with op.batch_alter_table('mailinglist') as batch_op: + # SQLite may not have removed it when downgrading. + batch_op.add_column('mailinglist', sa.Column( + 'unsubscription_policy', Enum(SubscriptionPolicy), + nullable=True)) # Now migrate the data. Don't import the table definition from the # models, it may break this migration when the model is updated in the # future (see the Alembic doc). mlist = sa.sql.table( 'mailinglist', sa.sql.column('unsubscription_policy', Enum(SubscriptionPolicy)) - ) + ) # There were no enforced subscription policy before, so all lists are # considered open. op.execute(mlist.update().values( @@ -37,6 +39,5 @@ def upgrade(): def downgrade(): - if not is_sqlite(op.get_bind()): - # SQLite does not support dropping columns. - op.drop_column('mailinglist', 'unsubscription_policy') + with op.batch_alter_table('mailinglist') as batch_op: + batch_op.drop_column('mailinglist', 'unsubscription_policy') diff --git a/src/mailman/interfaces/mailinglist.py b/src/mailman/interfaces/mailinglist.py index 40ee95187..366016872 100644 --- a/src/mailman/interfaces/mailinglist.py +++ b/src/mailman/interfaces/mailinglist.py @@ -58,7 +58,6 @@ class SubscriptionPolicy(Enum): confirm_then_moderate = 3 - @public class IMailingList(Interface): """A mailing list.""" diff --git a/src/mailman/model/mailinglist.py b/src/mailman/model/mailinglist.py index 396b8ed3d..4e76e61c1 100644 --- a/src/mailman/model/mailinglist.py +++ b/src/mailman/model/mailinglist.py @@ -457,11 +457,7 @@ class MailingList(Model): Member.role == role, Member.list_id == self._list_id, Member._user == subscriber).first() - - if member: - return True - else: - return False + return member is not None @dbconnection def subscribe(self, store, subscriber, role=MemberRole.member): @@ -470,10 +466,8 @@ class MailingList(Model): email = subscriber.email elif IUser.providedBy(subscriber): email = subscriber.preferred_address.email - if self.is_subscribed(subscriber, role): raise AlreadySubscribedError(self.fqdn_listname, email, role) - member = Member(role=role, list_id=self._list_id, subscriber=subscriber) diff --git a/src/mailman/model/tests/test_mailinglist.py b/src/mailman/model/tests/test_mailinglist.py index 3e3c5814c..0a5c8a5e9 100644 --- a/src/mailman/model/tests/test_mailinglist.py +++ b/src/mailman/model/tests/test_mailinglist.py @@ -17,7 +17,6 @@ """Test MailingLists and related model objects..""" -import pdb import unittest from mailman.app.lifecycle import create_list @@ -106,6 +105,7 @@ class TestMailingList(unittest.TestCase): self._mlist.subscribe(address) self.assertEqual(True, self._mlist.is_subscribed(address)) + class TestListArchiver(unittest.TestCase): layer = ConfigLayer diff --git a/src/mailman/model/tests/test_workflow.py b/src/mailman/model/tests/test_workflow.py index afcba613b..2b6a593a9 100644 --- a/src/mailman/model/tests/test_workflow.py +++ b/src/mailman/model/tests/test_workflow.py @@ -124,7 +124,8 @@ class TestWorkflow(unittest.TestCase): self.assertEqual(self._manager.count, 1) def test_discard(self): - # Discard some workflow state. This is use by IWorkflowManager.discard(). + # Discard some workflow state. This is use by + # IWorkflowManager.discard(). self._manager.save('ant', 'token', 'one') self._manager.save('bee', 'token', 'two') self._manager.save('ant', 'nekot', 'three') diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py index ac3fd96b4..ed2246366 100644 --- a/src/mailman/rest/members.py +++ b/src/mailman/rest/members.py @@ -37,7 +37,7 @@ from mailman.rest.preferences import Preferences, ReadOnlyPreferences from mailman.rest.validator import ( Validator, enum_validator, subscriber_validator) from uuid import UUID -from zope.component import getUtility, getAdapter +from zope.component import getAdapter, getUtility class _MemberBase(CollectionMixin): diff --git a/src/mailman/rest/sub_moderation.py b/src/mailman/rest/sub_moderation.py index a6ef2ad35..f37e62ac5 100644 --- a/src/mailman/rest/sub_moderation.py +++ b/src/mailman/rest/sub_moderation.py @@ -28,8 +28,7 @@ from mailman.rest.helpers import ( CollectionMixin, bad_request, child, conflict, etag, no_content, not_found, okay) from mailman.rest.validator import Validator, enum_validator -from mailman.utilities.i18n import _ -from zope.component import getUtility, getAdapter +from zope.component import getAdapter, getUtility class _ModerationBase: diff --git a/src/mailman/rest/tests/test_membership.py b/src/mailman/rest/tests/test_membership.py index 9dec17fc8..2726a480f 100644 --- a/src/mailman/rest/tests/test_membership.py +++ b/src/mailman/rest/tests/test_membership.py @@ -35,7 +35,7 @@ from mailman.testing.helpers import ( from mailman.testing.layers import ConfigLayer, RESTLayer from mailman.utilities.datetime import now from urllib.error import HTTPError -from zope.component import getUtility, getAdapter +from zope.component import getAdapter, getUtility class TestMembership(unittest.TestCase): diff --git a/src/mailman/rest/tests/test_moderation.py b/src/mailman/rest/tests/test_moderation.py index 1b9febdce..1adc1aa83 100644 --- a/src/mailman/rest/tests/test_moderation.py +++ b/src/mailman/rest/tests/test_moderation.py @@ -32,7 +32,7 @@ from mailman.testing.helpers import ( specialized_message_from_string as mfs) from mailman.testing.layers import RESTLayer from urllib.error import HTTPError -from zope.component import getUtility, getAdapter +from zope.component import getAdapter, getUtility class TestPostModeration(unittest.TestCase): diff --git a/src/mailman/runners/tests/test_confirm.py b/src/mailman/runners/tests/test_confirm.py index d49ba217b..d6bc4c34d 100644 --- a/src/mailman/runners/tests/test_confirm.py +++ b/src/mailman/runners/tests/test_confirm.py @@ -31,7 +31,7 @@ from mailman.testing.helpers import ( get_queue_messages, make_testable_runner, specialized_message_from_string as mfs) from mailman.testing.layers import ConfigLayer -from zope.component import getUtility, getAdapter +from zope.component import getAdapter, getUtility class TestConfirm(unittest.TestCase): diff --git a/src/mailman/runners/tests/test_join.py b/src/mailman/runners/tests/test_join.py index 7409c8e89..b565ca048 100644 --- a/src/mailman/runners/tests/test_join.py +++ b/src/mailman/runners/tests/test_join.py @@ -31,7 +31,7 @@ from mailman.testing.helpers import ( get_queue_messages, make_testable_runner, specialized_message_from_string as mfs) from mailman.testing.layers import ConfigLayer -from zope.component import getUtility, getAdapter +from zope.component import getAdapter, getUtility class TestJoin(unittest.TestCase): diff --git a/src/mailman/runners/tests/test_leave.py b/src/mailman/runners/tests/test_leave.py index 23257124a..64a46a4da 100644 --- a/src/mailman/runners/tests/test_leave.py +++ b/src/mailman/runners/tests/test_leave.py @@ -17,9 +17,9 @@ """Test mailing list un-subscriptions.""" -import pdb import unittest +from email.iterators import body_line_iterator from mailman.app.lifecycle import create_list from mailman.config import config from mailman.database.transaction import transaction @@ -30,8 +30,8 @@ from mailman.testing.helpers import ( specialized_message_from_string as mfs) from mailman.testing.layers import ConfigLayer from mailman.testing.helpers import set_preferred -from mailman.utilities.datetime import now -from zope.component import getUtility, getAdapter +from zope.component import getUtility + class TestLeave(unittest.TestCase): """Test mailing list un-subscriptions""" @@ -49,7 +49,7 @@ class TestLeave(unittest.TestCase): anne = getUtility(IUserManager).create_user('anne@example.org') set_preferred(anne) self._mlist.subscribe(list(anne.addresses)[0]) - msg = mfs("""\ + msg = mfs("""\ From: anne@example.org To: test-leave@example.com @@ -60,9 +60,6 @@ leave self._runner.run() items = get_queue_messages('virgin', sort_on='subject', expected_count=1) - print(items[0].msg) - print(anne.addresses) - pdb.set_trace() self.assertTrue(str(items[0].msg['subject']).startswith('confirm')) confirmation_lines = [] in_results = False diff --git a/src/mailman/styles/base.py b/src/mailman/styles/base.py index 7226d762a..b4c1c336d 100644 --- a/src/mailman/styles/base.py +++ b/src/mailman/styles/base.py @@ -67,7 +67,7 @@ class BasicOperation: mlist.default_member_action = Action.defer mlist.default_nonmember_action = Action.hold mlist.subscription_policy = SubscriptionPolicy.confirm - mlist.unsubscription_policy = SubscriptionPolicy.confirm + mlist.unsubscription_policy = SubscriptionPolicy.open # Notify the administrator of pending requests and membership changes. mlist.admin_immed_notify = True mlist.admin_notify_mchanges = False |