From c11a2c0a6ed5bd1c4cf633ec984f9a57f224303a Mon Sep 17 00:00:00 2001
From: Alex Dusenbery <adusenbery@edx.org>
Date: Mon, 15 Oct 2018 16:45:33 -0400
Subject: [PATCH] EDUCATOR-3471 | Add a grade override bulk update view.
---
.../grades/api/v1/tests/test_views.py | 289 +++++++++++++++++-
lms/djangoapps/grades/api/v1/urls.py | 5 +
lms/djangoapps/grades/api/v1/views.py | 239 ++++++++++++++-
.../docs/decisions/0001-gradebook-api.rst | 79 +++++
.../decisions/0002-gradebook-front-end.rst | 26 ++
lms/djangoapps/grades/signals/handlers.py | 1 +
lms/djangoapps/grades/tasks.py | 8 +-
lms/djangoapps/grades/tests/test_tasks.py | 1 +
.../core/djangoapps/user_authn/tests/utils.py | 12 +-
9 files changed, 626 insertions(+), 34 deletions(-)
create mode 100644 lms/djangoapps/grades/docs/decisions/0001-gradebook-api.rst
create mode 100644 lms/djangoapps/grades/docs/decisions/0002-gradebook-front-end.rst
diff --git a/lms/djangoapps/grades/api/v1/tests/test_views.py b/lms/djangoapps/grades/api/v1/tests/test_views.py
index 32286a9949d..46993c5f409 100644
--- a/lms/djangoapps/grades/api/v1/tests/test_views.py
+++ b/lms/djangoapps/grades/api/v1/tests/test_views.py
@@ -2,7 +2,8 @@
Tests for v1 views
"""
from __future__ import unicode_literals
-from collections import OrderedDict
+import json
+from collections import OrderedDict, namedtuple
from datetime import datetime
import ddt
@@ -19,6 +20,7 @@ from lms.djangoapps.grades.api.v1.views import CourseGradesView
from lms.djangoapps.grades.config.waffle import waffle_flags, WRITABLE_GRADEBOOK
from lms.djangoapps.grades.course_data import CourseData
from lms.djangoapps.grades.course_grade import CourseGrade
+from lms.djangoapps.grades.models import PersistentSubsectionGrade
from lms.djangoapps.grades.subsection_grade import ReadSubsectionGrade
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from openedx.core.djangoapps.user_authn.tests.utils import AuthAndScopesTestMixin
@@ -371,18 +373,18 @@ class CourseGradesViewTest(GradeViewTestMixin, APITestCase):
self.assertEqual(expected_data, resp.data)
-class GradebookViewTest(GradeViewTestMixin, APITestCase):
+class GradebookViewTestBase(GradeViewTestMixin, APITestCase):
"""
- Tests for the gradebook view.
+ Base class for the gradebook GET and POST view tests.
"""
-
@classmethod
def setUpClass(cls):
- super(GradebookViewTest, cls).setUpClass()
+ super(GradebookViewTestBase, cls).setUpClass()
cls.namespaced_url = 'grades_api:v1:course_gradebook'
cls.waffle_flag = waffle_flags()[WRITABLE_GRADEBOOK]
cls.course = CourseFactory.create(display_name='test-course', run='run-1')
+ cls.course_key = cls.course.id
cls.course_overview = CourseOverviewFactory.create(id=cls.course.id)
cls.chapter_1 = ItemFactory.create(
@@ -434,16 +436,34 @@ class GradebookViewTest(GradeViewTestMixin, APITestCase):
],
}
- def get_url(self, course_key=None, username=None):
+ def get_url(self, course_key=None):
"""
- Helper function to create the course gradebook API read url.
+ Helper function to create the course gradebook API url.
"""
- base_url = reverse(
+ return reverse(
self.namespaced_url,
kwargs={
'course_id': course_key or self.course_key,
}
)
+
+ def login_staff(self):
+ """
+ Helper function to login the global staff user, who has permissions to read from the
+ Gradebook API.
+ """
+ self.client.login(username=self.global_staff.username, password=self.password)
+
+
+class GradebookViewTest(GradebookViewTestBase):
+ """
+ Tests for the gradebook view.
+ """
+ def get_url(self, course_key=None, username=None): # pylint: disable=arguments-differ
+ """
+ Helper function to create the course gradebook API read url.
+ """
+ base_url = super(GradebookViewTest, self).get_url(course_key)
if username:
return "{0}?username={1}".format(base_url, username)
return base_url
@@ -504,13 +524,6 @@ class GradebookViewTest(GradeViewTestMixin, APITestCase):
])
return course_grade
- def login_staff(self):
- """
- Helper function to login the global staff user, who has permissions to read from the
- Gradebook API.
- """
- self.client.login(username=self.global_staff.username, password=self.password)
-
def expected_subsection_grades(self, letter_grade=None):
"""
Helper function to generate expected subsection detail results.
@@ -772,3 +785,249 @@ class GradebookViewTest(GradeViewTestMixin, APITestCase):
self.assertEqual(status.HTTP_200_OK, resp.status_code)
actual_data = dict(resp.data)
self.assertEqual(expected_results, actual_data)
+
+
+class GradebookBulkUpdateViewTest(GradebookViewTestBase):
+ """
+ Tests for the gradebook bulk-update view.
+ """
+ @classmethod
+ def setUpClass(cls):
+ super(GradebookBulkUpdateViewTest, cls).setUpClass()
+ cls.namespaced_url = 'grades_api:v1:course_gradebook_bulk_update'
+
+ def test_feature_not_enabled(self):
+ self.client.login(username=self.global_staff.username, password=self.password)
+ with override_waffle_flag(self.waffle_flag, active=False):
+ resp = self.client.post(
+ self.get_url(course_key=self.empty_course.id)
+ )
+ self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+
+ def test_anonymous(self):
+ with override_waffle_flag(self.waffle_flag, active=True):
+ resp = self.client.post(self.get_url())
+ self.assertEqual(status.HTTP_401_UNAUTHORIZED, resp.status_code)
+
+ def test_student(self):
+ self.client.login(username=self.student.username, password=self.password)
+ with override_waffle_flag(self.waffle_flag, active=True):
+ resp = self.client.post(self.get_url())
+ self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+
+ def test_course_does_not_exist(self):
+ with override_waffle_flag(self.waffle_flag, active=True):
+ self.login_staff()
+ resp = self.client.post(
+ self.get_url(course_key='course-v1:MITx+8.MechCX+2014_T1')
+ )
+ self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code)
+
+ def test_user_not_enrolled(self):
+ with override_waffle_flag(self.waffle_flag, active=True):
+ self.login_staff()
+ unenrolled_student = UserFactory()
+ post_data = [
+ {
+ 'user_id': unenrolled_student.id,
+ 'usage_id': text_type(self.subsections[self.chapter_1.location][0].location),
+ 'grade': {}, # doesn't matter what we put here.
+ }
+ ]
+
+ resp = self.client.post(
+ self.get_url(),
+ data=json.dumps(post_data),
+ content_type='application/json',
+ )
+
+ expected_data = [
+ {
+ 'user_id': unenrolled_student.id,
+ 'usage_id': text_type(self.subsections[self.chapter_1.location][0].location),
+ 'success': False,
+ 'reason': 'CourseEnrollment matching query does not exist.',
+ },
+ ]
+ self.assertEqual(status.HTTP_422_UNPROCESSABLE_ENTITY, resp.status_code)
+ self.assertEqual(expected_data, json.loads(resp.data))
+
+ def test_user_does_not_exist(self):
+ with override_waffle_flag(self.waffle_flag, active=True):
+ self.login_staff()
+ post_data = [
+ {
+ 'user_id': -123,
+ 'usage_id': text_type(self.subsections[self.chapter_1.location][0].location),
+ 'grade': {}, # doesn't matter what we put here.
+ }
+ ]
+
+ resp = self.client.post(
+ self.get_url(),
+ data=json.dumps(post_data),
+ content_type='application/json',
+ )
+
+ expected_data = [
+ {
+ 'user_id': -123,
+ 'usage_id': text_type(self.subsections[self.chapter_1.location][0].location),
+ 'success': False,
+ 'reason': 'User matching query does not exist.',
+ },
+ ]
+ self.assertEqual(status.HTTP_422_UNPROCESSABLE_ENTITY, resp.status_code)
+ self.assertEqual(expected_data, json.loads(resp.data))
+
+ def test_invalid_usage_key(self):
+ with override_waffle_flag(self.waffle_flag, active=True):
+ self.login_staff()
+ post_data = [
+ {
+ 'user_id': self.student.id,
+ 'usage_id': 'not-a-valid-usage-key',
+ 'grade': {}, # doesn't matter what we put here.
+ }
+ ]
+
+ resp = self.client.post(
+ self.get_url(),
+ data=json.dumps(post_data),
+ content_type='application/json',
+ )
+
+ expected_data = [
+ {
+ 'user_id': self.student.id,
+ 'usage_id': 'not-a-valid-usage-key',
+ 'success': False,
+ 'reason': "<class 'opaque_keys.edx.locator.BlockUsageLocator'>: not-a-valid-usage-key",
+ },
+ ]
+ self.assertEqual(status.HTTP_422_UNPROCESSABLE_ENTITY, resp.status_code)
+ self.assertEqual(expected_data, json.loads(resp.data))
+
+ def test_subsection_does_not_exist(self):
+ """
+ When trying to override a grade for a valid usage key that does not exist in the requested course,
+ we should get an error reason specifying that the key does not exist in the course.
+ """
+ with override_waffle_flag(self.waffle_flag, active=True):
+ self.login_staff()
+ usage_id = 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@workflow'
+ post_data = [
+ {
+ 'user_id': self.student.id,
+ 'usage_id': usage_id,
+ 'grade': {}, # doesn't matter what we put here.
+ }
+ ]
+
+ resp = self.client.post(
+ self.get_url(),
+ data=json.dumps(post_data),
+ content_type='application/json',
+ )
+
+ expected_data = [
+ {
+ 'user_id': self.student.id,
+ 'usage_id': usage_id,
+ 'success': False,
+ 'reason': 'usage_key {} does not exist in this course.'.format(usage_id),
+ },
+ ]
+ self.assertEqual(status.HTTP_422_UNPROCESSABLE_ENTITY, resp.status_code)
+ self.assertEqual(expected_data, json.loads(resp.data))
+
+ def test_override_is_created(self):
+ """
+ Test that when we make multiple requests to update grades for the same user/subsection,
+ the score from the most recent request is recorded.
+ """
+ with override_waffle_flag(self.waffle_flag, active=True):
+ self.login_staff()
+ post_data = [
+ {
+ 'user_id': self.student.id,
+ 'usage_id': text_type(self.subsections[self.chapter_1.location][0].location),
+ 'grade': {
+ 'earned_all_override': 3,
+ 'possible_all_override': 3,
+ 'earned_graded_override': 2,
+ 'possible_graded_override': 2,
+ },
+ },
+ {
+ 'user_id': self.student.id,
+ 'usage_id': text_type(self.subsections[self.chapter_1.location][1].location),
+ 'grade': {
+ 'earned_all_override': 1,
+ 'possible_all_override': 4,
+ 'earned_graded_override': 1,
+ 'possible_graded_override': 4,
+ },
+ }
+ ]
+
+ resp = self.client.post(
+ self.get_url(),
+ data=json.dumps(post_data),
+ content_type='application/json',
+ )
+
+ expected_data = [
+ {
+ 'user_id': self.student.id,
+ 'usage_id': text_type(self.subsections[self.chapter_1.location][0].location),
+ 'success': True,
+ 'reason': None,
+ },
+ {
+ 'user_id': self.student.id,
+ 'usage_id': text_type(self.subsections[self.chapter_1.location][1].location),
+ 'success': True,
+ 'reason': None,
+ },
+ ]
+ self.assertEqual(status.HTTP_202_ACCEPTED, resp.status_code)
+ self.assertEqual(expected_data, json.loads(resp.data))
+
+ second_post_data = [
+ {
+ 'user_id': self.student.id,
+ 'usage_id': text_type(self.subsections[self.chapter_1.location][1].location),
+ 'grade': {
+ 'earned_all_override': 3,
+ 'possible_all_override': 4,
+ 'earned_graded_override': 3,
+ 'possible_graded_override': 4,
+ },
+ },
+ ]
+
+ self.client.post(
+ self.get_url(),
+ data=json.dumps(second_post_data),
+ content_type='application/json',
+ )
+
+ GradeFields = namedtuple('GradeFields', ['earned_all', 'possible_all', 'earned_graded', 'possible_graded'])
+
+ # We should now have PersistentSubsectionGradeOverride records corresponding to
+ # our bulk-update request, and PersistentSubsectionGrade records with grade values
+ # equal to those of the override.
+ for usage_key, expected_grades in (
+ (self.subsections[self.chapter_1.location][0].location, GradeFields(3, 3, 2, 2)),
+ (self.subsections[self.chapter_1.location][1].location, GradeFields(3, 4, 3, 4)),
+ ):
+ # this selects related PersistentSubsectionGradeOverride objects.
+ grade = PersistentSubsectionGrade.read_grade(
+ user_id=self.student.id,
+ usage_key=usage_key,
+ )
+ for field_name in expected_grades._fields:
+ expected_value = getattr(expected_grades, field_name)
+ self.assertEqual(expected_value, getattr(grade, field_name))
+ self.assertEqual(expected_value, getattr(grade.override, field_name + '_override'))
diff --git a/lms/djangoapps/grades/api/v1/urls.py b/lms/djangoapps/grades/api/v1/urls.py
index 280651dc04e..5142c8f5107 100644
--- a/lms/djangoapps/grades/api/v1/urls.py
+++ b/lms/djangoapps/grades/api/v1/urls.py
@@ -29,4 +29,9 @@ urlpatterns = [
views.GradebookView.as_view(),
name='course_gradebook'
),
+ url(
+ r'^gradebook/{course_id}/bulk-update$'.format(course_id=settings.COURSE_ID_PATTERN),
+ views.GradebookBulkUpdateView.as_view(),
+ name='course_gradebook_bulk_update'
+ ),
]
diff --git a/lms/djangoapps/grades/api/v1/views.py b/lms/djangoapps/grades/api/v1/views.py
index 8d5aca13451..4744223287a 100644
--- a/lms/djangoapps/grades/api/v1/views.py
+++ b/lms/djangoapps/grades/api/v1/views.py
@@ -1,6 +1,7 @@
""" API v0 views. """
+import json
import logging
-from collections import defaultdict
+from collections import defaultdict, namedtuple
from contextlib import contextmanager
from functools import wraps
@@ -17,12 +18,16 @@ from courseware.courses import get_course_with_access
from edx_rest_framework_extensions import permissions
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser
-from enrollment import data as enrollment_data
from lms.djangoapps.grades.api.serializers import StudentGradebookEntrySerializer
from lms.djangoapps.grades.config.waffle import waffle_flags, WRITABLE_GRADEBOOK
+from lms.djangoapps.grades.constants import ScoreDatabaseTableEnum
+from lms.djangoapps.grades.course_data import CourseData
from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
+from lms.djangoapps.grades.models import PersistentSubsectionGrade, PersistentSubsectionGradeOverride
+from lms.djangoapps.grades.signals import signals
+from lms.djangoapps.grades.subsection_grade import CreateSubsectionGrade
from opaque_keys import InvalidKeyError
-from opaque_keys.edx.keys import CourseKey
+from opaque_keys.edx.keys import CourseKey, UsageKey
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.lib.api.authentication import OAuth2AuthenticationAllowInactiveUser
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin
@@ -106,13 +111,15 @@ class GradeViewMixin(DeveloperErrorViewMixin):
"""
Mixin class for Grades related views.
"""
- def _get_single_user(self, request, course_key):
+ def _get_single_user(self, request, course_key, user_id=None):
"""
- Returns a single USER_MODEL object corresponding to the request's `username` parameter,
- or the current `request.user` if no `username` was provided.
+ Returns a single USER_MODEL object corresponding to either the user_id provided, or if no id is provided,
+ then the request's `username` parameter, or the current `request.user` if no `username` was provided.
+
Args:
request (Request): django request object to check for username or request.user object
course_key (CourseLocator): The course to retrieve user grades for.
+ user_id (int): Optional user id to fetch the user object for.
Returns:
A USER_MODEL object.
@@ -121,15 +128,15 @@ class GradeViewMixin(DeveloperErrorViewMixin):
USER_MODEL.DoesNotExist if no such user exists.
CourseEnrollment.DoesNotExist if the user is not enrolled in the given course.
"""
- if 'username' in request.GET:
- username = request.GET.get('username')
+ if user_id:
+ # May raise USER_MODEL.DoesNotExist if no user with this id exists
+ grade_user = USER_MODEL.objects.get(id=user_id)
else:
- username = request.user.username
-
- grade_user = USER_MODEL.objects.get(username=username)
+ username = request.GET.get('username') or request.user.username
+ grade_user = USER_MODEL.objects.get(username=username)
- if not enrollment_data.get_course_enrollment(username, text_type(course_key)):
- raise CourseEnrollment.DoesNotExist
+ # May raise CourseEnrollment.DoesNotExist if no enrollment exists for this user/course.
+ _ = CourseEnrollment.objects.get(user=grade_user, course_id=course_key)
return grade_user
@@ -333,7 +340,7 @@ class CourseGradesView(GradeViewMixin, GenericAPIView):
if username:
# If there is a username passed, get grade for a single user
- with self._get_user_or_raise(request, course_id) as grade_user:
+ with self._get_user_or_raise(request, course_key) as grade_user:
return self._get_single_user_grade(grade_user, course_key)
else:
# If no username passed, get paginated list of grades for all users in course
@@ -559,7 +566,7 @@ class GradebookView(GradeViewMixin, GenericAPIView):
course = get_course_with_access(request.user, 'staff', course_key, depth=None)
if username:
- with self._get_user_or_raise(request, course_id) as grade_user:
+ with self._get_user_or_raise(request, course_key) as grade_user:
course_grade = CourseGradeFactory().read(grade_user, course)
entry = self._gradebook_entry(grade_user, course, course_grade)
@@ -573,3 +580,205 @@ class GradebookView(GradeViewMixin, GenericAPIView):
entries.append(self._gradebook_entry(user, course, course_grade))
serializer = StudentGradebookEntrySerializer(entries, many=True)
return self.get_paginated_response(serializer.data)
+
+
+GradebookUpdateResponseItem = namedtuple('GradebookUpdateResponseItem', ['user_id', 'usage_id', 'success', 'reason'])
+
+
+class GradebookBulkUpdateView(GradeViewMixin, GenericAPIView):
+ """
+ **Use Case**
+ Creates `PersistentSubsectionGradeOverride` objects for multiple (user_id, usage_id)
+ pairs in a given course, and invokes a Django signal to update subsection grades in
+ an asynchronous celery task.
+
+ **Example Request**
+ POST /api/grades/v1/gradebook/{course_id}/bulk-update
+
+ **POST Parameters**
+ This endpoint does not accept any URL parameters.
+
+ **Example POST Data**
+ [
+ {
+ "user_id": 9,
+ "usage_id": "block-v1:edX+DemoX+Demo_Course+type@sequential+block@basic_questions",
+ "grade": {
+ "earned_all_override": 11,
+ "possible_all_override": 11,
+ "earned_graded_override": 11,
+ "possible_graded_override": 11
+ }
+ },
+ {
+ "user_id": 9,
+ "usage_id": "block-v1:edX+DemoX+Demo_Course+type@sequential+block@advanced_questions",
+ "grade": {
+ "earned_all_override": 10,
+ "possible_all_override": 15,
+ "earned_graded_override": 9,
+ "possible_graded_override": 12
+ }
+ }
+ ]
+
+ **POST Response Values**
+ An HTTP 202 may be returned if a grade override was created for each of the requested (user_id, usage_id)
+ pairs in the request data.
+ An HTTP 403 may be returned if the `writable_gradebook` feature is not
+ enabled for this course.
+ An HTTP 404 may be returned for the following reasons:
+ * The requested course_key is invalid.
+ * No course corresponding to the requested key exists.
+ * The requesting user is not enrolled in the requested course.
+ An HTTP 422 may be returned if any of the requested (user_id, usage_id) pairs
+ did not have a grade override created due to some exception. A `reason` detailing the exception
+ is provided with each response item.
+
+ **Example successful POST Response**
+ [
+ {
+ "user_id": 9,
+ "usage_id": "some-requested-usage-id",
+ "success": true,
+ "reason": null
+ },
+ {
+ "user_id": 9,
+ "usage_id": "an-invalid-usage-id",
+ "success": false,
+ "reason": "<class 'opaque_keys.edx.locator.BlockUsageLocator'>: not-a-valid-usage-key"
+ },
+ {
+ "user_id": 9,
+ "usage_id": "a-valid-usage-key-that-doesn't-exist",
+ "success": false,
+ "reason": "a-valid-usage-key-that-doesn't-exist does not exist in this course"
+ },
+ {
+ "user_id": 1234-I-DO-NOT-EXIST,
+ "usage_id": "a-valid-usage-key",
+ "success": false,
+ "reason": "User matching query does not exist."
+ }
+ ]
+ """
+ authentication_classes = (
+ JwtAuthentication,
+ OAuth2AuthenticationAllowInactiveUser,
+ SessionAuthenticationAllowInactiveUser,
+ )
+
+ permission_classes = (permissions.JWT_RESTRICTED_APPLICATION_OR_USER_ACCESS,)
+
+ required_scopes = ['grades:write']
+
+ @verify_course_exists
+ @verify_writable_gradebook_enabled
+ def post(self, request, course_id):
+ """
+ Creates or updates `PersistentSubsectionGradeOverrides` for the (user_id, usage_key)
+ specified in the request data. The `SUBSECTION_OVERRIDE_CHANGED` signal is invoked
+ after the grade override is created, which triggers a celery task to update the
+ course and subsection grades for the specified user.
+ """
+ course_key = get_course_key(request, course_id)
+ course = get_course_with_access(request.user, 'staff', course_key, depth=None)
+
+ result = []
+
+ for user_data in request.data:
+ requested_user_id = user_data['user_id']
+ requested_usage_id = user_data['usage_id']
+ try:
+ user = self._get_single_user(request, course_key, requested_user_id)
+ usage_key = UsageKey.from_string(requested_usage_id)
+ except (USER_MODEL.DoesNotExist, InvalidKeyError, CourseEnrollment.DoesNotExist) as exc:
+ result.append(GradebookUpdateResponseItem(
+ user_id=requested_user_id,
+ usage_id=requested_usage_id,
+ success=False,
+ reason=text_type(exc)
+ ))
+ continue
+
+ try:
+ subsection_grade_model = PersistentSubsectionGrade.objects.get(
+ user_id=user.id,
+ course_id=course_key,
+ usage_key=usage_key
+ )
+ except PersistentSubsectionGrade.DoesNotExist:
+ subsection = course.get_child(usage_key)
+ if subsection:
+ subsection_grade_model = self._create_subsection_grade(user, course, subsection)
+ else:
+ result.append(GradebookUpdateResponseItem(
+ user_id=requested_user_id,
+ usage_id=requested_usage_id,
+ success=False,
+ reason='usage_key {} does not exist in this course.'.format(usage_key)
+ ))
+ continue
+
+ if subsection_grade_model:
+ self._create_override(subsection_grade_model, **user_data['grade'])
+ result.append(GradebookUpdateResponseItem(
+ user_id=user.id,
+ usage_id=text_type(usage_key),
+ success=True,
+ reason=None
+ ))
+
+ status_code = status.HTTP_422_UNPROCESSABLE_ENTITY
+ if all((item.success for item in result)):
+ status_code = status.HTTP_202_ACCEPTED
+
+ return Response(
+ json.dumps([item._asdict() for item in result]),
+ status=status_code,
+ content_type='application/json'
+ )
+
+ def _create_subsection_grade(self, user, course, subsection):
+ course_data = CourseData(user, course=course)
+ subsection_grade = CreateSubsectionGrade(subsection, course_data.structure, {}, {})
+ return subsection_grade.update_or_create_model(user, force_update_subsections=True)
+
+ def _create_override(self, subsection_grade_model, **override_data):
+ """
+ Helper method to create a `PersistentSubsectionGradeOverride` object
+ and send a `SUBSECTION_OVERRIDE_CHANGED` signal.
+ """
+ override, _ = PersistentSubsectionGradeOverride.objects.update_or_create(
+ grade=subsection_grade_model,
+ defaults=self._clean_override_data(override_data),
+ )
+ signals.SUBSECTION_OVERRIDE_CHANGED.send(
+ sender=None,
+ user_id=subsection_grade_model.user_id,
+ course_id=text_type(subsection_grade_model.course_id),
+ usage_id=text_type(subsection_grade_model.usage_key),
+ only_if_higher=False,
+ modified=override.modified,
+ score_deleted=False,
+ score_db_table=ScoreDatabaseTableEnum.overrides,
+ force_update_subsections=True,
+ )
+
+ def _clean_override_data(self, override_data):
+ """
+ Helper method to strip any grade override field names that won't work
+ as defaults when calling PersistentSubsectionGradeOverride.update_or_create().
+ """
+ allowed_fields = {
+ 'earned_all_override',
+ 'possible_all_override',
+ 'earned_graded_override',
+ 'possible_graded_override',
+ }
+ stripped_data = {}
+ for field in override_data.keys():
+ if field in allowed_fields:
+ stripped_data[field] = override_data[field]
+ return stripped_data
diff --git a/lms/djangoapps/grades/docs/decisions/0001-gradebook-api.rst b/lms/djangoapps/grades/docs/decisions/0001-gradebook-api.rst
new file mode 100644
index 00000000000..7f7f7a5fe55
--- /dev/null
+++ b/lms/djangoapps/grades/docs/decisions/0001-gradebook-api.rst
@@ -0,0 +1,79 @@
+1. Gradebook read and write APIs
+--------------------------------
+
+Status
+======
+
+Accepted
+
+Context
+=======
+
+We are implementing a "Writable Gradebook" feature from the instructor dashboard.
+This feature supports both the reading of subsection grades (e.g. for HW assignments, Labs, Exams)
+and the creation/modification of subsection grades from a user interface. This document captures
+decisions related to the design of the Django APIs that support this feature. This feature is heavily
+inspired by an implementation provided by Extension Engine (EE).
+
+Decisions
+=========
+
+#. **Feature-gating**
+
+ a. This feature will be gated behind a `CourseWaffleFlag`. This will allow us to roll out to a few courses
+ as a time and to defer decisions that may need to be made about the scalability of this feature when
+ applied to courses with a massive number (i.e. hundreds of thousands) of enrollments. We can eventually
+ remove the check for this flag. When the flag is not enabled for a course, the endpoints will provide a
+ response and status code indicating as much.
+
+#. **The read (GET) API**
+
+ a. The read API supports either fetching subsection scores for a single user, by `username`, or fetching
+ a paginated result of subsection grade data for all enrollees in the requested course.
+
+ b. We will use the data schema required by the EE's front-end implementation. This will allow us to port
+ over much of EE's front-end code with only minor modifications. Note that there are some fields specified
+ in EE's schema that will not be needed in the edX implementation, and we may remove those fields in future
+ versions of this API.
+
+ c. We will use the Django Rest Framework `CursorPagination` class as the base pagination class for all students' data
+ in a course. The query set that we paginate is the set of active enrollees for the requested course. As a result
+ of using this pagination class, paginated responses will not contain a `count` key, and the pagination query
+ parameter `cursor` will have very opaque values. Furthermore, due to the version of DRF installed in edx-platform,
+ there is no available page size parameter available to clients of this API (although this can easily be added
+ to our pagination class).
+
+ d. The same pagination class as above is used as the pagination class for the `CourseGradesView` API. This is for
+ consistency, and also so that responses from this endpoint will be properly paginated (they previously contained
+ only the paginated data, and relied on the client "knowing" that further pages were available by using the
+ `?page=N` query parameter).
+
+#. **The write (POST) API**
+
+ a. The write API will be a `bulk-update` endpoint that allows for the creation/modification of subsection
+ grades for multiple users and sections in a single request. This allows clients of the API to limit
+ the number of network requests made and to more easily manage client-side data.
+
+ b. The write API will act as a `create-or-update` endpoint. That is, if a persistent subsection grade record
+ does not currently exist for a given user/subsection that the client intends to update, part of the
+ request involves creating a corresponding persistent subsection grade record in addition to creating an override
+ associated with that grade. We do this because our grading system makes an assumption that a subsection's
+ grade value is 0 if no record for that subsection exists, and therefore, a client reading grade data
+ cannot make a distinction if a given subsection grade is 0 because no attempt or record exists, or if
+ there is a recorded score somewhere of zero. This should be completely opaque to that client, and thus,
+ we create a grade if necessary in this endpoint.
+
+ c. We won't fail the entire request if one item in the batch data raises an exception. Instead, we will
+ report the status (as a boolean value) for each requested grade override item in the request back to the client,
+ along with a reason for items that have ``success: false`` entry.
+
+ d. A status code of ``422`` will be returned for requests that contain any failed item. This allows a client
+ to easily tell if any item in their request payload was problematic and needs special handling. If all
+ requested items succeed, a ``202 (accepted)`` is returned. This status code was chosen because an
+ asynchronous celery task is enqueued for each subsection grade that needs to be updated.
+
+ e. We have to thread a ``force_update_subsections`` keyword argument through the Django signal invocation
+ that enqueues the subsection update task. This is because we may be creating a new subsection grade
+ with no score data available from either ``courseware.StudentModule`` records or from the `Submissions` API.
+ In this case, the only score data available exists in the grade override record, and the subsection ``update()``
+ call should be forced to read from this record.
diff --git a/lms/djangoapps/grades/docs/decisions/0002-gradebook-front-end.rst b/lms/djangoapps/grades/docs/decisions/0002-gradebook-front-end.rst
new file mode 100644
index 00000000000..f395c230970
--- /dev/null
+++ b/lms/djangoapps/grades/docs/decisions/0002-gradebook-front-end.rst
@@ -0,0 +1,26 @@
+2. Gradebook User Interface and Experience
+------------------------------------------
+
+Status
+======
+
+Proposed
+
+Context
+=======
+
+We are implementing a "Writable Gradebook" feature from the instructor dashboard. For additional
+background see 0001-gradebook-api_.
+
+.. _0001-gradebook-api: 0001-gradebook-api.rst
+
+Decisions
+=========
+
+#. **Don't display non-graded sections**
+
+ a. Any subsections that are marked as ``graded: false`` will not be displayed in the UI. This allows
+ us to ignore problems where such sections don't possess a ``label``.
+
+ b. We will eliminate the `comment` column/field in the modal that allows a user to edit grades. We currently
+ have no model in which such data can be stored.
diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py
index c900834c3ce..6e288dfbbab 100644
--- a/lms/djangoapps/grades/signals/handlers.py
+++ b/lms/djangoapps/grades/signals/handlers.py
@@ -227,6 +227,7 @@ def enqueue_subsection_update(sender, **kwargs): # pylint: disable=unused-argum
event_transaction_id=unicode(get_event_transaction_id()),
event_transaction_type=unicode(get_event_transaction_type()),
score_db_table=kwargs['score_db_table'],
+ force_update_subsections=kwargs.get('force_update_subsections', False),
),
countdown=RECALCULATE_GRADE_DELAY_SECONDS,
)
diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py
index 7e09ad7d851..0b5e149c56a 100644
--- a/lms/djangoapps/grades/tasks.py
+++ b/lms/djangoapps/grades/tasks.py
@@ -237,6 +237,7 @@ def _recalculate_subsection_grade(self, **kwargs):
kwargs['only_if_higher'],
kwargs['user_id'],
kwargs['score_deleted'],
+ kwargs.get('force_update_subsections', False),
)
except Exception as exc:
if not isinstance(exc, KNOWN_RETRY_ERRORS):
@@ -296,7 +297,9 @@ def _has_db_updated_with_new_score(self, scored_block_usage_key, **kwargs):
return db_is_updated
-def _update_subsection_grades(course_key, scored_block_usage_key, only_if_higher, user_id, score_deleted):
+def _update_subsection_grades(
+ course_key, scored_block_usage_key, only_if_higher, user_id, score_deleted, force_update_subsections=False
+):
"""
A helper function to update subsection grades in the database
for each subsection containing the given block, and to signal
@@ -321,7 +324,8 @@ def _update_subsection_grades(course_key, scored_block_usage_key, only_if_higher
subsection_grade = subsection_grade_factory.update(
course_structure[subsection_usage_key],
only_if_higher,
- score_deleted
+ score_deleted,
+ force_update_subsections,
)
SUBSECTION_SCORE_CHANGED.send(
sender=None,
diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py
index 66fd2e1919b..5400afa90cc 100644
--- a/lms/djangoapps/grades/tests/test_tasks.py
+++ b/lms/djangoapps/grades/tests/test_tasks.py
@@ -148,6 +148,7 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
send_args = self.problem_weighted_score_changed_kwargs
local_task_args = self.recalculate_subsection_grade_kwargs.copy()
local_task_args['event_transaction_type'] = u'edx.grades.problem.submitted'
+ local_task_args['force_update_subsections'] = False
with self.mock_csm_get_score() and patch(
'lms.djangoapps.grades.tasks.recalculate_subsection_grade_v3.apply_async',
return_value=None
diff --git a/openedx/core/djangoapps/user_authn/tests/utils.py b/openedx/core/djangoapps/user_authn/tests/utils.py
index 82318cce8f7..306ea9e3014 100644
--- a/openedx/core/djangoapps/user_authn/tests/utils.py
+++ b/openedx/core/djangoapps/user_authn/tests/utils.py
@@ -4,6 +4,7 @@ from enum import Enum
from itertools import product
import ddt
+import pytz
from mock import patch
from django.conf import settings
from oauth2_provider import models as dot_models
@@ -37,6 +38,13 @@ def setup_login_oauth_client():
)
+def utcnow():
+ """
+ Helper function to return the current UTC time localized to the UTC timezone.
+ """
+ return datetime.now(pytz.UTC)
+
+
@ddt.ddt
class AuthAndScopesTestMixin(object):
"""
@@ -102,7 +110,7 @@ class AuthAndScopesTestMixin(object):
return dot_models.AccessToken.objects.create(
user=user,
application=dot_app,
- expires=datetime.utcnow() + timedelta(weeks=1),
+ expires=utcnow() + timedelta(weeks=1),
scope='read write',
token='test_token',
)
@@ -244,7 +252,7 @@ class AuthAndScopesTestMixin(object):
def test_expired_oauth_token(self):
token = self._create_oauth_token(self.student)
- token.expires = datetime.utcnow() - timedelta(weeks=1)
+ token.expires = utcnow() - timedelta(weeks=1)
token.save()
resp = self.get_response(AuthType.oauth, token=token)
self.assertEqual(resp.status_code, status.HTTP_401_UNAUTHORIZED)
--
GitLab