diff --git a/bookwyrm/forms.py b/bookwyrm/forms.py index 9b410eaf..5acde9ea 100644 --- a/bookwyrm/forms.py +++ b/bookwyrm/forms.py @@ -268,7 +268,7 @@ class CreateInviteForm(CustomForm): class ShelfForm(CustomForm): class Meta: model = models.Shelf - fields = ["user", "name", "privacy"] + fields = ["user", "name", "privacy", "description"] class GoalForm(CustomForm): diff --git a/bookwyrm/migrations/0100_shelf_description.py b/bookwyrm/migrations/0100_shelf_description.py new file mode 100644 index 00000000..18185b17 --- /dev/null +++ b/bookwyrm/migrations/0100_shelf_description.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.5 on 2021-09-28 23:20 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("bookwyrm", "0099_readthrough_is_active"), + ] + + operations = [ + migrations.AddField( + model_name="shelf", + name="description", + field=models.TextField(blank=True, max_length=500, null=True), + ), + ] diff --git a/bookwyrm/models/shelf.py b/bookwyrm/models/shelf.py index 89ea9471..c578f082 100644 --- a/bookwyrm/models/shelf.py +++ b/bookwyrm/models/shelf.py @@ -21,6 +21,7 @@ class Shelf(OrderedCollectionMixin, BookWyrmModel): name = fields.CharField(max_length=100) identifier = models.CharField(max_length=100) + description = models.TextField(blank=True, null=True, max_length=500) user = fields.ForeignKey( "User", on_delete=models.PROTECT, activitypub_field="owner" ) @@ -52,6 +53,11 @@ class Shelf(OrderedCollectionMixin, BookWyrmModel): """list of books for this shelf, overrides OrderedCollectionMixin""" return self.books.order_by("shelfbook") + @property + def deletable(self): + """can the shelf be safely deleted?""" + return self.editable and not self.shelfbook_set.exists() + def get_remote_id(self): """shelf identifier instead of id""" base_path = self.user.remote_id @@ -61,7 +67,7 @@ class Shelf(OrderedCollectionMixin, BookWyrmModel): def raise_not_deletable(self, viewer): """don't let anyone delete a default shelf""" super().raise_not_deletable(viewer) - if not self.editable: + if not self.deletable: raise PermissionDenied() class Meta: diff --git a/bookwyrm/templates/shelf/create_shelf_form.html b/bookwyrm/templates/shelf/create_shelf_form.html new file mode 100644 index 00000000..e15e1cc1 --- /dev/null +++ b/bookwyrm/templates/shelf/create_shelf_form.html @@ -0,0 +1,13 @@ +{% extends 'components/inline_form.html' %} +{% load i18n %} + +{% block header %} +{% trans "Create Shelf" %} +{% endblock %} + +{% block form %} +
+ {% include "shelf/form.html" with editable=shelf.editable form=create_form %} +
+{% endblock %} + diff --git a/bookwyrm/templates/shelf/edit_shelf_form.html b/bookwyrm/templates/shelf/edit_shelf_form.html new file mode 100644 index 00000000..5951b6da --- /dev/null +++ b/bookwyrm/templates/shelf/edit_shelf_form.html @@ -0,0 +1,13 @@ +{% extends 'components/inline_form.html' %} +{% load i18n %} + +{% block header %} +{% trans "Edit Shelf" %} +{% endblock %} + +{% block form %} +
+ {% include "shelf/form.html" with editable=shelf.editable form=edit_form privacy=shelf.privacy %} +
+{% endblock %} + diff --git a/bookwyrm/templates/shelf/form.html b/bookwyrm/templates/shelf/form.html new file mode 100644 index 00000000..ff7f8b5e --- /dev/null +++ b/bookwyrm/templates/shelf/form.html @@ -0,0 +1,28 @@ +{% load i18n %} +{% load utilities %} +{% with 0|uuid as uuid %} +{% csrf_token %} + + +{% if editable %} +
+ + +
+{% else %} + +{% endif %} + +
+ + +
+
+
+ {% include 'snippets/privacy_select.html' with current=privacy %} +
+
+ +
+
+{% endwith %} diff --git a/bookwyrm/templates/user/shelf/shelf.html b/bookwyrm/templates/shelf/shelf.html similarity index 70% rename from bookwyrm/templates/user/shelf/shelf.html rename to bookwyrm/templates/shelf/shelf.html index 06507d3e..88f4b2bb 100644 --- a/bookwyrm/templates/user/shelf/shelf.html +++ b/bookwyrm/templates/shelf/shelf.html @@ -5,7 +5,7 @@ {% load i18n %} {% block title %} -{% include 'user/shelf/books_header.html' %} +{% include 'user/books_header.html' %} {% endblock %} {% block opengraph_images %} @@ -15,7 +15,7 @@ {% block content %}

- {% include 'user/shelf/books_header.html' %} + {% include 'user/books_header.html' %}

@@ -60,45 +60,62 @@
- {% include 'user/shelf/create_shelf_form.html' with controls_text='create_shelf_form' %} + {% include 'shelf/create_shelf_form.html' with controls_text='create_shelf_form' %}
-
-
-

- {{ shelf.name }} - - {% include 'snippets/privacy-icons.html' with item=shelf %} - - {% with count=books.paginator.count %} - {% if count %} -

- {% blocktrans trimmed count counter=count with formatted_count=count|intcomma %} - {{ formatted_count }} book - {% plural %} - {{ formatted_count }} books - {% endblocktrans %} - - {% if books.has_other_pages %} - {% blocktrans trimmed with start=books.start_index end=books.end_index %} - (showing {{ start }}-{{ end }}) +

+
+
+

+ {{ shelf.name }} + + {% include 'snippets/privacy-icons.html' with item=shelf %} + + {% with count=books.paginator.count %} + {% if count %} +

+ {% blocktrans trimmed count counter=count with formatted_count=count|intcomma %} + {{ formatted_count }} book + {% plural %} + {{ formatted_count }} books {% endblocktrans %} + + {% if books.has_other_pages %} + {% blocktrans trimmed with start=books.start_index end=books.end_index %} + (showing {{ start }}-{{ end }}) + {% endblocktrans %} + {% endif %} +

{% endif %} -

- {% endif %} - {% endwith %} -

-
- {% if is_self and shelf.id %} -
- {% trans "Edit shelf" as button_text %} - {% include 'snippets/toggle/open_button.html' with text=button_text icon_with_text="pencil" controls_text="edit_shelf_form" focus="edit_shelf_form_header" %} + {% endwith %} +

+
+ {% if is_self and shelf.id %} +
+
+ {% trans "Edit shelf" as button_text %} + {% include 'snippets/toggle/open_button.html' with text=button_text icon_with_text="pencil" controls_text="edit_shelf_form" focus="edit_shelf_form_header" %} + + {% if shelf.deletable %} +
+ {% csrf_token %} + + +
+ {% endif %} +
+
+ {% endif %}
+ {% if shelf.description %} +

{{ shelf.description }}

{% endif %}
- {% include 'user/shelf/edit_shelf_form.html' with controls_text="edit_shelf_form" %} + {% include 'shelf/edit_shelf_form.html' with controls_text="edit_shelf_form" %}
@@ -167,17 +184,7 @@ {% else %} -

{% trans "This shelf is empty." %}

- {% if shelf.id and shelf.editable %} -
- {% csrf_token %} - - -
- {% endif %} - +

{% trans "This shelf is empty." %}

{% endif %}
diff --git a/bookwyrm/templates/snippets/privacy_select.html b/bookwyrm/templates/snippets/privacy_select.html index d7184e13..e1053051 100644 --- a/bookwyrm/templates/snippets/privacy_select.html +++ b/bookwyrm/templates/snippets/privacy_select.html @@ -1,7 +1,7 @@ {% load i18n %} {% load utilities %}
- {% with 0|uuid as uuid %} + {% firstof uuid 0|uuid as uuid %} {% if not no_label %} {% endif %} @@ -20,6 +20,5 @@ {% trans "Private" %} - {% endwith %}
diff --git a/bookwyrm/templates/user/shelf/books_header.html b/bookwyrm/templates/user/books_header.html similarity index 100% rename from bookwyrm/templates/user/shelf/books_header.html rename to bookwyrm/templates/user/books_header.html diff --git a/bookwyrm/templates/goal.html b/bookwyrm/templates/user/goal.html similarity index 100% rename from bookwyrm/templates/goal.html rename to bookwyrm/templates/user/goal.html diff --git a/bookwyrm/templates/user/relationships/followers.html b/bookwyrm/templates/user/relationships/followers.html index 223f38c7..c7d6833b 100644 --- a/bookwyrm/templates/user/relationships/followers.html +++ b/bookwyrm/templates/user/relationships/followers.html @@ -9,6 +9,6 @@ {% block nullstate %}
- {% blocktrans with username=user.display_name %}{{ username }} has no followers{% endblocktrans %} + {% blocktrans with username=user.display_name %}{{ username }} has no followers{% endblocktrans %}
{% endblock %} diff --git a/bookwyrm/templates/user/relationships/following.html b/bookwyrm/templates/user/relationships/following.html index 475d40ab..3952d045 100644 --- a/bookwyrm/templates/user/relationships/following.html +++ b/bookwyrm/templates/user/relationships/following.html @@ -9,7 +9,7 @@ {% block nullstate %}
- {% blocktrans with username=user.display_name %}{{ username }} isn't following any users{% endblocktrans %} + {% blocktrans with username=user.display_name %}{{ username }} isn't following any users{% endblocktrans %}
{% endblock %} diff --git a/bookwyrm/templates/user/shelf/create_shelf_form.html b/bookwyrm/templates/user/shelf/create_shelf_form.html deleted file mode 100644 index b7ea27de..00000000 --- a/bookwyrm/templates/user/shelf/create_shelf_form.html +++ /dev/null @@ -1,27 +0,0 @@ -{% extends 'components/inline_form.html' %} -{% load i18n %} - -{% block header %} -{% trans "Create Shelf" %} -{% endblock %} - -{% block form %} -
- {% csrf_token %} - -
- - -
- -
-
- {% include 'snippets/privacy_select.html' %} -
-
- -
-
-
-{% endblock %} - diff --git a/bookwyrm/templates/user/shelf/edit_shelf_form.html b/bookwyrm/templates/user/shelf/edit_shelf_form.html deleted file mode 100644 index 753d0681..00000000 --- a/bookwyrm/templates/user/shelf/edit_shelf_form.html +++ /dev/null @@ -1,31 +0,0 @@ -{% extends 'components/inline_form.html' %} -{% load i18n %} - -{% block header %} -{% trans "Edit Shelf" %} -{% endblock %} - -{% block form %} -
- {% csrf_token %} - - {% if shelf.editable %} -
- - -
- {% else %} - - {% endif %} - -
-
- {% include 'snippets/privacy_select.html' with current=shelf.privacy %} -
-
- -
-
-
-{% endblock %} - diff --git a/bookwyrm/templates/user/user.html b/bookwyrm/templates/user/user.html index f360a30a..36e646aa 100755 --- a/bookwyrm/templates/user/user.html +++ b/bookwyrm/templates/user/user.html @@ -24,7 +24,7 @@ {% if user.bookwyrm_user %}

- {% include 'user/shelf/books_header.html' %} + {% include 'user/books_header.html' %}

{% for shelf in shelves %} diff --git a/bookwyrm/tests/views/test_shelf.py b/bookwyrm/tests/views/test_shelf.py index 0d7d01cb..d769d93c 100644 --- a/bookwyrm/tests/views/test_shelf.py +++ b/bookwyrm/tests/views/test_shelf.py @@ -1,11 +1,14 @@ """ test for app action functionality """ import json from unittest.mock import patch +from tidylib import tidy_document + +from django.core.exceptions import PermissionDenied from django.template.response import TemplateResponse from django.test import TestCase from django.test.client import RequestFactory -from bookwyrm import models, views +from bookwyrm import forms, models, views from bookwyrm.activitypub import ActivitypubResponse @@ -53,7 +56,16 @@ class ShelfViews(TestCase): is_api.return_value = False result = view(request, self.local_user.username, shelf.identifier) self.assertIsInstance(result, TemplateResponse) - result.render() + html = result.render() + _, errors = tidy_document( + html.content, + options={ + "drop-empty-elements": False, + "warn-proprietary-attributes": False, + }, + ) + if errors: + raise Exception(errors) self.assertEqual(result.status_code, 200) with patch("bookwyrm.views.shelf.is_api_request") as is_api: @@ -122,7 +134,7 @@ class ShelfViews(TestCase): self.assertEqual(shelf.name, "To Read") - def test_handle_shelve(self, *_): + def test_shelve(self, *_): """shelve a book""" request = self.factory.post( "", {"book": self.book.id, "shelf": self.shelf.identifier} @@ -140,7 +152,7 @@ class ShelfViews(TestCase): # make sure the book is on the shelf self.assertEqual(self.shelf.books.get(), self.book) - def test_handle_shelve_to_read(self, *_): + def test_shelve_to_read(self, *_): """special behavior for the to-read shelf""" shelf = models.Shelf.objects.get(identifier="to-read") request = self.factory.post( @@ -153,7 +165,7 @@ class ShelfViews(TestCase): # make sure the book is on the shelf self.assertEqual(shelf.books.get(), self.book) - def test_handle_shelve_reading(self, *_): + def test_shelve_reading(self, *_): """special behavior for the reading shelf""" shelf = models.Shelf.objects.get(identifier="reading") request = self.factory.post( @@ -166,7 +178,7 @@ class ShelfViews(TestCase): # make sure the book is on the shelf self.assertEqual(shelf.books.get(), self.book) - def test_handle_shelve_read(self, *_): + def test_shelve_read(self, *_): """special behavior for the read shelf""" shelf = models.Shelf.objects.get(identifier="read") request = self.factory.post( @@ -179,7 +191,7 @@ class ShelfViews(TestCase): # make sure the book is on the shelf self.assertEqual(shelf.books.get(), self.book) - def test_handle_unshelve(self, *_): + def test_unshelve(self, *_): """remove a book from a shelf""" with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): models.ShelfBook.objects.create( @@ -197,3 +209,76 @@ class ShelfViews(TestCase): self.assertEqual(activity["type"], "Remove") self.assertEqual(activity["object"]["id"], item.remote_id) self.assertEqual(self.shelf.books.count(), 0) + + def test_create_shelf(self, *_): + """a brand new custom shelf""" + form = forms.ShelfForm() + form.data["user"] = self.local_user.id + form.data["name"] = "new shelf name" + form.data["description"] = "desc" + form.data["privacy"] = "unlisted" + request = self.factory.post("", form.data) + request.user = self.local_user + + views.create_shelf(request) + + shelf = models.Shelf.objects.get(name="new shelf name") + self.assertEqual(shelf.privacy, "unlisted") + self.assertEqual(shelf.description, "desc") + self.assertEqual(shelf.user, self.local_user) + + def test_delete_shelf(self, *_): + """delete a brand new custom shelf""" + request = self.factory.post("") + request.user = self.local_user + shelf_id = self.shelf.id + + views.delete_shelf(request, shelf_id) + + self.assertFalse(models.Shelf.objects.filter(id=shelf_id).exists()) + + def test_delete_shelf_unauthorized(self, *_): + """delete a brand new custom shelf""" + with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( + "bookwyrm.activitystreams.populate_stream_task.delay" + ): + rat = models.User.objects.create_user( + "rat@local.com", + "rat@mouse.mouse", + "password", + local=True, + localname="rat", + ) + request = self.factory.post("") + request.user = rat + + with self.assertRaises(PermissionDenied): + views.delete_shelf(request, self.shelf.id) + + self.assertTrue(models.Shelf.objects.filter(id=self.shelf.id).exists()) + + def test_delete_shelf_has_book(self, *_): + """delete a brand new custom shelf""" + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + models.ShelfBook.objects.create( + book=self.book, user=self.local_user, shelf=self.shelf + ) + request = self.factory.post("") + request.user = self.local_user + + with self.assertRaises(PermissionDenied): + views.delete_shelf(request, self.shelf.id) + + self.assertTrue(models.Shelf.objects.filter(id=self.shelf.id).exists()) + + def test_delete_shelf_not_editable(self, *_): + """delete a brand new custom shelf""" + shelf = self.local_user.shelf_set.first() + self.assertFalse(shelf.editable) + request = self.factory.post("") + request.user = self.local_user + + with self.assertRaises(PermissionDenied): + views.delete_shelf(request, shelf.id) + + self.assertTrue(models.Shelf.objects.filter(id=shelf.id).exists()) diff --git a/bookwyrm/tests/views/test_user.py b/bookwyrm/tests/views/test_user.py index 614f772d..f2d9b861 100644 --- a/bookwyrm/tests/views/test_user.py +++ b/bookwyrm/tests/views/test_user.py @@ -1,5 +1,6 @@ """ test for app action functionality """ from unittest.mock import patch +from tidylib import tidy_document from django.contrib.auth.models import AnonymousUser from django.http.response import Http404 @@ -55,7 +56,16 @@ class UserViews(TestCase): is_api.return_value = False result = view(request, "mouse") self.assertIsInstance(result, TemplateResponse) - result.render() + html = result.render() + _, errors = tidy_document( + html.content, + options={ + "drop-empty-elements": False, + "warn-proprietary-attributes": False, + }, + ) + if errors: + raise Exception(errors) self.assertEqual(result.status_code, 200) request.user = self.anonymous_user @@ -63,7 +73,16 @@ class UserViews(TestCase): is_api.return_value = False result = view(request, "mouse") self.assertIsInstance(result, TemplateResponse) - result.render() + html = result.render() + _, errors = tidy_document( + html.content, + options={ + "drop-empty-elements": False, + "warn-proprietary-attributes": False, + }, + ) + if errors: + raise Exception(errors) self.assertEqual(result.status_code, 200) with patch("bookwyrm.views.user.is_api_request") as is_api: @@ -92,7 +111,16 @@ class UserViews(TestCase): is_api.return_value = False result = view(request, "mouse") self.assertIsInstance(result, TemplateResponse) - result.render() + html = result.render() + _, errors = tidy_document( + html.content, + options={ + "drop-empty-elements": False, + "warn-proprietary-attributes": False, + }, + ) + if errors: + raise Exception(errors) self.assertEqual(result.status_code, 200) with patch("bookwyrm.views.user.is_api_request") as is_api: @@ -123,7 +151,16 @@ class UserViews(TestCase): is_api.return_value = False result = view(request, "mouse") self.assertIsInstance(result, TemplateResponse) - result.render() + html = result.render() + _, errors = tidy_document( + html.content, + options={ + "drop-empty-elements": False, + "warn-proprietary-attributes": False, + }, + ) + if errors: + raise Exception(errors) self.assertEqual(result.status_code, 200) with patch("bookwyrm.views.user.is_api_request") as is_api: diff --git a/bookwyrm/views/goal.py b/bookwyrm/views/goal.py index 105c8b90..b28c0476 100644 --- a/bookwyrm/views/goal.py +++ b/bookwyrm/views/goal.py @@ -41,7 +41,7 @@ class Goal(View): "year": year, "is_self": request.user == user, } - return TemplateResponse(request, "goal.html", data) + return TemplateResponse(request, "user/goal.html", data) def post(self, request, username, year): """update or create an annual goal""" @@ -58,7 +58,7 @@ class Goal(View): "goal": goal, "year": year, } - return TemplateResponse(request, "goal.html", data) + return TemplateResponse(request, "user/goal.html", data) goal = form.save() if request.POST.get("post-status"): diff --git a/bookwyrm/views/shelf.py b/bookwyrm/views/shelf.py index 6e75cc99..35f660b5 100644 --- a/bookwyrm/views/shelf.py +++ b/bookwyrm/views/shelf.py @@ -85,12 +85,14 @@ class Shelf(View): "shelves": shelves, "shelf": shelf, "books": page, + "edit_form": forms.ShelfForm(instance=shelf if shelf_identifier else None), + "create_form": forms.ShelfForm(), "page_range": paginated.get_elided_page_range( page.number, on_each_side=2, on_ends=1 ), } - return TemplateResponse(request, "user/shelf/shelf.html", data) + return TemplateResponse(request, "shelf/shelf.html", data) @method_decorator(login_required, name="dispatch") # pylint: disable=unused-argument @@ -128,7 +130,7 @@ def create_shelf(request): def delete_shelf(request, shelf_id): """user generated shelves""" shelf = get_object_or_404(models.Shelf, id=shelf_id) - shelf.raise_not_deletable() + shelf.raise_not_deletable(request.user) shelf.delete() return redirect("user-shelves", request.user.localname)