Skip to content

E
edx-platform
Commit 0fb3a7d2 by Adam
Options

Merge pull request #3282 from edx/hotfix/2014-04-09 

Hotfix 2014-04-09
parents eb4096f1 5f337df6
Showing with 245 additions and 43 deletions
common/djangoapps/course_groups/cohorts.py
......@@ -226,18 +226,16 @@ def add_user_to_cohort(cohort, username_or_email):
username_or_email: string. Treated as email if has '@'
Returns:
User object.
Tuple of User object and string (or None) indicating previous cohort
Raises:
User.DoesNotExist if can't find user.
ValueError if user already present in this cohort.
CohortConflict if user already in another cohort.
"""
user = get_user_by_username_or_email(username_or_email)
previous_cohort = None
# If user in any cohorts in this course already, complain
course_cohorts = CourseUserGroup.objects.filter(
course_id=cohort.course_id,
users__id=user.id,
......@@ -248,12 +246,11 @@ def add_user_to_cohort(cohort, username_or_email):
user.username,
cohort.name))
else:
raise CohortConflict("User {0} is in another cohort {1} in course"
.format(user.username,
course_cohorts[0].name))
previous_cohort = course_cohorts[0].name
course_cohorts[0].users.remove(user)
cohort.users.add(user)
return user
return (user, previous_cohort)
def get_course_cohort_names(course_id):
......
common/djangoapps/course_groups/tests/test_views.py 0 → 100644
import json
from django.test.client import RequestFactory
from django.test.utils import override_settings
from factory import post_generation, Sequence
from factory.django import DjangoModelFactory
from course_groups.models import CourseUserGroup
from course_groups.views import add_users_to_cohort
from courseware.tests.tests import TEST_DATA_MIXED_MODULESTORE
from student.tests.factories import UserFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
class CohortFactory(DjangoModelFactory):
FACTORY_FOR = CourseUserGroup
name = Sequence("cohort{}".format)
course_id = "dummy_id"
group_type = CourseUserGroup.COHORT
@post_generation
def users(self, create, extracted, **kwargs): # pylint: disable=W0613
self.users.add(*extracted)
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
class AddUsersToCohortTestCase(ModuleStoreTestCase):
def setUp(self):
self.course = CourseFactory.create()
self.staff_user = UserFactory.create(is_staff=True)
self.cohort1_users = [UserFactory.create() for _ in range(3)]
self.cohort2_users = [UserFactory.create() for _ in range(2)]
self.cohort3_users = [UserFactory.create() for _ in range(2)]
self.cohortless_users = [UserFactory.create() for _ in range(3)]
self.cohort1 = CohortFactory.create(course_id=self.course.id, users=self.cohort1_users)
self.cohort2 = CohortFactory.create(course_id=self.course.id, users=self.cohort2_users)
self.cohort3 = CohortFactory.create(course_id=self.course.id, users=self.cohort3_users)
def check_request(
self,
users_string,
expected_added=None,
expected_changed=None,
expected_present=None,
expected_unknown=None
):
"""
Check that add_users_to_cohort returns the expected result and has the
expected side effects. The given users will be added to cohort1.
users_string is the string input entered by the client
expected_added is a list of users
expected_changed is a list of (user, previous_cohort) tuples
expected_present is a list of (user, email/username) tuples where
email/username corresponds to the input
expected_unknown is a list of strings corresponding to the input
"""
expected_added = expected_added or []
expected_changed = expected_changed or []
expected_present = expected_present or []
expected_unknown = expected_unknown or []
request = RequestFactory().post("dummy_url", {"users": users_string})
request.user = self.staff_user
response = add_users_to_cohort(request, self.course.id, self.cohort1.id)
self.assertEqual(response.status_code, 200)
result = json.loads(response.content)
self.assertEqual(result.get("success"), True)
self.assertItemsEqual(
result.get("added"),
[
{"username": user.username, "name": user.profile.name, "email": user.email}
for user in expected_added
]
)
self.assertItemsEqual(
result.get("changed"),
[
{
"username": user.username,
"name": user.profile.name,
"email": user.email,
"previous_cohort": previous_cohort
}
for (user, previous_cohort) in expected_changed
]
)
self.assertItemsEqual(
result.get("present"),
[username_or_email for (_, username_or_email) in expected_present]
)
self.assertItemsEqual(result.get("unknown"), expected_unknown)
for user in expected_added + [user for (user, _) in expected_changed + expected_present]:
self.assertEqual(
CourseUserGroup.objects.get(
course_id=self.course.id,
group_type=CourseUserGroup.COHORT,
users__id=user.id
),
self.cohort1
)
def test_empty(self):
self.check_request("")
def test_only_added(self):
self.check_request(
",".join([user.username for user in self.cohortless_users]),
expected_added=self.cohortless_users
)
def test_only_changed(self):
self.check_request(
",".join([user.username for user in self.cohort2_users + self.cohort3_users]),
expected_changed=(
[(user, self.cohort2.name) for user in self.cohort2_users] +
[(user, self.cohort3.name) for user in self.cohort3_users]
)
)
def test_only_present(self):
usernames = [user.username for user in self.cohort1_users]
self.check_request(
",".join(usernames),
expected_present=[(user, user.username) for user in self.cohort1_users]
)
def test_only_unknown(self):
usernames = ["unknown_user{}".format(i) for i in range(3)]
self.check_request(
",".join(usernames),
expected_unknown=usernames
)
def test_all(self):
unknowns = ["unknown_user{}".format(i) for i in range(3)]
self.check_request(
",".join(
unknowns +
[
user.username
for user in self.cohortless_users + self.cohort1_users + self.cohort2_users + self.cohort3_users
]
),
self.cohortless_users,
(
[(user, self.cohort2.name) for user in self.cohort2_users] +
[(user, self.cohort3.name) for user in self.cohort3_users]
),
[(user, user.username) for user in self.cohort1_users],
unknowns
)
def test_emails(self):
unknown = "unknown_user@example.com"
self.check_request(
",".join([
self.cohort1_users[0].email,
self.cohort2_users[0].email,
self.cohortless_users[0].email,
unknown
]),
[self.cohortless_users[0]],
[(self.cohort2_users[0], self.cohort2.name)],
[(self.cohort1_users[0], self.cohort1_users[0].email)],
[unknown]
)
def test_delimiters(self):
unknown = "unknown_user"
self.check_request(
" {} {}\t{}, \r\n{}".format(
unknown,
self.cohort1_users[0].username,
self.cohort2_users[0].username,
self.cohortless_users[0].username
),
[self.cohortless_users[0]],
[(self.cohort2_users[0], self.cohort2.name)],
[(self.cohort1_users[0], self.cohort1_users[0].username)],
[unknown]
)
common/djangoapps/course_groups/views.py
......@@ -134,11 +134,13 @@ def add_users_to_cohort(request, course_id, cohort_id):
Return json dict of:
{'success': True,
'added': [{'username': username,
'name': name,
'email': email}, ...],
'conflict': [{'username_or_email': ...,
'msg': ...}], # in another cohort
'added': [{'username': ...,
'name': ...,
'email': ...}, ...],
'changed': [{'username': ...,
'name': ...,
'email': ...,
'previous_cohort': ...}, ...],
'present': [str1, str2, ...], # already there
'unknown': [str1, str2, ...]}
"""
......@@ -148,29 +150,34 @@ def add_users_to_cohort(request, course_id, cohort_id):
users = request.POST.get('users', '')
added = []
changed = []
present = []
conflict = []
unknown = []
for username_or_email in split_by_comma_and_whitespace(users):
if not username_or_email:
continue
try:
user = cohorts.add_user_to_cohort(cohort, username_or_email)
added.append({'username': user.username,
'name': "{0} {1}".format(user.first_name, user.last_name),
'email': user.email,
})
(user, previous_cohort) = cohorts.add_user_to_cohort(cohort, username_or_email)
info = {
'username': user.username,
'name': user.profile.name,
'email': user.email,
}
if previous_cohort:
info['previous_cohort'] = previous_cohort
changed.append(info)
else:
added.append(info)
except ValueError:
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 json_http_response({'success': True,
'added': added,
'changed': changed,
'present': present,
'conflict': conflict,
'unknown': unknown})
......
common/static/js/course_groups/cohorts.js
......@@ -160,32 +160,43 @@ var CohortManager = (function ($) {
log_error(response.msg ||
"There was an error loading users for " + cohort.title);
}
op_results.empty();
detail.show();
}
function added_users(response) {
function adder(note, color) {
return function(item) {
var li = $('<li></li>')
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);
}
li.css('color', color);
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"));
function add_to_list(text, color) {
op_results.append($("<li/>").text(text).css("color", color));
}
response.added.forEach(
function(item) {
add_to_list(
"Added: " + item.name + ", " + item.username + ", " + item.email,
"green"
);
}
);
response.changed.forEach(
function(item) {
add_to_list(
"Moved from cohort " + item.previous_cohort + ": " + item.name + ", " + item.username + ", " + item.email,
"purple"
)
}
);
response.present.forEach(
function(item) {
add_to_list("Already present: " + item, "black");
}
);
response.unknown.forEach(
function(item) {
add_to_list("Unknown user: " + item, "red")
}
);
users_area.val('')
} else {
log_error(response.msg || "There was an error adding users");
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment