From 79b4c0843b1c75e7358fbceb7ccd301f45b30d2f Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Mon, 7 May 2012 16:04:45 +0000 Subject: [PATCH] Add confirmation step so that the new charter effort page allows one to continue from a BoF (with a warning with confirmation being ticked) and continue from a WG (with another warning and confirmation not being ticked). Also allow overriding a historically used acronym (one that's in the GroupHistory) after confirmation. - Legacy-Id: 4402 --- ietf/templates/wginfo/edit.html | 60 +++++++++++---------- ietf/wginfo/edit.py | 95 +++++++++++++++++++++++---------- ietf/wginfo/tests.py | 32 +++++++++++ 3 files changed, 131 insertions(+), 56 deletions(-) diff --git a/ietf/templates/wginfo/edit.html b/ietf/templates/wginfo/edit.html index 82fea836f..5a740b3de 100644 --- a/ietf/templates/wginfo/edit.html +++ b/ietf/templates/wginfo/edit.html @@ -4,35 +4,20 @@ {% if wg %} Edit WG {{ wg.acronym }} {% else %} -Create WG +Start chartering new WG {% endif %} {% endblock %} {% block morecss %} -form.edit-info #id_name { - width: 396px; -} - -form.edit-info #id_list_email { - width: 396px; -} - -form.edit-info #id_list_subscribe { - width: 396px; -} - -form.edit-info #id_list_archive { - width: 396px; -} - -form.edit-info #id_urls { - width: 400px; -} - -form.edit-info #id_comments { - width: 400px; -} - +form.edit #id_name, +form.edit #id_list_email, +form.edit #id_list_subscribe, +form.edit #id_list_archive, +form.edit #id_urls, +form.edit #id_comments { width: 400px; } +form.edit input[type=checkbox] { vertical-align: middle; } +ul.errorlist { border-width: 0px; padding: 0px; margin: 0px;} +ul.errorlist li { color: #a00; margin: 0px; padding: 0px; list-style: none; } {% endblock %} {% block pagehead %} @@ -44,11 +29,11 @@ form.edit-info #id_comments {

{% if wg %} Edit WG {{ wg.acronym }} {% else %} -Create WG +Start chartering new WG {% endif %}

-
+ {% for field in form.visible_fields %} @@ -63,6 +48,14 @@ Create WG {{ field.errors }} + {% if field.name == "acronym" and form.confirm_msg %} + + + + + {% endif %} {% endfor %} @@ -71,7 +64,7 @@ Create WG Back {% else %} - + {% endif %} @@ -83,4 +76,15 @@ Create WG + {% endblock %} diff --git a/ietf/wginfo/edit.py b/ietf/wginfo/edit.py index ac7f56ccc..6d0340d18 100644 --- a/ietf/wginfo/edit.py +++ b/ietf/wginfo/edit.py @@ -33,7 +33,9 @@ class WGForm(forms.Form): urls = forms.CharField(widget=forms.Textarea, label="Additional URLs", help_text="Format: http://site/path (Optional description). Separate multiple entries with newline.", required=False) def __init__(self, *args, **kwargs): - self.cur_acronym = kwargs.pop('cur_acronym') + self.wg = kwargs.pop('wg', None) + self.confirmed = kwargs.pop('confirmed', False) + super(self.__class__, self).__init__(*args, **kwargs) # if previous AD is now ex-AD, append that person to the list @@ -42,16 +44,49 @@ class WGForm(forms.Form): 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 + def clean_acronym(self): + self.confirm_msg = "" + self.autoenable_confirm = False + acronym = self.cleaned_data['acronym'].strip().lower() - if not re.match(r'^[-\w]+$', acronym): + # be careful with acronyms, requiring confirmation to take existing or override historic + + if self.wg and acronym == self.wg.acronym: + return acronym # no change, no check + + if not re.match(r'^[-a-z0-9]+$', acronym): raise forms.ValidationError("Acronym is invalid, may only contain letters, numbers and dashes.") - if acronym != self.cur_acronym: - if Group.objects.filter(acronym__iexact=acronym): - raise forms.ValidationError("Acronym used in an existing group. Please pick another.") - if GroupHistory.objects.filter(acronym__iexact=acronym): - raise forms.ValidationError("Acronym used in a previous group. Please pick another.") + + existing = Group.objects.filter(acronym__iexact=acronym) + if existing: + existing = existing[0] + + if not self.wg and existing and existing.type_id == "wg": + if self.confirmed: + return acronym # take over confirmed + + if existing.state_id == "bof": + self.confirm_msg = "Turn BoF %s into proposed WG and start chartering it" % existing.acronym + self.autoenable_confirm = True + raise forms.ValidationError("Warning: Acronym used for an existing BoF (%s)." % existing.name) + else: + self.confirm_msg = "Set state of %s WG to proposed and start chartering it" % existing.acronym + self.autoenable_confirm = False + raise forms.ValidationError("Warning: Acronym used for an existing WG (%s, %s)." % (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="wg") + 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 WG.") + return acronym def clean_urls(self): @@ -71,10 +106,7 @@ def edit(request, acronym=None, action="edit"): """Edit or create a WG, notifying parties as necessary and logging changes as group events.""" if action == "edit": - # Editing. Get group wg = get_object_or_404(Group, acronym=acronym) - if not wg.charter: - raise Http404 new_wg = False elif action == "create": wg = None @@ -85,27 +117,34 @@ def edit(request, acronym=None, action="edit"): login = request.user.get_profile() if request.method == 'POST': - form = WGForm(request.POST, cur_acronym=wg.acronym if wg else None) + form = WGForm(request.POST, wg=wg, confirmed=request.POST.get("confirmed", False)) if form.is_valid(): - r = form.cleaned_data + clean = form.cleaned_data if new_wg: - # Create WG - wg = Group(name=r["name"], - acronym=r["acronym"], - type=GroupTypeName.objects.get(slug="wg"), - state=GroupStateName.objects.get(slug="proposed")) - wg.save() - + # get ourselves a proposed WG + try: + wg = Group.objects.get(acronym=clean["acronym"]) + + save_group_in_history(wg) + wg.state = GroupStateName.objects.get(slug="proposed") + wg.time = datetime.datetime.now() + wg.save() + except Group.DoesNotExist: + wg = Group.objects.create(name=clean["name"], + acronym=clean["acronym"], + type=GroupTypeName.objects.get(slug="wg"), + state=GroupStateName.objects.get(slug="proposed")) + e = ChangeStateGroupEvent(group=wg, type="changed_state") - e.time = datetime.datetime.now() + e.time = wg.time e.by = login e.state_id = "proposed" e.desc = "Proposed group" e.save() else: - gh = save_group_in_history(wg) + save_group_in_history(wg) - if not wg.charter: + if not wg.charter: # make sure we have a charter try: charter = Document.objects.get(docalias__name="charter-ietf-%s" % wg.acronym) except Document.DoesNotExist: @@ -139,9 +178,9 @@ def edit(request, acronym=None, action="edit"): def diff(attr, name): v = getattr(wg, attr) - if r[attr] != v: - changes.append(desc(name, r[attr], v)) - setattr(wg, attr, r[attr]) + if clean[attr] != v: + changes.append(desc(name, clean[attr], v)) + setattr(wg, attr, clean[attr]) prev_acronym = wg.acronym @@ -167,7 +206,7 @@ def edit(request, acronym=None, action="edit"): # update roles for attr, slug, title in [('chairs', 'chair', "Chairs"), ('secretaries', 'secr', "Secretaries"), ('techadv', 'techadv', "Tech Advisors")]: - new = r[attr] + new = clean[attr] old = Email.objects.filter(role__group=wg, role__name=slug).select_related("person") if set(new) != set(old): changes.append(desc(title, @@ -178,7 +217,7 @@ def edit(request, acronym=None, action="edit"): Role.objects.get_or_create(name_id=slug, email=e, group=wg, person=e.person) # update urls - new_urls = r['urls'] + new_urls = clean['urls'] old_urls = format_urls(wg.groupurl_set.order_by('url'), ", ") if ", ".join(sorted(new_urls)) != old_urls: changes.append(desc('Urls', ", ".join(sorted(new_urls)), old_urls)) @@ -223,7 +262,7 @@ def edit(request, acronym=None, action="edit"): else: init = dict(ad=login.id if has_role(request.user, "Area Director") else None, ) - form = WGForm(initial=init, cur_acronym=wg.acronym if wg else None) + form = WGForm(initial=init, wg=wg) return render_to_response('wginfo/edit.html', dict(wg=wg, diff --git a/ietf/wginfo/tests.py b/ietf/wginfo/tests.py index 51e34d0b2..66e89db51 100644 --- a/ietf/wginfo/tests.py +++ b/ietf/wginfo/tests.py @@ -120,6 +120,38 @@ class WgEditTestCase(django.test.TestCase): self.assertEquals(group.charter.name, "charter-ietf-testwg") self.assertEquals(group.charter.rev, "00-00") + def test_create_based_on_existing(self): + make_test_data() + + url = urlreverse('wg_create') + login_testing_unauthorized(self, "secretary", url) + + group = Group.objects.get(acronym="mars") + + # try hijacking area - faulty + r = self.client.post(url, dict(name="Test", acronym=group.parent.acronym)) + self.assertEquals(r.status_code, 200) + q = PyQuery(r.content) + self.assertTrue(len(q('form ul.errorlist')) > 0) + self.assertEquals(len(q('form input[name="confirmed"]')), 0) # can't confirm us out of this + + # try elevating BoF to WG + group.state_id = "bof" + group.save() + + r = self.client.post(url, dict(name="Test", acronym=group.acronym)) + self.assertEquals(r.status_code, 200) + q = PyQuery(r.content) + self.assertTrue(len(q('form ul.errorlist')) > 0) + self.assertEquals(len(q('form input[name="confirmed"]')), 1) + + self.assertEquals(Group.objects.get(acronym=group.acronym).state_id, "bof") + + # confirm elevation + r = self.client.post(url, dict(name="Test", acronym=group.acronym, confirmed="1")) + self.assertEquals(r.status_code, 302) + self.assertEquals(Group.objects.get(acronym=group.acronym).state_id, "proposed") + self.assertEquals(Group.objects.get(acronym=group.acronym).name, "Test") def test_edit_info(self): make_test_data()
+ {{ form.confirm_msg }} +