Skip to content

Commit

Permalink
Merge pull request #164 from nelc/tehreem/handle_fxcoded_exception
Browse files Browse the repository at this point in the history
feat: handle fxcoded exception and return readable response
  • Loading branch information
tehreem-sadat authored Dec 10, 2024
2 parents d4538ad + fc6f92e commit 05a3c4d
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 13 deletions.
26 changes: 13 additions & 13 deletions futurex_openedx_extensions/dashboard/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
default_auth_classes = FX_VIEW_DEFAULT_AUTH_CLASSES.copy()


class TotalCountsView(APIView, FXViewRoleInfoMixin):
class TotalCountsView(FXViewRoleInfoMixin, APIView):
"""
View to get the total count statistics
Expand Down Expand Up @@ -184,7 +184,7 @@ def get(self, request: Any, *args: Any, **kwargs: Any) -> Response | JsonRespons
return JsonResponse(result)


class LearnersView(ListAPIView, FXViewRoleInfoMixin):
class LearnersView(FXViewRoleInfoMixin, ListAPIView):
"""View to get the list of learners"""
authentication_classes = default_auth_classes
permission_classes = [FXHasTenantCourseAccess]
Expand All @@ -206,7 +206,7 @@ def get_queryset(self) -> QuerySet:
)


class CoursesView(ListAPIView, FXViewRoleInfoMixin):
class CoursesView(FXViewRoleInfoMixin, ListAPIView):
"""View to get the list of courses"""
authentication_classes = default_auth_classes
permission_classes = [FXHasTenantCourseAccess]
Expand Down Expand Up @@ -235,7 +235,7 @@ def get_queryset(self) -> QuerySet:
)


class CourseStatusesView(APIView, FXViewRoleInfoMixin):
class CourseStatusesView(FXViewRoleInfoMixin, APIView):
"""View to get the course statuses"""
authentication_classes = default_auth_classes
permission_classes = [FXHasTenantCourseAccess]
Expand Down Expand Up @@ -269,7 +269,7 @@ def get(self, request: Any, *args: Any, **kwargs: Any) -> JsonResponse:
return JsonResponse(self.to_json(result))


class LearnerInfoView(APIView, FXViewRoleInfoMixin):
class LearnerInfoView(FXViewRoleInfoMixin, APIView):
"""View to get the information of a learner"""
authentication_classes = default_auth_classes
permission_classes = [FXHasTenantCourseAccess]
Expand Down Expand Up @@ -304,7 +304,7 @@ def get(self, request: Any, username: str, *args: Any, **kwargs: Any) -> JsonRes
)


class DataExportManagementView(viewsets.ModelViewSet, FXViewRoleInfoMixin): # pylint: disable=too-many-ancestors
class DataExportManagementView(FXViewRoleInfoMixin, viewsets.ModelViewSet): # pylint: disable=too-many-ancestors
"""View to list and retrieve data export tasks."""
authentication_classes = default_auth_classes
permission_classes = [FXHasTenantCourseAccess]
Expand Down Expand Up @@ -335,7 +335,7 @@ def get_object(self) -> DataExportTask:
return task


class LearnerCoursesView(APIView, FXViewRoleInfoMixin):
class LearnerCoursesView(FXViewRoleInfoMixin, APIView):
"""View to get the list of courses for a learner"""
authentication_classes = default_auth_classes
permission_classes = [FXHasTenantCourseAccess]
Expand Down Expand Up @@ -407,7 +407,7 @@ def get(self, request: Any, *args: Any, **kwargs: Any) -> JsonResponse: # pylin
return JsonResponse(get_tenants_info(tenant_ids))


class LearnersDetailsForCourseView(ExportCSVMixin, ListAPIView, FXViewRoleInfoMixin):
class LearnersDetailsForCourseView(ExportCSVMixin, FXViewRoleInfoMixin, ListAPIView):
"""View to get the list of learners for a course"""
authentication_classes = default_auth_classes
serializer_class = serializers.LearnerDetailsForCourseSerializer
Expand Down Expand Up @@ -443,7 +443,7 @@ def get_serializer_context(self) -> Dict[str, Any]:
return context


class LearnersEnrollmentView(ListAPIView, FXViewRoleInfoMixin):
class LearnersEnrollmentView(FXViewRoleInfoMixin, ListAPIView):
"""View to get the list of learners for a course"""
serializer_class = serializers.LearnerEnrollmentSerializer
permission_classes = [FXHasTenantCourseAccess]
Expand Down Expand Up @@ -484,7 +484,7 @@ def get_serializer_context(self) -> Dict[str, Any]:
return context


class GlobalRatingView(APIView, FXViewRoleInfoMixin):
class GlobalRatingView(FXViewRoleInfoMixin, APIView):
"""View to get the global rating"""
authentication_classes = default_auth_classes
permission_classes = [FXHasTenantCourseAccess]
Expand Down Expand Up @@ -512,7 +512,7 @@ def get(self, request: Any, *args: Any, **kwargs: Any) -> JsonResponse:
return JsonResponse(result)


class UserRolesManagementView(viewsets.ModelViewSet, FXViewRoleInfoMixin): # pylint: disable=too-many-ancestors
class UserRolesManagementView(FXViewRoleInfoMixin, viewsets.ModelViewSet): # pylint: disable=too-many-ancestors
"""View to get the user roles"""
authentication_classes = default_auth_classes
permission_classes = [FXHasTenantAllCoursesAccess]
Expand Down Expand Up @@ -675,7 +675,7 @@ def destroy(self, request: Any, *args: Any, **kwargs: Any) -> Response:
return Response(status=http_status.HTTP_204_NO_CONTENT)


class MyRolesView(APIView, FXViewRoleInfoMixin):
class MyRolesView(FXViewRoleInfoMixin, APIView):
"""View to get the user roles of the caller"""
authentication_classes = default_auth_classes
permission_classes = [FXHasTenantCourseAccess]
Expand All @@ -692,7 +692,7 @@ def get(self, request: Any, *args: Any, **kwargs: Any) -> JsonResponse:
return JsonResponse(data)


class ClickhouseQueryView(APIView, FXViewRoleInfoMixin):
class ClickhouseQueryView(FXViewRoleInfoMixin, APIView):
"""View to get the Clickhouse query"""
authentication_classes = default_auth_classes
permission_classes = [FXHasTenantCourseAccess]
Expand Down
12 changes: 12 additions & 0 deletions futurex_openedx_extensions/helpers/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
from django.db.models.functions import Lower
from opaque_keys.edx.django.models import CourseKeyField
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from rest_framework import status as http_status
from rest_framework.response import Response
from xmodule.modulestore.django import modulestore

from futurex_openedx_extensions.helpers import constants as cs
Expand Down Expand Up @@ -473,6 +475,16 @@ def is_view_support_write(view_name: str) -> bool:

class FXViewRoleInfoMixin(metaclass=FXViewRoleInfoMetaClass):
"""View mixin to provide role information to the view."""

def handle_exception(self, exc: Exception) -> Any:
"""Override to handle fx exception"""
if isinstance(exc, FXCodedException):
return Response(
error_details_to_dictionary(reason=str(exc)),
status=http_status.HTTP_400_BAD_REQUEST
)
return super().handle_exception(exc) # type: ignore

@property
def fx_permission_info(self) -> dict:
"""Get fx_permission_info from the request."""
Expand Down
25 changes: 25 additions & 0 deletions tests/test_helpers/test_roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from opaque_keys.edx.keys import CourseKey
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from organizations.models import Organization
from rest_framework import status

from futurex_openedx_extensions.helpers import constants as cs
from futurex_openedx_extensions.helpers.exceptions import FXCodedException, FXExceptionCodes
Expand Down Expand Up @@ -2573,3 +2574,27 @@ def test_add_missing_signup_source_record(mock_signup, base_data): # pylint: di

assert add_missing_signup_source_record(user_id, 'org1') is True
mock_signup.assert_called_once_with(user=user, orgs=['org1'])


@pytest.mark.parametrize(
'exception, expected_result', [
(FXCodedException(message='This is an FX coded exception', code=11), {
'reason': 'This is an FX coded exception',
'details': {}
}),
(Exception('Generic Exception'), None),
(ValueError('A value error occurred'), None),
]
)
def test_fx_exception_handler(exception, expected_result):
"""
Test fx_exception_handler with different types of exceptions.
"""
mixin = FXViewRoleInfoMixin()
if expected_result is None:
with pytest.raises(Exception or ValueError):
mixin.handle_exception(exception)
else:
response = mixin.handle_exception(exception)
assert response.data == expected_result
assert response.status_code == status.HTTP_400_BAD_REQUEST

0 comments on commit 05a3c4d

Please sign in to comment.