feat: Diff arbitrary versions from new HTMLization page (#4863)

* feat: Diff arbitrary versions from new HTMLization page

Fixes #4859

* Rework this based on @rjsparks' suggestion. Not quite done yet.

* Progress

* Fix HTML

* Don't show compare buttons if there aren't at least two versions

* Remove spurious title attribute

* Use and style select2 for the version diff dropdowns

* Roll in code review suggestions

* Some tests!

ietf/doc/tests.py

@@ -744,6 +744,12 @@ Man                    Expires September 22, 2015               [Page 3]
         q = PyQuery(r.content)
         self.assertEqual(q('title').text(), 'draft-ietf-mars-test-01')
 
+        # check that revision list has expected versions
+        self.assertEqual(len(q('#sidebar .revision-list .page-item.active a.page-link[href$="draft-ietf-mars-test-01"]')), 1)
+
+        # check that diff dropdowns have expected versions
+        self.assertEqual(len(q('#sidebar option[value="draft-ietf-mars-test-00"][selected="selected"]')), 1)
+
         rfc = WgRfcFactory()
         (Path(settings.RFC_PATH) / rfc.get_base_name()).touch()
         r = self.client.get(urlreverse("ietf.doc.views_doc.document_html", kwargs=dict(name=rfc.canonical_name())))

ietf/doc/views_doc.py

@@ -533,6 +533,7 @@ def document_main(request, name, rev=None, document_html=False):
                                        review_assignments=review_assignments,
                                        no_review_from_teams=no_review_from_teams,
                                        due_date=due_date,
+                                       diff_revisions=get_diff_revisions(request, name, doc if isinstance(doc,Document) else doc.doc) if document_html else None
                                        ))
 
     if doc.type_id == "charter":
@@ -901,44 +902,77 @@ def document_email(request,name):
                  )
 
 
-def document_history(request, name):
-    doc = get_object_or_404(Document, docalias__name=name)
-    top = render_document_top(request, doc, "history", name)
+def get_diff_revisions(request, name, doc):
+    diffable = any(
+        [
+            name.startswith(prefix)
+            for prefix in [
+                "rfc",
+                "draft",
+                "charter",
+                "conflict-review",
+                "status-change",
+            ]
+        ]
+    )
+
+    if not diffable:
+        return []
 
     # pick up revisions from events
     diff_revisions = []
 
-    diffable = [ name.startswith(prefix) for prefix in ["rfc", "draft", "charter", "conflict-review", "status-change", ]]
-    if any(diffable):
-        diff_documents = [ doc ]
-        diff_documents.extend(Document.objects.filter(docalias__relateddocument__source=doc, docalias__relateddocument__relationship="replaces"))
+    diff_documents = [doc]
+    diff_documents.extend(
+        Document.objects.filter(
+            docalias__relateddocument__source=doc,
+            docalias__relateddocument__relationship="replaces",
+        )
+    )
 
-        if doc.get_state_slug() == "rfc":
-            e = doc.latest_event(type="published_rfc")
-            aliases = doc.docalias.filter(name__startswith="rfc")
-            if aliases:
-                name = aliases[0].name
-            diff_revisions.append((name, "", e.time if e else doc.time, name))
+    if doc.get_state_slug() == "rfc":
+        e = doc.latest_event(type="published_rfc")
+        aliases = doc.docalias.filter(name__startswith="rfc")
+        if aliases:
+            name = aliases[0].name
+        diff_revisions.append((name, "", e.time if e else doc.time, name))
 
-        seen = set()
-        for e in NewRevisionDocEvent.objects.filter(type="new_revision", doc__in=diff_documents).select_related('doc').order_by("-time", "-id"):
-            if (e.doc.name, e.rev) in seen:
-                continue
+    seen = set()
+    for e in (
+        NewRevisionDocEvent.objects.filter(type="new_revision", doc__in=diff_documents)
+        .select_related("doc")
+        .order_by("-time", "-id")
+    ):
+        if (e.doc.name, e.rev) in seen:
+            continue
 
-            seen.add((e.doc.name, e.rev))
+        seen.add((e.doc.name, e.rev))
 
-            url = ""
-            if name.startswith("charter"):
-                url = request.build_absolute_uri(urlreverse('ietf.doc.views_charter.charter_with_milestones_txt', kwargs=dict(name=e.doc.name, rev=e.rev)))
-            elif name.startswith("conflict-review"):
-                url = find_history_active_at(e.doc, e.time).get_href()
-            elif name.startswith("status-change"):
-                url = find_history_active_at(e.doc, e.time).get_href()
-            elif name.startswith("draft") or name.startswith("rfc"):
-                # rfcdiff tool has special support for IDs
-                url = e.doc.name + "-" + e.rev
+        url = ""
+        if name.startswith("charter"):
+            url = request.build_absolute_uri(
+                urlreverse(
+                    "ietf.doc.views_charter.charter_with_milestones_txt",
+                    kwargs=dict(name=e.doc.name, rev=e.rev),
+                )
+            )
+        elif name.startswith("conflict-review"):
+            url = find_history_active_at(e.doc, e.time).get_href()
+        elif name.startswith("status-change"):
+            url = find_history_active_at(e.doc, e.time).get_href()
+        elif name.startswith("draft") or name.startswith("rfc"):
+            # 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))
+
+    return diff_revisions
+
+
+def document_history(request, name):
+    doc = get_object_or_404(Document, docalias__name=name)
+    top = render_document_top(request, doc, "history", name)
+    diff_revisions = get_diff_revisions(request, name, doc)
 
     # grab event history
     events = doc.docevent_set.all().order_by("-time", "-id").select_related("by")

ietf/static/css/document_html.scss

@@ -206,6 +206,15 @@ tbody.meta tr {
     }
 }
 
+.navbar {
+
+    td:not(:first-child),
+    th:not(:first-child) {
+        padding-top: map.get($spacers, 3);
+    }
+
+}
+
 // Add some padding when there are multiple buttons in a line that can wrap
 .buttonlist .btn {
     margin-bottom: map.get($spacers, 1);
@@ -318,3 +327,15 @@ tbody.meta tr {
         display: none;
     }
 }
+
+// Select2 styling
+@import "select2";
+
+.select2-results__option,
+.select2-search__field {
+    font-size: small !important;
+}
+
+.select2-container--open {
+    z-index: 9999999;
+}

ietf/static/js/document_html.js

@@ -8,6 +8,7 @@ import {
 
 import Cookies from "js-cookie";
 import { populate_nav } from "./nav.js";
+import "./select2.js";
 
 const cookies = Cookies.withAttributes({ sameSite: "strict" });
 
@@ -48,7 +49,7 @@ document.addEventListener("DOMContentLoaded", function (event) {
 
     // activate pref buttons selected by pref cookies or localStorage
     const in_localStorage = ["deftab"];
-    document.querySelectorAll(".btn-check")
+    document.querySelectorAll("#pref-tab-pane .btn-check")
         .forEach(btn => {
             const id = btn.id.replace("-radio", "");
 

ietf/static/js/select2.js

@@ -109,4 +109,11 @@ $(document)
                     return;
                 setupSelect2Field($(this));
             });
+
+        // Remove spurious title attribute (https://github.com/select2/select2/pull/3988)
+        $(".select2-selection__rendered")
+            .hover(function () {
+                $(this)
+                    .removeAttr("title");
+            });
     });

ietf/templates/doc/document_history.html

@@ -18,85 +18,7 @@
     {{ top|safe }}
     {% if diff_revisions and diff_revisions|length > 1 or doc.name|rfcbis %}
         <h2 class="my-3">Revision differences</h2>
-        <form class="form-horizontal diff-form"
-              action="{{ rfcdiff_base_url }}"
-              method="get"
-              target="_blank">
-            <div class="row mb-3">
-                <label for="url1" class="col-form-label col-sm-2 fw-bold">From revision</label>
-                <div class="col-sm-10">
-                    <select class="form-select select2-field" data-max-entries="1" data-minimum-input-length="0" id="url1" name="url1">
-                        {% for name, rev, time, url in diff_revisions %}
-                            <option value="{{ url }}"
-                                    {% if diff_revisions|length > 1 and forloop.counter == 2 %} selected="selected"{% endif %}>
-                                {{ name }}
-                                {% if rev %}-{{ rev }}{% endif %}
-                                ({{ time|date:"Y-m-d" }})
-                            </option>
-                        {% endfor %}
-                        {% if doc.name|rfcbis %}
-                            <option value="{{ doc.name|rfcbis }}"
-                                    {% if diff_revisions and diff_revisions|length == 1 %} selected="selected"{% endif %}>
-                                {{ doc.name|rfcbis }}
-                            </option>
-                        {% endif %}
-                    </select>
-                </div>
-            </div>
-            <div class="row mb-3">
-                <label for="url2" class="col-form-label col-sm-2 fw-bold">To revision</label>
-                <div class="col-sm-10">
-                    <select class="form-select select2-field" data-max-entries="1" data-minimum-input-length="0" id="url2" name="url2">
-                        {% for name, rev, time, url in diff_revisions %}
-                            <option value="{{ url }}"
-                                    {% if forloop.counter == 1 %} selected="selected"{% endif %}>
-                                {{ name }}
-                                {% if rev %}-{{ rev }}{% endif %}
-                                ({{ time|date:"Y-m-d" }})
-                            </option>
-                        {% endfor %}
-                        {% if doc.name|rfcbis %}
-                            <option value="{{ doc.name|rfcbis }}">
-                                {{ doc.name|rfcbis }}
-                            </option>
-                        {% endif %}
-                    </select>
-                </div>
-            </div>
-            <div class="row mb-3">
-                <label class="col-form-label col-sm-2 fw-bold">Diff format</label>
-                <div class="col-sm-10">
-                    <div class="btn-group" data-bs-toggle="buttons">
-                        <input type="radio"
-                               class="btn-check"
-                               checked
-                               name="difftype"
-                               value="--html"
-                               id="html">
-                        <label for="html" class="btn btn-outline-primary">Side-by-side</label>
-                        <input type="radio"
-                               class="btn-check"
-                               name="difftype"
-                               value="--abdiff"
-                               id="abdiff">
-                        <label for="abdiff" class="btn btn-outline-primary">Before-after</label>
-                        <input type="radio"
-                               class="btn-check"
-                               name="difftype"
-                               value="--chbars"
-                               id="chbars">
-                        <label for="chbars" class="btn btn-outline-primary">Change bars</label>
-                        <input type="radio"
-                               class="btn-check"
-                               name="difftype"
-                               value="--hwdiff"
-                               id="hwdiff">
-                        <label for="hwdiff" class="btn btn-outline-primary">Wdiff</label>
-                    </div>
-                </div>
-            </div>
-            <button type="submit" class="btn btn-primary mb-3">Submit</button>
-        </form>
+        {% include "doc/document_history_form.html" with doc=doc diff_revisions=diff_revisions action=rfcdiff_base_url only %}
     {% endif %}
     <h2 class="my-3">Document history</h2>
     {% if can_add_comment %}

ietf/templates/doc/document_history_form.html

@@ -0,0 +1,97 @@
+{# Copyright The IETF Trust 2015-2022, All Rights Reserved #}
+{% load origin %}
+{% load ietf_filters %}
+{% origin %}
+<form class="form-horizontal diff-form"
+      action="{{ action }}"
+      method="get"
+      target="_blank">
+{% if not document_html %}
+    <div class="row mb-3">
+        <label for="url1" class="col-form-label col-sm-2 fw-bold">From revision</label>
+        <div class="col-sm-10">
+{% endif %}
+            <select class="form-select{% if document_html %} form-select-sm mb-1{% endif %} select2-field"
+                    data-max-entries="1"
+                    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 diff_revisions|length > 1 and forloop.counter == 2 %} selected="selected"{% endif %}>
+                        {{ name|prettystdname }}{% if rev %}-{{ rev }}{% endif %}
+                        {% if not document_html %}({{ time|date:"Y-m-d" }}){% endif %}
+                    </option>
+                {% endfor %}
+                {% if doc.name|rfcbis %}
+                    <option value="{{ doc.name|rfcbis }}"
+                            {% if diff_revisions and diff_revisions|length == 1 %} selected="selected"{% endif %}>
+                        {{ doc.name|rfcbis|prettystdname }}
+                    </option>
+                {% endif %}
+            </select>
+{% if not document_html %}
+        </div>
+    </div>
+    <div class="row mb-3">
+        <label for="url2" class="col-form-label col-sm-2 fw-bold">To revision</label>
+        <div class="col-sm-10">
+{% endif %}
+            <select class="form-select{% if document_html %} form-select-sm mb-1{% endif %} select2-field"
+                    data-max-entries="1"
+                    data-allow-clear="false"
+                    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 forloop.counter == 1 %} selected="selected"{% endif %}>
+                        {{ name|prettystdname }}{% if rev %}-{{ rev }}{% endif %}
+                        {% if not document_html %}({{ time|date:"Y-m-d" }}){% endif %}
+                    </option>
+                {% endfor %}
+                {% if doc.name|rfcbis %}
+                    <option value="{{ doc.name|rfcbis }}">
+                        {{ doc.name|rfcbis|prettystdname }}
+                    </option>
+                {% endif %}
+            </select>
+{% if not document_html %}
+        </div>
+    </div>
+    <div class="row mb-3">
+        <label class="col-form-label col-sm-2 fw-bold">Diff format</label>
+        <div class="col-sm-10">
+{% endif %}
+            <button type="submit"
+                    class="btn btn-primary{% if document_html %} btn-sm{% endif %}"
+                    value="--html"
+                    name="difftype">
+                Side-by-side
+            </button>
+            {% if not document_html %}
+                <button type="submit"
+                        class="btn btn-primary{% if document_html %} btn-sm{% endif %}"
+                        value="--abdiff"
+                        name="difftype">
+                    Before-after
+                </button>
+                <button type="submit"
+                        class="btn btn-primary{% if document_html %} btn-sm{% endif %}"
+                        value="--chbars"
+                        name="difftype">
+                    Change bars
+                </button>
+            {% endif %}
+            <button type="submit"
+                    class="btn btn-primary{% if document_html %} btn-sm{% endif %}"
+                    value="--hwdiff"
+                    name="difftype">
+                Inline
+            </button>
+{% if not document_html %}
+        </div>
+    </div>
+{% endif %}
+</form>

ietf/templates/doc/document_info.html

@@ -79,14 +79,13 @@
                 {% include "doc/revisions_list.html" with document_html=document_html %}
             </td>
         </tr>
-        {% if doc.rev != "00" %}
+        {% if diff_revisions|length > 1 %}
             <tr>
                 <td></td>
                 <th scope="row">Compare versions</th>
                 <td class="edit"></td>
                 <td>
-                    <a class="btn btn-primary btn-sm" href="{{ settings.RFCDIFF_BASE_URL }}?difftype=--hwdiff&amp;url2={{ doc.name }}-{{ doc.rev }}.txt" title="Inline diff (wdiff)">Inline</a>
-                    <a class="btn btn-primary btn-sm" href="{{ settings.RFCDIFF_BASE_URL }}?url2={{ doc.name }}-{{ doc.rev }}.txt" title="Side-by-side diff">Side-by-side</a>
+                    {% include "doc/document_history_form.html" with doc=doc diff_revisions=diff_revisions action=rfcdiff_base_url document_html=document_html only %}
                 </td>
             </tr>
         {% endif %}