diff --git a/bookmarks/api/routes.py b/bookmarks/api/routes.py index 1f1005b..f114f07 100644 --- a/bookmarks/api/routes.py +++ b/bookmarks/api/routes.py @@ -6,7 +6,7 @@ from rest_framework.routers import DefaultRouter from bookmarks import queries from bookmarks.api.serializers import BookmarkSerializer, TagSerializer -from bookmarks.models import Bookmark, BookmarkFilters, Tag, User +from bookmarks.models import Bookmark, BookmarkSearch, Tag, User from bookmarks.services.bookmarks import archive_bookmark, unarchive_bookmark, website_loader from bookmarks.services.website_loader import WebsiteMetadata @@ -34,8 +34,8 @@ class BookmarkViewSet(viewsets.GenericViewSet, user = self.request.user # For list action, use query set that applies search and tag projections if self.action == 'list': - query_string = self.request.GET.get('q') - return queries.query_bookmarks(user, user.profile, query_string) + search = BookmarkSearch.from_request(self.request) + return queries.query_bookmarks(user, user.profile, search) # For single entity actions use default query set without projections return Bookmark.objects.all().filter(owner=user) @@ -46,8 +46,8 @@ class BookmarkViewSet(viewsets.GenericViewSet, @action(methods=['get'], detail=False) def archived(self, request): user = request.user - query_string = request.GET.get('q') - query_set = queries.query_archived_bookmarks(user, user.profile, query_string) + search = BookmarkSearch.from_request(request) + query_set = queries.query_archived_bookmarks(user, user.profile, search) page = self.paginate_queryset(query_set) serializer = self.get_serializer_class() data = serializer(page, many=True).data @@ -55,10 +55,10 @@ class BookmarkViewSet(viewsets.GenericViewSet, @action(methods=['get'], detail=False) def shared(self, request): - filters = BookmarkFilters(request) - user = User.objects.filter(username=filters.user).first() + search = BookmarkSearch.from_request(request) + user = User.objects.filter(username=search.user).first() public_only = not request.user.is_authenticated - query_set = queries.query_shared_bookmarks(user, request.user_profile, filters.query, public_only) + query_set = queries.query_shared_bookmarks(user, request.user_profile, search, public_only) page = self.paginate_queryset(query_set) serializer = self.get_serializer_class() data = serializer(page, many=True).data diff --git a/bookmarks/context_processors.py b/bookmarks/context_processors.py index a179811..71a33c7 100644 --- a/bookmarks/context_processors.py +++ b/bookmarks/context_processors.py @@ -1,5 +1,5 @@ from bookmarks import queries -from bookmarks.models import Toast +from bookmarks.models import BookmarkSearch, Toast from bookmarks import utils @@ -17,7 +17,7 @@ def toasts(request): def public_shares(request): # Only check for public shares for anonymous users if not request.user.is_authenticated: - query_set = queries.query_shared_bookmarks(None, request.user_profile, '', True) + query_set = queries.query_shared_bookmarks(None, request.user_profile, BookmarkSearch(), True) has_public_shares = query_set.count() > 0 return { 'has_public_shares': has_public_shares, diff --git a/bookmarks/e2e/e2e_test_bookmark_page_partial_updates.py b/bookmarks/e2e/e2e_test_bookmark_page_partial_updates.py index a883c56..f218bb2 100644 --- a/bookmarks/e2e/e2e_test_bookmark_page_partial_updates.py +++ b/bookmarks/e2e/e2e_test_bookmark_page_partial_updates.py @@ -50,6 +50,21 @@ class BookmarkPagePartialUpdatesE2ETestCase(LinkdingE2ETestCase): self.locate_bookmark('foo 2').get_by_text('Archive').click() self.assertVisibleBookmarks(['foo 1', 'foo 3', 'foo 4', 'foo 5']) + def test_partial_update_respects_sort(self): + self.setup_numbered_bookmarks(5, prefix='foo') + + with sync_playwright() as p: + url = reverse('bookmarks:index') + '?sort=title_asc' + page = self.open(url, p) + + first_item = page.locator('li[ld-bookmark-item]').first + expect(first_item).to_contain_text('foo 1') + + first_item.get_by_text('Archive').click() + + first_item = page.locator('li[ld-bookmark-item]').first + expect(first_item).to_contain_text('foo 2') + def test_partial_update_respects_page(self): # add a suffix, otherwise 'foo 1' also matches 'foo 10' self.setup_numbered_bookmarks(50, prefix='foo', suffix='-') diff --git a/bookmarks/feeds.py b/bookmarks/feeds.py index 3097315..92fdde6 100644 --- a/bookmarks/feeds.py +++ b/bookmarks/feeds.py @@ -4,7 +4,7 @@ from django.contrib.syndication.views import Feed from django.db.models import QuerySet from django.urls import reverse -from bookmarks.models import Bookmark, FeedToken +from bookmarks.models import Bookmark, BookmarkSearch, FeedToken from bookmarks import queries @@ -17,8 +17,8 @@ class FeedContext: class BaseBookmarksFeed(Feed): def get_object(self, request, feed_key: str): feed_token = FeedToken.objects.get(key__exact=feed_key) - query_string = request.GET.get('q') - query_set = queries.query_bookmarks(feed_token.user, feed_token.user.profile, query_string) + search = BookmarkSearch(query=request.GET.get('q', '')) + query_set = queries.query_bookmarks(feed_token.user, feed_token.user.profile, search) return FeedContext(feed_token, query_set) def item_title(self, item: Bookmark): diff --git a/bookmarks/frontend/api.js b/bookmarks/frontend/api.js index 9062116..dfe682f 100644 --- a/bookmarks/frontend/api.js +++ b/bookmarks/frontend/api.js @@ -3,10 +3,10 @@ export class ApiClient { this.baseUrl = baseUrl; } - listBookmarks(filters, options = { limit: 100, offset: 0, path: "" }) { + listBookmarks(search, options = { limit: 100, offset: 0, path: "" }) { const query = [`limit=${options.limit}`, `offset=${options.offset}`]; - Object.keys(filters).forEach((key) => { - const value = filters[key]; + Object.keys(search).forEach((key) => { + const value = search[key]; if (value) { query.push(`${key}=${encodeURIComponent(value)}`); } diff --git a/bookmarks/frontend/components/SearchAutoComplete.svelte b/bookmarks/frontend/components/SearchAutoComplete.svelte index e96a9e1..94f4e33 100644 --- a/bookmarks/frontend/components/SearchAutoComplete.svelte +++ b/bookmarks/frontend/components/SearchAutoComplete.svelte @@ -10,7 +10,7 @@ export let tags; export let mode = ''; export let apiClient; - export let filters; + export let search; export let linkTarget = '_blank'; let isFocus = false; @@ -115,11 +115,11 @@ if (value && value.length >= 3) { const path = mode ? `/${mode}` : '' - const suggestionFilters = { - ...filters, + const suggestionSearch = { + ...search, q: value } - const fetchedBookmarks = await apiClient.listBookmarks(suggestionFilters, {limit: 5, offset: 0, path}) + const fetchedBookmarks = await apiClient.listBookmarks(suggestionSearch, {limit: 5, offset: 0, path}) bookmarks = fetchedBookmarks.map(bookmark => { const fullLabel = bookmark.title || bookmark.website_title || bookmark.url const label = clampText(fullLabel, 60) diff --git a/bookmarks/management/commands/enable_wal.py b/bookmarks/management/commands/enable_wal.py index 91833e2..6c66e3d 100644 --- a/bookmarks/management/commands/enable_wal.py +++ b/bookmarks/management/commands/enable_wal.py @@ -11,7 +11,7 @@ class Command(BaseCommand): help = "Enable WAL journal mode when using an SQLite database" def handle(self, *args, **options): - if settings.DATABASES['default']['ENGINE'] != 'django.db.backends.sqlite3': + if not settings.USE_SQLITE: return connection = connections['default'] diff --git a/bookmarks/models.py b/bookmarks/models.py index 5023b29..e563d4e 100644 --- a/bookmarks/models.py +++ b/bookmarks/models.py @@ -124,10 +124,86 @@ class BookmarkForm(forms.ModelForm): return self.instance and self.instance.notes -class BookmarkFilters: - def __init__(self, request: WSGIRequest): - self.query = request.GET.get('q') or '' - self.user = request.GET.get('user') or '' +class BookmarkSearch: + SORT_ADDED_ASC = 'added_asc' + SORT_ADDED_DESC = 'added_desc' + SORT_TITLE_ASC = 'title_asc' + SORT_TITLE_DESC = 'title_desc' + + params = ['q', 'user', 'sort'] + defaults = { + 'q': '', + 'user': '', + 'sort': SORT_ADDED_DESC, + } + + def __init__(self, + q: str = defaults['q'], + query: str = defaults['q'], # alias for q + user: str = defaults['user'], + sort: str = defaults['sort']): + self.q = q or query + self.user = user + self.sort = sort + + @property + def query(self): + return self.q + + def is_modified(self, param): + value = self.__dict__[param] + return value and value != BookmarkSearch.defaults[param] + + @property + def modified_params(self): + return [field for field in self.params if self.is_modified(field)] + + @property + def query_params(self): + return {param: self.__dict__[param] for param in self.modified_params} + + @staticmethod + def from_request(request: WSGIRequest): + initial_values = {} + for param in BookmarkSearch.params: + value = request.GET.get(param) + if value: + initial_values[param] = value + + return BookmarkSearch(**initial_values) + + +class BookmarkSearchForm(forms.Form): + SORT_CHOICES = [ + (BookmarkSearch.SORT_ADDED_ASC, 'Added ↑'), + (BookmarkSearch.SORT_ADDED_DESC, 'Added ↓'), + (BookmarkSearch.SORT_TITLE_ASC, 'Title ↑'), + (BookmarkSearch.SORT_TITLE_DESC, 'Title ↓'), + ] + + q = forms.CharField() + user = forms.ChoiceField() + sort = forms.ChoiceField(choices=SORT_CHOICES) + + def __init__(self, search: BookmarkSearch, editable_fields: List[str] = None, users: List[User] = None): + super().__init__() + editable_fields = editable_fields or [] + + # set choices for user field if users are provided + if users: + user_choices = [(user.username, user.username) for user in users] + user_choices.insert(0, ('', 'Everyone')) + self.fields['user'].choices = user_choices + + for param in search.params: + # set initial values for modified params + self.fields[param].initial = search.__dict__[param] + + # Mark non-editable modified fields as hidden. That way, templates + # rendering a form can just loop over hidden_fields to ensure that + # all necessary search options are kept when submitting the form. + if search.is_modified(param) and param not in editable_fields: + self.fields[param].widget = forms.HiddenInput() class UserProfile(models.Model): diff --git a/bookmarks/queries.py b/bookmarks/queries.py index 6af1b5c..398db80 100644 --- a/bookmarks/queries.py +++ b/bookmarks/queries.py @@ -1,32 +1,35 @@ from typing import Optional +from django.conf import settings from django.contrib.auth.models import User -from django.db.models import Q, QuerySet, Exists, OuterRef +from django.db.models import Q, QuerySet, Exists, OuterRef, Case, When, CharField +from django.db.models.expressions import RawSQL +from django.db.models.functions import Lower -from bookmarks.models import Bookmark, Tag, UserProfile +from bookmarks.models import Bookmark, BookmarkSearch, Tag, UserProfile from bookmarks.utils import unique -def query_bookmarks(user: User, profile: UserProfile, query_string: str) -> QuerySet: - return _base_bookmarks_query(user, profile, query_string) \ +def query_bookmarks(user: User, profile: UserProfile, search: BookmarkSearch) -> QuerySet: + return _base_bookmarks_query(user, profile, search) \ .filter(is_archived=False) -def query_archived_bookmarks(user: User, profile: UserProfile, query_string: str) -> QuerySet: - return _base_bookmarks_query(user, profile, query_string) \ +def query_archived_bookmarks(user: User, profile: UserProfile, search: BookmarkSearch) -> QuerySet: + return _base_bookmarks_query(user, profile, search) \ .filter(is_archived=True) -def query_shared_bookmarks(user: Optional[User], profile: UserProfile, query_string: str, +def query_shared_bookmarks(user: Optional[User], profile: UserProfile, search: BookmarkSearch, public_only: bool) -> QuerySet: conditions = Q(shared=True) & Q(owner__profile__enable_sharing=True) if public_only: conditions = conditions & Q(owner__profile__enable_public_sharing=True) - return _base_bookmarks_query(user, profile, query_string).filter(conditions) + return _base_bookmarks_query(user, profile, search).filter(conditions) -def _base_bookmarks_query(user: Optional[User], profile: UserProfile, query_string: str) -> QuerySet: +def _base_bookmarks_query(user: Optional[User], profile: UserProfile, search: BookmarkSearch) -> QuerySet: query_set = Bookmark.objects # Filter for user @@ -34,7 +37,7 @@ def _base_bookmarks_query(user: Optional[User], profile: UserProfile, query_stri query_set = query_set.filter(owner=user) # Split query into search terms and tags - query = parse_query_string(query_string) + query = parse_query_string(search.query) # Filter for search terms and tags for term in query['search_terms']: @@ -67,38 +70,66 @@ def _base_bookmarks_query(user: Optional[User], profile: UserProfile, query_stri ) # Sort by date added - query_set = query_set.order_by('-date_added') + if search.sort == BookmarkSearch.SORT_ADDED_ASC: + query_set = query_set.order_by('date_added') + elif search.sort == BookmarkSearch.SORT_ADDED_DESC: + query_set = query_set.order_by('-date_added') + + # Sort by title + if search.sort == BookmarkSearch.SORT_TITLE_ASC or search.sort == BookmarkSearch.SORT_TITLE_DESC: + # For the title, the resolved_title logic from the Bookmark entity needs + # to be replicated as there is no corresponding database field + 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() + )) + + # For SQLite, if the ICU extension is loaded, use the custom collation + # loaded into the connection. This results in an improved sort order for + # unicode characters (umlauts, etc.) + if settings.USE_SQLITE and settings.USE_SQLITE_ICU_EXTENSION: + order_field = RawSQL('effective_title COLLATE ICU', ()) + else: + order_field = 'effective_title' + + if search.sort == BookmarkSearch.SORT_TITLE_ASC: + query_set = query_set.order_by(order_field) + elif search.sort == BookmarkSearch.SORT_TITLE_DESC: + query_set = query_set.order_by(order_field).reverse() return query_set -def query_bookmark_tags(user: User, profile: UserProfile, query_string: str) -> QuerySet: - bookmarks_query = query_bookmarks(user, profile, query_string) +def query_bookmark_tags(user: User, profile: UserProfile, search: BookmarkSearch) -> QuerySet: + bookmarks_query = query_bookmarks(user, profile, search) query_set = Tag.objects.filter(bookmark__in=bookmarks_query) return query_set.distinct() -def query_archived_bookmark_tags(user: User, profile: UserProfile, query_string: str) -> QuerySet: - bookmarks_query = query_archived_bookmarks(user, profile, query_string) +def query_archived_bookmark_tags(user: User, profile: UserProfile, search: BookmarkSearch) -> QuerySet: + bookmarks_query = query_archived_bookmarks(user, profile, search) query_set = Tag.objects.filter(bookmark__in=bookmarks_query) return query_set.distinct() -def query_shared_bookmark_tags(user: Optional[User], profile: UserProfile, query_string: str, +def query_shared_bookmark_tags(user: Optional[User], profile: UserProfile, search: BookmarkSearch, public_only: bool) -> QuerySet: - bookmarks_query = query_shared_bookmarks(user, profile, query_string, public_only) + bookmarks_query = query_shared_bookmarks(user, profile, search, public_only) query_set = Tag.objects.filter(bookmark__in=bookmarks_query) return query_set.distinct() -def query_shared_bookmark_users(profile: UserProfile, query_string: str, public_only: bool) -> QuerySet: - bookmarks_query = query_shared_bookmarks(None, profile, query_string, public_only) +def query_shared_bookmark_users(profile: UserProfile, search: BookmarkSearch, public_only: bool) -> QuerySet: + bookmarks_query = query_shared_bookmarks(None, profile, search, public_only) query_set = User.objects.filter(bookmark__in=bookmarks_query) diff --git a/bookmarks/signals.py b/bookmarks/signals.py index 915352a..0fd3b15 100644 --- a/bookmarks/signals.py +++ b/bookmarks/signals.py @@ -1,14 +1,10 @@ -from os import path - +from django.conf import settings from django.contrib.auth import user_logged_in from django.db.backends.signals import connection_created from django.dispatch import receiver from bookmarks.services import tasks -icu_extension_path = './libicu.so' -icu_extension_exists = path.exists(icu_extension_path) - @receiver(user_logged_in) def user_logged_in(sender, request, user, **kwargs): @@ -19,9 +15,9 @@ def user_logged_in(sender, request, user, **kwargs): def extend_sqlite(connection=None, **kwargs): # Load ICU extension into Sqlite connection to support case-insensitive # comparisons with unicode characters - if connection.vendor == 'sqlite' and icu_extension_exists: + if connection.vendor == 'sqlite' and settings.USE_SQLITE_ICU_EXTENSION: connection.connection.enable_load_extension(True) - connection.connection.load_extension('./libicu') + connection.connection.load_extension(settings.SQLITE_ICU_EXTENSION_PATH.rstrip('.so')) with connection.cursor() as cursor: # Load an ICU collation for case-insensitive ordering. diff --git a/bookmarks/styles/bookmark-page.scss b/bookmarks/styles/bookmark-page.scss index cdba178..3f4aef6 100644 --- a/bookmarks/styles/bookmark-page.scss +++ b/bookmarks/styles/bookmark-page.scss @@ -40,6 +40,41 @@ } } } + + // Group search options button with search button + .input-group input[type='submit'] { + border-top-right-radius: 0; + border-bottom-right-radius: 0; + } + + .dropdown button { + border-top-left-radius: 0; + border-bottom-left-radius: 0; + } + + .dropdown { + margin-left: -1px; + } + + // Search option menu styles + .dropdown { + .menu { + padding: $unit-4; + min-width: 220px; + } + + &:focus-within { + .menu { + display: block; + } + } + + .menu .actions { + margin-top: $unit-4; + display: flex; + justify-content: space-between; + } + } } /* Bookmark list */ diff --git a/bookmarks/templates/bookmarks/archive.html b/bookmarks/templates/bookmarks/archive.html index 80eaed4..dae54fa 100644 --- a/bookmarks/templates/bookmarks/archive.html +++ b/bookmarks/templates/bookmarks/archive.html @@ -15,13 +15,14 @@