Skip to content

Commit 2de9eb9

Browse files
rjsparksrjsparks
committed
Added validation to the migrations. Added external resources to the draft display page. Built an editor for DocExternalResources.
 - Legacy-Id: 17683
1 parent 672f9bc commit 2de9eb9

File tree

12 files changed

+14795
-14306
lines changed

12 files changed

+14795
-14306
lines changed

‎ietf/doc/migrations/0032_extres.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
class Migration(migrations.Migration):
1111

1212
dependencies = [
13-
('name', '0011_populate_extres'),
13+
('name', '0010_extres'),
1414
('doc', '0031_set_state_for_charters_of_replaced_groups'),
1515
]
1616

‎ietf/doc/migrations/0033_populate_docextresources.py‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,14 @@
77

88
import debug
99

10-
from collections import OrderedDict
10+
from collections import OrderedDict, Counter
1111

1212
from django.db import migrations
1313

14+
from ietf.utils.validators import validate_external_resource_value
15+
from django.core.exceptions import ValidationError
16+
17+
1418
name_map = {
1519
"Issue.*": "tracker",
1620
".*FAQ.*": "faq",
@@ -55,16 +59,14 @@ def forward(apps, schema_editor):
5559
ExtResourceName = apps.get_model('name', 'ExtResourceName')
5660
DocumentUrl = apps.get_model('doc', 'DocumentUrl')
5761

58-
mapped = 0
59-
not_mapped = 0
60-
ignored = 0
62+
stats = Counter()
6163

6264
for doc_url in DocumentUrl.objects.all():
6365
match_found = False
6466
for regext,slug in name_map.items():
6567
if re.match(regext, doc_url.desc):
6668
match_found = True
67-
mapped += 1
69+
stats['mapped'] += 1
6870
name = ExtResourceName.objects.get(slug=slug)
6971
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
7072
break
@@ -74,7 +76,7 @@ def forward(apps, schema_editor):
7476
if re.search(regext, doc_url.url):
7577
match_found = True
7678
if slug:
77-
mapped +=1
79+
stats['mapped'] +=1
7880
name = ExtResourceName.objects.get(slug=slug)
7981
# Munge the URL if it's the first github repo match
8082
# Remove "/tree/master" substring if it exists
@@ -84,14 +86,19 @@ def forward(apps, schema_editor):
8486
doc_url.url = doc_url.url.replace("/tree/master","")
8587
doc_url.url = re.sub('/issues$', '', doc_url.url)
8688
doc_url.url = re.sub('/blob/master.*$', '', doc_url.url)
87-
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
89+
try:
90+
validate_external_resource_value(name, doc_url.url)
91+
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
92+
except ValidationError as e: # pyflakes:ignore
93+
debug.show('("Failed validation:", doc_url.url, e)')
94+
stats['failed_validation'] +=1
8895
else:
89-
ignored +=1
96+
stats['ignored'] +=1
9097
break
9198
if not match_found:
9299
debug.show('("Not Mapped:",doc_url.desc, doc_url.tag.slug, doc_url.doc.name, doc_url.url)')
93-
not_mapped += 1
94-
debug.show('(mapped, ignored, not_mapped)')
100+
stats['not_mapped'] += 1
101+
print (stats)
95102

96103
def reverse(apps, schema_editor):
97104
DocExtResource = apps.get_model('doc', 'DocExtResource')

‎ietf/doc/tests_draft.py‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,6 +1125,46 @@ def test_doc_change_document_urls(self):
11251125
self.assertIn('wiki https://wiki.org/', doc.latest_event(DocEvent,type="changed_document").desc)
11261126
self.assertIn('https://wiki.org/', [ u.url for u in doc.documenturl_set.all() ])
11271127

1128+
def test_edit_doc_extresources(self):
1129+
url = urlreverse('ietf.doc.views_draft.edit_doc_extresources', kwargs=dict(name=self.docname))
1130+
1131+
login_testing_unauthorized(self, "secretary", url)
1132+
1133+
r = self.client.get(url)
1134+
self.assertEqual(r.status_code,200)
1135+
q = PyQuery(r.content)
1136+
self.assertEqual(len(q('form textarea[id=id_resources]')),1)
1137+
1138+
# AMHERE
1139+
badlines = (
1140+
'github_repo https://github3.com/some/repo',
1141+
'github_notify badaddr',
1142+
'website /not/a/good/url'
1143+
'notavalidtag blahblahblah'
1144+
)
1145+
1146+
for line in badlines:
1147+
r = self.client.post(url, dict(resources=line, submit="1"))
1148+
self.assertEqual(r.status_code, 200)
1149+
q = PyQuery(r.content)
1150+
self.assertTrue(q('.alert-danger'))
1151+
1152+
goodlines = """
1153+
github_repo https://github.com/some/repo Some display text
1154+
github_notify notify@example.com
1155+
github_username githubuser
1156+
website http://example.com/http/is/fine
1157+
"""
1158+
1159+
r = self.client.post(url, dict(resources=goodlines, submit="1"))
1160+
self.assertEqual(r.status_code,302)
1161+
doc = Document.objects.get(name=self.docname)
1162+
self.assertEqual(doc.latest_event(DocEvent,type="changed_document").desc[:35], 'Changed document external resources')
1163+
self.assertIn('github_username githubuser', doc.latest_event(DocEvent,type="changed_document").desc)
1164+
self.assertEqual(doc.docextresource_set.count(), 4)
1165+
self.assertEqual(doc.docextresource_set.get(name__slug='github_repo').display_name, 'Some display text')
1166+
1167+
11281168
class SubmitToIesgTests(TestCase):
11291169

11301170
def setUp(self):

‎ietf/doc/urls.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@
131131
url(r'^%(name)s/edit/approvedownrefs/$' % settings.URL_REGEXPS, views_ballot.approve_downrefs),
132132
url(r'^%(name)s/edit/makelastcall/$' % settings.URL_REGEXPS, views_ballot.make_last_call),
133133
url(r'^%(name)s/edit/urls/$' % settings.URL_REGEXPS, views_draft.edit_document_urls),
134+
url(r'^%(name)s/edit/resources/$' % settings.URL_REGEXPS, views_draft.edit_doc_extresources),
134135
url(r'^%(name)s/edit/issueballot/irsg/$' % settings.URL_REGEXPS, views_ballot.issue_irsg_ballot),
135136
url(r'^%(name)s/edit/closeballot/irsg/$' % settings.URL_REGEXPS, views_ballot.close_irsg_ballot),
136137

‎ietf/doc/views_draft.py‎

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,12 @@
4545
from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream, user_is_person, is_individual_draft_author
4646
from ietf.ietfauth.utils import role_required
4747
from ietf.message.models import Message
48-
from ietf.name.models import IntendedStdLevelName, DocTagName, StreamName, DocUrlTagName
48+
from ietf.name.models import IntendedStdLevelName, DocTagName, StreamName, DocUrlTagName, ExtResourceName
4949
from ietf.person.fields import SearchableEmailField
5050
from ietf.person.models import Person, Email
5151
from ietf.utils.mail import send_mail, send_mail_message, on_behalf_of
5252
from ietf.utils.textupload import get_cleaned_text_file_content
53+
from ietf.utils.validators import validate_external_resource_value
5354
from ietf.utils import log
5455
from ietf.mailtrigger.utils import gather_address_lists
5556

@@ -1255,6 +1256,88 @@ def format_urls(urls, fs="\n"):
12551256
title = "Additional document URLs"
12561257
return render(request, 'doc/edit_field.html',dict(doc=doc, form=form, title=title, info=info) )
12571258

1259+
1260+
def edit_doc_extresources(request, name):
1261+
class DocExtResourceForm(forms.Form):
1262+
resources = forms.CharField(widget=forms.Textarea, label="Additional Resources", required=False,
1263+
help_text=("Format: 'tag value (Optional description)'."
1264+
" Separate multiple entries with newline. When the value is a URL, use https:// where possible.") )
1265+
1266+
def clean_resources(self):
1267+
lines = [x.strip() for x in self.cleaned_data["resources"].splitlines() if x.strip()]
1268+
errors = []
1269+
for l in lines:
1270+
parts = l.split()
1271+
if len(parts) == 1:
1272+
errors.append("Too few fields: Expected at least tag and value: '%s'" % l)
1273+
elif len(parts) >= 2:
1274+
name_slug = parts[0]
1275+
try:
1276+
name = ExtResourceName.objects.get(slug=name_slug)
1277+
except ObjectDoesNotExist:
1278+
errors.append("Bad tag in '%s': Expected one of %s" % (l, ', '.join([ o.slug for o in ExtResourceName.objects.all() ])))
1279+
continue
1280+
value = parts[1]
1281+
try:
1282+
validate_external_resource_value(name, value)
1283+
except ValidationError as e:
1284+
e.message += " : " + value
1285+
errors.append(e)
1286+
if errors:
1287+
raise ValidationError(errors)
1288+
return lines
1289+
1290+
def format_resources(resources, fs="\n"):
1291+
res = []
1292+
for r in resources:
1293+
if r.display_name:
1294+
res.append("%s %s (%s)" % (r.name.slug, r.value, r.display_name.strip('()')))
1295+
else:
1296+
res.append("%s %s" % (r.name.slug, r.value))
1297+
# 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.
1298+
# Might be better to shift to a formset instead of parsing these lines.
1299+
return fs.join(res)
1300+
1301+
doc = get_object_or_404(Document, name=name)
1302+
1303+
if not (has_role(request.user, ("Secretariat", "Area Director"))
1304+
or is_authorized_in_doc_stream(request.user, doc)
1305+
or is_individual_draft_author(request.user, doc)):
1306+
return HttpResponseForbidden("You do not have the necessary permissions to view this page")
1307+
1308+
old_resources = format_resources(doc.docextresource_set.all())
1309+
1310+
if request.method == 'POST':
1311+
form = DocExtResourceForm(request.POST)
1312+
if form.is_valid():
1313+
old_resources = sorted(old_resources.splitlines())
1314+
new_resources = sorted(form.cleaned_data['resources'])
1315+
if old_resources != new_resources:
1316+
doc.docextresource_set.all().delete()
1317+
for u in new_resources:
1318+
parts = u.split(None, 2)
1319+
name = parts[0]
1320+
value = parts[1]
1321+
display_name = ' '.join(parts[2:]).strip('()')
1322+
doc.docextresource_set.create(value=value, name_id=name, display_name=display_name)
1323+
new_resources = format_resources(doc.docextresource_set.all())
1324+
e = DocEvent(doc=doc, rev=doc.rev, by=request.user.person, type='changed_document')
1325+
e.desc = "Changed document external resources from:\n\n%s\n\nto:\n\n%s" % (old_resources, new_resources)
1326+
e.save()
1327+
doc.save_with_history([e])
1328+
messages.success(request,"Document resources updated.")
1329+
else:
1330+
messages.info(request,"No change in Document resources.")
1331+
return redirect('ietf.doc.views_doc.document_main', name=doc.name)
1332+
else:
1333+
form = DocExtResourceForm(initial={'resources': old_resources, })
1334+
1335+
info = "Valid tags:<br><br> %s" % ', '.join([ o.slug for o in ExtResourceName.objects.all().order_by('slug') ])
1336+
# May need to explain the tags more - probably more reason to move to a formset.
1337+
title = "Additional document resources"
1338+
return render(request, 'doc/edit_field.html',dict(doc=doc, form=form, title=title, info=info) )
1339+
1340+
12581341
def request_publication(request, name):
12591342
"""Request publication by RFC Editor for a document which hasn't
12601343
been through the IESG ballot process."""

‎ietf/group/migrations/0024_extres.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
class Migration(migrations.Migration):
1111

1212
dependencies = [
13-
('name', '0011_populate_extres'),
13+
('name', '0010_extres'),
1414
('group', '0023_use_milestone_dates_default_to_true'),
1515
]
1616

‎ietf/group/migrations/0025_populate_groupextresources.py‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,13 @@
77

88
import debug
99

10-
from collections import OrderedDict
10+
from collections import OrderedDict, Counter
1111

1212
from django.db import migrations
13+
from django.core.exceptions import ValidationError
14+
15+
16+
from ietf.utils.validators import validate_external_resource_value
1317

1418
name_map = {
1519
"Issue.*": "tracker",
@@ -63,17 +67,14 @@ def forward(apps, schema_editor):
6367
ExtResourceName = apps.get_model('name', 'ExtResourceName')
6468
GroupUrl = apps.get_model('group', 'GroupUrl')
6569

66-
mapped = 0
67-
not_mapped = 0
68-
ignored = 0
70+
stats = Counter()
6971

70-
debug.say("Matching...")
7172
for group_url in GroupUrl.objects.all():
7273
match_found = False
7374
for regext,slug in name_map.items():
7475
if re.match(regext, group_url.name):
7576
match_found = True
76-
mapped += 1
77+
stats['mapped'] += 1
7778
name = ExtResourceName.objects.get(slug=slug)
7879
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
7980
break
@@ -83,7 +84,7 @@ def forward(apps, schema_editor):
8384
if re.search(regext, group_url.url):
8485
match_found = True
8586
if slug:
86-
mapped +=1
87+
stats['mapped'] +=1
8788
name = ExtResourceName.objects.get(slug=slug)
8889
# Munge the URL if it's the first github repo match
8990
# Remove "/tree/master" substring if it exists
@@ -93,14 +94,19 @@ def forward(apps, schema_editor):
9394
group_url.url = group_url.url.replace("/tree/master","")
9495
group_url.url = re.sub('/issues$', '', group_url.url)
9596
group_url.url = re.sub('/blob/master.*$', '', group_url.url)
96-
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
97+
try:
98+
validate_external_resource_value(name, group_url.url)
99+
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
100+
except ValidationError as e: # pyflakes:ignore
101+
debug.show('("Failed validation:", group_url.url, e)')
102+
stats['failed_validation'] +=1
97103
else:
98-
ignored +=1
104+
stats['ignored'] +=1
99105
break
100106
if not match_found:
101107
debug.show('("Not Mapped:",group_url.group.acronym, group_url.name, group_url.url)')
102-
not_mapped += 1
103-
debug.show('(mapped, ignored, not_mapped)')
108+
stats['not_mapped'] += 1
109+
print(stats)
104110

105111
def reverse(apps, schema_editor):
106112
GroupExtResource = apps.get_model('group', 'GroupExtResource')

0 commit comments

Comments
 (0)