Cleans up status creation book lookup flow

fedireads/books_manager.py

@@ -1,5 +1,6 @@
 ''' select and call a connector for whatever book task needs doing '''
 import importlib
+from urllib.parse import urlparse
 
 from fedireads import models
 from fedireads.tasks import app
@@ -13,6 +14,13 @@ def get_or_create_book(value, key='id', connector_id=None):
     except models.Book.DoesNotExist:
         pass
 
+    if key == 'remote_id':
+        book = get_by_absolute_id(value, models.Book)
+        if book:
+            return book
+        connector = get_or_create_connector(value)
+        return connector.get_or_create_book(value)
+
     connector_info = models.Connector.objects.get(id=connector_id)
     connector = load_connector(connector_info)
     book = connector.get_or_create_book(value)
@@ -20,6 +28,58 @@ def get_or_create_book(value, key='id', connector_id=None):
     return book
 
 
+def get_or_create_connector(remote_id):
+    ''' get the connector related to the author's server '''
+    url = urlparse(remote_id)
+    identifier = url.netloc
+    if not identifier:
+        raise(ValueError)
+
+    try:
+        connector_info = models.Connector.objects.get(identifier=identifier)
+    except models.Connector.DoesNotExist:
+        connector_info = models.Connector.objects.create(
+            identifier=identifier,
+            connector_file='fedireads_connector',
+            base_url='https://%s' % identifier,
+            books_url='https://%s/book' % identifier,
+            covers_url='https://%s/images/covers' % identifier,
+            search_url='https://%s/search?q=' % identifier,
+            key_name='remote_id',
+            priority=3
+        )
+
+    return load_connector(connector_info)
+
+
+def get_by_absolute_id(absolute_id, model):
+    ''' generalized function to get from a model with a remote_id field '''
+    if not absolute_id:
+        return None
+
+    # check if it's a remote status
+    try:
+        return model.objects.get(remote_id=absolute_id)
+    except model.DoesNotExist:
+        pass
+
+    # try finding a local status with that id
+    local_id = absolute_id.split('/')[-1]
+    try:
+        if hasattr(model.objects, 'select_subclasses'):
+            possible_match = model.objects.select_subclasses().get(id=local_id)
+        else:
+            possible_match = model.objects.get(id=local_id)
+    except model.DoesNotExist:
+        return None
+
+    # make sure it's not actually a remote status with an id that
+    # clashes with a local id
+    if possible_match.absolute_id == absolute_id:
+        return possible_match
+    return None
+
+
 @app.task
 def load_more_data(book_id):
     ''' background the work of getting all 10,000 editions of LoTR '''

fedireads/connectors/fedireads_connector.py

@@ -102,6 +102,10 @@ class Connector(AbstractConnector):
         return author
 
 
+    def expand_book_data(self, book):
+        pass
+
+
 def get_cover(cover_url):
     ''' ask openlibrary for the cover '''
     image_name = cover_url.split('/')[-1]

fedireads/outgoing.py

@@ -7,7 +7,7 @@ from django.http import HttpResponseNotFound, JsonResponse
 from django.views.decorators.csrf import csrf_exempt
 import requests
 
-from fedireads import activitypub
+from fedireads import activitypub, books_manager
 from fedireads import models
 from fedireads.broadcast import broadcast
 from fedireads.status import create_review, create_status
@@ -260,9 +260,10 @@ def handle_comment(user, book, content):
         user, book, builder, fr_serializer, ap_serializer, content)
 
 
-def handle_status(user, book, \
+def handle_status(user, book_id, \
         builder, fr_serializer, ap_serializer, *args):
     ''' generic handler for statuses '''
+    book = books_manager.get_or_create_book(book_id)
     status = builder(user, book, *args)
 
     activity = fr_serializer(status)

fedireads/status.py

@@ -1,24 +1,21 @@
 ''' Handle user activity '''
-from urllib.parse import urlparse
-
 from django.db import IntegrityError
 
-from fedireads import books_manager, models
+from fedireads import models
+from fedireads.books_manager import get_or_create_book, get_by_absolute_id
 from fedireads.sanitize_html import InputHtmlParser
 
 
 def create_review_from_activity(author, activity):
     ''' parse an activity json blob into a status '''
     book_id = activity['inReplyToBook']
-    book_id = book_id.split('/')[-1]
+    book = get_or_create_book(book_id, key='remote_id')
     name = activity.get('name')
     rating = activity.get('rating')
     content = activity.get('content')
     published = activity.get('published')
     remote_id = activity['id']
 
-    book = get_or_create_book(book_id)
-
     review = create_review(author, book, name, content, rating)
     review.published_date = published
     review.remote_id = remote_id
@@ -26,41 +23,6 @@ def create_review_from_activity(author, activity):
     return review
 
 
-def get_or_create_book(remote_id):
-    ''' try the remote id and then figure out the right connector '''
-    book = get_by_absolute_id(remote_id, models.Book)
-    if book:
-        return book
-
-    connector = get_or_create_connector(remote_id)
-    return books_manager.get_or_create_book(
-        remote_id,
-        key=connector.key_name,
-        connector_id=connector.id
-    )
-
-
-def get_or_create_connector(remote_id):
-    ''' get the connector related to the author's server '''
-    url = urlparse(remote_id)
-    identifier = url.netloc
-    try:
-        connector_info = models.Connector.objects.get(identifier=identifier)
-    except models.Connector.DoesNotExist:
-        models.Connector.objects.create(
-            identifier=identifier,
-            connector_file='fedireads_connector',
-            base_url='https://%s' % identifier,
-            books_url='https://%s/book' % identifier,
-            covers_url='https://%s/images/covers' % identifier,
-            search_url='https://%s/search?q=' % identifier,
-            key_name='remote_id',
-            priority=3
-        )
-
-    return books_manager.load_connector(connector_info)
-
-
 def create_rating(user, book, rating):
     ''' a review that's just a rating '''
     if not rating or rating < 1 or rating > 5:
@@ -94,8 +56,8 @@ def create_review(user, book, name, content, rating):
 
 def create_quotation_from_activity(author, activity):
     ''' parse an activity json blob into a status '''
-    book = activity['inReplyToBook']
+    book_id = activity['inReplyToBook']
-    book = book.split('/')[-1]
+    book = get_or_create_book(book_id, key='remote_id')
     quote = activity.get('quote')
     content = activity.get('content')
     published = activity.get('published')
@@ -108,10 +70,9 @@ def create_quotation_from_activity(author, activity):
     return quotation
 
 
-def create_quotation(user, possible_book, content, quote):
+def create_quotation(user, book, content, quote):
     ''' a quotation has been added '''
     # throws a value error if the book is not found
-    book = get_or_create_book(possible_book)
     content = sanitize(content)
     quote = sanitize(quote)
 
@@ -123,11 +84,10 @@ def create_quotation(user, possible_book, content, quote):
     )
 
 
-
 def create_comment_from_activity(author, activity):
     ''' parse an activity json blob into a status '''
-    book = activity['inReplyToBook']
+    book_id = activity['inReplyToBook']
-    book = book.split('/')[-1]
+    book = get_or_create_book(book_id, key='remote_id')
     content = activity.get('content')
     published = activity.get('published')
     remote_id = activity['id']
@@ -139,10 +99,9 @@ def create_comment_from_activity(author, activity):
     return comment
 
 
-def create_comment(user, possible_book, content):
+def create_comment(user, book, content):
     ''' a book comment has been added '''
     # throws a value error if the book is not found
-    book = get_or_create_book(possible_book)
     content = sanitize(content)
 
     return models.Comment.objects.create(
@@ -206,34 +165,6 @@ def get_favorite(absolute_id):
     return get_by_absolute_id(absolute_id, models.Favorite)
 
 
-def get_by_absolute_id(absolute_id, model):
-    ''' generalized function to get from a model with a remote_id field '''
-    if not absolute_id:
-        return None
-
-    # check if it's a remote status
-    try:
-        return model.objects.get(remote_id=absolute_id)
-    except model.DoesNotExist:
-        pass
-
-    # try finding a local status with that id
-    local_id = absolute_id.split('/')[-1]
-    try:
-        if hasattr(model.objects, 'select_subclasses'):
-            possible_match = model.objects.select_subclasses().get(id=local_id)
-        else:
-            possible_match = model.objects.get(id=local_id)
-    except model.DoesNotExist:
-        return None
-
-    # make sure it's not actually a remote status with an id that
-    # clashes with a local id
-    if possible_match.absolute_id == absolute_id:
-        return possible_match
-    return None
-
-
 def create_status(user, content, reply_parent=None, mention_books=None,
                   remote_id=None):
     ''' a status update '''
@@ -260,7 +191,7 @@ def create_status(user, content, reply_parent=None, mention_books=None,
 
 def create_tag(user, possible_book, name):
     ''' add a tag to a book '''
-    book = get_or_create_book(possible_book)
+    book = get_or_create_book(possible_book, key='remote_id')
 
     try:
         tag = models.Tag.objects.create(name=name, book=book, user=user)

fedireads/view_actions.py

@@ -189,27 +189,25 @@ def shelve(request):
 def rate(request):
     ''' just a star rating for a book '''
     form = forms.RatingForm(request.POST)
-    book_identifier = request.POST.get('book')
+    book_id = request.POST.get('book')
     # TODO: better failure behavior
     if not form.is_valid():
-        return redirect('/book/%s' % book_identifier)
+        return redirect('/book/%s' % book_id)
 
     rating = form.cleaned_data.get('rating')
     # throws a value error if the book is not found
-    book = get_or_create_book(book_identifier)
 
-    outgoing.handle_rate(request.user, book, rating)
+    outgoing.handle_rate(request.user, book_id, rating)
-    return redirect('/book/%s' % book_identifier)
+    return redirect('/book/%s' % book_id)
 
 
 @login_required
 def review(request):
     ''' create a book review '''
     form = forms.ReviewForm(request.POST)
-    book_identifier = request.POST.get('book')
+    book_id = request.POST.get('book')
-    # TODO: better failure behavior
     if not form.is_valid():
-        return redirect('/book/%s' % book_identifier)
+        return redirect('/book/%s' % book_id)
 
     # TODO: validation, htmlification
     name = form.cleaned_data.get('name')
@@ -220,42 +218,39 @@ def review(request):
     except ValueError:
         rating = None
 
-    # throws a value error if the book is not found
+    outgoing.handle_review(request.user, book_id, name, content, rating)
-    book = get_or_create_book(book_identifier)
+    return redirect('/book/%s' % book_id)
-
-    outgoing.handle_review(request.user, book, name, content, rating)
-    return redirect('/book/%s' % book_identifier)
 
 
 @login_required
 def quotate(request):
     ''' create a book quotation '''
     form = forms.QuotationForm(request.POST)
-    book_identifier = request.POST.get('book')
+    book_id = request.POST.get('book')
     if not form.is_valid():
-        return redirect('/book/%s' % book_identifier)
+        return redirect('/book/%s' % book_id)
 
     quote = form.cleaned_data.get('quote')
     content = form.cleaned_data.get('content')
 
-    outgoing.handle_quotation(request.user, book_identifier, content, quote)
+    outgoing.handle_quotation(request.user, book_id, content, quote)
-    return redirect('/book/%s' % book_identifier)
+    return redirect('/book/%s' % book_id)
 
 
 @login_required
 def comment(request):
     ''' create a book comment '''
     form = forms.CommentForm(request.POST)
-    book_identifier = request.POST.get('book')
+    book_id = request.POST.get('book')
     # TODO: better failure behavior
     if not form.is_valid():
-        return redirect('/book/%s' % book_identifier)
+        return redirect('/book/%s' % book_id)
 
     # TODO: validation, htmlification
     content = form.data.get('content')
 
-    outgoing.handle_comment(request.user, book_identifier, content)
+    outgoing.handle_comment(request.user, book_id, content)
-    return redirect('/book/%s' % book_identifier)
+    return redirect('/book/%s' % book_id)
 
 
 @login_required
@@ -264,20 +259,20 @@ def tag(request):
     # I'm not using a form here because sometimes "name" is sent as a hidden
     # field which doesn't validate
     name = request.POST.get('name')
-    book_identifier = request.POST.get('book')
+    book_id = request.POST.get('book')
 
-    outgoing.handle_tag(request.user, book_identifier, name)
+    outgoing.handle_tag(request.user, book_id, name)
-    return redirect('/book/%s' % book_identifier)
+    return redirect('/book/%s' % book_id)
 
 
 @login_required
 def untag(request):
     ''' untag a book '''
     name = request.POST.get('name')
-    book_identifier = request.POST.get('book')
+    book_id = request.POST.get('book')
 
-    outgoing.handle_untag(request.user, book_identifier, name)
+    outgoing.handle_untag(request.user, book_id, name)
-    return redirect('/book/%s' % book_identifier)
+    return redirect('/book/%s' % book_id)
 
 
 @login_required