Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Change video transcripts to use locators instead of locations.

Browse Source 
Part of STUD-870
This commit is contained in:
cahrens
2013-11-26 15:02:25 -05:00
parent f94565d699
commit 5fdfc60957
16 changed files with 130 additions and 138 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
cms/djangoapps/contentstore/tests/test_contentstore.py
View File

@@ -137,8 +137,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
locator = loc_mapper().translate_location(course.location.course_id, descriptor.location, False, True)
resp = self.client.get_html(locator.url_reverse('unit'))
self.assertEqual(resp.status_code, 200)
# TODO: uncomment when video transcripts no longer require IDs.
# _test_no_locations(self, resp)
_test_no_locations(self, resp)
for expected in expected_types:
self.assertIn(expected, resp.content)
@@ -1354,8 +1353,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
unit_locator = loc_mapper().translate_location(course_id, descriptor.location, False, True)
resp = self.client.get_html(unit_locator.url_reverse('unit'))
self.assertEqual(resp.status_code, 200)
# TODO: uncomment when video transcripts no longer require IDs.
# _test_no_locations(self, resp)
_test_no_locations(self, resp)
@override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE, MODULESTORE=TEST_MODULESTORE)
@@ -1682,8 +1680,7 @@ class ContentStoreTest(ModuleStoreTestCase):
unit_locator = loc_mapper().translate_location(loc.course_id, unit_location, False, True)
resp = self.client.get_html(unit_locator.url_reverse('unit'))
self.assertEqual(resp.status_code, 200)
# TODO: uncomment when video transcripts no longer require IDs.
# _test_no_locations(self, resp)
_test_no_locations(self, resp)
def delete_item(category, name):
""" Helper method for testing the deletion of an xblock item. """

80
cms/djangoapps/contentstore/tests/test_transcripts.py
View File

@@ -60,7 +60,7 @@ class Basetranscripts(CourseTestCase):
'type': 'video'
}
resp = self.client.ajax_post('/xblock', data)
self.item_location = self._get_location(resp)
self.item_locator, self.item_location = self._get_locator(resp)
self.assertEqual(resp.status_code, 200)
# hI10vDNYz4M - valid Youtube ID with transcripts.
@@ -73,10 +73,10 @@ class Basetranscripts(CourseTestCase):
# Remove all transcripts for current module.
self.clear_subs_content()
def _get_location(self, resp):
""" Returns the location (as a string) from the response returned by a create operation. """
def _get_locator(self, resp):
""" 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')
return loc_mapper().translate_locator_to_location(BlockUsageLocator(locator)).url()
return locator, loc_mapper().translate_locator_to_location(BlockUsageLocator(locator)).url()
def get_youtube_ids(self):
"""Return youtube speeds and ids."""
@@ -142,7 +142,7 @@ class TestUploadtranscripts(Basetranscripts):
link = reverse('upload_transcripts')
filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0]
resp = self.client.post(link, {
'id': self.item_location,
'locator': self.item_locator,
'file': self.good_srt_file,
'video_list': json.dumps([{
'type': 'html5',
@@ -164,20 +164,20 @@ class TestUploadtranscripts(Basetranscripts):
link = reverse('upload_transcripts')
resp = self.client.post(link, {'file': self.good_srt_file})
self.assertEqual(resp.status_code, 400)
self.assertEqual(json.loads(resp.content).get('status'), 'POST data without "id" form data.')
self.assertEqual(json.loads(resp.content).get('status'), 'POST data without "locator" form data.')
def test_fail_data_without_file(self):
link = reverse('upload_transcripts')
resp = self.client.post(link, {'id': self.item_location})
resp = self.client.post(link, {'locator': self.item_locator})
self.assertEqual(resp.status_code, 400)
self.assertEqual(json.loads(resp.content).get('status'), 'POST data without "file" form data.')
def test_fail_data_with_bad_location(self):
def test_fail_data_with_bad_locator(self):
# Test for raising `InvalidLocationError` exception.
link = reverse('upload_transcripts')
filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0]
resp = self.client.post(link, {
'id': 'BAD_LOCATION',
'locator': 'BAD_LOCATOR',
'file': self.good_srt_file,
'video_list': json.dumps([{
'type': 'html5',
@@ -186,13 +186,13 @@ class TestUploadtranscripts(Basetranscripts):
}])
})
self.assertEqual(resp.status_code, 400)
self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by location.")
self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.")
# Test for raising `ItemNotFoundError` exception.
link = reverse('upload_transcripts')
filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0]
resp = self.client.post(link, {
'id': '{0}_{1}'.format(self.item_location, 'BAD_LOCATION'),
'locator': '{0}_{1}'.format(self.item_locator, 'BAD_LOCATOR'),
'file': self.good_srt_file,
'video_list': json.dumps([{
'type': 'html5',
@@ -201,7 +201,7 @@ class TestUploadtranscripts(Basetranscripts):
}])
})
self.assertEqual(resp.status_code, 400)
self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by location.")
self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.")
def test_fail_for_non_video_module(self):
# non_video module: setup
@@ -211,7 +211,7 @@ class TestUploadtranscripts(Basetranscripts):
'type': 'non_video'
}
resp = self.client.ajax_post('/xblock', data)
item_location = self._get_location(resp)
item_locator, item_location = self._get_locator(resp)
data = '<non_video youtube="0.75:JMD_ifUUfsU,1.0:hI10vDNYz4M" />'
modulestore().update_item(item_location, data)
@@ -220,7 +220,7 @@ class TestUploadtranscripts(Basetranscripts):
link = reverse('upload_transcripts')
filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0]
resp = self.client.post(link, {
'id': item_location,
'locator': item_locator,
'file': self.good_srt_file,
'video_list': json.dumps([{
'type': 'html5',
@@ -238,7 +238,7 @@ class TestUploadtranscripts(Basetranscripts):
link = reverse('upload_transcripts')
filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0]
resp = self.client.post(link, {
'id': self.item_location,
'locator': self.item_locator,
'file': self.good_srt_file,
'video_list': json.dumps([{
'type': 'html5',
@@ -255,7 +255,7 @@ class TestUploadtranscripts(Basetranscripts):
link = reverse('upload_transcripts')
filename = os.path.splitext(os.path.basename(self.bad_data_srt_file.name))[0]
resp = self.client.post(link, {
'id': self.item_location,
'locator': self.item_locator,
'file': self.bad_data_srt_file,
'video_list': json.dumps([{
'type': 'html5',
@@ -270,7 +270,7 @@ class TestUploadtranscripts(Basetranscripts):
link = reverse('upload_transcripts')
filename = os.path.splitext(os.path.basename(self.bad_name_srt_file.name))[0]
resp = self.client.post(link, {
'id': self.item_location,
'locator': self.item_locator,
'file': self.bad_name_srt_file,
'video_list': json.dumps([{
'type': 'html5',
@@ -297,7 +297,7 @@ class TestUploadtranscripts(Basetranscripts):
link = reverse('upload_transcripts')
filename = os.path.splitext(os.path.basename(srt_file.name))[0]
resp = self.client.post(link, {
'id': self.item_location,
'locator': self.item_locator,
'file': srt_file,
'video_list': json.dumps([{
'type': 'html5',
@@ -359,7 +359,7 @@ class TestDownloadtranscripts(Basetranscripts):
self.save_subs_to_store(subs, 'JMD_ifUUfsU')
link = reverse('download_transcripts')
resp = self.client.get(link, {'id': self.item_location, 'subs_id': "JMD_ifUUfsU"})
resp = self.client.get(link, {'locator': self.item_locator, 'subs_id': "JMD_ifUUfsU"})
self.assertEqual(resp.status_code, 200)
self.assertEqual(resp.content, """0\n00:00:00,100 --> 00:00:00,200\nsubs #1\n\n1\n00:00:00,200 --> 00:00:00,240\nsubs #2\n\n2\n00:00:00,240 --> 00:00:00,380\nsubs #3\n\n""")
@@ -386,7 +386,7 @@ class TestDownloadtranscripts(Basetranscripts):
self.save_subs_to_store(subs, subs_id)
link = reverse('download_transcripts')
resp = self.client.get(link, {'id': self.item_location, 'subs_id': subs_id})
resp = self.client.get(link, {'locator': self.item_locator, 'subs_id': subs_id})
self.assertEqual(resp.status_code, 200)
self.assertEqual(
resp.content,
@@ -397,21 +397,21 @@ class TestDownloadtranscripts(Basetranscripts):
def test_fail_data_without_file(self):
link = reverse('download_transcripts')
resp = self.client.get(link, {'id': ''})
resp = self.client.get(link, {'locator': ''})
self.assertEqual(resp.status_code, 404)
resp = self.client.get(link, {})
self.assertEqual(resp.status_code, 404)
def test_fail_data_with_bad_location(self):
def test_fail_data_with_bad_locator(self):
# Test for raising `InvalidLocationError` exception.
link = reverse('download_transcripts')
resp = self.client.get(link, {'id': 'BAD_LOCATION'})
resp = self.client.get(link, {'locator': 'BAD_LOCATOR'})
self.assertEqual(resp.status_code, 404)
# Test for raising `ItemNotFoundError` exception.
link = reverse('download_transcripts')
resp = self.client.get(link, {'id': '{0}_{1}'.format(self.item_location, 'BAD_LOCATION')})
resp = self.client.get(link, {'locator': '{0}_{1}'.format(self.item_locator, 'BAD_LOCATOR')})
self.assertEqual(resp.status_code, 404)
def test_fail_for_non_video_module(self):
@@ -422,7 +422,7 @@ class TestDownloadtranscripts(Basetranscripts):
'type': 'videoalpha'
}
resp = self.client.ajax_post('/xblock', data)
item_location = self._get_location(resp)
item_locator, item_location = self._get_locator(resp)
subs_id = str(uuid4())
data = textwrap.dedent("""
<videoalpha youtube="" sub="{}">
@@ -445,7 +445,7 @@ class TestDownloadtranscripts(Basetranscripts):
self.save_subs_to_store(subs, subs_id)
link = reverse('download_transcripts')
resp = self.client.get(link, {'id': item_location})
resp = self.client.get(link, {'locator': item_locator})
self.assertEqual(resp.status_code, 404)
def test_fail_nonyoutube_subs_dont_exist(self):
@@ -459,7 +459,7 @@ class TestDownloadtranscripts(Basetranscripts):
modulestore().update_item(self.item_location, data)
link = reverse('download_transcripts')
resp = self.client.get(link, {'id': self.item_location})
resp = self.client.get(link, {'locator': self.item_locator})
self.assertEqual(resp.status_code, 404)
def test_empty_youtube_attr_and_sub_attr(self):
@@ -473,7 +473,7 @@ class TestDownloadtranscripts(Basetranscripts):
modulestore().update_item(self.item_location, data)
link = reverse('download_transcripts')
resp = self.client.get(link, {'id': self.item_location})
resp = self.client.get(link, {'locator': self.item_locator})
self.assertEqual(resp.status_code, 404)
@@ -498,7 +498,7 @@ class TestDownloadtranscripts(Basetranscripts):
self.save_subs_to_store(subs, 'JMD_ifUUfsU')
link = reverse('download_transcripts')
resp = self.client.get(link, {'id': self.item_location})
resp = self.client.get(link, {'locator': self.item_locator})
self.assertEqual(resp.status_code, 404)
@@ -553,7 +553,7 @@ class TestChecktranscripts(Basetranscripts):
self.save_subs_to_store(subs, subs_id)
data = {
'id': self.item_location,
'locator': self.item_locator,
'videos': [{
'type': 'html5',
'video': subs_id,
@@ -597,7 +597,7 @@ class TestChecktranscripts(Basetranscripts):
self.save_subs_to_store(subs, 'JMD_ifUUfsU')
link = reverse('check_transcripts')
data = {
'id': self.item_location,
'locator': self.item_locator,
'videos': [{
'type': 'youtube',
'video': 'JMD_ifUUfsU',
@@ -625,7 +625,7 @@ class TestChecktranscripts(Basetranscripts):
def test_fail_data_without_id(self):
link = reverse('check_transcripts')
data = {
'id': '',
'locator': '',
'videos': [{
'type': '',
'video': '',
@@ -634,13 +634,13 @@ class TestChecktranscripts(Basetranscripts):
}
resp = self.client.get(link, {'data': json.dumps(data)})
self.assertEqual(resp.status_code, 400)
self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by location.")
self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.")
def test_fail_data_with_bad_location(self):
def test_fail_data_with_bad_locator(self):
# Test for raising `InvalidLocationError` exception.
link = reverse('check_transcripts')
data = {
'id': '',
'locator': '',
'videos': [{
'type': '',
'video': '',
@@ -649,11 +649,11 @@ class TestChecktranscripts(Basetranscripts):
}
resp = self.client.get(link, {'data': json.dumps(data)})
self.assertEqual(resp.status_code, 400)
self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by location.")
self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.")
# Test for raising `ItemNotFoundError` exception.
data = {
'id': '{0}_{1}'.format(self.item_location, 'BAD_LOCATION'),
'locator': '{0}_{1}'.format(self.item_locator, 'BAD_LOCATOR'),
'videos': [{
'type': '',
'video': '',
@@ -662,7 +662,7 @@ class TestChecktranscripts(Basetranscripts):
}
resp = self.client.get(link, {'data': json.dumps(data)})
self.assertEqual(resp.status_code, 400)
self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by location.")
self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.")
def test_fail_for_non_video_module(self):
# Not video module: setup
@@ -672,7 +672,7 @@ class TestChecktranscripts(Basetranscripts):
'type': 'not_video'
}
resp = self.client.ajax_post('/xblock', data)
item_location = self._get_location(resp)
item_locator, item_location = self._get_locator(resp)
subs_id = str(uuid4())
data = textwrap.dedent("""
<not_video youtube="" sub="{}">
@@ -695,7 +695,7 @@ class TestChecktranscripts(Basetranscripts):
self.save_subs_to_store(subs, subs_id)
data = {
'id': item_location,
'locator': item_locator,
'videos': [{
'type': '',
'video': '',

10
cms/djangoapps/contentstore/views/component.py
View File

@@ -223,13 +223,9 @@ def unit_handler(request, tag=None, course_id=None, branch=None, version_guid=No
)
components = [
[
# TODO: old location needed for video transcripts.
component.location.url(),
loc_mapper().translate_location(
course.location.course_id, component.location, False, True
)
]
loc_mapper().translate_location(
course.location.course_id, component.location, False, True
)
for component
in item.get_children()
]

97
cms/djangoapps/contentstore/views/transcripts_ajax.py
View File

@@ -18,11 +18,12 @@ from django.conf import settings
from xmodule.contentstore.content import StaticContent
from xmodule.exceptions import NotFoundError
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.django import modulestore, loc_mapper
from xmodule.contentstore.django import contentstore
from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError
from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError
from util.json_request import JsonResponse
from xmodule.modulestore.locator import BlockUsageLocator
from ..transcripts_utils import (
generate_subs_from_source,
@@ -77,20 +78,14 @@ def upload_transcripts(request):
'subs': '',
}
item_location = request.POST.get('id')
if not item_location:
return error_response(response, 'POST data without "id" form data.')
locator = request.POST.get('locator')
if not locator:
return error_response(response, 'POST data without "locator" form data.')
# This is placed before has_access() to validate item_location,
# because has_access() raises InvalidLocationError if location is invalid.
try:
item = modulestore().get_item(item_location)
except (ItemNotFoundError, InvalidLocationError):
return error_response(response, "Can't find item by location.")
# Check permissions for this user within this course.
if not has_access(request.user, item_location):
raise PermissionDenied()
item = _get_item(request, request.POST)
except (ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError):
return error_response(response, "Can't find item by locator.")
if 'file' not in request.FILES:
return error_response(response, 'POST data without "file" form data.')
@@ -156,23 +151,17 @@ def download_transcripts(request):
Raises Http404 if unsuccessful.
"""
item_location = request.GET.get('id')
if not item_location:
log.debug('GET data without "id" property.')
locator = request.GET.get('locator')
if not locator:
log.debug('GET data without "locator" property.')
raise Http404
# This is placed before has_access() to validate item_location,
# because has_access() raises InvalidLocationError if location is invalid.
try:
item = modulestore().get_item(item_location)
except (ItemNotFoundError, InvalidLocationError):
log.debug("Can't find item by location.")
item = _get_item(request, request.GET)
except (ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError):
log.debug("Can't find item by locator.")
raise Http404
# Check permissions for this user within this course.
if not has_access(request.user, item_location):
raise PermissionDenied()
subs_id = request.GET.get('subs_id')
if not subs_id:
log.debug('GET data without "subs_id" property.')
@@ -240,7 +229,7 @@ def check_transcripts(request):
'status': 'Error',
}
try:
__, videos, item = validate_transcripts_data(request)
__, videos, item = _validate_transcripts_data(request)
except TranscriptsRequestValidationException as e:
return error_response(transcripts_presence, e.message)
@@ -303,7 +292,7 @@ def check_transcripts(request):
if len(html5_subs) == 2: # check html5 transcripts for equality
transcripts_presence['html5_equal'] = json.loads(html5_subs[0]) == json.loads(html5_subs[1])
command, subs_to_use = transcripts_logic(transcripts_presence, videos)
command, subs_to_use = _transcripts_logic(transcripts_presence, videos)
transcripts_presence.update({
'command': command,
'subs': subs_to_use,
@@ -311,7 +300,7 @@ def check_transcripts(request):
return JsonResponse(transcripts_presence)
def transcripts_logic(transcripts_presence, videos):
def _transcripts_logic(transcripts_presence, videos):
"""
By `transcripts_presence` content, figure what show to user:
@@ -386,7 +375,7 @@ def choose_transcripts(request):
}
try:
data, videos, item = validate_transcripts_data(request)
data, videos, item = _validate_transcripts_data(request)
except TranscriptsRequestValidationException as e:
return error_response(response, e.message)
@@ -416,7 +405,7 @@ def replace_transcripts(request):
response = {'status': 'Error', 'subs': ''}
try:
__, videos, item = validate_transcripts_data(request)
__, videos, item = _validate_transcripts_data(request)
except TranscriptsRequestValidationException as e:
return error_response(response, e.message)
@@ -435,7 +424,7 @@ def replace_transcripts(request):
return JsonResponse(response)
def validate_transcripts_data(request):
def _validate_transcripts_data(request):
"""
Validates, that request contains all proper data for transcripts processing.
@@ -452,18 +441,10 @@ def validate_transcripts_data(request):
if not data:
raise TranscriptsRequestValidationException('Incoming video data is empty.')
item_location = data.get('id')
# This is placed before has_access() to validate item_location,
# because has_access() raises InvalidLocationError if location is invalid.
try:
item = modulestore().get_item(item_location)
except (ItemNotFoundError, InvalidLocationError):
raise TranscriptsRequestValidationException("Can't find item by location.")
# Check permissions for this user within this course.
if not has_access(request.user, item_location):
raise PermissionDenied()
item = _get_item(request, data)
except (ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError):
raise TranscriptsRequestValidationException("Can't find item by locator.")
if item.category != 'video':
raise TranscriptsRequestValidationException('Transcripts are supported only for "video" modules.')
@@ -492,7 +473,7 @@ def rename_transcripts(request):
response = {'status': 'Error', 'subs': ''}
try:
__, videos, item = validate_transcripts_data(request)
__, videos, item = _validate_transcripts_data(request)
except TranscriptsRequestValidationException as e:
return error_response(response, e.message)
@@ -525,11 +506,10 @@ def save_transcripts(request):
if not data:
return error_response(response, 'Incoming video data is empty.')
item_location = data.get('id')
try:
item = modulestore().get_item(item_location)
except (ItemNotFoundError, InvalidLocationError):
return error_response(response, "Can't find item by location.")
item = _get_item(request, data)
except (ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError):
return error_response(response, "Can't find item by locator.")
metadata = data.get('metadata')
if metadata is not None:
@@ -553,3 +533,24 @@ def save_transcripts(request):
response['status'] = 'Success'
return JsonResponse(response)
def _get_item(request, data):
"""
Obtains from 'data' the locator for an item.
Next, gets that item from the modulestore (allowing any errors to raise up).
Finally, verifies that the user has access to the item.
Returns the item.
"""
locator = BlockUsageLocator(data.get('locator'))
old_location = loc_mapper().translate_locator_to_location(locator)
# This is placed before has_access() to validate the location,
# because has_access() raises InvalidLocationError if location is invalid.
item = modulestore().get_item(old_location)
if not has_access(request.user, locator):
raise PermissionDenied()
return item