diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index 18ccb942c..606678150 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -30,7 +30,6 @@ class AbstractMinimalConnector(ABC): "covers_url", "search_url", "isbn_search_url", - "max_query_count", "name", "identifier", "local", @@ -102,13 +101,6 @@ class AbstractConnector(AbstractMinimalConnector): # title we handle separately. self.book_mappings = [] - def is_available(self): - """check if you're allowed to use this connector""" - if self.max_query_count is not None: - if self.connector.query_count >= self.max_query_count: - return False - return True - def get_or_create_book(self, remote_id): """translate arbitrary json into an Activitypub dataclass""" # first, check if we have the origin_id saved diff --git a/bookwyrm/connectors/connector_manager.py b/bookwyrm/connectors/connector_manager.py index 43489669b..95c5959df 100644 --- a/bookwyrm/connectors/connector_manager.py +++ b/bookwyrm/connectors/connector_manager.py @@ -87,7 +87,7 @@ def first_search_result(query, min_confidence=0.1): def get_connectors(): """load all connectors""" - for info in models.Connector.objects.order_by("priority").all(): + for info in models.Connector.objects.filter(active=True).order_by("priority").all(): yield load_connector(info) diff --git a/bookwyrm/migrations/0074_auto_20210511_1829.py b/bookwyrm/migrations/0074_auto_20210511_1829.py new file mode 100644 index 000000000..287a51ad6 --- /dev/null +++ b/bookwyrm/migrations/0074_auto_20210511_1829.py @@ -0,0 +1,48 @@ +# Generated by Django 3.2 on 2021-05-11 18:29 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("bookwyrm", "0073_sitesettings_footer_item"), + ] + + operations = [ + migrations.RemoveField( + model_name="connector", + name="max_query_count", + ), + migrations.RemoveField( + model_name="connector", + name="politeness_delay", + ), + migrations.RemoveField( + model_name="connector", + name="query_count", + ), + migrations.RemoveField( + model_name="connector", + name="query_count_expiry", + ), + migrations.AddField( + model_name="connector", + name="active", + field=models.BooleanField(default=True), + ), + migrations.AddField( + model_name="connector", + name="deactivation_reason", + field=models.CharField( + blank=True, + choices=[ + ("self_deletion", "Self Deletion"), + ("moderator_deletion", "Moderator Deletion"), + ("domain_block", "Domain Block"), + ], + max_length=255, + null=True, + ), + ), + ] diff --git a/bookwyrm/models/base_model.py b/bookwyrm/models/base_model.py index e85ff7338..2cb7c0365 100644 --- a/bookwyrm/models/base_model.py +++ b/bookwyrm/models/base_model.py @@ -6,6 +6,16 @@ from bookwyrm.settings import DOMAIN from .fields import RemoteIdField +DeactivationReason = models.TextChoices( + "DeactivationReason", + [ + "self_deletion", + "moderator_deletion", + "domain_block", + ], +) + + class BookWyrmModel(models.Model): """shared fields""" diff --git a/bookwyrm/models/connector.py b/bookwyrm/models/connector.py index 625cdbed9..2d6717908 100644 --- a/bookwyrm/models/connector.py +++ b/bookwyrm/models/connector.py @@ -2,7 +2,7 @@ from django.db import models from bookwyrm.connectors.settings import CONNECTORS -from .base_model import BookWyrmModel +from .base_model import BookWyrmModel, DeactivationReason ConnectorFiles = models.TextChoices("ConnectorFiles", CONNECTORS) @@ -17,6 +17,10 @@ class Connector(BookWyrmModel): local = models.BooleanField(default=False) connector_file = models.CharField(max_length=255, choices=ConnectorFiles.choices) api_key = models.CharField(max_length=255, null=True, blank=True) + active = models.BooleanField(default=True) + deactivation_reason = models.CharField( + max_length=255, choices=DeactivationReason.choices, null=True, blank=True + ) base_url = models.CharField(max_length=255) books_url = models.CharField(max_length=255) @@ -24,13 +28,6 @@ class Connector(BookWyrmModel): search_url = models.CharField(max_length=255, null=True, blank=True) isbn_search_url = models.CharField(max_length=255, null=True, blank=True) - politeness_delay = models.IntegerField(null=True, blank=True) # seconds - max_query_count = models.IntegerField(null=True, blank=True) - # how many queries executed in a unit of time, like a day - query_count = models.IntegerField(default=0) - # when to reset the query count back to 0 (ie, after 1 day) - query_count_expiry = models.DateTimeField(auto_now_add=True, blank=True) - def __str__(self): return "{} ({})".format( self.identifier, diff --git a/bookwyrm/models/federated_server.py b/bookwyrm/models/federated_server.py index 7d446ca0d..e297c46c4 100644 --- a/bookwyrm/models/federated_server.py +++ b/bookwyrm/models/federated_server.py @@ -1,5 +1,6 @@ """ connections to external ActivityPub servers """ from urllib.parse import urlparse +from django.apps import apps from django.db import models from .base_model import BookWyrmModel @@ -34,6 +35,13 @@ class FederatedServer(BookWyrmModel): is_active=False, deactivation_reason="domain_block" ) + # check for related connectors + if self.application_type == "bookwyrm": + connector_model = apps.get_model("bookwyrm.Connector", require_ready=True) + connector_model.objects.filter( + identifier=self.server_name, active=True + ).update(active=False, deactivation_reason="domain_block") + def unblock(self): """unblock a server""" self.status = "federated" @@ -43,6 +51,15 @@ class FederatedServer(BookWyrmModel): is_active=True, deactivation_reason=None ) + # check for related connectors + if self.application_type == "bookwyrm": + connector_model = apps.get_model("bookwyrm.Connector", require_ready=True) + connector_model.objects.filter( + identifier=self.server_name, + active=False, + deactivation_reason="domain_block", + ).update(active=True, deactivation_reason=None) + @classmethod def is_blocked(cls, url): """look up if a domain is blocked""" diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index 5f0e64e3b..7d821c5ba 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -19,21 +19,11 @@ from bookwyrm.signatures import create_key_pair from bookwyrm.tasks import app from bookwyrm.utils import regex from .activitypub_mixin import OrderedCollectionPageMixin, ActivitypubMixin -from .base_model import BookWyrmModel +from .base_model import BookWyrmModel, DeactivationReason from .federated_server import FederatedServer from . import fields, Review -DeactivationReason = models.TextChoices( - "DeactivationReason", - [ - "self_deletion", - "moderator_deletion", - "domain_block", - ], -) - - class User(OrderedCollectionPageMixin, AbstractUser): """a user who wants to read books""" diff --git a/bookwyrm/templates/author.html b/bookwyrm/templates/author.html deleted file mode 100644 index a7ea9db56..000000000 --- a/bookwyrm/templates/author.html +++ /dev/null @@ -1,40 +0,0 @@ -{% extends 'layout.html' %} -{% load i18n %} -{% load bookwyrm_tags %} - -{% block title %}{{ author.name }}{% endblock %} - -{% block content %} -
{{ form.name }}
+{{ form.name }}
{% for error in form.name.errors %}{{ error | escape }}
{% endfor %} -{{ form.bio }}
++ + {{ form.aliases }} + {% trans "Separate multiple values with commas." %} +
+ {% for error in form.aliases.errors %} +{{ error | escape }}
+ {% endfor %} + +{{ form.bio }}
{% for error in form.bio.errors %}{{ error | escape }}
{% endfor %} -{{ form.wikipedia_link }}
+{{ form.wikipedia_link }}
{% for error in form.wikipedia_link.errors %}{{ error | escape }}
{% endfor %} -{{ form.born }}
++ + +
{% for error in form.born.errors %}{{ error | escape }}
{% endfor %} -{{ form.died }}
++ + +
{% for error in form.died.errors %}{{ error | escape }}
{% endfor %}{{ form.openlibrary_key }}
+{{ form.openlibrary_key }}
{% for error in form.openlibrary_key.errors %}{{ error | escape }}
{% endfor %} -{{ form.librarything_key }}
+{{ form.inventaire_id }}
+ {% for error in form.inventaire_id.errors %} +{{ error | escape }}
+ {% endfor %} + +{{ form.librarything_key }}
{% for error in form.librarything_key.errors %}{{ error | escape }}
{% endfor %} -{{ form.goodreads_key }}
+{{ form.goodreads_key }}
{% for error in form.goodreads_key.errors %}{{ error | escape }}
{% endfor %} diff --git a/bookwyrm/templates/book/book.html b/bookwyrm/templates/book/book.html index 40da64861..ff027a3dc 100644 --- a/bookwyrm/templates/book/book.html +++ b/bookwyrm/templates/book/book.html @@ -38,9 +38,8 @@ {% if user_authenticated and can_edit_book %} {% endif %} @@ -163,12 +162,9 @@{% trans "You don't have any reading activity for this book." %}
- {% endif %}{% trans "You don't have any reading activity for this book." %}
+ {% endif %} {% for readthrough in readthroughs %} {% include 'snippets/readthrough.html' with readthrough=readthrough %} {% endfor %} @@ -195,7 +194,7 @@ {% endif %}{% trans "rated it" %}
+ + {% include 'snippets/stars.html' with rating=rating.rating %} +{{ site.registration_closed_text | safe}}
+{{ site.registration_closed_text|safe}}
{% if site.allow_invite_requests %} {% if request_received %} @@ -64,7 +64,7 @@ {% for error in request_form.email.errors %} -{{ error | escape }}
+{{ error|escape }}
{% endfor %} @@ -80,7 +80,7 @@ {% include 'user/user_preview.html' with user=request.user %} {% if request.user.summary %}{{ item.fail_reason }}. @@ -90,8 +89,8 @@
Line {{ item.index }}: - {{ item.data|dict_key:'Title' }} by - {{ item.data|dict_key:'Author' }} + {{ item.data.Title }} by + {{ item.data.Author }}
{{ item.fail_reason }}. @@ -130,10 +129,10 @@ {% endif %}
- {% trans 'BookWyrm is open source software. You can contribute or report issues on GitHub.' %} + {% blocktrans %}BookWyrm's source code is freely available. You can contribute or report issues on GitHub.{% endblocktrans %}
{% if site.footer_item %} diff --git a/bookwyrm/templates/lists/list.html b/bookwyrm/templates/lists/list.html index 2902f793f..d740ee80f 100644 --- a/bookwyrm/templates/lists/list.html +++ b/bookwyrm/templates/lists/list.html @@ -1,12 +1,13 @@ {% extends 'lists/list_layout.html' %} {% load i18n %} {% load bookwyrm_tags %} +{% load markdown %} {% block panel %} {% if request.user == list.user and pending_count %}- {{ pending_count }} book{{ pending_count | pluralize }} awaiting your approval + {{ pending_count }} book{{ pending_count|pluralize }} awaiting your approval
{% include 'snippets/stars.html' with rating=item.book|rating:request.user %}
-+
{{ server.notes|to_markdown|safe }}
+hi
") - result = bookwyrm_tags.get_markdown("") + result = markdown.get_markdown("") self.assertEqual(result, "hi
") def test_get_mentions(self, _): """list of people mentioned""" status = models.Status.objects.create(content="hi", user=self.remote_user) - result = bookwyrm_tags.get_mentions(status, self.user) + result = status_display.get_mentions(status, self.user) self.assertEqual(result, "@rat@example.com ") - def test_get_status_preview_name(self, _): - """status context string""" - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - status = models.Status.objects.create(content="hi", user=self.user) - result = bookwyrm_tags.get_status_preview_name(status) - self.assertEqual(result, "status") - - status = models.Review.objects.create( - content="hi", user=self.user, book=self.book - ) - result = bookwyrm_tags.get_status_preview_name(status) - self.assertEqual(result, "review of Test Book") - - status = models.Comment.objects.create( - content="hi", user=self.user, book=self.book - ) - result = bookwyrm_tags.get_status_preview_name(status) - self.assertEqual(result, "comment on Test Book") - - status = models.Quotation.objects.create( - content="hi", user=self.user, book=self.book - ) - result = bookwyrm_tags.get_status_preview_name(status) - self.assertEqual(result, "quotation from Test Book") - def test_related_status(self, _): """gets the subclass model for a notification status""" with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): diff --git a/bookwyrm/tests/views/test_book.py b/bookwyrm/tests/views/test_book.py index 2acdb369f..909718ec3 100644 --- a/bookwyrm/tests/views/test_book.py +++ b/bookwyrm/tests/views/test_book.py @@ -59,7 +59,6 @@ class BookViews(TestCase): result.render() self.assertEqual(result.status_code, 200) - request = self.factory.get("") with patch("bookwyrm.views.books.is_api_request") as is_api: is_api.return_value = True result = view(request, self.book.id) diff --git a/bookwyrm/tests/views/test_federation.py b/bookwyrm/tests/views/test_federation.py index f17f7624b..4469040ee 100644 --- a/bookwyrm/tests/views/test_federation.py +++ b/bookwyrm/tests/views/test_federation.py @@ -60,7 +60,12 @@ class FederationViews(TestCase): def test_server_page_block(self): """block a server""" - server = models.FederatedServer.objects.create(server_name="hi.there.com") + server = models.FederatedServer.objects.create( + server_name="hi.there.com", application_type="bookwyrm" + ) + connector = models.Connector.objects.get( + identifier="hi.there.com", + ) self.remote_user.federated_server = server self.remote_user.save() @@ -72,17 +77,32 @@ class FederationViews(TestCase): request.user.is_superuser = True view(request, server.id) + server.refresh_from_db() self.remote_user.refresh_from_db() self.assertEqual(server.status, "blocked") + # and the user was deactivated self.assertFalse(self.remote_user.is_active) + self.assertEqual(self.remote_user.deactivation_reason, "domain_block") + + # and the connector was disabled + connector.refresh_from_db() + self.assertFalse(connector.active) + self.assertEqual(connector.deactivation_reason, "domain_block") def test_server_page_unblock(self): """unblock a server""" server = models.FederatedServer.objects.create( - server_name="hi.there.com", status="blocked" + server_name="hi.there.com", status="blocked", application_type="bookwyrm" ) + connector = models.Connector.objects.get( + identifier="hi.there.com", + ) + connector.active = False + connector.deactivation_reason = "domain_block" + connector.save() + self.remote_user.federated_server = server self.remote_user.is_active = False self.remote_user.deactivation_reason = "domain_block" @@ -96,8 +116,15 @@ class FederationViews(TestCase): server.refresh_from_db() self.remote_user.refresh_from_db() self.assertEqual(server.status, "federated") + # and the user was re-activated self.assertTrue(self.remote_user.is_active) + self.assertIsNone(self.remote_user.deactivation_reason) + + # and the connector was re-enabled + connector.refresh_from_db() + self.assertTrue(connector.active) + self.assertIsNone(connector.deactivation_reason) def test_add_view_get(self): """there are so many views, this just makes sure it LOADS""" diff --git a/bookwyrm/tests/views/test_list.py b/bookwyrm/tests/views/test_list.py index 3de35b1ed..d6ad0e865 100644 --- a/bookwyrm/tests/views/test_list.py +++ b/bookwyrm/tests/views/test_list.py @@ -122,6 +122,14 @@ class ListViews(TestCase): view = views.List.as_view() request = self.factory.get("") request.user = self.local_user + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + models.ListItem.objects.create( + book_list=self.list, + user=self.local_user, + book=self.book, + approved=True, + order=1, + ) with patch("bookwyrm.views.list.is_api_request") as is_api: is_api.return_value = False @@ -130,6 +138,81 @@ class ListViews(TestCase): result.render() self.assertEqual(result.status_code, 200) + def test_list_page_sorted(self): + """there are so many views, this just makes sure it LOADS""" + view = views.List.as_view() + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + for (i, book) in enumerate([self.book, self.book_two, self.book_three]): + models.ListItem.objects.create( + book_list=self.list, + user=self.local_user, + book=book, + approved=True, + order=i + 1, + ) + + request = self.factory.get("/?sort_by=order") + request.user = self.local_user + with patch("bookwyrm.views.list.is_api_request") as is_api: + is_api.return_value = False + result = view(request, self.list.id) + self.assertIsInstance(result, TemplateResponse) + result.render() + self.assertEqual(result.status_code, 200) + + request = self.factory.get("/?sort_by=title") + request.user = self.local_user + with patch("bookwyrm.views.list.is_api_request") as is_api: + is_api.return_value = False + result = view(request, self.list.id) + self.assertIsInstance(result, TemplateResponse) + result.render() + self.assertEqual(result.status_code, 200) + + request = self.factory.get("/?sort_by=rating") + request.user = self.local_user + with patch("bookwyrm.views.list.is_api_request") as is_api: + is_api.return_value = False + result = view(request, self.list.id) + self.assertIsInstance(result, TemplateResponse) + result.render() + self.assertEqual(result.status_code, 200) + + request = self.factory.get("/?sort_by=sdkfh") + request.user = self.local_user + with patch("bookwyrm.views.list.is_api_request") as is_api: + is_api.return_value = False + result = view(request, self.list.id) + self.assertIsInstance(result, TemplateResponse) + result.render() + self.assertEqual(result.status_code, 200) + + def test_list_page_empty(self): + """there are so many views, this just makes sure it LOADS""" + view = views.List.as_view() + request = self.factory.get("") + request.user = self.local_user + + with patch("bookwyrm.views.list.is_api_request") as is_api: + is_api.return_value = False + result = view(request, self.list.id) + self.assertIsInstance(result, TemplateResponse) + result.render() + self.assertEqual(result.status_code, 200) + + def test_list_page_logged_out(self): + """there are so many views, this just makes sure it LOADS""" + view = views.List.as_view() + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + models.ListItem.objects.create( + book_list=self.list, + user=self.local_user, + book=self.book, + approved=True, + order=1, + ) + + request = self.factory.get("") request.user = self.anonymous_user with patch("bookwyrm.views.list.is_api_request") as is_api: is_api.return_value = False @@ -138,12 +221,32 @@ class ListViews(TestCase): result.render() self.assertEqual(result.status_code, 200) + def test_list_page_json_view(self): + """there are so many views, this just makes sure it LOADS""" + view = views.List.as_view() + request = self.factory.get("") + request.user = self.local_user + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + models.ListItem.objects.create( + book_list=self.list, + user=self.local_user, + book=self.book, + approved=True, + order=1, + ) + with patch("bookwyrm.views.list.is_api_request") as is_api: is_api.return_value = True result = view(request, self.list.id) self.assertIsInstance(result, ActivitypubResponse) self.assertEqual(result.status_code, 200) + def test_list_page_json_view_page(self): + """there are so many views, this just makes sure it LOADS""" + view = views.List.as_view() + request = self.factory.get("") + request.user = self.local_user + request = self.factory.get("/?page=1") request.user = self.local_user with patch("bookwyrm.views.list.is_api_request") as is_api: @@ -204,466 +307,34 @@ class ListViews(TestCase): result = view(request, self.list.id) self.assertEqual(result.status_code, 302) - def test_curate_approve(self): - """approve a pending item""" - view = views.Curate.as_view() + def test_user_lists_page(self): + """there are so many views, this just makes sure it LOADS""" + view = views.UserLists.as_view() with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - pending = models.ListItem.objects.create( - book_list=self.list, - user=self.local_user, - book=self.book, - approved=False, - order=1, + models.List.objects.create(name="Public list", user=self.local_user) + models.List.objects.create( + name="Private list", privacy="direct", user=self.local_user ) - - request = self.factory.post( - "", - { - "item": pending.id, - "approved": "true", - }, - ) + request = self.factory.get("") request.user = self.local_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock: - view(request, self.list.id) + result = view(request, self.local_user.localname) + self.assertIsInstance(result, TemplateResponse) + result.render() + self.assertEqual(result.status_code, 200) - self.assertEqual(mock.call_count, 2) - activity = json.loads(mock.call_args[0][1]) - self.assertEqual(activity["type"], "Add") - self.assertEqual(activity["actor"], self.local_user.remote_id) - self.assertEqual(activity["target"], self.list.remote_id) - - pending.refresh_from_db() - self.assertEqual(self.list.books.count(), 1) - self.assertEqual(self.list.listitem_set.first(), pending) - self.assertTrue(pending.approved) - - def test_curate_reject(self): - """approve a pending item""" - view = views.Curate.as_view() + def test_user_lists_page_logged_out(self): + """there are so many views, this just makes sure it LOADS""" + view = views.UserLists.as_view() with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - pending = models.ListItem.objects.create( - book_list=self.list, - user=self.local_user, - book=self.book, - approved=False, - order=1, + models.List.objects.create(name="Public list", user=self.local_user) + models.List.objects.create( + name="Private list", privacy="direct", user=self.local_user ) + request = self.factory.get("") + request.user = self.anonymous_user - request = self.factory.post( - "", - { - "item": pending.id, - "approved": "false", - }, - ) - request.user = self.local_user - - view(request, self.list.id) - - self.assertFalse(self.list.books.exists()) - self.assertFalse(models.ListItem.objects.exists()) - - def test_add_book(self): - """put a book on a list""" - request = self.factory.post( - "", - { - "book": self.book.id, - "list": self.list.id, - }, - ) - request.user = self.local_user - - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock: - views.list.add_book(request) - self.assertEqual(mock.call_count, 1) - activity = json.loads(mock.call_args[0][1]) - self.assertEqual(activity["type"], "Add") - self.assertEqual(activity["actor"], self.local_user.remote_id) - self.assertEqual(activity["target"], self.list.remote_id) - - item = self.list.listitem_set.get() - self.assertEqual(item.book, self.book) - self.assertEqual(item.user, self.local_user) - self.assertTrue(item.approved) - - def test_add_two_books(self): - """ - Putting two books on the list. The first should have an order value of - 1 and the second should have an order value of 2. - """ - request_one = self.factory.post( - "", - { - "book": self.book.id, - "list": self.list.id, - }, - ) - request_one.user = self.local_user - - request_two = self.factory.post( - "", - { - "book": self.book_two.id, - "list": self.list.id, - }, - ) - request_two.user = self.local_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - views.list.add_book(request_one) - views.list.add_book(request_two) - - items = self.list.listitem_set.order_by("order").all() - self.assertEqual(items[0].book, self.book) - self.assertEqual(items[1].book, self.book_two) - self.assertEqual(items[0].order, 1) - self.assertEqual(items[1].order, 2) - - def test_add_three_books_and_remove_second(self): - """ - Put three books on a list and then remove the one in the middle. The - ordering of the list should adjust to not have a gap. - """ - request_one = self.factory.post( - "", - { - "book": self.book.id, - "list": self.list.id, - }, - ) - request_one.user = self.local_user - - request_two = self.factory.post( - "", - { - "book": self.book_two.id, - "list": self.list.id, - }, - ) - request_two.user = self.local_user - - request_three = self.factory.post( - "", - { - "book": self.book_three.id, - "list": self.list.id, - }, - ) - request_three.user = self.local_user - - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - views.list.add_book(request_one) - views.list.add_book(request_two) - views.list.add_book(request_three) - - items = self.list.listitem_set.order_by("order").all() - self.assertEqual(items[0].book, self.book) - self.assertEqual(items[1].book, self.book_two) - self.assertEqual(items[2].book, self.book_three) - self.assertEqual(items[0].order, 1) - self.assertEqual(items[1].order, 2) - self.assertEqual(items[2].order, 3) - - remove_request = self.factory.post("", {"item": items[1].id}) - remove_request.user = self.local_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - views.list.remove_book(remove_request, self.list.id) - items = self.list.listitem_set.order_by("order").all() - self.assertEqual(items[0].book, self.book) - self.assertEqual(items[1].book, self.book_three) - self.assertEqual(items[0].order, 1) - self.assertEqual(items[1].order, 2) - - def test_adding_book_with_a_pending_book(self): - """ - When a list contains any pending books, the pending books should have - be at the end of the list by order. If a book is added while a book is - pending, its order should precede the pending books. - """ - request = self.factory.post( - "", - { - "book": self.book_three.id, - "list": self.list.id, - }, - ) - request.user = self.local_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - models.ListItem.objects.create( - book_list=self.list, - user=self.local_user, - book=self.book, - approved=True, - order=1, - ) - models.ListItem.objects.create( - book_list=self.list, - user=self.rat, - book=self.book_two, - approved=False, - order=2, - ) - views.list.add_book(request) - - items = self.list.listitem_set.order_by("order").all() - self.assertEqual(items[0].book, self.book) - self.assertEqual(items[0].order, 1) - self.assertTrue(items[0].approved) - - self.assertEqual(items[1].book, self.book_three) - self.assertEqual(items[1].order, 2) - self.assertTrue(items[1].approved) - - self.assertEqual(items[2].book, self.book_two) - self.assertEqual(items[2].order, 3) - self.assertFalse(items[2].approved) - - def test_approving_one_pending_book_from_multiple(self): - """ - When a list contains any pending books, the pending books should have - be at the end of the list by order. If a pending book is approved, then - its order should be at the end of the approved books and before the - remaining pending books. - """ - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - models.ListItem.objects.create( - book_list=self.list, - user=self.local_user, - book=self.book, - approved=True, - order=1, - ) - models.ListItem.objects.create( - book_list=self.list, - user=self.local_user, - book=self.book_two, - approved=True, - order=2, - ) - models.ListItem.objects.create( - book_list=self.list, - user=self.rat, - book=self.book_three, - approved=False, - order=3, - ) - to_be_approved = models.ListItem.objects.create( - book_list=self.list, - user=self.rat, - book=self.book_four, - approved=False, - order=4, - ) - - view = views.Curate.as_view() - request = self.factory.post( - "", - { - "item": to_be_approved.id, - "approved": "true", - }, - ) - request.user = self.local_user - - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - view(request, self.list.id) - - items = self.list.listitem_set.order_by("order").all() - self.assertEqual(items[0].book, self.book) - self.assertEqual(items[0].order, 1) - self.assertTrue(items[0].approved) - - self.assertEqual(items[1].book, self.book_two) - self.assertEqual(items[1].order, 2) - self.assertTrue(items[1].approved) - - self.assertEqual(items[2].book, self.book_four) - self.assertEqual(items[2].order, 3) - self.assertTrue(items[2].approved) - - self.assertEqual(items[3].book, self.book_three) - self.assertEqual(items[3].order, 4) - self.assertFalse(items[3].approved) - - def test_add_three_books_and_move_last_to_first(self): - """ - Put three books on the list and move the last book to the first - position. - """ - request_one = self.factory.post( - "", - { - "book": self.book.id, - "list": self.list.id, - }, - ) - request_one.user = self.local_user - - request_two = self.factory.post( - "", - { - "book": self.book_two.id, - "list": self.list.id, - }, - ) - request_two.user = self.local_user - - request_three = self.factory.post( - "", - { - "book": self.book_three.id, - "list": self.list.id, - }, - ) - request_three.user = self.local_user - - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - views.list.add_book(request_one) - views.list.add_book(request_two) - views.list.add_book(request_three) - - items = self.list.listitem_set.order_by("order").all() - self.assertEqual(items[0].book, self.book) - self.assertEqual(items[1].book, self.book_two) - self.assertEqual(items[2].book, self.book_three) - self.assertEqual(items[0].order, 1) - self.assertEqual(items[1].order, 2) - self.assertEqual(items[2].order, 3) - - set_position_request = self.factory.post("", {"position": 1}) - set_position_request.user = self.local_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - views.list.set_book_position(set_position_request, items[2].id) - items = self.list.listitem_set.order_by("order").all() - self.assertEqual(items[0].book, self.book_three) - self.assertEqual(items[1].book, self.book) - self.assertEqual(items[2].book, self.book_two) - self.assertEqual(items[0].order, 1) - self.assertEqual(items[1].order, 2) - self.assertEqual(items[2].order, 3) - - def test_add_book_outsider(self): - """put a book on a list""" - self.list.curation = "open" - self.list.save(broadcast=False) - request = self.factory.post( - "", - { - "book": self.book.id, - "list": self.list.id, - }, - ) - request.user = self.rat - - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock: - views.list.add_book(request) - self.assertEqual(mock.call_count, 1) - activity = json.loads(mock.call_args[0][1]) - self.assertEqual(activity["type"], "Add") - self.assertEqual(activity["actor"], self.rat.remote_id) - self.assertEqual(activity["target"], self.list.remote_id) - - item = self.list.listitem_set.get() - self.assertEqual(item.book, self.book) - self.assertEqual(item.user, self.rat) - self.assertTrue(item.approved) - - def test_add_book_pending(self): - """put a book on a list awaiting approval""" - self.list.curation = "curated" - self.list.save(broadcast=False) - request = self.factory.post( - "", - { - "book": self.book.id, - "list": self.list.id, - }, - ) - request.user = self.rat - - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock: - views.list.add_book(request) - - self.assertEqual(mock.call_count, 1) - activity = json.loads(mock.call_args[0][1]) - - self.assertEqual(activity["type"], "Add") - self.assertEqual(activity["actor"], self.rat.remote_id) - self.assertEqual(activity["target"], self.list.remote_id) - - item = self.list.listitem_set.get() - self.assertEqual(activity["object"]["id"], item.remote_id) - - self.assertEqual(item.book, self.book) - self.assertEqual(item.user, self.rat) - self.assertFalse(item.approved) - - def test_add_book_self_curated(self): - """put a book on a list automatically approved""" - self.list.curation = "curated" - self.list.save(broadcast=False) - request = self.factory.post( - "", - { - "book": self.book.id, - "list": self.list.id, - }, - ) - request.user = self.local_user - - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock: - views.list.add_book(request) - self.assertEqual(mock.call_count, 1) - activity = json.loads(mock.call_args[0][1]) - self.assertEqual(activity["type"], "Add") - self.assertEqual(activity["actor"], self.local_user.remote_id) - self.assertEqual(activity["target"], self.list.remote_id) - - item = self.list.listitem_set.get() - self.assertEqual(item.book, self.book) - self.assertEqual(item.user, self.local_user) - self.assertTrue(item.approved) - - def test_remove_book(self): - """take an item off a list""" - - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - item = models.ListItem.objects.create( - book_list=self.list, - user=self.local_user, - book=self.book, - order=1, - ) - self.assertTrue(self.list.listitem_set.exists()) - - request = self.factory.post( - "", - { - "item": item.id, - }, - ) - request.user = self.local_user - - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - views.list.remove_book(request, self.list.id) - self.assertFalse(self.list.listitem_set.exists()) - - def test_remove_book_unauthorized(self): - """take an item off a list""" - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - item = models.ListItem.objects.create( - book_list=self.list, user=self.local_user, book=self.book, order=1 - ) - self.assertTrue(self.list.listitem_set.exists()) - request = self.factory.post( - "", - { - "item": item.id, - }, - ) - request.user = self.rat - - views.list.remove_book(request, self.list.id) - self.assertTrue(self.list.listitem_set.exists()) + result = view(request, self.local_user.username) + self.assertIsInstance(result, TemplateResponse) + result.render() + self.assertEqual(result.status_code, 200) diff --git a/bookwyrm/tests/views/test_list_actions.py b/bookwyrm/tests/views/test_list_actions.py new file mode 100644 index 000000000..30c44a2cb --- /dev/null +++ b/bookwyrm/tests/views/test_list_actions.py @@ -0,0 +1,529 @@ +""" test for app action functionality """ +import json +from unittest.mock import patch + +from django.contrib.auth.models import AnonymousUser +from django.test import TestCase +from django.test.client import RequestFactory + +from bookwyrm import models, views + +# pylint: disable=unused-argument +class ListActionViews(TestCase): + """tag views""" + + def setUp(self): + """we need basic test data and mocks""" + self.factory = RequestFactory() + self.local_user = models.User.objects.create_user( + "mouse@local.com", + "mouse@mouse.com", + "mouseword", + local=True, + localname="mouse", + remote_id="https://example.com/users/mouse", + ) + self.rat = models.User.objects.create_user( + "rat@local.com", + "rat@rat.com", + "ratword", + local=True, + localname="rat", + remote_id="https://example.com/users/rat", + ) + work = models.Work.objects.create(title="Work") + self.book = models.Edition.objects.create( + title="Example Edition", + remote_id="https://example.com/book/1", + parent_work=work, + ) + work_two = models.Work.objects.create(title="Labori") + self.book_two = models.Edition.objects.create( + title="Example Edition 2", + remote_id="https://example.com/book/2", + parent_work=work_two, + ) + work_three = models.Work.objects.create(title="Trabajar") + self.book_three = models.Edition.objects.create( + title="Example Edition 3", + remote_id="https://example.com/book/3", + parent_work=work_three, + ) + work_four = models.Work.objects.create(title="Travailler") + self.book_four = models.Edition.objects.create( + title="Example Edition 4", + remote_id="https://example.com/book/4", + parent_work=work_four, + ) + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + self.list = models.List.objects.create( + name="Test List", user=self.local_user + ) + self.anonymous_user = AnonymousUser + self.anonymous_user.is_authenticated = False + models.SiteSettings.objects.create() + + def test_curate_approve(self): + """approve a pending item""" + view = views.Curate.as_view() + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + pending = models.ListItem.objects.create( + book_list=self.list, + user=self.local_user, + book=self.book, + approved=False, + order=1, + ) + + request = self.factory.post( + "", + { + "item": pending.id, + "approved": "true", + }, + ) + request.user = self.local_user + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock: + view(request, self.list.id) + + self.assertEqual(mock.call_count, 2) + activity = json.loads(mock.call_args[0][1]) + self.assertEqual(activity["type"], "Add") + self.assertEqual(activity["actor"], self.local_user.remote_id) + self.assertEqual(activity["target"], self.list.remote_id) + + pending.refresh_from_db() + self.assertEqual(self.list.books.count(), 1) + self.assertEqual(self.list.listitem_set.first(), pending) + self.assertTrue(pending.approved) + + def test_curate_reject(self): + """approve a pending item""" + view = views.Curate.as_view() + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + pending = models.ListItem.objects.create( + book_list=self.list, + user=self.local_user, + book=self.book, + approved=False, + order=1, + ) + + request = self.factory.post( + "", + { + "item": pending.id, + "approved": "false", + }, + ) + request.user = self.local_user + + view(request, self.list.id) + + self.assertFalse(self.list.books.exists()) + self.assertFalse(models.ListItem.objects.exists()) + + def test_add_book(self): + """put a book on a list""" + request = self.factory.post( + "", + { + "book": self.book.id, + "list": self.list.id, + }, + ) + request.user = self.local_user + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock: + views.list.add_book(request) + self.assertEqual(mock.call_count, 1) + activity = json.loads(mock.call_args[0][1]) + self.assertEqual(activity["type"], "Add") + self.assertEqual(activity["actor"], self.local_user.remote_id) + self.assertEqual(activity["target"], self.list.remote_id) + + item = self.list.listitem_set.get() + self.assertEqual(item.book, self.book) + self.assertEqual(item.user, self.local_user) + self.assertTrue(item.approved) + + def test_add_two_books(self): + """ + Putting two books on the list. The first should have an order value of + 1 and the second should have an order value of 2. + """ + request_one = self.factory.post( + "", + { + "book": self.book.id, + "list": self.list.id, + }, + ) + request_one.user = self.local_user + + request_two = self.factory.post( + "", + { + "book": self.book_two.id, + "list": self.list.id, + }, + ) + request_two.user = self.local_user + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + views.list.add_book(request_one) + views.list.add_book(request_two) + + items = self.list.listitem_set.order_by("order").all() + self.assertEqual(items[0].book, self.book) + self.assertEqual(items[1].book, self.book_two) + self.assertEqual(items[0].order, 1) + self.assertEqual(items[1].order, 2) + + def test_add_three_books_and_remove_second(self): + """ + Put three books on a list and then remove the one in the middle. The + ordering of the list should adjust to not have a gap. + """ + request_one = self.factory.post( + "", + { + "book": self.book.id, + "list": self.list.id, + }, + ) + request_one.user = self.local_user + + request_two = self.factory.post( + "", + { + "book": self.book_two.id, + "list": self.list.id, + }, + ) + request_two.user = self.local_user + + request_three = self.factory.post( + "", + { + "book": self.book_three.id, + "list": self.list.id, + }, + ) + request_three.user = self.local_user + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + views.list.add_book(request_one) + views.list.add_book(request_two) + views.list.add_book(request_three) + + items = self.list.listitem_set.order_by("order").all() + self.assertEqual(items[0].book, self.book) + self.assertEqual(items[1].book, self.book_two) + self.assertEqual(items[2].book, self.book_three) + self.assertEqual(items[0].order, 1) + self.assertEqual(items[1].order, 2) + self.assertEqual(items[2].order, 3) + + remove_request = self.factory.post("", {"item": items[1].id}) + remove_request.user = self.local_user + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + views.list.remove_book(remove_request, self.list.id) + items = self.list.listitem_set.order_by("order").all() + self.assertEqual(items[0].book, self.book) + self.assertEqual(items[1].book, self.book_three) + self.assertEqual(items[0].order, 1) + self.assertEqual(items[1].order, 2) + + def test_adding_book_with_a_pending_book(self): + """ + When a list contains any pending books, the pending books should have + be at the end of the list by order. If a book is added while a book is + pending, its order should precede the pending books. + """ + request = self.factory.post( + "", + { + "book": self.book_three.id, + "list": self.list.id, + }, + ) + request.user = self.local_user + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + models.ListItem.objects.create( + book_list=self.list, + user=self.local_user, + book=self.book, + approved=True, + order=1, + ) + models.ListItem.objects.create( + book_list=self.list, + user=self.rat, + book=self.book_two, + approved=False, + order=2, + ) + views.list.add_book(request) + + items = self.list.listitem_set.order_by("order").all() + self.assertEqual(items[0].book, self.book) + self.assertEqual(items[0].order, 1) + self.assertTrue(items[0].approved) + + self.assertEqual(items[1].book, self.book_three) + self.assertEqual(items[1].order, 2) + self.assertTrue(items[1].approved) + + self.assertEqual(items[2].book, self.book_two) + self.assertEqual(items[2].order, 3) + self.assertFalse(items[2].approved) + + def test_approving_one_pending_book_from_multiple(self): + """ + When a list contains any pending books, the pending books should have + be at the end of the list by order. If a pending book is approved, then + its order should be at the end of the approved books and before the + remaining pending books. + """ + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + models.ListItem.objects.create( + book_list=self.list, + user=self.local_user, + book=self.book, + approved=True, + order=1, + ) + models.ListItem.objects.create( + book_list=self.list, + user=self.local_user, + book=self.book_two, + approved=True, + order=2, + ) + models.ListItem.objects.create( + book_list=self.list, + user=self.rat, + book=self.book_three, + approved=False, + order=3, + ) + to_be_approved = models.ListItem.objects.create( + book_list=self.list, + user=self.rat, + book=self.book_four, + approved=False, + order=4, + ) + + view = views.Curate.as_view() + request = self.factory.post( + "", + { + "item": to_be_approved.id, + "approved": "true", + }, + ) + request.user = self.local_user + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + view(request, self.list.id) + + items = self.list.listitem_set.order_by("order").all() + self.assertEqual(items[0].book, self.book) + self.assertEqual(items[0].order, 1) + self.assertTrue(items[0].approved) + + self.assertEqual(items[1].book, self.book_two) + self.assertEqual(items[1].order, 2) + self.assertTrue(items[1].approved) + + self.assertEqual(items[2].book, self.book_four) + self.assertEqual(items[2].order, 3) + self.assertTrue(items[2].approved) + + self.assertEqual(items[3].book, self.book_three) + self.assertEqual(items[3].order, 4) + self.assertFalse(items[3].approved) + + def test_add_three_books_and_move_last_to_first(self): + """ + Put three books on the list and move the last book to the first + position. + """ + request_one = self.factory.post( + "", + { + "book": self.book.id, + "list": self.list.id, + }, + ) + request_one.user = self.local_user + + request_two = self.factory.post( + "", + { + "book": self.book_two.id, + "list": self.list.id, + }, + ) + request_two.user = self.local_user + + request_three = self.factory.post( + "", + { + "book": self.book_three.id, + "list": self.list.id, + }, + ) + request_three.user = self.local_user + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + views.list.add_book(request_one) + views.list.add_book(request_two) + views.list.add_book(request_three) + + items = self.list.listitem_set.order_by("order").all() + self.assertEqual(items[0].book, self.book) + self.assertEqual(items[1].book, self.book_two) + self.assertEqual(items[2].book, self.book_three) + self.assertEqual(items[0].order, 1) + self.assertEqual(items[1].order, 2) + self.assertEqual(items[2].order, 3) + + set_position_request = self.factory.post("", {"position": 1}) + set_position_request.user = self.local_user + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + views.list.set_book_position(set_position_request, items[2].id) + items = self.list.listitem_set.order_by("order").all() + self.assertEqual(items[0].book, self.book_three) + self.assertEqual(items[1].book, self.book) + self.assertEqual(items[2].book, self.book_two) + self.assertEqual(items[0].order, 1) + self.assertEqual(items[1].order, 2) + self.assertEqual(items[2].order, 3) + + def test_add_book_outsider(self): + """put a book on a list""" + self.list.curation = "open" + self.list.save(broadcast=False) + request = self.factory.post( + "", + { + "book": self.book.id, + "list": self.list.id, + }, + ) + request.user = self.rat + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock: + views.list.add_book(request) + self.assertEqual(mock.call_count, 1) + activity = json.loads(mock.call_args[0][1]) + self.assertEqual(activity["type"], "Add") + self.assertEqual(activity["actor"], self.rat.remote_id) + self.assertEqual(activity["target"], self.list.remote_id) + + item = self.list.listitem_set.get() + self.assertEqual(item.book, self.book) + self.assertEqual(item.user, self.rat) + self.assertTrue(item.approved) + + def test_add_book_pending(self): + """put a book on a list awaiting approval""" + self.list.curation = "curated" + self.list.save(broadcast=False) + request = self.factory.post( + "", + { + "book": self.book.id, + "list": self.list.id, + }, + ) + request.user = self.rat + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock: + views.list.add_book(request) + + self.assertEqual(mock.call_count, 1) + activity = json.loads(mock.call_args[0][1]) + + self.assertEqual(activity["type"], "Add") + self.assertEqual(activity["actor"], self.rat.remote_id) + self.assertEqual(activity["target"], self.list.remote_id) + + item = self.list.listitem_set.get() + self.assertEqual(activity["object"]["id"], item.remote_id) + + self.assertEqual(item.book, self.book) + self.assertEqual(item.user, self.rat) + self.assertFalse(item.approved) + + def test_add_book_self_curated(self): + """put a book on a list automatically approved""" + self.list.curation = "curated" + self.list.save(broadcast=False) + request = self.factory.post( + "", + { + "book": self.book.id, + "list": self.list.id, + }, + ) + request.user = self.local_user + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock: + views.list.add_book(request) + self.assertEqual(mock.call_count, 1) + activity = json.loads(mock.call_args[0][1]) + self.assertEqual(activity["type"], "Add") + self.assertEqual(activity["actor"], self.local_user.remote_id) + self.assertEqual(activity["target"], self.list.remote_id) + + item = self.list.listitem_set.get() + self.assertEqual(item.book, self.book) + self.assertEqual(item.user, self.local_user) + self.assertTrue(item.approved) + + def test_remove_book(self): + """take an item off a list""" + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + item = models.ListItem.objects.create( + book_list=self.list, + user=self.local_user, + book=self.book, + order=1, + ) + self.assertTrue(self.list.listitem_set.exists()) + + request = self.factory.post( + "", + { + "item": item.id, + }, + ) + request.user = self.local_user + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + views.list.remove_book(request, self.list.id) + self.assertFalse(self.list.listitem_set.exists()) + + def test_remove_book_unauthorized(self): + """take an item off a list""" + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + item = models.ListItem.objects.create( + book_list=self.list, user=self.local_user, book=self.book, order=1 + ) + self.assertTrue(self.list.listitem_set.exists()) + request = self.factory.post( + "", + { + "item": item.id, + }, + ) + request.user = self.rat + + views.list.remove_book(request, self.list.id) + self.assertTrue(self.list.listitem_set.exists()) diff --git a/bookwyrm/views/author.py b/bookwyrm/views/author.py index 41298161c..e51dc7d83 100644 --- a/bookwyrm/views/author.py +++ b/bookwyrm/views/author.py @@ -29,7 +29,7 @@ class Author(View): "author": author, "books": [b.default_edition for b in books], } - return TemplateResponse(request, "author.html", data) + return TemplateResponse(request, "author/author.html", data) @method_decorator(login_required, name="dispatch") @@ -43,7 +43,7 @@ class EditAuthor(View): """info about a book""" author = get_object_or_404(models.Author, id=author_id) data = {"author": author, "form": forms.AuthorForm(instance=author)} - return TemplateResponse(request, "edit_author.html", data) + return TemplateResponse(request, "author/edit_author.html", data) def post(self, request, author_id): """edit a author cool""" @@ -52,7 +52,7 @@ class EditAuthor(View): form = forms.AuthorForm(request.POST, request.FILES, instance=author) if not form.is_valid(): data = {"author": author, "form": form} - return TemplateResponse(request, "edit_author.html", data) + return TemplateResponse(request, "author/edit_author.html", data) author = form.save() return redirect("/author/%s" % author.id) diff --git a/bookwyrm/views/books.py b/bookwyrm/views/books.py index 6005c9fde..fee22e89a 100644 --- a/bookwyrm/views/books.py +++ b/bookwyrm/views/books.py @@ -30,6 +30,7 @@ class Book(View): def get(self, request, book_id, user_statuses=False): """info about a book""" + user_statuses = user_statuses if request.user.is_authenticated else False try: book = models.Book.objects.select_subclasses().get(id=book_id) except models.Book.DoesNotExist: @@ -51,9 +52,9 @@ class Book(View): ) # the reviews to show - if user_statuses and request.user.is_authenticated: + if user_statuses: if user_statuses == "review": - queryset = book.review_set + queryset = book.review_set.select_subclasses() elif user_statuses == "comment": queryset = book.comment_set else: @@ -67,7 +68,9 @@ class Book(View): "book": book, "statuses": paginated.get_page(request.GET.get("page")), "review_count": reviews.count(), - "ratings": reviews.filter(Q(content__isnull=True) | Q(content="")), + "ratings": reviews.filter(Q(content__isnull=True) | Q(content="")) + if not user_statuses + else None, "rating": reviews.aggregate(Avg("rating"))["rating__avg"], "lists": privacy_filter( request.user, book.list_set.filter(listitem__approved=True) diff --git a/bookwyrm/views/list.py b/bookwyrm/views/list.py index bfd617907..75bb5d48c 100644 --- a/bookwyrm/views/list.py +++ b/bookwyrm/views/list.py @@ -5,7 +5,7 @@ from urllib.parse import urlencode from django.contrib.auth.decorators import login_required from django.core.paginator import Paginator from django.db import IntegrityError, transaction -from django.db.models import Avg, Count, Q, Max +from django.db.models import Avg, Count, DecimalField, Q, Max from django.db.models.functions import Coalesce from django.http import HttpResponseNotFound, HttpResponseBadRequest, HttpResponse from django.shortcuts import get_object_or_404, redirect @@ -108,31 +108,23 @@ class List(View): if direction not in ("ascending", "descending"): direction = "ascending" - internal_sort_by = { + directional_sort_by = { "order": "order", "title": "book__title", "rating": "average_rating", - } - directional_sort_by = internal_sort_by[sort_by] + }[sort_by] if direction == "descending": directional_sort_by = "-" + directional_sort_by - if sort_by == "order": - items = book_list.listitem_set.filter(approved=True).order_by( - directional_sort_by - ) - elif sort_by == "title": - items = book_list.listitem_set.filter(approved=True).order_by( - directional_sort_by - ) - elif sort_by == "rating": - items = ( - book_list.listitem_set.annotate( - average_rating=Avg(Coalesce("book__review__rating", 0)) + items = book_list.listitem_set + if sort_by == "rating": + items = items.annotate( + average_rating=Avg( + Coalesce("book__review__rating", 0.0), + output_field=DecimalField(), ) - .filter(approved=True) - .order_by(directional_sort_by) ) + items = items.filter(approved=True).order_by(directional_sort_by) paginated = Paginator(items, PAGE_LENGTH) diff --git a/bookwyrm/views/shelf.py b/bookwyrm/views/shelf.py index 5312ac212..758e290ed 100644 --- a/bookwyrm/views/shelf.py +++ b/bookwyrm/views/shelf.py @@ -2,6 +2,7 @@ from collections import namedtuple from django.db import IntegrityError +from django.db.models import Count, OuterRef, Subquery, F, Q from django.contrib.auth.decorators import login_required from django.core.paginator import Paginator from django.http import HttpResponseBadRequest, HttpResponseNotFound @@ -37,30 +38,41 @@ class Shelf(View): return HttpResponseNotFound() if not shelf.visible_to_user(request.user): return HttpResponseNotFound() + books = shelf.books # this is a constructed "all books" view, with a fake "shelf" obj else: FakeShelf = namedtuple( "Shelf", ("identifier", "name", "user", "books", "privacy") ) books = models.Edition.objects.filter( + # privacy is ensured because the shelves are already filtered above shelfbook__shelf__in=shelves.all() ).distinct() shelf = FakeShelf("all", _("All books"), user, books, "public") - is_self = request.user == user - if is_api_request(request): return ActivitypubResponse(shelf.to_activity(**request.GET)) + reviews = privacy_filter( + request.user, + models.Review.objects.filter( + user=user, + rating__isnull=False, + book__id=OuterRef("id"), + ), + ).order_by("-published_date") + + books = books.annotate(rating=Subquery(reviews.values("rating")[:1])) + paginated = Paginator( - shelf.books.order_by("-updated_date"), + books.order_by("-updated_date"), PAGE_LENGTH, ) page = paginated.get_page(request.GET.get("page")) data = { "user": user, - "is_self": is_self, + "is_self": request.user == user, "shelves": shelves.all(), "shelf": shelf, "books": page, diff --git a/locale/de_DE/LC_MESSAGES/django.po b/locale/de_DE/LC_MESSAGES/django.po index 7f274af9d..7b9a929da 100644 --- a/locale/de_DE/LC_MESSAGES/django.po +++ b/locale/de_DE/LC_MESSAGES/django.po @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: 0.0.1\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2021-05-10 13:23-0700\n" +"POT-Creation-Date: 2021-05-14 15:12-0700\n" "PO-Revision-Date: 2021-03-02 17:19-0800\n" "Last-Translator: Mouse Reeve