Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SE-4175] Add CCX ID to generated filename prefixes #27028

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions common/djangoapps/util/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
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 _
Expand Down Expand Up @@ -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):
Expand Down
54 changes: 53 additions & 1 deletion common/djangoapps/util/tests/test_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@
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,
Expand All @@ -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):
Expand Down
11 changes: 11 additions & 0 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,17 @@
# .. 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"]

Expand Down