From 7b89db153eb5048e4810150218eaeb54d12e30a9 Mon Sep 17 00:00:00 2001
From: Robert Sparks <rjsparks@nostrum.com>
Date: Wed, 7 Feb 2018 17:58:27 +0000
Subject: [PATCH] Take a different approach to avoiding the crash when a team
 secretary uses the bulk assignment form to unassign a request. Reverts
 r14570. Fixes #2443. Commit ready for merge.  - Legacy-Id: 14623

---
 ietf/group/forms.py        |  2 +-
 ietf/group/tests_review.py | 13 +++++++++++++
 ietf/group/views.py        |  3 ++-
 ietf/review/utils.py       |  2 +-
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/ietf/group/forms.py b/ietf/group/forms.py
index dc05343b9..023d2e0ab 100644
--- a/ietf/group/forms.py
+++ b/ietf/group/forms.py
@@ -219,7 +219,7 @@ class ManageReviewRequestForm(forms.Form):
 
     action = forms.ChoiceField(choices=ACTIONS, widget=forms.HiddenInput, required=False)
     close = forms.ModelChoiceField(queryset=close_review_request_states(), required=False)
-    reviewer = PersonEmailChoiceField(empty_label=None, required=False, label_with="person")
+    reviewer = PersonEmailChoiceField(empty_label="(None)", required=False, label_with="person")
     add_skip = forms.BooleanField(required=False)
 
     def __init__(self, review_req, *args, **kwargs):
diff --git a/ietf/group/tests_review.py b/ietf/group/tests_review.py
index 0eaf2f840..66c05bb45 100644
--- a/ietf/group/tests_review.py
+++ b/ietf/group/tests_review.py
@@ -296,6 +296,19 @@ class ReviewTests(TestCase):
         self.assertEqual(settings.skip_next,1)
         self.assertEqual(review_req3.state_id, "requested")
 
+        r = self.client.post(assigned_url, {
+            "reviewrequest": [str(review_req2.pk)],
+            "r{}-existing_reviewer".format(review_req2.pk): review_req2.reviewer_id or "",
+            "r{}-action".format(review_req2.pk): "assign",
+            "r{}-reviewer".format(review_req2.pk): "",
+            "r{}-add_skip".format(review_req2.pk) : 0,
+            "action": "save",
+        })
+        self.assertEqual(r.status_code, 302)
+        review_req2 = reload_db_objects(review_req2)
+        self.assertEqual(review_req2.state_id, "requested")
+        self.assertEqual(review_req2.reviewer, None)
+
     def test_email_open_review_assignments(self):
         doc = make_test_data()
         review_req1 = make_review_data(doc)
diff --git a/ietf/group/views.py b/ietf/group/views.py
index d82ae51e8..95d800cb6 100644
--- a/ietf/group/views.py
+++ b/ietf/group/views.py
@@ -1475,7 +1475,8 @@ def manage_review_requests(request, acronym, group_type=None, assignment_status=
 
             assignments_by_person = dict()
             for r in reqs_to_assign:
-                assignments_by_person[r.form.cleaned_data["reviewer"].person] = r
+                if r.form.cleaned_data["reviewer"]:
+                    assignments_by_person[r.form.cleaned_data["reviewer"].person] = r
             
             # Make sure the any assignments to the person at the head
             # of the rotation queue are processed first so that the queue
diff --git a/ietf/review/utils.py b/ietf/review/utils.py
index b61ed27ec..407a5e23d 100644
--- a/ietf/review/utils.py
+++ b/ietf/review/utils.py
@@ -699,7 +699,7 @@ def setup_reviewer_field(field, review_req):
         field.initial = review_req.reviewer_id
 
     choices = make_assignment_choices(field.queryset, review_req)
-    if not field.required and not field.empty_label is None:
+    if not field.required:
         choices = [("", field.empty_label)] + choices
 
     field.choices = choices