Skip to content

E
edx-platform
Commit 1febdbfa by Nimisha Asthagiri
Options

Grades cleanup: remove read_only param and create method 

EDUCATOR-171
parent 649f3ccd
Showing with 169 additions and 191 deletions
common/djangoapps/student/views.py
......@@ -413,7 +413,7 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa
if status in {'generating', 'ready', 'notpassing', 'restricted', 'auditing', 'unverified'}:
cert_grade_percent = -1
persisted_grade_percent = -1
persisted_grade = CourseGradeFactory().read(user, course=course_overview)
persisted_grade = CourseGradeFactory().read(user, course=course_overview, create_if_needed=False)
if persisted_grade is not None:
persisted_grade_percent = persisted_grade.percent
......
lms/djangoapps/ccx/tests/test_field_override_performance.py
......@@ -237,18 +237,18 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase):
# # of sql queries to default,
# # of mongo queries,
# )
('no_overrides', 1, True, False): (26, 1),
('no_overrides', 2, True, False): (26, 1),
('no_overrides', 3, True, False): (26, 1),
('ccx', 1, True, False): (26, 1),
('ccx', 2, True, False): (26, 1),
('ccx', 3, True, False): (26, 1),
('no_overrides', 1, False, False): (26, 1),
('no_overrides', 2, False, False): (26, 1),
('no_overrides', 3, False, False): (26, 1),
('ccx', 1, False, False): (26, 1),
('ccx', 2, False, False): (26, 1),
('ccx', 3, False, False): (26, 1),
('no_overrides', 1, True, False): (19, 1),
('no_overrides', 2, True, False): (19, 1),
('no_overrides', 3, True, False): (19, 1),
('ccx', 1, True, False): (19, 1),
('ccx', 2, True, False): (19, 1),
('ccx', 3, True, False): (19, 1),
('no_overrides', 1, False, False): (19, 1),
('no_overrides', 2, False, False): (19, 1),
('no_overrides', 3, False, False): (19, 1),
('ccx', 1, False, False): (19, 1),
('ccx', 2, False, False): (19, 1),
('ccx', 3, False, False): (19, 1),
}
......@@ -260,19 +260,19 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
__test__ = True
TEST_DATA = {
('no_overrides', 1, True, False): (26, 3),
('no_overrides', 2, True, False): (26, 3),
('no_overrides', 3, True, False): (26, 3),
('ccx', 1, True, False): (26, 3),
('ccx', 2, True, False): (26, 3),
('ccx', 3, True, False): (26, 3),
('ccx', 1, True, True): (27, 3),
('ccx', 2, True, True): (27, 3),
('ccx', 3, True, True): (27, 3),
('no_overrides', 1, False, False): (26, 3),
('no_overrides', 2, False, False): (26, 3),
('no_overrides', 3, False, False): (26, 3),
('ccx', 1, False, False): (26, 3),
('ccx', 2, False, False): (26, 3),
('ccx', 3, False, False): (26, 3),
('no_overrides', 1, True, False): (19, 3),
('no_overrides', 2, True, False): (19, 3),
('no_overrides', 3, True, False): (19, 3),
('ccx', 1, True, False): (19, 3),
('ccx', 2, True, False): (19, 3),
('ccx', 3, True, False): (19, 3),
('ccx', 1, True, True): (20, 3),
('ccx', 2, True, True): (20, 3),
('ccx', 3, True, True): (20, 3),
('no_overrides', 1, False, False): (19, 3),
('no_overrides', 2, False, False): (19, 3),
('no_overrides', 3, False, False): (19, 3),
('ccx', 1, False, False): (19, 3),
('ccx', 2, False, False): (19, 3),
('ccx', 3, False, False): (19, 3),
}
lms/djangoapps/ccx/tests/test_views.py
......@@ -36,6 +36,7 @@ from lms.djangoapps.ccx.tests.factories import CcxFactory
from lms.djangoapps.ccx.tests.utils import CcxTestCase, flatten
from lms.djangoapps.ccx.utils import ccx_course, is_email
from lms.djangoapps.ccx.views import get_date
from lms.djangoapps.grades.tasks import compute_all_grades_for_course
from lms.djangoapps.instructor.access import allow_access, list_with_level
from request_cache.middleware import RequestCache
from student.models import CourseEnrollment, CourseEnrollmentAllowed
......@@ -110,6 +111,8 @@ def setup_students_and_grades(context):
module_state_key=problem.location
)
compute_all_grades_for_course.apply_async(kwargs={'course_key': unicode(context.course.id)})
def unhide(unit):
"""
......
lms/djangoapps/ccx/utils.py
......@@ -265,12 +265,6 @@ def ccx_students_enrolling_center(action, identifiers, email_students, course_ke
return errors
def prep_course_for_grading(course, request):
"""Set up course module for overrides to function properly"""
course._field_data_cache = {} # pylint: disable=protected-access
course.set_grading_policy(course.grading_policy)
@contextmanager
def ccx_course(ccx_locator):
"""Create a context in which the course identified by course_locator exists
......
lms/djangoapps/ccx/views.py
......@@ -46,7 +46,6 @@ from lms.djangoapps.ccx.utils import (
get_ccx_for_coach,
get_date,
parse_date,
prep_course_for_grading
)
from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
from lms.djangoapps.instructor.enrollment import enroll_email, get_email_params
......@@ -519,7 +518,6 @@ def ccx_gradebook(request, course, ccx=None):
ccx_key = CCXLocator.from_course_locator(course.id, unicode(ccx.id))
with ccx_course(ccx_key) as course:
prep_course_for_grading(course, request)
student_info, page = get_grade_book_page(request, course, course_key=ccx_key)
return render_to_response('courseware/gradebook.html', {
......@@ -547,7 +545,6 @@ def ccx_grades_csv(request, course, ccx=None):
ccx_key = CCXLocator.from_course_locator(course.id, unicode(ccx.id))
with ccx_course(ccx_key) as course:
prep_course_for_grading(course, request)
enrolled_students = User.objects.filter(
courseenrollment__course_id=ccx_key,
......
lms/djangoapps/certificates/management/commands/fix_ungraded_certs.py
......@@ -51,7 +51,7 @@ class Command(BaseCommand):
course = courses.get_course_by_id(course_id)
for cert in ungraded:
# grade the student
grade = CourseGradeFactory().create(cert.user, course)
grade = CourseGradeFactory().read(cert.user, course)
log.info('grading %s - %s', cert.user, grade.percent)
cert.grade = grade.percent
if not options['noop']:
......
lms/djangoapps/certificates/queue.py
......@@ -269,7 +269,7 @@ class XQueueCertInterface(object):
self.request.session = {}
is_whitelisted = self.whitelist.filter(user=student, course_id=course_id, whitelist=True).exists()
course_grade = CourseGradeFactory().create(student, course)
course_grade = CourseGradeFactory().read(student, course)
enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(student, course_id)
mode_is_verified = enrollment_mode in GeneratedCertificate.VERIFIED_CERTS_MODES
user_is_verified = SoftwareSecurePhotoVerification.user_is_verified(student)
......
lms/djangoapps/certificates/tests/test_queue.py
......@@ -265,7 +265,7 @@ class XQueueCertInterfaceAddCertificateTest(ModuleStoreTestCase):
)
# Run grading/cert generation again
with mock_passing_grade(grade_pass=grade):
with mock_passing_grade(letter_grade=grade):
with patch.object(XQueueInterface, 'send_to_queue') as mock_send:
mock_send.return_value = (0, None)
self.xqueue.add_cert(self.user_2, self.course.id)
......
lms/djangoapps/courseware/tests/test_submitting_problems.py
......@@ -29,6 +29,7 @@ from course_modes.models import CourseMode
from courseware.models import BaseStudentModuleHistory, StudentModule
from courseware.tests.helpers import LoginEnrollmentTestCase
from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
from lms.djangoapps.grades.tasks import compute_all_grades_for_course
from openedx.core.djangoapps.credit.api import get_credit_requirement_status, set_credit_requirements
from openedx.core.djangoapps.credit.models import CreditCourse, CreditProvider
from openedx.core.djangoapps.user_api.tests.factories import UserCourseTagFactory
......@@ -143,6 +144,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl
COURSE_NAME = "test_course"
ENABLED_CACHES = ['default', 'mongo_metadata_inheritance', 'loc_cache']
ENABLED_SIGNALS = ['course_published']
def setUp(self):
super(TestSubmittingProblems, self).setUp()
......@@ -156,25 +158,6 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl
self.enroll(self.course)
self.student_user = User.objects.get(email=self.student)
self.factory = RequestFactory()
# Disable the score change signal to prevent other components from being pulled into tests.
self.score_changed_signal_patch = patch(
'lms.djangoapps.grades.signals.handlers.PROBLEM_WEIGHTED_SCORE_CHANGED.send'
)
self.score_changed_signal_patch.start()
def tearDown(self):
super(TestSubmittingProblems, self).tearDown()
self._stop_signal_patch()
def _stop_signal_patch(self):
"""
Stops the signal patch for the PROBLEM_WEIGHTED_SCORE_CHANGED event.
In case a test wants to test with the event actually
firing.
"""
if self.score_changed_signal_patch:
self.score_changed_signal_patch.stop()
self.score_changed_signal_patch = None
def add_dropdown_to_section(self, section_location, name, num_inputs=2):
"""
......@@ -278,7 +261,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl
"""
Return CourseGrade for current user and course.
"""
return CourseGradeFactory().create(self.student_user, self.course)
return CourseGradeFactory().read(self.student_user, self.course)
def check_grade_percent(self, percent):
"""
......@@ -424,6 +407,7 @@ class TestCourseGrader(TestSubmittingProblems):
]
}
self.add_grading_policy(grading_policy)
compute_all_grades_for_course.apply_async(kwargs={'course_key': unicode(self.course.id)})
def dropping_setup(self):
"""
......@@ -597,10 +581,6 @@ class TestCourseGrader(TestSubmittingProblems):
self.check_grade_percent(0.67)
self.assertEqual(self.get_course_grade().letter_grade, 'B')
# But now, set the score with the submissions API and watch
# as it overrides the score read from StudentModule and our
# student gets an A instead.
self._stop_signal_patch()
student_item = {
'student_id': anonymous_id_for_user(self.student_user, self.course.id),
'course_id': unicode(self.course.id),
......@@ -619,7 +599,6 @@ class TestCourseGrader(TestSubmittingProblems):
self.basic_setup()
self.submit_question_answer('p1', {'2_1': 'Correct'})
self.submit_question_answer('p2', {'2_1': 'Correct'})
self.submit_question_answer('p3', {'2_1': 'Incorrect'})
with patch('submissions.api.get_scores') as mock_get_scores:
mock_get_scores.return_value = {
......@@ -629,7 +608,7 @@ class TestCourseGrader(TestSubmittingProblems):
'created_at': now(),
},
}
self.get_course_grade()
self.submit_question_answer('p3', {'2_1': 'Incorrect'})
# Verify that the submissions API was sent an anonymized student ID
mock_get_scores.assert_called_with(
......@@ -764,7 +743,6 @@ class TestCourseGrader(TestSubmittingProblems):
req_status = get_credit_requirement_status(self.course.id, self.student_user.username, 'grade', 'grade')
self.assertEqual(req_status[0]["status"], None)
self._stop_signal_patch()
self.submit_question_answer('p1', {'2_1': 'Correct'})
self.submit_question_answer('p2', {'2_1': 'Correct'})
......
lms/djangoapps/courseware/tests/test_views.py
......@@ -42,8 +42,6 @@ from django.test.utils import override_settings
from lms.djangoapps.commerce.utils import EcommerceService # pylint: disable=import-error
from lms.djangoapps.grades.config.waffle import waffle as grades_waffle
from lms.djangoapps.grades.config.waffle import ASSUME_ZERO_GRADE_IF_ABSENT
from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
from lms.djangoapps.grades.tests.utils import mock_get_score
from milestones.tests.utils import MilestonesTestCaseMixin
from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locations import Location
......@@ -1399,7 +1397,7 @@ class ProgressPageTests(ProgressPageBaseTests):
self.course.save()
self.store.update_item(self.course, self.user.id)
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') as mock_create:
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create:
course_grade = mock_create.return_value
course_grade.passed = True
course_grade.summary = {'grade': 'Pass', 'percent': 0.75, 'section_breakdown': [], 'grade_breakdown': {}}
......@@ -1442,7 +1440,7 @@ class ProgressPageTests(ProgressPageBaseTests):
# Enable certificate generation for this course
certs_api.set_cert_generation_enabled(self.course.id, True)
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') as mock_create:
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create:
course_grade = mock_create.return_value
course_grade.passed = True
course_grade.summary = {'grade': 'Pass', 'percent': 0.75, 'section_breakdown': [], 'grade_breakdown': {}}
......@@ -1458,9 +1456,10 @@ class ProgressPageTests(ProgressPageBaseTests):
"""Test that query counts remain the same for self-paced and instructor-paced courses."""
SelfPacedConfiguration(enabled=self_paced_enabled).save()
self.setup_course(self_paced=self_paced)
with self.assertNumQueries(43, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST), check_mongo_calls(1):
with self.assertNumQueries(36, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST), check_mongo_calls(1):
self._get_progress_page()
@patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
@ddt.data(
(False, 43, 27),
(True, 36, 23)
......@@ -1504,7 +1503,7 @@ class ProgressPageTests(ProgressPageBaseTests):
'lms.djangoapps.verify_student.models.SoftwareSecurePhotoVerification.user_is_verified'
) as user_verify:
user_verify.return_value = user_verified
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') as mock_create:
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create:
course_grade = mock_create.return_value
course_grade.passed = True
course_grade.summary = {
......@@ -1548,7 +1547,7 @@ class ProgressPageTests(ProgressPageBaseTests):
self.course.save()
self.store.update_item(self.course, self.user.id)
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') as mock_create:
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create:
course_grade = mock_create.return_value
course_grade.passed = True
course_grade.summary = {
......@@ -1568,7 +1567,7 @@ class ProgressPageTests(ProgressPageBaseTests):
"http://www.example.com/certificate.pdf", "honor"
)
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') as mock_create:
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create:
course_grade = mock_create.return_value
course_grade.passed = True
course_grade.summary = {'grade': 'Pass', 'percent': 0.75, 'section_breakdown': [], 'grade_breakdown': {}}
......@@ -1586,7 +1585,7 @@ class ProgressPageTests(ProgressPageBaseTests):
self.assertTrue(self.client.login(username=user.username, password='test'))
CourseEnrollmentFactory(user=user, course_id=self.course.id, mode=CourseMode.AUDIT)
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') as mock_create:
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create:
course_grade = mock_create.return_value
course_grade.passed = True
course_grade.summary = {'grade': 'Pass', 'percent': 0.75, 'section_breakdown': [], 'grade_breakdown': {}}
......@@ -2090,7 +2089,7 @@ class GenerateUserCertTests(ModuleStoreTestCase):
status=CertificateStatuses.generating,
mode='verified'
)
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') as mock_create:
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create:
course_grade = mock_create.return_value
course_grade.passed = True
course_grade.summary = {'grade': 'Pass', 'percent': 0.75}
......@@ -2111,7 +2110,7 @@ class GenerateUserCertTests(ModuleStoreTestCase):
mode='verified'
)
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') as mock_create:
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create:
course_grade = mock_create.return_value
course_grade.passed = True
course_grade.summay = {'grade': 'Pass', 'percent': 0.75}
......
lms/djangoapps/courseware/views/index.py
......@@ -425,7 +425,7 @@ class CoursewareIndex(View):
if course_has_entrance_exam(self.course) and getattr(self.chapter, 'is_entrance_exam', False):
courseware_context['entrance_exam_passed'] = user_has_passed_entrance_exam(self.effective_user, self.course)
courseware_context['entrance_exam_current_score'] = get_entrance_exam_score_ratio(
CourseGradeFactory().create(self.effective_user, self.course),
CourseGradeFactory().read(self.effective_user, self.course),
get_entrance_exam_usage_key(self.course),
)
......
lms/djangoapps/courseware/views/views.py
......@@ -60,7 +60,6 @@ from enrollment.api import add_enrollment
from eventtracking import tracker
from ipware.ip import get_ip
from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException
from lms.djangoapps.ccx.utils import prep_course_for_grading
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect, Redirect
from lms.djangoapps.experiments.utils import get_experiment_user_metadata_context
from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
......@@ -922,12 +921,11 @@ def _progress(request, course_key, student_id):
if request.user.id != student.id:
# refetch the course as the assumed student
course = get_course_with_access(student, 'load', course_key, check_if_enrolled=True)
prep_course_for_grading(course, request)
# NOTE: To make sure impersonation by instructor works, use
# student instead of request.user in the rest of the function.
course_grade = CourseGradeFactory().create(student, course)
course_grade = CourseGradeFactory().read(student, course)
courseware_summary = course_grade.chapter_grades.values()
studio_url = get_studio_url(course, 'settings/grading')
......@@ -1015,7 +1013,7 @@ def _get_cert_data(student, course, enrollment_mode, course_grade=None):
certificates_enabled_for_course = certs_api.cert_generation_enabled(course.id)
if course_grade is None:
course_grade = CourseGradeFactory().create(student, course)
course_grade = CourseGradeFactory().read(student, course)
if not auto_certs_api.can_show_certificate_message(course, student, course_grade, certificates_enabled_for_course):
return
......@@ -1290,7 +1288,7 @@ def is_course_passed(student, course, course_grade=None):
returns bool value
"""
if course_grade is None:
course_grade = CourseGradeFactory().create(student, course)
course_grade = CourseGradeFactory().read(student, course)
return course_grade.passed
......
lms/djangoapps/gating/tests/test_integration.py
......@@ -149,7 +149,7 @@ class TestGatedContent(MilestonesTestCaseMixin, SharedModuleStoreTestCase):
all problems in the course, whether or not they are currently
gated.
"""
course_grade = CourseGradeFactory().create(user, self.course)
course_grade = CourseGradeFactory().read(user, self.course)
for prob in [self.gating_prob1, self.gated_prob2, self.prob3]:
self.assertIn(prob.location, course_grade.problem_scores)
......
lms/djangoapps/grades/api/tests/test_views.py
......@@ -15,7 +15,7 @@ from rest_framework.test import APITestCase
from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory
from lms.djangoapps.courseware.tests.factories import GlobalStaffFactory, StaffFactory
from lms.djangoapps.grades.tests.utils import mock_get_score
from lms.djangoapps.grades.tests.utils import mock_passing_grade
from student.tests.factories import CourseEnrollmentFactory, UserFactory
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, SharedModuleStoreTestCase
......@@ -149,7 +149,7 @@ class CurrentGradeViewTest(SharedModuleStoreTestCase, APITestCase):
"""
Test that a user can successfully request her own grade.
"""
with check_mongo_calls(6):
with check_mongo_calls(3):
resp = self.client.get(self.get_url(self.student.username))
self.assertEqual(resp.status_code, status.HTTP_200_OK)
......@@ -286,22 +286,21 @@ class CurrentGradeViewTest(SharedModuleStoreTestCase, APITestCase):
self.assertEqual(resp.data, expected_data) # pylint: disable=no-member
@ddt.data(
((2, 5), {'letter_grade': None, 'percent': 0.4, 'passed': False}),
((5, 5), {'letter_grade': 'Pass', 'percent': 1, 'passed': True}),
({'letter_grade': None, 'percent': 0.4, 'passed': False}),
({'letter_grade': 'Pass', 'percent': 1, 'passed': True}),
)
@ddt.unpack
def test_grade(self, grade, result):
def test_grade(self, grade):
"""
Test that the user gets her grade in case she answered tests with an insufficient score.
"""
with mock_get_score(*grade):
with mock_passing_grade(letter_grade=grade['letter_grade'], percent=grade['percent']):
resp = self.client.get(self.get_url(self.student.username))
self.assertEqual(resp.status_code, status.HTTP_200_OK)
expected_data = {
'username': self.student.username,
'course_key': str(self.course_key),
}
expected_data.update(result)
expected_data.update(grade)
self.assertEqual(resp.data, [expected_data]) # pylint: disable=no-member
......
lms/djangoapps/grades/api/views.py
......@@ -11,7 +11,6 @@ from rest_framework.generics import GenericAPIView, ListAPIView
from rest_framework.response import Response
from courseware.access import has_access
from lms.djangoapps.ccx.utils import prep_course_for_grading
from lms.djangoapps.courseware import courses
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
from lms.djangoapps.grades.api.serializers import GradingPolicySerializer
......@@ -184,8 +183,7 @@ class UserGradeView(GradeViewMixin, GenericAPIView):
# or a 404 if the requested user does not exist.
return grade_user
prep_course_for_grading(course, request)
course_grade = CourseGradeFactory().create(grade_user, course)
course_grade = CourseGradeFactory().read(grade_user, course)
return Response([{
'username': grade_user.username,
'course_key': course_id,
......
lms/djangoapps/grades/config/__init__.py
from django.conf import settings
from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag
from lms.djangoapps.grades.config.waffle import waffle as waffle_func, ASSUME_ZERO_GRADE_IF_ABSENT
......@@ -6,7 +8,12 @@ def assume_zero_if_absent(course_key):
"""
Returns whether an absent grade should be assumed to be zero.
"""
return should_persist_grades(course_key) and waffle_func().is_enabled(ASSUME_ZERO_GRADE_IF_ABSENT)
return (
should_persist_grades(course_key) and (
settings.FEATURES.get('ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS') or
waffle_func().is_enabled(ASSUME_ZERO_GRADE_IF_ABSENT)
)
)
def should_persist_grades(course_key):
......
lms/djangoapps/grades/course_data.py
......@@ -92,8 +92,9 @@ class CourseData(object):
def edited_on(self):
# get course block from structure only; subtree_edited_on field on modulestore's course block isn't optimized.
structure = self._effective_structure
course_block = structure[self.location]
return getattr(course_block, 'subtree_edited_on', None)
if structure:
course_block = structure[self.location]
return getattr(course_block, 'subtree_edited_on', None)
def __unicode__(self):
return u'Course: course_key: {}'.format(self.course_key)
......
lms/djangoapps/grades/course_grade.py
......@@ -7,6 +7,7 @@ from collections import OrderedDict, defaultdict
from django.conf import settings
from lazy import lazy
from ccx_keys.locator import CCXLocator
from xmodule import block_metadata_utils
from .subsection_grade import ZeroSubsectionGrade
......@@ -21,7 +22,7 @@ class CourseGradeBase(object):
"""
Base class for Course Grades.
"""
def __init__(self, user, course_data, percent=0, letter_grade=None, passed=False, force_update_subsections=False):
def __init__(self, user, course_data, percent=0.0, letter_grade=None, passed=False, force_update_subsections=False):
self.user = user
self.course_data = course_data
......@@ -137,8 +138,7 @@ class CourseGradeBase(object):
"""
Returns the result from the course grader.
"""
course = self.course_data.course
course.set_grading_policy(course.grading_policy)
course = self._prep_course_for_grading()
return course.grader.grade(
self.graded_subsections_by_format,
generate_random_scores=settings.GENERATE_PROFILE_SCORES,
......@@ -156,6 +156,27 @@ class CourseGradeBase(object):
grade_summary['grade'] = self.letter_grade
return grade_summary
def _prep_course_for_grading(self):
"""
Make sure any overrides to the grading policy are used.
This is most relevant for CCX courses.
Right now, we still access the grading policy from the course
object. Once we get the grading policy from the BlockStructure
this will no longer be needed - since BlockStructure correctly
retrieves/uses all field overrides.
"""
course = self.course_data.course
if isinstance(self.course_data.course_key, CCXLocator):
# clean out any field values that may have been set from the
# parent course of the CCX course.
course._field_data_cache = {} # pylint: disable=protected-access
# this is "magic" code that automatically retrieves any overrides
# to the grading policy and updates the course object.
course.set_grading_policy(course.grading_policy)
return course
def _get_chapter_grade_info(self, chapter, course_structure):
"""
Helper that returns a dictionary of chapter grade information.
......@@ -212,6 +233,7 @@ class CourseGrade(CourseGradeBase):
self.percent = self._compute_percent(self.grader_result)
self.letter_grade = self._compute_letter_grade(grade_cutoffs, self.percent)
self.passed = self._compute_passed(grade_cutoffs, self.percent)
return self
@lazy
def attempted(self):
......@@ -226,10 +248,10 @@ class CourseGrade(CourseGradeBase):
return False
def _get_subsection_grade(self, subsection):
# Pass read_only here so the subsection grades can be persisted in bulk at the end.
if self.force_update_subsections:
return self._subsection_grade_factory.update(subsection)
else:
# Pass read_only here so the subsection grades can be persisted in bulk at the end.
return self._subsection_grade_factory.create(subsection, read_only=True)
@staticmethod
......
lms/djangoapps/grades/course_grade_factory.py
......@@ -20,48 +20,33 @@ class CourseGradeFactory(object):
"""
GradeResult = namedtuple('GradeResult', ['student', 'course_grade', 'error'])
def create(self, user, course=None, collected_block_structure=None, course_structure=None, course_key=None):
def read(
self,
user,
course=None,
collected_block_structure=None,
course_structure=None,
course_key=None,
create_if_needed=True,
):
"""
Returns the CourseGrade for the given user in the course.
Reads the value from storage and validates that the grading
policy hasn't changed since the grade was last computed.
Reads the value from storage.
If not in storage, returns a ZeroGrade if ASSUME_ZERO_GRADE_IF_ABSENT.
Else, if changed or not in storage, computes and returns a new value.
At least one of course, collected_block_structure, course_structure,
or course_key should be provided.
"""
course_data = CourseData(user, course, collected_block_structure, course_structure, course_key)
try:
course_grade, read_policy_hash = self._read(user, course_data)
if read_policy_hash == course_data.grading_policy_hash:
return course_grade
read_only = False # update the persisted grade since the policy changed; TODO(TNL-6786) remove soon
except PersistentCourseGrade.DoesNotExist:
if assume_zero_if_absent(course_data.course_key):
return self._create_zero(user, course_data)
read_only = True # keep the grade un-persisted; TODO(TNL-6786) remove once all grades are backfilled
return self._update(user, course_data, read_only)
def read(self, user, course=None, collected_block_structure=None, course_structure=None, course_key=None):
"""
Returns the CourseGrade for the given user in the course as
persisted in storage. Does NOT verify whether the grading
policy is still valid since the grade was last computed.
If not in storage, returns a ZeroGrade if ASSUME_ZERO_GRADE_IF_ABSENT
else returns None.
Else if create_if_needed, computes and returns a new value.
Else, returns None.
At least one of course, collected_block_structure, course_structure,
or course_key should be provided.
"""
course_data = CourseData(user, course, collected_block_structure, course_structure, course_key)
try:
course_grade, _ = self._read(user, course_data)
return course_grade
return self._read(user, course_data)
except PersistentCourseGrade.DoesNotExist:
if assume_zero_if_absent(course_data.course_key):
return self._create_zero(user, course_data)
elif create_if_needed:
return self._update(user, course_data)
else:
return None
......@@ -82,15 +67,7 @@ class CourseGradeFactory(object):
or course_key should be provided.
"""
course_data = CourseData(user, course, collected_block_structure, course_structure, course_key)
return self._update(user, course_data, read_only=False, force_update_subsections=force_update_subsections)
@contextmanager
def _course_transaction(self, course_key):
"""
Provides a transaction context in which GradeResults are created.
"""
yield
VisibleBlocks.clear_cache(course_key)
return self._update(user, course_data, force_update_subsections=force_update_subsections)
def iter(
self,
......@@ -123,6 +100,14 @@ class CourseGradeFactory(object):
with dog_stats_api.timer('lms.grades.CourseGradeFactory.iter', tags=stats_tags):
yield self._iter_grade_result(user, course_data, force_update)
@contextmanager
def _course_transaction(self, course_key):
"""
Provides a transaction context in which GradeResults are created.
"""
yield
VisibleBlocks.clear_cache(course_key)
def _iter_grade_result(self, user, course_data, force_update):
try:
kwargs = {
......@@ -134,7 +119,7 @@ class CourseGradeFactory(object):
if force_update:
kwargs['force_update_subsections'] = True
method = CourseGradeFactory().update if force_update else CourseGradeFactory().create
method = CourseGradeFactory().update if force_update else CourseGradeFactory().read
course_grade = method(**kwargs)
return self.GradeResult(user, course_grade, None)
except Exception as exc: # pylint: disable=broad-except
......@@ -166,7 +151,9 @@ class CourseGradeFactory(object):
raise PersistentCourseGrade.DoesNotExist
persistent_grade = PersistentCourseGrade.read(user.id, course_data.course_key)
course_grade = CourseGrade(
log.debug(u'Grades: Read, %s, User: %s, %s', unicode(course_data), user.id, persistent_grade)
return CourseGrade(
user,
course_data,
persistent_grade.percent_grade,
......@@ -174,12 +161,8 @@ class CourseGradeFactory(object):
persistent_grade.passed_timestamp is not None,
)
log.debug(u'Grades: Read, %s, User: %s, %s', unicode(course_data), user.id, persistent_grade)
return course_grade, persistent_grade.grading_policy_hash
@staticmethod
def _update(user, course_data, read_only, force_update_subsections=False):
def _update(user, course_data, force_update_subsections=False):
"""
Computes, saves, and returns a CourseGrade object for the
given user and course.
......@@ -187,10 +170,9 @@ class CourseGradeFactory(object):
COURSE_GRADE_NOW_PASSED if learner has passed course.
"""
course_grade = CourseGrade(user, course_data, force_update_subsections=force_update_subsections)
course_grade.update()
course_grade = course_grade.update()
should_persist = (
(not read_only) and # TODO(TNL-6786) Remove the read_only boolean once all grades are back-filled.
should_persist_grades(course_data.course_key) and
course_grade.attempted
)
......
lms/djangoapps/grades/signals/handlers.py
......@@ -254,7 +254,8 @@ def force_recalculate_course_and_subsection_grades(sender, user, course_key, **k
Updates a saved course grade, forcing the subsection grades
from which it is calculated to update along the way.
"""
if CourseGradeFactory().read(user, course_key=course_key):
previous_course_grade = CourseGradeFactory().read(user, course_key=course_key)
if previous_course_grade and previous_course_grade.attempted:
CourseGradeFactory().update(user=user, course_key=course_key, force_update_subsections=True)
......
lms/djangoapps/grades/tests/test_grades.py
......@@ -79,7 +79,7 @@ class TestGradeIteration(SharedModuleStoreTestCase):
self.assertIsNone(course_grade.letter_grade)
self.assertEqual(course_grade.percent, 0.0)
@patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create')
@patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read')
def test_grading_exception(self, mock_course_grade):
"""Test that we correctly capture exception messages that bubble up from
grading. Note that we only see errors at this level if the grading
......@@ -287,7 +287,7 @@ class TestScoreForModule(SharedModuleStoreTestCase):
answer_problem(cls.course, cls.request, cls.l, score=1, max_value=3)
answer_problem(cls.course, cls.request, cls.n, score=3, max_value=10)
cls.course_grade = CourseGradeFactory().create(cls.request.user, cls.course)
cls.course_grade = CourseGradeFactory().read(cls.request.user, cls.course)
def test_score_chapter(self):
earned, possible = self.course_grade.score_for_module(self.a.location)
......
lms/djangoapps/grades/tests/test_tasks.py
......@@ -164,9 +164,9 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
self.assertEquals(mock_block_structure_create.call_count, 1)
@ddt.data(
(ModuleStoreEnum.Type.mongo, 1, 31, True),
(ModuleStoreEnum.Type.mongo, 1, 27, True),
(ModuleStoreEnum.Type.mongo, 1, 27, False),
(ModuleStoreEnum.Type.split, 3, 31, True),
(ModuleStoreEnum.Type.split, 3, 27, True),
(ModuleStoreEnum.Type.split, 3, 27, False),
)
@ddt.unpack
......@@ -179,8 +179,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
self._apply_recalculate_subsection_grade()
@ddt.data(
(ModuleStoreEnum.Type.mongo, 1, 31),
(ModuleStoreEnum.Type.split, 3, 31),
(ModuleStoreEnum.Type.mongo, 1, 27),
(ModuleStoreEnum.Type.split, 3, 27),
)
@ddt.unpack
def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls):
......
lms/djangoapps/grades/tests/utils.py
......@@ -12,17 +12,21 @@ from xmodule.graders import ProblemScore
@contextmanager
def mock_passing_grade(grade_pass='Pass', percent=0.75, ):
def mock_passing_grade(letter_grade='Pass', percent=0.75, ):
"""
Mock the grading function to always return a passing grade.
"""
with patch('lms.djangoapps.grades.course_grade.CourseGrade._compute_letter_grade') as mock_letter_grade:
with patch('lms.djangoapps.grades.course_grade.CourseGrade._compute_percent') as mock_percent_grade:
with patch('lms.djangoapps.grades.course_grade.CourseGrade.attempted') as mock_attempted:
mock_letter_grade.return_value = grade_pass
mock_percent_grade.return_value = percent
mock_attempted.return_value = True
yield
passing_grade_fields = dict(
letter_grade=letter_grade,
percent=percent,
passed=letter_grade is not None,
attempted=True,
)
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_grade_read:
mock_grade_read.return_value = MagicMock(**passing_grade_fields)
with patch('lms.djangoapps.grades.course_grade.CourseGrade.update') as mock_grade_update:
mock_grade_update.return_value = MagicMock(**passing_grade_fields)
yield
@contextmanager
......
lms/djangoapps/instructor/tests/test_spoc_gradebook.py
......@@ -7,6 +7,7 @@ from nose.plugins.attrib import attr
from capa.tests.response_xml_factory import StringResponseXMLFactory
from courseware.tests.factories import StudentModuleFactory
from lms.djangoapps.grades.tasks import compute_all_grades_for_course
from student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
......@@ -73,6 +74,7 @@ class TestGradebook(SharedModuleStoreTestCase):
course_id=self.course.id,
module_state_key=item.location
)
compute_all_grades_for_course.apply_async(kwargs={'course_key': unicode(self.course.id)})
self.response = self.client.get(reverse(
'spoc_gradebook',
......
lms/djangoapps/instructor/views/gradebook_api.py
......@@ -89,7 +89,7 @@ def get_grade_book_page(request, course, course_key):
'username': student.username,
'id': student.id,
'email': student.email,
'grade_summary': CourseGradeFactory().create(student, course).summary
'grade_summary': CourseGradeFactory().read(student, course).summary
}
for student in enrolled_students
]
......
lms/djangoapps/instructor_task/tests/test_integration.py
......@@ -131,7 +131,7 @@ class TestRescoringTask(TestIntegrationTask):
# are in sync.
expected_subsection_grade = expected_score
course_grade = CourseGradeFactory().create(user, self.course)
course_grade = CourseGradeFactory().read(user, self.course)
self.assertEquals(
course_grade.graded_subsections_by_format['Homework'][self.problem_section.location].graded_total.earned,
expected_subsection_grade,
......
lms/djangoapps/instructor_task/tests/test_tasks_helper.py
......@@ -395,7 +395,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase):
RequestCache.clear_request_cache()
expected_query_count = 41
expected_query_count = 36
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
with check_mongo_calls(mongo_count):
with self.assertNumQueries(expected_query_count):
......@@ -1976,7 +1976,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase):
'failed': 3,
'skipped': 2
}
with self.assertNumQueries(176):
with self.assertNumQueries(106):
self.assertCertificatesGenerated(task_input, expected_results)
expected_results = {
......
lms/djangoapps/lti_provider/tasks.py
......@@ -110,7 +110,7 @@ def send_composite_outcome(user_id, course_id, assignment_id, version):
mapped_usage_key = assignment.usage_key.map_into_course(course_key)
user = User.objects.get(id=user_id)
course = modulestore().get_course(course_key, depth=0)
course_grade = CourseGradeFactory().create(user, course)
course_grade = CourseGradeFactory().read(user, course)
earned, possible = course_grade.score_for_module(mapped_usage_key)
if possible == 0:
weighted_score = 0
......
lms/djangoapps/lti_provider/tests/test_tasks.py
......@@ -101,7 +101,7 @@ class SendCompositeOutcomeTest(BaseOutcomeTest):
)
self.course_grade = MagicMock()
self.course_grade_mock = self.setup_patch(
'lti_provider.tasks.CourseGradeFactory.create', self.course_grade
'lti_provider.tasks.CourseGradeFactory.read', self.course_grade
)
self.module_store = MagicMock()
self.module_store.get_item = MagicMock(return_value=self.descriptor)
......
lms/djangoapps/shoppingcart/tests/test_models.py
......@@ -247,12 +247,8 @@ class OrderTest(ModuleStoreTestCase):
self.assertEqual(cart.status, status)
self.assertEqual(item.status, status)
@override_settings(
LMS_SEGMENT_KEY="foobar",
FEATURES={
'STORE_BILLING_INFO': True,
}
)
@override_settings(LMS_SEGMENT_KEY="foobar")
@patch.dict(settings.FEATURES, {'STORE_BILLING_INFO': True})
def test_purchase(self):
# This test is for testing the subclassing functionality of OrderItem, but in
# order to do this, we end up testing the specific functionality of
......@@ -914,12 +910,8 @@ class CertificateItemTest(ModuleStoreTestCase):
cert_item = CertificateItem.add_to_order(cart, self.course_key, self.cost, 'honor')
self.assertEquals(cert_item.single_item_receipt_template, 'shoppingcart/receipt.html')
@override_settings(
LMS_SEGMENT_KEY="foobar",
FEATURES={
'STORE_BILLING_INFO': True,
}
)
@override_settings(LMS_SEGMENT_KEY="foobar")
@patch.dict(settings.FEATURES, {'STORE_BILLING_INFO': True})
@patch('student.models.CourseEnrollment.refund_cutoff_date')
def test_refund_cert_callback_no_expiration(self, cutoff_date):
# When there is no expiration date on a verified mode, the user can always get a refund
......@@ -956,12 +948,8 @@ class CertificateItemTest(ModuleStoreTestCase):
self.assertFalse(target_certs[0].refund_requested_time)
self.assertEquals(target_certs[0].order.status, 'purchased')
@override_settings(
LMS_SEGMENT_KEY="foobar",
FEATURES={
'STORE_BILLING_INFO': True,
}
)
@override_settings(LMS_SEGMENT_KEY="foobar")
@patch.dict(settings.FEATURES, {'STORE_BILLING_INFO': True})
@patch('student.models.CourseEnrollment.refund_cutoff_date')
def test_refund_cert_callback_before_expiration(self, cutoff_date):
# If the expiration date has not yet passed on a verified mode, the user can be refunded
......
lms/envs/bok_choy.py
......@@ -89,13 +89,17 @@ BLOCK_STRUCTURES_SETTINGS = dict(
TASK_DEFAULT_RETRY_DELAY=0,
)
###################### Grade Downloads ######################
###################### Grades ######################
GRADES_DOWNLOAD = {
'STORAGE_TYPE': 'localfs',
'BUCKET': 'edx-grades',
'ROOT_PATH': os.path.join(mkdtemp(), 'edx-s3', 'grades'),
}
FEATURES['PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS'] = True
FEATURES['ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS'] = True
# Configure the LMS to use our stub XQueue implementation
XQUEUE_INTERFACE['url'] = 'http://localhost:8040'
......
lms/envs/test.py
......@@ -307,6 +307,7 @@ FEATURES['ENABLE_VIDEO_ABSTRACTION_LAYER_API'] = True
########################### Grades #################################
FEATURES['PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS'] = True
FEATURES['ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS'] = True
###################### Payment ##############################3
# Enable fake payment processing page
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment