Commit a3ff3d5d361fb40b3aaf5f4951725dd03fa58c48
Exists in
master
and in
28 other branches
Merge branch 'moderated_private_email_lists' into 'master'
Added moderated email lists subscriptions/unsubscriptions notifications See merge request !82
Showing
11 changed files
with
123 additions
and
58 deletions
Show diff stats
colab/accounts/forms.py
| ... | ... | @@ -141,8 +141,9 @@ class UserUpdateForm(UserForm): |
| 141 | 141 | |
| 142 | 142 | class ListsForm(forms.Form): |
| 143 | 143 | LISTS_NAMES = (( |
| 144 | - listname, u'{} ({})'.format(listname, description) | |
| 145 | - ) for listname, description in mailman.all_lists(description=True)) | |
| 144 | + mlist.get('listname'), u'{} ({})'.format(mlist.get('listname'), | |
| 145 | + mlist.get('description')) | |
| 146 | + ) for mlist in mailman.all_lists()) | |
| 146 | 147 | |
| 147 | 148 | lists = forms.MultipleChoiceField(label=_(u'Mailing lists'), |
| 148 | 149 | required=False, | ... | ... |
colab/accounts/models.py
| ... | ... | @@ -60,7 +60,7 @@ class User(AbstractUser): |
| 60 | 60 | return mailman.user_lists(self) |
| 61 | 61 | |
| 62 | 62 | def update_subscription(self, email, lists): |
| 63 | - mailman.update_subscription(email, lists) | |
| 63 | + return mailman.update_subscription(email, lists) | |
| 64 | 64 | |
| 65 | 65 | def save(self, *args, **kwargs): |
| 66 | 66 | ... | ... |
colab/accounts/templates/accounts/user_detail.html
colab/accounts/utils/mailman.py
| ... | ... | @@ -4,66 +4,99 @@ import requests |
| 4 | 4 | import logging |
| 5 | 5 | |
| 6 | 6 | from django.conf import settings |
| 7 | +from django.contrib import messages | |
| 7 | 8 | |
| 8 | 9 | TIMEOUT = 1 |
| 9 | 10 | |
| 10 | 11 | LOGGER = logging.getLogger('colab.mailman') |
| 11 | 12 | |
| 12 | - | |
| 13 | -def get_url(listname=None): | |
| 13 | +S = 'success' | |
| 14 | +I = 'info' | |
| 15 | +E = 'error' | |
| 16 | + | |
| 17 | +MAILMAN_MSGS = { | |
| 18 | + 0: (S, '%s: Success!'), | |
| 19 | + 1: (S, '%s: An email confirmation was sent to you, please check your inbox.'), | |
| 20 | + 2: (I, '%s: Your subscription was sent successfully! Please wait for the list\'s admin approval.'), | |
| 21 | + 3: (I, '%s: You are already a member of this list.'), | |
| 22 | + 4: (E, '%s: You are banned from this list!'), | |
| 23 | + 5: (E, '%s: You appear to have an invalid email address.'), | |
| 24 | + 6: (E, '%s: Your email address is considered to be hostile.'), | |
| 25 | + 7: (E, '%s: You are not a member of this list.'), | |
| 26 | + 8: (E, 'Missing information: `email_from`, `subject` and `body` are mandatory.'), | |
| 27 | +} | |
| 28 | + | |
| 29 | + | |
| 30 | +def get_url(path, listname=None): | |
| 31 | + url = urlparse.urljoin(settings.MAILMAN_API_URL, path) | |
| 14 | 32 | if listname: |
| 15 | - return urlparse.urljoin(settings.MAILMAN_API_URL, '/' + listname) | |
| 16 | - | |
| 17 | - return settings.MAILMAN_API_URL | |
| 33 | + return urlparse.urljoin(url, listname) | |
| 34 | + return url | |
| 18 | 35 | |
| 19 | 36 | |
| 20 | 37 | def subscribe(listname, address): |
| 21 | - url = get_url(listname) | |
| 38 | + url = get_url('subscribe/', listname=listname) | |
| 22 | 39 | try: |
| 23 | - requests.put(url, timeout=TIMEOUT, data={'address': address}) | |
| 40 | + result = requests.put(url, timeout=TIMEOUT, data={'address': address}) | |
| 41 | + msg_type, message = MAILMAN_MSGS[result.json()] | |
| 42 | + return msg_type, message % listname | |
| 24 | 43 | except: |
| 25 | 44 | LOGGER.exception('Unable to subscribe user') |
| 26 | - return False | |
| 27 | - return True | |
| 45 | + return E, 'Error: Unable to subscribe user' | |
| 28 | 46 | |
| 29 | 47 | |
| 30 | 48 | def unsubscribe(listname, address): |
| 31 | - url = get_url(listname) | |
| 49 | + url = get_url('subscribe/', listname) | |
| 32 | 50 | try: |
| 33 | - requests.delete(url, timeout=TIMEOUT, data={'address': address}) | |
| 51 | + result = requests.delete(url, timeout=TIMEOUT, data={'address': address}) | |
| 52 | + msg_type, message = MAILMAN_MSGS[result.json()] | |
| 53 | + return msg_type, message % listname | |
| 34 | 54 | except: |
| 35 | 55 | LOGGER.exception('Unable to unsubscribe user') |
| 36 | - return False | |
| 37 | - return True | |
| 56 | + return E, 'Error: Unable to subscribe user' | |
| 38 | 57 | |
| 39 | 58 | |
| 40 | 59 | def update_subscription(address, lists): |
| 41 | - current_lists = mailing_lists(address=address) | |
| 60 | + current_lists = mailing_lists(address=address, names_only=True) | |
| 61 | + info_messages = [] | |
| 42 | 62 | |
| 43 | 63 | for maillist in current_lists: |
| 44 | 64 | if maillist not in lists: |
| 45 | - unsubscribe(maillist, address) | |
| 65 | + info_messages.append(unsubscribe(maillist, address)) | |
| 46 | 66 | |
| 47 | 67 | for maillist in lists: |
| 48 | 68 | if maillist not in current_lists: |
| 49 | - subscribe(maillist, address) | |
| 69 | + info_messages.append(subscribe(maillist, address)) | |
| 70 | + | |
| 71 | + return info_messages | |
| 50 | 72 | |
| 51 | 73 | |
| 52 | 74 | def mailing_lists(**kwargs): |
| 53 | - url = get_url() | |
| 75 | + url = get_url('lists/') | |
| 54 | 76 | |
| 55 | 77 | try: |
| 56 | - lists = requests.get(url, timeout=TIMEOUT, params=kwargs) | |
| 78 | + lists = requests.get(url, timeout=TIMEOUT, params=kwargs).json() | |
| 79 | + if not isinstance(lists, (list, tuple)): | |
| 80 | + raise | |
| 57 | 81 | except: |
| 58 | 82 | LOGGER.exception('Unable to list mailing lists') |
| 59 | 83 | return [] |
| 60 | 84 | |
| 61 | - return lists.json() | |
| 85 | + if kwargs.get('names_only'): | |
| 86 | + names_only = [] | |
| 87 | + for l in lists: | |
| 88 | + names_only.append(l['listname']) | |
| 89 | + return names_only | |
| 90 | + else: | |
| 91 | + return lists | |
| 62 | 92 | |
| 63 | 93 | |
| 64 | 94 | def is_private_list(name): |
| 65 | 95 | try: |
| 66 | - return dict(all_lists(private=True))[name] | |
| 96 | + privacy = {} | |
| 97 | + privacy.update({mlist.get('listname'): mlist.get('archive_private') | |
| 98 | + for mlist in all_lists()}) | |
| 99 | + return privacy[name] | |
| 67 | 100 | except KeyError: |
| 68 | 101 | return [] |
| 69 | 102 | |
| ... | ... | @@ -76,22 +109,25 @@ def user_lists(user): |
| 76 | 109 | list_set = set() |
| 77 | 110 | |
| 78 | 111 | for email in user.emails.values_list('address', flat=True): |
| 79 | - list_set.update(mailing_lists(address=email)) | |
| 112 | + mlists = mailing_lists(address=email) | |
| 113 | + list_set.update(mlist.get('listname') for mlist in mlists) | |
| 80 | 114 | |
| 81 | 115 | return tuple(list_set) |
| 82 | 116 | |
| 83 | 117 | |
| 84 | 118 | def get_list_description(listname, lists=None): |
| 85 | 119 | if not lists: |
| 86 | - lists = dict(all_lists(description=True)) | |
| 87 | - elif not isinstance(lists, dict): | |
| 88 | - lists = dict(lists) | |
| 120 | + lists = all_lists() | |
| 121 | + | |
| 122 | + desc = "".join(mlist.get('description') for mlist in lists | |
| 123 | + if isinstance(mlist, dict) and | |
| 124 | + mlist.get('listname') == listname) | |
| 89 | 125 | |
| 90 | - return lists.get(listname) | |
| 126 | + return desc | |
| 91 | 127 | |
| 92 | 128 | |
| 93 | 129 | def list_users(listname): |
| 94 | - url = get_url(listname) | |
| 130 | + url = get_url('members/', listname) | |
| 95 | 131 | |
| 96 | 132 | params = {} |
| 97 | 133 | |
| ... | ... | @@ -114,3 +150,10 @@ def get_user_mailinglists(user): |
| 114 | 150 | lists_for_user.extend(mailing_lists(address=email)) |
| 115 | 151 | |
| 116 | 152 | return lists_for_user |
| 153 | + | |
| 154 | + | |
| 155 | +def extract_listname_from_list(lists): | |
| 156 | + try: | |
| 157 | + return [mlist.get('listname') for mlist in lists] | |
| 158 | + except ValueError: | |
| 159 | + return [] | ... | ... |
colab/accounts/views.py
| ... | ... | @@ -144,7 +144,11 @@ class ManageUserSubscriptionsView(UserProfileBaseMixin, DetailView): |
| 144 | 144 | user = self.get_object() |
| 145 | 145 | for email in user.emails.values_list('address', flat=True): |
| 146 | 146 | lists = self.request.POST.getlist(email) |
| 147 | - user.update_subscription(email, lists) | |
| 147 | + info_messages = user.update_subscription(email, lists) | |
| 148 | + for msg_type, message in info_messages: | |
| 149 | + show_message = getattr(messages, msg_type) | |
| 150 | + show_message(request, _(message)) | |
| 151 | + | |
| 148 | 152 | |
| 149 | 153 | return redirect('user_profile', username=user.username) |
| 150 | 154 | |
| ... | ... | @@ -154,18 +158,19 @@ class ManageUserSubscriptionsView(UserProfileBaseMixin, DetailView): |
| 154 | 158 | |
| 155 | 159 | user = self.get_object() |
| 156 | 160 | emails = user.emails.values_list('address', flat=True) |
| 157 | - all_lists = mailman.all_lists(description=True) | |
| 161 | + all_lists = mailman.all_lists() | |
| 158 | 162 | |
| 159 | 163 | for email in emails: |
| 160 | 164 | lists = [] |
| 161 | - lists_for_address = mailman.mailing_lists(address=email) | |
| 162 | - for listname, description in all_lists: | |
| 163 | - if listname in lists_for_address: | |
| 165 | + lists_for_address = mailman.mailing_lists(address=email, names_only=True) | |
| 166 | + for mlist in all_lists: | |
| 167 | + if mlist.get('listname') in lists_for_address: | |
| 164 | 168 | checked = True |
| 165 | 169 | else: |
| 166 | 170 | checked = False |
| 167 | 171 | lists.append(( |
| 168 | - {'listname': listname, 'description': description}, | |
| 172 | + {'listname': mlist.get('listname'), | |
| 173 | + 'description': mlist.get('description')}, | |
| 169 | 174 | checked |
| 170 | 175 | )) |
| 171 | 176 | ... | ... |
colab/home/views.py
| ... | ... | @@ -10,9 +10,10 @@ from colab.accounts.models import User |
| 10 | 10 | |
| 11 | 11 | def get_user_threads(threads, lists_for_user, key): |
| 12 | 12 | visible_threads = [] |
| 13 | + listnames_for_user = mailman.extract_listname_from_list(lists_for_user) | |
| 13 | 14 | for t in threads: |
| 14 | 15 | if not t.mailinglist.is_private or \ |
| 15 | - t.mailinglist.name in lists_for_user: | |
| 16 | + t.mailinglist.name in listnames_for_user: | |
| 16 | 17 | visible_threads.append(key(t)) |
| 17 | 18 | |
| 18 | 19 | return visible_threads | ... | ... |
colab/search/utils.py
| ... | ... | @@ -15,11 +15,12 @@ from colab.accounts.utils import mailman |
| 15 | 15 | |
| 16 | 16 | def get_visible_threads_queryset(logged_user): |
| 17 | 17 | queryset = Thread.objects |
| 18 | - lists_for_user = [] | |
| 18 | + listnames_for_user = [] | |
| 19 | 19 | if logged_user: |
| 20 | 20 | lists_for_user = mailman.get_user_mailinglists(logged_user) |
| 21 | + listnames_for_user = mailman.extract_listname_from_list(lists_for_user) | |
| 21 | 22 | |
| 22 | - user_lists = Condition(mailinglist__name__in=lists_for_user) | |
| 23 | + user_lists = Condition(mailinglist__name__in=listnames_for_user) | |
| 23 | 24 | public_lists = Condition(mailinglist__is_private=False) |
| 24 | 25 | queryset = Thread.objects.filter(user_lists | public_lists) |
| 25 | 26 | |
| ... | ... | @@ -28,6 +29,7 @@ def get_visible_threads_queryset(logged_user): |
| 28 | 29 | |
| 29 | 30 | def get_visible_threads(logged_user, filter_by_user=None): |
| 30 | 31 | thread_qs = get_visible_threads_queryset(logged_user) |
| 32 | + | |
| 31 | 33 | if filter_by_user: |
| 32 | 34 | message_qs = Message.objects.filter(thread__in=thread_qs) |
| 33 | 35 | messages = message_qs.filter( | ... | ... |
colab/settings.py
| ... | ... | @@ -239,7 +239,7 @@ SUPER_ARCHIVES_EXCLUDE = [] |
| 239 | 239 | SUPER_ARCHIVES_LOCK_FILE = '/var/lock/colab/import_emails.lock' |
| 240 | 240 | |
| 241 | 241 | # Mailman API settings |
| 242 | -MAILMAN_API_URL = 'http://localhost:8124' | |
| 242 | +MAILMAN_API_URL = 'http://localhost:8124/v2/' | |
| 243 | 243 | |
| 244 | 244 | LOGIN_URL = '/user/login' |
| 245 | 245 | LOGIN_REDIRECT_URL = '/' | ... | ... |
colab/super_archives/fixtures/mailinglistdata.json
| ... | ... | @@ -6,23 +6,23 @@ |
| 6 | 6 | "twitter": null, |
| 7 | 7 | "is_staff": false, |
| 8 | 8 | "user_permissions": [ |
| 9 | - | |
| 9 | + | |
| 10 | 10 | ], |
| 11 | 11 | "date_joined": "2015-02-24T21:10:35.004Z", |
| 12 | 12 | "google_talk": null, |
| 13 | - "first_name": "Gust", | |
| 13 | + "first_name": "John", | |
| 14 | 14 | "is_superuser": false, |
| 15 | 15 | "last_login": "2015-02-26T17:56:13.378Z", |
| 16 | 16 | "verification_hash": null, |
| 17 | 17 | "role": null, |
| 18 | - "email": "gustmax@hotmail.com", | |
| 19 | - "username": "gustmax", | |
| 18 | + "email": "johndoe@example.com", | |
| 19 | + "username": "johndoe", | |
| 20 | 20 | "bio": null, |
| 21 | 21 | "needs_update": false, |
| 22 | 22 | "is_active": true, |
| 23 | 23 | "facebook": null, |
| 24 | 24 | "groups": [ |
| 25 | - | |
| 25 | + | |
| 26 | 26 | ], |
| 27 | 27 | "password": "pbkdf2_sha256$12000$ez83ccNOUQZk$vYT/QcYMukXZ7D7L1qQPyYlzCUEEEF20J7/Xjef0Rqg=", |
| 28 | 28 | "institution": null, |
| ... | ... | @@ -37,7 +37,7 @@ |
| 37 | 37 | "real_name": "", |
| 38 | 38 | "user": 1, |
| 39 | 39 | "md5": "ed8f47ae6048f8d4456c0554578f53ff", |
| 40 | - "address": "gustmax@hotmail.com" | |
| 40 | + "address": "johndoe@example.com" | |
| 41 | 41 | }, |
| 42 | 42 | "model": "super_archives.emailaddress", |
| 43 | 43 | "pk": 1 | ... | ... |
colab/super_archives/tests/test_privatelist.py
| ... | ... | @@ -2,7 +2,7 @@ |
| 2 | 2 | import mock |
| 3 | 3 | |
| 4 | 4 | from colab.accounts.utils import mailman |
| 5 | -from django.test import TestCase, Client | |
| 5 | +from django.test import TestCase, Client | |
| 6 | 6 | |
| 7 | 7 | |
| 8 | 8 | class ArchivesViewTest(TestCase): |
| ... | ... | @@ -13,11 +13,14 @@ class ArchivesViewTest(TestCase): |
| 13 | 13 | self.client = Client() |
| 14 | 14 | |
| 15 | 15 | def authenticate_user(self): |
| 16 | - self.client.login(username='gustmax', password='1234') | |
| 16 | + self.client.login(username='johndoe', password='1234') | |
| 17 | 17 | |
| 18 | 18 | def test_see_only_private_list_if_member(self): |
| 19 | 19 | mailman.get_user_mailinglists = mock.Mock( |
| 20 | + return_value="[{'listname': 'privatelist'}]") | |
| 21 | + mailman.extract_listname_from_list = mock.Mock( | |
| 20 | 22 | return_value="['privatelist']") |
| 23 | + mailman.list_users = mock.Mock(return_value="['johndoe@example.com']") | |
| 21 | 24 | |
| 22 | 25 | self.authenticate_user() |
| 23 | 26 | request = self.client.get('/archives/thread/') |
| ... | ... | @@ -26,7 +29,7 @@ class ArchivesViewTest(TestCase): |
| 26 | 29 | |
| 27 | 30 | self.assertEqual('lista', list_data[0][0]) |
| 28 | 31 | self.assertEqual('privatelist', list_data[1][0]) |
| 29 | - self.assertEqual(2, len(list_data)) | |
| 32 | + self.assertEqual(2, len(list_data)) | |
| 30 | 33 | |
| 31 | 34 | def test_see_only_public_if_not_logged_in(self): |
| 32 | 35 | request = self.client.get('/archives/thread/') |
| ... | ... | @@ -34,10 +37,12 @@ class ArchivesViewTest(TestCase): |
| 34 | 37 | list_data = request.context['lists'] |
| 35 | 38 | |
| 36 | 39 | self.assertEqual('lista', list_data[0][0]) |
| 37 | - self.assertEqual(1, len(list_data)) | |
| 40 | + self.assertEqual(1, len(list_data)) | |
| 38 | 41 | |
| 39 | 42 | def test_see_private_thread_in_dashboard_if_member(self): |
| 40 | 43 | mailman.get_user_mailinglists = mock.Mock( |
| 44 | + return_value="[{'listname': 'privatelist'}]") | |
| 45 | + mailman.extract_listname_from_list = mock.Mock( | |
| 41 | 46 | return_value="['privatelist']") |
| 42 | 47 | |
| 43 | 48 | self.authenticate_user() |
| ... | ... | @@ -59,7 +64,7 @@ class ArchivesViewTest(TestCase): |
| 59 | 64 | self.assertEqual(1, len(hottest_threads)) |
| 60 | 65 | |
| 61 | 66 | def test_dont_see_private_threads_in_profile_if_logged_out(self): |
| 62 | - request = self.client.get('/account/gustmax') | |
| 67 | + request = self.client.get('/account/johndoe') | |
| 63 | 68 | |
| 64 | 69 | emails = request.context['emails'] |
| 65 | 70 | ... | ... |
colab/super_archives/views.py
| ... | ... | @@ -33,17 +33,23 @@ class ThreadView(View): |
| 33 | 33 | thread = get_object_or_404(Thread, subject_token=thread_token, |
| 34 | 34 | mailinglist__name=mailinglist) |
| 35 | 35 | |
| 36 | - # TODO: Refactor this code | |
| 37 | - # Use local flag is_private instead of always check the API!! | |
| 38 | - all_privates = dict(mailman.all_lists(private=True)) | |
| 39 | - if all_privates[thread.mailinglist.name]: | |
| 36 | + all_privates = [] | |
| 37 | + all_privates.extend( | |
| 38 | + [mlist.get('listname') | |
| 39 | + for mlist in mailman.all_lists() | |
| 40 | + if mlist.get('archive_private')] | |
| 41 | + ) | |
| 42 | + | |
| 43 | + if all_privates.count(thread.mailinglist.name): | |
| 40 | 44 | if not request.user.is_authenticated(): |
| 41 | 45 | raise PermissionDenied |
| 42 | 46 | else: |
| 43 | 47 | user = User.objects.get(username=request.user) |
| 44 | 48 | emails = user.emails.values_list('address', flat=True) |
| 45 | 49 | lists_for_user = mailman.get_user_mailinglists(user) |
| 46 | - if thread.mailinglist.name not in lists_for_user: | |
| 50 | + listnames_for_user = mailman.extract_listname_from_list( | |
| 51 | + lists_for_user) | |
| 52 | + if thread.mailinglist.name not in listnames_for_user: | |
| 47 | 53 | raise PermissionDenied |
| 48 | 54 | |
| 49 | 55 | thread.hit(request) |
| ... | ... | @@ -145,13 +151,16 @@ class ThreadDashboardView(View): |
| 145 | 151 | |
| 146 | 152 | context['lists'] = [] |
| 147 | 153 | |
| 148 | - lists_for_user = [] | |
| 154 | + listnames_for_user = [] | |
| 149 | 155 | if request.user.is_authenticated(): |
| 150 | 156 | user = User.objects.get(username=request.user) |
| 151 | 157 | lists_for_user = mailman.get_user_mailinglists(user) |
| 158 | + listnames_for_user = mailman.extract_listname_from_list( | |
| 159 | + lists_for_user) | |
| 152 | 160 | |
| 153 | 161 | for list_ in MailingList.objects.order_by('name'): |
| 154 | - if list_.name not in all_privates or list_.name in lists_for_user: | |
| 162 | + if list_.name not in all_privates\ | |
| 163 | + or list_.name in listnames_for_user: | |
| 155 | 164 | context['lists'].append(( |
| 156 | 165 | list_.name, |
| 157 | 166 | mailman.get_list_description(list_.name), | ... | ... |