scores must be positive (#29)

* scores must be positive

CHANGELOG.rst

@@ -16,6 +16,10 @@ Unreleased
 
 *
 
+[0.5.10] - 2019-09-06
+~~~~~~~~~~~~~~~~~~~~~
+* Prevent user from setting negative grades
+
 [0.5.9] - 2019-08-28
 ~~~~~~~~~~~~~~~~~~~~
 * Make intervention report display either grade override if exists or original grade.

bulk_grades/__init__.py

@@ -4,6 +4,6 @@
 
 from __future__ import absolute_import, unicode_literals
 
-__version__ = '0.5.9'
+__version__ = '0.5.10'
 
 default_app_config = 'bulk_grades.apps.BulkGradesConfig'  # pylint: disable=invalid-name

bulk_grades/api.py

@@ -281,6 +281,8 @@ def preprocess_row(self, row):
                         operation['new_grade'] = float(value)
                     except ValueError:
                         raise ValidationError(_('Grade must be a number'))
+                    if operation['new_grade'] < 0:
+                        raise ValidationError(_('Grade must be positive'))
         self._users_seen.add(row['user_id'])
         return operation
 
@@ -479,6 +481,8 @@ def set_score(usage_key, student_id, score, max_points, override_user_id=None, *
     if not isinstance(usage_key, UsageKey):
         usage_key = UsageKey.from_string(usage_key)
     defaults['module_type'] = 'problem'
+    if score < 0:
+        raise ValueError(_('score must be positive'))
     defaults['grade'] = score
     defaults['max_grade'] = max_points
     module = apps.get_model('courseware', 'StudentModule').objects.update_or_create(

tests/test_api.py

@@ -48,6 +48,10 @@ def test_set_score(self):
         score = api.get_score(self.block_id, 11)
         assert score is None
 
+    def test_negative_score(self):
+        with self.assertRaisesMessage(ValueError, 'score must be positive'):
+            api.set_score(self.block_id, self.learner.id, -2, 22, override_user_id=self.staff.id)
+
 
 class TestScoreProcessor(BaseTests):
     """
@@ -163,6 +167,23 @@ def test_course_grade_filters(self, course_grade_factory_mock):
         rows = list(processor.get_iterator())
         self.assertEqual(len(rows), self.NUM_USERS+1)
 
+    @patch('lms.djangoapps.grades.api.CourseGradeFactory.read')
+    def test_less_than_zero(self, course_grade_factory_mock):
+        self._make_enrollments()
+
+        course_grade_factory_mock.return_value = Mock(percent=0.50)
+        processor = api.GradeCSVProcessor(course_id=self.course_id)
+
+        row = {
+            'block_id': self.block_id,
+            'new_grade-block-v1': '-1',
+            'user_id': self.learner.id,
+            'csum': '07ec',
+            'Previous Points': '',
+        }
+        with self.assertRaisesMessage(ValidationError, 'Grade must be positive'):
+            processor.preprocess_row(row)
+
 
 class TestInterventionProcessor(BaseTests):
     """