Allow bookmarks to have empty title and description (#843)

* add migration for merging fields

* remove usage of website title and description

* keep empty website title and description in API for compatibility

* restore scraping in API and add option for disabling it

* document API scraping behavior

* remove deprecated fields from API docs

* improve form layout

* cleanup migration

* cleanup website loader

* update tests
This commit is contained in:
Sascha Ißbrücker
2024-09-22 07:52:00 +02:00
committed by GitHub
parent afa57aa10b
commit fe7ddbe645
22 changed files with 411 additions and 366 deletions

View File

@@ -56,7 +56,12 @@ class BookmarkViewSet(
return Bookmark.objects.all().filter(owner=user)
def get_serializer_context(self):
return {"request": self.request, "user": self.request.user}
disable_scraping = "disable_scraping" in self.request.GET
return {
"request": self.request,
"user": self.request.user,
"disable_scraping": disable_scraping,
}
@action(methods=["get"], detail=False)
def archived(self, request):
@@ -101,16 +106,7 @@ class BookmarkViewSet(
self.get_serializer(bookmark).data if bookmark else None
)
# Either return metadata from existing bookmark, or scrape from URL
if bookmark:
metadata = WebsiteMetadata(
url,
bookmark.website_title,
bookmark.website_description,
None,
)
else:
metadata = website_loader.load_website_metadata(url)
metadata = website_loader.load_website_metadata(url)
# Return tags that would be automatically applied to the bookmark
profile = request.user.profile
@@ -120,7 +116,7 @@ class BookmarkViewSet(
auto_tags = auto_tagging.get_tags(profile.auto_tagging_rules, url)
except Exception as e:
logger.error(
f"Failed to auto-tag bookmark. url={bookmark.url}",
f"Failed to auto-tag bookmark. url={url}",
exc_info=e,
)

View File

@@ -4,7 +4,11 @@ from rest_framework import serializers
from rest_framework.serializers import ListSerializer
from bookmarks.models import Bookmark, Tag, build_tag_string, UserProfile
from bookmarks.services.bookmarks import create_bookmark, update_bookmark
from bookmarks.services.bookmarks import (
create_bookmark,
update_bookmark,
enhance_with_website_metadata,
)
from bookmarks.services.tags import get_or_create_tag
@@ -29,8 +33,6 @@ class BookmarkSerializer(serializers.ModelSerializer):
"title",
"description",
"notes",
"website_title",
"website_description",
"web_archive_snapshot_url",
"favicon_url",
"preview_image_url",
@@ -40,15 +42,17 @@ class BookmarkSerializer(serializers.ModelSerializer):
"tag_names",
"date_added",
"date_modified",
]
read_only_fields = [
"website_title",
"website_description",
]
read_only_fields = [
"web_archive_snapshot_url",
"favicon_url",
"preview_image_url",
"date_added",
"date_modified",
"website_title",
"website_description",
]
list_serializer_class = BookmarkListSerializer
@@ -63,6 +67,9 @@ class BookmarkSerializer(serializers.ModelSerializer):
tag_names = TagListField(required=False, default=[])
favicon_url = serializers.SerializerMethodField()
preview_image_url = serializers.SerializerMethodField()
# Add dummy website title and description fields for backwards compatibility but keep them empty
website_title = serializers.SerializerMethodField()
website_description = serializers.SerializerMethodField()
def get_favicon_url(self, obj: Bookmark):
if not obj.favicon_file:
@@ -80,6 +87,12 @@ class BookmarkSerializer(serializers.ModelSerializer):
preview_image_url = request.build_absolute_uri(preview_image_file_path)
return preview_image_url
def get_website_title(self, obj: Bookmark):
return None
def get_website_description(self, obj: Bookmark):
return None
def create(self, validated_data):
bookmark = Bookmark()
bookmark.url = validated_data["url"]
@@ -90,7 +103,14 @@ class BookmarkSerializer(serializers.ModelSerializer):
bookmark.unread = validated_data["unread"]
bookmark.shared = validated_data["shared"]
tag_string = build_tag_string(validated_data["tag_names"])
return create_bookmark(bookmark, tag_string, self.context["user"])
saved_bookmark = create_bookmark(bookmark, tag_string, self.context["user"])
# Unless scraping is explicitly disabled, enhance bookmark with website
# metadata to preserve backwards compatibility with clients that expect
# title and description to be populated automatically when left empty
if not self.context.get("disable_scraping", False):
enhance_with_website_metadata(saved_bookmark)
return saved_bookmark
def update(self, instance: Bookmark, validated_data):
# Update fields if they were provided in the payload

View File

@@ -135,6 +135,9 @@ class BookmarkDetailsModalE2ETestCase(LinkdingE2ETestCase):
details_modal = self.open_details_modal(bookmark)
# Wait for confirm button to be initialized
self.page.wait_for_timeout(1000)
# Delete bookmark, verify return url
with self.page.expect_navigation(url=self.live_server_url + url):
details_modal.get_by_text("Delete...").click()

View File

@@ -1,26 +1,48 @@
from unittest.mock import patch
from django.urls import reverse
from playwright.sync_api import sync_playwright, expect
from bookmarks.e2e.helpers import LinkdingE2ETestCase
from bookmarks.services import website_loader
class BookmarkFormE2ETestCase(LinkdingE2ETestCase):
def test_create_enter_url_prefills_title_and_description(self):
with patch.object(
website_loader, "load_website_metadata"
) as mock_load_website_metadata:
mock_load_website_metadata.return_value = website_loader.WebsiteMetadata(
url="https://example.com",
title="Example Domain",
description="This domain is for use in illustrative examples in documents. You may use this domain in literature without prior coordination or asking for permission.",
preview_image=None,
)
with sync_playwright() as p:
page = self.open(reverse("bookmarks:new"), p)
page.get_by_label("URL").fill("https://example.com")
title = page.get_by_label("Title")
description = page.get_by_label("Description")
expect(title).to_have_value("Example Domain")
expect(description).to_have_value(
"This domain is for use in illustrative examples in documents. You may use this domain in literature without prior coordination or asking for permission."
)
def test_create_should_check_for_existing_bookmark(self):
existing_bookmark = self.setup_bookmark(
title="Existing title",
description="Existing description",
notes="Existing notes",
tags=[self.setup_tag(name="tag1"), self.setup_tag(name="tag2")],
website_title="Existing website title",
website_description="Existing website description",
unread=True,
)
tag_names = " ".join(existing_bookmark.tag_names)
with sync_playwright() as p:
browser = self.setup_browser(p)
page = browser.new_page()
page.goto(self.live_server_url + reverse("bookmarks:new"))
page = self.open(reverse("bookmarks:new"), p)
# Enter bookmarked URL
page.get_by_label("URL").fill(existing_bookmark.url)
@@ -37,14 +59,6 @@ class BookmarkFormE2ETestCase(LinkdingE2ETestCase):
self.assertEqual(
existing_bookmark.notes, page.get_by_label("Notes").input_value()
)
self.assertEqual(
existing_bookmark.website_title,
page.get_by_label("Title").get_attribute("placeholder"),
)
self.assertEqual(
existing_bookmark.website_description,
page.get_by_label("Description").get_attribute("placeholder"),
)
self.assertEqual(tag_names, page.get_by_label("Tags").input_value())
self.assertTrue(tag_names, page.get_by_label("Mark as unread").is_checked())
@@ -55,30 +69,66 @@ class BookmarkFormE2ETestCase(LinkdingE2ETestCase):
state="hidden", timeout=2000
)
browser.close()
def test_edit_should_not_check_for_existing_bookmark(self):
bookmark = self.setup_bookmark()
with sync_playwright() as p:
browser = self.setup_browser(p)
page = browser.new_page()
page.goto(
self.live_server_url + reverse("bookmarks:edit", args=[bookmark.id])
)
page = self.open(reverse("bookmarks:edit", args=[bookmark.id]), p)
page.wait_for_timeout(timeout=1000)
page.get_by_text("This URL is already bookmarked.").wait_for(state="hidden")
def test_edit_should_not_prefill_title_and_description(self):
bookmark = self.setup_bookmark()
with patch.object(
website_loader, "load_website_metadata"
) as mock_load_website_metadata:
mock_load_website_metadata.return_value = website_loader.WebsiteMetadata(
url="https://example.com",
title="Example Domain",
description="This domain is for use in illustrative examples in documents. You may use this domain in literature without prior coordination or asking for permission.",
preview_image=None,
)
with sync_playwright() as p:
page = self.open(reverse("bookmarks:edit", args=[bookmark.id]), p)
page.wait_for_timeout(timeout=1000)
title = page.get_by_label("Title")
description = page.get_by_label("Description")
expect(title).to_have_value(bookmark.title)
expect(description).to_have_value(bookmark.description)
def test_edit_enter_url_should_not_prefill_title_and_description(self):
bookmark = self.setup_bookmark()
with patch.object(
website_loader, "load_website_metadata"
) as mock_load_website_metadata:
mock_load_website_metadata.return_value = website_loader.WebsiteMetadata(
url="https://example.com",
title="Example Domain",
description="This domain is for use in illustrative examples in documents. You may use this domain in literature without prior coordination or asking for permission.",
preview_image=None,
)
with sync_playwright() as p:
page = self.open(reverse("bookmarks:edit", args=[bookmark.id]), p)
page.get_by_label("URL").fill("https://example.com")
page.wait_for_timeout(timeout=1000)
title = page.get_by_label("Title")
description = page.get_by_label("Description")
expect(title).to_have_value(bookmark.title)
expect(description).to_have_value(bookmark.description)
def test_enter_url_of_existing_bookmark_should_show_notes(self):
bookmark = self.setup_bookmark(
notes="Existing notes", description="Existing description"
)
with sync_playwright() as p:
browser = self.setup_browser(p)
page = browser.new_page()
page.goto(self.live_server_url + reverse("bookmarks:new"))
page = self.open(reverse("bookmarks:new"), p)
details = page.locator("details.notes")
expect(details).not_to_have_attribute("open", value="")
@@ -93,11 +143,11 @@ class BookmarkFormE2ETestCase(LinkdingE2ETestCase):
with sync_playwright() as p:
# Open page with URL that should have auto tags
browser = self.setup_browser(p)
page = browser.new_page()
url = self.live_server_url + reverse("bookmarks:new")
url += f"?url=https%3A%2F%2Fgithub.com%2Fsissbruecker%2Flinkding"
page.goto(url)
url = (
reverse("bookmarks:new")
+ "?url=https%3A%2F%2Fgithub.com%2Fsissbruecker%2Flinkding"
)
page = self.open(url, p)
auto_tags_hint = page.locator(".form-input-hint.auto-tags")
expect(auto_tags_hint).to_be_visible()

View File

@@ -122,7 +122,7 @@
}
const fetchedBookmarks = await api.listBookmarks(suggestionSearch, {limit: 5, offset: 0, path})
bookmarks = fetchedBookmarks.map(bookmark => {
const fullLabel = bookmark.title || bookmark.website_title || bookmark.url
const fullLabel = bookmark.title || bookmark.url
const label = clampText(fullLabel, 60)
return {
type: 'bookmark',

View File

@@ -0,0 +1,36 @@
# Generated by Django 5.1.1 on 2024-09-21 08:13
from django.db import migrations
from django.db.models import Q
from django.db.models.expressions import RawSQL
from bookmarks.models import Bookmark
def forwards(apps, schema_editor):
Bookmark.objects.filter(
Q(title__isnull=True) | Q(title__exact=""),
).extra(
where=["website_title IS NOT NULL"]
).update(title=RawSQL("website_title", ()))
Bookmark.objects.filter(
Q(description__isnull=True) | Q(description__exact=""),
).extra(where=["website_description IS NOT NULL"]).update(
description=RawSQL("website_description", ())
)
def reverse(apps, schema_editor):
pass
class Migration(migrations.Migration):
dependencies = [
("bookmarks", "0040_userprofile_items_per_page_and_more"),
]
operations = [
migrations.RunPython(forwards, reverse),
]

View File

@@ -56,7 +56,9 @@ class Bookmark(models.Model):
title = models.CharField(max_length=512, blank=True)
description = models.TextField(blank=True)
notes = models.TextField(blank=True)
# Obsolete field, kept to not remove column when generating migrations
website_title = models.CharField(max_length=512, blank=True, null=True)
# Obsolete field, kept to not remove column when generating migrations
website_description = models.TextField(blank=True, null=True)
web_archive_snapshot_url = models.CharField(max_length=2048, blank=True)
favicon_file = models.CharField(max_length=512, blank=True)
@@ -74,14 +76,12 @@ class Bookmark(models.Model):
def resolved_title(self):
if self.title:
return self.title
elif self.website_title:
return self.website_title
else:
return self.url
@property
def resolved_description(self):
return self.website_description if not self.description else self.description
return self.description
@property
def tag_names(self):
@@ -141,14 +141,9 @@ class BookmarkForm(forms.ModelForm):
# Use URLField for URL
url = forms.CharField(validators=[BookmarkURLValidator()])
tag_string = forms.CharField(required=False)
# Do not require title and description in form as we fill these automatically if they are empty
# Do not require title and description as they may be empty
title = forms.CharField(max_length=512, 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
@@ -162,8 +157,6 @@ class BookmarkForm(forms.ModelForm):
"title",
"description",
"notes",
"website_title",
"website_description",
"unread",
"shared",
"auto_close",

View File

@@ -53,8 +53,6 @@ def _base_bookmarks_query(
Q(title__icontains=term)
| Q(description__icontains=term)
| Q(notes__icontains=term)
| Q(website_title__icontains=term)
| Q(website_description__icontains=term)
| Q(url__icontains=term)
)
@@ -97,10 +95,6 @@ def _base_bookmarks_query(
query_set = query_set.annotate(
effective_title=Case(
When(Q(title__isnull=False) & ~Q(title__exact=""), then=Lower("title")),
When(
Q(website_title__isnull=False) & ~Q(website_title__exact=""),
then=Lower("website_title"),
),
default=Lower("url"),
output_field=CharField(),
)

View File

@@ -26,8 +26,6 @@ def create_bookmark(bookmark: Bookmark, tag_string: str, current_user: User):
_merge_bookmark_data(bookmark, existing_bookmark)
return update_bookmark(existing_bookmark, tag_string, current_user)
# Update website info
_update_website_metadata(bookmark)
# Set currently logged in user as owner
bookmark.owner = current_user
# Set dates
@@ -67,13 +65,22 @@ def update_bookmark(bookmark: Bookmark, tag_string, current_user: User):
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)
bookmark.save()
return bookmark
def enhance_with_website_metadata(bookmark: Bookmark):
metadata = website_loader.load_website_metadata(bookmark.url)
if not bookmark.title:
bookmark.title = metadata.title or ""
if not bookmark.description:
bookmark.description = metadata.description or ""
bookmark.save()
def archive_bookmark(bookmark: Bookmark):
bookmark.is_archived = True
bookmark.date_modified = timezone.now()
@@ -235,12 +242,6 @@ def _merge_bookmark_data(from_bookmark: Bookmark, to_bookmark: Bookmark):
to_bookmark.shared = from_bookmark.shared
def _update_website_metadata(bookmark: Bookmark):
metadata = website_loader.load_website_metadata(bookmark.url)
bookmark.website_title = metadata.title
bookmark.website_description = metadata.description
def _update_bookmark_tags(bookmark: Bookmark, tag_string: str, user: User):
tag_names = parse_tag_string(tag_string)

View File

@@ -14,8 +14,8 @@ logger = logging.getLogger(__name__)
@dataclass
class WebsiteMetadata:
url: str
title: str
description: str
title: str | None
description: str | None
preview_image: str | None
def to_dict(self):
@@ -43,7 +43,8 @@ def load_website_metadata(url: str):
start = timezone.now()
soup = BeautifulSoup(page_text, "html.parser")
title = soup.title.string.strip() if soup.title is not None else None
if soup.title and soup.title.string:
title = soup.title.string.strip()
description_tag = soup.find("meta", attrs={"name": "description"})
description = (
description_tag["content"].strip()

View File

@@ -3,12 +3,13 @@
<div class="bookmarks-form">
{% csrf_token %}
{{ form.website_title }}
{{ form.website_description }}
{{ form.auto_close|attr:"type:hidden" }}
<div class="form-group {% if form.url.errors %}has-error{% endif %}">
<label for="{{ form.url.id_for_label }}" class="form-label">URL</label>
{{ form.url|add_class:"form-input"|attr:"autofocus"|attr:"placeholder: " }}
<div class="has-icon-right">
{{ form.url|add_class:"form-input"|attr:"autofocus"|attr:"placeholder: " }}
<i class="form-icon loading"></i>
</div>
{% if form.url.errors %}
<div class="form-input-hint">
{{ form.url.errors }}
@@ -29,44 +30,14 @@
<div class="form-input-hint auto-tags"></div>
{{ form.tag_string.errors }}
</div>
<div class="form-group has-icon-right">
<div class="form-group">
<label for="{{ form.title.id_for_label }}" class="form-label">Title</label>
<div class="has-icon-right">
{{ form.title|add_class:"form-input"|attr:"autocomplete:off" }}
<i class="form-icon loading"></i>
<button type="button" class="btn btn-link form-icon" title="Edit title from website">
<svg xmlns="http://www.w3.org/2000/svg" width="24px" height="24px" viewBox="0 0 24 24" fill="none"
stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round">
<path stroke="none" d="M0 0h24v24H0z" fill="none"/>
<path d="M7 7h-1a2 2 0 0 0 -2 2v9a2 2 0 0 0 2 2h9a2 2 0 0 0 2 -2v-1"/>
<path d="M20.385 6.585a2.1 2.1 0 0 0 -2.97 -2.97l-8.415 8.385v3h3l8.385 -8.415z"/>
<path d="M16 5l3 3"/>
</svg>
</button>
</div>
<div class="form-input-hint">
Optional, leave empty to use title from website.
</div>
{{ form.title|add_class:"form-input"|attr:"autocomplete:off" }}
{{ form.title.errors }}
</div>
<div class="form-group">
<label for="{{ form.description.id_for_label }}" class="form-label">Description</label>
<div class="has-icon-right">
{{ form.description|add_class:"form-input"|attr:"rows:2" }}
<i class="form-icon loading"></i>
<button type="button" class="btn btn-link form-icon" title="Edit description from website">
<svg xmlns="http://www.w3.org/2000/svg" width="24px" height="24px" viewBox="0 0 24 24" fill="none"
stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round">
<path stroke="none" d="M0 0h24v24H0z" fill="none"/>
<path d="M7 7h-1a2 2 0 0 0 -2 2v9a2 2 0 0 0 2 2h9a2 2 0 0 0 2 -2v-1"/>
<path d="M20.385 6.585a2.1 2.1 0 0 0 -2.97 -2.97l-8.415 8.385v3h3l8.385 -8.415z"/>
<path d="M16 5l3 3"/>
</svg>
</button>
</div>
<div class="form-input-hint">
Optional, leave empty to use description from website.
</div>
{{ form.description|add_class:"form-input"|attr:"rows:3" }}
{{ form.description.errors }}
</div>
<div class="form-group">
@@ -76,10 +47,10 @@
</summary>
<label for="{{ form.notes.id_for_label }}" class="text-assistive">Notes</label>
{{ form.notes|add_class:"form-input"|attr:"rows:8" }}
<div class="form-input-hint">
Additional notes, supports Markdown.
</div>
</details>
<div class="form-input-hint">
Additional notes, supports Markdown.
</div>
{{ form.notes.errors }}
</div>
<div class="form-group">
@@ -119,9 +90,8 @@
</div>
<script type="application/javascript">
/**
* - Pre-fill title and description placeholders with metadata from website as soon as URL changes
* - Pre-fill title and description with metadata from website as soon as URL changes
* - Show hint if URL is already bookmarked
* - Setup buttons that allow editing of scraped website values
*/
(function init() {
const urlInput = document.getElementById('{{ form.url.id_for_label }}');
@@ -131,8 +101,7 @@
const notesInput = document.getElementById('{{ form.notes.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 bookmarkExistsHint = document.querySelector('.form-input-hint.bookmark-exists');
const editedBookmarkId = {{ bookmark_id }};
function toggleLoadingIcon(input, show) {
@@ -140,14 +109,6 @@
icon.style['visibility'] = show ? 'visible' : 'hidden';
}
function updatePlaceholder(input, value) {
if (value) {
input.setAttribute('placeholder', value);
} else {
input.removeAttribute('placeholder');
}
}
function updateInput(input, value) {
if (!input) {
return;
@@ -163,10 +124,11 @@
}
function checkUrl() {
toggleLoadingIcon(titleInput, true);
toggleLoadingIcon(descriptionInput, true);
updatePlaceholder(titleInput, null);
updatePlaceholder(descriptionInput, null);
if (!urlInput.value) {
return;
}
toggleLoadingIcon(urlInput, true);
const websiteUrl = encodeURIComponent(urlInput.value);
const requestUrl = `{% url 'bookmarks:api-root' %}bookmarks/check?url=${websiteUrl}`;
@@ -174,16 +136,14 @@
.then(response => response.json())
.then(data => {
const metadata = data.metadata;
updatePlaceholder(titleInput, metadata.title);
updatePlaceholder(descriptionInput, metadata.description);
toggleLoadingIcon(titleInput, false);
toggleLoadingIcon(descriptionInput, false);
toggleLoadingIcon(urlInput, false);
// Prefill form and display hint if URL is already bookmarked
// Display hint if URL is already bookmarked
const existingBookmark = data.bookmark;
const bookmarkExistsHint = document.querySelector('.form-input-hint.bookmark-exists');
bookmarkExistsHint.style['display'] = existingBookmark ? 'block' : 'none';
if (existingBookmark && !editedBookmarkId) {
// Prefill form with existing bookmark data
if (existingBookmark) {
// Workaround: tag input will be replaced by tag autocomplete, so
// defer getting the input until we need it
const tagsInput = document.getElementById('{{ form.tag_string.id_for_label }}');
@@ -197,7 +157,9 @@
updateCheckbox(unreadCheckbox, existingBookmark.unread);
updateCheckbox(sharedCheckbox, existingBookmark.shared);
} else {
bookmarkExistsHint.style['display'] = 'none';
// Set title and description to website metadata
updateInput(titleInput, metadata.title);
updateInput(descriptionInput, metadata.description);
}
// Preview auto tags
@@ -214,31 +176,10 @@
});
}
function setupEditAutoValueButton(input) {
const editAutoValueButton = input.parentNode.querySelector('.btn.form-icon');
if (!editAutoValueButton) return;
editAutoValueButton.addEventListener('click', function (event) {
event.preventDefault();
input.value = input.getAttribute('placeholder');
input.focus();
input.select();
});
}
setupEditAutoValueButton(titleInput);
setupEditAutoValueButton(descriptionInput);
// 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) {
// Fetch website metadata when page loads and when URL changes, unless we are editing an existing bookmark
if (!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);
urlInput.addEventListener('input', checkUrl);
}
})();
</script>

View File

@@ -41,8 +41,6 @@ class BookmarkFactoryMixin:
title: str = None,
description: str = "",
notes: str = "",
website_title: str = "",
website_description: str = "",
web_archive_snapshot_url: str = "",
favicon_file: str = "",
preview_image_file: str = "",
@@ -64,8 +62,6 @@ class BookmarkFactoryMixin:
title=title,
description=description,
notes=notes,
website_title=website_title,
website_description=website_description,
date_added=added,
date_modified=timezone.now(),
owner=user,

View File

@@ -150,16 +150,8 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin
self.assertIsNotNone(title)
self.assertEqual(title.text.strip(), bookmark.title)
# with website title
bookmark = self.setup_bookmark(title="", website_title="Website title")
soup = self.get_index_details_modal(bookmark)
title = soup.find("h2")
self.assertIsNotNone(title)
self.assertEqual(title.text.strip(), bookmark.website_title)
# with URL only
bookmark = self.setup_bookmark(title="", website_title="")
bookmark = self.setup_bookmark(title="")
soup = self.get_index_details_modal(bookmark)
title = soup.find("h2")
@@ -478,7 +470,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin
def test_description(self):
# without description
bookmark = self.setup_bookmark(description="", website_description="")
bookmark = self.setup_bookmark(description="")
soup = self.get_index_details_modal(bookmark)
section = self.find_section(soup, "Description")
@@ -491,15 +483,6 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin
section = self.get_section(soup, "Description")
self.assertEqual(section.text.strip(), bookmark.description)
# with website description
bookmark = self.setup_bookmark(
description="", website_description="Website description"
)
soup = self.get_index_details_modal(bookmark)
section = self.get_section(soup, "Description")
self.assertEqual(section.text.strip(), bookmark.website_description)
def test_notes(self):
# without notes
bookmark = self.setup_bookmark()

View File

@@ -80,8 +80,6 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
title="edited title",
description="edited description",
notes="edited notes",
website_title="website title",
website_description="website description",
)
response = self.client.get(reverse("bookmarks:edit", args=[bookmark.id]))
@@ -114,7 +112,7 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
self.assertInHTML(
f"""
<textarea name="description" cols="40" rows="2" class="form-input" id="id_description">
<textarea name="description" cols="40" rows="3" class="form-input" id="id_description">
{bookmark.description}
</textarea>
""",
@@ -130,22 +128,6 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
html,
)
self.assertInHTML(
f"""
<input type="hidden" name="website_title" id="id_website_title"
value="{bookmark.website_title}">
""",
html,
)
self.assertInHTML(
f"""
<input type="hidden" name="website_description" id="id_website_description"
value="{bookmark.website_description}">
""",
html,
)
def test_should_redirect_to_return_url(self):
bookmark = self.setup_bookmark()
form_data = self.create_form_data()

View File

@@ -96,7 +96,7 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
self.assertInHTML(
'<textarea name="description" class="form-input" cols="40" '
'rows="2" id="id_description">Example Site Description</textarea>',
'rows="3" id="id_description">Example Site Description</textarea>',
html,
)
@@ -115,6 +115,9 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
</summary>
<label for="id_notes" class="text-assistive">Notes</label>
<textarea name="notes" cols="40" rows="8" class="form-input" id="id_notes">**Find** more info [here](http://example.com)</textarea>
<div class="form-input-hint">
Additional notes, supports Markdown.
</div>
</details>
""",
html,

View File

@@ -33,8 +33,6 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
expectation["title"] = bookmark.title
expectation["description"] = bookmark.description
expectation["notes"] = bookmark.notes
expectation["website_title"] = bookmark.website_title
expectation["website_description"] = bookmark.website_description
expectation["web_archive_snapshot_url"] = bookmark.web_archive_snapshot_url
expectation["favicon_url"] = (
f"http://testserver/static/{bookmark.favicon_file}"
@@ -56,6 +54,8 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
expectation["date_modified"] = bookmark.date_modified.isoformat().replace(
"+00:00", "Z"
)
expectation["website_title"] = None
expectation["website_description"] = None
expectations.append(expectation)
for data in data_list:
@@ -87,6 +87,19 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
)
self.assertBookmarkListEqual(response.data["results"], bookmarks)
def test_list_bookmarks_returns_none_for_website_title_and_description(self):
self.authenticate()
bookmark = self.setup_bookmark()
bookmark.website_title = "Website title"
bookmark.website_description = "Website description"
bookmark.save()
response = self.get(
reverse("bookmarks:bookmark-list"), expected_status_code=status.HTTP_200_OK
)
self.assertIsNone(response.data["results"][0]["website_title"])
self.assertIsNone(response.data["results"][0]["website_description"])
def test_list_bookmarks_does_not_return_archived_bookmarks(self):
self.authenticate()
bookmarks = self.setup_numbered_bookmarks(5)
@@ -382,6 +395,44 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
self.assertEqual(bookmark.tags.filter(name=data["tag_names"][0]).count(), 1)
self.assertEqual(bookmark.tags.filter(name=data["tag_names"][1]).count(), 1)
def test_create_bookmark_enhances_with_metadata_by_default(self):
self.authenticate()
data = {"url": "https://example.com/"}
with patch.object(website_loader, "load_website_metadata") as mock_load:
mock_load.return_value = WebsiteMetadata(
url="https://example.com/",
title="Website title",
description="Website description",
preview_image=None,
)
self.post(reverse("bookmarks:bookmark-list"), data, status.HTTP_201_CREATED)
bookmark = Bookmark.objects.get(url=data["url"])
self.assertEqual(bookmark.title, "Website title")
self.assertEqual(bookmark.description, "Website description")
def test_create_bookmark_does_not_enhance_with_metadata_if_scraping_is_disabled(
self,
):
self.authenticate()
data = {"url": "https://example.com/"}
with patch.object(website_loader, "load_website_metadata") as mock_load:
mock_load.return_value = WebsiteMetadata(
url="https://example.com/",
title="Website title",
description="Website description",
preview_image=None,
)
self.post(
reverse("bookmarks:bookmark-list") + "?disable_scraping",
data,
status.HTTP_201_CREATED,
)
bookmark = Bookmark.objects.get(url=data["url"])
self.assertEqual(bookmark.title, "")
self.assertEqual(bookmark.description, "")
def test_create_bookmark_with_same_url_updates_existing_bookmark(self):
self.authenticate()
@@ -775,18 +826,24 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
"http://testserver/static/preview.png", bookmark_data["preview_image_url"]
)
def test_check_returns_existing_metadata_if_url_is_bookmarked(self):
def test_check_returns_scraped_metadata_if_url_is_bookmarked(self):
self.authenticate()
bookmark = self.setup_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:
expected_metadata = WebsiteMetadata(
"https://example.com",
"Scraped metadata",
"Scraped description",
"https://example.com/preview.png",
)
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(
@@ -794,12 +851,11 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
)
metadata = response.data["metadata"]
mock_load_website_metadata.assert_not_called()
self.assertIsNotNone(metadata)
self.assertEqual(bookmark.url, metadata["url"])
self.assertEqual(bookmark.website_title, metadata["title"])
self.assertEqual(bookmark.website_description, metadata["description"])
self.assertIsNone(metadata["preview_image"])
self.assertEqual(expected_metadata.url, metadata["url"])
self.assertEqual(expected_metadata.title, metadata["title"])
self.assertEqual(expected_metadata.description, metadata["description"])
self.assertEqual(expected_metadata.preview_image, metadata["preview_image"])
def test_check_returns_no_auto_tags_if_none_configured(self):
self.authenticate()

View File

@@ -8,15 +8,9 @@ class BookmarkTestCase(TestCase):
def test_bookmark_resolved_title(self):
bookmark = Bookmark(
title="Custom title",
website_title="Website title",
url="https://example.com",
)
self.assertEqual(bookmark.resolved_title, "Custom title")
bookmark = Bookmark(
title="", website_title="Website title", url="https://example.com"
)
self.assertEqual(bookmark.resolved_title, "Website title")
bookmark = Bookmark(title="", website_title="", url="https://example.com")
bookmark = Bookmark(title="", url="https://example.com")
self.assertEqual(bookmark.resolved_title, "https://example.com")

View File

@@ -25,8 +25,8 @@ from bookmarks.services.bookmarks import (
share_bookmarks,
unshare_bookmarks,
upload_asset,
enhance_with_website_metadata,
)
from bookmarks.services.website_loader import WebsiteMetadata
from bookmarks.tests.helpers import BookmarkFactoryMixin
User = get_user_model()
@@ -37,22 +37,14 @@ class BookmarkServiceTestCase(TestCase, BookmarkFactoryMixin):
def setUp(self) -> None:
self.get_or_create_test_user()
def test_create_should_update_website_metadata(self):
def test_create_should_not_update_website_metadata(self):
with patch.object(
website_loader, "load_website_metadata"
) as mock_load_website_metadata:
expected_metadata = WebsiteMetadata(
"https://example.com",
"Website title",
"Website description",
"https://example.com/preview.png",
)
mock_load_website_metadata.return_value = expected_metadata
bookmark_data = Bookmark(
url="https://example.com",
title="Updated Title",
description="Updated description",
title="Initial Title",
description="Initial description",
unread=True,
shared=True,
is_archived=True,
@@ -62,10 +54,9 @@ class BookmarkServiceTestCase(TestCase, BookmarkFactoryMixin):
)
created_bookmark.refresh_from_db()
self.assertEqual(expected_metadata.title, created_bookmark.website_title)
self.assertEqual(
expected_metadata.description, created_bookmark.website_description
)
self.assertEqual("Initial Title", created_bookmark.title)
self.assertEqual("Initial description", created_bookmark.description)
mock_load_website_metadata.assert_not_called()
def test_create_should_update_existing_bookmark_with_same_url(self):
original_bookmark = self.setup_bookmark(
@@ -164,37 +155,28 @@ 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:
expected_metadata = WebsiteMetadata(
"https://example.com/updated",
"Updated website title",
"Updated website description",
"https://example.com/preview.png",
)
mock_load_website_metadata.return_value = expected_metadata
bookmark = self.setup_bookmark()
bookmark.url = "https://example.com/updated"
update_bookmark(bookmark, "tag1,tag2", self.user)
bookmark.refresh_from_db()
mock_load_website_metadata.assert_called_once()
self.assertEqual(expected_metadata.title, bookmark.website_title)
self.assertEqual(
expected_metadata.description, bookmark.website_description
)
def test_update_should_not_update_website_metadata_if_url_did_not_change(self):
def test_update_should_not_update_website_metadata(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)
bookmark.refresh_from_db()
self.assertEqual("updated title", bookmark.title)
mock_load_website_metadata.assert_not_called()
def test_update_should_not_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(title="initial title")
bookmark.url = "https://example.com/updated"
update_bookmark(bookmark, "tag1,tag2", self.user)
bookmark.refresh_from_db()
self.assertEqual("initial title", bookmark.title)
mock_load_website_metadata.assert_not_called()
def test_update_should_update_favicon(self):
@@ -914,3 +896,61 @@ class BookmarkServiceTestCase(TestCase, BookmarkFactoryMixin):
self.assertIsNone(asset.file_size)
self.assertEqual(BookmarkAsset.STATUS_FAILURE, asset.status)
self.assertEqual("", asset.file)
def test_enhance_with_website_metadata(self):
bookmark = self.setup_bookmark(url="https://example.com")
with patch.object(
website_loader, "load_website_metadata"
) as mock_load_website_metadata:
mock_load_website_metadata.return_value = website_loader.WebsiteMetadata(
url="https://example.com",
title="Website title",
description="Website description",
preview_image=None,
)
# missing title and description
bookmark.title = ""
bookmark.description = ""
bookmark.save()
enhance_with_website_metadata(bookmark)
bookmark.refresh_from_db()
self.assertEqual("Website title", bookmark.title)
self.assertEqual("Website description", bookmark.description)
# missing title only
bookmark.title = ""
bookmark.description = "Initial description"
bookmark.save()
enhance_with_website_metadata(bookmark)
bookmark.refresh_from_db()
self.assertEqual("Website title", bookmark.title)
self.assertEqual("Initial description", bookmark.description)
# missing description only
bookmark.title = "Initial title"
bookmark.description = ""
bookmark.save()
enhance_with_website_metadata(bookmark)
bookmark.refresh_from_db()
self.assertEqual("Initial title", bookmark.title)
self.assertEqual("Website description", bookmark.description)
# metadata returns None
mock_load_website_metadata.return_value = website_loader.WebsiteMetadata(
url="https://example.com",
title=None,
description=None,
preview_image=None,
)
bookmark.title = ""
bookmark.description = ""
bookmark.save()
enhance_with_website_metadata(bookmark)
bookmark.refresh_from_db()
self.assertEqual("", bookmark.title)
self.assertEqual("", bookmark.description)

View File

@@ -98,7 +98,5 @@ class ExporterTestCase(TestCase, BookmarkFactoryMixin):
bookmark = self.setup_bookmark()
bookmark.title = ""
bookmark.description = ""
bookmark.website_title = None
bookmark.website_description = None
bookmark.save()
exporter.export_netscape_html([bookmark])

View File

@@ -31,11 +31,18 @@ class FeedsTestCase(TestCase, BookmarkFactoryMixin):
for tag in bookmark.tag_names:
categories.append(f"<category>{tag}</category>")
if bookmark.resolved_description:
expected_description = (
f"<description>{bookmark.resolved_description}</description>"
)
else:
expected_description = "<description/>"
expected_item = (
"<item>"
f"<title>{bookmark.resolved_title}</title>"
f"<link>{bookmark.url}</link>"
f"<description>{bookmark.resolved_description}</description>"
f"{expected_description}"
f"<pubDate>{rfc2822_date(bookmark.date_added)}</pubDate>"
f"<guid>{bookmark.url}</guid>"
f"{''.join(categories)}"
@@ -63,7 +70,7 @@ class FeedsTestCase(TestCase, BookmarkFactoryMixin):
def test_all_returns_all_unarchived_bookmarks(self):
bookmarks = [
self.setup_bookmark(description="test description"),
self.setup_bookmark(website_description="test website description"),
self.setup_bookmark(description=""),
self.setup_bookmark(unread=True, description="test description"),
]
self.setup_bookmark(is_archived=True)
@@ -118,9 +125,7 @@ class FeedsTestCase(TestCase, BookmarkFactoryMixin):
unread_bookmarks = [
self.setup_bookmark(unread=True, description="test description"),
self.setup_bookmark(
unread=True, website_description="test website description"
),
self.setup_bookmark(unread=True, description=""),
self.setup_bookmark(unread=True, description="test description"),
]

View File

@@ -36,14 +36,6 @@ class QueriesTestCase(TestCase, BookmarkFactoryMixin):
self.setup_bookmark(description=random_sentence(including_word="TERM1")),
self.setup_bookmark(notes=random_sentence(including_word="term1")),
self.setup_bookmark(notes=random_sentence(including_word="TERM1")),
self.setup_bookmark(website_title=random_sentence(including_word="term1")),
self.setup_bookmark(website_title=random_sentence(including_word="TERM1")),
self.setup_bookmark(
website_description=random_sentence(including_word="term1")
),
self.setup_bookmark(
website_description=random_sentence(including_word="TERM1")
),
]
self.term1_term2_bookmarks = [
self.setup_bookmark(url="http://example.com/term1/term2"),
@@ -55,30 +47,16 @@ class QueriesTestCase(TestCase, BookmarkFactoryMixin):
description=random_sentence(including_word="term1"),
title=random_sentence(including_word="term2"),
),
self.setup_bookmark(
website_title=random_sentence(including_word="term1"),
title=random_sentence(including_word="term2"),
),
self.setup_bookmark(
website_description=random_sentence(including_word="term1"),
title=random_sentence(including_word="term2"),
),
]
self.tag1_bookmarks = [
self.setup_bookmark(tags=[tag1]),
self.setup_bookmark(title=random_sentence(), tags=[tag1]),
self.setup_bookmark(description=random_sentence(), tags=[tag1]),
self.setup_bookmark(website_title=random_sentence(), tags=[tag1]),
self.setup_bookmark(website_description=random_sentence(), tags=[tag1]),
]
self.tag1_as_term_bookmarks = [
self.setup_bookmark(url="http://example.com/tag1"),
self.setup_bookmark(title=random_sentence(including_word="tag1")),
self.setup_bookmark(description=random_sentence(including_word="tag1")),
self.setup_bookmark(website_title=random_sentence(including_word="tag1")),
self.setup_bookmark(
website_description=random_sentence(including_word="tag1")
),
]
self.term1_tag1_bookmarks = [
self.setup_bookmark(url="http://example.com/term1", tags=[tag1]),
@@ -88,12 +66,6 @@ class QueriesTestCase(TestCase, BookmarkFactoryMixin):
self.setup_bookmark(
description=random_sentence(including_word="term1"), tags=[tag1]
),
self.setup_bookmark(
website_title=random_sentence(including_word="term1"), tags=[tag1]
),
self.setup_bookmark(
website_description=random_sentence(including_word="term1"), tags=[tag1]
),
]
self.tag2_bookmarks = [
self.setup_bookmark(tags=[tag2]),
@@ -136,22 +108,6 @@ class QueriesTestCase(TestCase, BookmarkFactoryMixin):
self.setup_bookmark(
notes=random_sentence(including_word="TERM1"), tags=[self.setup_tag()]
),
self.setup_bookmark(
website_title=random_sentence(including_word="term1"),
tags=[self.setup_tag()],
),
self.setup_bookmark(
website_title=random_sentence(including_word="TERM1"),
tags=[self.setup_tag()],
),
self.setup_bookmark(
website_description=random_sentence(including_word="term1"),
tags=[self.setup_tag()],
),
self.setup_bookmark(
website_description=random_sentence(including_word="TERM1"),
tags=[self.setup_tag()],
),
]
self.term1_term2_bookmarks = [
self.setup_bookmark(
@@ -167,16 +123,6 @@ class QueriesTestCase(TestCase, BookmarkFactoryMixin):
title=random_sentence(including_word="term2"),
tags=[self.setup_tag()],
),
self.setup_bookmark(
website_title=random_sentence(including_word="term1"),
title=random_sentence(including_word="term2"),
tags=[self.setup_tag()],
),
self.setup_bookmark(
website_description=random_sentence(including_word="term1"),
title=random_sentence(including_word="term2"),
tags=[self.setup_tag()],
),
]
self.tag1_bookmarks = [
self.setup_bookmark(tags=[tag1, self.setup_tag()]),
@@ -184,21 +130,11 @@ class QueriesTestCase(TestCase, BookmarkFactoryMixin):
self.setup_bookmark(
description=random_sentence(), tags=[tag1, self.setup_tag()]
),
self.setup_bookmark(
website_title=random_sentence(), tags=[tag1, self.setup_tag()]
),
self.setup_bookmark(
website_description=random_sentence(), tags=[tag1, self.setup_tag()]
),
]
self.tag1_as_term_bookmarks = [
self.setup_bookmark(url="http://example.com/tag1"),
self.setup_bookmark(title=random_sentence(including_word="tag1")),
self.setup_bookmark(description=random_sentence(including_word="tag1")),
self.setup_bookmark(website_title=random_sentence(including_word="tag1")),
self.setup_bookmark(
website_description=random_sentence(including_word="tag1")
),
]
self.term1_tag1_bookmarks = [
self.setup_bookmark(
@@ -212,14 +148,6 @@ class QueriesTestCase(TestCase, BookmarkFactoryMixin):
description=random_sentence(including_word="term1"),
tags=[tag1, self.setup_tag()],
),
self.setup_bookmark(
website_title=random_sentence(including_word="term1"),
tags=[tag1, self.setup_tag()],
),
self.setup_bookmark(
website_description=random_sentence(including_word="term1"),
tags=[tag1, self.setup_tag()],
),
]
self.tag2_bookmarks = [
self.setup_bookmark(tags=[tag2, self.setup_tag()]),
@@ -1260,30 +1188,18 @@ class QueriesTestCase(TestCase, BookmarkFactoryMixin):
self.setup_bookmark(title="A_1_2"),
self.setup_bookmark(title="b_1_1"),
self.setup_bookmark(title="B_1_2"),
self.setup_bookmark(title="", website_title="a_2_1"),
self.setup_bookmark(title="", website_title="A_2_2"),
self.setup_bookmark(title="", website_title="b_2_1"),
self.setup_bookmark(title="", website_title="B_2_2"),
self.setup_bookmark(title="", website_title="", url="a_3_1"),
self.setup_bookmark(title="", website_title="", url="A_3_2"),
self.setup_bookmark(title="", website_title="", url="b_3_1"),
self.setup_bookmark(title="", website_title="", url="B_3_2"),
self.setup_bookmark(title="a_4_1", website_title="0"),
self.setup_bookmark(title="A_4_2", website_title="0"),
self.setup_bookmark(title="b_4_1", website_title="0"),
self.setup_bookmark(title="B_4_2", website_title="0"),
self.setup_bookmark(title="", url="a_3_1"),
self.setup_bookmark(title="", url="A_3_2"),
self.setup_bookmark(title="", url="b_3_1"),
self.setup_bookmark(title="", url="B_3_2"),
self.setup_bookmark(title="a_5_1", url="0"),
self.setup_bookmark(title="A_5_2", url="0"),
self.setup_bookmark(title="b_5_1", url="0"),
self.setup_bookmark(title="B_5_2", url="0"),
self.setup_bookmark(title="", website_title="a_6_1", url="0"),
self.setup_bookmark(title="", website_title="A_6_2", url="0"),
self.setup_bookmark(title="", website_title="b_6_1", url="0"),
self.setup_bookmark(title="", website_title="B_6_2", url="0"),
self.setup_bookmark(title="a_7_1", website_title="0", url="0"),
self.setup_bookmark(title="A_7_2", website_title="0", url="0"),
self.setup_bookmark(title="b_7_1", website_title="0", url="0"),
self.setup_bookmark(title="B_7_2", website_title="0", url="0"),
self.setup_bookmark(title="", url="0"),
self.setup_bookmark(title="", url="0"),
self.setup_bookmark(title="", url="0"),
self.setup_bookmark(title="", url="0"),
]
return bookmarks