Merge pull request #416 from mouse-reeve/book-data-model

Remove sync fields and share fields between book and author

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'

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()

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),
+        ),
+    ]

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)

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):

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):

bookwyrm/templates/author.html

@@ -2,13 +2,31 @@
 {% load bookwyrm_tags %}
 {% block content %}
 <div class="block">
+    <div class="columns">
+        <div class="column">
             <h1 class="title">{{ author.name }}</h1>
+        </div>
+        {% if request.user.is_authenticated and perms.bookwyrm.edit_book %}
+        <div class="column is-narrow">
+            <a href="/author/{{ author.id }}/edit">
+                <span class="icon icon-pencil">
+                    <span class="is-sr-only">Edit Author</span>
+                </span>
+            </a>
+        </div>
+        {% endif %}
+    </div>
+</div>
 
+<div class="block">
     {% if author.bio %}
     <p>
-        {{ author.bio }}
+        {{ author.bio | to_markdown | safe }}
     </p>
     {% endif %}
+    {% if author.wikipedia_link %}
+    <p><a href="{{ author.wikipedia_link }}" rel=”noopener” target="_blank">Wikipedia</a></p>
+    {% endif %}
 </div>
 
 <div class="block">

bookwyrm/templates/edit_author.html

@@ -0,0 +1,89 @@
+{% extends 'layout.html' %}
+{% load humanize %}
+{% block content %}
+<div class="block">
+    <div class="level">
+        <h1 class="title level-left">
+            Edit "{{ author.name }}"
+        </h1>
+        <div class="level-right">
+            <a href="/author/{{ author.id }}">
+                <span class="edit-link icon icon-close">
+                    <span class="is-sr-only">Close</span>
+                </span>
+            </a>
+        </div>
+    </div>
+    <div>
+        <p>Added: {{ author.created_date | naturaltime }}</p>
+        <p>Updated: {{ author.updated_date | naturaltime }}</p>
+        <p>Last edited by: <a href="{{ author.last_edited_by.remote_id }}">{{ author.last_edited_by.display_name }}</a></p>
+    </div>
+</div>
+
+{% if form.non_field_errors %}
+<div class="block">
+    <p class="notification is-danger">{{ form.non_field_errors }}</p>
+</div>
+{% endif %}
+
+<form class="block" name="edit-author" action="/edit-author/{{ author.id }}" method="post">
+    {% csrf_token %}
+    <input type="hidden" name="last_edited_by" value="{{ request.user.id }}">
+
+    <div class="columns">
+        <div class="column">
+            <h2 class="title is-4">Metadata</h2>
+            <p><label class="label" for="id_name">Name:</label> {{ form.name }}</p>
+            {% for error in form.name.errors %}
+            <p class="help is-danger">{{ error | escape }}</p>
+            {% endfor %}
+
+            <p><label class="label" for="id_bio">Bio:</label> {{ form.bio }}</p>
+            {% for error in form.bio.errors %}
+            <p class="help is-danger">{{ error | escape }}</p>
+            {% endfor %}
+
+            <p><label class="label" for="id_wikipedia_link">Wikipedia link:</label> {{ form.wikipedia_link }}</p>
+            {% for error in form.wikipedia_link.errors %}
+            <p class="help is-danger">{{ error | escape }}</p>
+            {% endfor %}
+
+            <p><label class="label" for="id_born">Birth date:</label> {{ form.born }}</p>
+            {% for error in form.born.errors %}
+            <p class="help is-danger">{{ error | escape }}</p>
+            {% endfor %}
+
+            <p><label class="label" for="id_died">Death date:</label> {{ form.died }}</p>
+            {% for error in form.died.errors %}
+            <p class="help is-danger">{{ error | escape }}</p>
+            {% endfor %}
+        </div>
+        <div class="column">
+            <h2 class="title is-4">Author Identifiers</h2>
+            <p><label class="label" for="id_openlibrary_key">Openlibrary key:</label> {{ form.openlibrary_key }}</p>
+            {% for error in form.openlibrary_key.errors %}
+            <p class="help is-danger">{{ error | escape }}</p>
+            {% endfor %}
+
+            <p><label class="label" for="id_librarything_key">Librarything key:</label> {{ form.librarything_key }}</p>
+            {% for error in form.librarything_key.errors %}
+            <p class="help is-danger">{{ error | escape }}</p>
+            {% endfor %}
+
+            <p><label class="label" for="id_goodreads_key">Goodreads key:</label> {{ form.goodreads_key }}</p>
+            {% for error in form.goodreads_key.errors %}
+            <p class="help is-danger">{{ error | escape }}</p>
+            {% endfor %}
+
+        </div>
+    </div>
+
+    <div class="block">
+        <button class="button is-primary" type="submit">Save</button>
+        <a class="button" href="/author/{{ author.id }}">Cancel</a>
+    </div>
+</form>
+
+{% endblock %}
+

bookwyrm/templates/edit_book.html

@@ -17,31 +17,19 @@
     <div>
         <p>Added: {{ book.created_date | naturaltime }}</p>
         <p>Updated: {{ book.updated_date | naturaltime }}</p>
+        <p>Last edited by: <a href="{{ book.last_edited_by.remote_id }}">{{ book.last_edited_by.display_name }}</a></p>
     </div>
 </div>
 
-{% if login_form.non_field_errors %}
+{% if form.non_field_errors %}
 <div class="block">
-    <p class="notification is-danger">{{ login_form.non_field_errors }}</p>
+    <p class="notification is-danger">{{ form.non_field_errors }}</p>
 </div>
 {% endif %}
 
 <form class="block" name="edit-book" action="/edit-book/{{ book.id }}" method="post" enctype="multipart/form-data">
     {% csrf_token %}
-    <div class="block">
-        <h2 class="title is-4">Data sync
-            <p class="subtitle is-6">If sync is enabled, any changes will be over-written</p>
-        </h2>
-        <div class="columns">
-            <div class="column is-narrow">
-                <label class="checkbox" for="id_sync"><input class="checkbox" type="checkbox" name="sync" id="id_sync"> Sync</label>
-            </div>
-            <div class="column is-narrow">
-                <label class="checkbox" for="id_sync_cover"><input class="checkbox" type="checkbox" name="sync_cover" id="id_sync_cover"> Sync cover</label>
-            </div>
-        </div>
-    </div>
-
+    <input type="hidden" name="last_edited_by" value="{{ request.user.id }}">
     <div class="columns">
         <div class="column">
             <h2 class="title is-4">Metadata</h2>

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')

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<author_id>[\w\-]+)/edit/?$', views.edit_author_page),
     re_path(r'%s/editions(.json)?/?$' % book_path, views.editions_page),
 
     re_path(r'^author/(?P<author_id>[\w\-]+)(.json)?/?$', views.author_page),
@@ -104,6 +105,7 @@ urlpatterns = [
     re_path(r'^edit-book/(?P<book_id>\d+)/?$', actions.edit_book),
     re_path(r'^upload-cover/(?P<book_id>\d+)/?$', actions.upload_cover),
     re_path(r'^add-description/(?P<book_id>\d+)/?$', actions.add_description),
+    re_path(r'^edit-author/(?P<author_id>\d+)/?$', actions.edit_author),
 
     re_path(r'^switch-edition/?$', actions.switch_edition),
     re_path(r'^edit-readthrough/?$', actions.edit_readthrough),

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):

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 '''