Add read it later functionality (#304)

* Allow marking bookmarks as unread

* Restructure navigation to include preset filters

* Add mark as read action

* Improve description

* Highlight unread bookmarks visually

* Mark bookmarks as read by default

* Add tests

* Implement toread flag in importer

* Implement admin actions

* Add query tests

* Remove untagged link

* Update api docs

* Reduce height of description textarea

Co-authored-by: Sascha Ißbrücker <sascha.issbruecker@gmail.com>

bookmarks/tests/helpers.py

@@ -23,6 +23,7 @@ class BookmarkFactoryMixin:
 
     def setup_bookmark(self,
                        is_archived: bool = False,
+                       unread: bool = False,
                        tags=None,
                        user: User = None,
                        url: str = '',
@@ -49,6 +50,7 @@ class BookmarkFactoryMixin:
             date_modified=timezone.now(),
             owner=user,
             is_archived=is_archived,
+            unread=unread,
             web_archive_snapshot_url=web_archive_snapshot_url,
         )
         bookmark.save()
@@ -95,12 +97,19 @@ class LinkdingApiTestCase(APITestCase):
 
 
 class BookmarkHtmlTag:
-    def __init__(self, href: str = '', title: str = '', description: str = '', add_date: str = '', tags: str = ''):
+    def __init__(self,
+                 href: str = '',
+                 title: str = '',
+                 description: str = '',
+                 add_date: str = '',
+                 tags: str = '',
+                 to_read: bool = False):
         self.href = href
         self.title = title
         self.description = description
         self.add_date = add_date
         self.tags = tags
+        self.to_read = to_read
 
 
 class ImportTestMixin:
@@ -109,7 +118,8 @@ class ImportTestMixin:
         <DT>
         <A {f'HREF="{tag.href}"' if tag.href else ''}
            {f'ADD_DATE="{tag.add_date}"' if tag.add_date else ''}
-           {f'TAGS="{tag.tags}"' if tag.tags else ''}>
+           {f'TAGS="{tag.tags}"' if tag.tags else ''}
+           TOREAD="{1 if tag.to_read else 0}">
            {tag.title if tag.title else ''}
         </A>
         {f'<DD>{tag.description}' if tag.description else ''}

bookmarks/tests/test_bookmark_action_view.py

@@ -84,6 +84,16 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin):
         self.assertEqual(response.status_code, 404)
         self.assertTrue(Bookmark.objects.filter(id=bookmark.id).exists())
 
+    def test_mark_as_read(self):
+        bookmark = self.setup_bookmark(unread=True)
+
+        self.client.post(reverse('bookmarks:action'), {
+            'mark_as_read': [bookmark.id],
+        })
+        bookmark.refresh_from_db()
+
+        self.assertFalse(bookmark.unread)
+
     def test_bulk_archive(self):
         bookmark1 = self.setup_bookmark()
         bookmark2 = self.setup_bookmark()

bookmarks/tests/test_bookmark_archived_view.py

@@ -18,7 +18,7 @@ class BookmarkArchivedViewTestCase(TestCase, BookmarkFactoryMixin):
 
         for bookmark in bookmarks:
             self.assertInHTML(
-                f'<a href="{bookmark.url}" target="{link_target}" rel="noopener">{bookmark.resolved_title}</a>',
+                f'<a href="{bookmark.url}" target="{link_target}" rel="noopener" class="">{bookmark.resolved_title}</a>',
                 html
             )
 
@@ -27,7 +27,7 @@ class BookmarkArchivedViewTestCase(TestCase, BookmarkFactoryMixin):
 
         for bookmark in bookmarks:
             self.assertInHTML(
-                f'<a href="{bookmark.url}" target="{link_target}" rel="noopener">{bookmark.resolved_title}</a>',
+                f'<a href="{bookmark.url}" target="{link_target}" rel="noopener" class="">{bookmark.resolved_title}</a>',
                 html,
                 count=0
             )

bookmarks/tests/test_bookmark_edit_view.py

@@ -20,6 +20,7 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
             'tag_string': 'editedtag1 editedtag2',
             'title': 'edited title',
             'description': 'edited description',
+            'unread': False,
         }
         return {**form_data, **overrides}
 
@@ -35,10 +36,21 @@ 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.unread, form_data['unread'])
         self.assertEqual(bookmark.tags.count(), 2)
         self.assertEqual(bookmark.tags.all()[0].name, 'editedtag1')
         self.assertEqual(bookmark.tags.all()[1].name, 'editedtag2')
 
+    def test_should_mark_bookmark_as_unread(self):
+        bookmark = self.setup_bookmark()
+        form_data = self.create_form_data({'id': bookmark.id, 'unread': True})
+
+        self.client.post(reverse('bookmarks:edit', args=[bookmark.id]), form_data)
+
+        bookmark.refresh_from_db()
+
+        self.assertTrue(bookmark.unread)
+
     def test_should_prefill_bookmark_form_fields(self):
         tag1 = self.setup_tag()
         tag2 = self.setup_tag()
@@ -65,7 +77,7 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
                           'value="{0}" id="id_title">'.format(bookmark.title),
                           html)
 
-        self.assertInHTML('<textarea name="description" cols="40" rows="4" class="form-input" id="id_description">{0}'
+        self.assertInHTML('<textarea name="description" cols="40" rows="2" class="form-input" id="id_description">{0}'
                           '</textarea>'.format(bookmark.description),
                           html)
 

bookmarks/tests/test_bookmark_index_view.py

@@ -18,7 +18,7 @@ class BookmarkIndexViewTestCase(TestCase, BookmarkFactoryMixin):
 
         for bookmark in bookmarks:
             self.assertInHTML(
-                f'<a href="{bookmark.url}" target="{link_target}" rel="noopener">{bookmark.resolved_title}</a>',
+                f'<a href="{bookmark.url}" target="{link_target}" rel="noopener" class="">{bookmark.resolved_title}</a>',
                 html
             )
 
@@ -27,7 +27,7 @@ class BookmarkIndexViewTestCase(TestCase, BookmarkFactoryMixin):
 
         for bookmark in bookmarks:
             self.assertInHTML(
-                f'<a href="{bookmark.url}" target="{link_target}" rel="noopener">{bookmark.resolved_title}</a>',
+                f'<a href="{bookmark.url}" target="{link_target}" rel="noopener" class="">{bookmark.resolved_title}</a>',
                 html,
                 count=0
             )
@@ -156,8 +156,3 @@ class BookmarkIndexViewTestCase(TestCase, BookmarkFactoryMixin):
         response = self.client.get(reverse('bookmarks:index'))
 
         self.assertVisibleBookmarks(response, visible_bookmarks, '_self')
-
-    def test_should_show_link_for_untagged_bookmarks(self):
-        response = self.client.get(reverse('bookmarks:index'))
-
-        self.assertContains(response, '<a href="?q=!untagged" class="text-gray text-sm">Show Untagged</a>')

bookmarks/tests/test_bookmark_new_view.py

@@ -19,6 +19,7 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
             'tag_string': 'tag1 tag2',
             'title': 'test title',
             'description': 'test description',
+            'unread': False,
             'auto_close': '',
         }
         return {**form_data, **overrides}
@@ -35,10 +36,21 @@ 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.unread, form_data['unread'])
         self.assertEqual(bookmark.tags.count(), 2)
         self.assertEqual(bookmark.tags.all()[0].name, 'tag1')
         self.assertEqual(bookmark.tags.all()[1].name, 'tag2')
 
+    def test_should_create_new_unread_bookmark(self):
+        form_data = self.create_form_data({'unread': True})
+
+        self.client.post(reverse('bookmarks:new'), form_data)
+
+        self.assertEqual(Bookmark.objects.count(), 1)
+
+        bookmark = Bookmark.objects.first()
+        self.assertTrue(bookmark.unread)
+
     def test_should_prefill_url_from_url_parameter(self):
         response = self.client.get(reverse('bookmarks:new') + '?url=http://example.com')
         html = response.content.decode()
@@ -64,7 +76,7 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
         response = self.client.get(reverse('bookmarks:new'))
         html = response.content.decode()
 
-        self.assertInHTML('<input type="hidden" name="auto_close" id="id_auto_close">',html)
+        self.assertInHTML('<input type="hidden" name="auto_close" id="id_auto_close">', html)
 
     def test_should_redirect_to_index_view(self):
         form_data = self.create_form_data()

bookmarks/tests/test_bookmarks_api.py

@@ -35,6 +35,7 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
             expectation['website_title'] = bookmark.website_title
             expectation['website_description'] = bookmark.website_description
             expectation['is_archived'] = bookmark.is_archived
+            expectation['unread'] = bookmark.unread
             expectation['tag_names'] = tag_names
             expectation['date_added'] = bookmark.date_added.isoformat().replace('+00:00', 'Z')
             expectation['date_modified'] = bookmark.date_modified.isoformat().replace('+00:00', 'Z')
@@ -51,6 +52,7 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
             'title': 'Test title',
             'description': 'Test description',
             'is_archived': False,
+            'unread': False,
             'tag_names': ['tag1', 'tag2']
         }
         self.post(reverse('bookmarks:bookmark-list'), data, status.HTTP_201_CREATED)
@@ -59,6 +61,7 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
         self.assertEqual(bookmark.title, data['title'])
         self.assertEqual(bookmark.description, data['description'])
         self.assertFalse(bookmark.is_archived, data['is_archived'])
+        self.assertFalse(bookmark.unread, data['unread'])
         self.assertEqual(bookmark.tags.count(), 2)
         self.assertEqual(bookmark.tags.filter(name=data['tag_names'][0]).count(), 1)
         self.assertEqual(bookmark.tags.filter(name=data['tag_names'][1]).count(), 1)
@@ -113,6 +116,31 @@ 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_is_not_archived_by_default(self):
+        data = {
+            'url': 'https://example.com/',
+        }
+        self.post(reverse('bookmarks:bookmark-list'), data, status.HTTP_201_CREATED)
+        bookmark = Bookmark.objects.get(url=data['url'])
+        self.assertFalse(bookmark.is_archived)
+
+    def test_create_unread_bookmark(self):
+        data = {
+            'url': 'https://example.com/',
+            'unread': True,
+        }
+        self.post(reverse('bookmarks:bookmark-list'), data, status.HTTP_201_CREATED)
+        bookmark = Bookmark.objects.get(url=data['url'])
+        self.assertTrue(bookmark.unread)
+
+    def test_create_bookmark_is_not_unread_by_default(self):
+        data = {
+            'url': 'https://example.com/',
+        }
+        self.post(reverse('bookmarks:bookmark-list'), data, status.HTTP_201_CREATED)
+        bookmark = Bookmark.objects.get(url=data['url'])
+        self.assertFalse(bookmark.unread)
+
     def test_create_bookmark_minimal_payload_does_not_archive(self):
         data = {'url': 'https://example.com/'}
         self.post(reverse('bookmarks:bookmark-list'), data, status.HTTP_201_CREATED)
@@ -165,6 +193,18 @@ class BookmarksApiTestCase(LinkdingApiTestCase, BookmarkFactoryMixin):
         self.bookmark1.refresh_from_db()
         self.assertEqual(self.bookmark1.description, data['description'])
 
+        data = {'unread': True}
+        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.assertTrue(self.bookmark1.unread)
+
+        data = {'unread': False}
+        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.assertFalse(self.bookmark1.unread)
+
         data = {'tag_names': ['updated-tag-1', 'updated-tag-2']}
         url = reverse('bookmarks:bookmark-detail', args=[self.bookmark1.id])
         self.patch(url, data, expected_status_code=status.HTTP_200_OK)

bookmarks/tests/test_bookmarks_list_tag.py

@@ -10,9 +10,14 @@ from bookmarks.tests.helpers import BookmarkFactoryMixin
 
 class BookmarkListTagTest(TestCase, BookmarkFactoryMixin):
 
-    def assertBookmarksLink(self, html: str, bookmark: Bookmark, link_target: str = '_blank'):
+    def assertBookmarksLink(self, html: str, bookmark: Bookmark, link_target: str = '_blank', unread: bool = False):
         self.assertInHTML(
-            f'<a href="{bookmark.url}" target="{link_target}" rel="noopener">{bookmark.resolved_title}</a>',
+            f'''
+            <a href="{bookmark.url}" 
+                target="{link_target}" 
+                rel="noopener" 
+                class="{'text-italic' if unread else ''}">{bookmark.resolved_title}</a>
+            ''',
             html
         )
 
@@ -114,6 +119,15 @@ class BookmarkListTagTest(TestCase, BookmarkFactoryMixin):
 
         self.assertBookmarksLink(html, bookmark, link_target='_self')
 
+    def test_bookmark_link_target_should_respect_unread_flag(self):
+        bookmark = self.setup_bookmark()
+        html = self.render_template_with_link_target([bookmark], '_self')
+        self.assertBookmarksLink(html, bookmark, link_target='_self', unread=False)
+
+        bookmark = self.setup_bookmark(unread=True)
+        html = self.render_template_with_link_target([bookmark], '_self')
+        self.assertBookmarksLink(html, bookmark, link_target='_self', unread=True)
+
     def test_web_archive_link_target_should_be_blank_by_default(self):
         bookmark = self.setup_bookmark()
         bookmark.date_added = timezone.now() - relativedelta(days=8)

bookmarks/tests/test_importer.py

@@ -21,6 +21,7 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
             self.assertEqual(bookmark.title, html_tag.title)
             self.assertEqual(bookmark.description, html_tag.description)
             self.assertEqual(bookmark.date_added, parse_timestamp(html_tag.add_date))
+            self.assertEqual(bookmark.unread, html_tag.to_read)
 
             tag_names = parse_tag_string(html_tag.tags)
 
@@ -34,52 +35,16 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
         html_tags = [
             BookmarkHtmlTag(href='https://example.com', title='Example title', description='Example description',
                             add_date='1', tags='example-tag'),
-            BookmarkHtmlTag(href='https://foo.com', title='Foo title', description='',
+            BookmarkHtmlTag(href='https://example.com/foo', title='Foo title', description='',
                             add_date='2', tags=''),
-            BookmarkHtmlTag(href='https://bar.com', title='Bar title', description='Bar description',
+            BookmarkHtmlTag(href='https://example.com/bar', title='Bar title', description='Bar description',
                             add_date='3', tags='bar-tag, other-tag'),
+            BookmarkHtmlTag(href='https://example.com/baz', title='Baz title', description='Baz description',
+                            add_date='4', to_read=True),
         ]
         import_html = self.render_html(tags=html_tags)
         result = import_netscape_html(import_html, self.get_or_create_test_user())
 
-        # Check result
-        self.assertEqual(result.total, 3)
-        self.assertEqual(result.success, 3)
-        self.assertEqual(result.failed, 0)
-
-        # Check bookmarks
-        bookmarks = Bookmark.objects.all()
-        self.assertEqual(len(bookmarks), 3)
-        self.assertBookmarksImported(html_tags)
-
-    def test_synchronize(self):
-        # Initial import
-        html_tags = [
-            BookmarkHtmlTag(href='https://example.com', title='Example title', description='Example description',
-                            add_date='1', tags='example-tag'),
-            BookmarkHtmlTag(href='https://foo.com', title='Foo title', description='',
-                            add_date='2', tags=''),
-            BookmarkHtmlTag(href='https://bar.com', title='Bar title', description='Bar description',
-                            add_date='3', tags='bar-tag, other-tag'),
-        ]
-        import_html = self.render_html(tags=html_tags)
-        import_netscape_html(import_html, self.get_or_create_test_user())
-
-        # Change data, add some new data
-        html_tags = [
-            BookmarkHtmlTag(href='https://example.com', title='Updated Example title',
-                            description='Updated Example description', add_date='111', tags='updated-example-tag'),
-            BookmarkHtmlTag(href='https://foo.com', title='Updated Foo title', description='Updated Foo description',
-                            add_date='222', tags='new-tag'),
-            BookmarkHtmlTag(href='https://bar.com', title='Updated Bar title', description='Updated Bar description',
-                            add_date='333', tags='updated-bar-tag, updated-other-tag'),
-            BookmarkHtmlTag(href='https://baz.com', add_date='444', tags='baz-tag')
-        ]
-
-        # Import updated data
-        import_html = self.render_html(tags=html_tags)
-        result = import_netscape_html(import_html, self.get_or_create_test_user())
-
         # Check result
         self.assertEqual(result.total, 4)
         self.assertEqual(result.success, 4)
@@ -90,6 +55,48 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
         self.assertEqual(len(bookmarks), 4)
         self.assertBookmarksImported(html_tags)
 
+    def test_synchronize(self):
+        # Initial import
+        html_tags = [
+            BookmarkHtmlTag(href='https://example.com', title='Example title', description='Example description',
+                            add_date='1', tags='example-tag'),
+            BookmarkHtmlTag(href='https://example.com/foo', title='Foo title', description='',
+                            add_date='2', tags=''),
+            BookmarkHtmlTag(href='https://example.com/bar', title='Bar title', description='Bar description',
+                            add_date='3', tags='bar-tag, other-tag'),
+            BookmarkHtmlTag(href='https://example.com/unread', title='Unread title', description='Unread description',
+                            add_date='3', to_read=True),
+        ]
+        import_html = self.render_html(tags=html_tags)
+        import_netscape_html(import_html, self.get_or_create_test_user())
+
+        # Change data, add some new data
+        html_tags = [
+            BookmarkHtmlTag(href='https://example.com', title='Updated Example title',
+                            description='Updated Example description', add_date='111', tags='updated-example-tag'),
+            BookmarkHtmlTag(href='https://example.com/foo', title='Updated Foo title', description='Updated Foo description',
+                            add_date='222', tags='new-tag'),
+            BookmarkHtmlTag(href='https://example.com/bar', title='Updated Bar title', description='Updated Bar description',
+                            add_date='333', tags='updated-bar-tag, updated-other-tag'),
+            BookmarkHtmlTag(href='https://example.com/unread', title='Unread title', description='Unread description',
+                            add_date='3', to_read=False),
+            BookmarkHtmlTag(href='https://baz.com', add_date='444', tags='baz-tag')
+        ]
+
+        # Import updated data
+        import_html = self.render_html(tags=html_tags)
+        result = import_netscape_html(import_html, self.get_or_create_test_user())
+
+        # Check result
+        self.assertEqual(result.total, 5)
+        self.assertEqual(result.success, 5)
+        self.assertEqual(result.failed, 0)
+
+        # Check bookmarks
+        bookmarks = Bookmark.objects.all()
+        self.assertEqual(len(bookmarks), 5)
+        self.assertBookmarksImported(html_tags)
+
     def test_import_with_some_invalid_bookmarks(self):
         html_tags = [
             BookmarkHtmlTag(href='https://example.com'),

bookmarks/tests/test_parser.py

@@ -17,15 +17,18 @@ class ParserTestCase(TestCase, ImportTestMixin):
             self.assertEqual(bookmark.date_added, html_tag.add_date)
             self.assertEqual(bookmark.description, html_tag.description)
             self.assertEqual(bookmark.tag_string, html_tag.tags)
+            self.assertEqual(bookmark.to_read, html_tag.to_read)
 
     def test_parse_bookmarks(self):
         html_tags = [
             BookmarkHtmlTag(href='https://example.com', title='Example title', description='Example description',
                             add_date='1', tags='example-tag'),
-            BookmarkHtmlTag(href='https://foo.com', title='Foo title', description='',
+            BookmarkHtmlTag(href='https://example.com/foo', title='Foo title', description='',
                             add_date='2', tags=''),
-            BookmarkHtmlTag(href='https://bar.com', title='Bar title', description='Bar description',
+            BookmarkHtmlTag(href='https://example.com/bar', title='Bar title', description='Bar description',
                             add_date='3', tags='bar-tag, other-tag'),
+            BookmarkHtmlTag(href='https://example.com/baz', title='Baz title', description='Baz description',
+                            add_date='3', to_read=True),
         ]
         html = self.render_html(html_tags)
         bookmarks = parse(html)

bookmarks/tests/test_queries.py

@@ -343,6 +343,32 @@ class QueriesTestCase(TestCase, BookmarkFactoryMixin):
         query = queries.query_archived_bookmarks(self.user, f'!untagged #{tag.name}')
         self.assertCountEqual(list(query), [])
 
+    def test_query_bookmarks_unread_should_return_unread_bookmarks_only(self):
+        unread_bookmarks = [
+            self.setup_bookmark(unread=True),
+            self.setup_bookmark(unread=True),
+            self.setup_bookmark(unread=True),
+        ]
+        self.setup_bookmark()
+        self.setup_bookmark()
+        self.setup_bookmark()
+
+        query = queries.query_bookmarks(self.user, '!unread')
+        self.assertCountEqual(list(query), unread_bookmarks)
+
+    def test_query_archived_bookmarks_unread_should_return_unread_bookmarks_only(self):
+        unread_bookmarks = [
+            self.setup_bookmark(is_archived=True, unread=True),
+            self.setup_bookmark(is_archived=True, unread=True),
+            self.setup_bookmark(is_archived=True, unread=True),
+        ]
+        self.setup_bookmark(is_archived=True)
+        self.setup_bookmark(is_archived=True)
+        self.setup_bookmark(is_archived=True)
+
+        query = queries.query_archived_bookmarks(self.user, '!unread')
+        self.assertCountEqual(list(query), unread_bookmarks)
+
     def test_query_bookmark_tags_should_return_all_tags_for_empty_query(self):
         self.setup_tag_search_data()