From b20ad51dde36eba2a8ad709ee11a93dc78f5e1ae Mon Sep 17 00:00:00 2001
From: Simon Cross <hodgestar@gmail.com>
Date: Thu, 19 Jun 2014 11:26:13 +0200
Subject: [PATCH 01/12] Add loginas.

---
 go/base/admin.py | 3 +++
 go/settings.py   | 1 +
 go/urls.py       | 4 ++++
 setup.py         | 1 +
 4 files changed, 9 insertions(+)

diff --git a/go/base/admin.py b/go/base/admin.py
index fadddbab3..5105b7c9d 100644
--- a/go/base/admin.py
+++ b/go/base/admin.py
@@ -19,6 +19,9 @@ def get_inline_instances(self, request, obj=None):
             return []
         return super(GoUserAdmin, self).get_inline_instances(request, obj=obj)
 
+    # loginas form template
+    change_form_template = 'loginas/change_form.html'
+
     # The forms to add and change user instances
     inlines = (UserProfileInline,)
 
diff --git a/go/settings.py b/go/settings.py
index 528b0f49a..a62986b09 100644
--- a/go/settings.py
+++ b/go/settings.py
@@ -161,6 +161,7 @@ def static_paths(paths):
     'djcelery',
     'djcelery_email',
     'crispy_forms',
+    'loginas',
     'go.base',
     'go.conversation',
     'go.router',
diff --git a/go/urls.py b/go/urls.py
index 13bfefaf9..e7b0d8224 100644
--- a/go/urls.py
+++ b/go/urls.py
@@ -53,6 +53,10 @@ def health(request):
     url(r'^help/$', 'flatpage', {'url': '/help/'}, name='help'),
 )
 
+urlpatterns += patterns('loginas.views',
+    url(r"^login/user/(?P<user_id>.+)/$", "user_login", name="loginas-user-login"),
+)
+
 # HAProxy health check
 urlpatterns += patterns('',
     url(r'^health/$', health, name='health'),
diff --git a/setup.py b/setup.py
index 8bbd9f5e6..c36f5cb74 100644
--- a/setup.py
+++ b/setup.py
@@ -40,6 +40,7 @@
         'django-pipeline==1.3.6',
         'txpostgres==1.1.0',
         'django-crispy-forms==1.4.0',
+        'django-loginas==0.1.3',
     ],
     classifiers=[
         'Development Status :: 4 - Beta',

From 74d3beb8c4e20dbd9c95703c61c9c1bdd1274726 Mon Sep 17 00:00:00 2001
From: Simon Cross <hodgestar@gmail.com>
Date: Wed, 25 Jun 2014 11:12:49 +0200
Subject: [PATCH 02/12] Add ability to make superusers using make_django_user.

---
 go/base/tests/helpers.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/go/base/tests/helpers.py b/go/base/tests/helpers.py
index e1e6da0e3..9177dd9a6 100644
--- a/go/base/tests/helpers.py
+++ b/go/base/tests/helpers.py
@@ -140,9 +140,13 @@ def patch_settings(self, **kwargs):
 
     @proxyable
     def make_django_user(self, email='user@domain.com', password='password',
-                         first_name="Test", last_name="User"):
-        user = get_user_model().objects.create_user(
-            email=email, password=password)
+                         first_name="Test", last_name="User", superuser=False):
+        if superuser:
+            user = get_user_model().objects.create_superuser(
+                email=email, password=password)
+        else:
+            user = get_user_model().objects.create_user(
+                email=email, password=password)
         user.first_name = first_name
         user.last_name = last_name
         user.save()

From a3d5736fdd8ae747bb7c375ddf1e7934f2227d25 Mon Sep 17 00:00:00 2001
From: Simon Cross <hodgestar@gmail.com>
Date: Wed, 25 Jun 2014 11:13:09 +0200
Subject: [PATCH 03/12] Add tests for login-as URLs.

---
 go/tests/test_urls.py | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 go/tests/test_urls.py

diff --git a/go/tests/test_urls.py b/go/tests/test_urls.py
new file mode 100644
index 000000000..8640affd6
--- /dev/null
+++ b/go/tests/test_urls.py
@@ -0,0 +1,38 @@
+from django.core.urlresolvers import reverse
+
+from go.base.tests.helpers import GoDjangoTestCase, DjangoVumiApiHelper
+
+
+class TestLoginAs(GoDjangoTestCase):
+
+    def setUp(self):
+        self.vumi_helper = self.add_helper(DjangoVumiApiHelper())
+        self.superuser_helper = self.vumi_helper.make_django_user(
+            superuser=True, email='superuser@example.com')
+        self.user_helper_1 = self.vumi_helper.make_django_user(
+            email='user1@example.com')
+        self.user_helper_2 = self.vumi_helper.make_django_user(
+            email='user2@example.com')
+
+        self.superuser_client = self.vumi_helper.get_client(
+            username='superuser@example.com')
+        self.user_client_1 = self.vumi_helper.get_client(
+            username='user1@example.com')
+
+    def test_successful_login_as(self):
+        """ Superusers should be able to use login-as. """
+        user_2_pk = self.user_helper_2.get_django_user().pk
+        response = self.superuser_client.get(
+            reverse('loginas-user-login', kwargs={'user_id': str(user_2_pk)}))
+        self.assertRedirects(response, reverse('home'), target_status_code=302)
+        self.assertEqual(response.client.session.get('_go_user_account_key'),
+                         'test-2-user')
+
+    def test_failed_login_as(self):
+        """ Ordinary users should not be able to use login-as. """
+        user_2_pk = self.user_helper_2.get_django_user().pk
+        response = self.user_client_1.get(
+            reverse('loginas-user-login', kwargs={'user_id': str(user_2_pk)}))
+        self.assertRedirects(response, reverse('home'), target_status_code=302)
+        self.assertEqual(response.client.session.get('_go_user_account_key'),
+                         'test-1-user')

From 7c7021ed6ce7efd1b8b3621b01b3181de10244ee Mon Sep 17 00:00:00 2001
From: Jeremy Thurgood <jeremy@praekelt.com>
Date: Wed, 25 Jun 2014 11:20:22 +0200
Subject: [PATCH 04/12] Enable contact search on users created through Django
 test infrastructure so the tests that need it pass by design instead of by
 accident.

---
 go/base/tests/helpers.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/go/base/tests/helpers.py b/go/base/tests/helpers.py
index e1e6da0e3..a24a4ead5 100644
--- a/go/base/tests/helpers.py
+++ b/go/base/tests/helpers.py
@@ -160,7 +160,7 @@ def create_user_profile(self, sender, instance, created, **kwargs):
             return
 
         user_helper = self.make_user(
-            unicode(instance.email), enable_search=False,
+            unicode(instance.email), enable_search=True,
             django_user_pk=instance.pk)
         base_models.UserProfile.objects.create(
             user=instance, user_account=user_helper.account_key)

From ea8417ca5e6798aa08885c5e26b99912cb7155fb Mon Sep 17 00:00:00 2001
From: Simon Cross <hodgestar@gmail.com>
Date: Wed, 25 Jun 2014 11:29:03 +0200
Subject: [PATCH 05/12] Use multiline docstrings always.

---
 go/tests/test_urls.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/go/tests/test_urls.py b/go/tests/test_urls.py
index 8640affd6..f188843d7 100644
--- a/go/tests/test_urls.py
+++ b/go/tests/test_urls.py
@@ -20,7 +20,9 @@ def setUp(self):
             username='user1@example.com')
 
     def test_successful_login_as(self):
-        """ Superusers should be able to use login-as. """
+        """
+        Superusers should be able to use login-as.
+        """
         user_2_pk = self.user_helper_2.get_django_user().pk
         response = self.superuser_client.get(
             reverse('loginas-user-login', kwargs={'user_id': str(user_2_pk)}))
@@ -29,7 +31,9 @@ def test_successful_login_as(self):
                          'test-2-user')
 
     def test_failed_login_as(self):
-        """ Ordinary users should not be able to use login-as. """
+        """
+        Ordinary users should not be able to use login-as.
+        """
         user_2_pk = self.user_helper_2.get_django_user().pk
         response = self.user_client_1.get(
             reverse('loginas-user-login', kwargs={'user_id': str(user_2_pk)}))

From 3820eee75cb9816a8445679146b2080d4ebc3cfc Mon Sep 17 00:00:00 2001
From: Simon Cross <hodgestar@gmail.com>
Date: Wed, 25 Jun 2014 11:36:31 +0200
Subject: [PATCH 06/12] Better names for user helpers.

---
 go/tests/test_urls.py | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/go/tests/test_urls.py b/go/tests/test_urls.py
index f188843d7..b4b1edc88 100644
--- a/go/tests/test_urls.py
+++ b/go/tests/test_urls.py
@@ -9,34 +9,38 @@ def setUp(self):
         self.vumi_helper = self.add_helper(DjangoVumiApiHelper())
         self.superuser_helper = self.vumi_helper.make_django_user(
             superuser=True, email='superuser@example.com')
-        self.user_helper_1 = self.vumi_helper.make_django_user(
-            email='user1@example.com')
-        self.user_helper_2 = self.vumi_helper.make_django_user(
-            email='user2@example.com')
+        self.ordinary_user_helper = self.vumi_helper.make_django_user(
+            email='ordinary_user@example.com')
+        self.target_user_helper = self.vumi_helper.make_django_user(
+            email='loginas_target@example.com')
 
         self.superuser_client = self.vumi_helper.get_client(
             username='superuser@example.com')
-        self.user_client_1 = self.vumi_helper.get_client(
-            username='user1@example.com')
+        self.ordinary_user_client = self.vumi_helper.get_client(
+            username='ordinary_user@example.com')
 
     def test_successful_login_as(self):
         """
         Superusers should be able to use login-as.
         """
-        user_2_pk = self.user_helper_2.get_django_user().pk
+        target_user_pk = self.target_user_helper.get_django_user().pk
         response = self.superuser_client.get(
-            reverse('loginas-user-login', kwargs={'user_id': str(user_2_pk)}))
+            reverse('loginas-user-login',
+                    kwargs={'user_id': str(target_user_pk)})
+        )
         self.assertRedirects(response, reverse('home'), target_status_code=302)
         self.assertEqual(response.client.session.get('_go_user_account_key'),
-                         'test-2-user')
+                         self.target_user_helper.account_key)
 
     def test_failed_login_as(self):
         """
         Ordinary users should not be able to use login-as.
         """
-        user_2_pk = self.user_helper_2.get_django_user().pk
-        response = self.user_client_1.get(
-            reverse('loginas-user-login', kwargs={'user_id': str(user_2_pk)}))
+        target_user_pk = self.target_user_helper.get_django_user().pk
+        response = self.ordinary_user_client.get(
+            reverse('loginas-user-login',
+                    kwargs={'user_id': str(target_user_pk)})
+        )
         self.assertRedirects(response, reverse('home'), target_status_code=302)
         self.assertEqual(response.client.session.get('_go_user_account_key'),
-                         'test-1-user')
+                         self.ordinary_user_helper.account_key)

From 462a9b8b4732d9f1dac18a6f059f6a9371568279 Mon Sep 17 00:00:00 2001
From: Jeremy Thurgood <jeremy@praekelt.com>
Date: Wed, 25 Jun 2014 11:43:56 +0200
Subject: [PATCH 07/12] Handle empty POST variables in smart group view.

---
 go/contacts/tests.py | 18 ++++++++++++++----
 go/contacts/views.py |  4 ++++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/go/contacts/tests.py b/go/contacts/tests.py
index 1dbc5d8bf..a9a0f2724 100644
--- a/go/contacts/tests.py
+++ b/go/contacts/tests.py
@@ -1104,6 +1104,9 @@ def mksmart_group(self, query, name='a smart group'):
         self.assertRedirects(response, group_url(group.key))
         return group
 
+    def list_group_keys(self):
+        return [group.key for group in self.contact_store.list_groups()]
+
     def add_to_group(self, contact, group):
         contact.add_to_group(group)
         contact.save()
@@ -1114,14 +1117,21 @@ def test_smart_groups_creation(self):
         self.assertEqual(u'a smart group', group.name)
         self.assertEqual(u'msisdn:\+12*', group.query)
 
+    def test_smart_group_empty_post(self):
+        group = self.mksmart_group('msisdn:\+12*')
+        group_url = reverse('contacts:group', kwargs={'group_key': group.key})
+        response = self.client.post(group_url)
+        self.assertRedirects(response, group_url)
+        self.assertEqual(self.list_group_keys(), [group.key])
+
     def test_smart_group_deletion(self):
         group = self.mksmart_group('msisdn:\+12*')
-        response = self.client.post(
-            reverse('contacts:group', kwargs={'group_key': group.key}),
-            {'_delete_group': 1})
+        self.assertEqual(self.list_group_keys(), [group.key])
+        group_url = reverse('contacts:group', kwargs={'group_key': group.key})
+        response = self.client.post(group_url, {'_delete_group': 1})
         self.assertRedirects(response, reverse('contacts:index'),
                              target_status_code=302)
-        self.assertTrue(group not in self.contact_store.list_groups())
+        self.assertEqual(self.list_group_keys(), [])
 
     def test_smart_group_clearing(self):
         contact = self.mkcontact()
diff --git a/go/contacts/views.py b/go/contacts/views.py
index ff32a0e28..abafeb384 100644
--- a/go/contacts/views.py
+++ b/go/contacts/views.py
@@ -294,6 +294,10 @@ def _smart_group(request, contact_store, group):
                                      group.key)
             messages.info(request, 'The group will be deleted shortly.')
             return redirect(reverse('contacts:index'))
+        else:
+            # We didn't get any useful POST variables, so just redirect back to
+            # the group page without doing anything.
+            return redirect(_group_url(group.key))
     else:
         smart_group_form = SmartGroupForm({
             'name': group.name,

From 3f2e3553c45638b2f965fddb7459aaf0ba4c341e Mon Sep 17 00:00:00 2001
From: Jeremy Thurgood <jeremy@praekelt.com>
Date: Wed, 25 Jun 2014 12:07:42 +0200
Subject: [PATCH 08/12] Handle empty POST variables in static group view.

---
 go/contacts/tests.py | 13 +++++++++++++
 go/contacts/views.py |  5 +++++
 2 files changed, 18 insertions(+)

diff --git a/go/contacts/tests.py b/go/contacts/tests.py
index a9a0f2724..0557cc8e2 100644
--- a/go/contacts/tests.py
+++ b/go/contacts/tests.py
@@ -801,6 +801,9 @@ def get_all_contacts(self, keys=None):
     def get_latest_contact(self):
         return max(self.get_all_contacts(), key=lambda c: c.created_at)
 
+    def list_group_keys(self):
+        return [group.key for group in self.contact_store.list_groups()]
+
     def test_groups_creation(self):
         response = self.client.post(reverse('contacts:groups'), {
             'name': 'a new group',
@@ -894,6 +897,16 @@ def test_removing_contacts_from_group(self):
             [c2.key],
             self.contact_store.get_contacts_for_group(group))
 
+    def test_group_empty_post(self):
+        group = self.contact_store.new_group(TEST_GROUP_NAME)
+
+        self.assertEqual(self.list_group_keys(), [group.key])
+        group_url = reverse('contacts:group', kwargs={'group_key': group.key})
+        response = self.client.post(group_url)
+        self.assertRedirects(response, group_url)
+
+        self.assertEqual(self.list_group_keys(), [group.key])
+
     def test_group_deletion(self):
         group = self.contact_store.new_group(TEST_GROUP_NAME)
 
diff --git a/go/contacts/views.py b/go/contacts/views.py
index abafeb384..e907b13d4 100644
--- a/go/contacts/views.py
+++ b/go/contacts/views.py
@@ -209,6 +209,11 @@ def _static_group(request, contact_store, group):
                 utils.store_file_hints_in_session(
                     request, file_name, file_path)
                 return redirect(_group_url(group.key))
+            else:
+                # We didn't get any useful POST variables, so just redirect
+                # back to the group page without doing anything.
+                return redirect(_group_url(group.key))
+
     else:
         group_form = ContactGroupForm({
             'name': group.name,

From 3df6d5009d9cea23390e3da21cada5b8d980b493 Mon Sep 17 00:00:00 2001
From: Jeremy Thurgood <jeremy@praekelt.com>
Date: Wed, 25 Jun 2014 12:58:08 +0200
Subject: [PATCH 09/12] Check for missing or extra fields in CSV contact
 import.

---
 ...contacts-with-headers-and-extra-fields.csv | 24 ++++++++++
 ...ntacts-with-headers-and-missing-fields.csv | 24 ++++++++++
 go/contacts/parsers/csv_parser.py             | 16 ++++++-
 go/contacts/parsers/test_parsers.py           | 45 +++++++++++++++++++
 4 files changed, 108 insertions(+), 1 deletion(-)
 create mode 100644 go/base/fixtures/sample-contacts-with-headers-and-extra-fields.csv
 create mode 100644 go/base/fixtures/sample-contacts-with-headers-and-missing-fields.csv

diff --git a/go/base/fixtures/sample-contacts-with-headers-and-extra-fields.csv b/go/base/fixtures/sample-contacts-with-headers-and-extra-fields.csv
new file mode 100644
index 000000000..776b9e4d1
--- /dev/null
+++ b/go/base/fixtures/sample-contacts-with-headers-and-extra-fields.csv
@@ -0,0 +1,24 @@
+name,surname,msisdn
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Name 1,Surname 1,+27761234561
+Name 2,Surname 2,+27761234562,baz
+Name 3,Surname 3,+27761234563,foo,bar
+Name 4,Surname 4,+27761234564
diff --git a/go/base/fixtures/sample-contacts-with-headers-and-missing-fields.csv b/go/base/fixtures/sample-contacts-with-headers-and-missing-fields.csv
new file mode 100644
index 000000000..b3b427cb6
--- /dev/null
+++ b/go/base/fixtures/sample-contacts-with-headers-and-missing-fields.csv
@@ -0,0 +1,24 @@
+name,surname,msisdn
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Extra rows,to get past dialect sniffing,+27761234560
+Name 1,Surname 1,+27761234561
+Name 2,Surname 2
++27761234563
+Name 4,Surname 4,+27761234564
diff --git a/go/contacts/parsers/csv_parser.py b/go/contacts/parsers/csv_parser.py
index 68249b722..185ce68a4 100644
--- a/go/contacts/parsers/csv_parser.py
+++ b/go/contacts/parsers/csv_parser.py
@@ -39,10 +39,24 @@ def read_data_from_file(self, file_path, field_names, has_header):
             for row in reader:
                 # Only process rows that actually have data
                 if any([column for column in row]):
+                    if None in row:
+                        # Any extra fields are stuck in a list with a key of
+                        # `None`. The presence of this key means we have a row
+                        # with too many fields. We don't know how to handle
+                        # this case, so we abort.
+                        raise ContactParserException(
+                            'Invalid row: too many fields.')
+                    if None in row.values():
+                        # Any missing fields are given a value of `None`. Since
+                        # all legitimate field values are strings, this is a
+                        # reliable indicator of a missing field. We don't know
+                        # how to handle this case, so we abort.
+                        raise ContactParserException(
+                            'Invalid row: not enough fields.')
                     # Our Riak client requires unicode for all keys & values
                     # stored.
                     unicoded_row = dict([(key, unicode(value or '', 'utf-8'))
-                                            for key, value in row.items()])
+                                         for key, value in row.items()])
                     yield unicoded_row
         except csv.Error as e:
             raise ContactParserException(e)
diff --git a/go/contacts/parsers/test_parsers.py b/go/contacts/parsers/test_parsers.py
index 6f2784cd8..2b1193944 100644
--- a/go/contacts/parsers/test_parsers.py
+++ b/go/contacts/parsers/test_parsers.py
@@ -5,6 +5,7 @@
 from django.core.files.base import ContentFile
 
 from go.base.tests.helpers import GoDjangoTestCase
+from go.contacts.parsers import ContactParserException
 from go.contacts.parsers.csv_parser import CSVFileParser
 from go.contacts.parsers.xls_parser import XLSFileParser
 
@@ -103,6 +104,50 @@ def test_contacts_with_none_entries(self):
                 'name': 'Name 3'},
             ])
 
+    def test_contacts_with_missing_fields(self):
+        csv_file = self.fixture(
+            'sample-contacts-with-headers-and-missing-fields.csv')
+        fp = default_storage.open(csv_file, 'rU')
+        contacts_iter = self.parser.parse_file(fp, zip(
+            ['name', 'surname', 'msisdn'],
+            ['string', 'string', 'msisdn_za']), has_header=True)
+        contacts = []
+        try:
+            for contact in contacts_iter:
+                if contact['name'] == 'Extra rows':
+                    # We don't care about these rows.
+                    continue
+                contacts.append(contact)
+        except ContactParserException as err:
+            self.assertEqual(err.args[0], 'Invalid row: not enough fields.')
+        self.assertEqual(contacts, [{
+            'msisdn': '+27761234561',
+            'surname': 'Surname 1',
+            'name': 'Name 1',
+        }])
+
+    def test_contacts_with_extra_fields(self):
+        csv_file = self.fixture(
+            'sample-contacts-with-headers-and-extra-fields.csv')
+        fp = default_storage.open(csv_file, 'rU')
+        contacts_iter = self.parser.parse_file(fp, zip(
+            ['name', 'surname', 'msisdn'],
+            ['string', 'string', 'msisdn_za']), has_header=True)
+        contacts = []
+        try:
+            for contact in contacts_iter:
+                if contact['name'] == 'Extra rows':
+                    # We don't care about these rows.
+                    continue
+                contacts.append(contact)
+        except ContactParserException as err:
+            self.assertEqual(err.args[0], 'Invalid row: too many fields.')
+        self.assertEqual(contacts, [{
+            'msisdn': '+27761234561',
+            'surname': 'Surname 1',
+            'name': 'Name 1',
+        }])
+
 
 class TestXLSParser(ParserTestCase):
     PARSER_CLASS = XLSFileParser

From 98bcaf37e4ad185bd7d9e82d7d1bc0181d716798 Mon Sep 17 00:00:00 2001
From: Jeremy Thurgood <jeremy@praekelt.com>
Date: Wed, 25 Jun 2014 13:15:12 +0200
Subject: [PATCH 10/12] More group view POST redirecting.

---
 go/contacts/tests.py | 30 ++++++++++++++++++++----------
 go/contacts/views.py |  9 ++++++---
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/go/contacts/tests.py b/go/contacts/tests.py
index 0557cc8e2..b668dfa99 100644
--- a/go/contacts/tests.py
+++ b/go/contacts/tests.py
@@ -278,7 +278,8 @@ def test_contact_upload_into_new_group(self):
         self.assertRedirects(response, group_url(group.key))
         self.assertEqual(len(group.backlinks.contacts()), 0)
 
-        self.specify_columns(group.key)
+        response = self.specify_columns(group.key)
+        self.assertRedirects(response, group_url(group.key))
         self.assertEqual(len(group.backlinks.contacts()), 3)
         self.assertEqual(default_storage.listdir("tmp"), ([], []))
 
@@ -294,7 +295,8 @@ def test_contact_upload_into_existing_group(self):
         self.assertRedirects(response, group_url(group.key))
         group = self.contact_store.get_group(group.key)
         self.assertEqual(len(group.backlinks.contacts()), 0)
-        self.specify_columns(group.key)
+        response = self.specify_columns(group.key)
+        self.assertRedirects(response, group_url(group.key))
         self.assertEqual(len(group.backlinks.contacts()), 3)
         self.assertEqual(default_storage.listdir("tmp"), ([], []))
 
@@ -309,7 +311,8 @@ def test_uploading_unicode_chars_in_csv(self):
         })
         self.assertRedirects(response, group_url(group.key))
 
-        self.specify_columns(group.key)
+        response = self.specify_columns(group.key)
+        self.assertRedirects(response, group_url(group.key))
         group = self.contact_store.get_group(group.key)
         self.assertEqual(len(group.backlinks.contacts()), 3)
         self.assertEqual(len(mail.outbox), 1)
@@ -328,7 +331,7 @@ def test_uploading_windows_linebreaks_in_csv(self):
         })
         self.assertRedirects(response, group_url(group.key))
 
-        self.specify_columns(group.key, columns={
+        response = self.specify_columns(group.key, columns={
             'column-0': 'msisdn',
             'column-1': 'area',
             'column-2': 'nairobi_1',
@@ -346,6 +349,7 @@ def test_uploading_windows_linebreaks_in_csv(self):
             'normalize-6': '',
             'normalize-7': '',
         })
+        self.assertRedirects(response, group_url(group.key))
         group = self.contact_store.get_group(group.key)
         self.assertEqual(len(group.backlinks.contacts()), 2)
         self.assertEqual(len(mail.outbox), 1)
@@ -364,10 +368,11 @@ def test_uploading_single_colum(self):
         })
         self.assertRedirects(response, group_url(group.key))
 
-        self.specify_columns(group.key, columns={
+        response = self.specify_columns(group.key, columns={
             'column-0': 'msisdn',
             'normalize-0': '',
         })
+        self.assertRedirects(response, group_url(group.key))
 
         group = self.contact_store.get_group(group.key)
         self.assertEqual(len(group.backlinks.contacts()), 2)
@@ -452,7 +457,7 @@ def test_import_upload_is_truth(self):
         })
 
         self.assertRedirects(response, group_url(group.key))
-        self.specify_columns(group.key, columns={
+        response = self.specify_columns(group.key, columns={
             'column-0': 'key',
             'column-1': 'name',
             'column-2': 'surname',
@@ -461,6 +466,7 @@ def test_import_upload_is_truth(self):
             'column-5': 'litmus_new',
             'normalize-3': 'msisdn_za',
         }, import_rule='upload_is_truth')
+        self.assertRedirects(response, group_url(group.key))
 
         group = self.contact_store.get_group(group.key)
         self.assertEqual(len(group.backlinks.contacts()), 3)
@@ -544,7 +550,7 @@ def test_import_existing_is_truth(self):
         })
 
         self.assertRedirects(response, group_url(group1.key))
-        self.specify_columns(group1.key, columns={
+        response = self.specify_columns(group1.key, columns={
             'column-0': 'key',
             'column-1': 'name',
             'column-2': 'surname',
@@ -553,6 +559,7 @@ def test_import_existing_is_truth(self):
             'column-5': 'litmus_new',
             'normalize-3': 'msisdn_za',
         }, import_rule='existing_is_truth')
+        self.assertRedirects(response, group_url(group1.key))
 
         group = self.contact_store.get_group(group1.key)
         self.assertEqual(len(group.backlinks.contacts()), 3)
@@ -619,7 +626,8 @@ def test_uploading_unicode_chars_in_csv_into_new_group(self):
         group = newest(self.contact_store.list_groups())
         self.assertEqual(group.name, new_group_name)
         self.assertRedirects(response, group_url(group.key))
-        self.specify_columns(group_key=group.key)
+        response = self.specify_columns(group_key=group.key)
+        self.assertRedirects(response, group_url(group.key))
         self.assertEqual(len(group.backlinks.contacts()), 3)
         self.assertEqual(len(mail.outbox), 1)
         self.assertTrue('successfully' in mail.outbox[0].subject)
@@ -876,10 +884,11 @@ def test_multiple_group_deletion(self):
 
         # Delete the groups
         groups_url = reverse('contacts:groups')
-        self.client.post(groups_url, {
+        response = self.client.post(groups_url, {
             'group': [group_1.key, group_2.key],
             '_delete': True,
         })
+        self.assertRedirects(response, groups_url)
         self.assertEqual(self.contact_store.list_groups(), [])
 
     def test_removing_contacts_from_group(self):
@@ -888,10 +897,11 @@ def test_removing_contacts_from_group(self):
         c2 = self.mkcontact(groups=[group])
 
         group_url = reverse('contacts:group', kwargs={'group_key': group.key})
-        self.client.post(group_url, {
+        response = self.client.post(group_url, {
             '_remove': True,
             'contact': [c1.key]
         })
+        self.assertRedirects(response, group_url)
 
         self.assertEqual(
             [c2.key],
diff --git a/go/contacts/views.py b/go/contacts/views.py
index e907b13d4..f97a7d5bd 100644
--- a/go/contacts/views.py
+++ b/go/contacts/views.py
@@ -69,6 +69,7 @@ def groups(request, type=None):
                                          group.key)
             messages.info(request, '%d groups will be deleted '
                                    'shortly.' % len(groups))
+            return redirect(reverse('contacts:groups'))
         elif '_export' in request.POST:
             tasks.export_many_group_contacts.delay(
                 request.user_api.user_account_key,
@@ -76,6 +77,7 @@ def groups(request, type=None):
 
             messages.info(request, 'The export is scheduled and should '
                                    'complete within a few minutes.')
+            return redirect(reverse('contacts:groups'))
     else:
         contact_group_form = ContactGroupForm()
         smart_group_form = SmartGroupForm()
@@ -158,7 +160,7 @@ def _static_group(request, contact_store, group):
                           'The export is scheduled and should '
                           'complete within a few minutes.')
             return redirect(_group_url(group.key))
-        if '_remove' in request.POST:
+        elif '_remove' in request.POST:
             contacts = request.POST.getlist('contact')
             for person_key in contacts:
                 contact = contact_store.get_contact_by_key(person_key)
@@ -167,6 +169,7 @@ def _static_group(request, contact_store, group):
             messages.info(
                 request,
                 '%d Contacts removed from group' % len(contacts))
+            return redirect(_group_url(group.key))
         elif '_delete_group_contacts' in request.POST:
             tasks.delete_group_contacts.delay(
                 request.user_api.user_account_key, group.key)
@@ -193,13 +196,13 @@ def _static_group(request, contact_store, group):
                     'We will notify you via email when the import '
                     'has been completed')
 
-                return redirect(_group_url(group.key))
-
             except (ContactParserException,):
                 messages.error(request, 'Something is wrong with the file')
                 _, file_path = utils.get_file_hints_from_session(request)
                 default_storage.delete(file_path)
 
+            return redirect(_group_url(group.key))
+
         else:
             upload_contacts_form = UploadContactsForm(request.POST,
                                                       request.FILES)

From 341137f1b442423a80260807eb16b3586e57094f Mon Sep 17 00:00:00 2001
From: Jeremy Thurgood <jeremy@praekelt.com>
Date: Wed, 25 Jun 2014 15:27:29 +0200
Subject: [PATCH 11/12] Move CSV parser row checks.

---
 go/contacts/parsers/csv_parser.py | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/go/contacts/parsers/csv_parser.py b/go/contacts/parsers/csv_parser.py
index 185ce68a4..3d2daaaf8 100644
--- a/go/contacts/parsers/csv_parser.py
+++ b/go/contacts/parsers/csv_parser.py
@@ -37,22 +37,23 @@ def read_data_from_file(self, file_path, field_names, has_header):
             if has_header:
                 reader.next()
             for row in reader:
+                if None in row:
+                    # Any extra fields are stuck in a list with a key of
+                    # `None`. The presence of this key means we have a row
+                    # with too many fields. We don't know how to handle
+                    # this case, so we abort.
+                    raise ContactParserException(
+                        'Invalid row: too many fields.')
+                if None in row.values():
+                    # Any missing fields are given a value of `None`. Since
+                    # all legitimate field values are strings, this is a
+                    # reliable indicator of a missing field. We don't know
+                    # how to handle this case, so we abort.
+                    raise ContactParserException(
+                        'Invalid row: not enough fields.')
+
                 # Only process rows that actually have data
                 if any([column for column in row]):
-                    if None in row:
-                        # Any extra fields are stuck in a list with a key of
-                        # `None`. The presence of this key means we have a row
-                        # with too many fields. We don't know how to handle
-                        # this case, so we abort.
-                        raise ContactParserException(
-                            'Invalid row: too many fields.')
-                    if None in row.values():
-                        # Any missing fields are given a value of `None`. Since
-                        # all legitimate field values are strings, this is a
-                        # reliable indicator of a missing field. We don't know
-                        # how to handle this case, so we abort.
-                        raise ContactParserException(
-                            'Invalid row: not enough fields.')
                     # Our Riak client requires unicode for all keys & values
                     # stored.
                     unicoded_row = dict([(key, unicode(value or '', 'utf-8'))

From d47c474e3e9ea64b50d06304d8f3a4ae4e69be43 Mon Sep 17 00:00:00 2001
From: Jeremy Thurgood <jeremy@praekelt.com>
Date: Wed, 25 Jun 2014 17:52:25 +0200
Subject: [PATCH 12/12] Increase test timeout on Travis because node.js takes a
 long time to start. (@hodgestar)

---
 .travis.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 48cac507d..a2fbf1446 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -5,7 +5,7 @@ python:
 node_js:
   - "0.10"
 env:
-  - VUMITEST_REDIS_DB=1 VUMIGO_TEST_DB=postgres
+  - VUMITEST_REDIS_DB=1 VUMIGO_TEST_DB=postgres VUMI_TEST_TIMEOUT=10
 services:
   - riak
   - postgresql