Defer Person creation/linkage until registration is confirmed

We currently create Person objects when a registration is submitted, not
when it is confirmed. This can lead to stale Person objects for
unconfirmed registrations.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

5 files changed, 33 insertions, 14 deletions

diff --git a/apps/patchwork/models.py b/apps/patchwork/models.py
index 7371d8f..9de2a22 100644
--- a/apps/patchwork/models.py
+++ b/apps/patchwork/models.py
@@ -112,18 +112,6 @@ class UserProfile(models.Model):
                          .values('pk').query)
         return qs
 
-    def save(self):
-        super(UserProfile, self).save()
-        people = Person.objects.filter(email = self.user.email)
-        if not people:
-            person = Person(email = self.user.email,
-                    name = self.name(), user = self.user)
-            person.save()
-        else:
-            for person in people:
-                 person.link_to_user(self.user)
-                 person.save()
-
     def __unicode__(self):
         return self.name()
 
diff --git a/apps/patchwork/tests/registration.py b/apps/patchwork/tests/registration.py
index 08ed66a..845b60b 100644
--- a/apps/patchwork/tests/registration.py
+++ b/apps/patchwork/tests/registration.py
@@ -155,12 +155,15 @@ class RegistrationConfirmationTest(TestCase):
         self.assertEqual(EmailConfirmation.objects.count(), 0)
         response = self.client.post('/register/', self.default_data)
         self.assertEquals(response.status_code, 200)
+        self.assertFalse(Person.objects.exists())
 
         # confirm
         conf = EmailConfirmation.objects.filter()[0]
         response = self.client.get(_confirmation_url(conf))
         self.assertEquals(response.status_code, 200)
 
+        qs = Person.objects.filter(email = self.user.email)
+        self.assertTrue(qs.exists())
         person = Person.objects.get(email = self.user.email)
 
         self.assertEquals(person.name,
@@ -188,3 +191,20 @@ class RegistrationConfirmationTest(TestCase):
 
         self.assertEquals(person.name, fullname)
 
+    def testRegistrationExistingPersonUnmodified(self):
+        """ Check that an unconfirmed registration can't modify an existing
+            Person object"""
+
+        fullname = self.user.firstname + ' '  + self.user.lastname
+        person = Person(name = fullname, email = self.user.email)
+        person.save()
+
+        # register
+        data = self.default_data.copy()
+        data['first_name'] = 'invalid'
+        data['last_name'] = 'invalid'
+        self.assertEquals(data['email'], person.email)
+        response = self.client.post('/register/', data)
+        self.assertEquals(response.status_code, 200)
+
+        self.assertEquals(Person.objects.get(pk = person.pk).name, fullname)
diff --git a/apps/patchwork/tests/user.py b/apps/patchwork/tests/user.py
index d35eacd..08b9fa5 100644
--- a/apps/patchwork/tests/user.py
+++ b/apps/patchwork/tests/user.py
@@ -105,12 +105,12 @@ class UserPersonConfirmTest(TestCase):
         self.conf.save()
 
     def testUserPersonConfirm(self):
-        self.assertEquals(Person.objects.count(), 1)
+        self.assertEquals(Person.objects.count(), 0)
         response = self.client.get(_confirmation_url(self.conf))
         self.assertEquals(response.status_code, 200)
 
         # check that the Person object has been created and linked
-        self.assertEquals(Person.objects.count(), 2)
+        self.assertEquals(Person.objects.count(), 1)
         person = Person.objects.get(email = self.user.secondary_email)
         self.assertEquals(person.email, self.user.secondary_email)
         self.assertEquals(person.user, self.user.user)
diff --git a/apps/patchwork/tests/utils.py b/apps/patchwork/tests/utils.py
index d4708aa..ec6a9ce 100644
--- a/apps/patchwork/tests/utils.py
+++ b/apps/patchwork/tests/utils.py
@@ -71,6 +71,9 @@ def create_user():
     user = User.objects.create_user(userid, email, userid)
     user.save()
 
+    person = Person(email = email, name = userid, user = user)
+    person.save()
+
     return user
 
 def create_maintainer(project):
diff --git a/apps/patchwork/views/user.py b/apps/patchwork/views/user.py
index 4a0e845..a9d6c4c 100644
--- a/apps/patchwork/views/user.py
+++ b/apps/patchwork/views/user.py
@@ -82,6 +82,14 @@ def register_confirm(request, conf):
     conf.user.is_active = True
     conf.user.save()
     conf.deactivate()
+    try:
+        person = Person.objects.get(email__iexact = conf.user.email)
+    except Person.DoesNotExist:
+        person = Person(email = conf.user.email,
+                name = conf.user.get_profile().name())
+    person.user = conf.user
+    person.save()
+
     return render_to_response('patchwork/registration-confirm.html')
 
 @login_required