Commit c6c5bc9e by stv

Fix Opaque Keys issues

parent e490e71d
""" Unit tests for utility methods in views.py. """ """ Unit tests for utility methods in views.py. """
from django.conf import settings from django.conf import settings
from contentstore.utils import get_modulestore from contentstore.utils import get_modulestore
from contentstore.utils import reverse_course_url
from contentstore.views.utility import expand_utility_action_url from contentstore.views.utility import expand_utility_action_url
from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.factories import CourseFactory
from xmodule.modulestore.django import loc_mapper
import json import json
import mock import mock
...@@ -16,8 +16,7 @@ class UtilitiesTestCase(CourseTestCase): ...@@ -16,8 +16,7 @@ class UtilitiesTestCase(CourseTestCase):
""" Creates the test course. """ """ Creates the test course. """
super(UtilitiesTestCase, self).setUp() super(UtilitiesTestCase, self).setUp()
self.course = CourseFactory.create(org='mitX', number='333', display_name='Utilities Course') self.course = CourseFactory.create(org='mitX', number='333', display_name='Utilities Course')
self.location = loc_mapper().translate_location(self.course.location.course_id, self.course.location, False, True) self.utilities_url = reverse_course_url('utility_handler', self.course.id)
self.utilities_url = self.location.url_reverse('utilities/', '')
def get_persisted_utilities(self): def get_persisted_utilities(self):
""" Returns the utilities. """ """ Returns the utilities. """
...@@ -41,16 +40,15 @@ class UtilitiesTestCase(CourseTestCase): ...@@ -41,16 +40,15 @@ class UtilitiesTestCase(CourseTestCase):
response = self.client.get(self.utilities_url) response = self.client.get(self.utilities_url)
self.assertContains(response, "Bulk Operations") self.assertContains(response, "Bulk Operations")
# Verify expansion of action URL happened. # Verify expansion of action URL happened.
self.assertContains(response, 'captions/mitX.333.Utilities_Course') self.assertContains(response, '/utility/captions/slashes:mitX+333+Utilities_Course')
# Verify persisted utility does NOT have expanded URL. # Verify persisted utility does NOT have expanded URL.
utility_0 = self.get_persisted_utilities()[0] utility_0 = self.get_persisted_utilities()[0]
self.assertEqual('utility/captions', get_action_url(utility_0, 0)) self.assertEqual('utility_captions_handler', get_action_url(utility_0, 0))
payload = response.content payload = response.content
# Now delete the utilities from the course and verify they get repopulated (for courses # Now delete the utilities from the course and verify they get repopulated (for courses
# created before utilities were introduced). # created before utilities were introduced).
with mock.patch('django.conf.settings.COURSE_UTILITIES', []): with mock.patch('django.conf.settings.COURSE_UTILITIES', []):
print settings.COURSE_UTILITIES
modulestore = get_modulestore(self.course.location) modulestore = get_modulestore(self.course.location)
modulestore.update_item(self.course, self.user.id) modulestore.update_item(self.course, self.user.id)
self.assertEqual(self.get_persisted_utilities(), []) self.assertEqual(self.get_persisted_utilities(), [])
...@@ -68,25 +66,25 @@ class UtilitiesTestCase(CourseTestCase): ...@@ -68,25 +66,25 @@ class UtilitiesTestCase(CourseTestCase):
# Verify that persisted utilities do not have expanded action URLs. # Verify that persisted utilities do not have expanded action URLs.
# compare_utilities will verify that returned_utilities DO have expanded action URLs. # compare_utilities will verify that returned_utilities DO have expanded action URLs.
pers = self.get_persisted_utilities() pers = self.get_persisted_utilities()
self.assertEqual('utility/captions', get_first_item(pers[0]).get('action_url')) self.assertEqual('utility_captions_handler', get_first_item(pers[0]).get('action_url'))
for pay, resp in zip(pers, returned_utilities): for pay, resp in zip(pers, returned_utilities):
self.compare_utilities(pay, resp) self.compare_utilities(pay, resp)
def test_utilities_post_unsupported(self): def test_utilities_post_unsupported(self):
""" Post operation is not supported. """ """ Post operation is not supported. """
update_url = self.location.url_reverse('utilities/', '100') update_url = reverse_course_url('utility_handler', self.course.id)
response = self.client.post(update_url) response = self.client.post(update_url)
self.assertEqual(response.status_code, 404) self.assertEqual(response.status_code, 404)
def test_utilities_put_unsupported(self): def test_utilities_put_unsupported(self):
""" Put operation is not supported. """ """ Put operation is not supported. """
update_url = self.location.url_reverse('utilities/', '100') update_url = reverse_course_url('utility_handler', self.course.id)
response = self.client.put(update_url) response = self.client.put(update_url)
self.assertEqual(response.status_code, 404) self.assertEqual(response.status_code, 404)
def test_utilities_delete_unsupported(self): def test_utilities_delete_unsupported(self):
""" Delete operation is not supported. """ """ Delete operation is not supported. """
update_url = self.location.url_reverse('utilities/', '100') update_url = reverse_course_url('utility_handler', self.course.id)
response = self.client.delete(update_url) response = self.client.delete(update_url)
self.assertEqual(response.status_code, 404) self.assertEqual(response.status_code, 404)
...@@ -107,7 +105,7 @@ class UtilitiesTestCase(CourseTestCase): ...@@ -107,7 +105,7 @@ class UtilitiesTestCase(CourseTestCase):
# Verify no side effect in the original list. # Verify no side effect in the original list.
self.assertEqual(get_action_url(utility, index), stored) self.assertEqual(get_action_url(utility, index), stored)
test_expansion(settings.COURSE_UTILITIES[0], 0, 'utility/captions', '/utility/captions/mitX.333.Utilities_Course/branch/draft/block/Utilities_Course') test_expansion(settings.COURSE_UTILITIES[0], 0, 'utility_captions_handler', '/utility/captions/slashes:mitX+333+Utilities_Course')
def get_first_item(utility): def get_first_item(utility):
......
...@@ -12,6 +12,7 @@ from edxmako.shortcuts import render_to_response ...@@ -12,6 +12,7 @@ from edxmako.shortcuts import render_to_response
from django.http import HttpResponseNotFound from django.http import HttpResponseNotFound
from django.core.exceptions import PermissionDenied from django.core.exceptions import PermissionDenied
from xmodule.modulestore.keys import CourseKey from xmodule.modulestore.keys import CourseKey
from xmodule.modulestore.django import loc_mapper
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from contentstore.utils import get_modulestore, reverse_course_url from contentstore.utils import get_modulestore, reverse_course_url
...@@ -113,12 +114,12 @@ def expand_checklist_action_url(course_module, checklist): ...@@ -113,12 +114,12 @@ def expand_checklist_action_url(course_module, checklist):
expanded_checklist = copy.deepcopy(checklist) expanded_checklist = copy.deepcopy(checklist)
urlconf_map = { urlconf_map = {
"ManageUsers": "course_team", "ManageUsers": "course_team_handler",
"CourseOutline": "course", "CourseOutline": "course_handler",
"StaticPages": "tabs", "StaticPages": "tabs_handler",
"SettingsDetails": "settings/details", "SettingsDetails": "settings_handler",
"SettingsGrading": "settings/grading", "SettingsGrading": "grading_handler",
"SettingsAdvanced": "settings/advanced", "SettingsAdvanced": "advanced_settings_handler",
} }
lms_urlconf_map = { lms_urlconf_map = {
"Wiki": "course_wiki", "Wiki": "course_wiki",
...@@ -128,16 +129,15 @@ def expand_checklist_action_url(course_module, checklist): ...@@ -128,16 +129,15 @@ def expand_checklist_action_url(course_module, checklist):
for item in expanded_checklist.get('items'): for item in expanded_checklist.get('items'):
action_url = item.get('action_url') action_url = item.get('action_url')
if action_url in urlconf_map: if action_url in urlconf_map:
url_prefix = urlconf_map[action_url] item['action_url'] = reverse_course_url(urlconf_map[action_url], course_module.id)
ctx_loc = course_module.location
location = loc_mapper().translate_location(ctx_loc.course_id, ctx_loc, False, True)
item['action_url'] = location.url_reverse(url_prefix, '')
elif action_url in lms_urlconf_map: elif action_url in lms_urlconf_map:
lms_base = settings.LMS_BASE item['action_url'] = '/'.join([
course_id = course_module.location.course_id 'http:/',
url_postfix = lms_urlconf_map[action_url] settings.LMS_BASE,
url = 'http://' + lms_base + '/courses/' + course_id + '/' + url_postfix 'courses',
item['action_url'] = url course_module.id.to_deprecated_string(),
lms_urlconf_map[action_url],
])
return expanded_checklist return expanded_checklist
......
...@@ -11,12 +11,15 @@ from django.conf import settings ...@@ -11,12 +11,15 @@ from django.conf import settings
from xmodule.video_module import transcripts_utils from xmodule.video_module import transcripts_utils
from contentstore.tests.utils import CourseTestCase from contentstore.tests.utils import CourseTestCase
from contentstore.utils import reverse_course_url
from contentstore.views.utilities.captions import get_videos
from cache_toolbox.core import del_cached_content from cache_toolbox.core import del_cached_content
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.contentstore.django import contentstore, _CONTENTSTORE from xmodule.contentstore.django import contentstore, _CONTENTSTORE
from xmodule.contentstore.content import StaticContent from xmodule.contentstore.content import StaticContent
from xmodule.exceptions import NotFoundError from xmodule.exceptions import NotFoundError
from xmodule.modulestore.django import loc_mapper from xmodule.modulestore.django import loc_mapper
from xmodule.modulestore.keys import UsageKey
from xmodule.modulestore.locator import BlockUsageLocator from xmodule.modulestore.locator import BlockUsageLocator
from contentstore.tests.modulestore_config import TEST_MODULESTORE from contentstore.tests.modulestore_config import TEST_MODULESTORE
...@@ -35,8 +38,7 @@ class Basetranscripts(CourseTestCase): ...@@ -35,8 +38,7 @@ class Basetranscripts(CourseTestCase):
"""Remove, if transcripts content exists.""" """Remove, if transcripts content exists."""
for youtube_id in self.get_youtube_ids().values(): for youtube_id in self.get_youtube_ids().values():
filename = 'subs_{0}.srt.sjson'.format(youtube_id) filename = 'subs_{0}.srt.sjson'.format(youtube_id)
content_location = StaticContent.compute_location( content_location = StaticContent.compute_location(self.course.id, filename)
self.org, self.number, filename)
try: try:
content = contentstore().find(content_location) content = contentstore().find(content_location)
contentstore().delete(content.get_id()) contentstore().delete(content.get_id())
...@@ -46,20 +48,22 @@ class Basetranscripts(CourseTestCase): ...@@ -46,20 +48,22 @@ class Basetranscripts(CourseTestCase):
def setUp(self): def setUp(self):
"""Create initial data.""" """Create initial data."""
super(Basetranscripts, self).setUp() super(Basetranscripts, self).setUp()
self.location = loc_mapper().translate_location( self.location = self.course.id.make_usage_key('course', self.course.id.run)
self.course.location.course_id, self.course.location, False, True self.captions_url = reverse_course_url('utility_captions_handler', self.course.id)
)
self.captions_url = self.location.url_reverse('utility/captions/', '')
self.unicode_locator = unicode(self.location) self.unicode_locator = unicode(self.location)
# Add video module # Add video module
data = { data = {
# 'parent_locator': self.location.to_deprecated_string(),
'parent_locator': self.unicode_locator, 'parent_locator': self.unicode_locator,
'category': 'video', 'category': 'video',
'type': 'video' 'type': 'video'
} }
resp = self.client.ajax_post('/xblock', data) resp = self.client.ajax_post('http://testserver/xblock/', data)
self.item_locator, self.item_location = self._get_locator(resp) videos = get_videos(self.course)
self.item_location = self._get_locator(resp)
self.item_location_string = str(self.item_location)
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
self.item = modulestore().get_item(self.item_location) self.item = modulestore().get_item(self.item_location)
...@@ -74,8 +78,10 @@ class Basetranscripts(CourseTestCase): ...@@ -74,8 +78,10 @@ class Basetranscripts(CourseTestCase):
def _get_locator(self, resp): def _get_locator(self, resp):
""" Returns the locator and old-style location (as a string) from the response returned by a create operation. """ """ Returns the locator and old-style location (as a string) from the response returned by a create operation. """
locator = json.loads(resp.content).get('locator') locator_string = json.loads(resp.content).get('locator')
return locator, loc_mapper().translate_locator_to_location(BlockUsageLocator(locator)).url() locator = UsageKey.from_string(locator_string)
# locator = self.course.id.make_usage_key('course', locator_string)
return locator
def get_youtube_ids(self): def get_youtube_ids(self):
"""Return youtube speeds and ids.""" """Return youtube speeds and ids."""
...@@ -102,8 +108,7 @@ class TestCheckcaptions(Basetranscripts): ...@@ -102,8 +108,7 @@ class TestCheckcaptions(Basetranscripts):
mime_type = 'application/json' mime_type = 'application/json'
filename = 'subs_{0}.srt.sjson'.format(subs_id) filename = 'subs_{0}.srt.sjson'.format(subs_id)
content_location = StaticContent.compute_location( content_location = StaticContent.compute_location(self.course.id, filename)
self.org, self.number, filename)
content = StaticContent(content_location, filename, mime_type, filedata) content = StaticContent(content_location, filename, mime_type, filedata)
contentstore().save(content) contentstore().save(content)
del_cached_content(content_location) del_cached_content(content_location)
...@@ -133,7 +138,7 @@ class TestCheckcaptions(Basetranscripts): ...@@ -133,7 +138,7 @@ class TestCheckcaptions(Basetranscripts):
link = self.captions_url link = self.captions_url
data = { data = {
'video': json.dumps({'location': self.item_location}), 'video': json.dumps({'location': self.item_location_string}),
} }
resp = self.client.get(link, data, HTTP_ACCEPT='application/json') resp = self.client.get(link, data, HTTP_ACCEPT='application/json')
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
...@@ -141,7 +146,7 @@ class TestCheckcaptions(Basetranscripts): ...@@ -141,7 +146,7 @@ class TestCheckcaptions(Basetranscripts):
json.loads(resp.content), json.loads(resp.content),
{ {
u'status': True, u'status': True,
u'location': self.item_location, u'location': self.item_location_string,
u'command': u'use_existing', u'command': u'use_existing',
u'current_item_subs': subs_id, u'current_item_subs': subs_id,
u'html5_equal': False, u'html5_equal': False,
...@@ -173,7 +178,7 @@ class TestCheckcaptions(Basetranscripts): ...@@ -173,7 +178,7 @@ class TestCheckcaptions(Basetranscripts):
self.save_subs_to_store(subs, 'JMD_ifUUfsU') self.save_subs_to_store(subs, 'JMD_ifUUfsU')
link = self.captions_url link = self.captions_url
data = { data = {
'video': json.dumps({'location': self.item_location}), 'video': json.dumps({'location': self.item_location_string}),
} }
resp = self.client.get(link, data, HTTP_ACCEPT='application/json') resp = self.client.get(link, data, HTTP_ACCEPT='application/json')
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
...@@ -190,7 +195,7 @@ class TestCheckcaptions(Basetranscripts): ...@@ -190,7 +195,7 @@ class TestCheckcaptions(Basetranscripts):
u'youtube_diff': True, u'youtube_diff': True,
u'youtube_local': True, u'youtube_local': True,
u'youtube_server': False, u'youtube_server': False,
u'location': self.item_location u'location': self.item_location_string,
} }
) )
...@@ -216,7 +221,7 @@ class TestCheckcaptions(Basetranscripts): ...@@ -216,7 +221,7 @@ class TestCheckcaptions(Basetranscripts):
# Test for raising `ItemNotFoundError` exception. # Test for raising `ItemNotFoundError` exception.
data = { data = {
'video': json.dumps({'location': '{0}_{1}'.format(self.item_location, 'BAD_LOCATION')}), 'video': json.dumps({'location': '{0}_{1}'.format(self.item_location_string, 'BAD_LOCATION')}),
} }
resp = self.client.get(link, data) resp = self.client.get(link, data)
self.assertEqual(resp.status_code, 400) self.assertEqual(resp.status_code, 400)
...@@ -229,8 +234,9 @@ class TestCheckcaptions(Basetranscripts): ...@@ -229,8 +234,9 @@ class TestCheckcaptions(Basetranscripts):
'category': 'not_video', 'category': 'not_video',
'type': 'not_video' 'type': 'not_video'
} }
resp = self.client.ajax_post('/xblock', data) resp = self.client.ajax_post('http://testserver/xblock/', data)
item_locator, item_location = self._get_locator(resp) item_location = self._get_locator(resp)
item_location_string = str(item_location)
subs_id = str(uuid4()) subs_id = str(uuid4())
item = modulestore().get_item(item_location) item = modulestore().get_item(item_location)
item.data = textwrap.dedent(""" item.data = textwrap.dedent("""
...@@ -255,7 +261,7 @@ class TestCheckcaptions(Basetranscripts): ...@@ -255,7 +261,7 @@ class TestCheckcaptions(Basetranscripts):
link = self.captions_url link = self.captions_url
data = { data = {
'video': json.dumps({'location': item_location}), 'video': json.dumps({'location': item_location_string}),
} }
resp = self.client.get(link, data) resp = self.client.get(link, data)
self.assertEqual(resp.status_code, 400) self.assertEqual(resp.status_code, 400)
......
...@@ -139,6 +139,7 @@ class ChecklistTestCase(CourseTestCase): ...@@ -139,6 +139,7 @@ class ChecklistTestCase(CourseTestCase):
test_expansion(self.course.checklists[0], 0, 'ManageUsers', '/course_team/slashes:mitX+333+Checklists_Course/') test_expansion(self.course.checklists[0], 0, 'ManageUsers', '/course_team/slashes:mitX+333+Checklists_Course/')
test_expansion(self.course.checklists[1], 1, 'CourseOutline', '/course/slashes:mitX+333+Checklists_Course') test_expansion(self.course.checklists[1], 1, 'CourseOutline', '/course/slashes:mitX+333+Checklists_Course')
raise Exception('FUNK CHECKLIST {}'.format(self.course.checklists[2]))
test_expansion(self.course.checklists[2], 0, 'http://help.edge.edx.org/', 'http://help.edge.edx.org/') test_expansion(self.course.checklists[2], 0, 'http://help.edge.edx.org/', 'http://help.edge.edx.org/')
......
...@@ -15,6 +15,8 @@ from models.settings.course_grading import CourseGradingModel ...@@ -15,6 +15,8 @@ from models.settings.course_grading import CourseGradingModel
from util.json_request import JsonResponse from util.json_request import JsonResponse
from xmodule.modulestore.django import modulestore, loc_mapper from xmodule.modulestore.django import modulestore, loc_mapper
from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError
from xmodule.modulestore.keys import CourseKey
from xmodule.modulestore.keys import UsageKey
from xmodule.modulestore.locator import BlockUsageLocator from xmodule.modulestore.locator import BlockUsageLocator
from xmodule.video_module.transcripts_utils import ( from xmodule.video_module.transcripts_utils import (
GetTranscriptsFromYouTubeException, GetTranscriptsFromYouTubeException,
...@@ -22,7 +24,7 @@ from xmodule.video_module.transcripts_utils import ( ...@@ -22,7 +24,7 @@ from xmodule.video_module.transcripts_utils import (
download_youtube_subs) download_youtube_subs)
from ..access import has_course_access from ..access import has_course_access
from ..transcripts_ajax import get_transcripts_presence from ..transcripts_ajax import get_transcripts_presence
from ..course import _get_locator_and_course from ..course import _get_course_module
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
...@@ -32,7 +34,7 @@ __all__ = ['utility_captions_handler'] ...@@ -32,7 +34,7 @@ __all__ = ['utility_captions_handler']
# pylint: disable=unused-argument # pylint: disable=unused-argument
@login_required @login_required
def utility_captions_handler(request, tag=None, package_id=None, branch=None, version_guid=None, block=None): def utility_captions_handler(request, course_key_string):
""" """
The restful handler for captions requests in the utilities area. The restful handler for captions requests in the utilities area.
It provides the list of course videos as well as their status. It also lets It provides the list of course videos as well as their status. It also lets
...@@ -44,24 +46,25 @@ def utility_captions_handler(request, tag=None, package_id=None, branch=None, ve ...@@ -44,24 +46,25 @@ def utility_captions_handler(request, tag=None, package_id=None, branch=None, ve
POST POST
json: update the captions of a given video by copying the version of the captions hosted in youtube. json: update the captions of a given video by copying the version of the captions hosted in youtube.
""" """
course_key = CourseKey.from_string(course_key_string)
response_format = request.REQUEST.get('format', 'html') response_format = request.REQUEST.get('format', 'html')
if response_format == 'json' or 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'): if response_format == 'json' or 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'):
if request.method == 'POST': # update if request.method == 'POST': # update
try: try:
locations = _validate_captions_data_update(request) locations = _validate_captions_data_update(request, course_key)
except TranscriptsRequestValidationException as e: except TranscriptsRequestValidationException as e:
return error_response(e.message) return error_response(e.message)
return json_update_videos(request, locations) return json_update_videos(request, locations)
elif request.method == 'GET': # get status elif request.method == 'GET': # get status
try: try:
data, item = _validate_captions_data_get(request) data, item = _validate_captions_data_get(request, course_key)
except TranscriptsRequestValidationException as e: except TranscriptsRequestValidationException as e:
return error_response(e.message) return error_response(e.message)
return json_get_video_status(data, item) return json_get_video_status(data, item)
else: else:
return HttpResponseBadRequest() return HttpResponseBadRequest()
elif request.method == 'GET': # assume html elif request.method == 'GET': # assume html
return captions_index(request, package_id, branch, version_guid, block) return captions_index(request, course_key)
else: else:
return HttpResponseNotFound() return HttpResponseNotFound()
...@@ -77,7 +80,8 @@ def json_update_videos(request, locations): ...@@ -77,7 +80,8 @@ def json_update_videos(request, locations):
locations: list of locations of videos to be updated locations: list of locations of videos to be updated
""" """
results = [] results = []
for key in locations: for key_string in locations:
key = UsageKey.from_string(key_string)
try: try:
#update transcripts #update transcripts
item = modulestore().get_item(key) item = modulestore().get_item(key)
...@@ -91,26 +95,28 @@ def json_update_videos(request, locations): ...@@ -91,26 +95,28 @@ def json_update_videos(request, locations):
html5[name] = 'html5' html5[name] = 'html5'
videos['html5'] = html5 videos['html5'] = html5
captions_dict = get_transcripts_presence(videos, item) captions_dict = get_transcripts_presence(videos, item)
captions_dict.update({'location': key}) captions_dict.update({'location': key_string})
results.append(captions_dict) results.append(captions_dict)
except GetTranscriptsFromYouTubeException as e: except GetTranscriptsFromYouTubeException as e:
log.debug(e) log.debug(e)
results.append({'location': key, 'command': e}) results.append({'location': key_string, 'command': e})
return JsonResponse(results) return JsonResponse(results)
@login_required @login_required
@ensure_csrf_cookie @ensure_csrf_cookie
def captions_index(request, package_id, branch, version_guid, block): def captions_index(request, course_key):
""" """
Display a list of course videos as well as their status (up to date, or out of date) Display a list of course videos as well as their status (up to date, or out of date)
org, course, name: Attributes of the Location for the item to edit org, course, name: Attributes of the Location for the item to edit
""" """
locator, course = _get_locator_and_course( course = _get_course_module(
package_id, branch, version_guid, block, request.user, depth=3 course_key,
request.user,
depth=2,
) )
return render_to_response('captions.html', return render_to_response('captions.html',
...@@ -134,7 +140,7 @@ def error_response(message, response=None, status_code=400): ...@@ -134,7 +140,7 @@ def error_response(message, response=None, status_code=400):
return JsonResponse(response, status_code) return JsonResponse(response, status_code)
def _validate_captions_data_get(request): def _validate_captions_data_get(request, course_key):
""" """
Happens on 'GET'. Validates, that request contains all proper data for transcripts processing. Happens on 'GET'. Validates, that request contains all proper data for transcripts processing.
...@@ -154,11 +160,11 @@ def _validate_captions_data_get(request): ...@@ -154,11 +160,11 @@ def _validate_captions_data_get(request):
raise TranscriptsRequestValidationException(_('Incoming video data is empty.')) raise TranscriptsRequestValidationException(_('Incoming video data is empty.'))
location = data.get('location') location = data.get('location')
item = _validate_location(location) item = _validate_location(location, course_key)
return data, item return data, item
def _validate_captions_data_update(request): def _validate_captions_data_update(request, course_key):
""" """
Happens on 'POST'. Validates, that request contains all proper data for transcripts processing. Happens on 'POST'. Validates, that request contains all proper data for transcripts processing.
...@@ -176,11 +182,12 @@ def _validate_captions_data_update(request): ...@@ -176,11 +182,12 @@ def _validate_captions_data_update(request):
raise TranscriptsRequestValidationException(_('Incoming update_array data is empty.')) raise TranscriptsRequestValidationException(_('Incoming update_array data is empty.'))
for location in data: for location in data:
_validate_location(location) _validate_location(location, course_key)
return data return data
def _validate_location(location): def _validate_location(location, course_key):
location = UsageKey.from_string(location)
try: try:
item = modulestore().get_item(location) item = modulestore().get_item(location)
except (ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError): except (ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError):
......
...@@ -8,11 +8,11 @@ from django.views.decorators.http import require_http_methods ...@@ -8,11 +8,11 @@ from django.views.decorators.http import require_http_methods
from django_future.csrf import ensure_csrf_cookie from django_future.csrf import ensure_csrf_cookie
from edxmako.shortcuts import render_to_response from edxmako.shortcuts import render_to_response
from util.json_request import JsonResponse from util.json_request import JsonResponse
from xmodule.course_module import CourseDescriptor
from xmodule.modulestore.django import loc_mapper
from xmodule.modulestore.locator import BlockUsageLocator
from ..utils import get_modulestore from contentstore.utils import reverse_course_url
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.keys import CourseKey
from .access import has_course_access from .access import has_course_access
__all__ = ['utility_handler'] __all__ = ['utility_handler']
...@@ -22,7 +22,7 @@ __all__ = ['utility_handler'] ...@@ -22,7 +22,7 @@ __all__ = ['utility_handler']
@require_http_methods(("GET")) @require_http_methods(("GET"))
@login_required @login_required
@ensure_csrf_cookie @ensure_csrf_cookie
def utility_handler(request, tag=None, package_id=None, branch=None, version_guid=None, block=None): def utility_handler(request, course_key_string):
""" """
The restful handler for utilities. The restful handler for utilities.
...@@ -30,22 +30,17 @@ def utility_handler(request, tag=None, package_id=None, branch=None, version_gui ...@@ -30,22 +30,17 @@ def utility_handler(request, tag=None, package_id=None, branch=None, version_gui
html: return html page for all utilities html: return html page for all utilities
json: return json representing all utilities. json: return json representing all utilities.
""" """
location = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block) course_key = CourseKey.from_string(course_key_string)
if not has_course_access(request.user, location): if not has_course_access(request.user, course_key):
raise PermissionDenied() raise PermissionDenied()
course_module = modulestore().get_course(course_key)
old_location = loc_mapper().translate_locator_to_location(location)
modulestore = get_modulestore(old_location)
course_module = modulestore.get_item(old_location)
json_request = 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json') json_request = 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json')
if request.method == 'GET': if request.method == 'GET':
expanded_utilities = expand_all_action_urls(course_module) expanded_utilities = expand_all_action_urls(course_module)
if json_request: if json_request:
return JsonResponse(expanded_utilities) return JsonResponse(expanded_utilities)
else: else:
handler_url = location.url_reverse('utilities/', '') handler_url = reverse_course_url('utility_handler', course_module.id)
return render_to_response('utilities.html', return render_to_response('utilities.html',
{ {
'handler_url': handler_url, 'handler_url': handler_url,
...@@ -80,8 +75,6 @@ def expand_utility_action_url(course_module, utility): ...@@ -80,8 +75,6 @@ def expand_utility_action_url(course_module, utility):
for item in expanded_utility.get('items'): for item in expanded_utility.get('items'):
url_prefix = item.get('action_url') url_prefix = item.get('action_url')
ctx_loc = course_module.location item['action_url'] = reverse_course_url(url_prefix, course_module.id)
location = loc_mapper().translate_location(ctx_loc.course_id, ctx_loc, False, True)
item['action_url'] = location.url_reverse(url_prefix, '')
return expanded_utility return expanded_utility
...@@ -476,7 +476,7 @@ COURSE_UTILITIES = [ ...@@ -476,7 +476,7 @@ COURSE_UTILITIES = [
"items": [ "items": [
{"short_description": "Get all captions from YouTube", {"short_description": "Get all captions from YouTube",
"long_description": "This utility will attempt to get or update captions for all videos in the course from YouTube. Please allow it a couple of minutes to run.", "long_description": "This utility will attempt to get or update captions for all videos in the course from YouTube. Please allow it a couple of minutes to run.",
"action_url": "utility/captions", "action_url": "utility_captions_handler",
"action_text": "Check Captions", "action_text": "Check Captions",
"action_external": False}]} "action_external": False}]}
] ]
......
...@@ -90,8 +90,8 @@ urlpatterns += patterns( ...@@ -90,8 +90,8 @@ urlpatterns += patterns(
url(r'^settings/advanced/(?P<course_key_string>[^/]+)$', 'advanced_settings_handler'), url(r'^settings/advanced/(?P<course_key_string>[^/]+)$', 'advanced_settings_handler'),
url(r'^textbooks/(?P<course_key_string>[^/]+)$', 'textbooks_list_handler'), url(r'^textbooks/(?P<course_key_string>[^/]+)$', 'textbooks_list_handler'),
url(r'^textbooks/(?P<course_key_string>[^/]+)/(?P<textbook_id>\d[^/]*)$', 'textbooks_detail_handler'), url(r'^textbooks/(?P<course_key_string>[^/]+)/(?P<textbook_id>\d[^/]*)$', 'textbooks_detail_handler'),
url(r'^utilities/<?P<course_key_string>[^/]+)$', 'utility_handler'), url(r'^utilities/(?P<course_key_string>[^/]+)$', 'utility_handler'),
url(r'^utility/captions/<?P<course_key_string>[^/]+$', 'utility_captions_handler'), url(r'^utility/captions/(?P<course_key_string>[^/]+)$', 'utility_captions_handler'),
) )
js_info_dict = { js_info_dict = {
......
...@@ -708,13 +708,15 @@ def _check_can_enroll_in_course(user, course_id, access_type="enroll"): ...@@ -708,13 +708,15 @@ def _check_can_enroll_in_course(user, course_id, access_type="enroll"):
Returns (bool, error_message), where error message is only applicable if bool == False Returns (bool, error_message), where error message is only applicable if bool == False
""" """
try: try:
course = course_from_id(course_id) course = modulestore().get_course(course_id)
except ItemNotFoundError: except ItemNotFoundError:
log.warning("User {0} tried to enroll in non-existent course {1}" log.warning(
.format(user.username, course_id)) "User {0} tried to enroll in non-existent course {1}"
.format(user.username, course_id),
)
return False, _("Course id is invalid") return False, _("Course id is invalid")
if not has_access(user, course, access_type): if not has_access(user, access_type, course):
return False, _("Enrollment is closed") return False, _("Enrollment is closed")
return True, "" return True, ""
......
...@@ -12,7 +12,6 @@ from django.conf import settings ...@@ -12,7 +12,6 @@ from django.conf import settings
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase 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 xmodule.modulestore import Location
from courseware.tests import BaseTestXmodule from courseware.tests import BaseTestXmodule
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
...@@ -148,29 +147,18 @@ class TestLTIModuleListing(ModuleStoreTestCase): ...@@ -148,29 +147,18 @@ class TestLTIModuleListing(ModuleStoreTestCase):
display_name="section2", display_name="section2",
category='sequential') category='sequential')
self.published_location_dict = {'tag': 'i4x',
'org': self.course.location.org,
'category': 'lti',
'course': self.course.location.course,
'name': 'lti_published'}
self.draft_location_dict = {'tag': 'i4x',
'org': self.course.location.org,
'category': 'lti',
'course': self.course.location.course,
'name': 'lti_draft',
'revision': 'draft'}
# creates one draft and one published lti module, in different sections # creates one draft and one published lti module, in different sections
self.lti_published = ItemFactory.create( self.lti_published = ItemFactory.create(
parent_location=self.section1.location, parent_location=self.section1.location,
display_name="lti published", display_name="lti published",
category="lti", category="lti",
location=Location(self.published_location_dict), location=self.course.id.make_usage_key('lti', 'lti_published'),
) )
self.lti_draft = ItemFactory.create( self.lti_draft = ItemFactory.create(
parent_location=self.section2.location, parent_location=self.section2.location,
display_name="lti draft", display_name="lti draft",
category="lti", category="lti",
location=Location(self.draft_location_dict), location=self.course.id.make_usage_key('lti', 'lti_published').replace(revision='draft'),
) )
def expected_handler_url(self, handler): def expected_handler_url(self, handler):
......
...@@ -111,17 +111,17 @@ class ModuleRenderTestCase(ModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -111,17 +111,17 @@ class ModuleRenderTestCase(ModuleStoreTestCase, LoginEnrollmentTestCase):
""" """
mock_request = MagicMock() mock_request = MagicMock()
mock_request.user = self.mock_user mock_request.user = self.mock_user
course = get_course_with_access(self.mock_user, self.course_id, 'load') course = get_course_with_access(self.mock_user, 'load', self.course_key)
field_data_cache = FieldDataCache.cache_for_descriptor_descendents( field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
self.course_id, self.mock_user, course, depth=2) self.toy_course.id, self.mock_user, course, depth=2)
module = render.get_module( module = render.get_module(
self.mock_user, self.mock_user,
mock_request, mock_request,
['i4x', 'edX', 'toy', 'html', 'toyjumpto'], self.location,
field_data_cache, field_data_cache,
self.course_id self.toy_course.id,
) )
self.assertTrue(module.xmodule_runtime.send_users_emailaddr_with_coderesponse) self.assertTrue(module.xmodule_runtime.send_users_emailaddr_with_coderesponse)
self.assertEqual(module.xmodule_runtime.deanonymized_user_email, self.mock_user.email) self.assertEqual(module.xmodule_runtime.deanonymized_user_email, self.mock_user.email)
...@@ -133,17 +133,17 @@ class ModuleRenderTestCase(ModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -133,17 +133,17 @@ class ModuleRenderTestCase(ModuleStoreTestCase, LoginEnrollmentTestCase):
""" """
mock_request = MagicMock() mock_request = MagicMock()
mock_request.user = self.mock_user mock_request.user = self.mock_user
course = get_course_with_access(self.mock_user, self.course_id, 'load') course = get_course_with_access(self.mock_user, 'load', self.course_key)
field_data_cache = FieldDataCache.cache_for_descriptor_descendents( field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
self.course_id, self.mock_user, course, depth=2) self.toy_course.id, self.mock_user, course, depth=2)
module = render.get_module( module = render.get_module(
self.mock_user, self.mock_user,
mock_request, mock_request,
['i4x', 'edX', 'toy', 'html', 'toyjumpto'], self.location,
field_data_cache, field_data_cache,
self.course_id self.toy_course.id,
) )
self.assertFalse(hasattr(module.xmodule_runtime, 'send_users_emailaddr_with_coderesponse')) self.assertFalse(hasattr(module.xmodule_runtime, 'send_users_emailaddr_with_coderesponse'))
self.assertFalse(hasattr(module.xmodule_runtime, 'deanonymized_user_email')) self.assertFalse(hasattr(module.xmodule_runtime, 'deanonymized_user_email'))
......
...@@ -596,13 +596,14 @@ def course_about(request, course_id): ...@@ -596,13 +596,14 @@ def course_about(request, course_id):
# 1) within enrollment period # 1) within enrollment period
# 2) course specifies it's okay # 2) course specifies it's okay
# 3) request.user is not a registered user. # 3) request.user is not a registered user.
sneakpeek_allowed = (has_access(request.user, course, 'within_enrollment_period') and sneakpeek_allowed = (has_access(request.user, 'within_enrollment_period', course) and
CoursePreference.course_allows_nonregistered_access(course_id) and CoursePreference.course_allows_nonregistered_access(course.id.to_deprecated_string()) and
not UserProfile.has_registered(request.user)) not UserProfile.has_registered(request.user))
# see if we have already filled up all allowed enrollments # see if we have already filled up all allowed enrollments
is_course_full = CourseEnrollment.is_course_full(course) is_course_full = CourseEnrollment.is_course_full(course)
# python -m coverage run --rcfile=lms/.coveragerc hich ./manage.pyu
return render_to_response('courseware/course_about.html', { return render_to_response('courseware/course_about.html', {
'course': course, 'course': course,
'regularly_registered': regularly_registered, 'regularly_registered': regularly_registered,
......
...@@ -59,7 +59,7 @@ ...@@ -59,7 +59,7 @@
$(".course_sneakpeek").click(function(event) { $(".course_sneakpeek").click(function(event) {
$.ajax({ $.ajax({
url: "${reverse('course_sneakpeek', args=[course.id])}", url: "${reverse('course_sneakpeek', args=[course.id.to_deprecated_string()])}",
type: "POST", type: "POST",
complete: sneakpeek_handler complete: sneakpeek_handler
}); });
......
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