Merge pull request #599 from prathamesh920/csv-upload-fix

Fix user CSV upload issue

4 files changed, 78 insertions, 16 deletions

diff --git a/yaksh/fixtures/existing_user.csv b/yaksh/fixtures/existing_user.csv
new file mode 100644
index 0000000..6584580
--- /dev/null
+++ b/yaksh/fixtures/existing_user.csv
@@ -0,0 +1,2 @@
+firstname, lastname, email, username
+abcd, abcd, demo2@test.com, demo_user2
diff --git a/yaksh/fixtures/users_add_update_reject.csv b/yaksh/fixtures/users_add_update_reject.csv
index 43881d8..1990179 100644
--- a/yaksh/fixtures/users_add_update_reject.csv
+++ b/yaksh/fixtures/users_add_update_reject.csv
@@ -1,4 +1,4 @@
 firstname, lastname, email, institute,department,roll_no,remove,password,username
 test, test, test@g.com, TEST, TEST, TEST101, FALSE, TEST, test
-test2, test, test@g.com, TEST, TEST, TEST101, FALSE, TEST, test
-test2, test, test@g.com, TEST, TEST, TEST101, TRUE, TEST, test
+test2, test, test@g.com, TEST, TEST, TEST101, FALSE, TEST, test2
+test2, test, test@g.com, TEST, TEST, TEST101, TRUE, TEST, test2
diff --git a/yaksh/test_views.py b/yaksh/test_views.py
index ee7c911..714fc67 100644
--- a/yaksh/test_views.py
+++ b/yaksh/test_views.py
@@ -2532,6 +2532,10 @@ class TestCourseDetail(TestCase):
             name="Python Course",
             enrollment="Enroll Request", creator=self.user1
             )
+        self.user1_othercourse = Course.objects.create(
+            name="Python Course II",
+            enrollment="Enroll Request", creator=self.user1
+            )
         self.learning_module = LearningModule.objects.create(
             name="test module", description="test description module",
             html_data="test html description module", creator=self.user1,
@@ -2550,6 +2554,7 @@ class TestCourseDetail(TestCase):
         self.user2.delete()
         self.student.delete()
         self.user1_course.delete()
+        self.user1_othercourse.delete()
         self.mod_group.delete()
 
     def test_upload_users_with_correct_csv(self):
@@ -2574,7 +2579,63 @@ class TestCourseDetail(TestCase):
         self.assertEqual(uploaded_user.count(), 1)
         self.assertEqual(response.status_code, 200)
         self.assertIn('upload_details', response.context)
+        self.assertIn(uploaded_user.first(), self.user1_course.students.all())
+        self.assertTemplateUsed(response, 'yaksh/course_detail.html')
+
+    def test_upload_existing_user(self):
+        # Given
+        self.client.login(
+            username=self.user1.username, password=self.user1_plaintext_pass)
+        csv_file_path = os.path.join(FIXTURES_DIR_PATH, 'existing_user.csv')
+        csv_file = open(csv_file_path, 'rb')
+        upload_file = SimpleUploadedFile(csv_file_path, csv_file.read())
+        csv_file.close()
+
+        # When
+        response = self.client.post(
+            reverse('yaksh:upload_users',
+                    kwargs={'course_id': self.user1_course.id}),
+            data={'csv_file': upload_file})
+
+        # Then
+        self.assertEqual(response.status_code, 200)
+        self.assertIn('upload_details', response.context)
         self.assertTemplateUsed(response, 'yaksh/course_detail.html')
+        self.assertIn(self.user2, self.user1_course.students.all())
+
+    def test_upload_same_user_multiple_course(self):
+        # Given
+        self.client.login(
+            username=self.user1.username, password=self.user1_plaintext_pass)
+        csv_file_path = os.path.join(FIXTURES_DIR_PATH, 'users_correct.csv')
+        csv_file = open(csv_file_path, 'rb')
+        upload_file1 = SimpleUploadedFile(csv_file_path, csv_file.read())
+        csv_file.seek(0)
+        upload_file2 = SimpleUploadedFile(csv_file_path, csv_file.read())
+        csv_file.close()
+
+        # When
+        response1 = self.client.post(
+            reverse('yaksh:upload_users',
+                    kwargs={'course_id': self.user1_course.id}),
+            data={'csv_file': upload_file1})
+
+        response2 = self.client.post(
+            reverse('yaksh:upload_users',
+                    kwargs={'course_id': self.user1_othercourse.id}),
+            data={'csv_file': upload_file2})
+
+        # Then
+        uploaded_users = User.objects.filter(email='abc@xyz.com')
+        self.assertEqual(response1.status_code, 200)
+        self.assertIn('upload_details', response1.context)
+        self.assertTemplateUsed(response1, 'yaksh/course_detail.html')
+        self.assertEqual(response2.status_code, 200)
+        self.assertIn('upload_details', response2.context)
+        self.assertTemplateUsed(response2, 'yaksh/course_detail.html')
+        self.assertIn(uploaded_users.first(), self.user1_course.students.all())
+        self.assertIn(uploaded_users.first(),
+                      self.user1_othercourse.students.all())
 
     def test_upload_users_add_update_reject(self):
         # Given
@@ -2595,7 +2656,7 @@ class TestCourseDetail(TestCase):
         csv_file.close()
 
         # Then
-        uploaded_user = User.objects.filter(username="test")
+        uploaded_user = User.objects.filter(username="test2")
         user = uploaded_user[0]
         self.assertEqual(uploaded_user.count(), 1)
         self.assertEqual(user.first_name, "test2")
diff --git a/yaksh/views.py b/yaksh/views.py
index d634e0c..7202826 100644
--- a/yaksh/views.py
+++ b/yaksh/views.py
@@ -2224,18 +2224,15 @@ def _read_user_csv(reader, course):
         if users.exists():
             user = users[0]
             if remove.strip().lower() == 'true':
-                if _remove_from_course(user, course):
-                    upload_details.append("{0} -- {1} -- User rejected".format(
-                                          counter, user.username))
-                    continue
+                _remove_from_course(user, course)
+                upload_details.append("{0} -- {1} -- User rejected".format(
+                                      counter, user.username))
             else:
-                if _add_to_course(user, course):
-                    upload_details.append("{0} -- {1} -- User rejected".format(
-                                          counter, user.username))
-            if user not in course.get_enrolled():
-                upload_details.append("{0} -- {1} not added to course".format(
-                    counter, user))
-                continue
+                _add_to_course(user, course)
+                upload_details.append(
+                    "{0} -- {1} -- User Added Successfully".format(
+                        counter, user.username))
+            continue
         user_defaults = {'email': email, 'first_name': first_name,
                          'last_name': last_name}
         user, created = _create_or_update_user(username, password,
@@ -2285,13 +2282,15 @@ def _get_csv_values(row, fields):
 def _remove_from_course(user, course):
     if user in course.get_enrolled():
         course.reject(True, user)
-        return True
+    else:
+        course.rejected.add(user)
 
 
 def _add_to_course(user, course):
     if user in course.get_rejected():
         course.enroll(True, user)
-        return True
+    else:
+        course.students.add(user)
 
 
 def _create_or_update_user(username, password, defaults):