Skip to content

WIP: Asynchronous download button for ORA2 data - #11167

Merged
efischer19 merged 1 commit into
masterfrom
christina/ora-data-download
Mar 10, 2016
Merged

WIP: Asynchronous download button for ORA2 data#11167
efischer19 merged 1 commit into
masterfrom
christina/ora-data-download

Conversation

@cahrens

@cahrens cahrens commented Jan 7, 2016

Copy link
Copy Markdown

Conflicts:
lms/djangoapps/instructor/tests/test_api.py
lms/djangoapps/instructor/utils.py
lms/djangoapps/instructor/views/api.py
lms/djangoapps/instructor/views/api_urls.py
lms/djangoapps/instructor/views/instructor_dashboard.py
lms/djangoapps/instructor_task/api.py
lms/djangoapps/instructor_task/tasks.py
lms/djangoapps/instructor_task/tasks_helper.py
lms/djangoapps/instructor_task/tests/test_api.py
lms/djangoapps/instructor_task/tests/test_tasks.py
lms/djangoapps/instructor_task/tests/test_tasks_helper.py
lms/envs/aws.py
lms/envs/common.py
lms/static/coffee/src/instructor_dashboard/data_download.coffee
lms/templates/instructor/instructor_dashboard_2/data_download.html

@cahrens
cahrens force-pushed the christina/ora-data-download branch from b6ccc83 to 9f27b29 Compare January 7, 2016 21:03
@cahrens

cahrens commented Jan 8, 2016

Copy link
Copy Markdown
Author

jenkins run js

@dianakhuang

Copy link
Copy Markdown
Contributor

@catong this is the current version of the code. The sandbox where this code is running is here: http://dianakhuang.sandbox.edx.org

This code is not yet final.

@dianakhuang
dianakhuang force-pushed the christina/ora-data-download branch 2 times, most recently from d520ab3 to 2034c00 Compare February 5, 2016 21:30
@catong

catong commented Feb 8, 2016

Copy link
Copy Markdown
Contributor

@dianakhuang Given that most users won't know/care that our current ORA feature is "ORA2" (am I right?) I'd suggest that instead of referring to "ORA2" on the button and in the report name, we just go with "ORA". I wasn't sure of the best place to comment in the files view. What do you think?

@dianakhuang

Copy link
Copy Markdown
Contributor

@catong I'm happy to switch that around. I have a few other naming things to clean up and then I'll do the switch from "ORA2" to "ORA" in user-facing strings.

@dianakhuang
dianakhuang force-pushed the christina/ora-data-download branch 3 times, most recently from 3c7d2c0 to a157928 Compare February 8, 2016 19:46
@catong

catong commented Feb 8, 2016

Copy link
Copy Markdown
Contributor

Thanks @dianakhuang. I guess in the absence of Product we can make the call :-)

@dianakhuang
dianakhuang force-pushed the christina/ora-data-download branch 7 times, most recently from 0ce089a to b634451 Compare February 9, 2016 18:46
@dianakhuang

Copy link
Copy Markdown
Contributor

@catong The sandbox has been updated with the latest version of the code.

Comment thread lms/djangoapps/instructor/views/api.py Outdated
except AlreadyRunningError:
already_running_status = _(
"An ORA data report generation task is already in "
"progress. Check the 'Pending Instructor Tasks' table "

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section is now called "Pending Tasks" in the UI (strike "Instructor").

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Pending Instructor Tasks" also appears in other status messages below.

@catong

catong commented Feb 10, 2016

Copy link
Copy Markdown
Contributor

@dianakhuang Thanks for making the "ORA2" -> "ORA" changes. I have been able to start the report generation task as well as get the "already running" message in the Demo course in the sandbox, but have not yet been seen a completed report. If there are no peer responses are found in the course, should the report get returned with empty rows or a specific failure?

@dianakhuang

Copy link
Copy Markdown
Contributor

@catong Could you try it with the Testing course while I figure out what's going on with the Demo course?

@dianakhuang
dianakhuang force-pushed the christina/ora-data-download branch 2 times, most recently from d028e76 to c173b9b Compare February 11, 2016 14:13
else if e.target.name == 'problem-grade-report'
@$grades_request_response_error.text gettext("Error generating problem grade report. Please try again.")
else if e.target.name == 'ora2-response-btn'
@$grades_request_response_error.text gettext("Error generating ORA2 responses. Please try again.")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this error msg is user-facing, can we also change text to: "Error generating the ORA data report. Please try again."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dianakhuang This is an old comment from @catong. Maybe you discussed this elsewhere?

@catong

catong commented Feb 11, 2016

Copy link
Copy Markdown
Contributor

@dianakhuang I also noticed that the file name for the report might still use ORA2, it also seems less readable than other report filenames? Ideally the report name should match the button label: "ORA data report" rather than "ORA2 responses".
ora_datareport_filename

@dianakhuang
dianakhuang force-pushed the christina/ora-data-download branch from aae1875 to b766907 Compare March 4, 2016 15:25
@catong

catong commented Mar 4, 2016

Copy link
Copy Markdown
Contributor

@dianakhuang The readable headings look great!
Just one potential issue. I tested the report with fresh data in the Test Course. It seems that the text input for "Feedback on Peer Assessments" that is done by learners when all assessments are completed, isn't being shown in the "Feedback on Peer Assessments" column. This column previously was displaying text from that input field.

@dianakhuang

Copy link
Copy Markdown
Contributor

Thanks for that @catong ! I'll take a look to see why we might not be getting that information.

@dianakhuang
dianakhuang force-pushed the christina/ora-data-download branch 2 times, most recently from 4dbf85f to 8d279da Compare March 9, 2016 18:10

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of leaving this except in place, in case it's needed to diagnose prod issues.

Will this log the stack trace of the error? Is that easy/worthwhile to add?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line right below this logs the stack trace: TASK_LOG.exception will spit out the trace along with the error message.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. I figured it did since we found the bug earlier, just wanted to confirm.

@efischer19

Copy link
Copy Markdown
Contributor

jenkins run lettuce

@efischer19

Copy link
Copy Markdown
Contributor

👍 Looks great!

@dianakhuang
dianakhuang force-pushed the christina/ora-data-download branch from 8d279da to e76ab40 Compare March 9, 2016 19:54
Comment thread requirements/edx/github.txt Outdated

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still need to be updated.

@dianakhuang

Copy link
Copy Markdown
Contributor

jenkins run bokchoy

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Odd to use GRADES_DOWNLOAD.

@dianakhuang
dianakhuang force-pushed the christina/ora-data-download branch 2 times, most recently from 2689e98 to 918fb42 Compare March 9, 2016 21:39
@robrap

robrap commented Mar 9, 2016

Copy link
Copy Markdown
Contributor

👍 Thanks.

@dianakhuang
dianakhuang force-pushed the christina/ora-data-download branch from 918fb42 to fabc0a8 Compare March 9, 2016 22:06
@efischer19
efischer19 force-pushed the christina/ora-data-download branch from fabc0a8 to bd7b266 Compare March 10, 2016 14:58
Conflicts:
	lms/djangoapps/instructor/tests/test_api.py
	lms/djangoapps/instructor/utils.py
	lms/djangoapps/instructor/views/api.py
	lms/djangoapps/instructor/views/api_urls.py
	lms/djangoapps/instructor/views/instructor_dashboard.py
	lms/djangoapps/instructor_task/api.py
	lms/djangoapps/instructor_task/tasks.py
	lms/djangoapps/instructor_task/tasks_helper.py
	lms/djangoapps/instructor_task/tests/test_api.py
	lms/djangoapps/instructor_task/tests/test_tasks.py
	lms/djangoapps/instructor_task/tests/test_tasks_helper.py
	lms/envs/aws.py
	lms/envs/common.py
	lms/static/coffee/src/instructor_dashboard/data_download.coffee
	lms/templates/instructor/instructor_dashboard_2/data_download.html
@efischer19
efischer19 force-pushed the christina/ora-data-download branch from bd7b266 to 2b1a7ee Compare March 10, 2016 14:59
@efischer19

Copy link
Copy Markdown
Contributor

jenkins run bokchoy

efischer19 pushed a commit that referenced this pull request Mar 10, 2016
WIP: Asynchronous download button for ORA2 data
@efischer19
efischer19 merged commit a647169 into master Mar 10, 2016
@efischer19
efischer19 deleted the christina/ora-data-download branch March 10, 2016 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants