Add OAuth2 support for problem response reports endpoints #24873

pcockwell · 2020-08-31T12:37:03Z

This PR attempts to re-add functionality that was originally included with https://github.com/edx/edx-platform/pull/19635.

JIRA tickets:
OSPR-4933

Sandbox URL: TBD - sandbox is being provisioned.

Testing instructions:

Run tests related to this new endpoint:

paver test_system -t lms/djangoapps/instructor/tests/test_api.py

Go to the DemoX edX Demonstration Course as staff user.
Check if the generation of problem response reports is working correctly via the instructor interface.
Create an Oauth client here if one does not exists for testing.
Use the Oauth2 endpoint to get an auth token (cUrl, Postman or other tool).
Check get_response_report endpoint. Example:

POST /courses/course-v1:edX+DemoX+Demo_Course/instructor/api/get_problem_responses
Body:
{
    "problem_location": "block-v1:edX+DemoX+Demo_Course+type@problem+block@c554538a57664fac80783b99d9d6da7c"
}

Check list_instructor_tasks endpoint. Example:

POST /courses/course-v1:edX+DemoX+Demo_Course/instructor/api/list_instructor_tasks
Body: {}

Check list_report_downloads endpoint. Example:

POST /courses/course-v1:edX+DemoX+Demo_Course/instructor/api/list_report_downloads
Body: {}

Author notes and concerns:

The code from the original PR was cherry-picked to the current master branch and then conflicts were manually resolved.

Reviewers

@xitij2000
edX reviewer[s] TBD

openedx-webhooks · 2020-08-31T12:37:07Z

Thanks for the pull request, @pcockwell! I've created OSPR-4933 to keep track of it in JIRA, where we prioritize reviews. Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

pcockwell · 2020-09-01T10:36:31Z

@ormsbee Did the investigation for #19635 produce any useful information as the cause of the problems? Could you describe what was happening such that these errors were occurring and/or how to reproduce them? Any information you have on this would be appreciated.

Right now, the requests made to these endpoints succeed when providing an Authorization header with the Bearer access token included, but they do not appear to require any CSRF Token for the request. My understanding of the Django Rest Framework APIView, is that when [SessionAuthentication](https://www.django-rest-framework.org/api-guide/authentication/#sessionauthentication) is included in authentication_classes, then any requests that use Session based credentials should require a CSRF Token. As of yet though, I can't seem to figure out how to test these endpoints using the CSRF token

lms/djangoapps/instructor/views/api.py

lms/djangoapps/instructor/views/instructor_dashboard.py

xitij2000 · 2020-09-02T08:30:38Z

jenkins run all

lms/djangoapps/instructor/views/api.py

xitij2000

Please look at the failing tests and rectify.

lms/djangoapps/instructor/views/api.py

natabene · 2020-09-02T18:42:42Z

@pcockwell Thank you for your contribution. Please let me know once it is ready for our review.

xitij2000

👍 Seems to be working OK. I've just suggested a few minor corrections and updates. See to them and this is done. Doesn't need another review pass from me.

I tested this: tested using regular and via instructor dashboard.
I read through the code
Includes documentation

xitij2000 · 2020-09-04T10:23:40Z

lms/djangoapps/instructor/views/api.py

@@ -218,7 +219,7 @@ def require_course_permission(permission):
    user. If the requirement is not satisfied, returns an
    HttpResponseForbidden (403).

-    Assumes that request is in args[0].
+    Assumes that request is in args[0]


Suggested change

Assumes that request is in args[0]

Assumes that request is in args[0].

xitij2000 · 2020-09-04T10:27:29Z

lms/djangoapps/instructor/views/api.py

+
+    **Example Requests**:
+
+        POST /api/instructor/v1/course/{}/tasks


Suggested change

POST /api/instructor/v1/course/{}/tasks

POST /api/instructor/v1/course/{}/reports

xitij2000 · 2020-09-04T10:29:56Z

lms/djangoapps/instructor/views/api.py

@@ -1038,31 +1038,39 @@ def get_problem_responses(request, course_id):
        "task_id": "4e49522f-31d9-431a-9cff-dd2a2bf4c85a"
    }
    ```


Could you update this documentation to also point to the new API endpoints?

@xitij2000

Is this what you meant?

lms/djangoapps/instructor/views/api.py

xitij2000

👍

I tested this: tested endpoints using REST requests
I read through the code
Includes documentation

pcockwell · 2020-09-07T07:15:55Z

@natabene This is now ready for edX review. Thanks!

pcockwell · 2020-09-07T10:22:23Z

jenkins run py38

natabene · 2020-09-08T17:02:02Z

@pcockwell Thanks for letting me know, I am lining this up for our review.

pcockwell · 2020-09-21T12:28:50Z

jenkins run python-3.8

bradenmacdonald · 2020-10-29T22:31:53Z

@pcockwell If this PR is no longer WIP, could you please update the description to reflect its current status?

Please note this PR is a WIP. Functionality testing still needs to occur.

pcockwell · 2020-10-30T09:41:43Z

@pcockwell If this PR is no longer WIP, could you please update the description to reflect its current status?

Please note this PR is a WIP. Functionality testing still needs to occur.

Done. Sorry about that

xitij2000 · 2021-01-11T11:11:58Z

jenkins run a11y

edx-status-bot · 2021-03-23T11:35:35Z

Your PR has finished running tests. The following contexts failed:

jenkins/a11y
jenkins/quality
jenkins/python

xitij2000 · 2021-04-15T14:01:33Z

@natabene This can be closed since I've opened a new PR here: https://github.com/edx/edx-platform/pull/27313

openedx-webhooks · 2021-04-19T02:44:38Z

@pcockwell Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Aug 31, 2020

xitij2000 reviewed Sep 1, 2020

View reviewed changes

lms/djangoapps/instructor/views/api.py Outdated Show resolved Hide resolved

lms/djangoapps/instructor/views/api.py Outdated Show resolved Hide resolved

lms/djangoapps/instructor/views/instructor_dashboard.py Outdated Show resolved Hide resolved

xitij2000 reviewed Sep 2, 2020

View reviewed changes

lms/djangoapps/instructor/views/api.py Outdated Show resolved Hide resolved

xitij2000 reviewed Sep 2, 2020

View reviewed changes

lms/djangoapps/instructor/views/api.py Outdated Show resolved Hide resolved

lms/djangoapps/instructor/views/api.py Outdated Show resolved Hide resolved

pcockwell force-pushed the patrick/BB-2853-upstream-bb-728 branch from ec88404 to 1fa00bf Compare September 2, 2020 10:10

openedx-webhooks added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed needs triage labels Sep 2, 2020

xitij2000 approved these changes Sep 4, 2020

View reviewed changes

xitij2000 approved these changes Sep 7, 2020

View reviewed changes

pcockwell force-pushed the patrick/BB-2853-upstream-bb-728 branch from d7ed737 to f82500e Compare September 7, 2020 07:14

pcockwell changed the title ~~Add OAuth2 support for problem response reports endpoints and move API~~ Add OAuth2 support for problem response reports endpoints Sep 7, 2020

openedx-webhooks added awaiting prioritization and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Sep 8, 2020

xitij2000 force-pushed the patrick/BB-2853-upstream-bb-728 branch from bea09ba to f54cf80 Compare January 8, 2021 13:45

Add OAuth2 support for problem response reports endpoints

5fb1e03

xitij2000 force-pushed the patrick/BB-2853-upstream-bb-728 branch from 2ac60e7 to 5fb1e03 Compare March 23, 2021 10:56

xitij2000 mentioned this pull request Apr 13, 2021

[BB-2853] Add OAuth2-enabled endpoints for problem response report generation #27313

Merged

2 tasks

natabene closed this Apr 19, 2021

openedx-webhooks added rejected and removed awaiting prioritization labels Apr 19, 2021

xitij2000 deleted the patrick/BB-2853-upstream-bb-728 branch August 26, 2021 05:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add OAuth2 support for problem response reports endpoints #24873

Add OAuth2 support for problem response reports endpoints #24873

pcockwell commented Aug 31, 2020 •

edited

Loading

openedx-webhooks commented Aug 31, 2020 •

edited

Loading

pcockwell commented Sep 1, 2020

xitij2000 commented Sep 2, 2020

xitij2000 left a comment

natabene commented Sep 2, 2020

xitij2000 left a comment •

edited

Loading

xitij2000 Sep 4, 2020

xitij2000 Sep 4, 2020

xitij2000 Sep 4, 2020

pcockwell Sep 7, 2020

xitij2000 left a comment

pcockwell commented Sep 7, 2020

pcockwell commented Sep 7, 2020

natabene commented Sep 8, 2020

pcockwell commented Sep 21, 2020

bradenmacdonald commented Oct 29, 2020

pcockwell commented Oct 30, 2020

xitij2000 commented Jan 11, 2021

edx-status-bot commented Mar 23, 2021

xitij2000 commented Apr 15, 2021

openedx-webhooks commented Apr 19, 2021

	Assumes that request is in args[0]
	Assumes that request is in args[0].


		Example Requests:

		POST /api/instructor/v1/course/{}/tasks

	POST /api/instructor/v1/course/{}/tasks
	POST /api/instructor/v1/course/{}/reports

Add OAuth2 support for problem response reports endpoints #24873

Add OAuth2 support for problem response reports endpoints #24873

Conversation

pcockwell commented Aug 31, 2020 • edited Loading

openedx-webhooks commented Aug 31, 2020 • edited Loading

pcockwell commented Sep 1, 2020

xitij2000 commented Sep 2, 2020

xitij2000 left a comment

Choose a reason for hiding this comment

natabene commented Sep 2, 2020

xitij2000 left a comment • edited Loading

Choose a reason for hiding this comment

xitij2000 Sep 4, 2020

Choose a reason for hiding this comment

xitij2000 Sep 4, 2020

Choose a reason for hiding this comment

xitij2000 Sep 4, 2020

Choose a reason for hiding this comment

pcockwell Sep 7, 2020

Choose a reason for hiding this comment

xitij2000 left a comment

Choose a reason for hiding this comment

pcockwell commented Sep 7, 2020

pcockwell commented Sep 7, 2020

natabene commented Sep 8, 2020

pcockwell commented Sep 21, 2020

bradenmacdonald commented Oct 29, 2020

pcockwell commented Oct 30, 2020

xitij2000 commented Jan 11, 2021

edx-status-bot commented Mar 23, 2021

xitij2000 commented Apr 15, 2021

openedx-webhooks commented Apr 19, 2021

pcockwell commented Aug 31, 2020 •

edited

Loading

openedx-webhooks commented Aug 31, 2020 •

edited

Loading

xitij2000 left a comment •

edited

Loading