#934: first pass on API errors handling

misago/markup/api.py

@@ -1,4 +1,3 @@
-from rest_framework import status
 from rest_framework.decorators import api_view
 from rest_framework.response import Response
 
@@ -10,11 +9,7 @@ from .serializers import MarkupSerializer
 def parse_markup(request):
     serializer = MarkupSerializer(data=request.data)
     if not serializer.is_valid():
-        errors_list = list(serializer.errors.values())[0]
-        return Response(
-            {'detail': errors_list[0]},
-            status=status.HTTP_400_BAD_REQUEST,
-        )
+        return Response(serializer.errors, status=400)
 
     parsing_result = common_flavour(
         request,

misago/threads/api/attachments.py

@@ -17,11 +17,7 @@ class AttachmentViewSet(viewsets.ViewSet):
     def create(self, request):
         if not request.user.acl_cache['max_attachment_size']:
             raise PermissionDenied(_("You don't have permission to upload new files."))
-
-        try:
-            return self.create_attachment(request)
-        except ValidationError as e:
-            return Response({'detail': e.args[0]}, status=400)
+        return self.create_attachment(request)
 
     def create_attachment(self, request):
         upload = request.FILES.get('upload')

misago/threads/api/pollvotecreateendpoint.py

@@ -24,11 +24,7 @@ def poll_vote_create(request, thread, poll):
 
     if not serializer.is_valid():
         return Response(
-            {
-                'detail': serializer.errors['choices'][0],
-            },
-            status=400,
-        )
+            {'detail': serializer.errors}, status=400)
 
     remove_user_votes(request.user, poll, serializer.data['choices'])
     set_new_votes(request, poll, serializer.data['choices'])
@@ -58,7 +54,10 @@ def remove_user_votes(user, poll, final_votes):
             removed_votes.append(choice['hash'])
 
     if removed_votes:
-        poll.pollvote_set.filter(voter=user, choice_hash__in=removed_votes).delete()
+        poll.pollvote_set.filter(
+            voter=user,
+            choice_hash__in=removed_votes,
+        ).delete()
 
 
 def set_new_votes(request, poll, final_votes):

misago/threads/api/postendpoints/delete.py

@@ -36,11 +36,7 @@ def delete_bulk(request, thread):
     )
 
     if not serializer.is_valid():
-        if 'posts' in serializer.errors:
-            errors = serializer.errors['posts']
-        else:
-            errors = list(serializer.errors.values())[0]
-        return Response({'detail': errors[0]}, status=400)
+        return Response(serializer.errors, status=400)
 
     for post in serializer.validated_data['posts']:
         post.delete()

misago/threads/api/postendpoints/edits.py

@@ -86,7 +86,7 @@ def get_edit(post, pk=None):
 
     edit = post.edits_record.first()
     if not edit:
-        raise PermissionDenied(_("Edits record is unavailable for this post."))
+        raise Http404(_("Edits record is unavailable for this post."))
     return edit
 
 

misago/threads/api/postendpoints/merge.py

@@ -20,12 +20,7 @@ def posts_merge_endpoint(request, thread):
     )
 
     if not serializer.is_valid():
-        return Response(
-            {
-                'detail': list(serializer.errors.values())[0][0],
-            },
-            status=400,
-        )
+        return Response(serializer.errors, status=400)
 
     posts = serializer.validated_data['posts']
     first_post, merged_posts = posts[0], posts[1:]

misago/threads/api/postendpoints/move.py

@@ -20,11 +20,7 @@ def posts_move_endpoint(request, thread, viewmodel):
     )
 
     if not serializer.is_valid():
-        if 'new_thread' in serializer.errors:
-            errors = serializer.errors['new_thread']
-        else:
-            errors = list(serializer.errors.values())[0]
-        return Response({'detail': errors[0]}, status=400)
+        return Response(serializer.errors, status=400)
 
     new_thread = serializer.validated_data['new_thread']
 

misago/threads/api/postendpoints/split.py

@@ -22,14 +22,7 @@ def posts_split_endpoint(request, thread):
     )
 
     if not serializer.is_valid():
-        if 'posts' in serializer.errors:
-            errors = {
-                'detail': serializer.errors['posts'][0]
-            }
-        else :
-            errors = serializer.errors
-
-        return Response(errors, status=400)
+        return Response(serializer.errors, status=400)
 
     split_posts_to_new_thread(request, thread, serializer.validated_data)
 

misago/threads/api/threadendpoints/delete.py

@@ -27,15 +27,7 @@ def delete_bulk(request, viewmodel):
     )
 
     if not serializer.is_valid():
-        if 'threads' in serializer.errors:
-            errors = serializer.errors['threads']
-            if 'details' in errors:
-                return Response(
-                    hydrate_error_details(errors['details']), status=400)
-            return Response({'detail': errors[0]}, status=403)
-        else:
-            errors = list(serializer.errors)[0][0]
-            return Response({'detail': errors}, status=400)
+        return Response(serializer.errors, status=400)
 
     for thread in serializer.validated_data['threads']:
         with transaction.atomic():

misago/threads/api/threadendpoints/merge.py

@@ -27,13 +27,7 @@ def thread_merge_endpoint(request, thread, viewmodel):
     )
 
     if not serializer.is_valid():
-        if 'other_thread' in serializer.errors:
-            errors = serializer.errors['other_thread']
-        elif 'poll' in serializer.errors:
-            errors = serializer.errors['poll']
-        else:
-            errors = list(serializer.errors.values())[0]
-        return Response({'detail': errors[0]}, status=400)
+        return Response(serializer.errors, status=400)
 
     # interrupt merge with request for poll resolution?
     if serializer.validated_data.get('polls'):
@@ -81,14 +75,7 @@ def threads_merge_endpoint(request):
     )
 
     if not serializer.is_valid():
-        if 'threads' in serializer.errors:
-            errors = {'detail': serializer.errors['threads'][0]}
-            return Response(errors, status=403)
-        elif 'non_field_errors' in serializer.errors:
-            errors = {'detail': serializer.errors['non_field_errors'][0]}
-            return Response(errors, status=403)
-        else:
-            return Response(serializer.errors, status=400)
+        return Response(serializer.errors, status=400)
 
     threads = serializer.validated_data['threads']
     invalid_threads = []

misago/threads/api/threadendpoints/patch.py

@@ -49,12 +49,9 @@ def patch_title(request, thread, value):
     try:
         value_cleaned = six.text_type(value).strip()
     except (TypeError, ValueError):
-        raise PermissionDenied(_("Invalid thread title."))
+        raise ValidationError(_("Invalid thread title."))
 
-    try:
-        validate_title(value_cleaned)
-    except ValidationError as e:
-        raise PermissionDenied(e.args[0])
+    validate_title(value_cleaned)
 
     allow_edit_thread(request.user, thread)
 
@@ -101,7 +98,7 @@ def patch_move(request, thread, value):
     allow_start_thread(request.user, new_category)
 
     if new_category == thread.category:
-        raise PermissionDenied(_("You can't move thread to the category it's already in."))
+        raise ValidationError(_("You can't move thread to the category it's already in."))
 
     moderation.move_thread(request, thread, new_category)
 
@@ -204,13 +201,13 @@ def patch_add_participant(request, thread, value):
     try:
         username = six.text_type(value).strip().lower()
         if not username:
-            raise PermissionDenied(_("You have to enter new participant's username."))
+            raise ValidationError(_("You have to enter new participant's username."))
         participant = UserModel.objects.get(slug=username)
     except UserModel.DoesNotExist:
-        raise PermissionDenied(_("No user with such name exists."))
+        raise ValidationError(_("No user with such name exists."))
 
     if participant in [p.user for p in thread.participants_list]:
-        raise PermissionDenied(_("This user is already thread participant."))
+        raise ValidationError(_("This user is already thread participant."))
 
     allow_add_participant(request.user, participant)
     add_participant(request, thread, participant)
@@ -234,7 +231,7 @@ def patch_remove_participant(request, thread, value):
         if participant.user_id == user_id:
             break
     else:
-        raise PermissionDenied(_("Participant doesn't exist."))
+        raise ValidationError(_("Participant doesn't exist."))
 
     allow_remove_participant(request.user, thread, participant.user)
     remove_participant(request, thread, participant.user)
@@ -263,11 +260,11 @@ def patch_replace_owner(request, thread, value):
     for participant in thread.participants_list:
         if participant.user_id == user_id:
             if participant.is_owner:
-                raise PermissionDenied(_("This user already is thread owner."))
+                raise ValidationError(_("This user already is thread owner."))
             else:
                 break
     else:
-        raise PermissionDenied(_("Participant doesn't exist."))
+        raise ValidationError(_("Participant doesn't exist."))
 
     allow_change_owner(request.user, thread)
     change_owner(request, thread, participant.user)
@@ -371,10 +368,7 @@ def bulk_patch_endpoint(request, viewmodel):
 def clean_threads_for_patch(request, viewmodel, threads_ids):
     threads = []
     for thread_id in sorted(set(threads_ids), reverse=True):
-        try:
-            threads.append(viewmodel(request, thread_id).unwrap())
-        except (Http404, PermissionDenied):
-            raise PermissionDenied(_("One or more threads to update could not be found."))
+        threads.append(viewmodel(request, thread_id).unwrap())
     return threads
 
 

misago/users/api/auth.py

@@ -1,4 +1,3 @@
-from rest_framework import status
 from rest_framework.decorators import api_view, permission_classes
 from rest_framework.response import Response
 
@@ -46,10 +45,7 @@ def login(request):
             AuthenticatedUserSerializer(form.user_cache).data,
         )
     else:
-        return Response(
-            form.get_errors_dict(),
-            status=status.HTTP_400_BAD_REQUEST,
-        )
+        return Response(form.get_errors_dict(), status=400)
 
 
 @api_view()
@@ -119,7 +115,7 @@ def send_activation(request):
     else:
         return Response(
             form.get_errors_dict(),
-            status=status.HTTP_400_BAD_REQUEST,
+            status=400,
         )
 
 
@@ -158,10 +154,7 @@ def send_password_form(request):
             'email': form.user_cache.email,
         })
     else:
-        return Response(
-            form.get_errors_dict(),
-            status=status.HTTP_400_BAD_REQUEST,
-        )
+        return Response(form.get_errors_dict(), status=400)
 
 
 class PasswordChangeFailed(Exception):
@@ -196,24 +189,11 @@ def change_forgotten_password(request, pk, token):
         if get_user_ban(user):
             raise PasswordChangeFailed(expired_message)
     except PasswordChangeFailed as e:
-        return Response(
-            {
-                'detail': e.args[0],
-            },
-            status=status.HTTP_400_BAD_REQUEST,
-        )
+        return Response({'detail': e.args}, status=400)
 
-    try:
-        new_password = request.data.get('password', '')
-        validate_password(new_password, user=user)
-        user.set_password(new_password)
-        user.save()
-    except ValidationError as e:
-        return Response(
-            {
-                'detail': e.messages[0],
-            },
-            status=status.HTTP_400_BAD_REQUEST,
-        )
+    new_password = request.data.get('password', '')
+    validate_password(new_password, user=user)
+    user.set_password(new_password)
+    user.save()
 
     return Response({'username': user.username})

misago/users/api/userendpoints/avatar.py

@@ -1,6 +1,5 @@
 import json
 
-from rest_framework import status
 from rest_framework.response import Response
 
 from django.core.exceptions import ValidationError
@@ -18,16 +17,16 @@ from misago.users.serializers import ModerateAvatarSerializer
 def avatar_endpoint(request, pk=None):
     if request.user.is_avatar_locked:
         if request.user.avatar_lock_user_message:
-            reason = format_plaintext_for_html(request.user.avatar_lock_user_message)
+            extra = format_plaintext_for_html(request.user.avatar_lock_user_message)
         else:
-            reason = None
+            extra = None
 
         return Response(
             {
                 'detail': _("Your avatar is locked. You can't change it."),
-                'reason': reason,
+                'extra': extra,
             },
-            status=status.HTTP_403_FORBIDDEN,
+            status=403,
         )
 
     avatar_options = get_avatar_options(request.user)
@@ -110,7 +109,7 @@ def avatar_post(options, user, data):
                 {
                     'detail': _("This avatar type is not allowed."),
                 },
-                status=status.HTTP_400_BAD_REQUEST,
+                status=400,
             )
 
         rpc_handler = AVATAR_TYPES[data.get('avatar', 'nope')]
@@ -119,7 +118,7 @@ def avatar_post(options, user, data):
             {
                 'detail': _("Unknown avatar type."),
             },
-            status=status.HTTP_400_BAD_REQUEST,
+            status=400,
         )
 
     try:
@@ -129,7 +128,7 @@ def avatar_post(options, user, data):
             {
                 'detail': e.args[0],
             },
-            status=status.HTTP_400_BAD_REQUEST,
+            status=400,
         )
 
     user.save()
@@ -225,7 +224,7 @@ def moderate_avatar_endpoint(request, profile):
         else:
             return Response(
                 serializer.errors,
-                status=status.HTTP_400_BAD_REQUEST,
+                status=400,
             )
     else:
         return Response({

misago/users/api/userendpoints/changeemail.py

@@ -1,4 +1,3 @@
-from rest_framework import status
 from rest_framework.response import Response
 
 from django.utils.translation import ugettext as _
@@ -15,20 +14,19 @@ def change_email_endpoint(request, pk=None):
         context={'user': request.user},
     )
 
-    if serializer.is_valid():
-        token = store_new_credential(request, 'email', serializer.validated_data['new_email'])
+    if not serializer.is_valid():
+        return Response(serializer.errors, status=400)
 
-        mail_subject = _("Confirm e-mail change on %(forum_name)s forums")
-        mail_subject = mail_subject % {'forum_name': settings.forum_name}
+    token = store_new_credential(request, 'email', serializer.validated_data['new_email'])
 
-        # swap address with new one so email is sent to new address
-        request.user.email = serializer.validated_data['new_email']
+    mail_subject = _("Confirm e-mail change on %(forum_name)s forums")
+    mail_subject = mail_subject % {'forum_name': settings.forum_name}
 
-        mail_user(
-            request, request.user, mail_subject, 'misago/emails/change_email', {'token': token}
-        )
+    # swap address with new one so email is sent to new address
+    request.user.email = serializer.validated_data['new_email']
 
-        message = _("E-mail change confirmation link was sent to new address.")
-        return Response({'detail': message})
-    else:
-        return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
+    mail_user(
+        request, request.user, mail_subject, 'misago/emails/change_email', {'token': token}
+    )
+
+    return Response(status=204)

misago/users/api/userendpoints/changepassword.py

@@ -1,4 +1,3 @@
-from rest_framework import status
 from rest_framework.response import Response
 
 from django.utils.translation import ugettext as _
@@ -15,20 +14,18 @@ def change_password_endpoint(request, pk=None):
         context={'user': request.user},
     )
 
-    if serializer.is_valid():
-        token = store_new_credential(
-            request, 'password', serializer.validated_data['new_password']
-        )
+    if not serializer.is_valid():
+        return Response(serializer.errors, status=400)
 
-        mail_subject = _("Confirm password change on %(forum_name)s forums")
-        mail_subject = mail_subject % {'forum_name': settings.forum_name}
+    token = store_new_credential(
+        request, 'password', serializer.validated_data['new_password']
+    )
+
+    mail_subject = _("Confirm password change on %(forum_name)s forums")
+    mail_subject = mail_subject % {'forum_name': settings.forum_name}
 
-        mail_user(
-            request, request.user, mail_subject, 'misago/emails/change_password', {'token': token}
-        )
+    mail_user(
+        request, request.user, mail_subject, 'misago/emails/change_password', {'token': token}
+    )
 
-        return Response({
-            'detail': _("Password change confirmation link was sent to your address.")
-        })
-    else:
-        return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
+    return Response(status=204)

misago/users/api/userendpoints/create.py

@@ -1,4 +1,3 @@
-from rest_framework import status
 from rest_framework.response import Response
 
 from django.contrib.auth import authenticate, get_user_model, login
@@ -31,7 +30,7 @@ def create_endpoint(request):
         form.add_error('captcha', e)
 
     if not form.is_valid():
-        return Response(form.errors, status=status.HTTP_400_BAD_REQUEST)
+        return Response(form.errors, status=400)
 
     activation_kwargs = {}
     if settings.account_activation == 'user':
@@ -51,9 +50,9 @@ def create_endpoint(request):
     except IntegrityError:
         return Response(
             {
-                '__all__': _("Please try resubmitting the form.")
+                'detail': _("Please try resubmitting the form."),
             },
-            status=status.HTTP_400_BAD_REQUEST,
+            status=400,
         )
 
     mail_subject = _("Welcome on %(forum_name)s forums!")

misago/users/api/userendpoints/signature.py

@@ -1,4 +1,3 @@
-from rest_framework import status
 from rest_framework.response import Response
 
 from django.core.exceptions import PermissionDenied
@@ -18,15 +17,17 @@ def signature_endpoint(request):
 
     if user.is_signature_locked:
         if user.signature_lock_user_message:
-            reason = format_plaintext_for_html(user.signature_lock_user_message)
+            extra = format_plaintext_for_html(user.signature_lock_user_message)
         else:
-            reason = None
+            extra = None
 
-        return Response({
-            'detail': _("Your signature is locked. You can't change it."),
-            'reason': reason
-        },
-                        status=status.HTTP_403_FORBIDDEN)
+        return Response(
+            {
+                'detail': _("Your signature is locked. You can't change it."),
+                'extra': extra
+            },
+            status=403,
+        )
 
     if request.method == 'POST':
         return edit_signature(request, user)
@@ -59,7 +60,9 @@ def edit_signature(request, user):
         user.save(update_fields=['signature', 'signature_parsed', 'signature_checksum'])
         return get_signature_options(user)
     else:
-        return Response({
-            'detail': serializer.errors['non_field_errors'][0]
-        },
-                        status=status.HTTP_400_BAD_REQUEST)
+        return Response(
+            {
+                'detail': serializer.errors
+            },
+            status=400,
+        )

misago/users/api/userendpoints/username.py

@@ -1,4 +1,3 @@
-from rest_framework import status
 from rest_framework.response import Response
 
 from django.db import IntegrityError
@@ -35,55 +34,58 @@ def options_response(options):
 def change_username(request):
     options = get_username_options(request.user)
     if not options['changes_left']:
-        return Response({
-            'detail': _("You can't change your username now."),
-            'options': options
-        },
-                        status=status.HTTP_400_BAD_REQUEST)
+        return Response(
+            {
+                'detail': _("You can't change your username now."),
+                'options': options,
+            },
+            status=400,
+        )
 
-    serializer = ChangeUsernameSerializer(data=request.data, context={'user': request.user})
+    serializer = ChangeUsernameSerializer(
+        data=request.data,
+        context={'user': request.user},
+    )
 
-    if serializer.is_valid():
-        try:
-            serializer.change_username(changed_by=request.user)
-            return Response({
-                'username': request.user.username,
-                'slug': request.user.slug,
-                'options': get_username_options(request.user)
-            })
-        except IntegrityError:
-            return Response({
+    if not serializer.is_valid():
+        return Response(serializer.errors, status=400)
+
+    try:
+        serializer.change_username(changed_by=request.user)
+        return Response({
+            'username': request.user.username,
+            'slug': request.user.slug,
+            'options': get_username_options(request.user)
+        })
+    except IntegrityError:
+        return Response(
+            {
                 'detail': _("Error changing username. Please try again."),
             },
-                            status=status.HTTP_400_BAD_REQUEST)
-    else:
-        return Response({
-            'detail': serializer.errors['non_field_errors'][0]
-        },
-                        status=status.HTTP_400_BAD_REQUEST)
+            status=400,
+        )
 
 
 def moderate_username_endpoint(request, profile):
     if request.method == 'POST':
         serializer = ChangeUsernameSerializer(data=request.data, context={'user': profile})
 
-        if serializer.is_valid():
-            try:
-                serializer.change_username(changed_by=request.user)
-                return Response({
-                    'username': profile.username,
-                    'slug': profile.slug,
-                })
-            except IntegrityError:
-                return Response({
+        if not serializer.is_valid():
+            return Response({'detail': serializer.errors}, status=400)
+
+        try:
+            serializer.change_username(changed_by=request.user)
+            return Response({
+                'username': profile.username,
+                'slug': profile.slug,
+            })
+        except IntegrityError:
+            return Response(
+                {
                     'detail': _("Error changing username. Please try again."),
                 },
-                                status=status.HTTP_400_BAD_REQUEST)
-        else:
-            return Response({
-                'detail': serializer.errors['non_field_errors'][0]
-            },
-                            status=status.HTTP_400_BAD_REQUEST)
+                status=400,
+            )
     else:
         return Response({
             'length_min': settings.username_length_min,

misago/users/api/users.py

@@ -1,4 +1,4 @@
-from rest_framework import status, viewsets
+from rest_framework import viewsets
 from rest_framework.decorators import detail_route
 from rest_framework.parsers import FormParser, JSONParser, MultiPartParser
 from rest_framework.response import Response
@@ -106,9 +106,9 @@ class UserViewSet(viewsets.GenericViewSet):
         serializer = ForumOptionsSerializer(request.user, data=request.data)
         if serializer.is_valid():
             serializer.save()
-            return Response({'detail': _("Your forum options have been changed.")})
+            return Response(status=204)
         else:
-            return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
+            return Response(serializer.errors, status=400)
 
     @detail_route(methods=['get', 'post'])
     def username(self, request, pk=None):
@@ -176,7 +176,10 @@ class UserViewSet(viewsets.GenericViewSet):
             profile.save(update_fields=['followers'])
             request.user.save(update_fields=['following'])
 
-            return Response({'is_followed': followed, 'followers': profile_followers})
+            return Response({
+                'is_followed': followed,
+                'followers': profile_followers,
+            })
 
     @detail_route()
     def ban(self, request, pk=None):