Merge branch 'main' into production

bookwyrm/models/shelf.py

@@ -37,9 +37,13 @@ class Shelf(OrderedCollectionMixin, BookWyrmModel):
         """ set the identifier """
         super().save(*args, **kwargs)
         if not self.identifier:
-            slug = re.sub(r"[^\w]", "", self.name).lower()
+            self.identifier = self.get_identifier()
-            self.identifier = "%s-%d" % (slug, self.id)
+            super().save(*args, **kwargs, broadcast=False)
-            super().save(*args, **kwargs)
+
+    def get_identifier(self):
+        """ custom-shelf-123 for the url """
+        slug = re.sub(r"[^\w]", "", self.name).lower()
+        return "{:s}-{:d}".format(slug, self.id)
 
     @property
     def collection_queryset(self):
@@ -49,7 +53,8 @@ class Shelf(OrderedCollectionMixin, BookWyrmModel):
     def get_remote_id(self):
         """ shelf identifier instead of id """
         base_path = self.user.remote_id
-        return "%s/shelf/%s" % (base_path, self.identifier)
+        identifier = self.identifier or self.get_identifier()
+        return "%s/books/%s" % (base_path, identifier)
 
     class Meta:
         """ user/shelf unqiueness """

bookwyrm/templates/book/book.html

@@ -165,32 +165,11 @@
                 {% include 'snippets/readthrough.html' with readthrough=readthrough %}
                 {% endfor %}
             </section>
-            {% endif %}
 
-            {% if request.user.is_authenticated %}
             <section class="box">
                 {% include 'snippets/create_status.html' with book=book hide_cover=True %}
             </section>
-
-            <section class="block">
-                <form name="tag" action="/tag/" method="post">
-                    <label for="tags" class="is-3">{% trans "Tags" %}</label>
-                    {% csrf_token %}
-                    <input type="hidden" name="book" value="{{ book.id }}">
-                    <input id="tags" class="input" type="text" name="name">
-                    <button class="button" type="submit">{% trans "Add tag" %}</button>
-                </form>
-            </section>
             {% endif %}
-
-            <div class="block">
-                <div class="field is-grouped is-grouped-multiline">
-                {% for tag in tags %}
-                    {% include 'snippets/tag.html' with book=book tag=tag user_tags=user_tags %}
-                {% endfor %}
-                </div>
-            </div>
-
         </div>
         <div class="column is-one-fifth">
             {% if book.subjects %}

bookwyrm/templates/layout.html

@@ -55,7 +55,7 @@
         <div class="navbar-start">
             {% if request.user.is_authenticated %}
             <a href="{% url 'user-shelves' request.user.localname %}" class="navbar-item">
-                {% trans "Your shelves" %}
+                {% trans "Your books" %}
             </a>
             <a href="/#feed" class="navbar-item">
                 {% trans "Feed" %}

bookwyrm/templates/snippets/shelf_selector.html

@@ -1,7 +1,7 @@
 {% extends 'components/dropdown.html' %}
 {% load i18n %}
 {% block dropdown-trigger %}
-<span>{% trans "Change shelf" %}</span>
+<span>{% trans "Move book" %}</span>
 <span class="icon icon-arrow-down" aria-hidden="true"></span>
 {% endblock %}
 
@@ -25,7 +25,7 @@
         {% csrf_token %}
         <input type="hidden" name="book" value="{{ book.id }}">
         <input type="hidden" name="shelf" value="{{ current.id }}">
-        <button class="button is-fullwidth is-small is-danger is-light" type="submit">{% trans "Unshelve" %}</button>
+        <button class="button is-fullwidth is-small is-danger is-light" type="submit">{% trans "Remove" %}</button>
     </form>
 </li>
 {% endblock %}

bookwyrm/templates/snippets/shelve_button/shelve_button_options.html

@@ -45,7 +45,7 @@
             {% csrf_token %}
             <input type="hidden" name="book" value="{{ active_shelf.book.id }}">
             <input type="hidden" name="shelf" value="{{ active_shelf.shelf.id }}">
-            <button class="button is-fullwidth is-small is-danger is-light" type="submit">{% trans "Unshelve" %}</button>
+            <button class="button is-fullwidth is-small is-danger is-light" type="submit">{% blocktrans with name=active_shelf.shelf.name %}Remove from {{ name }}{% endblocktrans %}</button>
         </form>
     </div>
 </li>

bookwyrm/templates/user/books_header.html

@@ -0,0 +1,6 @@
+{% load i18n %}
+{% if is_self %}
+{% trans "Your books" %}
+{% else %}
+{% blocktrans with username=user.display_name %}{{ username }}'s books{% endblocktrans %}
+{% endif %}

bookwyrm/templates/user/shelf.html

@@ -3,14 +3,14 @@
 {% load humanize %}
 {% load i18n %}
 
+{% block title %}
+{% include 'user/books_header.html' %}
+{% endblock %}
+
 {% block header %}
 <header class="columns">
     <h1 class="title">
-        {% if is_self %}
+        {% include 'user/books_header.html' %}
-        {% trans "Your Shelves" %}
-        {% else %}
-        {% blocktrans with username=user.display_name %}{{ username }}: Shelves{% endblocktrans %}
-        {% endif %}
     </h1>
 </header>
 {% endblock %}
@@ -20,10 +20,13 @@
     <div class="column">
         <div class="tabs">
             <ul>
+            <li class="{% if shelf.identifier == 'all' %}is-active{% endif %}">
+                <a href="{% url 'user-shelves' user|username %}"{% if shelf.identifier == 'all' %} aria-current="page"{% endif %}>{% trans "All books" %}</a>
+            </li>
             {% for shelf_tab in shelves %}
             <li class="{% if shelf_tab.identifier == shelf.identifier %}is-active{% endif %}">
-                <a href="/user/{{ user | username }}/shelf/{{ shelf_tab.identifier }}"{% if shelf_tab.identifier == shelf.identifier %} aria-current="page"{% endif %}>{% if shelf_tab.identifier == 'to-read' %}{% trans "To Read" %}{% elif shelf_tab.identifier == 'reading' %}{% trans "Currently Reading" %}{% elif shelf_tab.identifier == 'read' %}{% trans "Read" %}{% else %}{{ shelf_tab.name }}{% endif %}</a>
+                <a href="{{ shelf_tab.local_path }}"{% if shelf_tab.identifier == shelf.identifier %} aria-current="page"{% endif %}>{% if shelf_tab.identifier == 'to-read' %}{% trans "To Read" %}{% elif shelf_tab.identifier == 'reading' %}{% trans "Currently Reading" %}{% elif shelf_tab.identifier == 'read' %}{% trans "Read" %}{% else %}{{ shelf_tab.name }}{% endif %}</a>
-                </li>
+            </li>
             {% endfor %}
             </ul>
         </div>
@@ -50,7 +53,7 @@
             </span>
         </h2>
     </div>
-    {% if is_self %}
+    {% if is_self and shelf.id %}
     <div class="column is-narrow">
         {% trans "Edit shelf" as button_text %}
         {% include 'snippets/toggle/open_button.html' with text=button_text icon="pencil" controls_text="edit-shelf-form" focus="edit-shelf-form-header" %}
@@ -81,7 +84,6 @@
             {% endif %}
         </tr>
         {% for book in books %}
-        {% with book=book.book %}
         <tr class="book-preview">
             <td>
                 <a href="{{ book.local_path }}">{% include 'snippets/book_cover.html' with book=book size="small" %}</a>
@@ -113,13 +115,12 @@
             </td>
             {% endif %}
         </tr>
-        {% endwith %}
         {% endfor %}
         </table>
         </div>
         {% else %}
         <p>{% trans "This shelf is empty." %}</p>
-        {% if shelf.editable %}
+        {% if shelf.id and shelf.editable %}
         <form name="delete-shelf" action="/delete-shelf/{{ shelf.id }}" method="post">
             {% csrf_token %}
             <input type="hidden" name="user" value="{{ request.user.id }}">

bookwyrm/templates/user/user.html

@@ -1,5 +1,6 @@
 {% extends 'user/user_layout.html' %}
 {% load i18n %}
+{% load bookwyrm_tags %}
 
 {% block title %}{{ user.display_name }}{% endblock %}
 
@@ -23,12 +24,14 @@
 {% block panel %}
 {% if user.bookwyrm_user %}
 <div class="block">
-    <h2 class="title">{% trans "Shelves" %}</h2>
+    <h2 class="title">
+        {% include 'user/books_header.html' %}
+    </h2>
     <div class="columns">
         {% for shelf in shelves %}
         <div class="column is-narrow">
             <h3>{{ shelf.name }}
-                {% if shelf.size > 3 %}<small>(<a href="{{ shelf.local_path }}">{% blocktrans with size=shelf.size %}See all {{ size }}{% endblocktrans %}</a>)</small>{% endif %}</h3>
+                {% if shelf.size > 3 %}<small>(<a href="{{ shelf.local_path }}">{% blocktrans with size=shelf.size %}View all {{ size }}{% endblocktrans %}</a>)</small>{% endif %}</h3>
             <div class="is-mobile field is-grouped">
             {% for book in shelf.books %}
             <div class="control">
@@ -41,7 +44,7 @@
         </div>
         {% endfor %}
     </div>
-    <small><a href="{{ user.local_path }}/shelves">{% blocktrans %}See all {{ shelf_count }} shelves{% endblocktrans %}</a></small>
+    <small><a href="{% url 'user-shelves' user|username %}">{% trans "View all books" %}</a></small>
 </div>
 {% endif %}
 

bookwyrm/templates/user/user_layout.html

@@ -42,7 +42,7 @@
     {% endif %}
 </div>
 {% with user|username as username %}
-{% if 'user/'|add:username|add:'/shelf' not in request.path  and 'user/'|add:username|add:'/shelves' not in request.path %}
+{% 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 %}
@@ -65,7 +65,7 @@
         {% if user.shelf_set.exists %}
         {% url 'user-shelves' user|username as url %}
         <li{% if url in request.path %} class="is-active"{% endif %}>
-            <a href="{{ url }}">{% trans "Shelves" %}</a>
+            <a href="{{ url }}">{% trans "Books" %}</a>
         </li>
         {% endif %}
     </ul>

bookwyrm/tests/models/test_shelf_model.py

@@ -27,7 +27,7 @@ class Shelf(TestCase):
         shelf = models.Shelf.objects.create(
             name="Test Shelf", identifier="test-shelf", user=self.local_user
         )
-        expected_id = "https://%s/user/mouse/shelf/test-shelf" % settings.DOMAIN
+        expected_id = "https://%s/user/mouse/books/test-shelf" % settings.DOMAIN
         self.assertEqual(shelf.get_remote_id(), expected_id)
         models.Shelf.broadcast = real_broadcast
 

bookwyrm/urls.py

@@ -138,16 +138,8 @@ urlpatterns = [
         views.Following.as_view(),
         name="user-following",
     ),
-    re_path(r"%s/shelves/?$" % user_path, views.user_shelves_page, name="user-shelves"),
-    re_path(r"%s/lists/?$" % user_path, views.UserLists.as_view(), name="user-lists"),
-    # goals
-    re_path(
-        r"%s/goal/(?P<year>\d{4})/?$" % user_path,
-        views.Goal.as_view(),
-        name="user-goal",
-    ),
-    re_path(r"^hide-goal/?$", views.hide_goal, name="hide-goal"),
     # lists
+    re_path(r"%s/lists/?$" % user_path, views.UserLists.as_view(), name="user-lists"),
     re_path(r"^list/?$", views.Lists.as_view(), name="lists"),
     re_path(r"^list/(?P<list_id>\d+)(.json)?/?$", views.List.as_view(), name="list"),
     re_path(r"^list/add-book/?$", views.list.add_book, name="list-add-book"),
@@ -159,6 +151,29 @@ urlpatterns = [
     re_path(
         r"^list/(?P<list_id>\d+)/curate/?$", views.Curate.as_view(), name="list-curate"
     ),
+    # Uyser books
+    re_path(r"%s/books/?$" % user_path, views.Shelf.as_view(), name="user-shelves"),
+    re_path(
+        r"^%s/(helf|books)/(?P<shelf_identifier>[\w-]+)(.json)?/?$" % user_path,
+        views.Shelf.as_view(),
+        name="shelf",
+    ),
+    re_path(
+        r"^%s/(books|shelf)/(?P<shelf_identifier>[\w-]+)(.json)?/?$" % local_user_path,
+        views.Shelf.as_view(),
+        name="shelf",
+    ),
+    re_path(r"^create-shelf/?$", views.create_shelf, name="shelf-create"),
+    re_path(r"^delete-shelf/(?P<shelf_id>\d+)?$", views.delete_shelf),
+    re_path(r"^shelve/?$", views.shelve),
+    re_path(r"^unshelve/?$", views.unshelve),
+    # goals
+    re_path(
+        r"%s/goal/(?P<year>\d{4})/?$" % user_path,
+        views.Goal.as_view(),
+        name="user-goal",
+    ),
+    re_path(r"^hide-goal/?$", views.hide_goal, name="hide-goal"),
     # preferences
     re_path(r"^preferences/profile/?$", views.EditUser.as_view(), name="prefs-profile"),
     re_path(r"^preferences/password/?$", views.ChangePassword.as_view()),
@@ -199,20 +214,6 @@ urlpatterns = [
     re_path(r"^tag/(?P<tag_id>.+)/?$", views.Tag.as_view()),
     re_path(r"^tag/?$", views.AddTag.as_view()),
     re_path(r"^untag/?$", views.RemoveTag.as_view()),
-    # shelf
-    re_path(
-        r"^%s/shelf/(?P<shelf_identifier>[\w-]+)(.json)?/?$" % user_path,
-        views.Shelf.as_view(),
-        name="shelf",
-    ),
-    re_path(
-        r"^%s/shelf/(?P<shelf_identifier>[\w-]+)(.json)?/?$" % local_user_path,
-        views.Shelf.as_view(),
-    ),
-    re_path(r"^create-shelf/?$", views.create_shelf, name="shelf-create"),
-    re_path(r"^delete-shelf/(?P<shelf_id>\d+)?$", views.delete_shelf),
-    re_path(r"^shelve/?$", views.shelve),
-    re_path(r"^unshelve/?$", views.unshelve),
     # reading progress
     re_path(r"^edit-readthrough/?$", views.edit_readthrough, name="edit-readthrough"),
     re_path(r"^delete-readthrough/?$", views.delete_readthrough),

bookwyrm/views/__init__.py

@@ -27,7 +27,7 @@ from .rss_feed import RssFeed
 from .password import PasswordResetRequest, PasswordReset, ChangePassword
 from .search import Search
 from .shelf import Shelf
-from .shelf import user_shelves_page, create_shelf, delete_shelf
+from .shelf import create_shelf, delete_shelf
 from .shelf import shelve, unshelve
 from .site import Site
 from .status import CreateStatus, DeleteStatus

bookwyrm/views/shelf.py

@@ -1,4 +1,6 @@
 """ shelf views"""
+from collections import namedtuple
+
 from django.db import IntegrityError
 from django.contrib.auth.decorators import login_required
 from django.core.paginator import Paginator
@@ -6,6 +8,7 @@ from django.http import HttpResponseBadRequest, HttpResponseNotFound
 from django.shortcuts import get_object_or_404, redirect
 from django.template.response import TemplateResponse
 from django.utils.decorators import method_decorator
+from django.utils.translation import gettext as _
 from django.views import View
 from django.views.decorators.http import require_POST
 
@@ -13,14 +16,14 @@ from bookwyrm import forms, models
 from bookwyrm.activitypub import ActivitypubResponse
 from bookwyrm.settings import PAGE_LENGTH
 from .helpers import is_api_request, get_edition, get_user_from_username
-from .helpers import handle_reading_status
+from .helpers import handle_reading_status, privacy_filter, object_visible_to_user
 
 
 # pylint: disable= no-self-use
 class Shelf(View):
     """ shelf page """
 
-    def get(self, request, username, shelf_identifier):
+    def get(self, request, username, shelf_identifier=None):
         """ display a shelf """
         try:
             user = get_user_from_username(request.user, username)
@@ -32,35 +35,30 @@ class Shelf(View):
         except ValueError:
             page = 1
 
+        shelves = privacy_filter(request.user, user.shelf_set)
+
+        # get the shelf and make sure the logged in user should be able to see it
         if shelf_identifier:
             shelf = user.shelf_set.get(identifier=shelf_identifier)
+            if not object_visible_to_user(request.user, shelf):
+                return HttpResponseNotFound()
+        # this is a constructed "all books" view, with a fake "shelf" obj
         else:
-            shelf = user.shelf_set.first()
+            FakeShelf = namedtuple(
+                "Shelf", ("identifier", "name", "user", "books", "privacy")
+            )
+            books = models.Edition.objects.filter(
+                shelfbook__shelf__in=shelves.all()
+            ).distinct()
+            shelf = FakeShelf("all", _("All books"), user, books, "public")
 
         is_self = request.user == user
 
-        shelves = user.shelf_set
-        if not is_self:
-            follower = user.followers.filter(id=request.user.id).exists()
-            # make sure the user has permission to view the shelf
-            if shelf.privacy == "direct" or (
-                shelf.privacy == "followers" and not follower
-            ):
-                return HttpResponseNotFound()
-
-            # only show other shelves that should be visible
-            if follower:
-                shelves = shelves.filter(privacy__in=["public", "followers"])
-            else:
-                shelves = shelves.filter(privacy="public")
-
         if is_api_request(request):
             return ActivitypubResponse(shelf.to_activity(**request.GET))
 
         paginated = Paginator(
-            models.ShelfBook.objects.filter(user=user, shelf=shelf)
+            shelf.books.order_by("-updated_date").all(),
-            .order_by("-updated_date")
-            .all(),
             PAGE_LENGTH,
         )
 
@@ -95,11 +93,6 @@ class Shelf(View):
         return redirect(shelf.local_path)
 
 
-def user_shelves_page(request, username):
-    """ default shelf """
-    return Shelf.as_view()(request, username, None)
-
-
 @login_required
 @require_POST
 def create_shelf(request):
@@ -176,7 +169,8 @@ def shelve(request):
             models.ShelfBook.objects.create(
                 book=book, shelf=desired_shelf, user=request.user
             )
-        # The book is already on this shelf. Might be good to alert, or reject the action?
+        # The book is already on this shelf.
+        # Might be good to alert, or reject the action?
         except IntegrityError:
             pass
     return redirect(request.headers.get("Referer", "/"))