Skip to content

Commit

Permalink
fix: support files with a unicode signature in the Instructor Dashboa…
Browse files Browse the repository at this point in the history
…rd API

Without this, files with BOM (byte order mark; generated e.g., by Microsoft
Excel) cannot be read properly.

(cherry picked from commit 7d8a502)
  • Loading branch information
Agrendalath authored and xitij2000 committed Jun 26, 2023
1 parent c410212 commit 75aa91d
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 27 deletions.
32 changes: 8 additions & 24 deletions lms/djangoapps/instructor/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,7 @@ def test_instructor_level(self):


@patch.dict(settings.FEATURES, {'ALLOW_AUTOMATED_SIGNUPS': True})
@ddt.ddt
class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
"""
Test Bulk account creation and enrollment from csv file
Expand Down Expand Up @@ -641,32 +642,15 @@ def setUp(self):
)

@patch('lms.djangoapps.instructor.views.api.log.info')
def test_account_creation_and_enrollment_with_csv(self, info_log):
"""
Happy path test to create a single new user
"""
csv_content = b"test_student@example.com,test_student_1,tester1,USA"
uploaded_file = SimpleUploadedFile("temp.csv", csv_content)
response = self.client.post(self.url, {'students_list': uploaded_file, 'email-students': True})
assert response.status_code == 200
data = json.loads(response.content.decode('utf-8'))
assert len(data['row_errors']) == 0
assert len(data['warnings']) == 0
assert len(data['general_errors']) == 0

manual_enrollments = ManualEnrollmentAudit.objects.all()
assert manual_enrollments.count() == 1
assert manual_enrollments[0].state_transition == UNENROLLED_TO_ENROLLED

# test the log for email that's send to new created user.
info_log.assert_called_with('email sent to new created user at %s', 'test_student@example.com')

@patch('lms.djangoapps.instructor.views.api.log.info')
def test_account_creation_and_enrollment_with_csv_with_blank_lines(self, info_log):
@ddt.data(
b"test_student@example.com,test_student_1,tester1,USA", # Typical use case.
b"\ntest_student@example.com,test_student_1,tester1,USA\n\n", # Blank lines.
b"\xef\xbb\xbftest_student@example.com,test_student_1,tester1,USA", # Unicode signature (BOM).
)
def test_account_creation_and_enrollment_with_csv(self, csv_content, info_log):
"""
Happy path test to create a single new user
"""
csv_content = b"\ntest_student@example.com,test_student_1,tester1,USA\n\n"
uploaded_file = SimpleUploadedFile("temp.csv", csv_content)
response = self.client.post(self.url, {'students_list': uploaded_file, 'email-students': True})
assert response.status_code == 200
Expand Down Expand Up @@ -4385,7 +4369,7 @@ def call_add_users_to_cohorts(self, csv_data, suffix='.csv'):
"""
# this temporary file will be removed in `self.tearDown()`
__, file_name = tempfile.mkstemp(suffix=suffix, dir=self.tempdir)
with open(file_name, 'w') as file_pointer:
with open(file_name, 'w', encoding='utf-8-sig') as file_pointer:
file_pointer.write(csv_data)
with open(file_name) as file_pointer:
url = reverse('add_users_to_cohorts', kwargs={'course_id': str(self.course.id)})
Expand Down
6 changes: 3 additions & 3 deletions lms/djangoapps/instructor/views/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ def register_and_enroll_students(request, course_id): # pylint: disable=too-man
try:
upload_file = request.FILES.get('students_list')
if upload_file.name.endswith('.csv'):
students = list(csv.reader(upload_file.read().decode('utf-8').splitlines()))
students = list(csv.reader(upload_file.read().decode('utf-8-sig').splitlines()))
course = get_course_by_id(course_id)
else:
general_errors.append({
Expand Down Expand Up @@ -1519,7 +1519,7 @@ def _cohorts_csv_validator(file_storage, file_to_validate):
Verifies that the expected columns are present in the CSV used to add users to cohorts.
"""
with file_storage.open(file_to_validate) as f:
reader = csv.reader(f.read().decode('utf-8').splitlines())
reader = csv.reader(f.read().decode('utf-8-sig').splitlines())

try:
fieldnames = next(reader)
Expand Down Expand Up @@ -3332,7 +3332,7 @@ def build_row_errors(key, _user, row_count):
try:
upload_file = request.FILES.get('students_list')
if upload_file.name.endswith('.csv'):
students = list(csv.reader(upload_file.read().decode('utf-8').splitlines()))
students = list(csv.reader(upload_file.read().decode('utf-8-sig').splitlines()))
else:
general_errors.append(_('Make sure that the file you upload is in CSV format with no '
'extraneous characters or rows.'))
Expand Down

0 comments on commit 75aa91d

Please sign in to comment.