Skip to content

Commit

Permalink
fix: csv import should throw error on repeat user
Browse files Browse the repository at this point in the history
if the user show up more than once in the csv, the import will fail and send back the error message with line number.

Ticket[AU-20]

update error message for repeat user

version bump
  • Loading branch information
leangseu-edx committed Jul 12, 2021
1 parent d5ffd4f commit 4bdf6a9
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 9 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ Change Log
Unreleased
~~~~~~~~~~
[0.8.13] - 2021-07-12
* Fix bug where we ignore repeat user in the csv import

[0.8.12] - 2012-06-21
[0.8.12] - 2021-06-21
* Fixed import csv to not working with multiple sections per-user override

[0.8.11] - 2021-07-09
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.8.12'
__version__ = '0.8.13'

default_app_config = 'bulk_grades.apps.BulkGradesConfig' # pylint: disable=invalid-name
9 changes: 5 additions & 4 deletions bulk_grades/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,12 +357,13 @@ def preprocess_row(self, row):
Preprocess the CSV row.
"""
operation = {}
if row['user_id'] in self._users_seen:
return operation
user_id = row['user_id']
if user_id in self._users_seen:
raise ValidationError(_('Repeated user_id: ') + str(user_id))

operation['new_override_grades'] = []
operation['course_id'] = self.course_id
operation['user_id'] = row['user_id']
operation['user_id'] = user_id

for key in row:
if key.startswith('new_override-'):
Expand All @@ -380,7 +381,7 @@ 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(row['user_id'])
self._users_seen.add(user_id)
return operation

def process_row(self, row):
Expand Down
5 changes: 5 additions & 0 deletions bulk_grades/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ def post(self, request, course_id, *args, **kwargs):
the_file = request.FILES['csv']
self.processor.process_file(the_file, autocommit=True)
data = self.processor.status()
data['error_messages'] = []
for error_message in self.processor.error_messages:
line_numbers = ', #'.join(str(line_number+1) for line_number in self.processor.error_messages[error_message])
data['error_messages'].append(f'{error_message} on lines (#{line_numbers})')

log.info('Processed file %s for %s -> %s saved, %s processed, %s error. (async=%s)',
the_file.name,
course_id,
Expand Down
9 changes: 6 additions & 3 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -475,10 +475,12 @@ def test_multiple_subsection_override(self):

def test_repeat_user(self):
processor = api.GradeCSVProcessor(course_id=self.course_id)
username = 'ditto'
row = {
'block_id': self.usage_key,
'new_override-85bb02db': '1',
'user_id': self.learner.id,
'username': username,
'Previous Points': '',
}
operation = processor.preprocess_row(row)
Expand All @@ -488,12 +490,13 @@ def test_repeat_user(self):
'block_id': self.usage_key,
'new_override-123402db': '2',
'user_id': self.learner.id,
'username': username,
'Previous Points': ''
}

# different row with the same user id should not get processed
operation = processor.preprocess_row(row2)
assert not operation
# different row with the same user id throw error
with self.assertRaisesMessage(ValidationError, 'Repeated user'):
processor.preprocess_row(row2)

def test_empty_grade(self):
processor = api.GradeCSVProcessor(course_id=self.course_id)
Expand Down

0 comments on commit 4bdf6a9

Please sign in to comment.