[BB-873] Support for filters, and multiple roots in problem response reports by xitij2000 · Pull Request #19781 · openedx/openedx-platform

xitij2000 · 2019-02-11T11:27:07Z

This PR includes the following changes to the problem response report API:

Support for specifying multiple root blocks.
With this feature, you can now pass a comma-separated list of problem locations, and the generated report will include both of them (and children).
Support for specifying a problem type filter.
This feature allows you to request a report for only one or more types of blocks. In this case you can request a report for the entire course, but filter it to only blocks of specific types.

JIRA tickets: OSPR-3070

Discussions: https://github.com/edx/edx-platform/pull/19507

Dependencies: https://github.com/edx/edx-platform/pull/19507, https://github.com/edx/edx-platform/pull/19635

Sandbox URL: TBD - sandbox is being provisioned.

Merge deadline: None

Testing instructions:

Log in as an instructor
Post to the get_problem_responses API (this needs to include session information till [BB-728] Add problem response report API #19635 lands) and with problem_location set to a comma-separated list of problem locations. (API url: http://localhost:18000/courses/course-v1:edX+DemoX+Demo_Course/instructor/api/get_problem_responses )
This should start generating a report with all those problem locations
Post to the same api again, this time use the course (or a section) as the problem_location, but also provide a problem_types_filter
Check the generated report, it should include only blocks of the type specified in problem_types_filter.

Concerns:

The locations to be included in the report are stored in this field which is limited to 255 characters. This limits us to generate a report for a maximum of around 3/4 blocks (or less if the locations are longer). It would be nice if we could somehow remove this restriction by increasing the size of this field.
The task_input is limited to 255 chars, yet this line only throws an error if the task_input is longer than 265 characters, I'm not sure why that is.

Reviewers

@giovannicimolin
edX reviewer[s] TBD

openedx-webhooks · 2019-02-11T11:27:15Z

Thanks for the pull request, @xitij2000! I've created OSPR-3070 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams.

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

supporting documentation
edx-code email 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 still be done via the GitHub pull request interface. As a reminder, our process documentation is here.

natabene · 2019-02-11T20:26:20Z

@xitij2000 Thank you for your contribution. Can you let me know once all checks are green?

xitij2000 · 2019-02-14T11:12:36Z

@natabene Certainly. This will need a somewhat complex rebase after https://github.com/edx/edx-platform/pull/19635 lands, so I will wait on that.

giovannicimolin

👍

I tested this:

I used the EdX Demo Course as base for testing this.
Logged in as staff and answered some problems and assessments on the demo course.
Generate problem response reports for 2 problems sending comma-separated values. I used these two:

block-v1:edX+DemoX+Demo_Course+type@problem+block@c554538a57664fac80783b99d9d6da7c
block-v1:edX+DemoX+Demo_Course+type@problem+block@d2e35c1d294b4ba0b3b1048615605d2a

Checked that the report was correctly generated and responses for both problems were included ✔️
Checked that we can generate a problem response report for the entire course with block-v1:edX+DemoX+Demo_Course+type@course+block@course ✔️
Used section block-v1:edX+DemoX+Demo_Course+type@course+block@course and tested the problem_types_filter with three values:

openassessment
problem
openassessment,problem

Checked that the generated reports contained only the problems that were specified on each operation ✔️

I read through the code
Includes documentation: on the docstrings
~~I checked for accessibility issues~~ NA

Nice work! 😁
This is blocked on https://github.com/edx/edx-platform/pull/19635, I can test this again when that is merged.

Note: To make testing this easier, I logged in LMS on Chrome, generated a problem response report and copied a cURL request like the one below.

curl 'http://edx.devstack.lms:18000/courses/course-v1:edX+DemoX+Demo_Course/instructor/api/get_problem_responses' -H 'Pragma: no-cache' -H 'Origin: http://edx.devstack.lms:18000' -H 'Accept-Encoding: gzip, deflate' -H 'Accept-Language: en-US,en;q=0.9' -H 'User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.96 Safari/537.36' -H 'Content-Type: application/x-www-form-urlencoded; charset=UTF-8' -H 'Accept: application/json, text/javascript, */*; q=0.01' -H 'Cache-Control: no-cache' -H 'X-Requested-With: XMLHttpRequest' -H 'Cookie: openedx-language-preference=en; Ooyala=RpOGxhMTE6p6DkTB8MBGtKN6v0_A_BdQ|28:1544803457&o|28:1544803457&5zdHcxOlM7fQJOMrCdwnnu16WP-d|28:1544803457&RpOGxhMTE6p6DkTB8MBGtKN6v0_A_BdQ|2:1544803457&o|2:1544803457&5zdHcxOlM7fQJOMrCdwnnu16WP-d|2:1544803457&RpOGxhMTE6p6DkTB8MBGtKN6v0_A_BdQ|1:1544803457&o|1:1544803457&5zdHcxOlM7fQJOMrCdwnnu16WP-d|1:1544803457&o|19:1544803456&5zdHcxOlM7fQJOMrCdwnnu16WP-d|19:1544803456&o|5:1544546327&5zdHcxOlM7fQJOMrCdwnnu16WP-d|5:1544546327&RpOGxhMTE6p6DkTB8MBGtKN6v0_A_BdQ|5:1544546327; experiments_is_enterprise=false; djdt=show; ecommerce_sessionid=jpwfbe6n4keygeozechyp1d1x0homgph; edxloggedin=true; edx-jwt-cookie-header-payload=eyJhbGciOiJSUzUxMiIsImtpZCI6ImRldnN0YWNrX2tleSJ9.eyJzY29wZXMiOiBbInVzZXJfaWQiLCAiZW1haWwiLCAicHJvZmlsZSJdLCAiYWRtaW5pc3RyYXRvciI6IHRydWUsICJwcmVmZXJyZWRfdXNlcm5hbWUiOiAic3RhZmYiLCAiZmlsdGVycyI6IFsidXNlcjptZSJdLCAiaWF0IjogMTU1MDY2NTgwMywgInN1YiI6ICI5MDIzNGM1NmE5NGNiODJkNWRjZDgxYTIzYmRhNjAwNyIsICJmYW1pbHlfbmFtZSI6ICIiLCAidXNlcl9pZCI6IDksICJhdWQiOiAibG1zLWtleSIsICJpc3MiOiAiaHR0cDovL2xvY2FsaG9zdDoxODAwMC9vYXV0aDIiLCAiZW1haWxfdmVyaWZpZWQiOiB0cnVlLCAibmFtZSI6ICIiLCAidmVyc2lvbiI6ICIxLjEuMCIsICJnaXZlbl9uYW1lIjogIiIsICJleHAiOiAxNTUwNjY5NDAzLCAiaXNfcmVzdHJpY3RlZCI6IGZhbHNlLCAiZW1haWwiOiAic3RhZmZAZXhhbXBsZS5jb20ifQ; edx-jwt-cookie-signature=I-Ib0NtTRY_mr0sKuBeTgGRU6wdjxHAhFpEjH4DGvh5nCCSzS0I_5b3nMeb0PzMo0KGkVGaDInpIsK--fV50mKAV_A7BZD06dmHK9m8nJw9Fd1iTweZ8f7kKdGLcUSIvQ9mH_yWwLPO_ICsULx2SbmcB_-rGbKxfV7FwBFc93FI7VO0MBRlzdpZM_B5-qhT-NgCDv-JruLinbtVU388_FWzUWm9l8Sbefh0_pO4akftXjjS3o9b7Ic_EU_atRKvdx80EqvZuFNUo8PzUeS2oJ4GBcbWT7-w6pG11FWTkR2oK2nuu2ffX5LVrCfJBqrxb8vgdhhw_0RJ9Szjwnke_1A; csrftoken=giMBf2s0gYqzk8mh8NvuZ8lfXzGsKUAWDM6VBsaxYU1Gc1yJfoGaUHO0r7olSZSd; edx-user-info="{\"username\": \"staff\"\054 \"version\": 1\054 \"enrollmentStatusHash\": \"09d498ab1dd69884525f4cdfa9cca6a6\"\054 \"header_urls\": {\"learner_profile\": \"http://edx.devstack.lms:18000/u/staff\"\054 \"resume_block\": \"http://edx.devstack.lms:18000/dashboard\"\054 \"logout\": \"http://edx.devstack.lms:18000/logout\"\054 \"account_settings\": \"http://edx.devstack.lms:18000/account/settings\"}}"; edx-jwt-refresh-cookie=TkaF7obCsGUrssLel2eRKC3Da7cSS6; sessionid="1|p0twgpm0ftuhfbineuhvwrv09c7xtonc|k8jFi8FgoWHO|IjVjZTc3M2NhMDZmMjNhYWQ5ZDA3YWY2Njc3ZDNhZmMwMzU5NzliN2QzZjI3MTE2MTVlZjU5N2U2NWVkNmM2Mzci:1gwR0p:KnavxYauF92EKRulzSvjOzMIdaM"' -H 'Connection: keep-alive' -H 'X-CSRFToken: giMBf2s0gYqzk8mh8NvuZ8lfXzGsKUAWDM6VBsaxYU1Gc1yJfoGaUHO0r7olSZSd' -H 'Referer: http://edx.devstack.lms:18000/courses/course-v1:edX+DemoX+Demo_Course/instructor' --data 'problem_location=block-v1%3AedX%2BDemoX%2BDemo_Course%2Btype%40problem%2Bblock%40c554538a57664fac80783b99d9d6da7c%2Cblock-v1%3AedX%2BDemoX%2BDemo_Course%2Btype%40problem%2Bblock%40d2e35c1d294b4ba0b3b1048615605d2a' --compressed

xitij2000 · 2019-03-18T21:57:20Z

@natabene This is ready for review now.

natabene · 2019-04-08T20:26:13Z

@xitij2000 Sorry, missed this somehow. Could you fix the failing test?

natabene · 2019-04-08T20:26:21Z

jenkins run all

xitij2000 · 2019-04-11T06:28:04Z

Sorry about that. I've made the fixes, no problems locally, will ping once the tests pass.

natabene · 2019-04-11T14:48:15Z

@ormsbee This is now ready for your review.

xitij2000 · 2019-05-06T11:38:03Z

@ormsbee Would you be able to give this a look?
CC: @natabene

feanil · 2019-06-19T15:42:51Z

Ok, @ormsbee can take a look at that and will reply as soon as he can.

xitij2000 · 2019-06-21T13:16:53Z

jenkins run js

xitij2000 · 2019-07-02T21:46:52Z

@ormsbee I have rebased the branch on the latest changes on master, and all tests are now passing. It seems some of the tests were stuck earlier.

natabene · 2019-07-15T19:53:22Z

@ormsbee This is ready for your now.

…ks, or filtered block types. This change adds support for specifying multiple root blocks while generating problem response reports. It also allows specifying a block type filter so that only blocks of the filtered types will be included in the report. Finally, this change also consistenly uses absolute path for the location in the report instead of relative paths.

xitij2000 · 2019-08-08T09:36:14Z

@ormsbee This seems to be already be approved. I've just rebased it on the latest changes in master. Could you have a look and merge it if seems OK?

openedx-webhooks · 2019-08-08T17:36:46Z

@xitij2000 🎉 Your pull request was merged!

Please take a moment to answer a two question survey so we can improve your experience in the future.

ormsbee · 2019-08-08T17:37:17Z

Please add changes to the Juniper page if you have not done so already.

Thank you.

edx-pipeline-bot · 2019-08-08T18:33:58Z

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production on Friday, August 09, 2019.

xitij2000 · 2019-08-09T10:20:37Z

@ormsbee Thanks! Will do.

edx-status-bot · 2019-08-09T10:47:40Z

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

jenkins/js
codecov/project

DawoudSheraz · 2019-08-09T12:17:28Z

@xitij2000 @ormsbee I am afraid we will have to revert this PR. Following error is happening for both local and stage:

edx.devstack.lms     |   File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/celery/app/trace.py", line 240, in trace_task
edx.devstack.lms     |     R = retval = fun(*args, **kwargs)
edx.devstack.lms     |   File "/edx/app/edxapp/edx-platform/lms/djangoapps/instructor_task/tasks.py", line 177, in calculate_problem_responses_csv
edx.devstack.lms     |     return run_main_task(entry_id, task_fn, action_name)
edx.devstack.lms     |   File "/edx/app/edxapp/edx-platform/lms/djangoapps/instructor_task/tasks_helper/runner.py", line 116, in run_main_task
edx.devstack.lms     |     task_progress = task_fcn(entry_id, course_id, task_input, action_name)
edx.devstack.lms     |   File "/edx/app/edxapp/edx-platform/lms/djangoapps/instructor_task/tasks_helper/grades.py", line 778, in generate
edx.devstack.lms     |     usage_key_str_list=problem_locations.split(','),
edx.devstack.lms     | AttributeError: 'NoneType' object has no attribute 'split'

and here is a screenshot from the Splunk:

I am creating a revert, and if you think it is not necessary, please let me know.

xitij2000 · 2019-08-09T13:02:58Z

@DawoudSheraz I'm a bit surprised this is happening. I did not expect this API to be called without a problem_location.

I'll look into and make a corrected PR.

ormsbee · 2019-08-09T13:21:23Z

@DawoudSheraz: Thank you so much for catching this.

edx-pipeline-bot · 2019-08-09T15:16:47Z

EdX Release Notice: This PR has been deployed to the production environment.

xitij2000 · 2019-08-09T15:56:29Z

    task_class = calculate_problem_responses_csv
-    task_input = {'problem_location': problem_location, 'user_id': request.user.pk}
+    task_input = {
+        'problem_location': problem_locations,


The error is right here. This should be problem_locations.
I'm afraid I changed this back to problem_locaiton for backwards compatibility but didn't update it in the final place. I will update it here now since the task name has already been changed to avoid clashes.

openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Feb 11, 2019

xitij2000 changed the title ~~WIP: Support for fitlers, and multiple roots in problem response reports~~ WIP: BB-873: Support for fitlers, and multiple roots in problem response reports Feb 11, 2019

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

xitij2000 force-pushed the kshtij/problem-response-multi-root-and-fitler branch from 409bd2f to 262463c Compare February 13, 2019 13:18

xitij2000 force-pushed the kshtij/problem-response-multi-root-and-fitler branch from c74049e to 02d24a5 Compare February 18, 2019 10:52

xitij2000 mentioned this pull request Feb 18, 2019

[MCKIN-9474] [BB-873] Support for filters, and multiple roots in problem response reports edx/edx-solutions-edx-platform#1378

Merged

xitij2000 changed the title ~~WIP: BB-873: Support for fitlers, and multiple roots in problem response reports~~ WIP: BB-873: Support for filters, and multiple roots in problem response reports Feb 18, 2019

giovannicimolin approved these changes Feb 20, 2019

View reviewed changes

xitij2000 mentioned this pull request Feb 28, 2019

[BB-753] Fix issue with multiple responses for a single user returned by generate_report_data #19507

Merged

2 tasks

xitij2000 force-pushed the kshtij/problem-response-multi-root-and-fitler branch from 02d24a5 to 106a94a Compare March 18, 2019 21:43

xitij2000 changed the title ~~WIP: BB-873: Support for filters, and multiple roots in problem response reports~~ [BB-873] Support for filters, and multiple roots in problem response reports Mar 18, 2019

xitij2000 force-pushed the kshtij/problem-response-multi-root-and-fitler branch from 106a94a to c4d25f9 Compare March 18, 2019 22:33

xitij2000 force-pushed the kshtij/problem-response-multi-root-and-fitler branch from c4d25f9 to c7cfec4 Compare April 11, 2019 06:26

xitij2000 force-pushed the kshtij/problem-response-multi-root-and-fitler branch from c7cfec4 to 163dfe6 Compare April 11, 2019 08:24

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

xitij2000 force-pushed the kshtij/problem-response-multi-root-and-fitler branch from 163dfe6 to f374b42 Compare May 6, 2019 11:43

xitij2000 requested a review from a team May 6, 2019 11:43

openedx-webhooks removed the awaiting prioritization label May 21, 2019

xitij2000 force-pushed the kshtij/problem-response-multi-root-and-fitler branch 2 times, most recently from 3a30509 to fbc7b63 Compare June 19, 2019 15:27

xitij2000 force-pushed the kshtij/problem-response-multi-root-and-fitler branch from a0dff34 to ec2e257 Compare June 21, 2019 06:11

ormsbee approved these changes Jun 21, 2019

View reviewed changes

xitij2000 force-pushed the kshtij/problem-response-multi-root-and-fitler branch from ec2e257 to 4514e10 Compare July 2, 2019 20:42

openedx-webhooks added the engineering review label Jul 15, 2019

xitij2000 force-pushed the kshtij/problem-response-multi-root-and-fitler branch from 4514e10 to 98af749 Compare August 8, 2019 09:33

ormsbee merged commit 98deba6 into openedx:master Aug 8, 2019

xitij2000 deleted the kshtij/problem-response-multi-root-and-fitler branch August 9, 2019 10:20

DawoudSheraz mentioned this pull request Aug 9, 2019

Revert "[BB-873] Support for filters, and multiple roots in problem response reports" #21323

Merged

xitij2000 commented Aug 9, 2019

View reviewed changes

xitij2000 mentioned this pull request Aug 22, 2019

[BB-873] Support for filters, and multiple roots in problem response reports #21413

Merged

2 tasks

openedx-webhooks added merged and removed engineering review needs triage labels Jan 22, 2021

Uh oh!

Conversation

xitij2000 commented Feb 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openedx-webhooks commented Feb 11, 2019

Uh oh!

natabene commented Feb 11, 2019

Uh oh!

xitij2000 commented Feb 14, 2019

Uh oh!

giovannicimolin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xitij2000 commented Mar 18, 2019

Uh oh!

natabene commented Apr 8, 2019

Uh oh!

natabene commented Apr 8, 2019

Uh oh!

xitij2000 commented Apr 11, 2019

Uh oh!

natabene commented Apr 11, 2019

Uh oh!

xitij2000 commented May 6, 2019

Uh oh!

feanil commented Jun 19, 2019

Uh oh!

xitij2000 commented Jun 21, 2019

Uh oh!

xitij2000 commented Jul 2, 2019

Uh oh!

natabene commented Jul 15, 2019

Uh oh!

xitij2000 commented Aug 8, 2019

Uh oh!

openedx-webhooks commented Aug 8, 2019

Uh oh!

ormsbee commented Aug 8, 2019

Uh oh!

edx-pipeline-bot commented Aug 8, 2019

Uh oh!

xitij2000 commented Aug 9, 2019

Uh oh!

edx-status-bot commented Aug 9, 2019

Uh oh!

DawoudSheraz commented Aug 9, 2019

Uh oh!

xitij2000 commented Aug 9, 2019

Uh oh!

ormsbee commented Aug 9, 2019

Uh oh!

edx-pipeline-bot commented Aug 9, 2019

Uh oh!

xitij2000 Aug 9, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

xitij2000 commented Feb 11, 2019 •

edited

Loading

giovannicimolin left a comment •

edited

Loading