From 1997a90d103017b31c8c84a19222a5edbbcc389b Mon Sep 17 00:00:00 2001 From: Henrik Levkowetz Date: Sun, 7 Jul 2019 12:49:17 +0000 Subject: [PATCH] Removed the insertion of 'confirm_acronym' in the active form in clean_acronym(). It triggers an exception due to changing a dictionary while the django validation code is iterating through the form fields. XXX Check if we need to put this back some other way, or if we can handle acronym re-use through the admin instead. - Legacy-Id: 16412 --- ietf/group/forms.py | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/ietf/group/forms.py b/ietf/group/forms.py index c84c6a318..6663ec796 100644 --- a/ietf/group/forms.py +++ b/ietf/group/forms.py @@ -19,7 +19,7 @@ from ietf.review.models import ReviewerSettings, UnavailablePeriod, ReviewSecret from ietf.review.utils import close_review_request_states, setup_reviewer_field from ietf.utils.textupload import get_cleaned_text_file_content from ietf.utils.text import strip_suffix -from ietf.utils.ordereddict import insert_after_in_ordered_dict +#from ietf.utils.ordereddict import insert_after_in_ordered_dict from ietf.utils.fields import DatepickerDateField, MultiEmailField # --- Constants -------------------------------------------------------- @@ -122,6 +122,7 @@ class GroupForm(forms.Form): del self.fields[f] def clean_acronym(self): + try: # 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 @@ -140,42 +141,45 @@ class GroupForm(forms.Form): 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 +# 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": - insert_confirm_field(label="Turn BoF %s into proposed %s and start chartering it" % (existing.acronym, existing.type.name), initial=True) + #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) + raise forms.ValidationError("Warning: Acronym used for an existing BoF (%s)." % existing.acronym) else: - insert_confirm_field(label="Set state of %s %s to proposed and start chartering it" % (existing.acronym, existing.type.name), initial=False) + #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")) + raise forms.ValidationError("Warning: Acronym used for an existing %s (%s, %s)." % (existing.type.name, existing.acronym, existing.state.name if existing.state else "unknown state")) if existing: - raise forms.ValidationError("Acronym used for an existing group (%s)." % existing.name) + raise forms.ValidationError("Acronym used for an existing group (%s)." % existing.acronym) - # TODO: Why is this limited to types wg and rg? We would want to be warned about _any_ old collision I think? - old = GroupHistory.objects.filter(acronym__iexact=acronym, type__in=("wg", "rg")) + old = GroupHistory.objects.filter(acronym__iexact=acronym) if old: - insert_confirm_field(label="Confirm reusing acronym %s" % old[0].acronym, initial=False) + #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 + except forms.ValidationError: + pass + except Exception: + import traceback + traceback.print_exc() def clean_urls(self): return [x.strip() for x in self.cleaned_data["urls"].splitlines() if x.strip()] @@ -199,7 +203,7 @@ class GroupForm(forms.Form): else: raise forms.ValidationError("A group cannot be its own ancestor. " "Found that the group '%s' would end up being the ancestor of (%s)" % (p.acronym, ', '.join([g.acronym for g in seen]))) - + def clean(self): cleaned_data = super(GroupForm, self).clean() state = cleaned_data.get('state', None)