diff --git a/bookmarks/api/routes.py b/bookmarks/api/routes.py index 0351976..a4cae8a 100644 --- a/bookmarks/api/routes.py +++ b/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.website_loader import load_website_metadata +from bookmarks.services.bookmarks import archive_bookmark, unarchive_bookmark, website_loader +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: - existing_bookmark_data = { - 'id': bookmark.id, - 'edit_url': reverse('bookmarks:edit', args=[bookmark.id]) - } - - metadata = load_website_metadata(url) + # Either return metadata from existing bookmark, or scrape from URL + if bookmark: + metadata = WebsiteMetadata(url, bookmark.website_title, bookmark.website_description) + else: + metadata = website_loader.load_website_metadata(url) return Response({ 'bookmark': existing_bookmark_data, diff --git a/bookmarks/templates/bookmarks/form.html b/bookmarks/templates/bookmarks/form.html index 8c44136..ccbde82 100644 --- a/bookmarks/templates/bookmarks/form.html +++ b/bookmarks/templates/bookmarks/form.html @@ -15,8 +15,8 @@ {% endif %}
- This URL is already bookmarked. You can edit it or you can overwrite the existing bookmark - by saving this form. + This URL is already bookmarked. + The form has been pre-filled with the existing bookmark, and saving the form will update the existing bookmark.
@@ -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'; } diff --git a/bookmarks/tests/test_bookmarks_api.py b/bookmarks/tests/test_bookmarks_api.py index 485814d..d1c9362 100644 --- a/bookmarks/tests/test_bookmarks_api.py +++ b/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'])