Skip to content

Commit

Permalink
feat: row first row of error on duplicate user import
Browse files Browse the repository at this point in the history
Detect duplicate user_id on the first occurence.

Ticket[AU-44]

update unit testing

update the logic to figure row number
  • Loading branch information
leangseu-edx committed Aug 3, 2021
1 parent 9756ed5 commit efa9a78
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 8 deletions.
8 changes: 6 additions & 2 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion bulk_grades/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 9 additions & 3 deletions bulk_grades/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@


import logging
from collections import OrderedDict
from collections import OrderedDict, defaultdict
from itertools import product

from django.apps import apps
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -349,17 +350,23 @@ 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()

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
Expand All @@ -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):
Expand Down
21 changes: 19 additions & 2 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)

Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit efa9a78

Please sign in to comment.