From e00fe03d9d83b7d57845942e3e82ba1c323a2a11 Mon Sep 17 00:00:00 2001 From: Eric Fischer Date: Fri, 30 Sep 2016 10:05:44 -0400 Subject: [PATCH] Revert "Merge pull request #13539 from edx/sstudent/TNL-5601" This reverts commit e3db08f1946e34c128c4fa9d56c180ca9bbca4c1, reversing changes made to 51c2444de30afaa9061fcff7a6dce1ddb9ed99df. --- lms/djangoapps/grades/scores.py | 38 ++++----- lms/djangoapps/grades/tests/test_scores.py | 10 +-- lms/djangoapps/instructor/enrollment.py | 47 +---------- .../instructor/tests/test_enrollment.py | 79 ------------------- 4 files changed, 26 insertions(+), 148 deletions(-) diff --git a/lms/djangoapps/grades/scores.py b/lms/djangoapps/grades/scores.py index 86832ef156..27abc562cf 100644 --- a/lms/djangoapps/grades/scores.py +++ b/lms/djangoapps/grades/scores.py @@ -124,23 +124,6 @@ def get_score(submissions_scores, csm_scores, persisted_block, block): ) -def weighted_score(raw_earned, raw_possible, weight): - """ - Returns a tuple that represents the weighted (earned, possible) score. - If weight is None or raw_possible is 0, returns the original values. - - When weight is used, it defines the weighted_possible. This allows - course authors to specify the exact maximum value for a problem when - they provide a weight. - """ - assert raw_possible is not None - cannot_compute_with_weight = weight is None or raw_possible == 0 - if cannot_compute_with_weight: - return raw_earned, raw_possible - else: - return float(raw_earned) * weight / raw_possible, float(weight) - - def _get_score_from_submissions(submissions_scores, block): """ Returns the score values from the submissions API if found. @@ -174,7 +157,7 @@ def _get_score_from_csm(csm_scores, block, weight): if has_valid_score: raw_earned = score.correct if score.correct is not None else 0.0 raw_possible = score.total - return (raw_earned, raw_possible) + weighted_score(raw_earned, raw_possible, weight) + return (raw_earned, raw_possible) + _weighted_score(raw_earned, raw_possible, weight) def _get_score_from_persisted_or_latest_block(persisted_block, block, weight): @@ -191,7 +174,7 @@ def _get_score_from_persisted_or_latest_block(persisted_block, block, weight): else: raw_possible = block.transformer_data[GradesTransformer].max_score - return (raw_earned, raw_possible) + weighted_score(raw_earned, raw_possible, weight) + return (raw_earned, raw_possible) + _weighted_score(raw_earned, raw_possible, weight) def _get_weight_from_block(persisted_block, block): @@ -233,6 +216,23 @@ def _get_explicit_graded(block): return True if field_value is None else field_value +def _weighted_score(raw_earned, raw_possible, weight): + """ + Returns a tuple that represents the weighted (earned, possible) score. + If weight is None or raw_possible is 0, returns the original values. + + When weight is used, it defines the weighted_possible. This allows + course authors to specify the exact maximum value for a problem when + they provide a weight. + """ + assert raw_possible is not None + cannot_compute_with_weight = weight is None or raw_possible == 0 + if cannot_compute_with_weight: + return raw_earned, raw_possible + else: + return float(raw_earned) * weight / raw_possible, float(weight) + + @memoized def _block_types_possibly_scored(): """ diff --git a/lms/djangoapps/grades/tests/test_scores.py b/lms/djangoapps/grades/tests/test_scores.py index f7d6e34055..a4002fbef2 100644 --- a/lms/djangoapps/grades/tests/test_scores.py +++ b/lms/djangoapps/grades/tests/test_scores.py @@ -162,9 +162,9 @@ class TestGetScore(TestCase): @ddt.ddt -class TestWeightedScore(TestCase): +class TestInternalWeightedScore(TestCase): """ - Tests the helper method: weighted_score + Tests the internal helper method: _weighted_score """ @ddt.data( (0, 0, 1), @@ -177,7 +177,7 @@ class TestWeightedScore(TestCase): @ddt.unpack def test_cannot_compute(self, raw_earned, raw_possible, weight): self.assertEquals( - scores.weighted_score(raw_earned, raw_possible, weight), + scores._weighted_score(raw_earned, raw_possible, weight), (raw_earned, raw_possible), ) @@ -192,13 +192,13 @@ class TestWeightedScore(TestCase): @ddt.unpack def test_computed(self, raw_earned, raw_possible, weight, expected_score): self.assertEquals( - scores.weighted_score(raw_earned, raw_possible, weight), + scores._weighted_score(raw_earned, raw_possible, weight), expected_score, ) def test_assert_on_invalid_r_possible(self): with self.assertRaises(AssertionError): - scores.weighted_score(raw_earned=1, raw_possible=None, weight=1) + scores._weighted_score(raw_earned=1, raw_possible=None, weight=1) @ddt.ddt diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index becae8f2ba..3540593483 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -4,7 +4,6 @@ Enrollment operations for use by instructor APIs. Does not include any access control, be sure to check access before calling. """ -import crum import json import logging from django.contrib.auth.models import User @@ -14,14 +13,11 @@ from django.core.mail import send_mail from django.utils.translation import override as override_language from course_modes.models import CourseMode -from courseware.model_data import FieldDataCache -from courseware.module_render import get_module_for_descriptor +from student.models import CourseEnrollment, CourseEnrollmentAllowed from courseware.models import StudentModule from edxmako.shortcuts import render_to_string -from grades.scores import weighted_score -from grades.signals.signals import SCORE_CHANGED from lang_pref import LANGUAGE_KEY -from student.models import CourseEnrollment, CourseEnrollmentAllowed + from submissions import api as sub_api # installed from the edx-submissions repository from student.models import anonymous_id_for_user from openedx.core.djangoapps.user_api.models import UserPreference @@ -249,7 +245,6 @@ def reset_student_attempts(course_id, student, module_state_key, requesting_user ) submission_cleared = True except ItemNotFoundError: - block = None log.warning("Could not find %s in modulestore when attempting to reset attempts.", module_state_key) # Reset the student's score in the submissions API, if xblock.clear_student_state has not done so already. @@ -272,7 +267,6 @@ def reset_student_attempts(course_id, student, module_state_key, requesting_user if delete_module: module_to_reset.delete() - _fire_score_changed_for_block(course_id, student, block, module_state_key) else: _reset_module_attempts(module_to_reset) @@ -293,43 +287,6 @@ def _reset_module_attempts(studentmodule): studentmodule.save() -def _fire_score_changed_for_block(course_id, student, block, module_state_key): - """ - Fires a SCORE_CHANGED event for the given module. The earned points are - always zero. We must retrieve the possible points from the XModule, as - noted below. - """ - if block and block.has_score: - cache = FieldDataCache.cache_for_descriptor_descendents( - course_id=course_id, - user=student, - descriptor=block, - depth=0 - ) - # For implementation reasons, we need to pull the max_score from the XModule, - # even though the data is not user-specific. Here we bind the data to the - # current user. - request = crum.get_current_request() - module = get_module_for_descriptor( - user=student, - request=request, - descriptor=block, - field_data_cache=cache, - course_key=course_id - ) - points_earned, points_possible = weighted_score(0, module.max_score(), getattr(module, 'weight', None)) - else: - points_earned, points_possible = 0, 0 - SCORE_CHANGED.send( - sender=None, - points_possible=points_possible, - points_earned=points_earned, - user=student, - course_id=course_id, - usage_id=module_state_key - ) - - def get_email_params(course, auto_enroll, secure=True, course_key=None, display_name=None): """ Generate parameters used when parsing email templates. diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index 0b65f1c8a4..01559dfd8f 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -8,7 +8,6 @@ import mock from mock import patch from abc import ABCMeta from courseware.models import StudentModule -from courseware.tests.helpers import get_request_for_user from django.conf import settings from django.utils.translation import get_language from django.utils.translation import override as override_language @@ -17,11 +16,7 @@ from ccx_keys.locator import CCXLocator from student.tests.factories import UserFactory from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory -from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory -from grades.new.subsection_grade import SubsectionGradeFactory -from grades.tests.utils import answer_problem from lms.djangoapps.ccx.tests.factories import CcxFactory -from lms.djangoapps.course_blocks.api import get_course_blocks from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from student.models import CourseEnrollment, CourseEnrollmentAllowed from student.roles import CourseCcxCoachRole @@ -499,80 +494,6 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase): self.assertEqual(unrelated_state['brains'], 'zombie') -class TestStudentModuleGrading(SharedModuleStoreTestCase): - """ - Tests the effects of student module manipulations - on student grades. - """ - @classmethod - def setUpClass(cls): - super(TestStudentModuleGrading, cls).setUpClass() - cls.course = CourseFactory.create() - 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 - ) - 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.request = get_request_for_user(UserFactory()) - cls.user = cls.request.user - - def _get_subsection_grade_and_verify(self, all_earned, all_possible, graded_earned, graded_possible): - """ - Retrieves the subsection grade and verifies that - its scores match those expected. - """ - subsection_grade_factory = SubsectionGradeFactory( - self.user, - self.course, - get_course_blocks(self.user, self.course.location) - ) - grade = subsection_grade_factory.update(self.sequence) - self.assertEqual(grade.all_total.earned, all_earned) - self.assertEqual(grade.graded_total.earned, graded_earned) - self.assertEqual(grade.all_total.possible, all_possible) - self.assertEqual(grade.graded_total.possible, graded_possible) - - @patch('crum.get_current_request') - def test_delete_student_state(self, _crum_mock): - problem_location = self.problem.location - self._get_subsection_grade_and_verify(0, 1, 0, 1) - answer_problem(course=self.course, request=self.request, problem=self.problem, score=1, max_value=1) - self._get_subsection_grade_and_verify(1, 1, 1, 1) - - # Delete student state using the instructor dash - reset_student_attempts( - self.course.id, - self.user, - problem_location, - requesting_user=self.user, - delete_module=True, - ) - # Verify that the student's grades are reset - self._get_subsection_grade_and_verify(0, 1, 0, 1) - - class EnrollmentObjects(object): """ Container for enrollment objects.