Fixed addresses serialization with JSON-based sessions

Fixes jazzband#104.

newsletter/admin.py

@@ -424,6 +424,7 @@ def subscribers_import(self, request):
             form = ImportForm(request.POST, request.FILES)
             if form.is_valid():
                 request.session['addresses'] = form.get_addresses()
+                request.session['newsletter_pk'] = form.cleaned_data['newsletter'].pk
                 return HttpResponseRedirect('confirm/')
         else:
             form = ImportForm()
@@ -440,15 +441,18 @@ def subscribers_import_confirm(self, request):
             return HttpResponseRedirect('../')
 
         addresses = request.session['addresses']
+        newsletter = Newsletter.objects.get(pk=request.session['newsletter_pk'])
         logger.debug('Confirming addresses: %s', addresses)
         if request.POST:
             form = ConfirmForm(request.POST)
             if form.is_valid():
                 try:
-                    for address in addresses.values():
-                        address.save()
+                    for address in addresses:
+                        addr = make_subscription(newsletter, address['email'], address['name'])
+                        addr.save()
                 finally:
                     del request.session['addresses']
+                    del request.session['newsletter_pk']
 
                 messages.success(
                     request,

newsletter/admin_forms.py

@@ -174,7 +174,8 @@ def parse_csv(myfile, newsletter, ignore_errors=False):
 
     logger.debug('Extracting data.')
 
-    addresses = {}
+    addresses = []
+    emails = set()
     for row in myreader:
         if not max(namecol, mailcol) < len(row):
             logger.warn("Column count does not match for row number %d",
@@ -210,7 +211,7 @@ def parse_csv(myfile, newsletter, ignore_errors=False):
                 )
 
         if addr:
-            if email in addresses:
+            if email in emails:
                 logger.warn(
                     "Entry '%s' at line %d contains a "
                     "duplicate entry for '%s'",
@@ -222,7 +223,8 @@ def parse_csv(myfile, newsletter, ignore_errors=False):
                         "The address file contains duplicate entries "
                         "for '%s'.") % email)
 
-            addresses.update({email: addr})
+            emails.add(email)
+            addresses.append({'email': email, 'name': name})
         else:
             logger.warn(
                 "Entry '%s' at line %d is already subscribed to "
@@ -246,7 +248,8 @@ def parse_vcard(myfile, newsletter, ignore_errors=False):
             _(u"Error reading vCard file: %s" % e)
         )
 
-    addresses = {}
+    addresses = []
+    emails = set()
 
     for myvcard in myvcards:
         if hasattr(myvcard, 'fn'):
@@ -276,13 +279,14 @@ def parse_vcard(myfile, newsletter, ignore_errors=False):
                 )
 
         if addr:
-            if email in addresses and not ignore_errors:
+            if email in emails and not ignore_errors:
                 raise forms.ValidationError(
                     _("The address file contains duplicate entries for '%s'.")
                     % email
                 )
 
-            addresses.update({email: addr})
+            emails.add(email)
+            addresses.append({'email': email, 'name': name})
         elif not ignore_errors:
             raise forms.ValidationError(
                 _("Some entries are already subscribed to."))
@@ -294,7 +298,8 @@ def parse_ldif(myfile, newsletter, ignore_errors=False):
     from addressimport import ldif
 
     class AddressParser(ldif.LDIFParser):
-        addresses = {}
+        addresses = []
+        emails = set()
 
         def handle(self, dn, entry):
             if 'mail' in entry:
@@ -315,13 +320,14 @@ def handle(self, dn, entry):
                         )
 
                 if addr:
-                    if email in self.addresses and not ignore_errors:
+                    if email in self.emails and not ignore_errors:
                         raise forms.ValidationError(_(
                             "The address file contains duplicate entries "
                             "for '%s'.") % email
                         )
 
-                    self.addresses.update({email: addr})
+                    emails.add(email)
+                    self.addresses.append({'email': email, 'name': name})
                 elif not ignore_errors:
                     raise forms.ValidationError(
                         _("Some entries are already subscribed to."))
@@ -368,8 +374,6 @@ def clean(self):
             raise forms.ValidationError(_(
                 "File type '%s' was not recognized.") % content_type)
 
-        self.addresses = []
-
         ext = myvalue.name.rsplit('.', 1)[-1].lower()
         if ext == 'vcf':
             self.addresses = parse_vcard(
@@ -398,7 +402,7 @@ def get_addresses(self):
             logger.debug('Getting addresses: %s', self.addresses)
             return self.addresses
         else:
-            return {}
+            return []
 
     newsletter = forms.ModelChoiceField(
         label=_("Newsletter"),

newsletter/tests/files/addresses.csv

@@ -0,0 +1,3 @@
+name;email
+John Smith;[email protected]
+Jill Martin;[email protected]

newsletter/tests/test_admin.py

@@ -0,0 +1,54 @@
+import os
+
+from django.core.urlresolvers import reverse
+from django.test import TestCase
+
+from newsletter import admin  # Triggers model admin registration
+from newsletter.models import Newsletter
+from newsletter.utils import get_user_model
+
+test_files_dir = os.path.join(os.path.dirname(__file__), 'files')
+
+
+class AdminTestCase(TestCase):
+    def setUp(self):
+        super(AdminTestCase, self).setUp()
+        User = get_user_model()
+        self.password = 'johnpassword'
+        self.admin_user = User.objects.create_superuser(
+            'john', '[email protected]', self.password
+        )
+        self.client.login(username=self.admin_user.username, password=self.password)
+        self.newsletter = Newsletter.objects.create(
+            sender='Test Sender', title='Test Newsletter',
+            slug='test-newsletter', visible=True, email='[email protected]',
+        )
+
+    def test_admin_import_subscribers(self):
+        """
+        Import process of a CSV file containing subscription addresses from
+        the admin site.
+        """
+        import_url = reverse('admin:newsletter_subscription_import')
+        response = self.client.get(import_url)
+        self.assertContains(response, "<h1>Import addresses</h1>")
+
+        with open(os.path.join(test_files_dir, 'addresses.csv'), 'rb') as fh:
+            response = self.client.post(import_url, {
+                'newsletter': self.newsletter.pk,
+                'address_file': fh,
+                'ignore_errors': '',
+            }, follow=True)
+        self.assertContains(response, "<h1>Confirm import</h1>")
+
+        import_confirm_url = reverse(
+            'admin:newsletter_subscription_import_confirm'
+        )
+        response = self.client.post(
+            import_confirm_url, {'confirm': True}, follow=True
+        )
+        self.assertContains(
+            response,
+            "2 subscriptions have been successfully added."
+        )
+        self.assertEqual(self.newsletter.subscription_set.count(), 2)

tests/settings.py

@@ -11,6 +11,7 @@
     'django.contrib.contenttypes',
     'django.contrib.sessions',
     'django.contrib.auth',
+    'django.contrib.admin',
     'django.contrib.sites',
     'django_extensions',
     'sorl.thumbnail',

tests/urls.py

@@ -1,6 +1,8 @@
 from django.conf.urls import include, url
+from django.contrib import admin
 
 
 urlpatterns = [
+    url(r'^admin/', admin.site.urls),
     url(r'^newsletter/', include('newsletter.urls')),
 ]