diff --git a/bookmarks/models.py b/bookmarks/models.py index 4705a1f..a89b39b 100644 --- a/bookmarks/models.py +++ b/bookmarks/models.py @@ -101,6 +101,11 @@ class BookmarkForm(forms.ModelForm): required=False) description = forms.CharField(required=False, widget=forms.Textarea()) + # Include website title and description as hidden field as they only provide info when editing bookmarks + website_title = forms.CharField(max_length=512, + required=False, widget=forms.HiddenInput()) + website_description = forms.CharField(required=False, + widget=forms.HiddenInput()) unread = forms.BooleanField(required=False) shared = forms.BooleanField(required=False) # Hidden field that determines whether to close window/tab after saving the bookmark @@ -108,7 +113,17 @@ class BookmarkForm(forms.ModelForm): class Meta: model = Bookmark - fields = ['url', 'tag_string', 'title', 'description', 'unread', 'shared', 'auto_close'] + fields = [ + 'url', + 'tag_string', + 'title', + 'description', + 'website_title', + 'website_description', + 'unread', + 'shared', + 'auto_close', + ] class BookmarkFilters: diff --git a/bookmarks/services/bookmarks.py b/bookmarks/services/bookmarks.py index 30394ce..8cc72be 100644 --- a/bookmarks/services/bookmarks.py +++ b/bookmarks/services/bookmarks.py @@ -5,7 +5,7 @@ from django.utils import timezone from bookmarks.models import Bookmark, parse_tag_string from bookmarks.services.tags import get_or_create_tags -from bookmarks.services.website_loader import load_website_metadata +from bookmarks.services import website_loader from bookmarks.services import tasks @@ -38,16 +38,16 @@ def update_bookmark(bookmark: Bookmark, tag_string, current_user: User): # Detect URL change original_bookmark = Bookmark.objects.get(id=bookmark.id) has_url_changed = original_bookmark.url != bookmark.url - # Update website info - _update_website_metadata(bookmark) # Update tag list _update_bookmark_tags(bookmark, tag_string, current_user) # Update dates bookmark.date_modified = timezone.now() bookmark.save() - # Update web archive snapshot, if URL changed if has_url_changed: + # Update web archive snapshot, if URL changed tasks.create_web_archive_snapshot(current_user, bookmark, True) + # Only update website metadata if URL changed + _update_website_metadata(bookmark) return bookmark @@ -121,7 +121,7 @@ def _merge_bookmark_data(from_bookmark: Bookmark, to_bookmark: Bookmark): def _update_website_metadata(bookmark: Bookmark): - metadata = load_website_metadata(bookmark.url) + metadata = website_loader.load_website_metadata(bookmark.url) bookmark.website_title = metadata.title bookmark.website_description = metadata.description diff --git a/bookmarks/templates/bookmarks/form.html b/bookmarks/templates/bookmarks/form.html index 556141c..a193752 100644 --- a/bookmarks/templates/bookmarks/form.html +++ b/bookmarks/templates/bookmarks/form.html @@ -3,6 +3,8 @@
{% csrf_token %} + {{ form.website_title }} + {{ form.website_description }} {{ form.auto_close|attr:"type:hidden" }}
@@ -126,6 +128,8 @@ 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 websiteTitleInput = document.getElementById('{{ form.website_title.id_for_label }}'); + const websiteDescriptionInput = document.getElementById('{{ form.website_description.id_for_label }}'); const editedBookmarkId = {{ bookmark_id }}; function toggleLoadingIcon(input, show) { @@ -182,8 +186,19 @@ }); } - if (urlInput.value) checkUrl(); + // Fetch initial website data if we have a URL, and we are not editing an existing bookmark + // For existing bookmarks we get the website metadata through hidden inputs + if (urlInput.value && !editedBookmarkId) { + checkUrl(); + } urlInput.addEventListener('input', checkUrl); + + // Set initial website title and description for edited bookmarks + if (editedBookmarkId) { + updatePlaceholder(titleInput, websiteTitleInput.value); + updatePlaceholder(descriptionInput, websiteDescriptionInput.value); + } + setupEditAutoValueButton(titleInput); setupEditAutoValueButton(descriptionInput); })(); diff --git a/bookmarks/tests/test_bookmark_edit_view.py b/bookmarks/tests/test_bookmark_edit_view.py index a639b8f..fc07e3a 100644 --- a/bookmarks/tests/test_bookmark_edit_view.py +++ b/bookmarks/tests/test_bookmark_edit_view.py @@ -73,32 +73,43 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin): def test_should_prefill_bookmark_form_fields(self): tag1 = self.setup_tag() tag2 = self.setup_tag() - bookmark = self.setup_bookmark(tags=[tag1, tag2], title='edited title', description='edited description') + bookmark = self.setup_bookmark(tags=[tag1, tag2], title='edited title', description='edited description', + website_title='website title', website_description='website description') response = self.client.get(reverse('bookmarks:edit', args=[bookmark.id])) html = response.content.decode() - self.assertInHTML(''.format(bookmark.url), - html) + self.assertInHTML(f''' + + ''', html) tag_string = build_tag_string(bookmark.tag_names, ' ') - self.assertInHTML(''.format(tag_string), - html) + self.assertInHTML(f''' + + ''', html) - self.assertInHTML(''.format(bookmark.title), - html) + self.assertInHTML(f''' + + ''', html) - self.assertInHTML(''.format(bookmark.description), - html) + self.assertInHTML(f''' + + ''', html) + + self.assertInHTML(f''' + + ''', html) + + self.assertInHTML(f''' + + ''', html) def test_should_redirect_to_return_url(self): bookmark = self.setup_bookmark() @@ -173,4 +184,3 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin): Share ''', html, count=1) - diff --git a/bookmarks/tests/test_bookmarks_service.py b/bookmarks/tests/test_bookmarks_service.py index 23bb03e..2c1664a 100644 --- a/bookmarks/tests/test_bookmarks_service.py +++ b/bookmarks/tests/test_bookmarks_service.py @@ -7,6 +7,7 @@ from django.utils import timezone from bookmarks.models import Bookmark, Tag from bookmarks.services.bookmarks import create_bookmark, update_bookmark, archive_bookmark, archive_bookmarks, \ unarchive_bookmark, unarchive_bookmarks, delete_bookmarks, tag_bookmarks, untag_bookmarks +from bookmarks.services import website_loader from bookmarks.tests.helpers import BookmarkFactoryMixin from bookmarks.services import tasks @@ -60,6 +61,22 @@ class BookmarkServiceTestCase(TestCase, BookmarkFactoryMixin): mock_create_web_archive_snapshot.assert_not_called() + def test_update_should_update_website_metadata_if_url_did_change(self): + with patch.object(website_loader, 'load_website_metadata') as mock_load_website_metadata: + bookmark = self.setup_bookmark() + bookmark.url = 'https://example.com/updated' + update_bookmark(bookmark, 'tag1,tag2', self.user) + + mock_load_website_metadata.assert_called_once() + + def test_update_should_not_update_website_metadata_if_url_did_not_change(self): + with patch.object(website_loader, 'load_website_metadata') as mock_load_website_metadata: + bookmark = self.setup_bookmark() + bookmark.title = 'updated title' + update_bookmark(bookmark, 'tag1,tag2', self.user) + + mock_load_website_metadata.assert_not_called() + def test_archive_bookmark(self): bookmark = Bookmark( url='https://example.com',