diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 82a4c5e..e5c5bd8 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -12,14 +12,18 @@ Change Log .. There should always be an "Unreleased" section for changes pending release. Unreleased -[0.8.14] - 2021-07-12 -* Update csv import error message +[0.9.1] - 2021-08-02 +* Feat repeat user error should also show the first occurence ~~~~~~~~~~ [0.9.0] - 2021-07-20 ~~~~~~~~~~~~~~~~~~~~ * Added support for django 3.2 +[0.8.14] - 2021-07-12 +* Update csv import error message +======= + [0.8.13] - 2021-07-12 ~~~~~~~~~~~~~~~~~~~~~ * Fix bug where we ignore repeat user in the csv import diff --git a/bulk_grades/__init__.py b/bulk_grades/__init__.py index 88f161a..f8e88d4 100644 --- a/bulk_grades/__init__.py +++ b/bulk_grades/__init__.py @@ -2,6 +2,6 @@ Support for bulk scoring and grading. """ -__version__ = '0.9.0' +__version__ = '0.9.1' default_app_config = 'bulk_grades.apps.BulkGradesConfig' # pylint: disable=invalid-name diff --git a/bulk_grades/api.py b/bulk_grades/api.py index e0f04ee..7be51f4 100644 --- a/bulk_grades/api.py +++ b/bulk_grades/api.py @@ -4,7 +4,7 @@ import logging -from collections import OrderedDict +from collections import OrderedDict, defaultdict from itertools import product from django.apps import apps @@ -316,7 +316,8 @@ def __init__(self, **kwargs): self.subsection_prefixes ) ) - self._users_seen = set() + self._users_seen = defaultdict(list) + self._row_num = 0 # pylint: disable=inconsistent-return-statements @cached_property @@ -349,6 +350,7 @@ def preprocess_file(self, reader): """ Preprocess the file, saving original data no matter whether there are errors. """ + self._row_num = 0 # reset private row number count super().preprocess_file(reader) self.save() @@ -356,10 +358,15 @@ def preprocess_row(self, row): """ Preprocess the CSV row. """ + self._row_num += 1 operation = {} user_id = row['user_id'] if user_id in self._users_seen: + if len(self._users_seen[user_id]) == 1: + self.add_error(_('Repeated user_id: ') + str(user_id), self._users_seen[user_id][0]) + self._users_seen[user_id].append(self._row_num) raise ValidationError(_('Repeated user_id: ') + str(user_id)) + self._users_seen[user_id].append(self._row_num) operation['new_override_grades'] = [] operation['course_id'] = self.course_id @@ -381,7 +388,6 @@ def preprocess_row(self, row): raise ValidationError(_('Grade must not be negative')) operation['new_override_grades'].append((block_id, new_grade)) - self._users_seen.add(user_id) return operation def process_row(self, row): diff --git a/tests/test_api.py b/tests/test_api.py index ea25537..cd848bf 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -442,7 +442,7 @@ def test_course_grade_filters(self, course_grade_factory_mock): rows = list(processor.get_iterator()) self.assertEqual(len(rows), (self.NUM_USERS - 2)+1) - def test_preprocess_error(self): + def test_preprocess_negative_number_error(self): processor = api.GradeCSVProcessor(course_id=self.course_id) row = { 'block_id': self.usage_key, @@ -454,7 +454,17 @@ def test_preprocess_error(self): } with self.assertRaisesMessage(ValidationError, 'Grade must not be negative'): processor.preprocess_row(row) - row['new_override-85bb02db'] = 'not a number' + + def test_preprocess_nan_error(self): + processor = api.GradeCSVProcessor(course_id=self.course_id) + row = { + 'block_id': self.usage_key, + 'new_override-123402db': '1', + 'new_override-85bb02db': 'not a number', + 'user_id': self.learner.id, + 'csum': '07ec', + 'Previous Points': '', + } with self.assertRaisesMessage(ValueError, 'Grade must be a number'): processor.preprocess_row(row) @@ -497,6 +507,13 @@ def test_repeat_user(self): # different row with the same user id throw error with self.assertRaisesMessage(ValidationError, 'Repeated user'): processor.preprocess_row(row2) + # there should be 2 errors for the first repeat user error + self.assertCountEqual(len(processor.error_messages), 2) + + with self.assertRaisesMessage(ValidationError, 'Repeated user'): + processor.preprocess_row(row2) + # there should be 3 errors for the second repeat user error + self.assertCountEqual(len(processor.error_messages), 3) def test_empty_grade(self): processor = api.GradeCSVProcessor(course_id=self.course_id)