Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions cms/djangoapps/contentstore/tests/test_course_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,52 @@ def compare_date_fields(self, details, encoded, context, field):
elif field in encoded and encoded[field] is not None:
self.fail(field + " included in encoding but missing from details at " + context)

@mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True})
def test_pre_requisite_course_list_present(self):
settings_details_url = get_url(self.course.id)
response = self.client.get_html(settings_details_url)
self.assertContains(response, "Prerequisite Course")

@mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True})
def test_pre_requisite_course_update_and_fetch(self):
url = get_url(self.course.id)
resp = self.client.get_json(url)
course_detail_json = json.loads(resp.content)
# assert pre_requisite_courses is initialized
self.assertEqual([], course_detail_json['pre_requisite_courses'])

# update pre requisite courses with a new course keys
pre_requisite_course = CourseFactory.create(org='edX', course='900', run='test_run')
pre_requisite_course2 = CourseFactory.create(org='edX', course='902', run='test_run')
pre_requisite_course_keys = [unicode(pre_requisite_course.id), unicode(pre_requisite_course2.id)]
course_detail_json['pre_requisite_courses'] = pre_requisite_course_keys
self.client.ajax_post(url, course_detail_json)

# fetch updated course to assert pre_requisite_courses has new values
resp = self.client.get_json(url)
course_detail_json = json.loads(resp.content)
self.assertEqual(pre_requisite_course_keys, course_detail_json['pre_requisite_courses'])

# remove pre requisite course
course_detail_json['pre_requisite_courses'] = []
self.client.ajax_post(url, course_detail_json)
resp = self.client.get_json(url)
course_detail_json = json.loads(resp.content)
self.assertEqual([], course_detail_json['pre_requisite_courses'])

@mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True})
def test_invalid_pre_requisite_course(self):
url = get_url(self.course.id)
resp = self.client.get_json(url)
course_detail_json = json.loads(resp.content)

# update pre requisite courses one valid and one invalid key
pre_requisite_course = CourseFactory.create(org='edX', course='900', run='test_run')
pre_requisite_course_keys = [unicode(pre_requisite_course.id), 'invalid_key']
course_detail_json['pre_requisite_courses'] = pre_requisite_course_keys
response = self.client.ajax_post(url, course_detail_json)
self.assertEqual(400, response.status_code)


@ddt.ddt
class CourseGradingTest(CourseTestCase):
Expand Down
111 changes: 72 additions & 39 deletions cms/djangoapps/contentstore/views/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@
from course_action_state.managers import CourseActionStateItemNotFoundError
from microsite_configuration import microsite
from xmodule.course_module import CourseFields
from util.milestones_helpers import (
set_prerequisite_courses,
is_valid_course_key
)


__all__ = ['course_info_handler', 'course_handler', 'course_info_update_handler',
Expand Down Expand Up @@ -346,34 +350,8 @@ def _accessible_courses_list_from_groups(request):
def course_listing(request):
"""
List all courses available to the logged in user
Try to get all courses by first reversing django groups and fallback to old method if it fails
Note: overhead of pymongo reads will increase if getting courses from django groups fails
"""
if GlobalStaff().has_user(request.user):
# user has global access so no need to get courses from django groups
courses, in_process_course_actions = _accessible_courses_list(request)
else:
try:
courses, in_process_course_actions = _accessible_courses_list_from_groups(request)
except AccessListFallback:
# user have some old groups or there was some error getting courses from django groups
# so fallback to iterating through all courses
courses, in_process_course_actions = _accessible_courses_list(request)

def format_course_for_view(course):
"""
Return a dict of the data which the view requires for each course
"""
return {
'display_name': course.display_name,
'course_key': unicode(course.location.course_key),
'url': reverse_course_url('course_handler', course.id),
'lms_link': get_lms_link_for_item(course.location),
'rerun_link': _get_rerun_link_for_item(course.id),
'org': course.display_org_with_default,
'number': course.display_number_with_default,
'run': course.location.run
}
courses, in_process_course_actions = get_courses_accessible_to_user(request)

def format_in_process_course_view(uca):
"""
Expand All @@ -396,14 +374,7 @@ def format_in_process_course_view(uca):
) if uca.state == CourseRerunUIStateManager.State.FAILED else ''
}

# remove any courses in courses that are also in the in_process_course_actions list
in_process_action_course_keys = [uca.course_key for uca in in_process_course_actions]
courses = [
format_course_for_view(c)
for c in courses
if not isinstance(c, ErrorDescriptor) and (c.id not in in_process_action_course_keys)
]

courses = _remove_in_process_courses(courses, in_process_course_actions)
in_process_course_actions = [format_in_process_course_view(uca) for uca in in_process_course_actions]

return render_to_response('index.html', {
Expand Down Expand Up @@ -469,6 +440,52 @@ def course_index(request, course_key):
})


def get_courses_accessible_to_user(request):
"""
Try to get all courses by first reversing django groups and fallback to old method if it fails
Note: overhead of pymongo reads will increase if getting courses from django groups fails
"""
if GlobalStaff().has_user(request.user):
# user has global access so no need to get courses from django groups
courses, in_process_course_actions = _accessible_courses_list(request)
else:
try:
courses, in_process_course_actions = _accessible_courses_list_from_groups(request)
except AccessListFallback:
# user have some old groups or there was some error getting courses from django groups
# so fallback to iterating through all courses
courses, in_process_course_actions = _accessible_courses_list(request)
return courses, in_process_course_actions


def _remove_in_process_courses(courses, in_process_course_actions):
"""
remove any courses in courses that are also in the in_process_course_actions list and formats them for view
"""
def format_course_for_view(course):
"""
Return a dict of the data which the view requires for each course
"""
return {
'display_name': course.display_name,
'course_key': unicode(course.location.course_key),
'url': reverse_course_url('course_handler', course.id),
'lms_link': get_lms_link_for_item(course.location),
'rerun_link': _get_rerun_link_for_item(course.id),
'org': course.display_org_with_default,
'number': course.display_number_with_default,
'run': course.location.run
}

in_process_action_course_keys = [uca.course_key for uca in in_process_course_actions]
courses = [
format_course_for_view(c)
for c in courses
if not isinstance(c, ErrorDescriptor) and (c.id not in in_process_action_course_keys)
]
return courses


def course_outline_initial_state(locator_to_show, course_structure):
"""
Returns the desired initial state for the course outline view. If the 'show' request parameter
Expand Down Expand Up @@ -744,6 +761,7 @@ def settings_handler(request, course_key_string):
json: update the Course and About xblocks through the CourseDetails model
"""
course_key = CourseKey.from_string(course_key_string)
prerequisite_course_enabled = settings.FEATURES.get('ENABLE_PREREQUISITE_COURSES', False)
with modulestore().bulk_operations(course_key):
course_module = get_course_and_check_access(course_key, request.user)
if 'text/html' in request.META.get('HTTP_ACCEPT', '') and request.method == 'GET':
Expand All @@ -758,8 +776,7 @@ def settings_handler(request, course_key_string):
)

short_description_editable = settings.FEATURES.get('EDITABLE_SHORT_DESCRIPTION', True)

return render_to_response('settings.html', {
settings_context = {
'context_course': course_module,
'course_locator': course_key,
'lms_link_for_about_page': utils.get_lms_link_for_about_page(course_key),
Expand All @@ -768,15 +785,31 @@ def settings_handler(request, course_key_string):
'about_page_editable': about_page_editable,
'short_description_editable': short_description_editable,
'upload_asset_url': upload_asset_url
})
}
if prerequisite_course_enabled:
courses, in_process_course_actions = get_courses_accessible_to_user(request)
# exclude current course from the list of available courses
courses = [course for course in courses if course.id != course_key]
if courses:
courses = _remove_in_process_courses(courses, in_process_course_actions)
settings_context.update({'pre_requisite_courses': courses})

return render_to_response('settings.html', settings_context)
elif 'application/json' in request.META.get('HTTP_ACCEPT', ''):
if request.method == 'GET':
course_details = CourseDetails.fetch(course_key)
return JsonResponse(
CourseDetails.fetch(course_key),
course_details,
# encoder serializes dates, old locations, and instances
encoder=CourseSettingsEncoder
)
else: # post or put, doesn't matter.
# if pre-requisite course feature is enabled set pre-requisite course
if prerequisite_course_enabled:
prerequisite_course_keys = request.json.get('pre_requisite_courses', [])
if not all(is_valid_course_key(course_key) for course_key in prerequisite_course_keys):
return JsonResponseBadRequest({_("error"): _("Invalid prerequisite course key")})
set_prerequisite_courses(course_key, prerequisite_course_keys)
return JsonResponse(
CourseDetails.update_from_json(course_key, request.json, request.user),
encoder=CourseSettingsEncoder
Expand Down
7 changes: 7 additions & 0 deletions cms/djangoapps/models/settings/course_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ def __init__(self, org, course_id, run):
self.effort = None # int hours/week
self.course_image_name = ""
self.course_image_asset_path = "" # URL of the course image
self.pre_requisite_courses = [] # pre-requisite courses

@classmethod
def _fetch_about_attribute(cls, course_key, attribute):
Expand All @@ -64,6 +65,7 @@ def fetch(cls, course_key):
course_details.end_date = descriptor.end
course_details.enrollment_start = descriptor.enrollment_start
course_details.enrollment_end = descriptor.enrollment_end
course_details.pre_requisite_courses = descriptor.pre_requisite_courses
course_details.course_image_name = descriptor.course_image
course_details.course_image_asset_path = course_image_url(descriptor)

Expand Down Expand Up @@ -155,6 +157,11 @@ def update_from_json(cls, course_key, jsondict, user):
descriptor.course_image = jsondict['course_image_name']
dirty = True

if 'pre_requisite_courses' in jsondict \
and sorted(jsondict['pre_requisite_courses']) != sorted(descriptor.pre_requisite_courses):
descriptor.pre_requisite_courses = jsondict['pre_requisite_courses']
dirty = True

if dirty:
module_store.update_item(descriptor, user.id)

Expand Down
2 changes: 2 additions & 0 deletions cms/envs/bok_choy.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@

# Use the auto_auth workflow for creating users and logging them in
FEATURES['AUTOMATIC_AUTH_FOR_TESTING'] = True
# Enable pre-requisite course
FEATURES['ENABLE_PREREQUISITE_COURSES'] = True

# Unfortunately, we need to use debug mode to serve staticfiles
DEBUG = True
Expand Down
7 changes: 6 additions & 1 deletion cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,8 @@
'openassessment.xblock',

# edxval
'edxval'
'edxval',
'milestones'
)


Expand All @@ -732,6 +733,10 @@
continue
INSTALLED_APPS += (app_name,)

############## Pre-requisite Courses ##############
# Turn of pre-requisite course by default. This feature depends on milestones app
FEATURES['ENABLE_PREREQUISITE_COURSES'] = False

### ADVANCED_SECURITY_CONFIG
# Empty by default
ADVANCED_SECURITY_CONFIG = {}
Expand Down
3 changes: 3 additions & 0 deletions cms/envs/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@
# Add external_auth to Installed apps for testing
INSTALLED_APPS += ('external_auth', )

# Add milestones to Installed apps for testing
INSTALLED_APPS += ('milestones', )

# hide ratelimit warnings while running tests
filterwarnings('ignore', message='No request passed to the backend, unable to rate-limit')

Expand Down
3 changes: 2 additions & 1 deletion cms/static/js/models/settings/course_details.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ var CourseDetails = Backbone.Model.extend({
intro_video: null,
effort: null, // an int or null,
course_image_name: '', // the filename
course_image_asset_path: '' // the full URL (/c4x/org/course/num/asset/filename)
course_image_asset_path: '', // the full URL (/c4x/org/course/num/asset/filename)
pre_requisite_courses: []
},

validate: function(newattrs) {
Expand Down
25 changes: 21 additions & 4 deletions cms/static/js/spec/views/settings/main_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ define([
], function($, CourseDetailsModel, MainView, AjaxHelpers) {
'use strict';
describe('Settings/Main', function () {
var urlRoot = '/course-details',
var urlRoot = '/course/settings/org/DemoX/Demo_Course',
modelData = {
start_date: "2014-10-05T00:00:00Z",
end_date: "2014-11-05T20:00:00Z",
Expand All @@ -19,7 +19,8 @@ define([
intro_video : null,
effort : null,
course_image_name : '',
course_image_asset_path : ''
course_image_asset_path : '',
pre_requisite_courses : []
},
mockSettingsPage = readFixtures('mock/mock-settings-page.underscore');

Expand Down Expand Up @@ -47,7 +48,6 @@ define([
// Expect to see changes just in `start_date` field.
start_date: "2014-10-05T22:00:00.000Z"
});

this.view.$el.find('#course-start-time')
.val('22:00')
.trigger('input');
Expand All @@ -56,8 +56,25 @@ define([
// It sends `POST` request, because the model doesn't have `id`. In
// this case, it is considered to be new according to Backbone documentation.
AjaxHelpers.expectJsonRequest(
requests, 'POST', '/course-details', expectedJson
requests, 'POST', urlRoot, expectedJson
);
});

it('Selecting a course in pre-requisite drop down should save it as part of course details', function () {
var pre_requisite_courses = ['test/CSS101/2012_T1'];
var requests = AjaxHelpers.requests(this),
expectedJson = $.extend(true, {}, modelData, {
pre_requisite_courses: pre_requisite_courses
});
this.view.$el.find('#pre-requisite-course')
.val(pre_requisite_courses[0])
.trigger('change');

this.view.saveView();
AjaxHelpers.expectJsonRequest(
requests, 'POST', urlRoot, expectedJson
);
AjaxHelpers.respondWithJson(requests, expectedJson);
});
});
});
12 changes: 11 additions & 1 deletion cms/static/js/views/settings/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ var DetailsView = ValidatingView.extend({
// Leaving change in as fallback for older browsers
"change input" : "updateModel",
"change textarea" : "updateModel",
"change select" : "updateModel",
'click .remove-course-introduction-video' : "removeVideo",
'focus #course-overview' : "codeMirrorize",
'mouseover .timezone' : "updateTime",
Expand Down Expand Up @@ -63,6 +64,9 @@ var DetailsView = ValidatingView.extend({
var imageURL = this.model.get('course_image_asset_path');
this.$el.find('#course-image-url').val(imageURL);
this.$el.find('#course-image').attr('src', imageURL);
var pre_requisite_courses = this.model.get('pre_requisite_courses');
pre_requisite_courses = pre_requisite_courses.length > 0 ? pre_requisite_courses : '';
this.$el.find('#' + this.fieldToSelectorMap['pre_requisite_courses']).val(pre_requisite_courses);

return this;
},
Expand All @@ -75,7 +79,8 @@ var DetailsView = ValidatingView.extend({
'short_description' : 'course-short-description',
'intro_video' : 'course-introduction-video',
'effort' : "course-effort",
'course_image_asset_path': 'course-image-url'
'course_image_asset_path': 'course-image-url',
'pre_requisite_courses': 'pre-requisite-course'
},

updateTime : function(e) {
Expand Down Expand Up @@ -154,6 +159,11 @@ var DetailsView = ValidatingView.extend({
case 'course-short-description':
this.setField(event);
break;
case 'pre-requisite-course':
var value = $(event.currentTarget).val();
value = value == "" ? [] : [value];
this.model.set('pre_requisite_courses', value);
break;
// Don't make the user reload the page to check the Youtube ID.
// Wait for a second to load the video, avoiding egregious AJAX calls.
case 'course-introduction-video':
Expand Down
Loading