diff --git a/common/test/acceptance/tests/discussion/test_cohort_management.py b/common/test/acceptance/tests/discussion/test_cohort_management.py index 815971b732..4e49cd29c8 100644 --- a/common/test/acceptance/tests/discussion/test_cohort_management.py +++ b/common/test/acceptance/tests/discussion/test_cohort_management.py @@ -149,8 +149,8 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin confirmation_messages = self.cohort_management_page.get_cohort_confirmation_messages() self.assertEqual( [ - "2 students have been added to this cohort", - "1 student was removed from " + self.manual_cohort_name + "2 learners have been added to this cohort.", + "1 learner was moved from " + self.manual_cohort_name ], confirmation_messages ) @@ -217,16 +217,16 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin self.assertEqual( [ - "0 students have been added to this cohort", - "1 student was already in the cohort" + "0 learners have been added to this cohort.", + "1 learner was already in the cohort" ], self.cohort_management_page.get_cohort_confirmation_messages() ) self.assertEqual( [ - "There was an error when trying to add students:", - "Unknown user: unknown_user" + "There was an error when trying to add learners:", + "Unknown username: unknown_user" ], self.cohort_management_page.get_cohort_error_messages() ) diff --git a/lms/djangoapps/instructor_task/tasks_helper/misc.py b/lms/djangoapps/instructor_task/tasks_helper/misc.py index cdb23149e0..77d769fed9 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/misc.py +++ b/lms/djangoapps/instructor_task/tasks_helper/misc.py @@ -10,6 +10,7 @@ from time import time import unicodecsv from django.contrib.auth.models import User +from django.core.exceptions import ValidationError from django.core.files.storage import DefaultStorage from openassessment.data import OraAggregateData from pytz import UTC @@ -137,9 +138,9 @@ def cohort_students_and_upload(_xmodule_instance_args, _entry_id, course_id, tas # cohorts_status is a mapping from cohort_name to metadata about # that cohort. The metadata will include information about users - # successfully added to the cohort, users not found, and a cached - # reference to the corresponding cohort object to prevent - # redundant cohort queries. + # successfully added to the cohort, users not found, Preassigned + # users, and a cached reference to the corresponding cohort object + # to prevent redundant cohort queries. cohorts_status = {} with DefaultStorage().open(task_input['file_name']) as f: @@ -152,8 +153,10 @@ def cohort_students_and_upload(_xmodule_instance_args, _entry_id, course_id, tas if not cohorts_status.get(cohort_name): cohorts_status[cohort_name] = { 'Cohort Name': cohort_name, - 'Students Added': 0, - 'Students Not Found': set() + 'Learners Added': 0, + 'Learners Not Found': set(), + 'Invalid Email Addresses': set(), + 'Preassigned Learners': set() } try: cohorts_status[cohort_name]['cohort'] = CourseUserGroup.objects.get( @@ -170,11 +173,25 @@ def cohort_students_and_upload(_xmodule_instance_args, _entry_id, course_id, tas continue try: - add_user_to_cohort(cohorts_status[cohort_name]['cohort'], username_or_email) - cohorts_status[cohort_name]['Students Added'] += 1 - task_progress.succeeded += 1 + # If add_user_to_cohort successfully adds a user, a user object is returned. + # If a user is preassigned to a cohort, no user object is returned (we already have the email address). + (user, previous_cohort, preassigned) = add_user_to_cohort(cohorts_status[cohort_name]['cohort'], username_or_email) + if preassigned: + cohorts_status[cohort_name]['Preassigned Learners'].add(username_or_email) + task_progress.preassigned += 1 + else: + cohorts_status[cohort_name]['Learners Added'] += 1 + task_progress.succeeded += 1 except User.DoesNotExist: - cohorts_status[cohort_name]['Students Not Found'].add(username_or_email) + # Raised when a user with the username could not be found, and the email is not valid + cohorts_status[cohort_name]['Learners Not Found'].add(username_or_email) + task_progress.failed += 1 + except ValidationError: + # Raised when a user with the username could not be found, and the email is not valid, + # but the entered string contains an "@" + # Since there is no way to know if the entered string is an invalid username or an invalid email, + # assume that a string with the "@" symbol in it is an attempt at entering an email + cohorts_status[cohort_name]['Invalid Email Addresses'].add(username_or_email) task_progress.failed += 1 except ValueError: # Raised when the user is already in the given cohort @@ -186,10 +203,12 @@ def cohort_students_and_upload(_xmodule_instance_args, _entry_id, course_id, tas task_progress.update_task_state(extra_meta=current_step) # Filter the output of `add_users_to_cohorts` in order to upload the result. - output_header = ['Cohort Name', 'Exists', 'Students Added', 'Students Not Found'] + output_header = ['Cohort Name', 'Exists', 'Learners Added', 'Learners Not Found', 'Invalid Email Addresses', 'Preassigned Learners'] output_rows = [ [ - ','.join(status_dict.get(column_name, '')) if column_name == 'Students Not Found' + ','.join(status_dict.get(column_name, '')) if (column_name == 'Learners Not Found' + or column_name == 'Invalid Email Addresses' + or column_name == 'Preassigned Learners') else status_dict[column_name] for column_name in output_header ] diff --git a/lms/djangoapps/instructor_task/tasks_helper/runner.py b/lms/djangoapps/instructor_task/tasks_helper/runner.py index e276720564..3bdae3d805 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/runner.py +++ b/lms/djangoapps/instructor_task/tasks_helper/runner.py @@ -26,6 +26,7 @@ class TaskProgress(object): self.succeeded = 0 self.skipped = 0 self.failed = 0 + self.preassigned = 0 def update_task_state(self, extra_meta=None): """ @@ -47,6 +48,7 @@ class TaskProgress(object): 'skipped': self.skipped, 'failed': self.failed, 'total': self.total, + 'preassigned': self.preassigned, 'duration_ms': int((time() - self.start_time) * 1000), } if extra_meta is not None: diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 67dcaff608..6e82663896 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -1464,7 +1464,7 @@ class TestCohortStudents(TestReportMixin, InstructorTaskCourseTestCase): self.cohort_2 = CohortFactory(course_id=self.course.id, name='Cohort 2') self.student_1 = self.create_student(username=u'student_1\xec', email='student_1@example.com') self.student_2 = self.create_student(username='student_2', email='student_2@example.com') - self.csv_header_row = ['Cohort Name', 'Exists', 'Students Added', 'Students Not Found'] + self.csv_header_row = ['Cohort Name', 'Exists', 'Learners Added', 'Learners Not Found', 'Invalid Email Addresses', 'Preassigned Learners'] def _cohort_students_and_upload(self, csv_data): """ @@ -1485,8 +1485,8 @@ class TestCohortStudents(TestReportMixin, InstructorTaskCourseTestCase): self.assertDictContainsSubset({'total': 2, 'attempted': 2, 'succeeded': 2, 'failed': 0}, result) self.verify_rows_in_csv( [ - dict(zip(self.csv_header_row, ['Cohort 1', 'True', '1', ''])), - dict(zip(self.csv_header_row, ['Cohort 2', 'True', '1', ''])), + dict(zip(self.csv_header_row, ['Cohort 1', 'True', '1', '', '', ''])), + dict(zip(self.csv_header_row, ['Cohort 2', 'True', '1', '', '', ''])), ], verify_order=False ) @@ -1500,8 +1500,8 @@ class TestCohortStudents(TestReportMixin, InstructorTaskCourseTestCase): self.assertDictContainsSubset({'total': 2, 'attempted': 2, 'succeeded': 2, 'failed': 0}, result) self.verify_rows_in_csv( [ - dict(zip(self.csv_header_row, ['Cohort 1', 'True', '1', ''])), - dict(zip(self.csv_header_row, ['Cohort 2', 'True', '1', ''])), + dict(zip(self.csv_header_row, ['Cohort 1', 'True', '1', '', '', ''])), + dict(zip(self.csv_header_row, ['Cohort 2', 'True', '1', '', '', ''])), ], verify_order=False ) @@ -1515,8 +1515,8 @@ class TestCohortStudents(TestReportMixin, InstructorTaskCourseTestCase): self.assertDictContainsSubset({'total': 2, 'attempted': 2, 'succeeded': 2, 'failed': 0}, result) self.verify_rows_in_csv( [ - dict(zip(self.csv_header_row, ['Cohort 1', 'True', '1', ''])), - dict(zip(self.csv_header_row, ['Cohort 2', 'True', '1', ''])), + dict(zip(self.csv_header_row, ['Cohort 1', 'True', '1', '', '', ''])), + dict(zip(self.csv_header_row, ['Cohort 2', 'True', '1', '', '', ''])), ], verify_order=False ) @@ -1536,8 +1536,8 @@ class TestCohortStudents(TestReportMixin, InstructorTaskCourseTestCase): self.assertDictContainsSubset({'total': 2, 'attempted': 2, 'succeeded': 2, 'failed': 0}, result) self.verify_rows_in_csv( [ - dict(zip(self.csv_header_row, ['Cohort 1', 'True', '1', ''])), - dict(zip(self.csv_header_row, ['Cohort 2', 'True', '1', ''])), + dict(zip(self.csv_header_row, ['Cohort 1', 'True', '1', '', '', ''])), + dict(zip(self.csv_header_row, ['Cohort 2', 'True', '1', '', '', ''])), ], verify_order=False ) @@ -1546,13 +1546,11 @@ class TestCohortStudents(TestReportMixin, InstructorTaskCourseTestCase): result = self._cohort_students_and_upload( 'username,email,cohort\n' 'Invalid,,Cohort 1\n' - 'student_2,also_fake@bad.com,Cohort 2' ) - self.assertDictContainsSubset({'total': 2, 'attempted': 2, 'succeeded': 0, 'failed': 2}, result) + self.assertDictContainsSubset({'total': 1, 'attempted': 1, 'succeeded': 0, 'failed': 1}, result) self.verify_rows_in_csv( [ - dict(zip(self.csv_header_row, ['Cohort 1', 'True', '0', 'Invalid'])), - dict(zip(self.csv_header_row, ['Cohort 2', 'True', '0', 'also_fake@bad.com'])), + dict(zip(self.csv_header_row, ['Cohort 1', 'True', '0', 'Invalid', '', ''])), ], verify_order=False ) @@ -1566,8 +1564,35 @@ class TestCohortStudents(TestReportMixin, InstructorTaskCourseTestCase): self.assertDictContainsSubset({'total': 2, 'attempted': 2, 'succeeded': 1, 'failed': 1}, result) self.verify_rows_in_csv( [ - dict(zip(self.csv_header_row, ['Does Not Exist', 'False', '0', ''])), - dict(zip(self.csv_header_row, ['Cohort 2', 'True', '1', ''])), + dict(zip(self.csv_header_row, ['Does Not Exist', 'False', '0', '', '', ''])), + dict(zip(self.csv_header_row, ['Cohort 2', 'True', '1', '', '', ''])), + ], + verify_order=False + ) + + def test_preassigned_user(self): + result = self._cohort_students_and_upload( + 'username,email,cohort\n' + ',example_email@example.com,Cohort 1' + ) + self.assertDictContainsSubset({'total': 1, 'attempted': 1, 'succeeded': 0, 'failed': 0}, + result) + self.verify_rows_in_csv( + [ + dict(zip(self.csv_header_row, ['Cohort 1', 'True', '0', '', '', 'example_email@example.com'])), + ], + verify_order=False + ) + + def test_invalid_email(self): + result = self._cohort_students_and_upload( + 'username,email,cohort\n' + ',student_1@,Cohort 1\n' + ) + self.assertDictContainsSubset({'total': 1, 'attempted': 1, 'succeeded': 0, 'failed': 1}, result) + self.verify_rows_in_csv( + [ + dict(zip(self.csv_header_row, ['Cohort 1', 'True', '0', '', 'student_1@', ''])), ], verify_order=False ) @@ -1592,7 +1617,7 @@ class TestCohortStudents(TestReportMixin, InstructorTaskCourseTestCase): self.assertDictContainsSubset({'total': 2, 'attempted': 2, 'succeeded': 0, 'failed': 2}, result) self.verify_rows_in_csv( [ - dict(zip(self.csv_header_row, ['', 'False', '0', ''])), + dict(zip(self.csv_header_row, ['', 'False', '0', '', '', ''])), ], verify_order=False ) @@ -1616,8 +1641,8 @@ class TestCohortStudents(TestReportMixin, InstructorTaskCourseTestCase): self.assertDictContainsSubset({'total': 2, 'attempted': 2, 'succeeded': 2, 'failed': 0}, result) self.verify_rows_in_csv( [ - dict(zip(self.csv_header_row, ['Cohort 1', 'True', '1', ''])), - dict(zip(self.csv_header_row, ['Cohort 2', 'True', '1', ''])), + dict(zip(self.csv_header_row, ['Cohort 1', 'True', '1', '', '', ''])), + dict(zip(self.csv_header_row, ['Cohort 2', 'True', '1', '', '', ''])), ], verify_order=False ) @@ -1634,8 +1659,8 @@ class TestCohortStudents(TestReportMixin, InstructorTaskCourseTestCase): self.assertDictContainsSubset({'total': 2, 'attempted': 2, 'succeeded': 2, 'failed': 0}, result) self.verify_rows_in_csv( [ - dict(zip(self.csv_header_row, ['Cohort 1', 'True', '1', ''])), - dict(zip(self.csv_header_row, ['Cohort 2', 'True', '1', ''])), + dict(zip(self.csv_header_row, ['Cohort 1', 'True', '1', '', '', ''])), + dict(zip(self.csv_header_row, ['Cohort 2', 'True', '1', '', '', ''])), ], verify_order=False ) @@ -1654,8 +1679,8 @@ class TestCohortStudents(TestReportMixin, InstructorTaskCourseTestCase): self.assertDictContainsSubset({'total': 2, 'attempted': 2, 'succeeded': 2, 'failed': 0}, result) self.verify_rows_in_csv( [ - dict(zip(self.csv_header_row, ['Cohort 1', 'True', '1', ''])), - dict(zip(self.csv_header_row, ['Cohort 2', 'True', '1', ''])), + dict(zip(self.csv_header_row, ['Cohort 1', 'True', '1', '', '', ''])), + dict(zip(self.csv_header_row, ['Cohort 2', 'True', '1', '', '', ''])), ], verify_order=False ) @@ -1674,8 +1699,8 @@ class TestCohortStudents(TestReportMixin, InstructorTaskCourseTestCase): self.assertDictContainsSubset({'total': 2, 'attempted': 2, 'skipped': 2, 'failed': 0}, result) self.verify_rows_in_csv( [ - dict(zip(self.csv_header_row, ['Cohort 1', 'True', '0', ''])), - dict(zip(self.csv_header_row, ['Cohort 2', 'True', '0', ''])), + dict(zip(self.csv_header_row, ['Cohort 1', 'True', '0', '', '', ''])), + dict(zip(self.csv_header_row, ['Cohort 2', 'True', '0', '', '', ''])), ], verify_order=False ) diff --git a/lms/static/js/groups/views/cohort_editor.js b/lms/static/js/groups/views/cohort_editor.js index 2c65cd0ff6..5081f888d0 100644 --- a/lms/static/js/groups/views/cohort_editor.js +++ b/lms/static/js/groups/views/cohort_editor.js @@ -1,7 +1,8 @@ +/* globals _, NotificationModel, NotificationView, interpolate_text */ (function(define) { 'use strict'; define(['backbone', 'underscore', 'jquery', 'gettext', 'js/groups/views/cohort_form', 'string_utils', - 'js/models/notification', 'js/views/notification'], + 'js/models/notification', 'js/views/notification'], function(Backbone, _, $, gettext, CohortFormView) { var CohortEditorView = Backbone.View.extend({ @@ -24,6 +25,8 @@ errorNotifications: null, // Any confirmation messages that are currently being displayed (for example, number of students added). confirmationNotifications: null, + // Any messages about preassigned email addresses currently being displayed to the instructor. + preassignedNotifications: null, render: function() { this.$el.html(this.template({ @@ -47,13 +50,18 @@ }, selectTab: function(event) { - var tabElement = $(event.currentTarget), - tabName = tabElement.data('tab'); + var $tabElement = $(event.currentTarget), + tabName = $tabElement.data('tab'); event.preventDefault(); this.$('.wrapper-tabs .tab').removeClass('is-selected'); this.$('.wrapper-tabs .tab').find('span.sr').remove(); - tabElement.addClass('is-selected'); - tabElement.find('a').prepend('' + gettext('Selected tab') + ' '); + $tabElement.addClass('is-selected'); + edx.HtmlUtils.prepend( + $($tabElement.find('a')), + edx.HtmlUtils.interpolateHtml(' {selectedTab} ', + {selectedTab: gettext('Selected tab')} + ) + ); this.$('.tab-content').addClass('is-hidden'); this.$('.tab-content-' + tabName).removeClass('is-hidden').focus(); }, @@ -108,7 +116,7 @@ } }); }).fail(function() { - self.showErrorMessage(gettext('Error adding students.'), true); + self.showErrorMessage(gettext('Error adding learners.'), true); }); } else { self.showErrorMessage(gettext('Enter a username or email.'), true); @@ -151,18 +159,20 @@ }, addNotifications: function(modifiedUsers) { - var oldCohort, title, details, numPresent, numUsersAdded, numErrors, - createErrorDetails, errorActionCallback, errorModel, + var oldCohort, title, details, numPresent, numUsersAdded, numPreassigned, + numErrors, createErrorDetails, errorActionCallback, errorModel, i, errorLimit = 5; // Show confirmation messages. this.undelegateViewEvents(this.confirmationNotifications); numUsersAdded = modifiedUsers.added.length + modifiedUsers.changed.length; numPresent = modifiedUsers.present.length; + numPreassigned = modifiedUsers.preassigned.length; + title = ''; if (numUsersAdded > 0 || numPresent > 0) { - title = interpolate_text( - ngettext('{numUsersAdded} student has been added to this cohort', - '{numUsersAdded} students have been added to this cohort', numUsersAdded), + title += interpolate_text( + ngettext('{numUsersAdded} learner has been added to this cohort. ', + '{numUsersAdded} learners have been added to this cohort. ', numUsersAdded), {numUsersAdded: numUsersAdded} ); @@ -171,27 +181,28 @@ oldCohort = changedInfo.previous_cohort; if (oldCohort in movedByCohort) { movedByCohort[oldCohort] = movedByCohort[oldCohort] + 1; - } - else { + } else { movedByCohort[oldCohort] = 1; } }); details = []; - for (oldCohort in movedByCohort) { + + _.each(movedByCohort, function(numMoved, prevCohort) { details.push( interpolate_text( - ngettext('{numMoved} student was removed from {oldCohort}', - '{numMoved} students were removed from {oldCohort}', movedByCohort[oldCohort]), - {numMoved: movedByCohort[oldCohort], oldCohort: oldCohort} + ngettext('{numMoved} learner was moved from {prevCohort}', + '{numMoved} learners were moved from {prevCohort}', numMoved), + {numMoved: numMoved, prevCohort: prevCohort} ) ); - } + }); + if (numPresent > 0) { details.push( interpolate_text( - ngettext('{numPresent} student was already in the cohort', - '{numPresent} students were already in the cohort', numPresent), + ngettext('{numPresent} learner was already in the cohort', + '{numPresent} learners were already in the cohort', numPresent), {numPresent: numPresent} ) ); @@ -206,35 +217,81 @@ }) }); this.confirmationNotifications.render(); - } - else if (this.confirmationNotifications) { + } else if (this.confirmationNotifications) { this.confirmationNotifications.$el.html(''); this.confirmationNotifications = null; } + // Show preassigned email addresses. + this.undelegateViewEvents(this.preassignedNotifications); + if (numPreassigned > 0) { + details = []; + for (i = 0; i < modifiedUsers.preassigned.length; i++) { + details.push(interpolate_text(gettext('{email}'), + {email: modifiedUsers.preassigned[i]})); + } + + title = ( + interpolate_text( + ngettext('{numPreassigned} learner was pre-assigned for this cohort. ' + + 'This learner will automatically be added to the cohort when ' + + 'they enroll in the course.', + '{numPreassigned} learners were pre-assigned for this cohort. ' + + 'These learners will automatically be added to the cohort when ' + + 'they enroll in the course.', + numPreassigned), + {numPreassigned: numPreassigned} + ) + ); + + this.preassignedNotifications = new NotificationView({ + el: this.$('.cohort-preassigned'), + model: new NotificationModel({ + type: 'warning', + title: title, + details: details + }) + }); + this.preassignedNotifications.render(); + } else if (this.preassignedNotifications) { + this.preassignedNotifications.$el.html(''); + this.preassignedNotifications = null; + } + // Show error messages. this.undelegateViewEvents(this.errorNotifications); - numErrors = modifiedUsers.unknown.length; + numErrors = modifiedUsers.unknown.length + modifiedUsers.invalid.length; if (numErrors > 0) { - createErrorDetails = function(unknownUsers, showAllErrors) { - var numErrors = unknownUsers.length, details = []; + createErrorDetails = function(unknownUsers, invalidEmails, showAllErrors) { + var unknownErrorsShown = showAllErrors ? unknownUsers.length : + Math.min(errorLimit, unknownUsers.length); + var invalidErrorsShown = showAllErrors ? invalidEmails.length : + Math.min(errorLimit - unknownUsers.length, invalidEmails.length); + details = []; - for (var i = 0; i < (showAllErrors ? numErrors : Math.min(errorLimit, numErrors)); i++) { - details.push(interpolate_text(gettext('Unknown user: {user}'), {user: unknownUsers[i]})); + + for (i = 0; i < unknownErrorsShown; i++) { + details.push(interpolate_text(gettext('Unknown username: {user}'), + {user: unknownUsers[i]})); + } + for (i = 0; i < invalidErrorsShown; i++) { + details.push(interpolate_text(gettext('Invalid email address: {email}'), + {email: invalidEmails[i]})); } return details; }; title = interpolate_text( - ngettext('There was an error when trying to add students:', - 'There were {numErrors} errors when trying to add students:', numErrors), + ngettext('There was an error when trying to add learners:', + '{numErrors} learners could not be added to this cohort:', numErrors), {numErrors: numErrors} ); - details = createErrorDetails(modifiedUsers.unknown, false); + details = createErrorDetails(modifiedUsers.unknown, modifiedUsers.invalid, false); errorActionCallback = function(view) { view.model.set('actionText', null); - view.model.set('details', createErrorDetails(modifiedUsers.unknown, true)); + view.model.set('details', + createErrorDetails(modifiedUsers.unknown, modifiedUsers.invalid, true)); view.render(); }; diff --git a/lms/static/js/spec/groups/views/cohorts_spec.js b/lms/static/js/spec/groups/views/cohorts_spec.js index 5b8a8855a5..2485dc4027 100644 --- a/lms/static/js/spec/groups/views/cohorts_spec.js +++ b/lms/static/js/spec/groups/views/cohorts_spec.js @@ -1,3 +1,5 @@ +/* globals _ */ + define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', 'common/js/spec_helpers/template_helpers', 'js/groups/views/cohorts', 'js/groups/collections/cohort', 'js/groups/models/content_group', @@ -10,11 +12,11 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers describe('Cohorts View', function() { var catLoversInitialCount = 123, dogLoversInitialCount = 456, unknownUserMessage, - createMockCohort, createMockCohorts, createMockContentGroups, createMockCohortSettingsJson, - createMockVerifiedTrackCohortsJson, flushVerifiedTrackCohortRequests, createCohortsView, - cohortsView, requests, respondToRefresh, verifyMessage, verifyNoMessage, verifyDetailedMessage, - verifyHeader, verifyVerifiedTrackMessage, verifyVerifiedTrackUIUpdates, expectCohortAddRequest, - getAddModal, selectContentGroup, clearContentGroup, + invalidEmailMessage, createMockCohort, createMockCohorts, createMockContentGroups, + createMockCohortSettingsJson, createMockVerifiedTrackCohortsJson, flushVerifiedTrackCohortRequests, + createCohortsView, cohortsView, requests, respondToRefresh, verifyMessage, verifyNoMessage, + verifyDetailedMessage, verifyHeader, verifyVerifiedTrackMessage, verifyVerifiedTrackUIUpdates, + expectCohortAddRequest, getAddModal, selectContentGroup, clearContentGroup, saveFormAndExpectErrors, createMockCohortSettings, MOCK_COHORTED_USER_PARTITION_ID, MOCK_UPLOAD_COHORTS_CSV_URL, MOCK_STUDIO_ADVANCED_SETTINGS_URL, MOCK_STUDIO_GROUP_CONFIGURATIONS_URL, MOCK_VERIFIED_TRACK_COHORTING_URL, MOCK_MANUAL_ASSIGNMENT, MOCK_RANDOM_ASSIGNMENT; @@ -249,7 +251,11 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers }; unknownUserMessage = function(name) { - return 'Unknown user: ' + name; + return 'Unknown username: ' + name; + }; + + invalidEmailMessage = function(name) { + return 'Invalid email address: ' + name; }; beforeEach(function() { @@ -299,7 +305,7 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers expect(cohortsView.$(fileUploadFormCss).length).toBe(0); uploadCsvToggle = cohortsView.$('.toggle-cohort-management-secondary'); expect(uploadCsvToggle.text()). - toContain('Assign students to cohorts by uploading a CSV file'); + toContain('Assign learners to cohorts by uploading a CSV file'); uploadCsvToggle.click(); // After toggle is clicked, it should be hidden. expect(uploadCsvToggle).toHaveClass('hidden'); @@ -690,7 +696,8 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers respondToAdd = function(result) { AjaxHelpers.respondWithJson( requests, - _.extend({unknown: [], added: [], present: [], changed: [], success: true}, result) + _.extend({unknown: [], added: [], present: [], changed: [], + success: true, preassigned: [], invalid: []}, result) ); }; @@ -709,27 +716,57 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers respondToAdd({added: ['student@sample.com']}); respondToRefresh(catLoversUpdatedCount, dogLoversInitialCount); verifyHeader(1, 'Cat Lovers', catLoversUpdatedCount); - verifyMessage('1 student has been added to this cohort', 'confirmation'); + verifyMessage('1 learner has been added to this cohort.', 'confirmation'); expect(getStudentInput().val()).toBe(''); }); - it('shows an error when adding a student that does not exist', function() { + it('preassigns an email address if it is not associated with a user', function() { createCohortsView(this, {selectCohort: 1}); addStudents('unknown@sample.com'); AjaxHelpers.expectRequest( requests, 'POST', '/mock_service/cohorts/1/add', 'users=unknown%40sample.com' ); - respondToAdd({unknown: ['unknown@sample.com']}); + respondToAdd({preassigned: ['unknown@sample.com']}); respondToRefresh(catLoversInitialCount, dogLoversInitialCount); verifyHeader(1, 'Cat Lovers', catLoversInitialCount); - verifyDetailedMessage('There was an error when trying to add students:', 'error', - [unknownUserMessage('unknown@sample.com')] - ); - expect(getStudentInput().val()).toBe('unknown@sample.com'); + verifyDetailedMessage('1 learner was pre-assigned for this cohort. ' + + 'This learner will automatically be added to the cohort when they enroll in the course.', + 'warning', + ['unknown@sample.com']); + expect(getStudentInput().val()).toBe(''); }); + it('shows an error when adding an invalid email address', function() { + createCohortsView(this, {selectCohort: 1}); + addStudents('unknown@'); + AjaxHelpers.expectRequest( + requests, 'POST', '/mock_service/cohorts/1/add', 'users=unknown%40' + ); + respondToAdd({invalid: ['unknown@']}); + respondToRefresh(catLoversInitialCount, dogLoversInitialCount); + verifyHeader(1, 'Cat Lovers', catLoversInitialCount); + verifyDetailedMessage('There was an error when trying to add learners:', 'error', + [invalidEmailMessage('unknown@')] + ); + }); + + it('shows an error when adding an unknown user', function() { + createCohortsView(this, {selectCohort: 1}); + addStudents('unknown'); + AjaxHelpers.expectRequest( + requests, 'POST', '/mock_service/cohorts/1/add', 'users=unknown' + ); + respondToAdd({unknown: ['unknown']}); + respondToRefresh(catLoversInitialCount, dogLoversInitialCount); + verifyHeader(1, 'Cat Lovers', catLoversInitialCount); + verifyDetailedMessage('There was an error when trying to add learners:', 'error', + [unknownUserMessage('unknown')] + ); + }); + + it('shows a "view all" button when more than 5 students do not exist', function() { - var sixUsers = 'unknown1@sample.com, unknown2@sample.com, unknown3@sample.com, unknown4@sample.com, unknown5@sample.com, unknown6@sample.com'; + var sixUsers = 'unknown1, unknown2, unknown3, unknown4, unknown5, unknown6'; createCohortsView(this, {selectCohort: 1}); addStudents(sixUsers); @@ -738,30 +775,30 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers 'users=' + sixUsers.replace(/@/g, '%40').replace(/, /g, '%2C+') ); respondToAdd({unknown: [ - 'unknown1@sample.com', - 'unknown2@sample.com', - 'unknown3@sample.com', - 'unknown4@sample.com', - 'unknown5@sample.com', - 'unknown6@sample.com'] + 'unknown1', + 'unknown2', + 'unknown3', + 'unknown4', + 'unknown5', + 'unknown6'] }); respondToRefresh(catLoversInitialCount + 6, dogLoversInitialCount); - verifyDetailedMessage('There were 6 errors when trying to add students:', 'error', + verifyDetailedMessage('6 learners could not be added to this cohort:', 'error', [ - unknownUserMessage('unknown1@sample.com'), unknownUserMessage('unknown2@sample.com'), - unknownUserMessage('unknown3@sample.com'), unknownUserMessage('unknown4@sample.com'), - unknownUserMessage('unknown5@sample.com') + unknownUserMessage('unknown1'), unknownUserMessage('unknown2'), + unknownUserMessage('unknown3'), unknownUserMessage('unknown4'), + unknownUserMessage('unknown5') ], 'View all errors' ); expect(getStudentInput().val()).toBe(sixUsers); // Click "View all" cohortsView.$('.action-expand').click(); - verifyDetailedMessage('There were 6 errors when trying to add students:', 'error', + verifyDetailedMessage('6 learners could not be added to this cohort:', 'error', [ - unknownUserMessage('unknown1@sample.com'), unknownUserMessage('unknown2@sample.com'), - unknownUserMessage('unknown3@sample.com'), unknownUserMessage('unknown4@sample.com'), - unknownUserMessage('unknown5@sample.com'), unknownUserMessage('unknown6@sample.com') + unknownUserMessage('unknown1'), unknownUserMessage('unknown2'), + unknownUserMessage('unknown3'), unknownUserMessage('unknown4'), + unknownUserMessage('unknown5'), unknownUserMessage('unknown6') ] ); }); @@ -784,11 +821,11 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers }); respondToRefresh(); - verifyDetailedMessage('3 students have been added to this cohort', 'confirmation', + verifyDetailedMessage('3 learners have been added to this cohort.', 'confirmation', [ - '2 students were removed from cohort 2', - '1 student was removed from cohort 3', - '1 student was already in the cohort' + '2 learners were moved from cohort 2', + '1 learner was moved from cohort 3', + '1 learner was already in the cohort' ] ); expect(getStudentInput().val()).toBe(''); @@ -798,7 +835,7 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers createCohortsView(this, {selectCohort: 1}); addStudents('student@sample.com'); AjaxHelpers.respondWithError(requests); - verifyMessage('Error adding students.', 'error'); + verifyMessage('Error adding learners.', 'error'); expect(getStudentInput().val()).toBe('student@sample.com'); }); @@ -808,13 +845,13 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers // First verify that an error is shown addStudents('student@sample.com'); AjaxHelpers.respondWithError(requests); - verifyMessage('Error adding students.', 'error'); + verifyMessage('Error adding learners.', 'error'); // Now verify that the error is removed on a subsequent add addStudents('student@sample.com'); respondToAdd({added: ['student@sample.com']}); respondToRefresh(catLoversInitialCount + 1, dogLoversInitialCount); - verifyMessage('1 student has been added to this cohort', 'confirmation'); + verifyMessage('1 learner has been added to this cohort.', 'confirmation'); }); }); diff --git a/lms/templates/instructor/instructor_dashboard_2/cohort-editor.underscore b/lms/templates/instructor/instructor_dashboard_2/cohort-editor.underscore index 688155c9a4..806947ac5c 100644 --- a/lms/templates/instructor/instructor_dashboard_2/cohort-editor.underscore +++ b/lms/templates/instructor/instructor_dashboard_2/cohort-editor.underscore @@ -2,26 +2,27 @@
-

<%- gettext('Add students to this cohort') %>

+

<%- gettext('Add learners to this cohort') %>

-

<%- gettext('Note: Students can be in only one cohort. Adding students to this group overrides any previous group assignment.') %>

+

<%- gettext('Note: Learners can be in only one cohort. Adding learners to this group overrides any previous group assignment.') %>

+