Merge pull request #1021 from bookwyrm-social/following-display

Cleans up user/followers/following pages

bookwyrm/templates/goal.html

@@ -1,4 +1,4 @@
-{% extends 'user/user_layout.html' %}
+{% extends 'user/layout.html' %}
 {% load i18n %}
 
 {% block header %}

bookwyrm/templates/user/followers.html

@@ -1,34 +0,0 @@
-{% extends 'user/user_layout.html' %}
-{% load i18n %}
-{% load bookwyrm_tags %}
-
-{% block header %}
-<h1 class="title">
-    {% trans "User Profile" %}
-</h1>
-{% endblock %}
-
-{% block panel %}
-<div class="block">
-    <h2 class="title">{% trans "Followers" %}</h2>
-    {% for follower in followers %}
-    <div class="block columns">
-        <div class="column">
-            <a href="{{ follower.local_path }}">
-                {% include 'snippets/avatar.html' with user=follower %}
-                {{ follower.display_name }}
-            </a>
-            ({{ follower.username }})
-        </div>
-        <div class="column is-narrow">
-            {% include 'snippets/follow_button.html' with user=follower %}
-        </div>
-    </div>
-    {% endfor %}
-    {% if not followers.count %}
-    <div>{% blocktrans with username=user.display_name %}{{ username }} has no followers{% endblocktrans %}</div>
-    {% endif %}
-</div>
-
-{% include 'snippets/pagination.html' with page=followers path=request.path %}
-{% endblock %}

bookwyrm/templates/user/following.html

@@ -1,34 +0,0 @@
-{% extends 'user/user_layout.html' %}
-{% load i18n %}
-{% load bookwyrm_tags %}
-
-{% block header %}
-<h1 class="title">
-    {% trans "User Profile" %}
-</h1>
-{% endblock %}
-
-{% block panel %}
-<div class="block">
-    <h2 class="title">{% trans "Following" %}</h2>
-    {% for follower in user.following.all %}
-    <div class="block columns">
-        <div class="column">
-            <a href="{{ follower.local_path }}">
-                {% include 'snippets/avatar.html' with user=follower %}
-                {{ follower.display_name }}
-            </a>
-            ({{ follower.username }})
-        </div>
-        <div class="column">
-            {% include 'snippets/follow_button.html' with user=follower %}
-        </div>
-    </div>
-    {% endfor %}
-    {% if not following.count %}
-    <div>{% blocktrans with username=user|username %}{{ username }} isn't following any users{% endblocktrans %}</div>
-    {% endif %}
-</div>
-
-{% include 'snippets/pagination.html' with page=following path=request.path %}
-{% endblock %}

bookwyrm/templates/user/layout.html

@@ -7,7 +7,11 @@
 
 {% block content %}
 <header class="block">
-    {% block header %}{% endblock %}
+    {% block header %}
+    <h1 class="title">
+        {% trans "User Profile" %}
+    </h1>
+    {% endblock %}
 </header>
 
 {# user bio #}
@@ -41,8 +45,9 @@
     </div>
     {% endif %}
 </div>
+
+{% block tabs %}
 {% with user|username as username %}
-{% if 'user/'|add:username|add:'/books' not in request.path and 'user/'|add:username|add:'/shelf' not in request.path %}
 <nav class="tabs">
     <ul>
         {% url 'user-feed' user|username as url %}
@@ -70,8 +75,8 @@
         {% endif %}
     </ul>
 </nav>
-{% endif %}
 {% endwith %}
+{% endblock %}
 
 {% block panel %}{% endblock %}
 

bookwyrm/templates/user/lists.html

@@ -1,4 +1,4 @@
-{% extends 'user/user_layout.html' %}
+{% extends 'user/layout.html' %}
 {% load i18n %}
 
 {% block header %}

bookwyrm/templates/user/relationships/followers.html

@@ -0,0 +1,14 @@
+{% extends 'user/relationships/layout.html' %}
+{% load i18n %}
+
+{% block header %}
+<h1 class="title">
+    {% trans "Followers" %}
+</h1>
+{% endblock %}
+
+{% block nullstate %}
+<div>
+    {% blocktrans with username=user.display_name %}{{ username }} has no followers{% endblocktrans %}
+</div>
+{% endblock %}

bookwyrm/templates/user/relationships/following.html

@@ -0,0 +1,14 @@
+{% extends 'user/relationships/layout.html' %}
+{% load i18n %}
+
+{% block header %}
+<h1 class="title">
+    {% trans "Following" %}
+</h1>
+{% endblock %}
+
+{% block nullstate %}
+<div>
+    {% blocktrans with username=user.display_name %}{{ username }} isn't following any users{% endblocktrans %}
+</div>
+{% endblock %}

bookwyrm/templates/user/relationships/layout.html

@@ -0,0 +1,46 @@
+{% extends 'user/layout.html' %}
+{% load i18n %}
+{% load bookwyrm_tags %}
+
+{% block tabs %}
+{% with user|username as username %}
+<nav class="tabs">
+    <ul>
+        {% url 'user-followers' user|username as url %}
+        <li{% if url == request.path or url == request.path|add:'/' %} class="is-active"{% endif %}>
+            <a href="{{ url }}">{% trans "Followers" %}</a>
+        </li>
+        {% url 'user-following' user|username as url %}
+        <li{% if url == request.path or url == request.path|add:'/' %} class="is-active"{% endif %}>
+            <a href="{{ url }}">{% trans "Following" %}</a>
+        </li>
+    </ul>
+</nav>
+{% endwith %}
+{% endblock %}
+
+{% block panel %}
+<div class="block">
+    {% for follow in follow_list %}
+    <div class="block columns">
+        <div class="column">
+            <a href="{{ follower.local_path }}">
+                {% include 'snippets/avatar.html' with user=follow %}
+                {{ follow.display_name }}
+            </a>
+            ({{ follow.username }})
+        </div>
+        <div class="column is-narrow">
+            {% include 'snippets/follow_button.html' with user=follow %}
+        </div>
+    </div>
+    {% endfor %}
+
+    {% if not follow_list %}
+        {% block nullstate %}
+        {% endblock %}
+    {% endif %}
+</div>
+
+{% include 'snippets/pagination.html' with page=follow_list path=request.path %}
+{% endblock %}

bookwyrm/templates/user/shelf/shelf.html

@@ -1,21 +1,21 @@
-{% extends 'user/user_layout.html' %}
+{% extends 'user/layout.html' %}
 {% load bookwyrm_tags %}
 {% load humanize %}
 {% load i18n %}
 
 {% block title %}
-{% include 'user/books_header.html' %}
+{% include 'user/shelf/books_header.html' %}
 {% endblock %}
 
 {% block header %}
 <header class="columns">
     <h1 class="title">
-        {% include 'user/books_header.html' %}
+        {% include 'user/shelf/books_header.html' %}
     </h1>
 </header>
 {% endblock %}
 
-{% block panel %}
+{% block tabs %}
 <div class="block columns">
     <div class="column">
         <div class="tabs">
@@ -39,9 +39,11 @@
     </div>
     {% endif %}
 </div>
+{% endblock %}
 
+{% block panel %}
 <div class="block">
-    {% include 'user/create_shelf_form.html' with controls_text='create-shelf-form' %}
+    {% include 'user/shelf/create_shelf_form.html' with controls_text='create-shelf-form' %}
 </div>
 
 <div class="block columns is-mobile">
@@ -62,7 +64,7 @@
 </div>
 
 <div class="block">
-    {% include 'user/edit_shelf_form.html' with controls_text="edit-shelf-form" %}
+    {% include 'user/shelf/edit_shelf_form.html' with controls_text="edit-shelf-form" %}
 </div>
 
 <div class="block">

bookwyrm/templates/user/user.html

@@ -1,4 +1,4 @@
-{% extends 'user/user_layout.html' %}
+{% extends 'user/layout.html' %}
 {% load i18n %}
 {% load bookwyrm_tags %}
 
@@ -11,7 +11,7 @@
     </div>
     {% if is_self %}
     <div class="column is-narrow">
-        <a href="/preferences/profile">
+        <a href="{% url 'prefs-profile' %}">
             <span class="icon icon-pencil" title="Edit profile">
                 <span class="is-sr-only">{% trans "Edit profile" %}</span>
             </span>
@@ -25,7 +25,7 @@
 {% if user.bookwyrm_user %}
 <div class="block">
     <h2 class="title">
-        {% include 'user/books_header.html' %}
+        {% include 'user/shelf/books_header.html' %}
     </h2>
     <div class="columns">
         {% for shelf in shelves %}

bookwyrm/templates/user/user_preview.html

@@ -1,5 +1,6 @@
 {% load i18n %}
 {% load humanize %}
+{% load bookwyrm_tags %}
 
 <div class="media block">
     <div class="media-left">
@@ -12,8 +13,19 @@
         <p><a href="{{ user.remote_id }}">{{ user.username }}</a></p>
         <p>{% blocktrans with date=user.created_date|naturaltime %}Joined {{ date }}{% endblocktrans %}</p>
         <p>
-        <a href="{{ user.local_path }}/followers">{% blocktrans count counter=user.followers.count %}{{ counter }} follower{% plural %}{{ counter }} followers{% endblocktrans %}</a>,
+            {% if is_self %}
-        <a href="{{ user.local_path }}/following">{% blocktrans with counter=user.following.count %}{{ counter }} following{% endblocktrans %}</a>
+
+            <a href="{% url 'user-followers' user|username %}">{% blocktrans count counter=user.followers.count %}{{ counter }} follower{% plural %}{{ counter }} followers{% endblocktrans %}</a>,
+            <a href="{% url 'user-following' user|username %}">{% blocktrans with counter=user.following.count %}{{ counter }} following{% endblocktrans %}</a>
+
+            {% elif request.user.is_authenticated %}
+
+            {% mutuals_count user as mutuals %}
+            <a href="{% url 'user-followers' user|username %}">
+                {% blocktrans with mutuals_display=mutuals|intcomma count counter=mutuals %}{{ mutuals_display }} follower you follow{% plural %}{{ mutuals_display }} followers you follow{% endblocktrans %}
+            </a>
+
+            {% endif %}
         </p>
     </div>
 </div>

bookwyrm/templatetags/bookwyrm_tags.py

@@ -235,3 +235,12 @@ def get_lang():
     """get current language, strip to the first two letters"""
     language = utils.translation.get_language()
     return language[0 : language.find("-")]
+
+
+@register.simple_tag(takes_context=True)
+def mutuals_count(context, user):
+    """how many users that you follow, follow them"""
+    viewer = context["request"].user
+    if not viewer.is_authenticated:
+        return None
+    return user.followers.filter(id__in=viewer.following.all()).count()

bookwyrm/tests/views/test_helpers.py

@@ -3,6 +3,7 @@ import json
 from unittest.mock import patch
 import pathlib
 from django.db.models import Q
+from django.http import Http404
 from django.test import TestCase
 from django.test.client import RequestFactory
 import responses
@@ -67,7 +68,7 @@ class ViewsHelpers(TestCase):
             views.helpers.get_user_from_username(self.local_user, "mouse@local.com"),
             self.local_user,
         )
-        with self.assertRaises(models.User.DoesNotExist):
+        with self.assertRaises(Http404):
             views.helpers.get_user_from_username(self.local_user, "mojfse@example.com")
 
     def test_is_api_request(self, _):

bookwyrm/tests/views/test_user.py

@@ -6,6 +6,7 @@ from PIL import Image
 from django.contrib.auth.models import AnonymousUser
 from django.core.files.base import ContentFile
 from django.core.files.uploadedfile import SimpleUploadedFile
+from django.http.response import Http404
 from django.template.response import TemplateResponse
 from django.test import TestCase
 from django.test.client import RequestFactory
@@ -76,8 +77,8 @@ class UserViews(TestCase):
         self.rat.blocks.add(self.local_user)
         with patch("bookwyrm.views.user.is_api_request") as is_api:
             is_api.return_value = False
-            result = view(request, "rat")
+            with self.assertRaises(Http404):
-        self.assertEqual(result.status_code, 404)
+                view(request, "rat")
 
     def test_followers_page(self):
         """there are so many views, this just makes sure it LOADS"""
@@ -105,8 +106,8 @@ class UserViews(TestCase):
         self.rat.blocks.add(self.local_user)
         with patch("bookwyrm.views.user.is_api_request") as is_api:
             is_api.return_value = False
-            result = view(request, "rat")
+            with self.assertRaises(Http404):
-        self.assertEqual(result.status_code, 404)
+                view(request, "rat")
 
     def test_following_page(self):
         """there are so many views, this just makes sure it LOADS"""
@@ -134,8 +135,8 @@ class UserViews(TestCase):
         self.rat.blocks.add(self.local_user)
         with patch("bookwyrm.views.user.is_api_request") as is_api:
             is_api.return_value = False
-            result = view(request, "rat")
+            with self.assertRaises(Http404):
-        self.assertEqual(result.status_code, 404)
+                view(request, "rat")
 
     def test_edit_user_page(self):
         """there are so many views, this just makes sure it LOADS"""
@@ -166,7 +167,6 @@ class UserViews(TestCase):
         self.assertEqual(self.local_user.name, "New Name")
         self.assertEqual(self.local_user.email, "wow@email.com")
 
-    # idk how to mock the upload form, got tired of triyng to make it work
     def test_edit_user_avatar(self):
         """use a form to update a user"""
         view = views.EditUser.as_view()

bookwyrm/views/feed.py

@@ -2,7 +2,7 @@
 from django.contrib.auth.decorators import login_required
 from django.core.paginator import Paginator
 from django.db.models import Q
-from django.http import HttpResponseNotFound
+from django.http import HttpResponseNotFound, Http404
 from django.template.response import TemplateResponse
 from django.utils import timezone
 from django.utils.decorators import method_decorator
@@ -62,7 +62,7 @@ class DirectMessage(View):
         if username:
             try:
                 user = get_user_from_username(request.user, username)
-            except models.User.DoesNotExist:
+            except Http404:
                 pass
         if user:
             queryset = queryset.filter(Q(user=user) | Q(mention_users=user))
@@ -94,7 +94,7 @@ class Status(View):
             status = models.Status.objects.select_subclasses().get(
                 id=status_id, deleted=False
             )
-        except (ValueError, models.Status.DoesNotExist, models.User.DoesNotExist):
+        except (ValueError, models.Status.DoesNotExist):
             return HttpResponseNotFound()
 
         # the url should have the poster's username in it

bookwyrm/views/follow.py

@@ -14,10 +14,7 @@ from .helpers import get_user_from_username
 def follow(request):
     """follow another user, here or abroad"""
     username = request.POST["user"]
-    try:
     to_follow = get_user_from_username(request.user, username)
-    except models.User.DoesNotExist:
-        return HttpResponseBadRequest()
 
     try:
         models.UserFollowRequest.objects.create(
@@ -35,10 +32,7 @@ def follow(request):
 def unfollow(request):
     """unfollow a user"""
     username = request.POST["user"]
-    try:
     to_unfollow = get_user_from_username(request.user, username)
-    except models.User.DoesNotExist:
-        return HttpResponseBadRequest()
 
     try:
         models.UserFollows.objects.get(
@@ -63,10 +57,7 @@ def unfollow(request):
 def accept_follow_request(request):
     """a user accepts a follow request"""
     username = request.POST["user"]
-    try:
     requester = get_user_from_username(request.user, username)
-    except models.User.DoesNotExist:
-        return HttpResponseBadRequest()
 
     try:
         follow_request = models.UserFollowRequest.objects.get(
@@ -85,10 +76,7 @@ def accept_follow_request(request):
 def delete_follow_request(request):
     """a user rejects a follow request"""
     username = request.POST["user"]
-    try:
     requester = get_user_from_username(request.user, username)
-    except models.User.DoesNotExist:
-        return HttpResponseBadRequest()
 
     try:
         follow_request = models.UserFollowRequest.objects.get(

bookwyrm/views/helpers.py

@@ -3,6 +3,7 @@ import re
 from requests import HTTPError
 from django.core.exceptions import FieldError
 from django.db.models import Count, Max, Q
+from django.http import Http404
 
 from bookwyrm import activitypub, models
 from bookwyrm.connectors import ConnectorException, get_data
@@ -12,11 +13,17 @@ from bookwyrm.utils import regex
 
 def get_user_from_username(viewer, username):
     """helper function to resolve a localname or a username to a user"""
-    # raises DoesNotExist if user is now found
+    # raises 404 if the user isn't found
     try:
         return models.User.viewer_aware_objects(viewer).get(localname=username)
     except models.User.DoesNotExist:
+        pass
+
+    # if the localname didn't match, try the username
+    try:
         return models.User.viewer_aware_objects(viewer).get(username=username)
+    except models.User.DoesNotExist:
+        raise Http404()
 
 
 def is_api_request(request):

bookwyrm/views/shelf.py

@@ -25,10 +25,7 @@ class Shelf(View):
 
     def get(self, request, username, shelf_identifier=None):
         """display a shelf"""
-        try:
         user = get_user_from_username(request.user, username)
-        except models.User.DoesNotExist:
-            return HttpResponseNotFound()
 
         shelves = privacy_filter(request.user, user.shelf_set)
 
@@ -68,7 +65,7 @@ class Shelf(View):
             "books": paginated.get_page(request.GET.get("page")),
         }
 
-        return TemplateResponse(request, "user/shelf.html", data)
+        return TemplateResponse(request, "user/shelf/shelf.html", data)
 
     @method_decorator(login_required, name="dispatch")
     # pylint: disable=unused-argument

bookwyrm/views/user.py

@@ -6,7 +6,6 @@ from PIL import Image
 from django.contrib.auth.decorators import login_required
 from django.core.files.base import ContentFile
 from django.core.paginator import Paginator
-from django.http import HttpResponseNotFound
 from django.shortcuts import redirect
 from django.template.response import TemplateResponse
 from django.utils import timezone
@@ -17,7 +16,7 @@ from bookwyrm import forms, models
 from bookwyrm.activitypub import ActivitypubResponse
 from bookwyrm.settings import PAGE_LENGTH
 from .helpers import get_user_from_username, is_api_request
-from .helpers import is_blocked, privacy_filter
+from .helpers import privacy_filter
 
 
 # pylint: disable= no-self-use
@@ -26,14 +25,7 @@ class User(View):
 
     def get(self, request, username):
         """profile page for a user"""
-        try:
         user = get_user_from_username(request.user, username)
-        except models.User.DoesNotExist:
-            return HttpResponseNotFound()
-
-        # make sure we're not blocked
-        if is_blocked(request.user, user):
-            return HttpResponseNotFound()
 
         if is_api_request(request):
             # we have a json request
@@ -94,14 +86,7 @@ class Followers(View):
 
     def get(self, request, username):
         """list of followers"""
-        try:
         user = get_user_from_username(request.user, username)
-        except models.User.DoesNotExist:
-            return HttpResponseNotFound()
-
-        # make sure we're not blocked
-        if is_blocked(request.user, user):
-            return HttpResponseNotFound()
 
         if is_api_request(request):
             return ActivitypubResponse(user.to_followers_activity(**request.GET))
@@ -110,9 +95,9 @@ class Followers(View):
         data = {
             "user": user,
             "is_self": request.user.id == user.id,
-            "followers": paginated.page(request.GET.get("page", 1)),
+            "follow_list": paginated.page(request.GET.get("page", 1)),
         }
-        return TemplateResponse(request, "user/followers.html", data)
+        return TemplateResponse(request, "user/relationships/followers.html", data)
 
 
 class Following(View):
@@ -120,25 +105,18 @@ class Following(View):
 
     def get(self, request, username):
         """list of followers"""
-        try:
         user = get_user_from_username(request.user, username)
-        except models.User.DoesNotExist:
-            return HttpResponseNotFound()
-
-        # make sure we're not blocked
-        if is_blocked(request.user, user):
-            return HttpResponseNotFound()
 
         if is_api_request(request):
             return ActivitypubResponse(user.to_following_activity(**request.GET))
 
-        paginated = Paginator(user.followers.all(), PAGE_LENGTH)
+        paginated = Paginator(user.following.all(), PAGE_LENGTH)
         data = {
             "user": user,
             "is_self": request.user.id == user.id,
-            "following": paginated.page(request.GET.get("page", 1)),
+            "follow_list": paginated.page(request.GET.get("page", 1)),
         }
-        return TemplateResponse(request, "user/following.html", data)
+        return TemplateResponse(request, "user/relationships/following.html", data)
 
 
 @method_decorator(login_required, name="dispatch")