diff --git a/src/postorius/templates/postorius/base.html b/src/postorius/templates/postorius/base.html index 063e08b..3218a44 100644 --- a/src/postorius/templates/postorius/base.html +++ b/src/postorius/templates/postorius/base.html @@ -45,7 +45,7 @@ {% if messages %} {% endif %} diff --git a/src/postorius/tests/mailman_api_tests/test_archival_options.py b/src/postorius/tests/mailman_api_tests/test_archival_options.py index e646737..5974087 100644 --- a/src/postorius/tests/mailman_api_tests/test_archival_options.py +++ b/src/postorius/tests/mailman_api_tests/test_archival_options.py @@ -23,11 +23,12 @@ __metaclass__ = type +import mock import logging from django.contrib.auth.models import User from django.core.urlresolvers import reverse -from django.test import Client, TestCase +from django.test import Client, RequestFactory, TestCase from django.test.utils import override_settings from urllib2 import HTTPError @@ -35,6 +36,7 @@ from postorius.tests import MM_VCR from postorius.tests.mailman_api_tests import API_CREDENTIALS from postorius.utils import get_client +from postorius.views.list import _add_archival_messages logger = logging.getLogger(__name__) @@ -134,3 +136,33 @@ reverse('list_archival_options', args=('test_list.example.com', )), {'archivers': ['mail-archive']}) self.assertTrue(self.m_list.archivers['mail-archive']) + + +class ArchivalMessagesTest(TestCase): + """ + Tests the ``_add_archival_messages`` helper method. + """ + + def setUp(self): + factory = RequestFactory() + self.request = factory.get('/') + + @mock.patch('django.contrib.messages.success') + @mock.patch('django.contrib.messages.warning') + def test_warning_messages(self, mock_warning, mock_success): + # foo-archiver enabled, but not stored adds warning message. + _add_archival_messages(['foo-archiver'], [], {'foo-archiver': False}, self.request) + self.assertTrue('could not be enabled' in mock_warning.call_args[0][1]) + self.assertTrue('foo-archiver' in mock_warning.call_args[0][1]) + # messages.success should not have been called. + self.assertEqual(mock_success.call_count, 0) + + @mock.patch('django.contrib.messages.success') + @mock.patch('django.contrib.messages.warning') + def test_success_messages(self, mock_warning, mock_success): + # foo-archiver enabled and stored adds success message. + _add_archival_messages(['foo-archiver'], [], {'foo-archiver': True}, self.request) + self.assertTrue('activated new archivers' in mock_success.call_args[0][1]) + self.assertTrue('foo-archiver' in mock_success.call_args[0][1]) + # messages.warning should not have been called. + self.assertEqual(mock_warning.call_count, 0) diff --git a/src/postorius/views/list.py b/src/postorius/views/list.py index 0704671..a22de29 100644 --- a/src/postorius/views/list.py +++ b/src/postorius/views/list.py @@ -659,6 +659,42 @@ context_instance=RequestContext(request)) +def _add_archival_messages(to_activate, to_disable, after_submission, + request): + """ + Add feedback messages to session, depending on previously set archivers. + """ + # There are archivers to enable. + if len(to_activate) > 0: + # If the archiver shows up in the data set *after* the update, + # we can show a success message. + activation_postponed = [] + activation_success = [] + for archiver in to_activate: + if after_submission[archiver] == True: + activation_success.append(archiver) + else: + activation_postponed.append(archiver) + # If archivers couldn't be updated, show a message: + if len(activation_postponed) > 0: + messages.warning(request, + _('Some archivers could not be enabled, probably ' + 'because they are not enabled in the Mailman ' + 'configuration. They will be enabled for ' + 'this list, if the archiver is enabled in the ' + 'Mailman configuration. {0}.' + ''.format(', '.join(activation_postponed)))) + if len(activation_success) > 0: + messages.success(request, + _('You activated new archivers for this list: ' + '{0}'.format(', '.join(activation_success)))) + # There are archivers to disable. + if len(to_disable) > 0: + messages.success(request, + _('You disabled the following archivers: ' + '{0}'.format(', '.join(to_disable)))) + + @list_owner_required def list_archival_options(request, list_id): """ @@ -683,19 +719,12 @@ for arc in to_disable: archivers[arc] = False - # Add feedback messages to session. - if len(to_activate) > 0: - messages.success(request, - _('You activated new archivers for this list: ' - '{0}'.format(', '.join(to_activate)))) - if len(to_disable) > 0: - messages.success(request, - _('You disabled the following archivers: ' - '{0}'.format(', '.join(to_disable)))) - # Re-cache list of archivers after update. archivers = m_list.archivers + # Show success/error messages. + _add_archival_messages(to_activate, to_disable, archivers, request) + # Instantiate form with current archiver data. initial = {'archivers': [key for key in archivers.keys() if archivers[key] is True]}