code style cleanup

fedireads/__init__.py

@@ -1,3 +1,4 @@
+''' we need this file to initialize celery '''
 from __future__ import absolute_import, unicode_literals
 
 # This will make sure the app is always imported when

fedireads/broadcast.py

@@ -13,7 +13,12 @@ from fedireads import models
 
 def get_recipients(user, post_privacy, direct_recipients=None, limit=False):
     ''' deduplicated list of recipient inboxes '''
-    recipients = direct_recipients or []
+    # we're always going to broadcast to any direct recipients
+    direct_recipients = direct_recipients or []
+    recipients = [u.inbox for u in direct_recipients]
+
+    # if we're federating a book, it isn't related to any user's followers, we
+    # just want to send it out. To whom? I'm not sure, but for now, everyone.
     if not user:
         users = models.User.objects.filter(local=False).all()
         recipients += list(set(
@@ -22,16 +27,19 @@ def get_recipients(user, post_privacy, direct_recipients=None, limit=False):
         return recipients
 
     if post_privacy == 'direct':
-        # all we care about is direct_recipients, not followers
-        return [u.inbox for u in recipients]
+        # all we care about is direct_recipients, not followers, so we're done
+        return recipients
 
     # load all the followers of the user who is sending the message
+    # "limit" refers to whether we want to send to other fedireads instances,
+    # or to only non-fedireads instances. this is confusing (TODO)
     if not limit:
         followers = user.followers.all()
     else:
         fedireads_user = limit == 'fedireads'
         followers = user.followers.filter(fedireads_user=fedireads_user).all()
 
+    # TODO I don't think this is actually accomplishing pubic/followers only?
     if post_privacy == 'public':
         # post to public shared inboxes
         shared_inboxes = set(
@@ -39,11 +47,12 @@ def get_recipients(user, post_privacy, direct_recipients=None, limit=False):
         )
         recipients += list(shared_inboxes)
         recipients += [u.inbox for u in followers if not u.shared_inbox]
-        # TODO: direct to anyone who's mentioned
+
     if post_privacy == 'followers':
         # don't send it to the shared inboxes
         inboxes = set(u.inbox for u in followers)
         recipients += list(inboxes)
+
     return recipients
 
 

fedireads/goodreads_import.py

@@ -1,3 +1,4 @@
+''' handle reading a csv from goodreads '''
 import re
 import csv
 import itertools
@@ -5,22 +6,28 @@ from requests import HTTPError
 
 from fedireads import books_manager
 
+
 # Mapping goodreads -> fedireads shelf titles.
 GOODREADS_SHELVES = {
-        'read': 'read',
-        'currently-reading': 'reading',
-        'to-read': 'to-read',
+    'read': 'read',
+    'currently-reading': 'reading',
+    'to-read': 'to-read',
 }
+# TODO: remove or notify about this in the UI
 MAX_ENTRIES = 20
 
+
 def unquote_string(text):
+    ''' resolve csv quote weirdness '''
     match = re.match(r'="([^"]*)"', text)
     if match:
         return match.group(1)
     else:
         return text
 
+
 def construct_search_term(title, author):
+    ''' formulate a query for the data connector '''
     # Strip brackets (usually series title from search term)
     title = re.sub(r'\s*\([^)]*\)\s*', '', title)
     # Open library doesn't like including author initials in search term.
@@ -28,7 +35,9 @@ def construct_search_term(title, author):
 
     return ' '.join([title, author])
 
+
 class GoodreadsCsv(object):
+    ''' define a goodreads csv '''
     def __init__(self, csv_file):
         self.reader = csv.DictReader(csv_file)
 
@@ -41,30 +50,42 @@ class GoodreadsCsv(object):
                 pass
             yield entry
 
+
 class GoodreadsItem(object):
+    ''' a processed line in a goodreads csv '''
     def __init__(self, line):
         self.line = line
         self.book = None
 
+
     def resolve(self):
+        ''' try various ways to lookup a book '''
         self.book = self.get_book_from_isbn()
         if not self.book:
             self.book = self.get_book_from_title_author()
 
+
     def get_book_from_isbn(self):
+        ''' search by isbn '''
         isbn = unquote_string(self.line['ISBN13'])
         search_results = books_manager.search(isbn)
         if search_results:
             return books_manager.get_or_create_book(search_results[0].key)
 
+
     def get_book_from_title_author(self):
-        search_term = construct_search_term(self.line['Title'], self.line['Author'])
+        ''' search by title and author '''
+        search_term = construct_search_term(
+            self.line['Title'],
+            self.line['Author']
+        )
         search_results = books_manager.search(search_term)
         if search_results:
             return books_manager.get_or_create_book(search_results[0].key)
 
     @property
     def shelf(self):
+        ''' the goodreads shelf field '''
         if self.line['Exclusive Shelf']:
             return GOODREADS_SHELVES[self.line['Exclusive Shelf']]
 
@@ -73,3 +94,4 @@ class GoodreadsItem(object):
 
     def __str__(self):
         return "{} by {}".format(self.line['Title'], self.line['Author'])
+

fedireads/incoming.py

@@ -1,17 +1,16 @@
 ''' handles all of the activity coming in to the server '''
 from base64 import b64decode
+from Crypto.Hash import SHA256
 from Crypto.PublicKey import RSA
 from Crypto.Signature import pkcs1_15
-from Crypto.Hash import SHA256
-from django.http import HttpResponse, HttpResponseBadRequest, \
-    HttpResponseNotFound
-from django.views.decorators.csrf import csrf_exempt
 import django.db.utils
+from django.http import HttpResponse
+from django.http import HttpResponseBadRequest, HttpResponseNotFound
+from django.views.decorators.csrf import csrf_exempt
 import json
 import requests
 
-from fedireads import models
-from fedireads import outgoing
+from fedireads import models, outgoing
 from fedireads import status as status_builder
 from fedireads.remote_user import get_or_create_remote_user
 
@@ -35,23 +34,30 @@ def shared_inbox(request):
 
     handlers = {
         'Follow': handle_follow,
-        'Create': handle_create,
         'Accept': handle_follow_accept,
         'Reject': handle_follow_reject,
+        'Create': handle_create,
         'Like': handle_favorite,
         'Add': handle_add,
     }
     activity_type = activity['type']
 
+    handler = None
     if activity_type in handlers:
         handler = handlers[activity_type]
     elif activity_type == 'Undo' and 'object' in activity:
         if activity['object']['type'] == 'Follow':
-            handler = handle_undo
+            handler = handle_unfollow
         elif activity['object']['type'] == 'Like':
             handler = handle_unfavorite
+    elif activity_type == 'Update' and 'object' in activity:
+        if activity['object']['type'] == 'Person':
+            handler = None# TODO: handle_update_user
+        elif activity_type['object']['type'] == 'Book':
+            handler = None# TODO: handle_update_book
+
     if handler:
-        return handlers[activity_type](activity)
+        return handler(activity)
 
     return HttpResponseNotFound()
 
@@ -152,7 +158,7 @@ def handle_follow(activity):
     return HttpResponse()
 
 
-def handle_undo(activity):
+def handle_unfollow(activity):
     ''' unfollow a local user '''
     obj = activity['object']
     if not obj['type'] == 'Follow':
@@ -210,36 +216,25 @@ def handle_create(activity):
     if not 'object' in activity:
         return HttpResponseBadRequest()
 
-    # TODO: should only create notes if they are relevent to a book,
-    # so, not every single thing someone posts on mastodon
-    response = HttpResponse()
-    if activity['object'].get('fedireadsType') == 'Review' and \
+    if user.local:
+        # we really oughtn't even be sending in this case
+        return HttpResponse()
+
+    if activity['object'].get('fedireadsType') in ['Review', 'Comment']  and \
             'inReplyToBook' in activity['object']:
-        if user.local:
-            review_id = activity['object']['id'].split('/')[-1]
-            models.Review.objects.get(id=review_id)
-        else:
-            try:
-                status_builder.create_review_from_activity(
-                    user,
-                    activity['object']
-                )
-            except ValueError:
-                return HttpResponseBadRequest()
-    elif activity['object'].get('fedireadsType') == 'Comment' and \
-            'inReplyToBook' in activity['object']:
-        if user.local:
-            comment_id = activity['object']['id'].split('/')[-1]
-            models.Comment.objects.get(id=comment_id)
-        else:
-            try:
-                status_builder.create_comment_from_activity(
-                    user,
-                    activity['object']
-                )
-            except ValueError:
-                return HttpResponseBadRequest()
-    elif not user.local:
+        try:
+            if activity['object']['fedireadsType'] == 'Review':
+                builder = status_builder.create_review_from_activity
+            else:
+                builder = status_builder.create_comment_from_activity
+
+            # create the status, it'll throw a valueerror if anything is missing
+            builder(user, activity['object'])
+        except ValueError:
+            return HttpResponseBadRequest()
+    else:
+        # TODO: should only create notes if they are relevent to a book,
+        # so, not every single thing someone posts on mastodon
         try:
             status = status_builder.create_status_from_activity(
                 user,
@@ -255,7 +250,7 @@ def handle_create(activity):
         except ValueError:
             return HttpResponseBadRequest()
 
-    return response
+    return HttpResponse()
 
 
 def handle_favorite(activity):

fedireads/models/__init__.py

@@ -2,5 +2,5 @@
 from .book import Connector, Book, Work, Edition, Author
 from .shelf import Shelf, ShelfBook
 from .status import Status, Review, Comment, Favorite, Tag, Notification
-from .user import User, FederatedServer, UserFollows, UserFollowRequest, \
-    UserBlocks
+from .user import User, UserFollows, UserFollowRequest, UserBlocks
+from .user import FederatedServer

fedireads/outgoing.py

@@ -7,10 +7,10 @@ from urllib.parse import urlencode
 
 from fedireads import activitypub
 from fedireads import models
-from fedireads.status import create_review, create_status, create_tag, \
-    create_notification, create_comment
-from fedireads.remote_user import get_or_create_remote_user
 from fedireads.broadcast import get_recipients, broadcast
+from fedireads.status import create_review, create_status, create_comment
+from fedireads.status import create_tag, create_notification
+from fedireads.remote_user import get_or_create_remote_user
 
 
 @csrf_exempt
@@ -110,6 +110,7 @@ def handle_accept(user, to_follow, follow_request):
     recipient = get_recipients(to_follow, 'direct', direct_recipients=[user])
     broadcast(to_follow, activity, recipient)
 
+
 def handle_reject(user, to_follow, relationship):
     ''' a local user who managed follows rejects a follow request '''
     relationship.delete()

fedireads/remote_user.py

@@ -13,7 +13,7 @@ def get_or_create_remote_user(actor):
     except models.User.DoesNotExist:
         pass
 
-    # TODO: also bring in the user's prevous reviews and books
+    # TODO: handle remote server and connector
 
     # load the user's info from the actor url
     response = requests.get(
@@ -54,6 +54,7 @@ def get_or_create_remote_user(actor):
 
 def get_remote_reviews(user):
     ''' ingest reviews by a new remote fedireads user '''
+    # TODO: use the server as the data source instead of OL
     outbox_page = user.outbox + '?page=true'
     response = requests.get(
         outbox_page,

fedireads/status.py

@@ -1,8 +1,9 @@
 ''' Handle user activity '''
+from django.db import IntegrityError
+
 from fedireads import models
 from fedireads.books_manager import get_or_create_book
 from fedireads.sanitize_html import InputHtmlParser
-from django.db import IntegrityError
 
 
 def create_review_from_activity(author, activity):
@@ -113,9 +114,10 @@ def get_favorite(absolute_id):
 
 def get_by_absolute_id(absolute_id, model):
     ''' generalized function to get from a model with a remote_id field '''
-    # check if it's a remote status
     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:

fedireads/urls.py

@@ -11,6 +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\-]+)'
 
 urlpatterns = [
     path('admin/', admin.site.urls),
@@ -25,7 +26,9 @@ urlpatterns = [
     re_path(r'^.well-known/nodeinfo/?$', wellknown.nodeinfo_pointer),
     re_path(r'^nodeinfo/2\.0/?$', wellknown.nodeinfo),
     re_path(r'^api/v1/instance/?$', wellknown.instance_info),
+    re_path(r'^api/v1/instance/peers/?$', wellknown.peers),
     # TODO: re_path(r'^.well-known/host-meta/?$', incoming.host_meta),
+    # TODO: robots.txt
 
     # ui views
     re_path(r'^login/?$', views.login_page),
@@ -52,9 +55,9 @@ urlpatterns = [
     re_path(r'%s/replies(.json)?/?$' % status_path, views.replies_page),
 
     # books
-    re_path(r'^book/(?P<book_identifier>[\w\-]+)(.json)?/?$', views.book_page),
-    re_path(r'^book/(?P<book_identifier>[\w\-]+)/(?P<tab>friends|local|federated)?$', views.book_page),
-    re_path(r'^book/(?P<book_identifier>[\w\-]+)/edit/?$', views.edit_book_page),
+    re_path(r'%s(.json)?/?$' % book_path, 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'^author/(?P<author_identifier>[\w\-]+)/?$', views.author_page),
     re_path(r'^tag/(?P<tag_id>.+)/?$', views.tag_page),

fedireads/view_actions.py

@@ -354,12 +354,10 @@ def import_data(request):
                 failures.append(item)
 
         outgoing.handle_import_books(request.user, results)
-        if failures:
-            return TemplateResponse(request, 'import_results.html', {
-                'success_count': len(results),
-                'failures': failures,
-            })
-        else:
-            return redirect('/')
+        return TemplateResponse(request, 'import_results.html', {
+            'success_count': len(results),
+            'failures': failures,
+        })
     else:
         return HttpResponseBadRequest()
+

fedireads/views.py

@@ -21,7 +21,6 @@ def get_user_from_username(username):
 
 def is_api_request(request):
     ''' check whether a request is asking for html or data '''
-    # TODO: this should probably be the full content type? maybe?
     return 'json' in request.headers.get('Accept') or \
             request.path[-5:] == '.json'
 
@@ -369,8 +368,6 @@ def book_page(request, book_identifier, tab='friends'):
         'book', 'name', 'identifier'
     ).distinct().all()
 
-    review_form = forms.ReviewForm()
-    tag_form = forms.TagForm()
     data = {
         'book': book,
         'shelf': shelf,
@@ -381,8 +378,8 @@ def book_page(request, book_identifier, tab='friends'):
         'tags': tags,
         'user_tags': user_tags,
         'user_tag_names': user_tag_names,
-        'review_form': review_form,
-        'tag_form': tag_form,
+        'review_form': forms.ReviewForm(),
+        'tag_form': forms.TagForm(),
         'feed_tabs': [
             {'id': 'friends', 'display': 'Friends'},
             {'id': 'local', 'display': 'Local'},
@@ -434,7 +431,6 @@ def tag_page(request, tag_id):
 
 def shelf_page(request, username, shelf_identifier):
     ''' display a shelf '''
-    # TODO: json view
     try:
         user = get_user_from_username(username)
     except models.User.DoesNotExist:

fedireads/wellknown.py

@@ -44,12 +44,13 @@ def nodeinfo_pointer(request):
         ]
     })
 
+
 def nodeinfo(request):
     ''' basic info about the server '''
     if request.method != 'GET':
         return HttpResponseNotFound()
 
-    status_count = models.Status.objects.count()
+    status_count = models.Status.objects.filter(user__local=True).count()
     user_count = models.User.objects.count()
     return JsonResponse({
         "version": "2.0",
@@ -66,13 +67,12 @@ def nodeinfo(request):
                 "activeMonth": user_count, # TODO
                 "activeHalfyear": user_count, # TODO
             },
-            "localPosts": status_count, # TODO: mark local
+            "localPosts": status_count,
         },
         "openRegistrations": True,
     })
 
 
-
 def instance_info(request):
     ''' what this place is TODO: should be settable/editable '''
     if request.method != 'GET':
@@ -98,3 +98,13 @@ def instance_info(request):
         'registrations': True,
         'approval_required': False,
     })
+
+
+def peers(request):
+    ''' list of federated servers this instance connects with '''
+    if request.method != 'GET':
+        return HttpResponseNotFound()
+
+    # TODO
+    return JsonResponse([])
+

fedireads/wsgi.py

@@ -8,9 +8,7 @@ https://docs.djangoproject.com/en/2.0/howto/deployment/wsgi/
 """
 
 import os
-
 from environs import Env
-
 from django.core.wsgi import get_wsgi_application
 
 Env.read_env()