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"