Merge branch 'main' into add-feed-filters

bookwyrm/models/author.py

@@ -27,7 +27,7 @@ class Author(BookDataModel):
     # idk probably other keys would be useful here?
     born = fields.DateTimeField(blank=True, null=True)
     died = fields.DateTimeField(blank=True, null=True)
-    name = fields.CharField(max_length=255, deduplication_field=True)
+    name = fields.CharField(max_length=255)
     aliases = fields.ArrayField(
         models.CharField(max_length=255), blank=True, default=list
     )

bookwyrm/models/fields.py

@@ -296,7 +296,7 @@ class ManyToManyField(ActivitypubFieldMixin, models.ManyToManyField):
         super().__init__(*args, **kwargs)
 
     def set_field_from_activity(self, instance, data, overwrite=True):
-        """helper function for assinging a value to the field"""
+        """helper function for assigning a value to the field"""
         if not overwrite and getattr(instance, self.name).exists():
             return False
 
@@ -398,7 +398,11 @@ class ImageField(ActivitypubFieldMixin, models.ImageField):
         if formatted is None or formatted is MISSING:
             return False
 
-        if not overwrite and hasattr(instance, self.name):
+        if (
+            not overwrite
+            and hasattr(instance, self.name)
+            and getattr(instance, self.name)
+        ):
             return False
 
         getattr(instance, self.name).save(*formatted, save=save)

bookwyrm/preview_images.py

@@ -317,15 +317,21 @@ def save_and_cleanup(image, instance=None):
     """Save and close the file"""
     if not isinstance(instance, (models.Book, models.User, models.SiteSettings)):
         return False
-    uuid = uuid4()
-    file_name = f"{instance.id}-{uuid}.jpg"
     image_buffer = BytesIO()
 
     try:
         try:
-            old_path = instance.preview_image.name
+            file_name = instance.preview_image.name
         except ValueError:
-            old_path = None
+            file_name = None
+
+        if not file_name or file_name == "":
+            uuid = uuid4()
+            file_name = f"{instance.id}-{uuid}.jpg"
+
+        # Clean up old file before saving
+        if file_name and default_storage.exists(file_name):
+            default_storage.delete(file_name)
 
         # Save
         image.save(image_buffer, format="jpeg", quality=75)
@@ -345,10 +351,6 @@ def save_and_cleanup(image, instance=None):
         else:
             instance.save(update_fields=["preview_image"])
 
-        # Clean up old file after saving
-        if old_path and default_storage.exists(old_path):
-            default_storage.delete(old_path)
-
     finally:
         image_buffer.close()
     return True

bookwyrm/static/js/bookwyrm.js

@@ -45,6 +45,13 @@ let BookWyrm = new class {
                 'change',
                 this.disableIfTooLarge.bind(this)
             ));
+        
+        document.querySelectorAll('[data-duplicate]')
+                .forEach(node => node.addEventListener(
+                    'click',
+                    this.duplicateInput.bind(this)
+            
+            ))
     }
 
     /**
@@ -403,4 +410,24 @@ let BookWyrm = new class {
             );
         }
     }
+
+    duplicateInput (event ) {
+        const trigger = event.currentTarget;
+        const input_id = trigger.dataset['duplicate']
+        const orig = document.getElementById(input_id);
+        const parent = orig.parentNode;
+        const new_count = parent.querySelectorAll("input").length + 1
+
+        let input = orig.cloneNode();
+
+        input.id += ("-" + (new_count))
+        input.value = ""
+
+        let label = parent.querySelector("label").cloneNode();
+
+        label.setAttribute("for", input.id)
+
+        parent.appendChild(label)
+        parent.appendChild(input)
+    }
 }();

bookwyrm/static/js/status_cache.js

@@ -187,6 +187,7 @@ let StatusCache = new class {
             .forEach(item => BookWyrm.addRemoveClass(item, "is-hidden", false));
 
         // Remove existing disabled states
+
         button.querySelectorAll("[data-shelf-dropdown-identifier] button")
             .forEach(item => item.disabled = false);
 

bookwyrm/templates/author/author.html

@@ -2,6 +2,7 @@
 {% load i18n %}
 {% load markdown %}
 {% load humanize %}
+{% load utilities %}
 
 {% block title %}{{ author.name }}{% endblock %}
 
@@ -25,7 +26,7 @@
 <div class="block columns content" itemscope itemtype="https://schema.org/Person">
     <meta itemprop="name" content="{{ author.name }}">
 
-    {% if author.aliases or author.born or author.died or author.wikipedia_link or author.openlibrary_key or author.inventaire_id %}
+    {% if author.aliases or author.born or author.died or author.wikipedia_link or author.openlibrary_key or author.inventaire_id or author.isni %}
     <div class="column is-two-fifths">
         <div class="box py-2">
             <dl>
@@ -63,6 +64,14 @@
             </p>
             {% endif %}
 
+            {% if author.isni %}
+            <p class="my-1">
+                <a itemprop="sameAs" href="https://isni.org/isni/{{ author.isni|remove_spaces }}" rel="noopener" target="_blank">
+                    {% trans "View ISNI record" %}
+                </a>
+            </p>
+            {% endif %}
+
             {% if author.openlibrary_key %}
             <p class="my-1">
                 <a itemprop="sameAs" href="https://openlibrary.org/authors/{{ author.openlibrary_key }}" target="_blank" rel="noopener">

bookwyrm/templates/book/book.html

@@ -153,12 +153,21 @@
 
             {# user's relationship to the book #}
             <div class="block">
+                {% if user_shelfbooks.count > 0 %}
+                <h2 class="title is-5">
+                    {% trans "You have shelved this edition in:" %}
+                </h2>
+                <ul>
                 {% for shelf in user_shelfbooks %}
-                <p>
-                    {% blocktrans with path=shelf.shelf.local_path shelf_name=shelf.shelf.name %}This edition is on your <a href="{{ path }}">{{ shelf_name }}</a> shelf.{% endblocktrans %}
-                    {% include 'snippets/shelf_selector.html' with current=shelf.shelf %}
-                </p>
+                    <li class="box">
+                        {% blocktrans with path=shelf.shelf.local_path shelf_name=shelf.shelf.name %}<a href="{{ path }}">{{ shelf_name }}</a>{% endblocktrans %}
+                        <div class="mb-3">
+                            {% include 'snippets/shelf_selector.html' with shelf=shelf.shelf class="is-small" readthrough=readthrough %}
+                        </div>
+                    </li>
                 {% endfor %}
+                </ul>
+                {% endif %}
                 {% for shelf in other_edition_shelves %}
                 <p>
                 {% blocktrans with book_path=shelf.book.local_path shelf_path=shelf.shelf.local_path shelf_name=shelf.shelf.name %}A <a href="{{ book_path }}">different edition</a> of this book is on your <a href="{{ shelf_path }}">{{ shelf_name }}</a> shelf.{% endblocktrans %}

bookwyrm/templates/book/edit/edit_book.html

@@ -1,6 +1,7 @@
 {% extends 'layout.html' %}
 {% load i18n %}
 {% load humanize %}
+{% load utilities %}
 
 {% block title %}{% if book %}{% blocktrans with book_title=book.title %}Edit "{{ book_title }}"{% endblocktrans %}{% else %}{% trans "Add Book" %}{% endif %}{% endblock %}
 
@@ -52,19 +53,29 @@
                     {% for author in author_matches %}
                     <fieldset>
                         <legend class="title is-5 mb-1">
-                            {% blocktrans with name=author.name %}Is "{{ name }}" an existing author?{% endblocktrans %}
+                            {% blocktrans with name=author.name %}Is "{{ name }}" one of these authors?{% endblocktrans %}
                         </legend>
                         {% with forloop.counter0 as counter %}
                         {% for match in author.matches %}
-                        <label class="label mb-2">
+                        <label class="label">
                             <input type="radio" name="author_match-{{ counter }}" value="{{ match.id }}" required>
                             {{ match.name }}
                         </label>
-                        <p class="help">
-                            <a href="{{ match.local_path }}" target="_blank">{% blocktrans with book_title=match.book_set.first.title %}Author of <em>{{ book_title }}</em>{% endblocktrans %}</a>
+                        <p class="help ml-5 mb-2">
+                            {% with book_title=match.book_set.first.title alt_title=match.bio %}
+                            {% if book_title %}
+                            <a href="{{ match.local_path }}" target="_blank">{% trans "Author of " %}<em>{{ book_title }}</em></a>
+                            {% else %} 
+                            <a href="{{ match.id }}" target="_blank">{% if alt_title %}{% trans "Author of " %}<em>{{ alt_title }}</em>{% else %} {% trans "Find more information at isni.org" %}{% endif %}</a>
+                            {% endif %}
+                            {% endwith %}
                         </p>
+                        <p class="help ml-5">
+                            {{ author.existing_isnis|get_isni_bio:match }}
+                        </p>
+                        {{ author.existing_isnis|get_isni:match }}
                         {% endfor %}
-                        <label class="label">
+                        <label class="label mt-2">
                             <input type="radio" name="author_match-{{ counter }}" value="{{ author.name }}" required> {% trans "This is a new author" %}
                         </label>
                         {% endwith %}

bookwyrm/templates/book/edit/edit_book_form.html

@@ -119,10 +119,16 @@
                 </fieldset>
                 {% endif %}
                 <div class="field">
-                    <label class="label" for="id_add_author">{% trans "Add Authors:" %}</label>
-                    <input class="input" type="text" name="add_author" id="id_add_author" placeholder="{% trans 'John Doe, Jane Smith' %}" value="{{ add_author }}" {% if confirm_mode %}readonly{% endif %}>
-                    <span class="help">{% trans "Separate multiple values with commas." %}</span>
+                    <label class="label">{% trans "Add Authors:" %}</label>
+                    {% for author in add_author %}
+                    <label class="label is-sr-only" for="id_add_author{% if not forloop.first %}-{{forloop.counter}}{% endif %}">{% trans "Add Author" %}</label>
+                    <input class="input" type="text" name="add_author" id="id_add_author{% if not forloop.first %}-{{forloop.counter}}{% endif %}" placeholder="{% trans 'Jane Doe' %}" value="{{ author }}" {% if confirm_mode %}readonly{% endif %}>
+                    {% empty %}
+                    <label class="label is-sr-only" for="id_add_author">{% trans "Add Author" %}</label>
+                    <input class="input" type="text" name="add_author" id="id_add_author" placeholder="{% trans 'Jane Doe' %}" value="{{ author }}" {% if confirm_mode %}readonly{% endif %}>
+                    {% endfor %}
                 </div>
+                <span class="help"><button class="button is-small" type="button" data-duplicate="id_add_author" id="another_author_field">{% trans "Add Another Author" %}</button></span>
             </div>
         </section>
     </div>

bookwyrm/templates/snippets/reading_modals/finish_reading_modal.html

@@ -9,10 +9,11 @@ Finish "<em>{{ book_title }}</em>"
 {% endblock %}
 
 {% block modal-form-open %}
-<form name="finish-reading" action="{% url 'reading-status' 'finish' book.id %}" method="post" class="submit-status">
+<form name="finish-reading" action="{% url 'reading-status' 'finish' book.id %}" method="post" {% if not refresh %}class="submit-status"{% endif %}>
 {% csrf_token %}
 <input type="hidden" name="id" value="{{ readthrough.id }}">
 <input type="hidden" name="reading_status" value="read">
+<input type="hidden" name="shelf" value="{{ move_from }}"> 
 {% endblock %}
 
 {% block reading-dates %}

bookwyrm/templates/snippets/reading_modals/start_reading_modal.html

@@ -9,8 +9,9 @@ Start "<em>{{ book_title }}</em>"
 {% endblock %}
 
 {% block modal-form-open %}
-<form name="start-reading" action="{% url 'reading-status' 'start' book.id %}" method="post" class="submit-status">
+<form name="start-reading" action="{% url 'reading-status' 'start' book.id %}" method="post" {% if not refresh %}class="submit-status"{% endif %}>
 <input type="hidden" name="reading_status" value="reading">
+<input type="hidden" name="shelf" value="{{ move_from }}"> 
 {% csrf_token %}
 {% endblock %}
 

bookwyrm/templates/snippets/reading_modals/want_to_read_modal.html

@@ -9,8 +9,9 @@ Want to Read "<em>{{ book_title }}</em>"
 {% endblock %}
 
 {% block modal-form-open %}
-<form name="shelve" action="{% url 'reading-status' 'want' book.id %}" method="post" class="submit-status">
+<form name="shelve" action="{% url 'reading-status' 'want' book.id %}" method="post" {% if not refresh %}class="submit-status"{% endif %}>
 <input type="hidden" name="reading_status" value="to-read">
+<input type="hidden" name="shelf" value="{{ move_from }}"> 
 {% csrf_token %}
 {% endblock %}
 

bookwyrm/templates/snippets/shelf_selector.html

@@ -1,29 +1,88 @@
 {% extends 'components/dropdown.html' %}
 {% load i18n %}
+{% load bookwyrm_tags %}
+{% load utilities %}
+
 {% block dropdown-trigger %}
 <span>{% trans "Move book" %}</span>
 <span class="icon icon-arrow-down" aria-hidden="true"></span>
 {% endblock %}
 
 {% block dropdown-list %}
+{% with book.id|uuid as uuid %}
+{% active_shelf book as active_shelf %}
+{% latest_read_through book request.user as readthrough %}
+
 {% for shelf in user_shelves %}
+
+{% if shelf.editable %}
 <li role="menuitem" class="dropdown-item p-0">
     <form name="shelve" action="/shelve/" method="post">
         {% csrf_token %}
         <input type="hidden" name="book" value="{{ book.id }}">
         <input type="hidden" name="change-shelf-from" value="{{ current.identifier }}">
         <input type="hidden" name="shelf" value="{{ shelf.identifier }}">
-        <button class="button is-fullwidth is-small shelf-option is-radiusless is-white" type="submit" {% if shelf.identifier == current.identifier %}disabled{% endif %}><span>{{ shelf.name }}</span></button>
+        <button class="button is-fullwidth is-small shelf-option is-radiusless is-white" type="submit" {% if shelf in book.shelf_set.all %} disabled {% endif %}><span>{{ shelf.name }}</span></button>
     </form>
 </li>
+{% else%}
+{% comparison_bool shelf.identifier active_shelf.shelf.identifier as is_current %}
+{% with button_class="is-fullwidth is-small shelf-option is-radiusless is-white" %}
+<li role="menuitem" class="dropdown-item p-0">
+{% if shelf.identifier == 'reading' %}
+
+{% trans "Start reading" as button_text %}
+{% url 'reading-status' 'start' book.id as fallback_url %}
+{% include 'snippets/toggle/toggle_button.html' with class=button_class text=button_text controls_text="start_reading" controls_uid=uuid focus="modal_title_start_reading" disabled=is_current fallback_url=fallback_url %}
+
+
+{% elif shelf.identifier == 'read' %}
+
+{% trans "Read" as button_text %}
+{% url 'reading-status' 'finish' book.id as fallback_url %}
+{% include 'snippets/toggle/toggle_button.html' with class=button_class text=button_text controls_text="finish_reading" controls_uid=uuid focus="modal_title_finish_reading" disabled=is_current fallback_url=fallback_url %}
+
+{% elif shelf.identifier == 'to-read' %}
+
+{% trans "Want to read" as button_text %}
+{% url 'reading-status' 'want' book.id as fallback_url %}
+{% include 'snippets/toggle/toggle_button.html' with class=button_class text=button_text controls_text="want_to_read" controls_uid=uuid focus="modal_title_want_to_read" disabled=is_current fallback_url=fallback_url %}
+
+{% endif %}
+</li>
+{% endwith %}
+{% endif %}
 {% endfor %}
-<li class="navbar-divider" role="separator"></li>
+
+{% if shelf.identifier == 'all' %}
+{% for shelved_in in book.shelves.all %}
+<li class="navbar-divider m-0" role="separator" ></li>
 <li role="menuitem" class="dropdown-item p-0">
     <form name="shelve" action="/unshelve/" method="post">
         {% csrf_token %}
         <input type="hidden" name="book" value="{{ book.id }}">
-        <input type="hidden" name="shelf" value="{{ current.id }}">
-        <button class="button is-fullwidth is-small is-radiusless is-danger is-light" type="submit">{% trans "Remove" %}</button>
+        <input type="hidden" name="shelf" value="{{ shelved_in.id }}">
+        <button class="button is-fullwidth is-small is-radiusless is-danger is-light" type="submit">{% trans "Remove from" %} {{ shelved_in.name }}</button>
     </form>
 </li>
+{% endfor %}
+{% else %}
+<li class="navbar-divider" role="separator" ></li>
+<li role="menuitem" class="dropdown-item p-0">
+    <form name="shelve" action="/unshelve/" method="post">
+        {% csrf_token %}
+        <input type="hidden" name="book" value="{{ book.id }}">
+        <input type="hidden" name="shelf" value="{{ shelf.id }}">
+        <button class="button is-fullwidth is-small is-radiusless is-danger is-light" type="submit">{% trans "Remove from" %} {{ shelf.name }}</button>
+    </form>
+</li>
+{% endif %}
+
+{% include 'snippets/reading_modals/want_to_read_modal.html' with book=active_shelf.book controls_text="want_to_read" controls_uid=uuid move_from=current.id refresh=True %}
+
+{% include 'snippets/reading_modals/start_reading_modal.html' with book=active_shelf.book controls_text="start_reading" controls_uid=uuid move_from=current.id refresh=True %}
+
+{% include 'snippets/reading_modals/finish_reading_modal.html' with book=active_shelf.book controls_text="finish_reading" controls_uid=uuid  move_from=current.id readthrough=readthrough refresh=True %}
+
+{% endwith %}
 {% endblock %}

bookwyrm/templates/snippets/shelve_button/shelve_button_dropdown_options.html

@@ -32,7 +32,7 @@
 
         {% elif shelf.editable %}
 
-            <form name="shelve" action="/shelve/" method="post">
+            <form name="shelve" action="/shelve/" method="post" autocomplete="off">
                 {% csrf_token %}
                 <input type="hidden" name="book" value="{{ active_shelf.book.id }}">
                 <button class="button {{ class }}" name="shelf" type="submit" value="{{ shelf.identifier }}" {% if shelf in book.shelf_set.all %} disabled {% endif %}>

bookwyrm/templates/snippets/toggle/toggle_button.html

@@ -1,6 +1,6 @@
 {% load utilities %}
 {% if fallback_url %}
-<form name="fallback_form_{{ 0|uuid }}" method="GET" action="{{ fallback_url }}">
+<form name="fallback_form_{{ 0|uuid }}" method="GET" action="{{ fallback_url }}" autocomplete="off">
 {% endif %}
 <button
     {% if not fallback_url %}

bookwyrm/templatetags/bookwyrm_tags.py

@@ -77,7 +77,12 @@ def related_status(notification):
 def active_shelf(context, book):
     """check what shelf a user has a book on, if any"""
     if hasattr(book, "current_shelves"):
-        return book.current_shelves[0] if len(book.current_shelves) else {"book": book}
+        read_shelves = [
+            s
+            for s in book.current_shelves
+            if s.shelf.identifier in models.Shelf.READ_STATUS_IDENTIFIERS
+        ]
+        return read_shelves[0] if len(read_shelves) else {"book": book}
 
     shelf = (
         models.ShelfBook.objects.filter(

bookwyrm/templatetags/utilities.py

@@ -1,8 +1,11 @@
 """ template filters for really common utilities """
 import os
+import re
 from uuid import uuid4
 from django import template
+from django.utils.safestring import mark_safe
 from django.utils.translation import gettext_lazy as _
+from django.template.defaultfilters import stringfilter
 from django.templatetags.static import static
 
 
@@ -66,3 +69,39 @@ def get_book_cover_thumbnail(book, size="medium", ext="jpg"):
         return cover_thumbnail.url
     except OSError:
         return static("images/no_cover.jpg")
+
+
+@register.filter(name="get_isni_bio")
+def get_isni_bio(existing, author):
+    """Returns the isni bio string if an existing author has an isni listed"""
+    auth_isni = re.sub(r"\D", "", str(author.isni))
+    if len(existing) == 0:
+        return ""
+    for value in existing:
+        if hasattr(value, "bio") and auth_isni == re.sub(r"\D", "", str(value.isni)):
+            return mark_safe(f"Author of <em>{value.bio}</em>")
+
+    return ""
+
+
+# pylint: disable=unused-argument
+@register.filter(name="get_isni", needs_autoescape=True)
+def get_isni(existing, author, autoescape=True):
+    """Returns the isni ID if an existing author has an ISNI listing"""
+    auth_isni = re.sub(r"\D", "", str(author.isni))
+    if len(existing) == 0:
+        return ""
+    for value in existing:
+        if hasattr(value, "isni") and auth_isni == re.sub(r"\D", "", str(value.isni)):
+            isni = value.isni
+            return mark_safe(
+                f'<input type="text" name="isni-for-{author.id}" value="{isni}" hidden>'
+            )
+    return ""
+
+
+@register.filter(name="remove_spaces")
+@stringfilter
+def remove_spaces(arg):
+    """Removes spaces from argument passed in"""
+    return re.sub(r"\s", "", str(arg))

bookwyrm/tests/models/test_fields.py

@@ -19,7 +19,7 @@ from django.utils import timezone
 
 from bookwyrm import activitypub
 from bookwyrm.activitypub.base_activity import ActivityObject
-from bookwyrm.models import fields, User, Status
+from bookwyrm.models import fields, User, Status, Edition
 from bookwyrm.models.base_model import BookWyrmModel
 from bookwyrm.models.activitypub_mixin import ActivitypubMixin
 from bookwyrm.settings import DOMAIN
@@ -215,7 +215,7 @@ class ModelFields(TestCase):
             "rat", "rat@rat.rat", "ratword", local=True, localname="rat"
         )
         public = "https://www.w3.org/ns/activitystreams#Public"
-        followers = "%s/followers" % user.remote_id
+        followers = f"{user.remote_id}/followers"
 
         instance = fields.PrivacyField()
         instance.name = "privacy_field"
@@ -409,11 +409,10 @@ class ModelFields(TestCase):
         """loadin' a list of items from Links"""
         # TODO
 
-    @responses.activate
     @patch("bookwyrm.models.activitypub_mixin.ObjectMixin.broadcast")
     @patch("bookwyrm.suggested_users.remove_user_task.delay")
-    def test_image_field(self, *_):
-        """storing images"""
+    def test_image_field_to_activity(self, *_):
+        """serialize an image field to activitypub"""
         user = User.objects.create_user(
             "mouse", "mouse@mouse.mouse", "mouseword", local=True, localname="mouse"
         )
@@ -437,16 +436,155 @@ class ModelFields(TestCase):
         self.assertEqual(output.name, "")
         self.assertEqual(output.type, "Document")
 
+    @responses.activate
+    def test_image_field_from_activity(self, *_):
+        """load an image from activitypub"""
+        image_file = pathlib.Path(__file__).parent.joinpath(
+            "../../static/images/default_avi.jpg"
+        )
+        image = Image.open(image_file)
+        output = BytesIO()
+        image.save(output, format=image.format)
+
+        instance = fields.ImageField()
+
         responses.add(
             responses.GET,
             "http://www.example.com/image.jpg",
-            body=user.avatar.file.read(),
+            body=image.tobytes(),
             status=200,
         )
         loaded_image = instance.field_from_activity("http://www.example.com/image.jpg")
         self.assertIsInstance(loaded_image, list)
         self.assertIsInstance(loaded_image[1], ContentFile)
 
+    @responses.activate
+    def test_image_field_set_field_from_activity(self, *_):
+        """update a model instance from an activitypub object"""
+        image_file = pathlib.Path(__file__).parent.joinpath(
+            "../../static/images/default_avi.jpg"
+        )
+        image = Image.open(image_file)
+        output = BytesIO()
+        image.save(output, format=image.format)
+
+        instance = fields.ImageField(activitypub_field="cover", name="cover")
+
+        responses.add(
+            responses.GET,
+            "http://www.example.com/image.jpg",
+            body=image.tobytes(),
+            status=200,
+        )
+        book = Edition.objects.create(title="hello")
+
+        MockActivity = namedtuple("MockActivity", ("cover"))
+        mock_activity = MockActivity("http://www.example.com/image.jpg")
+
+        instance.set_field_from_activity(book, mock_activity)
+        self.assertIsNotNone(book.cover.name)
+        self.assertEqual(book.cover.size, 43200)
+
+    @responses.activate
+    def test_image_field_set_field_from_activity_no_overwrite_no_cover(self, *_):
+        """update a model instance from an activitypub object"""
+        image_file = pathlib.Path(__file__).parent.joinpath(
+            "../../static/images/default_avi.jpg"
+        )
+        image = Image.open(image_file)
+        output = BytesIO()
+        image.save(output, format=image.format)
+
+        instance = fields.ImageField(activitypub_field="cover", name="cover")
+
+        responses.add(
+            responses.GET,
+            "http://www.example.com/image.jpg",
+            body=image.tobytes(),
+            status=200,
+        )
+        book = Edition.objects.create(title="hello")
+
+        MockActivity = namedtuple("MockActivity", ("cover"))
+        mock_activity = MockActivity("http://www.example.com/image.jpg")
+
+        instance.set_field_from_activity(book, mock_activity, overwrite=False)
+        self.assertIsNotNone(book.cover.name)
+        self.assertEqual(book.cover.size, 43200)
+
+    @responses.activate
+    def test_image_field_set_field_from_activity_no_overwrite_with_cover(self, *_):
+        """update a model instance from an activitypub object"""
+        image_file = pathlib.Path(__file__).parent.joinpath(
+            "../../static/images/default_avi.jpg"
+        )
+        image = Image.open(image_file)
+        output = BytesIO()
+        image.save(output, format=image.format)
+
+        another_image_file = pathlib.Path(__file__).parent.joinpath(
+            "../../static/images/logo.png"
+        )
+        another_image = Image.open(another_image_file)
+        another_output = BytesIO()
+        another_image.save(another_output, format=another_image.format)
+
+        instance = fields.ImageField(activitypub_field="cover", name="cover")
+
+        responses.add(
+            responses.GET,
+            "http://www.example.com/image.jpg",
+            body=another_image.tobytes(),
+            status=200,
+        )
+        book = Edition.objects.create(title="hello")
+        book.cover.save("test.jpg", ContentFile(output.getvalue()))
+        self.assertEqual(book.cover.size, 2136)
+
+        MockActivity = namedtuple("MockActivity", ("cover"))
+        mock_activity = MockActivity("http://www.example.com/image.jpg")
+
+        instance.set_field_from_activity(book, mock_activity, overwrite=False)
+        # same cover as before
+        self.assertEqual(book.cover.size, 2136)
+
+    @responses.activate
+    def test_image_field_set_field_from_activity_with_overwrite_with_cover(self, *_):
+        """update a model instance from an activitypub object"""
+        image_file = pathlib.Path(__file__).parent.joinpath(
+            "../../static/images/default_avi.jpg"
+        )
+        image = Image.open(image_file)
+        output = BytesIO()
+        image.save(output, format=image.format)
+        book = Edition.objects.create(title="hello")
+        book.cover.save("test.jpg", ContentFile(output.getvalue()))
+        self.assertEqual(book.cover.size, 2136)
+
+        another_image_file = pathlib.Path(__file__).parent.joinpath(
+            "../../static/images/logo.png"
+        )
+        another_image = Image.open(another_image_file)
+        another_output = BytesIO()
+        another_image.save(another_output, format=another_image.format)
+
+        instance = fields.ImageField(activitypub_field="cover", name="cover")
+
+        responses.add(
+            responses.GET,
+            "http://www.example.com/image.jpg",
+            body=another_image.tobytes(),
+            status=200,
+        )
+
+        MockActivity = namedtuple("MockActivity", ("cover"))
+        mock_activity = MockActivity("http://www.example.com/image.jpg")
+
+        instance.set_field_from_activity(book, mock_activity, overwrite=True)
+        # new cover
+        self.assertIsNotNone(book.cover.name)
+        self.assertEqual(book.cover.size, 376800)
+
     def test_datetime_field(self, *_):
         """this one is pretty simple, it just has to use isoformat"""
         instance = fields.DateTimeField()

bookwyrm/utils/isni.py

@@ -0,0 +1,183 @@
+"""ISNI author checking utilities"""
+import xml.etree.ElementTree as ET
+import requests
+
+from bookwyrm import activitypub, models
+
+
+def request_isni_data(search_index, search_term, max_records=5):
+    """Request data from the ISNI API"""
+
+    search_string = f'{search_index}="{search_term}"'
+    query_params = {
+        "query": search_string,
+        "version": "1.1",
+        "operation": "searchRetrieve",
+        "recordSchema": "isni-b",
+        "maximumRecords": max_records,
+        "startRecord": "1",
+        "recordPacking": "xml",
+        "sortKeys": "RLV,pica,0,,",
+    }
+    result = requests.get("http://isni.oclc.org/sru/", params=query_params, timeout=10)
+    # the OCLC ISNI server asserts the payload is encoded
+    # in latin1, but we know better
+    result.encoding = "utf-8"
+    return result.text
+
+
+def make_name_string(element):
+    """create a string of form 'personal_name surname'"""
+
+    # NOTE: this will often be incorrect, many naming systems
+    # list "surname" before personal name
+    forename = element.find(".//forename")
+    surname = element.find(".//surname")
+    if forename is not None:
+        return "".join([forename.text, " ", surname.text])
+    return surname.text
+
+
+def get_other_identifier(element, code):
+    """Get other identifiers associated with an author from their ISNI record"""
+
+    identifiers = element.findall(".//otherIdentifierOfIdentity")
+    for section_head in identifiers:
+        if (
+            section_head.find(".//type") is not None
+            and section_head.find(".//type").text == code
+            and section_head.find(".//identifier") is not None
+        ):
+            return section_head.find(".//identifier").text
+
+    # if we can't find it in otherIdentifierOfIdentity,
+    # try sources
+    for source in element.findall(".//sources"):
+        code_of_source = source.find(".//codeOfSource")
+        if code_of_source is not None and code_of_source.text.lower() == code.lower():
+            return source.find(".//sourceIdentifier").text
+
+    return ""
+
+
+def get_external_information_uri(element, match_string):
+    """Get URLs associated with an author from their ISNI record"""
+
+    sources = element.findall(".//externalInformation")
+    for source in sources:
+        information = source.find(".//information")
+        uri = source.find(".//URI")
+        if (
+            uri is not None
+            and information is not None
+            and information.text.lower() == match_string.lower()
+        ):
+            return uri.text
+    return ""
+
+
+def find_authors_by_name(name_string, description=False):
+    """Query the ISNI database for possible author matches by name"""
+
+    payload = request_isni_data("pica.na", name_string)
+    # parse xml
+    root = ET.fromstring(payload)
+    # build list of possible authors
+    possible_authors = []
+    for element in root.iter("responseRecord"):
+        personal_name = element.find(".//forename/..")
+        if not personal_name:
+            continue
+
+        author = get_author_from_isni(element.find(".//isniUnformatted").text)
+
+        if bool(description):
+
+            titles = []
+            # prefer title records from LoC+ coop, Australia, Ireland, or Singapore
+            # in that order
+            for source in ["LCNACO", "NLA", "N6I", "NLB"]:
+                for parent in element.findall(f'.//titleOfWork/[@source="{source}"]'):
+                    titles.append(parent.find(".//title"))
+                for parent in element.findall(f'.//titleOfWork[@subsource="{source}"]'):
+                    titles.append(parent.find(".//title"))
+            # otherwise just grab the first title listing
+            titles.append(element.find(".//title"))
+
+            if titles is not None:
+                # some of the "titles" in ISNI are a little ...iffy
+                # '@' is used by ISNI/OCLC to index the starting point ignoring stop words
+                # (e.g. "The @Government of no one")
+                title_elements = [
+                    e for e in titles if not e.text.replace("@", "").isnumeric()
+                ]
+                if len(title_elements):
+                    author.bio = title_elements[0].text.replace("@", "")
+                else:
+                    author.bio = None
+
+        possible_authors.append(author)
+
+    return possible_authors
+
+
+def get_author_from_isni(isni):
+    """Find data to populate a new author record from their ISNI"""
+
+    payload = request_isni_data("pica.isn", isni)
+    # parse xml
+    root = ET.fromstring(payload)
+    # there should only be a single responseRecord
+    # but let's use the first one just in case
+    element = root.find(".//responseRecord")
+    name = make_name_string(element.find(".//forename/.."))
+    viaf = get_other_identifier(element, "viaf")
+    # use a set to dedupe aliases in ISNI
+    aliases = set()
+    aliases_element = element.findall(".//personalNameVariant")
+    for entry in aliases_element:
+        aliases.add(make_name_string(entry))
+    # aliases needs to be list not set
+    aliases = list(aliases)
+    bio = element.find(".//nameTitle")
+    bio = bio.text if bio is not None else ""
+    wikipedia = get_external_information_uri(element, "Wikipedia")
+
+    author = activitypub.Author(
+        id=element.find(".//isniURI").text,
+        name=name,
+        isni=isni,
+        viafId=viaf,
+        aliases=aliases,
+        bio=bio,
+        wikipediaLink=wikipedia,
+    )
+
+    return author
+
+
+def build_author_from_isni(match_value):
+    """Build basic author class object from ISNI URL"""
+
+    # if it is an isni value get the data
+    if match_value.startswith("https://isni.org/isni/"):
+        isni = match_value.replace("https://isni.org/isni/", "")
+        return {"author": get_author_from_isni(isni)}
+    # otherwise it's a name string
+    return {}
+
+
+def augment_author_metadata(author, isni):
+    """Update any missing author fields from ISNI data"""
+
+    isni_author = get_author_from_isni(isni)
+    isni_author.to_model(model=models.Author, instance=author, overwrite=False)
+
+    # we DO want to overwrite aliases because we're adding them to the
+    # existing aliases and ISNI will usually have more.
+    # We need to dedupe because ISNI records often have lots of dupe aliases
+    aliases = set(isni_author.aliases)
+    for alias in author.aliases:
+        aliases.add(alias)
+    author.aliases = list(aliases)
+    author.save()

bookwyrm/views/books/edit_book.py

@@ -1,4 +1,5 @@
 """ the good stuff! the books! """
+from re import sub
 from dateutil.parser import parse as dateparse
 from django.contrib.auth.decorators import login_required, permission_required
 from django.contrib.postgres.search import SearchRank, SearchVector
@@ -11,10 +12,16 @@ from django.utils.decorators import method_decorator
 from django.views import View
 
 from bookwyrm import book_search, forms, models
+
+# from bookwyrm.activitypub.base_activity import ActivityObject
+from bookwyrm.utils.isni import (
+    find_authors_by_name,
+    build_author_from_isni,
+    augment_author_metadata,
+)
 from bookwyrm.views.helpers import get_edition
 from .books import set_cover_from_url
 
-
 # pylint: disable=no-self-use
 @method_decorator(login_required, name="dispatch")
 @method_decorator(
@@ -33,6 +40,7 @@ class EditBook(View):
         data = {"book": book, "form": forms.EditionForm(instance=book)}
         return TemplateResponse(request, "book/edit/edit_book.html", data)
 
+    # pylint: disable=too-many-locals
     def post(self, request, book_id=None):
         """edit a book cool"""
         # returns None if no match is found
@@ -43,12 +51,14 @@ class EditBook(View):
         if not form.is_valid():
             return TemplateResponse(request, "book/edit/edit_book.html", data)
 
-        add_author = request.POST.get("add_author")
-        # we're adding an author through a free text field
+        # filter out empty author fields
+        add_author = [author for author in request.POST.getlist("add_author") if author]
         if add_author:
             data["add_author"] = add_author
             data["author_matches"] = []
-            for author in add_author.split(","):
+            data["isni_matches"] = []
+
+            for author in add_author:
                 if not author:
                     continue
                 # check for existing authors
@@ -56,15 +66,35 @@ class EditBook(View):
                     "aliases", weight="B"
                 )
 
+                author_matches = (
+                    models.Author.objects.annotate(search=vector)
+                    .annotate(rank=SearchRank(vector, author))
+                    .filter(rank__gt=0.4)
+                    .order_by("-rank")[:5]
+                )
+
+                isni_authors = find_authors_by_name(
+                    author, description=True
+                )  # find matches from ISNI API
+
+                # dedupe isni authors we already have in the DB
+                exists = [
+                    i
+                    for i in isni_authors
+                    for a in author_matches
+                    if sub(r"\D", "", str(i.isni)) == sub(r"\D", "", str(a.isni))
+                ]
+
+                # pylint: disable=cell-var-from-loop
+                matches = list(filter(lambda x: x not in exists, isni_authors))
+                # combine existing and isni authors
+                matches.extend(author_matches)
+
                 data["author_matches"].append(
                     {
                         "name": author.strip(),
-                        "matches": (
-                            models.Author.objects.annotate(search=vector)
-                            .annotate(rank=SearchRank(vector, author))
-                            .filter(rank__gt=0.4)
-                            .order_by("-rank")[:5]
-                        ),
+                        "matches": matches,
+                        "existing_isnis": exists,
                     }
                 )
 
@@ -122,6 +152,8 @@ class EditBook(View):
 class ConfirmEditBook(View):
     """confirm edits to a book"""
 
+    # pylint: disable=too-many-locals
+    # pylint: disable=too-many-branches
     def post(self, request, book_id=None):
         """edit a book cool"""
         # returns None if no match is found
@@ -147,9 +179,25 @@ class ConfirmEditBook(View):
                     author = get_object_or_404(
                         models.Author, id=request.POST[f"author_match-{i}"]
                     )
+                    # update author metadata if the ISNI record is more complete
+                    isni = request.POST.get(f"isni-for-{match}", None)
+                    if isni is not None:
+                        augment_author_metadata(author, isni)
                 except ValueError:
-                    # otherwise it's a name
-                    author = models.Author.objects.create(name=match)
+                    # otherwise it's a new author
+                    isni_match = request.POST.get(f"author_match-{i}")
+                    author_object = build_author_from_isni(isni_match)
+                    # with author data class from isni id
+                    if "author" in author_object:
+                        skeleton = models.Author.objects.create(
+                            name=author_object["author"].name
+                        )
+                        author = author_object["author"].to_model(
+                            model=models.Author, overwrite=True, instance=skeleton
+                        )
+                    else:
+                        # or it's just a name
+                        author = models.Author.objects.create(name=match)
                 book.authors.add(author)
 
             # create work, if needed

bookwyrm/views/reading.py

@@ -9,6 +9,7 @@ from django.views import View
 from django.views.decorators.http import require_POST
 
 from bookwyrm import models
+from bookwyrm.views.shelf.shelf_actions import unshelve
 from .status import CreateStatus
 from .helpers import get_edition, handle_reading_status, is_api_request
 from .helpers import load_date_in_user_tz_as_utc
@@ -16,6 +17,7 @@ from .helpers import load_date_in_user_tz_as_utc
 
 @method_decorator(login_required, name="dispatch")
 # pylint: disable=no-self-use
+# pylint: disable=too-many-return-statements
 class ReadingStatus(View):
     """consider reading a book"""
 
@@ -89,8 +91,21 @@ class ReadingStatus(View):
             privacy = request.POST.get("privacy")
             handle_reading_status(request.user, desired_shelf, book, privacy)
 
+        # if the request includes a "shelf" value we are using the 'move' button
+        if bool(request.POST.get("shelf")):
+            # unshelve the existing shelf
+            this_shelf = request.POST.get("shelf")
+            if (
+                bool(current_status_shelfbook)
+                and int(this_shelf) != int(current_status_shelfbook.shelf.id)
+                and current_status_shelfbook.shelf.identifier
+                != desired_shelf.identifier
+            ):
+                return unshelve(request, book_id=book_id)
+
         if is_api_request(request):
             return HttpResponse()
+
         return redirect(referer)
 
 

bookwyrm/views/shelf/shelf_actions.py

@@ -91,13 +91,13 @@ def shelve(request):
 
 @login_required
 @require_POST
-def unshelve(request):
+def unshelve(request, book_id=False):
     """remove a book from a user's shelf"""
-    book = get_object_or_404(models.Edition, id=request.POST.get("book"))
+    identity = book_id if book_id else request.POST.get("book")
+    book = get_object_or_404(models.Edition, id=identity)
     shelf_book = get_object_or_404(
         models.ShelfBook, book=book, shelf__id=request.POST["shelf"]
     )
     shelf_book.raise_not_deletable(request.user)
-
     shelf_book.delete()
     return redirect(request.headers.get("Referer", "/"))

bookwyrm/views/status.py

@@ -54,6 +54,7 @@ class CreateStatus(View):
         data = {"book": book}
         return TemplateResponse(request, "compose.html", data)
 
+    # pylint: disable=too-many-branches
     def post(self, request, status_type, existing_status_id=None):
         """create status of whatever type"""
         created = not existing_status_id
@@ -117,11 +118,12 @@ class CreateStatus(View):
 
         status.save(created=created)
 
-        # update a readthorugh, if needed
-        try:
-            edit_readthrough(request)
-        except Http404:
-            pass
+        # update a readthrough, if needed
+        if bool(request.POST.get("id")):
+            try:
+                edit_readthrough(request)
+            except Http404:
+                pass
 
         if is_api_request(request):
             return HttpResponse()