Merge pull request #805 from mouse-reeve/shared-books

Adds shared books as a metric for recommending follows

bookwyrm/static/css/format.css

@@ -163,9 +163,15 @@ html {
     vertical-align: middle;
     display: inline;
 }
-.navbar .avatar {
+.is-32x32 {
-    max-height: none;
+    min-width: 32px;
+    min-height: 32px;
 }
+.is-96x96 {
+    min-width: 96px;
+    min-height: 96px;
+}
+
 
 
 /* --- QUOTES --- */

bookwyrm/templates/directory.html

@@ -121,12 +121,21 @@
             </div>
             <footer class="card-footer content">
                 {% if user != request.user %}
+                {% if user.mutuals %}
                 <div class="card-footer-item">
                     <div class="has-text-centered">
                         <p class="title is-6 mb-0">{{ user.mutuals }}</p>
-                        <p class="help">{% trans "followers you follow" %}</p>
+                        <p class="help">{% blocktrans count counter=user.mutuals %}follower you follow{% plural %}followers you follow{% endblocktrans %}</p>
                     </div>
                 </div>
+                {% elif user.shared_books %}
+                <div class="card-footer-item">
+                    <div class="has-text-centered">
+                        <p class="title is-6 mb-0">{{ user.shared_books }}</p>
+                        <p class="help">{% blocktrans count counter=user.shared_books %}book on your shelves{% plural %}books on your shelves{% endblocktrans %}</p>
+                    </div>
+                </div>
+                {% endif %}
                 {% endif %}
                 <div class="card-footer-item">
                     <div class="has-text-centered">

bookwyrm/templates/feed/feed.html

@@ -54,25 +54,28 @@
 {# suggested users on the first page, two statuses down #}
 <section class="block">
     <h2 class="title is-5">{% trans "Who to follow" %}</h2>
-    <div class="columns is-mobile is-gapless scroll-x">
+    <div class="columns is-mobile scroll-x mb-0">
-    {% for user in suggested_users %}
+        {% for user in suggested_users %}
-    <div class="column is-flex">
+        <div class="column is-flex">
-        <div class="box has-text-centered is-shadowless has-background-white-bis">
+            <div class="box has-text-centered is-shadowless has-background-white-bis m-0">
-            <a href="{{ user.local_path }}" class="has-text-black">
+                <a href="{{ user.local_path }}" class="has-text-black">
-                {% include 'snippets/avatar.html' with user=user large=True %}
+                    {% include 'snippets/avatar.html' with user=user large=True %}
-                <span title="{{ user.display_name }}" class="is-block is-6 has-text-weight-bold">{{ user.display_name|truncatechars:10 }}</span>
+                    <span title="{{ user.display_name }}" class="is-block is-6 has-text-weight-bold">{{ user.display_name|truncatechars:10 }}</span>
-                <span title="@{{ user|username }}" class="is-block pb-3">@{{ user|username|truncatechars:8 }}</span>
+                    <span title="@{{ user|username }}" class="is-block pb-3">@{{ user|username|truncatechars:8 }}</span>
-            </a>
+                </a>
-            {% include 'snippets/follow_button.html' with user=user minimal=True %}
+                {% include 'snippets/follow_button.html' with user=user minimal=True %}
-            {% if user.mutuals %}
+                {% if user.mutuals %}
-            <p class="help">
+                <p class="help">
-            {% blocktrans with mutuals=user.mutuals|intcomma count counter=user.mutuals %}{{ mutuals }} follower you follow{% plural %}{{ mutuals }} followers you follow{% endblocktrans %}
+                {% blocktrans with mutuals=user.mutuals|intcomma count counter=user.mutuals %}{{ mutuals }} follower you follow{% plural %}{{ mutuals }} followers you follow{% endblocktrans %}
-            </p>
+                </p>
-            {% endif %}
+                {% elif user.shared_books %}
+                <p class="help">{% blocktrans with shared_books=user.shared_books|intcomma count counter=user.shared_books %}{{ shared_books }} book on your shelves{% plural %}{{ shared_books }} books on your shelves{% endblocktrans %}</p>
+                {% endif %}
+            </div>
         </div>
+        {% endfor %}
     </div>
-    {% endfor %}
+    <a class="help" href="{% url 'directory' %}">View directory <span class="icon icon-arrow-right"></a>
-    </div>
 </section>
 {% endif %}
 <div class="block">

bookwyrm/tests/views/test_helpers.py

@@ -10,6 +10,7 @@ from bookwyrm import models, views
 from bookwyrm.settings import USER_AGENT
 
 
+@patch("bookwyrm.activitystreams.ActivityStream.add_status")
 class ViewsHelpers(TestCase):
     """ viewing and creating statuses """
 
@@ -21,6 +22,7 @@ class ViewsHelpers(TestCase):
             "mouse@mouse.com",
             "mouseword",
             local=True,
+            discoverable=True,
             localname="mouse",
             remote_id="https://example.com/users/mouse",
         )
@@ -37,6 +39,7 @@ class ViewsHelpers(TestCase):
                 "ratword",
                 local=False,
                 remote_id="https://example.com/users/rat",
+                discoverable=True,
                 inbox="https://example.com/users/rat/inbox",
                 outbox="https://example.com/users/rat/outbox",
             )
@@ -48,12 +51,12 @@ class ViewsHelpers(TestCase):
                 name="Test Shelf", identifier="test-shelf", user=self.local_user
             )
 
-    def test_get_edition(self):
+    def test_get_edition(self, _):
         """ given an edition or a work, returns an edition """
         self.assertEqual(views.helpers.get_edition(self.book.id), self.book)
         self.assertEqual(views.helpers.get_edition(self.work.id), self.book)
 
-    def test_get_user_from_username(self):
+    def test_get_user_from_username(self, _):
         """ works for either localname or username """
         self.assertEqual(
             views.helpers.get_user_from_username(self.local_user, "mouse"),
@@ -66,7 +69,7 @@ class ViewsHelpers(TestCase):
         with self.assertRaises(models.User.DoesNotExist):
             views.helpers.get_user_from_username(self.local_user, "mojfse@example.com")
 
-    def test_is_api_request(self):
+    def test_is_api_request(self, _):
         """ should it return html or json """
         request = self.factory.get("/path")
         request.headers = {"Accept": "application/json"}
@@ -80,7 +83,7 @@ class ViewsHelpers(TestCase):
         request.headers = {"Accept": "Praise"}
         self.assertFalse(views.helpers.is_api_request(request))
 
-    def test_is_bookwyrm_request(self):
+    def test_is_bookwyrm_request(self, _):
         """ checks if a request came from a bookwyrm instance """
         request = self.factory.get("", {"q": "Test Book"})
         self.assertFalse(views.helpers.is_bookwyrm_request(request))
@@ -95,7 +98,7 @@ class ViewsHelpers(TestCase):
         request = self.factory.get("", {"q": "Test Book"}, HTTP_USER_AGENT=USER_AGENT)
         self.assertTrue(views.helpers.is_bookwyrm_request(request))
 
-    def test_existing_user(self):
+    def test_existing_user(self, _):
         """ simple database lookup by username """
         result = views.helpers.handle_remote_webfinger("@mouse@local.com")
         self.assertEqual(result, self.local_user)
@@ -104,7 +107,7 @@ class ViewsHelpers(TestCase):
         self.assertEqual(result, self.local_user)
 
     @responses.activate
-    def test_load_user(self):
+    def test_load_user(self, _):
         """ find a remote user using webfinger """
         username = "mouse@example.com"
         wellknown = {
@@ -134,7 +137,6 @@ class ViewsHelpers(TestCase):
             self.assertIsInstance(result, models.User)
             self.assertEqual(result.username, "mouse@example.com")
 
-    @patch("bookwyrm.activitystreams.ActivityStream.add_status")
     def test_handle_reading_status_to_read(self, _):
         """ posts shelve activities """
         shelf = self.local_user.shelf_set.get(identifier="to-read")
@@ -147,7 +149,6 @@ class ViewsHelpers(TestCase):
         self.assertEqual(status.mention_books.first(), self.book)
         self.assertEqual(status.content, "wants to read")
 
-    @patch("bookwyrm.activitystreams.ActivityStream.add_status")
     def test_handle_reading_status_reading(self, _):
         """ posts shelve activities """
         shelf = self.local_user.shelf_set.get(identifier="reading")
@@ -160,7 +161,6 @@ class ViewsHelpers(TestCase):
         self.assertEqual(status.mention_books.first(), self.book)
         self.assertEqual(status.content, "started reading")
 
-    @patch("bookwyrm.activitystreams.ActivityStream.add_status")
     def test_handle_reading_status_read(self, _):
         """ posts shelve activities """
         shelf = self.local_user.shelf_set.get(identifier="read")
@@ -173,7 +173,6 @@ class ViewsHelpers(TestCase):
         self.assertEqual(status.mention_books.first(), self.book)
         self.assertEqual(status.content, "finished reading")
 
-    @patch("bookwyrm.activitystreams.ActivityStream.add_status")
     def test_handle_reading_status_other(self, _):
         """ posts shelve activities """
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
@@ -182,7 +181,6 @@ class ViewsHelpers(TestCase):
             )
         self.assertFalse(models.GeneratedNote.objects.exists())
 
-    @patch("bookwyrm.activitystreams.ActivityStream.add_status")
     def test_object_visible_to_user(self, _):
         """ does a user have permission to view an object """
         obj = models.Status.objects.create(
@@ -211,7 +209,6 @@ class ViewsHelpers(TestCase):
         obj.mention_users.add(self.local_user)
         self.assertTrue(views.helpers.object_visible_to_user(self.local_user, obj))
 
-    @patch("bookwyrm.activitystreams.ActivityStream.add_status")
     def test_object_visible_to_user_follower(self, _):
         """ what you can see if you follow a user """
         self.remote_user.followers.add(self.local_user)
@@ -231,7 +228,6 @@ class ViewsHelpers(TestCase):
         obj.mention_users.add(self.local_user)
         self.assertTrue(views.helpers.object_visible_to_user(self.local_user, obj))
 
-    @patch("bookwyrm.activitystreams.ActivityStream.add_status")
     def test_object_visible_to_user_blocked(self, _):
         """ you can't see it if they block you """
         self.remote_user.blocks.add(self.local_user)
@@ -244,3 +240,50 @@ class ViewsHelpers(TestCase):
             name="test", user=self.remote_user, privacy="unlisted"
         )
         self.assertFalse(views.helpers.object_visible_to_user(self.local_user, obj))
+
+    def test_get_suggested_users(self, _):
+        """ list of people you might know """
+        user_1 = models.User.objects.create_user(
+            "nutria@local.com",
+            "nutria@nutria.com",
+            "nutriaword",
+            local=True,
+            localname="nutria",
+            discoverable=True,
+        )
+        user_2 = models.User.objects.create_user(
+            "fish@local.com",
+            "fish@fish.com",
+            "fishword",
+            local=True,
+            localname="fish",
+        )
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            # 1 shared follow
+            self.local_user.following.add(user_2)
+            user_1.following.add(user_2)
+
+            # 1 shared book
+            models.ShelfBook.objects.create(
+                user=self.local_user,
+                book=self.book,
+                shelf=self.local_user.shelf_set.first(),
+            )
+            models.ShelfBook.objects.create(
+                user=user_1, book=self.book, shelf=user_1.shelf_set.first()
+            )
+
+        result = views.helpers.get_suggested_users(self.local_user)
+        self.assertEqual(result.count(), 3)
+        self.assertTrue(user_1 in result)
+        self.assertFalse(user_2 in result)
+        self.assertTrue(self.local_user in result)
+        self.assertTrue(self.remote_user in result)
+
+        user_1_annotated = result.get(id=user_1.id)
+        self.assertEqual(user_1_annotated.mutuals, 1)
+        self.assertEqual(user_1_annotated.shared_books, 1)
+
+        remote_user_annotated = result.get(id=self.remote_user.id)
+        self.assertEqual(remote_user_annotated.mutuals, 0)
+        self.assertEqual(remote_user_annotated.shared_books, 0)

bookwyrm/views/directory.py

@@ -1,13 +1,12 @@
 """ who all's here? """
 from django.contrib.auth.decorators import login_required
 from django.core.paginator import Paginator
-from django.db.models import Count, Q
 from django.shortcuts import redirect
 from django.template.response import TemplateResponse
 from django.views import View
 from django.utils.decorators import method_decorator
 
-from bookwyrm import models
+from .helpers import get_suggested_users
 
 # pylint: disable=no-self-use
 @method_decorator(login_required, name="dispatch")
@@ -30,21 +29,12 @@ class Directory(View):
         if scope == "local":
             filters["local"] = True
 
-        users = models.User.objects.filter(
+        users = get_suggested_users(request.user, **filters)
-            discoverable=True, is_active=True, **filters
-        ).annotate(
-            mutuals=Count(
-                "following",
-                filter=Q(
-                    ~Q(id=request.user.id), following__in=request.user.following.all()
-                ),
-            )
-        )
         sort = request.GET.get("sort")
         if sort == "recent":
             users = users.order_by("-last_active_date")
         else:
-            users = users.order_by("-mutuals", "-last_active_date")
+            users = users.order_by("-mutuals", "-shared_books", "-last_active_date")
 
         paginated = Paginator(users, 12)
 

bookwyrm/views/feed.py

@@ -1,7 +1,7 @@
 """ non-interactive pages """
 from django.contrib.auth.decorators import login_required
 from django.core.paginator import Paginator
-from django.db.models import Count, Q
+from django.db.models import Q
 from django.http import HttpResponseNotFound
 from django.template.response import TemplateResponse
 from django.utils import timezone
@@ -11,7 +11,7 @@ from django.views import View
 from bookwyrm import activitystreams, forms, models
 from bookwyrm.activitypub import ActivitypubResponse
 from bookwyrm.settings import PAGE_LENGTH, STREAMS
-from .helpers import get_user_from_username, privacy_filter
+from .helpers import get_user_from_username, privacy_filter, get_suggested_users
 from .helpers import is_api_request, is_bookwyrm_request, object_visible_to_user
 
 
@@ -35,19 +35,12 @@ class Feed(View):
         paginated = Paginator(activities, PAGE_LENGTH)
 
         suggested_users = (
-            models.User.objects.filter(
+            get_suggested_users(
-                ~Q(id__in=request.user.following.all()),
+                request.user,
                 ~Q(id=request.user.id),
-                discoverable=True,
+                ~Q(followers=request.user),
-                is_active=True,
                 bookwyrm_user=True,
             )
-            .annotate(
-                mutuals=Count(
-                    "following",
-                    filter=Q(following__in=request.user.following.all()),
-                )
-            )
             .order_by("-mutuals", "-last_active_date")
             .all()[:5]
         )

bookwyrm/views/helpers.py

@@ -2,7 +2,7 @@
 import re
 from requests import HTTPError
 from django.core.exceptions import FieldError
-from django.db.models import Max, Q
+from django.db.models import Count, Max, Q
 
 from bookwyrm import activitypub, models
 from bookwyrm.connectors import ConnectorException, get_data
@@ -190,3 +190,30 @@ def get_discover_books():
             .order_by("-review__published_date__max")[:6]
         )
     )
+
+
+def get_suggested_users(user, *args, **kwargs):
+    """ Users, annotated with things they have in common """
+    return (
+        models.User.objects.filter(discoverable=True, is_active=True, *args, **kwargs)
+        .exclude(Q(id__in=user.blocks.all()) | Q(blocks=user))
+        .annotate(
+            mutuals=Count(
+                "following",
+                filter=Q(
+                    ~Q(id=user.id),
+                    ~Q(id__in=user.following.all()),
+                    following__in=user.following.all(),
+                ),
+            ),
+            shared_books=Count(
+                "shelfbook",
+                filter=Q(
+                    ~Q(id=user.id),
+                    shelfbook__book__parent_work__in=[
+                        s.book.parent_work for s in user.shelfbook_set.all()
+                    ],
+                ),
+            ),
+        )
+    )