cleans up urls and views

fedireads/incoming.py

@@ -108,8 +108,11 @@ def inbox(request, username):
     ''' incoming activitypub events '''
     # TODO: should do some kind of checking if the user accepts
     # this action from the sender probably? idk
-    # but this will just throw an error if the user doesn't exist I guess
-    models.User.objects.get(localname=username)
+    # but this will just throw a 404 if the user doesn't exist
+    try:
+        models.User.objects.get(localname=username)
+    except models.User.DoesNotExist:
+        return HttpResponseNotFound()
 
     return shared_inbox(request)
 
@@ -120,7 +123,10 @@ def get_actor(request, username):
     if request.method != 'GET':
         return HttpResponseBadRequest()
 
-    user = models.User.objects.get(localname=username)
+    try:
+        user = models.User.objects.get(localname=username)
+    except models.User.DoesNotExist:
+        return HttpResponseNotFound()
     return JsonResponse(activitypub.get_actor(user))
 
 

fedireads/models/__init__.py

@@ -1,5 +1,5 @@
 ''' bring all the models into the app namespace '''
 from .book import Shelf, ShelfBook, Book, Author
 from .user import User, UserRelationship, FederatedServer
-from .activity import Status, Review, Favorite, Tag
+from .status import Status, Review, Favorite, Tag
 

fedireads/outgoing.py

@@ -15,10 +15,14 @@ from fedireads.broadcast import get_recipients, broadcast
 @csrf_exempt
 def outbox(request, username):
     ''' outbox for the requested user '''
-    user = models.User.objects.get(localname=username)
     if request.method != 'GET':
         return HttpResponseNotFound()
 
+    try:
+        user = models.User.objects.get(localname=username)
+    except models.User.DoesNotExist:
+        return HttpResponseNotFound()
+
     # paginated list of messages
     if request.GET.get('page'):
         limit = 20

fedireads/templates/user.html

@@ -16,7 +16,7 @@
 
         {% if is_self %}
         <div class="interaction">
-            <a href="/user/{{ user.localname }}/edit">Edit profile</a>
+            <a href="/user/edit">Edit profile</a>
         </div>
         {% endif %}
     </div>

fedireads/urls.py

@@ -4,30 +4,26 @@ from django.contrib import admin
 from django.urls import path, re_path
 
 from fedireads import incoming, outgoing, views, settings, wellknown
+from fedireads import view_actions as actions
 
+username_regex = r'[\w@\.-]+'
+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)/(?P<status_id>\d+)' % local_user_path
 
 urlpatterns = [
     path('admin/', admin.site.urls),
 
     # federation endpoints
     re_path(r'^inbox/?$', incoming.shared_inbox),
-    re_path(r'^user/(?P<username>\w+).json/?$', incoming.get_actor),
-    re_path(r'^user/(?P<username>\w+)/inbox/?$', incoming.inbox),
-    re_path(r'^user/(?P<username>\w+)/outbox/?$', outgoing.outbox),
-    re_path(r'^user/(?P<username>\w+)/followers/?$', incoming.get_followers),
-    re_path(r'^user/(?P<username>\w+)/following/?$', incoming.get_following),
-    re_path(
-        r'^user/(?P<username>\w+)/(status|review)/(?P<status_id>\d+)/?$',
-        incoming.get_status
-    ),
-    re_path(
-        r'^user/(?P<username>\w+)/(status|review)/(?P<status_id>\d+)/activity/?$',
-        incoming.get_status
-    ),
-    re_path(
-        r'^user/(?P<username>\w+)/(status|review)/(?P<status_id>\d+)/replies/?$',
-        incoming.get_replies
-    ),
+    re_path(r'%s.json/?$' % local_user_path, incoming.get_actor),
+    re_path(r'%s/inbox/?$' % local_user_path, incoming.inbox),
+    re_path(r'%s/outbox/?$' % local_user_path, outgoing.outbox),
+    re_path(r'%s/followers/?$' % local_user_path, incoming.get_followers),
+    re_path(r'%s/following/?$' % local_user_path, incoming.get_following),
+    re_path(r'%s(/activity/?)?$' % status_path, incoming.get_status),
+    re_path(r'%s/replies/?$' % status_path, incoming.get_replies),
 
     # .well-known endpoints
     re_path(r'^.well-known/webfinger/?$', wellknown.webfinger),
@@ -43,23 +39,24 @@ urlpatterns = [
     re_path(r'^login/?$', views.user_login),
     re_path(r'^logout/?$', views.user_logout),
     # this endpoint is both ui and fed depending on Accept type
-    re_path(r'^user/(?P<username>[\w@\.-]+)/?$', views.user_profile),
-    re_path(r'^user/(?P<username>\w+)/edit/?$', views.user_profile_edit),
+    re_path(r'%s/?$' % user_path, views.user_page),
+    re_path(r'%s/edit/?$' % user_path, views.edit_profile_page),
+    re_path(r'^user/edit/?$', views.edit_profile_page),
     re_path(r'^book/(?P<book_identifier>\w+)/?$', views.book_page),
     re_path(r'^author/(?P<author_identifier>\w+)/?$', views.author_page),
     re_path(r'^tag/(?P<tag_id>[\w-]+)/?$', views.tag_page),
-    re_path(r'^shelf/(?P<username>[\w@\.-]+)/(?P<shelf_identifier>[\w-]+)/?$', views.shelf_page),
+    re_path(r'^shelf/%s/(?P<shelf_identifier>[\w-]+)/?$' % username_regex, views.shelf_page),
 
     # internal action endpoints
-    re_path(r'^review/?$', views.review),
-    re_path(r'^tag/?$', views.tag),
-    re_path(r'^untag/?$', views.untag),
-    re_path(r'^comment/?$', views.comment),
-    re_path(r'^favorite/(?P<status_id>\d+)/?$', views.favorite),
-    re_path(r'^shelve/?$', views.shelve),
-    re_path(r'^follow/?$', views.follow),
-    re_path(r'^unfollow/?$', views.unfollow),
-    re_path(r'^search/?$', views.search),
-    re_path(r'^edit_profile/?$', views.edit_profile),
+    re_path(r'^review/?$', actions.review),
+    re_path(r'^tag/?$', actions.tag),
+    re_path(r'^untag/?$', actions.untag),
+    re_path(r'^comment/?$', actions.comment),
+    re_path(r'^favorite/(?P<status_id>\d+)/?$', actions.favorite),
+    re_path(r'^shelve/?$', actions.shelve),
+    re_path(r'^follow/?$', actions.follow),
+    re_path(r'^unfollow/?$', actions.unfollow),
+    re_path(r'^search/?$', actions.search),
+    re_path(r'^edit_profile/?$', actions.edit_profile),
 
 ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT)

fedireads/view_actions.py

@@ -0,0 +1,159 @@
+''' views for actions you can take in the application '''
+from django.contrib.auth.decorators import login_required
+from django.http import HttpResponseBadRequest
+from django.shortcuts import redirect
+from django.template.response import TemplateResponse
+import re
+
+from fedireads import forms, models, openlibrary, outgoing
+from fedireads.views import get_user_from_username
+
+
+@login_required
+def edit_profile(request):
+    ''' les get fancy with images '''
+    if not request.method == 'POST':
+        return redirect('/user/%s' % request.user.localname)
+
+    form = forms.EditUserForm(request.POST, request.FILES)
+    if not form.is_valid():
+        return redirect('/')
+
+    request.user.name = form.data['name']
+    if 'avatar' in form.files:
+        request.user.avatar = form.files['avatar']
+    request.user.summary = form.data['summary']
+    request.user.save()
+    return redirect('/user/%s' % request.user.localname)
+
+
+@login_required
+def shelve(request):
+    ''' put a book on a user's shelf '''
+    book = models.Book.objects.get(id=request.POST['book'])
+    desired_shelf = models.Shelf.objects.filter(
+        identifier=request.POST['shelf'],
+        user=request.user
+    ).first()
+
+    if request.POST.get('reshelve', True):
+        try:
+            current_shelf = models.Shelf.objects.get(
+                user=request.user,
+                book=book
+            )
+            outgoing.handle_unshelve(request.user, book, current_shelf)
+        except models.Shelf.DoesNotExist:
+            # this just means it isn't currently on the user's shelves
+            pass
+    outgoing.handle_shelve(request.user, book, desired_shelf)
+    return redirect('/')
+
+
+@login_required
+def review(request):
+    ''' create a book review note '''
+    form = forms.ReviewForm(request.POST)
+    book_identifier = request.POST.get('book')
+    # TODO: better failure behavior
+    if not form.is_valid():
+        return redirect('/book/%s' % book_identifier)
+
+    # TODO: validation, htmlification
+    name = form.data.get('name')
+    content = form.data.get('content')
+    rating = int(form.data.get('rating'))
+
+    outgoing.handle_review(request.user, book_identifier, name, content, rating)
+    return redirect('/book/%s' % book_identifier)
+
+
+@login_required
+def tag(request):
+    ''' tag a book '''
+    # 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')
+
+    outgoing.handle_tag(request.user, book_identifier, name)
+    return redirect('/book/%s' % book_identifier)
+
+
+@login_required
+def untag(request):
+    ''' untag a book '''
+    name = request.POST.get('name')
+    book_identifier = request.POST.get('book')
+
+    outgoing.handle_untag(request.user, book_identifier, name)
+    return redirect('/book/%s' % book_identifier)
+
+
+@login_required
+def comment(request):
+    ''' respond to a book review '''
+    form = forms.CommentForm(request.POST)
+    # this is a bit of a formality, the form is just one text field
+    if not form.is_valid():
+        return redirect('/')
+    parent_id = request.POST['parent']
+    parent = models.Status.objects.get(id=parent_id)
+    outgoing.handle_comment(request.user, parent, form.data['content'])
+    return redirect('/')
+
+
+@login_required
+def favorite(request, status_id):
+    ''' like a status '''
+    status = models.Status.objects.get(id=status_id)
+    outgoing.handle_outgoing_favorite(request.user, status)
+    return redirect(request.headers.get('Referer', '/'))
+
+
+@login_required
+def follow(request):
+    ''' follow another user, here or abroad '''
+    username = request.POST['user']
+    try:
+        to_follow = get_user_from_username(username)
+    except models.User.DoesNotExist:
+        return HttpResponseBadRequest()
+
+    outgoing.handle_outgoing_follow(request.user, to_follow)
+    user_slug = to_follow.localname if to_follow.localname \
+        else to_follow.username
+    return redirect('/user/%s' % user_slug)
+
+
+@login_required
+def unfollow(request):
+    ''' unfollow a user '''
+    username = request.POST['user']
+    try:
+        to_unfollow = get_user_from_username(username)
+    except models.User.DoesNotExist:
+        return HttpResponseBadRequest()
+
+    outgoing.handle_outgoing_unfollow(request.user, to_unfollow)
+    user_slug = to_unfollow.localname if to_unfollow.localname \
+        else to_unfollow.username
+    return redirect('/user/%s' % user_slug)
+
+
+@login_required
+def search(request):
+    ''' that search bar up top '''
+    query = request.GET.get('q')
+    if re.match(r'\w+@\w+.\w+', query):
+        # if something looks like a username, search with webfinger
+        results = [outgoing.handle_account_search(query)]
+        template = 'user_results.html'
+    else:
+        # just send the question over to openlibrary for book search
+        results = openlibrary.book_search(query)
+        template = 'book_results.html'
+
+    return TemplateResponse(request, template, {'results': results})
+
+

fedireads/views.py

@@ -1,16 +1,24 @@
-''' application views/pages '''
+''' views for pages you can go to in the application '''
 from django.contrib.auth import authenticate, login, logout
 from django.contrib.auth.decorators import login_required
 from django.db.models import Avg, Q
-from django.http import HttpResponseBadRequest, HttpResponseNotFound
+from django.http import HttpResponseNotFound
 from django.shortcuts import redirect
 from django.template.response import TemplateResponse
-import re
 
-from fedireads import forms, models, openlibrary, outgoing, incoming
+from fedireads import forms, models, openlibrary, incoming
 from fedireads.settings import DOMAIN
 
 
+def get_user_from_username(username):
+    ''' helper function to resolve a localname or a username to a user '''
+    try:
+        user = models.User.objects.get(localname=username)
+    except models.User.DoesNotExist:
+        user = models.User.objects.get(username=username)
+    return user
+
+
 @login_required
 def home(request):
     ''' this is the same as the feed on the home tab '''
@@ -133,7 +141,7 @@ def register(request):
     return redirect('/')
 
 
-def user_profile(request, username):
+def user_page(request, username):
     ''' profile page for a user '''
     content = request.headers.get('Accept')
     # TODO: this should probably be the full content type? maybe?
@@ -160,17 +168,9 @@ def user_profile(request, username):
     }
     return TemplateResponse(request, 'user.html', data)
 
-def get_user_from_username(username):
-    ''' resolve a localname or a username to a user '''
-    try:
-        user = models.User.objects.get(localname=username)
-    except models.User.DoesNotExist:
-        user = models.User.objects.get(username=username)
-    return user
-
 
 @login_required
-def user_profile_edit(request, username):
+def edit_profile_page(request, username):
     ''' profile page for a user '''
     try:
         user = models.User.objects.get(localname=username)
@@ -185,26 +185,7 @@ def user_profile_edit(request, username):
     return TemplateResponse(request, 'edit_user.html', data)
 
 
-# TODO: there oughta be clear naming between endpoints and pages
-@login_required
-def edit_profile(request):
-    ''' les get fancy with images '''
-    if not request.method == 'POST':
-        return redirect('/user/%s' % request.user.localname)
 
-    form = forms.EditUserForm(request.POST, request.FILES)
-    if not form.is_valid():
-        return redirect('/')
-
-    request.user.name = form.data['name']
-    if 'avatar' in form.files:
-        request.user.avatar = form.files['avatar']
-    request.user.summary = form.data['summary']
-    request.user.save()
-    return redirect('/user/%s' % request.user.localname)
-
-
-@login_required
 def book_page(request, book_identifier):
     ''' info about a book '''
     book = openlibrary.get_or_create_book(book_identifier)
@@ -234,7 +215,6 @@ def book_page(request, book_identifier):
     return TemplateResponse(request, 'book.html', data)
 
 
-@login_required
 def author_page(request, author_identifier):
     ''' landing page for an author '''
     try:
@@ -276,133 +256,3 @@ def shelf_page(request, username, shelf_identifier):
     }
     return TemplateResponse(request, 'shelf.html', data)
 
-
-@login_required
-def shelve(request):
-    ''' put a book on a user's shelf '''
-    book = models.Book.objects.get(id=request.POST['book'])
-    desired_shelf = models.Shelf.objects.filter(
-        identifier=request.POST['shelf'],
-        user=request.user
-    ).first()
-
-    if request.POST.get('reshelve', True):
-        try:
-            current_shelf = models.Shelf.objects.get(
-                user=request.user,
-                book=book
-            )
-            outgoing.handle_unshelve(request.user, book, current_shelf)
-        except models.Shelf.DoesNotExist:
-            # this just means it isn't currently on the user's shelves
-            pass
-    outgoing.handle_shelve(request.user, book, desired_shelf)
-    return redirect('/')
-
-
-@login_required
-def review(request):
-    ''' create a book review note '''
-    form = forms.ReviewForm(request.POST)
-    book_identifier = request.POST.get('book')
-    # TODO: better failure behavior
-    if not form.is_valid():
-        return redirect('/book/%s' % book_identifier)
-
-    # TODO: validation, htmlification
-    name = form.data.get('name')
-    content = form.data.get('content')
-    rating = int(form.data.get('rating'))
-
-    outgoing.handle_review(request.user, book_identifier, name, content, rating)
-    return redirect('/book/%s' % book_identifier)
-
-
-@login_required
-def tag(request):
-    ''' tag a book '''
-    # 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')
-
-    outgoing.handle_tag(request.user, book_identifier, name)
-    return redirect('/book/%s' % book_identifier)
-
-
-@login_required
-def untag(request):
-    ''' untag a book '''
-    name = request.POST.get('name')
-    book_identifier = request.POST.get('book')
-
-    outgoing.handle_untag(request.user, book_identifier, name)
-    return redirect('/book/%s' % book_identifier)
-
-
-@login_required
-def comment(request):
-    ''' respond to a book review '''
-    form = forms.CommentForm(request.POST)
-    # this is a bit of a formality, the form is just one text field
-    if not form.is_valid():
-        return redirect('/')
-    parent_id = request.POST['parent']
-    parent = models.Status.objects.get(id=parent_id)
-    outgoing.handle_comment(request.user, parent, form.data['content'])
-    return redirect('/')
-
-
-@login_required
-def favorite(request, status_id):
-    ''' like a status '''
-    status = models.Status.objects.get(id=status_id)
-    outgoing.handle_outgoing_favorite(request.user, status)
-    return redirect(request.headers.get('Referer', '/'))
-
-
-@login_required
-def follow(request):
-    ''' follow another user, here or abroad '''
-    username = request.POST['user']
-    try:
-        to_follow = get_user_from_username(username)
-    except models.User.DoesNotExist:
-        return HttpResponseBadRequest()
-
-    outgoing.handle_outgoing_follow(request.user, to_follow)
-    user_slug = to_follow.localname if to_follow.localname \
-        else to_follow.username
-    return redirect('/user/%s' % user_slug)
-
-
-@login_required
-def unfollow(request):
-    ''' unfollow a user '''
-    username = request.POST['user']
-    try:
-        to_unfollow = get_user_from_username(username)
-    except models.User.DoesNotExist:
-        return HttpResponseBadRequest()
-
-    outgoing.handle_outgoing_unfollow(request.user, to_unfollow)
-    user_slug = to_unfollow.localname if to_unfollow.localname \
-        else to_unfollow.username
-    return redirect('/user/%s' % user_slug)
-
-
-@login_required
-def search(request):
-    ''' that search bar up top '''
-    query = request.GET.get('q')
-    if re.match(r'\w+@\w+.\w+', query):
-        # if something looks like a username, search with webfinger
-        results = [outgoing.handle_account_search(query)]
-        template = 'user_results.html'
-    else:
-        # just send the question over to openlibrary for book search
-        results = openlibrary.book_search(query)
-        template = 'book_results.html'
-
-    return TemplateResponse(request, template, {'results': results})
-