Skip to content

E
edx-platform
Commit fe081136 by Nimisha Asthagiri Committed by GitHub
Options

Merge pull request #13440 from edx/tnl/progress-page-queries 

Reduce sql queries with persisted grades
parents 6fd8452d 232d99d0
Showing with 237 additions and 228 deletions
lms/djangoapps/ccx/tests/test_field_override_performance.py
......@@ -36,7 +36,6 @@ from openedx.core.djangoapps.content.block_structure.api import get_course_in_ca
'django.conf.settings.FEATURES',
{
'ENABLE_XBLOCK_VIEW_ENDPOINT': True,
'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False # disable persistent grades until TNL-5458 (reduces queries)
}
)
@ddt.ddt
......@@ -230,18 +229,18 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase):
# # of sql queries to default,
# # of mongo queries,
# )
('no_overrides', 1, True, False): (17, 6),
('no_overrides', 2, True, False): (17, 6),
('no_overrides', 3, True, False): (17, 6),
('ccx', 1, True, False): (17, 6),
('ccx', 2, True, False): (17, 6),
('ccx', 3, True, False): (17, 6),
('no_overrides', 1, False, False): (17, 6),
('no_overrides', 2, False, False): (17, 6),
('no_overrides', 3, False, False): (17, 6),
('ccx', 1, False, False): (17, 6),
('ccx', 2, False, False): (17, 6),
('ccx', 3, False, False): (17, 6),
('no_overrides', 1, True, False): (20, 6),
('no_overrides', 2, True, False): (20, 6),
('no_overrides', 3, True, False): (20, 6),
('ccx', 1, True, False): (20, 6),
('ccx', 2, True, False): (20, 6),
('ccx', 3, True, False): (20, 6),
('no_overrides', 1, False, False): (20, 6),
('no_overrides', 2, False, False): (20, 6),
('no_overrides', 3, False, False): (20, 6),
('ccx', 1, False, False): (20, 6),
('ccx', 2, False, False): (20, 6),
('ccx', 3, False, False): (20, 6),
}
......@@ -253,19 +252,19 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
__test__ = True
TEST_DATA = {
('no_overrides', 1, True, False): (17, 3),
('no_overrides', 2, True, False): (17, 3),
('no_overrides', 3, True, False): (17, 3),
('ccx', 1, True, False): (17, 3),
('ccx', 2, True, False): (17, 3),
('ccx', 3, True, False): (17, 3),
('ccx', 1, True, True): (18, 3),
('ccx', 2, True, True): (18, 3),
('ccx', 3, True, True): (18, 3),
('no_overrides', 1, False, False): (17, 3),
('no_overrides', 2, False, False): (17, 3),
('no_overrides', 3, False, False): (17, 3),
('ccx', 1, False, False): (17, 3),
('ccx', 2, False, False): (17, 3),
('ccx', 3, False, False): (17, 3),
('no_overrides', 1, True, False): (20, 3),
('no_overrides', 2, True, False): (20, 3),
('no_overrides', 3, True, False): (20, 3),
('ccx', 1, True, False): (20, 3),
('ccx', 2, True, False): (20, 3),
('ccx', 3, True, False): (20, 3),
('ccx', 1, True, True): (21, 3),
('ccx', 2, True, True): (21, 3),
('ccx', 3, True, True): (21, 3),
('no_overrides', 1, False, False): (20, 3),
('no_overrides', 2, False, False): (20, 3),
('no_overrides', 3, False, False): (20, 3),
('ccx', 1, False, False): (20, 3),
('ccx', 2, False, False): (20, 3),
('ccx', 3, False, False): (20, 3),
}
lms/djangoapps/courseware/tests/test_views.py
......@@ -1136,25 +1136,38 @@ class ProgressPageTests(ModuleStoreTestCase):
self.section = ItemFactory.create(category='sequential', parent_location=self.chapter.location)
self.vertical = ItemFactory.create(category='vertical', parent_location=self.section.location)
@ddt.data('"><script>alert(1)</script>', '<script>alert(1)</script>', '</script><script>alert(1)</script>')
def test_progress_page_xss_prevent(self, malicious_code):
def _get_progress_page(self, expected_status_code=200):
"""
Test that XSS attack is prevented
Gets the progress page for the user in the course.
"""
resp = self.client.get(
reverse('progress', args=[unicode(self.course.id)])
)
self.assertEqual(resp.status_code, expected_status_code)
return resp
def _get_student_progress_page(self, expected_status_code=200):
"""
Gets the progress page for the user in the course.
"""
resp = self.client.get(
reverse('student_progress', args=[unicode(self.course.id), self.user.id])
)
self.assertEqual(resp.status_code, 200)
self.assertEqual(resp.status_code, expected_status_code)
return resp
@ddt.data('"><script>alert(1)</script>', '<script>alert(1)</script>', '</script><script>alert(1)</script>')
def test_progress_page_xss_prevent(self, malicious_code):
"""
Test that XSS attack is prevented
"""
resp = self._get_student_progress_page()
# Test that malicious code does not appear in html
self.assertNotIn(malicious_code, resp.content)
def test_pure_ungraded_xblock(self):
ItemFactory.create(category='acid', parent_location=self.vertical.location)
resp = self.client.get(
reverse('progress', args=[unicode(self.course.id)])
)
self.assertEqual(resp.status_code, 200)
self._get_progress_page()
@ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
def test_student_progress_with_valid_and_invalid_id(self, default_store):
......@@ -1180,11 +1193,8 @@ class ProgressPageTests(ModuleStoreTestCase):
)
self.assertEquals(resp.status_code, 404)
resp = self.client.get(
reverse('student_progress', args=[unicode(self.course.id), self.user.id])
)
# Assert that valid 'student_id' returns 200 status
self.assertEqual(resp.status_code, 200)
self._get_student_progress_page()
@ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
def test_unenrolled_student_progress_for_credit_course(self, default_store):
......@@ -1219,38 +1229,26 @@ class ProgressPageTests(ModuleStoreTestCase):
# Add a single credit requirement (final grade)
set_credit_requirements(course.id, requirements)
resp = self.client.get(
reverse('student_progress', args=[unicode(course.id), not_enrolled_user.id])
)
self.assertEqual(resp.status_code, 200)
self._get_student_progress_page()
def test_non_ascii_grade_cutoffs(self):
resp = self.client.get(
reverse('progress', args=[unicode(self.course.id)])
)
self.assertEqual(resp.status_code, 200)
self._get_progress_page()
def test_generate_cert_config(self):
resp = self.client.get(
reverse('progress', args=[unicode(self.course.id)])
)
resp = self._get_progress_page()
self.assertNotContains(resp, 'Request Certificate')
# Enable the feature, but do not enable it for this course
CertificateGenerationConfiguration(enabled=True).save()
resp = self.client.get(
reverse('progress', args=[unicode(self.course.id)])
)
resp = self._get_progress_page()
self.assertNotContains(resp, 'Request Certificate')
# Enable certificate generation for this course
certs_api.set_cert_generation_enabled(self.course.id, True)
resp = self.client.get(
reverse('progress', args=[unicode(self.course.id)])
)
resp = self._get_progress_page()
self.assertNotContains(resp, 'Request Certificate')
@patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': True})
......@@ -1295,9 +1293,7 @@ class ProgressPageTests(ModuleStoreTestCase):
self.course.save()
self.store.update_item(self.course, self.user.id)
resp = self.client.get(
reverse('progress', args=[unicode(self.course.id)])
)
resp = self._get_progress_page()
self.assertContains(resp, u"View Certificate")
self.assertContains(resp, u"You can keep working for a higher grade")
......@@ -1308,9 +1304,7 @@ class ProgressPageTests(ModuleStoreTestCase):
certificates[0]['is_active'] = False
self.store.update_item(self.course, self.user.id)
resp = self.client.get(
reverse('progress', args=[unicode(self.course.id)])
)
resp = self._get_progress_page()
self.assertNotContains(resp, u"View Your Certificate")
self.assertNotContains(resp, u"You can now view your certificate")
self.assertContains(resp, "working on it...")
......@@ -1340,26 +1334,29 @@ class ProgressPageTests(ModuleStoreTestCase):
# Enable certificate generation for this course
certs_api.set_cert_generation_enabled(self.course.id, True)
resp = self.client.get(
reverse('progress', args=[unicode(self.course.id)])
)
resp = self._get_progress_page()
self.assertContains(resp, u"Download Your Certificate")
# disable persistent grades until TNL-5458 (reduces query counts)
@patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False})
@ddt.data(
*itertools.product(((34, 4, True), (34, 4, False)), (True, False))
*itertools.product((True, False), (True, False))
)
@ddt.unpack
def test_query_counts(self, (sql_calls, mongo_calls, self_paced), self_paced_enabled):
def test_progress_queries_paced_courses(self, self_paced, self_paced_enabled):
"""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(sql_calls), check_mongo_calls(mongo_calls):
resp = self.client.get(
reverse('progress', args=[unicode(self.course.id)])
)
self.assertEqual(resp.status_code, 200)
with self.assertNumQueries(37), check_mongo_calls(4):
self._get_progress_page()
def test_progress_queries(self):
self.setup_course()
with self.assertNumQueries(37), check_mongo_calls(4):
self._get_progress_page()
# subsequent accesses to the progress page require fewer queries.
for _ in range(2):
with self.assertNumQueries(20), check_mongo_calls(4):
self._get_progress_page()
@patch(
'lms.djangoapps.grades.new.course_grade.CourseGrade.summary',
......@@ -1397,7 +1394,8 @@ class ProgressPageTests(ModuleStoreTestCase):
self.assertEqual(
cert_button_hidden,
'Request Certificate' not in resp.content)
'Request Certificate' not in resp.content
)
@patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': True})
@patch(
......@@ -1430,9 +1428,7 @@ class ProgressPageTests(ModuleStoreTestCase):
self.course.save()
self.store.update_item(self.course, self.user.id)
resp = self.client.get(
reverse('progress', args=[unicode(self.course.id)])
)
resp = self._get_progress_page()
self.assertContains(resp, u"View Certificate")
self.assert_invalidate_certificate(generated_certificate)
......@@ -1449,9 +1445,7 @@ class ProgressPageTests(ModuleStoreTestCase):
"http://www.example.com/certificate.pdf", "honor"
)
resp = self.client.get(
reverse('progress', args=[unicode(self.course.id)])
)
resp = self._get_progress_page()
self.assertContains(resp, u'Download Your Certificate')
self.assert_invalidate_certificate(generated_certificate)
......@@ -1466,9 +1460,7 @@ class ProgressPageTests(ModuleStoreTestCase):
user = UserFactory.create()
self.assertTrue(self.client.login(username=user.username, password='test'))
CourseEnrollmentFactory(user=user, course_id=self.course.id, mode=CourseMode.AUDIT)
response = self.client.get(
reverse('progress', args=[unicode(self.course.id)])
)
response = self._get_progress_page()
self.assertContains(
response,
......@@ -1557,9 +1549,7 @@ class ProgressPageTests(ModuleStoreTestCase):
)
# Invalidate user certificate
certificate.invalidate()
resp = self.client.get(
reverse('progress', args=[unicode(self.course.id)])
)
resp = self._get_progress_page()
self.assertNotContains(resp, u'Request Certificate')
self.assertContains(resp, u'Your certificate has been invalidated')
......
lms/djangoapps/courseware/views/views.py
......@@ -719,7 +719,7 @@ def _progress(request, course_key, student_id):
# additional DB lookup (this kills the Progress page in particular).
student = User.objects.prefetch_related("groups").get(id=student.id)
course_grade = CourseGradeFactory(student).create(course)
course_grade = CourseGradeFactory(student).create(course, read_only=False)
if not course_grade.has_access_to_course:
# This means the student didn't have access to the course (which the instructor requested)
raise Http404
......
lms/djangoapps/grades/migrations/0004_visibleblocks_course_id.py 0 → 100644
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
from django.db import migrations, models
from opaque_keys.edx.keys import CourseKey
import xmodule_django.models
class Migration(migrations.Migration):
dependencies = [
('grades', '0003_coursepersistentgradesflag_persistentgradesenabledflag'),
]
operations = [
migrations.AddField(
model_name='visibleblocks',
name='course_id',
field=xmodule_django.models.CourseKeyField(default=CourseKey.from_string('edX/BerylMonkeys/TNL-5458'), max_length=255, db_index=True),
preserve_default=False,
),
]
lms/djangoapps/grades/new/course_grade.py
......@@ -40,10 +40,7 @@ class CourseGrade(object):
graded_total = subsection_grade.graded_total
if graded_total.possible > 0:
subsections_by_format[subsection_grade.format].append(graded_total)
log.info(u"Persistent Grades: Calculated subsections_by_format. course id: {0}, user: {1}".format(
self.course.location,
self.student.id
))
self._log_event(log.info, u"subsections_by_format")
return subsections_by_format
@lazy
......@@ -55,10 +52,7 @@ class CourseGrade(object):
for chapter in self.chapter_grades:
for subsection_grade in chapter['sections']:
locations_to_weighted_scores.update(subsection_grade.locations_to_weighted_scores)
log.info(u"Persistent Grades: Calculated locations_to_weighted_scores. course id: {0}, user: {1}".format(
self.course.id,
self.student.id
))
self._log_event(log.info, u"locations_to_weighted_scores")
return locations_to_weighted_scores
@lazy
......@@ -72,10 +66,7 @@ class CourseGrade(object):
self.subsection_grade_totals_by_format,
generate_random_scores=settings.GENERATE_PROFILE_SCORES
)
log.info(u"Persistent Grades: Calculated grade_value. course id: {0}, user: {1}".format(
self.course.location,
self.student.id
))
self._log_event(log.info, u"grade_value")
return grade_value
@property
......@@ -123,31 +114,34 @@ class CourseGrade(object):
grade_summary['totaled_scores'] = self.subsection_grade_totals_by_format
grade_summary['raw_scores'] = list(self.locations_to_weighted_scores.itervalues())
self._log_event(log.warning, u"grade_summary, percent: {0}, grade: {1}".format(self.percent, self.letter_grade))
return grade_summary
def compute(self):
def compute_and_update(self, read_only=False):
"""
Computes the grade for the given student and course.
If read_only is True, doesn't save any updates to the grades.
"""
subsection_grade_factory = SubsectionGradeFactory(self.student)
self._log_event(log.warning, u"compute_and_update, read_only: {}".format(read_only))
subsection_grade_factory = SubsectionGradeFactory(self.student, self.course, self.course_structure)
for chapter_key in self.course_structure.get_children(self.course.location):
chapter = self.course_structure[chapter_key]
subsection_grades = []
chapter_subsection_grades = []
for subsection_key in self.course_structure.get_children(chapter_key):
subsection_grades.append(
subsection_grade_factory.create(
self.course_structure[subsection_key],
self.course_structure,
self.course,
)
chapter_subsection_grades.append(
subsection_grade_factory.create(self.course_structure[subsection_key], read_only=True)
)
self.chapter_grades.append({
'display_name': block_metadata_utils.display_name_with_default_escaped(chapter),
'url_name': block_metadata_utils.url_name_for_block(chapter),
'sections': subsection_grades
'sections': chapter_subsection_grades
})
if not read_only:
subsection_grade_factory.bulk_create_unsaved()
self._signal_listeners_when_grade_computed()
def score_for_module(self, location):
......@@ -212,6 +206,16 @@ class CourseGrade(object):
receiver, response
)
def _log_event(self, log_func, log_statement):
"""
Logs the given statement, for this instance.
"""
log_func(u"Persistent Grades: CourseGrade.{0}, course: {1}, user: {2}".format(
log_statement,
self.course.id,
self.student.id
))
class CourseGradeFactory(object):
"""
......@@ -220,22 +224,26 @@ class CourseGradeFactory(object):
def __init__(self, student):
self.student = student
def create(self, course):
def create(self, course, read_only=False):
"""
Returns the CourseGrade object for the given student and course.
If read_only is True, doesn't save any updates to the grades.
"""
course_structure = get_course_blocks(self.student, course.location)
return (
self._get_saved_grade(course, course_structure) or
self._compute_and_update_grade(course, course_structure)
self._compute_and_update_grade(course, course_structure, read_only)
)
def _compute_and_update_grade(self, course, course_structure):
def _compute_and_update_grade(self, course, course_structure, read_only):
"""
Freshly computes and updates the grade for the student and course.
If read_only is True, doesn't save any updates to the grades.
"""
course_grade = CourseGrade(self.student, course, course_structure)
course_grade.compute()
course_grade.compute_and_update(read_only)
return course_grade
def _get_saved_grade(self, course, course_structure): # pylint: disable=unused-argument
......
lms/djangoapps/grades/scores.py
"""
Functionality for problem scores.
"""
from logging import getLogger
from openedx.core.lib.cache_utils import memoized
from xblock.core import XBlock
from .transformer import GradesTransformer
log = getLogger(__name__)
@memoized
def block_types_possibly_scored():
"""
......@@ -30,15 +35,17 @@ def possibly_scored(usage_key):
return usage_key.block_type in block_types_possibly_scored()
def weighted_score(raw_earned, raw_possible, weight):
"""Return a tuple that represents the weighted (correct, total) score."""
# If there is no weighting, or weighting can't be applied, return input.
def weighted_score(raw_earned, raw_possible, weight=None):
"""
Return a tuple that represents the weighted (earned, possible) score.
If weight is None or raw_possible is 0, returns the original values.
"""
if weight is None or raw_possible == 0:
return (raw_earned, raw_possible)
return float(raw_earned) * weight / raw_possible, float(weight)
def get_score(user, block, scores_client, submissions_scores_cache):
def get_score(user, block, scores_client, submissions_scores_cache, weight, possible=None):
"""
Return the score for a user on a problem, as a tuple (earned, possible).
e.g. (5,7) if you got 5 out of 7 points.
......@@ -49,8 +56,13 @@ def get_score(user, block, scores_client, submissions_scores_cache):
user: a Student object
block: a BlockStructure's BlockData object
scores_client: an initialized ScoresClient
submissions_scores_cache: A dict of location names to (earned, possible) point tuples.
If an entry is found in this cache, it takes precedence.
submissions_scores_cache: A dict of location names to (earned, possible)
point tuples. If an entry is found in this cache, it takes precedence.
weight: The weight of the problem to use in the calculation. A value of
None signifies that the weight should not be applied.
possible (optional): The possible maximum score of the problem to use in the
calculation. If None, uses the value found either in scores_client or
from the block.
"""
submissions_scores_cache = submissions_scores_cache or {}
......@@ -74,16 +86,20 @@ def get_score(user, block, scores_client, submissions_scores_cache):
if score and score.total is not None:
# We have a valid score, just use it.
earned = score.correct if score.correct is not None else 0.0
possible = score.total
if possible is None:
possible = score.total
elif possible != score.total:
log.error(u"Persisted Grades: possible value {} != score.total value {}".format(possible, score.total))
else:
# This means we don't have a valid score entry and we don't have a
# cached_max_score on hand. We know they've earned 0.0 points on this.
earned = 0.0
possible = block.transformer_data[GradesTransformer].max_score
if possible is None:
possible = block.transformer_data[GradesTransformer].max_score
# Problem may be an error module (if something in the problem builder failed)
# In which case possible might be None
if possible is None:
return (None, None)
return weighted_score(earned, possible, block.weight)
return weighted_score(earned, possible, weight)
lms/djangoapps/grades/signals/handlers.py
......@@ -108,10 +108,13 @@ def recalculate_subsection_grade_handler(sender, **kwargs): # pylint: disable=u
'subsections',
set()
)
subsection_grade_factory = SubsectionGradeFactory(student, course, collected_block_structure)
for subsection_usage_key in subsections_to_update:
transformed_subsection_structure = get_course_blocks(
student,
subsection_usage_key,
collected_block_structure=collected_block_structure,
)
SubsectionGradeFactory(student).update(subsection_usage_key, transformed_subsection_structure, course)
subsection_grade_factory.update(
transformed_subsection_structure[subsection_usage_key], transformed_subsection_structure
)
lms/djangoapps/grades/tests/test_grades.py
......@@ -395,7 +395,7 @@ class TestGetModuleScore(LoginEnrollmentTestCase, SharedModuleStoreTestCase):
# then stored in the request).
with self.assertNumQueries(1):
score = get_module_score(self.request.user, self.course, self.seq1)
new_score = SubsectionGradeFactory(self.request.user).create(self.seq1, self.course_structure, self.course)
new_score = SubsectionGradeFactory(self.request.user, self.course, self.course_structure).create(self.seq1)
self.assertEqual(score, 0)
self.assertEqual(new_score.all_total.earned, 0)
......@@ -404,7 +404,7 @@ class TestGetModuleScore(LoginEnrollmentTestCase, SharedModuleStoreTestCase):
with self.assertNumQueries(1):
score = get_module_score(self.request.user, self.course, self.seq1)
new_score = SubsectionGradeFactory(self.request.user).create(self.seq1, self.course_structure, self.course)
new_score = SubsectionGradeFactory(self.request.user, self.course, self.course_structure).create(self.seq1)
self.assertEqual(score, 1.0)
self.assertEqual(new_score.all_total.earned, 2.0)
# These differ because get_module_score normalizes the subsection score
......@@ -416,7 +416,7 @@ class TestGetModuleScore(LoginEnrollmentTestCase, SharedModuleStoreTestCase):
with self.assertNumQueries(1):
score = get_module_score(self.request.user, self.course, self.seq1)
new_score = SubsectionGradeFactory(self.request.user).create(self.seq1, self.course_structure, self.course)
new_score = SubsectionGradeFactory(self.request.user, self.course, self.course_structure).create(self.seq1)
self.assertEqual(score, .5)
self.assertEqual(new_score.all_total.earned, 1.0)
......
lms/djangoapps/grades/tests/test_models.py
......@@ -6,7 +6,6 @@ from collections import OrderedDict
import ddt
from hashlib import sha1
import json
from mock import patch
from django.db.utils import IntegrityError
from django.test import TestCase
......@@ -24,17 +23,25 @@ class BlockRecordListTestCase(TestCase):
"""
Verify the behavior of BlockRecordList, particularly around edge cases
"""
empty_json = '{"blocks":[],"course_key":null}'
def setUp(self):
super(BlockRecordListTestCase, self).setUp()
self.course_key = CourseLocator(
org='some_org',
course='some_course',
run='some_run'
)
def test_empty_block_record_set(self):
brs = BlockRecordList(())
empty_json = '{0}"blocks":[],"course_key":"{1}"{2}'.format('{', unicode(self.course_key), '}')
brs = BlockRecordList((), self.course_key)
self.assertFalse(brs)
self.assertEqual(
brs.to_json(),
self.empty_json
brs.json_value,
empty_json
)
self.assertEqual(
BlockRecordList.from_json(self.empty_json),
BlockRecordList.from_json(empty_json),
brs
)
......@@ -112,7 +119,7 @@ class VisibleBlocksTest(GradesModelTestCase):
"""
Creates and returns a BlockRecordList for the given blocks.
"""
return VisibleBlocks.objects.create_from_blockrecords(BlockRecordList.from_list(blocks))
return VisibleBlocks.objects.create_from_blockrecords(BlockRecordList.from_list(blocks, self.course_key))
def test_creation(self):
"""
......@@ -159,7 +166,7 @@ class VisibleBlocksTest(GradesModelTestCase):
and accessing visible_blocks.blocks yields a copy of the initial array.
Also, trying to set the blocks property should raise an exception.
"""
expected_blocks = (self.record_a, self.record_b)
expected_blocks = BlockRecordList.from_list([self.record_a, self.record_b], self.course_key)
visible_blocks = self._create_block_record_list(expected_blocks)
self.assertEqual(expected_blocks, visible_blocks.blocks)
with self.assertRaises(AttributeError):
......@@ -178,6 +185,7 @@ class PersistentSubsectionGradeTest(GradesModelTestCase):
block_type='subsection',
block_id='subsection_12345',
)
self.block_records = BlockRecordList([self.record_a, self.record_b], self.course_key)
self.params = {
"user_id": 12345,
"usage_key": self.usage_key,
......@@ -187,21 +195,23 @@ class PersistentSubsectionGradeTest(GradesModelTestCase):
"possible_all": 12.0,
"earned_graded": 6.0,
"possible_graded": 8.0,
"visible_blocks": [self.record_a, self.record_b],
"visible_blocks": self.block_records,
}
def test_create(self):
"""
Tests model creation, and confirms error when trying to recreate model.
"""
created_grade = PersistentSubsectionGrade.objects.create(**self.params)
read_grade = PersistentSubsectionGrade.read_grade(
user_id=self.params["user_id"],
usage_key=self.params["usage_key"],
)
self.assertEqual(created_grade, read_grade)
created_grade = PersistentSubsectionGrade.create_grade(**self.params)
with self.assertNumQueries(1):
read_grade = PersistentSubsectionGrade.read_grade(
user_id=self.params["user_id"],
usage_key=self.params["usage_key"],
)
self.assertEqual(created_grade, read_grade)
self.assertEquals(read_grade.visible_blocks.blocks, self.block_records)
with self.assertRaises(IntegrityError):
created_grade = PersistentSubsectionGrade.objects.create(**self.params)
PersistentSubsectionGrade.create_grade(**self.params)
def test_create_bad_params(self):
"""
......@@ -209,56 +219,19 @@ class PersistentSubsectionGradeTest(GradesModelTestCase):
"""
del self.params["earned_graded"]
with self.assertRaises(IntegrityError):
PersistentSubsectionGrade.objects.create(**self.params)
PersistentSubsectionGrade.create_grade(**self.params)
def test_course_version_is_optional(self):
del self.params["course_version"]
PersistentSubsectionGrade.objects.create(**self.params)
def test_update_grade(self):
"""
Tests model update, and confirms error when updating a nonexistent model.
"""
with self.assertRaises(PersistentSubsectionGrade.DoesNotExist):
PersistentSubsectionGrade.update_grade(**self.params)
PersistentSubsectionGrade.objects.create(**self.params)
self.params['earned_all'] = 12.0
self.params['earned_graded'] = 8.0
with patch('lms.djangoapps.grades.models.log') as log_mock:
PersistentSubsectionGrade.update_grade(**self.params)
read_grade = PersistentSubsectionGrade.read_grade(
user_id=self.params["user_id"],
usage_key=self.params["usage_key"],
)
log_mock.info.assert_called_with(
u"Persistent Grades: Grade model updated: {0}".format(read_grade)
)
self.assertEqual(read_grade.earned_all, 12.0)
self.assertEqual(read_grade.earned_graded, 8.0)
PersistentSubsectionGrade.create_grade(**self.params)
@ddt.data(True, False)
def test_save(self, already_created):
if already_created:
PersistentSubsectionGrade.objects.create(**self.params)
module_prefix = "lms.djangoapps.grades.models.PersistentSubsectionGrade."
with patch(
module_prefix + "objects.get_or_create",
wraps=PersistentSubsectionGrade.objects.get_or_create
) as mock_get_or_create:
with patch(module_prefix + "update") as mock_update:
PersistentSubsectionGrade.save_grade(**self.params)
self.assertTrue(mock_get_or_create.called)
self.assertEqual(mock_update.called, already_created)
def test_update_or_create_grade(self, already_created):
created_grade = PersistentSubsectionGrade.create_grade(**self.params) if already_created else None
def test_logging_for_save(self):
with patch('lms.djangoapps.grades.models.log') as log_mock:
PersistentSubsectionGrade.save_grade(**self.params)
read_grade = PersistentSubsectionGrade.read_grade(
user_id=self.params["user_id"],
usage_key=self.params["usage_key"],
)
log_mock.info.assert_called_with(
u"Persistent Grades: Grade model saved: {0}".format(read_grade)
)
self.params["earned_all"] = 7
updated_grade = PersistentSubsectionGrade.update_or_create_grade(**self.params)
self.assertEquals(updated_grade.earned_all, 7)
if already_created:
self.assertEquals(created_grade.id, updated_grade.id)
self.assertEquals(created_grade.earned_all, 6)
lms/djangoapps/grades/tests/test_new.py
......@@ -61,8 +61,8 @@ class GradeTestBase(SharedModuleStoreTestCase):
super(GradeTestBase, self).setUp()
self.request = get_request_for_user(UserFactory())
self.client.login(username=self.request.user.username, password="test")
self.subsection_grade_factory = SubsectionGradeFactory(self.request.user)
self.course_structure = get_course_blocks(self.request.user, self.course.location)
self.subsection_grade_factory = SubsectionGradeFactory(self.request.user, self.course, self.course_structure)
CourseEnrollment.enroll(self.request.user, self.course.id)
......@@ -110,33 +110,25 @@ class SubsectionGradeFactoryTest(GradeTestBase):
Tests to ensure that a persistent subsection grade is created, saved, then fetched on re-request.
"""
with patch(
'lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory._save_grade',
wraps=self.subsection_grade_factory._save_grade # pylint: disable=protected-access
) as mock_save_grades:
'lms.djangoapps.grades.new.subsection_grade.PersistentSubsectionGrade.create_grade',
wraps=PersistentSubsectionGrade.create_grade
) as mock_create_grade:
with patch(
'lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory._get_saved_grade',
wraps=self.subsection_grade_factory._get_saved_grade # pylint: disable=protected-access
) as mock_get_saved_grade:
with self.assertNumQueries(19):
grade_a = self.subsection_grade_factory.create(
self.sequence,
self.course_structure,
self.course
)
with self.assertNumQueries(12):
grade_a = self.subsection_grade_factory.create(self.sequence)
self.assertTrue(mock_get_saved_grade.called)
self.assertTrue(mock_save_grades.called)
self.assertTrue(mock_create_grade.called)
mock_get_saved_grade.reset_mock()
mock_save_grades.reset_mock()
with self.assertNumQueries(3):
grade_b = self.subsection_grade_factory.create(
self.sequence,
self.course_structure,
self.course
)
mock_create_grade.reset_mock()
with self.assertNumQueries(0):
grade_b = self.subsection_grade_factory.create(self.sequence)
self.assertTrue(mock_get_saved_grade.called)
self.assertFalse(mock_save_grades.called)
self.assertFalse(mock_create_grade.called)
self.assertEqual(grade_a.url_name, grade_b.url_name)
self.assertEqual(grade_a.all_total, grade_b.all_total)
......@@ -153,7 +145,7 @@ class SubsectionGradeFactoryTest(GradeTestBase):
# Grades are only saved if the feature flag and the advanced setting are
# both set to True.
with patch(
'lms.djangoapps.grades.models.PersistentSubsectionGrade.read_grade'
'lms.djangoapps.grades.models.PersistentSubsectionGrade.bulk_read_grades'
) as mock_read_saved_grade:
with persistent_grades_feature_flags(
global_flag=feature_flag,
......@@ -161,7 +153,7 @@ class SubsectionGradeFactoryTest(GradeTestBase):
course_id=self.course.id,
enabled_for_course=course_setting
):
self.subsection_grade_factory.create(self.sequence, self.course_structure, self.course)
self.subsection_grade_factory.create(self.sequence)
self.assertEqual(mock_read_saved_grade.called, feature_flag and course_setting)
......@@ -174,10 +166,8 @@ class SubsectionGradeTest(GradeTestBase):
"""
Assuming the underlying score reporting methods work, test that the score is calculated properly.
"""
grade = self.subsection_grade_factory.create(self.sequence, self.course_structure, self.course)
with mock_get_score(1, 2):
# The final 2 parameters are only passed through to our mocked-out get_score method
grade.compute(self.request.user, self.course_structure, None, None)
grade = self.subsection_grade_factory.create(self.sequence)
self.assertEqual(grade.all_total.earned, 1)
self.assertEqual(grade.all_total.possible, 2)
......@@ -186,9 +176,8 @@ class SubsectionGradeTest(GradeTestBase):
Test that grades are persisted to the database properly, and that loading saved grades returns the same data.
"""
# Create a grade that *isn't* saved to the database
self.subsection_grade_factory._prefetch_scores(self.course_structure, self.course) # pylint: disable=protected-access
input_grade = SubsectionGrade(self.sequence)
input_grade.compute(
input_grade = SubsectionGrade(self.sequence, self.course)
input_grade.init_from_structure(
self.request.user,
self.course_structure,
self.subsection_grade_factory._scores_client, # pylint: disable=protected-access
......@@ -197,16 +186,17 @@ class SubsectionGradeTest(GradeTestBase):
self.assertEqual(PersistentSubsectionGrade.objects.count(), 0)
# save to db, and verify object is in database
input_grade.save(self.request.user, self.sequence, self.course)
input_grade.create_model(self.request.user)
self.assertEqual(PersistentSubsectionGrade.objects.count(), 1)
# load from db, and ensure output matches input
loaded_grade = SubsectionGrade(self.sequence)
loaded_grade = SubsectionGrade(self.sequence, self.course)
saved_model = PersistentSubsectionGrade.read_grade(
user_id=self.request.user.id,
usage_key=self.sequence.location,
)
loaded_grade.load_from_data(
loaded_grade.init_from_model(
self.request.user,
saved_model,
self.course_structure,
self.subsection_grade_factory._scores_client, # pylint: disable=protected-access
......
lms/djangoapps/grades/tests/test_signals.py
......@@ -216,7 +216,7 @@ class ScoreChangedUpdatesSubsectionGradeTest(ModuleStoreTestCase):
with self.store.default_store(default_store):
self.set_up_course()
self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id))
with check_mongo_calls(2) and self.assertNumQueries(15):
with check_mongo_calls(2) and self.assertNumQueries(11):
recalculate_subsection_grade_handler(None, **self.score_changed_kwargs)
def test_single_call_to_create_block_structure(self):
......@@ -236,7 +236,7 @@ class ScoreChangedUpdatesSubsectionGradeTest(ModuleStoreTestCase):
self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id))
ItemFactory.create(parent=self.sequential, category='problem', display_name='problem2')
ItemFactory.create(parent=self.sequential, category='problem', display_name='problem3')
with check_mongo_calls(2) and self.assertNumQueries(15):
with check_mongo_calls(2) and self.assertNumQueries(11):
recalculate_subsection_grade_handler(None, **self.score_changed_kwargs)
@ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
......
lms/djangoapps/instructor_task/tests/test_tasks_helper.py
......@@ -1641,8 +1641,6 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase):
super(TestCertificateGeneration, self).setUp()
self.initialize_course()
# disable persistent grades until TNL-5458 (reduces query counts)
@patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False})
def test_certificate_generation_for_students(self):
"""
Verify that certificates generated for all eligible students enrolled in a course.
......@@ -1672,8 +1670,18 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase):
'failed': 3,
'skipped': 2
}
with self.assertNumQueries(175):
self.assertCertificatesGenerated(task_input, expected_results)
with self.assertNumQueries(151):
expected_results = {
'action_name': 'certificates generated',
'total': 10,
'attempted': 0,
'succeeded': 0,
'failed': 0,
'skipped': 10
}
with self.assertNumQueries(3):
self.assertCertificatesGenerated(task_input, expected_results)
@ddt.data(
......
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