Allow saving search preferences (#540)

* Add indicator for modified filters

* Rename shared filter values

* Add update search preferences handler

* Separate search and preferences forms

* Properly initialize bookmark search from get or post

* Add tests for applying search preferences

* Implement saving search preferences

* Remove bookmark search query alias

* Use search preferences as default

* Only show save button for authenticated users

* Only show modified indicator if preferences are modified

* Fix overriding search preferences

* Add missing migration
This commit is contained in:
Sascha Ißbrücker
2023-10-01 21:22:44 +02:00
committed by GitHub
parent 4a2642f16c
commit 41f79e35a0
22 changed files with 1094 additions and 442 deletions

View File

@@ -5,7 +5,7 @@ from django.contrib.auth.models import User
from django.test import TestCase
from django.urls import reverse
from bookmarks.models import Bookmark, Tag, UserProfile
from bookmarks.models import Bookmark, BookmarkSearch, Tag, UserProfile
from bookmarks.tests.helpers import BookmarkFactoryMixin, HtmlTestMixin, collapse_whitespace
@@ -16,38 +16,51 @@ class BookmarkArchivedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin
self.client.force_login(user)
def assertVisibleBookmarks(self, response, bookmarks: List[Bookmark], link_target: str = '_blank'):
html = response.content.decode()
self.assertContains(response, '<li ld-bookmark-item>', count=len(bookmarks))
soup = self.make_soup(response.content.decode())
bookmark_list = soup.select_one(f'ul.bookmark-list[data-bookmarks-total="{len(bookmarks)}"]')
self.assertIsNotNone(bookmark_list)
bookmark_items = bookmark_list.select('li[ld-bookmark-item]')
self.assertEqual(len(bookmark_items), len(bookmarks))
for bookmark in bookmarks:
self.assertInHTML(
f'<a href="{bookmark.url}" target="{link_target}" rel="noopener">{bookmark.resolved_title}</a>',
html
)
bookmark_item = bookmark_list.select_one(
f'li[ld-bookmark-item] a[href="{bookmark.url}"][target="{link_target}"]')
self.assertIsNotNone(bookmark_item)
def assertInvisibleBookmarks(self, response, bookmarks: List[Bookmark], link_target: str = '_blank'):
html = response.content.decode()
soup = self.make_soup(response.content.decode())
for bookmark in bookmarks:
self.assertInHTML(
f'<a href="{bookmark.url}" target="{link_target}" rel="noopener">{bookmark.resolved_title}</a>',
html,
count=0
)
bookmark_item = soup.select_one(
f'li[ld-bookmark-item] a[href="{bookmark.url}"][target="{link_target}"]')
self.assertIsNone(bookmark_item)
def assertVisibleTags(self, response, tags: List[Tag]):
self.assertContains(response, 'data-is-tag-item', count=len(tags))
soup = self.make_soup(response.content.decode())
tag_cloud = soup.select_one('div.tag-cloud')
self.assertIsNotNone(tag_cloud)
tag_items = tag_cloud.select('a[data-is-tag-item]')
self.assertEqual(len(tag_items), len(tags))
tag_item_names = [tag_item.text.strip() for tag_item in tag_items]
for tag in tags:
self.assertContains(response, tag.name)
self.assertTrue(tag.name in tag_item_names)
def assertInvisibleTags(self, response, tags: List[Tag]):
soup = self.make_soup(response.content.decode())
tag_items = soup.select('a[data-is-tag-item]')
tag_item_names = [tag_item.text.strip() for tag_item in tag_items]
for tag in tags:
self.assertNotContains(response, tag.name)
self.assertFalse(tag.name in tag_item_names)
def assertSelectedTags(self, response, tags: List[Tag]):
soup = self.make_soup(response.content.decode())
selected_tags = soup.select('p.selected-tags')[0]
selected_tags = soup.select_one('p.selected-tags')
self.assertIsNotNone(selected_tags)
tag_list = selected_tags.select('a')
@@ -73,67 +86,36 @@ class BookmarkArchivedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin
def test_should_list_archived_and_user_owned_bookmarks(self):
other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
visible_bookmarks = [
self.setup_bookmark(is_archived=True),
self.setup_bookmark(is_archived=True),
self.setup_bookmark(is_archived=True)
]
visible_bookmarks = self.setup_numbered_bookmarks(3, archived=True)
invisible_bookmarks = [
self.setup_bookmark(is_archived=False),
self.setup_bookmark(is_archived=True, user=other_user),
]
response = self.client.get(reverse('bookmarks:archived'))
html = collapse_whitespace(response.content.decode())
# Should render list
self.assertIn('<ul class="bookmark-list" data-bookmarks-total="3">', html)
self.assertVisibleBookmarks(response, visible_bookmarks)
self.assertInvisibleBookmarks(response, invisible_bookmarks)
def test_should_list_bookmarks_matching_query(self):
visible_bookmarks = [
self.setup_bookmark(is_archived=True, title='searchvalue'),
self.setup_bookmark(is_archived=True, title='searchvalue'),
self.setup_bookmark(is_archived=True, title='searchvalue')
]
invisible_bookmarks = [
self.setup_bookmark(is_archived=True),
self.setup_bookmark(is_archived=True),
self.setup_bookmark(is_archived=True)
]
visible_bookmarks = self.setup_numbered_bookmarks(3, prefix='foo', archived=True)
invisible_bookmarks = self.setup_numbered_bookmarks(3, prefix='bar', archived=True)
response = self.client.get(reverse('bookmarks:archived') + '?q=searchvalue')
response = self.client.get(reverse('bookmarks:archived') + '?q=foo')
html = collapse_whitespace(response.content.decode())
# Should render list
self.assertIn('<ul class="bookmark-list" data-bookmarks-total="3">', html)
self.assertVisibleBookmarks(response, visible_bookmarks)
self.assertInvisibleBookmarks(response, invisible_bookmarks)
def test_should_list_tags_for_archived_and_user_owned_bookmarks(self):
other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
visible_tags = [
self.setup_tag(),
self.setup_tag(),
self.setup_tag(),
]
invisible_tags = [
self.setup_tag(), # unused tag
self.setup_tag(), # used in archived bookmark
self.setup_tag(user=other_user), # belongs to other user
]
visible_bookmarks = self.setup_numbered_bookmarks(3, with_tags=True, archived=True)
unarchived_bookmarks = self.setup_numbered_bookmarks(3, with_tags=True, archived=False, tag_prefix='unarchived')
other_user_bookmarks = self.setup_numbered_bookmarks(3, with_tags=True, archived=True, user=other_user,
tag_prefix='otheruser')
# Assign tags to some bookmarks with duplicates
self.setup_bookmark(is_archived=True, tags=[visible_tags[0]])
self.setup_bookmark(is_archived=True, tags=[visible_tags[0]])
self.setup_bookmark(is_archived=True, tags=[visible_tags[1]])
self.setup_bookmark(is_archived=True, tags=[visible_tags[1]])
self.setup_bookmark(is_archived=True, tags=[visible_tags[2]])
self.setup_bookmark(is_archived=True, tags=[visible_tags[2]])
self.setup_bookmark(is_archived=False, tags=[invisible_tags[1]])
self.setup_bookmark(is_archived=True, tags=[invisible_tags[2]], user=other_user)
visible_tags = self.get_tags_from_bookmarks(visible_bookmarks)
invisible_tags = self.get_tags_from_bookmarks(unarchived_bookmarks + other_user_bookmarks)
response = self.client.get(reverse('bookmarks:archived'))
@@ -141,29 +123,40 @@ class BookmarkArchivedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin
self.assertInvisibleTags(response, invisible_tags)
def test_should_list_tags_for_bookmarks_matching_query(self):
visible_tags = [
self.setup_tag(),
self.setup_tag(),
self.setup_tag(),
]
invisible_tags = [
self.setup_tag(),
self.setup_tag(),
self.setup_tag(),
]
visible_bookmarks = self.setup_numbered_bookmarks(3, with_tags=True, archived=True, prefix='foo',
tag_prefix='foo')
invisible_bookmarks = self.setup_numbered_bookmarks(3, with_tags=True, archived=True, prefix='bar',
tag_prefix='bar')
self.setup_bookmark(is_archived=True, tags=[visible_tags[0]], title='searchvalue')
self.setup_bookmark(is_archived=True, tags=[visible_tags[1]], title='searchvalue')
self.setup_bookmark(is_archived=True, tags=[visible_tags[2]], title='searchvalue')
self.setup_bookmark(is_archived=True, tags=[invisible_tags[0]])
self.setup_bookmark(is_archived=True, tags=[invisible_tags[1]])
self.setup_bookmark(is_archived=True, tags=[invisible_tags[2]])
visible_tags = self.get_tags_from_bookmarks(visible_bookmarks)
invisible_tags = self.get_tags_from_bookmarks(invisible_bookmarks)
response = self.client.get(reverse('bookmarks:archived') + '?q=searchvalue')
response = self.client.get(reverse('bookmarks:archived') + '?q=foo')
self.assertVisibleTags(response, visible_tags)
self.assertInvisibleTags(response, invisible_tags)
def test_should_list_bookmarks_and_tags_for_search_preferences(self):
user_profile = self.user.profile
user_profile.search_preferences = {
'unread': BookmarkSearch.FILTER_UNREAD_YES,
}
user_profile.save()
unread_bookmarks = self.setup_numbered_bookmarks(3, archived=True, unread=True, with_tags=True, prefix='unread',
tag_prefix='unread')
read_bookmarks = self.setup_numbered_bookmarks(3, archived=True, unread=False, with_tags=True, prefix='read',
tag_prefix='read')
unread_tags = self.get_tags_from_bookmarks(unread_bookmarks)
read_tags = self.get_tags_from_bookmarks(read_bookmarks)
response = self.client.get(reverse('bookmarks:archived'))
self.assertVisibleBookmarks(response, unread_bookmarks)
self.assertInvisibleBookmarks(response, read_bookmarks)
self.assertVisibleTags(response, unread_tags)
self.assertInvisibleTags(response, read_tags)
def test_should_display_selected_tags_from_query(self):
tags = [
self.setup_tag(),
@@ -210,11 +203,7 @@ class BookmarkArchivedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin
self.assertSelectedTags(response, [tags[0], tags[1]])
def test_should_open_bookmarks_in_new_page_by_default(self):
visible_bookmarks = [
self.setup_bookmark(is_archived=True),
self.setup_bookmark(is_archived=True),
self.setup_bookmark(is_archived=True)
]
visible_bookmarks = self.setup_numbered_bookmarks(3, archived=True)
response = self.client.get(reverse('bookmarks:archived'))
@@ -225,11 +214,7 @@ class BookmarkArchivedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin
user.profile.bookmark_link_target = UserProfile.BOOKMARK_LINK_TARGET_SELF
user.profile.save()
visible_bookmarks = [
self.setup_bookmark(is_archived=True),
self.setup_bookmark(is_archived=True),
self.setup_bookmark(is_archived=True)
]
visible_bookmarks = self.setup_numbered_bookmarks(3, archived=True)
response = self.client.get(reverse('bookmarks:archived'))
@@ -328,6 +313,106 @@ class BookmarkArchivedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin
</select>
''', html)
def test_apply_search_preferences(self):
# no params
response = self.client.post(reverse('bookmarks:archived'))
self.assertEqual(response.status_code, 302)
self.assertEqual(response.url, reverse('bookmarks:archived'))
# some params
response = self.client.post(reverse('bookmarks:archived'), {
'q': 'foo',
'sort': BookmarkSearch.SORT_TITLE_ASC,
})
self.assertEqual(response.status_code, 302)
self.assertEqual(response.url, reverse('bookmarks:archived') + '?q=foo&sort=title_asc')
# params with default value are removed
response = self.client.post(reverse('bookmarks:archived'), {
'q': 'foo',
'user': '',
'sort': BookmarkSearch.SORT_ADDED_DESC,
'shared': BookmarkSearch.FILTER_SHARED_OFF,
'unread': BookmarkSearch.FILTER_UNREAD_YES,
})
self.assertEqual(response.status_code, 302)
self.assertEqual(response.url, reverse('bookmarks:archived') + '?q=foo&unread=yes')
# page is removed
response = self.client.post(reverse('bookmarks:archived'), {
'q': 'foo',
'page': '2',
'sort': BookmarkSearch.SORT_TITLE_ASC,
})
self.assertEqual(response.status_code, 302)
self.assertEqual(response.url, reverse('bookmarks:archived') + '?q=foo&sort=title_asc')
def test_save_search_preferences(self):
user_profile = self.user.profile
# no params
self.client.post(reverse('bookmarks:archived'), {
'save': '',
})
user_profile.refresh_from_db()
self.assertEqual(user_profile.search_preferences, {
'sort': BookmarkSearch.SORT_ADDED_DESC,
'shared': BookmarkSearch.FILTER_SHARED_OFF,
'unread': BookmarkSearch.FILTER_UNREAD_OFF,
})
# with param
self.client.post(reverse('bookmarks:archived'), {
'save': '',
'sort': BookmarkSearch.SORT_TITLE_ASC,
})
user_profile.refresh_from_db()
self.assertEqual(user_profile.search_preferences, {
'sort': BookmarkSearch.SORT_TITLE_ASC,
'shared': BookmarkSearch.FILTER_SHARED_OFF,
'unread': BookmarkSearch.FILTER_UNREAD_OFF,
})
# add a param
self.client.post(reverse('bookmarks:archived'), {
'save': '',
'sort': BookmarkSearch.SORT_TITLE_ASC,
'unread': BookmarkSearch.FILTER_UNREAD_YES,
})
user_profile.refresh_from_db()
self.assertEqual(user_profile.search_preferences, {
'sort': BookmarkSearch.SORT_TITLE_ASC,
'shared': BookmarkSearch.FILTER_SHARED_OFF,
'unread': BookmarkSearch.FILTER_UNREAD_YES,
})
# remove a param
self.client.post(reverse('bookmarks:archived'), {
'save': '',
'unread': BookmarkSearch.FILTER_UNREAD_YES,
})
user_profile.refresh_from_db()
self.assertEqual(user_profile.search_preferences, {
'sort': BookmarkSearch.SORT_ADDED_DESC,
'shared': BookmarkSearch.FILTER_SHARED_OFF,
'unread': BookmarkSearch.FILTER_UNREAD_YES,
})
# ignores non-preferences
self.client.post(reverse('bookmarks:archived'), {
'save': '',
'q': 'foo',
'user': 'john',
'page': '3',
'sort': BookmarkSearch.SORT_TITLE_ASC,
})
user_profile.refresh_from_db()
self.assertEqual(user_profile.search_preferences, {
'sort': BookmarkSearch.SORT_TITLE_ASC,
'shared': BookmarkSearch.FILTER_SHARED_OFF,
'unread': BookmarkSearch.FILTER_UNREAD_OFF,
})
def test_url_encode_bookmark_actions_url(self):
url = reverse('bookmarks:archived') + '?q=%23foo'
response = self.client.get(url)