merge(#29101): [SE-4631] add CCX ID to generated filename prefixes (#…

…27028)

This is the backport of https://github.com/edx/edx-platform/pull/27028.

---
This pull requests backport a feature to include CCX course IDs in the generated course filenames used for downloading reports, etc.

The suffix appended to the generated filename prefix is following the  `<separator>_ccx_<ccx id>` pattern, to ensure unique filename generation per ccx - previously all CCX courses were downloaded with the same name.

**Dependencies**: None

**Screenshots**: 
![Screenshot 2021-03-16 at 16 22 12](https://user-images.githubusercontent.com/19173947/111349994-f1c42980-8681-11eb-8fff-d28024899957.png)

**Sandbox URL**: TBD

**Merge deadline**: None

**Testing instructions**:

1. Navigate to https://pr29101.sandbox.opencraft.hosting
2. Login as staff
3. Navigate to https://pr29101.sandbox.opencraft.hosting/courses/ccx-v1:edX+DemoX+Demo_Course+ccx@1/instructor#view-data_download
4. Click "Download profile information as a CSV" button
5. Check that the file name contains CCX

common/djangoapps/util/file.py

@@ -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 _
@@ -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):

common/djangoapps/util/tests/test_file.py

@@ -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,
@@ -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):

lms/envs/common.py

@@ -635,6 +635,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"]