Fixed addresses serialization with JSON-based sessions

Fixes jazzband#104.

newsletter/admin.py

@@ -430,6 +430,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()
@@ -446,15 +447,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

@@ -56,7 +56,7 @@ def check_email(email, ignore_errors=False):
         Subscription._meta.get_field_by_name('email_field')[0].max_length
 
     if len(email) <= email_length or ignore_errors:
-        return email[:email_length]
+        return email[:email_length].strip()
     else:
         raise forms.ValidationError(
             _(
@@ -195,7 +195,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",
@@ -231,7 +232,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'",
@@ -243,7 +244,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 "
@@ -272,7 +274,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'):
@@ -302,13 +305,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."))
@@ -326,7 +330,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:
@@ -347,13 +352,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})
+                    self.emails.add(email)
+                    self.addresses.append({'email': email, 'name': name})
                 elif not ignore_errors:
                     raise forms.ValidationError(
                         _("Some entries are already subscribed to."))
@@ -400,8 +406,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(
@@ -430,7 +434,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/files/addresses.ldif

@@ -0,0 +1,17 @@
+dn: uid=jsmith,ou=People,dc=example,dc=org
+uid: jsmith
+cn: John Smith
+sn: Smith
+objectClass: top
+objectClass: person
+objectClass: inetOrgPerson
+mail: [email protected]
+
+dn: uid=jmartin,ou=People,dc=example,dc=org
+uid: jmartin
+cn: Jill Martin
+sn: Martin
+objectClass: top
+objectClass: person
+objectClass: inetOrgPerson
+mail: [email protected] 

newsletter/tests/files/addresses.vcf

@@ -0,0 +1,17 @@
+BEGIN:VCARD
+BDAY;VALUE=DATE:1973-12-12
+VERSION:3.0
+N:Smith;John
+FN:John Smith
+ORG:Example Corporation
+ADR;TYPE=WORK,POSTAL,PARCEL:;;Example Inc.;Redmond;WA;98052-6399;USA
+TEL;TYPE=WORK,MSG:+1-425-936-5522
+TEL;TYPE=WORK,FAX:+1-425-936-7329
+EMAIL;TYPE=INTERNET:[email protected]
+END:VCARD
+BEGIN:VCARD
+VERSION:3.0
+N:Martin;Jill
+FN:Jill Martin
+EMAIL;TYPE=INTERNET:[email protected]
+END:VCARD

newsletter/tests/test_admin.py

@@ -0,0 +1,66 @@
+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 admin_import_subscribers(self, source_file):
+        """
+        Import process of a CSV/LDIF/VCARD 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(source_file, '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)
+
+    def test_admin_import_subscribers_csv(self):
+        source_file = os.path.join(test_files_dir, 'addresses.csv')
+        self.admin_import_subscribers(source_file)
+
+    def test_admin_import_subscribers_ldif(self):
+        source_file = os.path.join(test_files_dir, 'addresses.ldif')
+        self.admin_import_subscribers(source_file)
+
+    def test_admin_import_subscribers_vcf(self):
+        source_file = os.path.join(test_files_dir, 'addresses.vcf')
+        self.admin_import_subscribers(source_file)

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')),
 ]