Skip to content

Reverted to using difflab for revisions comparsions [redux] #4610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wes-otf merged 4 commits into from
Aug 13, 2025
Merged

Reverted to using difflab for revisions comparsions [redux] #4610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0aba09c
Reverted the revisions compare view to side by side, using difflab
wes-otf Aug 11, 2025
df1a049
Fixed regex flaws
wes-otf Aug 11, 2025
a67034e
Fixed timestamp bug & made latest revision uncomparable
wes-otf Aug 12, 2025
8251f77
Handle unintended `h2` element formatting
wes-otf Aug 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions hypha/apply/funds/differ.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import re
from difflib import SequenceMatcher
from typing import Tuple

import nh3
from django.utils.html import format_html
from django.utils.safestring import mark_safe


def wrap_deleted(text):
return format_html("<del>{}</del>", mark_safe(text))


def wrap_added(text):
return format_html("<ins>{}</ins>", mark_safe(text))


def compare(answer_a: str, answer_b: str, should_clean: bool = True) -> Tuple[str, str]:
"""Compare two strings, populate diff HTML and insert it, and return a tuple of the given strings.

Args:
answer_a:
The original string
answer_b:
The string to compare to the original
should_clean:
Optional boolean to determine if the string should be sanitized with NH3 (default=True)

Returns:
A tuple of the original strings with diff HTML inserted.
"""

if should_clean:
answer_a = re.sub("(<li[^>]*>)", r"\1◦ ", answer_a)
answer_b = re.sub("(<li[^>]*>)", r"\1◦ ", answer_b)
answer_a = nh3.clean(answer_a, tags=set(), attributes={})
answer_b = nh3.clean(answer_b, tags=set(), attributes={})

diff = SequenceMatcher(None, answer_a, answer_b)
from_diff = []
to_diff = []
for opcode, a0, a1, b0, b1 in diff.get_opcodes():
if opcode == "equal":
from_diff.append(mark_safe(diff.a[a0:a1]))
to_diff.append(mark_safe(diff.b[b0:b1]))
elif opcode == "insert":
from_diff.append(mark_safe(diff.a[a0:a1]))
to_diff.append(wrap_added(diff.b[b0:b1]))
elif opcode == "delete":
from_diff.append(wrap_deleted(diff.a[a0:a1]))
to_diff.append(mark_safe(diff.b[b0:b1]))
elif opcode == "replace":
from_diff.append(wrap_deleted(diff.a[a0:a1]))
to_diff.append(wrap_added(diff.b[b0:b1]))

from_display = "".join(from_diff)

to_display = "".join(to_diff)
from_display = re.sub("(\\.\n)", r"\1<br><br>", from_display)
to_display = re.sub("(\\.\n)", r"\1<br><br>", to_display)
from_display = re.sub(r"([◦])", r"<br>\1", from_display)
to_display = re.sub(r"([◦])", r"<br>\1", to_display)
from_display = mark_safe(from_display)
to_display = mark_safe(to_display)

return (from_display, to_display)
175 changes: 98 additions & 77 deletions hypha/apply/funds/templates/funds/revisions_compare.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,113 +13,134 @@
{% endblock %}

{% block content %}
<div class="my-4 mx-auto layout layout-flowrow-until-md layout-sidebar-flowrow-start">

<article class="layout-main">
<div class="my-4 layout layout-flowrow-until-md layout-sidebar-flowrow-start">
<div class="layout-main">
<div class="mb-4">
<h2 class="section-header">{% trans "Changes" %}</h2>
<div class="flex flex-wrap gap-2">
<span class="badge badge-soft">{{ from_revision.timestamp|date:"SHORT_DATETIME_FORMAT" }}</span>
<span class="badge badge-soft">
<relative-time format='datetime' datetime="{{ from_revision.timestamp|date:'c' }}">
{{ from_revision.timestamp|date:'SHORT_DATETIME_FORMAT' }}
</relative-time>
</span>

{% if to_revision.id != from_revision.id %}
{% trans "↔" %}
<span class="badge badge-soft">{{ to_revision.timestamp|date:"SHORT_DATETIME_FORMAT" }}</span>
<span class="badge badge-soft">
<relative-time format='datetime' precision='second' datetime="{{ to_revision.timestamp|date:'c' }}">
{{ to_revision.timestamp|date:'SHORT_DATETIME_FORMAT' }}
</relative-time>
</span>
{% endif %}
</div>
</div>
<table class="px-4 pb-4 w-full max-w-none prose card card-border prose-h2:mt-0 prose-h2:text-lg html-diff">

<div class="card card-border">
<div class="gap-8 max-w-full card-body html-diff">
{% for diff in required_fields %}
{% if forloop.first %}
<section>
<h4 class="pb-1 mb-2 font-medium border-b text-h3 border-base-300 question">{% trans "Title" %}</h4>
<div>{{ diff }}</div>
</section>
{% elif forloop.counter == 2 %}
<section>
<h4 class="pb-1 mb-2 font-medium border-b text-h3 border-base-300 question">{% trans "Legal Name" %}</h4>
<div>{{ diff }}</div>
</section>
{% elif forloop.counter == 3 %}
<section>
<h4 class="pb-1 mb-2 font-medium border-b text-h3 border-base-300 question">{% trans "E-mail" %}</h4>
<div>{{ diff }}</div>
</section>
{% elif forloop.counter == 4 %}
<section>
<h4 class="pb-1 mb-2 font-medium border-b text-h3 border-base-300 question">{% trans "Address" %}</h4>
<div>{{ diff }}</div>
</section>
{% elif forloop.counter == 5 %}
<section>
<h4 class="pb-1 mb-2 font-medium border-b text-h3 border-base-300 question">{% trans "Project Duration" %}</h4>
<div>{{ diff }}</div>
</section>
{% elif forloop.counter == 6 %}
<section>
<h4 class="pb-1 mb-2 font-medium border-b text-h3 border-base-300 question">{% trans "Requested Funding" %}</h4>
<div>{{ diff }}</div>
</section>
{% elif forloop.counter == 7 %}
<section>
<h4 class="pb-1 mb-2 font-medium border-b text-h3 border-base-300 question">{% trans "Organization" %}</h4>
<div>{{ diff }}</div>
</section>
{% else %}
<section>
<div>{{ diff }}</div>
</section>
{% endif %}
{% endfor %}
{% for diff in stream_fields %}
{{ diff }}
{% endfor %}
</div>
</div>
</article>
{% for from_field, to_field in required_fields %}
{% if forloop.first %}
<tr>
<td><h2>{% trans "Title" %}</h2>{{ from_field }}</td>
<td><h2>{% trans "Title" %}</h2>{{ to_field }}</td>
</tr>
{% elif forloop.counter == 2 %}
<tr>
<td><h2>{% trans "Legal Name" %}</h2>{{ from_field }}</td>
<td><h2>{% trans "Legal Name" %}</h2>{{ to_field }}</td>
</tr>
{% elif forloop.counter == 3 %}
<tr>
<td><h2>{% trans "E-mail" %}</h2>{{ from_field }}</td>
<td><h2>{% trans "E-mail" %}</h2>{{ to_field }}</td>
</tr>
{% elif forloop.counter == 4 %}
<tr>
<td><h2>{% trans "Address" %}</h2>{{ from_field }}</td>
<td><h2>{% trans "Address" %}</h2>{{ to_field }}</td>
</tr>
{% elif forloop.counter == 5 %}
<tr>
<td><h2>{% trans "Project Duration" %}</h2>{{ from_field }}</td>
<td><h2>{% trans "Project Duration" %}</h2>{{ to_field }}</td>
</tr>
{% elif forloop.counter == 6 %}
<tr>
<td><h2>{% trans "Requested Funding" %}</h2>{{ from_field }}</td>
<td><h2>{% trans "Requested Funding" %}</h2>{{ to_field }}</td>
</tr>
{% else %}
<tr>
<td>{{ from_field }}</td>
<td>{{ to_field }}</td>
</tr>
{% endif %}
{% endfor %}
{% for from_field, to_field in stream_fields %}
<tr>
<td>{{ from_field }}</td>
<td>{{ to_field }}</td>
</tr>
{% endfor %}
</table>
</div>

<aside class="layout-sidebar">
<div class="sticky top-4">
<h2 class="mb-4 card-title">{% trans "Revisions" %}</h2>
<div class="list">
{% for revision in all_revisions %}
<a
class="list-row {% if revision.id == from_revision.id %} bg-base-300 {% else %}hover:bg-base-200{% endif %}"
href="{{ revision.get_compare_url_to_latest }}"
>
<div class="list-col-grow">
<span class="font-semibold">{{ revision.author }}</span> {% trans "edited" %}
{% if forloop.first %}
<div class="list-row">
<div class="list-col-grow">
<span class="font-semibold">{{ revision.author }}</span> {% trans "edited" %}

<relative-time datetime={{ revision.timestamp|date:"c" }} class="text-fg-muted">
{{ revision.timestamp|date:"SHORT_DATETIME_FORMAT" }}
</relative-time>
<relative-time datetime={{ revision.timestamp|date:"c" }} class="text-fg-muted">
{{ revision.timestamp|date:"SHORT_DATETIME_FORMAT" }}
</relative-time>

{% if revision.is_draft %}
<span class="uppercase badge badge-warning badge-outline">
({% trans "draft" %})
</span>
{% endif %}
</div>
{% if revision.is_draft %}
<span class="uppercase badge badge-warning badge-outline">
({% trans "draft" %})
</span>
{% endif %}
</div>

<div>
{% if forloop.first %}
<div>
<span class="uppercase badge badge-info badge-outline">
{% trans "latest" %}
</span>
{% else %}
{% if revision.id != from_revision.id %}
</div>
</div>
{% else %}
<a
class="list-row {% if revision.id == from_revision.id %} bg-base-300 {% else %}hover:bg-base-200{% endif %}"
href="{{ revision.get_compare_url_to_latest }}"
>
<div class="list-col-grow">
<span class="font-semibold">{{ revision.author }}</span> {% trans "edited" %}

<relative-time datetime={{ revision.timestamp|date:"c" }} class="text-fg-muted">
{{ revision.timestamp|date:"SHORT_DATETIME_FORMAT" }}
</relative-time>

{% if revision.is_draft %}
<span class="uppercase badge badge-warning badge-outline">
({% trans "draft" %})
</span>
{% endif %}
</div>
{% if revision.id != from_revision.id %}
<div>
<span
class="btn btn-sm"
href="{{ revision.get_compare_url_to_latest }}"
>
{% trans "view" %}
{% heroicon_mini "arrow-right" class="size-4" %}
</span>
{% endif %}
</div>
{% endif %}
</div>
</a>
</a>
{% endif %}
{% endfor %}
</div>
</div>
Expand Down
49 changes: 44 additions & 5 deletions hypha/apply/funds/views/revisions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import html_diff
import re
from typing import List

import nh3
from django.shortcuts import get_object_or_404
from django.utils.decorators import method_decorator
from django.utils.html import format_html
from django.views.generic import (
DetailView,
ListView,
Expand All @@ -11,6 +13,7 @@
staff_required,
)

from ..differ import compare
from ..models import (
ApplicationRevision,
ApplicationSubmission,
Expand Down Expand Up @@ -96,12 +99,11 @@ def compare_revisions(self, from_data, to_data):
to_required = self.render_required()

required_fields = [
format_html(html_diff.diff(*fields))
for fields in zip(from_required, to_required, strict=False)
compare(*fields) for fields in zip(from_required, to_required, strict=False)
]

stream_fields = [
format_html(html_diff.diff(*fields))
compare(*self.cleanse_stream_fields(*fields), should_clean=False)
for fields in zip(
from_rendered_text_fields, to_rendered_text_fields, strict=False
)
Expand Down Expand Up @@ -135,3 +137,40 @@ def get_context_data(self, **kwargs):
"stream_fields": stream_fields,
}
return super().get_context_data(**ctx, **kwargs)

def cleanse_stream_fields(self, a_field, b_field) -> List[str]:
"""Sanitizes the HTML outside of the h2 heading
This is a temp fix and we should move to full HTML diffing

Args:
a_field: the field to sanitize
b_field: the field to sanitize

Returns:
The sanitized stream field answers in a list
"""

sanitized_answers = []

for field in (a_field, b_field):
# TODO: Using regex with HTML is not ideal but this temp until we move to xml parsing
field_match = re.match(
r"^\s*<section class=\".*\">\s*(<h2 class=\".*\">[\s\S]*?</h2>)([\s\S]*?)</section>",
field,
)
try:
# Keep h2 tags but purge any classes/attributes
heading = nh3.clean(field_match.group(1), tags={"h2"}, attributes={})

# Handle lists on the answer fields by subbing HTML for chars
answer = re.sub("(<li[^>]*>)", r"\1◦ ", field_match.group(2))
# Cleanse answer of HTML
answer = nh3.clean(answer, attributes={}, tags=set())

sanitized_answers.append(f"{heading}{answer}")
except AttributeError:
# If it fails to match for some reason just cleanse the fields but leave h2s
answer = nh3.clean(answer, attributes={}, tags={"h2"})
sanitized_answers.append(field)

return sanitized_answers
9 changes: 5 additions & 4 deletions hypha/static_src/tailwind/components/html-diff.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
--marker-ms: -10px;
--marker-ps: 8px;

:is(h1, h2, h3, h4, h5, h6):has(ins),
/* :is(h1, h2, h3, h4, h5, h6):has(ins),
:is(h1, h2, h3, h4, h5, h6):has(del),
tr:has(ins),
tr:has(del),
ul:has(ins),
ul:has(del),
ol:has(ins),
Expand All @@ -14,7 +16,7 @@
border-inline-start: var(--marker-border-width) solid var(--color-warning);
margin-inline-start: var(--marker-ms);
padding-inline-start: var(--marker-ps);
}
} */

table,
.prose {
Expand Down Expand Up @@ -42,8 +44,7 @@
border-inline-start: var(--marker-border-width) solid var(--color-success);
}

del,
del * {
del {
@apply bg-error text-error-content;
}

Expand Down
Loading