This addresses ease of editing various group attributes, and in particular is

intended to make it easier to see that you can edit things like the
external/additional URLs:

 - Added the ability to edit individual fields in a group's about page, and
   added edit buttons for editable fields on the about page, just as for
   documents (the ability to edit all editable fields already was available
   from the 'Edit group' button on the /group//about/ page).

 - Made the tab label for the group-about tab consistently say 'About',
   instead of 'Charter' for some groups.

 = Shifted the position of the about tab to the start of the tab line.

 - Removed the datatracker account requirement information at the top of the
   group edit page for users logged in to their account.

 - Tweaked the 'Show update' link on the 'Status Update' line.

 - Changed the label for the external URLs from 'More Info' to 'Additional
   URLs', which was already in use on the edit form.
 - Legacy-Id: 12904
This commit is contained in:
Henrik Levkowetz 2017-02-23 20:55:38 +00:00
parent ffa19c9847
commit 23ebe5d35d
8 changed files with 147 additions and 38 deletions

2
PLAN
View file

@ -10,8 +10,6 @@ Planned work in rough order
* Add a 'rev' field to DocEvent, migrate 'rev' information from New * Add a 'rev' field to DocEvent, migrate 'rev' information from New
Revision and Submit DocEvents, and infer 'rev' information for others Revision and Submit DocEvents, and infer 'rev' information for others
* Schema Changes: tags, external links for groups.
* Add "Waiting for implementation" and "Held by WG" states, with comment field * Add "Waiting for implementation" and "Held by WG" states, with comment field
* Break out the htmlzation code used on tools in a library, and use that * Break out the htmlzation code used on tools in a library, and use that

View file

@ -620,6 +620,44 @@ class GroupEditTests(TestCase):
for prefix in ['ad1','ad2','aread','marschairman','marsdelegate']: for prefix in ['ad1','ad2','aread','marschairman','marsdelegate']:
self.assertTrue(prefix+'@' in outbox[0]['To']) self.assertTrue(prefix+'@' in outbox[0]['To'])
def test_edit_field(self):
make_test_data()
group = Group.objects.get(acronym="mars")
# Edit name
url = urlreverse('ietf.group.views_edit.edit', kwargs=dict(acronym=group.acronym, action="edit", field="name"))
login_testing_unauthorized(self, "secretary", url)
# normal get
r = self.client.get(url)
self.assertEqual(r.status_code, 200)
q = PyQuery(r.content)
self.assertEqual(len(q('div#content > form input[name=name]')), 1)
self.assertEqual(len(q('form input[name=acronym]')), 0)
# edit info
r = self.client.post(url, dict(name="Mars Not Special Interest Group"))
self.assertEqual(r.status_code, 302)
#
group = Group.objects.get(acronym="mars")
self.assertEqual(group.name, "Mars Not Special Interest Group")
# Edit list email
url = urlreverse('ietf.group.views_edit.edit', kwargs=dict(group_type=group.type_id, acronym=group.acronym, action="edit", field="list_email"))
# normal get
r = self.client.get(url)
self.assertEqual(r.status_code, 200)
q = PyQuery(r.content)
self.assertEqual(len(q('div#content > form input[name=list_email]')), 1)
self.assertEqual(len(q('div#content > form input[name=name]')), 0)
# edit info
r = self.client.post(url, dict(list_email="mars@mail"))
self.assertEqual(r.status_code, 302)
#
group = Group.objects.get(acronym="mars")
self.assertEqual(group.list_email, "mars@mail")
def test_edit_reviewers(self): def test_edit_reviewers(self):
doc = make_test_data() doc = make_test_data()
review_req = make_review_data(doc) review_req = make_review_data(doc)

View file

@ -23,6 +23,7 @@ urlpatterns = [
url(r'^deps/(?P<output_type>[\w-]+)/$', views.dependencies), url(r'^deps/(?P<output_type>[\w-]+)/$', views.dependencies),
url(r'^meetings/$', views.meetings), url(r'^meetings/$', views.meetings),
url(r'^edit/$', views_edit.edit, {'action': "edit"}), url(r'^edit/$', views_edit.edit, {'action': "edit"}),
url(r'^edit/(?P<field>\w+)/?$', views_edit.edit, {'action': "edit"}),
url(r'^conclude/$', views_edit.conclude), url(r'^conclude/$', views_edit.conclude),
url(r'^milestones/$', milestone_views.edit_milestones, {'milestone_set': "current"}, "group_edit_milestones"), url(r'^milestones/$', milestone_views.edit_milestones, {'milestone_set': "current"}, "group_edit_milestones"),
url(r'^milestones/charter/$', milestone_views.edit_milestones, {'milestone_set': "charter"}, "group_edit_charter_milestones"), url(r'^milestones/charter/$', milestone_views.edit_milestones, {'milestone_set': "charter"}, "group_edit_charter_milestones"),

View file

@ -169,12 +169,9 @@ def construct_group_menu_context(request, group, selected, group_type, others):
# menu entries # menu entries
entries = [] entries = []
entries.append(("About", urlreverse("ietf.group.views.group_about", kwargs=kwargs)))
if group.features.has_documents: if group.features.has_documents:
entries.append(("Documents", urlreverse("ietf.group.views.group_documents", kwargs=kwargs))) entries.append(("Documents", urlreverse("ietf.group.views.group_documents", kwargs=kwargs)))
if group.features.has_chartering_process:
entries.append(("Charter", urlreverse("ietf.group.views.group_about", kwargs=kwargs)))
else:
entries.append(("About", urlreverse("ietf.group.views.group_about", kwargs=kwargs)))
if group.features.has_materials and get_group_materials(group).exists(): if group.features.has_materials and get_group_materials(group).exists():
entries.append(("Materials", urlreverse("ietf.group.views.materials", kwargs=kwargs))) entries.append(("Materials", urlreverse("ietf.group.views.materials", kwargs=kwargs)))
if group.features.has_reviews: if group.features.has_reviews:
@ -199,6 +196,7 @@ def construct_group_menu_context(request, group, selected, group_type, others):
is_admin = group.has_role(request.user, group.features.admin_roles) is_admin = group.has_role(request.user, group.features.admin_roles)
can_manage = can_manage_group_type(request.user, group) can_manage = can_manage_group_type(request.user, group)
can_edit_group = False # we'll set this further down
if group.features.has_milestones: if group.features.has_milestones:
if group.state_id != "proposed" and (is_admin or can_manage): if group.state_id != "proposed" and (is_admin or can_manage):
@ -223,6 +221,7 @@ def construct_group_menu_context(request, group, selected, group_type, others):
if group.state_id != "conclude" and (is_admin or can_manage): if group.state_id != "conclude" and (is_admin or can_manage):
can_edit_group = True
actions.append((u"Edit group", urlreverse("ietf.group.views_edit.edit", kwargs=dict(kwargs, action="edit")))) actions.append((u"Edit group", urlreverse("ietf.group.views_edit.edit", kwargs=dict(kwargs, action="edit"))))
if group.features.customize_workflow and (is_admin or can_manage): if group.features.customize_workflow and (is_admin or can_manage):
@ -237,6 +236,7 @@ def construct_group_menu_context(request, group, selected, group_type, others):
"menu_entries": entries, "menu_entries": entries,
"menu_actions": actions, "menu_actions": actions,
"group_type": group_type, "group_type": group_type,
"can_edit_group": can_edit_group,
} }
d.update(others) d.update(others)

View file

@ -435,7 +435,7 @@ def group_about(request, acronym, group_type=None):
return render(request, 'group/group_about.html', return render(request, 'group/group_about.html',
construct_group_menu_context(request, group, "charter" if group.features.has_chartering_process else "about", group_type, { construct_group_menu_context(request, group, "about", group_type, {
"milestones_in_review": group.groupmilestone_set.filter(state="review"), "milestones_in_review": group.groupmilestone_set.filter(state="review"),
"milestone_reviewer": milestone_reviewer_for_group_type(group_type), "milestone_reviewer": milestone_reviewer_for_group_type(group_type),
"requested_close": requested_close, "requested_close": requested_close,

View file

@ -57,6 +57,13 @@ class GroupForm(forms.Form):
def __init__(self, *args, **kwargs): def __init__(self, *args, **kwargs):
self.group = kwargs.pop('group', None) self.group = kwargs.pop('group', None)
self.group_type = kwargs.pop('group_type', False) self.group_type = kwargs.pop('group_type', False)
if "field" in kwargs:
field = kwargs["field"]
del kwargs["field"]
if field in roles_for_group_type(self.group_type):
field = field + "_roles"
else:
field = None
super(self.__class__, self).__init__(*args, **kwargs) super(self.__class__, self).__init__(*args, **kwargs)
@ -85,6 +92,10 @@ class GroupForm(forms.Form):
- set(roles_for_group_type(self.group_type))) - set(roles_for_group_type(self.group_type)))
for r in role_fields_to_remove: for r in role_fields_to_remove:
del self.fields[r + "_roles"] del self.fields[r + "_roles"]
if field:
for f in self.fields:
if f != field:
del self.fields[f]
def clean_acronym(self): def clean_acronym(self):
# Changing the acronym of an already existing group will cause 404s all # Changing the acronym of an already existing group will cause 404s all
@ -158,6 +169,7 @@ class GroupForm(forms.Form):
raise forms.ValidationError("You requested the creation of a BoF, but specified no parent area. A parent is required when creating a bof.") raise forms.ValidationError("You requested the creation of a BoF, but specified no parent area. A parent is required when creating a bof.")
return cleaned_data return cleaned_data
def format_urls(urls, fs="\n"): def format_urls(urls, fs="\n"):
res = [] res = []
for u in urls: for u in urls:
@ -221,7 +233,7 @@ def format_urls(urls, fs="\n"):
# return redirect('charter_submit', name=group.charter.name, option="initcharter") # return redirect('charter_submit', name=group.charter.name, option="initcharter")
@login_required @login_required
def edit(request, group_type=None, acronym=None, action="edit"): def edit(request, group_type=None, acronym=None, action="edit", field=None):
"""Edit or create a group, notifying parties as """Edit or create a group, notifying parties as
necessary and logging changes as group events.""" necessary and logging changes as group events."""
if action == "edit": if action == "edit":
@ -241,7 +253,7 @@ def edit(request, group_type=None, acronym=None, action="edit"):
return HttpResponseForbidden("You don't have permission to access this view") return HttpResponseForbidden("You don't have permission to access this view")
if request.method == 'POST': if request.method == 'POST':
form = GroupForm(request.POST, group=group, group_type=group_type) form = GroupForm(request.POST, group=group, group_type=group_type, field=field)
if form.is_valid(): if form.is_valid():
clean = form.cleaned_data clean = form.cleaned_data
if new_group: if new_group:
@ -284,6 +296,8 @@ def edit(request, group_type=None, acronym=None, action="edit"):
return entry % dict(attr=attr, new=new, old=old) return entry % dict(attr=attr, new=new, old=old)
def diff(attr, name): def diff(attr, name):
if field and attr != field:
return
v = getattr(group, attr) v = getattr(group, attr)
if clean[attr] != v: if clean[attr] != v:
changes.append((attr, clean[attr], desc(name, clean[attr], v))) changes.append((attr, clean[attr], desc(name, clean[attr], v)))
@ -335,20 +349,21 @@ def edit(request, group_type=None, acronym=None, action="edit"):
email_personnel_change(request, group, personnel_change_text, changed_personnel) email_personnel_change(request, group, personnel_change_text, changed_personnel)
# update urls # update urls
new_urls = clean['urls'] if 'urls' in clean:
old_urls = format_urls(group.groupurl_set.order_by('url'), ", ") new_urls = clean['urls']
if ", ".join(sorted(new_urls)) != old_urls: old_urls = format_urls(group.groupurl_set.order_by('url'), ", ")
changes.append(('urls', new_urls, desc('Urls', ", ".join(sorted(new_urls)), old_urls))) if ", ".join(sorted(new_urls)) != old_urls:
group.groupurl_set.all().delete() changes.append(('urls', new_urls, desc('Urls', ", ".join(sorted(new_urls)), old_urls)))
# Add new ones group.groupurl_set.all().delete()
for u in new_urls: # Add new ones
m = re.search('(?P<url>[\w\d:#@%/;$()~_?\+-=\\\.&]+)( \((?P<name>.+)\))?', u) for u in new_urls:
if m: m = re.search('(?P<url>[\w\d:#@%/;$()~_?\+-=\\\.&]+)( \((?P<name>.+)\))?', u)
if m.group('name'): if m:
url = GroupURL(url=m.group('url'), name=m.group('name'), group=group) if m.group('name'):
else: url = GroupURL(url=m.group('url'), name=m.group('name'), group=group)
url = GroupURL(url=m.group('url'), name='', group=group) else:
url.save() url = GroupURL(url=m.group('url'), name='', group=group)
url.save()
group.time = datetime.datetime.now() group.time = datetime.datetime.now()
@ -384,15 +399,13 @@ def edit(request, group_type=None, acronym=None, action="edit"):
else: else:
init = dict(ad=request.user.person.id if group_type == "wg" and has_role(request.user, "Area Director") else None, init = dict(ad=request.user.person.id if group_type == "wg" and has_role(request.user, "Area Director") else None,
) )
form = GroupForm(initial=init, group=group, group_type=group_type) form = GroupForm(initial=init, group=group, group_type=group_type, field=field)
return render(request, 'group/edit.html', return render(request, 'group/edit.html',
dict(group=group, dict(group=group,
form=form, form=form,
action=action)) action=action))
class ConcludeForm(forms.Form): class ConcludeForm(forms.Form):
instructions = forms.CharField(widget=forms.Textarea(attrs={'rows': 30}), required=True, strip=False) instructions = forms.CharField(widget=forms.Textarea(attrs={'rows': 30}), required=True, strip=False)

View file

@ -30,9 +30,13 @@
{% endif %} {% endif %}
</h1> </h1>
{% if not request.user.is_authenticated %}
<p class="alert alert-info">Note that persons with authorization to manage information, e.g. <p class="alert alert-info">Note that persons with authorization to manage information, e.g.
chairs and delegates, need a datatracker account to actually do chairs and delegates, need a datatracker account to actually do
so. New accounts can be <a href="{% url "ietf.ietfauth.views.create_account" %}">created here</a>.</p> so. New accounts can be <a href="{% url "ietf.ietfauth.views.create_account" %}">created here</a>.</p>
{% else %}
<p></p>
{% endif %}
<form class="form-horizontal" method="post"> <form class="form-horizontal" method="post">
{% csrf_token %} {% csrf_token %}

View file

@ -11,17 +11,23 @@
{% endif %} {% endif %}
<table class="table table-condensed"> <table class="table table-condensed">
<thead><tr><th colspan="3"></th></tr></thead> <thead><tr><th colspan="4"></th></tr></thead>
<tbody class="meta"> <tbody class="meta">
<tr> <tr>
<th>{{ group.type.name }}</th> <th>{{ group.type.name }}</th>
<th>Name</th> <th>Name</th>
<td class="edit">
{% if can_edit_group %}
<a class="btn btn-default btn-xs" href="{% url 'ietf.group.views_edit.edit' acronym=group.acronym field='name' %}">Edit</a>
{% endif %}
</td>
<td>{{ group.name }}</td> <td>{{ group.name }}</td>
</tr> </tr>
<tr> <tr>
<td></td> <td></td>
<th>Acronym</th> <th>Acronym</th>
<td class="edit"></td>
<td>{{ group.acronym }}</td> <td>{{ group.acronym }}</td>
</tr> </tr>
@ -29,15 +35,23 @@
<td></td> <td></td>
{% if group.parent and group.parent.type_id == "area" %} {% if group.parent and group.parent.type_id == "area" %}
<th>{{ group.parent.type.name }}</th> <th>{{ group.parent.type.name }}</th>
<td class="edit"></td>
<td>{{ group.parent.name }} ({{ group.parent.acronym }})</td> <td>{{ group.parent.name }} ({{ group.parent.acronym }})</td>
{% else %} {% else %}
<th></th><td></td> <th></th>
<td class="edit"></td>
<td></td>
{% endif %} {% endif %}
</tr> </tr>
<tr> <tr>
<td></td> <td></td>
<th>State</th> <th>State</th>
<td class="edit">
{% if can_edit_group %}
<a class="btn btn-default btn-xs" href="{% url 'ietf.group.views_edit.edit' acronym=group.acronym field='state' %}">Edit</a>
{% endif %}
</td>
<td> <td>
{{ group.state.name }} {{ group.state.name }}
{% if requested_close %} {% if requested_close %}
@ -50,6 +64,7 @@
<tr> <tr>
<td></td> <td></td>
<th>Charter</th> <th>Charter</th>
<td class="edit"></td>
<td> <td>
{% if group.charter %} {% if group.charter %}
<a href="{% url "ietf.doc.views_doc.document_main" name=group.charter.name %}">{{ group.charter.name }}-{{ group.charter.rev }}</a> <a href="{% url "ietf.doc.views_doc.document_main" name=group.charter.name %}">{{ group.charter.name }}-{{ group.charter.rev }}</a>
@ -68,16 +83,19 @@
<tr id='status_update'> <tr id='status_update'>
<td></td> <td></td>
<th>Status Update</th> <th>Status Update</th>
<td class="edit">
{% if can_provide_status_update %}
<a class="btn btn-default btn-xs" href="{% url "ietf.group.views.group_about_status_edit" acronym=group.acronym %}">Edit</a>
{% endif %}
</td>
<td> <td>
{% if status_update %} {% if status_update %}
<a href="{% url "ietf.group.views.group_about_status" acronym=group.acronym %}">Show update</a>
(last changed {{status_update.time|date:"Y-m-d"}}) (last changed {{status_update.time|date:"Y-m-d"}})
{% else %} {% else %}
(None) (None)
{% endif %} {% endif %}
<a class="btn btn-default btn-xs" href="{% url "ietf.group.views.group_about_status" acronym=group.acronym %}">Show</a>
{% if can_provide_status_update %}
<a class="btn btn-default btn-xs" href="{% url "ietf.group.views.group_about_status_edit" acronym=group.acronym %}">Edit</a>
{% endif %}
</td> </td>
</tr> </tr>
{% endif %} {% endif %}
@ -86,6 +104,7 @@
<tr id='dependency_graph'> <tr id='dependency_graph'>
<td></td> <td></td>
<th>Dependencies</th> <th>Dependencies</th>
<td class="edit"></td>
<td> <td>
<a href="{% url 'ietf.group.views.dependencies' group_type=group.type_id acronym=group.acronym output_type='svg' %}"> <a href="{% url 'ietf.group.views.dependencies' group_type=group.type_id acronym=group.acronym output_type='svg' %}">
Document dependency graph (SVG) Document dependency graph (SVG)
@ -98,7 +117,12 @@
{% if urls %} {% if urls %}
<tr> <tr>
<td></td> <td></td>
<th>More info</th> <th>Additional URLs</th>
<td class="edit">
{% if can_edit_group %}
<a class="btn btn-default btn-xs" href="{% url 'ietf.group.views_edit.edit' acronym=group.acronym field='urls' %}">Edit</a>
{% endif %}
</td>
<td> <td>
{% for url in urls %} {% for url in urls %}
<a href="{{ url.url }}">{% firstof url.name url.url %}</a>{% if not forloop.last %}<br>{% endif %} <a href="{{ url.url }}">{% firstof url.name url.url %}</a>{% if not forloop.last %}<br>{% endif %}
@ -118,6 +142,11 @@
<td></td> <td></td>
{% endif %} {% endif %}
<th>{{ label }}</th> <th>{{ label }}</th>
<td class="edit">
{% if can_edit_group and not slug == "ad" %}
<a class="btn btn-default btn-xs" href="{% url 'ietf.group.views_edit.edit' acronym=group.acronym field=slug %}">Edit</a>
{% endif %}
</td>
<td> <td>
@ -135,10 +164,34 @@
<tbody class="meta"> <tbody class="meta">
<tr> <tr>
<th>Mailing list</th> <th>Mailing list</th>
<th>Address</th><td>{{ group.list_email|urlize }}</td> <th>Address</th>
<td class="edit">
{% if can_edit_group %}
<a class="btn btn-default btn-xs" href="{% url 'ietf.group.views_edit.edit' acronym=group.acronym field='list_email' %}">Edit</a>
{% endif %}
</td>
<td>{{ group.list_email|urlize }}</td>
</tr>
<tr>
<td></td>
<th>To subscribe</th>
<td class="edit">
{% if can_edit_group %}
<a class="btn btn-default btn-xs" href="{% url 'ietf.group.views_edit.edit' acronym=group.acronym field='list_subscribe' %}">Edit</a>
{% endif %}
</td>
<td>{{ group.list_subscribe|urlize }}</td>
</tr>
<tr>
<td></td>
<th>Archive</th>
<td class="edit">
{% if can_edit_group %}
<a class="btn btn-default btn-xs" href="{% url 'ietf.group.views_edit.edit' acronym=group.acronym field='list_archive' %}">Edit</a>
{% endif %}
</td>
<td>{{ group.list_archive|urlize }}</td>
</tr> </tr>
<tr><td></td><th>To subscribe</th><td>{{ group.list_subscribe|urlize }}</td></tr>
<tr><td></td><th>Archive</th><td>{{ group.list_archive|urlize }}</td></tr>
</tbody> </tbody>
{% endif %} {% endif %}
@ -146,13 +199,15 @@
<tbody class="meta"> <tbody class="meta">
<tr> <tr>
<th>Jabber chat</th> <th>Jabber chat</th>
<th>Room address</th> <th>Room address</th>
<td><a href="xmpp:{{ group.acronym }}@jabber.ietf.org?join">xmpp:{{ group.acronym }}@jabber.ietf.org?join</a></td> <td class="edit"></td>
<td><a href="xmpp:{{ group.acronym }}@jabber.ietf.org?join">xmpp:{{ group.acronym }}@jabber.ietf.org?join</a></td>
</tr> </tr>
<tr> <tr>
<td></td> <td></td>
<th>Logs</th> <th>Logs</th>
<td class="edit"></td>
<td><a href="https://jabber.ietf.org/logs/{{ group.acronym }}/">https://jabber.ietf.org/logs/{{ group.acronym }}/</a></td> <td><a href="https://jabber.ietf.org/logs/{{ group.acronym }}/">https://jabber.ietf.org/logs/{{ group.acronym }}/</a></td>
</tr> </tr>
</tbody> </tbody>