From bcdf2ee1421bc3a31c108050d887fb65464bb62a Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 24 Feb 2021 11:35:19 -0800 Subject: [PATCH 1/5] Improves privacy-related display --- bookwyrm/templatetags/bookwyrm_tags.py | 7 ++----- bookwyrm/views/books.py | 5 ++--- bookwyrm/views/feed.py | 18 ++++++++++++------ bookwyrm/views/helpers.py | 15 ++++++++------- bookwyrm/views/list.py | 6 +++--- bookwyrm/views/search.py | 3 ++- 6 files changed, 29 insertions(+), 25 deletions(-) diff --git a/bookwyrm/templatetags/bookwyrm_tags.py b/bookwyrm/templatetags/bookwyrm_tags.py index d251d8e4..67354ac6 100644 --- a/bookwyrm/templatetags/bookwyrm_tags.py +++ b/bookwyrm/templatetags/bookwyrm_tags.py @@ -22,11 +22,8 @@ def dict_key(d, k): @register.filter(name='rating') def get_rating(book, user): ''' get the overall rating of a book ''' - queryset = views.helpers.get_activity_feed( - user, - ['public', 'followers', 'unlisted', 'direct'], - queryset=models.Review.objects.filter(book=book), - ) + queryset = views.helpers.privacy_filter( + user, models.Review.objects.filter(book=book)) return queryset.aggregate(Avg('rating'))['rating__avg'] diff --git a/bookwyrm/views/books.py b/bookwyrm/views/books.py index 9d196563..3d4b3faf 100644 --- a/bookwyrm/views/books.py +++ b/bookwyrm/views/books.py @@ -96,9 +96,8 @@ class Book(View): 'rating': reviews.aggregate(Avg('rating'))['rating__avg'], 'tags': models.UserTag.objects.filter(book=book), 'lists': privacy_filter( - request.user, - book.list_set.all(), - ['public', 'unlisted', 'followers']), + request.user, book.list_set.all() + ), 'user_tags': user_tags, 'user_shelves': user_shelves, 'other_edition_shelves': other_edition_shelves, diff --git a/bookwyrm/views/feed.py b/bookwyrm/views/feed.py index c229b7c9..d309df94 100644 --- a/bookwyrm/views/feed.py +++ b/bookwyrm/views/feed.py @@ -28,15 +28,21 @@ class Feed(View): page = 1 if tab == 'home': - activities = get_activity_feed( - request.user, ['public', 'unlisted', 'followers'], - following_only=True) + activities = get_activity_feed(request.user, following_only=True) + # we only want to show private messages if they're related to books + activities = activities.exclude( + review__isnull=True, + comment__isnull=True, + quotation__isnull=True, + generatednote__isnull=True, + privacy='direct' + ) elif tab == 'local': activities = get_activity_feed( - request.user, ['public', 'followers'], local_only=True) + request.user, privacy=['public', 'followers'], local_only=True) else: activities = get_activity_feed( - request.user, ['public', 'followers']) + request.user, privacy=['public', 'followers']) paginated = Paginator(activities, PAGE_LENGTH) data = {**feed_page_data(request.user), **{ @@ -72,7 +78,7 @@ class DirectMessage(View): queryset = queryset.filter(Q(user=user) | Q(mention_users=user)) activities = get_activity_feed( - request.user, 'direct', queryset=queryset) + request.user, privacy=['direct'], queryset=queryset) paginated = Paginator(activities, PAGE_LENGTH) activity_page = paginated.page(page) diff --git a/bookwyrm/views/helpers.py b/bookwyrm/views/helpers.py index fac3ab0c..1e96ec1c 100644 --- a/bookwyrm/views/helpers.py +++ b/bookwyrm/views/helpers.py @@ -59,8 +59,11 @@ def object_visible_to_user(viewer, obj): return False -def privacy_filter(viewer, queryset, privacy_levels, following_only=False): +def privacy_filter(viewer, queryset, privacy_levels=None, following_only=False): ''' filter objects that have "user" and "privacy" fields ''' + privacy_levels = privacy_levels or \ + ['public', 'unlisted', 'followers', 'direct'] + # exclude blocks from both directions if not viewer.is_anonymous: blocked = models.User.objects.filter(id__in=viewer.blocks.all()).all() @@ -104,18 +107,16 @@ def privacy_filter(viewer, queryset, privacy_levels, following_only=False): def get_activity_feed( - user, privacy, local_only=False, following_only=False, - queryset=models.Status.objects): + user, privacy=None, local_only=False, following_only=False, + queryset=None): ''' get a filtered queryset of statuses ''' - # if we're looking at Status, we need this. We don't if it's Comment - if hasattr(queryset, 'select_subclasses'): - queryset = queryset.select_subclasses() + if not queryset: + queryset = models.Status.objects.select_subclasses() # exclude deleted queryset = queryset.exclude(deleted=True).order_by('-published_date') # apply privacy filters - privacy = privacy if isinstance(privacy, list) else [privacy] queryset = privacy_filter( user, queryset, privacy, following_only=following_only) diff --git a/bookwyrm/views/list.py b/bookwyrm/views/list.py index 7286bfb4..e7b70a28 100644 --- a/bookwyrm/views/list.py +++ b/bookwyrm/views/list.py @@ -35,7 +35,8 @@ class Lists(View): ).filter( item_count__gt=0 ).distinct().all() - lists = privacy_filter(request.user, lists, ['public', 'followers']) + lists = privacy_filter( + request.user, lists, privacy_levels=['public', 'followers']) paginated = Paginator(lists, 12) data = { @@ -67,8 +68,7 @@ class UserLists(View): page = 1 user = get_user_from_username(request.user, username) lists = models.List.objects.filter(user=user).all() - lists = privacy_filter( - request.user, lists, ['public', 'followers', 'unlisted']) + lists = privacy_filter(request.user, lists) paginated = Paginator(lists, 12) data = { diff --git a/bookwyrm/views/search.py b/bookwyrm/views/search.py index 98be166f..8acb2836 100644 --- a/bookwyrm/views/search.py +++ b/bookwyrm/views/search.py @@ -44,7 +44,8 @@ class Search(View): # any relevent lists? list_results = privacy_filter( - request.user, models.List.objects, ['public', 'followers'] + request.user, models.List.objects, + privacy_levels=['public', 'followers'] ).annotate( similarity=Greatest( TrigramSimilarity('name', query), From 2a5d4b83d8f020bee7cf37077fdafe57dfe9bd54 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 24 Feb 2021 11:59:21 -0800 Subject: [PATCH 2/5] Show dms in the right places --- bookwyrm/templates/discover/large-book.html | 2 +- bookwyrm/templates/discover/small-book.html | 4 +-- bookwyrm/tests/views/test_helpers.py | 16 ++++++------ bookwyrm/views/books.py | 4 +-- bookwyrm/views/feed.py | 14 +++------- bookwyrm/views/helpers.py | 29 ++++++++++++++++----- bookwyrm/views/landing.py | 13 ++------- bookwyrm/views/rss_feed.py | 2 +- bookwyrm/views/user.py | 4 +-- 9 files changed, 42 insertions(+), 46 deletions(-) diff --git a/bookwyrm/templates/discover/large-book.html b/bookwyrm/templates/discover/large-book.html index 401411a1..7881a33a 100644 --- a/bookwyrm/templates/discover/large-book.html +++ b/bookwyrm/templates/discover/large-book.html @@ -3,7 +3,7 @@
{% include 'snippets/book_cover.html' with book=book size="large" %} - {% include 'snippets/stars.html' with rating=ratings|dict_key:book.id %} + {% include 'snippets/stars.html' with rating=book|rating:request.user %}

{{ book.title }}

diff --git a/bookwyrm/templates/discover/small-book.html b/bookwyrm/templates/discover/small-book.html index d1676b6b..72108c30 100644 --- a/bookwyrm/templates/discover/small-book.html +++ b/bookwyrm/templates/discover/small-book.html @@ -1,9 +1,7 @@ {% load bookwyrm_tags %} {% if book %} {% include 'snippets/book_cover.html' with book=book %} -{% if ratings %} -{% include 'snippets/stars.html' with rating=ratings|dict_key:book.id %} -{% endif %} +{% include 'snippets/stars.html' with rating=book|rating:request.user %}

{{ book.title }}

{% if book.authors %} diff --git a/bookwyrm/tests/views/test_helpers.py b/bookwyrm/tests/views/test_helpers.py index 10221933..c61505c2 100644 --- a/bookwyrm/tests/views/test_helpers.py +++ b/bookwyrm/tests/views/test_helpers.py @@ -106,7 +106,7 @@ class ViewsHelpers(TestCase): statuses = views.helpers.get_activity_feed( self.local_user, - ['public', 'unlisted', 'followers'], + privacy=['public', 'unlisted', 'followers'], following_only=True, queryset=models.Comment.objects ) @@ -115,7 +115,7 @@ class ViewsHelpers(TestCase): statuses = views.helpers.get_activity_feed( self.local_user, - ['public', 'followers'], + privacy=['public', 'followers'], local_only=True ) self.assertEqual(len(statuses), 2) @@ -128,7 +128,7 @@ class ViewsHelpers(TestCase): statuses = views.helpers.get_activity_feed( self.local_user, - ['public', 'followers'], + privacy=['public', 'followers'], ) self.assertEqual(len(statuses), 3) self.assertEqual(statuses[2], public_status) @@ -137,7 +137,7 @@ class ViewsHelpers(TestCase): statuses = views.helpers.get_activity_feed( self.local_user, - ['public', 'unlisted', 'followers'], + privacy=['public', 'unlisted', 'followers'], following_only=True ) self.assertEqual(len(statuses), 2) @@ -147,7 +147,7 @@ class ViewsHelpers(TestCase): rat.followers.add(self.local_user) statuses = views.helpers.get_activity_feed( self.local_user, - ['public', 'unlisted', 'followers'], + privacy=['public', 'unlisted', 'followers'], following_only=True ) self.assertEqual(len(statuses), 5) @@ -170,18 +170,18 @@ class ViewsHelpers(TestCase): content='blah blah', user=rat) statuses = views.helpers.get_activity_feed( - self.local_user, ['public']) + self.local_user, privacy=['public']) self.assertEqual(len(statuses), 2) # block relationship rat.blocks.add(self.local_user) statuses = views.helpers.get_activity_feed( - self.local_user, ['public']) + self.local_user, privacy=['public']) self.assertEqual(len(statuses), 1) self.assertEqual(statuses[0], public_status) statuses = views.helpers.get_activity_feed( - rat, ['public']) + rat, privacy=['public']) self.assertEqual(len(statuses), 1) self.assertEqual(statuses[0], rat_public) diff --git a/bookwyrm/views/books.py b/bookwyrm/views/books.py index 3d4b3faf..d59eb4aa 100644 --- a/bookwyrm/views/books.py +++ b/bookwyrm/views/books.py @@ -50,9 +50,7 @@ class Book(View): ) # all reviews for the book reviews = get_activity_feed( - request.user, - ['public', 'unlisted', 'followers', 'direct'], - queryset=reviews + request.user, queryset=reviews ) # the reviews to show diff --git a/bookwyrm/views/feed.py b/bookwyrm/views/feed.py index d309df94..a320162d 100644 --- a/bookwyrm/views/feed.py +++ b/bookwyrm/views/feed.py @@ -11,8 +11,7 @@ from django.views import View from bookwyrm import forms, models from bookwyrm.activitypub import ActivitypubResponse from bookwyrm.settings import PAGE_LENGTH -from .helpers import get_activity_feed -from .helpers import get_user_from_username +from .helpers import get_activity_feed, get_user_from_username from .helpers import is_api_request, is_bookwyrm_request, object_visible_to_user @@ -28,15 +27,8 @@ class Feed(View): page = 1 if tab == 'home': - activities = get_activity_feed(request.user, following_only=True) - # we only want to show private messages if they're related to books - activities = activities.exclude( - review__isnull=True, - comment__isnull=True, - quotation__isnull=True, - generatednote__isnull=True, - privacy='direct' - ) + activities = get_activity_feed( + request.user, following_only=True, hide_dms=True) elif tab == 'local': activities = get_activity_feed( request.user, privacy=['public', 'followers'], local_only=True) diff --git a/bookwyrm/views/helpers.py b/bookwyrm/views/helpers.py index 1e96ec1c..78f210bf 100644 --- a/bookwyrm/views/helpers.py +++ b/bookwyrm/views/helpers.py @@ -1,6 +1,7 @@ ''' helper functions used in various views ''' import re from requests import HTTPError +from django.core.exceptions import FieldError from django.db.models import Q from bookwyrm import activitypub, models @@ -98,17 +99,23 @@ def privacy_filter(viewer, queryset, privacy_levels=None, following_only=False): # exclude direct messages not intended for the user if 'direct' in privacy_levels: - queryset = queryset.exclude( - ~Q( - Q(user=viewer) | Q(mention_users=viewer) - ), privacy='direct' - ) + try: + queryset = queryset.exclude( + ~Q( + Q(user=viewer) | Q(mention_users=viewer) + ), privacy='direct' + ) + except FieldError: + queryset = queryset.exclude( + ~Q(user=viewer), privacy='direct' + ) + return queryset def get_activity_feed( user, privacy=None, local_only=False, following_only=False, - queryset=None): + queryset=None, hide_dms=False): ''' get a filtered queryset of statuses ''' if not queryset: queryset = models.Status.objects.select_subclasses() @@ -120,6 +127,16 @@ def get_activity_feed( queryset = privacy_filter( user, queryset, privacy, following_only=following_only) + if hide_dms: + # dms are direct statuses not related to books + queryset = queryset.exclude( + review__isnull=True, + comment__isnull=True, + quotation__isnull=True, + generatednote__isnull=True, + privacy='direct' + ) + # filter for only local status if local_only: queryset = queryset.filter(user__local=True) diff --git a/bookwyrm/views/landing.py b/bookwyrm/views/landing.py index 3be3eb31..2774e742 100644 --- a/bookwyrm/views/landing.py +++ b/bookwyrm/views/landing.py @@ -1,11 +1,10 @@ ''' non-interactive pages ''' -from django.db.models import Avg, Max +from django.db.models import Max from django.template.response import TemplateResponse from django.views import View from bookwyrm import forms, models from .feed import Feed -from .helpers import get_activity_feed # pylint: disable= no-self-use @@ -34,6 +33,7 @@ class Discover(View): ''' tiled book activity page ''' books = models.Edition.objects.filter( review__published_date__isnull=False, + review__deleted=False, review__user__local=True, review__privacy__in=['public', 'unlisted'], ).exclude( @@ -42,18 +42,9 @@ class Discover(View): Max('review__published_date') ).order_by('-review__published_date__max')[:6] - ratings = {} - for book in books: - reviews = models.Review.objects.filter( - book__in=book.parent_work.editions.all() - ) - reviews = get_activity_feed( - request.user, ['public', 'unlisted'], queryset=reviews) - ratings[book.id] = reviews.aggregate(Avg('rating'))['rating__avg'] data = { 'title': 'Discover', 'register_form': forms.RegisterForm(), 'books': list(set(books)), - 'ratings': ratings } return TemplateResponse(request, 'discover/discover.html', data) diff --git a/bookwyrm/views/rss_feed.py b/bookwyrm/views/rss_feed.py index aad227bf..c211f991 100644 --- a/bookwyrm/views/rss_feed.py +++ b/bookwyrm/views/rss_feed.py @@ -27,7 +27,7 @@ class RssFeed(Feed): def items(self, obj): ''' the user's activity feed ''' return get_activity_feed( - obj, ['public', 'unlisted'], queryset=obj.status_set) + obj, privacy=['public', 'unlisted'], queryset=obj.status_set) def item_link(self, item): diff --git a/bookwyrm/views/user.py b/bookwyrm/views/user.py index 7a238ce7..01002ce5 100644 --- a/bookwyrm/views/user.py +++ b/bookwyrm/views/user.py @@ -71,8 +71,8 @@ class User(View): # user's posts activities = get_activity_feed( request.user, - ['public', 'unlisted', 'followers'], - queryset=user.status_set + queryset=user.status_set.select_subclasses(), + hide_dms=True ) paginated = Paginator(activities, PAGE_LENGTH) goal = models.AnnualGoal.objects.filter( From 779581c6f480d13612fe998fbeeb9536a6235abd Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 24 Feb 2021 12:06:00 -0800 Subject: [PATCH 3/5] Only show dms in dm page --- bookwyrm/views/feed.py | 2 +- bookwyrm/views/helpers.py | 12 ++++++++++-- bookwyrm/views/user.py | 1 - 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/bookwyrm/views/feed.py b/bookwyrm/views/feed.py index a320162d..3a2805b4 100644 --- a/bookwyrm/views/feed.py +++ b/bookwyrm/views/feed.py @@ -28,7 +28,7 @@ class Feed(View): if tab == 'home': activities = get_activity_feed( - request.user, following_only=True, hide_dms=True) + request.user, following_only=True) elif tab == 'local': activities = get_activity_feed( request.user, privacy=['public', 'followers'], local_only=True) diff --git a/bookwyrm/views/helpers.py b/bookwyrm/views/helpers.py index 78f210bf..10b6ed92 100644 --- a/bookwyrm/views/helpers.py +++ b/bookwyrm/views/helpers.py @@ -115,7 +115,7 @@ def privacy_filter(viewer, queryset, privacy_levels=None, following_only=False): def get_activity_feed( user, privacy=None, local_only=False, following_only=False, - queryset=None, hide_dms=False): + queryset=None): ''' get a filtered queryset of statuses ''' if not queryset: queryset = models.Status.objects.select_subclasses() @@ -127,8 +127,16 @@ def get_activity_feed( queryset = privacy_filter( user, queryset, privacy, following_only=following_only) - if hide_dms: + # only show dms if we only want dms + if privacy == ['direct']: # dms are direct statuses not related to books + queryset = queryset.filter( + review__isnull=True, + comment__isnull=True, + quotation__isnull=True, + generatednote__isnull=True, + ) + else: queryset = queryset.exclude( review__isnull=True, comment__isnull=True, diff --git a/bookwyrm/views/user.py b/bookwyrm/views/user.py index 01002ce5..a218375f 100644 --- a/bookwyrm/views/user.py +++ b/bookwyrm/views/user.py @@ -72,7 +72,6 @@ class User(View): activities = get_activity_feed( request.user, queryset=user.status_set.select_subclasses(), - hide_dms=True ) paginated = Paginator(activities, PAGE_LENGTH) goal = models.AnnualGoal.objects.filter( From a5ee535ab64305cf73529d892a513ccff6c70f04 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 24 Feb 2021 12:24:19 -0800 Subject: [PATCH 4/5] fixes book reviews queryset aggregators --- bookwyrm/tests/views/test_helpers.py | 3 ++- bookwyrm/views/books.py | 8 ++------ bookwyrm/views/helpers.py | 20 ++++++++++++-------- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/bookwyrm/tests/views/test_helpers.py b/bookwyrm/tests/views/test_helpers.py index c61505c2..eff08307 100644 --- a/bookwyrm/tests/views/test_helpers.py +++ b/bookwyrm/tests/views/test_helpers.py @@ -122,7 +122,8 @@ class ViewsHelpers(TestCase): self.assertEqual(statuses[1], public_status) self.assertEqual(statuses[0], rat_public) - statuses = views.helpers.get_activity_feed(self.local_user, 'direct') + statuses = views.helpers.get_activity_feed( + self.local_user, privacy=['direct']) self.assertEqual(len(statuses), 1) self.assertEqual(statuses[0], direct_status) diff --git a/bookwyrm/views/books.py b/bookwyrm/views/books.py index d59eb4aa..e05c4726 100644 --- a/bookwyrm/views/books.py +++ b/bookwyrm/views/books.py @@ -45,13 +45,9 @@ class Book(View): if not work: return HttpResponseNotFound() - reviews = models.Review.objects.filter( - book__in=work.editions.all(), - ) # all reviews for the book - reviews = get_activity_feed( - request.user, queryset=reviews - ) + reviews = models.Review.objects.filter(book__in=work.editions.all()) + reviews = get_activity_feed(request.user, queryset=reviews) # the reviews to show paginated = Paginator(reviews.exclude( diff --git a/bookwyrm/views/helpers.py b/bookwyrm/views/helpers.py index 10b6ed92..cecb65e3 100644 --- a/bookwyrm/views/helpers.py +++ b/bookwyrm/views/helpers.py @@ -117,7 +117,7 @@ def get_activity_feed( user, privacy=None, local_only=False, following_only=False, queryset=None): ''' get a filtered queryset of statuses ''' - if not queryset: + if queryset is None: queryset = models.Status.objects.select_subclasses() # exclude deleted @@ -137,13 +137,17 @@ def get_activity_feed( generatednote__isnull=True, ) else: - queryset = queryset.exclude( - review__isnull=True, - comment__isnull=True, - quotation__isnull=True, - generatednote__isnull=True, - privacy='direct' - ) + try: + queryset = queryset.exclude( + review__isnull=True, + comment__isnull=True, + quotation__isnull=True, + generatednote__isnull=True, + privacy='direct' + ) + except FieldError: + # if we're looking at a subtype of Status (like Review) + pass # filter for only local status if local_only: From c7d85486142ff9e89551e8fe0895cc30669fb111 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 24 Feb 2021 12:35:43 -0800 Subject: [PATCH 5/5] Select subclasses in rss feed --- bookwyrm/views/rss_feed.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bookwyrm/views/rss_feed.py b/bookwyrm/views/rss_feed.py index c211f991..d24b636e 100644 --- a/bookwyrm/views/rss_feed.py +++ b/bookwyrm/views/rss_feed.py @@ -27,7 +27,10 @@ class RssFeed(Feed): def items(self, obj): ''' the user's activity feed ''' return get_activity_feed( - obj, privacy=['public', 'unlisted'], queryset=obj.status_set) + obj, + privacy=['public', 'unlisted'], + queryset=obj.status_set.select_subclasses() + ) def item_link(self, item):