diff --git a/ietf/doc/tests_draft.py b/ietf/doc/tests_draft.py index 627ea98d7..d915995d9 100644 --- a/ietf/doc/tests_draft.py +++ b/ietf/doc/tests_draft.py @@ -1135,7 +1135,6 @@ class IndividualInfoFormsTests(TestCase): q = PyQuery(r.content) self.assertEqual(len(q('form textarea[id=id_resources]')),1) - # AMHERE badlines = ( 'github_repo https://github3.com/some/repo', 'github_notify badaddr', diff --git a/ietf/doc/views_draft.py b/ietf/doc/views_draft.py index 721aa58ca..79cca2057 100644 --- a/ietf/doc/views_draft.py +++ b/ietf/doc/views_draft.py @@ -1320,11 +1320,11 @@ def edit_doc_extresources(request, name): value = parts[1] display_name = ' '.join(parts[2:]).strip('()') doc.docextresource_set.create(value=value, name_id=name, display_name=display_name) - new_resources = format_resources(doc.docextresource_set.all()) - e = DocEvent(doc=doc, rev=doc.rev, by=request.user.person, type='changed_document') - e.desc = "Changed document external resources from:\n\n%s\n\nto:\n\n%s" % (old_resources, new_resources) - e.save() - doc.save_with_history([e]) + new_resources = format_resources(doc.docextresource_set.all()) + e = DocEvent(doc=doc, rev=doc.rev, by=request.user.person, type='changed_document') + e.desc = "Changed document external resources from:\n\n%s\n\nto:\n\n%s" % (old_resources, new_resources) + e.save() + doc.save_with_history([e]) messages.success(request,"Document resources updated.") else: messages.info(request,"No change in Document resources.") diff --git a/ietf/group/forms.py b/ietf/group/forms.py index 2e68461be..e83afee44 100644 --- a/ietf/group/forms.py +++ b/ietf/group/forms.py @@ -13,10 +13,11 @@ import debug # pyflakes:ignore # Django imports from django import forms from django.utils.html import mark_safe # type:ignore +from django.core.exceptions import ValidationError, ObjectDoesNotExist # IETF imports from ietf.group.models import Group, GroupHistory, GroupStateName -from ietf.name.models import ReviewTypeName +from ietf.name.models import ReviewTypeName, ExtResourceName from ietf.person.fields import SearchableEmailsField, PersonEmailChoiceField from ietf.person.models import Person from ietf.review.models import ReviewerSettings, UnavailablePeriod, ReviewSecretarySettings @@ -26,6 +27,7 @@ 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.fields import DatepickerDateField, MultiEmailField +from ietf.utils.validators import validate_external_resource_value # --- Constants -------------------------------------------------------- @@ -82,6 +84,7 @@ class GroupForm(forms.Form): list_subscribe = forms.CharField(max_length=255, required=False) list_archive = forms.CharField(max_length=255, required=False) urls = forms.CharField(widget=forms.Textarea, label="Additional URLs", help_text="Format: https://site/path (Optional description). Separate multiple entries with newline. Prefer HTTPS URLs where possible.", required=False) + resources = forms.CharField(widget=forms.Textarea, label="Additional Resources", help_text="UPDATEME: Format: https://site/path (Optional description). Separate multiple entries with newline. Prefer HTTPS URLs where possible.", required=False) closing_note = forms.CharField(widget=forms.Textarea, label="Closing note", required=False) def __init__(self, *args, **kwargs): @@ -127,6 +130,12 @@ class GroupForm(forms.Form): for f in keys: if f != field and not (f == 'closing_note' and field == 'state'): del self.fields[f] + if 'resources' in self.fields: + info = "Format: 'tag value (Optional description)'. " \ + + "Separate multiple entries with newline. When the value is a URL, use https:// where possible.
" \ + + "Valid tags: %s" % ', '.join([ o.slug for o in ExtResourceName.objects.all().order_by('slug') ]) + self.fields['resources'].help_text = mark_safe('
'+info+'
') + def clean_acronym(self): # Changing the acronym of an already existing group will cause 404s all @@ -186,6 +195,30 @@ class GroupForm(forms.Form): def clean_urls(self): return [x.strip() for x in self.cleaned_data["urls"].splitlines() if x.strip()] + def clean_resources(self): + lines = [x.strip() for x in self.cleaned_data["resources"].splitlines() if x.strip()] + errors = [] + for l in lines: + parts = l.split() + if len(parts) == 1: + errors.append("Too few fields: Expected at least tag and value: '%s'" % l) + elif len(parts) >= 2: + name_slug = parts[0] + try: + name = ExtResourceName.objects.get(slug=name_slug) + except ObjectDoesNotExist: + errors.append("Bad tag in '%s': Expected one of %s" % (l, ', '.join([ o.slug for o in ExtResourceName.objects.all() ]))) + continue + value = parts[1] + try: + validate_external_resource_value(name, value) + except ValidationError as e: + e.message += " : " + value + errors.append(e) + if errors: + raise ValidationError(errors) + return lines + def clean_delegates(self): if len(self.cleaned_data["delegates"]) > MAX_GROUP_DELEGATES: raise forms.ValidationError("At most %s delegates can be appointed at the same time, please remove %s delegates." % ( diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index 950f099a2..5a49006c4 100644 --- a/ietf/group/tests_info.py +++ b/ietf/group/tests_info.py @@ -634,6 +634,46 @@ class GroupEditTests(TestCase): self.assertTrue(prefix+'@' in outbox[0]['To']) self.assertTrue(outbox[0].get_payload(decode=True).decode(str(outbox[0].get_charset())).startswith('Sec Retary')) + def test_edit_extresources(self): + group = GroupFactory(acronym='mars',parent=GroupFactory(type_id='area')) + CharterFactory(group=group) + + url = urlreverse('ietf.group.views.edit', kwargs=dict(group_type=group.type_id, acronym=group.acronym, action="edit", field="resources")) + login_testing_unauthorized(self, "secretary", url) + + r = self.client.get(url) + self.assertEqual(r.status_code,200) + q = PyQuery(r.content) + self.assertEqual(len(q('form textarea[id=id_resources]')),1) + + badlines = ( + 'github_repo https://github3.com/some/repo', + 'github_notify badaddr', + 'website /not/a/good/url' + 'notavalidtag blahblahblah' + ) + + for line in badlines: + r = self.client.post(url, dict(resources=line, submit="1")) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + self.assertTrue(q('.alert-danger')) + + goodlines = """ + github_repo https://github.com/some/repo Some display text + github_notify notify@example.com + github_username githubuser + website http://example.com/http/is/fine + """ + + r = self.client.post(url, dict(resources=goodlines, submit="1")) + self.assertEqual(r.status_code,302) + group = Group.objects.get(acronym=group.acronym) + self.assertEqual(group.latest_event(GroupEvent,type="info_changed").desc[:20], 'Resources changed to') + self.assertIn('github_username githubuser', group.latest_event(GroupEvent,type="info_changed").desc) + self.assertEqual(group.groupextresource_set.count(), 4) + self.assertEqual(group.groupextresource_set.get(name__slug='github_repo').display_name, 'Some display text') + def test_edit_field(self): group = GroupFactory(acronym="mars") diff --git a/ietf/group/views.py b/ietf/group/views.py index 8f32bd34b..204e500fb 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -880,6 +880,17 @@ def edit(request, group_type=None, acronym=None, action="edit", field=None): res.append(u.url) return fs.join(res) + def format_resources(resources, fs="\n"): + res = [] + for r in resources: + if r.display_name: + res.append("%s %s (%s)" % (r.name.slug, r.value, r.display_name.strip('()'))) + else: + res.append("%s %s" % (r.name.slug, r.value)) + # TODO: This is likely problematic if value has spaces. How then to delineate value and display_name? Perhaps in the short term move to comma or pipe separation. + # Might be better to shift to a formset instead of parsing these lines. + return fs.join(res) + def diff(attr, name): if field and attr != field: return @@ -933,11 +944,6 @@ def edit(request, group_type=None, acronym=None, action="edit", field=None): else: save_group_in_history(group) - -## XXX Remove after testing -# if action == "charter" and not group.charter: # make sure we have a charter -# group.charter = get_or_create_initial_charter(group, group_type) - changes = [] # update the attributes, keeping track of what we're doing @@ -1023,6 +1029,19 @@ def edit(request, group_type=None, acronym=None, action="edit", field=None): url = GroupURL(url=m.group('url'), name='', group=group) url.save() + if 'resources' in clean: + old_resources = sorted(format_resources(group.groupextresource_set.all()).splitlines()) + new_resources = sorted(clean['resources']) + if old_resources != new_resources: + group.groupextresource_set.all().delete() + for u in new_resources: + parts = u.split(None, 2) + name = parts[0] + value = parts[1] + display_name = ' '.join(parts[2:]).strip('()') + group.groupextresource_set.create(value=value, name_id=name, display_name=display_name) + changes.append(('resources', new_resources, desc('Resources', ", ".join(new_resources), ", ".join(old_resources)))) + group.time = datetime.datetime.now() if changes and not new_group: @@ -1075,6 +1094,7 @@ def edit(request, group_type=None, acronym=None, action="edit", field=None): list_subscribe=group.list_subscribe if group.list_subscribe else None, list_archive=group.list_archive if group.list_archive else None, urls=format_urls(group.groupurl_set.all()), + resources=format_resources(group.groupextresource_set.all()), closing_note = closing_note, ) diff --git a/ietf/templates/group/group_about.html b/ietf/templates/group/group_about.html index a71051892..ff5c51bf7 100644 --- a/ietf/templates/group/group_about.html +++ b/ietf/templates/group/group_about.html @@ -138,6 +138,36 @@ {% endif %} {% endwith %} + + {% with group.groupextresource_set.all as resources %} + {% if resources or can_edit_group %} + + + Additional Resources + + {% if can_edit_group %} + Edit + {% endif %} + + + {% if resources %} + + + {% for resource in resources|dictsort:"display_name" %} + {% if resource.name.type.slug == 'url' or resource.name.type.slug == 'email' %} + + {# Maybe make how a resource displays itself a method on the class so templates aren't doing this switching #} + {% else %} + + {% endif %} + {% endfor %} + +
- {% firstof resource.display_name resource.name.name %}
- {% firstof resource.display_name resource.name.name %}: {{resource.value}}
+ {% endif %} + + + {% endif %} + {% endwith %}