Add notes to bookmarks (#472)

* Add basic bookmark notes

* Add bookmark list JS to shared bookmarks page

* Allow testing through ngrok

* Improve CSS

* Set notes through API

* Improve notes editing

* Improve notes icon

* Remove transitions for now

* Update keyboard shortcut

* Add bookmark list tests

* Add setting for showing notes permanently

* Add test for toggling notes

* Update API docs

* Allow searching for notes content

* Skip test
This commit is contained in:
Sascha Ißbrücker
2023-05-20 11:54:26 +02:00
committed by GitHub
parent 67ee896a46
commit 43115fd8f2
31 changed files with 609 additions and 144 deletions

View File

@@ -30,6 +30,7 @@ class BookmarkFactoryMixin:
url: str = '',
title: str = '',
description: str = '',
notes: str = '',
website_title: str = '',
website_description: str = '',
web_archive_snapshot_url: str = '',
@@ -48,6 +49,7 @@ class BookmarkFactoryMixin:
url=url,
title=title,
description=description,
notes=notes,
website_title=website_title,
website_description=website_description,
date_added=timezone.now(),

View File

@@ -20,6 +20,7 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
'tag_string': 'editedtag1 editedtag2',
'title': 'edited title',
'description': 'edited description',
'notes': 'edited notes',
'unread': False,
'shared': False,
}
@@ -37,6 +38,7 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
self.assertEqual(bookmark.url, form_data['url'])
self.assertEqual(bookmark.title, form_data['title'])
self.assertEqual(bookmark.description, form_data['description'])
self.assertEqual(bookmark.notes, form_data['notes'])
self.assertEqual(bookmark.unread, form_data['unread'])
self.assertEqual(bookmark.shared, form_data['shared'])
self.assertEqual(bookmark.tags.count(), 2)
@@ -74,7 +76,8 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
tag1 = self.setup_tag()
tag2 = self.setup_tag()
bookmark = self.setup_bookmark(tags=[tag1, tag2], title='edited title', description='edited description',
website_title='website title', website_description='website description')
notes='edited notes', website_title='website title',
website_description='website description')
response = self.client.get(reverse('bookmarks:edit', args=[bookmark.id]))
html = response.content.decode()
@@ -101,6 +104,12 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
</textarea>
''', html)
self.assertInHTML(f'''
<textarea name="notes" cols="40" rows="8" class="form-input" id="id_notes">
{bookmark.notes}
</textarea>
''', html)
self.assertInHTML(f'''
<input type="hidden" name="website_title" id="id_website_title"
value="{bookmark.website_title}">
@@ -184,3 +193,15 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
<span>Share</span>
</label>
''', html, count=1)
def test_should_hide_notes_if_there_are_no_notes(self):
bookmark = self.setup_bookmark()
response = self.client.get(reverse('bookmarks:edit', args=[bookmark.id]))
self.assertContains(response, '<details class="notes">', count=1)
def test_should_show_notes_if_there_are_notes(self):
bookmark = self.setup_bookmark(notes='test notes')
response = self.client.get(reverse('bookmarks:edit', args=[bookmark.id]))
self.assertContains(response, '<details class="notes" open>', count=1)

View File

@@ -227,8 +227,7 @@ class BookmarkIndexViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin):
return_url = urllib.parse.quote_plus(url)
self.assertInHTML(f'''
<a href="{edit_url}?return_url={return_url}"
class="btn btn-link btn-sm">Edit</a>
<a href="{edit_url}?return_url={return_url}">Edit</a>
''', html)
# with query params
@@ -239,6 +238,5 @@ class BookmarkIndexViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin):
return_url = urllib.parse.quote_plus(url)
self.assertInHTML(f'''
<a href="{edit_url}?return_url={return_url}"
class="btn btn-link btn-sm">Edit</a>
<a href="{edit_url}?return_url={return_url}">Edit</a>
''', html)

View File

@@ -19,6 +19,7 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
'tag_string': 'tag1 tag2',
'title': 'test title',
'description': 'test description',
'notes': 'test notes',
'unread': False,
'shared': False,
'auto_close': '',
@@ -37,6 +38,7 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
self.assertEqual(bookmark.url, form_data['url'])
self.assertEqual(bookmark.title, form_data['title'])
self.assertEqual(bookmark.description, form_data['description'])
self.assertEqual(bookmark.notes, form_data['notes'])
self.assertEqual(bookmark.unread, form_data['unread'])
self.assertEqual(bookmark.shared, form_data['shared'])
self.assertEqual(bookmark.tags.count(), 2)
@@ -138,3 +140,9 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
<span>Share</span>
</label>
''', html, count=1)
def test_should_hide_notes_if_there_are_no_notes(self):
bookmark = self.setup_bookmark()
response = self.client.get(reverse('bookmarks:edit', args=[bookmark.id]))
self.assertContains(response, '<details class="notes">', count=1)

View File

@@ -20,7 +20,7 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
self.client.credentials(HTTP_AUTHORIZATION='Token ' + self.api_token.key)
self.tag1 = self.setup_tag()
self.tag2 = self.setup_tag()
self.bookmark1 = self.setup_bookmark(tags=[self.tag1, self.tag2])
self.bookmark1 = self.setup_bookmark(tags=[self.tag1, self.tag2], notes='Test notes')
self.bookmark2 = self.setup_bookmark()
self.bookmark3 = self.setup_bookmark(tags=[self.tag2])
self.archived_bookmark1 = self.setup_bookmark(is_archived=True, tags=[self.tag1, self.tag2])
@@ -36,6 +36,7 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
expectation['url'] = bookmark.url
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['is_archived'] = bookmark.is_archived
@@ -134,6 +135,7 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
'url': 'https://example.com/',
'title': 'Test title',
'description': 'Test description',
'notes': 'Test notes',
'is_archived': False,
'unread': False,
'shared': False,
@@ -144,6 +146,7 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
self.assertEqual(bookmark.url, data['url'])
self.assertEqual(bookmark.title, data['title'])
self.assertEqual(bookmark.description, data['description'])
self.assertEqual(bookmark.notes, data['notes'])
self.assertFalse(bookmark.is_archived, data['is_archived'])
self.assertFalse(bookmark.unread, data['unread'])
self.assertFalse(bookmark.shared, data['shared'])
@@ -157,6 +160,7 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
'url': original_bookmark.url,
'title': 'Updated title',
'description': 'Updated description',
'notes': 'Updated notes',
'unread': True,
'shared': True,
'is_archived': True,
@@ -168,6 +172,7 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
self.assertEqual(bookmark.url, data['url'])
self.assertEqual(bookmark.title, data['title'])
self.assertEqual(bookmark.description, data['description'])
self.assertEqual(bookmark.notes, data['notes'])
# Saving a duplicate bookmark should not modify archive flag - right?
self.assertFalse(bookmark.is_archived)
self.assertEqual(bookmark.unread, data['unread'])
@@ -265,6 +270,7 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
self.assertEqual(updated_bookmark.url, data['url'])
self.assertEqual(updated_bookmark.title, '')
self.assertEqual(updated_bookmark.description, '')
self.assertEqual(updated_bookmark.notes, '')
self.assertEqual(updated_bookmark.tag_names, [])
def test_update_bookmark_unread_flag(self):
@@ -300,6 +306,12 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
self.bookmark1.refresh_from_db()
self.assertEqual(self.bookmark1.description, data['description'])
data = {'notes': 'Updated notes'}
url = reverse('bookmarks:bookmark-detail', args=[self.bookmark1.id])
self.patch(url, data, expected_status_code=status.HTTP_200_OK)
self.bookmark1.refresh_from_db()
self.assertEqual(self.bookmark1.notes, data['notes'])
data = {'unread': True}
url = reverse('bookmarks:bookmark-detail', args=[self.bookmark1.id])
self.patch(url, data, expected_status_code=status.HTTP_200_OK)

View File

@@ -24,22 +24,22 @@ class BookmarkListTagTest(TestCase, BookmarkFactoryMixin):
def assertDateLabel(self, html: str, label_content: str):
self.assertInHTML(f'''
<span class="date-label text-gray text-sm">
<span>
<span>{label_content}</span>
</span>
<span class="text-gray text-sm">|</span>
<span class="separator">|</span>
''', html)
def assertWebArchiveLink(self, html: str, label_content: str, url: str, link_target: str = '_blank'):
self.assertInHTML(f'''
<span class="date-label text-gray text-sm">
<span>
<a href="{url}"
title="Show snapshot on the Internet Archive Wayback Machine" target="{link_target}" rel="noopener">
<span>{label_content}</span>
<span>∞</span>
</a>
</span>
<span class="text-gray text-sm">|</span>
<span class="separator">|</span>
''', html)
def assertBookmarkActions(self, html: str, bookmark: Bookmark):
@@ -52,8 +52,7 @@ class BookmarkListTagTest(TestCase, BookmarkFactoryMixin):
# Edit link
edit_url = reverse('bookmarks:edit', args=[bookmark.id])
self.assertInHTML(f'''
<a href="{edit_url}?return_url=/test"
class="btn btn-link btn-sm">Edit</a>
<a href="{edit_url}?return_url=/test">Edit</a>
''', html, count=count)
# Archive link
self.assertInHTML(f'''
@@ -74,8 +73,8 @@ class BookmarkListTagTest(TestCase, BookmarkFactoryMixin):
def assertShareInfoCount(self, html: str, bookmark: Bookmark, count=1):
self.assertInHTML(f'''
<span class="text-gray text-sm">Shared by
<a class="text-gray" href="?user={bookmark.owner.username}">{bookmark.owner.username}</a>
<span>Shared by
<a href="?user={bookmark.owner.username}">{bookmark.owner.username}</a>
</span>
''', html, count=count)
@@ -93,21 +92,43 @@ class BookmarkListTagTest(TestCase, BookmarkFactoryMixin):
def assertBookmarkURLCount(self, html: str, bookmark: Bookmark, link_target: str = '_blank', count=0):
self.assertInHTML(f'''
<div class="url-path truncate">
<a href="{ bookmark.url }" target="{ link_target }" rel="noopener"
<a href="{bookmark.url}" target="{link_target}" rel="noopener"
class="url-display text-sm">
{ bookmark.url }
{bookmark.url}
</a>
</div>
''', html, count)
def assertBookmarkURLVisible(self, html: str, bookmark: Bookmark):
self.assertBookmarkURLCount(html, bookmark, count=1)
def assertBookmarkURLHidden(self, html: str, bookmark: Bookmark, link_target: str = '_blank'):
self.assertBookmarkURLCount(html, bookmark, count=0)
def assertNotes(self, html: str, notes_html: str, count=1):
self.assertInHTML(f'''
<div class="notes bg-gray text-gray-dark">
<div class="notes-content">
{notes_html}
</div>
</div>
''', html, count=count)
def assertNotesToggle(self, html: str, count=1):
self.assertInHTML(f'''
<button class="btn btn-link btn-sm toggle-notes" title="Toggle notes">
<svg xmlns="http://www.w3.org/2000/svg" class="icon icon-tabler icon-tabler-notes" width="16" height="16"
viewBox="0 0 24 24" stroke-width="2" stroke="currentColor" fill="none" stroke-linecap="round"
stroke-linejoin="round">
<path stroke="none" d="M0 0h24v24H0z" fill="none"></path>
<path d="M5 3m0 2a2 2 0 0 1 2 -2h10a2 2 0 0 1 2 2v14a2 2 0 0 1 -2 2h-10a2 2 0 0 1 -2 -2z"></path>
<path d="M9 7l6 0"></path>
<path d="M9 11l6 0"></path>
<path d="M9 15l4 0"></path>
</svg>
<span>Notes</span>
</button>
''', html, count=count)
def render_template(self, bookmarks: [Bookmark], template: Template, url: str = '/test') -> str:
rf = RequestFactory()
@@ -237,8 +258,8 @@ class BookmarkListTagTest(TestCase, BookmarkFactoryMixin):
html = self.render_default_template([bookmark], url='/test?q=foo')
self.assertInHTML(f'''
<span class="text-gray text-sm">Shared by
<a class="text-gray" href="?q=foo&user={bookmark.owner.username}">{bookmark.owner.username}</a>
<span>Shared by
<a href="?q=foo&user={bookmark.owner.username}">{bookmark.owner.username}</a>
</span>
''', html)
@@ -279,8 +300,8 @@ class BookmarkListTagTest(TestCase, BookmarkFactoryMixin):
bookmark = self.setup_bookmark()
html = self.render_default_template([bookmark])
self.assertBookmarkURLHidden(html,bookmark)
self.assertBookmarkURLHidden(html, bookmark)
def test_show_bookmark_url_when_enabled(self):
profile = self.get_or_create_test_user().profile
profile.display_url = True
@@ -289,7 +310,7 @@ class BookmarkListTagTest(TestCase, BookmarkFactoryMixin):
bookmark = self.setup_bookmark()
html = self.render_default_template([bookmark])
self.assertBookmarkURLVisible(html,bookmark)
self.assertBookmarkURLVisible(html, bookmark)
def test_hide_bookmark_url_when_disabled(self):
profile = self.get_or_create_test_user().profile
@@ -299,6 +320,85 @@ class BookmarkListTagTest(TestCase, BookmarkFactoryMixin):
bookmark = self.setup_bookmark()
html = self.render_default_template([bookmark])
self.assertBookmarkURLHidden(html,bookmark)
self.assertBookmarkURLHidden(html, bookmark)
def test_without_notes(self):
bookmark = self.setup_bookmark()
html = self.render_default_template([bookmark])
self.assertNotes(html, '', 0)
self.assertNotesToggle(html, 0)
def test_with_notes(self):
bookmark = self.setup_bookmark(notes='Test note')
html = self.render_default_template([bookmark])
note_html = '<p>Test note</p>'
self.assertNotes(html, note_html, 1)
def test_note_renders_markdown(self):
bookmark = self.setup_bookmark(notes='**Example:** `print("Hello world!")`')
html = self.render_default_template([bookmark])
note_html = '<p><strong>Example:</strong> <code>print("Hello world!")</code></p>'
self.assertNotes(html, note_html, 1)
def test_note_cleans_html(self):
bookmark = self.setup_bookmark(notes='<script>alert("test")</script>')
html = self.render_default_template([bookmark])
note_html = '&lt;script&gt;alert("test")&lt;/script&gt;'
self.assertNotes(html, note_html, 1)
def test_notes_are_hidden_initially_by_default(self):
html = self.render_default_template([])
self.assertInHTML("""
<ul class="bookmark-list"></ul>
""", html)
def test_notes_are_hidden_initially_with_permanent_notes_disabled(self):
profile = self.get_or_create_test_user().profile
profile.permanent_notes = False
profile.save()
html = self.render_default_template([])
self.assertInHTML("""
<ul class="bookmark-list"></ul>
""", html)
def test_notes_are_visible_initially_with_permanent_notes_enabled(self):
profile = self.get_or_create_test_user().profile
profile.permanent_notes = True
profile.save()
html = self.render_default_template([])
self.assertInHTML("""
<ul class="bookmark-list show-notes"></ul>
""", html)
def test_toggle_notes_is_visible_by_default(self):
bookmark = self.setup_bookmark(notes='Test note')
html = self.render_default_template([bookmark])
self.assertNotesToggle(html, 1)
def test_toggle_notes_is_visible_with_permanent_notes_disabled(self):
profile = self.get_or_create_test_user().profile
profile.permanent_notes = False
profile.save()
bookmark = self.setup_bookmark(notes='Test note')
html = self.render_default_template([bookmark])
self.assertNotesToggle(html, 1)
def test_toggle_notes_is_hidden_with_permanent_notes_enabled(self):
profile = self.get_or_create_test_user().profile
profile.permanent_notes = True
profile.save()
bookmark = self.setup_bookmark(notes='Test note')
html = self.render_default_template([bookmark])
self.assertNotesToggle(html, 0)

View File

@@ -46,6 +46,7 @@ class BookmarkServiceTestCase(TestCase, BookmarkFactoryMixin):
bookmark_data = Bookmark(url='https://example.com',
title='Updated Title',
description='Updated description',
notes='Updated notes',
unread=True,
shared=True,
is_archived=True)
@@ -55,6 +56,7 @@ class BookmarkServiceTestCase(TestCase, BookmarkFactoryMixin):
self.assertEqual(updated_bookmark.id, original_bookmark.id)
self.assertEqual(updated_bookmark.title, bookmark_data.title)
self.assertEqual(updated_bookmark.description, bookmark_data.description)
self.assertEqual(updated_bookmark.notes, bookmark_data.notes)
self.assertEqual(updated_bookmark.unread, bookmark_data.unread)
self.assertEqual(updated_bookmark.shared, bookmark_data.shared)
# Saving a duplicate bookmark should not modify archive flag - right?

View File

@@ -32,6 +32,8 @@ class QueriesTestCase(TestCase, BookmarkFactoryMixin):
self.setup_bookmark(title=random_sentence(including_word='TERM1')),
self.setup_bookmark(description=random_sentence(including_word='term1')),
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')),
@@ -92,6 +94,8 @@ class QueriesTestCase(TestCase, BookmarkFactoryMixin):
self.setup_bookmark(title=random_sentence(including_word='TERM1'), tags=[self.setup_tag()]),
self.setup_bookmark(description=random_sentence(including_word='term1'), tags=[self.setup_tag()]),
self.setup_bookmark(description=random_sentence(including_word='TERM1'), tags=[self.setup_tag()]),
self.setup_bookmark(notes=random_sentence(including_word='term1'), tags=[self.setup_tag()]),
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()]),

View File

@@ -30,6 +30,7 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin):
'enable_favicons': False,
'tag_search': UserProfile.TAG_SEARCH_STRICT,
'display_url': False,
'permanent_notes': False,
}
return {**form_data, **overrides}
@@ -56,6 +57,7 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin):
'enable_favicons': True,
'tag_search': UserProfile.TAG_SEARCH_LAX,
'display_url': True,
'permanent_notes': True,
}
response = self.client.post(reverse('bookmarks:settings.general'), form_data)
html = response.content.decode()
@@ -71,6 +73,7 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin):
self.assertEqual(self.user.profile.enable_favicons, form_data['enable_favicons'])
self.assertEqual(self.user.profile.tag_search, form_data['tag_search'])
self.assertEqual(self.user.profile.display_url, form_data['display_url'])
self.assertEqual(self.user.profile.permanent_notes, form_data['permanent_notes'])
self.assertInHTML('''
<p class="form-input-hint">Profile updated</p>
''', html)