Revert "Inform openassessment to clear submission"

[efischer19]

This reverts commit 920cc3d.

@mikekatz I've run into an issue with a feature I fixed up as part of this PR, so I'm putting this up.

I may be able to fix things tomorrow morning, but if not I'd rather have this PR up and ready with tests passing so as not to slow anyone down. I'll touch base tomorrow.

[efischer19]

Error trace:

Feb 22 22:12:48 ip-10-3-11-173 [service_variant=lms][django.request][env:stage-edx-edxapp] ERROR [ip-10-3-11-173  3211] [base.py:256] - Internal Server Error: /courses/edX/DemoX.1/2014/instructor/api/reset_student_attempts
Traceback (most recent call last):
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 132, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/utils/decorators.py", line 145, in inner
    return func(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/newrelic-2.56.0.42/newrelic/hooks/framework_django.py", line 499, in wrapper
    return wrapped(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/utils/decorators.py", line 110, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/views/decorators/cache.py", line 43, in _cache_controlled
    response = viewfunc(request, *args, **kw)
  File "/edx/app/edxapp/edx-platform/lms/djangoapps/instructor/views/api.py", line 239, in wrapped
    return func(*args, **kwargs)
  File "/edx/app/edxapp/edx-platform/lms/djangoapps/instructor/views/api.py", line 175, in wrapped
    return func(*args, **kwargs)
  File "/edx/app/edxapp/edx-platform/lms/djangoapps/instructor/views/api.py", line 126, in wrapped
    return func(request, *args, **kwargs)
  File "/edx/app/edxapp/edx-platform/lms/djangoapps/instructor/views/api.py", line 1983, in reset_student_attempts
    enrollment.reset_student_attempts(course_id, student, module_state_key, delete_module=delete_module)
  File "/edx/app/edxapp/edx-platform/lms/djangoapps/instructor/enrollment.py", line 242, in reset_student_attempts
    item_id=unicode(module_state_key)
  File "/edx/app/edxapp/venvs/edxapp/src/ora2/openassessment/xblock/staff_area_mixin.py", line 512, in clear_student_state
    self._cancel_workflow(sub['uuid'], "Student state cleared")
  File "/edx/app/edxapp/venvs/edxapp/src/ora2/openassessment/xblock/staff_area_mixin.py", line 556, in _cancel_workflow
    student_item_dict = self.get_student_item_dict()
  File "/edx/app/edxapp/venvs/edxapp/src/ora2/openassessment/xblock/openassessmentblock.py", line 297, in get_student_item_dict
    course_id = self.course_id  # pylint:disable=E1101
  File "/edx/app/edxapp/venvs/edxapp/src/ora2/openassessment/xblock/openassessmentblock.py", line 225, in course_id
    return self._serialize_opaque_key(self.xmodule_runtime.course_id)  # pylint:disable=E1101
AttributeError: 'NoneType' object has no attribute 'course_id'

Relevant code:
edx-platform is calling into the openassessment xblock method clear_student_state, which is calling self._cancel_workflow, which calls self.get_student_item_dict(): https://github.com/edx/edx-ora2/blob/master/openassessment/xblock/staff_area_mixin.py#L556.
That ends up calling some dodgy logic to get the current course_id from the xblock runtime, but errors: https://github.com/edx/edx-ora2/blob/master/openassessment/xblock/openassessmentblock.py#L296

Points:

• _cancel_workflow() succeeds when called by a different codepath ("Remove Submission From Peer Grading").
• we don't even need the course_id at this juncture, it's just a part of the student_item. We only need the student_id here.

[efischer19]

@maxrothman heads up, we may be reverting the code with the edx-submissions migration we've been discussing. The error is above the edx-submissions API, so if you wanted, we could leave the edx-submissions version bump in this week's release and make next week's fix a code-only change. I'll leave that up to you.

[efischer19]

jenkins run lettuce

[cpennington]

@efischer19: In short: you're loading the XBlock from the modulestore (modulestore().get_item(...)), but you aren't ever binding it to a student, so xmodule_runtime is never set.

You need to call https://github.com/edx/edx-platform/blob/master/common/lib/xmodule/xmodule/x_module.py#L572, or load the xblock using an api (like module_render) that already does.

[efischer19]

I think we need to take a longer look at this than the impending release will allow.

I found this method, already in the same Instructor djangoapp, that would seem to do what we need: https://github.com/edx/edx-platform/blob/master/lms/djangoapps/instructor/utils.py#L30

However, reset_student_attempts, as it currently stands, has no access to the user we wish to bind to (the staff user performing the reset). In searching for where this method is called, I found this API method which does have easy access to that information (https://github.com/edx/edx-platform/blob/master/lms/djangoapps/instructor/views/api.py#L1927), but I also found this other location in InstructorService which does not have this information (https://github.com/edx/edx-platform/blob/master/lms/djangoapps/instructor/services.py#L30).

For the purposes of this RC, I'm going to revert the edx-platform code changes that call into the new clear_student_state method, effectively feature-flagging that behavior to OFF. We'll still keep the edx-ora2 and edx-submission version bumps in this week's release, as that will allow us to perform the migration needed and release a few other features from those repos. This change has already been pushed, I'll merge it once tests have passed.

The big questions to answer before this behavior gets re-released:

• Is it acceptable to use get_module_for_student(...), to bind things using a dummy request?
• If so, how can we get the current (staff) user's information, especially from the perspective of the InstructorService?

[efischer19]

jenkins run bokchoy

[efischer19]

The bokchoy run that just failed was https://build.testeng.edx.org/job/edx-platform-bok-choy-pr/14283/. I don't know why that would be related to anything in this PR.

At least 1 other branched-on-master PR has failed on that recently, hopefully it's just flaky. https://build.testeng.edx.org/job/edx-platform-bok-choy-pr/14277/testReport/

The test in question is https://github.com/edx/edx-platform/blob/master/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py#L257, I've manually confirmed that functionality to be working on stage.

[mikekatz]

@efischer19 is this ready to merge?

[efischer19]

Just now finished. The latest bokchoy run had one failure (https://build.testeng.edx.org/job/edx-platform-bok-choy-pr/14286/testReport/), but that's since been marked flaky on master (https://github.com/edx/edx-platform/pull/11629), so we're good to merge.