fix: repair many revision related issues with the main and htmlized views

Author: rjsparks

ietf/api/views.py

@@ -328,7 +328,7 @@ def rfcdiff_latest_json(request, name, rev=None):
     response = dict()
     condition, document, history, found_rev = find_doc_for_rfcdiff(name, rev)
     if document.type_id == "rfc":
-        draft = next(iter(document.related_that('became_rfc')), None)
+        draft = document.came_from_draft()
     if condition == 'no such document':
         raise Http404
     elif condition in ('historic version', 'current version'):

ietf/doc/factories.py

@@ -51,7 +51,11 @@ class Meta:
     def name(self, n):
         return draft_name_generator(self.type_id,self.group,n)
 
-    newrevisiondocevent = factory.RelatedFactory('ietf.doc.factories.NewRevisionDocEventFactory','doc')
+    @factory.post_generation
+    def newrevisiondocevent(obj, create, extracted, **kwargs): # pylint: disable=no-self-argument
+        if create:
+            if obj.type_id != "rfc":
+                NewRevisionDocEventFactory(doc=obj)
 
     @factory.post_generation
     def states(obj, create, extracted, **kwargs): # pylint: disable=no-self-argument
@@ -106,6 +110,7 @@ class DocumentFactory(BaseDocumentFactory):
 
 class RfcFactory(BaseDocumentFactory):
     type_id = "rfc"
+    rev = ""
     rfc_number = factory.Sequence(lambda n: n + 1000)
     name = factory.LazyAttribute(lambda o: f"rfc{o.rfc_number:d}")
     expires = None

ietf/doc/management/commands/generate_draft_aliases.py

@@ -130,7 +130,7 @@ def handle(self, *args, **options):
         for draft in interesting_drafts.distinct().iterator():
             # Omit drafts that became RFCs, unless they were published in the last DEFAULT_YEARS
             if draft.get_state_slug()=="rfc":
-                rfc = next(iter(draft.related_that_doc("became_rfc")), None)
+                rfc = draft.became_rfc()
                 log.assertion("rfc is not None")
                 if rfc.latest_event(type='published_rfc').time < show_since:
                     continue

ietf/doc/models.py

@@ -198,17 +198,28 @@ def get_file_name(self):
             self._cached_file_name = os.path.join(self.get_file_path(), self.get_base_name())
         return self._cached_file_name
 
-    def revisions(self):
+
+    def revisions_by_dochistory(self):
         revisions = []
-        doc = self.doc if isinstance(self, DocHistory) else self
-        for e in doc.docevent_set.filter(type='new_revision').distinct():
-            if e.rev and not e.rev in revisions:
-                revisions.append(e.rev)
-        if not doc.rev in revisions:
-            revisions.append(doc.rev)
-        revisions.sort()
+        if self.type_id != "rfc":
+            for h in self.history_set.order_by("time", "id"):
+                if h.rev and not h.rev in revisions:
+                    revisions.append(h.rev)
+            if not self.rev in revisions:
+                revisions.append(self.rev)
         return revisions
 
+    def revisions_by_newrevisionevent(self):
+        revisions = []
+        if self.type_id != "rfc":
+            doc = self.doc if isinstance(self, DocHistory) else self
+            for e in doc.docevent_set.filter(type='new_revision').distinct():
+                if e.rev and not e.rev in revisions:
+                    revisions.append(e.rev)
+            if not doc.rev in revisions:
+                revisions.append(doc.rev)
+            revisions.sort()
+        return revisions
 
     def get_href(self, meeting=None):
         return self._get_ref(meeting=meeting,meeting_doc_refs=settings.MEETING_DOC_HREFS)
@@ -654,6 +665,15 @@ def referenced_by_rfcs(self):
             source__type__slug="rfc"
         )
 
+    def became_rfc(self):
+        doc = self if isinstance(self, Document) else self.doc
+        return next(iter(doc.related_that_doc("became_rfc")), None)
+
+    def came_from_draft(self):
+        doc = self if isinstance(self, Document) else self.doc
+        return next(iter(doc.related_that("became_rfc")), None)
+
+
     class Meta:
         abstract = True
 

ietf/doc/tests.py

@@ -2883,3 +2883,30 @@ def test_gets_request_conflict_review_action_button(self):
         r = self.client.get(url)
         self.assertContains(r, target_string)
 
+class DocInfoMethodsTests(TestCase):
+
+    def test_became_rfc(self):
+        draft = WgDraftFactory()
+        rfc = WgRfcFactory()
+        draft.relateddocument_set.create(relationship_id="became_rfc",target=rfc)
+        self.assertEqual(draft.became_rfc(), rfc)
+        self.assertEqual(rfc.came_from_draft(), draft)
+
+        charter = CharterFactory()
+        self.assertIsNone(charter.became_rfc())
+        self.assertIsNone(charter.came_from_draft())
+
+    def test_revisions(self):
+        draft = WgDraftFactory(rev="09",create_revisions=range(0,10))
+        self.assertEqual(draft.revisions_by_dochistory(),[f"{i:02d}" for i in range(0,10)])
+        self.assertEqual(draft.revisions_by_newrevisionevent(),[f"{i:02d}" for i in range(0,10)])
+        rfc = WgRfcFactory()
+        self.assertEqual(rfc.revisions_by_newrevisionevent(),[])
+        self.assertEqual(rfc.revisions_by_dochistory(),[])
+
+        draft.history_set.filter(rev__lt="08").delete()
+        draft.docevent_set.filter(newrevisiondocevent__rev="05").delete()
+        self.assertEqual(draft.revisions_by_dochistory(),[f"{i:02d}" for i in range(8,10)])
+        self.assertEqual(draft.revisions_by_newrevisionevent(),[f"{i:02d}" for i in [*range(0,5), *range(6,10)]])      
+
+

ietf/doc/views_doc.py

@@ -197,29 +197,28 @@ def document_main(request, name, rev=None, document_html=False):
 
     # take care of possible redirections
     if document_html is False and rev is None:
-        became_rfc = next(iter(doc.related_that_doc("became_rfc")), None)
+        became_rfc = doc.became_rfc()
         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"):
-        if h.rev and not h.rev in revisions:
-            revisions.append(h.rev)
-    if not doc.rev in revisions:
-        revisions.append(doc.rev)
+
+    revisions = doc.revisions_by_dochistory()
     latest_rev = doc.rev
 
     snapshot = False
 
     gh = None
     if rev:
         # find the entry in the history
+        debug.show('doc.name')
         for h in doc.history_set.order_by("-time"):
             if rev == h.rev:
                 snapshot = True
                 doc = h
                 break
 
+        debug.show('doc.name')
+
+
         if not snapshot and document_html is False:
             return redirect('ietf.doc.views_doc.document_main', name=name)
 
@@ -239,7 +238,6 @@ def document_main(request, name, rev=None, document_html=False):
     if telechat and (not telechat.telechat_date or telechat.telechat_date < date_today(settings.TIME_ZONE)):
        telechat = None
 
-
     # specific document types
     if doc.type_id == "rfc":
         split_content = request.COOKIES.get("full_draft", settings.USER_PREFERENCE_DEFAULTS["full_draft"]) == "off"
@@ -302,7 +300,7 @@ def document_main(request, name, rev=None, document_html=False):
         diff_revisions = None
         simple_diff_revisions = None
         if document_html:
-            diff_revisions=get_diff_revisions(request, name, doc if isinstance(doc,Document) else doc.doc)
+            diff_revisions=get_diff_revisions(request, name, doc)
             simple_diff_revisions = [t[1] for t in diff_revisions if t[0] == doc.name]
             simple_diff_revisions.reverse()
             html = doc.html_body()
@@ -311,10 +309,8 @@ 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 = next(iter(doc.related_that("became_rfc")), None)
-        if became_rfc:
-            draft_that_became_rfc = became_rfc
+        draft_that_became_rfc = doc.came_from_draft()
+
         # submission
         submission = ""
         if group is None:
@@ -651,6 +647,7 @@ 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()
+        debug.show("doc.name")
 
         return render(request, "doc/document_draft.html" if document_html is False else "doc/document_html.html",
                                   dict(doc=doc,
@@ -1134,6 +1131,9 @@ def document_email(request,name):
 
 
 def get_diff_revisions(request, name, doc):
+    """ returns list of (name, rev, time, url, is_this_doc, is_previous_doc)
+        ordered by -time for use by forms used to get to the diff tools.
+    """
     diffable = any(
         [
             name.startswith(prefix)
@@ -1160,16 +1160,17 @@ def get_diff_revisions(request, name, doc):
             relateddocument__relationship="replaces",
         )
     )
-    diff_documents.extend(
-        Document.objects.filter(
-            relateddocument__target=doc,
-            relateddocument__relationship="became_rfc"
-        )
-    )
+    if doc.came_from_draft():
+        diff_documents.append(doc.came_from_draft())
+
+    if doc.became_rfc():
+        rfc = doc.became_rfc()
+        e = rfc.latest_event(type="published_rfc")
+        diff_revisions.append((rfc.name, "", e.time if e else rfc.time, rfc.name, False, False))
 
     if doc.type_id == "rfc":
         e = doc.latest_event(type="published_rfc")
-        diff_revisions.append((name, "", e.time if e else doc.time, name))
+        diff_revisions.append((name, "", e.time if e else doc.time, name, True, False))
 
     seen = set()
     for e in (
@@ -1198,7 +1199,16 @@ def get_diff_revisions(request, name, doc):
             # rfcdiff tool has special support for IDs
             url = e.doc.name + "-" + e.rev
 
-        diff_revisions.append((e.doc.name, e.rev, e.time, url))
+        diff_revisions.append((e.doc.name, e.rev, e.time, url, e.doc == doc and e.rev == doc.rev, False))
+    
+    diff_revisions.sort(key=lambda t: t[2], reverse=True)
+    for index, t in enumerate(diff_revisions):
+        if t[4]: # is_this_doc
+            n = index+1
+            if n < len(diff_revisions):
+                t_name, rev, time, url, _, _ = diff_revisions[n]
+                diff_revisions[n] = (t_name, rev, time, url, False, True)
+                break
 
     return diff_revisions
 
@@ -1280,7 +1290,7 @@ def document_bibtex(request, name, rev=None):
     if doc.type_id == "draft":
         latest_revision = doc.latest_event(NewRevisionDocEvent, type="new_revision")
         replaced_by = [d.name for d in doc.related_that("replaces")]
-        draft_became_rfc = next(iter(doc.related_that_doc("became_rfc")), None)
+        draft_became_rfc = doc.became_rfc()
 
         if rev != None and rev != doc.rev:
             # find the entry in the history
@@ -2170,7 +2180,7 @@ def idnits2_state(request, name, rev=None):
         raise Http404
     zero_revision = None
     if doc.type_id == "rfc":
-        draft = next(iter(doc.related_that('became_rfc')), None)
+        draft = doc.came_from_draft()
         if draft:
             zero_revision = NewRevisionDocEvent.objects.filter(doc=draft,rev='00').first()
     else:

ietf/idindex/index.py

@@ -33,7 +33,7 @@ def formatted_rev_date(name):
 
     rfcs = dict()
     for rfc in Document.objects.filter(type_id="rfc"):
-        draft = next(iter(rfc.related_that("became_rfc")), None)
+        draft = rfc.came_from_draft()
         if draft is not None:
             rfcs[draft.name] = rfc.name
 
@@ -117,7 +117,7 @@ def all_id2_txt():
 
     rfcs = dict()
     for rfc in Document.objects.filter(type_id="rfc"):
-        draft = next(iter(rfc.related_that("became_rfc")), None)
+        draft = rfc.came_from_draft()
         if draft is not None:
             rfcs[draft.name] = rfc.name
 

ietf/meeting/views.py

@@ -235,7 +235,7 @@ def _get_materials_doc(meeting, name):
         docname, rev = name.rsplit("-", 1)
         if len(rev) == 2 and rev.isdigit():
             doc = Document.objects.get(name=docname)  # may raise Document.DoesNotExist
-            if doc.get_related_meeting() == meeting and rev in doc.revisions():
+            if doc.get_related_meeting() == meeting and rev in doc.revisions_by_newrevisionevent():
                 return doc, rev
     # give up
     raise Document.DoesNotExist

ietf/templates/doc/document_history_form.html

@@ -11,34 +11,22 @@
         <label for="url1" class="col-form-label col-sm-2 fw-bold">From revision</label>
         <div class="col-sm-10">
 {% endif %}
-{% with prev_rev=doc.rev|add:"-1"|stringformat:"02d" %}
             <select class="form-select{% if document_html %} form-select-sm mb-1{% endif %} select2-field"
                     data-max-entries="1"
                     data-width="resolve"
                     data-allow-clear="false"
                     data-minimum-input-length="0"
                     {% if not document_html %}id="url1"{% else %}aria-label="From revision"{% endif %}
                     name="url1">
-                {% for name, rev, time, url in diff_revisions %}
-                    <option value="{{ url }}"
-                            {% if document_html %}
-                                {% if name == doc.name %}
-                                    {% if rev == prev_rev or doc.rev == "00" and rev == "00" %} selected{% endif %}
-                                {% endif %}
-                            {% else %}
-                                {% if diff_revisions|length > 1 and forloop.counter == 2 %} selected{% endif %}
-                            {% endif %}>
+                {% for name, rev, time, url, is_this_doc, is_previous_doc in diff_revisions %}
+                    <option value="{{ url }}"{% if is_previous_doc %} selected{% endif %}>
                         {{ name|prettystdname }}{% if rev %}-{{ rev }}{% endif %}
                         {% if not document_html %}({{ time|date:"Y-m-d" }}){% endif %}
                     </option>
                 {% endfor %}
-                {% if doc.name|rfcbis %}
+                {% if doc.name|rfcbis %} {# TODO - move this into the diffable versions logic #}
                     <option value="{{ doc.name|rfcbis }}"
-                            {% if document_html %}
-                                {% if rev == prev_rev %} selected{% endif %}
-                            {% else %}
                                 {% if diff_revisions and diff_revisions|length == 1 %} selected{% endif %}
-                            {% endif %}>
                         {{ doc.name|rfcbis|prettystdname }}
                     </option>
                 {% endif %}
@@ -57,13 +45,8 @@
                     data-minimum-input-length="0"
                     {% if not document_html %}id="url2"{% else %}aria-label="To revision"{% endif %}
                     name="url2">
-                {% for name, rev, time, url in diff_revisions %}
-                    <option value="{{ url }}"
-                        {% if document_html %}
-                            {% if not snapshot and doc.get_state_slug == "rfc" and name == "rfc"|add:doc.rfc_number or snapshot and name == doc.name and rev == doc.rev %} selected{% endif %}
-                        {% else %}
-                            {% if forloop.counter == 1 %} selected{% endif %}
-                        {% endif %}>
+                {% for name, rev, time, url, is_this_doc, is_previous_doc in diff_revisions %}
+                    <option value="{{ url }}"{% if is_this_doc %} selected{%endif%}>
                         {{ name|prettystdname }}{% if rev %}-{{ rev }}{% endif %}
                         {% if not document_html %}({{ time|date:"Y-m-d" }}){% endif %}
                     </option>
@@ -107,7 +90,6 @@
                     name="difftype">
                 Inline
             </button>
-{% endwith %}
 {% if not document_html %}
         </div>
     </div>

ietf/templates/doc/document_html.html

@@ -33,8 +33,8 @@
         {% endif %}
         <link rel="alternate" type="application/atom+xml" title="Document changes" href="/feed/document-changes/{{ doc.name }}/">
         <meta name="description"
-            {% if not snapshot and doc.get_state_slug == 'rfc' %}
-                content="{{ doc.title }} (RFC {{ doc.rfc_number }}{% if published %}, {{ published.time|date:'F Y' }}{% endif %}{% if obsoleted_by %}; obsoleted by {% for rel in obsoleted_by %}{{ rel.source.name|prettystdname}}{% if not forloop.last%}, {% endif %}{% endfor %}{% endif %})"
+            {% if doc.type_id == "rfc" %}
+                content="{{ doc.title }} (RFC {{ doc.rfc_number }}, {{ published.time|date:'F Y' }}{% if obsoleted_by %}; obsoleted by {% for rel in obsoleted_by %}{{ rel.source.name|prettystdname}}{% if not forloop.last%}, {% endif %}{% endfor %}{% endif %})"
             {% else %}
                 content="{{ doc.title }} (Internet-Draft, {{ doc.time|date:'Y' }})"
             {% endif %}>
@@ -98,9 +98,9 @@
         <nav class="navbar bg-light-subtle px-1 fixed-top d-print-none d-md-none">
             <a class="nav-link ps-1"
                href="{% url 'ietf.doc.views_doc.document_main' name=doc.name %}">
-                {% if not snapshot and doc.get_state_slug == "rfc" %}
+                {% if doc.type_id == "rfc" %}
                     RFC {{ doc.rfc_number }}
-                {% else %}
+                {% else %} 
                     {{ doc.name }}-{{ doc.rev }}
                 {% endif %}
                 <br class="d-sm-none">
@@ -175,7 +175,7 @@
                     <div class="pt-2 pt-lg-3 px-md-2 px-lg-3">
                         <a class="btn btn-primary btn-sm" href="{% url 'ietf.doc.views_doc.document_main' name=doc.name %}">Datatracker</a>
                         <p class="fw-bold pt-2">
-                                {% if not snapshot and doc.get_state_slug == "rfc" %}
+                                {% if doc.type_id == "rfc" %}
                                     RFC {{ doc.rfc_number }}
                                 {% else %}
                                     {{ doc.name }}-{{ doc.rev }}