diff --git a/lms/djangoapps/grades/config/waffle.py b/lms/djangoapps/grades/config/waffle.py index 80468be50c..c53c63eb73 100644 --- a/lms/djangoapps/grades/config/waffle.py +++ b/lms/djangoapps/grades/config/waffle.py @@ -9,7 +9,6 @@ WAFFLE_NAMESPACE = u'grades' # Switches ASSUME_ZERO_GRADE_IF_ABSENT = u'assume_zero_grade_if_absent' -ESTIMATE_FIRST_ATTEMPTED = u'estimate_first_attempted' DISABLE_REGRADE_ON_POLICY_CHANGE = u'disable_regrade_on_policy_change' # Course Flags diff --git a/lms/djangoapps/grades/course_grade.py b/lms/djangoapps/grades/course_grade.py index 5e10ce0336..333118ce63 100644 --- a/lms/djangoapps/grades/course_grade.py +++ b/lms/djangoapps/grades/course_grade.py @@ -14,10 +14,6 @@ from .subsection_grade import ZeroSubsectionGrade from .subsection_grade_factory import SubsectionGradeFactory -def uniqueify(iterable): - return OrderedDict([(item, None) for item in iterable]).keys() - - class CourseGradeBase(object): """ Base class for Course Grades. @@ -194,7 +190,7 @@ class CourseGradeBase(object): """ return [ self._get_subsection_grade(course_structure[subsection_key]) - for subsection_key in uniqueify(course_structure.get_children(chapter_key)) + for subsection_key in _uniqueify_and_keep_order(course_structure.get_children(chapter_key)) ] @abstractmethod @@ -229,6 +225,11 @@ class CourseGrade(CourseGradeBase): if self.force_update_subsections is true, via the lazy call to self.grader_result. """ + # TODO update this code to be more functional and readable. + # Currently, it is hard to follow since there are plenty of + # side-effects. Once functional, force_update_subsections + # can be passed through and not confusingly stored and used + # at a later time. grade_cutoffs = self.course_data.course.grade_cutoffs self.percent = self._compute_percent(self.grader_result) self.letter_grade = self._compute_letter_grade(grade_cutoffs, self.percent) @@ -289,3 +290,7 @@ class CourseGrade(CourseGradeBase): nonzero_cutoffs = [cutoff for cutoff in grade_cutoffs.values() if cutoff > 0] success_cutoff = min(nonzero_cutoffs) if nonzero_cutoffs else None return success_cutoff and percent >= success_cutoff + + +def _uniqueify_and_keep_order(iterable): + return OrderedDict([(item, None) for item in iterable]).keys() diff --git a/lms/djangoapps/grades/course_grade_factory.py b/lms/djangoapps/grades/course_grade_factory.py index c239bb5c1c..44f1418aee 100644 --- a/lms/djangoapps/grades/course_grade_factory.py +++ b/lms/djangoapps/grades/course_grade_factory.py @@ -1,5 +1,4 @@ from collections import namedtuple -from contextlib import contextmanager from logging import getLogger import dogstats_wrapper as dog_stats_api @@ -9,7 +8,7 @@ from openedx.core.djangoapps.signals.signals import COURSE_GRADE_CHANGED, COURSE from .config import assume_zero_if_absent, should_persist_grades from .course_data import CourseData from .course_grade import CourseGrade, ZeroCourseGrade -from .models import PersistentCourseGrade, VisibleBlocks +from .models import PersistentCourseGrade, prefetch log = getLogger(__name__) @@ -95,18 +94,9 @@ class CourseGradeFactory(object): user=None, course=course, collected_block_structure=collected_block_structure, course_key=course_key, ) stats_tags = [u'action:{}'.format(course_data.course_key)] - with self._course_transaction(course_data.course_key): - for user in users: - 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) + for user in users: + with dog_stats_api.timer('lms.grades.CourseGradeFactory.iter', tags=stats_tags): + yield self._iter_grade_result(user, course_data, force_update) def _iter_grade_result(self, user, course_data, force_update): try: @@ -169,13 +159,15 @@ class CourseGradeFactory(object): Sends a COURSE_GRADE_CHANGED signal to listeners and a COURSE_GRADE_NOW_PASSED if learner has passed course. """ + should_persist = should_persist_grades(course_data.course_key) + + if should_persist and force_update_subsections: + prefetch(user, course_data.course_key) + course_grade = CourseGrade(user, course_data, force_update_subsections=force_update_subsections) course_grade = course_grade.update() - should_persist = ( - should_persist_grades(course_data.course_key) and - course_grade.attempted - ) + should_persist = should_persist and course_grade.attempted if should_persist: course_grade._subsection_grade_factory.bulk_create_unsaved() PersistentCourseGrade.update_or_create( diff --git a/lms/djangoapps/grades/events.py b/lms/djangoapps/grades/events.py new file mode 100644 index 0000000000..dfc54c2a13 --- /dev/null +++ b/lms/djangoapps/grades/events.py @@ -0,0 +1,134 @@ +from crum import get_current_user +from eventtracking import tracker +from track import contexts +from track.event_transaction_utils import ( + create_new_event_transaction_id, + get_event_transaction_id, + get_event_transaction_type, + set_event_transaction_type +) + + +COURSE_GRADE_CALCULATED = u'edx.grades.course.grade_calculated' +GRADES_OVERRIDE_EVENT_TYPE = u'edx.grades.problem.score_overridden' +GRADES_RESCORE_EVENT_TYPE = u'edx.grades.problem.rescored' +PROBLEM_SUBMITTED_EVENT_TYPE = u'edx.grades.problem.submitted' +STATE_DELETED_EVENT_TYPE = u'edx.grades.problem.state_deleted' +SUBSECTION_OVERRIDE_EVENT_TYPE = u'edx.grades.subsection.score_overridden' +SUBSECTION_GRADE_CALCULATED = u'edx.grades.subsection.grade_calculated' + + +def grade_updated(**kwargs): + """ + Emits the appropriate grade-related event after checking for which + event-transaction is active. + + Emits a problem.submitted event only if there is no current event + transaction type, i.e. we have not reached this point in the code via + an outer event type (such as problem.rescored or score_overridden). + """ + root_type = get_event_transaction_type() + + if not root_type: + root_id = get_event_transaction_id() + if not root_id: + root_id = create_new_event_transaction_id() + set_event_transaction_type(PROBLEM_SUBMITTED_EVENT_TYPE) + tracker.emit( + unicode(PROBLEM_SUBMITTED_EVENT_TYPE), + { + 'user_id': unicode(kwargs['user_id']), + 'course_id': unicode(kwargs['course_id']), + 'problem_id': unicode(kwargs['usage_id']), + 'event_transaction_id': unicode(root_id), + 'event_transaction_type': unicode(PROBLEM_SUBMITTED_EVENT_TYPE), + 'weighted_earned': kwargs.get('weighted_earned'), + 'weighted_possible': kwargs.get('weighted_possible'), + } + ) + + elif root_type in [GRADES_RESCORE_EVENT_TYPE, GRADES_OVERRIDE_EVENT_TYPE]: + current_user = get_current_user() + instructor_id = getattr(current_user, 'id', None) + tracker.emit( + unicode(root_type), + { + 'course_id': unicode(kwargs['course_id']), + 'user_id': unicode(kwargs['user_id']), + 'problem_id': unicode(kwargs['usage_id']), + 'new_weighted_earned': kwargs.get('weighted_earned'), + 'new_weighted_possible': kwargs.get('weighted_possible'), + 'only_if_higher': kwargs.get('only_if_higher'), + 'instructor_id': unicode(instructor_id), + 'event_transaction_id': unicode(get_event_transaction_id()), + 'event_transaction_type': unicode(root_type), + } + ) + + elif root_type in [SUBSECTION_OVERRIDE_EVENT_TYPE]: + tracker.emit( + unicode(root_type), + { + 'course_id': unicode(kwargs['course_id']), + 'user_id': unicode(kwargs['user_id']), + 'problem_id': unicode(kwargs['usage_id']), + 'only_if_higher': kwargs.get('only_if_higher'), + 'override_deleted': kwargs.get('score_deleted', False), + 'event_transaction_id': unicode(get_event_transaction_id()), + 'event_transaction_type': unicode(root_type), + } + ) + + +def subsection_grade_calculated(subsection_grade): + """ + Emits an edx.grades.subsection.grade_calculated event + with data from the passed subsection_grade. + """ + event_name = SUBSECTION_GRADE_CALCULATED + context = contexts.course_context_from_course_id(subsection_grade.course_id) + # TODO (AN-6134): remove this context manager + with tracker.get_tracker().context(event_name, context): + tracker.emit( + event_name, + { + 'user_id': unicode(subsection_grade.user_id), + 'course_id': unicode(subsection_grade.course_id), + 'block_id': unicode(subsection_grade.usage_key), + 'course_version': unicode(subsection_grade.course_version), + 'weighted_total_earned': subsection_grade.earned_all, + 'weighted_total_possible': subsection_grade.possible_all, + 'weighted_graded_earned': subsection_grade.earned_graded, + 'weighted_graded_possible': subsection_grade.possible_graded, + 'first_attempted': unicode(subsection_grade.first_attempted), + 'subtree_edited_timestamp': unicode(subsection_grade.subtree_edited_timestamp), + 'event_transaction_id': unicode(get_event_transaction_id()), + 'event_transaction_type': unicode(get_event_transaction_type()), + 'visible_blocks_hash': unicode(subsection_grade.visible_blocks_id), + } + ) + + +def course_grade_calculated(course_grade): + """ + Emits an edx.grades.course.grade_calculated event + with data from the passed course_grade. + """ + event_name = COURSE_GRADE_CALCULATED + context = contexts.course_context_from_course_id(course_grade.course_id) + # TODO (AN-6134): remove this context manager + with tracker.get_tracker().context(event_name, context): + tracker.emit( + event_name, + { + 'user_id': unicode(course_grade.user_id), + 'course_id': unicode(course_grade.course_id), + 'course_version': unicode(course_grade.course_version), + 'percent_grade': course_grade.percent_grade, + 'letter_grade': unicode(course_grade.letter_grade), + 'course_edited_timestamp': unicode(course_grade.course_edited_timestamp), + 'event_transaction_id': unicode(get_event_transaction_id()), + 'event_transaction_type': unicode(get_event_transaction_type()), + 'grading_policy_hash': unicode(course_grade.grading_policy_hash), + } + ) diff --git a/lms/djangoapps/grades/management/commands/recalculate_subsection_grades.py b/lms/djangoapps/grades/management/commands/recalculate_subsection_grades.py index 9cd4ffeccc..d06cb68f95 100644 --- a/lms/djangoapps/grades/management/commands/recalculate_subsection_grades.py +++ b/lms/djangoapps/grades/management/commands/recalculate_subsection_grades.py @@ -13,7 +13,7 @@ from pytz import utc from courseware.models import StudentModule from lms.djangoapps.grades.constants import ScoreDatabaseTableEnum -from lms.djangoapps.grades.signals.handlers import PROBLEM_SUBMITTED_EVENT_TYPE +from lms.djangoapps.grades.events import PROBLEM_SUBMITTED_EVENT_TYPE from lms.djangoapps.grades.tasks import recalculate_subsection_grade_v3 from student.models import user_by_anonymous_id from submissions.models import Submission diff --git a/lms/djangoapps/grades/models.py b/lms/djangoapps/grades/models.py index 9dbe7a22a9..b57e33ed99 100644 --- a/lms/djangoapps/grades/models.py +++ b/lms/djangoapps/grades/models.py @@ -21,13 +21,11 @@ from model_utils.models import TimeStampedModel from opaque_keys.edx.keys import CourseKey, UsageKey from coursewarehistoryextended.fields import UnsignedBigIntAutoField, UnsignedBigIntOneToOneField -from eventtracking import tracker from openedx.core.djangoapps.xmodule_django.models import CourseKeyField, UsageKeyField from request_cache import get_cache -from track import contexts -from track.event_transaction_utils import get_event_transaction_id, get_event_transaction_type -from .config import waffle +import events + log = logging.getLogger(__name__) @@ -122,24 +120,6 @@ class BlockRecordList(tuple): return cls(blocks, course_key) -class VisibleBlocksQuerySet(models.QuerySet): - """ - A custom QuerySet representing VisibleBlocks. - """ - - def create_from_blockrecords(self, blocks): - """ - Creates a new VisibleBlocks model object. - - Argument 'blocks' should be a BlockRecordList. - """ - model, _ = self.get_or_create( - hashed=blocks.hash_value, - defaults={u'blocks_json': blocks.json_value, u'course_id': blocks.course_key}, - ) - return model - - class VisibleBlocks(models.Model): """ A django model used to track the state of a set of visible blocks under a @@ -149,12 +129,11 @@ class VisibleBlocks(models.Model): in the blocks_json field. A hash of this json array is used for lookup purposes. """ - CACHE_NAMESPACE = u"grades.models.VisibleBlocks" blocks_json = models.TextField() hashed = models.CharField(max_length=100, unique=True) course_id = CourseKeyField(blank=False, max_length=255, db_index=True) - objects = VisibleBlocksQuerySet.as_manager() + _CACHE_NAMESPACE = u"grades.models.VisibleBlocks" class Meta(object): app_label = "grades" @@ -184,11 +163,28 @@ class VisibleBlocks(models.Model): Arguments: course_key: The course identifier for the desired records """ - prefetched = get_cache(cls.CACHE_NAMESPACE).get(cls._cache_key(course_key)) - if not prefetched: + prefetched = get_cache(cls._CACHE_NAMESPACE).get(cls._cache_key(course_key), None) + if prefetched is None: prefetched = cls._initialize_cache(course_key) return prefetched + @classmethod + def cached_get_or_create(cls, blocks): + prefetched = get_cache(cls._CACHE_NAMESPACE).get(cls._cache_key(blocks.course_key)) + if prefetched is not None: + model = prefetched.get(blocks.hash_value) + if not model: + model = cls.objects.create( + hashed=blocks.hash_value, blocks_json=blocks.json_value, course_id=blocks.course_key, + ) + cls._update_cache(blocks.course_key, [model]) + else: + model, _ = cls.objects.get_or_create( + hashed=blocks.hash_value, + defaults={u'blocks_json': blocks.json_value, u'course_id': blocks.course_key}, + ) + return model + @classmethod def bulk_create(cls, course_key, block_record_lists): """ @@ -227,7 +223,7 @@ class VisibleBlocks(models.Model): block record objects. """ prefetched = {record.hashed: record for record in cls.objects.filter(course_id=course_key)} - get_cache(cls.CACHE_NAMESPACE)[cls._cache_key(course_key)] = prefetched + get_cache(cls._CACHE_NAMESPACE)[cls._cache_key(course_key)] = prefetched return prefetched @classmethod @@ -236,18 +232,10 @@ class VisibleBlocks(models.Model): Adds a specific set of visible blocks to the request cache. This assumes that prefetch has already been called. """ - get_cache(cls.CACHE_NAMESPACE)[cls._cache_key(course_key)].update( + get_cache(cls._CACHE_NAMESPACE)[cls._cache_key(course_key)].update( {visible_block.hashed: visible_block for visible_block in visible_blocks} ) - @classmethod - def clear_cache(cls, course_key): - """ - Clears the cache of all contents for a given course. - """ - cache = get_cache(cls.CACHE_NAMESPACE) - cache.pop(cls._cache_key(course_key), None) - @classmethod def _cache_key(cls, course_key): return u"visible_blocks_cache.{}".format(course_key) @@ -348,7 +336,7 @@ class PersistentSubsectionGrade(TimeStampedModel): Raises PersistentSubsectionGrade.DoesNotExist if applicable """ - return cls.objects.select_related('visible_blocks').get( + return cls.objects.select_related('visible_blocks', 'override').get( user_id=user_id, course_id=usage_key.course_key, # course_id is included to take advantage of db indexes usage_key=usage_key, @@ -363,7 +351,7 @@ class PersistentSubsectionGrade(TimeStampedModel): user_id: The user associated with the desired grades course_key: The course identifier for the desired grades """ - return cls.objects.select_related('visible_blocks').filter( + return cls.objects.select_related('visible_blocks', 'override').filter( user_id=user_id, course_id=course_key, ) @@ -373,30 +361,15 @@ class PersistentSubsectionGrade(TimeStampedModel): """ Wrapper for objects.update_or_create. """ - cls._prepare_params_and_visible_blocks(params) + cls._prepare_params(params) + VisibleBlocks.cached_get_or_create(params['visible_blocks']) + cls._prepare_params_visible_blocks_id(params) + cls._prepare_params_override(params) first_attempted = params.pop('first_attempted') user_id = params.pop('user_id') usage_key = params.pop('usage_key') - # apply grade override if one exists before saving model - try: - override = PersistentSubsectionGradeOverride.objects.get( - grade__user_id=user_id, - grade__course_id=usage_key.course_key, - grade__usage_key=usage_key, - ) - if override.earned_all_override is not None: - params['earned_all'] = override.earned_all_override - if override.possible_all_override is not None: - params['possible_all'] = override.possible_all_override - if override.earned_graded_override is not None: - params['earned_graded'] = override.earned_graded_override - if override.possible_graded_override is not None: - params['possible_graded'] = override.possible_graded_override - except PersistentSubsectionGradeOverride.DoesNotExist: - pass - grade, _ = cls.objects.update_or_create( user_id=user_id, course_id=usage_key.course_key, @@ -404,63 +377,33 @@ class PersistentSubsectionGrade(TimeStampedModel): defaults=params, ) if first_attempted is not None and grade.first_attempted is None: - if waffle.waffle().is_enabled(waffle.ESTIMATE_FIRST_ATTEMPTED): - grade.first_attempted = first_attempted - else: - grade.first_attempted = now() + grade.first_attempted = first_attempted grade.save() cls._emit_grade_calculated_event(grade) return grade @classmethod - def _prepare_first_attempted_for_create(cls, params): - """ - Update the value of 'first_attempted' to now() if we aren't - using score-based estimates. - """ - if params['first_attempted'] is not None and not waffle.waffle().is_enabled(waffle.ESTIMATE_FIRST_ATTEMPTED): - params['first_attempted'] = now() - - @classmethod - def create_grade(cls, **params): - """ - Wrapper for objects.create. - """ - cls._prepare_params_and_visible_blocks(params) - cls._prepare_first_attempted_for_create(params) - - grade = cls.objects.create(**params) - cls._emit_grade_calculated_event(grade) - return grade - - @classmethod - def bulk_create_grades(cls, grade_params_iter, course_key): + def bulk_create_grades(cls, grade_params_iter, user_id, course_key): """ Bulk creation of grades. """ if not grade_params_iter: return + PersistentSubsectionGradeOverride.prefetch(user_id, course_key) + map(cls._prepare_params, grade_params_iter) VisibleBlocks.bulk_get_or_create([params['visible_blocks'] for params in grade_params_iter], course_key) map(cls._prepare_params_visible_blocks_id, grade_params_iter) - map(cls._prepare_first_attempted_for_create, grade_params_iter) + map(cls._prepare_params_override, grade_params_iter) + grades = [PersistentSubsectionGrade(**params) for params in grade_params_iter] grades = cls.objects.bulk_create(grades) for grade in grades: cls._emit_grade_calculated_event(grade) return grades - @classmethod - def _prepare_params_and_visible_blocks(cls, params): - """ - Prepares the fields for the grade record, while - creating the related VisibleBlocks, if needed. - """ - cls._prepare_params(params) - params['visible_blocks'] = VisibleBlocks.objects.create_from_blockrecords(params['visible_blocks']) - @classmethod def _prepare_params(cls, params): """ @@ -484,34 +427,22 @@ class PersistentSubsectionGrade(TimeStampedModel): params['visible_blocks_id'] = params['visible_blocks'].hash_value del params['visible_blocks'] + @classmethod + def _prepare_params_override(cls, params): + override = PersistentSubsectionGradeOverride.get_override(params['user_id'], params['usage_key']) + if override: + if override.earned_all_override is not None: + params['earned_all'] = override.earned_all_override + if override.possible_all_override is not None: + params['possible_all'] = override.possible_all_override + if override.earned_graded_override is not None: + params['earned_graded'] = override.earned_graded_override + if override.possible_graded_override is not None: + params['possible_graded'] = override.possible_graded_override + @staticmethod def _emit_grade_calculated_event(grade): - """ - Emits an edx.grades.subsection.grade_calculated event - with data from the passed grade. - """ - # TODO: remove this context manager after completion of AN-6134 - event_name = u'edx.grades.subsection.grade_calculated' - context = contexts.course_context_from_course_id(grade.course_id) - with tracker.get_tracker().context(event_name, context): - tracker.emit( - event_name, - { - 'user_id': unicode(grade.user_id), - 'course_id': unicode(grade.course_id), - 'block_id': unicode(grade.usage_key), - 'course_version': unicode(grade.course_version), - 'weighted_total_earned': grade.earned_all, - 'weighted_total_possible': grade.possible_all, - 'weighted_graded_earned': grade.earned_graded, - 'weighted_graded_possible': grade.possible_graded, - 'first_attempted': unicode(grade.first_attempted), - 'subtree_edited_timestamp': unicode(grade.subtree_edited_timestamp), - 'event_transaction_id': unicode(get_event_transaction_id()), - 'event_transaction_type': unicode(get_event_transaction_type()), - 'visible_blocks_hash': unicode(grade.visible_blocks_id), - } - ) + events.subsection_grade_calculated(grade) class PersistentCourseGrade(TimeStampedModel): @@ -553,7 +484,7 @@ class PersistentCourseGrade(TimeStampedModel): # Information related to course completion passed_timestamp = models.DateTimeField(u'Date learner earned a passing grade', blank=True, null=True) - CACHE_NAMESPACE = u"grades.models.PersistentCourseGrade" + _CACHE_NAMESPACE = u"grades.models.PersistentCourseGrade" def __unicode__(self): """ @@ -568,16 +499,12 @@ class PersistentCourseGrade(TimeStampedModel): u"passed timestamp: {}".format(self.passed_timestamp), ]) - @classmethod - def _cache_key(cls, course_id): - return u"grades_cache.{}".format(course_id) - @classmethod def prefetch(cls, course_id, users): """ Prefetches grades for the given users for the given course. """ - get_cache(cls.CACHE_NAMESPACE)[cls._cache_key(course_id)] = { + get_cache(cls._CACHE_NAMESPACE)[cls._cache_key(course_id)] = { grade.user_id: grade for grade in cls.objects.filter(user_id__in=[user.id for user in users], course_id=course_id) @@ -595,7 +522,7 @@ class PersistentCourseGrade(TimeStampedModel): Raises PersistentCourseGrade.DoesNotExist if applicable """ try: - prefetched_grades = get_cache(cls.CACHE_NAMESPACE)[cls._cache_key(course_id)] + prefetched_grades = get_cache(cls._CACHE_NAMESPACE)[cls._cache_key(course_id)] try: return prefetched_grades[user_id] except KeyError: @@ -625,33 +552,24 @@ class PersistentCourseGrade(TimeStampedModel): if passed and not grade.passed_timestamp: grade.passed_timestamp = now() grade.save() + cls._emit_grade_calculated_event(grade) + cls._update_cache(course_id, user_id, grade) return grade + @classmethod + def _update_cache(cls, course_id, user_id, grade): + course_cache = get_cache(cls._CACHE_NAMESPACE).get(cls._cache_key(course_id)) + if course_cache is not None: + course_cache[user_id] = grade + + @classmethod + def _cache_key(cls, course_id): + return u"grades_cache.{}".format(course_id) + @staticmethod def _emit_grade_calculated_event(grade): - """ - Emits an edx.grades.course.grade_calculated event - with data from the passed grade. - """ - # TODO: remove this context manager after completion of AN-6134 - event_name = u'edx.grades.course.grade_calculated' - context = contexts.course_context_from_course_id(grade.course_id) - with tracker.get_tracker().context(event_name, context): - tracker.emit( - event_name, - { - 'user_id': unicode(grade.user_id), - 'course_id': unicode(grade.course_id), - 'course_version': unicode(grade.course_version), - 'percent_grade': grade.percent_grade, - 'letter_grade': unicode(grade.letter_grade), - 'course_edited_timestamp': unicode(grade.course_edited_timestamp), - 'event_transaction_id': unicode(get_event_transaction_id()), - 'event_transaction_type': unicode(get_event_transaction_type()), - 'grading_policy_hash': unicode(grade.grading_policy_hash), - } - ) + events.course_grade_calculated(grade) class PersistentSubsectionGradeOverride(models.Model): @@ -673,3 +591,32 @@ class PersistentSubsectionGradeOverride(models.Model): possible_all_override = models.FloatField(null=True, blank=True) earned_graded_override = models.FloatField(null=True, blank=True) possible_graded_override = models.FloatField(null=True, blank=True) + + _CACHE_NAMESPACE = u"grades.models.PersistentSubsectionGradeOverride" + + @classmethod + def prefetch(cls, user_id, course_key): + get_cache(cls._CACHE_NAMESPACE)[(user_id, str(course_key))] = { + override.grade.usage_key: override + for override in + cls.objects.filter(grade__user_id=user_id, grade__course_id=course_key) + } + + @classmethod + def get_override(cls, user_id, usage_key): + prefetch_values = get_cache(cls._CACHE_NAMESPACE).get((user_id, str(usage_key.course_key)), None) + if prefetch_values is not None: + return prefetch_values.get(usage_key) + try: + return cls.objects.get( + grade__user_id=user_id, + grade__course_id=usage_key.course_key, + grade__usage_key=usage_key, + ) + except PersistentSubsectionGradeOverride.DoesNotExist: + pass + + +def prefetch(user, course_key): + PersistentSubsectionGradeOverride.prefetch(user.id, course_key) + VisibleBlocks.bulk_read(course_key) diff --git a/lms/djangoapps/grades/services.py b/lms/djangoapps/grades/services.py index 475e29c860..c0dc48ae4d 100644 --- a/lms/djangoapps/grades/services.py +++ b/lms/djangoapps/grades/services.py @@ -4,10 +4,10 @@ import pytz from opaque_keys.edx.keys import CourseKey, UsageKey from track.event_transaction_utils import create_new_event_transaction_id, set_event_transaction_type -from util.date_utils import to_timestamp from .config.waffle import waffle_flags, REJECTED_EXAM_OVERRIDES_GRADE from .constants import ScoreDatabaseTableEnum +from .events import SUBSECTION_OVERRIDE_EVENT_TYPE from .models import PersistentSubsectionGrade, PersistentSubsectionGradeOverride from .signals.signals import SUBSECTION_OVERRIDE_CHANGED @@ -70,9 +70,6 @@ class GradesService(object): Fires off a recalculate_subsection_grade async task to update the PersistentSubsectionGrade table. Will not override earned_all or earned_graded value if they are None. Both default to None. """ - # prevent circular imports: - from .signals.handlers import SUBSECTION_OVERRIDE_EVENT_TYPE - course_key = _get_key(course_key_or_id, CourseKey) usage_key = _get_key(usage_key_or_id, UsageKey) @@ -113,9 +110,6 @@ class GradesService(object): Fires off a recalculate_subsection_grade async task to update the PersistentSubsectionGrade table. If the override does not exist, no error is raised, it just triggers the recalculation. """ - # prevent circular imports: - from .signals.handlers import SUBSECTION_OVERRIDE_EVENT_TYPE - course_key = _get_key(course_key_or_id, CourseKey) usage_key = _get_key(usage_key_or_id, UsageKey) diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index b6b9f2b0c3..a5420049ec 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/lms/djangoapps/grades/signals/handlers.py @@ -5,21 +5,13 @@ from contextlib import contextmanager from logging import getLogger from courseware.model_data import get_score, set_score -from crum import get_current_user from django.dispatch import receiver -from eventtracking import tracker -from lms.djangoapps.instructor_task.tasks_helper.module_state import GRADES_OVERRIDE_EVENT_TYPE from openedx.core.djangoapps.course_groups.signals.signals import COHORT_MEMBERSHIP_UPDATED from openedx.core.lib.grade_utils import is_score_higher_or_equal from student.models import user_by_anonymous_id from student.signals.signals import ENROLLMENT_TRACK_UPDATED from submissions.models import score_reset, score_set -from track.event_transaction_utils import ( - create_new_event_transaction_id, - get_event_transaction_id, - get_event_transaction_type, - set_event_transaction_type -) +from track.event_transaction_utils import get_event_transaction_id, get_event_transaction_type from util.date_utils import to_timestamp from xblock.scorable import ScorableXBlockMixin, Score @@ -32,17 +24,12 @@ from .signals import ( ) from ..constants import ScoreDatabaseTableEnum from ..course_grade_factory import CourseGradeFactory +from .. import events from ..scores import weighted_score from ..tasks import RECALCULATE_GRADE_DELAY, recalculate_subsection_grade_v3 log = getLogger(__name__) -# define values to be used in grading events -GRADES_RESCORE_EVENT_TYPE = 'edx.grades.problem.rescored' -PROBLEM_SUBMITTED_EVENT_TYPE = 'edx.grades.problem.submitted' -SUBSECTION_OVERRIDE_EVENT_TYPE = 'edx.grades.subsection.score_overridden' -STATE_DELETED_EVENT_TYPE = 'edx.grades.problem.state_deleted' - @receiver(score_set) def submissions_score_set_handler(sender, **kwargs): # pylint: disable=unused-argument @@ -127,7 +114,7 @@ def disconnect_submissions_signal_receiver(signal): handler = submissions_score_set_handler else: if signal != score_reset: - raise ValueError("This context manager only deal with score_set and score_reset signals.") + raise ValueError("This context manager only handles score_set and score_reset signals.") handler = submissions_score_reset_handler signal.disconnect(handler) @@ -220,8 +207,8 @@ def enqueue_subsection_update(sender, **kwargs): # pylint: disable=unused-argum Handles the PROBLEM_WEIGHTED_SCORE_CHANGED or SUBSECTION_OVERRIDE_CHANGED signals by enqueueing a subsection update operation to occur asynchronously. """ - _emit_event(kwargs) - result = recalculate_subsection_grade_v3.apply_async( + events.grade_updated(**kwargs) + recalculate_subsection_grade_v3.apply_async( kwargs=dict( user_id=kwargs['user_id'], anonymous_user_id=kwargs.get('anonymous_user_id'), @@ -249,7 +236,7 @@ def recalculate_course_grade_only(sender, course, course_structure, user, **kwar @receiver(ENROLLMENT_TRACK_UPDATED) @receiver(COHORT_MEMBERSHIP_UPDATED) -def force_recalculate_course_and_subsection_grades(sender, user, course_key, **kwargs): +def recalculate_course_and_subsection_grades(sender, user, course_key, **kwargs): """ Updates a saved course grade, forcing the subsection grades from which it is calculated to update along the way. @@ -257,65 +244,3 @@ def force_recalculate_course_and_subsection_grades(sender, user, course_key, **k 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) - - -def _emit_event(kwargs): - """ - Emits a problem submitted event only if there is no current event - transaction type, i.e. we have not reached this point in the code via a - rescore or student state deletion. - - If the event transaction type has already been set and the transacation is - a rescore, emits a problem rescored event. - """ - root_type = get_event_transaction_type() - - if not root_type: - root_id = get_event_transaction_id() - if not root_id: - root_id = create_new_event_transaction_id() - set_event_transaction_type(PROBLEM_SUBMITTED_EVENT_TYPE) - tracker.emit( - unicode(PROBLEM_SUBMITTED_EVENT_TYPE), - { - 'user_id': unicode(kwargs['user_id']), - 'course_id': unicode(kwargs['course_id']), - 'problem_id': unicode(kwargs['usage_id']), - 'event_transaction_id': unicode(root_id), - 'event_transaction_type': unicode(PROBLEM_SUBMITTED_EVENT_TYPE), - 'weighted_earned': kwargs.get('weighted_earned'), - 'weighted_possible': kwargs.get('weighted_possible'), - } - ) - - if root_type in [GRADES_RESCORE_EVENT_TYPE, GRADES_OVERRIDE_EVENT_TYPE]: - current_user = get_current_user() - instructor_id = getattr(current_user, 'id', None) - tracker.emit( - unicode(GRADES_RESCORE_EVENT_TYPE), - { - 'course_id': unicode(kwargs['course_id']), - 'user_id': unicode(kwargs['user_id']), - 'problem_id': unicode(kwargs['usage_id']), - 'new_weighted_earned': kwargs.get('weighted_earned'), - 'new_weighted_possible': kwargs.get('weighted_possible'), - 'only_if_higher': kwargs.get('only_if_higher'), - 'instructor_id': unicode(instructor_id), - 'event_transaction_id': unicode(get_event_transaction_id()), - 'event_transaction_type': unicode(root_type), - } - ) - - if root_type in [SUBSECTION_OVERRIDE_EVENT_TYPE]: - tracker.emit( - unicode(SUBSECTION_OVERRIDE_EVENT_TYPE), - { - 'course_id': unicode(kwargs['course_id']), - 'user_id': unicode(kwargs['user_id']), - 'problem_id': unicode(kwargs['usage_id']), - 'only_if_higher': kwargs.get('only_if_higher'), - 'override_deleted': kwargs.get('score_deleted', False), - 'event_transaction_id': unicode(get_event_transaction_id()), - 'event_transaction_type': unicode(root_type), - } - ) diff --git a/lms/djangoapps/grades/subsection_grade.py b/lms/djangoapps/grades/subsection_grade.py index bc2617ce05..1e97fe6d59 100644 --- a/lms/djangoapps/grades/subsection_grade.py +++ b/lms/djangoapps/grades/subsection_grade.py @@ -101,46 +101,57 @@ class SubsectionGrade(SubsectionGradeBase): """ Class for Subsection Grades. """ - def __init__(self, subsection): + def __init__(self, subsection, problem_scores, all_total, graded_total, override=None): super(SubsectionGrade, self).__init__(subsection) - self.problem_scores = OrderedDict() # dict of problem locations to ProblemScore + self.problem_scores = problem_scores + self.all_total = all_total + self.graded_total = graded_total + self.override = override - def init_from_structure(self, student, course_structure, submissions_scores, csm_scores): + @classmethod + def create(cls, subsection, course_structure, submissions_scores, csm_scores): """ - Compute the grade of this subsection for the given student and course. + Compute and create the subsection grade. """ - for descendant_key in course_structure.post_order_traversal( + problem_scores = OrderedDict() + for block_key in course_structure.post_order_traversal( filter_func=possibly_scored, - start_node=self.location, + start_node=subsection.location, ): - self._compute_block_score(descendant_key, course_structure, submissions_scores, csm_scores) + problem_score = cls._compute_block_score(block_key, course_structure, submissions_scores, csm_scores) + if problem_score: + problem_scores[block_key] = problem_score + all_total, graded_total = graders.aggregate_scores(problem_scores.values()) - self.all_total, self.graded_total = graders.aggregate_scores(self.problem_scores.values()) - self._log_event(log.debug, u"init_from_structure", student) - return self + return cls(subsection, problem_scores, all_total, graded_total) - def init_from_model(self, student, model, course_structure, submissions_scores, csm_scores): + @classmethod + def read(cls, subsection, model, course_structure, submissions_scores, csm_scores): """ - Load the subsection grade from the persisted model. + Read the subsection grade from the persisted model. """ + problem_scores = OrderedDict() for block in model.visible_blocks.blocks: - self._compute_block_score(block.locator, course_structure, submissions_scores, csm_scores, block) + problem_score = cls._compute_block_score( + block.locator, course_structure, submissions_scores, csm_scores, block, + ) + if problem_score: + problem_scores[block.locator] = problem_score - self.graded_total = AggregatedScore( - tw_earned=model.earned_graded, - tw_possible=model.possible_graded, - graded=True, - first_attempted=model.first_attempted, - ) - self.all_total = AggregatedScore( + all_total = AggregatedScore( tw_earned=model.earned_all, tw_possible=model.possible_all, graded=False, first_attempted=model.first_attempted, ) - self.override = model.override if hasattr(model, 'override') else None - self._log_event(log.debug, u"init_from_model", student) - return self + graded_total = AggregatedScore( + tw_earned=model.earned_graded, + tw_possible=model.possible_graded, + graded=True, + first_attempted=model.first_attempted, + ) + override = model.override if hasattr(model, 'override') else None + return cls(subsection, problem_scores, all_total, graded_total, override) @classmethod def bulk_create_models(cls, student, subsection_grades, course_key): @@ -153,17 +164,9 @@ class SubsectionGrade(SubsectionGradeBase): if subsection_grade if subsection_grade._should_persist_per_attempted() # pylint: disable=protected-access ] - return PersistentSubsectionGrade.bulk_create_grades(params, course_key) + return PersistentSubsectionGrade.bulk_create_grades(params, student.id, course_key) - def create_model(self, student): - """ - Saves the subsection grade in a persisted model. - """ - if self._should_persist_per_attempted(): - self._log_event(log.debug, u"create_model", student) - return PersistentSubsectionGrade.create_grade(**self._persisted_model_params(student)) - - def update_or_create_model(self, student, score_deleted): + def update_or_create_model(self, student, score_deleted=False): """ Saves or updates the subsection grade in a persisted model. """ @@ -184,8 +187,8 @@ class SubsectionGrade(SubsectionGradeBase): score_deleted ) + @staticmethod def _compute_block_score( - self, block_key, course_structure, submissions_scores, @@ -205,14 +208,12 @@ class SubsectionGrade(SubsectionGradeBase): pass else: if getattr(block, 'has_score', False): - problem_score = get_score( + return get_score( submissions_scores, csm_scores, persisted_block, block, ) - if problem_score: - self.problem_scores[block_key] = problem_score def _persisted_model_params(self, student): """ diff --git a/lms/djangoapps/grades/subsection_grade_factory.py b/lms/djangoapps/grades/subsection_grade_factory.py index b39203bdce..08cfe01a22 100644 --- a/lms/djangoapps/grades/subsection_grade_factory.py +++ b/lms/djangoapps/grades/subsection_grade_factory.py @@ -43,14 +43,14 @@ class SubsectionGradeFactory(object): if assume_zero_if_absent(self.course_data.course_key): subsection_grade = ZeroSubsectionGrade(subsection, self.course_data) else: - subsection_grade = SubsectionGrade(subsection).init_from_structure( - self.student, self.course_data.structure, self._submissions_scores, self._csm_scores, + subsection_grade = SubsectionGrade.create( + subsection, self.course_data.structure, self._submissions_scores, self._csm_scores, ) if should_persist_grades(self.course_data.course_key): if read_only: self._unsaved_subsection_grades[subsection_grade.location] = subsection_grade else: - grade_model = subsection_grade.create_model(self.student) + grade_model = subsection_grade.update_or_create_model(self.student) self._update_saved_subsection_grade(subsection.location, grade_model) return subsection_grade @@ -69,8 +69,8 @@ class SubsectionGradeFactory(object): """ self._log_event(log.debug, u"update, subsection: {}".format(subsection.location), subsection) - calculated_grade = SubsectionGrade(subsection).init_from_structure( - self.student, self.course_data.structure, self._submissions_scores, self._csm_scores, + calculated_grade = SubsectionGrade.create( + subsection, self.course_data.structure, self._submissions_scores, self._csm_scores, ) if should_persist_grades(self.course_data.course_key): @@ -80,8 +80,8 @@ class SubsectionGradeFactory(object): except PersistentSubsectionGrade.DoesNotExist: pass else: - orig_subsection_grade = SubsectionGrade(subsection).init_from_model( - self.student, grade_model, self.course_data.structure, self._submissions_scores, self._csm_scores, + orig_subsection_grade = SubsectionGrade.read( + subsection, grade_model, self.course_data.structure, self._submissions_scores, self._csm_scores, ) if not is_score_higher_or_equal( orig_subsection_grade.graded_total.earned, @@ -123,10 +123,10 @@ class SubsectionGradeFactory(object): """ if should_persist_grades(self.course_data.course_key): saved_subsection_grades = self._get_bulk_cached_subsection_grades() - subsection_grade = saved_subsection_grades.get(subsection.location) - if subsection_grade: - return SubsectionGrade(subsection).init_from_model( - self.student, subsection_grade, self.course_data.structure, self._submissions_scores, self._csm_scores, + grade = saved_subsection_grades.get(subsection.location) + if grade: + return SubsectionGrade.read( + subsection, grade, self.course_data.structure, self._submissions_scores, self._csm_scores, ) def _get_bulk_cached_subsection_grades(self): diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index 93b99f149d..6b7963a408 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -24,7 +24,7 @@ from track.event_transaction_utils import set_event_transaction_id, set_event_tr from util.date_utils import from_timestamp from xmodule.modulestore.django import modulestore -from .config.waffle import ESTIMATE_FIRST_ATTEMPTED, DISABLE_REGRADE_ON_POLICY_CHANGE, waffle +from .config.waffle import DISABLE_REGRADE_ON_POLICY_CHANGE, waffle from .constants import ScoreDatabaseTableEnum from .course_grade_factory import CourseGradeFactory from .exceptions import DatabaseNotReadyError @@ -83,14 +83,6 @@ def compute_grades_for_course_v2(self, **kwargs): TODO: Roll this back into compute_grades_for_course once all workers have the version with **kwargs. - - Sets the ESTIMATE_FIRST_ATTEMPTED flag, then calls the original task as a - synchronous function. - - estimate_first_attempted: - controls whether to unconditionally set the ESTIMATE_FIRST_ATTEMPTED - waffle switch. If false or not provided, use the global value of - the ESTIMATE_FIRST_ATTEMPTED waffle switch. """ if 'event_transaction_id' in kwargs: set_event_transaction_id(kwargs['event_transaction_id']) @@ -98,9 +90,6 @@ def compute_grades_for_course_v2(self, **kwargs): if 'event_transaction_type' in kwargs: set_event_transaction_type(kwargs['event_transaction_type']) - if kwargs.get('estimate_first_attempted'): - waffle().override_for_request(ESTIMATE_FIRST_ATTEMPTED, True) - try: return compute_grades_for_course(kwargs['course_key'], kwargs['offset'], kwargs['batch_size']) except Exception as exc: # pylint: disable=broad-except diff --git a/lms/djangoapps/grades/tests/base.py b/lms/djangoapps/grades/tests/base.py new file mode 100644 index 0000000000..47d44e3dee --- /dev/null +++ b/lms/djangoapps/grades/tests/base.py @@ -0,0 +1,100 @@ +from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory +from lms.djangoapps.course_blocks.api import get_course_blocks +from openedx.core.djangolib.testing.utils import get_mock_request +from student.models import CourseEnrollment +from student.tests.factories import UserFactory +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory + +from ..subsection_grade_factory import SubsectionGradeFactory + + +class GradeTestBase(SharedModuleStoreTestCase): + """ + Base class for some Grades tests. + """ + @classmethod + def setUpClass(cls): + super(GradeTestBase, cls).setUpClass() + cls.course = CourseFactory.create() + with cls.store.bulk_operations(cls.course.id): + cls.chapter = ItemFactory.create( + parent=cls.course, + category="chapter", + display_name="Test Chapter" + ) + cls.sequence = ItemFactory.create( + parent=cls.chapter, + category='sequential', + display_name="Test Sequential X", + graded=True, + format="Homework" + ) + cls.vertical = ItemFactory.create( + parent=cls.sequence, + category='vertical', + display_name='Test Vertical 1' + ) + problem_xml = MultipleChoiceResponseXMLFactory().build_xml( + question_text='The correct answer is Choice 3', + choices=[False, False, True, False], + choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3'] + ) + cls.problem = ItemFactory.create( + parent=cls.vertical, + category="problem", + display_name="Test Problem", + data=problem_xml + ) + cls.sequence2 = ItemFactory.create( + parent=cls.chapter, + category='sequential', + display_name="Test Sequential A", + graded=True, + format="Homework" + ) + cls.problem2 = ItemFactory.create( + parent=cls.sequence2, + category="problem", + display_name="Test Problem", + data=problem_xml + ) + # AED 2017-06-19: make cls.sequence belong to multiple parents, + # so we can test that DAGs with this shape are handled correctly. + cls.chapter_2 = ItemFactory.create( + parent=cls.course, + category='chapter', + display_name='Test Chapter 2' + ) + cls.chapter_2.children.append(cls.sequence.location) + cls.store.update_item(cls.chapter_2, UserFactory().id) + + def setUp(self): + super(GradeTestBase, self).setUp() + self.request = get_mock_request(UserFactory()) + self.client.login(username=self.request.user.username, password="test") + self._set_grading_policy() + 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) + + def _set_grading_policy(self, passing=0.5): + """ + Updates the course's grading policy. + """ + self.grading_policy = { + "GRADER": [ + { + "type": "Homework", + "min_count": 1, + "drop_count": 0, + "short_label": "HW", + "weight": 1.0, + }, + ], + "GRADE_CUTOFFS": { + "Pass": passing, + }, + } + self.course.set_grading_policy(self.grading_policy) + self.store.update_item(self.course, 0) diff --git a/lms/djangoapps/grades/tests/integration/test_events.py b/lms/djangoapps/grades/tests/integration/test_events.py index 6950775a3f..6a4823b98c 100644 --- a/lms/djangoapps/grades/tests/integration/test_events.py +++ b/lms/djangoapps/grades/tests/integration/test_events.py @@ -3,7 +3,7 @@ Test grading events across apps. """ # pylint: disable=protected-access -from mock import patch +from mock import call as mock_call, patch from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory from courseware.tests.test_submitting_problems import ProblemSubmissionTestMixin @@ -16,9 +16,7 @@ from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory -STATE_DELETED_TYPE = 'edx.grades.problem.state_deleted' -RESCORE_TYPE = 'edx.grades.problem.rescored' -SUBMITTED_TYPE = 'edx.grades.problem.submitted' +from ... import events class GradesEventIntegrationTest(ProblemSubmissionTestMixin, SharedModuleStoreTestCase): @@ -75,99 +73,84 @@ class GradesEventIntegrationTest(ProblemSubmissionTestMixin, SharedModuleStoreTe self.instructor = UserFactory.create(is_staff=True, username=u'test_instructor', password=u'test') self.refresh_course() - @patch('lms.djangoapps.instructor.enrollment.tracker') - @patch('lms.djangoapps.grades.signals.handlers.tracker') - @patch('lms.djangoapps.grades.models.tracker') - def test_delete_student_state_events(self, models_tracker, handlers_tracker, enrollment_tracker): - # submit answer + @patch('lms.djangoapps.grades.events.tracker') + def test_submit_answer(self, events_tracker): + self.submit_question_answer('p1', {'2_1': 'choice_choice_2'}) + course = self.store.get_course(self.course.id, depth=0) + + event_transaction_id = events_tracker.emit.mock_calls[0][1][1]['event_transaction_id'] + events_tracker.emit.assert_has_calls( + [ + mock_call( + events.PROBLEM_SUBMITTED_EVENT_TYPE, + { + 'user_id': unicode(self.student.id), + 'event_transaction_id': event_transaction_id, + 'event_transaction_type': events.PROBLEM_SUBMITTED_EVENT_TYPE, + 'course_id': unicode(self.course.id), + 'problem_id': unicode(self.problem.location), + 'weighted_earned': 2.0, + 'weighted_possible': 2.0, + }, + ), + mock_call( + events.COURSE_GRADE_CALCULATED, + { + 'course_version': unicode(course.course_version), + 'percent_grade': 0.02, + 'grading_policy_hash': u'ChVp0lHGQGCevD0t4njna/C44zQ=', + 'user_id': unicode(self.student.id), + 'letter_grade': u'', + 'event_transaction_id': event_transaction_id, + 'event_transaction_type': events.PROBLEM_SUBMITTED_EVENT_TYPE, + 'course_id': unicode(self.course.id), + 'course_edited_timestamp': unicode(course.subtree_edited_on), + } + ), + ], + any_order=True, + ) + + def test_delete_student_state(self): self.submit_question_answer('p1', {'2_1': 'choice_choice_2'}) - # check logging to make sure id's are tracked correctly across events - event_transaction_id = handlers_tracker.emit.mock_calls[0][1][1]['event_transaction_id'] - for call in models_tracker.emit.mock_calls: - self.assertEqual(event_transaction_id, call[1][1]['event_transaction_id']) - self.assertEqual(unicode(SUBMITTED_TYPE), call[1][1]['event_transaction_type']) - - handlers_tracker.emit.assert_called_with( - unicode(SUBMITTED_TYPE), - { - 'user_id': unicode(self.student.id), - 'event_transaction_id': event_transaction_id, - 'event_transaction_type': unicode(SUBMITTED_TYPE), - 'course_id': unicode(self.course.id), - 'problem_id': unicode(self.problem.location), - 'weighted_earned': 2.0, - 'weighted_possible': 2.0, - } - ) - + with patch('lms.djangoapps.instructor.enrollment.tracker') as enrollment_tracker: + with patch('lms.djangoapps.grades.events.tracker') as events_tracker: + reset_student_attempts( + self.course.id, self.student, self.problem.location, self.instructor, delete_module=True, + ) course = self.store.get_course(self.course.id, depth=0) - models_tracker.emit.assert_called_with( - u'edx.grades.course.grade_calculated', - { - 'course_version': unicode(course.course_version), - 'percent_grade': 0.02, - 'grading_policy_hash': u'ChVp0lHGQGCevD0t4njna/C44zQ=', - 'user_id': unicode(self.student.id), - 'letter_grade': u'', - 'event_transaction_id': event_transaction_id, - 'event_transaction_type': unicode(SUBMITTED_TYPE), - 'course_id': unicode(self.course.id), - 'course_edited_timestamp': unicode(course.subtree_edited_on), - } - ) - models_tracker.reset_mock() - handlers_tracker.reset_mock() - # delete state - reset_student_attempts(self.course.id, self.student, self.problem.location, self.instructor, delete_module=True) - - # check logging to make sure id's are tracked correctly across events event_transaction_id = enrollment_tracker.method_calls[0][1][1]['event_transaction_id'] - - # make sure the id is propagated throughout the event flow - for call in models_tracker.emit.mock_calls: - self.assertEqual(event_transaction_id, call[1][1]['event_transaction_id']) - self.assertEqual(unicode(STATE_DELETED_TYPE), call[1][1]['event_transaction_type']) - - # ensure we do not log a problem submitted event when state is deleted - handlers_tracker.assert_not_called() enrollment_tracker.emit.assert_called_with( - unicode(STATE_DELETED_TYPE), + events.STATE_DELETED_EVENT_TYPE, { 'user_id': unicode(self.student.id), 'course_id': unicode(self.course.id), 'problem_id': unicode(self.problem.location), 'instructor_id': unicode(self.instructor.id), 'event_transaction_id': event_transaction_id, - 'event_transaction_type': unicode(STATE_DELETED_TYPE), + 'event_transaction_type': events.STATE_DELETED_EVENT_TYPE, } ) - course = self.store.get_course(self.course.id, depth=0) - models_tracker.emit.assert_called_with( - u'edx.grades.course.grade_calculated', + events_tracker.emit.assert_called_with( + events.COURSE_GRADE_CALCULATED, { 'percent_grade': 0.0, 'grading_policy_hash': u'ChVp0lHGQGCevD0t4njna/C44zQ=', 'user_id': unicode(self.student.id), 'letter_grade': u'', 'event_transaction_id': event_transaction_id, - 'event_transaction_type': unicode(STATE_DELETED_TYPE), + 'event_transaction_type': events.STATE_DELETED_EVENT_TYPE, 'course_id': unicode(self.course.id), 'course_edited_timestamp': unicode(course.subtree_edited_on), 'course_version': unicode(course.course_version), } ) - @patch('lms.djangoapps.grades.signals.handlers.tracker') - @patch('lms.djangoapps.grades.models.tracker') - def test_rescoring_events(self, models_tracker, handlers_tracker): - # submit answer + def test_rescoring_events(self): self.submit_question_answer('p1', {'2_1': 'choice_choice_3'}) - models_tracker.reset_mock() - handlers_tracker.reset_mock() - new_problem_xml = MultipleChoiceResponseXMLFactory().build_xml( question_text='The correct answer is Choice 3', choices=[False, False, False, True], @@ -178,56 +161,53 @@ class GradesEventIntegrationTest(ProblemSubmissionTestMixin, SharedModuleStoreTe self.store.update_item(self.problem, self.instructor.id) self.store.publish(self.problem.location, self.instructor.id) - submit_rescore_problem_for_student( - request=get_mock_request(self.instructor), - usage_key=self.problem.location, - student=self.student, - only_if_higher=False - ) - # check logging to make sure id's are tracked correctly across - # events - event_transaction_id = handlers_tracker.emit.mock_calls[0][1][1]['event_transaction_id'] + with patch('lms.djangoapps.grades.events.tracker') as events_tracker: + submit_rescore_problem_for_student( + request=get_mock_request(self.instructor), + usage_key=self.problem.location, + student=self.student, + only_if_higher=False + ) + course = self.store.get_course(self.course.id, depth=0) - # make sure the id is propagated throughout the event flow - for call in models_tracker.emit.mock_calls: - self.assertEqual(event_transaction_id, call[1][1]['event_transaction_id']) - self.assertEqual(unicode(RESCORE_TYPE), call[1][1]['event_transaction_type']) - - # make sure the models calls have re-added the course id to the context - for args in models_tracker.get_tracker().context.call_args_list: + # make sure the tracker's context is updated with course info + for args in events_tracker.get_tracker().context.call_args_list: self.assertEqual( args[0][1], {'course_id': unicode(self.course.id), 'org_id': unicode(self.course.org)} ) - handlers_tracker.assert_not_called() - - handlers_tracker.emit.assert_called_with( - unicode(RESCORE_TYPE), - { - 'course_id': unicode(self.course.id), - 'user_id': unicode(self.student.id), - 'problem_id': unicode(self.problem.location), - 'new_weighted_earned': 2, - 'new_weighted_possible': 2, - 'only_if_higher': False, - 'instructor_id': unicode(self.instructor.id), - 'event_transaction_id': event_transaction_id, - 'event_transaction_type': unicode(RESCORE_TYPE), - } - ) - course = self.store.get_course(self.course.id, depth=0) - models_tracker.emit.assert_called_with( - u'edx.grades.course.grade_calculated', - { - 'course_version': unicode(course.course_version), - 'percent_grade': 0.02, - 'grading_policy_hash': u'ChVp0lHGQGCevD0t4njna/C44zQ=', - 'user_id': unicode(self.student.id), - 'letter_grade': u'', - 'event_transaction_id': event_transaction_id, - 'event_transaction_type': unicode(RESCORE_TYPE), - 'course_id': unicode(self.course.id), - 'course_edited_timestamp': unicode(course.subtree_edited_on), - } + event_transaction_id = events_tracker.emit.mock_calls[0][1][1]['event_transaction_id'] + events_tracker.emit.assert_has_calls( + [ + mock_call( + events.GRADES_RESCORE_EVENT_TYPE, + { + 'course_id': unicode(self.course.id), + 'user_id': unicode(self.student.id), + 'problem_id': unicode(self.problem.location), + 'new_weighted_earned': 2, + 'new_weighted_possible': 2, + 'only_if_higher': False, + 'instructor_id': unicode(self.instructor.id), + 'event_transaction_id': event_transaction_id, + 'event_transaction_type': events.GRADES_RESCORE_EVENT_TYPE, + }, + ), + mock_call( + events.COURSE_GRADE_CALCULATED, + { + 'course_version': unicode(course.course_version), + 'percent_grade': 0.02, + 'grading_policy_hash': u'ChVp0lHGQGCevD0t4njna/C44zQ=', + 'user_id': unicode(self.student.id), + 'letter_grade': u'', + 'event_transaction_id': event_transaction_id, + 'event_transaction_type': events.GRADES_RESCORE_EVENT_TYPE, + 'course_id': unicode(self.course.id), + 'course_edited_timestamp': unicode(course.subtree_edited_on), + }, + ), + ], + any_order=True, ) diff --git a/lms/djangoapps/grades/tests/integration/test_problems.py b/lms/djangoapps/grades/tests/integration/test_problems.py new file mode 100644 index 0000000000..d122858864 --- /dev/null +++ b/lms/djangoapps/grades/tests/integration/test_problems.py @@ -0,0 +1,306 @@ +import datetime +import itertools + +import ddt +import pytz +from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory +from courseware.tests.test_submitting_problems import ProblemSubmissionTestMixin +from lms.djangoapps.course_blocks.api import get_course_blocks +from openedx.core.djangolib.testing.utils import get_mock_request +from student.models import CourseEnrollment +from student.tests.factories import UserFactory +from xmodule.graders import ProblemScore +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.modulestore.tests.utils import TEST_DATA_DIR +from xmodule.modulestore.xml_importer import import_course_from_xml + +from ...subsection_grade_factory import SubsectionGradeFactory +from ..utils import answer_problem, mock_get_submissions_score + + +@ddt.ddt +class TestMultipleProblemTypesSubsectionScores(SharedModuleStoreTestCase): + """ + Test grading of different problem types. + """ + + SCORED_BLOCK_COUNT = 7 + ACTUAL_TOTAL_POSSIBLE = 17.0 + + @classmethod + def setUpClass(cls): + super(TestMultipleProblemTypesSubsectionScores, cls).setUpClass() + cls.load_scoreable_course() + chapter1 = cls.course.get_children()[0] + cls.seq1 = chapter1.get_children()[0] + + def setUp(self): + super(TestMultipleProblemTypesSubsectionScores, self).setUp() + password = u'test' + self.student = UserFactory.create(is_staff=False, username=u'test_student', password=password) + self.client.login(username=self.student.username, password=password) + self.request = get_mock_request(self.student) + self.course_structure = get_course_blocks(self.student, self.course.location) + + @classmethod + def load_scoreable_course(cls): + """ + This test course lives at `common/test/data/scoreable`. + + For details on the contents and structure of the file, see + `common/test/data/scoreable/README`. + """ + + course_items = import_course_from_xml( + cls.store, + 'test_user', + TEST_DATA_DIR, + source_dirs=['scoreable'], + static_content_store=None, + target_id=cls.store.make_course_key('edX', 'scoreable', '3000'), + raise_on_failure=True, + create_if_not_present=True, + ) + + cls.course = course_items[0] + + def test_score_submission_for_all_problems(self): + subsection_factory = SubsectionGradeFactory( + self.student, + course_structure=self.course_structure, + course=self.course, + ) + score = subsection_factory.create(self.seq1) + + self.assertEqual(score.all_total.earned, 0.0) + self.assertEqual(score.all_total.possible, self.ACTUAL_TOTAL_POSSIBLE) + + # Choose arbitrary, non-default values for earned and possible. + earned_per_block = 3.0 + possible_per_block = 7.0 + with mock_get_submissions_score(earned_per_block, possible_per_block) as mock_score: + # Configure one block to return no possible score, the rest to return 3.0 earned / 7.0 possible + block_count = self.SCORED_BLOCK_COUNT - 1 + mock_score.side_effect = itertools.chain( + [(earned_per_block, None, earned_per_block, None, datetime.datetime(2000, 1, 1))], + itertools.repeat(mock_score.return_value) + ) + score = subsection_factory.update(self.seq1) + self.assertEqual(score.all_total.earned, earned_per_block * block_count) + self.assertEqual(score.all_total.possible, possible_per_block * block_count) + + +@ddt.ddt +class TestVariedMetadata(ProblemSubmissionTestMixin, ModuleStoreTestCase): + """ + Test that changing the metadata on a block has the desired effect on the + persisted score. + """ + default_problem_metadata = { + u'graded': True, + u'weight': 2.5, + u'due': datetime.datetime(2099, 3, 15, 12, 30, 0, tzinfo=pytz.utc), + } + + def setUp(self): + super(TestVariedMetadata, self).setUp() + self.course = CourseFactory.create() + with self.store.bulk_operations(self.course.id): + self.chapter = ItemFactory.create( + parent=self.course, + category="chapter", + display_name="Test Chapter" + ) + self.sequence = ItemFactory.create( + parent=self.chapter, + category='sequential', + display_name="Test Sequential 1", + graded=True + ) + self.vertical = ItemFactory.create( + parent=self.sequence, + category='vertical', + display_name='Test Vertical 1' + ) + self.problem_xml = u''' + + + + + + + ''' + self.request = get_mock_request(UserFactory()) + self.client.login(username=self.request.user.username, password="test") + CourseEnrollment.enroll(self.request.user, self.course.id) + + def _get_altered_metadata(self, alterations): + """ + Returns a copy of the default_problem_metadata dict updated with the + specified alterations. + """ + metadata = self.default_problem_metadata.copy() + metadata.update(alterations) + return metadata + + def _add_problem_with_alterations(self, alterations): + """ + Add a problem to the course with the specified metadata alterations. + """ + + metadata = self._get_altered_metadata(alterations) + ItemFactory.create( + parent=self.vertical, + category="problem", + display_name="problem", + data=self.problem_xml, + metadata=metadata, + ) + + def _get_score(self): + """ + Return the score of the test problem when one correct problem (out of + two) is submitted. + """ + + self.submit_question_answer(u'problem', {u'2_1': u'Correct'}) + course_structure = get_course_blocks(self.request.user, self.course.location) + subsection_factory = SubsectionGradeFactory( + self.request.user, + course_structure=course_structure, + course=self.course, + ) + return subsection_factory.create(self.sequence) + + @ddt.data( + ({}, 1.25, 2.5), + ({u'weight': 27}, 13.5, 27), + ({u'weight': 1.0}, 0.5, 1.0), + ({u'weight': 0.0}, 0.0, 0.0), + ({u'weight': None}, 1.0, 2.0), + ) + @ddt.unpack + def test_weight_metadata_alterations(self, alterations, expected_earned, expected_possible): + self._add_problem_with_alterations(alterations) + score = self._get_score() + self.assertEqual(score.all_total.earned, expected_earned) + self.assertEqual(score.all_total.possible, expected_possible) + + @ddt.data( + ({u'graded': True}, 1.25, 2.5), + ({u'graded': False}, 0.0, 0.0), + ) + @ddt.unpack + def test_graded_metadata_alterations(self, alterations, expected_earned, expected_possible): + self._add_problem_with_alterations(alterations) + score = self._get_score() + self.assertEqual(score.graded_total.earned, expected_earned) + self.assertEqual(score.graded_total.possible, expected_possible) + + +@ddt.ddt +class TestWeightedProblems(SharedModuleStoreTestCase): + """ + Test scores and grades with various problem weight values. + """ + @classmethod + def setUpClass(cls): + super(TestWeightedProblems, cls).setUpClass() + cls.course = CourseFactory.create() + with cls.store.bulk_operations(cls.course.id): + cls.chapter = ItemFactory.create(parent=cls.course, category="chapter", display_name="chapter") + cls.sequential = ItemFactory.create(parent=cls.chapter, category="sequential", display_name="sequential") + cls.vertical = ItemFactory.create(parent=cls.sequential, category="vertical", display_name="vertical1") + problem_xml = cls._create_problem_xml() + cls.problems = [] + for i in range(2): + cls.problems.append( + ItemFactory.create( + parent=cls.vertical, + category="problem", + display_name="problem_{}".format(i), + data=problem_xml, + ) + ) + + def setUp(self): + super(TestWeightedProblems, self).setUp() + self.user = UserFactory() + self.request = get_mock_request(self.user) + + @classmethod + def _create_problem_xml(cls): + """ + Creates and returns XML for a multiple choice response problem + """ + return MultipleChoiceResponseXMLFactory().build_xml( + question_text='The correct answer is Choice 3', + choices=[False, False, True, False], + choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3'] + ) + + def _verify_grades(self, raw_earned, raw_possible, weight, expected_score): + """ + Verifies the computed grades are as expected. + """ + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + # pylint: disable=no-member + for problem in self.problems: + problem.weight = weight + self.store.update_item(problem, self.user.id) + self.store.publish(self.course.location, self.user.id) + + course_structure = get_course_blocks(self.request.user, self.course.location) + + # answer all problems + for problem in self.problems: + answer_problem(self.course, self.request, problem, score=raw_earned, max_value=raw_possible) + + # get grade + subsection_grade = SubsectionGradeFactory( + self.request.user, self.course, course_structure + ).update(self.sequential) + + # verify all problem grades + for problem in self.problems: + problem_score = subsection_grade.problem_scores[problem.location] + self.assertEqual(type(expected_score.first_attempted), type(problem_score.first_attempted)) + expected_score.first_attempted = problem_score.first_attempted + self.assertEquals(problem_score, expected_score) + + # verify subsection grades + self.assertEquals(subsection_grade.all_total.earned, expected_score.earned * len(self.problems)) + self.assertEquals(subsection_grade.all_total.possible, expected_score.possible * len(self.problems)) + + @ddt.data( + *itertools.product( + (0.0, 0.5, 1.0, 2.0), # raw_earned + (-2.0, -1.0, 0.0, 0.5, 1.0, 2.0), # raw_possible + (-2.0, -1.0, -0.5, 0.0, 0.5, 1.0, 2.0, 50.0, None), # weight + ) + ) + @ddt.unpack + def test_problem_weight(self, raw_earned, raw_possible, weight): + + use_weight = weight is not None and raw_possible != 0 + if use_weight: + expected_w_earned = raw_earned / raw_possible * weight + expected_w_possible = weight + else: + expected_w_earned = raw_earned + expected_w_possible = raw_possible + + expected_graded = expected_w_possible > 0 + + expected_score = ProblemScore( + raw_earned=raw_earned, + raw_possible=raw_possible, + weighted_earned=expected_w_earned, + weighted_possible=expected_w_possible, + weight=weight, + graded=expected_graded, + first_attempted=datetime.datetime(2010, 1, 1), + ) + self._verify_grades(raw_earned, raw_possible, weight, expected_score) diff --git a/lms/djangoapps/grades/tests/test_course_grade.py b/lms/djangoapps/grades/tests/test_course_grade.py new file mode 100644 index 0000000000..54dd63bb70 --- /dev/null +++ b/lms/djangoapps/grades/tests/test_course_grade.py @@ -0,0 +1,145 @@ +import ddt +from django.conf import settings +from mock import patch + +from openedx.core.djangolib.testing.utils import get_mock_request +from student.models import CourseEnrollment +from student.tests.factories import UserFactory +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory + +from ..config.waffle import ASSUME_ZERO_GRADE_IF_ABSENT, waffle +from ..course_data import CourseData +from ..course_grade import ZeroCourseGrade +from ..course_grade_factory import CourseGradeFactory +from .base import GradeTestBase +from .utils import answer_problem + + +@patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) +@ddt.ddt +class ZeroGradeTest(GradeTestBase): + """ + Tests ZeroCourseGrade (and, implicitly, ZeroSubsectionGrade) + functionality. + """ + @ddt.data(True, False) + def test_zero(self, assume_zero_enabled): + """ + Creates a ZeroCourseGrade and ensures it's empty. + """ + with waffle().override(ASSUME_ZERO_GRADE_IF_ABSENT, active=assume_zero_enabled): + course_data = CourseData(self.request.user, structure=self.course_structure) + chapter_grades = ZeroCourseGrade(self.request.user, course_data).chapter_grades + for chapter in chapter_grades: + for section in chapter_grades[chapter]['sections']: + for score in section.problem_scores.itervalues(): + self.assertEqual(score.earned, 0) + self.assertEqual(score.first_attempted, None) + self.assertEqual(section.all_total.earned, 0) + + @ddt.data(True, False) + def test_zero_null_scores(self, assume_zero_enabled): + """ + Creates a zero course grade and ensures that null scores aren't included in the section problem scores. + """ + with waffle().override(ASSUME_ZERO_GRADE_IF_ABSENT, active=assume_zero_enabled): + with patch('lms.djangoapps.grades.subsection_grade.get_score', return_value=None): + course_data = CourseData(self.request.user, structure=self.course_structure) + chapter_grades = ZeroCourseGrade(self.request.user, course_data).chapter_grades + for chapter in chapter_grades: + self.assertNotEqual({}, chapter_grades[chapter]['sections']) + for section in chapter_grades[chapter]['sections']: + self.assertEqual({}, section.problem_scores) + + +class TestScoreForModule(SharedModuleStoreTestCase): + """ + Test the method that calculates the score for a given block based on the + cumulative scores of its children. This test class uses a hard-coded block + hierarchy with scores as follows: + a + +--------+--------+ + b c + +--------------+-----------+ | + d e f g + +-----+ +-----+-----+ | | + h i j k l m n + (2/5) (3/5) (0/1) - (1/3) - (3/10) + + """ + @classmethod + def setUpClass(cls): + super(TestScoreForModule, cls).setUpClass() + cls.course = CourseFactory.create() + with cls.store.bulk_operations(cls.course.id): + cls.a = ItemFactory.create(parent=cls.course, category="chapter", display_name="a") + cls.b = ItemFactory.create(parent=cls.a, category="sequential", display_name="b") + cls.c = ItemFactory.create(parent=cls.a, category="sequential", display_name="c") + cls.d = ItemFactory.create(parent=cls.b, category="vertical", display_name="d") + cls.e = ItemFactory.create(parent=cls.b, category="vertical", display_name="e") + cls.f = ItemFactory.create(parent=cls.b, category="vertical", display_name="f") + cls.g = ItemFactory.create(parent=cls.c, category="vertical", display_name="g") + cls.h = ItemFactory.create(parent=cls.d, category="problem", display_name="h") + cls.i = ItemFactory.create(parent=cls.d, category="problem", display_name="i") + cls.j = ItemFactory.create(parent=cls.e, category="problem", display_name="j") + cls.k = ItemFactory.create(parent=cls.e, category="html", display_name="k") + cls.l = ItemFactory.create(parent=cls.e, category="problem", display_name="l") + cls.m = ItemFactory.create(parent=cls.f, category="html", display_name="m") + cls.n = ItemFactory.create(parent=cls.g, category="problem", display_name="n") + + cls.request = get_mock_request(UserFactory()) + CourseEnrollment.enroll(cls.request.user, cls.course.id) + + answer_problem(cls.course, cls.request, cls.h, score=2, max_value=5) + answer_problem(cls.course, cls.request, cls.i, score=3, max_value=5) + answer_problem(cls.course, cls.request, cls.j, score=0, max_value=1) + 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().read(cls.request.user, cls.course) + + def test_score_chapter(self): + earned, possible = self.course_grade.score_for_module(self.a.location) + self.assertEqual(earned, 9) + self.assertEqual(possible, 24) + + def test_score_section_many_leaves(self): + earned, possible = self.course_grade.score_for_module(self.b.location) + self.assertEqual(earned, 6) + self.assertEqual(possible, 14) + + def test_score_section_one_leaf(self): + earned, possible = self.course_grade.score_for_module(self.c.location) + self.assertEqual(earned, 3) + self.assertEqual(possible, 10) + + def test_score_vertical_two_leaves(self): + earned, possible = self.course_grade.score_for_module(self.d.location) + self.assertEqual(earned, 5) + self.assertEqual(possible, 10) + + def test_score_vertical_two_leaves_one_unscored(self): + earned, possible = self.course_grade.score_for_module(self.e.location) + self.assertEqual(earned, 1) + self.assertEqual(possible, 4) + + def test_score_vertical_no_score(self): + earned, possible = self.course_grade.score_for_module(self.f.location) + self.assertEqual(earned, 0) + self.assertEqual(possible, 0) + + def test_score_vertical_one_leaf(self): + earned, possible = self.course_grade.score_for_module(self.g.location) + self.assertEqual(earned, 3) + self.assertEqual(possible, 10) + + def test_score_leaf(self): + earned, possible = self.course_grade.score_for_module(self.h.location) + self.assertEqual(earned, 2) + self.assertEqual(possible, 5) + + def test_score_leaf_no_score(self): + earned, possible = self.course_grade.score_for_module(self.m.location) + self.assertEqual(earned, 0) + self.assertEqual(possible, 0) diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py new file mode 100644 index 0000000000..5f97c3997d --- /dev/null +++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py @@ -0,0 +1,299 @@ +import itertools +from nose.plugins.attrib import attr + +import ddt +from courseware.access import has_access +from django.conf import settings +from lms.djangoapps.grades.config.tests.utils import persistent_grades_feature_flags +from mock import patch +from openedx.core.djangoapps.content.block_structure.factory import BlockStructureFactory +from student.tests.factories import UserFactory +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +from ..config.waffle import ASSUME_ZERO_GRADE_IF_ABSENT, waffle +from ..course_grade import CourseGrade, ZeroCourseGrade +from ..course_grade_factory import CourseGradeFactory +from ..subsection_grade import SubsectionGrade, ZeroSubsectionGrade +from .base import GradeTestBase +from .utils import mock_get_score + + +@ddt.ddt +class TestCourseGradeFactory(GradeTestBase): + """ + Test that CourseGrades are calculated properly + """ + def _assert_zero_grade(self, course_grade, expected_grade_class): + """ + Asserts whether the given course_grade is as expected with + zero values. + """ + self.assertIsInstance(course_grade, expected_grade_class) + self.assertIsNone(course_grade.letter_grade) + self.assertEqual(course_grade.percent, 0.0) + self.assertIsNotNone(course_grade.chapter_grades) + + def test_course_grade_no_access(self): + """ + Test to ensure a grade can ba calculated for a student in a course, even if they themselves do not have access. + """ + invisible_course = CourseFactory.create(visible_to_staff_only=True) + access = has_access(self.request.user, 'load', invisible_course) + self.assertEqual(access.has_access, False) + self.assertEqual(access.error_code, 'not_visible_to_user') + + # with self.assertNoExceptionRaised: <- this isn't a real method, it's an implicit assumption + grade = CourseGradeFactory().read(self.request.user, invisible_course) + self.assertEqual(grade.percent, 0) + + @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) + @ddt.data( + (True, True), + (True, False), + (False, True), + (False, False), + ) + @ddt.unpack + def test_course_grade_feature_gating(self, feature_flag, course_setting): + # Grades are only saved if the feature flag and the advanced setting are + # both set to True. + grade_factory = CourseGradeFactory() + with persistent_grades_feature_flags( + global_flag=feature_flag, + enabled_for_all_courses=False, + course_id=self.course.id, + enabled_for_course=course_setting + ): + with patch('lms.djangoapps.grades.models.PersistentCourseGrade.read') as mock_read_grade: + grade_factory.read(self.request.user, self.course) + self.assertEqual(mock_read_grade.called, feature_flag and course_setting) + + def test_read_and_update(self): + grade_factory = CourseGradeFactory() + + def _assert_read(expected_pass, expected_percent): + """ + Creates the grade, ensuring it is as expected. + """ + course_grade = grade_factory.read(self.request.user, self.course) + _assert_grade_values(course_grade, expected_pass, expected_percent) + _assert_section_order(course_grade) + + def _assert_grade_values(course_grade, expected_pass, expected_percent): + self.assertEqual(course_grade.letter_grade, u'Pass' if expected_pass else None) + self.assertEqual(course_grade.percent, expected_percent) + + def _assert_section_order(course_grade): + sections = course_grade.chapter_grades[self.chapter.location]['sections'] + self.assertEqual( + [section.display_name for section in sections], + [self.sequence.display_name, self.sequence2.display_name] + ) + + with self.assertNumQueries(2), mock_get_score(1, 2): + _assert_read(expected_pass=False, expected_percent=0) # start off with grade of 0 + + with self.assertNumQueries(29), mock_get_score(1, 2): + grade_factory.update(self.request.user, self.course, force_update_subsections=True) + + with self.assertNumQueries(4): + _assert_read(expected_pass=True, expected_percent=0.5) # updated to grade of .5 + + with self.assertNumQueries(6), mock_get_score(1, 4): + grade_factory.update(self.request.user, self.course, force_update_subsections=False) + + with self.assertNumQueries(4): + _assert_read(expected_pass=True, expected_percent=0.5) # NOT updated to grade of .25 + + with self.assertNumQueries(12), mock_get_score(2, 2): + grade_factory.update(self.request.user, self.course, force_update_subsections=True) + + with self.assertNumQueries(4): + _assert_read(expected_pass=True, expected_percent=1.0) # updated to grade of 1.0 + + @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) + @ddt.data(*itertools.product((True, False), (True, False))) + @ddt.unpack + def test_read_zero(self, assume_zero_enabled, create_if_needed): + with waffle().override(ASSUME_ZERO_GRADE_IF_ABSENT, active=assume_zero_enabled): + grade_factory = CourseGradeFactory() + course_grade = grade_factory.read(self.request.user, self.course, create_if_needed=create_if_needed) + if create_if_needed or assume_zero_enabled: + self._assert_zero_grade(course_grade, ZeroCourseGrade if assume_zero_enabled else CourseGrade) + else: + self.assertIsNone(course_grade) + + def test_create_zero_subs_grade_for_nonzero_course_grade(self): + subsection = self.course_structure[self.sequence.location] + with mock_get_score(1, 2): + self.subsection_grade_factory.update(subsection) + course_grade = CourseGradeFactory().update(self.request.user, self.course) + subsection1_grade = course_grade.subsection_grades[self.sequence.location] + subsection2_grade = course_grade.subsection_grades[self.sequence2.location] + self.assertIsInstance(subsection1_grade, SubsectionGrade) + self.assertIsInstance(subsection2_grade, ZeroSubsectionGrade) + + @ddt.data(True, False) + def test_iter_force_update(self, force_update): + with patch('lms.djangoapps.grades.subsection_grade_factory.SubsectionGradeFactory.update') as mock_update: + set(CourseGradeFactory().iter( + users=[self.request.user], course=self.course, force_update=force_update, + )) + self.assertEqual(mock_update.called, force_update) + + def test_course_grade_summary(self): + with mock_get_score(1, 2): + self.subsection_grade_factory.update(self.course_structure[self.sequence.location]) + course_grade = CourseGradeFactory().update(self.request.user, self.course) + + actual_summary = course_grade.summary + + # We should have had a zero subsection grade for sequential 2, since we never + # gave it a mock score above. + expected_summary = { + 'grade': None, + 'grade_breakdown': { + 'Homework': { + 'category': 'Homework', + 'percent': 0.25, + 'detail': 'Homework = 25.00% of a possible 100.00%', + } + }, + 'percent': 0.25, + 'section_breakdown': [ + { + 'category': 'Homework', + 'detail': u'Homework 1 - Test Sequential X - 50% (1/2)', + 'label': u'HW 01', + 'percent': 0.5 + }, + { + 'category': 'Homework', + 'detail': u'Homework 2 - Test Sequential A - 0% (0/1)', + 'label': u'HW 02', + 'percent': 0.0 + }, + { + 'category': 'Homework', + 'detail': u'Homework Average = 25%', + 'label': u'HW Avg', + 'percent': 0.25, + 'prominent': True + }, + ] + } + self.assertEqual(expected_summary, actual_summary) + + +@attr(shard=1) +class TestGradeIteration(SharedModuleStoreTestCase): + """ + Test iteration through student course grades. + """ + COURSE_NUM = "1000" + COURSE_NAME = "grading_test_course" + + @classmethod + def setUpClass(cls): + super(TestGradeIteration, cls).setUpClass() + cls.course = CourseFactory.create( + display_name=cls.COURSE_NAME, + number=cls.COURSE_NUM + ) + + def setUp(self): + """ + Create a course and a handful of users to assign grades + """ + super(TestGradeIteration, self).setUp() + + self.students = [ + UserFactory.create(username='student1'), + UserFactory.create(username='student2'), + UserFactory.create(username='student3'), + UserFactory.create(username='student4'), + UserFactory.create(username='student5'), + ] + + def test_empty_student_list(self): + """ + If we don't pass in any students, it should return a zero-length + iterator, but it shouldn't error. + """ + grade_results = list(CourseGradeFactory().iter([], self.course)) + self.assertEqual(grade_results, []) + + def test_all_empty_grades(self): + """ + No students have grade entries. + """ + with patch.object( + BlockStructureFactory, + 'create_from_store', + wraps=BlockStructureFactory.create_from_store + ) as mock_create_from_store: + all_course_grades, all_errors = self._course_grades_and_errors_for(self.course, self.students) + self.assertEquals(mock_create_from_store.call_count, 1) + + self.assertEqual(len(all_errors), 0) + for course_grade in all_course_grades.values(): + self.assertIsNone(course_grade.letter_grade) + self.assertEqual(course_grade.percent, 0.0) + + @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 + process for this student fails entirely due to an unexpected event -- + having errors in the problem sets will not trigger this. + + We patch the grade() method with our own, which will generate the errors + for student3 and student4. + """ + + student1, student2, student3, student4, student5 = self.students + mock_course_grade.side_effect = [ + Exception("Error for {}.".format(student.username)) + if student.username in ['student3', 'student4'] + else mock_course_grade.return_value + for student in self.students + ] + with self.assertNumQueries(4): + all_course_grades, all_errors = self._course_grades_and_errors_for(self.course, self.students) + self.assertEqual( + {student: all_errors[student].message for student in all_errors}, + { + student3: "Error for student3.", + student4: "Error for student4.", + } + ) + + # But we should still have five gradesets + self.assertEqual(len(all_course_grades), 5) + + # Even though two will simply be empty + self.assertIsNone(all_course_grades[student3]) + self.assertIsNone(all_course_grades[student4]) + + # The rest will have grade information in them + self.assertIsNotNone(all_course_grades[student1]) + self.assertIsNotNone(all_course_grades[student2]) + self.assertIsNotNone(all_course_grades[student5]) + + def _course_grades_and_errors_for(self, course, students): + """ + Simple helper method to iterate through student grades and give us + two dictionaries -- one that has all students and their respective + course grades, and one that has only students that could not be graded + and their respective error messages. + """ + students_to_course_grades = {} + students_to_errors = {} + + for student, course_grade, error in CourseGradeFactory().iter(students, course): + students_to_course_grades[student] = course_grade + if error: + students_to_errors[student] = error + + return students_to_course_grades, students_to_errors diff --git a/lms/djangoapps/grades/tests/test_grades.py b/lms/djangoapps/grades/tests/test_grades.py deleted file mode 100644 index ca253031ea..0000000000 --- a/lms/djangoapps/grades/tests/test_grades.py +++ /dev/null @@ -1,335 +0,0 @@ -""" -Test grade calculation. -""" - -import datetime -import itertools - -import ddt -from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory -from lms.djangoapps.course_blocks.api import get_course_blocks -from mock import patch -from nose.plugins.attrib import attr -from openedx.core.djangoapps.content.block_structure.factory import BlockStructureFactory -from openedx.core.djangolib.testing.utils import get_mock_request -from student.models import CourseEnrollment -from student.tests.factories import UserFactory -from xmodule.graders import ProblemScore -from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory - -from ..course_grade_factory import CourseGradeFactory -from ..subsection_grade_factory import SubsectionGradeFactory -from .utils import answer_problem - - -@attr(shard=1) -class TestGradeIteration(SharedModuleStoreTestCase): - """ - Test iteration through student course grades. - """ - COURSE_NUM = "1000" - COURSE_NAME = "grading_test_course" - - @classmethod - def setUpClass(cls): - super(TestGradeIteration, cls).setUpClass() - cls.course = CourseFactory.create( - display_name=cls.COURSE_NAME, - number=cls.COURSE_NUM - ) - - def setUp(self): - """ - Create a course and a handful of users to assign grades - """ - super(TestGradeIteration, self).setUp() - - self.students = [ - UserFactory.create(username='student1'), - UserFactory.create(username='student2'), - UserFactory.create(username='student3'), - UserFactory.create(username='student4'), - UserFactory.create(username='student5'), - ] - - def test_empty_student_list(self): - """ - If we don't pass in any students, it should return a zero-length - iterator, but it shouldn't error. - """ - grade_results = list(CourseGradeFactory().iter([], self.course)) - self.assertEqual(grade_results, []) - - def test_all_empty_grades(self): - """ - No students have grade entries. - """ - with patch.object( - BlockStructureFactory, - 'create_from_store', - wraps=BlockStructureFactory.create_from_store - ) as mock_create_from_store: - all_course_grades, all_errors = self._course_grades_and_errors_for(self.course, self.students) - self.assertEquals(mock_create_from_store.call_count, 1) - - self.assertEqual(len(all_errors), 0) - for course_grade in all_course_grades.values(): - self.assertIsNone(course_grade.letter_grade) - self.assertEqual(course_grade.percent, 0.0) - - @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 - process for this student fails entirely due to an unexpected event -- - having errors in the problem sets will not trigger this. - - We patch the grade() method with our own, which will generate the errors - for student3 and student4. - """ - - student1, student2, student3, student4, student5 = self.students - mock_course_grade.side_effect = [ - Exception("Error for {}.".format(student.username)) - if student.username in ['student3', 'student4'] - else mock_course_grade.return_value - for student in self.students - ] - with self.assertNumQueries(4): - all_course_grades, all_errors = self._course_grades_and_errors_for(self.course, self.students) - self.assertEqual( - {student: all_errors[student].message for student in all_errors}, - { - student3: "Error for student3.", - student4: "Error for student4.", - } - ) - - # But we should still have five gradesets - self.assertEqual(len(all_course_grades), 5) - - # Even though two will simply be empty - self.assertIsNone(all_course_grades[student3]) - self.assertIsNone(all_course_grades[student4]) - - # The rest will have grade information in them - self.assertIsNotNone(all_course_grades[student1]) - self.assertIsNotNone(all_course_grades[student2]) - self.assertIsNotNone(all_course_grades[student5]) - - def _course_grades_and_errors_for(self, course, students): - """ - Simple helper method to iterate through student grades and give us - two dictionaries -- one that has all students and their respective - course grades, and one that has only students that could not be graded - and their respective error messages. - """ - students_to_course_grades = {} - students_to_errors = {} - - for student, course_grade, error in CourseGradeFactory().iter(students, course): - students_to_course_grades[student] = course_grade - if error: - students_to_errors[student] = error - - return students_to_course_grades, students_to_errors - - -@ddt.ddt -class TestWeightedProblems(SharedModuleStoreTestCase): - """ - Test scores and grades with various problem weight values. - """ - @classmethod - def setUpClass(cls): - super(TestWeightedProblems, cls).setUpClass() - cls.course = CourseFactory.create() - with cls.store.bulk_operations(cls.course.id): - cls.chapter = ItemFactory.create(parent=cls.course, category="chapter", display_name="chapter") - cls.sequential = ItemFactory.create(parent=cls.chapter, category="sequential", display_name="sequential") - cls.vertical = ItemFactory.create(parent=cls.sequential, category="vertical", display_name="vertical1") - problem_xml = cls._create_problem_xml() - cls.problems = [] - for i in range(2): - cls.problems.append( - ItemFactory.create( - parent=cls.vertical, - category="problem", - display_name="problem_{}".format(i), - data=problem_xml, - ) - ) - - def setUp(self): - super(TestWeightedProblems, self).setUp() - self.user = UserFactory() - self.request = get_mock_request(self.user) - - @classmethod - def _create_problem_xml(cls): - """ - Creates and returns XML for a multiple choice response problem - """ - return MultipleChoiceResponseXMLFactory().build_xml( - question_text='The correct answer is Choice 3', - choices=[False, False, True, False], - choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3'] - ) - - def _verify_grades(self, raw_earned, raw_possible, weight, expected_score): - """ - Verifies the computed grades are as expected. - """ - with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): - # pylint: disable=no-member - for problem in self.problems: - problem.weight = weight - self.store.update_item(problem, self.user.id) - self.store.publish(self.course.location, self.user.id) - - course_structure = get_course_blocks(self.request.user, self.course.location) - - # answer all problems - for problem in self.problems: - answer_problem(self.course, self.request, problem, score=raw_earned, max_value=raw_possible) - - # get grade - subsection_grade = SubsectionGradeFactory( - self.request.user, self.course, course_structure - ).update(self.sequential) - - # verify all problem grades - for problem in self.problems: - problem_score = subsection_grade.problem_scores[problem.location] - self.assertEqual(type(expected_score.first_attempted), type(problem_score.first_attempted)) - expected_score.first_attempted = problem_score.first_attempted - self.assertEquals(problem_score, expected_score) - - # verify subsection grades - self.assertEquals(subsection_grade.all_total.earned, expected_score.earned * len(self.problems)) - self.assertEquals(subsection_grade.all_total.possible, expected_score.possible * len(self.problems)) - - @ddt.data( - *itertools.product( - (0.0, 0.5, 1.0, 2.0), # raw_earned - (-2.0, -1.0, 0.0, 0.5, 1.0, 2.0), # raw_possible - (-2.0, -1.0, -0.5, 0.0, 0.5, 1.0, 2.0, 50.0, None), # weight - ) - ) - @ddt.unpack - def test_problem_weight(self, raw_earned, raw_possible, weight): - - use_weight = weight is not None and raw_possible != 0 - if use_weight: - expected_w_earned = raw_earned / raw_possible * weight - expected_w_possible = weight - else: - expected_w_earned = raw_earned - expected_w_possible = raw_possible - - expected_graded = expected_w_possible > 0 - - expected_score = ProblemScore( - raw_earned=raw_earned, - raw_possible=raw_possible, - weighted_earned=expected_w_earned, - weighted_possible=expected_w_possible, - weight=weight, - graded=expected_graded, - first_attempted=datetime.datetime(2010, 1, 1), - ) - self._verify_grades(raw_earned, raw_possible, weight, expected_score) - - -class TestScoreForModule(SharedModuleStoreTestCase): - """ - Test the method that calculates the score for a given block based on the - cumulative scores of its children. This test class uses a hard-coded block - hierarchy with scores as follows: - a - +--------+--------+ - b c - +--------------+-----------+ | - d e f g - +-----+ +-----+-----+ | | - h i j k l m n - (2/5) (3/5) (0/1) - (1/3) - (3/10) - - """ - @classmethod - def setUpClass(cls): - super(TestScoreForModule, cls).setUpClass() - cls.course = CourseFactory.create() - with cls.store.bulk_operations(cls.course.id): - cls.a = ItemFactory.create(parent=cls.course, category="chapter", display_name="a") - cls.b = ItemFactory.create(parent=cls.a, category="sequential", display_name="b") - cls.c = ItemFactory.create(parent=cls.a, category="sequential", display_name="c") - cls.d = ItemFactory.create(parent=cls.b, category="vertical", display_name="d") - cls.e = ItemFactory.create(parent=cls.b, category="vertical", display_name="e") - cls.f = ItemFactory.create(parent=cls.b, category="vertical", display_name="f") - cls.g = ItemFactory.create(parent=cls.c, category="vertical", display_name="g") - cls.h = ItemFactory.create(parent=cls.d, category="problem", display_name="h") - cls.i = ItemFactory.create(parent=cls.d, category="problem", display_name="i") - cls.j = ItemFactory.create(parent=cls.e, category="problem", display_name="j") - cls.k = ItemFactory.create(parent=cls.e, category="html", display_name="k") - cls.l = ItemFactory.create(parent=cls.e, category="problem", display_name="l") - cls.m = ItemFactory.create(parent=cls.f, category="html", display_name="m") - cls.n = ItemFactory.create(parent=cls.g, category="problem", display_name="n") - - cls.request = get_mock_request(UserFactory()) - CourseEnrollment.enroll(cls.request.user, cls.course.id) - - answer_problem(cls.course, cls.request, cls.h, score=2, max_value=5) - answer_problem(cls.course, cls.request, cls.i, score=3, max_value=5) - answer_problem(cls.course, cls.request, cls.j, score=0, max_value=1) - 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().read(cls.request.user, cls.course) - - def test_score_chapter(self): - earned, possible = self.course_grade.score_for_module(self.a.location) - self.assertEqual(earned, 9) - self.assertEqual(possible, 24) - - def test_score_section_many_leaves(self): - earned, possible = self.course_grade.score_for_module(self.b.location) - self.assertEqual(earned, 6) - self.assertEqual(possible, 14) - - def test_score_section_one_leaf(self): - earned, possible = self.course_grade.score_for_module(self.c.location) - self.assertEqual(earned, 3) - self.assertEqual(possible, 10) - - def test_score_vertical_two_leaves(self): - earned, possible = self.course_grade.score_for_module(self.d.location) - self.assertEqual(earned, 5) - self.assertEqual(possible, 10) - - def test_score_vertical_two_leaves_one_unscored(self): - earned, possible = self.course_grade.score_for_module(self.e.location) - self.assertEqual(earned, 1) - self.assertEqual(possible, 4) - - def test_score_vertical_no_score(self): - earned, possible = self.course_grade.score_for_module(self.f.location) - self.assertEqual(earned, 0) - self.assertEqual(possible, 0) - - def test_score_vertical_one_leaf(self): - earned, possible = self.course_grade.score_for_module(self.g.location) - self.assertEqual(earned, 3) - self.assertEqual(possible, 10) - - def test_score_leaf(self): - earned, possible = self.course_grade.score_for_module(self.h.location) - self.assertEqual(earned, 2) - self.assertEqual(possible, 5) - - def test_score_leaf_no_score(self): - earned, possible = self.course_grade.score_for_module(self.m.location) - self.assertEqual(earned, 0) - self.assertEqual(possible, 0) diff --git a/lms/djangoapps/grades/tests/test_models.py b/lms/djangoapps/grades/tests/test_models.py index 39e31e273b..18b4d4f062 100644 --- a/lms/djangoapps/grades/tests/test_models.py +++ b/lms/djangoapps/grades/tests/test_models.py @@ -16,7 +16,6 @@ from freezegun import freeze_time from mock import patch from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator -from lms.djangoapps.grades.config import waffle from lms.djangoapps.grades.models import ( BLOCK_RECORD_LIST_VERSION, BlockRecord, @@ -131,7 +130,7 @@ class VisibleBlocksTest(GradesModelTestCase): """ Creates and returns a BlockRecordList for the given blocks. """ - return VisibleBlocks.objects.create_from_blockrecords(BlockRecordList.from_list(blocks, self.course_key)) + return VisibleBlocks.cached_get_or_create(BlockRecordList.from_list(blocks, self.course_key)) def test_creation(self): """ @@ -215,45 +214,30 @@ class PersistentSubsectionGradeTest(GradesModelTestCase): "first_attempted": datetime(2000, 1, 1, 12, 30, 45, tzinfo=pytz.UTC), } - def test_create(self): - """ - Tests model creation, and confirms error when trying to recreate model. - """ - 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.assertEqual(read_grade.visible_blocks.blocks, self.block_records) - with self.assertRaises(IntegrityError): - PersistentSubsectionGrade.create_grade(**self.params) - @ddt.data('course_version', 'subtree_edited_timestamp') def test_optional_fields(self, field): del self.params[field] - PersistentSubsectionGrade.create_grade(**self.params) + PersistentSubsectionGrade.update_or_create_grade(**self.params) @ddt.data( - ("user_id", IntegrityError), + ("user_id", KeyError), ("usage_key", KeyError), ("earned_all", IntegrityError), ("possible_all", IntegrityError), ("earned_graded", IntegrityError), ("possible_graded", IntegrityError), - ("first_attempted", KeyError), ("visible_blocks", KeyError), + ("first_attempted", KeyError), ) @ddt.unpack def test_non_optional_fields(self, field, error): del self.params[field] with self.assertRaises(error): - PersistentSubsectionGrade.create_grade(**self.params) + PersistentSubsectionGrade.update_or_create_grade(**self.params) @ddt.data(True, False) def test_update_or_create_grade(self, already_created): - created_grade = PersistentSubsectionGrade.create_grade(**self.params) if already_created else None + created_grade = PersistentSubsectionGrade.update_or_create_grade(**self.params) if already_created else None self.params["earned_all"] = 7 updated_grade = PersistentSubsectionGrade.update_or_create_grade(**self.params) @@ -262,53 +246,48 @@ class PersistentSubsectionGradeTest(GradesModelTestCase): self.assertEqual(created_grade.id, updated_grade.id) self.assertEqual(created_grade.earned_all, 6) - @ddt.unpack - @ddt.data( - (True, datetime(2000, 1, 1, 12, 30, 45, tzinfo=pytz.UTC)), - (False, None), # Use as now(). Freeze time needs this calculation to happen at test time. - ) - def test_update_or_create_attempted(self, is_active, expected_first_attempted): - with freeze_time(now()): - if expected_first_attempted is None: - expected_first_attempted = now() - with waffle.waffle().override(waffle.ESTIMATE_FIRST_ATTEMPTED, active=is_active): - grade = PersistentSubsectionGrade.update_or_create_grade(**self.params) - self.assertEqual(grade.first_attempted, expected_first_attempted) + with self.assertNumQueries(1): + read_grade = PersistentSubsectionGrade.read_grade( + user_id=self.params["user_id"], + usage_key=self.params["usage_key"], + ) + self.assertEqual(updated_grade, read_grade) + self.assertEqual(read_grade.visible_blocks.blocks, self.block_records) def test_unattempted(self): self.params['first_attempted'] = None self.params['earned_all'] = 0.0 self.params['earned_graded'] = 0.0 - grade = PersistentSubsectionGrade.create_grade(**self.params) + grade = PersistentSubsectionGrade.update_or_create_grade(**self.params) self.assertIsNone(grade.first_attempted) self.assertEqual(grade.earned_all, 0.0) self.assertEqual(grade.earned_graded, 0.0) def test_first_attempted_not_changed_on_update(self): - PersistentSubsectionGrade.create_grade(**self.params) + PersistentSubsectionGrade.update_or_create_grade(**self.params) moment = now() grade = PersistentSubsectionGrade.update_or_create_grade(**self.params) self.assertLess(grade.first_attempted, moment) def test_unattempted_save_does_not_remove_attempt(self): - PersistentSubsectionGrade.create_grade(**self.params) + PersistentSubsectionGrade.update_or_create_grade(**self.params) self.params['first_attempted'] = None grade = PersistentSubsectionGrade.update_or_create_grade(**self.params) self.assertIsInstance(grade.first_attempted, datetime) self.assertEqual(grade.earned_all, 6.0) def test_update_or_create_event(self): - with patch('lms.djangoapps.grades.models.tracker') as tracker_mock: + with patch('lms.djangoapps.grades.events.tracker') as tracker_mock: grade = PersistentSubsectionGrade.update_or_create_grade(**self.params) self._assert_tracker_emitted_event(tracker_mock, grade) def test_create_event(self): - with patch('lms.djangoapps.grades.models.tracker') as tracker_mock: - grade = PersistentSubsectionGrade.create_grade(**self.params) + with patch('lms.djangoapps.grades.events.tracker') as tracker_mock: + grade = PersistentSubsectionGrade.update_or_create_grade(**self.params) self._assert_tracker_emitted_event(tracker_mock, grade) def test_grade_override(self): - grade = PersistentSubsectionGrade.create_grade(**self.params) + grade = PersistentSubsectionGrade.update_or_create_grade(**self.params) override = PersistentSubsectionGradeOverride(grade=grade, earned_all_override=0.0, earned_graded_override=0.0) override.save() grade = PersistentSubsectionGrade.update_or_create_grade(**self.params) @@ -456,7 +435,7 @@ class PersistentCourseGradesTest(GradesModelTestCase): PersistentCourseGrade.read(self.params["user_id"], self.params["course_id"]) def test_update_or_create_event(self): - with patch('lms.djangoapps.grades.models.tracker') as tracker_mock: + with patch('lms.djangoapps.grades.events.tracker') as tracker_mock: grade = PersistentCourseGrade.update_or_create(**self.params) self._assert_tracker_emitted_event(tracker_mock, grade) diff --git a/lms/djangoapps/grades/tests/test_new.py b/lms/djangoapps/grades/tests/test_new.py deleted file mode 100644 index 6c8e35e4d1..0000000000 --- a/lms/djangoapps/grades/tests/test_new.py +++ /dev/null @@ -1,699 +0,0 @@ -""" -Test saved subsection grade functionality. -""" -# pylint: disable=protected-access - -import datetime -import itertools - -import ddt -import pytz -from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory -from courseware.access import has_access -from courseware.tests.test_submitting_problems import ProblemSubmissionTestMixin -from django.conf import settings -from lms.djangoapps.course_blocks.api import get_course_blocks -from lms.djangoapps.grades.config.tests.utils import persistent_grades_feature_flags -from mock import patch -from openedx.core.djangolib.testing.utils import get_mock_request -from student.models import CourseEnrollment -from student.tests.factories import UserFactory -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory -from xmodule.modulestore.tests.utils import TEST_DATA_DIR -from xmodule.modulestore.xml_importer import import_course_from_xml - -from ..config.waffle import ASSUME_ZERO_GRADE_IF_ABSENT, waffle -from ..course_data import CourseData -from ..course_grade import CourseGrade, ZeroCourseGrade -from ..course_grade_factory import CourseGradeFactory -from ..models import PersistentSubsectionGrade -from ..subsection_grade import SubsectionGrade, ZeroSubsectionGrade -from ..subsection_grade_factory import SubsectionGradeFactory -from .utils import mock_get_score, mock_get_submissions_score - - -class GradeTestBase(SharedModuleStoreTestCase): - """ - Base class for Course- and SubsectionGradeFactory tests. - """ - @classmethod - def setUpClass(cls): - super(GradeTestBase, cls).setUpClass() - cls.course = CourseFactory.create() - with cls.store.bulk_operations(cls.course.id): - cls.chapter = ItemFactory.create( - parent=cls.course, - category="chapter", - display_name="Test Chapter" - ) - cls.sequence = ItemFactory.create( - parent=cls.chapter, - category='sequential', - display_name="Test Sequential 1", - graded=True, - format="Homework" - ) - cls.vertical = ItemFactory.create( - parent=cls.sequence, - category='vertical', - display_name='Test Vertical 1' - ) - problem_xml = MultipleChoiceResponseXMLFactory().build_xml( - question_text='The correct answer is Choice 3', - choices=[False, False, True, False], - choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3'] - ) - cls.problem = ItemFactory.create( - parent=cls.vertical, - category="problem", - display_name="Test Problem", - data=problem_xml - ) - cls.sequence2 = ItemFactory.create( - parent=cls.chapter, - category='sequential', - display_name="Test Sequential 2", - graded=True, - format="Homework" - ) - cls.problem2 = ItemFactory.create( - parent=cls.sequence2, - category="problem", - display_name="Test Problem", - data=problem_xml - ) - # AED 2017-06-19: make cls.sequence belong to multiple parents, - # so we can test that DAGs with this shape are handled correctly. - cls.chapter_2 = ItemFactory.create( - parent=cls.course, - category='chapter', - display_name='Test Chapter 2' - ) - cls.chapter_2.children.append(cls.sequence.location) - cls.store.update_item(cls.chapter_2, UserFactory().id) - - def setUp(self): - super(GradeTestBase, self).setUp() - self.request = get_mock_request(UserFactory()) - self.client.login(username=self.request.user.username, password="test") - self._set_grading_policy() - 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) - - def _set_grading_policy(self, passing=0.5): - """ - Updates the course's grading policy. - """ - self.grading_policy = { - "GRADER": [ - { - "type": "Homework", - "min_count": 1, - "drop_count": 0, - "short_label": "HW", - "weight": 1.0, - }, - ], - "GRADE_CUTOFFS": { - "Pass": passing, - }, - } - self.course.set_grading_policy(self.grading_policy) - self.store.update_item(self.course, 0) - - -@ddt.ddt -class TestCourseGradeFactory(GradeTestBase): - """ - Test that CourseGrades are calculated properly - """ - def _assert_zero_grade(self, course_grade, expected_grade_class): - """ - Asserts whether the given course_grade is as expected with - zero values. - """ - self.assertIsInstance(course_grade, expected_grade_class) - self.assertIsNone(course_grade.letter_grade) - self.assertEqual(course_grade.percent, 0.0) - self.assertIsNotNone(course_grade.chapter_grades) - - def test_course_grade_no_access(self): - """ - Test to ensure a grade can ba calculated for a student in a course, even if they themselves do not have access. - """ - invisible_course = CourseFactory.create(visible_to_staff_only=True) - access = has_access(self.request.user, 'load', invisible_course) - self.assertEqual(access.has_access, False) - self.assertEqual(access.error_code, 'not_visible_to_user') - - # with self.assertNoExceptionRaised: <- this isn't a real method, it's an implicit assumption - grade = CourseGradeFactory().read(self.request.user, invisible_course) - self.assertEqual(grade.percent, 0) - - @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) - @ddt.data( - (True, True), - (True, False), - (False, True), - (False, False), - ) - @ddt.unpack - def test_course_grade_feature_gating(self, feature_flag, course_setting): - # Grades are only saved if the feature flag and the advanced setting are - # both set to True. - grade_factory = CourseGradeFactory() - with persistent_grades_feature_flags( - global_flag=feature_flag, - enabled_for_all_courses=False, - course_id=self.course.id, - enabled_for_course=course_setting - ): - with patch('lms.djangoapps.grades.models.PersistentCourseGrade.read') as mock_read_grade: - grade_factory.read(self.request.user, self.course) - self.assertEqual(mock_read_grade.called, feature_flag and course_setting) - - def test_read(self): - grade_factory = CourseGradeFactory() - - def _assert_read(expected_pass, expected_percent): - """ - Creates the grade, ensuring it is as expected. - """ - course_grade = grade_factory.read(self.request.user, self.course) - self.assertEqual(course_grade.letter_grade, u'Pass' if expected_pass else None) - self.assertEqual(course_grade.percent, expected_percent) - - with self.assertNumQueries(1), mock_get_score(1, 2): - _assert_read(expected_pass=False, expected_percent=0) - - with self.assertNumQueries(37), mock_get_score(1, 2): - grade_factory.update(self.request.user, self.course, force_update_subsections=True) - - with self.assertNumQueries(1): - _assert_read(expected_pass=True, expected_percent=0.5) - - @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) - @ddt.data(*itertools.product((True, False), (True, False))) - @ddt.unpack - def test_read_zero(self, assume_zero_enabled, create_if_needed): - with waffle().override(ASSUME_ZERO_GRADE_IF_ABSENT, active=assume_zero_enabled): - grade_factory = CourseGradeFactory() - course_grade = grade_factory.read(self.request.user, self.course, create_if_needed=create_if_needed) - if create_if_needed or assume_zero_enabled: - self._assert_zero_grade(course_grade, ZeroCourseGrade if assume_zero_enabled else CourseGrade) - else: - self.assertIsNone(course_grade) - - def test_create_zero_subs_grade_for_nonzero_course_grade(self): - subsection = self.course_structure[self.sequence.location] - with mock_get_score(1, 2): - self.subsection_grade_factory.update(subsection) - course_grade = CourseGradeFactory().update(self.request.user, self.course) - subsection1_grade = course_grade.subsection_grades[self.sequence.location] - subsection2_grade = course_grade.subsection_grades[self.sequence2.location] - self.assertIsInstance(subsection1_grade, SubsectionGrade) - self.assertIsInstance(subsection2_grade, ZeroSubsectionGrade) - - @ddt.data(True, False) - def test_iter_force_update(self, force_update): - with patch('lms.djangoapps.grades.subsection_grade_factory.SubsectionGradeFactory.update') as mock_update: - set(CourseGradeFactory().iter( - users=[self.request.user], course=self.course, force_update=force_update - )) - - self.assertEqual(mock_update.called, force_update) - - def test_course_grade_summary(self): - with mock_get_score(1, 2): - self.subsection_grade_factory.update(self.course_structure[self.sequence.location]) - course_grade = CourseGradeFactory().update(self.request.user, self.course) - - actual_summary = course_grade.summary - - # We should have had a zero subsection grade for sequential 2, since we never - # gave it a mock score above. - expected_summary = { - 'grade': None, - 'grade_breakdown': { - 'Homework': { - 'category': 'Homework', - 'percent': 0.25, - 'detail': 'Homework = 25.00% of a possible 100.00%', - } - }, - 'percent': 0.25, - 'section_breakdown': [ - { - 'category': 'Homework', - 'detail': u'Homework 1 - Test Sequential 1 - 50% (1/2)', - 'label': u'HW 01', - 'percent': 0.5 - }, - { - 'category': 'Homework', - 'detail': u'Homework 2 - Test Sequential 2 - 0% (0/1)', - 'label': u'HW 02', - 'percent': 0.0 - }, - { - 'category': 'Homework', - 'detail': u'Homework Average = 25%', - 'label': u'HW Avg', - 'percent': 0.25, - 'prominent': True - }, - ] - } - self.assertEqual(expected_summary, actual_summary) - - -@ddt.ddt -class TestSubsectionGradeFactory(ProblemSubmissionTestMixin, GradeTestBase): - """ - Tests for SubsectionGradeFactory functionality. - - Ensures that SubsectionGrades are created and updated properly, that - persistent grades are functioning as expected, and that the flag to - enable saving subsection grades blocks/enables that feature as expected. - """ - - def assert_grade(self, grade, expected_earned, expected_possible): - """ - Asserts that the given grade object has the expected score. - """ - self.assertEqual( - (grade.all_total.earned, grade.all_total.possible), - (expected_earned, expected_possible), - ) - - def test_create_zero(self): - """ - Test that a zero grade is returned. - """ - grade = self.subsection_grade_factory.create(self.sequence) - self.assertIsInstance(grade, ZeroSubsectionGrade) - self.assert_grade(grade, 0.0, 1.0) - - def test_update(self): - """ - Assuming the underlying score reporting methods work, - test that the score is calculated properly. - """ - with mock_get_score(1, 2): - grade = self.subsection_grade_factory.update(self.sequence) - self.assert_grade(grade, 1, 2) - - def test_write_only_if_engaged(self): - """ - Test that scores are not persisted when a learner has - never attempted a problem, but are persisted if the - learner's state has been deleted. - """ - with mock_get_score(0, 0, None): - self.subsection_grade_factory.update(self.sequence) - # ensure no grades have been persisted - self.assertEqual(0, len(PersistentSubsectionGrade.objects.all())) - - with mock_get_score(0, 0, None): - self.subsection_grade_factory.update(self.sequence, score_deleted=True) - # ensure a grade has been persisted - self.assertEqual(1, len(PersistentSubsectionGrade.objects.all())) - - def test_update_if_higher(self): - def verify_update_if_higher(mock_score, expected_grade): - """ - Updates the subsection grade and verifies the - resulting grade is as expected. - """ - with mock_get_score(*mock_score): - grade = self.subsection_grade_factory.update(self.sequence, only_if_higher=True) - self.assert_grade(grade, *expected_grade) - - verify_update_if_higher((1, 2), (1, 2)) # previous value was non-existent - verify_update_if_higher((2, 4), (2, 4)) # previous value was equivalent - verify_update_if_higher((1, 4), (2, 4)) # previous value was greater - verify_update_if_higher((3, 4), (3, 4)) # previous value was less - - @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) - @ddt.data( - (True, True), - (True, False), - (False, True), - (False, False), - ) - @ddt.unpack - def test_subsection_grade_feature_gating(self, feature_flag, course_setting): - # Grades are only saved if the feature flag and the advanced setting are - # both set to True. - with patch( - 'lms.djangoapps.grades.models.PersistentSubsectionGrade.bulk_read_grades' - ) as mock_read_saved_grade: - with persistent_grades_feature_flags( - global_flag=feature_flag, - enabled_for_all_courses=False, - course_id=self.course.id, - enabled_for_course=course_setting - ): - self.subsection_grade_factory.create(self.sequence) - self.assertEqual(mock_read_saved_grade.called, feature_flag and course_setting) - - -@patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) -@ddt.ddt -class ZeroGradeTest(GradeTestBase): - """ - Tests ZeroCourseGrade (and, implicitly, ZeroSubsectionGrade) - functionality. - """ - @ddt.data(True, False) - def test_zero(self, assume_zero_enabled): - """ - Creates a ZeroCourseGrade and ensures it's empty. - """ - with waffle().override(ASSUME_ZERO_GRADE_IF_ABSENT, active=assume_zero_enabled): - course_data = CourseData(self.request.user, structure=self.course_structure) - chapter_grades = ZeroCourseGrade(self.request.user, course_data).chapter_grades - for chapter in chapter_grades: - for section in chapter_grades[chapter]['sections']: - for score in section.problem_scores.itervalues(): - self.assertEqual(score.earned, 0) - self.assertEqual(score.first_attempted, None) - self.assertEqual(section.all_total.earned, 0) - - @ddt.data(True, False) - def test_zero_null_scores(self, assume_zero_enabled): - """ - Creates a zero course grade and ensures that null scores aren't included in the section problem scores. - """ - with waffle().override(ASSUME_ZERO_GRADE_IF_ABSENT, active=assume_zero_enabled): - with patch('lms.djangoapps.grades.subsection_grade.get_score', return_value=None): - course_data = CourseData(self.request.user, structure=self.course_structure) - chapter_grades = ZeroCourseGrade(self.request.user, course_data).chapter_grades - for chapter in chapter_grades: - self.assertNotEqual({}, chapter_grades[chapter]['sections']) - for section in chapter_grades[chapter]['sections']: - self.assertEqual({}, section.problem_scores) - - -class SubsectionGradeTest(GradeTestBase): - """ - Tests SubsectionGrade functionality. - """ - - def test_save_and_load(self): - """ - Test that grades are persisted to the database properly, - and that loading saved grades returns the same data. - """ - with mock_get_score(1, 2): - # Create a grade that *isn't* saved to the database - input_grade = SubsectionGrade(self.sequence) - input_grade.init_from_structure( - self.request.user, - self.course_structure, - self.subsection_grade_factory._submissions_scores, - self.subsection_grade_factory._csm_scores, - ) - self.assertEqual(PersistentSubsectionGrade.objects.count(), 0) - - # save to db, and verify object is in database - 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) - saved_model = PersistentSubsectionGrade.read_grade( - user_id=self.request.user.id, - usage_key=self.sequence.location, - ) - loaded_grade.init_from_model( - self.request.user, - saved_model, - self.course_structure, - self.subsection_grade_factory._submissions_scores, - self.subsection_grade_factory._csm_scores, - ) - - self.assertEqual(input_grade.url_name, loaded_grade.url_name) - loaded_grade.all_total.first_attempted = input_grade.all_total.first_attempted = None - self.assertEqual(input_grade.all_total, loaded_grade.all_total) - - -@ddt.ddt -class TestMultipleProblemTypesSubsectionScores(SharedModuleStoreTestCase): - """ - Test grading of different problem types. - """ - - SCORED_BLOCK_COUNT = 7 - ACTUAL_TOTAL_POSSIBLE = 17.0 - - @classmethod - def setUpClass(cls): - super(TestMultipleProblemTypesSubsectionScores, cls).setUpClass() - cls.load_scoreable_course() - chapter1 = cls.course.get_children()[0] - cls.seq1 = chapter1.get_children()[0] - - def setUp(self): - super(TestMultipleProblemTypesSubsectionScores, self).setUp() - password = u'test' - self.student = UserFactory.create(is_staff=False, username=u'test_student', password=password) - self.client.login(username=self.student.username, password=password) - self.request = get_mock_request(self.student) - self.course_structure = get_course_blocks(self.student, self.course.location) - - @classmethod - def load_scoreable_course(cls): - """ - This test course lives at `common/test/data/scoreable`. - - For details on the contents and structure of the file, see - `common/test/data/scoreable/README`. - """ - - course_items = import_course_from_xml( - cls.store, - 'test_user', - TEST_DATA_DIR, - source_dirs=['scoreable'], - static_content_store=None, - target_id=cls.store.make_course_key('edX', 'scoreable', '3000'), - raise_on_failure=True, - create_if_not_present=True, - ) - - cls.course = course_items[0] - - def test_score_submission_for_all_problems(self): - subsection_factory = SubsectionGradeFactory( - self.student, - course_structure=self.course_structure, - course=self.course, - ) - score = subsection_factory.create(self.seq1) - - self.assertEqual(score.all_total.earned, 0.0) - self.assertEqual(score.all_total.possible, self.ACTUAL_TOTAL_POSSIBLE) - - # Choose arbitrary, non-default values for earned and possible. - earned_per_block = 3.0 - possible_per_block = 7.0 - with mock_get_submissions_score(earned_per_block, possible_per_block) as mock_score: - # Configure one block to return no possible score, the rest to return 3.0 earned / 7.0 possible - block_count = self.SCORED_BLOCK_COUNT - 1 - mock_score.side_effect = itertools.chain( - [(earned_per_block, None, earned_per_block, None, datetime.datetime(2000, 1, 1))], - itertools.repeat(mock_score.return_value) - ) - score = subsection_factory.update(self.seq1) - self.assertEqual(score.all_total.earned, earned_per_block * block_count) - self.assertEqual(score.all_total.possible, possible_per_block * block_count) - - -@ddt.ddt -class TestVariedMetadata(ProblemSubmissionTestMixin, ModuleStoreTestCase): - """ - Test that changing the metadata on a block has the desired effect on the - persisted score. - """ - default_problem_metadata = { - u'graded': True, - u'weight': 2.5, - u'due': datetime.datetime(2099, 3, 15, 12, 30, 0, tzinfo=pytz.utc), - } - - def setUp(self): - super(TestVariedMetadata, self).setUp() - self.course = CourseFactory.create() - with self.store.bulk_operations(self.course.id): - self.chapter = ItemFactory.create( - parent=self.course, - category="chapter", - display_name="Test Chapter" - ) - self.sequence = ItemFactory.create( - parent=self.chapter, - category='sequential', - display_name="Test Sequential 1", - graded=True - ) - self.vertical = ItemFactory.create( - parent=self.sequence, - category='vertical', - display_name='Test Vertical 1' - ) - self.problem_xml = u''' - - - - - - - ''' - self.request = get_mock_request(UserFactory()) - self.client.login(username=self.request.user.username, password="test") - CourseEnrollment.enroll(self.request.user, self.course.id) - - def _get_altered_metadata(self, alterations): - """ - Returns a copy of the default_problem_metadata dict updated with the - specified alterations. - """ - metadata = self.default_problem_metadata.copy() - metadata.update(alterations) - return metadata - - def _add_problem_with_alterations(self, alterations): - """ - Add a problem to the course with the specified metadata alterations. - """ - - metadata = self._get_altered_metadata(alterations) - ItemFactory.create( - parent=self.vertical, - category="problem", - display_name="problem", - data=self.problem_xml, - metadata=metadata, - ) - - def _get_score(self): - """ - Return the score of the test problem when one correct problem (out of - two) is submitted. - """ - - self.submit_question_answer(u'problem', {u'2_1': u'Correct'}) - course_structure = get_course_blocks(self.request.user, self.course.location) - subsection_factory = SubsectionGradeFactory( - self.request.user, - course_structure=course_structure, - course=self.course, - ) - return subsection_factory.create(self.sequence) - - @ddt.data( - ({}, 1.25, 2.5), - ({u'weight': 27}, 13.5, 27), - ({u'weight': 1.0}, 0.5, 1.0), - ({u'weight': 0.0}, 0.0, 0.0), - ({u'weight': None}, 1.0, 2.0), - ) - @ddt.unpack - def test_weight_metadata_alterations(self, alterations, expected_earned, expected_possible): - self._add_problem_with_alterations(alterations) - score = self._get_score() - self.assertEqual(score.all_total.earned, expected_earned) - self.assertEqual(score.all_total.possible, expected_possible) - - @ddt.data( - ({u'graded': True}, 1.25, 2.5), - ({u'graded': False}, 0.0, 0.0), - ) - @ddt.unpack - def test_graded_metadata_alterations(self, alterations, expected_earned, expected_possible): - self._add_problem_with_alterations(alterations) - score = self._get_score() - self.assertEqual(score.graded_total.earned, expected_earned) - self.assertEqual(score.graded_total.possible, expected_possible) - - -class TestCourseGradeLogging(ProblemSubmissionTestMixin, SharedModuleStoreTestCase): - """ - Tests logging in the course grades module. - Uses a larger course structure than other - unit tests. - """ - def setUp(self): - super(TestCourseGradeLogging, self).setUp() - self.course = CourseFactory.create() - with self.store.bulk_operations(self.course.id): - self.chapter = ItemFactory.create( - parent=self.course, - category="chapter", - display_name="Test Chapter" - ) - self.sequence = ItemFactory.create( - parent=self.chapter, - category='sequential', - display_name="Test Sequential 1", - graded=True - ) - self.sequence_2 = ItemFactory.create( - parent=self.chapter, - category='sequential', - display_name="Test Sequential 2", - graded=True - ) - self.sequence_3 = ItemFactory.create( - parent=self.chapter, - category='sequential', - display_name="Test Sequential 3", - graded=False - ) - self.vertical = ItemFactory.create( - parent=self.sequence, - category='vertical', - display_name='Test Vertical 1' - ) - self.vertical_2 = ItemFactory.create( - parent=self.sequence_2, - category='vertical', - display_name='Test Vertical 2' - ) - self.vertical_3 = ItemFactory.create( - parent=self.sequence_3, - category='vertical', - display_name='Test Vertical 3' - ) - problem_xml = MultipleChoiceResponseXMLFactory().build_xml( - question_text='The correct answer is Choice 2', - choices=[False, False, True, False], - choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3'] - ) - self.problem = ItemFactory.create( - parent=self.vertical, - category="problem", - display_name="test_problem_1", - data=problem_xml - ) - self.problem_2 = ItemFactory.create( - parent=self.vertical_2, - category="problem", - display_name="test_problem_2", - data=problem_xml - ) - self.problem_3 = ItemFactory.create( - parent=self.vertical_3, - category="problem", - display_name="test_problem_3", - data=problem_xml - ) - self.request = get_mock_request(UserFactory()) - self.client.login(username=self.request.user.username, password="test") - 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) diff --git a/lms/djangoapps/grades/tests/test_services.py b/lms/djangoapps/grades/tests/test_services.py index 98262a88df..1a6cac7fa0 100644 --- a/lms/djangoapps/grades/tests/test_services.py +++ b/lms/djangoapps/grades/tests/test_services.py @@ -4,7 +4,6 @@ from datetime import datetime from freezegun import freeze_time from lms.djangoapps.grades.models import PersistentSubsectionGrade, PersistentSubsectionGradeOverride from lms.djangoapps.grades.services import GradesService, _get_key -from lms.djangoapps.grades.signals.handlers import SUBSECTION_OVERRIDE_EVENT_TYPE from mock import patch, call from opaque_keys.edx.keys import CourseKey, UsageKey from student.tests.factories import UserFactory diff --git a/lms/djangoapps/grades/tests/test_signals.py b/lms/djangoapps/grades/tests/test_signals.py index 6d679dc04b..3b109ae483 100644 --- a/lms/djangoapps/grades/tests/test_signals.py +++ b/lms/djangoapps/grades/tests/test_signals.py @@ -1,7 +1,6 @@ """ Tests for the score change signals defined in the courseware models module. """ -import itertools import re from datetime import datetime @@ -10,10 +9,6 @@ import pytz from django.test import TestCase from mock import MagicMock, patch -from opaque_keys.edx.locations import CourseLocator -from openedx.core.djangoapps.course_groups.signals.signals import COHORT_MEMBERSHIP_UPDATED -from student.signals.signals import ENROLLMENT_TRACK_UPDATED -from student.tests.factories import UserFactory from submissions.models import score_reset, score_set from util.date_utils import to_timestamp diff --git a/lms/djangoapps/grades/tests/test_subsection_grade.py b/lms/djangoapps/grades/tests/test_subsection_grade.py new file mode 100644 index 0000000000..a3187add45 --- /dev/null +++ b/lms/djangoapps/grades/tests/test_subsection_grade.py @@ -0,0 +1,38 @@ +from ..models import PersistentSubsectionGrade +from ..subsection_grade import SubsectionGrade +from .utils import mock_get_score +from .base import GradeTestBase + + +class SubsectionGradeTest(GradeTestBase): + def test_create_and_read(self): + with mock_get_score(1, 2): + # Create a grade that *isn't* saved to the database + created_grade = SubsectionGrade.create( + self.sequence, + self.course_structure, + self.subsection_grade_factory._submissions_scores, + self.subsection_grade_factory._csm_scores, + ) + self.assertEqual(PersistentSubsectionGrade.objects.count(), 0) + + # save to db, and verify object is in database + created_grade.update_or_create_model(self.request.user) + self.assertEqual(PersistentSubsectionGrade.objects.count(), 1) + + # read from db, and ensure output matches input + saved_model = PersistentSubsectionGrade.read_grade( + user_id=self.request.user.id, + usage_key=self.sequence.location, + ) + read_grade = SubsectionGrade.read( + self.sequence, + saved_model, + self.course_structure, + self.subsection_grade_factory._submissions_scores, + self.subsection_grade_factory._csm_scores, + ) + + self.assertEqual(created_grade.url_name, read_grade.url_name) + read_grade.all_total.first_attempted = created_grade.all_total.first_attempted = None + self.assertEqual(created_grade.all_total, read_grade.all_total) diff --git a/lms/djangoapps/grades/tests/test_subsection_grade_factory.py b/lms/djangoapps/grades/tests/test_subsection_grade_factory.py new file mode 100644 index 0000000000..eefef437ed --- /dev/null +++ b/lms/djangoapps/grades/tests/test_subsection_grade_factory.py @@ -0,0 +1,101 @@ +import ddt +from courseware.tests.test_submitting_problems import ProblemSubmissionTestMixin +from django.conf import settings +from lms.djangoapps.grades.config.tests.utils import persistent_grades_feature_flags +from mock import patch + +from ..models import PersistentSubsectionGrade +from ..subsection_grade_factory import ZeroSubsectionGrade +from .base import GradeTestBase +from .utils import mock_get_score + + +@ddt.ddt +class TestSubsectionGradeFactory(ProblemSubmissionTestMixin, GradeTestBase): + """ + Tests for SubsectionGradeFactory functionality. + + Ensures that SubsectionGrades are created and updated properly, that + persistent grades are functioning as expected, and that the flag to + enable saving subsection grades blocks/enables that feature as expected. + """ + + def assert_grade(self, grade, expected_earned, expected_possible): + """ + Asserts that the given grade object has the expected score. + """ + self.assertEqual( + (grade.all_total.earned, grade.all_total.possible), + (expected_earned, expected_possible), + ) + + def test_create_zero(self): + """ + Test that a zero grade is returned. + """ + grade = self.subsection_grade_factory.create(self.sequence) + self.assertIsInstance(grade, ZeroSubsectionGrade) + self.assert_grade(grade, 0.0, 1.0) + + def test_update(self): + """ + Assuming the underlying score reporting methods work, + test that the score is calculated properly. + """ + with mock_get_score(1, 2): + grade = self.subsection_grade_factory.update(self.sequence) + self.assert_grade(grade, 1, 2) + + def test_write_only_if_engaged(self): + """ + Test that scores are not persisted when a learner has + never attempted a problem, but are persisted if the + learner's state has been deleted. + """ + with mock_get_score(0, 0, None): + self.subsection_grade_factory.update(self.sequence) + # ensure no grades have been persisted + self.assertEqual(0, len(PersistentSubsectionGrade.objects.all())) + + with mock_get_score(0, 0, None): + self.subsection_grade_factory.update(self.sequence, score_deleted=True) + # ensure a grade has been persisted + self.assertEqual(1, len(PersistentSubsectionGrade.objects.all())) + + def test_update_if_higher(self): + def verify_update_if_higher(mock_score, expected_grade): + """ + Updates the subsection grade and verifies the + resulting grade is as expected. + """ + with mock_get_score(*mock_score): + grade = self.subsection_grade_factory.update(self.sequence, only_if_higher=True) + self.assert_grade(grade, *expected_grade) + + verify_update_if_higher((1, 2), (1, 2)) # previous value was non-existent + verify_update_if_higher((2, 4), (2, 4)) # previous value was equivalent + verify_update_if_higher((1, 4), (2, 4)) # previous value was greater + verify_update_if_higher((3, 4), (3, 4)) # previous value was less + + @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) + @ddt.data( + (True, True), + (True, False), + (False, True), + (False, False), + ) + @ddt.unpack + def test_subsection_grade_feature_gating(self, feature_flag, course_setting): + # Grades are only saved if the feature flag and the advanced setting are + # both set to True. + with patch( + 'lms.djangoapps.grades.models.PersistentSubsectionGrade.bulk_read_grades' + ) as mock_read_saved_grade: + with persistent_grades_feature_flags( + global_flag=feature_flag, + enabled_for_all_courses=False, + course_id=self.course.id, + enabled_for_course=course_setting + ): + self.subsection_grade_factory.create(self.sequence) + self.assertEqual(mock_read_saved_grade.called, feature_flag and course_setting) diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 0848d5c15b..1cf7085f53 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -164,10 +164,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEquals(mock_block_structure_create.call_count, 1) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 27, True), - (ModuleStoreEnum.Type.mongo, 1, 27, False), - (ModuleStoreEnum.Type.split, 3, 27, True), - (ModuleStoreEnum.Type.split, 3, 27, False), + (ModuleStoreEnum.Type.mongo, 1, 25, True), + (ModuleStoreEnum.Type.mongo, 1, 25, False), + (ModuleStoreEnum.Type.split, 3, 25, True), + (ModuleStoreEnum.Type.split, 3, 25, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -179,8 +179,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 27), - (ModuleStoreEnum.Type.split, 3, 27), + (ModuleStoreEnum.Type.mongo, 1, 25), + (ModuleStoreEnum.Type.split, 3, 25), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -240,8 +240,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 28), - (ModuleStoreEnum.Type.split, 3, 28), + (ModuleStoreEnum.Type.mongo, 1, 26), + (ModuleStoreEnum.Type.split, 3, 26), ) @ddt.unpack def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index 653d74884f..2a13ad5495 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -20,7 +20,8 @@ from courseware.models import StudentModule from edxmako.shortcuts import render_to_string from eventtracking import tracker from lms.djangoapps.grades.constants import ScoreDatabaseTableEnum -from lms.djangoapps.grades.signals.handlers import disconnect_submissions_signal_receiver, STATE_DELETED_EVENT_TYPE +from lms.djangoapps.grades.events import STATE_DELETED_EVENT_TYPE +from lms.djangoapps.grades.signals.handlers import disconnect_submissions_signal_receiver from lms.djangoapps.grades.signals.signals import PROBLEM_RAW_SCORE_CHANGED from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers diff --git a/lms/djangoapps/instructor_task/tasks_helper/module_state.py b/lms/djangoapps/instructor_task/tasks_helper/module_state.py index 175943c5ea..4cf7faebd8 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/module_state.py +++ b/lms/djangoapps/instructor_task/tasks_helper/module_state.py @@ -7,7 +7,6 @@ from time import time from django.contrib.auth.models import User from opaque_keys.edx.keys import UsageKey -from xblock.runtime import KvsFieldData import dogstats_wrapper as dog_stats_api from capa.responsetypes import LoncapaProblemError, ResponseError, StudentInputError @@ -15,16 +14,13 @@ from courseware.courses import get_course_by_id, get_problems_in_section from courseware.model_data import DjangoKeyValueStore, FieldDataCache from courseware.models import StudentModule from courseware.module_render import get_module_for_descriptor_internal -from eventtracking import tracker -from lms.djangoapps.grades.scores import weighted_score -from track.contexts import course_context_from_course_id +from lms.djangoapps.grades.events import GRADES_OVERRIDE_EVENT_TYPE, GRADES_RESCORE_EVENT_TYPE from track.event_transaction_utils import create_new_event_transaction_id, set_event_transaction_type from track.views import task_track from util.db import outer_atomic -from xmodule.modulestore.django import modulestore from xblock.runtime import KvsFieldData -from xblock.scorable import Score, ScorableXBlockMixin +from xblock.scorable import Score from xmodule.modulestore.django import modulestore from ..exceptions import UpdateProblemModuleStateError from .runner import TaskProgress @@ -32,10 +28,6 @@ from .utils import UNKNOWN_TASK_ID, UPDATE_STATUS_FAILED, UPDATE_STATUS_SKIPPED, TASK_LOG = logging.getLogger('edx.celery.task') -# define value to be used in grading events -GRADES_RESCORE_EVENT_TYPE = 'edx.grades.problem.rescored' -GRADES_OVERRIDE_EVENT_TYPE = 'edx.grades.problem.score_overridden' - def perform_module_state_update(update_fcn, filter_fcn, _entry_id, course_id, task_input, action_name): """