diff --git a/common/test/acceptance/tests/discussion/test_cohort_management.py b/common/test/acceptance/tests/discussion/test_cohort_management.py index 442707f042..8b61f8e3f2 100644 --- a/common/test/acceptance/tests/discussion/test_cohort_management.py +++ b/common/test/acceptance/tests/discussion/test_cohort_management.py @@ -16,6 +16,8 @@ from ...pages.lms.auto_auth import AutoAuthPage from ...pages.lms.instructor_dashboard import InstructorDashboardPage, DataDownloadPage from ...pages.studio.settings_group_configurations import GroupConfigurationsPage +import os +import unicodecsv import uuid @@ -39,23 +41,24 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin self.manual_cohort_id = self.add_manual_cohort(self.course_fixture, self.manual_cohort_name) # create a non-instructor who will be registered for the course and in the manual cohort. - self.student_name = "student_user" + self.student_name, self.student_email = self._generate_unique_user_data() self.student_id = AutoAuthPage( - self.browser, username=self.student_name, email="student_user@example.com", + self.browser, username=self.student_name, email=self.student_email, course_id=self.course_id, staff=False ).visit().get_user_id() self.add_user_to_cohort(self.course_fixture, self.student_name, self.manual_cohort_id) # create a second student user + self.other_student_name, self.other_student_email = self._generate_unique_user_data() self.other_student_id = AutoAuthPage( - self.browser, username="other_student_user", email="other_student_user@example.com", + self.browser, username=self.other_student_name, email=self.other_student_email, course_id=self.course_id, staff=False ).visit().get_user_id() # login as an instructor - self.instructor_name = "instructor_user" + self.instructor_name, self.instructor_email = self._generate_unique_user_data() self.instructor_id = AutoAuthPage( - self.browser, username=self.instructor_name, email="instructor_user@example.com", + self.browser, username=self.instructor_name, email=self.instructor_email, course_id=self.course_id, staff=True ).visit().get_user_id() @@ -310,6 +313,29 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin self.cohort_management_page.get_cohort_associated_assignment_type() ) + def _create_csv_file(self, filename, csv_text_as_lists): + """ + Create a csv file with the provided list of lists. + + :param filename: this is the name that will be used for the csv file. Its location will + be under the test upload data directory + :param csv_text_as_lists: provide the contents of the csv file int he form of a list of lists + """ + filename = self.instructor_dashboard_page.get_asset_path(filename) + with open(filename, 'w+') as csv_file: + writer = unicodecsv.writer(csv_file) + for line in csv_text_as_lists: + writer.writerow(line) + self.addCleanup(os.remove, filename) + + def _generate_unique_user_data(self): + """ + Produce unique username and e-mail. + """ + unique_username = 'user' + str(uuid.uuid4().hex)[:12] + unique_email = unique_username + "@example.com" + return unique_username, unique_email + def test_add_new_cohort(self): """ Scenario: A new manual cohort can be created, and a student assigned to it. @@ -471,9 +497,15 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin Then I can download a file with results And appropriate events have been emitted """ - # cohort_users_both_columns.csv adds instructor_user to ManualCohort1 via username and - # student_user to AutoCohort1 via email - self._verify_csv_upload_acceptable_file("cohort_users_both_columns.csv") + csv_contents = [ + ['username', 'email', 'ignored_column', 'cohort'], + [self.instructor_name, '', 'June', 'ManualCohort1'], + ['', self.student_email, 'Spring', 'AutoCohort1'], + [self.other_student_name, '', 'Fall', 'ManualCohort1'], + ] + filename = "cohort_csv_both_columns_1.csv" + self._create_csv_file(filename, csv_contents) + self._verify_csv_upload_acceptable_file(filename) def test_cohort_by_csv_only_email(self): """ @@ -485,8 +517,15 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin Then I can download a file with results And appropriate events have been emitted """ - # cohort_users_only_email.csv adds instructor_user to ManualCohort1 and student_user to AutoCohort1 via email - self._verify_csv_upload_acceptable_file("cohort_users_only_email.csv") + csv_contents = [ + ['email', 'cohort'], + [self.instructor_email, 'ManualCohort1'], + [self.student_email, 'AutoCohort1'], + [self.other_student_email, 'ManualCohort1'], + ] + filename = "cohort_csv_emails_only.csv" + self._create_csv_file(filename, csv_contents) + self._verify_csv_upload_acceptable_file(filename) def test_cohort_by_csv_only_username(self): """ @@ -498,13 +537,46 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin Then I can download a file with results And appropriate events have been emitted """ - # cohort_users_only_username.csv adds instructor_user to ManualCohort1 and - # student_user to AutoCohort1 via username - self._verify_csv_upload_acceptable_file("cohort_users_only_username.csv") + csv_contents = [ + ['username', 'cohort'], + [self.instructor_name, 'ManualCohort1'], + [self.student_name, 'AutoCohort1'], + [self.other_student_name, 'ManualCohort1'], + ] + filename = "cohort_users_only_username1.csv" + self._create_csv_file(filename, csv_contents) + self._verify_csv_upload_acceptable_file(filename) - def _verify_csv_upload_acceptable_file(self, filename): + def test_cohort_by_csv_unicode(self): + """ + Scenario: the instructor can upload a file with user and cohort assignments, using both emails and usernames. + + Given I have a course with two cohorts defined + And I add another cohort with a unicode name + When I go to the cohort management section of the instructor dashboard + I can upload a CSV file with assignments of users to the unicode cohort via both usernames and emails + Then I can download a file with results + + TODO: refactor events verification to handle this scenario. Events verification assumes movements + between other cohorts (manual and auto). + """ + unicode_hello_in_korean = u'안녕하세요' + self._verify_cohort_settings(cohort_name=unicode_hello_in_korean, assignment_type=None) + csv_contents = [ + ['username', 'email', 'cohort'], + [self.instructor_name, '', unicode_hello_in_korean], + ['', self.student_email, unicode_hello_in_korean], + [self.other_student_name, '', unicode_hello_in_korean] + ] + filename = "cohort_unicode_name.csv" + self._create_csv_file(filename, csv_contents) + self._verify_csv_upload_acceptable_file(filename, skip_events=True) + + def _verify_csv_upload_acceptable_file(self, filename, skip_events=None): """ Helper method to verify cohort assignments after a successful CSV upload. + + When skip_events is specified, no assertions are made on events. """ start_time = datetime.now(UTC) self.cohort_management_page.upload_cohort_file(filename) @@ -512,45 +584,46 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin "Your file '{}' has been uploaded. Allow a few minutes for processing.".format(filename) ) - # student_user is moved from manual cohort to auto cohort - self.assertEqual( - self.event_collection.find({ - "name": "edx.cohort.user_added", - "time": {"$gt": start_time}, - "event.user_id": {"$in": [int(self.student_id)]}, - "event.cohort_name": self.auto_cohort_name, - }).count(), - 1 - ) - self.assertEqual( - self.event_collection.find({ - "name": "edx.cohort.user_removed", - "time": {"$gt": start_time}, - "event.user_id": int(self.student_id), - "event.cohort_name": self.manual_cohort_name, - }).count(), - 1 - ) - # instructor_user (previously unassigned) is added to manual cohort - self.assertEqual( - self.event_collection.find({ - "name": "edx.cohort.user_added", - "time": {"$gt": start_time}, - "event.user_id": {"$in": [int(self.instructor_id)]}, - "event.cohort_name": self.manual_cohort_name, - }).count(), - 1 - ) - # other_student_user (previously unassigned) is added to manual cohort - self.assertEqual( - self.event_collection.find({ - "name": "edx.cohort.user_added", - "time": {"$gt": start_time}, - "event.user_id": {"$in": [int(self.other_student_id)]}, - "event.cohort_name": self.manual_cohort_name, - }).count(), - 1 - ) + if not skip_events: + # student_user is moved from manual cohort to auto cohort + self.assertEqual( + self.event_collection.find({ + "name": "edx.cohort.user_added", + "time": {"$gt": start_time}, + "event.user_id": {"$in": [int(self.student_id)]}, + "event.cohort_name": self.auto_cohort_name, + }).count(), + 1 + ) + self.assertEqual( + self.event_collection.find({ + "name": "edx.cohort.user_removed", + "time": {"$gt": start_time}, + "event.user_id": int(self.student_id), + "event.cohort_name": self.manual_cohort_name, + }).count(), + 1 + ) + # instructor_user (previously unassigned) is added to manual cohort + self.assertEqual( + self.event_collection.find({ + "name": "edx.cohort.user_added", + "time": {"$gt": start_time}, + "event.user_id": {"$in": [int(self.instructor_id)]}, + "event.cohort_name": self.manual_cohort_name, + }).count(), + 1 + ) + # other_student_user (previously unassigned) is added to manual cohort + self.assertEqual( + self.event_collection.find({ + "name": "edx.cohort.user_added", + "time": {"$gt": start_time}, + "event.user_id": {"$in": [int(self.other_student_id)]}, + "event.cohort_name": self.manual_cohort_name, + }).count(), + 1 + ) # Verify the results can be downloaded. data_download = self.instructor_dashboard_page.select_data_download() diff --git a/common/test/acceptance/tests/lms/test_learner_profile.py b/common/test/acceptance/tests/lms/test_learner_profile.py index d32a489451..1b27c1b8b6 100644 --- a/common/test/acceptance/tests/lms/test_learner_profile.py +++ b/common/test/acceptance/tests/lms/test_learner_profile.py @@ -604,7 +604,7 @@ class OwnLearnerProfilePageTest(LearnerProfileTestMixin, WebAppTest): self.assert_default_image_has_public_access(profile_page) - profile_page.upload_file(filename='cohort_users_only_username.csv') + profile_page.upload_file(filename='generic_csv.csv') self.assertEqual( profile_page.profile_image_message, "The file must be one of the following types: .gif, .png, .jpeg, .jpg." diff --git a/common/test/data/uploads/cohort_users_both_columns.csv b/common/test/data/uploads/cohort_users_both_columns.csv deleted file mode 100644 index ee13b56c7c..0000000000 --- a/common/test/data/uploads/cohort_users_both_columns.csv +++ /dev/null @@ -1,4 +0,0 @@ -username,email,ignored_column,cohort -instructor_user,,June,ManualCohort1 -,student_user@example.com,Spring,AutoCohort1 -other_student_user,,Fall,ManualCohort1 diff --git a/common/test/data/uploads/cohort_users_only_email.csv b/common/test/data/uploads/cohort_users_only_email.csv deleted file mode 100644 index 7fb6a85400..0000000000 --- a/common/test/data/uploads/cohort_users_only_email.csv +++ /dev/null @@ -1,5 +0,0 @@ -email,cohort -instructor_user@example.com,ManualCohort1 -student_user@example.com,AutoCohort1 -other_student_user@example.com,ManualCohort1 - diff --git a/common/test/data/uploads/cohort_users_only_username.csv b/common/test/data/uploads/cohort_users_only_username.csv deleted file mode 100644 index f33b77a0a2..0000000000 --- a/common/test/data/uploads/cohort_users_only_username.csv +++ /dev/null @@ -1,4 +0,0 @@ -username,cohort -instructor_user,ManualCohort1 -student_user,AutoCohort1 -other_student_user,ManualCohort1 diff --git a/common/test/data/uploads/generic_csv.csv b/common/test/data/uploads/generic_csv.csv new file mode 100644 index 0000000000..bad83f6563 --- /dev/null +++ b/common/test/data/uploads/generic_csv.csv @@ -0,0 +1,7 @@ +column_1,column_2 +foo,bar +foo,baz +hello,there +file_size,must_be_100_bytes +the story,was +very very very very very,interesting