diff --git a/bookwyrm/activitypub/book.py b/bookwyrm/activitypub/book.py index ee4b8851..6fa80b32 100644 --- a/bookwyrm/activitypub/book.py +++ b/bookwyrm/activitypub/book.py @@ -63,5 +63,7 @@ class Author(ActivityObject): aliases: List[str] = field(default_factory=lambda: []) bio: str = '' openlibraryKey: str = '' + librarythingKey: str = '' + goodreadsKey: str = '' wikipediaLink: str = '' type: str = 'Person' diff --git a/bookwyrm/forms.py b/bookwyrm/forms.py index 1422b4b9..686ac8b1 100644 --- a/bookwyrm/forms.py +++ b/bookwyrm/forms.py @@ -30,6 +30,7 @@ class CustomForm(ModelForm): visible.field.widget.attrs['rows'] = None visible.field.widget.attrs['class'] = css_classes[input_type] + # pylint: disable=missing-class-docstring class LoginForm(CustomForm): class Meta: @@ -121,14 +122,13 @@ class EditionForm(CustomForm): model = models.Edition exclude = [ 'remote_id', + 'origin_id', 'created_date', 'updated_date', - 'last_sync_date', 'authors',# TODO 'parent_work', 'shelves', - 'misc_identifiers', 'subjects',# TODO 'subject_places',# TODO @@ -136,6 +136,16 @@ class EditionForm(CustomForm): 'connector', ] +class AuthorForm(CustomForm): + class Meta: + model = models.Author + exclude = [ + 'remote_id', + 'origin_id', + 'created_date', + 'updated_date', + ] + class ImportForm(forms.Form): csv_file = forms.FileField() diff --git a/bookwyrm/migrations/0029_auto_20201221_2014.py b/bookwyrm/migrations/0029_auto_20201221_2014.py new file mode 100644 index 00000000..ebf27a74 --- /dev/null +++ b/bookwyrm/migrations/0029_auto_20201221_2014.py @@ -0,0 +1,61 @@ +# Generated by Django 3.0.7 on 2020-12-21 20:14 + +import bookwyrm.models.fields +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('bookwyrm', '0028_remove_book_author_text'), + ] + + operations = [ + migrations.RemoveField( + model_name='author', + name='last_sync_date', + ), + migrations.RemoveField( + model_name='author', + name='sync', + ), + migrations.RemoveField( + model_name='book', + name='last_sync_date', + ), + migrations.RemoveField( + model_name='book', + name='sync', + ), + migrations.RemoveField( + model_name='book', + name='sync_cover', + ), + migrations.AddField( + model_name='author', + name='goodreads_key', + field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True), + ), + migrations.AddField( + model_name='author', + name='last_edited_by', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL), + ), + migrations.AddField( + model_name='author', + name='librarything_key', + field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True), + ), + migrations.AddField( + model_name='book', + name='last_edited_by', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL), + ), + migrations.AlterField( + model_name='author', + name='origin_id', + field=models.CharField(blank=True, max_length=255, null=True), + ), + ] diff --git a/bookwyrm/models/author.py b/bookwyrm/models/author.py index a2eac507..d0cb8d19 100644 --- a/bookwyrm/models/author.py +++ b/bookwyrm/models/author.py @@ -1,21 +1,15 @@ ''' database schema for info about authors ''' from django.db import models -from django.utils import timezone from bookwyrm import activitypub from bookwyrm.settings import DOMAIN -from .base_model import ActivitypubMixin, BookWyrmModel +from .book import BookDataModel from . import fields -class Author(ActivitypubMixin, BookWyrmModel): +class Author(BookDataModel): ''' basic biographic info ''' - origin_id = models.CharField(max_length=255, null=True) - openlibrary_key = fields.CharField( - max_length=255, blank=True, null=True, deduplication_field=True) - sync = models.BooleanField(default=True) - last_sync_date = models.DateTimeField(default=timezone.now) wikipedia_link = fields.CharField( max_length=255, blank=True, null=True, deduplication_field=True) # idk probably other keys would be useful here? @@ -27,15 +21,6 @@ class Author(ActivitypubMixin, BookWyrmModel): ) bio = fields.HtmlField(null=True, blank=True) - def save(self, *args, **kwargs): - ''' handle remote vs origin ids ''' - if self.id: - self.remote_id = self.get_remote_id() - else: - self.origin_id = self.remote_id - self.remote_id = None - return super().save(*args, **kwargs) - def get_remote_id(self): ''' editions and works both use "book" instead of model_name ''' return 'https://%s/author/%s' % (DOMAIN, self.id) diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index 21311d6c..7e45eacd 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -2,7 +2,6 @@ import re from django.db import models -from django.utils import timezone from model_utils.managers import InheritanceManager from bookwyrm import activitypub @@ -12,10 +11,9 @@ from .base_model import BookWyrmModel from .base_model import ActivitypubMixin, OrderedCollectionPageMixin from . import fields -class Book(ActivitypubMixin, BookWyrmModel): - ''' a generic book, which can mean either an edition or a work ''' +class BookDataModel(ActivitypubMixin, BookWyrmModel): + ''' fields shared between editable book data (books, works, authors) ''' origin_id = models.CharField(max_length=255, null=True, blank=True) - # these identifiers apply to both works and editions openlibrary_key = fields.CharField( max_length=255, blank=True, null=True, deduplication_field=True) librarything_key = fields.CharField( @@ -23,15 +21,28 @@ class Book(ActivitypubMixin, BookWyrmModel): goodreads_key = fields.CharField( max_length=255, blank=True, null=True, deduplication_field=True) - # info about where the data comes from and where/if to sync - sync = models.BooleanField(default=True) - sync_cover = models.BooleanField(default=True) - last_sync_date = models.DateTimeField(default=timezone.now) + last_edited_by = models.ForeignKey( + 'User', on_delete=models.PROTECT, null=True) + + class Meta: + ''' can't initialize this model, that wouldn't make sense ''' + abstract = True + + def save(self, *args, **kwargs): + ''' ensure that the remote_id is within this instance ''' + if self.id: + self.remote_id = self.get_remote_id() + else: + self.origin_id = self.remote_id + self.remote_id = None + return super().save(*args, **kwargs) + + +class Book(BookDataModel): + ''' a generic book, which can mean either an edition or a work ''' connector = models.ForeignKey( 'Connector', on_delete=models.PROTECT, null=True) - # TODO: edit history - # book/work metadata title = fields.CharField(max_length=255) sort_title = fields.CharField(max_length=255, blank=True, null=True) @@ -86,12 +97,6 @@ class Book(ActivitypubMixin, BookWyrmModel): ''' can't be abstract for query reasons, but you shouldn't USE it ''' if not isinstance(self, Edition) and not isinstance(self, Work): raise ValueError('Books should be added as Editions or Works') - - if self.id: - self.remote_id = self.get_remote_id() - else: - self.origin_id = self.remote_id - self.remote_id = None return super().save(*args, **kwargs) def get_remote_id(self): diff --git a/bookwyrm/outgoing.py b/bookwyrm/outgoing.py index 13df9026..00154cf4 100644 --- a/bookwyrm/outgoing.py +++ b/bookwyrm/outgoing.py @@ -375,9 +375,9 @@ def handle_unboost(user, status): broadcast(user, activity) -def handle_update_book(user, book): +def handle_update_book_data(user, item): ''' broadcast the news about our book ''' - broadcast(user, book.to_update_activity(user)) + broadcast(user, item.to_update_activity(user)) def handle_update_user(user): diff --git a/bookwyrm/templates/author.html b/bookwyrm/templates/author.html index 9a7a20ab..e51ef302 100644 --- a/bookwyrm/templates/author.html +++ b/bookwyrm/templates/author.html @@ -2,13 +2,31 @@ {% load bookwyrm_tags %} {% block content %}
-

{{ author.name }}

+
+
+

{{ author.name }}

+
+ {% if request.user.is_authenticated and perms.bookwyrm.edit_book %} +
+ + + Edit Author + + +
+ {% endif %} +
+
+
{% if author.bio %}

- {{ author.bio }} + {{ author.bio | to_markdown | safe }}

{% endif %} + {% if author.wikipedia_link %} +

Wikipedia

+ {% endif %}
diff --git a/bookwyrm/templates/edit_author.html b/bookwyrm/templates/edit_author.html new file mode 100644 index 00000000..b08aa983 --- /dev/null +++ b/bookwyrm/templates/edit_author.html @@ -0,0 +1,89 @@ +{% extends 'layout.html' %} +{% load humanize %} +{% block content %} +
+
+

+ Edit "{{ author.name }}" +

+
+ + + Close + + +
+
+
+

Added: {{ author.created_date | naturaltime }}

+

Updated: {{ author.updated_date | naturaltime }}

+

Last edited by: {{ author.last_edited_by.display_name }}

+
+
+ +{% if form.non_field_errors %} +
+

{{ form.non_field_errors }}

+
+{% endif %} + +
+ {% csrf_token %} + + +
+
+

Metadata

+

{{ form.name }}

+ {% for error in form.name.errors %} +

{{ error | escape }}

+ {% endfor %} + +

{{ form.bio }}

+ {% for error in form.bio.errors %} +

{{ error | escape }}

+ {% endfor %} + +

{{ form.wikipedia_link }}

+ {% for error in form.wikipedia_link.errors %} +

{{ error | escape }}

+ {% endfor %} + +

{{ form.born }}

+ {% for error in form.born.errors %} +

{{ error | escape }}

+ {% endfor %} + +

{{ form.died }}

+ {% for error in form.died.errors %} +

{{ error | escape }}

+ {% endfor %} +
+
+

Author Identifiers

+

{{ form.openlibrary_key }}

+ {% for error in form.openlibrary_key.errors %} +

{{ error | escape }}

+ {% endfor %} + +

{{ form.librarything_key }}

+ {% for error in form.librarything_key.errors %} +

{{ error | escape }}

+ {% endfor %} + +

{{ form.goodreads_key }}

+ {% for error in form.goodreads_key.errors %} +

{{ error | escape }}

+ {% endfor %} + +
+
+ +
+ + Cancel +
+
+ +{% endblock %} + diff --git a/bookwyrm/templates/edit_book.html b/bookwyrm/templates/edit_book.html index 54cefb0a..b730f3e4 100644 --- a/bookwyrm/templates/edit_book.html +++ b/bookwyrm/templates/edit_book.html @@ -17,63 +17,51 @@

Added: {{ book.created_date | naturaltime }}

Updated: {{ book.updated_date | naturaltime }}

+

Last edited by: {{ book.last_edited_by.display_name }}

-{% if login_form.non_field_errors %} +{% if form.non_field_errors %}
-

{{ login_form.non_field_errors }}

+

{{ form.non_field_errors }}

{% endif %}
{% csrf_token %} -
-

Data sync -

If sync is enabled, any changes will be over-written

-

-
-
- -
-
- -
-
-
- +

Metadata

-

{{ form.title }}

+

{{ form.title }}

{% for error in form.title.errors %}

{{ error | escape }}

{% endfor %} -

{{ form.sort_title }}

+

{{ form.sort_title }}

{% for error in form.sort_title.errors %}

{{ error | escape }}

{% endfor %} -

{{ form.subtitle }}

+

{{ form.subtitle }}

{% for error in form.subtitle.errors %}

{{ error | escape }}

{% endfor %} -

{{ form.description }}

+

{{ form.description }}

{% for error in form.description.errors %}

{{ error | escape }}

{% endfor %} -

{{ form.series }}

+

{{ form.series }}

{% for error in form.series.errors %}

{{ error | escape }}

{% endfor %} -

{{ form.series_number }}

+

{{ form.series_number }}

{% for error in form.series_number.errors %}

{{ error | escape }}

{% endfor %} -

{{ form.first_published_date }}

+

{{ form.first_published_date }}

{% for error in form.first_published_date.errors %}

{{ error | escape }}

{% endfor %} -

{{ form.published_date }}

+

{{ form.published_date }}

{% for error in form.published_date.errors %}

{{ error | escape }}

{% endfor %} @@ -97,7 +85,7 @@

Physical Properties

-

{{ form.physical_format }}

+

{{ form.physical_format }}

{% for error in form.physical_format.errors %}

{{ error | escape }}

{% endfor %} @@ -105,7 +93,7 @@

{{ error | escape }}

{% endfor %} -

{{ form.pages }}

+

{{ form.pages }}

{% for error in form.pages.errors %}

{{ error | escape }}

{% endfor %} @@ -113,23 +101,23 @@

Book Identifiers

-

{{ form.isbn_13 }}

+

{{ form.isbn_13 }}

{% for error in form.isbn_13.errors %}

{{ error | escape }}

{% endfor %} -

{{ form.isbn_10 }}

+

{{ form.isbn_10 }}

{% for error in form.isbn_10.errors %}

{{ error | escape }}

{% endfor %} -

{{ form.openlibrary_key }}

+

{{ form.openlibrary_key }}

{% for error in form.openlibrary_key.errors %}

{{ error | escape }}

{% endfor %} -

{{ form.librarything_key }}

+

{{ form.librarything_key }}

{% for error in form.librarything_key.errors %}

{{ error | escape }}

{% endfor %} -

{{ form.goodreads_key }}

+

{{ form.goodreads_key }}

{% for error in form.goodreads_key.errors %}

{{ error | escape }}

{% endfor %} diff --git a/bookwyrm/tests/test_view_actions.py b/bookwyrm/tests/test_view_actions.py index c1e6f860..c846c248 100644 --- a/bookwyrm/tests/test_view_actions.py +++ b/bookwyrm/tests/test_view_actions.py @@ -2,6 +2,8 @@ from unittest.mock import patch from django.core.exceptions import PermissionDenied +from django.contrib.auth.models import Group, Permission +from django.contrib.contenttypes.models import ContentType from django.http.response import Http404 from django.test import TestCase from django.test.client import RequestFactory @@ -19,6 +21,13 @@ class ViewActions(TestCase): 'mouse', 'mouse@mouse.com', 'mouseword', local=True) self.local_user.remote_id = 'https://example.com/user/mouse' self.local_user.save() + self.group = Group.objects.create(name='editor') + self.group.permissions.add( + Permission.objects.create( + name='edit_book', + codename='edit_book', + content_type=ContentType.objects.get_for_model(models.User)).id + ) with patch('bookwyrm.models.user.set_remote_server.delay'): self.remote_user = models.User.objects.create_user( 'rat', 'rat@rat.com', 'ratword', @@ -248,7 +257,7 @@ class ViewActions(TestCase): }) request.user = self.local_user with patch('bookwyrm.view_actions.login'): - resp = actions.password_change(request) + actions.password_change(request) self.assertNotEqual(self.local_user.password, password_hash) def test_password_change_mismatch(self): @@ -259,7 +268,7 @@ class ViewActions(TestCase): 'confirm-password': 'hihi' }) request.user = self.local_user - resp = actions.password_change(request) + actions.password_change(request) self.assertEqual(self.local_user.password, password_hash) @@ -299,3 +308,46 @@ class ViewActions(TestCase): self.assertEqual(models.ShelfBook.objects.get().book, edition2) self.assertEqual(models.ReadThrough.objects.get().book, edition2) + + + def test_edit_author(self): + ''' edit an author ''' + author = models.Author.objects.create(name='Test Author') + self.local_user.groups.add(self.group) + form = forms.AuthorForm(instance=author) + form.data['name'] = 'New Name' + form.data['last_edited_by'] = self.local_user.id + request = self.factory.post('', form.data) + request.user = self.local_user + with patch('bookwyrm.broadcast.broadcast_task.delay'): + actions.edit_author(request, author.id) + author.refresh_from_db() + self.assertEqual(author.name, 'New Name') + self.assertEqual(author.last_edited_by, self.local_user) + + def test_edit_author_non_editor(self): + ''' edit an author with invalid post data''' + author = models.Author.objects.create(name='Test Author') + form = forms.AuthorForm(instance=author) + form.data['name'] = 'New Name' + form.data['last_edited_by'] = self.local_user.id + request = self.factory.post('', form.data) + request.user = self.local_user + with self.assertRaises(PermissionDenied): + actions.edit_author(request, author.id) + author.refresh_from_db() + self.assertEqual(author.name, 'Test Author') + + def test_edit_author_invalid_form(self): + ''' edit an author with invalid post data''' + author = models.Author.objects.create(name='Test Author') + self.local_user.groups.add(self.group) + form = forms.AuthorForm(instance=author) + form.data['name'] = '' + form.data['last_edited_by'] = self.local_user.id + request = self.factory.post('', form.data) + request.user = self.local_user + resp = actions.edit_author(request, author.id) + author.refresh_from_db() + self.assertEqual(author.name, 'Test Author') + self.assertEqual(resp.template_name, 'edit_author.html') diff --git a/bookwyrm/urls.py b/bookwyrm/urls.py index e6c3f79f..22edd38a 100644 --- a/bookwyrm/urls.py +++ b/bookwyrm/urls.py @@ -76,6 +76,7 @@ urlpatterns = [ # books re_path(r'%s(.json)?/?$' % book_path, views.book_page), re_path(r'%s/edit/?$' % book_path, views.edit_book_page), + re_path(r'^author/(?P[\w\-]+)/edit/?$', views.edit_author_page), re_path(r'%s/editions(.json)?/?$' % book_path, views.editions_page), re_path(r'^author/(?P[\w\-]+)(.json)?/?$', views.author_page), @@ -104,6 +105,7 @@ urlpatterns = [ re_path(r'^edit-book/(?P\d+)/?$', actions.edit_book), re_path(r'^upload-cover/(?P\d+)/?$', actions.upload_cover), re_path(r'^add-description/(?P\d+)/?$', actions.add_description), + re_path(r'^edit-author/(?P\d+)/?$', actions.edit_author), re_path(r'^switch-edition/?$', actions.switch_edition), re_path(r'^edit-readthrough/?$', actions.edit_readthrough), diff --git a/bookwyrm/view_actions.py b/bookwyrm/view_actions.py index 5a7a059d..78f76218 100644 --- a/bookwyrm/view_actions.py +++ b/bookwyrm/view_actions.py @@ -235,7 +235,7 @@ def edit_book(request, book_id): return TemplateResponse(request, 'edit_book.html', data) book = form.save() - outgoing.handle_update_book(request.user, book) + outgoing.handle_update_book_data(request.user, book) return redirect('/book/%s' % book.id) @@ -280,10 +280,9 @@ def upload_cover(request, book_id): return redirect('/book/%d' % book.id) book.cover = form.files['cover'] - book.sync_cover = False book.save() - outgoing.handle_update_book(request.user, book) + outgoing.handle_update_book_data(request.user, book) return redirect('/book/%s' % book.id) @@ -302,10 +301,31 @@ def add_description(request, book_id): book.description = description book.save() - outgoing.handle_update_book(request.user, book) + outgoing.handle_update_book_data(request.user, book) return redirect('/book/%s' % book.id) +@login_required +@permission_required('bookwyrm.edit_book', raise_exception=True) +@require_POST +def edit_author(request, author_id): + ''' edit a author cool ''' + author = get_object_or_404(models.Author, id=author_id) + + form = forms.AuthorForm(request.POST, request.FILES, instance=author) + if not form.is_valid(): + data = { + 'title': 'Edit Author', + 'author': author, + 'form': form + } + return TemplateResponse(request, 'edit_author.html', data) + author = form.save() + + outgoing.handle_update_book_data(request.user, author) + return redirect('/author/%s' % author.id) + + @login_required @require_POST def create_shelf(request): diff --git a/bookwyrm/views.py b/bookwyrm/views.py index 8e40f362..1afb8266 100644 --- a/bookwyrm/views.py +++ b/bookwyrm/views.py @@ -657,6 +657,20 @@ def edit_book_page(request, book_id): return TemplateResponse(request, 'edit_book.html', data) +@login_required +@permission_required('bookwyrm.edit_book', raise_exception=True) +@require_GET +def edit_author_page(request, author_id): + ''' info about a book ''' + author = get_object_or_404(models.Author, id=author_id) + data = { + 'title': 'Edit Author', + 'author': author, + 'form': forms.AuthorForm(instance=author) + } + return TemplateResponse(request, 'edit_author.html', data) + + @require_GET def editions_page(request, book_id): ''' list of editions of a book '''