From f7ac32cf4c5fed2a6c4cd3193e3aaa587e87440b Mon Sep 17 00:00:00 2001 From: Junwei Zhao Date: Thu, 16 Jul 2026 11:11:25 +1000 Subject: [PATCH] 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 --- links/views.py | 11 +++- tests/test_links_api.py | 111 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 119 insertions(+), 3 deletions(-) diff --git a/links/views.py b/links/views.py index 5105e26..ef31d18 100644 --- a/links/views.py +++ b/links/views.py @@ -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) diff --git a/tests/test_links_api.py b/tests/test_links_api.py index 2576c20..49a9159 100644 --- a/tests/test_links_api.py +++ b/tests/test_links_api.py @@ -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"