Summary: Fix problem with confirm checkbox for potentially dangerous

acronym choices on group edit/creation page, fixes the last remaining
facelift test failure
 - Legacy-Id: 8720

Author: OleLaursen

ietf/group/edit.py

@@ -22,6 +22,7 @@
 from ietf.person.fields import SearchableEmailsField
 from ietf.person.models import Person, Email
 from ietf.group.mails import email_iesg_secretary_re_charter
+from ietf.utils.ordereddict import insert_after_in_ordered_dict
 
 MAX_GROUP_DELEGATES = 3
 
@@ -43,7 +44,6 @@ class GroupForm(forms.Form):
 
     def __init__(self, *args, **kwargs):
         self.group = kwargs.pop('group', None)
-        self.confirmed = kwargs.pop('confirmed', False)
         self.group_type = kwargs.pop('group_type', False)
 
         super(self.__class__, self).__init__(*args, **kwargs)
@@ -57,8 +57,6 @@ def __init__(self, *args, **kwargs):
         if ad_pk and ad_pk not in [pk for pk, name in choices]:
             self.fields['ad'].choices = list(choices) + [("", "-------"), (ad_pk, Person.objects.get(pk=ad_pk).plain_name())]
 
-        self.confirm_msg = ""
-        self.autoenable_confirm = False
         if self.group:
             self.fields['acronym'].widget.attrs['readonly'] = ""
 
@@ -71,9 +69,6 @@ def __init__(self, *args, **kwargs):
             self.fields['parent'].label = "IETF Area"
 
     def clean_acronym(self):
-        self.confirm_msg = ""
-        self.autoenable_confirm = False
-
         # Changing the acronym of an already existing group will cause 404s all
         # over the place, loose history, and generally muck up a lot of
         # things, so we don't permit it
@@ -90,27 +85,41 @@ def clean_acronym(self):
         if existing:
             existing = existing[0]
 
-        if existing and existing.type_id == self.group_type:
-            if self.confirmed:
-                return acronym # take over confirmed
+        confirmed = self.data.get("confirm_acronym", False)
+
+        def insert_confirm_field(label, initial):
+            # set required to false, we don't need it since we do the
+            # validation of the field in here, and otherwise the
+            # browser and Django may barf
+            insert_after_in_ordered_dict(self.fields, "confirm_acronym", forms.BooleanField(label=label, required=False), after="acronym")
+            # we can't set initial, it's ignored since the form is bound, instead mutate the data
+            self.data = self.data.copy()
+            self.data["confirm_acronym"] = initial
 
+        if existing and existing.type_id == self.group_type:
             if existing.state_id == "bof":
-                self.confirm_msg = "Turn BoF %s into proposed %s and start chartering it" % (existing.acronym, existing.type.name)
-                self.autoenable_confirm = True
-                raise forms.ValidationError("Warning: Acronym used for an existing BoF (%s)." % existing.name)
+                insert_confirm_field(label="Turn BoF %s into proposed %s and start chartering it" % (existing.acronym, existing.type.name), initial=True)
+                if confirmed:
+                    return acronym
+                else:
+                    raise forms.ValidationError("Warning: Acronym used for an existing BoF (%s)." % existing.name)
             else:
-                self.confirm_msg = "Set state of %s %s to proposed and start chartering it" % (existing.acronym, existing.type.name)
-                self.autoenable_confirm = False
-                raise forms.ValidationError("Warning: Acronym used for an existing %s (%s, %s)." % (existing.type.name, existing.name, existing.state.name if existing.state else "unknown state"))
+                insert_confirm_field(label="Set state of %s %s to proposed and start chartering it" % (existing.acronym, existing.type.name), initial=False)
+                if confirmed:
+                    return acronym
+                else:
+                    raise forms.ValidationError("Warning: Acronym used for an existing %s (%s, %s)." % (existing.type.name, existing.name, existing.state.name if existing.state else "unknown state"))
 
         if existing:
             raise forms.ValidationError("Acronym used for an existing group (%s)." % existing.name)
 
         old = GroupHistory.objects.filter(acronym__iexact=acronym, type__in=("wg", "rg"))
-        if old and not self.confirmed:
-            self.confirm_msg = "Confirm reusing acronym %s" % old[0].acronym
-            self.autoenable_confirm = False
-            raise forms.ValidationError("Warning: Acronym used for a historic group.")
+        if old:
+            insert_confirm_field(label="Confirm reusing acronym %s" % old[0].acronym, initial=False)
+            if confirmed:
+                return acronym
+            else:
+                raise forms.ValidationError("Warning: Acronym used for a historic group.")
 
         return acronym
 
@@ -190,7 +199,7 @@ def edit(request, group_type=None, acronym=None, action="edit"):
         group_type = group.type_id
 
     if request.method == 'POST':
-        form = GroupForm(request.POST, group=group, confirmed=request.POST.get("confirmed", False), group_type=group_type)
+        form = GroupForm(request.POST, group=group, group_type=group_type)
         if form.is_valid():
             clean = form.cleaned_data
             if new_group:

ietf/group/tests_info.py

@@ -342,7 +342,7 @@ def test_create_based_on_existing_bof(self):
         self.assertEqual(r.status_code, 200)
         q = PyQuery(r.content)
         self.assertTrue(len(q('form .has-error')) > 0)
-        self.assertEqual(len(q('form input[name="confirmed"]')), 0) # can't confirm us out of this
+        self.assertEqual(len(q('form input[name="confirm_acronym"]')), 0) # can't confirm us out of this
 
         # try elevating BoF to WG
         group.state_id = "bof"
@@ -352,13 +352,13 @@ def test_create_based_on_existing_bof(self):
         self.assertEqual(r.status_code, 200)
         q = PyQuery(r.content)
         self.assertTrue(len(q('form .has-error')) > 0)
-        self.assertEqual(len(q('form input[name="confirmed"]')), 1)
+        self.assertEqual(len(q('form input[name="confirm_acronym"]')), 1)
 
         self.assertEqual(Group.objects.get(acronym=group.acronym).state_id, "bof")
 
         # confirm elevation
         state = GroupStateName.objects.get(slug="proposed")
-        r = self.client.post(url, dict(name="Test", acronym=group.acronym, confirmed="1",state=state.pk))
+        r = self.client.post(url, dict(name="Test", acronym=group.acronym, confirm_acronym="1", state=state.pk))
         self.assertEqual(r.status_code, 302)
         self.assertEqual(Group.objects.get(acronym=group.acronym).state_id, "proposed")
         self.assertEqual(Group.objects.get(acronym=group.acronym).name, "Test")

ietf/templates/group/edit.html

@@ -12,8 +12,8 @@
 {% endblock %}
 
 {% block pagehead %}
-<link rel="stylesheet" href="/facelift/css/lib/select2.css">
-<link rel="stylesheet" href="/facelift/css/lib/select2-bootstrap.css">
+  <link rel="stylesheet" href="/facelift/css/lib/select2.css">
+  <link rel="stylesheet" href="/facelift/css/lib/select2-bootstrap.css">
 {% endblock %}
 
 {% block content %}
@@ -51,5 +51,18 @@ <h1>
 {% endblock %}
 
 {% block js %}
-<script src="/facelift/js/lib/select2-3.5.2.min.js"></script>
+  <script src="/facelift/js/lib/select2-3.5.2.min.js"></script>
+  <script>
+   $(document).ready(function () {
+       $("#id_acronym").closest(".form-group").each(function() {
+           // fixup styling a bit in case the confirm checkbox is shown
+           if ($(this).next().find("#id_confirm_acronym").length > 0) {
+               $(this).css("margin-bottom", 0);
+               $(this).find(".help-block").css("margin-bottom", 0);
+               if ($(this).hasClass("has-error"))
+                   $(this).next().addClass("has-error");
+           }
+       });
+   });
+  </script>
 {% endblock %}