From ca715dd8d4d77416b04ebb404d77061430351ddc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C4=93teris=20Caune?= <cuu508@gmail.com>
Date: Mon, 13 Apr 2020 15:19:37 +0300
Subject: [PATCH] Check membership when initiating project's transfer. Use
 transaction.atomic() when completing the transfer.

---
 hc/accounts/tests/test_transfer_project.py |  7 +++++
 hc/accounts/views.py                       | 33 +++++++++++++---------
 2 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/hc/accounts/tests/test_transfer_project.py b/hc/accounts/tests/test_transfer_project.py
index ad94f77b..9a127e1a 100644
--- a/hc/accounts/tests/test_transfer_project.py
+++ b/hc/accounts/tests/test_transfer_project.py
@@ -34,6 +34,13 @@ class ProjectTestCase(BaseTestCase):
         r = self.client.post(self.url, form)
         self.assertEqual(r.status_code, 403)
 
+    def test_transfer_project_checks_membership(self):
+        self.client.login(username="alice@example.org", password="password")
+
+        form = {"transfer_project": "1", "email": "charlie@example.org"}
+        r = self.client.post(self.url, form)
+        self.assertEqual(r.status_code, 400)
+
     def test_cancel_works(self):
         self.bobs_membership.transfer_request_date = now()
         self.bobs_membership.save()
diff --git a/hc/accounts/views.py b/hc/accounts/views.py
index 43a1a652..47b083b6 100644
--- a/hc/accounts/views.py
+++ b/hc/accounts/views.py
@@ -2,6 +2,7 @@ from datetime import timedelta as td
 from urllib.parse import urlparse
 import uuid
 
+from django.db import transaction
 from django.conf import settings
 from django.contrib import messages
 from django.contrib.auth import login as auth_login
@@ -339,21 +340,27 @@ def project(request, code):
 
             form = forms.TransferForm(request.POST)
             if form.is_valid():
+                # Look up the proposed new owner
                 email = form.cleaned_data["email"]
+                try:
+                    membership = project.member_set.filter(user__email=email).get()
+                except Member.DoesNotExist:
+                    return HttpResponseBadRequest()
 
                 # Revoke any previous transfer requests
                 project.member_set.update(transfer_request_date=None)
 
                 # Initiate the new request
-                q = project.member_set.filter(user__email=email)
-                q.update(transfer_request_date=now())
+                membership.transfer_request_date = now()
+                membership.save()
+
+                # Send an email notification
+                profile = Profile.objects.for_user(membership.user)
+                profile.send_transfer_request(project)
 
                 ctx["transfer_initiated"] = True
                 ctx["transfer_status"] = "success"
 
-                profile = Profile.objects.get(user__email=email)
-                profile.send_transfer_request(project)
-
         elif "cancel_transfer" in request.POST:
             if not is_owner:
                 return HttpResponseForbidden()
@@ -370,15 +377,15 @@ def project(request, code):
             if not tr.can_accept():
                 return HttpResponseBadRequest()
 
-            # 1. Remove user's membership
-            tr.delete()
-
-            # 2. Invite the current owner as a member
-            Member.objects.create(user=project.owner, project=project)
+            with transaction.atomic():
+                # 1. Reuse the existing membership, and change its user
+                tr.user = project.owner
+                tr.transfer_request_date = None
+                tr.save()
 
-            # 3. Change project's owner
-            project.owner = request.user
-            project.save()
+                # 2. Change project's owner
+                project.owner = request.user
+                project.save()
 
             ctx["is_owner"] = True
             messages.success(request, "You are now the owner of this project!")