diff --git a/src/postorius/forms.py b/src/postorius/forms.py index 658a178..39f940e 100644 --- a/src/postorius/forms.py +++ b/src/postorius/forms.py @@ -95,20 +95,9 @@ "description"]] -class NewOwnerForm(forms.Form): - - """Add a list owner.""" - owner_email = forms.EmailField( - label=_('Email Address'), - error_messages={ - 'required': _('Please enter an email adddress.'), - 'invalid': _('Please enter a valid email adddress.')}) - - -class NewModeratorForm(forms.Form): - - """Add a list moderator.""" - moderator_email = forms.EmailField( +class MemberForm(forms.Form): + """Assing a role to the member""" + email = forms.EmailField( label=_('Email Address'), error_messages={ 'required': _('Please enter an email adddress.'), @@ -853,18 +842,18 @@ choices=((address, address) for address in user_emails)) -class HeldMessagesModerationForm(forms.Form): +class MultipleChoiceForm(forms.Form): - class HeldMessageMultipleChoiceField(forms.MultipleChoiceField): + class MultipleChoiceField(forms.MultipleChoiceField): def validate(self, value): pass - choices = HeldMessageMultipleChoiceField( + choices = MultipleChoiceField( widget=forms.CheckboxSelectMultiple, ) def clean_choices(self): if len(self.cleaned_data['choices']) < 1: - raise forms.ValidationError(_('Please select at least one message to perform an action')) + raise forms.ValidationError(_('Make at least one selection')) return self.cleaned_data['choices'] diff --git a/src/postorius/templates/postorius/base.html b/src/postorius/templates/postorius/base.html index 5ce979f..7cee8f8 100644 --- a/src/postorius/templates/postorius/base.html +++ b/src/postorius/templates/postorius/base.html @@ -1,4 +1,3 @@ -{% load url from future %} {% load i18n %} {% load staticfiles %} diff --git a/src/postorius/templates/postorius/lists/confirm_remove_role.html b/src/postorius/templates/postorius/lists/confirm_remove_role.html index 4d7cfae..816fcb5 100644 --- a/src/postorius/templates/postorius/lists/confirm_remove_role.html +++ b/src/postorius/templates/postorius/lists/confirm_remove_role.html @@ -15,7 +15,7 @@
{% csrf_token %} - {% trans 'Cancel' %} + {% trans 'Cancel' %}
diff --git a/src/postorius/templates/postorius/lists/members.html b/src/postorius/templates/postorius/lists/members.html index 6ab9b7f..ed0d4d3 100644 --- a/src/postorius/templates/postorius/lists/members.html +++ b/src/postorius/templates/postorius/lists/members.html @@ -4,99 +4,89 @@ {% load nav_helpers %} {% block subtitle %} -{% trans 'Members' %} | {{ list.fqdn_listname}} +{{ page_title }} | {{ list.fqdn_listname}} {% endblock %} {% block main %} {% list_nav 'list_members' 'List Members' %}
-

{% trans 'List Members' %}

+

{{ page_title }}

-

{% trans 'Owners' %}

-
- {% csrf_token %} - {% if owner_form.owner_email.errors %} -
{{ owner_form.owner_email.errors }}
+ {% if not member_form %} +
+
+ {% trans 'Unsubscribe all' %} +
+ + {% csrf_token %} + + +
+ {% else %} + {% if member_form.email.errors %} +
+ {{ member_form.email.errors }} +
{% endif %} - - {{ owner_form.owner_email|add_form_control }} - - -
-
- - - - - - - - - {% for member in list.owners %} - - - - - {% endfor %} - -
{% trans 'Address' %} 
{{ member }}{% trans 'Delete' %}
-
-

{% trans 'Moderators' %}

-
- {% csrf_token %} - {% if moderator_form.moderator_email.errors %} -
{{ moderator_form.moderator_email.errors }}
- {% endif %} - - {{ moderator_form.moderator_email|add_form_control }} - -
-
-
- - - - - - - - - {% for member in list.moderators %} - - - - - {% endfor %} - -
{% trans 'Address' %} 
{{ member }}{% trans 'Delete' %}
-
-

{% trans 'Members' %}

-
- {% csrf_token %} - + + {% csrf_token %} + {{ member_form.email.label_tag }} + {{ member_form.email|add_form_control }} +
-
-
- - - - - - - - - - {% for member in members %} - - - - - - {% endfor %} - -
{% trans 'Address' %}{% trans 'Role' %} 
{{ member.email }}{% trans member.role %}{% trans 'Unsubscribe' %}
-
- {% include 'postorius/_pagination.html' with page=members %} + {% endif %} +
+ {% if members|length > 0 %} +
+ {% csrf_token %} + {% if form.choices.errors %} +
{{ form.choices.errors }}
+ {% endif %} +
+ + + + {% if not member_form %} + + {% endif %} + + + + + + {% for member in members %} + + {% if not member_form %} + + + + {% else %} + + + {% endif %} + + {% endfor %} + +
{% trans 'Address' %} + {% if not member_form %} + + {% endif %} +
{{ member.email }}{% trans 'Unsubscribe' %}{{ member }}{% trans 'Delete' %}
+
+
+ {% if not member_form %} + {% include 'postorius/_pagination.html' with page=members %} + {% endif %} + {% else %} +

{{ empty_error }}

+ {% endif %}
{% endblock main %} +{% block additionaljs %} + +{% endblock %} diff --git a/src/postorius/templates/postorius/menu/list_nav.html b/src/postorius/templates/postorius/menu/list_nav.html index 155b7a4..bc3ea1a 100644 --- a/src/postorius/templates/postorius/menu/list_nav.html +++ b/src/postorius/templates/postorius/menu/list_nav.html @@ -20,7 +20,16 @@
  • {% trans 'Held Messages' %}
    • {% endif %} {% if user.is_superuser or user.is_list_owner %} -
  • {% trans 'Members' %}
    • +
  • + {% trans 'Memebers' %} + + +
  • {% trans 'Settings' %}
  • {% trans 'Mass Subscribe' %}
  • {% trans 'Mass Removal' %}
    • diff --git a/src/postorius/tests/mailman_api_tests/test_list_members.py b/src/postorius/tests/mailman_api_tests/test_list_members.py index 5e23e4f..bb1351d 100644 --- a/src/postorius/tests/mailman_api_tests/test_list_members.py +++ b/src/postorius/tests/mailman_api_tests/test_list_members.py @@ -75,7 +75,7 @@ @MM_VCR.use_cassette('list_members_access.yaml') def test_page_not_accessible_if_not_logged_in(self): - url = reverse('list_members', args=('foo@example.com', )) + url = reverse('list_members', args=('foo@example.com', 'subscribers',)) response = self.client.get(url) if "%40" not in url: # Django < 1.8 url = quote(url) @@ -88,28 +88,28 @@ def test_page_not_accessible_for_unprivileged_users(self): self.client.login(username='testuser', password='testpass') response = self.client.get(reverse('list_members', - args=('foo@example.com', ))) + args=('foo@example.com', 'subscribers',))) self.assertEqual(response.status_code, 403) @MM_VCR.use_cassette('list_members_page.yaml') def test_not_accessible_for_moderator(self): self.client.login(username='testmoderator', password='testpass') response = self.client.get(reverse('list_members', - args=('foo@example.com', ))) + args=('foo@example.com', 'subscribers',))) self.assertEqual(response.status_code, 403) @MM_VCR.use_cassette('list_members_page.yaml') def test_page_accessible_for_superuser(self): self.client.login(username='testsu', password='testpass') response = self.client.get(reverse('list_members', - args=('foo@example.com', ))) + args=('foo@example.com', 'subscribers',))) self.assertEqual(response.status_code, 200) @MM_VCR.use_cassette('list_members_page.yaml') def test_page_accessible_for_owner(self): self.client.login(username='testowner', password='testpass') response = self.client.get(reverse('list_members', - args=('foo@example.com', ))) + args=('foo@example.com', 'subscribers',))) self.assertEqual(response.status_code, 200) @@ -141,8 +141,8 @@ @MM_VCR.use_cassette('test_list_members_owner_add_remove.yaml') def test_add_remove_owner(self): self.client.post( - reverse('list_members', args=('foo@example.com', )), - {'owner_email': 'newowner@example.com'}) + reverse('list_members', args=('foo@example.com', 'owners',)), + {'email': 'newowner@example.com'}) self.assertTrue('newowner@example.com' in self.foo_list.owners) self.client.post( reverse('remove_role', args=('foo@example.com', 'owner', @@ -157,8 +157,8 @@ self.su.save() # It must still be allowed to create and remove owners self.client.post( - reverse('list_members', args=('foo@example.com', )), - {'owner_email': 'newowner@example.com'}) + reverse('list_members', args=('foo@example.com', 'owners',)), + {'email': 'newowner@example.com'}) self.assertTrue('newowner@example.com' in self.foo_list.owners) response = self.client.post( reverse('remove_role', args=('foo@example.com', 'owner', @@ -219,8 +219,8 @@ # login and post new moderator data to url self.client.login(username='su', password='pwd') self.client.post( - reverse('list_members', args=('foo@example.com', )), - {'moderator_email': 'newmod@example.com'}) + reverse('list_members', args=('foo@example.com', 'moderators',)), + {'email': 'newmod@example.com'}) moderators = self.foo_list.moderators @MM_VCR.use_cassette('test_list_members_add_moderator.yaml') diff --git a/src/postorius/urls.py b/src/postorius/urls.py index 59dd81c..7df73d3 100644 --- a/src/postorius/urls.py +++ b/src/postorius/urls.py @@ -29,12 +29,11 @@ per_list_urlpatterns = patterns('postorius.views', url(r'^csv_view/$', 'csv_view', name='csv_view'), - url(r'^members/$', - ListMembersView.as_view( - ), name='list_members'), url(r'^members/options/(?P[^/]+)/$', ListMemberOptionsView.as_view( ), name='list_member_options'), + url(r'^members/(?P\w+)/$', + 'list_members_view', name='list_members'), url(r'^$', ListSummaryView.as_view( ), name='list_summary'), diff --git a/src/postorius/views/list.py b/src/postorius/views/list.py index 4081c12..f7d7e30 100644 --- a/src/postorius/views/list.py +++ b/src/postorius/views/list.py @@ -44,72 +44,66 @@ logger = logging.getLogger(__name__) -class ListMembersView(MailingListView): - - """Display all members of a given list. - """ - - @property - def _common_context(self): - return { - 'list': self.mailing_list, - 'count_options': [25, 50, 100, 200], - } - - @method_decorator(list_owner_required) - def post(self, request, list_id): - # FIXME: form usage is wrong here, they should be instantiated only - # once or the form errors will be erased. - if 'owner_email' in request.POST: - owner_form = NewOwnerForm(request.POST) - if owner_form.is_valid(): +"""Display all members of a given list. +""" +@login_required +@list_owner_required +def list_members_view(request, list_id, role=None): + if role not in ['owners', 'moderators', 'subscribers']: + return redirect('list_members', list_id, 'subscribers') + mailing_list = List.objects.get_or_404(fqdn_listname=list_id) + if request.method == 'POST': + if role == 'subscribers': + form = MultipleChoiceForm(request.POST) + if form.is_valid(): + members = form.cleaned_data['choices'] + for member in members: + mailing_list.unsubscribe(member) + messages.success(request, _('The selected members have been unsubscribed')) + else: + member_form = MemberForm(request.POST) + if member_form.is_valid(): try: - self.mailing_list.add_owner( - owner_form.cleaned_data['owner_email']) - messages.success( - request, _('%s has been added as list owner.' - % request.POST['owner_email'])) + if role == 'moderators': + mailing_list.add_moderator(member_form.cleaned_data['email']) + messages.success( + request, _('%s has been added as list moderator.' + % member_form.cleaned_data['email'])) + elif role == 'owners': + mailing_list.add_owner(member_form.cleaned_data['email']) + messages.success( + request, _('%s has been added as list owner.' + % member_form.cleaned_data['email'])) except HTTPError as e: messages.error(request, _(e.msg)) - if 'moderator_email' in request.POST: - moderator_form = NewModeratorForm(request.POST) - if moderator_form.is_valid(): - try: - self.mailing_list.add_moderator( - moderator_form.cleaned_data['moderator_email']) - messages.success( - request, _('%s has been added as list moderator.' - % request.POST['moderator_email'])) - except HTTPError as e: - messages.error(request, _(e.msg)) - owner_form = NewOwnerForm() - moderator_form = NewModeratorForm() - members = utils.paginate( - request, self.mailing_list.get_member_page, + else: + form = MultipleChoiceForm() + member_form = MemberForm() + context = { + 'list': mailing_list, + } + if role == 'subscribers': + context['members'] = utils.paginate( + request, mailing_list.get_member_page, count=request.GET.get('count', 25), paginator_class=utils.MailmanPaginator) - context = { - 'owner_form': owner_form, - 'moderator_form': moderator_form, - 'members': members, - } - context.update(self._common_context) - return render(request, 'postorius/lists/members.html', context) - - @method_decorator(login_required) - @method_decorator(list_owner_required) - def get(self, request, list_id, page=1): - members = utils.paginate( - request, self.mailing_list.get_member_page, - count=request.GET.get('count', 25), - paginator_class=utils.MailmanPaginator) - context = { - 'owner_form': NewOwnerForm(), - 'moderator_form': NewModeratorForm(), - 'members': members, - } - context.update(self._common_context) - return render(request, 'postorius/lists/members.html', context) + context['page_title'] = _('List Subscribers') + context['empty_error'] = _('List has no Subscribers') + context['count_options'] = [25, 50, 100, 200] + else: + context['member_form'] = member_form + if role == 'owners': + context['members'] = mailing_list.owners + context['page_title'] = _('List Owners') + context['form_action'] = _('Add Owner') + context['role'] = 'owner' + elif role == 'moderators': + context['members'] = mailing_list.moderators + context['page_title'] = _('List Moderators') + context['empty_error'] = _('List has no Moderators') + context['form_action'] = _('Add Moderator') + context['role'] = 'moderator' + return render(request, 'postorius/lists/members.html', context) class ListMemberOptionsView(MailingListView): @@ -120,7 +114,7 @@ try: client = utils.get_client() mm_member = client.get_member(list_id, email) - mm_list = client.get_list(list_id) + mm_list = List.objects.get_or_404(fqdn_listname=list_id) preferences_form = UserPreferences(request.POST) if preferences_form.is_valid(): preferences = mm_member.preferences @@ -152,7 +146,7 @@ try: client = utils.get_client() mm_member = client.get_member(list_id, email) - mm_list = client.get_list(list_id) + mm_list = List.objects.get_or_404(fqdn_listname=list_id) settingsform = UserPreferences(initial=mm_member.preferences) except MailmanApiError: return utils.render_api_error(request) @@ -378,14 +372,14 @@ count=request.GET.get('count', 20)) context = { 'held_messages': held_messages, - 'form': HeldMessagesModerationForm(), + 'form': MultipleChoiceForm(), } context.update(self._common_context) return render(request, 'postorius/lists/held_messages.html', context) @method_decorator(list_moderator_required) def post(self, request, *args, **kwargs): - form = HeldMessagesModerationForm(request.POST) + form = MultipleChoiceForm(request.POST) if form.is_valid(): message_ids = form.cleaned_data['choices'] try: @@ -416,8 +410,7 @@ """ mm_lists = [] try: - client = utils.get_client() - mm_lists = client.get_list(list_id) + mm_lists = List.objects.get_or_404(fqdn_listname=list_id) except MailmanApiError: return utils.render_api_error(request) @@ -617,7 +610,7 @@ """Shows a list of held messages. """ try: - m_list = utils.get_client().get_list(list_id) + m_list = List.objects.get_or_404(fqdn_listname=list_id) except MailmanApiError: return utils.render_api_error(request) return render_to_response('postorius/lists/subscription_requests.html', @@ -642,7 +635,7 @@ 'defer': _('The request has been defered.'), } try: - m_list = utils.get_client().get_list(list_id) + m_list = List.objects.get_or_404(fqdn_listname=list_id) # Moderate request and add feedback message to session. m_list.moderate_request(request_id, action) messages.success(request, confirmation_messages[action]) @@ -732,16 +725,16 @@ except MailmanApiError: return utils.render_api_error(request) - redirect_on_success = redirect('list_members', the_list.list_id) + redirect_on_success = redirect('list_members', the_list.list_id, '{}s'.format(role)) if role == 'owner': owners = the_list.owners if address not in owners: messages.error(request, _('The user %s is not an owner') % address) - return redirect('list_members', the_list.list_id) + return redirect('list_members', the_list.list_id, 'owners') if len(owners) == 1: messages.error(request, _('Removing the last owner is impossible')) - return redirect('list_members', the_list.list_id) + return redirect('list_members', the_list.list_id, 'owners') # the user may not have a other_emails property if it's a superuser user_addresses = set([request.user.email]) | \ set(getattr(request.user, 'other_emails', [])) @@ -752,7 +745,7 @@ elif role == 'moderator': if address not in the_list.moderators: messages.error(request, _('The user %s is not a moderator') % address) - return redirect('list_members', the_list.list_id) + return redirect('list_members', the_list.list_id, 'moderators') if request.method == 'POST': try: the_list.remove_role(role, address) @@ -761,7 +754,7 @@ except HTTPError as e: messages.error(request, _('The %(role)s could not be removed: %(msg)s') % {'role':role, 'msg': e.msg}) - return redirect('list_members', the_list.list_id) + return redirect('list_members', the_list.list_id, '{}s'.format(role)) messages.success(request, _('The user %(address)s has been removed as %(role)s.') % {'address': address, 'role': role}) return redirect_on_success @@ -838,7 +831,7 @@ Activate or deactivate list archivers. """ # Get the list and cache the archivers property. - m_list = utils.get_client().get_list(list_id) + m_list = List.objects.get_or_404(fqdn_listname=list_id) archivers = m_list.archivers # Process form submission.