From 23ebe5d35ddf006e2a32cb211f183f8ff36f240a Mon Sep 17 00:00:00 2001 From: Henrik Levkowetz Date: Thu, 23 Feb 2017 20:55:38 +0000 Subject: [PATCH] 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 --- PLAN | 2 - ietf/group/tests_info.py | 38 +++++++++++++ ietf/group/urls_info_details.py | 1 + ietf/group/utils.py | 8 +-- ietf/group/views.py | 2 +- ietf/group/views_edit.py | 51 ++++++++++------- ietf/templates/group/edit.html | 4 ++ ietf/templates/group/group_about.html | 79 +++++++++++++++++++++++---- 8 files changed, 147 insertions(+), 38 deletions(-) diff --git a/PLAN b/PLAN index e40b04a50..0f7773783 100644 --- a/PLAN +++ b/PLAN @@ -10,8 +10,6 @@ Planned work in rough order * Add a 'rev' field to DocEvent, migrate 'rev' information from New 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 * Break out the htmlzation code used on tools in a library, and use that diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index 79e28a7ae..0321daf49 100644 --- a/ietf/group/tests_info.py +++ b/ietf/group/tests_info.py @@ -620,6 +620,44 @@ class GroupEditTests(TestCase): for prefix in ['ad1','ad2','aread','marschairman','marsdelegate']: 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): doc = make_test_data() review_req = make_review_data(doc) diff --git a/ietf/group/urls_info_details.py b/ietf/group/urls_info_details.py index 8bb4284b0..8ce9fd722 100644 --- a/ietf/group/urls_info_details.py +++ b/ietf/group/urls_info_details.py @@ -23,6 +23,7 @@ urlpatterns = [ url(r'^deps/(?P[\w-]+)/$', views.dependencies), url(r'^meetings/$', views.meetings), url(r'^edit/$', views_edit.edit, {'action': "edit"}), + url(r'^edit/(?P\w+)/?$', views_edit.edit, {'action': "edit"}), url(r'^conclude/$', views_edit.conclude), 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"), diff --git a/ietf/group/utils.py b/ietf/group/utils.py index 263ffd039..a5b53065f 100644 --- a/ietf/group/utils.py +++ b/ietf/group/utils.py @@ -169,12 +169,9 @@ def construct_group_menu_context(request, group, selected, group_type, others): # menu entries entries = [] + entries.append(("About", urlreverse("ietf.group.views.group_about", kwargs=kwargs))) if group.features.has_documents: 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(): entries.append(("Materials", urlreverse("ietf.group.views.materials", kwargs=kwargs))) 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) 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.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): + can_edit_group = True 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): @@ -237,6 +236,7 @@ def construct_group_menu_context(request, group, selected, group_type, others): "menu_entries": entries, "menu_actions": actions, "group_type": group_type, + "can_edit_group": can_edit_group, } d.update(others) diff --git a/ietf/group/views.py b/ietf/group/views.py index 4b4ec20d0..4e93a6c5a 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -435,7 +435,7 @@ def group_about(request, acronym, group_type=None): 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"), "milestone_reviewer": milestone_reviewer_for_group_type(group_type), "requested_close": requested_close, diff --git a/ietf/group/views_edit.py b/ietf/group/views_edit.py index e9372b881..d5a63c3bc 100644 --- a/ietf/group/views_edit.py +++ b/ietf/group/views_edit.py @@ -57,6 +57,13 @@ class GroupForm(forms.Form): def __init__(self, *args, **kwargs): self.group = kwargs.pop('group', None) 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) @@ -85,6 +92,10 @@ class GroupForm(forms.Form): - set(roles_for_group_type(self.group_type))) for r in role_fields_to_remove: del self.fields[r + "_roles"] + if field: + for f in self.fields: + if f != field: + del self.fields[f] def clean_acronym(self): # 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.") return cleaned_data + def format_urls(urls, fs="\n"): res = [] for u in urls: @@ -221,7 +233,7 @@ def format_urls(urls, fs="\n"): # return redirect('charter_submit', name=group.charter.name, option="initcharter") @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 necessary and logging changes as group events.""" 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") 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(): clean = form.cleaned_data 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) def diff(attr, name): + if field and attr != field: + return v = getattr(group, attr) if 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) # update urls - new_urls = clean['urls'] - old_urls = format_urls(group.groupurl_set.order_by('url'), ", ") - if ", ".join(sorted(new_urls)) != old_urls: - changes.append(('urls', new_urls, desc('Urls', ", ".join(sorted(new_urls)), old_urls))) - group.groupurl_set.all().delete() - # Add new ones - for u in new_urls: - m = re.search('(?P[\w\d:#@%/;$()~_?\+-=\\\.&]+)( \((?P.+)\))?', u) - if m: - if m.group('name'): - url = GroupURL(url=m.group('url'), name=m.group('name'), group=group) - else: - url = GroupURL(url=m.group('url'), name='', group=group) - url.save() + if 'urls' in clean: + new_urls = clean['urls'] + old_urls = format_urls(group.groupurl_set.order_by('url'), ", ") + if ", ".join(sorted(new_urls)) != old_urls: + changes.append(('urls', new_urls, desc('Urls', ", ".join(sorted(new_urls)), old_urls))) + group.groupurl_set.all().delete() + # Add new ones + for u in new_urls: + m = re.search('(?P[\w\d:#@%/;$()~_?\+-=\\\.&]+)( \((?P.+)\))?', u) + if m: + if m.group('name'): + url = GroupURL(url=m.group('url'), name=m.group('name'), group=group) + else: + url = GroupURL(url=m.group('url'), name='', group=group) + url.save() group.time = datetime.datetime.now() @@ -384,15 +399,13 @@ def edit(request, group_type=None, acronym=None, action="edit"): else: 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', dict(group=group, form=form, action=action)) - - class ConcludeForm(forms.Form): instructions = forms.CharField(widget=forms.Textarea(attrs={'rows': 30}), required=True, strip=False) diff --git a/ietf/templates/group/edit.html b/ietf/templates/group/edit.html index 9ab6986da..47acfbfd9 100644 --- a/ietf/templates/group/edit.html +++ b/ietf/templates/group/edit.html @@ -30,9 +30,13 @@ {% endif %} + {% if not request.user.is_authenticated %}

Note that persons with authorization to manage information, e.g. chairs and delegates, need a datatracker account to actually do so. New accounts can be created here.

+ {% else %} +

+ {% endif %}
{% csrf_token %} diff --git a/ietf/templates/group/group_about.html b/ietf/templates/group/group_about.html index f35d58a0f..c9e91548b 100644 --- a/ietf/templates/group/group_about.html +++ b/ietf/templates/group/group_about.html @@ -11,17 +11,23 @@ {% endif %} - + + + @@ -29,15 +35,23 @@ {% if group.parent and group.parent.type_id == "area" %} + {% else %} - + + + {% endif %} + + + {% endif %} @@ -86,6 +104,7 @@ + - + + {% endif %} + - + + + + + + + + + + + + + + + - - {% endif %} @@ -146,13 +199,15 @@ - - + + + +
{{ group.type.name }} Name + {% if can_edit_group %} + Edit + {% endif %} + {{ group.name }}
Acronym {{ group.acronym }}
{{ group.parent.type.name }} {{ group.parent.name }} ({{ group.parent.acronym }})
State + {% if can_edit_group %} + Edit + {% endif %} + {{ group.state.name }} {% if requested_close %} @@ -50,6 +64,7 @@
Charter {% if group.charter %} {{ group.charter.name }}-{{ group.charter.rev }} @@ -68,16 +83,19 @@
Status Update + {% if can_provide_status_update %} + Edit + {% endif %} + {% if status_update %} + Show update (last changed {{status_update.time|date:"Y-m-d"}}) {% else %} (None) {% endif %} - Show - {% if can_provide_status_update %} - Edit - {% endif %} +
Dependencies Document dependency graph (SVG) @@ -98,7 +117,12 @@ {% if urls %}
More infoAdditional URLs + {% if can_edit_group %} + Edit + {% endif %} + {% for url in urls %} {% firstof url.name url.url %}{% if not forloop.last %}
{% endif %} @@ -118,6 +142,11 @@ 		{{ label }} + {% if can_edit_group and not slug == "ad" %} + Edit + {% endif %} + @@ -135,10 +164,34 @@
Mailing listAddress{{ group.list_email|urlize }}Address + {% if can_edit_group %} + Edit + {% endif %} + {{ group.list_email|urlize }}
To subscribe + {% if can_edit_group %} + Edit + {% endif %} + {{ group.list_subscribe|urlize }}
Archive + {% if can_edit_group %} + Edit + {% endif %} + {{ group.list_archive|urlize }}
To subscribe{{ group.list_subscribe|urlize }}
Archive{{ group.list_archive|urlize }}
Jabber chatRoom addressxmpp:{{ group.acronym }}@jabber.ietf.org?joinRoom addressxmpp:{{ group.acronym }}@jabber.ietf.org?join
Logs https://jabber.ietf.org/logs/{{ group.acronym }}/