fix: do not prematurely dereference status change RelatedDocuments (ietf-tools#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

Author: jennifer-richards

ietf/doc/factories.py

@@ -273,7 +273,7 @@ def changes_status_of(obj, create, extracted, **kwargs):
             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())
 

ietf/doc/templatetags/ietf_filters.py

@@ -232,7 +232,7 @@ def urlize_ietf_docs(string, autoescape=None):
 
 @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()

ietf/doc/tests.py

@@ -797,6 +797,51 @@ def assert_correct_wg_group_link(self, r, group):
             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 <a href="{url}" title="{title}">{name}</a>'.format(
+                name=status_change_doc.name,
+                title=status_change_doc.title,
+                url=status_change_url,
+            ),
+            response_content,
+        )
+        self.assertInHTML(
+            'Proposed status changed by <a href="{url}" title="{title}">{name}</a>'.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(

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()
 

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' **"), {})

ietf/templates/doc/document_draft.html

@@ -65,10 +65,10 @@
                         {% if obsoletes %}<div>Obsoletes {{ obsoletes|urlize_related_target_list|join:", " }}</div>{% endif %}
                         {% if updates %}<div>Updates {{ updates|urlize_related_target_list|join:", " }}</div>{% endif %}
                         {% if status_changes %}
-                            <div>Status changed by {{ status_changes|join:", "|urlize_related_source_list }}</div>
+                            <div>Status changed by {{ status_changes|urlize_related_source_list|join:", " }}</div>
                         {% endif %}
                         {% if proposed_status_changes %}
-                            <div>Proposed status changed by {{ proposed_status_changes|join:", "|urlize_related_source_list }}</div>
+                            <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 %}