Skip to content

Commit

Permalink
feat: converting existing api to drf base api. (#35039)
Browse files Browse the repository at this point in the history
Adding generic permission class. Added standard authentication classes.
  • Loading branch information
awais786 authored Jul 30, 2024
1 parent 085a2f5 commit 39dd3c0
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 26 deletions.
12 changes: 11 additions & 1 deletion lms/djangoapps/instructor/permissions.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
"""
Permissions for the instructor dashboard and associated actions
"""

from bridgekeeper import perms
from bridgekeeper.rules import is_staff
from opaque_keys.edx.keys import CourseKey
from rest_framework.permissions import BasePermission

from lms.djangoapps.courseware.rules import HasAccessRule, HasRolesRule
from openedx.core.lib.courses import get_course_by_id

ALLOW_STUDENT_TO_BYPASS_ENTRANCE_EXAM = 'instructor.allow_student_to_bypass_entrance_exam'
ASSIGN_TO_COHORTS = 'instructor.assign_to_cohorts'
Expand Down Expand Up @@ -72,3 +74,11 @@
) | HasAccessRule('staff') | HasAccessRule('instructor')
perms[VIEW_ENROLLMENTS] = HasAccessRule('staff')
perms[VIEW_FORUM_MEMBERS] = HasAccessRule('staff')


class InstructorPermission(BasePermission):
"""Generic permissions"""
def has_permission(self, request, view):
course = get_course_by_id(CourseKey.from_string(view.kwargs.get('course_id')))
permission = getattr(view, 'permission_name', None)
return request.user.has_perm(permission, course)
51 changes: 50 additions & 1 deletion lms/djangoapps/instructor/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2945,7 +2945,37 @@ def test_get_student_progress_url(self):
response = self.client.post(url, data)
assert response.status_code == 200
res_json = json.loads(response.content.decode('utf-8'))
assert 'progress_url' in res_json
expected_data = {
'course_id': str(self.course.id),
'progress_url': f'/courses/{self.course.id}/progress/{self.students[0].id}/'
}

for key, value in expected_data.items():
self.assertIn(key, res_json)
self.assertEqual(res_json[key], value)

def test_get_student_progress_url_response_headers(self):
"""
Test that the progress_url endpoint returns the correct headers.
"""
url = reverse('get_student_progress_url', kwargs={'course_id': str(self.course.id)})
data = {'unique_student_identifier': self.students[0].email}
response = self.client.post(url, data)
assert response.status_code == 200

expected_headers = {
'Allow': 'POST, OPTIONS', # drf view brings this key.
'Cache-Control': 'no-cache, no-store, must-revalidate',
'Content-Language': 'en',
'Content-Length': str(len(response.content.decode('utf-8'))),
'Content-Type': 'application/json',
'Vary': 'Cookie, Accept-Language, origin',
'X-Frame-Options': 'DENY'
}

for key, value in expected_headers.items():
self.assertIn(key, response.headers)
self.assertEqual(response.headers[key], value)

def test_get_student_progress_url_from_uname(self):
""" Test that progress_url is in the successful response. """
Expand All @@ -2955,6 +2985,14 @@ def test_get_student_progress_url_from_uname(self):
assert response.status_code == 200
res_json = json.loads(response.content.decode('utf-8'))
assert 'progress_url' in res_json
expected_data = {
'course_id': str(self.course.id),
'progress_url': f'/courses/{self.course.id}/progress/{self.students[0].id}/'
}

for key, value in expected_data.items():
self.assertIn(key, res_json)
self.assertEqual(res_json[key], value)

def test_get_student_progress_url_noparams(self):
""" Test that the endpoint 404's without the required query params. """
Expand All @@ -2968,6 +3006,17 @@ def test_get_student_progress_url_nostudent(self):
response = self.client.post(url)
assert response.status_code == 400

def test_get_student_progress_url_without_permissions(self):
""" Test that progress_url returns 403 without credentials. """

# removed both roles from courses for instructor
CourseDataResearcherRole(self.course.id).remove_users(self.instructor)
CourseInstructorRole(self.course.id).remove_users(self.instructor)
url = reverse('get_student_progress_url', kwargs={'course_id': str(self.course.id)})
data = {'unique_student_identifier': self.students[0].email}
response = self.client.post(url, data)
assert response.status_code == 403


class TestInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
"""
Expand Down
71 changes: 48 additions & 23 deletions lms/djangoapps/instructor/views/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes
from openedx.core.lib.courses import get_course_by_id
from openedx.core.lib.api.serializers import CourseKeyField
from openedx.features.course_experience.url_helpers import get_learning_mfe_home_url
from .tools import (
dump_block_extensions,
Expand Down Expand Up @@ -1718,15 +1719,35 @@ def get_student_enrollment_status(request, course_id):
return JsonResponse(response_payload)


@require_POST
@ensure_csrf_cookie
@cache_control(no_cache=True, no_store=True, must_revalidate=True)
@require_course_permission(permissions.ENROLLMENT_REPORT)
@require_post_params(
unique_student_identifier="email or username of student for whom to get progress url"
)
@common_exceptions_400
def get_student_progress_url(request, course_id):
class StudentProgressUrlSerializer(serializers.Serializer):
"""Serializer for course renders"""
unique_student_identifier = serializers.CharField(write_only=True)
course_id = CourseKeyField(required=False)
progress_url = serializers.SerializerMethodField()

def get_progress_url(self, obj): # pylint: disable=unused-argument
"""
Return the progress URL for the student.
Args:
obj (dict): The dictionary containing data for the serializer.
Returns:
str: The URL for the progress of the student in the course.
"""
user = get_student_from_identifier(obj.get('unique_student_identifier'))
course_id = obj.get('course_id') # Adjust based on your data structure

if course_home_mfe_progress_tab_is_active(course_id):
progress_url = get_learning_mfe_home_url(course_id, url_fragment='progress')
if user is not None:
progress_url += '/{}/'.format(user.id)
else:
progress_url = reverse('student_progress', kwargs={'course_id': str(course_id), 'student_id': user.id})

return progress_url


@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch')
class StudentProgressUrl(APIView):
"""
Get the progress url of a student.
Limited to staff access.
Expand All @@ -1736,21 +1757,25 @@ def get_student_progress_url(request, course_id):
'progress_url': '/../...'
}
"""
course_id = CourseKey.from_string(course_id)
user = get_student_from_identifier(request.POST.get('unique_student_identifier'))

if course_home_mfe_progress_tab_is_active(course_id):
progress_url = get_learning_mfe_home_url(course_id, url_fragment='progress')
if user is not None:
progress_url += '/{}/'.format(user.id)
else:
progress_url = reverse('student_progress', kwargs={'course_id': str(course_id), 'student_id': user.id})
authentication_classes = (
JwtAuthentication,
BearerAuthenticationAllowInactiveUser,
SessionAuthenticationAllowInactiveUser,
)
permission_classes = (IsAuthenticated, permissions.InstructorPermission)
serializer_class = StudentProgressUrlSerializer
permission_name = permissions.ENROLLMENT_REPORT

response_payload = {
'course_id': str(course_id),
'progress_url': progress_url,
}
return JsonResponse(response_payload)
@method_decorator(ensure_csrf_cookie)
def post(self, request, course_id):
"""Post method for validating incoming data and generating progress URL"""
data = {
'course_id': course_id,
'unique_student_identifier': request.data.get('unique_student_identifier')
}
serializer = self.serializer_class(data=data)
serializer.is_valid(raise_exception=True)
return Response(serializer.data)


@transaction.non_atomic_requests
Expand Down
3 changes: 2 additions & 1 deletion lms/djangoapps/instructor/views/api_urls.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

"""
Instructor API endpoint urls.
"""
Expand Down Expand Up @@ -32,7 +33,7 @@
path('get_students_who_may_enroll', api.get_students_who_may_enroll, name='get_students_who_may_enroll'),
path('get_anon_ids', api.get_anon_ids, name='get_anon_ids'),
path('get_student_enrollment_status', api.get_student_enrollment_status, name="get_student_enrollment_status"),
path('get_student_progress_url', api.get_student_progress_url, name='get_student_progress_url'),
path('get_student_progress_url', api.StudentProgressUrl.as_view(), name='get_student_progress_url'),
path('reset_student_attempts', api.reset_student_attempts, name='reset_student_attempts'),
path('rescore_problem', api.rescore_problem, name='rescore_problem'),
path('override_problem_score', api.override_problem_score, name='override_problem_score'),
Expand Down

0 comments on commit 39dd3c0

Please sign in to comment.