diff --git a/AUTHORS b/AUTHORS index 7b9f399c83..cdac79c676 100644 --- a/AUTHORS +++ b/AUTHORS @@ -278,3 +278,4 @@ Casey Litton Jhony Avella Tanmay Mohapatra Brian Mesick +Jeff LaJoie diff --git a/common/djangoapps/enrollment/api.py b/common/djangoapps/enrollment/api.py index 46ea7562e8..d97e31111f 100644 --- a/common/djangoapps/enrollment/api.py +++ b/common/djangoapps/enrollment/api.py @@ -37,8 +37,9 @@ def get_enrollments(user_id): "mode": "honor", "is_active": True, "user": "Bob", - "course": { + "course_details": { "course_id": "edX/DemoX/2014T2", + "course_name": "edX Demonstration Course", "enrollment_end": "2014-12-20T20:18:00Z", "enrollment_start": "2014-10-15T20:18:00Z", "course_start": "2015-02-03T00:00:00Z", @@ -64,8 +65,9 @@ def get_enrollments(user_id): "mode": "verified", "is_active": True, "user": "Bob", - "course": { + "course_details": { "course_id": "edX/edX-Insider/2014T2", + "course_name": "edX Insider Course", "enrollment_end": "2014-12-20T20:18:00Z", "enrollment_start": "2014-10-15T20:18:00Z", "course_start": "2015-02-03T00:00:00Z", @@ -111,8 +113,9 @@ def get_enrollment(user_id, course_id): "mode": "honor", "is_active": True, "user": "Bob", - "course": { + "course_details": { "course_id": "edX/DemoX/2014T2", + "course_name": "edX Demonstration Course", "enrollment_end": "2014-12-20T20:18:00Z", "enrollment_start": "2014-10-15T20:18:00Z", "course_start": "2015-02-03T00:00:00Z", @@ -163,8 +166,9 @@ def add_enrollment(user_id, course_id, mode=None, is_active=True): "mode": "audit", "is_active": True, "user": "Bob", - "course": { + "course_details": { "course_id": "edX/DemoX/2014T2", + "course_name": "edX Demonstration Course", "enrollment_end": "2014-12-20T20:18:00Z", "enrollment_start": "2014-10-15T20:18:00Z", "course_start": "2015-02-03T00:00:00Z", @@ -217,8 +221,9 @@ def update_enrollment(user_id, course_id, mode=None, is_active=None, enrollment_ "mode": "honor", "is_active": True, "user": "Bob", - "course": { + "course_details": { "course_id": "edX/DemoX/2014T2", + "course_name": "edX Demonstration Course", "enrollment_end": "2014-12-20T20:18:00Z", "enrollment_start": "2014-10-15T20:18:00Z", "course_start": "2015-02-03T00:00:00Z", @@ -282,6 +287,7 @@ def get_course_enrollment_details(course_id, include_expired=False): >>> get_course_enrollment_details("edX/DemoX/2014T2") { "course_id": "edX/DemoX/2014T2", + "course_name": "edX Demonstration Course", "enrollment_end": "2014-12-20T20:18:00Z", "enrollment_start": "2014-10-15T20:18:00Z", "course_start": "2015-02-03T00:00:00Z", diff --git a/common/djangoapps/enrollment/serializers.py b/common/djangoapps/enrollment/serializers.py index 1e894f210d..8f4d5357c9 100644 --- a/common/djangoapps/enrollment/serializers.py +++ b/common/djangoapps/enrollment/serializers.py @@ -36,6 +36,7 @@ class CourseSerializer(serializers.Serializer): # pylint: disable=abstract-meth """ course_id = serializers.CharField(source="id") + course_name = serializers.CharField(source="display_name_with_default") enrollment_start = serializers.DateTimeField(format=None) enrollment_end = serializers.DateTimeField(format=None) course_start = serializers.DateTimeField(source="start", format=None) diff --git a/common/djangoapps/enrollment/tests/test_data.py b/common/djangoapps/enrollment/tests/test_data.py index a8143488f4..36185cbb6d 100644 --- a/common/djangoapps/enrollment/tests/test_data.py +++ b/common/djangoapps/enrollment/tests/test_data.py @@ -71,6 +71,7 @@ class EnrollmentDataTest(ModuleStoreTestCase): # Confirm the returned enrollment and the data match up. self.assertEqual(course_mode, enrollment['mode']) self.assertEqual(is_active, enrollment['is_active']) + self.assertEqual(self.course.display_name_with_default, enrollment['course_details']['course_name']) def test_unenroll(self): # Enroll the user in the course diff --git a/common/djangoapps/enrollment/tests/test_views.py b/common/djangoapps/enrollment/tests/test_views.py index 8639480c1a..88bc30c398 100644 --- a/common/djangoapps/enrollment/tests/test_views.py +++ b/common/djangoapps/enrollment/tests/test_views.py @@ -191,8 +191,13 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase): ) # Create an enrollment - self.assert_enrollment_status() + resp = self.assert_enrollment_status() + # Verify that the response contains the correct course_name + data = json.loads(resp.content) + self.assertEqual(self.course.display_name_with_default, data['course_details']['course_name']) + + # Verify that the enrollment was created correctly self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course.id)) course_mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id) self.assertTrue(is_active) @@ -212,6 +217,7 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase): self.assertEqual(resp.status_code, status.HTTP_200_OK) data = json.loads(resp.content) self.assertEqual(unicode(self.course.id), data['course_details']['course_id']) + self.assertEqual(self.course.display_name_with_default, data['course_details']['course_name']) self.assertEqual(CourseMode.DEFAULT_MODE_SLUG, data['mode']) self.assertTrue(data['is_active']) @@ -329,8 +335,8 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) data = json.loads(response.content) self.assertItemsEqual( - [enrollment['course_details']['course_id'] for enrollment in data], - [unicode(course.id) for course in courses] + [(datum['course_details']['course_id'], datum['course_details']['course_name']) for datum in data], + [(unicode(course.id), course.display_name_with_default) for course in courses] ) def test_enrollment_list_permissions(self): @@ -411,6 +417,7 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase): data = json.loads(resp.content) self.assertEqual(unicode(self.course.id), data['course_id']) + self.assertEqual(self.course.display_name_with_default, data['course_name']) mode = data['course_modes'][0] self.assertEqual(mode['slug'], CourseMode.HONOR) self.assertEqual(mode['sku'], '123') diff --git a/common/djangoapps/enrollment/views.py b/common/djangoapps/enrollment/views.py index 776a5b8bbe..9f61346e4d 100644 --- a/common/djangoapps/enrollment/views.py +++ b/common/djangoapps/enrollment/views.py @@ -99,6 +99,7 @@ class EnrollmentView(APIView, ApiKeyPermissionMixIn): * course_end: The date and time when the course closes. If null, the course never ends. * course_id: The unique identifier for the course. + * course_name: The name of the course. * course_modes: An array of data about the enrollment modes supported for the course. If the request uses the parameter include_expired=1, the array also includes expired @@ -216,6 +217,7 @@ class EnrollmentCourseDetailView(APIView): * course_end: The date and time when the course closes. If null, the course never ends. * course_id: The unique identifier for the course. + * course_name: The name of the course. * course_modes: An array of data about the enrollment modes supported for the course. If the request uses the parameter include_expired=1, the array also includes expired @@ -400,6 +402,8 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): * course_id: The unique identifier for the course. + * course_name: The name of the course. + * course_modes: An array of data about the enrollment modes supported for the course. If the request uses the parameter include_expired=1, the array also includes expired diff --git a/common/lib/capa/capa/javascript_problem_generator.js b/common/lib/capa/capa/javascript_problem_generator.js deleted file mode 100644 index 473a5d8e8c..0000000000 --- a/common/lib/capa/capa/javascript_problem_generator.js +++ /dev/null @@ -1,28 +0,0 @@ -require('coffee-script'); -var importAll = function(modulePath) { - module = require(modulePath); - for (key in module) { - global[key] = module[key]; - } -}; - -importAll('mersenne-twister-min'); -importAll('xproblem'); - -generatorModulePath = process.argv[2]; -dependencies = JSON.parse(process.argv[3]); -seed = JSON.parse(process.argv[4]); -params = JSON.parse(process.argv[5]); - -if (seed == null) { - seed = 4; -} - -for (var i = 0; i < dependencies.length; i++) { - importAll(dependencies[i]); -} - -generatorModule = require(generatorModulePath); -generatorClass = generatorModule.generatorClass; -generator = new generatorClass(seed, params); -console.log(JSON.stringify(generator.generate())); diff --git a/common/lib/capa/capa/javascript_problem_grader.js b/common/lib/capa/capa/javascript_problem_grader.js deleted file mode 100644 index 02cb7da7f2..0000000000 --- a/common/lib/capa/capa/javascript_problem_grader.js +++ /dev/null @@ -1,26 +0,0 @@ -require('coffee-script'); -var importAll = function(modulePath) { - module = require(modulePath); - for (key in module) { - global[key] = module[key]; - } -}; - -importAll('xproblem'); - -graderModulePath = process.argv[2]; -dependencies = JSON.parse(process.argv[3]); -submission = JSON.parse(process.argv[4]); -problemState = JSON.parse(process.argv[5]); -params = JSON.parse(process.argv[6]); - -for (var i = 0; i < dependencies.length; i++) { - importAll(dependencies[i]); -} - -graderModule = require(graderModulePath); -graderClass = graderModule.graderClass; -grader = new graderClass(submission, problemState, params); -console.log(JSON.stringify(grader.grade())); -console.log(JSON.stringify(grader.evaluation)); -console.log(JSON.stringify(grader.solution)); diff --git a/common/lib/capa/capa/templates/chemicalequationinput.html b/common/lib/capa/capa/templates/chemicalequationinput.html index b61172951a..3f18a699e6 100644 --- a/common/lib/capa/capa/templates/chemicalequationinput.html +++ b/common/lib/capa/capa/templates/chemicalequationinput.html @@ -11,7 +11,7 @@ % endif /> -

+

${value|h} <%include file="status_span.html" args="status=status, status_id=id"/>

diff --git a/common/lib/capa/capa/templates/drag_and_drop_input.html b/common/lib/capa/capa/templates/drag_and_drop_input.html index a955453416..758968937e 100644 --- a/common/lib/capa/capa/templates/drag_and_drop_input.html +++ b/common/lib/capa/capa/templates/drag_and_drop_input.html @@ -18,7 +18,7 @@ style="display:none;"/> -

+

<%include file="status_span.html" args="status=status, status_id=id"/>

diff --git a/common/lib/capa/capa/templates/editageneinput.html b/common/lib/capa/capa/templates/editageneinput.html index c3dd75a3da..ae211835e0 100644 --- a/common/lib/capa/capa/templates/editageneinput.html +++ b/common/lib/capa/capa/templates/editageneinput.html @@ -11,7 +11,7 @@ -

+

<%include file="status_span.html" args="status=status, status_id=id"/>

diff --git a/common/lib/capa/capa/templates/editamolecule.html b/common/lib/capa/capa/templates/editamolecule.html index 83ca14a4f0..b4db77ab0c 100644 --- a/common/lib/capa/capa/templates/editamolecule.html +++ b/common/lib/capa/capa/templates/editamolecule.html @@ -16,7 +16,7 @@

-

+

<%include file="status_span.html" args="status=status, status_id=id"/>

diff --git a/common/lib/capa/capa/templates/jsinput.html b/common/lib/capa/capa/templates/jsinput.html index 7c108817a8..61a04a40eb 100644 --- a/common/lib/capa/capa/templates/jsinput.html +++ b/common/lib/capa/capa/templates/jsinput.html @@ -43,9 +43,9 @@

-

- <%include file="status_span.html" args="status=status, status_id=id"/> -

+
+ <%include file="status_span.html" args="status=status, status_id=id"/> +
diff --git a/common/lib/xmodule/xmodule/css/capa/display.scss b/common/lib/xmodule/xmodule/css/capa/display.scss index 8e24e08b66..ea5a2f0769 100644 --- a/common/lib/xmodule/xmodule/css/capa/display.scss +++ b/common/lib/xmodule/xmodule/css/capa/display.scss @@ -389,64 +389,31 @@ div.problem { } } - .unanswered { - p.status.drag-and-drop--status { - @include margin(8px, 0, 0, ($baseline/2)); - text-indent: 100%; - white-space: nowrap; - overflow: hidden; - } - } - &.correct, &.ui-icon-check { - p.status { - @include status-icon($correct, $checkmark-icon); - } - input { border-color: $correct; } } &.partially-correct, &.ui-icon-check { - p.status { - @include status-icon($partially-correct, $asterisk-icon); - } - input { border-color: $partially-correct; } } &.processing { - p.status { - display: inline-block; - width: 20px; - height: 20px; - background: url('#{$static-path}/images/spinner.gif') center center no-repeat; - } - input { border-color: #aaa; } } &.ui-icon-close { - p.status { - @include status-icon($incorrect, $cross-icon); - } - input { border-color: $incorrect; } } &.incorrect, &.incomplete { - - p.status { - @include status-icon($incorrect, $cross-icon); - } - input { border-color: $incorrect; } diff --git a/common/lib/xmodule/xmodule/templates/problem/jsinput_response.yaml b/common/lib/xmodule/xmodule/templates/problem/jsinput_response.yaml index 5a37b6ede3..20e7874695 100644 --- a/common/lib/xmodule/xmodule/templates/problem/jsinput_response.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/jsinput_response.yaml @@ -1,6 +1,6 @@ --- metadata: - display_name: Custom Javascript Display and Grading + display_name: Custom JavaScript Display and Grading markdown: !!null showanswer: never data: | @@ -8,8 +8,8 @@ data: |

In these problems (also called custom JavaScript problems or JS Input problems), you add a problem or tool that uses JavaScript in Studio. - Studio embeds the problem in an IFrame so that your students can - interact with it in the LMS. You can grade your students' work using + Studio embeds the problem in an IFrame so that your learners can + interact with it in the LMS. You can grade your learners' work using JavaScript and some basic Python, and the grading is integrated into the edX grading system.

@@ -31,42 +31,47 @@ data: |

When you add the problem, be sure to select Settings to specify a Display Name and other values that apply. + Also, be sure to specify a title attribute on the jsinput tag; + this title is used for the title attribute on the generated IFrame. Generally, + the title attribute on the IFrame should match the title tag of the HTML hosted + within the IFrame, which is specified by the html_file attribute.

You can use the following example problem as a model.

- + -

In the following image, click the objects until the cone is yellow and the cube is blue.

- This is paragraph text displayed before the IFrame.

+
diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index abeb12b143..e7653ef81c 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -13,7 +13,6 @@ import textwrap import unittest import ddt -import flaky from lxml import etree from mock import Mock, patch, DEFAULT import webob @@ -1412,7 +1411,6 @@ class CapaModuleTest(unittest.TestCase): RANDOMIZATION.ALWAYS, RANDOMIZATION.ONRESET ) - @flaky.flaky # TNL-6041 def test_random_seed_with_reset(self, rerandomize): """ Run the test for each possible rerandomize value @@ -1470,13 +1468,13 @@ class CapaModuleTest(unittest.TestCase): # to another valid seed else: - # Since there's a small chance we might get the - # same seed again, give it 5 chances + # Since there's a small chance (expected) we might get the + # same seed again, give it 10 chances # to generate a different seed - success = _retry_and_check(5, lambda: _reset_and_get_seed(module) != seed) + success = _retry_and_check(10, lambda: _reset_and_get_seed(module) != seed) self.assertIsNotNone(module.seed) - msg = 'Could not get a new seed from reset after 5 tries' + msg = 'Could not get a new seed from reset after 10 tries' self.assertTrue(success, msg) @ddt.data( diff --git a/common/static/css/capa/jsinput_css.css b/common/static/css/capa/jsinput_css.css deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/common/static/js/capa/jsinput/jsinput_example.css b/common/static/js/capa/jsinput/jsinput_example.css new file mode 100644 index 0000000000..8f1144ac40 --- /dev/null +++ b/common/static/js/capa/jsinput/jsinput_example.css @@ -0,0 +1,9 @@ +.directions { + font-size: large +} + +.feedback { + font-size: medium; + border: 2px solid cornflowerblue; + padding: 5px; +} diff --git a/common/static/js/capa/jsinput/jsinput_example.html b/common/static/js/capa/jsinput/jsinput_example.html new file mode 100644 index 0000000000..caa034fff9 --- /dev/null +++ b/common/static/js/capa/jsinput/jsinput_example.html @@ -0,0 +1,15 @@ + + + + Dropdown with Dynamic Text + + + + + + + + + diff --git a/common/static/js/capa/jsinput/jsinput_example.js b/common/static/js/capa/jsinput/jsinput_example.js new file mode 100644 index 0000000000..6aa125fd69 --- /dev/null +++ b/common/static/js/capa/jsinput/jsinput_example.js @@ -0,0 +1,86 @@ +/* globals Channel */ + +(function() { + 'use strict'; + + // state will be populated via initial_state via the `setState` method. Defining dummy values here + // to make the expected structure clear. + var state = { + availableChoices: [], + selectedChoice: '' + }, + channel, + select = document.getElementsByClassName('choices')[0], + feedback = document.getElementsByClassName('feedback')[0]; + + function populateSelect() { + // Populate the select from `state.availableChoices`. + var i, option; + + // Clear out any pre-existing options. + while (select.firstChild) { + select.removeChild(select.firstChild); + } + + // Populate the select with the available choices. + for (i = 0; i < state.availableChoices.length; i++) { + option = document.createElement('option'); + option.value = i; + option.innerHTML = state.availableChoices[i]; + if (state.availableChoices[i] === state.selectedChoice) { + option.selected = true; + } + select.appendChild(option); + } + feedback.innerText = "The currently selected answer is '" + state.selectedChoice + "'."; + } + + function getGrade() { + // The following return value may or may not be used to grade server-side. + // If getState and setState are used, then the Python grader also gets access + // to the return value of getState and can choose it instead to grade. + return JSON.stringify(state.selectedChoice); + } + + function getState() { + // Returns the current state (which can be used for grading). + return JSON.stringify(state); + } + + // This function will be called with 1 argument when JSChannel is not used, + // 2 otherwise. In the latter case, the first argument is a transaction + // object that will not be used here + // (see http://mozilla.github.io/jschannel/docs/) + function setState() { + var stateString = arguments.length === 1 ? arguments[0] : arguments[1]; + state = JSON.parse(stateString); + populateSelect(); + } + + // Establish a channel only if this application is embedded in an iframe. + // This will let the parent window communicate with this application using + // RPC and bypass SOP restrictions. + if (window.parent !== window) { + channel = Channel.build({ + window: window.parent, + origin: '*', + scope: 'JSInput' + }); + + channel.bind('getGrade', getGrade); + channel.bind('getState', getState); + channel.bind('setState', setState); + } + + select.addEventListener('change', function() { + state.selectedChoice = select.options[select.selectedIndex].text; + feedback.innerText = "You have selected '" + state.selectedChoice + + "'. Click Submit to grade your answer."; + }); + + return { + getState: getState, + setState: setState, + getGrade: getGrade + }; +}()); diff --git a/docs/en_us/platform_api/source/enrollment/enrollment.rst b/docs/en_us/platform_api/source/enrollment/enrollment.rst index 271f1979cd..f4dc8bfba3 100644 --- a/docs/en_us/platform_api/source/enrollment/enrollment.rst +++ b/docs/en_us/platform_api/source/enrollment/enrollment.rst @@ -33,6 +33,7 @@ Get the User's Enrollment Status in a Course "is_active": true, "course_details": { "course_id": "edX/DemoX/Demo_Course", + "course_name": "edX Demonstration Course", "enrollment_end": null, "course_modes": [ { @@ -70,6 +71,7 @@ Get the User's Enrollment Information for a Course { "course_id": "edX/DemoX/Demo_Course", + "course_name": "edX Demonstration Course", "enrollment_end": null, "course_modes": [ { @@ -112,6 +114,7 @@ View a User's Enrollments or Enroll a User in a Course "is_active": true, "course_details": { "course_id": "edX/DemoX/Demo_Course", + "course_name": "edX Demonstration Course", "enrollment_end": null, "course_modes": [ { @@ -135,6 +138,7 @@ View a User's Enrollments or Enroll a User in a Course "is_active": true, "course_details": { "course_id": "ArbisoftX/BulkyEmail101/2014-15", + "course_name": "Course Name Here", "enrollment_end": null, "course_modes": [ { diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index d213ec46e2..c58647adc4 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -5,6 +5,7 @@ This module contains tasks for asynchronous execution of grade updates. from celery import task from django.conf import settings from django.contrib.auth.models import User +from django.core.exceptions import ValidationError from django.db.utils import DatabaseError from logging import getLogger @@ -30,6 +31,8 @@ from .transformer import GradesTransformer log = getLogger(__name__) +KNOWN_RETRY_ERRORS = (DatabaseError, ValidationError) # Errors we expect occasionally, should be resolved on retry + @task(default_retry_delay=30, routing_key=settings.RECALCULATE_GRADES_ROUTING_KEY) def recalculate_subsection_grade( @@ -72,41 +75,46 @@ def recalculate_subsection_grade_v2(**kwargs): event_transaction_type(string): human-readable type of the event at the root of the current event transaction. """ - course_key = CourseLocator.from_string(kwargs['course_id']) - if not PersistentGradesEnabledFlag.feature_enabled(course_key): - return + try: + course_key = CourseLocator.from_string(kwargs['course_id']) + if not PersistentGradesEnabledFlag.feature_enabled(course_key): + return - score_deleted = kwargs['score_deleted'] - scored_block_usage_key = UsageKey.from_string(kwargs['usage_id']).replace(course_key=course_key) - expected_modified_time = from_timestamp(kwargs['expected_modified_time']) + score_deleted = kwargs['score_deleted'] + scored_block_usage_key = UsageKey.from_string(kwargs['usage_id']).replace(course_key=course_key) + expected_modified_time = from_timestamp(kwargs['expected_modified_time']) - # The request cache is not maintained on celery workers, - # where this code runs. So we take the values from the - # main request cache and store them in the local request - # cache. This correlates model-level grading events with - # higher-level ones. - set_event_transaction_id(kwargs.pop('event_transaction_id', None)) - set_event_transaction_type(kwargs.pop('event_transaction_type', None)) + # The request cache is not maintained on celery workers, + # where this code runs. So we take the values from the + # main request cache and store them in the local request + # cache. This correlates model-level grading events with + # higher-level ones. + set_event_transaction_id(kwargs.pop('event_transaction_id', None)) + set_event_transaction_type(kwargs.pop('event_transaction_type', None)) - # Verify the database has been updated with the scores when the task was - # created. This race condition occurs if the transaction in the task - # creator's process hasn't committed before the task initiates in the worker - # process. - if not _has_database_updated_with_new_score( - kwargs['user_id'], scored_block_usage_key, expected_modified_time, score_deleted, - ): - raise _retry_recalculate_subsection_grade(**kwargs) + # Verify the database has been updated with the scores when the task was + # created. This race condition occurs if the transaction in the task + # creator's process hasn't committed before the task initiates in the worker + # process. + if not _has_database_updated_with_new_score( + kwargs['user_id'], scored_block_usage_key, expected_modified_time, score_deleted, + ): + raise _retry_recalculate_subsection_grade(**kwargs) - _update_subsection_grades( - course_key, - scored_block_usage_key, - kwargs['only_if_higher'], - kwargs['course_id'], - kwargs['user_id'], - kwargs['usage_id'], - kwargs['expected_modified_time'], - score_deleted, - ) + _update_subsection_grades( + course_key, + scored_block_usage_key, + kwargs['only_if_higher'], + kwargs['user_id'], + ) + + except Exception as exc: # pylint: disable=broad-except + if not isinstance(exc, KNOWN_RETRY_ERRORS): + log.info("tnl-6244 grades unexpected failure: {}. kwargs={}".format( + repr(exc), + kwargs + )) + raise _retry_recalculate_subsection_grade(exc=exc, **kwargs) def _has_database_updated_with_new_score( @@ -138,7 +146,7 @@ def _has_database_updated_with_new_score( if api_score is None: # Same case as the initial 'if' above, for submissions-specific scores return score_deleted - reported_modified_time = api_score.created_at + reported_modified_time = api_score['created_at'] else: reported_modified_time = score.modified @@ -149,11 +157,7 @@ def _update_subsection_grades( course_key, scored_block_usage_key, only_if_higher, - course_id, user_id, - usage_id, - expected_modified_time, - score_deleted, ): """ A helper function to update subsection grades in the database @@ -174,31 +178,19 @@ def _update_subsection_grades( course = store.get_course(course_key, depth=0) subsection_grade_factory = SubsectionGradeFactory(student, course, course_structure) - try: - for subsection_usage_key in subsections_to_update: - if subsection_usage_key in course_structure: - subsection_grade = subsection_grade_factory.update( - course_structure[subsection_usage_key], - only_if_higher, - ) - SUBSECTION_SCORE_CHANGED.send( - sender=recalculate_subsection_grade, - course=course, - course_structure=course_structure, - user=student, - subsection_grade=subsection_grade, - ) - - except DatabaseError as exc: - raise _retry_recalculate_subsection_grade( - user_id, - course_id, - usage_id, - only_if_higher, - expected_modified_time, - score_deleted, - exc, - ) + for subsection_usage_key in subsections_to_update: + if subsection_usage_key in course_structure: + subsection_grade = subsection_grade_factory.update( + course_structure[subsection_usage_key], + only_if_higher, + ) + SUBSECTION_SCORE_CHANGED.send( + sender=recalculate_subsection_grade, + course=course, + course_structure=course_structure, + user=student, + subsection_grade=subsection_grade, + ) def _retry_recalculate_subsection_grade( diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 50691d5275..2cfde45b3c 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -235,6 +235,18 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): ) self._assert_retry_called(mock_retry) + @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.retry') + def test_retry_subsection_grade_on_update_not_complete_sub(self, mock_retry): + self.set_up_course() + with patch('lms.djangoapps.grades.tasks.sub_api.get_score') as mock_sub_score: + mock_sub_score.return_value = { + 'created_at': datetime.utcnow().replace(tzinfo=pytz.UTC) - timedelta(days=1) + } + self._apply_recalculate_subsection_grade( + mock_score=MagicMock(module_type='openassessment') + ) + self._assert_retry_called(mock_retry) + @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.retry') def test_retry_subsection_grade_on_no_score(self, mock_retry): self.set_up_course() @@ -262,6 +274,32 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): self._apply_recalculate_subsection_grade() self.assertEquals(mock_course_signal.call_count, 1) + @patch('lms.djangoapps.grades.tasks.log') + @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.retry') + @patch('lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory.update') + def test_log_unknown_error(self, mock_update, mock_retry, mock_log): + """ + Ensures that unknown errors are logged before a retry. + """ + self.set_up_course() + mock_update.side_effect = Exception("General exception with no further detail!") + self._apply_recalculate_subsection_grade() + self.assertIn("General exception with no further detail!", mock_log.info.call_args[0][0]) + self._assert_retry_called(mock_retry) + + @patch('lms.djangoapps.grades.tasks.log') + @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.retry') + @patch('lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory.update') + def test_no_log_known_error(self, mock_update, mock_retry, mock_log): + """ + Ensures that known errors are not logged before a retry. + """ + self.set_up_course() + mock_update.side_effect = IntegrityError("race condition oh noes") + self._apply_recalculate_subsection_grade() + self.assertFalse(mock_log.info.called) + self._assert_retry_called(mock_retry) + def _apply_recalculate_subsection_grade( self, mock_score=MagicMock(modified=datetime.utcnow().replace(tzinfo=pytz.UTC) + timedelta(days=1)) diff --git a/lms/djangoapps/teams/models.py b/lms/djangoapps/teams/models.py index 2c6b7249d0..21967f2127 100644 --- a/lms/djangoapps/teams/models.py +++ b/lms/djangoapps/teams/models.py @@ -263,5 +263,5 @@ class CourseTeamMembership(models.Model): membership.team.save() membership.save() emit_team_event('edx.team.activity_updated', membership.team.course_id, { - 'team_id': membership.team_id, + 'team_id': membership.team.team_id, }) diff --git a/lms/djangoapps/teams/tests/test_models.py b/lms/djangoapps/teams/tests/test_models.py index 1938852af2..ea820329c2 100644 --- a/lms/djangoapps/teams/tests/test_models.py +++ b/lms/djangoapps/teams/tests/test_models.py @@ -172,7 +172,7 @@ class TeamSignalsTest(EventTestMixin, SharedModuleStoreTestCase): self.assertGreater(now, team_membership.last_activity_at) self.assert_event_emitted( 'edx.team.activity_updated', - team_id=team.id, + team_id=team.team_id, ) else: self.assertEqual(team.last_activity_at, team_last_activity) diff --git a/lms/envs/common.py b/lms/envs/common.py index a3ef8fca31..6c58a774cf 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -49,7 +49,7 @@ from lms.djangoapps.lms_xblock.mixin import LmsBlockMixin PLATFORM_NAME = "Your Platform Name Here" CC_MERCHANT_NAME = PLATFORM_NAME # Shows up in the platform footer, eg "(c) COPYRIGHT_YEAR" -COPYRIGHT_YEAR = "2016" +COPYRIGHT_YEAR = "2017" PLATFORM_FACEBOOK_ACCOUNT = "http://www.facebook.com/YourPlatformFacebookAccount" PLATFORM_TWITTER_ACCOUNT = "@YourPlatformTwitterAccount" diff --git a/pavelib/utils/test/bokchoy_utils.py b/pavelib/utils/test/bokchoy_utils.py index a760395550..ea9e7a49d2 100644 --- a/pavelib/utils/test/bokchoy_utils.py +++ b/pavelib/utils/test/bokchoy_utils.py @@ -83,7 +83,7 @@ def wait_for_server(server, port): attempts = 0 server_ok = False - while attempts < 20: + while attempts < 30: try: connection = httplib.HTTPConnection(server, port, timeout=10) connection.request('GET', '/') diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 238bf9e699..84c7cc2e1d 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -53,7 +53,7 @@ git+https://github.com/edx/MongoDBProxy.git@25b99097615bda06bd7cdfe5669ed80dc2a7 git+https://github.com/edx/nltk.git@2.0.6#egg=nltk==2.0.6 -e git+https://github.com/dementrock/pystache_custom.git@776973740bdaad83a3b029f96e415a7d1e8bec2f#egg=pystache_custom-dev -e git+https://github.com/appliedsec/pygeoip.git@95e69341cebf5a6a9fbf7c4f5439d458898bdc3b#egg=pygeoip --e git+https://github.com/jazkarta/edx-jsme.git@0908b4db16168382be5685e7e9b7b4747ac410e0#egg=edx-jsme +-e git+https://github.com/jazkarta/edx-jsme.git@690dbf75441fa91c7c4899df0b83d77f7deb5458#egg=edx-jsme git+https://github.com/edx/django-pyfs.git@1.0.3#egg=django-pyfs==1.0.3 git+https://github.com/mitodl/django-cas.git@v2.1.1#egg=django-cas -e git+https://github.com/dgrtwo/ParsePy.git@7949b9f754d1445eff8e8f20d0e967b9a6420639#egg=parse_rest