Skip to content
Merged
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
54 changes: 54 additions & 0 deletions cms/djangoapps/contentstore/tests/test_course_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import ddt
from xmodule.modulestore import ModuleStoreEnum

from util.milestones_helpers import seed_milestone_relationship_types


def get_url(course_id, handler_name='settings_handler'):
return reverse_course_url(handler_name, course_id)
Expand Down Expand Up @@ -171,6 +173,9 @@ class CourseDetailsViewTest(CourseTestCase):
"""
Tests for modifying content on the first course settings page (course dates, overview, etc.).
"""
def setUp(self):
super(CourseDetailsViewTest, self).setUp()

def alter_field(self, url, details, field, val):
"""
Change the one field to the given value and then invoke the update post to see if it worked.
Expand Down Expand Up @@ -243,6 +248,55 @@ 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, 'MILESTONES_APP': True})
def test_pre_requisite_course_list_present(self):
seed_milestone_relationship_types()
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, 'MILESTONES_APP': True})
def test_pre_requisite_course_update_and_fetch(self):
seed_milestone_relationship_types()
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, 'MILESTONES_APP': True})
def test_invalid_pre_requisite_course(self):
seed_milestone_relationship_types()
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
114 changes: 74 additions & 40 deletions cms/djangoapps/contentstore/views/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@
from xmodule.course_module import CourseFields
from xmodule.split_test_module import get_split_user_partitions

from util.milestones_helpers import (
set_prerequisite_courses,
is_valid_course_key
)

MINIMUM_GROUP_ID = 100

# Note: the following content group configuration strings are not
Expand Down Expand Up @@ -368,37 +373,10 @@ def _accessible_libraries_list(user):
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)

courses, in_process_course_actions = get_courses_accessible_to_user(request)
libraries = _accessible_libraries_list(request.user) if LIBRARIES_ENABLED else []

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
}

def format_in_process_course_view(uca):
"""
Return a dict of the data which the view requires for each unsucceeded course
Expand Down Expand Up @@ -433,14 +411,7 @@ def format_library_for_view(library):
'can_edit': has_studio_write_access(request.user, library.location.library_key),
}

# 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 @@ -508,6 +479,53 @@ 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):
"""
removes any in-process courses in courses list. in-process actually refers to courses
that are in the process of being generated for re-run
"""
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 @@ -783,6 +801,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 @@ -797,8 +816,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 @@ -807,15 +825,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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am having trouble following this code. Can you describe to me what the 2 return values are and how the code below ends up with "course" just being the pre_requisite_courses (on line 795)?

Actually, I'm starting to understand-- "pre_requisite_courses" here is just the list of courses from which the prerequisite can be selected. I still don't understand "in_process_course_actions" though.

I'm also curious about when multiple prerequisite courses will be supported.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added backend support for multiple prerequisite courses.

# 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({'possible_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
1 change: 1 addition & 0 deletions cms/djangoapps/models/settings/course_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class CourseMetadata(object):
'tags', # from xblock
'visible_to_staff_only',
'group_access',
'pre_requisite_courses'
]

@classmethod
Expand Down
6 changes: 6 additions & 0 deletions cms/envs/bok_choy.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@
# Use the auto_auth workflow for creating users and logging them in
FEATURES['AUTOMATIC_AUTH_FOR_TESTING'] = True

# Enable milestones app
FEATURES['MILESTONES_APP'] = True

# Enable pre-requisite course
FEATURES['ENABLE_PREREQUISITE_COURSES'] = True

# Unfortunately, we need to use debug mode to serve staticfiles
DEBUG = True

Expand Down
9 changes: 8 additions & 1 deletion cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@
# DEFAULT_STORE_FOR_NEW_COURSE to be 'split' to have future courses
# and libraries created with split.
'ENABLE_CONTENT_LIBRARIES': False,

# Milestones application flag
'MILESTONES_APP': False,

# Prerequisite courses feature flag
'ENABLE_PREREQUISITE_COURSES': False,
}
ENABLE_JASMINE = False

Expand Down Expand Up @@ -744,7 +750,8 @@
'openassessment.xblock',

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


Expand Down
3 changes: 3 additions & 0 deletions cms/envs/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,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);
});
});
});
Loading