Commit 9811926d by Calen Pennington

Make course ids and usage ids opaque to LMS and Studio [partial commit]

This commit updates lms/djangoapps/courseware.

These keys are now objects with a limited interface, and the particular
internal representation is managed by the data storage layer (the
modulestore).

For the LMS, there should be no outward-facing changes to the system.
The keys are, for now, a change to internal representation only. For
Studio, the new serialized form of the keys is used in urls, to allow
for further migration in the future.

Co-Author: Andy Armstrong <andya@edx.org>
Co-Author: Christina Roberts <christina@edx.org>
Co-Author: David Baumgold <db@edx.org>
Co-Author: Diana Huang <dkh@edx.org>
Co-Author: Don Mitchell <dmitchell@edx.org>
Co-Author: Julia Hansbrough <julia@edx.org>
Co-Author: Nimisha Asthagiri <nasthagiri@edx.org>
Co-Author: Sarina Canelake <sarina@edx.org>

[LMS-2370]
parent 7852906c
...@@ -43,7 +43,7 @@ def log_in(username='robot', password='test', email='robot@edx.org', name="Robot ...@@ -43,7 +43,7 @@ def log_in(username='robot', password='test', email='robot@edx.org', name="Robot
@world.absorb @world.absorb
def register_by_course_id(course_id, username='robot', password='test', is_staff=False): def register_by_course_key(course_key, username='robot', password='test', is_staff=False):
create_user(username, password) create_user(username, password)
user = User.objects.get(username=username) user = User.objects.get(username=username)
# Note: this flag makes the user global staff - that is, an edX employee - not a course staff. # Note: this flag makes the user global staff - that is, an edX employee - not a course staff.
...@@ -51,17 +51,17 @@ def register_by_course_id(course_id, username='robot', password='test', is_staff ...@@ -51,17 +51,17 @@ def register_by_course_id(course_id, username='robot', password='test', is_staff
if is_staff: if is_staff:
user.is_staff = True user.is_staff = True
user.save() user.save()
CourseEnrollment.enroll(user, course_id) CourseEnrollment.enroll(user, course_key)
@world.absorb @world.absorb
def enroll_user(user, course_id): def enroll_user(user, course_key):
# Activate user # Activate user
registration = world.RegistrationFactory(user=user) registration = world.RegistrationFactory(user=user)
registration.register(user) registration.register(user)
registration.activate() registration.activate()
# Enroll them in the course # Enroll them in the course
CourseEnrollment.enroll(user, course_id) CourseEnrollment.enroll(user, course_key)
@world.absorb @world.absorb
......
...@@ -21,6 +21,8 @@ from .course_helpers import * ...@@ -21,6 +21,8 @@ from .course_helpers import *
from .ui_helpers import * from .ui_helpers import *
from nose.tools import assert_equals # pylint: disable=E0611 from nose.tools import assert_equals # pylint: disable=E0611
from xmodule.modulestore.locations import SlashSeparatedCourseKey
from logging import getLogger from logging import getLogger
logger = getLogger(__name__) logger = getLogger(__name__)
...@@ -110,7 +112,8 @@ def i_am_not_logged_in(step): ...@@ -110,7 +112,8 @@ def i_am_not_logged_in(step):
@step('I am staff for course "([^"]*)"$') @step('I am staff for course "([^"]*)"$')
def i_am_staff_for_course_by_id(step, course_id): def i_am_staff_for_course_by_id(step, course_id):
world.register_by_course_id(course_id, True) course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
world.register_by_course_key(course_key, True)
@step(r'click (?:the|a) link (?:called|with the text) "([^"]*)"$') @step(r'click (?:the|a) link (?:called|with the text) "([^"]*)"$')
......
...@@ -8,12 +8,13 @@ from django.http import Http404 ...@@ -8,12 +8,13 @@ from django.http import Http404
from django.conf import settings from django.conf import settings
from edxmako.shortcuts import render_to_string from edxmako.shortcuts import render_to_string
from xmodule.course_module import CourseDescriptor from xmodule.modulestore import XML_MODULESTORE_TYPE
from xmodule.modulestore import Location, XML_MODULESTORE_TYPE, MONGO_MODULESTORE_TYPE from xmodule.modulestore.keys import CourseKey
from xmodule.modulestore.django import modulestore, loc_mapper from xmodule.modulestore.django import modulestore
from xmodule.contentstore.content import StaticContent from xmodule.contentstore.content import StaticContent
from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError
from static_replace import replace_static_urls from static_replace import replace_static_urls
from xmodule.modulestore import MONGO_MODULESTORE_TYPE
from courseware.access import has_access from courseware.access import has_access
from courseware.model_data import FieldDataCache from courseware.model_data import FieldDataCache
...@@ -49,15 +50,15 @@ def get_course(course_id, depth=0): ...@@ -49,15 +50,15 @@ def get_course(course_id, depth=0):
None means infinite depth. Default is to fetch no children. None means infinite depth. Default is to fetch no children.
""" """
try: try:
course_loc = CourseDescriptor.id_to_location(course_id) return modulestore().get_course(course_id, depth=depth)
return modulestore().get_instance(course_id, course_loc, depth=depth)
except (KeyError, ItemNotFoundError): except (KeyError, ItemNotFoundError):
raise ValueError(u"Course not found: {0}".format(course_id)) raise ValueError(u"Course not found: {0}".format(course_id))
except InvalidLocationError: except InvalidLocationError:
raise ValueError(u"Invalid location: {0}".format(course_id)) raise ValueError(u"Invalid location: {0}".format(course_id))
def get_course_by_id(course_id, depth=0): # TODO please rename this function to get_course_by_key at next opportunity!
def get_course_by_id(course_key, depth=0):
""" """
Given a course id, return the corresponding course descriptor. Given a course id, return the corresponding course descriptor.
...@@ -66,50 +67,55 @@ def get_course_by_id(course_id, depth=0): ...@@ -66,50 +67,55 @@ def get_course_by_id(course_id, depth=0):
depth: The number of levels of children for the modulestore to cache. None means infinite depth depth: The number of levels of children for the modulestore to cache. None means infinite depth
""" """
try: try:
course_loc = CourseDescriptor.id_to_location(course_id) course = modulestore().get_course(course_key, depth=depth)
return modulestore().get_instance(course_id, course_loc, depth=depth) if course:
return course
else:
raise Http404("Course not found.")
except (KeyError, ItemNotFoundError): except (KeyError, ItemNotFoundError):
raise Http404("Course not found.") raise Http404("Course not found.")
except InvalidLocationError: except InvalidLocationError:
raise Http404("Invalid location") raise Http404("Invalid location")
def get_course_with_access(user, course_id, action, depth=0): def get_course_with_access(user, action, course_key, depth=0):
""" """
Given a course_id, look up the corresponding course descriptor, Given a course_key, look up the corresponding course descriptor,
check that the user has the access to perform the specified action check that the user has the access to perform the specified action
on the course, and return the descriptor. on the course, and return the descriptor.
Raises a 404 if the course_id is invalid, or the user doesn't have access. Raises a 404 if the course_key is invalid, or the user doesn't have access.
depth: The number of levels of children for the modulestore to cache. None means infinite depth depth: The number of levels of children for the modulestore to cache. None means infinite depth
""" """
course = get_course_by_id(course_id, depth=depth) assert isinstance(course_key, CourseKey)
if not has_access(user, course, action): course = get_course_by_id(course_key, depth=depth)
if not has_access(user, action, course, course_key):
# Deliberately return a non-specific error message to avoid # Deliberately return a non-specific error message to avoid
# leaking info about access control settings # leaking info about access control settings
raise Http404("Course not found.") raise Http404("Course not found.")
return course return course
def get_opt_course_with_access(user, course_id, action): def get_opt_course_with_access(user, action, course_key):
""" """
Same as get_course_with_access, except that if course_id is None, Same as get_course_with_access, except that if course_key is None,
return None without performing any access checks. return None without performing any access checks.
""" """
if course_id is None: if course_key is None:
return None return None
return get_course_with_access(user, course_id, action) return get_course_with_access(user, action, course_key)
def course_image_url(course): def course_image_url(course):
"""Try to look up the image url for the course. If it's not found, """ Determine whether this is an XML or Studio-backed course, and return the appropriate course_image URL """
log an error and return the dead link""" if course.static_asset_path or modulestore().get_modulestore_type(course.id) == XML_MODULESTORE_TYPE:
if course.static_asset_path or modulestore().get_modulestore_type(course.location.course_id) == XML_MODULESTORE_TYPE:
return '/static/' + (course.static_asset_path or getattr(course, 'data_dir', '')) + "/images/course_image.jpg" return '/static/' + (course.static_asset_path or getattr(course, 'data_dir', '')) + "/images/course_image.jpg"
else: else:
loc = StaticContent.compute_location(course.location.org, course.location.course, course.course_image) loc = StaticContent.compute_location(course.location.course_key, course.course_image)
_path = StaticContent.get_url_path_from_location(loc) _path = loc.to_deprecated_string()
return _path return _path
...@@ -158,7 +164,7 @@ def get_course_about_section(course, section_key): ...@@ -158,7 +164,7 @@ def get_course_about_section(course, section_key):
# markup. This can change without effecting this interface when we find a # markup. This can change without effecting this interface when we find a
# good format for defining so many snippets of text/html. # good format for defining so many snippets of text/html.
# TODO: Remove number, instructors from this list # TODO: Remove number, instructors from this list
if section_key in ['short_description', 'description', 'key_dates', 'video', if section_key in ['short_description', 'description', 'key_dates', 'video',
'course_staff_short', 'course_staff_extended', 'course_staff_short', 'course_staff_extended',
'requirements', 'syllabus', 'textbook', 'faq', 'more_info', 'requirements', 'syllabus', 'textbook', 'faq', 'more_info',
...@@ -199,7 +205,7 @@ def get_course_about_section(course, section_key): ...@@ -199,7 +205,7 @@ def get_course_about_section(course, section_key):
except ItemNotFoundError: except ItemNotFoundError:
log.warning( log.warning(
u"Missing about section {key} in course {url}".format(key=section_key, url=course.location.url()) u"Missing about section {key} in course {url}".format(key=section_key, url=course.location.to_deprecated_string())
) )
return None return None
elif section_key == "title": elif section_key == "title":
...@@ -223,14 +229,14 @@ def get_course_info_section(request, course, section_key): ...@@ -223,14 +229,14 @@ def get_course_info_section(request, course, section_key):
- updates - updates
- guest_updates - guest_updates
""" """
loc = Location(course.location.tag, course.location.org, course.location.course, 'course_info', section_key) usage_key = course.id.make_usage_key('course_info', section_key)
# Use an empty cache # Use an empty cache
field_data_cache = FieldDataCache([], course.id, request.user) field_data_cache = FieldDataCache([], course.id, request.user)
info_module = get_module( info_module = get_module(
request.user, request.user,
request, request,
loc, usage_key,
field_data_cache, field_data_cache,
course.id, course.id,
wrap_xmodule_display=False, wrap_xmodule_display=False,
...@@ -279,12 +285,12 @@ def get_course_syllabus_section(course, section_key): ...@@ -279,12 +285,12 @@ def get_course_syllabus_section(course, section_key):
return replace_static_urls( return replace_static_urls(
html_file.read().decode('utf-8'), html_file.read().decode('utf-8'),
getattr(course, 'data_dir', None), getattr(course, 'data_dir', None),
course_id=course.location.course_id, course_id=course.id,
static_asset_path=course.static_asset_path, static_asset_path=course.static_asset_path,
) )
except ResourceNotFoundError: except ResourceNotFoundError:
log.exception( log.exception(
u"Missing syllabus section {key} in course {url}".format(key=section_key, url=course.location.url()) u"Missing syllabus section {key} in course {url}".format(key=section_key, url=course.location.to_deprecated_string())
) )
return "! Syllabus missing !" return "! Syllabus missing !"
...@@ -312,7 +318,7 @@ def get_courses(user, domain=None): ...@@ -312,7 +318,7 @@ def get_courses(user, domain=None):
Returns a list of courses available, sorted by course.number Returns a list of courses available, sorted by course.number
''' '''
courses = branding.get_visible_courses() courses = branding.get_visible_courses()
courses = [c for c in courses if has_access(user, c, 'see_exists')] courses = [c for c in courses if has_access(user, 'see_exists', c)]
courses = sorted(courses, key=lambda course: course.number) courses = sorted(courses, key=lambda course: course.number)
...@@ -332,15 +338,14 @@ def sort_by_announcement(courses): ...@@ -332,15 +338,14 @@ def sort_by_announcement(courses):
return courses return courses
def get_cms_course_link(course): def get_cms_course_link(course, page='course'):
""" """
Returns a link to course_index for editing the course in cms, Returns a link to course_index for editing the course in cms,
assuming that the course is actually cms-backed. assuming that the course is actually cms-backed.
""" """
locator = loc_mapper().translate_location( # This is fragile, but unfortunately the problem is that within the LMS we
course.location.course_id, course.location, False, True # can't use the reverse calls from the CMS
) return u"//{}/{}/{}".format(settings.CMS_BASE, page, unicode(course.id))
return "//" + settings.CMS_BASE + locator.url_reverse('course/', '')
def get_cms_block_link(block, page): def get_cms_block_link(block, page):
...@@ -348,20 +353,20 @@ def get_cms_block_link(block, page): ...@@ -348,20 +353,20 @@ def get_cms_block_link(block, page):
Returns a link to block_index for editing the course in cms, Returns a link to block_index for editing the course in cms,
assuming that the block is actually cms-backed. assuming that the block is actually cms-backed.
""" """
locator = loc_mapper().translate_location( # This is fragile, but unfortunately the problem is that within the LMS we
block.location.course_id, block.location, False, True # can't use the reverse calls from the CMS
) return u"//{}/{}/{}".format(settings.CMS_BASE, page, block.location)
return "//" + settings.CMS_BASE + locator.url_reverse(page, '')
def get_studio_url(course_id, page): def get_studio_url(course_key, page):
""" """
Get the Studio URL of the page that is passed in. Get the Studio URL of the page that is passed in.
""" """
course = get_course_by_id(course_id) assert(isinstance(course_key, CourseKey))
course = get_course_by_id(course_key)
is_studio_course = course.course_edit_method == "Studio" is_studio_course = course.course_edit_method == "Studio"
is_mongo_course = modulestore().get_modulestore_type(course_id) == MONGO_MODULESTORE_TYPE is_mongo_course = modulestore().get_modulestore_type(course_key) == MONGO_MODULESTORE_TYPE
studio_link = None studio_link = None
if is_studio_course and is_mongo_course: if is_studio_course and is_mongo_course:
studio_link = get_cms_block_link(course, page) studio_link = get_cms_course_link(course, page)
return studio_link return studio_link
...@@ -140,7 +140,7 @@ class AnnotatableSteps(object): ...@@ -140,7 +140,7 @@ class AnnotatableSteps(object):
def active_problem_selector(self, subselector): def active_problem_selector(self, subselector):
return 'div[data-problem-id="{}"] {}'.format( return 'div[data-problem-id="{}"] {}'.format(
world.scenario_dict['PROBLEMS'][self.active_problem].location.url(), world.scenario_dict['PROBLEMS'][self.active_problem].location.to_deprecated_string(),
subselector, subselector,
) )
......
...@@ -12,6 +12,7 @@ from django.core.urlresolvers import reverse ...@@ -12,6 +12,7 @@ from django.core.urlresolvers import reverse
from student.models import CourseEnrollment from student.models import CourseEnrollment
from xmodule.modulestore import Location from xmodule.modulestore import Location
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.modulestore.locations import SlashSeparatedCourseKey
from xmodule.course_module import CourseDescriptor from xmodule.course_module import CourseDescriptor
from courseware.courses import get_course_by_id from courseware.courses import get_course_by_id
from xmodule import seq_module, vertical_module from xmodule import seq_module, vertical_module
...@@ -119,16 +120,19 @@ def go_into_course(step): ...@@ -119,16 +120,19 @@ def go_into_course(step):
def course_id(course_num): def course_id(course_num):
return "%s/%s/%s" % (world.scenario_dict['COURSE'].org, course_num, return SlashSeparatedCourseKey(
world.scenario_dict['COURSE'].url_name) world.scenario_dict['COURSE'].org,
course_num,
world.scenario_dict['COURSE'].url_name
)
def course_location(course_num): def course_location(course_num):
return world.scenario_dict['COURSE'].location._replace(course=course_num) return world.scenario_dict['COURSE'].location.replace(course=course_num)
def section_location(course_num): def section_location(course_num):
return world.scenario_dict['SECTION'].location._replace(course=course_num) return world.scenario_dict['SECTION'].location.replace(course=course_num)
def visit_scenario_item(item_key): def visit_scenario_item(item_key):
...@@ -140,8 +144,8 @@ def visit_scenario_item(item_key): ...@@ -140,8 +144,8 @@ def visit_scenario_item(item_key):
url = django_url(reverse( url = django_url(reverse(
'jump_to', 'jump_to',
kwargs={ kwargs={
'course_id': world.scenario_dict['COURSE'].id, 'course_id': world.scenario_dict['COURSE'].id.to_deprecated_string(),
'location': str(world.scenario_dict[item_key].location), 'location': world.scenario_dict[item_key].location.to_deprecated_string(),
} }
)) ))
......
...@@ -46,16 +46,16 @@ class ConditionalSteps(object): ...@@ -46,16 +46,16 @@ class ConditionalSteps(object):
metadata = { metadata = {
'xml_attributes': { 'xml_attributes': {
'sources': world.scenario_dict['CONDITION_SOURCE'].location.url() condition: cond_value
} }
} }
metadata['xml_attributes'][condition] = cond_value
world.scenario_dict['CONDITIONAL'] = world.ItemFactory( world.scenario_dict['CONDITIONAL'] = world.ItemFactory(
parent_location=world.scenario_dict['WRAPPER'].location, parent_location=world.scenario_dict['WRAPPER'].location,
category='conditional', category='conditional',
display_name="Test Conditional", display_name="Test Conditional",
metadata=metadata metadata=metadata,
sources_list=[world.scenario_dict['CONDITION_SOURCE'].location],
) )
world.ItemFactory( world.ItemFactory(
......
...@@ -201,26 +201,24 @@ def i_am_registered_for_the_course(coursenum, metadata, user='Instructor'): ...@@ -201,26 +201,24 @@ def i_am_registered_for_the_course(coursenum, metadata, user='Instructor'):
metadata.update({'days_early_for_beta': 5, 'start': tomorrow}) metadata.update({'days_early_for_beta': 5, 'start': tomorrow})
create_course_for_lti(coursenum, metadata) create_course_for_lti(coursenum, metadata)
course_descriptor = world.scenario_dict['COURSE'] course_descriptor = world.scenario_dict['COURSE']
course_location = world.scenario_dict['COURSE'].location
# create beta tester # create beta tester
user = BetaTesterFactory(course=course_location) user = BetaTesterFactory(course=course_descriptor.id)
normal_student = UserFactory() normal_student = UserFactory()
instructor = InstructorFactory(course=course_location) instructor = InstructorFactory(course=course_descriptor.id)
assert not has_access(normal_student, course_descriptor, 'load') assert not has_access(normal_student, 'load', course_descriptor)
assert has_access(user, course_descriptor, 'load') assert has_access(user, 'load', course_descriptor)
assert has_access(instructor, course_descriptor, 'load') assert has_access(instructor, 'load', course_descriptor)
else: else:
metadata.update({'start': datetime.datetime(1970, 1, 1, tzinfo=UTC)}) metadata.update({'start': datetime.datetime(1970, 1, 1, tzinfo=UTC)})
create_course_for_lti(coursenum, metadata) create_course_for_lti(coursenum, metadata)
course_descriptor = world.scenario_dict['COURSE'] course_descriptor = world.scenario_dict['COURSE']
course_location = world.scenario_dict['COURSE'].location user = InstructorFactory(course=course_descriptor.id)
user = InstructorFactory(course=course_location)
# Enroll the user in the course and log them in # Enroll the user in the course and log them in
if has_access(user, course_descriptor, 'load'): if has_access(user, 'load', course_descriptor):
world.enroll_user(user, course_id(coursenum)) world.enroll_user(user, course_descriptor.id)
world.log_in(username=user.username, password='test') world.log_in(username=user.username, password='test')
......
...@@ -5,6 +5,7 @@ from lettuce import world, step ...@@ -5,6 +5,7 @@ from lettuce import world, step
from common import course_id, course_location from common import course_id, course_location
from problems_setup import PROBLEM_DICT from problems_setup import PROBLEM_DICT
from nose.tools import assert_in from nose.tools import assert_in
from xmodule.modulestore.locations import SlashSeparatedCourseKey
@step(u'I am viewing a course with multiple sections') @step(u'I am viewing a course with multiple sections')
...@@ -148,7 +149,7 @@ def create_course(): ...@@ -148,7 +149,7 @@ def create_course():
def create_user_and_visit_course(): def create_user_and_visit_course():
world.register_by_course_id('edx/999/Test_Course') world.register_by_course_key(SlashSeparatedCourseKey('edx', '999', 'Test_Course'))
world.log_in() world.log_in()
world.visit('/courses/edx/999/Test_Course/courseware/') world.visit('/courses/edx/999/Test_Course/courseware/')
......
...@@ -10,7 +10,7 @@ logger = getLogger(__name__) ...@@ -10,7 +10,7 @@ logger = getLogger(__name__)
@step('I navigate to an openended question$') @step('I navigate to an openended question$')
def navigate_to_an_openended_question(step): def navigate_to_an_openended_question(step):
world.register_by_course_id('MITx/3.091x/2012_Fall') world.register_by_course_key('MITx/3.091x/2012_Fall')
world.log_in(email='robot@edx.org', password='test') world.log_in(email='robot@edx.org', password='test')
problem = '/courses/MITx/3.091x/2012_Fall/courseware/Week_10/Polymer_Synthesis/' problem = '/courses/MITx/3.091x/2012_Fall/courseware/Week_10/Polymer_Synthesis/'
world.browser.visit(django_url(problem)) world.browser.visit(django_url(problem))
...@@ -20,7 +20,7 @@ def navigate_to_an_openended_question(step): ...@@ -20,7 +20,7 @@ def navigate_to_an_openended_question(step):
@step('I navigate to an openended question as staff$') @step('I navigate to an openended question as staff$')
def navigate_to_an_openended_question_as_staff(step): def navigate_to_an_openended_question_as_staff(step):
world.register_by_course_id('MITx/3.091x/2012_Fall', True) world.register_by_course_key('MITx/3.091x/2012_Fall', True)
world.log_in(email='robot@edx.org', password='test') world.log_in(email='robot@edx.org', password='test')
problem = '/courses/MITx/3.091x/2012_Fall/courseware/Week_10/Polymer_Synthesis/' problem = '/courses/MITx/3.091x/2012_Fall/courseware/Week_10/Polymer_Synthesis/'
world.browser.visit(django_url(problem)) world.browser.visit(django_url(problem))
......
...@@ -7,7 +7,7 @@ from lettuce.django import django_url ...@@ -7,7 +7,7 @@ from lettuce.django import django_url
@step('I register for the course "([^"]*)"$') @step('I register for the course "([^"]*)"$')
def i_register_for_the_course(_step, course): def i_register_for_the_course(_step, course):
url = django_url('courses/%s/about' % world.scenario_dict['COURSE'].id) url = django_url('courses/%s/about' % world.scenario_dict['COURSE'].id.to_deprecated_string())
world.browser.visit(url) world.browser.visit(url)
world.css_click('section.intro a.register') world.css_click('section.intro a.register')
......
...@@ -195,7 +195,7 @@ def add_vertical_to_course(course_num): ...@@ -195,7 +195,7 @@ def add_vertical_to_course(course_num):
def last_vertical_location(course_num): def last_vertical_location(course_num):
return world.scenario_dict['LAST_VERTICAL'].location._replace(course=course_num) return world.scenario_dict['LAST_VERTICAL'].location.replace(course=course_num)
def upload_file(filename, location): def upload_file(filename, location):
...@@ -204,7 +204,7 @@ def upload_file(filename, location): ...@@ -204,7 +204,7 @@ def upload_file(filename, location):
mime_type = "application/json" mime_type = "application/json"
content_location = StaticContent.compute_location( content_location = StaticContent.compute_location(
location.org, location.course, filename location.course_key, filename
) )
content = StaticContent(content_location, filename, mime_type, f.read()) content = StaticContent(content_location, filename, mime_type, f.read())
contentstore().save(content) contentstore().save(content)
......
...@@ -23,6 +23,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError ...@@ -23,6 +23,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.util.duedate import get_extended_due_date from xmodule.util.duedate import get_extended_due_date
from .models import StudentModule from .models import StudentModule
from .module_render import get_module_for_descriptor from .module_render import get_module_for_descriptor
from opaque_keys import InvalidKeyError
log = logging.getLogger("edx.courseware") log = logging.getLogger("edx.courseware")
...@@ -50,9 +51,9 @@ def yield_dynamic_descriptor_descendents(descriptor, module_creator): ...@@ -50,9 +51,9 @@ def yield_dynamic_descriptor_descendents(descriptor, module_creator):
yield next_descriptor yield next_descriptor
def answer_distributions(course_id): def answer_distributions(course_key):
""" """
Given a course_id, return answer distributions in the form of a dictionary Given a course_key, return answer distributions in the form of a dictionary
mapping: mapping:
(problem url_name, problem display_name, problem_id) -> {dict: answer -> count} (problem url_name, problem display_name, problem_id) -> {dict: answer -> count}
...@@ -82,44 +83,40 @@ def answer_distributions(course_id): ...@@ -82,44 +83,40 @@ def answer_distributions(course_id):
# dict: { module.module_state_key : (url_name, display_name) } # dict: { module.module_state_key : (url_name, display_name) }
state_keys_to_problem_info = {} # For caching, used by url_and_display_name state_keys_to_problem_info = {} # For caching, used by url_and_display_name
def url_and_display_name(module_state_key): def url_and_display_name(usage_key):
""" """
For a given module_state_key, return the problem's url and display_name. For a given usage_key, return the problem's url and display_name.
Handle modulestore access and caching. This method ignores permissions. Handle modulestore access and caching. This method ignores permissions.
May throw an ItemNotFoundError if there is no content that corresponds
to this module_state_key. Raises:
InvalidKeyError: if the usage_key does not parse
ItemNotFoundError: if there is no content that corresponds
to this usage_key.
""" """
problem_store = modulestore() problem_store = modulestore()
if module_state_key not in state_keys_to_problem_info: if usage_key not in state_keys_to_problem_info:
problems = problem_store.get_items(module_state_key, course_id=course_id, depth=1) problem = problem_store.get_item(usage_key)
if not problems:
# Likely means that the problem was deleted from the course
# after the student had answered. We log this suspicion where
# this exception is caught.
raise ItemNotFoundError(
"Answer Distribution: Module {} not found for course {}"
.format(module_state_key, course_id)
)
problem = problems[0]
problem_info = (problem.url_name, problem.display_name_with_default) problem_info = (problem.url_name, problem.display_name_with_default)
state_keys_to_problem_info[module_state_key] = problem_info state_keys_to_problem_info[usage_key] = problem_info
return state_keys_to_problem_info[module_state_key] return state_keys_to_problem_info[usage_key]
# Iterate through all problems submitted for this course in no particular # Iterate through all problems submitted for this course in no particular
# order, and build up our answer_counts dict that we will eventually return # order, and build up our answer_counts dict that we will eventually return
answer_counts = defaultdict(lambda: defaultdict(int)) answer_counts = defaultdict(lambda: defaultdict(int))
for module in StudentModule.all_submitted_problems_read_only(course_id): for module in StudentModule.all_submitted_problems_read_only(course_key):
try: try:
state_dict = json.loads(module.state) if module.state else {} state_dict = json.loads(module.state) if module.state else {}
raw_answers = state_dict.get("student_answers", {}) raw_answers = state_dict.get("student_answers", {})
except ValueError: except ValueError:
log.error( log.error(
"Answer Distribution: Could not parse module state for " + "Answer Distribution: Could not parse module state for " +
"StudentModule id={}, course={}".format(module.id, course_id) "StudentModule id={}, course={}".format(module.id, course_key)
) )
continue continue
try:
url, display_name = url_and_display_name(module.module_id.map_into_course(course_key))
# Each problem part has an ID that is derived from the # Each problem part has an ID that is derived from the
# module.module_state_key (with some suffix appended) # module.module_state_key (with some suffix appended)
for problem_part_id, raw_answer in raw_answers.items(): for problem_part_id, raw_answer in raw_answers.items():
...@@ -128,22 +125,19 @@ def answer_distributions(course_id): ...@@ -128,22 +125,19 @@ def answer_distributions(course_id):
# unicode and not str -- state comes from the json decoder, and that # unicode and not str -- state comes from the json decoder, and that
# always returns unicode for strings. # always returns unicode for strings.
answer = unicode(raw_answer) answer = unicode(raw_answer)
answer_counts[(url, display_name, problem_part_id)][answer] += 1
try: except (ItemNotFoundError, InvalidKeyError):
url, display_name = url_and_display_name(module.module_state_key)
except ItemNotFoundError:
msg = "Answer Distribution: Item {} referenced in StudentModule {} " + \ msg = "Answer Distribution: Item {} referenced in StudentModule {} " + \
"for user {} in course {} not found; " + \ "for user {} in course {} not found; " + \
"This can happen if a student answered a question that " + \ "This can happen if a student answered a question that " + \
"was later deleted from the course. This answer will be " + \ "was later deleted from the course. This answer will be " + \
"omitted from the answer distribution CSV." "omitted from the answer distribution CSV."
log.warning( log.warning(
msg.format(module.module_state_key, module.id, module.student_id, course_id) msg.format(module.module_state_key, module.id, module.student_id, course_key)
) )
continue continue
answer_counts[(url, display_name, problem_part_id)][answer] += 1
return answer_counts return answer_counts
@transaction.commit_manually @transaction.commit_manually
...@@ -183,7 +177,9 @@ def _grade(student, request, course, keep_raw_scores): ...@@ -183,7 +177,9 @@ def _grade(student, request, course, keep_raw_scores):
# Dict of item_ids -> (earned, possible) point tuples. This *only* grabs # Dict of item_ids -> (earned, possible) point tuples. This *only* grabs
# scores that were registered with the submissions API, which for the moment # scores that were registered with the submissions API, which for the moment
# means only openassessment (edx-ora2) # means only openassessment (edx-ora2)
submissions_scores = sub_api.get_scores(course.id, anonymous_id_for_user(student, course.id)) submissions_scores = sub_api.get_scores(
course.id.to_deprecated_string(), anonymous_id_for_user(student, course.id)
)
totaled_scores = {} totaled_scores = {}
# This next complicated loop is just to collect the totaled_scores, which is # This next complicated loop is just to collect the totaled_scores, which is
...@@ -206,7 +202,7 @@ def _grade(student, request, course, keep_raw_scores): ...@@ -206,7 +202,7 @@ def _grade(student, request, course, keep_raw_scores):
# API. If scores exist, we have to calculate grades for this section. # API. If scores exist, we have to calculate grades for this section.
if not should_grade_section: if not should_grade_section:
should_grade_section = any( should_grade_section = any(
descriptor.location.url() in submissions_scores descriptor.location.to_deprecated_string() in submissions_scores
for descriptor in section['xmoduledescriptors'] for descriptor in section['xmoduledescriptors']
) )
...@@ -214,7 +210,7 @@ def _grade(student, request, course, keep_raw_scores): ...@@ -214,7 +210,7 @@ def _grade(student, request, course, keep_raw_scores):
with manual_transaction(): with manual_transaction():
should_grade_section = StudentModule.objects.filter( should_grade_section = StudentModule.objects.filter(
student=student, student=student,
module_state_key__in=[ module_id__in=[
descriptor.location for descriptor in section['xmoduledescriptors'] descriptor.location for descriptor in section['xmoduledescriptors']
] ]
).exists() ).exists()
...@@ -350,7 +346,7 @@ def _progress_summary(student, request, course): ...@@ -350,7 +346,7 @@ def _progress_summary(student, request, course):
# This student must not have access to the course. # This student must not have access to the course.
return None return None
submissions_scores = sub_api.get_scores(course.id, anonymous_id_for_user(student, course.id)) submissions_scores = sub_api.get_scores(course.id.to_deprecated_string(), anonymous_id_for_user(student, course.id))
chapters = [] chapters = []
# Don't include chapters that aren't displayable (e.g. due to error) # Don't include chapters that aren't displayable (e.g. due to error)
...@@ -427,7 +423,7 @@ def get_score(course_id, user, problem_descriptor, module_creator, scores_cache= ...@@ -427,7 +423,7 @@ def get_score(course_id, user, problem_descriptor, module_creator, scores_cache=
if not user.is_authenticated(): if not user.is_authenticated():
return (None, None) return (None, None)
location_url = problem_descriptor.location.url() location_url = problem_descriptor.location.to_deprecated_string()
if location_url in scores_cache: if location_url in scores_cache:
return scores_cache[location_url] return scores_cache[location_url]
...@@ -451,7 +447,7 @@ def get_score(course_id, user, problem_descriptor, module_creator, scores_cache= ...@@ -451,7 +447,7 @@ def get_score(course_id, user, problem_descriptor, module_creator, scores_cache=
student_module = StudentModule.objects.get( student_module = StudentModule.objects.get(
student=user, student=user,
course_id=course_id, course_id=course_id,
module_state_key=problem_descriptor.location module_id=problem_descriptor.location
) )
except StudentModule.DoesNotExist: except StudentModule.DoesNotExist:
student_module = None student_module = None
......
...@@ -73,7 +73,7 @@ def import_with_checks(course_dir, verbose=True): ...@@ -73,7 +73,7 @@ def import_with_checks(course_dir, verbose=True):
return (False, None) return (False, None)
course = courses[0] course = courses[0]
errors = modulestore.get_item_errors(course.location) errors = modulestore.get_course_errors(course.id)
if len(errors) != 0: if len(errors) != 0:
all_ok = False all_ok = False
print '\n' print '\n'
......
...@@ -24,18 +24,12 @@ class Command(BaseCommand): ...@@ -24,18 +24,12 @@ class Command(BaseCommand):
) )
def handle(self, *args, **options): def handle(self, *args, **options):
results = []
try: try:
name = options['modulestore'] name = options['modulestore']
store = modulestore(name) store = modulestore(name)
except KeyError: except KeyError:
raise CommandError("Unknown modulestore {}".format(name)) raise CommandError("Unknown modulestore {}".format(name))
for course in store.get_courses(): output = u'\n'.join(course.id.to_deprecated_string() for course in store.get_courses()) + '\n'
course_id = course.location.course_id
results.append(course_id)
output = '\n'.join(results) + '\n'
return output.encode('utf-8') return output.encode('utf-8')
...@@ -25,6 +25,8 @@ from django.core.management.base import BaseCommand, CommandError ...@@ -25,6 +25,8 @@ from django.core.management.base import BaseCommand, CommandError
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.modulestore.inheritance import own_metadata, compute_inherited_metadata from xmodule.modulestore.inheritance import own_metadata, compute_inherited_metadata
from xblock.fields import Scope from xblock.fields import Scope
from opaque_keys import InvalidKeyError
from xmodule.modulestore.locations import SlashSeparatedCourseKey
FILTER_LIST = ['xml_attributes', 'checklists'] FILTER_LIST = ['xml_attributes', 'checklists']
INHERITED_FILTER_LIST = ['children', 'xml_attributes', 'checklists'] INHERITED_FILTER_LIST = ['children', 'xml_attributes', 'checklists']
...@@ -66,7 +68,11 @@ class Command(BaseCommand): ...@@ -66,7 +68,11 @@ class Command(BaseCommand):
# Get the course data # Get the course data
course_id = args[0] try:
course_id = SlashSeparatedCourseKey.from_deprecated_string(args[0])
except InvalidKeyError:
raise CommandError("Invalid course_id")
course = store.get_course(course_id) course = store.get_course(course_id)
if course is None: if course is None:
raise CommandError("Invalid course_id") raise CommandError("Invalid course_id")
...@@ -90,12 +96,12 @@ def dump_module(module, destination=None, inherited=False, defaults=False): ...@@ -90,12 +96,12 @@ def dump_module(module, destination=None, inherited=False, defaults=False):
destination = destination if destination else {} destination = destination if destination else {}
items = own_metadata(module).iteritems() items = own_metadata(module)
filtered_metadata = {k: v for k, v in items if k not in FILTER_LIST} filtered_metadata = {k: v for k, v in items.iteritems() if k not in FILTER_LIST}
destination[module.location.url()] = { destination[module.location.to_deprecated_string()] = {
'category': module.location.category, 'category': module.location.category,
'children': [str(child) for child in getattr(module, 'children', [])], 'children': [child.to_deprecated_string() for child in getattr(module, 'children', [])],
'metadata': filtered_metadata, 'metadata': filtered_metadata,
} }
...@@ -116,7 +122,7 @@ def dump_module(module, destination=None, inherited=False, defaults=False): ...@@ -116,7 +122,7 @@ def dump_module(module, destination=None, inherited=False, defaults=False):
return field.values != field.default return field.values != field.default
inherited_metadata = {field.name: field.read_json(module) for field in module.fields.values() if is_inherited(field)} inherited_metadata = {field.name: field.read_json(module) for field in module.fields.values() if is_inherited(field)}
destination[module.location.url()]['inherited_metadata'] = inherited_metadata destination[module.location.to_deprecated_string()]['inherited_metadata'] = inherited_metadata
for child in module.get_children(): for child in module.get_children():
dump_module(child, destination, inherited, defaults) dump_module(child, destination, inherited, defaults)
......
...@@ -17,6 +17,8 @@ from django.core.management.base import BaseCommand, CommandError ...@@ -17,6 +17,8 @@ from django.core.management.base import BaseCommand, CommandError
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.modulestore.xml_exporter import export_to_xml from xmodule.modulestore.xml_exporter import export_to_xml
from opaque_keys import InvalidKeyError
from xmodule.modulestore.locations import SlashSeparatedCourseKey
class Command(BaseCommand): class Command(BaseCommand):
...@@ -39,8 +41,10 @@ class Command(BaseCommand): ...@@ -39,8 +41,10 @@ class Command(BaseCommand):
def _parse_arguments(self, args): def _parse_arguments(self, args):
"""Parse command line arguments""" """Parse command line arguments"""
try: try:
course_id = args[0] course_id = SlashSeparatedCourseKey.from_deprecated_string(args[0])
filename = args[1] filename = args[1]
except InvalidKeyError:
raise CommandError("Unparsable course_id")
except IndexError: except IndexError:
raise CommandError("Insufficient arguments") raise CommandError("Insufficient arguments")
...@@ -54,7 +58,6 @@ class Command(BaseCommand): ...@@ -54,7 +58,6 @@ class Command(BaseCommand):
def _get_results(self, filename): def _get_results(self, filename):
"""Load results from file""" """Load results from file"""
results = None
with open(filename) as f: with open(filename) as f:
results = f.read() results = f.read()
os.remove(filename) os.remove(filename)
...@@ -78,8 +81,8 @@ def export_course_to_directory(course_id, root_dir): ...@@ -78,8 +81,8 @@ def export_course_to_directory(course_id, root_dir):
if course is None: if course is None:
raise CommandError("Invalid course_id") raise CommandError("Invalid course_id")
course_name = course.location.course_id.replace('/', '-') course_name = course.id.to_deprecated_string().replace('/', '-')
export_to_xml(store, None, course.location, root_dir, course_name) export_to_xml(store, None, course.id, root_dir, course_name)
course_dir = path(root_dir) / course_name course_dir = path(root_dir) / course_name
return course_dir return course_dir
......
...@@ -38,7 +38,7 @@ def import_course(course_dir, verbose=True): ...@@ -38,7 +38,7 @@ def import_course(course_dir, verbose=True):
return None return None
course = courses[0] course = courses[0]
errors = modulestore.get_item_errors(course.location) errors = modulestore.get_course_errors(course.id)
if len(errors) != 0: if len(errors) != 0:
sys.stderr.write('ERRORs during import: {0}\n'.format('\n'.join(map(str_of_err, errors)))) sys.stderr.write('ERRORs during import: {0}\n'.format('\n'.join(map(str_of_err, errors))))
......
...@@ -22,6 +22,7 @@ from xmodule.modulestore.django import modulestore ...@@ -22,6 +22,7 @@ from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.factories import CourseFactory
from xmodule.modulestore.xml_importer import import_from_xml from xmodule.modulestore.xml_importer import import_from_xml
from xmodule.modulestore.locations import SlashSeparatedCourseKey
DATA_DIR = 'common/test/data/' DATA_DIR = 'common/test/data/'
...@@ -53,7 +54,8 @@ class CommandsTestBase(TestCase): ...@@ -53,7 +54,8 @@ class CommandsTestBase(TestCase):
modulestore=store) modulestore=store)
courses = store.get_courses() courses = store.get_courses()
if TEST_COURSE_ID not in [c.id for c in courses]: # NOTE: if xml store owns these, it won't import them into mongo
if SlashSeparatedCourseKey.from_deprecated_string(TEST_COURSE_ID) not in [c.id for c in courses]:
import_from_xml(store, DATA_DIR, ['toy', 'simple']) import_from_xml(store, DATA_DIR, ['toy', 'simple'])
return [course.id for course in store.get_courses()] return [course.id for course in store.get_courses()]
...@@ -70,7 +72,9 @@ class CommandsTestBase(TestCase): ...@@ -70,7 +72,9 @@ class CommandsTestBase(TestCase):
output = self.call_command('dump_course_ids', **kwargs) output = self.call_command('dump_course_ids', **kwargs)
dumped_courses = output.decode('utf-8').strip().split('\n') dumped_courses = output.decode('utf-8').strip().split('\n')
self.assertEqual(self.loaded_courses, dumped_courses) course_ids = {course_id.to_deprecated_string() for course_id in self.loaded_courses}
dumped_ids = set(dumped_courses)
self.assertEqual(course_ids, dumped_ids)
def test_dump_course_structure(self): def test_dump_course_structure(self):
args = [TEST_COURSE_ID] args = [TEST_COURSE_ID]
...@@ -81,16 +85,15 @@ class CommandsTestBase(TestCase): ...@@ -81,16 +85,15 @@ class CommandsTestBase(TestCase):
# check that all elements in the course structure have metadata, # check that all elements in the course structure have metadata,
# but not inherited metadata: # but not inherited metadata:
for element_name in dump: for element in dump.itervalues():
element = dump[element_name]
self.assertIn('metadata', element) self.assertIn('metadata', element)
self.assertIn('children', element) self.assertIn('children', element)
self.assertIn('category', element) self.assertIn('category', element)
self.assertNotIn('inherited_metadata', element) self.assertNotIn('inherited_metadata', element)
# Check a few elements in the course dump # Check a few elements in the course dump
test_course_key = SlashSeparatedCourseKey.from_deprecated_string(TEST_COURSE_ID)
parent_id = 'i4x://edX/simple/chapter/Overview' parent_id = test_course_key.make_usage_key('chapter', 'Overview').to_deprecated_string()
self.assertEqual(dump[parent_id]['category'], 'chapter') self.assertEqual(dump[parent_id]['category'], 'chapter')
self.assertEqual(len(dump[parent_id]['children']), 3) self.assertEqual(len(dump[parent_id]['children']), 3)
...@@ -98,7 +101,7 @@ class CommandsTestBase(TestCase): ...@@ -98,7 +101,7 @@ class CommandsTestBase(TestCase):
self.assertEqual(dump[child_id]['category'], 'videosequence') self.assertEqual(dump[child_id]['category'], 'videosequence')
self.assertEqual(len(dump[child_id]['children']), 2) self.assertEqual(len(dump[child_id]['children']), 2)
video_id = 'i4x://edX/simple/video/Welcome' video_id = test_course_key.make_usage_key('video', 'Welcome').to_deprecated_string()
self.assertEqual(dump[video_id]['category'], 'video') self.assertEqual(dump[video_id]['category'], 'video')
self.assertEqual(len(dump[video_id]['metadata']), 4) self.assertEqual(len(dump[video_id]['metadata']), 4)
self.assertIn('youtube_id_1_0', dump[video_id]['metadata']) self.assertIn('youtube_id_1_0', dump[video_id]['metadata'])
...@@ -114,8 +117,7 @@ class CommandsTestBase(TestCase): ...@@ -114,8 +117,7 @@ class CommandsTestBase(TestCase):
dump = json.loads(output) dump = json.loads(output)
# check that all elements in the course structure have inherited metadata, # check that all elements in the course structure have inherited metadata,
# and that it contains a particular value as well: # and that it contains a particular value as well:
for element_name in dump: for element in dump.itervalues():
element = dump[element_name]
self.assertIn('metadata', element) self.assertIn('metadata', element)
self.assertIn('children', element) self.assertIn('children', element)
self.assertIn('category', element) self.assertIn('category', element)
...@@ -131,8 +133,7 @@ class CommandsTestBase(TestCase): ...@@ -131,8 +133,7 @@ class CommandsTestBase(TestCase):
dump = json.loads(output) dump = json.loads(output)
# check that all elements in the course structure have inherited metadata, # check that all elements in the course structure have inherited metadata,
# and that it contains a particular value as well: # and that it contains a particular value as well:
for element_name in dump: for element in dump.itervalues():
element = dump[element_name]
self.assertIn('metadata', element) self.assertIn('metadata', element)
self.assertIn('children', element) self.assertIn('children', element)
self.assertIn('category', element) self.assertIn('category', element)
...@@ -158,7 +159,7 @@ class CommandsTestBase(TestCase): ...@@ -158,7 +159,7 @@ class CommandsTestBase(TestCase):
self.check_export_file(tar_file) self.check_export_file(tar_file)
def run_export_course(self, filename): # pylint: disable=missing-docstring def run_export_course(self, filename): # pylint: disable=missing-docstring
args = ['edX/simple/2012_Fall', filename] args = [TEST_COURSE_ID, filename]
kwargs = {'modulestore': 'default'} kwargs = {'modulestore': 'default'}
return self.call_command('export_course', *args, **kwargs) return self.call_command('export_course', *args, **kwargs)
......
...@@ -12,6 +12,7 @@ from .models import ( ...@@ -12,6 +12,7 @@ from .models import (
XModuleStudentInfoField XModuleStudentInfoField
) )
import logging import logging
from xmodule.modulestore.locations import SlashSeparatedCourseKey, Location
from django.db import DatabaseError from django.db import DatabaseError
from django.contrib.auth.models import User from django.contrib.auth.models import User
...@@ -59,6 +60,8 @@ class FieldDataCache(object): ...@@ -59,6 +60,8 @@ class FieldDataCache(object):
self.cache = {} self.cache = {}
self.descriptors = descriptors self.descriptors = descriptors
self.select_for_update = select_for_update self.select_for_update = select_for_update
assert isinstance(course_id, SlashSeparatedCourseKey)
self.course_id = course_id self.course_id = course_id
self.user = user self.user = user
...@@ -141,8 +144,8 @@ class FieldDataCache(object): ...@@ -141,8 +144,8 @@ class FieldDataCache(object):
if scope == Scope.user_state: if scope == Scope.user_state:
return self._chunked_query( return self._chunked_query(
StudentModule, StudentModule,
'module_state_key__in', 'module_id__in',
(str(descriptor.scope_ids.usage_id) for descriptor in self.descriptors), (descriptor.scope_ids.usage_id for descriptor in self.descriptors),
course_id=self.course_id, course_id=self.course_id,
student=self.user.pk, student=self.user.pk,
) )
...@@ -150,7 +153,7 @@ class FieldDataCache(object): ...@@ -150,7 +153,7 @@ class FieldDataCache(object):
return self._chunked_query( return self._chunked_query(
XModuleUserStateSummaryField, XModuleUserStateSummaryField,
'usage_id__in', 'usage_id__in',
(str(descriptor.scope_ids.usage_id) for descriptor in self.descriptors), (descriptor.scope_ids.usage_id for descriptor in self.descriptors),
field_name__in=set(field.name for field in fields), field_name__in=set(field.name for field in fields),
) )
elif scope == Scope.preferences: elif scope == Scope.preferences:
...@@ -185,9 +188,9 @@ class FieldDataCache(object): ...@@ -185,9 +188,9 @@ class FieldDataCache(object):
Return the key used in the FieldDataCache for the specified KeyValueStore key Return the key used in the FieldDataCache for the specified KeyValueStore key
""" """
if key.scope == Scope.user_state: if key.scope == Scope.user_state:
return (key.scope, key.block_scope_id.url()) return (key.scope, key.block_scope_id)
elif key.scope == Scope.user_state_summary: elif key.scope == Scope.user_state_summary:
return (key.scope, key.block_scope_id.url(), key.field_name) return (key.scope, key.block_scope_id, key.field_name)
elif key.scope == Scope.preferences: elif key.scope == Scope.preferences:
return (key.scope, key.block_scope_id, key.field_name) return (key.scope, key.block_scope_id, key.field_name)
elif key.scope == Scope.user_info: elif key.scope == Scope.user_info:
...@@ -199,9 +202,15 @@ class FieldDataCache(object): ...@@ -199,9 +202,15 @@ class FieldDataCache(object):
field field
""" """
if scope == Scope.user_state: if scope == Scope.user_state:
return (scope, field_object.module_state_key) assert (field_object.module_state_key.org == self.course_id.org and
field_object.module_state_key.course == self.course_id.course)
return (scope, field_object.module_state_key.map_into_course(self.course_id))
elif scope == Scope.user_state_summary: elif scope == Scope.user_state_summary:
return (scope, field_object.usage_id, field_object.field_name) assert (field_object.usage_id.org == self.course_id.org and
field_object.usage_id.course == self.course_id.course)
return (scope, field_object.usage_id.map_into_course(self.course_id), field_object.field_name)
elif scope == Scope.preferences: elif scope == Scope.preferences:
return (scope, field_object.module_type, field_object.field_name) return (scope, field_object.module_type, field_object.field_name)
elif scope == Scope.user_info: elif scope == Scope.user_info:
...@@ -233,10 +242,13 @@ class FieldDataCache(object): ...@@ -233,10 +242,13 @@ class FieldDataCache(object):
return field_object return field_object
if key.scope == Scope.user_state: if key.scope == Scope.user_state:
# When we start allowing block_scope_ids to be either Locations or Locators,
# this assertion will fail. Fix the code here when that happens!
assert(isinstance(key.block_scope_id, Location))
field_object, _ = StudentModule.objects.get_or_create( field_object, _ = StudentModule.objects.get_or_create(
course_id=self.course_id, course_id=self.course_id,
student=User.objects.get(id=key.user_id), student=User.objects.get(id=key.user_id),
module_state_key=key.block_scope_id.url(), module_id=key.block_scope_id.replace(run=None),
defaults={ defaults={
'state': json.dumps({}), 'state': json.dumps({}),
'module_type': key.block_scope_id.category, 'module_type': key.block_scope_id.category,
...@@ -245,7 +257,7 @@ class FieldDataCache(object): ...@@ -245,7 +257,7 @@ class FieldDataCache(object):
elif key.scope == Scope.user_state_summary: elif key.scope == Scope.user_state_summary:
field_object, _ = XModuleUserStateSummaryField.objects.get_or_create( field_object, _ = XModuleUserStateSummaryField.objects.get_or_create(
field_name=key.field_name, field_name=key.field_name,
usage_id=key.block_scope_id.url() usage_id=key.block_scope_id
) )
elif key.scope == Scope.preferences: elif key.scope == Scope.preferences:
field_object, _ = XModuleStudentPrefsField.objects.get_or_create( field_object, _ = XModuleStudentPrefsField.objects.get_or_create(
......
...@@ -18,6 +18,8 @@ from django.db import models ...@@ -18,6 +18,8 @@ from django.db import models
from django.db.models.signals import post_save from django.db.models.signals import post_save
from django.dispatch import receiver from django.dispatch import receiver
from xmodule_django.models import CourseKeyField, LocationKeyField
class StudentModule(models.Model): class StudentModule(models.Model):
""" """
...@@ -38,12 +40,31 @@ class StudentModule(models.Model): ...@@ -38,12 +40,31 @@ class StudentModule(models.Model):
# but for abtests and the like, this can be set to a shared value # but for abtests and the like, this can be set to a shared value
# for many instances of the module. # for many instances of the module.
# Filename for homeworks, etc. # Filename for homeworks, etc.
module_state_key = models.CharField(max_length=255, db_index=True, db_column='module_id') module_id = LocationKeyField(max_length=255, db_index=True, db_column='module_id')
student = models.ForeignKey(User, db_index=True) student = models.ForeignKey(User, db_index=True)
course_id = models.CharField(max_length=255, db_index=True) # TODO: This is a lie; course_id now represents something more like a course_key. We may
# or may not want to change references to this to something like course_key or course_key_field in
# this file. (Certain changes would require a DB migration which is probably not what we want.)
# Someone should look at this and reevaluate before the final merge into master.
course_id = CourseKeyField(max_length=255, db_index=True)
@property
def module_state_key(self):
"""
Returns a Location based on module_id and course_id
"""
return self.course_id.make_usage_key(self.module_id.category, self.module_id.name)
@module_state_key.setter
def module_state_key(self, usage_key):
"""
Set the module_id and course_id from the passed UsageKey
"""
self.course_id = usage_key.course_key
self.module_id = usage_key
class Meta: class Meta:
unique_together = (('student', 'module_state_key', 'course_id'),) unique_together = (('student', 'module_id', 'course_id'),)
## Internal state of the object ## Internal state of the object
state = models.TextField(null=True, blank=True) state = models.TextField(null=True, blank=True)
...@@ -110,7 +131,7 @@ class StudentModuleHistory(models.Model): ...@@ -110,7 +131,7 @@ class StudentModuleHistory(models.Model):
max_grade = models.FloatField(null=True, blank=True) max_grade = models.FloatField(null=True, blank=True)
@receiver(post_save, sender=StudentModule) @receiver(post_save, sender=StudentModule)
def save_history(sender, instance, **kwargs): def save_history(sender, instance, **kwargs): # pylint: disable=no-self-argument
if instance.module_type in StudentModuleHistory.HISTORY_SAVING_TYPES: if instance.module_type in StudentModuleHistory.HISTORY_SAVING_TYPES:
history_entry = StudentModuleHistory(student_module=instance, history_entry = StudentModuleHistory(student_module=instance,
version=None, version=None,
...@@ -133,7 +154,7 @@ class XModuleUserStateSummaryField(models.Model): ...@@ -133,7 +154,7 @@ class XModuleUserStateSummaryField(models.Model):
field_name = models.CharField(max_length=64, db_index=True) field_name = models.CharField(max_length=64, db_index=True)
# The definition id for the module # The definition id for the module
usage_id = models.CharField(max_length=255, db_index=True) usage_id = LocationKeyField(max_length=255, db_index=True)
# The value of the field. Defaults to None dumped as json # The value of the field. Defaults to None dumped as json
value = models.TextField(default='null') value = models.TextField(default='null')
...@@ -221,7 +242,7 @@ class OfflineComputedGrade(models.Model): ...@@ -221,7 +242,7 @@ class OfflineComputedGrade(models.Model):
Table of grades computed offline for a given user and course. Table of grades computed offline for a given user and course.
""" """
user = models.ForeignKey(User, db_index=True) user = models.ForeignKey(User, db_index=True)
course_id = models.CharField(max_length=255, db_index=True) course_id = CourseKeyField(max_length=255, db_index=True)
created = models.DateTimeField(auto_now_add=True, null=True, db_index=True) created = models.DateTimeField(auto_now_add=True, null=True, db_index=True)
updated = models.DateTimeField(auto_now=True, db_index=True) updated = models.DateTimeField(auto_now=True, db_index=True)
...@@ -244,10 +265,10 @@ class OfflineComputedGradeLog(models.Model): ...@@ -244,10 +265,10 @@ class OfflineComputedGradeLog(models.Model):
ordering = ["-created"] ordering = ["-created"]
get_latest_by = "created" get_latest_by = "created"
course_id = models.CharField(max_length=255, db_index=True) course_id = CourseKeyField(max_length=255, db_index=True)
created = models.DateTimeField(auto_now_add=True, null=True, db_index=True) created = models.DateTimeField(auto_now_add=True, null=True, db_index=True)
seconds = models.IntegerField(default=0) # seconds elapsed for computation seconds = models.IntegerField(default=0) # seconds elapsed for computation
nstudents = models.IntegerField(default=0) nstudents = models.IntegerField(default=0)
def __unicode__(self): def __unicode__(self):
return "[OCGLog] %s: %s" % (self.course_id, self.created) return "[OCGLog] %s: %s" % (self.course_id.to_deprecated_string(), self.created) # pylint: disable=no-member
...@@ -94,7 +94,7 @@ class BaseTestXmodule(ModuleStoreTestCase): ...@@ -94,7 +94,7 @@ class BaseTestXmodule(ModuleStoreTestCase):
#self.item_module = self.item_descriptor.xmodule_runtime.xmodule_instance #self.item_module = self.item_descriptor.xmodule_runtime.xmodule_instance
#self.item_module is None at this time #self.item_module is None at this time
self.item_url = Location(self.item_descriptor.location).url() self.item_url = self.item_descriptor.location.to_deprecated_string()
def setup_course(self): def setup_course(self):
self.course = CourseFactory.create(data=self.COURSE_DATA) self.course = CourseFactory.create(data=self.COURSE_DATA)
...@@ -139,7 +139,7 @@ class BaseTestXmodule(ModuleStoreTestCase): ...@@ -139,7 +139,7 @@ class BaseTestXmodule(ModuleStoreTestCase):
"""Return item url with dispatch.""" """Return item url with dispatch."""
return reverse( return reverse(
'xblock_handler', 'xblock_handler',
args=(self.course.id, quote_slashes(self.item_url), 'xmodule_handler', dispatch) args=(self.course.id.to_deprecated_string(), quote_slashes(self.item_url), 'xmodule_handler', dispatch)
) )
......
...@@ -6,9 +6,6 @@ from factory.django import DjangoModelFactory ...@@ -6,9 +6,6 @@ from factory.django import DjangoModelFactory
# Imported to re-export # Imported to re-export
# pylint: disable=unused-import # pylint: disable=unused-import
from student.tests.factories import UserFactory # Imported to re-export from student.tests.factories import UserFactory # Imported to re-export
from student.tests.factories import GroupFactory # Imported to re-export
from student.tests.factories import CourseEnrollmentAllowedFactory # Imported to re-export
from student.tests.factories import RegistrationFactory # Imported to re-export
# pylint: enable=unused-import # pylint: enable=unused-import
from student.tests.factories import UserProfileFactory as StudentUserProfileFactory from student.tests.factories import UserProfileFactory as StudentUserProfileFactory
...@@ -23,10 +20,11 @@ from student.roles import ( ...@@ -23,10 +20,11 @@ from student.roles import (
OrgInstructorRole, OrgInstructorRole,
) )
from xmodule.modulestore import Location from xmodule.modulestore.locations import SlashSeparatedCourseKey
location = partial(Location, 'i4x', 'edX', 'test_course', 'problem') course_id = SlashSeparatedCourseKey(u'edX', u'test_course', u'test')
location = partial(course_id.make_usage_key, u'problem')
class UserProfileFactory(StudentUserProfileFactory): class UserProfileFactory(StudentUserProfileFactory):
...@@ -41,9 +39,10 @@ class InstructorFactory(UserFactory): ...@@ -41,9 +39,10 @@ class InstructorFactory(UserFactory):
last_name = "Instructor" last_name = "Instructor"
@factory.post_generation @factory.post_generation
# TODO Change this from course to course_key at next opportunity
def course(self, create, extracted, **kwargs): def course(self, create, extracted, **kwargs):
if extracted is None: if extracted is None:
raise ValueError("Must specify a course location for a course instructor user") raise ValueError("Must specify a CourseKey for a course instructor user")
CourseInstructorRole(extracted).add_users(self) CourseInstructorRole(extracted).add_users(self)
...@@ -55,9 +54,10 @@ class StaffFactory(UserFactory): ...@@ -55,9 +54,10 @@ class StaffFactory(UserFactory):
last_name = "Staff" last_name = "Staff"
@factory.post_generation @factory.post_generation
# TODO Change this from course to course_key at next opportunity
def course(self, create, extracted, **kwargs): def course(self, create, extracted, **kwargs):
if extracted is None: if extracted is None:
raise ValueError("Must specify a course location for a course staff user") raise ValueError("Must specify a CourseKey for a course staff user")
CourseStaffRole(extracted).add_users(self) CourseStaffRole(extracted).add_users(self)
...@@ -69,9 +69,10 @@ class BetaTesterFactory(UserFactory): ...@@ -69,9 +69,10 @@ class BetaTesterFactory(UserFactory):
last_name = "Beta-Tester" last_name = "Beta-Tester"
@factory.post_generation @factory.post_generation
# TODO Change this from course to course_key at next opportunity
def course(self, create, extracted, **kwargs): def course(self, create, extracted, **kwargs):
if extracted is None: if extracted is None:
raise ValueError("Must specify a course location for a beta-tester user") raise ValueError("Must specify a CourseKey for a beta-tester user")
CourseBetaTesterRole(extracted).add_users(self) CourseBetaTesterRole(extracted).add_users(self)
...@@ -83,10 +84,11 @@ class OrgStaffFactory(UserFactory): ...@@ -83,10 +84,11 @@ class OrgStaffFactory(UserFactory):
last_name = "Org-Staff" last_name = "Org-Staff"
@factory.post_generation @factory.post_generation
# TODO Change this from course to course_key at next opportunity
def course(self, create, extracted, **kwargs): def course(self, create, extracted, **kwargs):
if extracted is None: if extracted is None:
raise ValueError("Must specify a course location for an org-staff user") raise ValueError("Must specify a CourseKey for an org-staff user")
OrgStaffRole(extracted).add_users(self) OrgStaffRole(extracted.org).add_users(self)
class OrgInstructorFactory(UserFactory): class OrgInstructorFactory(UserFactory):
...@@ -97,10 +99,11 @@ class OrgInstructorFactory(UserFactory): ...@@ -97,10 +99,11 @@ class OrgInstructorFactory(UserFactory):
last_name = "Org-Instructor" last_name = "Org-Instructor"
@factory.post_generation @factory.post_generation
# TODO Change this from course to course_key at next opportunity
def course(self, create, extracted, **kwargs): def course(self, create, extracted, **kwargs):
if extracted is None: if extracted is None:
raise ValueError("Must specify a course location for an org-instructor user") raise ValueError("Must specify a CourseKey for an org-instructor user")
OrgInstructorRole(extracted).add_users(self) OrgInstructorRole(extracted.org).add_users(self)
class GlobalStaffFactory(UserFactory): class GlobalStaffFactory(UserFactory):
...@@ -119,7 +122,7 @@ class StudentModuleFactory(DjangoModelFactory): ...@@ -119,7 +122,7 @@ class StudentModuleFactory(DjangoModelFactory):
module_type = "problem" module_type = "problem"
student = factory.SubFactory(UserFactory) student = factory.SubFactory(UserFactory)
course_id = "MITx/999/Robot_Super_Course" course_id = SlashSeparatedCourseKey("MITx", "999", "Robot_Super_Course")
state = None state = None
grade = None grade = None
max_grade = None max_grade = None
...@@ -131,7 +134,7 @@ class UserStateSummaryFactory(DjangoModelFactory): ...@@ -131,7 +134,7 @@ class UserStateSummaryFactory(DjangoModelFactory):
field_name = 'existing_field' field_name = 'existing_field'
value = json.dumps('old_value') value = json.dumps('old_value')
usage_id = location('usage_id').url() usage_id = location('usage_id')
class StudentPrefsFactory(DjangoModelFactory): class StudentPrefsFactory(DjangoModelFactory):
......
...@@ -130,7 +130,7 @@ class LoginEnrollmentTestCase(TestCase): ...@@ -130,7 +130,7 @@ class LoginEnrollmentTestCase(TestCase):
""" """
resp = self.client.post(reverse('change_enrollment'), { resp = self.client.post(reverse('change_enrollment'), {
'enrollment_action': 'enroll', 'enrollment_action': 'enroll',
'course_id': course.id, 'course_id': course.id.to_deprecated_string(),
}) })
result = resp.status_code == 200 result = resp.status_code == 200
if verify: if verify:
...@@ -142,5 +142,7 @@ class LoginEnrollmentTestCase(TestCase): ...@@ -142,5 +142,7 @@ class LoginEnrollmentTestCase(TestCase):
Unenroll the currently logged-in user, and check that it worked. Unenroll the currently logged-in user, and check that it worked.
`course` is an instance of CourseDescriptor. `course` is an instance of CourseDescriptor.
""" """
check_for_post_code(self, 200, reverse('change_enrollment'), {'enrollment_action': 'unenroll', check_for_post_code(self, 200, reverse('change_enrollment'), {
'course_id': course.id}) 'enrollment_action': 'unenroll',
'course_id': course.id.to_deprecated_string()
})
...@@ -9,6 +9,7 @@ from .helpers import LoginEnrollmentTestCase ...@@ -9,6 +9,7 @@ from .helpers import LoginEnrollmentTestCase
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.modulestore.locations import SlashSeparatedCourseKey
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
...@@ -22,13 +23,13 @@ class AboutTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): ...@@ -22,13 +23,13 @@ class AboutTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
def test_logged_in(self): def test_logged_in(self):
self.setup_user() self.setup_user()
url = reverse('about_course', args=[self.course.id]) url = reverse('about_course', args=[self.course.id.to_deprecated_string()])
resp = self.client.get(url) resp = self.client.get(url)
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
self.assertIn("OOGIE BLOOGIE", resp.content) self.assertIn("OOGIE BLOOGIE", resp.content)
def test_anonymous_user(self): def test_anonymous_user(self):
url = reverse('about_course', args=[self.course.id]) url = reverse('about_course', args=[self.course.id.to_deprecated_string()])
resp = self.client.get(url) resp = self.client.get(url)
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
self.assertIn("OOGIE BLOOGIE", resp.content) self.assertIn("OOGIE BLOOGIE", resp.content)
...@@ -39,7 +40,7 @@ class AboutTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): ...@@ -39,7 +40,7 @@ class AboutTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase):
# The following XML test course (which lives at common/test/data/2014) # The following XML test course (which lives at common/test/data/2014)
# is closed; we're testing that an about page still appears when # is closed; we're testing that an about page still appears when
# the course is already closed # the course is already closed
xml_course_id = 'edX/detached_pages/2014' xml_course_id = SlashSeparatedCourseKey('edX', 'detached_pages', '2014')
# this text appears in that course's about page # this text appears in that course's about page
# common/test/data/2014/about/overview.html # common/test/data/2014/about/overview.html
...@@ -48,14 +49,14 @@ class AboutTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): ...@@ -48,14 +49,14 @@ class AboutTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase):
@mock.patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) @mock.patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})
def test_logged_in_xml(self): def test_logged_in_xml(self):
self.setup_user() self.setup_user()
url = reverse('about_course', args=[self.xml_course_id]) url = reverse('about_course', args=[self.xml_course_id.to_deprecated_string()])
resp = self.client.get(url) resp = self.client.get(url)
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
self.assertIn(self.xml_data, resp.content) self.assertIn(self.xml_data, resp.content)
@mock.patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) @mock.patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})
def test_anonymous_user_xml(self): def test_anonymous_user_xml(self):
url = reverse('about_course', args=[self.xml_course_id]) url = reverse('about_course', args=[self.xml_course_id.to_deprecated_string()])
resp = self.client.get(url) resp = self.client.get(url)
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
self.assertIn(self.xml_data, resp.content) self.assertIn(self.xml_data, resp.content)
...@@ -82,7 +83,7 @@ class AboutWithCappedEnrollmentsTestCase(LoginEnrollmentTestCase, ModuleStoreTes ...@@ -82,7 +83,7 @@ class AboutWithCappedEnrollmentsTestCase(LoginEnrollmentTestCase, ModuleStoreTes
This test will make sure that enrollment caps are enforced This test will make sure that enrollment caps are enforced
""" """
self.setup_user() self.setup_user()
url = reverse('about_course', args=[self.course.id]) url = reverse('about_course', args=[self.course.id.to_deprecated_string()])
resp = self.client.get(url) resp = self.client.get(url)
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
self.assertIn('<a href="#" class="register">', resp.content) self.assertIn('<a href="#" class="register">', resp.content)
......
...@@ -22,13 +22,13 @@ class CourseInfoTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): ...@@ -22,13 +22,13 @@ class CourseInfoTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
def test_logged_in(self): def test_logged_in(self):
self.setup_user() self.setup_user()
url = reverse('info', args=[self.course.id]) url = reverse('info', args=[self.course.id.to_deprecated_string()])
resp = self.client.get(url) resp = self.client.get(url)
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
self.assertIn("OOGIE BLOOGIE", resp.content) self.assertIn("OOGIE BLOOGIE", resp.content)
def test_anonymous_user(self): def test_anonymous_user(self):
url = reverse('info', args=[self.course.id]) url = reverse('info', args=[self.course.id.to_deprecated_string()])
resp = self.client.get(url) resp = self.client.get(url)
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
self.assertNotIn("OOGIE BLOOGIE", resp.content) self.assertNotIn("OOGIE BLOOGIE", resp.content)
......
...@@ -4,7 +4,6 @@ Tests for course access ...@@ -4,7 +4,6 @@ Tests for course access
""" """
import mock import mock
from django.http import Http404
from django.test.utils import override_settings from django.test.utils import override_settings
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from xmodule.modulestore.django import get_default_store_name_for_current_request from xmodule.modulestore.django import get_default_store_name_for_current_request
...@@ -14,16 +13,12 @@ from xmodule.tests.xml import factories as xml ...@@ -14,16 +13,12 @@ from xmodule.tests.xml import factories as xml
from xmodule.tests.xml import XModuleXmlImportTest from xmodule.tests.xml import XModuleXmlImportTest
from courseware.courses import ( from courseware.courses import (
get_course_by_id, get_course_by_id, get_cms_course_link, course_image_url,
get_course, get_course_info_section, get_course_about_section, get_course
get_cms_course_link,
get_cms_block_link,
course_image_url,
get_course_info_section,
get_course_about_section
) )
from courseware.tests.helpers import get_request_for_user from courseware.tests.helpers import get_request_for_user
from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE, TEST_DATA_MIXED_MODULESTORE from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE, TEST_DATA_MIXED_MODULESTORE
from xmodule.modulestore.locations import SlashSeparatedCourseKey
CMS_BASE_TEST = 'testcms' CMS_BASE_TEST = 'testcms'
...@@ -34,25 +29,29 @@ class CoursesTest(ModuleStoreTestCase): ...@@ -34,25 +29,29 @@ class CoursesTest(ModuleStoreTestCase):
def test_get_course_by_id_invalid_chars(self): def test_get_course_by_id_invalid_chars(self):
""" """
Test that `get_course_by_id` throws a 404, rather than Test that `get_course` throws a 404, rather than an exception,
an exception, when faced with unexpected characters when faced with unexpected characters (such as unicode characters,
(such as unicode characters, and symbols such as = and ' ') and symbols such as = and ' ')
""" """
with self.assertRaises(Http404): with self.assertRaises(Http404):
get_course_by_id('MITx/foobar/statistics=introduction') get_course_by_id(SlashSeparatedCourseKey('MITx', 'foobar', 'business and management'))
get_course_by_id('MITx/foobar/business and management') with self.assertRaises(Http404):
get_course_by_id('MITx/foobar/NiñøJoséMaríáßç') get_course_by_id(SlashSeparatedCourseKey('MITx', 'foobar' 'statistics=introduction'))
with self.assertRaises(Http404):
get_course_by_id(SlashSeparatedCourseKey('MITx', 'foobar', 'NiñøJoséMaríáßç'))
def test_get_course_invalid_chars(self): def test_get_course_invalid_chars(self):
""" """
Test that `get_course` throws a ValueError, rather than Test that `get_course` throws a ValueError, rather than a 404,
a 404, when faced with unexpected characters when faced with unexpected characters (such as unicode characters,
(such as unicode characters, and symbols such as = and ' ') and symbols such as = and ' ')
""" """
with self.assertRaises(ValueError): with self.assertRaises(ValueError):
get_course('MITx/foobar/statistics=introduction') get_course(SlashSeparatedCourseKey('MITx', 'foobar', 'business and management'))
get_course('MITx/foobar/business and management') with self.assertRaises(ValueError):
get_course('MITx/foobar/NiñøJoséMaríáßç') get_course(SlashSeparatedCourseKey('MITx', 'foobar', 'statistics=introduction'))
with self.assertRaises(ValueError):
get_course(SlashSeparatedCourseKey('MITx', 'foobar', 'NiñøJoséMaríáßç'))
@override_settings( @override_settings(
MODULESTORE=TEST_DATA_MONGO_MODULESTORE, CMS_BASE=CMS_BASE_TEST MODULESTORE=TEST_DATA_MONGO_MODULESTORE, CMS_BASE=CMS_BASE_TEST
...@@ -67,6 +66,7 @@ class CoursesTest(ModuleStoreTestCase): ...@@ -67,6 +66,7 @@ class CoursesTest(ModuleStoreTestCase):
org='org', number='num', display_name='name' org='org', number='num', display_name='name'
) )
cms_url = u"//{}/course/slashes:org+num+name".format(CMS_BASE_TEST)
self.assertEqual(cms_url, get_cms_course_link(self.course)) self.assertEqual(cms_url, get_cms_course_link(self.course))
self.assertEqual(cms_url, get_cms_block_link(self.course, 'course')) self.assertEqual(cms_url, get_cms_block_link(self.course, 'course'))
...@@ -146,10 +146,11 @@ class XmlCourseImageTestCase(XModuleXmlImportTest): ...@@ -146,10 +146,11 @@ class XmlCourseImageTestCase(XModuleXmlImportTest):
class CoursesRenderTest(ModuleStoreTestCase): class CoursesRenderTest(ModuleStoreTestCase):
"""Test methods related to rendering courses content.""" """Test methods related to rendering courses content."""
toy_course_key = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
def test_get_course_info_section_render(self): def test_get_course_info_section_render(self):
course = get_course_by_id('edX/toy/2012_Fall') course = get_course_by_id(self.toy_course_key)
request = get_request_for_user(UserFactory.create()) request = get_request_for_user(UserFactory.create())
# Test render works okay # Test render works okay
...@@ -167,7 +168,7 @@ class CoursesRenderTest(ModuleStoreTestCase): ...@@ -167,7 +168,7 @@ class CoursesRenderTest(ModuleStoreTestCase):
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@mock.patch('courseware.courses.get_request_for_thread') @mock.patch('courseware.courses.get_request_for_thread')
def test_get_course_about_section_render(self, mock_get_request): def test_get_course_about_section_render(self, mock_get_request):
course = get_course_by_id('edX/toy/2012_Fall') course = get_course_by_id(self.toy_course_key)
request = get_request_for_user(UserFactory.create()) request = get_request_for_user(UserFactory.create())
mock_get_request.return_value = request mock_get_request.return_value = request
......
...@@ -2,7 +2,7 @@ from django.test import TestCase ...@@ -2,7 +2,7 @@ from django.test import TestCase
from django.test.utils import override_settings from django.test.utils import override_settings
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.modulestore import Location from xmodule.modulestore.locations import SlashSeparatedCourseKey
from modulestore_config import TEST_DATA_DRAFT_MONGO_MODULESTORE from modulestore_config import TEST_DATA_DRAFT_MONGO_MODULESTORE
...@@ -13,8 +13,7 @@ class TestDraftModuleStore(TestCase): ...@@ -13,8 +13,7 @@ class TestDraftModuleStore(TestCase):
store = modulestore() store = modulestore()
# fix was to allow get_items() to take the course_id parameter # fix was to allow get_items() to take the course_id parameter
store.get_items(Location(None, None, 'vertical', None, None), store.get_items(SlashSeparatedCourseKey('a', 'b', 'c'), category='vertical')
course_id='abc', depth=0)
# test success is just getting through the above statement. # test success is just getting through the above statement.
# The bug was that 'course_id' argument was # The bug was that 'course_id' argument was
......
...@@ -9,6 +9,7 @@ from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE ...@@ -9,6 +9,7 @@ from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.factories import CourseFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.locations import SlashSeparatedCourseKey
from courseware.grades import grade, iterate_grades_for from courseware.grades import grade, iterate_grades_for
...@@ -62,7 +63,7 @@ class TestGradeIteration(ModuleStoreTestCase): ...@@ -62,7 +63,7 @@ class TestGradeIteration(ModuleStoreTestCase):
should be raised. This is a horrible crossing of abstraction boundaries should be raised. This is a horrible crossing of abstraction boundaries
and should be fixed, but for now we're just testing the behavior. :-(""" and should be fixed, but for now we're just testing the behavior. :-("""
with self.assertRaises(Http404): with self.assertRaises(Http404):
gradeset_results = iterate_grades_for("I/dont/exist", []) gradeset_results = iterate_grades_for(SlashSeparatedCourseKey("I", "dont", "exist"), [])
gradeset_results.next() gradeset_results.next()
def test_all_empty_grades(self): def test_all_empty_grades(self):
......
...@@ -27,7 +27,8 @@ class TestLTI(BaseTestXmodule): ...@@ -27,7 +27,8 @@ class TestLTI(BaseTestXmodule):
mocked_signature_after_sign = u'my_signature%3D' mocked_signature_after_sign = u'my_signature%3D'
mocked_decoded_signature = u'my_signature=' mocked_decoded_signature = u'my_signature='
context_id = self.item_descriptor.course_id # TODO this course_id is actually a course_key; please change this ASAP!
context_id = self.item_descriptor.course_id.to_deprecated_string()
user_id = unicode(self.item_descriptor.xmodule_runtime.anonymous_student_id) user_id = unicode(self.item_descriptor.xmodule_runtime.anonymous_student_id)
hostname = self.item_descriptor.xmodule_runtime.hostname hostname = self.item_descriptor.xmodule_runtime.hostname
resource_link_id = unicode(urllib.quote('{}-{}'.format(hostname, self.item_descriptor.location.html_id()))) resource_link_id = unicode(urllib.quote('{}-{}'.format(hostname, self.item_descriptor.location.html_id())))
...@@ -38,10 +39,6 @@ class TestLTI(BaseTestXmodule): ...@@ -38,10 +39,6 @@ class TestLTI(BaseTestXmodule):
user_id=user_id user_id=user_id
) )
lis_outcome_service_url = 'https://{host}{path}'.format(
host=hostname,
path=self.item_descriptor.xmodule_runtime.handler_url(self.item_descriptor, 'grade_handler', thirdparty=True).rstrip('/?')
)
self.correct_headers = { self.correct_headers = {
u'user_id': user_id, u'user_id': user_id,
u'oauth_callback': u'about:blank', u'oauth_callback': u'about:blank',
......
...@@ -12,14 +12,14 @@ import json ...@@ -12,14 +12,14 @@ import json
from django.test.utils import override_settings from django.test.utils import override_settings
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.contrib.auth.models import User
from courseware.tests.factories import StaffFactory
from courseware.tests.helpers import LoginEnrollmentTestCase from courseware.tests.helpers import LoginEnrollmentTestCase
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
from student.roles import CourseStaffRole
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.django import modulestore, clear_existing_modulestores from xmodule.modulestore.django import modulestore, clear_existing_modulestores
from lms.lib.xblock.runtime import quote_slashes from lms.lib.xblock.runtime import quote_slashes
from xmodule.modulestore.locations import SlashSeparatedCourseKey
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
...@@ -33,26 +33,19 @@ class TestStaffMasqueradeAsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase) ...@@ -33,26 +33,19 @@ class TestStaffMasqueradeAsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
# Clear out the modulestores, causing them to reload # Clear out the modulestores, causing them to reload
clear_existing_modulestores() clear_existing_modulestores()
self.graded_course = modulestore().get_course("edX/graded/2012_Fall") self.graded_course = modulestore().get_course(SlashSeparatedCourseKey("edX", "graded", "2012_Fall"))
# Create staff account # Create staff account
self.instructor = 'view2@test.com' self.staff = StaffFactory(course=self.graded_course.id)
self.password = 'foo'
self.create_account('u2', self.instructor, self.password)
self.activate_user(self.instructor)
def make_instructor(course):
CourseStaffRole(course.location).add_users(User.objects.get(email=self.instructor))
make_instructor(self.graded_course)
self.logout() self.logout()
self.login(self.instructor, self.password) # self.staff.password is the sha hash but login takes the plain text
self.login(self.staff.email, 'test')
self.enroll(self.graded_course) self.enroll(self.graded_course)
def get_cw_section(self): def get_cw_section(self):
url = reverse('courseware_section', url = reverse('courseware_section',
kwargs={'course_id': self.graded_course.id, kwargs={'course_id': self.graded_course.id.to_deprecated_string(),
'chapter': 'GradedChapter', 'chapter': 'GradedChapter',
'section': 'Homework1'}) 'section': 'Homework1'})
...@@ -64,7 +57,7 @@ class TestStaffMasqueradeAsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase) ...@@ -64,7 +57,7 @@ class TestStaffMasqueradeAsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
def test_staff_debug_for_staff(self): def test_staff_debug_for_staff(self):
resp = self.get_cw_section() resp = self.get_cw_section()
sdebug = 'Staff Debug Info' sdebug = 'Staff Debug Info'
print resp.content
self.assertTrue(sdebug in resp.content) self.assertTrue(sdebug in resp.content)
def toggle_masquerade(self): def toggle_masquerade(self):
...@@ -88,11 +81,11 @@ class TestStaffMasqueradeAsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase) ...@@ -88,11 +81,11 @@ class TestStaffMasqueradeAsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
def get_problem(self): def get_problem(self):
pun = 'H1P1' pun = 'H1P1'
problem_location = "i4x://edX/graded/problem/%s" % pun problem_location = self.graded_course.id.make_usage_key("problem", pun)
modx_url = reverse('xblock_handler', modx_url = reverse('xblock_handler',
kwargs={'course_id': self.graded_course.id, kwargs={'course_id': self.graded_course.id.to_deprecated_string(),
'usage_id': quote_slashes(problem_location), 'usage_id': quote_slashes(problem_location.to_deprecated_string()),
'handler': 'xmodule_handler', 'handler': 'xmodule_handler',
'suffix': 'problem_get'}) 'suffix': 'problem_get'})
......
...@@ -11,12 +11,11 @@ from courseware.models import StudentModule, XModuleUserStateSummaryField ...@@ -11,12 +11,11 @@ from courseware.models import StudentModule, XModuleUserStateSummaryField
from courseware.models import XModuleStudentInfoField, XModuleStudentPrefsField from courseware.models import XModuleStudentInfoField, XModuleStudentPrefsField
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from courseware.tests.factories import StudentModuleFactory as cmfStudentModuleFactory from courseware.tests.factories import StudentModuleFactory as cmfStudentModuleFactory, location, course_id
from courseware.tests.factories import UserStateSummaryFactory from courseware.tests.factories import UserStateSummaryFactory
from courseware.tests.factories import StudentPrefsFactory, StudentInfoFactory from courseware.tests.factories import StudentPrefsFactory, StudentInfoFactory
from xblock.fields import Scope, BlockScope, ScopeIds from xblock.fields import Scope, BlockScope, ScopeIds
from xmodule.modulestore import Location
from django.test import TestCase from django.test import TestCase
from django.db import DatabaseError from django.db import DatabaseError
from xblock.core import KeyValueMultiSaveError from xblock.core import KeyValueMultiSaveError
...@@ -37,9 +36,6 @@ def mock_descriptor(fields=[]): ...@@ -37,9 +36,6 @@ def mock_descriptor(fields=[]):
descriptor.module_class.__name__ = 'MockProblemModule' descriptor.module_class.__name__ = 'MockProblemModule'
return descriptor return descriptor
location = partial(Location, 'i4x', 'edX', 'test_course', 'problem')
course_id = 'edX/test_course/test'
# The user ids here are 1 because we make a student in the setUp functions, and # The user ids here are 1 because we make a student in the setUp functions, and
# they get an id of 1. There's an assertion in setUp to ensure that assumption # they get an id of 1. There's an assertion in setUp to ensure that assumption
# is still true. # is still true.
...@@ -51,7 +47,7 @@ user_info_key = partial(DjangoKeyValueStore.Key, Scope.user_info, 1, None) ...@@ -51,7 +47,7 @@ user_info_key = partial(DjangoKeyValueStore.Key, Scope.user_info, 1, None)
class StudentModuleFactory(cmfStudentModuleFactory): class StudentModuleFactory(cmfStudentModuleFactory):
module_state_key = location('usage_id').url() module_state_key = location('usage_id')
course_id = course_id course_id = course_id
...@@ -204,7 +200,7 @@ class TestMissingStudentModule(TestCase): ...@@ -204,7 +200,7 @@ class TestMissingStudentModule(TestCase):
student_module = StudentModule.objects.all()[0] student_module = StudentModule.objects.all()[0]
self.assertEquals({'a_field': 'a_value'}, json.loads(student_module.state)) self.assertEquals({'a_field': 'a_value'}, json.loads(student_module.state))
self.assertEquals(self.user, student_module.student) self.assertEquals(self.user, student_module.student)
self.assertEquals(location('usage_id').url(), student_module.module_state_key) self.assertEquals(location('usage_id'), student_module.module_state_key)
self.assertEquals(course_id, student_module.course_id) self.assertEquals(course_id, student_module.course_id)
def test_delete_field_from_missing_student_module(self): def test_delete_field_from_missing_student_module(self):
...@@ -317,12 +313,12 @@ class StorageTestBase(object): ...@@ -317,12 +313,12 @@ class StorageTestBase(object):
self.assertEquals(exception.saved_field_names[0], 'existing_field') self.assertEquals(exception.saved_field_names[0], 'existing_field')
class TestContentStorage(StorageTestBase, TestCase): class TestUserStateSummaryStorage(StorageTestBase, TestCase):
"""Tests for ContentStorage""" """Tests for UserStateSummaryStorage"""
factory = UserStateSummaryFactory factory = UserStateSummaryFactory
scope = Scope.user_state_summary scope = Scope.user_state_summary
key_factory = user_state_summary_key key_factory = user_state_summary_key
storage_class = XModuleUserStateSummaryField storage_class = factory.FACTORY_FOR
class TestStudentPrefsStorage(OtherUserFailureTestMixin, StorageTestBase, TestCase): class TestStudentPrefsStorage(OtherUserFailureTestMixin, StorageTestBase, TestCase):
......
...@@ -75,10 +75,10 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -75,10 +75,10 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase):
self.enroll(self.test_course, True) self.enroll(self.test_course, True)
resp = self.client.get(reverse('courseware', resp = self.client.get(reverse('courseware',
kwargs={'course_id': self.course.id})) kwargs={'course_id': self.course.id.to_deprecated_string()}))
self.assertRedirects(resp, reverse( self.assertRedirects(resp, reverse(
'courseware_section', kwargs={'course_id': self.course.id, 'courseware_section', kwargs={'course_id': self.course.id.to_deprecated_string(),
'chapter': 'Overview', 'chapter': 'Overview',
'section': 'Welcome'})) 'section': 'Welcome'}))
...@@ -92,16 +92,22 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -92,16 +92,22 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase):
self.enroll(self.course, True) self.enroll(self.course, True)
self.enroll(self.test_course, True) self.enroll(self.test_course, True)
self.client.get(reverse('courseware_section', kwargs={'course_id': self.course.id, self.client.get(reverse('courseware_section', kwargs={
'course_id': self.course.id.to_deprecated_string(),
'chapter': 'Overview', 'chapter': 'Overview',
'section': 'Welcome'})) 'section': 'Welcome',
}))
resp = self.client.get(reverse('courseware', resp = self.client.get(reverse('courseware',
kwargs={'course_id': self.course.id})) kwargs={'course_id': self.course.id.to_deprecated_string()}))
self.assertRedirects(resp, reverse('courseware_chapter', self.assertRedirects(resp, reverse(
kwargs={'course_id': self.course.id, 'courseware_chapter',
'chapter': 'Overview'})) kwargs={
'course_id': self.course.id.to_deprecated_string(),
'chapter': 'Overview'
}
))
def test_accordion_state(self): def test_accordion_state(self):
""" """
...@@ -113,15 +119,19 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -113,15 +119,19 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase):
self.enroll(self.test_course, True) self.enroll(self.test_course, True)
# Now we directly navigate to a section in a chapter other than 'Overview'. # Now we directly navigate to a section in a chapter other than 'Overview'.
check_for_get_code(self, 200, reverse('courseware_section', check_for_get_code(self, 200, reverse(
kwargs={'course_id': self.course.id, 'courseware_section',
kwargs={
'course_id': self.course.id.to_deprecated_string(),
'chapter': 'factory_chapter', 'chapter': 'factory_chapter',
'section': 'factory_section'})) 'section': 'factory_section'
}
))
# And now hitting the courseware tab should redirect to 'factory_chapter' # And now hitting the courseware tab should redirect to 'factory_chapter'
resp = self.client.get(reverse('courseware', resp = self.client.get(reverse('courseware',
kwargs={'course_id': self.course.id})) kwargs={'course_id': self.course.id.to_deprecated_string()}))
self.assertRedirects(resp, reverse('courseware_chapter', self.assertRedirects(resp, reverse('courseware_chapter',
kwargs={'course_id': self.course.id, kwargs={'course_id': self.course.id.to_deprecated_string(),
'chapter': 'factory_chapter'})) 'chapter': 'factory_chapter'}))
...@@ -113,11 +113,10 @@ class SplitTestBase(ModuleStoreTestCase): ...@@ -113,11 +113,10 @@ class SplitTestBase(ModuleStoreTestCase):
resp = self.client.get(reverse( resp = self.client.get(reverse(
'courseware_section', 'courseware_section',
kwargs={'course_id': self.course.id, kwargs={'course_id': self.course.id.to_deprecated_string(),
'chapter': self.chapter.url_name, 'chapter': self.chapter.url_name,
'section': self.sequential.url_name} 'section': self.sequential.url_name}
)) ))
content = resp.content content = resp.content
# Assert we see the proper icon in the top display # Assert we see the proper icon in the top display
...@@ -176,15 +175,15 @@ class TestVertSplitTestVert(SplitTestBase): ...@@ -176,15 +175,15 @@ class TestVertSplitTestVert(SplitTestBase):
display_name="Split test vertical", display_name="Split test vertical",
) )
# pylint: disable=protected-access # pylint: disable=protected-access
c0_url = self.course.location._replace(category="vertical", name="split_test_cond0") c0_url = self.course.id.make_usage_key("vertical", "split_test_cond0")
c1_url = self.course.location._replace(category="vertical", name="split_test_cond1") c1_url = self.course.id.make_usage_key("vertical", "split_test_cond1")
split_test = ItemFactory.create( split_test = ItemFactory.create(
parent_location=vert1.location, parent_location=vert1.location,
category="split_test", category="split_test",
display_name="Split test", display_name="Split test",
user_partition_id='0', user_partition_id='0',
group_id_to_child={"0": c0_url.url(), "1": c1_url.url()}, group_id_to_child={"0": c0_url, "1": c1_url},
) )
cond0vert = ItemFactory.create( cond0vert = ItemFactory.create(
...@@ -242,15 +241,15 @@ class TestSplitTestVert(SplitTestBase): ...@@ -242,15 +241,15 @@ class TestSplitTestVert(SplitTestBase):
# split_test cond 0 = vert <- {video, problem} # split_test cond 0 = vert <- {video, problem}
# split_test cond 1 = vert <- {video, html} # split_test cond 1 = vert <- {video, html}
# pylint: disable=protected-access # pylint: disable=protected-access
c0_url = self.course.location._replace(category="vertical", name="split_test_cond0") c0_url = self.course.id.make_usage_key("vertical", "split_test_cond0")
c1_url = self.course.location._replace(category="vertical", name="split_test_cond1") c1_url = self.course.id.make_usage_key("vertical", "split_test_cond1")
split_test = ItemFactory.create( split_test = ItemFactory.create(
parent_location=self.sequential.location, parent_location=self.sequential.location,
category="split_test", category="split_test",
display_name="Split test", display_name="Split test",
user_partition_id='0', user_partition_id='0',
group_id_to_child={"0": c0_url.url(), "1": c1_url.url()}, group_id_to_child={"0": c0_url, "1": c1_url},
) )
cond0vert = ItemFactory.create( cond0vert = ItemFactory.create(
......
...@@ -46,6 +46,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -46,6 +46,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase):
def setUp(self): def setUp(self):
super(TestSubmittingProblems, self).setUp()
# Create course # Create course
self.course = CourseFactory.create(display_name=self.COURSE_NAME, number=self.COURSE_SLUG) self.course = CourseFactory.create(display_name=self.COURSE_NAME, number=self.COURSE_SLUG)
assert self.course, "Couldn't load course %r" % self.COURSE_NAME assert self.course, "Couldn't load course %r" % self.COURSE_NAME
...@@ -63,14 +64,14 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -63,14 +64,14 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase):
""" """
Re-fetch the course from the database so that the object being dealt with has everything added to it. Re-fetch the course from the database so that the object being dealt with has everything added to it.
""" """
self.course = modulestore().get_instance(self.course.id, self.course.location) self.course = modulestore().get_course(self.course.id)
def problem_location(self, problem_url_name): def problem_location(self, problem_url_name):
""" """
Returns the url of the problem given the problem's name Returns the url of the problem given the problem's name
""" """
return "i4x://" + self.course.org + "/{}/problem/{}".format(self.COURSE_SLUG, problem_url_name) return self.course.id.make_usage_key('problem', problem_url_name)
def modx_url(self, problem_location, dispatch): def modx_url(self, problem_location, dispatch):
""" """
...@@ -84,8 +85,8 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -84,8 +85,8 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase):
return reverse( return reverse(
'xblock_handler', 'xblock_handler',
kwargs={ kwargs={
'course_id': self.course.id, 'course_id': self.course.id.to_deprecated_string(),
'usage_id': quote_slashes(problem_location), 'usage_id': quote_slashes(problem_location.to_deprecated_string()),
'handler': 'xmodule_handler', 'handler': 'xmodule_handler',
'suffix': dispatch, 'suffix': dispatch,
} }
...@@ -247,7 +248,7 @@ class TestCourseGrader(TestSubmittingProblems): ...@@ -247,7 +248,7 @@ class TestCourseGrader(TestSubmittingProblems):
""" """
fake_request = self.factory.get( fake_request = self.factory.get(
reverse('progress', kwargs={'course_id': self.course.id}) reverse('progress', kwargs={'course_id': self.course.id.to_deprecated_string()})
) )
return grades.grade(self.student_user, fake_request, self.course) return grades.grade(self.student_user, fake_request, self.course)
...@@ -265,7 +266,7 @@ class TestCourseGrader(TestSubmittingProblems): ...@@ -265,7 +266,7 @@ class TestCourseGrader(TestSubmittingProblems):
""" """
fake_request = self.factory.get( fake_request = self.factory.get(
reverse('progress', kwargs={'course_id': self.course.id}) reverse('progress', kwargs={'course_id': self.course.id.to_deprecated_string()})
) )
progress_summary = grades.progress_summary( progress_summary = grades.progress_summary(
...@@ -493,7 +494,7 @@ class TestCourseGrader(TestSubmittingProblems): ...@@ -493,7 +494,7 @@ class TestCourseGrader(TestSubmittingProblems):
# score read from StudentModule and our student gets an A instead. # score read from StudentModule and our student gets an A instead.
with patch('submissions.api.get_scores') as mock_get_scores: with patch('submissions.api.get_scores') as mock_get_scores:
mock_get_scores.return_value = { mock_get_scores.return_value = {
self.problem_location('p3'): (1, 1) self.problem_location('p3').to_deprecated_string(): (1, 1)
} }
self.check_grade_percent(1.0) self.check_grade_percent(1.0)
self.assertEqual(self.get_grade_summary()['grade'], 'A') self.assertEqual(self.get_grade_summary()['grade'], 'A')
...@@ -509,12 +510,14 @@ class TestCourseGrader(TestSubmittingProblems): ...@@ -509,12 +510,14 @@ class TestCourseGrader(TestSubmittingProblems):
with patch('submissions.api.get_scores') as mock_get_scores: with patch('submissions.api.get_scores') as mock_get_scores:
mock_get_scores.return_value = { mock_get_scores.return_value = {
self.problem_location('p3'): (1, 1) self.problem_location('p3').to_deprecated_string(): (1, 1)
} }
self.get_grade_summary() self.get_grade_summary()
# Verify that the submissions API was sent an anonymized student ID # Verify that the submissions API was sent an anonymized student ID
mock_get_scores.assert_called_with(self.course.id, '99ac6730dc5f900d69fd735975243b31') mock_get_scores.assert_called_with(
self.course.id.to_deprecated_string(), '99ac6730dc5f900d69fd735975243b31'
)
def test_weighted_homework(self): def test_weighted_homework(self):
""" """
...@@ -631,7 +634,7 @@ class ProblemWithUploadedFilesTest(TestSubmittingProblems): ...@@ -631,7 +634,7 @@ class ProblemWithUploadedFilesTest(TestSubmittingProblems):
self.addCleanup(fileobj.close) self.addCleanup(fileobj.close)
self.problem_setup("the_problem", filenames) self.problem_setup("the_problem", filenames)
with patch('courseware.module_render.xqueue_interface.session') as mock_session: with patch('courseware.module_render.XQUEUE_INTERFACE.session') as mock_session:
resp = self.submit_question_answer("the_problem", {'2_1': fileobjs}) resp = self.submit_question_answer("the_problem", {'2_1': fileobjs})
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
...@@ -946,7 +949,7 @@ class TestAnswerDistributions(TestSubmittingProblems): ...@@ -946,7 +949,7 @@ class TestAnswerDistributions(TestSubmittingProblems):
user2 = UserFactory.create() user2 = UserFactory.create()
problems = StudentModule.objects.filter( problems = StudentModule.objects.filter(
course_id=self.course.id, course_id=self.course.id,
student_id=self.student_user.id student=self.student_user
) )
for problem in problems: for problem in problems:
problem.student_id = user2.id problem.student_id = user2.id
...@@ -981,7 +984,7 @@ class TestAnswerDistributions(TestSubmittingProblems): ...@@ -981,7 +984,7 @@ class TestAnswerDistributions(TestSubmittingProblems):
# Now fetch the state entry for that problem. # Now fetch the state entry for that problem.
student_module = StudentModule.objects.get( student_module = StudentModule.objects.get(
course_id=self.course.id, course_id=self.course.id,
student_id=self.student_user.id student=self.student_user
) )
for val in ('Correct', True, False, 0, 0.0, 1, 1.0, None): for val in ('Correct', True, False, 0, 0.0, 1, 1.0, None):
state = json.loads(student_module.state) state = json.loads(student_module.state)
...@@ -1008,9 +1011,11 @@ class TestAnswerDistributions(TestSubmittingProblems): ...@@ -1008,9 +1011,11 @@ class TestAnswerDistributions(TestSubmittingProblems):
# to a non-existent problem. # to a non-existent problem.
student_module = StudentModule.objects.get( student_module = StudentModule.objects.get(
course_id=self.course.id, course_id=self.course.id,
student_id=self.student_user.id student=self.student_user
)
student_module.module_state_key = student_module.module_state_key.replace(
name=student_module.module_state_key.name + "_fake"
) )
student_module.module_state_key += "_fake"
student_module.save() student_module.save()
# It should be empty (ignored) # It should be empty (ignored)
...@@ -1027,7 +1032,7 @@ class TestAnswerDistributions(TestSubmittingProblems): ...@@ -1027,7 +1032,7 @@ class TestAnswerDistributions(TestSubmittingProblems):
# Now fetch the StudentModule entry for p1 so we can corrupt its state # Now fetch the StudentModule entry for p1 so we can corrupt its state
prb1 = StudentModule.objects.get( prb1 = StudentModule.objects.get(
course_id=self.course.id, course_id=self.course.id,
student_id=self.student_user.id student=self.student_user
) )
# Submit p2 # Submit p2
......
...@@ -15,6 +15,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase ...@@ -15,6 +15,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from courseware.tests.helpers import get_request_for_user, LoginEnrollmentTestCase from courseware.tests.helpers import get_request_for_user, LoginEnrollmentTestCase
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
from xmodule.modulestore.locations import SlashSeparatedCourseKey
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
...@@ -27,29 +28,30 @@ class StaticTabDateTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): ...@@ -27,29 +28,30 @@ class StaticTabDateTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
category="static_tab", parent_location=self.course.location, category="static_tab", parent_location=self.course.location,
data="OOGIE BLOOGIE", display_name="new_tab" data="OOGIE BLOOGIE", display_name="new_tab"
) )
self.toy_course_key = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')
def test_logged_in(self): def test_logged_in(self):
self.setup_user() self.setup_user()
url = reverse('static_tab', args=[self.course.id, 'new_tab']) url = reverse('static_tab', args=[self.course.id.to_deprecated_string(), 'new_tab'])
resp = self.client.get(url) resp = self.client.get(url)
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
self.assertIn("OOGIE BLOOGIE", resp.content) self.assertIn("OOGIE BLOOGIE", resp.content)
def test_anonymous_user(self): def test_anonymous_user(self):
url = reverse('static_tab', args=[self.course.id, 'new_tab']) url = reverse('static_tab', args=[self.course.id.to_deprecated_string(), 'new_tab'])
resp = self.client.get(url) resp = self.client.get(url)
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
self.assertIn("OOGIE BLOOGIE", resp.content) self.assertIn("OOGIE BLOOGIE", resp.content)
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
def test_get_static_tab_contents(self): def test_get_static_tab_contents(self):
course = get_course_by_id('edX/toy/2012_Fall') course = get_course_by_id(self.toy_course_key)
request = get_request_for_user(UserFactory.create()) request = get_request_for_user(UserFactory.create())
tab = CourseTabList.get_tab_by_slug(course.tabs, 'resources') tab = CourseTabList.get_tab_by_slug(course.tabs, 'resources')
# Test render works okay # Test render works okay
tab_content = get_static_tab_contents(request, course, tab) tab_content = get_static_tab_contents(request, course, tab)
self.assertIn('edX/toy/2012_Fall', tab_content) self.assertIn(self.toy_course_key.to_deprecated_string(), tab_content)
self.assertIn('static_tab', tab_content) self.assertIn('static_tab', tab_content)
# Test when render raises an exception # Test when render raises an exception
...@@ -66,7 +68,7 @@ class StaticTabDateTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): ...@@ -66,7 +68,7 @@ class StaticTabDateTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase):
# The following XML test course (which lives at common/test/data/2014) # The following XML test course (which lives at common/test/data/2014)
# is closed; we're testing that tabs still appear when # is closed; we're testing that tabs still appear when
# the course is already closed # the course is already closed
xml_course_id = 'edX/detached_pages/2014' xml_course_key = SlashSeparatedCourseKey('edX', 'detached_pages', '2014')
# this text appears in the test course's tab # this text appears in the test course's tab
# common/test/data/2014/tabs/8e4cce2b4aaf4ba28b1220804619e41f.html # common/test/data/2014/tabs/8e4cce2b4aaf4ba28b1220804619e41f.html
...@@ -76,14 +78,14 @@ class StaticTabDateTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): ...@@ -76,14 +78,14 @@ class StaticTabDateTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase):
@patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})
def test_logged_in_xml(self): def test_logged_in_xml(self):
self.setup_user() self.setup_user()
url = reverse('static_tab', args=[self.xml_course_id, self.xml_url]) url = reverse('static_tab', args=[self.xml_course_key.to_deprecated_string(), self.xml_url])
resp = self.client.get(url) resp = self.client.get(url)
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
self.assertIn(self.xml_data, resp.content) self.assertIn(self.xml_data, resp.content)
@patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})
def test_anonymous_user_xml(self): def test_anonymous_user_xml(self):
url = reverse('static_tab', args=[self.xml_course_id, self.xml_url]) url = reverse('static_tab', args=[self.xml_course_key.to_deprecated_string(), self.xml_url])
resp = self.client.get(url) resp = self.client.get(url)
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
self.assertIn(self.xml_data, resp.content) self.assertIn(self.xml_data, resp.content)
......
...@@ -10,7 +10,6 @@ from datetime import timedelta ...@@ -10,7 +10,6 @@ from datetime import timedelta
from webob import Request from webob import Request
from xmodule.contentstore.content import StaticContent from xmodule.contentstore.content import StaticContent
from xmodule.modulestore import Location
from xmodule.contentstore.django import contentstore from xmodule.contentstore.django import contentstore
from . import BaseTestXmodule from . import BaseTestXmodule
from .test_video_xml import SOURCE_XML from .test_video_xml import SOURCE_XML
...@@ -21,6 +20,7 @@ from xmodule.video_module.transcripts_utils import ( ...@@ -21,6 +20,7 @@ from xmodule.video_module.transcripts_utils import (
TranscriptException, TranscriptException,
TranscriptsGenerationException, TranscriptsGenerationException,
) )
from xmodule.modulestore.mongo.base import MongoModuleStore
SRT_content = textwrap.dedent(""" SRT_content = textwrap.dedent("""
0 0
...@@ -46,7 +46,7 @@ def _check_asset(location, asset_name): ...@@ -46,7 +46,7 @@ def _check_asset(location, asset_name):
Check that asset with asset_name exists in assets. Check that asset with asset_name exists in assets.
""" """
content_location = StaticContent.compute_location( content_location = StaticContent.compute_location(
location.org, location.course, asset_name location.course_key, asset_name
) )
try: try:
contentstore().find(content_location) contentstore().find(content_location)
...@@ -61,16 +61,12 @@ def _clear_assets(location): ...@@ -61,16 +61,12 @@ def _clear_assets(location):
""" """
store = contentstore() store = contentstore()
content_location = StaticContent.compute_location( assets, __ = store.get_all_content_for_course(location.course_key)
location.org, location.course, location.name
)
assets, __ = store.get_all_content_for_course(content_location)
for asset in assets: for asset in assets:
asset_location = Location(asset["_id"]) asset_location = MongoModuleStore._location_from_id(asset["_id"], location.course_key.run)
del_cached_content(asset_location) del_cached_content(asset_location)
id = StaticContent.get_id_from_location(asset_location) mongo_id = StaticContent.get_id_from_location(asset_location)
store.delete(id) store.delete(mongo_id)
def _get_subs_id(filename): def _get_subs_id(filename):
...@@ -97,7 +93,7 @@ def _upload_sjson_file(subs_file, location, default_filename='subs_{}.srt.sjson' ...@@ -97,7 +93,7 @@ def _upload_sjson_file(subs_file, location, default_filename='subs_{}.srt.sjson'
def _upload_file(subs_file, location, filename): def _upload_file(subs_file, location, filename):
mime_type = subs_file.content_type mime_type = subs_file.content_type
content_location = StaticContent.compute_location( content_location = StaticContent.compute_location(
location.org, location.course, filename location.course_key, filename
) )
content = StaticContent(content_location, filename, mime_type, subs_file.read()) content = StaticContent(content_location, filename, mime_type, subs_file.read())
contentstore().save(content) contentstore().save(content)
......
...@@ -14,6 +14,7 @@ from xmodule.video_module import create_youtube_string ...@@ -14,6 +14,7 @@ from xmodule.video_module import create_youtube_string
from xmodule.tests import get_test_descriptor_system from xmodule.tests import get_test_descriptor_system
from xmodule.modulestore import Location from xmodule.modulestore import Location
from xmodule.video_module import VideoDescriptor from xmodule.video_module import VideoDescriptor
from xmodule.modulestore.locations import SlashSeparatedCourseKey
from . import BaseTestXmodule from . import BaseTestXmodule
from .test_video_xml import SOURCE_XML from .test_video_xml import SOURCE_XML
...@@ -511,10 +512,11 @@ class VideoDescriptorTest(unittest.TestCase): ...@@ -511,10 +512,11 @@ class VideoDescriptorTest(unittest.TestCase):
def setUp(self): def setUp(self):
system = get_test_descriptor_system() system = get_test_descriptor_system()
location = Location('i4x://org/course/video/name') course_key = SlashSeparatedCourseKey('org', 'course', 'run')
usage_key = course_key.make_usage_key('video', 'name')
self.descriptor = system.construct_xblock_from_class( self.descriptor = system.construct_xblock_from_class(
VideoDescriptor, VideoDescriptor,
scope_ids=ScopeIds(None, None, location, location), scope_ids=ScopeIds(None, None, usage_key, usage_key),
field_data=DictFieldData({}), field_data=DictFieldData({}),
) )
self.descriptor.runtime.handler_url = MagicMock() self.descriptor.runtime.handler_url = MagicMock()
......
...@@ -11,7 +11,7 @@ from textwrap import dedent ...@@ -11,7 +11,7 @@ from textwrap import dedent
from xmodule.error_module import ErrorDescriptor from xmodule.error_module import ErrorDescriptor
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.modulestore import Location from xmodule.modulestore.locations import SlashSeparatedCourseKey
from xmodule.modulestore.xml_importer import import_from_xml from xmodule.modulestore.xml_importer import import_from_xml
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
...@@ -48,6 +48,7 @@ class PageLoaderTestCase(LoginEnrollmentTestCase): ...@@ -48,6 +48,7 @@ class PageLoaderTestCase(LoginEnrollmentTestCase):
Base class that adds a function to load all pages in a modulestore. Base class that adds a function to load all pages in a modulestore.
""" """
# TODO once everything is merged can someone please check whether this function takes a course_id or course_key
def check_all_pages_load(self, course_id): def check_all_pages_load(self, course_id):
""" """
Assert that all pages in the course load correctly. Assert that all pages in the course load correctly.
...@@ -61,18 +62,7 @@ class PageLoaderTestCase(LoginEnrollmentTestCase): ...@@ -61,18 +62,7 @@ class PageLoaderTestCase(LoginEnrollmentTestCase):
self.enroll(course, True) self.enroll(course, True)
# Search for items in the course # Search for items in the course
# None is treated as a wildcard items = store.get_items(course_id)
course_loc = course.location
location_query = Location(
course_loc.tag, course_loc.org,
course_loc.course, None, None, None
)
items = store.get_items(
location_query,
course_id=course_id,
depth=2
)
if len(items) < 1: if len(items) < 1:
self.fail('Could not retrieve any items from course') self.fail('Could not retrieve any items from course')
...@@ -82,22 +72,22 @@ class PageLoaderTestCase(LoginEnrollmentTestCase): ...@@ -82,22 +72,22 @@ class PageLoaderTestCase(LoginEnrollmentTestCase):
if descriptor.location.category == 'about': if descriptor.location.category == 'about':
self._assert_loads('about_course', self._assert_loads('about_course',
{'course_id': course_id}, {'course_id': course_id.to_deprecated_string()},
descriptor) descriptor)
elif descriptor.location.category == 'static_tab': elif descriptor.location.category == 'static_tab':
kwargs = {'course_id': course_id, kwargs = {'course_id': course_id.to_deprecated_string(),
'tab_slug': descriptor.location.name} 'tab_slug': descriptor.location.name}
self._assert_loads('static_tab', kwargs, descriptor) self._assert_loads('static_tab', kwargs, descriptor)
elif descriptor.location.category == 'course_info': elif descriptor.location.category == 'course_info':
self._assert_loads('info', {'course_id': course_id}, self._assert_loads('info', {'course_id': course_id.to_deprecated_string()},
descriptor) descriptor)
else: else:
kwargs = {'course_id': course_id, kwargs = {'course_id': course_id.to_deprecated_string(),
'location': descriptor.location.url()} 'location': descriptor.location.to_deprecated_string()}
self._assert_loads('jump_to', kwargs, descriptor, self._assert_loads('jump_to', kwargs, descriptor,
expect_redirect=True, expect_redirect=True,
...@@ -118,7 +108,7 @@ class PageLoaderTestCase(LoginEnrollmentTestCase): ...@@ -118,7 +108,7 @@ class PageLoaderTestCase(LoginEnrollmentTestCase):
if response.status_code != 200: if response.status_code != 200:
self.fail('Status %d for page %s' % self.fail('Status %d for page %s' %
(response.status_code, descriptor.location.url())) (response.status_code, descriptor.location))
if expect_redirect: if expect_redirect:
self.assertEqual(response.redirect_chain[0][1], 302) self.assertEqual(response.redirect_chain[0][1], 302)
...@@ -142,7 +132,7 @@ class TestXmlCoursesLoad(ModuleStoreTestCase, PageLoaderTestCase): ...@@ -142,7 +132,7 @@ class TestXmlCoursesLoad(ModuleStoreTestCase, PageLoaderTestCase):
# Load one of the XML based courses # Load one of the XML based courses
# Our test mapping rules allow the MixedModuleStore # Our test mapping rules allow the MixedModuleStore
# to load this course from XML, not Mongo. # to load this course from XML, not Mongo.
self.check_all_pages_load('edX/toy/2012_Fall') self.check_all_pages_load(SlashSeparatedCourseKey('edX', 'toy', '2012_Fall'))
# Importing XML courses isn't possible with MixedModuleStore, # Importing XML courses isn't possible with MixedModuleStore,
...@@ -169,7 +159,7 @@ class TestMongoCoursesLoad(ModuleStoreTestCase, PageLoaderTestCase): ...@@ -169,7 +159,7 @@ class TestMongoCoursesLoad(ModuleStoreTestCase, PageLoaderTestCase):
</table_of_contents> </table_of_contents>
""").strip() """).strip()
location = Location(['i4x', 'edX', 'toy', 'course', '2012_Fall', None]) location = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall').make_usage_key('course', '2012_Fall')
course = self.store.get_item(location) course = self.store.get_item(location)
self.assertGreater(len(course.textbooks), 0) self.assertGreater(len(course.textbooks), 0)
...@@ -180,8 +170,7 @@ class TestDraftModuleStore(ModuleStoreTestCase): ...@@ -180,8 +170,7 @@ class TestDraftModuleStore(ModuleStoreTestCase):
store = modulestore() store = modulestore()
# fix was to allow get_items() to take the course_id parameter # fix was to allow get_items() to take the course_id parameter
store.get_items(Location(None, None, 'vertical', None, None), store.get_items(SlashSeparatedCourseKey('abc', 'def', 'ghi'), category='vertical')
course_id='abc', depth=0)
# test success is just getting through the above statement. # test success is just getting through the above statement.
# The bug was that 'course_id' argument was # The bug was that 'course_id' argument was
......
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