Merge branch 'main' into progress_update

.env.example

@@ -1,5 +1,5 @@
 # SECURITY WARNING: keep the secret key used in production secret!
-SECRET_KEY=7(2w1sedok=aznpq)ta1mc4i%4h=xx@hxwx*o57ctsuml0x%fr
+SECRET_KEY="7(2w1sedok=aznpq)ta1mc4i%4h=xx@hxwx*o57ctsuml0x%fr"
 
 # SECURITY WARNING: don't run with debug turned on in production!
 DEBUG=true
@@ -25,7 +25,7 @@ POSTGRES_HOST=db
 CELERY_BROKER=redis://redis:6379/0
 CELERY_RESULT_BACKEND=redis://redis:6379/0
 
-EMAIL_HOST='smtp.mailgun.org'
+EMAIL_HOST="smtp.mailgun.org"
 EMAIL_PORT=587
 EMAIL_HOST_USER=mail@your.domain.here
 EMAIL_HOST_PASSWORD=emailpassword123

.github/workflows/django-tests.yml

@@ -0,0 +1,68 @@
+name: Run Python Tests
+on:
+  push:
+    branches: [ main ]
+  pull_request:
+    branches: [ main ]
+
+jobs:
+  build:
+
+    runs-on: ubuntu-20.04
+    strategy:
+      max-parallel: 4
+      matrix:
+        db: [postgres]
+        python-version: [3.9]
+        include:
+          - db: postgres
+            db_port: 5432
+
+    services:
+      postgres:
+        image: postgres:10
+        env:
+          POSTGRES_USER: postgres
+          POSTGRES_PASSWORD: hunter2
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+        ports:
+          - 5432:5432
+    steps:
+    - uses: actions/checkout@v2
+    - name: Set up Python ${{ matrix.python-version }}
+      uses: actions/setup-python@v2
+      with:
+        python-version: ${{ matrix.python-version }}
+    - name: Install Dependencies
+      run: |
+        python -m pip install --upgrade pip
+        pip install -r requirements.txt
+    - name: Run Tests
+      env:
+        DB: ${{ matrix.db }}
+        DB_HOST: 127.0.0.1
+        DB_PORT: ${{ matrix.db_port }}
+        DB_PASSWORD: hunter2
+        SECRET_KEY: beepbeep
+        DEBUG: true
+        DOMAIN: your.domain.here
+        OL_URL: https://openlibrary.org
+        BOOKWYRM_DATABASE_BACKEND: postgres
+        MEDIA_ROOT: images/
+        POSTGRES_PASSWORD: hunter2
+        POSTGRES_USER: postgres
+        POSTGRES_DB: github_actions
+        POSTGRES_HOST: 127.0.0.1
+        CELERY_BROKER: ""
+        CELERY_RESULT_BACKEND: ""
+        EMAIL_HOST: "smtp.mailgun.org"
+        EMAIL_PORT: 587
+        EMAIL_HOST_USER: ""
+        EMAIL_HOST_PASSWORD: ""
+        EMAIL_USE_TLS: true
+      run: |
+        python manage.py test

bookwyrm/activitypub/__init__.py

@@ -5,6 +5,8 @@ import sys
 from .base_activity import ActivityEncoder, Image, PublicKey, Signature
 from .base_activity import Link, Mention
 from .base_activity import ActivitySerializerError
+from .base_activity import tag_formatter
+from .base_activity import image_formatter, image_attachments_formatter
 from .note import Note, GeneratedNote, Article, Comment, Review, Quotation
 from .note import Tombstone
 from .interaction import Boost, Like

bookwyrm/activitypub/base_activity.py

@@ -1,12 +1,17 @@
 ''' basics for an activitypub serializer '''
 from dataclasses import dataclass, fields, MISSING
 from json import JSONEncoder
+from uuid import uuid4
+
+from django.core.files.base import ContentFile
+from django.db.models.fields.related_descriptors \
+        import ForwardManyToOneDescriptor, ManyToManyDescriptor, \
+        ReverseManyToOneDescriptor
+from django.db.models.fields.files import ImageFileDescriptor
+import requests
 
 from bookwyrm import books_manager, models
 
-from django.db.models.fields.related_descriptors \
-        import ForwardManyToOneDescriptor
-
 
 class ActivitySerializerError(ValueError):
     ''' routine problems serializing activitypub json '''
@@ -69,7 +74,8 @@ class ActivityObject:
             try:
                 value = kwargs[field.name]
             except KeyError:
-                if field.default == MISSING:
+                if field.default == MISSING and \
+                        field.default_factory == MISSING:
                     raise ActivitySerializerError(\
                             'Missing required field: %s' % field.name)
                 value = field.default
@@ -90,6 +96,9 @@ class ActivityObject:
 
         model_fields = [m.name for m in model._meta.get_fields()]
         mapped_fields = {}
+        many_to_many_fields = {}
+        one_to_many_fields = {}
+        image_fields = {}
 
         for mapping in model.activity_mappings:
             if mapping.model_key not in model_fields:
@@ -101,23 +110,56 @@ class ActivityObject:
                 value = getattr(self, mapping.activity_key)
             model_field = getattr(model, mapping.model_key)
 
-            # remote_id -> foreign key resolver
+            formatted_value = mapping.model_formatter(value)
-            if isinstance(model_field, ForwardManyToOneDescriptor) and value:
+            if isinstance(model_field, ForwardManyToOneDescriptor) and \
+                    formatted_value:
+                # foreign key remote id reolver
                 fk_model = model_field.field.related_model
-                value = resolve_foreign_key(fk_model, value)
+                reference = resolve_foreign_key(fk_model, formatted_value)
+                mapped_fields[mapping.model_key] = reference
+            elif isinstance(model_field, ManyToManyDescriptor):
+                many_to_many_fields[mapping.model_key] = formatted_value
+            elif isinstance(model_field, ReverseManyToOneDescriptor):
+                # attachments on statuses, for example
+                one_to_many_fields[mapping.model_key] = formatted_value
+            elif isinstance(model_field, ImageFileDescriptor):
+                # image fields need custom handling
+                image_fields[mapping.model_key] = formatted_value
+            else:
+                mapped_fields[mapping.model_key] = formatted_value
 
-            mapped_fields[mapping.model_key] = mapping.model_formatter(value)
-
-
-        # updating an existing model isntance
         if instance:
+            # updating an existing model isntance
             for k, v in mapped_fields.items():
                 setattr(instance, k, v)
             instance.save()
-            return instance
+        else:
-
             # creating a new model instance
-        return model.objects.create(**mapped_fields)
+            instance = model.objects.create(**mapped_fields)
+
+        # add many-to-many fields
+        for (model_key, values) in many_to_many_fields.items():
+            getattr(instance, model_key).set(values)
+        instance.save()
+
+        # add images
+        for (model_key, value) in image_fields.items():
+            if not value:
+                continue
+            getattr(instance, model_key).save(*value, save=True)
+
+        # add one to many fields
+        for (model_key, values) in one_to_many_fields.items():
+            items = []
+            for item in values:
+                # the reference id wasn't available at creation time
+                setattr(item, instance.__class__.__name__.lower(), instance)
+                item.save()
+                items.append(item)
+            if items:
+                getattr(instance, model_key).set(items)
+                instance.save()
+        return instance
 
 
     def serialize(self):
@@ -129,7 +171,7 @@ class ActivityObject:
 
 def resolve_foreign_key(model, remote_id):
     ''' look up the remote_id on an activity json field '''
-    if model in [models.Edition, models.Work]:
+    if model in [models.Edition, models.Work, models.Book]:
         return books_manager.get_or_create_book(remote_id)
 
     result = model.objects
@@ -145,3 +187,62 @@ def resolve_foreign_key(model, remote_id):
             'Could not resolve remote_id in %s model: %s' % \
                 (model.__name__, remote_id))
     return result
+
+
+def tag_formatter(tags, tag_type):
+    ''' helper function to extract foreign keys from tag activity json '''
+    if not isinstance(tags, list):
+        return []
+    items = []
+    types = {
+        'Book': models.Book,
+        'Mention': models.User,
+    }
+    for tag in [t for t in tags if t.get('type') == tag_type]:
+        if not tag_type in types:
+            continue
+        remote_id = tag.get('href')
+        try:
+            item = resolve_foreign_key(types[tag_type], remote_id)
+        except ActivitySerializerError:
+            continue
+        items.append(item)
+    return items
+
+
+def image_formatter(image_json):
+    ''' helper function to load images and format them for a model '''
+    if isinstance(image_json, list):
+        try:
+            image_json = image_json[0]
+        except IndexError:
+            return None
+
+    if not image_json or not hasattr(image_json, 'url'):
+        return None
+    url = image_json.get('url')
+
+    try:
+        response = requests.get(url)
+    except ConnectionError:
+        return None
+    if not response.ok:
+        return None
+
+    image_name = str(uuid4()) + '.' + url.split('.')[-1]
+    image_content = ContentFile(response.content)
+    return [image_name, image_content]
+
+
+def image_attachments_formatter(images_json):
+    ''' deserialize a list of images '''
+    attachments = []
+    for image in images_json:
+        caption = image.get('name')
+        attachment = models.Attachment(caption=caption)
+        image_field = image_formatter(image)
+        if not image_field:
+            continue
+        attachment.image.save(*image_field, save=False)
+        attachments.append(attachment)
+    return attachments

bookwyrm/activitypub/book.py

@@ -25,7 +25,7 @@ class Book(ActivityObject):
     librarything_key: str
     goodreads_key: str
 
-    attachment: List[Image] = field(default=lambda: [])
+    attachment: List[Image] = field(default_factory=lambda: [])
     type: str = 'Book'
 
 
@@ -56,10 +56,10 @@ class Work(Book):
 class Author(ActivityObject):
     ''' author of a book '''
     name: str
-    born: str
+    born: str = ''
-    died: str
+    died: str = ''
-    aliases: str
+    aliases: str = ''
-    bio: str
+    bio: str = ''
-    openlibrary_key: str
+    openlibraryKey: str = ''
-    wikipedia_link: str
+    wikipediaLink: str = ''
     type: str = 'Person'

bookwyrm/activitypub/note.py

@@ -24,8 +24,8 @@ class Note(ActivityObject):
     cc: List[str]
     content: str
     replies: Dict
-    tag: List[Link] = field(default=lambda: [])
+    tag: List[Link] = field(default_factory=lambda: [])
-    attachment: List[Image] = field(default=lambda: [])
+    attachment: List[Image] = field(default_factory=lambda: [])
     sensitive: bool = False
     type: str = 'Note'
 

bookwyrm/activitypub/person.py

@@ -15,7 +15,7 @@ class Person(ActivityObject):
     summary: str
     publicKey: PublicKey
     endpoints: Dict
-    icon: Image = field(default=lambda: {})
+    icon: Image = field(default_factory=lambda: {})
     bookwyrmUser: bool = False
     manuallyApprovesFollowers: str = False
     discoverable: str = True

bookwyrm/connectors/abstract_connector.py

@@ -157,7 +157,7 @@ class AbstractConnector(ABC):
 
     def update_book_from_data(self, book, data, update_cover=True):
         ''' for creating a new book or syncing with data '''
-        book = update_from_mappings(book, data, self.book_mappings)
+        book = self.update_from_mappings(book, data, self.book_mappings)
 
         author_text = []
         for author in self.get_authors_from_data(data):
@@ -262,7 +262,7 @@ class AbstractConnector(ABC):
         ''' get more info on a book '''
 
 
-def update_from_mappings(obj, data, mappings):
+    def update_from_mappings(self, obj, data, mappings):
         ''' assign data to model with mappings '''
         for mapping in mappings:
             # check if this field is present in the data

bookwyrm/connectors/bookwyrm_connector.py

@@ -1,55 +1,21 @@
 ''' using another bookwyrm instance as a source of book data '''
-from uuid import uuid4
-
-from django.core.exceptions import ObjectDoesNotExist
-from django.core.files.base import ContentFile
 from django.db import transaction
-import requests
 
-from bookwyrm import models
+from bookwyrm import activitypub, models
-from .abstract_connector import AbstractConnector, SearchResult, Mapping
+from .abstract_connector import AbstractConnector, SearchResult
-from .abstract_connector import update_from_mappings, get_date, get_data
+from .abstract_connector import get_data
 
 
 class Connector(AbstractConnector):
     ''' interact with other instances '''
-    def __init__(self, identifier):
-        super().__init__(identifier)
-        self.key_mappings = [
-            Mapping('isbn_13', model=models.Edition),
-            Mapping('isbn_10', model=models.Edition),
-            Mapping('lccn', model=models.Work),
-            Mapping('oclc_number', model=models.Edition),
-            Mapping('openlibrary_key'),
-            Mapping('goodreads_key'),
-            Mapping('asin'),
-        ]
 
-        self.book_mappings = self.key_mappings + [
+    def update_from_mappings(self, obj, data, mappings):
-            Mapping('sort_title'),
+        ''' serialize book data into a model '''
-            Mapping('subtitle'),
+        if self.is_work_data(data):
-            Mapping('description'),
+            work_data = activitypub.Work(**data)
-            Mapping('languages'),
+            return work_data.to_model(models.Work, instance=obj)
-            Mapping('series'),
+        edition_data = activitypub.Edition(**data)
-            Mapping('series_number'),
+        return edition_data.to_model(models.Edition, instance=obj)
-            Mapping('subjects'),
-            Mapping('subject_places'),
-            Mapping('first_published_date'),
-            Mapping('published_date'),
-            Mapping('pages'),
-            Mapping('physical_format'),
-            Mapping('publishers'),
-        ]
-
-        self.author_mappings = [
-            Mapping('name'),
-            Mapping('bio'),
-            Mapping('openlibrary_key'),
-            Mapping('wikipedia_link'),
-            Mapping('aliases'),
-            Mapping('born', formatter=get_date),
-            Mapping('died', formatter=get_date),
-        ]
 
 
     def get_remote_id_from_data(self, data):
@@ -57,7 +23,7 @@ class Connector(AbstractConnector):
 
 
     def is_work_data(self, data):
-        return data['type'] == 'Work'
+        return data.get('type') == 'Work'
 
 
     def get_edition_from_work_data(self, data):
@@ -71,47 +37,21 @@ class Connector(AbstractConnector):
 
 
     def get_authors_from_data(self, data):
-        for author_url in data.get('authors', []):
+        ''' load author data '''
-            yield self.get_or_create_author(author_url)
+        for author_id in data.get('authors', []):
+            try:
+                yield models.Author.objects.get(origin_id=author_id)
+            except models.Author.DoesNotExist:
+                pass
+            data = get_data(author_id)
+            author_data = activitypub.Author(**data)
+            author = author_data.to_model(models.Author)
+            yield author
 
 
     def get_cover_from_data(self, data):
-        cover_data = data.get('attachment')
-        if not cover_data:
-            return None
-        try:
-            cover_url = cover_data[0].get('url')
-        except IndexError:
-            return None
-        try:
-            response = requests.get(cover_url)
-        except ConnectionError:
-            return None
-
-        if not response.ok:
-            return None
-
-        image_name = str(uuid4()) + '.' + cover_url.split('.')[-1]
-        image_content = ContentFile(response.content)
-        return [image_name, image_content]
-
-
-    def get_or_create_author(self, remote_id):
-        ''' load that author '''
-        try:
-            return models.Author.objects.get(origin_id=remote_id)
-        except ObjectDoesNotExist:
         pass
 
-        data = get_data(remote_id)
-
-        # ingest a new author
-        author = models.Author(origin_id=remote_id)
-        author = update_from_mappings(author, data, self.author_mappings)
-        author.save()
-
-        return author
-
 
     def parse_search_data(self, data):
         return data

bookwyrm/connectors/openlibrary.py

@@ -7,7 +7,6 @@ from django.core.files.base import ContentFile
 from bookwyrm import models
 from .abstract_connector import AbstractConnector, SearchResult, Mapping
 from .abstract_connector import ConnectorException
-from .abstract_connector import update_from_mappings
 from .abstract_connector import get_date, get_data
 from .openlibrary_languages import languages
 
@@ -185,7 +184,7 @@ class Connector(AbstractConnector):
         data = get_data(url)
 
         author = models.Author(openlibrary_key=olkey)
-        author = update_from_mappings(author, data, self.author_mappings)
+        author = self.update_from_mappings(author, data, self.author_mappings)
         name = data.get('name')
         # TODO this is making some BOLD assumption
         if name:

bookwyrm/incoming.py

@@ -253,7 +253,6 @@ def handle_delete_status(activity):
     status_builder.delete_status(status)
 
 
-
 @app.task
 def handle_favorite(activity):
     ''' approval of your good good post '''

bookwyrm/migrations/0012_attachment.py

@@ -0,0 +1,29 @@
+# Generated by Django 3.0.7 on 2020-11-24 19:39
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('bookwyrm', '0011_auto_20201113_1727'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='Attachment',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('created_date', models.DateTimeField(auto_now_add=True)),
+                ('updated_date', models.DateTimeField(auto_now=True)),
+                ('remote_id', models.CharField(max_length=255, null=True)),
+                ('image', models.ImageField(blank=True, null=True, upload_to='status/')),
+                ('caption', models.TextField(blank=True, null=True)),
+                ('status', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='attachments', to='bookwyrm.Status')),
+            ],
+            options={
+                'abstract': False,
+            },
+        ),
+    ]

bookwyrm/migrations/0013_book_origin_id.py

@@ -0,0 +1,18 @@
+# Generated by Django 3.0.7 on 2020-11-24 21:42
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('bookwyrm', '0012_attachment'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='book',
+            name='origin_id',
+            field=models.CharField(blank=True, max_length=255, null=True),
+        ),
+    ]

bookwyrm/models/__init__.py

@@ -2,12 +2,13 @@
 import inspect
 import sys
 
-from .book import Book, Work, Edition, Author
+from .book import Book, Work, Edition
+from .author import Author
 from .connector import Connector
 from .relationship import UserFollows, UserFollowRequest, UserBlocks
 from .shelf import Shelf, ShelfBook
 from .status import Status, GeneratedNote, Review, Comment, Quotation
-from .status import Favorite, Boost, Notification, ReadThrough, ProgressMode, ProgressUpdate
+from .status import Attachment, Favorite, Boost, Notification, ReadThrough, ProgressMode, ProgressUpdate
 from .tag import Tag
 from .user import User
 from .federated_server import FederatedServer

bookwyrm/models/author.py

@@ -0,0 +1,50 @@
+''' database schema for info about authors '''
+from django.db import models
+from django.utils import timezone
+
+from bookwyrm import activitypub
+from bookwyrm.utils.fields import ArrayField
+
+from .base_model import ActivitypubMixin, ActivityMapping, BookWyrmModel
+
+
+class Author(ActivitypubMixin, BookWyrmModel):
+    ''' basic biographic info '''
+    origin_id = models.CharField(max_length=255, null=True)
+    ''' copy of an author from OL '''
+    openlibrary_key = models.CharField(max_length=255, blank=True, null=True)
+    sync = models.BooleanField(default=True)
+    last_sync_date = models.DateTimeField(default=timezone.now)
+    wikipedia_link = models.CharField(max_length=255, blank=True, null=True)
+    # idk probably other keys would be useful here?
+    born = models.DateTimeField(blank=True, null=True)
+    died = models.DateTimeField(blank=True, null=True)
+    name = models.CharField(max_length=255)
+    last_name = models.CharField(max_length=255, blank=True, null=True)
+    first_name = models.CharField(max_length=255, blank=True, null=True)
+    aliases = ArrayField(
+        models.CharField(max_length=255), blank=True, default=list
+    )
+    bio = models.TextField(null=True, blank=True)
+
+    @property
+    def display_name(self):
+        ''' Helper to return a displayable name'''
+        if self.name:
+            return self.name
+        # don't want to return a spurious space if all of these are None
+        if self.first_name and self.last_name:
+            return self.first_name + ' ' + self.last_name
+        return self.last_name or self.first_name
+
+    activity_mappings = [
+        ActivityMapping('id', 'remote_id'),
+        ActivityMapping('name', 'name'),
+        ActivityMapping('born', 'born'),
+        ActivityMapping('died', 'died'),
+        ActivityMapping('aliases', 'aliases'),
+        ActivityMapping('bio', 'bio'),
+        ActivityMapping('openlibraryKey', 'openlibrary_key'),
+        ActivityMapping('wikipediaLink', 'wikipedia_link'),
+    ]
+    activity_serializer = activitypub.Author

bookwyrm/models/base_model.py

@@ -59,20 +59,34 @@ class ActivitypubMixin:
     def to_activity(self, pure=False):
         ''' convert from a model to an activity '''
         if pure:
+            # works around bookwyrm-specific fields for vanilla AP services
             mappings = self.pure_activity_mappings
         else:
+            # may include custom fields that bookwyrm instances will understand
             mappings = self.activity_mappings
 
         fields = {}
         for mapping in mappings:
             if not hasattr(self, mapping.model_key) or not mapping.activity_key:
+                # this field on the model isn't serialized
                 continue
             value = getattr(self, mapping.model_key)
             if hasattr(value, 'remote_id'):
+                # this is probably a foreign key field, which we want to
+                # serialize as just the remote_id url reference
                 value = value.remote_id
-            if isinstance(value, datetime):
+            elif isinstance(value, datetime):
                 value = value.isoformat()
-            fields[mapping.activity_key] = mapping.activity_formatter(value)
+
+            # run the custom formatter function set in the model
+            result = mapping.activity_formatter(value)
+            if mapping.activity_key in fields and \
+                    isinstance(fields[mapping.activity_key], list):
+                # there can be two database fields that map to the same AP list
+                # this happens in status tags, which combines user and book tags
+                fields[mapping.activity_key] += result
+            else:
+                fields[mapping.activity_key] = result
 
         if pure:
             return self.pure_activity_serializer(
@@ -242,3 +256,32 @@ class ActivityMapping:
     model_key: str
     activity_formatter: Callable = lambda x: x
     model_formatter: Callable = lambda x: x
+
+
+def tag_formatter(items, name_field, activity_type):
+    ''' helper function to format lists of foreign keys into Tags '''
+    tags = []
+    for item in items.all():
+        tags.append(activitypub.Link(
+            href=item.remote_id,
+            name=getattr(item, name_field),
+            type=activity_type
+        ))
+    return tags
+
+
+def image_formatter(image, default_path=None):
+    ''' convert images into activitypub json '''
+    if image and hasattr(image, 'url'):
+        url = image.url
+    elif default_path:
+        url = default_path
+    else:
+        return None
+    url = 'https://%s%s' % (DOMAIN, url)
+    return activitypub.Image(url=url)
+
+
+def image_attachments_formatter(images):
+    ''' create a list of image attachments '''
+    return [image_formatter(i) for i in images]

bookwyrm/models/book.py

@@ -12,10 +12,11 @@ from bookwyrm.utils.fields import ArrayField
 
 from .base_model import ActivityMapping, BookWyrmModel
 from .base_model import ActivitypubMixin, OrderedCollectionPageMixin
+from .base_model import image_attachments_formatter
 
 class Book(ActivitypubMixin, BookWyrmModel):
     ''' a generic book, which can mean either an edition or a work '''
-    origin_id = models.CharField(max_length=255, null=True)
+    origin_id = models.CharField(max_length=255, null=True, blank=True)
     # these identifiers apply to both works and editions
     openlibrary_key = models.CharField(max_length=255, blank=True, null=True)
     librarything_key = models.CharField(max_length=255, blank=True, null=True)
@@ -60,15 +61,6 @@ class Book(ActivitypubMixin, BookWyrmModel):
         ''' the activitypub serialization should be a list of author ids '''
         return [a.remote_id for a in self.authors.all()]
 
-    @property
-    def ap_cover(self):
-        ''' an image attachment '''
-        if not self.cover or not hasattr(self.cover, 'url'):
-            return []
-        return [activitypub.Image(
-            url='https://%s%s' % (DOMAIN, self.cover.url),
-        )]
-
     @property
     def ap_parent_work(self):
         ''' reference the work via local id not remote '''
@@ -110,7 +102,12 @@ class Book(ActivitypubMixin, BookWyrmModel):
 
         ActivityMapping('lccn', 'lccn'),
         ActivityMapping('editions', 'editions_path'),
-        ActivityMapping('attachment', 'ap_cover'),
+        ActivityMapping(
+            'attachment', 'cover',
+            # this expects an iterable and the field is just an image
+            lambda x: image_attachments_formatter([x]),
+            activitypub.image_formatter
+        ),
     ]
 
     def save(self, *args, **kwargs):
@@ -197,7 +194,7 @@ class Edition(Book):
         if self.isbn_10 and not self.isbn_13:
             self.isbn_13 = isbn_10_to_13(self.isbn_10)
 
-        super().save(*args, **kwargs)
+        return super().save(*args, **kwargs)
 
 
 def isbn_10_to_13(isbn_10):
@@ -241,44 +238,3 @@ def isbn_13_to_10(isbn_13):
     if checkdigit == 10:
         checkdigit = 'X'
     return converted + str(checkdigit)
-
-
-class Author(ActivitypubMixin, BookWyrmModel):
-    origin_id = models.CharField(max_length=255, null=True)
-    ''' copy of an author from OL '''
-    openlibrary_key = models.CharField(max_length=255, blank=True, null=True)
-    sync = models.BooleanField(default=True)
-    last_sync_date = models.DateTimeField(default=timezone.now)
-    wikipedia_link = models.CharField(max_length=255, blank=True, null=True)
-    # idk probably other keys would be useful here?
-    born = models.DateTimeField(blank=True, null=True)
-    died = models.DateTimeField(blank=True, null=True)
-    name = models.CharField(max_length=255)
-    last_name = models.CharField(max_length=255, blank=True, null=True)
-    first_name = models.CharField(max_length=255, blank=True, null=True)
-    aliases = ArrayField(
-        models.CharField(max_length=255), blank=True, default=list
-    )
-    bio = models.TextField(null=True, blank=True)
-
-    @property
-    def display_name(self):
-        ''' Helper to return a displayable name'''
-        if self.name:
-            return self.name
-        # don't want to return a spurious space if all of these are None
-        if self.first_name and self.last_name:
-            return self.first_name + ' ' + self.last_name
-        return self.last_name or self.first_name
-
-    activity_mappings = [
-        ActivityMapping('id', 'remote_id'),
-        ActivityMapping('name', 'display_name'),
-        ActivityMapping('born', 'born'),
-        ActivityMapping('died', 'died'),
-        ActivityMapping('aliases', 'aliases'),
-        ActivityMapping('bio', 'bio'),
-        ActivityMapping('openlibrary_key', 'openlibrary_key'),
-        ActivityMapping('wikipedia_link', 'wikipedia_link'),
-    ]
-    activity_serializer = activitypub.Author

bookwyrm/models/status.py

@@ -7,6 +7,7 @@ from model_utils.managers import InheritanceManager
 from bookwyrm import activitypub
 from .base_model import ActivitypubMixin, OrderedCollectionPageMixin
 from .base_model import ActivityMapping, BookWyrmModel, PrivacyLevels
+from .base_model import tag_formatter, image_attachments_formatter
 
 
 class Status(OrderedCollectionPageMixin, BookWyrmModel):
@@ -57,24 +58,6 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel):
         ''' structured replies block '''
         return self.to_replies()
 
-    @property
-    def ap_tag(self):
-        ''' references to books and/or users '''
-
-        tags = []
-        for book in self.mention_books.all():
-            tags.append(activitypub.Link(
-                href=book.remote_id,
-                name=book.title,
-                type='Book'
-            ))
-        for user in self.mention_users.all():
-            tags.append(activitypub.Mention(
-                href=user.remote_id,
-                name=user.username,
-            ))
-        return tags
-
     @property
     def ap_status_image(self):
         ''' attach a book cover, if relevent '''
@@ -94,7 +77,21 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel):
         ActivityMapping('to', 'ap_to'),
         ActivityMapping('cc', 'ap_cc'),
         ActivityMapping('replies', 'ap_replies'),
-        ActivityMapping('tag', 'ap_tag'),
+        ActivityMapping(
+            'tag', 'mention_books',
+            lambda x: tag_formatter(x, 'title', 'Book'),
+            lambda x: activitypub.tag_formatter(x, 'Book')
+        ),
+        ActivityMapping(
+            'tag', 'mention_users',
+            lambda x: tag_formatter(x, 'username', 'Mention'),
+            lambda x: activitypub.tag_formatter(x, 'Mention')
+        ),
+        ActivityMapping(
+            'attachment', 'attachments',
+            lambda x: image_attachments_formatter(x.all()),
+            activitypub.image_attachments_formatter
+        )
     ]
 
     # serializing to bookwyrm expanded activitypub
@@ -148,9 +145,21 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel):
 
     def save(self, *args, **kwargs):
         ''' update user active time '''
+        if self.user.local:
             self.user.last_active_date = timezone.now()
             self.user.save()
-        super().save(*args, **kwargs)
+        return super().save(*args, **kwargs)
+
+
+class Attachment(BookWyrmModel):
+    ''' an image (or, in the future, video etc) associated with a status '''
+    status = models.ForeignKey(
+        'Status',
+        on_delete=models.CASCADE,
+        related_name='attachments'
+    )
+    image = models.ImageField(upload_to='status/', null=True, blank=True)
+    caption = models.TextField(null=True, blank=True)
 
 
 class GeneratedNote(Status):

bookwyrm/models/user.py

@@ -10,8 +10,8 @@ from bookwyrm.models.shelf import Shelf
 from bookwyrm.models.status import Status
 from bookwyrm.settings import DOMAIN
 from bookwyrm.signatures import create_key_pair
-from .base_model import OrderedCollectionPageMixin
+from .base_model import ActivityMapping, OrderedCollectionPageMixin
-from .base_model import ActivityMapping
+from .base_model import image_formatter
 
 
 class User(OrderedCollectionPageMixin, AbstractUser):
@@ -78,16 +78,6 @@ class User(OrderedCollectionPageMixin, AbstractUser):
         ''' generates url for activitypub followers page '''
         return '%s/followers' % self.remote_id
 
-    @property
-    def ap_icon(self):
-        ''' send default icon if one isn't set '''
-        if self.avatar:
-            url = self.avatar.url
-        else:
-            url = '/static/images/default_avi.jpg'
-        url = 'https://%s%s' % (DOMAIN, url)
-        return activitypub.Image(url=url)
-
     @property
     def ap_public_key(self):
         ''' format the public key block for activitypub '''
@@ -122,7 +112,11 @@ class User(OrderedCollectionPageMixin, AbstractUser):
             activity_formatter=lambda x: {'sharedInbox': x},
             model_formatter=lambda x: x.get('sharedInbox')
         ),
-        ActivityMapping('icon', 'ap_icon'),
+        ActivityMapping(
+            'icon', 'avatar',
+            lambda x: image_formatter(x, '/static/images/default_avi.jpg'),
+            activitypub.image_formatter
+        ),
         ActivityMapping(
             'manuallyApprovesFollowers',
             'manually_approves_followers'

bookwyrm/remote_user.py

@@ -1,9 +1,7 @@
 ''' manage remote users '''
 from urllib.parse import urlparse
-from uuid import uuid4
 import requests
 
-from django.core.files.base import ContentFile
 from django.db import transaction
 
 from bookwyrm import activitypub, models
@@ -22,14 +20,9 @@ def get_or_create_remote_user(actor):
 
     actor_parts = urlparse(actor)
     with transaction.atomic():
-        user = create_remote_user(data)
+        user = activitypub.Person(**data).to_model(models.User)
         user.federated_server = get_or_create_remote_server(actor_parts.netloc)
         user.save()
-
-    avatar = get_avatar(data)
-    if avatar:
-        user.avatar.save(*avatar)
-
     if user.bookwyrm_user:
         get_remote_reviews.delay(user.id)
     return user
@@ -55,12 +48,6 @@ def fetch_user_data(actor):
     return data
 
 
-def create_remote_user(data):
-    ''' parse the activitypub actor data into a user '''
-    actor = activitypub.Person(**data)
-    return actor.to_model(models.User)
-
-
 def refresh_remote_user(user):
     ''' get updated user data from its home instance '''
     data = fetch_user_data(user.remote_id)
@@ -69,21 +56,6 @@ def refresh_remote_user(user):
     activity.to_model(models.User, instance=user)
 
 
-def get_avatar(data):
-    ''' find the icon attachment and load the image from the remote sever '''
-    icon_blob = data.get('icon')
-    if not icon_blob or not icon_blob.get('url'):
-        return None
-
-    response = requests.get(icon_blob['url'])
-    if not response.ok:
-        return None
-
-    image_name = str(uuid4()) + '.' + icon_blob['url'].split('.')[-1]
-    image_content = ContentFile(response.content)
-    return [image_name, image_content]
-
-
 @app.task
 def get_remote_reviews(user_id):
     ''' ingest reviews by a new remote bookwyrm user '''

bookwyrm/settings.py

@@ -40,6 +40,7 @@ INSTALLED_APPS = [
     'django.contrib.messages',
     'django.contrib.staticfiles',
     'django.contrib.humanize',
+    'django_rename_app',
     'bookwyrm',
     'celery',
 ]

bookwyrm/static/css/format.css

@@ -1,4 +1,7 @@
 /* --- --- */
+.image {
+    overflow: hidden;
+}
 .navbar .logo {
     max-height: 50px;
 }

bookwyrm/status.py

@@ -13,7 +13,7 @@ def delete_status(status):
 
 
 def create_status(activity):
-    ''' unfortunately, it's not QUITE as simple as deserialiing it '''
+    ''' unfortunately, it's not QUITE as simple as deserializing it '''
     # render the json into an activity object
     serializer = activitypub.activity_objects[activity['type']]
     activity = serializer(**activity)

bookwyrm/templates/edit_book.html

@@ -20,6 +20,12 @@
     </div>
 </div>
 
+{% if login_form.non_field_errors %}
+<div class="block">
+    <p class="notification is-danger">{{ login_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">
@@ -37,13 +43,40 @@
     </div>
 
     <div class="columns">
-        <div class="block column">
+        <div class="column">
-            <h2 class="title is-4">Book Identifiers</h2>
+            <h2 class="title is-4">Metadata</h2>
-            <p class="fields is-grouped"><label class="label"for="id_isbn_13">ISBN 13:</label> {{ form.isbn_13 }} </p>
+            <p class="fields is-grouped"><label class="label"for="id_title">Title:</label> {{ form.title }} </p>
-            <p class="fields is-grouped"><label class="label"for="id_isbn_10">ISBN 10:</label> {{ form.isbn_10 }} </p>
+            {% for error in form.title.errors %}
-            <p class="fields is-grouped"><label class="label"for="id_openlibrary_key">Openlibrary key:</label> {{ form.openlibrary_key }} </p>
+            <p class="help is-danger">{{ error | escape }}</p>
-            <p class="fields is-grouped"><label class="label"for="id_librarything_key">Librarything key:</label> {{ form.librarything_key }} </p>
+            {% endfor %}
-            <p class="fields is-grouped"><label class="label"for="id_goodreads_key">Goodreads key:</label> {{ form.goodreads_key }} </p>
+            <p class="fields is-grouped"><label class="label"for="id_sort_title">Sort title:</label> {{ form.sort_title }} </p>
+            {% for error in form.sort_title.errors %}
+            <p class="help is-danger">{{ error | escape }}</p>
+            {% endfor %}
+            <p class="fields is-grouped"><label class="label"for="id_subtitle">Subtitle:</label> {{ form.subtitle }} </p>
+            {% for error in form.subtitle.errors %}
+            <p class="help is-danger">{{ error | escape }}</p>
+            {% endfor %}
+            <p class="fields is-grouped"><label class="label"for="id_description">Description:</label> {{ form.description }} </p>
+            {% for error in form.description.errors %}
+            <p class="help is-danger">{{ error | escape }}</p>
+            {% endfor %}
+            <p class="fields is-grouped"><label class="label"for="id_series">Series:</label> {{ form.series }} </p>
+            {% for error in form.series.errors %}
+            <p class="help is-danger">{{ error | escape }}</p>
+            {% endfor %}
+            <p class="fields is-grouped"><label class="label"for="id_series_number">Series number:</label> {{ form.series_number }} </p>
+            {% for error in form.series_number.errors %}
+            <p class="help is-danger">{{ error | escape }}</p>
+            {% endfor %}
+            <p class="fields is-grouped"><label class="label"for="id_first_published_date">First published date:</label> {{ form.first_published_date }} </p>
+            {% for error in form.first_published_date.errors %}
+            <p class="help is-danger">{{ error | escape }}</p>
+            {% endfor %}
+            <p class="fields is-grouped"><label class="label"for="id_published_date">Published date:</label> {{ form.published_date }} </p>
+            {% for error in form.published_date.errors %}
+            <p class="help is-danger">{{ error | escape }}</p>
+            {% endfor %}
         </div>
 
         <div class="column">
@@ -55,6 +88,9 @@
                     <div class="block">
                         <h2 class="title is-4">Cover</h2>
                         <p>{{ form.cover }} </p>
+                        {% for error in form.cover.errors %}
+                        <p class="help is-danger">{{ error | escape }}</p>
+                        {% endfor %}
                     </div>
                 </div>
             </div>
@@ -62,22 +98,45 @@
             <div class="block">
                 <h2 class="title is-4">Physical Properties</h2>
                 <p class="fields is-grouped"><label class="label"for="id_physical_format">Format:</label> {{ form.physical_format }} </p>
+                {% for error in form.physical_format.errors %}
+                <p class="help is-danger">{{ error | escape }}</p>
+                {% endfor %}
+                {% for error in form.physical_format.errors %}
+                <p class="help is-danger">{{ error | escape }}</p>
+                {% endfor %}
+
                 <p class="fields is-grouped"><label class="label"for="id_pages">Pages:</label> {{ form.pages }} </p>
+                {% for error in form.pages.errors %}
+                <p class="help is-danger">{{ error | escape }}</p>
+                {% endfor %}
+            </div>
+
+            <div class="block">
+                <h2 class="title is-4">Book Identifiers</h2>
+                <p class="fields is-grouped"><label class="label"for="id_isbn_13">ISBN 13:</label> {{ form.isbn_13 }} </p>
+                {% for error in form.isbn_13.errors %}
+                <p class="help is-danger">{{ error | escape }}</p>
+                {% endfor %}
+                <p class="fields is-grouped"><label class="label"for="id_isbn_10">ISBN 10:</label> {{ form.isbn_10 }} </p>
+                {% for error in form.isbn_10.errors %}
+                <p class="help is-danger">{{ error | escape }}</p>
+                {% endfor %}
+                <p class="fields is-grouped"><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 class="fields is-grouped"><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 class="fields is-grouped"><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>
 
-    <div class="block">
-        <h2 class="title is-4">Metadata</h2>
-        <p class="fields is-grouped"><label class="label"for="id_title">Title:</label> {{ form.title }} </p>
-        <p class="fields is-grouped"><label class="label"for="id_sort_title">Sort title:</label> {{ form.sort_title }} </p>
-        <p class="fields is-grouped"><label class="label"for="id_subtitle">Subtitle:</label> {{ form.subtitle }} </p>
-        <p class="fields is-grouped"><label class="label"for="id_description">Description:</label> {{ form.description }} </p>
-        <p class="fields is-grouped"><label class="label"for="id_series">Series:</label> {{ form.series }} </p>
-        <p class="fields is-grouped"><label class="label"for="id_series_number">Series number:</label> {{ form.series_number }} </p>
-        <p class="fields is-grouped"><label class="label"for="id_first_published_date">First published date:</label> {{ form.first_published_date }} </p>
-        <p class="fields is-grouped"><label class="label"for="id_published_date">Published date:</label> {{ form.published_date }} </p>
-    </div>
     <div class="block">
         <button class="button is-primary" type="submit">Save</button>
         <a class="button" href="/book/{{ book.id }}">Cancel</a>
@@ -85,4 +144,3 @@
 </form>
 
 {% endblock %}
-

bookwyrm/templates/feed.html

@@ -68,6 +68,7 @@
     </div>
 
     <div class="column is-two-thirds" id="feed">
+        <h1 class="title">{{ tab | title }} Timeline</h1>
         <div class="tabs">
             <ul>
                 <li class="{% if tab == 'home' %}is-active{% endif %}">

bookwyrm/templates/layout.html

@@ -25,15 +25,19 @@
         <a class="navbar-item" href="/">
             <img class="image logo" src="/static/images/logo-small.png" alt="Home page">
         </a>
-        <form class="navbar-item" action="/search/">
+        <form class="navbar-item column" action="/search/">
-            <div class="field is-grouped">
+            <div class="field has-addons">
+                <div class="control">
                     <input aria-label="Search for a book or user" id="search-input" class="input" type="text" name="q" placeholder="Search for a book or user" value="{{ query }}">
+                </div>
+                <div class="control">
                     <button class="button" type="submit">
                         <span class="icon icon-search">
                             <span class="is-sr-only">search</span>
                         </span>
                     </button>
                 </div>
+            </div>
         </form>
 
         <label for="main-nav" role="button" class="navbar-burger" aria-label="menu" aria-expanded="false" data-target="mainNav" onclick="toggleMenu(this)" tabindex="0">

bookwyrm/templates/login.html

@@ -4,7 +4,7 @@
 <div class="columns">
     <div class="column">
         <div class="box">
-            <h2 class="title">Log in</h2>
+            <h1 class="title">Log in</h1>
             {% if login_form.non_field_errors %}
             <p class="notification is-danger">{{ login_form.non_field_errors }}</p>
             {% endif %}

bookwyrm/templates/search_results.html

@@ -1,6 +1,10 @@
 {% extends 'layout.html' %}
 {% block content %}
 {% with book_results|first as local_results %}
+<div class="block">
+    <h1 class="title">Search Results for "{{ query }}"</h1>
+</div>
+
 <div class="block columns">
     <div class="column">
         <h2 class="title">Matching Books</h2>

bookwyrm/templates/snippets/about.html

@@ -1,4 +1,4 @@
-<h2 class="title">About {{ site_settings.name }}</h2>
+<h1 class="title">About {{ site_settings.name }}</h1>
 <div class="block">
     <img src="/static/images/logo.png" alt="BookWyrm">
 </div>

bookwyrm/templates/snippets/shelf_selector.html

@@ -12,7 +12,7 @@
         <ul class="dropdown-content">
             {% for shelf in request.user.shelf_set.all %}
             {% if shelf.identifier != current.identifier %}
-            <li>
+            <li role="menuitem">
                 <form class="dropdown-item pt-0 pb-0" name="shelve" action="/shelve/" method="post">
                     {% csrf_token %}
                     <input type="hidden" name="book" value="{{ book.id }}">

bookwyrm/templates/snippets/shelve_button.html

@@ -1,9 +1,9 @@
 {% load fr_display %}
 {% if request.user.is_authenticated %}
 
+{% with book.id|uuid as uuid %}
 {% active_shelf book as active_shelf %}
 <div class="field is-grouped">
-    {% with book.id|uuid as uuid %}
     {% if active_shelf.identifier == 'read' %}
     <button class="button is-small" disabled>
         <span>Read</span> <span class="icon icon-check"></span>
@@ -26,20 +26,20 @@
         <button class="button is-small" type="submit">Want to read</button>
     </form>
     {% endif %}
-    {% endwith %}
 
-    <div class="dropdown is-hoverable">
+    <div class="dropdown">
         <div class="dropdown-trigger">
-            <button class="button is-small" aria-haspopup="true" aria-controls="dropdown-menu-{{ uuid }}">
+            <label for="shelf-select-dropdown-{{ uuid }}-toggle" role="button" aria-expanded="false" onclick="toggleMenu(this)" tabindex="0" aria-haspopup="true" aria-controls="shelf-select-{{ uuid }}">
+                <div class="button is-small">
                     <span class="icon icon-arrow-down"><span class="is-sr-only">More shelves</span></span>
-            </button>
                 </div>
-
+            </label>
-        {% with book.id|uuid as uuid %}
+        </div>
-        <div class="dropdown-menu" id="dropdown-menu-{{ uuid }}" role="menu">
+        <input type="checkbox" class="toggle-control" id="shelf-select-dropdown-{{ uuid }}-toggle">
+        <div class="dropdown-menu toggle-content hidden" id="shelf-select-{{ uuid }}" role="menu">
             <ul class="dropdown-content">
                 {% for shelf in request.user.shelf_set.all %}
-                <li>
+                <li role="menuitem">
                     {% if shelf.identifier == 'to-read' %}
                     <div class="dropdown-item pt-0 pb-0">
                         <label class="button is-small" for="start-reading-{{ uuid }}" role="button" tabindex="0">
@@ -61,8 +61,7 @@
                 {% endfor %}
             </ul>
         </div>
+    </div>
+</div>
 {% endwith %}
-    </div>
-</div>
-
 {% endif %}

bookwyrm/templates/snippets/status_content.html

@@ -18,6 +18,21 @@
     {% if status.content and status.status_type != 'GeneratedNote' and status.status_type != 'Boost' %}
     {% include 'snippets/trimmed_text.html' with full=status.content|safe %}
     {% endif %}
+    {% if status.attachments %}
+    <div class="block">
+        <div class="columns">
+            {% for attachment in status.attachments.all %}
+            <div class="column is-narrow">
+                <figure class="image is-128x128">
+                    <a href="/images/{{ attachment.image }}" target="_blank" aria-label="open image in new window">
+                        <img src="/images/{{ attachment.image }}" alt="{{ attachment.caption }}">
+                    </a>
+                </figure>
+            </div>
+            {% endfor %}
+        </div>
+    </div>
+    {% endif %}
 </div>
 
 {% if not hide_book %}

bookwyrm/templatetags/fr_display.py

@@ -168,25 +168,6 @@ def active_shelf(context, book):
     return shelf.shelf if shelf else None
 
 
-@register.simple_tag(takes_context=True)
-def shelve_button_text(context, book):
-    ''' check what shelf a user has a book on, if any '''
-    #TODO: books can be on multiple shelves
-    shelf = models.ShelfBook.objects.filter(
-        shelf__user=context['request'].user,
-        book=book
-    ).first()
-    if not shelf:
-        return 'Want to read'
-
-    identifier = shelf.shelf.identifier
-    if identifier == 'to-read':
-        return 'Start reading'
-    if identifier == 'reading':
-        return 'I\'m done!'
-    return 'Read'
-
-
 @register.simple_tag(takes_context=False)
 def latest_read_through(book, user):
     ''' the most recent read activity '''

bookwyrm/tests/connectors/test_abstract_connector.py

@@ -2,8 +2,7 @@
 from django.test import TestCase
 
 from bookwyrm import models
-from bookwyrm.connectors.abstract_connector import Mapping,\
+from bookwyrm.connectors.abstract_connector import Mapping
-        update_from_mappings
 from bookwyrm.connectors.bookwyrm_connector import Connector
 
 
@@ -64,29 +63,6 @@ class AbstractConnector(TestCase):
         self.assertEqual(mapping.formatter('bb'), 'aabb')
 
 
-    def test_update_from_mappings(self):
-        data = {
-            'title': 'Unused title',
-            'isbn_10': '1234567890',
-            'isbn_13': 'blahhh',
-            'blah': 'bip',
-            'format': 'hardcover',
-            'series': ['one', 'two'],
-        }
-        mappings = [
-            Mapping('isbn_10'),
-            Mapping('blah'),# not present on self.book
-            Mapping('physical_format', remote_field='format'),
-            Mapping('series', formatter=lambda x: x[0]),
-        ]
-        book = update_from_mappings(self.book, data, mappings)
-        self.assertEqual(book.title, 'Example Edition')
-        self.assertEqual(book.isbn_10, '1234567890')
-        self.assertEqual(book.isbn_13, None)
-        self.assertEqual(book.physical_format, 'hardcover')
-        self.assertEqual(book.series, 'one')
-
-
     def test_match_from_mappings(self):
         edition = models.Edition.objects.create(
             title='Blah',

bookwyrm/tests/connectors/test_self_connector.py

@@ -57,10 +57,9 @@ class SelfConnector(TestCase):
 
     def test_search_rank(self):
         results = self.connector.search('Anonymous')
-        self.assertEqual(len(results), 3)
+        self.assertEqual(len(results), 2)
-        self.assertEqual(results[0].title, 'Edition of Example Work')
+        self.assertEqual(results[0].title, 'More Editions')
-        self.assertEqual(results[1].title, 'More Editions')
+        self.assertEqual(results[1].title, 'Edition of Example Work')
-        self.assertEqual(results[2].title, 'Another Edition')
 
 
     def test_search_default_filter(self):

bookwyrm/tests/data/fr_work.json

@@ -28,9 +28,7 @@
     ],
     "lccn": null,
     "editions": [
-        "https://bookwyrm.social/book/5989",
+        "https://bookwyrm.social/book/5989"
-        "OL28439584M",
-        "OL28300471M"
     ],
     "@context": "https://www.w3.org/ns/activitystreams"
 }

bookwyrm/tests/incoming/test_follow.py

@@ -1,3 +1,4 @@
+from unittest.mock import patch
 from django.test import TestCase
 
 from bookwyrm import models, incoming
@@ -27,6 +28,7 @@ class IncomingFollow(TestCase):
             "object": "http://local.com/user/mouse"
         }
 
+        with patch('bookwyrm.broadcast.broadcast_task.delay') as _:
             incoming.handle_follow(activity)
 
         # notification created
@@ -55,6 +57,7 @@ class IncomingFollow(TestCase):
         self.local_user.manually_approves_followers = True
         self.local_user.save()
 
+        with patch('bookwyrm.broadcast.broadcast_task.delay') as _:
             incoming.handle_follow(activity)
 
         # notification created
@@ -81,6 +84,7 @@ class IncomingFollow(TestCase):
             "object": "http://local.com/user/nonexistent-user"
         }
 
+        with patch('bookwyrm.broadcast.broadcast_task.delay') as _:
             incoming.handle_follow(activity)
 
         # do nothing

bookwyrm/tests/models/test_user_model.py

@@ -27,9 +27,13 @@ class User(TestCase):
         shelves = models.Shelf.objects.filter(user=self.user).all()
         self.assertEqual(len(shelves), 3)
         names = [s.name for s in shelves]
-        self.assertEqual(names, ['To Read', 'Currently Reading', 'Read'])
+        self.assertTrue('To Read' in names)
+        self.assertTrue('Currently Reading' in names)
+        self.assertTrue('Read' in names)
         ids = [s.identifier for s in shelves]
-        self.assertEqual(ids, ['to-read', 'reading', 'read'])
+        self.assertTrue('to-read' in ids)
+        self.assertTrue('reading' in ids)
+        self.assertTrue('read' in ids)
 
 
     def test_activitypub_serialize(self):

bookwyrm/tests/outgoing/test_follow.py

@@ -1,3 +1,4 @@
+from unittest.mock import patch
 from django.test import TestCase
 
 from bookwyrm import models, outgoing
@@ -22,7 +23,9 @@ class Following(TestCase):
     def test_handle_follow(self):
         self.assertEqual(models.UserFollowRequest.objects.count(), 0)
 
+        with patch('bookwyrm.broadcast.broadcast_task.delay') as _:
             outgoing.handle_follow(self.local_user, self.remote_user)
+
         rel = models.UserFollowRequest.objects.get()
 
         self.assertEqual(rel.user_subject, self.local_user)
@@ -33,6 +36,7 @@ class Following(TestCase):
     def test_handle_unfollow(self):
         self.remote_user.followers.add(self.local_user)
         self.assertEqual(self.remote_user.followers.count(), 1)
+        with patch('bookwyrm.broadcast.broadcast_task.delay') as _:
             outgoing.handle_unfollow(self.local_user, self.remote_user)
 
         self.assertEqual(self.remote_user.followers.count(), 0)
@@ -45,6 +49,7 @@ class Following(TestCase):
         )
         rel_id = rel.id
 
+        with patch('bookwyrm.broadcast.broadcast_task.delay') as _:
             outgoing.handle_accept(rel)
         # request should be deleted
         self.assertEqual(
@@ -61,6 +66,7 @@ class Following(TestCase):
         )
         rel_id = rel.id
 
+        with patch('bookwyrm.broadcast.broadcast_task.delay') as _:
             outgoing.handle_reject(rel)
         # request should be deleted
         self.assertEqual(

bookwyrm/tests/outgoing/test_shelving.py

@@ -1,3 +1,4 @@
+from unittest.mock import patch
 from django.test import TestCase
 
 from bookwyrm import models, outgoing
@@ -26,6 +27,7 @@ class Shelving(TestCase):
 
 
     def test_handle_shelve(self):
+        with patch('bookwyrm.broadcast.broadcast_task.delay') as _:
             outgoing.handle_shelve(self.user, self.book, self.shelf)
         # make sure the book is on the shelf
         self.assertEqual(self.shelf.books.get(), self.book)
@@ -34,6 +36,7 @@ class Shelving(TestCase):
     def test_handle_shelve_to_read(self):
         shelf = models.Shelf.objects.get(identifier='to-read')
 
+        with patch('bookwyrm.broadcast.broadcast_task.delay') as _:
             outgoing.handle_shelve(self.user, self.book, shelf)
         # make sure the book is on the shelf
         self.assertEqual(shelf.books.get(), self.book)
@@ -42,6 +45,7 @@ class Shelving(TestCase):
     def test_handle_shelve_reading(self):
         shelf = models.Shelf.objects.get(identifier='reading')
 
+        with patch('bookwyrm.broadcast.broadcast_task.delay') as _:
             outgoing.handle_shelve(self.user, self.book, shelf)
         # make sure the book is on the shelf
         self.assertEqual(shelf.books.get(), self.book)
@@ -50,6 +54,7 @@ class Shelving(TestCase):
     def test_handle_shelve_read(self):
         shelf = models.Shelf.objects.get(identifier='read')
 
+        with patch('bookwyrm.broadcast.broadcast_task.delay') as _:
             outgoing.handle_shelve(self.user, self.book, shelf)
         # make sure the book is on the shelf
         self.assertEqual(shelf.books.get(), self.book)
@@ -59,5 +64,6 @@ class Shelving(TestCase):
         self.shelf.books.add(self.book)
         self.shelf.save()
         self.assertEqual(self.shelf.books.count(), 1)
+        with patch('bookwyrm.broadcast.broadcast_task.delay') as _:
             outgoing.handle_unshelve(self.user, self.book, self.shelf)
         self.assertEqual(self.shelf.books.count(), 0)

bookwyrm/tests/test_remote_user.py

@@ -21,50 +21,7 @@ class RemoteUser(TestCase):
         self.user_data = json.loads(datafile.read_bytes())
 
 
-
     def test_get_remote_user(self):
         actor = 'https://example.com/users/rat'
         user = remote_user.get_or_create_remote_user(actor)
         self.assertEqual(user, self.remote_user)
-
-
-    def test_create_remote_user(self):
-        user = remote_user.create_remote_user(self.user_data)
-        self.assertFalse(user.local)
-        self.assertEqual(user.remote_id, 'https://example.com/user/mouse')
-        self.assertEqual(user.username, 'mouse@example.com')
-        self.assertEqual(user.name, 'MOUSE?? MOUSE!!')
-        self.assertEqual(user.inbox, 'https://example.com/user/mouse/inbox')
-        self.assertEqual(user.outbox, 'https://example.com/user/mouse/outbox')
-        self.assertEqual(user.shared_inbox, 'https://example.com/inbox')
-        self.assertEqual(
-            user.public_key,
-            self.user_data['publicKey']['publicKeyPem']
-        )
-        self.assertEqual(user.local, False)
-        self.assertEqual(user.bookwyrm_user, True)
-        self.assertEqual(user.manually_approves_followers, False)
-
-
-    def test_create_remote_user_missing_inbox(self):
-        del self.user_data['inbox']
-        self.assertRaises(
-            TypeError,
-            remote_user.create_remote_user,
-            self.user_data
-        )
-
-
-    def test_create_remote_user_missing_outbox(self):
-        del self.user_data['outbox']
-        self.assertRaises(
-            TypeError,
-            remote_user.create_remote_user,
-            self.user_data
-        )
-
-
-    def test_create_remote_user_default_fields(self):
-        del self.user_data['manuallyApprovesFollowers']
-        user = remote_user.create_remote_user(self.user_data)
-        self.assertEqual(user.manually_approves_followers, False)

bookwyrm/tests/test_signing.py

@@ -2,6 +2,7 @@ import time
 from collections import namedtuple
 from urllib.parse import urlsplit
 import pathlib
+from unittest.mock import patch
 
 import json
 import responses
@@ -63,11 +64,13 @@ class Signature(TestCase):
             send_data=None,
             digest=None,
             date=None):
+        ''' sends a follow request to the "rat" user '''
         now = date or http_date()
         data = json.dumps(get_follow_data(sender, self.rat))
         digest = digest or make_digest(data)
         signature = make_signature(
             signer or sender, self.rat.inbox, now, digest)
+        with patch('bookwyrm.incoming.handle_follow.delay') as _:
             return self.send(signature, now, send_data or data, digest)
 
     def test_correct_signature(self):
@@ -104,6 +107,7 @@ class Signature(TestCase):
             status=200
         )
 
+        with patch('bookwyrm.remote_user.get_remote_reviews.delay') as _:
             response = self.send_test_request(sender=self.fake_remote)
             self.assertEqual(response.status_code, 200)
 
@@ -141,6 +145,7 @@ class Signature(TestCase):
             json=data,
             status=200)
 
+        with patch('bookwyrm.remote_user.get_remote_reviews.delay') as _:
             # Key correct:
             response = self.send_test_request(sender=self.fake_remote)
             self.assertEqual(response.status_code, 200)
@@ -172,6 +177,7 @@ class Signature(TestCase):
     @pytest.mark.integration
     def test_changed_data(self):
         '''Message data must match the digest header.'''
+        with patch('bookwyrm.remote_user.fetch_user_data') as _:
             response = self.send_test_request(
                 self.mouse,
                 send_data=get_follow_data(self.mouse, self.cat))
@@ -179,6 +185,7 @@ class Signature(TestCase):
 
     @pytest.mark.integration
     def test_invalid_digest(self):
+        with patch('bookwyrm.remote_user.fetch_user_data') as _:
             response = self.send_test_request(
                 self.mouse,
                 digest='SHA-256=AAAAAAAAAAAAAAAAAA')
@@ -187,6 +194,7 @@ class Signature(TestCase):
     @pytest.mark.integration
     def test_old_message(self):
         '''Old messages should be rejected to prevent replay attacks.'''
+        with patch('bookwyrm.remote_user.fetch_user_data') as _:
             response = self.send_test_request(
                 self.mouse,
                 date=http_date(time.time() - 301)

bookwyrm/view_actions.py

@@ -235,7 +235,12 @@ def edit_book(request, book_id):
 
     form = forms.EditionForm(request.POST, request.FILES, instance=book)
     if not form.is_valid():
-        return redirect(request.headers.get('Referer', '/'))
+        data = {
+            'title': 'Edit Book',
+            'book': book,
+            'form': form
+        }
+        return TemplateResponse(request, 'edit_book.html', data)
     form.save()
 
     outgoing.handle_update_book(request.user, book)

bw-dev

@@ -0,0 +1,102 @@
+#!/bin/bash
+
+# exit on errors
+set -e
+
+# import our ENV variables
+# catch exits and give a friendly error message
+function showerr {
+    echo "Failed to load configuration! You may need to update your .env and quote values with special characters in them."
+}
+trap showerr EXIT
+source .env
+trap - EXIT
+
+# show commands as they're executed
+set -x
+
+function clean {
+    docker-compose stop
+    docker-compose rm -f
+}
+
+function runweb {
+    docker-compose run --rm web "$@"
+    clean
+}
+
+function execdb {
+    docker-compose exec db $@
+}
+
+function execweb {
+    docker-compose exec web "$@"
+}
+
+function initdb {
+    execweb python manage.py migrate
+    execweb python manage.py initdb
+}
+
+case "$1" in
+    up)
+        docker-compose up --build
+        ;;
+    run)
+        docker-compose run --rm --service-ports web
+        ;;
+    initdb)
+        initdb
+        ;;
+    resetdb)
+        clean
+        docker-compose up --build -d
+        execdb dropdb -U ${POSTGRES_USER} ${POSTGRES_DB}
+        execdb createdb -U ${POSTGRES_USER} ${POSTGRES_DB}
+        initdb
+        clean
+        ;;
+    makemigrations)
+        execweb python manage.py makemigrations
+        ;;
+    migrate)
+        execweb python manage.py rename_app fedireads bookwyrm
+        shift 1
+        execweb python manage.py migrate "$@"
+        ;;
+    bash)
+        execweb bash
+        ;;
+    shell)
+        execweb python manage.py shell
+        ;;
+    dbshell)
+        execdb psql -U ${POSTGRES_USER} ${POSTGRES_DB}
+        ;;
+    restart_celery)
+        docker-compose restart celery_worker
+        ;;
+    test)
+        shift 1
+        execweb coverage run --source='.' --omit="*/test*,celerywyrm*,bookwyrm/migrations/*" manage.py test "$@"
+        ;;
+    pytest)
+        shift 1
+        execweb pytest "$@"
+        ;;
+    test_report)
+        execweb coverage report
+        ;;
+    collectstatic)
+        execweb python manage.py collectstatic --no-input
+        ;;
+    build)
+        docker-compose build
+        ;;
+    clean)
+        clean
+        ;;
+    *)
+        echo "Unrecognised command. Try: build, clean, up, initdb, resetdb, makemigrations, migrate, bash, shell, dbshell, restart_celery, test, pytest, test_report"
+        ;;
+esac

fr-dev

@@ -1,88 +0,0 @@
-#!/bin/bash
-
-set -e
-set -x
-
-function clean {
-    docker-compose stop
-    docker-compose rm -f
-}
-
-function runweb {
-    docker-compose run --rm web "$@"
-    clean
-}
-
-function execdb {
-    docker-compose exec db $@
-}
-
-function execweb {
-    docker-compose exec web "$@"
-}
-
-function initdb {
-    execweb python manage.py migrate
-    execweb python manage.py initdb
-}
-
-case "$1" in
-    up)
-        docker-compose up --build
-        ;;
-    run)
-        docker-compose run --rm --service-ports web
-        ;;
-    initdb)
-        initdb
-        ;;
-    resetdb)
-        clean
-        docker-compose up --build -d
-        execdb dropdb -U fedireads fedireads
-        execdb createdb -U fedireads fedireads
-        initdb
-        clean
-        ;;
-    makemigrations)
-        execweb python manage.py makemigrations
-        ;;
-    migrate)
-        execweb python manage.py migrate
-        ;;
-    bash)
-        execweb bash
-        ;;
-    shell)
-        execweb python manage.py shell
-        ;;
-    dbshell)
-        execdb psql -U fedireads fedireads
-        ;;
-    restart_celery)
-        docker-compose restart celery_worker
-        ;;
-    test)
-        shift 1
-        execweb coverage run --source='.' --omit="*/test*,celerywyrm*,bookwyrm/migrations/*" manage.py test "$@"
-        ;;
-    pytest)
-        shift 1
-        execweb pytest "$@"
-        ;;
-    test_report)
-        execweb coverage report
-        ;;
-    collectstatic)
-        execweb python manage.py collectstatic --no-input
-        ;;
-    build)
-        docker-compose build
-        ;;
-    clean)
-        clean
-        ;;
-    *)
-        echo "Unrecognised command. Try: build, clean, up, initdb, resetdb, makemigrations, migrate, bash, shell, dbshell, restart_celery, test, pytest, test_report"
-        ;;
-esac

fr-dev

@@ -0,0 +1 @@
+bw-dev

requirements.txt

@@ -14,3 +14,4 @@ python-dateutil==2.8.1
 redis==3.4.1
 requests==2.22.0
 responses==0.10.14
+django-rename-app==0.1.2