From 43952a8c693b311b9e6673e6a1f471024dd1f7a0 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 19 Apr 2022 09:04:38 -0300 Subject: [PATCH] fix: do not prematurely dereference status change RelatedDocuments (#3835) * test: add tests of the person_link tag * fix: do not prematurely dereference status change RelatedDocuments The urlize_related_source_list template tag expects the RelatedDocument instances, not the source Document. * test: add test cases for status changes on document_main view --- ietf/doc/factories.py | 2 +- ietf/doc/templatetags/ietf_filters.py | 4 +- ietf/doc/tests.py | 45 ++++++++++++++++++ ietf/doc/views_doc.py | 14 ++++-- ietf/person/templatetags/tests.py | 64 ++++++++++++++++++++++++++ ietf/templates/doc/document_draft.html | 4 +- 6 files changed, 125 insertions(+), 8 deletions(-) create mode 100644 ietf/person/templatetags/tests.py diff --git a/ietf/doc/factories.py b/ietf/doc/factories.py index 2c33c936f..fbb6d8156 100644 --- a/ietf/doc/factories.py +++ b/ietf/doc/factories.py @@ -273,7 +273,7 @@ class StatusChangeFactory(BaseDocumentFactory): return if extracted: for (rel, target) in extracted: - obj.relateddocument_set.create(relationship_id=rel,target=extracted) + obj.relateddocument_set.create(relationship_id=rel,target=target) else: obj.relateddocument_set.create(relationship_id='tobcp', target=WgRfcFactory().docalias.first()) diff --git a/ietf/doc/templatetags/ietf_filters.py b/ietf/doc/templatetags/ietf_filters.py index 8238c9c29..bc2fe5e33 100644 --- a/ietf/doc/templatetags/ietf_filters.py +++ b/ietf/doc/templatetags/ietf_filters.py @@ -232,7 +232,7 @@ urlize_ietf_docs = stringfilter(urlize_ietf_docs) @register.filter(name='urlize_related_source_list', is_safe=True, needs_autoescape=True) def urlize_related_source_list(related, autoescape=None): - """Convert a list of DocumentRelations into list of links using the source document's canonical name""" + """Convert a list of RelatedDocuments into list of links using the source document's canonical name""" links = [] names = set() titles = set() @@ -256,7 +256,7 @@ def urlize_related_source_list(related, autoescape=None): @register.filter(name='urlize_related_target_list', is_safe=True, needs_autoescape=True) def urlize_related_target_list(related, autoescape=None): - """Convert a list of DocumentRelations into list of links using the source document's canonical name""" + """Convert a list of RelatedDocuments into list of links using the target document's canonical name""" links = [] for rel in related: name=rel.target.document.canonical_name() diff --git a/ietf/doc/tests.py b/ietf/doc/tests.py index 9fdc3f6b2..f0cbd1f33 100644 --- a/ietf/doc/tests.py +++ b/ietf/doc/tests.py @@ -797,6 +797,51 @@ Man Expires September 22, 2015 [Page 3] msg_prefix='WG-like group %s (%s) should include group type in link' % (group.acronym, group.type), ) + def test_draft_status_changes(self): + draft = WgRfcFactory() + status_change_doc = StatusChangeFactory( + group=draft.group, + changes_status_of=[('tops', draft.docalias.first())], + ) + status_change_url = urlreverse( + 'ietf.doc.views_doc.document_main', + kwargs={'name': status_change_doc.name}, + ) + proposed_status_change_doc = StatusChangeFactory( + group=draft.group, + changes_status_of=[('tobcp', draft.docalias.first())], + states=[State.objects.get(slug='needshep', type='statchg')], + ) + proposed_status_change_url = urlreverse( + 'ietf.doc.views_doc.document_main', + kwargs={'name': proposed_status_change_doc.name}, + ) + + r = self.client.get( + urlreverse( + 'ietf.doc.views_doc.document_main', + kwargs={'name': draft.canonical_name()}, + ) + ) + self.assertEqual(r.status_code, 200) + response_content = r.content.decode() + self.assertInHTML( + 'Status changed by {name}'.format( + name=status_change_doc.name, + title=status_change_doc.title, + url=status_change_url, + ), + response_content, + ) + self.assertInHTML( + 'Proposed status changed by {name}'.format( + name=proposed_status_change_doc.name, + title=proposed_status_change_doc.title, + url=proposed_status_change_url, + ), + response_content, + ) + def assert_correct_non_wg_group_link(self, r, group): """Assert correct format for non-WG-like group types""" self.assertContains( diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index f64149ffd..674d64eca 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -348,9 +348,17 @@ def document_main(request, name, rev=None): # conflict reviews conflict_reviews = [r.source.name for r in interesting_relations_that.filter(relationship="conflrev")] - status_change_docs = interesting_relations_that.filter(relationship__in=STATUSCHANGE_RELATIONS) - status_changes = [ r.source for r in status_change_docs if r.source.get_state_slug() in ('appr-sent','appr-pend')] - proposed_status_changes = [ r.source for r in status_change_docs if r.source.get_state_slug() in ('needshep','adrev','iesgeval','defer','appr-pr')] + # status changes + status_changes = [] + proposed_status_changes = [] + for r in interesting_relations_that.filter(relationship__in=STATUSCHANGE_RELATIONS): + state_slug = r.source.get_state_slug() + if state_slug in ('appr-sent', 'appr-pend'): + status_changes.append(r) + elif state_slug in ('needshep','adrev','iesgeval','defer','appr-pr'): + proposed_status_changes.append(r) + else: + pass presentations = doc.future_presentations() diff --git a/ietf/person/templatetags/tests.py b/ietf/person/templatetags/tests.py new file mode 100644 index 000000000..327cfad6c --- /dev/null +++ b/ietf/person/templatetags/tests.py @@ -0,0 +1,64 @@ +# Copyright The IETF Trust 2022, All Rights Reserved +from ietf.person.factories import PersonFactory +from ietf.utils.test_utils import TestCase + +from .person_filters import person_link + + +class PersonLinkTests(TestCase): + # Tests of the person_link template tag. These assume it is implemented as an + # inclusion tag. + # TODO test that the template actually renders the data in the dict + def test_person_link(self): + person = PersonFactory() + self.assertEqual( + person_link(person), + { + 'name': person.name, + 'plain_name': person.plain_name(), + 'email': person.email_address(), + 'title': '', + 'class': '', + 'with_email': True, + } + ) + self.assertEqual( + person_link(person, with_email=False), + { + 'name': person.name, + 'plain_name': person.plain_name(), + 'email': person.email_address(), + 'title': '', + 'class': '', + 'with_email': False, + } + ) + self.assertEqual( + person_link(person, title='Random Title'), + { + 'name': person.name, + 'plain_name': person.plain_name(), + 'email': person.email_address(), + 'title': 'Random Title', + 'class': '', + 'with_email': True, + } + ) + self.assertEqual( + # funny syntax because 'class' is a Python keyword + person_link(person, **{'class': 'some-class'}), + { + 'name': person.name, + 'plain_name': person.plain_name(), + 'email': person.email_address(), + 'title': '', + 'class': 'some-class', + 'with_email': True, + } + ) + + def test_invalid_person(self): + """Generates correct context dict when input is invalid/missing""" + self.assertEqual(person_link(None), {}) + self.assertEqual(person_link(''), {}) + self.assertEqual(person_link("** No value found for 'somevar' **"), {}) diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html index 80c3f293f..b562aa670 100644 --- a/ietf/templates/doc/document_draft.html +++ b/ietf/templates/doc/document_draft.html @@ -65,10 +65,10 @@ {% if obsoletes %}
Obsoletes {{ obsoletes|urlize_related_target_list|join:", " }}
{% endif %} {% if updates %}
Updates {{ updates|urlize_related_target_list|join:", " }}
{% endif %} {% if status_changes %} -
Status changed by {{ status_changes|join:", "|urlize_related_source_list }}
+
Status changed by {{ status_changes|urlize_related_source_list|join:", " }}
{% endif %} {% if proposed_status_changes %} -
Proposed status changed by {{ proposed_status_changes|join:", "|urlize_related_source_list }}
+
Proposed status changed by {{ proposed_status_changes|urlize_related_source_list|join:", " }}
{% endif %} {% if rfc_aliases %}
Also known as {{ rfc_aliases|join:", "|urlize_ietf_docs }}
{% endif %} {% if draft_name %}