diff --git a/common/djangoapps/course_groups/cohorts.py b/common/djangoapps/course_groups/cohorts.py index 6c18491775..dfede1147a 100644 --- a/common/djangoapps/course_groups/cohorts.py +++ b/common/djangoapps/course_groups/cohorts.py @@ -117,6 +117,12 @@ def add_cohort(course_id, name): group_type=CourseUserGroup.COHORT, name=name) +class CohortConflict(Exception): + """ + Raised when user to be added is already in another cohort in same course. + """ + pass + def add_user_to_cohort(cohort, username_or_email): """ Look up the given user, and if successful, add them to the specified cohort. @@ -131,15 +137,29 @@ def add_user_to_cohort(cohort, username_or_email): Raises: User.DoesNotExist if can't find user. - ValueError if user already present. + ValueError if user already present in this cohort. + + CohortConflict if user already in another cohort. """ if '@' in username_or_email: user = User.objects.get(email=username_or_email) else: user = User.objects.get(username=username_or_email) - if cohort.users.filter(id=user.id).exists(): - raise ValueError("User {0} already present".format(user.username)) + # If user in any cohorts in this course already, complain + course_cohorts = CourseUserGroup.objects.filter( + course_id=cohort.course_id, + users__id=user.id, + group_type=CourseUserGroup.COHORT) + if course_cohorts.exists(): + if course_cohorts[0] == cohort: + raise ValueError("User {0} already present in cohort {1}".format( + user.username, + cohort.name)) + else: + raise CohortConflict("User {0} is in another cohort {1} in course" + .format(user.username, + course_cohorts[0].name)) cohort.users.add(user) return user diff --git a/common/djangoapps/course_groups/views.py b/common/djangoapps/course_groups/views.py index e82532ae50..d64622dcb0 100644 --- a/common/djangoapps/course_groups/views.py +++ b/common/djangoapps/course_groups/views.py @@ -133,6 +133,8 @@ def add_users_to_cohort(request, course_id, cohort_id): 'added': [{'username': username, 'name': name, 'email': email}, ...], + 'conflict': [{'username_or_email': ..., + 'msg': ...}], # in another cohort 'present': [str1, str2, ...], # already there 'unknown': [str1, str2, ...]} """ @@ -146,6 +148,7 @@ def add_users_to_cohort(request, course_id, cohort_id): users = request.POST.get('users', '') added = [] present = [] + conflict = [] unknown = [] for username_or_email in split_by_comma_and_whitespace(users): try: @@ -158,10 +161,15 @@ def add_users_to_cohort(request, course_id, cohort_id): present.append(username_or_email) except User.DoesNotExist: unknown.append(username_or_email) + except cohorts.CohortConflict as err: + conflict.append({'username_or_email': username_or_email, + 'msg': str(err)}) + return JsonHttpReponse({'success': True, 'added': added, 'present': present, + 'conflict': conflict, 'unknown': unknown}) @ensure_csrf_cookie diff --git a/common/static/js/course_groups/cohorts.js b/common/static/js/course_groups/cohorts.js index ada0b16bd5..aa3ce34b5b 100644 --- a/common/static/js/course_groups/cohorts.js +++ b/common/static/js/course_groups/cohorts.js @@ -166,8 +166,10 @@ var CohortManager = (function ($) { function adder(note, color) { return function(item) { var li = $('
  • ') - if (typeof item === "object") { + if (typeof item === "object" && item.username) { li.text(note + ' ' + item.name + ', ' + item.username + ', ' + item.email); + } else if (typeof item === "object" && item.msg) { + li.text(note + ' ' + item.username_or_email + ', ' + item.msg); } else { // string li.text(note + ' ' + item); @@ -176,10 +178,13 @@ var CohortManager = (function ($) { op_results.append(li); } } + op_results.empty(); if (response && response.success) { response.added.forEach(adder("Added", "green")); response.present.forEach(adder("Already present:", "black")); + response.conflict.forEach(adder("In another cohort:", "purple")); response.unknown.forEach(adder("Unknown user:", "red")); + users_area.val('') } else { log_error(response.msg || "There was an error adding users"); }