Take a different approach to avoiding the crash when a team secretary uses the bulk assignment form to unassign a request. Reverts r14570. Fixes ietf-tools#2443. Commit ready for merge.

 - Legacy-Id: 14623

Author: rjsparks

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):

ietf/group/tests_review.py

@@ -296,6 +296,19 @@ def test_manage_review_requests(self):
         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)

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

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