Skip to content

Commit

Permalink
fix: various corrections to rfc main document view and tests (#5931)
Browse files Browse the repository at this point in the history
  • Loading branch information
rjsparks authored Jul 6, 2023
1 parent 63a9920 commit f53a849
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 126 deletions.
22 changes: 10 additions & 12 deletions ietf/doc/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,18 +137,17 @@ def file_extension(self):

def get_file_path(self):
if not hasattr(self, '_cached_file_path'):
if self.type_id == "draft":
if self.type_id == "rfc":
self._cached_file_path = settings.RFC_PATH
elif self.type_id == "draft":
if self.is_dochistory():
self._cached_file_path = settings.INTERNET_ALL_DRAFTS_ARCHIVE_DIR
else:
if self.get_state_slug() == "rfc":
self._cached_file_path = settings.RFC_PATH
draft_state = self.get_state('draft')
if draft_state and draft_state.slug == 'active':
self._cached_file_path = settings.INTERNET_DRAFT_PATH
else:
draft_state = self.get_state('draft')
if draft_state and draft_state.slug == 'active':
self._cached_file_path = settings.INTERNET_DRAFT_PATH
else:
self._cached_file_path = settings.INTERNET_ALL_DRAFTS_ARCHIVE_DIR
self._cached_file_path = settings.INTERNET_ALL_DRAFTS_ARCHIVE_DIR
elif self.meeting_related() and self.type_id in (
"agenda", "minutes", "slides", "bluesheets", "procmaterials", "chatlog", "polls"
):
Expand All @@ -173,14 +172,13 @@ def get_base_name(self):
if not hasattr(self, '_cached_base_name'):
if self.uploaded_filename:
self._cached_base_name = self.uploaded_filename
if self.type_id == 'rfc':
self._cached_base_name = "%s.txt" % self.canonical_name()
elif self.type_id == 'draft':
if self.is_dochistory():
self._cached_base_name = "%s-%s.txt" % (self.doc.name, self.rev)
else:
if self.get_state_slug() == 'rfc':
self._cached_base_name = "%s.txt" % self.canonical_name()
else:
self._cached_base_name = "%s-%s.txt" % (self.name, self.rev)
self._cached_base_name = "%s-%s.txt" % (self.name, self.rev)
elif self.type_id in ["slides", "agenda", "minutes", "bluesheets", "procmaterials", ] and self.meeting_related():
ext = 'pdf' if self.type_id == 'procmaterials' else 'txt'
self._cached_base_name = f'{self.canonical_name()}-{self.rev}.{ext}'
Expand Down
99 changes: 14 additions & 85 deletions ietf/doc/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -619,21 +619,12 @@ def setUp(self):
def test_document_draft(self):
draft = WgDraftFactory(name='draft-ietf-mars-test',rev='01', create_revisions=range(0,2))


HolderIprDisclosureFactory(docs=[draft])

# Docs for testing relationships. Does not test 'possibly-replaces'. The 'replaced_by' direction
# is tested separately below.
replaced = IndividualDraftFactory()
draft.relateddocument_set.create(relationship_id='replaces',source=draft,target=replaced.docalias.first())
obsoleted = IndividualDraftFactory()
draft.relateddocument_set.create(relationship_id='obs',source=draft,target=obsoleted.docalias.first())
obsoleted_by = IndividualDraftFactory()
obsoleted_by.relateddocument_set.create(relationship_id='obs',source=obsoleted_by,target=draft.docalias.first())
updated = IndividualDraftFactory()
draft.relateddocument_set.create(relationship_id='updates',source=draft,target=updated.docalias.first())
updated_by = IndividualDraftFactory()
updated_by.relateddocument_set.create(relationship_id='updates',source=obsoleted_by,target=draft.docalias.first())

external_resource = DocExtResourceFactory(doc=draft)

Expand All @@ -648,16 +639,6 @@ def test_document_draft(self):
self.assertNotContains(r, "Deimos street")
self.assertContains(r, replaced.canonical_name())
self.assertContains(r, replaced.title)
# obs/updates not included until draft is RFC
self.assertNotContains(r, obsoleted.canonical_name())
self.assertNotContains(r, obsoleted.title)
self.assertNotContains(r, obsoleted_by.canonical_name())
self.assertNotContains(r, obsoleted_by.title)
self.assertNotContains(r, updated.canonical_name())
self.assertNotContains(r, updated.title)
self.assertNotContains(r, updated_by.canonical_name())
self.assertNotContains(r, updated_by.title)
self.assertContains(r, external_resource.value)

r = self.client.get(urlreverse("ietf.doc.views_doc.document_main", kwargs=dict(name=draft.name)) + "?include_text=0")
self.assertEqual(r.status_code, 200)
Expand All @@ -666,15 +647,6 @@ def test_document_draft(self):
self.assertNotContains(r, "Deimos street")
self.assertContains(r, replaced.canonical_name())
self.assertContains(r, replaced.title)
# obs/updates not included until draft is RFC
self.assertNotContains(r, obsoleted.canonical_name())
self.assertNotContains(r, obsoleted.title)
self.assertNotContains(r, obsoleted_by.canonical_name())
self.assertNotContains(r, obsoleted_by.title)
self.assertNotContains(r, updated.canonical_name())
self.assertNotContains(r, updated.title)
self.assertNotContains(r, updated_by.canonical_name())
self.assertNotContains(r, updated_by.title)

r = self.client.get(urlreverse("ietf.doc.views_doc.document_main", kwargs=dict(name=draft.name)) + "?include_text=foo")
self.assertEqual(r.status_code, 200)
Expand All @@ -683,15 +655,6 @@ def test_document_draft(self):
self.assertContains(r, "Deimos street")
self.assertContains(r, replaced.canonical_name())
self.assertContains(r, replaced.title)
# obs/updates not included until draft is RFC
self.assertNotContains(r, obsoleted.canonical_name())
self.assertNotContains(r, obsoleted.title)
self.assertNotContains(r, obsoleted_by.canonical_name())
self.assertNotContains(r, obsoleted_by.title)
self.assertNotContains(r, updated.canonical_name())
self.assertNotContains(r, updated.title)
self.assertNotContains(r, updated_by.canonical_name())
self.assertNotContains(r, updated_by.title)

r = self.client.get(urlreverse("ietf.doc.views_doc.document_main", kwargs=dict(name=draft.name)) + "?include_text=1")
self.assertEqual(r.status_code, 200)
Expand All @@ -700,15 +663,6 @@ def test_document_draft(self):
self.assertContains(r, "Deimos street")
self.assertContains(r, replaced.canonical_name())
self.assertContains(r, replaced.title)
# obs/updates not included until draft is RFC
self.assertNotContains(r, obsoleted.canonical_name())
self.assertNotContains(r, obsoleted.title)
self.assertNotContains(r, obsoleted_by.canonical_name())
self.assertNotContains(r, obsoleted_by.title)
self.assertNotContains(r, updated.canonical_name())
self.assertNotContains(r, updated.title)
self.assertNotContains(r, updated_by.canonical_name())
self.assertNotContains(r, updated_by.title)

self.client.cookies = SimpleCookie({str('full_draft'): str('on')})
r = self.client.get(urlreverse("ietf.doc.views_doc.document_main", kwargs=dict(name=draft.name)))
Expand All @@ -718,15 +672,6 @@ def test_document_draft(self):
self.assertContains(r, "Deimos street")
self.assertContains(r, replaced.canonical_name())
self.assertContains(r, replaced.title)
# obs/updates not included until draft is RFC
self.assertNotContains(r, obsoleted.canonical_name())
self.assertNotContains(r, obsoleted.title)
self.assertNotContains(r, obsoleted_by.canonical_name())
self.assertNotContains(r, obsoleted_by.title)
self.assertNotContains(r, updated.canonical_name())
self.assertNotContains(r, updated.title)
self.assertNotContains(r, updated_by.canonical_name())
self.assertNotContains(r, updated_by.title)

self.client.cookies = SimpleCookie({str('full_draft'): str('off')})
r = self.client.get(urlreverse("ietf.doc.views_doc.document_main", kwargs=dict(name=draft.name)))
Expand All @@ -736,15 +681,6 @@ def test_document_draft(self):
self.assertNotContains(r, "Deimos street")
self.assertContains(r, replaced.canonical_name())
self.assertContains(r, replaced.title)
# obs/updates not included until draft is RFC
self.assertNotContains(r, obsoleted.canonical_name())
self.assertNotContains(r, obsoleted.title)
self.assertNotContains(r, obsoleted_by.canonical_name())
self.assertNotContains(r, obsoleted_by.title)
self.assertNotContains(r, updated.canonical_name())
self.assertNotContains(r, updated.title)
self.assertNotContains(r, updated_by.canonical_name())
self.assertNotContains(r, updated_by.title)

self.client.cookies = SimpleCookie({str('full_draft'): str('foo')})
r = self.client.get(urlreverse("ietf.doc.views_doc.document_main", kwargs=dict(name=draft.name)))
Expand All @@ -755,15 +691,6 @@ def test_document_draft(self):
self.assertNotContains(r, "Deimos street")
self.assertContains(r, replaced.canonical_name())
self.assertContains(r, replaced.title)
# obs/updates not included until draft is RFC
self.assertNotContains(r, obsoleted.canonical_name())
self.assertNotContains(r, obsoleted.title)
self.assertNotContains(r, obsoleted_by.canonical_name())
self.assertNotContains(r, obsoleted_by.title)
self.assertNotContains(r, updated.canonical_name())
self.assertNotContains(r, updated.title)
self.assertNotContains(r, updated_by.canonical_name())
self.assertNotContains(r, updated_by.title)

r = self.client.get(urlreverse("ietf.doc.views_doc.document_html", kwargs=dict(name=draft.name)))
self.assertEqual(r.status_code, 200)
Expand Down Expand Up @@ -831,26 +758,29 @@ def test_document_draft(self):

# draft published as RFC
draft.set_state(State.objects.get(type="draft", slug="rfc"))
draft.std_level_id = "bcp"
draft.save_with_history([DocEvent.objects.create(doc=draft, rev=draft.rev, type="published_rfc", by=Person.objects.get(name="(System)"))])
draft.std_level_id = "ps"

rfc = WgRfcFactory(group=draft.group, name="rfc123456")
rfc.save_with_history([DocEvent.objects.create(doc=rfc, rev=None, type="published_rfc", by=Person.objects.get(name="(System)"))])

draft.relateddocument_set.create(relationship_id="became_rfc", target=rfc.docalias.first())

rfc_alias = DocAlias.objects.create(name="rfc123456")
rfc_alias.docs.add(draft)
bcp_alias = DocAlias.objects.create(name="bcp123456")
bcp_alias.docs.add(draft)
obsoleted = IndividualRfcFactory()
rfc.relateddocument_set.create(relationship_id='obs',target=obsoleted.docalias.first())
obsoleted_by = IndividualRfcFactory()
obsoleted_by.relateddocument_set.create(relationship_id='obs',target=rfc.docalias.first())
updated = IndividualRfcFactory()
rfc.relateddocument_set.create(relationship_id='updates',target=updated.docalias.first())
updated_by = IndividualRfcFactory()
updated_by.relateddocument_set.create(relationship_id='updates',target=rfc.docalias.first())

r = self.client.get(urlreverse("ietf.doc.views_doc.document_main", kwargs=dict(name=draft.name)))
self.assertEqual(r.status_code, 302)
r = self.client.get(urlreverse("ietf.doc.views_doc.document_main", kwargs=dict(name=bcp_alias.name)))
self.assertEqual(r.status_code, 302)

r = self.client.get(urlreverse("ietf.doc.views_doc.document_main", kwargs=dict(name=rfc_alias.name)))
r = self.client.get(urlreverse("ietf.doc.views_doc.document_main", kwargs=dict(name=rfc.name)))
self.assertEqual(r.status_code, 200)
self.assertContains(r, "RFC 123456")
self.assertContains(r, draft.name)
self.assertContains(r, replaced.canonical_name())
self.assertContains(r, replaced.title)
# obs/updates included with RFC
self.assertContains(r, obsoleted.canonical_name())
self.assertContains(r, obsoleted.title)
Expand Down Expand Up @@ -1505,7 +1435,6 @@ def test_draft_group_link(self):

rfc = WgRfcFactory(name='draft-rfc-document-%s' % group_type_id, group=group)
DocEventFactory.create(doc=rfc, type='published_rfc', time=event_datetime)
# get the rfc name to avoid a redirect
r = self.client.get(urlreverse("ietf.doc.views_doc.document_main", kwargs=dict(name=rfc.name)))
self.assertEqual(r.status_code, 200)
self.assert_correct_non_wg_group_link(r, group)
Expand Down
8 changes: 7 additions & 1 deletion ietf/doc/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1169,8 +1169,14 @@ def fuzzy_find_documents(name, rev=None):
if re.match("^[0-9]+$", name):
name = f'rfc{name}'

if name.startswith("rfc"):
sought_type = "rfc"
log.assertion("rev is None")
else:
sought_type = "draft"

# see if we can find a document using this name
docs = Document.objects.filter(docalias__name=name, type_id='draft')
docs = Document.objects.filter(docalias__name=name, type_id=sought_type)
if rev and not docs.exists():
# No document found, see if the name/rev split has been misidentified.
# Handles some special cases, like draft-ietf-tsvwg-ieee-802-11.
Expand Down
29 changes: 13 additions & 16 deletions ietf/doc/views_doc.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,10 @@ def document_main(request, name, rev=None, document_html=False):
doc = get_object_or_404(Document.objects.select_related(), docalias__name=name)

# take care of possible redirections
aliases = DocAlias.objects.filter(docs=doc).values_list("name", flat=True)
if document_html is False and rev==None and doc.type_id == "draft" and not name.startswith("rfc"):
for a in aliases:
if a.startswith("rfc"):
return redirect("ietf.doc.views_doc.document_main", name=a)
if document_html is False and rev is None:
became_rfc = next(iter(doc.related_that_doc("became_rfc")), None)
if became_rfc:
return redirect("ietf.doc.views_doc.document_main", name=became_rfc.name)

revisions = []
for h in doc.history_set.order_by("time", "id"):
Expand Down Expand Up @@ -265,9 +264,6 @@ def document_main(request, name, rev=None, document_html=False):

can_change_stream = bool(can_edit or roles)

rfc_aliases = [prettify_std_name(a) for a in aliases
if a.startswith("fyi") or a.startswith("std") or a.startswith("bcp")]

file_urls, found_types = build_file_urls(doc)
content = doc.text_or_error() # pyflakes:ignore
content = markup_txt.markup(maybe_split(content, split=split_content))
Expand Down Expand Up @@ -329,6 +325,10 @@ def document_main(request, name, rev=None, document_html=False):
css = Path(finders.find("ietf/css/document_html_inline.css")).read_text()
if html:
css += Path(finders.find("ietf/css/document_html_txt.css")).read_text()
draft_that_became_rfc = None
became_rfc_alias = next(iter(doc.related_that("became_rfc")), None)
if became_rfc_alias:
draft_that_became_rfc = became_rfc_alias.document
# submission
submission = ""
if group is None:
Expand All @@ -343,13 +343,12 @@ def document_main(request, name, rev=None, document_html=False):
else:
submission = group.acronym
submission = '<a href="%s">%s</a>' % (group.about_url(), submission)
draft_alias = next(iter(doc.related_that("became_rfc")), None)
# Should be unreachable?
if (
draft_alias
and draft_alias.document.stream_id
and draft_alias.document.get_state_slug(
"draft-stream-%s" % draft_alias.document.stream_id
draft_that_became_rfc
and draft_that_became_rfc.stream_id
and draft_that_became_rfc.get_state_slug(
"draft-stream-%s" % draft_that_became_rfc.stream_id
)
== "c-adopt"
):
Expand All @@ -375,14 +374,13 @@ def document_main(request, name, rev=None, document_html=False):
can_edit_authors=can_edit_authors,
can_change_stream=can_change_stream,
rfc_number=doc.rfc_number,
draft_name=doc.name,
draft_name=draft_that_became_rfc and draft_that_became_rfc.name,
updates=interesting_relations_that_doc.filter(relationship="updates"),
updated_by=interesting_relations_that.filter(relationship="updates"),
obsoletes=interesting_relations_that_doc.filter(relationship="obs"),
obsoleted_by=interesting_relations_that.filter(relationship="obs"),
status_changes=status_changes,
proposed_status_changes=proposed_status_changes,
rfc_aliases=rfc_aliases,
has_errata=doc.pk and doc.tags.filter(slug="errata"), # doc.pk == None if using a fake_history_obj
file_urls=file_urls,
additional_urls=additional_urls,
Expand Down Expand Up @@ -708,7 +706,6 @@ def document_main(request, name, rev=None, document_html=False):
conflict_reviews=conflict_reviews,
status_changes=status_changes,
proposed_status_changes=proposed_status_changes,
# rfc_aliases=rfc_aliases,
has_errata=doc.pk and doc.tags.filter(slug="errata"), # doc.pk == None if using a fake_history_obj
published=published,
file_urls=file_urls,
Expand Down
2 changes: 1 addition & 1 deletion ietf/templates/doc/document_html.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<title>
{% if not snapshot and doc.get_state_slug == "rfc" %}
{% if doc.type_id == "rfc" %}
RFC {{ doc.rfc_number }} - {{ doc.title }}
{% else %}
{{ doc.name }}-{{ doc.rev }}
Expand Down
1 change: 0 additions & 1 deletion ietf/templates/doc/document_info.html
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
{% if proposed_status_changes %}
<div>Proposed status changed by {{ proposed_status_changes|urlize_related_source_list|join:", " }}</div>
{% endif %}
{% if rfc_aliases %}<div>Also known as {{ rfc_aliases|join:", "|urlize_ietf_docs }}</div>{% endif %}
{% if draft_name %}
<div>
Was
Expand Down
10 changes: 0 additions & 10 deletions ietf/templates/doc/document_rfc.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,14 @@
{% endblock %}
{% block morecss %}.inline { display: inline; }{% endblock %}
{% block title %}
{% if doc.get_state_slug == "rfc" and not snapshot %}
RFC {{ rfc_number }} - {{ doc.title }}
{% else %}
{{ name }}-{{ doc.rev }} - {{ doc.title }}
{% endif %}
{% endblock %}
{% block content %}
{% origin %}
{{ top|safe }}
<div id="timeline"></div>
{% if doc.rev != latest_rev %}
<div class="alert alert-warning my-3">The information below is for an old version of the document.</div>
{% else %}
{% if doc.get_state_slug == "rfc" and snapshot %}
<div class="alert alert-warning my-3">
The information below is for an old version of the document that is already published as an RFC.
</div>
{% endif %}
{% endif %}
<table class="table table-sm table-borderless">
{% include "doc/document_info.html" %}
Expand Down

0 comments on commit f53a849

Please sign in to comment.