I did some more code-keeping in this commit. I added a lot of documentation, so

that most of my functions do indeed have effective docstrings. I also changed
the decorators so that they imply eachother in a logical way. I also modified
the one decorator get_media_comment_by_id to be more usable with the variable
urls of mediagoblin.user_pages.views:file_a_report. I also noticed a few tests
had broken, so I went through them and fixed them up, finding that mostly there
were problems in my actual writing of the tests. I also did a few other small
tasks such as creating a new User method to check whether or not a User is ban-
-ned.

===============================================================================
    Added in documentation
===============================================================================
--\  mediagoblin/db/models.py
--\  mediagoblin/decorators.py
--\  mediagoblin/moderation/forms.py
--\  mediagoblin/moderation/tools.py
--\  mediagoblin/moderation/views.py
--\  mediagoblin/user_pages/lib.py

===============================================================================
    Rearranged decorators to be more efficient
===============================================================================
--\  mediagoblin/decorators.py
--| Made it so that user_not_banned is encapsulated in require_active_login
--| Made it so that require_active_login is encapsulated in user_has_privilege
--| Changed get_media_comment_by_id into get_optional_media_comment_by_id. It
  | now returns valid code if the MediaComment id is absent. This makes it pos-
  | -sible to use this decorator for the function:
  |         mediagoblin.user_pages.views:file_a_report

--\  mediagoblin/user_pages/views.py
--| Replaced the mediagoblin.user_pages.views:file_a_comment_report with the
  | decorator mentioned above

--\  mediagoblin/user_pages/routing.py

        -----------------------------------------------------------
        |     took out unnecessary @user_not_banned decorators    |
        -----------------------------------------------------------
--\  mediagoblin/submit/views.py
--\  mediagoblin/user_pages/views.py

===============================================================================
    Fixed broken tests
===============================================================================
--\  mediagoblin/tests/test_auth.py
--\  mediagoblin/tests/test_privileges.py
--\  mediagoblin/tests/test_submission.py

===============================================================================
    Fixed broken code
===============================================================================
--\  mediagoblin/tools/response.py

===============================================================================
    Other Tasks
===============================================================================
--\  mediagoblin/db/models.py
--| Added in User.is_banned() method
--\  mediagoblin/decorators.py
--| Utitilized User.is_banned() method in the user_not_banned decorator

--\  mediagoblin/moderation/views.py
--| Made it impossible for an admin to ban themself.
--| Got rid of a vestigial print statement

--\  mediagoblin/templates/mediagoblin/base.html
--| Made it so the top panel does not show up for users that are banned.

--\  mediagoblin/templates/mediagoblin/moderation/user.html
--| Rearranged the javascript slightly

===============================================================================

1 files changed, 67 insertions, 42 deletions

diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py
index 1a8b124b..d5a10db1 100644
--- a/mediagoblin/decorators.py
+++ b/mediagoblin/decorators.py
@@ -31,11 +31,29 @@ from mediagoblin.tools.translate import pass_to_ugettext as _
 from mediagoblin.oauth.tools.request import decode_authorization_header
 from mediagoblin.oauth.oauth import GMGRequestValidator
 
+
+def user_not_banned(controller):
+    """
+    Requires that the user has not been banned. Otherwise redirects to the page
+    explaining why they have been banned
+    """
+    @wraps(controller)
+    def wrapper(request, *args, **kwargs):
+        if request.user:
+            if request.user.is_banned():
+                return render_user_banned(request)
+        return controller(request, *args, **kwargs)
+
+    return wrapper
+
+
 def require_active_login(controller):
     """
-    Require an active login from the user.
+    Require an active login from the user. If the user is banned, redirects to
+    the "You are Banned" page.
     """
     @wraps(controller)
+    @user_not_banned
     def new_controller_func(request, *args, **kwargs):
         if request.user and \
                 not request.user.has_privilege(u'active'):
@@ -55,29 +73,26 @@ def require_active_login(controller):
 
     return new_controller_func
 
-def active_user_from_url(controller):
-    """Retrieve User() from <user> URL pattern and pass in as url_user=...
-
-    Returns a 404 if no such active user has been found"""
-    @wraps(controller)
-    def wrapper(request, *args, **kwargs):
-        user = User.query.filter_by(username=request.matchdict['user']).first()
-        if user is None:
-            return render_404(request)
-
-        return controller(request, *args, url_user=user, **kwargs)
-
-    return wrapper
 
 def user_has_privilege(privilege_name):
+    """
+    Requires that a user have a particular privilege in order to access a page.
+    In order to require that a user have multiple privileges, use this
+    decorator twice on the same view. This decorator also makes sure that the
+    user is not banned, or else it redirects them to the "You are Banned" page.
+
+        :param privilege_name       A unicode object that is that represents
+                                        the privilege object. This object is
+                                        the name of the privilege, as assigned
+                                        in the Privilege.privilege_name column
+    """
 
     def user_has_privilege_decorator(controller):
         @wraps(controller)
+        @require_active_login
         def wrapper(request, *args, **kwargs):
             user_id = request.user.id
-            if UserBan.query.filter(UserBan.user_id==user_id).count():
-                return render_user_banned(request)
-            elif not request.user.has_privilege(privilege_name):
+            if not request.user.has_privilege(privilege_name):
                 raise Forbidden()
 
             return controller(request, *args, **kwargs)
@@ -86,6 +101,21 @@ def user_has_privilege(privilege_name):
     return user_has_privilege_decorator
 
 
+def active_user_from_url(controller):
+    """Retrieve User() from <user> URL pattern and pass in as url_user=...
+
+    Returns a 404 if no such active user has been found"""
+    @wraps(controller)
+    def wrapper(request, *args, **kwargs):
+        user = User.query.filter_by(username=request.matchdict['user']).first()
+        if user is None:
+            return render_404(request)
+
+        return controller(request, *args, url_user=user, **kwargs)
+
+    return wrapper
+
+
 def user_may_delete_media(controller):
     """
     Require user ownership of the MediaEntry to delete.
@@ -274,20 +304,31 @@ def allow_registration(controller):
 
     return wrapper
 
-def get_media_comment_by_id(controller):
+def get_optional_media_comment_by_id(controller):
     """
-    Pass in a MediaComment based off of a url component
+    Pass in a MediaComment based off of a url component. Because of this decor-
+    -ator's use in filing Media or Comment Reports, it has two valid outcomes.
+
+    :returns        The view function being wrapped with kwarg `comment` set to
+                        the MediaComment who's id is in the URL. If there is a
+                        comment id in the URL and if it is valid.
+    :returns        The view function being wrapped with kwarg `comment` set to
+                        None. If there is no comment id in the URL.
+    :returns        A 404 Error page, if there is a comment if in the URL and it
+                        is invalid.
     """
     @wraps(controller)
     def wrapper(request, *args, **kwargs):
-        comment = MediaComment.query.filter_by(
-                id=request.matchdict['comment']).first()
-        # Still no media?  Okay, 404.
-        if not comment:
-            return render_404(request)
+        if 'comment' in request.matchdict:
+            comment = MediaComment.query.filter_by(
+                    id=request.matchdict['comment']).first()
 
-        return controller(request, comment=comment, *args, **kwargs)
+            if comment is None:
+                return render_404(request)
 
+            return controller(request, comment=comment, *args, **kwargs)
+        else:
+            return controller(request, comment=None, *args, **kwargs)
     return wrapper
 
 
@@ -308,7 +349,7 @@ def auth_enabled(controller):
 
 def require_admin_or_moderator_login(controller):
     """
-    Require an login from an administrator or a moderator.
+    Require a login from an administrator or a moderator.
     """
     @wraps(controller)
     def new_controller_func(request, *args, **kwargs):
@@ -330,22 +371,6 @@ def require_admin_or_moderator_login(controller):
     return new_controller_func
 
 
-def user_not_banned(controller):
-    """
-    Requires that the user has not been banned. Otherwise redirects to the page
-    explaining why they have been banned
-    """
-    @wraps(controller)
-    def wrapper(request, *args, **kwargs):
-        if request.user:
-            user_banned = UserBan.query.get(request.user.id)
-            if user_banned:
-                return render_user_banned(request)
-        return controller(request, *args, **kwargs)
-
-    return wrapper
-
-
 
 def oauth_required(controller):
     """ Used to wrap API endpoints where oauth is required """