Merge branch 'master' of github.com:MITx/mitx

This commit is contained in:
John Hess
2013-02-25 18:42:39 -05:00
40 changed files with 598 additions and 67 deletions

View File

@@ -6,6 +6,7 @@ forums, and to the cohort admin views.
from django.contrib.auth.models import User
from django.http import Http404
import logging
import random
from courseware import courses
from student.models import get_user_by_username_or_email
@@ -64,7 +65,23 @@ def is_commentable_cohorted(course_id, commentable_id):
ans))
return ans
def get_cohorted_commentables(course_id):
"""
Given a course_id return a list of strings representing cohorted commentables
"""
course = courses.get_course_by_id(course_id)
if not course.is_cohorted:
# this is the easy case :)
ans = []
else:
ans = course.cohorted_discussions
return ans
def get_cohort(user, course_id):
"""
Given a django User and a course_id, return the user's cohort in that
@@ -96,9 +113,30 @@ def get_cohort(user, course_id):
group_type=CourseUserGroup.COHORT,
users__id=user.id)
except CourseUserGroup.DoesNotExist:
# TODO: add auto-cohorting logic here once we know what that will be.
# Didn't find the group. We'll go on to create one if needed.
pass
if not course.auto_cohort:
return None
choices = course.auto_cohort_groups
if len(choices) == 0:
# Nowhere to put user
log.warning("Course %s is auto-cohorted, but there are no"
" auto_cohort_groups specified",
course_id)
return None
# Put user in a random group, creating it if needed
group_name = random.choice(choices)
group, created = CourseUserGroup.objects.get_or_create(
course_id=course_id,
group_type=CourseUserGroup.COHORT,
name=group_name)
user.course_groups.add(group)
return group
def get_course_cohorts(course_id):
"""

View File

@@ -47,7 +47,10 @@ class TestCohorts(django.test.TestCase):
@staticmethod
def config_course_cohorts(course, discussions,
cohorted, cohorted_discussions=None):
cohorted,
cohorted_discussions=None,
auto_cohort=None,
auto_cohort_groups=None):
"""
Given a course with no discussion set up, add the discussions and set
the cohort config appropriately.
@@ -59,6 +62,9 @@ class TestCohorts(django.test.TestCase):
cohorted: bool.
cohorted_discussions: optional list of topic names. If specified,
converts them to use the same ids as topic names.
auto_cohort: optional bool.
auto_cohort_groups: optional list of strings
(names of groups to put students into).
Returns:
Nothing -- modifies course in place.
@@ -76,6 +82,12 @@ class TestCohorts(django.test.TestCase):
if cohorted_discussions is not None:
d["cohorted_discussions"] = [to_id(name)
for name in cohorted_discussions]
if auto_cohort is not None:
d["auto_cohort"] = auto_cohort
if auto_cohort_groups is not None:
d["auto_cohort_groups"] = auto_cohort_groups
course.metadata["cohort_config"] = d
@@ -89,12 +101,9 @@ class TestCohorts(django.test.TestCase):
def test_get_cohort(self):
# Need to fix this, but after we're testing on staging. (Looks like
# problem is that when get_cohort internally tries to look up the
# course.id, it fails, even though we loaded it through the modulestore.
# Proper fix: give all tests a standard modulestore that uses the test
# dir.
"""
Make sure get_cohort() does the right thing when the course is cohorted
"""
course = modulestore().get_course("edX/toy/2012_Fall")
self.assertEqual(course.id, "edX/toy/2012_Fall")
self.assertFalse(course.is_cohorted)
@@ -122,6 +131,54 @@ class TestCohorts(django.test.TestCase):
self.assertEquals(get_cohort(other_user, course.id), None,
"other_user shouldn't have a cohort")
def test_auto_cohorting(self):
"""
Make sure get_cohort() does the right thing when the course is auto_cohorted
"""
course = modulestore().get_course("edX/toy/2012_Fall")
self.assertEqual(course.id, "edX/toy/2012_Fall")
self.assertFalse(course.is_cohorted)
user1 = User.objects.create(username="test", email="a@b.com")
user2 = User.objects.create(username="test2", email="a2@b.com")
user3 = User.objects.create(username="test3", email="a3@b.com")
cohort = CourseUserGroup.objects.create(name="TestCohort",
course_id=course.id,
group_type=CourseUserGroup.COHORT)
# user1 manually added to a cohort
cohort.users.add(user1)
# Make the course auto cohorted...
self.config_course_cohorts(course, [], cohorted=True,
auto_cohort=True,
auto_cohort_groups=["AutoGroup"])
self.assertEquals(get_cohort(user1, course.id).id, cohort.id,
"user1 should stay put")
self.assertEquals(get_cohort(user2, course.id).name, "AutoGroup",
"user2 should be auto-cohorted")
# Now make the group list empty
self.config_course_cohorts(course, [], cohorted=True,
auto_cohort=True,
auto_cohort_groups=[])
self.assertEquals(get_cohort(user3, course.id), None,
"No groups->no auto-cohorting")
# Now make it different
self.config_course_cohorts(course, [], cohorted=True,
auto_cohort=True,
auto_cohort_groups=["OtherGroup"])
self.assertEquals(get_cohort(user3, course.id).name, "OtherGroup",
"New list->new group")
self.assertEquals(get_cohort(user2, course.id).name, "AutoGroup",
"user2 should still be in originally placed cohort")
def test_get_course_cohorts(self):
course1_id = 'a/b/c'

View File

@@ -429,6 +429,11 @@ class CapaModule(XModule):
# used by conditional module
return self.attempts > 0
def is_correct(self):
"""True if full points"""
d = self.get_score()
return d['score'] == d['total']
def answer_available(self):
'''
Is the user allowed to see an answer?
@@ -449,6 +454,9 @@ class CapaModule(XModule):
return self.lcp.done
elif self.show_answer == 'closed':
return self.closed()
elif self.show_answer == 'finished':
return self.closed() or self.is_correct()
elif self.show_answer == 'past_due':
return self.is_past_due()
elif self.show_answer == 'always':

View File

@@ -378,6 +378,28 @@ class CourseDescriptor(SequenceDescriptor):
return bool(config.get("cohorted"))
@property
def auto_cohort(self):
"""
Return whether the course is auto-cohorted.
"""
if not self.is_cohorted:
return False
return bool(self.metadata.get("cohort_config", {}).get(
"auto_cohort", False))
@property
def auto_cohort_groups(self):
"""
Return the list of groups to put students into. Returns [] if not
specified. Returns specified list even if is_cohorted and/or auto_cohort are
false.
"""
return self.metadata.get("cohort_config", {}).get(
"auto_cohort_groups", [])
@property
def top_level_discussion_topic_ids(self):
"""
@@ -714,7 +736,7 @@ class CourseDescriptor(SequenceDescriptor):
def get_test_center_exam(self, exam_series_code):
exams = [exam for exam in self.test_center_exams if exam.exam_series_code == exam_series_code]
return exams[0] if len(exams) == 1 else None
@property
def title(self):
return self.display_name

View File

@@ -23,6 +23,15 @@ URL_RE = re.compile("""
(@(?P<revision>[^/]+))?
""", re.VERBOSE)
MISSING_SLASH_URL_RE = re.compile("""
(?P<tag>[^:]+):/
(?P<org>[^/]+)/
(?P<course>[^/]+)/
(?P<category>[^/]+)/
(?P<name>[^@]+)
(@(?P<revision>[^/]+))?
""", re.VERBOSE)
# TODO (cpennington): We should decide whether we want to expand the
# list of valid characters in a location
INVALID_CHARS = re.compile(r"[^\w.-]")
@@ -164,12 +173,16 @@ class Location(_LocationBase):
if isinstance(location, basestring):
match = URL_RE.match(location)
if match is None:
log.debug('location is instance of %s but no URL match' % basestring)
raise InvalidLocationError(location)
else:
groups = match.groupdict()
check_dict(groups)
return _LocationBase.__new__(_cls, **groups)
# cdodge:
# check for a dropped slash near the i4x:// element of the location string. This can happen with some
# redirects (e.g. edx.org -> www.edx.org which I think happens in Nginx)
match = MISSING_SLASH_URL_RE.match(location)
if match is None:
log.debug('location is instance of %s but no URL match' % basestring)
raise InvalidLocationError(location)
groups = match.groupdict()
check_dict(groups)
return _LocationBase.__new__(_cls, **groups)
elif isinstance(location, (list, tuple)):
if len(location) not in (5, 6):
log.debug('location has wrong length')

View File

@@ -42,6 +42,7 @@ class CapaFactory(object):
force_save_button=None,
attempts=None,
problem_state=None,
correct=False
):
"""
All parameters are optional, and are added to the created problem if specified.
@@ -58,6 +59,7 @@ class CapaFactory(object):
module.
attempts: also added to instance state. Will be converted to an int.
correct: if True, the problem will be initialized to be answered correctly.
"""
definition = {'data': CapaFactory.sample_problem_xml, }
location = Location(["i4x", "edX", "capa_test", "problem",
@@ -81,10 +83,19 @@ class CapaFactory(object):
instance_state_dict = {}
if problem_state is not None:
instance_state_dict = problem_state
if attempts is not None:
# converting to int here because I keep putting "0" and "1" in the tests
# since everything else is a string.
instance_state_dict['attempts'] = int(attempts)
if correct:
# TODO: make this actually set an answer of 3.14, and mark it correct
#instance_state_dict['student_answers'] = {}
#instance_state_dict['correct_map'] = {}
pass
if len(instance_state_dict) > 0:
instance_state = json.dumps(instance_state_dict)
else:
@@ -94,13 +105,16 @@ class CapaFactory(object):
definition, descriptor,
instance_state, None, metadata=metadata)
if correct:
# TODO: probably better to actually set the internal state properly, but...
module.get_score = lambda: {'score': 1, 'total': 1}
return module
class CapaModuleTest(unittest.TestCase):
def setUp(self):
now = datetime.datetime.now()
day_delta = datetime.timedelta(days=1)
@@ -120,6 +134,18 @@ class CapaModuleTest(unittest.TestCase):
self.assertNotEqual(module.url_name, other_module.url_name,
"Factory should be creating unique names for each problem")
def test_correct(self):
"""
Check that the factory creates correct and incorrect problems properly.
"""
module = CapaFactory.create()
self.assertEqual(module.get_score()['score'], 0)
other_module = CapaFactory.create(correct=True)
self.assertEqual(other_module.get_score()['score'], 1)
def test_showanswer_default(self):
"""
Make sure the show answer logic does the right thing.
@@ -178,7 +204,7 @@ class CapaModuleTest(unittest.TestCase):
for everyone--e.g. after due date + grace period.
"""
# can see after attempts used up, even with due date in the future
# can't see after attempts used up, even with due date in the future
used_all_attempts = CapaFactory.create(showanswer='past_due',
max_attempts="1",
attempts="1",
@@ -209,3 +235,50 @@ class CapaModuleTest(unittest.TestCase):
due=self.yesterday_str,
graceperiod=self.two_day_delta_str)
self.assertFalse(still_in_grace.answer_available())
def test_showanswer_finished(self):
"""
With showanswer="finished" should show answer after the problem is closed,
or after the answer is correct.
"""
# can see after attempts used up, even with due date in the future
used_all_attempts = CapaFactory.create(showanswer='finished',
max_attempts="1",
attempts="1",
due=self.tomorrow_str)
self.assertTrue(used_all_attempts.answer_available())
# can see after due date
past_due_date = CapaFactory.create(showanswer='finished',
max_attempts="1",
attempts="0",
due=self.yesterday_str)
self.assertTrue(past_due_date.answer_available())
# can't see because attempts left and wrong
attempts_left_open = CapaFactory.create(showanswer='finished',
max_attempts="1",
attempts="0",
due=self.tomorrow_str)
self.assertFalse(attempts_left_open.answer_available())
# _can_ see because attempts left and right
correct_ans = CapaFactory.create(showanswer='finished',
max_attempts="1",
attempts="0",
due=self.tomorrow_str,
correct=True)
self.assertTrue(correct_ans.answer_available())
# Can see even though grace period hasn't expired, because have no more
# attempts.
still_in_grace = CapaFactory.create(showanswer='finished',
max_attempts="1",
attempts="1",
due=self.yesterday_str,
graceperiod=self.two_day_delta_str)
self.assertTrue(still_in_grace.answer_available())

View File

@@ -39,6 +39,8 @@ if Backbone?
url = DiscussionUtil.urlFor 'threads'
when 'followed'
url = DiscussionUtil.urlFor 'followed_threads', options.user_id
if options['group_id']
data['group_id'] = options['group_id']
data['sort_key'] = sort_options.sort_key || 'date'
data['sort_order'] = sort_options.sort_order || 'desc'
DiscussionUtil.safeAjax

View File

@@ -70,10 +70,21 @@ if Backbone?
DiscussionUtil.loadRoles(response.roles)
allow_anonymous = response.allow_anonymous
allow_anonymous_to_peers = response.allow_anonymous_to_peers
cohorts = response.cohorts
# $elem.html("Hide Discussion")
@discussion = new Discussion()
@discussion.reset(response.discussion_data, {silent: false})
$discussion = $(Mustache.render $("script#_inline_discussion").html(), {'threads':response.discussion_data, 'discussionId': discussionId, 'allow_anonymous_to_peers': allow_anonymous_to_peers, 'allow_anonymous': allow_anonymous})
#use same discussion template but different thread templated
#determined in the coffeescript based on whether or not there's a
#group id
if response.is_cohorted
source = "script#_inline_discussion_cohorted"
else
source = "script#_inline_discussion"
$discussion = $(Mustache.render $(source).html(), {'threads':response.discussion_data, 'discussionId': discussionId, 'allow_anonymous_to_peers': allow_anonymous_to_peers, 'allow_anonymous': allow_anonymous, 'cohorts':cohorts})
if @$('section.discussion').length
@$('section.discussion').replaceWith($discussion)
else

View File

@@ -9,6 +9,7 @@ if Backbone?
"click .browse-topic-drop-search-input": "ignoreClick"
"click .post-list .list-item a": "threadSelected"
"click .post-list .more-pages a": "loadMorePages"
"change .cohort-options": "chooseCohort"
'keyup .browse-topic-drop-search-input': DiscussionFilter.filterDrop
initialize: ->
@@ -128,10 +129,20 @@ if Backbone?
switch @mode
when 'search'
options.search_text = @current_search
if @group_id
options.group_id = @group_id
when 'followed'
options.user_id = window.user.id
options.group_id = "all"
when 'commentables'
options.commentable_ids = @discussionIds
if @group_id
options.group_id = @group_id
when 'all'
if @group_id
options.group_id = @group_id
@collection.retrieveAnotherPage(@mode, options, {sort_key: @sortBy})
renderThread: (thread) =>
@@ -263,13 +274,25 @@ if Backbone?
if discussionId == "#all"
@discussionIds = ""
@$(".post-search-field").val("")
@$('.cohort').show()
@retrieveAllThreads()
else if discussionId == "#following"
@retrieveFollowed(event)
@$('.cohort').hide()
else
discussionIds = _.map item.find(".board-name[data-discussion_id]"), (board) -> $(board).data("discussion_id").id
@retrieveDiscussions(discussionIds)
if $(event.target).attr('cohorted') == "True"
@retrieveDiscussions(discussionIds, "function(){$('.cohort').show();}")
else
@retrieveDiscussions(discussionIds, "function(){$('.cohort').hide();}")
chooseCohort: (event) ->
@group_id = @$('.cohort-options :selected').val()
@collection.current_page = 0
@collection.reset()
@loadMorePages(event)
retrieveDiscussion: (discussion_id, callback=null) ->
url = DiscussionUtil.urlFor("retrieve_discussion", discussion_id)
DiscussionUtil.safeAjax

View File

@@ -16,7 +16,10 @@ if Backbone?
@$delegateElement = @$local
render: ->
@template = DiscussionUtil.getTemplate("_inline_thread")
if @model.has('group_id')
@template = DiscussionUtil.getTemplate("_inline_thread_cohorted")
else
@template = DiscussionUtil.getTemplate("_inline_thread")
if not @model.has('abbreviatedBody')
@abbreviateBody()

View File

@@ -25,6 +25,7 @@ if Backbone?
event.preventDefault()
title = @$(".new-post-title").val()
body = @$(".new-post-body").find(".wmd-input").val()
group = @$(".new-post-group option:selected").attr("value")
# TODO tags: commenting out til we know what to do with them
#tags = @$(".new-post-tags").val()
@@ -45,6 +46,7 @@ if Backbone?
data:
title: title
body: body
group_id: group
# TODO tags: commenting out til we know what to do with them
#tags: tags

View File

@@ -14,8 +14,14 @@ if Backbone?
@setSelectedTopic()
DiscussionUtil.makeWmdEditor @$el, $.proxy(@$, @), "new-post-body"
@$(".new-post-tags").tagsInput DiscussionUtil.tagsInputOptions()
if @$($(".topic_menu li a")[0]).attr('cohorted') != "True"
$('.choose-cohort').hide();
events:
"submit .new-post-form": "createPost"
"click .topic_dropdown_button": "toggleTopicDropdown"
@@ -65,6 +71,11 @@ if Backbone?
@topicText = @getFullTopicName($target)
@topicId = $target.data('discussion_id')
@setSelectedTopic()
if $target.attr('cohorted') == "True"
$('.choose-cohort').show();
else
$('.choose-cohort').hide();
setSelectedTopic: ->
@dropdownButton.html(@fitName(@topicText) + ' <span class="drop-arrow">▾</span>')
@@ -116,6 +127,7 @@ if Backbone?
title = @$(".new-post-title").val()
body = @$(".new-post-body").find(".wmd-input").val()
tags = @$(".new-post-tags").val()
group = @$(".new-post-group option:selected").attr("value")
anonymous = false || @$("input.discussion-anonymous").is(":checked")
anonymous_to_peers = false || @$("input.discussion-anonymous-to-peers").is(":checked")
@@ -137,6 +149,7 @@ if Backbone?
anonymous: anonymous
anonymous_to_peers: anonymous_to_peers
auto_subscribe: follow
group_id: group
error: DiscussionUtil.formErrorHandler(@$(".new-post-form-errors"))
success: (response, textStatus) =>
# TODO: Move this out of the callback, this makes it feel sluggish

View File

@@ -144,6 +144,7 @@ var CohortManager = (function ($) {
$(".remove", tr).html('<a href="#">remove</a>')
.click(function() {
remove_user_from_cohort(item.username, current_cohort_id, tr);
return false;
});
detail_users.append(tr);
@@ -217,6 +218,7 @@ var CohortManager = (function ($) {
show_cohorts_button.click(function() {
state = state_summary;
render();
return false;
});
add_cohort_input.change(function() {
@@ -231,12 +233,14 @@ var CohortManager = (function ($) {
var add_url = url + '/add';
data = {'name': add_cohort_input.val()}
$.post(add_url, data).done(added_cohort);
return false;
});
add_members_button.click(function() {
var add_url = detail_url + '/add';
data = {'users': users_area.val()}
$.post(add_url, data).done(added_users);
return false;
});