Skip to content

Commit 79b4c08

Browse files
committed
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
1 parent 2dbed6a commit 79b4c08

3 files changed

Lines changed: 131 additions & 56 deletions

File tree

ietf/templates/wginfo/edit.html

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,35 +4,20 @@
44
{% if wg %}
55
Edit WG {{ wg.acronym }}
66
{% else %}
7-
Create WG
7+
Start chartering new WG
88
{% endif %}
99
{% endblock %}
1010

1111
{% block morecss %}
12-
form.edit-info #id_name {
13-
width: 396px;
14-
}
15-
16-
form.edit-info #id_list_email {
17-
width: 396px;
18-
}
19-
20-
form.edit-info #id_list_subscribe {
21-
width: 396px;
22-
}
23-
24-
form.edit-info #id_list_archive {
25-
width: 396px;
26-
}
27-
28-
form.edit-info #id_urls {
29-
width: 400px;
30-
}
31-
32-
form.edit-info #id_comments {
33-
width: 400px;
34-
}
35-
12+
form.edit #id_name,
13+
form.edit #id_list_email,
14+
form.edit #id_list_subscribe,
15+
form.edit #id_list_archive,
16+
form.edit #id_urls,
17+
form.edit #id_comments { width: 400px; }
18+
form.edit input[type=checkbox] { vertical-align: middle; }
19+
ul.errorlist { border-width: 0px; padding: 0px; margin: 0px;}
20+
ul.errorlist li { color: #a00; margin: 0px; padding: 0px; list-style: none; }
3621
{% endblock %}
3722

3823
{% block pagehead %}
@@ -44,11 +29,11 @@
4429
<h1>{% if wg %}
4530
Edit WG {{ wg.acronym }}
4631
{% else %}
47-
Create WG
32+
Start chartering new WG
4833
{% endif %}
4934
</h1>
5035

51-
<form class="edit-info" action="" method="POST">
36+
<form class="edit" action="" method="POST">
5237
<table>
5338
{% for field in form.visible_fields %}
5439
<tr>
@@ -63,6 +48,14 @@ <h1>{% if wg %}
6348
{{ field.errors }}
6449
</td>
6550
</tr>
51+
{% if field.name == "acronym" and form.confirm_msg %}
52+
<tr>
53+
<td></td>
54+
<td><input name="confirmed" type="checkbox"{% if form.autoenable_confirm %} checked="checked"{% endif %}/>
55+
{{ form.confirm_msg }}
56+
</td>
57+
</tr>
58+
{% endif %}
6659
{% endfor %}
6760
<tr>
6861
<td></td>
@@ -71,7 +64,7 @@ <h1>{% if wg %}
7164
<a href="{% url wg_charter acronym=wg.acronym %}">Back</a>
7265
<input type="submit" value="Save"/>
7366
{% else %}
74-
<input type="submit" value="Create"/>
67+
<input type="submit" value="Start chartering"/>
7568
{% endif %}
7669
</td>
7770
</tr>
@@ -83,4 +76,15 @@ <h1>{% if wg %}
8376
<script type="text/javascript" src="/js/lib/jquery.tokeninput.js"></script>
8477
<script type="text/javascript" src="/js/lib/json2.js"></script>
8578
<script type="text/javascript" src="/js/emails-field.js"></script>
79+
<script>
80+
jQuery(function () {
81+
if (jQuery('input[name="confirmed"]').length > 0) {
82+
jQuery('input[name="acronym"]').change(function() {
83+
// make sure we don't accidentally confirm another acronym
84+
jQuery('input[name="confirmed"]').closest("tr").remove();
85+
jQuery('input[name="acronym"]').siblings(".errorlist").remove();
86+
});
87+
}
88+
});
89+
</script>
8690
{% endblock %}

ietf/wginfo/edit.py

Lines changed: 67 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ class WGForm(forms.Form):
3333
urls = forms.CharField(widget=forms.Textarea, label="Additional URLs", help_text="Format: http://site/path (Optional description). Separate multiple entries with newline.", required=False)
3434

3535
def __init__(self, *args, **kwargs):
36-
self.cur_acronym = kwargs.pop('cur_acronym')
36+
self.wg = kwargs.pop('wg', None)
37+
self.confirmed = kwargs.pop('confirmed', False)
38+
3739
super(self.__class__, self).__init__(*args, **kwargs)
3840

3941
# if previous AD is now ex-AD, append that person to the list
@@ -42,16 +44,49 @@ def __init__(self, *args, **kwargs):
4244
if ad_pk and ad_pk not in [pk for pk, name in choices]:
4345
self.fields['ad'].choices = list(choices) + [("", "-------"), (ad_pk, Person.objects.get(pk=ad_pk).plain_name())]
4446

47+
self.confirm_msg = ""
48+
self.autoenable_confirm = False
49+
4550
def clean_acronym(self):
51+
self.confirm_msg = ""
52+
self.autoenable_confirm = False
53+
4654
acronym = self.cleaned_data['acronym'].strip().lower()
4755

48-
if not re.match(r'^[-\w]+$', acronym):
56+
# be careful with acronyms, requiring confirmation to take existing or override historic
57+
58+
if self.wg and acronym == self.wg.acronym:
59+
return acronym # no change, no check
60+
61+
if not re.match(r'^[-a-z0-9]+$', acronym):
4962
raise forms.ValidationError("Acronym is invalid, may only contain letters, numbers and dashes.")
50-
if acronym != self.cur_acronym:
51-
if Group.objects.filter(acronym__iexact=acronym):
52-
raise forms.ValidationError("Acronym used in an existing group. Please pick another.")
53-
if GroupHistory.objects.filter(acronym__iexact=acronym):
54-
raise forms.ValidationError("Acronym used in a previous group. Please pick another.")
63+
64+
existing = Group.objects.filter(acronym__iexact=acronym)
65+
if existing:
66+
existing = existing[0]
67+
68+
if not self.wg and existing and existing.type_id == "wg":
69+
if self.confirmed:
70+
return acronym # take over confirmed
71+
72+
if existing.state_id == "bof":
73+
self.confirm_msg = "Turn BoF %s into proposed WG and start chartering it" % existing.acronym
74+
self.autoenable_confirm = True
75+
raise forms.ValidationError("Warning: Acronym used for an existing BoF (%s)." % existing.name)
76+
else:
77+
self.confirm_msg = "Set state of %s WG to proposed and start chartering it" % existing.acronym
78+
self.autoenable_confirm = False
79+
raise forms.ValidationError("Warning: Acronym used for an existing WG (%s, %s)." % (existing.name, existing.state.name if existing.state else "unknown state"))
80+
81+
if existing:
82+
raise forms.ValidationError("Acronym used for an existing group (%s)." % existing.name)
83+
84+
old = GroupHistory.objects.filter(acronym__iexact=acronym, type="wg")
85+
if old and not self.confirmed:
86+
self.confirm_msg = "Confirm reusing acronym %s" % old[0].acronym
87+
self.autoenable_confirm = False
88+
raise forms.ValidationError("Warning: Acronym used for a historic WG.")
89+
5590
return acronym
5691

5792
def clean_urls(self):
@@ -71,10 +106,7 @@ def edit(request, acronym=None, action="edit"):
71106
"""Edit or create a WG, notifying parties as
72107
necessary and logging changes as group events."""
73108
if action == "edit":
74-
# Editing. Get group
75109
wg = get_object_or_404(Group, acronym=acronym)
76-
if not wg.charter:
77-
raise Http404
78110
new_wg = False
79111
elif action == "create":
80112
wg = None
@@ -85,27 +117,34 @@ def edit(request, acronym=None, action="edit"):
85117
login = request.user.get_profile()
86118

87119
if request.method == 'POST':
88-
form = WGForm(request.POST, cur_acronym=wg.acronym if wg else None)
120+
form = WGForm(request.POST, wg=wg, confirmed=request.POST.get("confirmed", False))
89121
if form.is_valid():
90-
r = form.cleaned_data
122+
clean = form.cleaned_data
91123
if new_wg:
92-
# Create WG
93-
wg = Group(name=r["name"],
94-
acronym=r["acronym"],
95-
type=GroupTypeName.objects.get(slug="wg"),
96-
state=GroupStateName.objects.get(slug="proposed"))
97-
wg.save()
98-
124+
# get ourselves a proposed WG
125+
try:
126+
wg = Group.objects.get(acronym=clean["acronym"])
127+
128+
save_group_in_history(wg)
129+
wg.state = GroupStateName.objects.get(slug="proposed")
130+
wg.time = datetime.datetime.now()
131+
wg.save()
132+
except Group.DoesNotExist:
133+
wg = Group.objects.create(name=clean["name"],
134+
acronym=clean["acronym"],
135+
type=GroupTypeName.objects.get(slug="wg"),
136+
state=GroupStateName.objects.get(slug="proposed"))
137+
99138
e = ChangeStateGroupEvent(group=wg, type="changed_state")
100-
e.time = datetime.datetime.now()
139+
e.time = wg.time
101140
e.by = login
102141
e.state_id = "proposed"
103142
e.desc = "Proposed group"
104143
e.save()
105144
else:
106-
gh = save_group_in_history(wg)
145+
save_group_in_history(wg)
107146

108-
if not wg.charter:
147+
if not wg.charter: # make sure we have a charter
109148
try:
110149
charter = Document.objects.get(docalias__name="charter-ietf-%s" % wg.acronym)
111150
except Document.DoesNotExist:
@@ -139,9 +178,9 @@ def desc(attr, new, old):
139178

140179
def diff(attr, name):
141180
v = getattr(wg, attr)
142-
if r[attr] != v:
143-
changes.append(desc(name, r[attr], v))
144-
setattr(wg, attr, r[attr])
181+
if clean[attr] != v:
182+
changes.append(desc(name, clean[attr], v))
183+
setattr(wg, attr, clean[attr])
145184

146185
prev_acronym = wg.acronym
147186

@@ -167,7 +206,7 @@ def diff(attr, name):
167206

168207
# update roles
169208
for attr, slug, title in [('chairs', 'chair', "Chairs"), ('secretaries', 'secr', "Secretaries"), ('techadv', 'techadv', "Tech Advisors")]:
170-
new = r[attr]
209+
new = clean[attr]
171210
old = Email.objects.filter(role__group=wg, role__name=slug).select_related("person")
172211
if set(new) != set(old):
173212
changes.append(desc(title,
@@ -178,7 +217,7 @@ def diff(attr, name):
178217
Role.objects.get_or_create(name_id=slug, email=e, group=wg, person=e.person)
179218

180219
# update urls
181-
new_urls = r['urls']
220+
new_urls = clean['urls']
182221
old_urls = format_urls(wg.groupurl_set.order_by('url'), ", ")
183222
if ", ".join(sorted(new_urls)) != old_urls:
184223
changes.append(desc('Urls', ", ".join(sorted(new_urls)), old_urls))
@@ -223,7 +262,7 @@ def diff(attr, name):
223262
else:
224263
init = dict(ad=login.id if has_role(request.user, "Area Director") else None,
225264
)
226-
form = WGForm(initial=init, cur_acronym=wg.acronym if wg else None)
265+
form = WGForm(initial=init, wg=wg)
227266

228267
return render_to_response('wginfo/edit.html',
229268
dict(wg=wg,

ietf/wginfo/tests.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,38 @@ def test_create(self):
120120
self.assertEquals(group.charter.name, "charter-ietf-testwg")
121121
self.assertEquals(group.charter.rev, "00-00")
122122

123+
def test_create_based_on_existing(self):
124+
make_test_data()
125+
126+
url = urlreverse('wg_create')
127+
login_testing_unauthorized(self, "secretary", url)
128+
129+
group = Group.objects.get(acronym="mars")
130+
131+
# try hijacking area - faulty
132+
r = self.client.post(url, dict(name="Test", acronym=group.parent.acronym))
133+
self.assertEquals(r.status_code, 200)
134+
q = PyQuery(r.content)
135+
self.assertTrue(len(q('form ul.errorlist')) > 0)
136+
self.assertEquals(len(q('form input[name="confirmed"]')), 0) # can't confirm us out of this
137+
138+
# try elevating BoF to WG
139+
group.state_id = "bof"
140+
group.save()
141+
142+
r = self.client.post(url, dict(name="Test", acronym=group.acronym))
143+
self.assertEquals(r.status_code, 200)
144+
q = PyQuery(r.content)
145+
self.assertTrue(len(q('form ul.errorlist')) > 0)
146+
self.assertEquals(len(q('form input[name="confirmed"]')), 1)
147+
148+
self.assertEquals(Group.objects.get(acronym=group.acronym).state_id, "bof")
149+
150+
# confirm elevation
151+
r = self.client.post(url, dict(name="Test", acronym=group.acronym, confirmed="1"))
152+
self.assertEquals(r.status_code, 302)
153+
self.assertEquals(Group.objects.get(acronym=group.acronym).state_id, "proposed")
154+
self.assertEquals(Group.objects.get(acronym=group.acronym).name, "Test")
123155

124156
def test_edit_info(self):
125157
make_test_data()

0 commit comments

Comments
 (0)