feat: converting existing api to drf base api ( 1st api ). get_student_progress_url

[awais786]

Issue for tracking.

Updating api code using DRF.

1. Added standard authentication_classes
2. Added permission_classes

• Test this on local before merge.

Steps to verify:

1. Log in to the local instance and open the instructor dashboard for course.
2. Navigate to the section labeled "View a specific learner's grades and progress."
3. Enter a user ID (e.g., admin) and click on "view progress page."
4. If successful, you will be redirected to the progress page, such as this example.
5. Repeat step 3 using a user email (e.g., [email protected]) and ensure it functions correctly as well.

---

[awais786]

I have added new decorator because in existing one they are not passing self in constructor and its not compatabile with DRF apiview.

[feanil]

Instead of making a new decorator, does it make more sense to make a custom permission class instead?

[feanil]

The approach mostly seems sound, I think I'd like to see if we can use standard DRF permissions management as much as possible instead of making our own decorators. What do you think?

[feanil]

I think you don't need to define this, since the base authentication classes should be the correct ones.

[awais786]

Now I simply matched the other api in this file https://github.com/openedx/edx-platform/blob/master/lms/djangoapps/instructor/views/api.py#L1587

[feanil]

Instead of making a new decorator, does it make more sense to make a custom permission class instead?

[feanil]

Unnecessary newline.

[awais786]

@feanil you can review this again.

[feanil]

@awais786 that looks great to me!

[awais786]

DRF returns this extra key Allow. DRF code reference here.

master branch returns this header response

{
'Content-Type': 'application/json', 
'Cache-Control': 'no-cache, no-store, must-revalidate',
'Vary': 'Cookie, Accept-Language, origin',
'X-Frame-Options': 'DENY', 
'Content-Language': 'en', 
'Content-Length': '130'
}

current branch returns this

{
'Content-Type': 'application/json',
 'Allow': 'POST, OPTIONS', 
'Cache-Control': 'no-cache, no-store, must-revalidate', 
'Vary': 'Cookie, Accept-Language, origin', 
'X-Frame-Options': 'DENY', 
'Content-Language': 'en', 
'Content-Length': '121'
}

[awais786]

via postman these are response header.

Current branch -------------------------------------------vs-----------------------master branch

[awais786]

@feanil I have only found one difference between new vs old api. Old apis are not using any IsAuthenticated method. So in case of anonymous user they return 403 which comes from permissions verification but in new API drf will return 401.

This test on this branch returns 401 which is standard DRF status code.

    def test_get_student_progress_url_with_anonymous_user(self):
        """ Test that progress_url returns 403 without credentials. """
        self.client.logout()
        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 == 401

But on master branch its 403 and this test fails on master.

FAILED lms/djangoapps/instructor/tests/test_api.py::TestInstructorAPILevelsDataDump::test_get_student_progress_url_with_anonymous_user - assert 403 == 401

What do you think about this?

[awais786]

I think its good to go now.

[feanil]

@awais786 thanks for verifying with Postman! I think you can merge this at your leisure. Don't forget to mention it in the #cc-edx-platform slack channel.

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX production environment.