feat: add CCX ID to generated filename prefixes (#27028)

This commit is contained in:
Gábor Boros
2021-05-25 16:25:07 +02:00
committed by GitHub
parent 1bc3430ba9
commit baca47782b
3 changed files with 84 additions and 6 deletions

View File

@@ -6,6 +6,7 @@ Utility methods related to file handling.
import os
from datetime import datetime
from django.conf import settings
from django.core.exceptions import PermissionDenied
from django.core.files.storage import DefaultStorage, get_valid_filename
from django.utils.translation import ugettext as _
@@ -89,16 +90,30 @@ def store_uploaded_file(
def course_filename_prefix_generator(course_id, separator='_'):
"""
Generates a course-identifying unicode string for use in a file
name.
Generates a course-identifying unicode string for use in a file name.
Args:
course_id (object): A course identification object.
separator (str): The character or chain of characters used for separating course details in
the filename.
Returns:
str: A unicode string which can safely be inserted into a
filename.
str: A unicode string which can safely be inserted into a filename.
"""
return get_valid_filename(str(separator).join([course_id.org, course_id.course, course_id.run]))
filename = str(separator).join([
course_id.org,
course_id.course,
course_id.run
])
enable_course_filename_ccx_suffix = settings.FEATURES.get(
'ENABLE_COURSE_FILENAME_CCX_SUFFIX',
False
)
if enable_course_filename_ccx_suffix and getattr(course_id, 'ccx', None):
filename = separator.join([filename, 'ccx', course_id.ccx])
return get_valid_filename(filename)
def course_and_time_based_filename_generator(course_id, base_name):

View File

@@ -14,10 +14,13 @@ from django.core import exceptions
from django.core.files.uploadedfile import SimpleUploadedFile
from django.http import HttpRequest
from django.test import TestCase
from django.test.utils import override_settings
from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locations import CourseLocator
from pytz import UTC
from ccx_keys.locator import CCXLocator
import common.djangoapps.util.file
from common.djangoapps.util.file import (
FileValidationException,
@@ -33,14 +36,63 @@ class FilenamePrefixGeneratorTestCase(TestCase):
"""
Tests for course_filename_prefix_generator
"""
@ddt.data(CourseLocator(org='foo', course='bar', run='baz'), CourseKey.from_string('foo/bar/baz'))
@ddt.data(
CourseLocator(org='foo', course='bar', run='baz'),
CourseKey.from_string('foo/bar/baz'),
CCXLocator.from_course_locator(CourseLocator(org='foo', course='bar', run='baz'), '1'),
)
def test_locators(self, course_key):
"""
Test filename prefix genaration from multiple course key formats.
Test that the filename prefix is generated from a CCX course locator or a course key. If the
filename is generated for a CCX course but the related 'ENABLE_COURSE_FILENAME_CCX_SUFFIX'
feature is not turned on, the generated filename shouldn't contain the CCX course ID.
"""
assert course_filename_prefix_generator(course_key) == 'foo_bar_baz'
@ddt.data(
[CourseLocator(org='foo', course='bar', run='baz'), 'foo_bar_baz'],
[CourseKey.from_string('foo/bar/baz'), 'foo_bar_baz'],
[CCXLocator.from_course_locator(CourseLocator(org='foo', course='bar', run='baz'), '1'), 'foo_bar_baz_ccx_1'],
)
@ddt.unpack
@override_settings(FEATURES={'ENABLE_COURSE_FILENAME_CCX_SUFFIX': True})
def test_include_ccx_id(self, course_key, expected_filename):
"""
Test filename prefix genaration from multiple course key formats.
Test that the filename prefix is generated from a CCX course locator or a course key. If the
filename is generated for a CCX course but the related 'ENABLE_COURSE_FILENAME_CCX_SUFFIX'
feature is not turned on, the generated filename shouldn't contain the CCX course ID.
"""
assert course_filename_prefix_generator(course_key) == expected_filename
@ddt.data(CourseLocator(org='foo', course='bar', run='baz'), CourseKey.from_string('foo/bar/baz'))
def test_custom_separator(self, course_key):
"""
Test filename prefix is generated with a custom separator.
The filename should be build up from the course locator separated by a custom separator.
"""
assert course_filename_prefix_generator(course_key, separator='-') == 'foo-bar-baz'
@ddt.data(
[CourseLocator(org='foo', course='bar', run='baz'), 'foo-bar-baz'],
[CourseKey.from_string('foo/bar/baz'), 'foo-bar-baz'],
[CCXLocator.from_course_locator(CourseLocator(org='foo', course='bar', run='baz'), '1'), 'foo-bar-baz-ccx-1'],
)
@ddt.unpack
@override_settings(FEATURES={'ENABLE_COURSE_FILENAME_CCX_SUFFIX': True})
def test_custom_separator_including_ccx_id(self, course_key, expected_filename):
"""
Test filename prefix is generated with a custom separator.
The filename should be build up from the course locator separated by a custom separator
including the CCX ID if the related 'ENABLE_COURSE_FILENAME_CCX_SUFFIX' is turned on.
"""
assert course_filename_prefix_generator(course_key, separator='-') == expected_filename
@ddt.ddt
class FilenameGeneratorTestCase(TestCase):

View File

@@ -646,6 +646,17 @@ FEATURES = {
# .. toggle_tickets: https://github.com/edx/edx-platform/pull/7845
'ENABLE_COURSE_DISCOVERY': False,
# .. toggle_name: FEATURES['ENABLE_COURSE_FILENAME_CCX_SUFFIX']
# .. toggle_implementation: DjangoSetting
# .. toggle_default: False
# .. toggle_description: If set to True, CCX ID will be included in the generated filename for CCX courses.
# .. toggle_use_cases: open_edx
# .. toggle_creation_date: 2021-03-16
# .. toggle_target_removal_date: None
# .. toggle_tickets: None
# .. toggle_warnings: Turning this feature ON will affect all generated filenames which are related to CCX courses.
'ENABLE_COURSE_FILENAME_CCX_SUFFIX': False,
# Setting for overriding default filtering facets for Course discovery
# COURSE_DISCOVERY_FILTERS = ["org", "language", "modes"]