Added validation to the migrations. Added external resources to the draft display page. Built an editor for DocExternalResources.
- Legacy-Id: 17683
This commit is contained in:
parent
672f9bce26
commit
2de9eb93b5
|
@ -10,7 +10,7 @@ import ietf.utils.models
|
|||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('name', '0011_populate_extres'),
|
||||
('name', '0010_extres'),
|
||||
('doc', '0031_set_state_for_charters_of_replaced_groups'),
|
||||
]
|
||||
|
||||
|
|
|
@ -7,10 +7,14 @@ import re
|
|||
|
||||
import debug
|
||||
|
||||
from collections import OrderedDict
|
||||
from collections import OrderedDict, Counter
|
||||
|
||||
from django.db import migrations
|
||||
|
||||
from ietf.utils.validators import validate_external_resource_value
|
||||
from django.core.exceptions import ValidationError
|
||||
|
||||
|
||||
name_map = {
|
||||
"Issue.*": "tracker",
|
||||
".*FAQ.*": "faq",
|
||||
|
@ -55,16 +59,14 @@ def forward(apps, schema_editor):
|
|||
ExtResourceName = apps.get_model('name', 'ExtResourceName')
|
||||
DocumentUrl = apps.get_model('doc', 'DocumentUrl')
|
||||
|
||||
mapped = 0
|
||||
not_mapped = 0
|
||||
ignored = 0
|
||||
stats = Counter()
|
||||
|
||||
for doc_url in DocumentUrl.objects.all():
|
||||
match_found = False
|
||||
for regext,slug in name_map.items():
|
||||
if re.match(regext, doc_url.desc):
|
||||
match_found = True
|
||||
mapped += 1
|
||||
stats['mapped'] += 1
|
||||
name = ExtResourceName.objects.get(slug=slug)
|
||||
DocExtResource.objects.create(doc=doc_url.doc, name_id=slug, value=doc_url.url, display_name=doc_url.desc) # TODO: validate this value against name.type
|
||||
break
|
||||
|
@ -74,7 +76,7 @@ def forward(apps, schema_editor):
|
|||
if re.search(regext, doc_url.url):
|
||||
match_found = True
|
||||
if slug:
|
||||
mapped +=1
|
||||
stats['mapped'] +=1
|
||||
name = ExtResourceName.objects.get(slug=slug)
|
||||
# Munge the URL if it's the first github repo match
|
||||
# Remove "/tree/master" substring if it exists
|
||||
|
@ -84,14 +86,19 @@ def forward(apps, schema_editor):
|
|||
doc_url.url = doc_url.url.replace("/tree/master","")
|
||||
doc_url.url = re.sub('/issues$', '', doc_url.url)
|
||||
doc_url.url = re.sub('/blob/master.*$', '', doc_url.url)
|
||||
DocExtResource.objects.create(doc=doc_url.doc, name_id=slug, value=doc_url.url, display_name=doc_url.desc) # TODO: validate this value against name.type
|
||||
try:
|
||||
validate_external_resource_value(name, doc_url.url)
|
||||
DocExtResource.objects.create(doc=doc_url.doc, name=name, value=doc_url.url, display_name=doc_url.desc) # TODO: validate this value against name.type
|
||||
except ValidationError as e: # pyflakes:ignore
|
||||
debug.show('("Failed validation:", doc_url.url, e)')
|
||||
stats['failed_validation'] +=1
|
||||
else:
|
||||
ignored +=1
|
||||
stats['ignored'] +=1
|
||||
break
|
||||
if not match_found:
|
||||
debug.show('("Not Mapped:",doc_url.desc, doc_url.tag.slug, doc_url.doc.name, doc_url.url)')
|
||||
not_mapped += 1
|
||||
debug.show('(mapped, ignored, not_mapped)')
|
||||
stats['not_mapped'] += 1
|
||||
print (stats)
|
||||
|
||||
def reverse(apps, schema_editor):
|
||||
DocExtResource = apps.get_model('doc', 'DocExtResource')
|
||||
|
|
|
@ -1125,6 +1125,46 @@ class IndividualInfoFormsTests(TestCase):
|
|||
self.assertIn('wiki https://wiki.org/', doc.latest_event(DocEvent,type="changed_document").desc)
|
||||
self.assertIn('https://wiki.org/', [ u.url for u in doc.documenturl_set.all() ])
|
||||
|
||||
def test_edit_doc_extresources(self):
|
||||
url = urlreverse('ietf.doc.views_draft.edit_doc_extresources', kwargs=dict(name=self.docname))
|
||||
|
||||
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)
|
||||
|
||||
# AMHERE
|
||||
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)
|
||||
doc = Document.objects.get(name=self.docname)
|
||||
self.assertEqual(doc.latest_event(DocEvent,type="changed_document").desc[:35], 'Changed document external resources')
|
||||
self.assertIn('github_username githubuser', doc.latest_event(DocEvent,type="changed_document").desc)
|
||||
self.assertEqual(doc.docextresource_set.count(), 4)
|
||||
self.assertEqual(doc.docextresource_set.get(name__slug='github_repo').display_name, 'Some display text')
|
||||
|
||||
|
||||
class SubmitToIesgTests(TestCase):
|
||||
|
||||
def setUp(self):
|
||||
|
|
|
@ -131,6 +131,7 @@ urlpatterns = [
|
|||
url(r'^%(name)s/edit/approvedownrefs/$' % settings.URL_REGEXPS, views_ballot.approve_downrefs),
|
||||
url(r'^%(name)s/edit/makelastcall/$' % settings.URL_REGEXPS, views_ballot.make_last_call),
|
||||
url(r'^%(name)s/edit/urls/$' % settings.URL_REGEXPS, views_draft.edit_document_urls),
|
||||
url(r'^%(name)s/edit/resources/$' % settings.URL_REGEXPS, views_draft.edit_doc_extresources),
|
||||
url(r'^%(name)s/edit/issueballot/irsg/$' % settings.URL_REGEXPS, views_ballot.issue_irsg_ballot),
|
||||
url(r'^%(name)s/edit/closeballot/irsg/$' % settings.URL_REGEXPS, views_ballot.close_irsg_ballot),
|
||||
|
||||
|
|
|
@ -45,11 +45,12 @@ from ietf.iesg.models import TelechatDate
|
|||
from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream, user_is_person, is_individual_draft_author
|
||||
from ietf.ietfauth.utils import role_required
|
||||
from ietf.message.models import Message
|
||||
from ietf.name.models import IntendedStdLevelName, DocTagName, StreamName, DocUrlTagName
|
||||
from ietf.name.models import IntendedStdLevelName, DocTagName, StreamName, DocUrlTagName, ExtResourceName
|
||||
from ietf.person.fields import SearchableEmailField
|
||||
from ietf.person.models import Person, Email
|
||||
from ietf.utils.mail import send_mail, send_mail_message, on_behalf_of
|
||||
from ietf.utils.textupload import get_cleaned_text_file_content
|
||||
from ietf.utils.validators import validate_external_resource_value
|
||||
from ietf.utils import log
|
||||
from ietf.mailtrigger.utils import gather_address_lists
|
||||
|
||||
|
@ -1255,6 +1256,88 @@ def edit_document_urls(request, name):
|
|||
title = "Additional document URLs"
|
||||
return render(request, 'doc/edit_field.html',dict(doc=doc, form=form, title=title, info=info) )
|
||||
|
||||
|
||||
def edit_doc_extresources(request, name):
|
||||
class DocExtResourceForm(forms.Form):
|
||||
resources = forms.CharField(widget=forms.Textarea, label="Additional Resources", required=False,
|
||||
help_text=("Format: 'tag value (Optional description)'."
|
||||
" Separate multiple entries with newline. When the value is a URL, use https:// where possible.") )
|
||||
|
||||
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 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)
|
||||
|
||||
doc = get_object_or_404(Document, name=name)
|
||||
|
||||
if not (has_role(request.user, ("Secretariat", "Area Director"))
|
||||
or is_authorized_in_doc_stream(request.user, doc)
|
||||
or is_individual_draft_author(request.user, doc)):
|
||||
return HttpResponseForbidden("You do not have the necessary permissions to view this page")
|
||||
|
||||
old_resources = format_resources(doc.docextresource_set.all())
|
||||
|
||||
if request.method == 'POST':
|
||||
form = DocExtResourceForm(request.POST)
|
||||
if form.is_valid():
|
||||
old_resources = sorted(old_resources.splitlines())
|
||||
new_resources = sorted(form.cleaned_data['resources'])
|
||||
if old_resources != new_resources:
|
||||
doc.docextresource_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('()')
|
||||
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])
|
||||
messages.success(request,"Document resources updated.")
|
||||
else:
|
||||
messages.info(request,"No change in Document resources.")
|
||||
return redirect('ietf.doc.views_doc.document_main', name=doc.name)
|
||||
else:
|
||||
form = DocExtResourceForm(initial={'resources': old_resources, })
|
||||
|
||||
info = "Valid tags:<br><br> %s" % ', '.join([ o.slug for o in ExtResourceName.objects.all().order_by('slug') ])
|
||||
# May need to explain the tags more - probably more reason to move to a formset.
|
||||
title = "Additional document resources"
|
||||
return render(request, 'doc/edit_field.html',dict(doc=doc, form=form, title=title, info=info) )
|
||||
|
||||
|
||||
def request_publication(request, name):
|
||||
"""Request publication by RFC Editor for a document which hasn't
|
||||
been through the IESG ballot process."""
|
||||
|
|
|
@ -10,7 +10,7 @@ import ietf.utils.models
|
|||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('name', '0011_populate_extres'),
|
||||
('name', '0010_extres'),
|
||||
('group', '0023_use_milestone_dates_default_to_true'),
|
||||
]
|
||||
|
||||
|
|
|
@ -7,9 +7,13 @@ import re
|
|||
|
||||
import debug
|
||||
|
||||
from collections import OrderedDict
|
||||
from collections import OrderedDict, Counter
|
||||
|
||||
from django.db import migrations
|
||||
from django.core.exceptions import ValidationError
|
||||
|
||||
|
||||
from ietf.utils.validators import validate_external_resource_value
|
||||
|
||||
name_map = {
|
||||
"Issue.*": "tracker",
|
||||
|
@ -63,17 +67,14 @@ def forward(apps, schema_editor):
|
|||
ExtResourceName = apps.get_model('name', 'ExtResourceName')
|
||||
GroupUrl = apps.get_model('group', 'GroupUrl')
|
||||
|
||||
mapped = 0
|
||||
not_mapped = 0
|
||||
ignored = 0
|
||||
stats = Counter()
|
||||
|
||||
debug.say("Matching...")
|
||||
for group_url in GroupUrl.objects.all():
|
||||
match_found = False
|
||||
for regext,slug in name_map.items():
|
||||
if re.match(regext, group_url.name):
|
||||
match_found = True
|
||||
mapped += 1
|
||||
stats['mapped'] += 1
|
||||
name = ExtResourceName.objects.get(slug=slug)
|
||||
GroupExtResource.objects.create(group=group_url.group, name_id=slug, value=group_url.url, display_name=group_url.name) # TODO: validate this value against name.type
|
||||
break
|
||||
|
@ -83,7 +84,7 @@ def forward(apps, schema_editor):
|
|||
if re.search(regext, group_url.url):
|
||||
match_found = True
|
||||
if slug:
|
||||
mapped +=1
|
||||
stats['mapped'] +=1
|
||||
name = ExtResourceName.objects.get(slug=slug)
|
||||
# Munge the URL if it's the first github repo match
|
||||
# Remove "/tree/master" substring if it exists
|
||||
|
@ -93,14 +94,19 @@ def forward(apps, schema_editor):
|
|||
group_url.url = group_url.url.replace("/tree/master","")
|
||||
group_url.url = re.sub('/issues$', '', group_url.url)
|
||||
group_url.url = re.sub('/blob/master.*$', '', group_url.url)
|
||||
GroupExtResource.objects.create(group=group_url.group, name_id=slug, value=group_url.url, display_name=group_url.name) # TODO: validate this value against name.type
|
||||
try:
|
||||
validate_external_resource_value(name, group_url.url)
|
||||
GroupExtResource.objects.create(group=group_url.group, name=name, value=group_url.url, display_name=group_url.name) # TODO: validate this value against name.type
|
||||
except ValidationError as e: # pyflakes:ignore
|
||||
debug.show('("Failed validation:", group_url.url, e)')
|
||||
stats['failed_validation'] +=1
|
||||
else:
|
||||
ignored +=1
|
||||
stats['ignored'] +=1
|
||||
break
|
||||
if not match_found:
|
||||
debug.show('("Not Mapped:",group_url.group.acronym, group_url.name, group_url.url)')
|
||||
not_mapped += 1
|
||||
debug.show('(mapped, ignored, not_mapped)')
|
||||
stats['not_mapped'] += 1
|
||||
print(stats)
|
||||
|
||||
def reverse(apps, schema_editor):
|
||||
GroupExtResource = apps.get_model('group', 'GroupExtResource')
|
||||
|
|
File diff suppressed because it is too large
Load diff
|
@ -55,6 +55,9 @@ class Migration(migrations.Migration):
|
|||
|
||||
dependencies = [
|
||||
('name', '0010_extres'),
|
||||
('group', '0024_extres'),
|
||||
('doc', '0032_extres'),
|
||||
('person', '0010_extres'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
|
|
|
@ -10,7 +10,7 @@ import ietf.utils.models
|
|||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('name', '0011_populate_extres'),
|
||||
('name', '0010_extres'),
|
||||
('person', '0009_auto_20190118_0725'),
|
||||
]
|
||||
|
||||
|
|
|
@ -272,6 +272,38 @@
|
|||
</tr>
|
||||
{% endif %}
|
||||
{% endwith %}
|
||||
{% with doc.docextresource_set.all as resources %}
|
||||
{% if resources or can_edit_stream_info or can_edit_individual %}
|
||||
<tr>
|
||||
<td></td>
|
||||
<th>Additional Resources</th>
|
||||
<td class="edit">
|
||||
{% if can_edit_stream_info or can_edit_individual %}
|
||||
<a class="btn btn-default btn-xs" href="{% url 'ietf.doc.views_draft.edit_doc_extresources' name=doc.name %}">Edit</a>
|
||||
{% endif %}
|
||||
</td>
|
||||
<td>
|
||||
{% if resources or doc.group and doc.group.list_archive %}
|
||||
<table class="col-md-12 col-sm-12 col-xs-12">
|
||||
<tbody>
|
||||
{% for resource in resources|dictsort:"display_name" %}
|
||||
{% if resource.name.type.slug == 'url' or resource.name.type.slug == 'email' %}
|
||||
<tr><td> - <a href="{{ resource.value }}" title="{{resource.name.name}}">{% firstof resource.display_name resource.name.name %}</a></td></tr>
|
||||
{# Maybe make how a resource displays itself a method on the class so templates aren't doing this switching #}
|
||||
{% else %}
|
||||
<tr><td> - <span title="{{resource.name.name}}">{% firstof resource.display_name resource.name.name %}: {{resource.value}}</span></td></tr>
|
||||
{% endif %}
|
||||
{% endfor %}
|
||||
{% if doc.group and doc.group.list_archive %}
|
||||
<tr><td> - <a href="{{doc.group.list_archive}}?q={{doc.name}}">Mailing list discussion</a><td></tr>
|
||||
{% endif %}
|
||||
</tbody>
|
||||
</table>
|
||||
{% endif %}
|
||||
</td>
|
||||
</tr>
|
||||
{% endif %}
|
||||
{% endwith %}
|
||||
|
||||
|
||||
</tbody>
|
||||
|
|
|
@ -6,10 +6,12 @@ from __future__ import absolute_import, print_function, unicode_literals
|
|||
import os
|
||||
import re
|
||||
from pyquery import PyQuery
|
||||
from urllib.parse import urlparse
|
||||
|
||||
|
||||
from django.conf import settings
|
||||
from django.core.exceptions import ValidationError
|
||||
from django.core.validators import RegexValidator
|
||||
from django.core.validators import RegexValidator, URLValidator, EmailValidator
|
||||
from django.template.defaultfilters import filesizeformat
|
||||
from django.utils.deconstruct import deconstructible
|
||||
|
||||
|
@ -84,3 +86,37 @@ def validate_no_html_frame(file):
|
|||
q = PyQuery(file.read())
|
||||
if q("frameset") or q("frame") or q("iframe"):
|
||||
raise ValidationError('Found content with html frames. Please upload a file that does not use frames')
|
||||
|
||||
# instantiations of sub-validiators used by the external_resource validator
|
||||
|
||||
validate_url = URLValidator()
|
||||
validate_http_url = URLValidator(schemes=['http','https'])
|
||||
validate_email = EmailValidator()
|
||||
|
||||
def validate_external_resource_value(name, value):
|
||||
""" validate a resource value using its name's properties """
|
||||
|
||||
if name.type.slug == 'url':
|
||||
|
||||
if name.slug in ( 'github_org', 'github_repo' ):
|
||||
validate_http_url(value)
|
||||
if urlparse(value).netloc.lower() != 'github.com':
|
||||
raise ValidationError('URL must be a github url')
|
||||
elif name.slug == 'jabber_room':
|
||||
pass
|
||||
# TODO - build a xmpp URL validator. See XEP-0032.
|
||||
# It should be easy to build one by copyhacking URLValidator,
|
||||
# but reading source says it would be better to wait to do that
|
||||
# until after we make the Django 2 transition
|
||||
else:
|
||||
validate_url(value)
|
||||
|
||||
elif name.type.slug == 'email':
|
||||
validate_email(value)
|
||||
|
||||
elif name.type.slug == 'string':
|
||||
pass
|
||||
|
||||
else:
|
||||
raise ValidationError('Unknown resource type '+name.type.name)
|
||||
|
||||
|
|
Loading…
Reference in a new issue