mirror of
https://github.com/bookwyrm-social/bookwyrm.git
synced 2024-12-24 09:00:33 +00:00
Merge pull request #2254 from bookwyrm-social/fixes-search-pagination
Fixes search pagination
This commit is contained in:
commit
9d7ece47ef
7 changed files with 129 additions and 81 deletions
|
@ -56,7 +56,7 @@ EMAIL_SENDER_NAME=admin
|
|||
EMAIL_SENDER_DOMAIN=
|
||||
|
||||
# Query timeouts
|
||||
SEARCH_TIMEOUT=15
|
||||
SEARCH_TIMEOUT=5
|
||||
QUERY_TIMEOUT=5
|
||||
|
||||
# Thumbnails Generation
|
||||
|
|
|
@ -3,10 +3,9 @@
|
|||
|
||||
{% block panel %}
|
||||
|
||||
{% if results %}
|
||||
{% with results|first as local_results %}
|
||||
{% if results or remote_results %}
|
||||
<ul class="block">
|
||||
{% for result in local_results.results %}
|
||||
{% for result in results %}
|
||||
<li class="pd-4 mb-5 local-book-search-result" id="tour-local-book-search-result">
|
||||
<div class="columns is-mobile is-gapless mb-0">
|
||||
<div class="column is-cover">
|
||||
|
@ -29,16 +28,12 @@
|
|||
</li>
|
||||
{% endfor %}
|
||||
</ul>
|
||||
{% endwith %}
|
||||
|
||||
<div class="block">
|
||||
{% for result_set in results|slice:"1:" %}
|
||||
{% for result_set in remote_results %}
|
||||
{% if result_set.results %}
|
||||
<section class="mb-5">
|
||||
{% if not result_set.connector.local %}
|
||||
<details class="details-panel box" open>
|
||||
{% endif %}
|
||||
{% if not result_set.connector.local %}
|
||||
<summary class="is-flex is-align-items-center is-flex-wrap-wrap is-gap-2 remote-book-search-result" id="tour-remote-search-result">
|
||||
<span class="mb-0 title is-5">
|
||||
{% trans 'Results from' %}
|
||||
|
@ -51,7 +46,6 @@
|
|||
|
||||
<span class="details-close icon icon-x" aria-hidden="true"></span>
|
||||
</summary>
|
||||
{% endif %}
|
||||
|
||||
<div class="mt-5">
|
||||
<div class="is-flex is-flex-direction-row-reverse">
|
||||
|
@ -92,17 +86,15 @@
|
|||
</ul>
|
||||
</div>
|
||||
</div>
|
||||
{% if not result_set.connector.local %}
|
||||
</details>
|
||||
{% endif %}
|
||||
</section>
|
||||
{% endif %}
|
||||
{% endfor %}
|
||||
</div>
|
||||
|
||||
|
||||
{% endif %}
|
||||
{% endblock %}
|
||||
|
||||
{% block search_footer %}
|
||||
<p class="block">
|
||||
{% if request.user.is_authenticated %}
|
||||
{% if not remote %}
|
||||
|
|
|
@ -1,5 +1,6 @@
|
|||
{% extends 'layout.html' %}
|
||||
{% load i18n %}
|
||||
{% load humanize %}
|
||||
|
||||
{% block title %}{% trans "Search" %}{% endblock %}
|
||||
|
||||
|
@ -53,17 +54,24 @@
|
|||
</nav>
|
||||
|
||||
<section class="block" id="search-results-block">
|
||||
{% if not results %}
|
||||
<p>
|
||||
<p class="block">
|
||||
{% if not results %}
|
||||
<em>{% blocktrans %}No results found for "{{ query }}"{% endblocktrans %}</em>
|
||||
{% else %}
|
||||
<em>{% blocktrans trimmed count counter=results.paginator.count with result_count=results.paginator.count|intcomma %}
|
||||
{{ result_count }} result found
|
||||
{% plural %}
|
||||
{{ result_count }} results found
|
||||
{% endblocktrans %}</em>
|
||||
{% endif %}
|
||||
</p>
|
||||
{% endif %}
|
||||
{% block panel %}
|
||||
{% endblock %}
|
||||
|
||||
<div>
|
||||
<div class="block">
|
||||
{% include 'snippets/pagination.html' with page=results path=request.path %}
|
||||
</div>
|
||||
{% block search_footer %}{% endblock %}
|
||||
</section>
|
||||
{% endif %}
|
||||
|
||||
|
|
|
@ -7,13 +7,14 @@ from django.test import TestCase
|
|||
from django.test.client import RequestFactory
|
||||
|
||||
from bookwyrm import models, views
|
||||
from bookwyrm.tests.validate_html import validate_html
|
||||
from bookwyrm.settings import DOMAIN
|
||||
|
||||
|
||||
class IsbnViews(TestCase):
|
||||
"""tag views"""
|
||||
|
||||
def setUp(self):
|
||||
def setUp(self): # pylint: disable=invalid-name
|
||||
"""we need basic test data and mocks"""
|
||||
self.factory = RequestFactory()
|
||||
with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch(
|
||||
|
@ -58,4 +59,4 @@ class IsbnViews(TestCase):
|
|||
is_api.return_value = False
|
||||
response = view(request, isbn="1234567890123")
|
||||
self.assertEqual(response.status_code, 200)
|
||||
response.render()
|
||||
validate_html(response.render())
|
||||
|
|
|
@ -17,7 +17,7 @@ from bookwyrm.tests.validate_html import validate_html
|
|||
class Views(TestCase):
|
||||
"""tag views"""
|
||||
|
||||
def setUp(self):
|
||||
def setUp(self): # pylint: disable=invalid-name
|
||||
"""we need basic test data and mocks"""
|
||||
self.factory = RequestFactory()
|
||||
with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch(
|
||||
|
@ -90,13 +90,29 @@ class Views(TestCase):
|
|||
|
||||
self.assertIsInstance(response, TemplateResponse)
|
||||
validate_html(response.render())
|
||||
connector_results = response.context_data["results"]
|
||||
self.assertEqual(len(connector_results), 2)
|
||||
self.assertEqual(connector_results[0]["results"][0].title, "Test Book")
|
||||
self.assertEqual(connector_results[1]["results"][0].title, "Mock Book")
|
||||
|
||||
# don't search remote
|
||||
local_results = response.context_data["results"]
|
||||
self.assertEqual(local_results[0].title, "Test Book")
|
||||
|
||||
connector_results = response.context_data["remote_results"]
|
||||
self.assertEqual(connector_results[0]["results"][0].title, "Mock Book")
|
||||
|
||||
def test_search_book_anonymous(self):
|
||||
"""Don't search remote for logged out user"""
|
||||
view = views.Search.as_view()
|
||||
|
||||
connector = models.Connector.objects.create(
|
||||
identifier="example.com",
|
||||
connector_file="openlibrary",
|
||||
base_url="https://example.com",
|
||||
books_url="https://example.com/books",
|
||||
covers_url="https://example.com/covers",
|
||||
search_url="https://example.com/search?q=",
|
||||
)
|
||||
mock_result = SearchResult(title="Mock Book", connector=connector, key="hello")
|
||||
|
||||
request = self.factory.get("", {"q": "Test Book", "remote": True})
|
||||
|
||||
anonymous_user = AnonymousUser
|
||||
anonymous_user.is_authenticated = False
|
||||
request.user = anonymous_user
|
||||
|
@ -107,11 +123,15 @@ class Views(TestCase):
|
|||
{"results": [mock_result], "connector": connector}
|
||||
]
|
||||
response = view(request)
|
||||
|
||||
self.assertIsInstance(response, TemplateResponse)
|
||||
validate_html(response.render())
|
||||
connector_results = response.context_data["results"]
|
||||
self.assertEqual(len(connector_results), 1)
|
||||
self.assertEqual(connector_results[0]["results"][0].title, "Test Book")
|
||||
|
||||
local_results = response.context_data["results"]
|
||||
self.assertEqual(local_results[0].title, "Test Book")
|
||||
|
||||
connector_results = response.context_data.get("remote_results")
|
||||
self.assertIsNone(connector_results)
|
||||
|
||||
def test_search_users(self):
|
||||
"""searches remote connectors"""
|
||||
|
|
|
@ -18,14 +18,17 @@ class Isbn(View):
|
|||
|
||||
if is_api_request(request):
|
||||
return JsonResponse(
|
||||
[book_search.format_search_result(r) for r in book_results], safe=False
|
||||
[book_search.format_search_result(r) for r in book_results[:10]],
|
||||
safe=False,
|
||||
)
|
||||
|
||||
paginated = Paginator(book_results, PAGE_LENGTH).get_page(
|
||||
request.GET.get("page")
|
||||
)
|
||||
paginated = Paginator(book_results, PAGE_LENGTH)
|
||||
page = paginated.get_page(request.GET.get("page"))
|
||||
data = {
|
||||
"results": [{"results": paginated}],
|
||||
"results": page,
|
||||
"page_range": paginated.get_elided_page_range(
|
||||
page.number, on_each_side=2, on_ends=1
|
||||
),
|
||||
"query": isbn,
|
||||
"type": "book",
|
||||
}
|
||||
|
|
|
@ -23,22 +23,14 @@ class Search(View):
|
|||
|
||||
def get(self, request):
|
||||
"""that search bar up top"""
|
||||
query = request.GET.get("q")
|
||||
# check if query is isbn
|
||||
query = isbn_check(query)
|
||||
min_confidence = request.GET.get("min_confidence", 0)
|
||||
search_type = request.GET.get("type")
|
||||
search_remote = (
|
||||
request.GET.get("remote", False) and request.user.is_authenticated
|
||||
)
|
||||
|
||||
if is_api_request(request):
|
||||
# only return local book results via json so we don't cascade
|
||||
book_results = search(query, min_confidence=min_confidence)
|
||||
return JsonResponse(
|
||||
[format_search_result(r) for r in book_results], safe=False
|
||||
)
|
||||
return api_book_search(request)
|
||||
|
||||
query = request.GET.get("q")
|
||||
if not query:
|
||||
return TemplateResponse(request, "search/book.html")
|
||||
|
||||
search_type = request.GET.get("type")
|
||||
if query and not search_type:
|
||||
search_type = "user" if "@" in query else "book"
|
||||
|
||||
|
@ -50,49 +42,65 @@ class Search(View):
|
|||
if not search_type in endpoints:
|
||||
search_type = "book"
|
||||
|
||||
data = {
|
||||
"query": query or "",
|
||||
"type": search_type,
|
||||
"remote": search_remote,
|
||||
}
|
||||
if query:
|
||||
results, search_remote = endpoints[search_type](
|
||||
query, request.user, min_confidence, search_remote
|
||||
)
|
||||
if results:
|
||||
paginated = Paginator(results, PAGE_LENGTH).get_page(
|
||||
request.GET.get("page")
|
||||
)
|
||||
data["results"] = paginated
|
||||
data["remote"] = search_remote
|
||||
|
||||
return TemplateResponse(request, f"search/{search_type}.html", data)
|
||||
return endpoints[search_type](request)
|
||||
|
||||
|
||||
def book_search(query, user, min_confidence, search_remote=False):
|
||||
def api_book_search(request):
|
||||
"""Return books via API response"""
|
||||
query = request.GET.get("q")
|
||||
query = isbn_check(query)
|
||||
min_confidence = request.GET.get("min_confidence", 0)
|
||||
# only return local book results via json so we don't cascade
|
||||
book_results = search(query, min_confidence=min_confidence)
|
||||
return JsonResponse(
|
||||
[format_search_result(r) for r in book_results[:10]], safe=False
|
||||
)
|
||||
|
||||
|
||||
def book_search(request):
|
||||
"""the real business is elsewhere"""
|
||||
query = request.GET.get("q")
|
||||
# check if query is isbn
|
||||
query = isbn_check(query)
|
||||
min_confidence = request.GET.get("min_confidence", 0)
|
||||
search_remote = request.GET.get("remote", False) and request.user.is_authenticated
|
||||
|
||||
# try a local-only search
|
||||
results = [{"results": search(query, min_confidence=min_confidence)}]
|
||||
if not user.is_authenticated or (results[0]["results"] and not search_remote):
|
||||
return results, False
|
||||
|
||||
# if there were no local results, or the request was for remote, search all sources
|
||||
results += connector_manager.search(query, min_confidence=min_confidence)
|
||||
return results, True
|
||||
local_results = search(query, min_confidence=min_confidence)
|
||||
paginated = Paginator(local_results, PAGE_LENGTH)
|
||||
page = paginated.get_page(request.GET.get("page"))
|
||||
data = {
|
||||
"query": query,
|
||||
"results": page,
|
||||
"type": "book",
|
||||
"remote": search_remote,
|
||||
"page_range": paginated.get_elided_page_range(
|
||||
page.number, on_each_side=2, on_ends=1
|
||||
),
|
||||
}
|
||||
# if a logged in user requested remote results or got no local results, try remote
|
||||
if request.user.is_authenticated and (not local_results or search_remote):
|
||||
data["remote_results"] = connector_manager.search(
|
||||
query, min_confidence=min_confidence
|
||||
)
|
||||
return TemplateResponse(request, "search/book.html", data)
|
||||
|
||||
|
||||
def user_search(query, viewer, *_):
|
||||
def user_search(request):
|
||||
"""cool kids members only user search"""
|
||||
viewer = request.user
|
||||
query = request.GET.get("q")
|
||||
data = {"type": "user", "query": query}
|
||||
# logged out viewers can't search users
|
||||
if not viewer.is_authenticated:
|
||||
return models.User.objects.none(), None
|
||||
return TemplateResponse(request, "search/user.html", data)
|
||||
|
||||
# use webfinger for mastodon style account@domain.com username to load the user if
|
||||
# they don't exist locally (handle_remote_webfinger will check the db)
|
||||
if re.match(regex.FULL_USERNAME, query):
|
||||
handle_remote_webfinger(query)
|
||||
|
||||
return (
|
||||
results = (
|
||||
models.User.viewer_aware_objects(viewer)
|
||||
.annotate(
|
||||
similarity=Greatest(
|
||||
|
@ -104,14 +112,23 @@ def user_search(query, viewer, *_):
|
|||
similarity__gt=0.5,
|
||||
)
|
||||
.order_by("-similarity")
|
||||
), None
|
||||
)
|
||||
paginated = Paginator(results, PAGE_LENGTH)
|
||||
page = paginated.get_page(request.GET.get("page"))
|
||||
data["results"] = page
|
||||
data["page_range"] = paginated.get_elided_page_range(
|
||||
page.number, on_each_side=2, on_ends=1
|
||||
)
|
||||
return TemplateResponse(request, "search/user.html", data)
|
||||
|
||||
|
||||
def list_search(query, viewer, *_):
|
||||
def list_search(request):
|
||||
"""any relevent lists?"""
|
||||
return (
|
||||
query = request.GET.get("q")
|
||||
data = {"query": query, "type": "list"}
|
||||
results = (
|
||||
models.List.privacy_filter(
|
||||
viewer,
|
||||
request.user,
|
||||
privacy_levels=["public", "followers"],
|
||||
)
|
||||
.annotate(
|
||||
|
@ -124,7 +141,14 @@ def list_search(query, viewer, *_):
|
|||
similarity__gt=0.1,
|
||||
)
|
||||
.order_by("-similarity")
|
||||
), None
|
||||
)
|
||||
paginated = Paginator(results, PAGE_LENGTH)
|
||||
page = paginated.get_page(request.GET.get("page"))
|
||||
data["results"] = page
|
||||
data["page_range"] = paginated.get_elided_page_range(
|
||||
page.number, on_each_side=2, on_ends=1
|
||||
)
|
||||
return TemplateResponse(request, "search/list.html", data)
|
||||
|
||||
|
||||
def isbn_check(query):
|
||||
|
|
Loading…
Reference in a new issue