mirror of
https://github.com/wahyd4/links.git
synced 2026-08-09 05:06:16 +10:00
fix(links): custom link edits spuriously logged URL changes
When editing a CUSTOM link's text content, the ViewUpdateView's
URL-change check was comparing the captured original_url (e.g.
'/custom/mynote') against form.cleaned_data['original_url'], which
for custom links is blank because the original_url input is hidden in
link_form.html and not submitted. They never matched, so every edit
produced a spurious LinkChangeLog 'URL changed' row.
Use the recomputed /custom/{alias} value (already set on
form.instance.original_url) for custom links, and only the submitted
form value for regular links. So the log now fires only when:
- a regular link's URL actually changes, or
- a custom link's alias changes (which derives a new /custom/{alias}).
Add TestCustomLinkEditUrlChangeLog covering:
- custom link text-edit produces no URL-change log
- regular link edit leaving URL unchanged produces no log
- regular link URL change logs once with old/new URL
- custom link alias change logs once with old/new derived URL
This commit is contained in:
+9
-2
@@ -198,8 +198,15 @@ class LinkUpdateView(UpdateView):
|
||||
# Invalidate the alias cache so the redirect hot-path sees the updated link
|
||||
cache.delete(f"link:alias:{form.instance.alias.lower()}")
|
||||
|
||||
new_url = form.cleaned_data['original_url']
|
||||
if self.original_url != new_url:
|
||||
# Compare against the URL that will actually be persisted. For custom
|
||||
# links the original_url form field is hidden (it's derived from the
|
||||
# alias), so use the recomputed value instead of the blank submitted
|
||||
# one — otherwise every edit logs a spurious "URL changed".
|
||||
if form.cleaned_data.get('link_type') == Link.LinkType.CUSTOM:
|
||||
new_url = f'/custom/{alias}'
|
||||
else:
|
||||
new_url = form.cleaned_data.get('original_url')
|
||||
if new_url is not None and self.original_url != new_url:
|
||||
logger.info(f"URL changed from {self.original_url} to {new_url}")
|
||||
try:
|
||||
self.object.log_url_change(self.original_url)
|
||||
|
||||
+110
-1
@@ -5,7 +5,7 @@ Links are managed via Django template views, tested through the Django test clie
|
||||
|
||||
import pytest
|
||||
|
||||
from links.models import Link, Tag
|
||||
from links.models import Link, LinkChangeLog, Tag
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
@@ -234,3 +234,112 @@ class TestLinkTagAutoCreate:
|
||||
assert tag.name == "spaced out"
|
||||
assert tag.slug == "spaced-out"
|
||||
assert Tag.objects.count() == before + 1
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
class TestCustomLinkEditUrlChangeLog:
|
||||
"""Editing a CUSTOM link's content (text) must NOT log a spurious
|
||||
"URL changed" entry, because custom-link URLs are derived from the
|
||||
alias and the original_url form field is hidden for custom links.
|
||||
Regression test for the bug where every edit of a custom link
|
||||
produced a LinkChangeLog row."""
|
||||
|
||||
def _update(self, client, link, **payload):
|
||||
data = {
|
||||
"alias": link.alias,
|
||||
# The original_url input is hidden for custom links and not
|
||||
# submitted by the browser, so keep it blank in this helper.
|
||||
"original_url": "",
|
||||
"link_type": Link.LinkType.CUSTOM,
|
||||
"description": "",
|
||||
"text": "",
|
||||
}
|
||||
data.update(payload)
|
||||
return client.post(f"/link/{link.pk}/edit/", data, follow=True)
|
||||
|
||||
def test_custom_link_text_edit_creates_no_url_change_log(self, client):
|
||||
link = Link.objects.create(
|
||||
alias="mynote",
|
||||
link_type=Link.LinkType.CUSTOM,
|
||||
original_url="/custom/mynote",
|
||||
text="old body text",
|
||||
)
|
||||
before = LinkChangeLog.objects.filter(link=link).count()
|
||||
|
||||
response = self._update(client, link, text="new body text")
|
||||
assert response.status_code == 200
|
||||
|
||||
link.refresh_from_db()
|
||||
assert link.text == "new body text"
|
||||
assert link.original_url == "/custom/mynote"
|
||||
assert LinkChangeLog.objects.filter(link=link).count() == before
|
||||
|
||||
def test_regular_link_text_unchanged_creates_no_url_change_log(self, client):
|
||||
link = Link.objects.create(
|
||||
alias="gh",
|
||||
link_type=Link.LinkType.LINK,
|
||||
original_url="https://github.com",
|
||||
)
|
||||
before = LinkChangeLog.objects.filter(link=link).count()
|
||||
|
||||
response = client.post(
|
||||
f"/link/{link.pk}/edit/",
|
||||
{
|
||||
"alias": "gh",
|
||||
"original_url": "https://github.com",
|
||||
"link_type": Link.LinkType.LINK,
|
||||
"description": "an edit that keeps the URL",
|
||||
"text": "",
|
||||
},
|
||||
follow=True,
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert LinkChangeLog.objects.filter(link=link).count() == before
|
||||
|
||||
def test_regular_link_url_change_logs_change(self, client):
|
||||
link = Link.objects.create(
|
||||
alias="gh",
|
||||
link_type=Link.LinkType.LINK,
|
||||
original_url="https://github.com",
|
||||
)
|
||||
before = LinkChangeLog.objects.filter(link=link).count()
|
||||
|
||||
response = client.post(
|
||||
f"/link/{link.pk}/edit/",
|
||||
{
|
||||
"alias": "gh",
|
||||
"original_url": "https://github.com/junv",
|
||||
"link_type": Link.LinkType.LINK,
|
||||
"description": "",
|
||||
"text": "",
|
||||
},
|
||||
follow=True,
|
||||
)
|
||||
assert response.status_code == 200
|
||||
logs = LinkChangeLog.objects.filter(link=link).order_by("-changed_at")
|
||||
assert logs.count() == before + 1
|
||||
assert logs.first().old_url == "https://github.com"
|
||||
assert logs.first().new_url == "https://github.com/junv"
|
||||
|
||||
def test_custom_link_alias_change_logs_url_change(self, client):
|
||||
"""When the alias of a custom link changes, the derived URL changes
|
||||
too, and that should be logged once."""
|
||||
link = Link.objects.create(
|
||||
alias="mynote",
|
||||
link_type=Link.LinkType.CUSTOM,
|
||||
original_url="/custom/mynote",
|
||||
text="body text",
|
||||
)
|
||||
before = LinkChangeLog.objects.filter(link=link).count()
|
||||
|
||||
# Helper defaults text to "" (custom links require non-empty text)
|
||||
response = self._update(client, link, alias="newnote", text="body text")
|
||||
assert response.status_code == 200
|
||||
|
||||
link.refresh_from_db()
|
||||
assert link.alias == "newnote"
|
||||
assert link.original_url == "/custom/newnote"
|
||||
logs = LinkChangeLog.objects.filter(link=link).order_by("-changed_at")
|
||||
assert logs.count() == before + 1
|
||||
assert logs.first().old_url == "/custom/mynote"
|
||||
assert logs.first().new_url == "/custom/newnote"
|
||||
|
||||
Reference in New Issue
Block a user