Prefill form if URL is already bookmarked (#402)

* Prefill form from existing bookmark

* add bookmark check api tests

bookmarks/api/routes.py

@@ -1,4 +1,3 @@
-from django.urls import reverse
 from rest_framework import viewsets, mixins, status
 from rest_framework.decorators import action
 from rest_framework.response import Response
@@ -7,8 +6,8 @@ from rest_framework.routers import DefaultRouter
 from bookmarks import queries
 from bookmarks.api.serializers import BookmarkSerializer, TagSerializer
 from bookmarks.models import Bookmark, BookmarkFilters, Tag, User
-from bookmarks.services.bookmarks import archive_bookmark, unarchive_bookmark
+from bookmarks.services.bookmarks import archive_bookmark, unarchive_bookmark, website_loader
-from bookmarks.services.website_loader import load_website_metadata
+from bookmarks.services.website_loader import WebsiteMetadata
 
 
 class BookmarkViewSet(viewsets.GenericViewSet,
@@ -68,15 +67,13 @@ class BookmarkViewSet(viewsets.GenericViewSet,
     def check(self, request):
         url = request.GET.get('url')
         bookmark = Bookmark.objects.filter(owner=request.user, url=url).first()
-        existing_bookmark_data = None
+        existing_bookmark_data = self.get_serializer(bookmark).data if bookmark else None
 
-        if bookmark is not None:
+        # Either return metadata from existing bookmark, or scrape from URL
-            existing_bookmark_data = {
+        if bookmark:
-                'id': bookmark.id,
+            metadata = WebsiteMetadata(url, bookmark.website_title, bookmark.website_description)
-                'edit_url': reverse('bookmarks:edit', args=[bookmark.id])
+        else:
-            }
+            metadata = website_loader.load_website_metadata(url)
-
-        metadata = load_website_metadata(url)
 
         return Response({
             'bookmark': existing_bookmark_data,

bookmarks/templates/bookmarks/form.html

@@ -15,8 +15,8 @@
       </div>
     {% endif %}
     <div class="form-input-hint bookmark-exists">
-      This URL is already bookmarked. You can <a href="#">edit</a> it or you can overwrite the existing bookmark
+      This URL is already bookmarked.
-      by saving this form.
+      The form has been pre-filled with the existing bookmark, and saving the form will update the existing bookmark.
     </div>
   </div>
   <div class="form-group">
@@ -128,6 +128,9 @@
       const urlInput = document.getElementById('{{ form.url.id_for_label }}');
       const titleInput = document.getElementById('{{ form.title.id_for_label }}');
       const descriptionInput = document.getElementById('{{ form.description.id_for_label }}');
+      const tagsInput = document.getElementById('{{ form.tag_string.id_for_label }}');
+      const unreadCheckbox = document.getElementById('{{ form.unread.id_for_label }}');
+      const sharedCheckbox = document.getElementById('{{ form.shared.id_for_label }}');
       const websiteTitleInput = document.getElementById('{{ form.website_title.id_for_label }}');
       const websiteDescriptionInput = document.getElementById('{{ form.website_description.id_for_label }}');
       const editedBookmarkId = {{ bookmark_id }};
@@ -145,6 +148,14 @@
         }
       }
 
+      function updateInput(input, value) {
+          input.value = value;
+      }
+
+      function updateCheckbox(input, value) {
+          input.checked = value;
+      }
+
       function checkUrl() {
         toggleLoadingIcon(titleInput, true);
         toggleLoadingIcon(descriptionInput, true);
@@ -162,13 +173,17 @@
             toggleLoadingIcon(titleInput, false);
             toggleLoadingIcon(descriptionInput, false);
 
-            // Display hint if URL is already bookmarked
+            // Prefill form and display hint if URL is already bookmarked
+            const existingBookmark = data.bookmark;
             const bookmarkExistsHint = document.querySelector('.form-input-hint.bookmark-exists');
-            const editExistingBookmarkLink = bookmarkExistsHint.querySelector('a');
 
-            if (data.bookmark && data.bookmark.id !== editedBookmarkId) {
+            if (existingBookmark && !editedBookmarkId) {
               bookmarkExistsHint.style['display'] = 'block';
-              editExistingBookmarkLink.href = data.bookmark.edit_url;
+              updateInput(titleInput, existingBookmark.title);
+              updateInput(descriptionInput, existingBookmark.description);
+              updateInput(tagsInput, existingBookmark.tag_names.join(" "));
+              updateCheckbox(unreadCheckbox, existingBookmark.unread);
+              updateCheckbox(sharedCheckbox, existingBookmark.shared);
             } else {
               bookmarkExistsHint.style['display'] = 'none';
             }

bookmarks/tests/test_bookmarks_api.py

@@ -1,4 +1,6 @@
+import urllib.parse
 from collections import OrderedDict
+from unittest.mock import patch
 
 from django.contrib.auth.models import User
 from django.urls import reverse
@@ -6,6 +8,8 @@ from rest_framework import status
 from rest_framework.authtoken.models import Token
 
 from bookmarks.models import Bookmark
+from bookmarks.services import website_loader
+from bookmarks.services.website_loader import WebsiteMetadata
 from bookmarks.tests.helpers import LinkdingApiTestCase, BookmarkFactoryMixin
 
 
@@ -353,6 +357,66 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
         bookmark = Bookmark.objects.get(id=self.archived_bookmark1.id)
         self.assertFalse(bookmark.is_archived)
 
+    def test_check_returns_no_bookmark_if_url_is_not_bookmarked(self):
+        url = reverse('bookmarks:bookmark-check')
+        check_url = urllib.parse.quote_plus('https://example.com')
+        response = self.get(f'{url}?url={check_url}', expected_status_code=status.HTTP_200_OK)
+        bookmark_data = response.data['bookmark']
+
+        self.assertIsNone(bookmark_data)
+
+    def test_check_returns_scraped_metadata_if_url_is_not_bookmarked(self):
+        with patch.object(website_loader, 'load_website_metadata') as mock_load_website_metadata:
+            expected_metadata = WebsiteMetadata(
+                'https://example.com',
+                'Scraped metadata',
+                'Scraped description'
+            )
+            mock_load_website_metadata.return_value = expected_metadata
+
+            url = reverse('bookmarks:bookmark-check')
+            check_url = urllib.parse.quote_plus('https://example.com')
+            response = self.get(f'{url}?url={check_url}', expected_status_code=status.HTTP_200_OK)
+            metadata = response.data['metadata']
+
+            self.assertIsNotNone(metadata)
+            self.assertIsNotNone(expected_metadata.url, metadata['url'])
+            self.assertIsNotNone(expected_metadata.title, metadata['title'])
+            self.assertIsNotNone(expected_metadata.description, metadata['description'])
+
+    def test_check_returns_bookmark_if_url_is_bookmarked(self):
+        bookmark = self.setup_bookmark(url='https://example.com',
+                                       title='Example title',
+                                       description='Example description')
+
+        url = reverse('bookmarks:bookmark-check')
+        check_url = urllib.parse.quote_plus('https://example.com')
+        response = self.get(f'{url}?url={check_url}', expected_status_code=status.HTTP_200_OK)
+        bookmark_data = response.data['bookmark']
+
+        self.assertIsNotNone(bookmark_data)
+        self.assertEqual(bookmark.id, bookmark_data['id'])
+        self.assertEqual(bookmark.url, bookmark_data['url'])
+        self.assertEqual(bookmark.title, bookmark_data['title'])
+        self.assertEqual(bookmark.description, bookmark_data['description'])
+
+    def test_check_returns_existing_metadata_if_url_is_bookmarked(self):
+        bookmark = self.setup_bookmark(url='https://example.com',
+                                       website_title='Existing title',
+                                       website_description='Existing description')
+
+        with patch.object(website_loader, 'load_website_metadata') as mock_load_website_metadata:
+            url = reverse('bookmarks:bookmark-check')
+            check_url = urllib.parse.quote_plus('https://example.com')
+            response = self.get(f'{url}?url={check_url}', expected_status_code=status.HTTP_200_OK)
+            metadata = response.data['metadata']
+
+            mock_load_website_metadata.assert_not_called()
+            self.assertIsNotNone(metadata)
+            self.assertIsNotNone(bookmark.url, metadata['url'])
+            self.assertIsNotNone(bookmark.website_title, metadata['title'])
+            self.assertIsNotNone(bookmark.website_description, metadata['description'])
+
     def test_can_only_access_own_bookmarks(self):
         other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
         inaccessible_bookmark = self.setup_bookmark(user=other_user)
@@ -396,3 +460,8 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
 
         url = reverse('bookmarks:bookmark-unarchive', args=[inaccessible_shared_bookmark.id])
         self.post(url, expected_status_code=status.HTTP_404_NOT_FOUND)
+
+        url = reverse('bookmarks:bookmark-check')
+        check_url = urllib.parse.quote_plus(inaccessible_bookmark.url)
+        response = self.get(f'{url}?url={check_url}', expected_status_code=status.HTTP_200_OK)
+        self.assertIsNone(response.data['bookmark'])