Prevent duplicates when editing (#853)

* prevent creating duplicate URLs on edit

* Prevent duplicates when editing

bookmarks/api/serializers.py

@@ -111,6 +111,24 @@ class BookmarkSerializer(serializers.ModelSerializer):
 
         return update_bookmark(instance, tag_string, self.context["user"])
 
+    def validate(self, attrs):
+        # When creating a bookmark, the service logic prevents duplicate URLs by
+        # updating the existing bookmark instead. When editing a bookmark,
+        # there is no assumption that it would update a different bookmark if
+        # the URL is a duplicate, so raise a validation error in that case.
+        if self.instance and "url" in attrs:
+            is_duplicate = (
+                Bookmark.objects.filter(owner=self.instance.owner, url=attrs["url"])
+                .exclude(pk=self.instance.pk)
+                .exists()
+            )
+            if is_duplicate:
+                raise serializers.ValidationError(
+                    {"url": "A bookmark with this URL already exists."}
+                )
+
+        return attrs
+
 
 class TagSerializer(serializers.ModelSerializer):
     class Meta:

bookmarks/models.py

@@ -168,6 +168,24 @@ class BookmarkForm(forms.ModelForm):
             self.instance and self.instance.notes
         )
 
+    def clean_url(self):
+        # When creating a bookmark, the service logic prevents duplicate URLs by
+        # updating the existing bookmark instead, which is also communicated in
+        # the form's UI. When editing a bookmark, there is no assumption that
+        # it would update a different bookmark if the URL is a duplicate, so
+        # raise a validation error in that case.
+        url = self.cleaned_data["url"]
+        if self.instance.pk:
+            is_duplicate = (
+                Bookmark.objects.filter(owner=self.instance.owner, url=url)
+                .exclude(pk=self.instance.pk)
+                .exists()
+            )
+            if is_duplicate:
+                raise forms.ValidationError("A bookmark with this URL already exists.")
+
+        return url
+
 
 class BookmarkSearch:
     SORT_ADDED_ASC = "added_asc"

bookmarks/tests/test_bookmark_edit_view.py

@@ -141,6 +141,40 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
             html,
         )
 
+    def test_should_prevent_duplicate_urls(self):
+        edited_bookmark = self.setup_bookmark(url="http://example.com/edited")
+        existing_bookmark = self.setup_bookmark(url="http://example.com/existing")
+        other_user_bookmark = self.setup_bookmark(
+            url="http://example.com/other-user", user=User.objects.create_user("other")
+        )
+
+        # if the URL isn't modified it's not a duplicate
+        form_data = self.create_form_data({"url": edited_bookmark.url})
+        response = self.client.post(
+            reverse("bookmarks:edit", args=[edited_bookmark.id]), form_data
+        )
+        self.assertEqual(response.status_code, 302)
+
+        # if the URL is already bookmarked by another user, it's not a duplicate
+        form_data = self.create_form_data({"url": other_user_bookmark.url})
+        response = self.client.post(
+            reverse("bookmarks:edit", args=[edited_bookmark.id]), form_data
+        )
+        self.assertEqual(response.status_code, 302)
+
+        # if the URL is already bookmarked by the same user, it's a duplicate
+        form_data = self.create_form_data({"url": existing_bookmark.url})
+        response = self.client.post(
+            reverse("bookmarks:edit", args=[edited_bookmark.id]), form_data
+        )
+        self.assertEqual(response.status_code, 422)
+        self.assertInHTML(
+            "<li>A bookmark with this URL already exists.</li>",
+            response.content.decode(),
+        )
+        edited_bookmark.refresh_from_db()
+        self.assertNotEqual(edited_bookmark.url, existing_bookmark.url)
+
     def test_should_redirect_to_return_url(self):
         bookmark = self.setup_bookmark()
         form_data = self.create_form_data()

bookmarks/tests/test_bookmarks_api.py

@@ -685,6 +685,29 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
         updated_bookmark = Bookmark.objects.get(id=bookmark.id)
         self.assertCountEqual(updated_bookmark.tags.all(), [tag1, tag2])
 
+    def test_update_bookmark_should_prevent_duplicate_urls(self):
+        self.authenticate()
+        edited_bookmark = self.setup_bookmark(url="https://example.com/edited")
+        existing_bookmark = self.setup_bookmark(url="https://example.com/existing")
+        other_user_bookmark = self.setup_bookmark(
+            url="https://example.com/other", user=self.setup_user()
+        )
+
+        # if the URL isn't modified it's not a duplicate
+        data = {"url": edited_bookmark.url}
+        url = reverse("bookmarks:bookmark-detail", args=[edited_bookmark.id])
+        self.put(url, data, expected_status_code=status.HTTP_200_OK)
+
+        # if the URL is already bookmarked by another user, it's not a duplicate
+        data = {"url": other_user_bookmark.url}
+        url = reverse("bookmarks:bookmark-detail", args=[edited_bookmark.id])
+        self.put(url, data, expected_status_code=status.HTTP_200_OK)
+
+        # if the URL is already bookmarked by the same user, it's a duplicate
+        data = {"url": existing_bookmark.url}
+        url = reverse("bookmarks:bookmark-detail", args=[edited_bookmark.id])
+        self.put(url, data, expected_status_code=status.HTTP_400_BAD_REQUEST)
+
     def test_patch_bookmark(self):
         self.authenticate()
         bookmark = self.setup_bookmark()