From 2fd7792f3415d0ac105da7cf26c8569adc13c765 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Sun, 3 May 2020 17:53:14 -0700
Subject: [PATCH] Remove fedireads_key field

we have ID
---
 fedireads/activitypub/book.py                 |  1 -
 fedireads/books_manager.py                    | 27 +++++--------------
 fedireads/connectors/abstract_connector.py    |  5 ++--
 fedireads/connectors/fedireads_connector.py   | 20 +++++++-------
 fedireads/connectors/self_connector.py        | 10 +++----
 fedireads/models/book.py                      |  2 --
 fedireads/outgoing.py                         |  2 +-
 fedireads/templates/book.html                 |  6 ++---
 fedireads/templates/book_results.html         |  2 +-
 fedireads/templates/edit_book.html            |  6 ++---
 fedireads/templates/editions.html             |  2 +-
 fedireads/templates/snippets/authors.html     |  2 +-
 .../templates/snippets/book_titleby.html      |  2 +-
 .../templates/snippets/covers_shelf.html      |  2 +-
 .../templates/snippets/create_status.html     |  6 ++---
 fedireads/templates/snippets/rate_action.html |  2 +-
 fedireads/templates/snippets/shelf.html       |  2 +-
 fedireads/templates/snippets/tag.html         |  4 +--
 fedireads/urls.py                             |  4 +--
 fedireads/view_actions.py                     |  4 +--
 fedireads/views.py                            | 24 +++++++++++------
 21 files changed, 65 insertions(+), 70 deletions(-)

diff --git a/fedireads/activitypub/book.py b/fedireads/activitypub/book.py
index cbaa7a04..80d8ff0f 100644
--- a/fedireads/activitypub/book.py
+++ b/fedireads/activitypub/book.py
@@ -11,7 +11,6 @@ def get_book(book):
         'oclc_number',
         'openlibrary_key',
         'librarything_key',
-        'fedireads_key',
         'lccn',
         'oclc_number',
         'pages',
diff --git a/fedireads/books_manager.py b/fedireads/books_manager.py
index 28effe7e..6f0d78cb 100644
--- a/fedireads/books_manager.py
+++ b/fedireads/books_manager.py
@@ -5,18 +5,17 @@ from fedireads import models
 from fedireads.tasks import app
 
 
-def get_or_create_book(key):
+def get_or_create_book(value, key='id', connector_id=None):
     ''' pull up a book record by whatever means possible '''
     try:
-        book = models.Book.objects.select_subclasses().get(
-            fedireads_key=key
-        )
+        book = models.Book.objects.select_subclasses().get(**{key: value})
         return book
     except models.Book.DoesNotExist:
         pass
 
-    connector = get_connector()
-    book = connector.get_or_create_book(key)
+    connector_info = models.Connector.objects.get(id=connector_id)
+    connector = load_connector(connector_info)
+    book = connector.get_or_create_book(value)
     load_more_data.delay(book.id)
     return book
 
@@ -25,7 +24,7 @@ def get_or_create_book(key):
 def load_more_data(book_id):
     ''' background the work of getting all 10,000 editions of LoTR '''
     book = models.Book.objects.select_subclasses().get(id=book_id)
-    connector = get_connector(book)
+    connector = load_connector(book.connector)
     connector.expand_book_data(book)
 
 
@@ -60,7 +59,7 @@ def first_search_result(query):
 
 def update_book(book):
     ''' re-sync with the original data source '''
-    connector = get_connector(book)
+    connector = load_connector(book.connector)
     connector.update_book(book)
 
 
@@ -70,18 +69,6 @@ def get_connectors():
     return [load_connector(c) for c in connectors_info]
 
 
-def get_connector(book=None):
-    ''' pick a book data connector '''
-    if book and book.connector:
-        connector_info = book.connector
-    else:
-        # only select from external connectors
-        connector_info = models.Connector.objects.filter(
-            local=False
-        ).order_by('priority').first()
-    return load_connector(connector_info)
-
-
 def load_connector(connector_info):
     ''' instantiate the connector class '''
     connector = importlib.import_module(
diff --git a/fedireads/connectors/abstract_connector.py b/fedireads/connectors/abstract_connector.py
index bba5cda8..fb28c3fc 100644
--- a/fedireads/connectors/abstract_connector.py
+++ b/fedireads/connectors/abstract_connector.py
@@ -22,12 +22,13 @@ class AbstractConnector(ABC):
         self.max_query_count = info.max_query_count
         self.name = info.name
         self.local = info.local
+        self.id = info.id
 
 
     def is_available(self):
         ''' check if you're allowed to use this connector '''
-        if self.connector.max_query_count is not None:
-            if self.connector.query_count >= self.connector.max_query_count:
+        if self.max_query_count is not None:
+            if self.connector.query_count >= self.max_query_count:
                 return False
         return True
 
diff --git a/fedireads/connectors/fedireads_connector.py b/fedireads/connectors/fedireads_connector.py
index ea06dbba..a83d2c93 100644
--- a/fedireads/connectors/fedireads_connector.py
+++ b/fedireads/connectors/fedireads_connector.py
@@ -9,6 +9,8 @@ from .abstract_connector import update_from_mappings, get_date
 
 
 class Connector(AbstractConnector):
+    ''' interact with other instances '''
+
     def search(self, query):
         ''' right now you can't search fedireads, but... '''
         resp = requests.get(
@@ -23,11 +25,11 @@ class Connector(AbstractConnector):
         return resp.json()
 
 
-    def get_or_create_book(self, fedireads_key):
+    def get_or_create_book(self, remote_id):
         ''' pull up a book record by whatever means possible '''
         try:
             book = models.Book.objects.select_subclasses().get(
-                fedireads_key=fedireads_key
+                remote_id=remote_id
             )
             return book
         except ObjectDoesNotExist:
@@ -35,14 +37,14 @@ class Connector(AbstractConnector):
                 # we can't load a book from a remote server, this is it
                 return None
             # no book was found, so we start creating a new one
-            book = models.Book(fedireads_key=fedireads_key)
+            book = models.Book(remote_id=remote_id)
 
 
     def update_book(self, book):
         ''' add remote data to a local book '''
-        fedireads_key = book.fedireads_key
+        remote_id = book.remote_id
         response = requests.get(
-            '%s/%s' % (self.base_url, fedireads_key),
+            '%s/%s' % (self.base_url, remote_id),
             headers={
                 'Accept': 'application/activity+json; charset=utf-8',
             },
@@ -81,21 +83,21 @@ class Connector(AbstractConnector):
         return book
 
 
-    def get_or_create_author(self, fedireads_key):
+    def get_or_create_author(self, remote_id):
         ''' load that author '''
         try:
-            return models.Author.objects.get(fedireads_key=fedireads_key)
+            return models.Author.objects.get(remote_id=remote_id)
         except ObjectDoesNotExist:
             pass
 
-        resp = requests.get('%s/authors/%s.json' % (self.url, fedireads_key))
+        resp = requests.get('%s/authors/%s.json' % (self.url, remote_id))
         if not resp.ok:
             resp.raise_for_status()
 
         data = resp.json()
 
         # ingest a new author
-        author = models.Author(fedireads_key=fedireads_key)
+        author = models.Author(remote_id=remote_id)
         mappings = {
             'born': ('born', get_date),
             'died': ('died', get_date),
diff --git a/fedireads/connectors/self_connector.py b/fedireads/connectors/self_connector.py
index e82b4f08..a91e8207 100644
--- a/fedireads/connectors/self_connector.py
+++ b/fedireads/connectors/self_connector.py
@@ -36,7 +36,7 @@ class Connector(AbstractConnector):
             search_results.append(
                 SearchResult(
                     book.title,
-                    book.fedireads_key,
+                    book.id,
                     book.author_text,
                     book.published_date.year if book.published_date else None,
                     None
@@ -45,21 +45,21 @@ class Connector(AbstractConnector):
         return search_results
 
 
-    def get_or_create_book(self, fedireads_key):
+    def get_or_create_book(self, book_id):
         ''' since this is querying its own data source, it can only
         get a book, not load one from an external source '''
         try:
             return models.Book.objects.select_subclasses().get(
-                fedireads_key=fedireads_key
+                id=book_id
             )
         except ObjectDoesNotExist:
             return None
 
 
-    def get_or_create_author(self, fedireads_key):
+    def get_or_create_author(self, author_id):
         ''' load that author '''
         try:
-            return models.Author.objects.get(fedreads_key=fedireads_key)
+            return models.Author.objects.get(id=author_id)
         except ObjectDoesNotExist:
             pass
 
diff --git a/fedireads/models/book.py b/fedireads/models/book.py
index 2585d92f..5f39ea97 100644
--- a/fedireads/models/book.py
+++ b/fedireads/models/book.py
@@ -51,7 +51,6 @@ class Connector(FedireadsModel):
 class Book(FedireadsModel):
     ''' a generic book, which can mean either an edition or a work '''
     # these identifiers apply to both works and editions
-    fedireads_key = models.CharField(max_length=255, unique=True, default=uuid4)
     openlibrary_key = models.CharField(max_length=255, blank=True, null=True)
     librarything_key = models.CharField(max_length=255, blank=True, null=True)
     goodreads_key = models.CharField(max_length=255, blank=True, null=True)
@@ -151,7 +150,6 @@ class Edition(Book):
 
 class Author(FedireadsModel):
     ''' copy of an author from OL '''
-    fedireads_key = models.CharField(max_length=255, unique=True, default=uuid4)
     openlibrary_key = models.CharField(max_length=255, blank=True, null=True)
     wikipedia_link = models.CharField(max_length=255, blank=True, null=True)
     # idk probably other keys would be useful here?
diff --git a/fedireads/outgoing.py b/fedireads/outgoing.py
index 717053a8..fc0e542c 100644
--- a/fedireads/outgoing.py
+++ b/fedireads/outgoing.py
@@ -286,7 +286,7 @@ def handle_tag(user, book, name):
 
 def handle_untag(user, book, name):
     ''' tag a book '''
-    book = models.Book.objects.get(fedireads_key=book)
+    book = models.Book.objects.get(id=book)
     tag = models.Tag.objects.get(name=name, book=book, user=user)
     tag_activity = activitypub.get_remove_tag(tag)
     tag.delete()
diff --git a/fedireads/templates/book.html b/fedireads/templates/book.html
index f4bd13ee..87a2f027 100644
--- a/fedireads/templates/book.html
+++ b/fedireads/templates/book.html
@@ -6,7 +6,7 @@
         {% include 'snippets/book_titleby.html' with book=book %}
 
         {% if request.user.is_authenticated %}
-        <a href="{{ book.fedireads_key }}/edit" class="edit-link">edit
+        <a href="{{ book.id }}/edit" class="edit-link">edit
             <span class="icon icon-pencil">
                 <span class="hidden-text">Edit Book</span>
             </span>
@@ -22,7 +22,7 @@
             {% include 'snippets/shelve_button.html' %}
 
             {% if request.user.is_authenticated and not book.cover %}
-            <form name="add-cover" method="POST" action="/upload_cover/{{book.id}}" enctype="multipart/form-data">
+            <form name="add-cover" method="POST" action="/upload_cover/{{ book.id }}" enctype="multipart/form-data">
                 {% csrf_token %}
                 {{ cover_form.as_p }}
                 <button type="submit">Add cover</button>
@@ -57,7 +57,7 @@
                 <h3>Tags</h3>
                 <form name="tag" action="/tag/" method="post">
                     {% csrf_token %}
-                    <input type="hidden" name="book" value="{{ book.fedireads_key }}">
+                    <input type="hidden" name="book" value="{{ book.id }}">
                     <input type="text" name="name">
                     <button type="submit">Add tag</button>
                 </form>
diff --git a/fedireads/templates/book_results.html b/fedireads/templates/book_results.html
index fdba919b..ed0c9b85 100644
--- a/fedireads/templates/book_results.html
+++ b/fedireads/templates/book_results.html
@@ -12,7 +12,7 @@
 
         {% for result in result_set.results %}
         <div>
-            <a href="/book/{{ result.key }}">{{ result.title }}</a> by {{ result.author }} ({{ result.year }})
+            <a href="/book/{% if not result_set.connector.local %}{{ result_set.connector.id }}:{{ result_set.connector.key_name}}:{% endif %}{{ result.key }}">{{ result.title }}</a> by {{ result.author }} ({{ result.year }})
         </div>
         {% endfor %}
     </section>
diff --git a/fedireads/templates/edit_book.html b/fedireads/templates/edit_book.html
index 053cecd6..d53409c9 100644
--- a/fedireads/templates/edit_book.html
+++ b/fedireads/templates/edit_book.html
@@ -4,7 +4,7 @@
 <div class="content-container">
     <h2>
         Edit "{{ book.title }}"
-        <a href="/book/{{ book.fedireads_key }}">
+        <a href="/book/{{ book.id }}">
             <span class="edit-link icon icon-close">
                 <span class="hidden-text">Close</span>
             </span>
@@ -40,8 +40,8 @@
 
     <h3>Book Identifiers</h2>
     <div>
-        <p><label for="id_isbn">ISBN 13:</label> {{ form.isbn_13 }} </p>
-        <p><label for="id_fedireads_key">Fedireads key:</label> {{ form.fedireads_key }} </p>
+        <p><label for="id_isbn_13">ISBN 13:</label> {{ form.isbn_13 }} </p>
+        <p><label for="id_isbn_10">ISBN 10:</label> {{ form.isbn_10 }} </p>
         <p><label for="id_openlibrary_key">Openlibrary key:</label> {{ form.openlibrary_key }} </p>
         <p><label for="id_librarything_key">Librarything key:</label> {{ form.librarything_key }} </p>
         <p><label for="id_goodreads_key">Goodreads key:</label> {{ form.goodreads_key }} </p>
diff --git a/fedireads/templates/editions.html b/fedireads/templates/editions.html
index d916ac11..21091b5f 100644
--- a/fedireads/templates/editions.html
+++ b/fedireads/templates/editions.html
@@ -2,7 +2,7 @@
 {% load fr_display %}
 {% block content %}
 <div class="content-container">
-    <h2>Editions of <a href="/book/{{ work.fedireads_key }}">"{{ work.title }}"</a></h2>
+    <h2>Editions of <a href="/book/{{ work.id }}">"{{ work.title }}"</a></h2>
     <ol class="book-grid row wrap">
         {% for book in editions %}
         <li class="book-preview">
diff --git a/fedireads/templates/snippets/authors.html b/fedireads/templates/snippets/authors.html
index 35fe6a03..e8106f5d 100644
--- a/fedireads/templates/snippets/authors.html
+++ b/fedireads/templates/snippets/authors.html
@@ -1 +1 @@
-<a href="/author/{{ book.authors.first.fedireads_key }}" class="author">{{ book.authors.first.name }}</a>
+<a href="/author/{{ book.authors.first.id }}" class="author">{{ book.authors.first.name }}</a>
diff --git a/fedireads/templates/snippets/book_titleby.html b/fedireads/templates/snippets/book_titleby.html
index 69d3da10..543c64c5 100644
--- a/fedireads/templates/snippets/book_titleby.html
+++ b/fedireads/templates/snippets/book_titleby.html
@@ -1,5 +1,5 @@
 <span class="title">
-    <a href="/book/{{ book.fedireads_key }}">{{ book.title }}</a>
+    <a href="/book/{{ book.id }}">{{ book.title }}</a>
 </span>
 {% if book.authors %}
 <span class="author">
diff --git a/fedireads/templates/snippets/covers_shelf.html b/fedireads/templates/snippets/covers_shelf.html
index f12cfcfd..0907ed0f 100644
--- a/fedireads/templates/snippets/covers_shelf.html
+++ b/fedireads/templates/snippets/covers_shelf.html
@@ -37,7 +37,7 @@
             <h2>
                 {% include 'snippets/avatar.html' with user=user %}
                 Your thoughts on
-            a   <a href="/book/{{ book.fedireads_key }}">{{ book.title }}</a>
+            a   <a href="/book/{{ book.id }}">{{ book.title }}</a>
                 by {% include 'snippets/authors.html' with book=book %}
             </h2>
 
diff --git a/fedireads/templates/snippets/create_status.html b/fedireads/templates/snippets/create_status.html
index 99a93446..0ae6e2f8 100644
--- a/fedireads/templates/snippets/create_status.html
+++ b/fedireads/templates/snippets/create_status.html
@@ -21,7 +21,7 @@
     {% endif %}
     <form class="tab-option-{{ book.id }} review-form" name="review" action="/review/" method="post" id="tab-review-{{ book.id }}">
         {% csrf_token %}
-        <input type="hidden" name="book" value="{{ book.fedireads_key }}">
+        <input type="hidden" name="book" value="{{ book.id }}">
         {% include 'snippets/rate_form.html' with book=book %}
         {{ review_form.as_p }}
         <button type="submit">post review</button>
@@ -29,14 +29,14 @@
 
     <form class="hidden tab-option-{{ book.id }} review-form" name="comment" action="/comment/" method="post" id="tab-comment-{{ book.id }}">
         {% csrf_token %}
-        <input type="hidden" name="book" value="{{ book.fedireads_key }}">
+        <input type="hidden" name="book" value="{{ book.id }}">
         {{ comment_form.as_p }}
         <button type="submit">post comment</button>
     </form>
 
     <form class="hidden tab-option-{{ book.id }} review-form quote-form" name="quotation" action="/quotate/" method="post" id="tab-quotation-{{ book.id }}">
         {% csrf_token %}
-        <input type="hidden" name="book" value="{{ book.fedireads_key }}">
+        <input type="hidden" name="book" value="{{ book.id }}">
         {{ quotation_form.as_p }}
         <button typr="submit">post quote</button>
     </form>
diff --git a/fedireads/templates/snippets/rate_action.html b/fedireads/templates/snippets/rate_action.html
index e6d1163a..50c44e83 100644
--- a/fedireads/templates/snippets/rate_action.html
+++ b/fedireads/templates/snippets/rate_action.html
@@ -4,7 +4,7 @@
     {% for i in '12345'|make_list %}
     <form name="rate" action="/rate/" method="POST" onsubmit="return rate_stars(event)">
         {% csrf_token %}
-        <input type="hidden" name="book" value="{{ book.fedireads_key }}">
+        <input type="hidden" name="book" value="{{ book.id }}">
         <input type="hidden" name="rating" value="{{ forloop.counter }}">
         <button type="submit" class="icon icon-star-{% if book|rating:user < forloop.counter %}empty{% else %}full{% endif %}">
             <span class="hidden-text">{{ forloop.counter }} star{{ forloop.counter | pluralize }}</span>
diff --git a/fedireads/templates/snippets/shelf.html b/fedireads/templates/snippets/shelf.html
index 9293fc59..990e11bc 100644
--- a/fedireads/templates/snippets/shelf.html
+++ b/fedireads/templates/snippets/shelf.html
@@ -39,7 +39,7 @@
         {% include 'snippets/book_cover.html' with book=book size="small" %}
     </td>
     <td>
-        <a href="/book/{{ book.fedireads_key }}">{{ book.title }}</a>
+        <a href="/book/{{ book.id }}">{{ book.title }}</a>
     </td>
     <td>
         {{ book.authors.first.name }}
diff --git a/fedireads/templates/snippets/tag.html b/fedireads/templates/snippets/tag.html
index cadc68ca..714d25dc 100644
--- a/fedireads/templates/snippets/tag.html
+++ b/fedireads/templates/snippets/tag.html
@@ -3,14 +3,14 @@
     {% if tag.identifier in user_tags %}
     <form class="tag-form" name="tag" action="/untag/" method="post">
         {% csrf_token %}
-        <input type="hidden" name="book" value="{{ book.fedireads_key }}">
+        <input type="hidden" name="book" value="{{ book.id }}">
         <input type="hidden" name="name" value="{{ tag.name }}">
         <button type="submit">x</button>
     </form>
     {% else %}
     <form class="tag-form" name="tag" action="/tag/" method="post">
         {% csrf_token %}
-        <input type="hidden" name="book" value="{{ book.fedireads_key }}">
+        <input type="hidden" name="book" value="{{ book.id }}">
         <input type="hidden" name="name" value="{{ tag.name }}">
         <button type="submit">+</button>
     </form>
diff --git a/fedireads/urls.py b/fedireads/urls.py
index 6574bbaa..f6a02bc8 100644
--- a/fedireads/urls.py
+++ b/fedireads/urls.py
@@ -11,7 +11,7 @@ localname_regex = r'(?P<username>[\w\-_]+)'
 user_path = r'^user/%s' % username_regex
 local_user_path = r'^user/%s' % localname_regex
 status_path = r'%s/(status|review|comment)/(?P<status_id>\d+)' % local_user_path
-book_path = r'^book/(?P<book_identifier>[\w\-]+)'
+book_path = r'^book/(?P<book_id>\d+)'
 
 handler404 = 'fedireads.views.not_found_page'
 handler500 = 'fedireads.views.server_error_page'
@@ -58,7 +58,7 @@ urlpatterns = [
     re_path(r'%s/replies(.json)?/?$' % status_path, views.replies_page),
 
     # books
-    re_path(r'%s(.json)?/?$' % book_path, views.book_page),
+    re_path(r'book/(?P<book_id>[\w_:\d]+)(.json)?/?$', views.book_page),
     re_path(r'%s/(?P<tab>friends|local|federated)?$' % book_path, views.book_page),
     re_path(r'%s/edit/?$' % book_path, views.edit_book_page),
     re_path(r'^editions/(?P<work_id>\d+)/?$', views.editions_page),
diff --git a/fedireads/view_actions.py b/fedireads/view_actions.py
index bf7918a3..98c71a9f 100644
--- a/fedireads/view_actions.py
+++ b/fedireads/view_actions.py
@@ -133,7 +133,7 @@ def edit_book(request, book_id):
     form.save()
 
     outgoing.handle_update_book(request.user, book)
-    return redirect('/book/%s' % book.fedireads_key)
+    return redirect('/book/%s' % book.id)
 
 
 @login_required
@@ -157,7 +157,7 @@ def upload_cover(request, book_id):
     book.save()
 
     outgoing.handle_update_book(request.user, book)
-    return redirect('/book/%s' % book.fedireads_key)
+    return redirect('/book/%s' % book.id)
 
 
 @login_required
diff --git a/fedireads/views.py b/fedireads/views.py
index 287ce095..46369672 100644
--- a/fedireads/views.py
+++ b/fedireads/views.py
@@ -8,8 +8,9 @@ from django.template.response import TemplateResponse
 from django.views.decorators.csrf import csrf_exempt
 
 from fedireads import activitypub
-from fedireads import forms, models, books_manager
+from fedireads import forms, models
 from fedireads import goodreads_import
+from fedireads.books_manager import get_or_create_book
 from fedireads.tasks import app
 
 
@@ -363,9 +364,16 @@ def edit_profile_page(request):
     return TemplateResponse(request, 'edit_user.html', data)
 
 
-def book_page(request, book_identifier, tab='friends'):
+def book_page(request, book_id, tab='friends'):
     ''' info about a book '''
-    book = books_manager.get_or_create_book(book_identifier)
+    key = 'id'
+    connector_id = None
+    if ':' in book_id:
+        try:
+            connector_id, key, book_id = book_id.split(':')
+        except ValueError:
+            return HttpResponseNotFound()
+    book = get_or_create_book(book_id, key=key, connector_id=connector_id)
 
     if is_api_request(request):
         return JsonResponse(activitypub.get_book(book))
@@ -430,7 +438,7 @@ def book_page(request, book_identifier, tab='friends'):
             {'id': 'federated', 'display': 'Federated'}
         ],
         'active_tab': tab,
-        'path': '/book/%s' % book_identifier,
+        'path': '/book/%s' % book_id,
         'cover_form': forms.CoverForm(instance=book),
         'info_fields': [
             {'name': 'ISBN', 'value': book.isbn_13},
@@ -445,9 +453,9 @@ def book_page(request, book_identifier, tab='friends'):
 
 
 @login_required
-def edit_book_page(request, book_identifier):
+def edit_book_page(request, book_id):
     ''' info about a book '''
-    book = books_manager.get_or_create_book(book_identifier)
+    book = get_or_create_book(book_id)
     if not book.description:
         book.description = book.parent_work.description
     data = {
@@ -468,10 +476,10 @@ def editions_page(request, work_id):
     return TemplateResponse(request, 'editions.html', data)
 
 
-def author_page(request, author_identifier):
+def author_page(request, author_id):
     ''' landing page for an author '''
     try:
-        author = models.Author.objects.get(fedireads_key=author_identifier)
+        author = models.Author.objects.get(id=author_id)
     except ValueError:
         return HttpResponseNotFound()