feat: read extra student profile CSV fields from course other settings

nau_openedx_extensions/apps.py

@@ -65,3 +65,18 @@ def ready(self):
             get_course_email_context_factory  # pylint: disable=import-outside-toplevel # noqa
         bulk_email_tasks._get_course_email_context = \
             get_course_email_context_factory(prev_email_context_func)  # pylint: disable=protected-access
+
+        # Monkey-patch get_additional_student_profile_attributes in instructor_analytics basic module
+        # to append NAU per-course additional student profile fields defined in the course
+        # advanced setting nau_additional_features_on_instructor_analytics_student_profile_info,
+        # filtered against the Django allowlist
+        # NAU_ALL_ADDITIONAL_FEATURES_ON_INSTRUCTOR_ANALYTICS_STUDENT_PROFILE_INFO.
+        from lms.djangoapps.instructor_analytics import \
+            basic as instructor_analytics_basic  # pylint: disable=import-error,import-outside-toplevel # noqa
+
+        from nau_openedx_extensions.utils.instructor_analytics import \
+            get_additional_student_profile_attributes_factory  # pylint: disable=import-outside-toplevel # noqa
+
+        prev_method = instructor_analytics_basic.get_additional_student_profile_attributes
+        instructor_analytics_basic.get_additional_student_profile_attributes = \
+            get_additional_student_profile_attributes_factory(prev_method)

nau_openedx_extensions/settings/common.py

@@ -90,3 +90,17 @@ def plugin_settings(settings):
     settings.NAU_INSTRUCTOR_TASK_MODULE = "nau_openedx_extensions.edxapp_wrapper.backends.instructor_task_r_v1"
     settings.NAU_SITE_CONFIGURATION_MODULE = "nau_openedx_extensions.edxapp_wrapper.backends.site_configuration_r_v1"
     settings.NAU_CONTENT_MODULE = "nau_openedx_extensions.edxapp_wrapper.backends.content_r_v1"
+
+    # Global allowlist of all User attribute names that may ever be appended to the
+    # Student Profile Info CSV report via the per-course advanced setting
+    # ``nau_additional_features_on_instructor_analytics_student_profile_info``.
+    # Only fields present here will be accepted; any other field requested at the
+    # course level will be skipped with a warning.
+    #
+    # Example::
+    #
+    #   NAU_ALL_ADDITIONAL_FEATURES_ON_INSTRUCTOR_ANALYTICS_STUDENT_PROFILE_INFO = [
+    #       "nau_nif",
+    #       "nau_user_extended_model_cc_nic",
+    #   ]
+    settings.NAU_ALL_ADDITIONAL_FEATURES_ON_INSTRUCTOR_ANALYTICS_STUDENT_PROFILE_INFO = []

nau_openedx_extensions/tests/test_instructor_analytics.py

@@ -0,0 +1,249 @@
+# -*- coding: utf-8 -*-
+"""
+Test cases for instructor analytics utilities.
+
+This module tests the monkey-patching of ``get_additional_student_profile_attributes``
+which appends additional student profile fields when the course advanced setting
+``nau_additional_features_on_instructor_analytics_student_profile_info`` is configured,
+subject to the global Django allowlist
+``NAU_ALL_ADDITIONAL_FEATURES_ON_INSTRUCTOR_ANALYTICS_STUDENT_PROFILE_INFO``.
+"""
+from unittest.mock import Mock, patch
+
+from django.test import TestCase, override_settings
+
+from nau_openedx_extensions.utils.instructor_analytics import (
+    _COURSE_SETTING_KEY,
+    _DJANGO_ALLOWLIST_SETTING,
+    get_additional_student_profile_attributes_factory,
+)
+
+_BASE_ATTRIBUTES = ["year_of_birth", "gender"]
+_ALL_EXTRA_FEATURES = ["nau_nif", "nau_user_extended_model_cc_nic"]
+
+
+def _make_course_key():
+    """Return a simple mock that behaves like a CourseKey."""
+    return Mock()
+
+
+def _make_original(attributes=None):
+    """Return a mock original function that always returns *attributes*."""
+    if attributes is None:
+        attributes = _BASE_ATTRIBUTES
+    return Mock(return_value=attributes)
+
+
+def _make_course_settings(extra_features=None):
+    """
+    Return a mocked return value for get_other_course_settings.
+
+    Args:
+        extra_features: list of extra feature names, or None for no setting present.
+    """
+    value = {}
+    if extra_features is not None:
+        value[_COURSE_SETTING_KEY] = extra_features
+    return {"value": value}
+
+
+class TestGetAdditionalStudentProfileAttributesFactory(TestCase):
+    """
+    Unit tests for get_additional_student_profile_attributes_factory.
+
+    Two layers of configuration are tested:
+
+    1. The Django allowlist (``NAU_ALL_ADDITIONAL_FEATURES_ON_INSTRUCTOR_ANALYTICS_STUDENT_PROFILE_INFO``)
+       defines every field name that is ever permitted site-wide.
+    2. The per-course advanced setting (``nau_additional_features_on_instructor_analytics_student_profile_info``)
+       selects which of those allowed fields are active for a given course.
+    """
+
+    # ------------------------------------------------------------------
+    # Original function delegation
+    # ------------------------------------------------------------------
+
+    @patch('nau_openedx_extensions.utils.instructor_analytics.get_other_course_settings')
+    @override_settings(**{_DJANGO_ALLOWLIST_SETTING: _ALL_EXTRA_FEATURES})
+    def test_original_function_always_called(self, mock_get_other_course_settings):
+        """The original get_additional_student_profile_attributes is always invoked regardless of settings."""
+        mock_get_other_course_settings.return_value = _make_course_settings(["nau_nif"])
+        original = _make_original()
+        wrapper = get_additional_student_profile_attributes_factory(original)
+        course_key = _make_course_key()
+
+        wrapper(course_key)
+
+        original.assert_called_once_with(course_key)
+
+    @patch('nau_openedx_extensions.utils.instructor_analytics.get_other_course_settings')
+    @override_settings(**{_DJANGO_ALLOWLIST_SETTING: _ALL_EXTRA_FEATURES})
+    def test_get_other_course_settings_called_with_course_key(self, mock_get_other_course_settings):
+        """get_other_course_settings is called with the same course_key passed to the wrapper."""
+        mock_get_other_course_settings.return_value = _make_course_settings()
+        original = _make_original()
+        wrapper = get_additional_student_profile_attributes_factory(original)
+        course_key = _make_course_key()
+
+        wrapper(course_key)
+
+        mock_get_other_course_settings.assert_called_once_with(course_key)
+
+    # ------------------------------------------------------------------
+    # No course-level setting present
+    # ------------------------------------------------------------------
+
+    @patch('nau_openedx_extensions.utils.instructor_analytics.get_other_course_settings')
+    @override_settings(**{_DJANGO_ALLOWLIST_SETTING: _ALL_EXTRA_FEATURES})
+    def test_no_course_setting_returns_original_attributes(self, mock_get_other_course_settings):
+        """When the course setting key is absent, the original attributes are returned unchanged."""
+        mock_get_other_course_settings.return_value = _make_course_settings()
+        original = _make_original()
+        wrapper = get_additional_student_profile_attributes_factory(original)
+
+        result = wrapper(_make_course_key())
+
+        self.assertEqual(result, _BASE_ATTRIBUTES)
+
+    @patch('nau_openedx_extensions.utils.instructor_analytics.get_other_course_settings')
+    @override_settings(**{_DJANGO_ALLOWLIST_SETTING: _ALL_EXTRA_FEATURES})
+    def test_empty_other_course_settings_returns_original_attributes(self, mock_get_other_course_settings):
+        """An empty dict from get_other_course_settings does not break the wrapper."""
+        mock_get_other_course_settings.return_value = {}
+
+        result = get_additional_student_profile_attributes_factory(_make_original())(_make_course_key())
+
+        self.assertEqual(result, _BASE_ATTRIBUTES)
+
+    @patch('nau_openedx_extensions.utils.instructor_analytics.get_other_course_settings')
+    @override_settings(**{_DJANGO_ALLOWLIST_SETTING: _ALL_EXTRA_FEATURES})
+    def test_empty_course_setting_list_returns_original_attributes(self, mock_get_other_course_settings):
+        """An empty list in the course setting returns original attributes unchanged."""
+        mock_get_other_course_settings.return_value = _make_course_settings([])
+
+        result = get_additional_student_profile_attributes_factory(_make_original())(_make_course_key())
+
+        self.assertEqual(result, _BASE_ATTRIBUTES)
+
+    # ------------------------------------------------------------------
+    # Happy path: fields appended correctly
+    # ------------------------------------------------------------------
+
+    @patch('nau_openedx_extensions.utils.instructor_analytics.get_other_course_settings')
+    @override_settings(**{_DJANGO_ALLOWLIST_SETTING: _ALL_EXTRA_FEATURES})
+    def test_allowlisted_course_fields_are_appended(self, mock_get_other_course_settings):
+        """All course-requested fields that are in the allowlist are appended in order."""
+        mock_get_other_course_settings.return_value = _make_course_settings(
+            ["nau_nif", "nau_user_extended_model_cc_nic"]
+        )
+
+        result = get_additional_student_profile_attributes_factory(_make_original())(_make_course_key())
+
+        self.assertEqual(result, _BASE_ATTRIBUTES + ["nau_nif", "nau_user_extended_model_cc_nic"])
+
+    @patch('nau_openedx_extensions.utils.instructor_analytics.get_other_course_settings')
+    @override_settings(**{_DJANGO_ALLOWLIST_SETTING: _ALL_EXTRA_FEATURES})
+    def test_field_order_is_preserved(self, mock_get_other_course_settings):
+        """Extra fields are appended in the order they appear in the course setting."""
+        mock_get_other_course_settings.return_value = _make_course_settings(
+            ["nau_user_extended_model_cc_nic", "nau_nif"]
+        )
+
+        result = get_additional_student_profile_attributes_factory(_make_original())(_make_course_key())
+
+        self.assertEqual(
+            result, _BASE_ATTRIBUTES + ["nau_user_extended_model_cc_nic", "nau_nif"]
+        )
+
+    # ------------------------------------------------------------------
+    # Duplicate field prevention
+    # ------------------------------------------------------------------
+
+    @patch('nau_openedx_extensions.utils.instructor_analytics.get_other_course_settings')
+    @override_settings(**{_DJANGO_ALLOWLIST_SETTING: ["gender", "nau_nif"]})
+    def test_field_already_in_base_not_duplicated(self, mock_get_other_course_settings):
+        """A field that is already in the original result is not appended again."""
+        mock_get_other_course_settings.return_value = _make_course_settings(
+            ["gender", "nau_nif"]  # "gender" already in _BASE_ATTRIBUTES
+        )
+
+        result = get_additional_student_profile_attributes_factory(_make_original())(_make_course_key())
+
+        self.assertEqual(result.count("gender"), 1)
+        self.assertIn("nau_nif", result)
+
+    # ------------------------------------------------------------------
+    # Allowlist filtering
+    # ------------------------------------------------------------------
+
+    @patch('nau_openedx_extensions.utils.instructor_analytics.get_other_course_settings')
+    @override_settings(**{_DJANGO_ALLOWLIST_SETTING: ["nau_nif"]})
+    def test_field_not_in_allowlist_is_skipped_with_warning(self, mock_get_other_course_settings):
+        """A course-requested field absent from the allowlist is skipped and a WARNING is emitted."""
+        mock_get_other_course_settings.return_value = _make_course_settings(
+            ["nau_nif", "nau_user_extended_model_cc_nic"]  # cc_nic not in allowlist
+        )
+
+        with self.assertLogs('nau_openedx_extensions.utils.instructor_analytics', level='WARNING') as log:
+            result = get_additional_student_profile_attributes_factory(_make_original())(_make_course_key())
+
+        self.assertIn("nau_nif", result)
+        self.assertNotIn("nau_user_extended_model_cc_nic", result)
+        self.assertTrue(any("nau_user_extended_model_cc_nic" in msg for msg in log.output))
+
+    @patch('nau_openedx_extensions.utils.instructor_analytics.get_other_course_settings')
+    @override_settings(**{_DJANGO_ALLOWLIST_SETTING: []})
+    def test_empty_allowlist_blocks_all_fields_with_warnings(self, mock_get_other_course_settings):
+        """An empty allowlist blocks every course-level field and warns for each one."""
+        mock_get_other_course_settings.return_value = _make_course_settings(
+            ["nau_nif", "nau_user_extended_model_cc_nic"]
+        )
+
+        with self.assertLogs('nau_openedx_extensions.utils.instructor_analytics', level='WARNING') as log:
+            result = get_additional_student_profile_attributes_factory(_make_original())(_make_course_key())
+
+        self.assertEqual(result, _BASE_ATTRIBUTES)
+        # One warning per blocked field
+        warning_messages = [m for m in log.output if 'WARNING' in m]
+        self.assertEqual(len(warning_messages), 2)
+
+    @patch('nau_openedx_extensions.utils.instructor_analytics.get_other_course_settings')
+    @override_settings(**{_DJANGO_ALLOWLIST_SETTING: _ALL_EXTRA_FEATURES})
+    def test_unknown_field_in_course_setting_skipped_with_warning(self, mock_get_other_course_settings):
+        """A field in the course setting that is not in the allowlist is skipped with a warning."""
+        mock_get_other_course_settings.return_value = _make_course_settings(
+            ["nau_nif", "unknown_field"]
+        )
+
+        with self.assertLogs('nau_openedx_extensions.utils.instructor_analytics', level='WARNING') as log:
+            result = get_additional_student_profile_attributes_factory(_make_original())(_make_course_key())
+
+        self.assertIn("nau_nif", result)
+        self.assertNotIn("unknown_field", result)
+        self.assertTrue(any("unknown_field" in msg for msg in log.output))
+
+    @patch('nau_openedx_extensions.utils.instructor_analytics.get_other_course_settings')
+    def test_absent_django_allowlist_setting_blocks_all_fields(self, mock_get_other_course_settings):
+        """When the Django allowlist setting is missing entirely, no extra fields are appended."""
+        mock_get_other_course_settings.return_value = _make_course_settings(["nau_nif"])
+
+        with self.assertLogs('nau_openedx_extensions.utils.instructor_analytics', level='WARNING'):
+            result = get_additional_student_profile_attributes_factory(_make_original())(_make_course_key())
+
+        self.assertEqual(result, _BASE_ATTRIBUTES)
+
+    # ------------------------------------------------------------------
+    # Malformed course setting value
+    # ------------------------------------------------------------------
+
+    @patch('nau_openedx_extensions.utils.instructor_analytics.get_other_course_settings')
+    @override_settings(**{_DJANGO_ALLOWLIST_SETTING: _ALL_EXTRA_FEATURES})
+    def test_non_list_course_setting_is_ignored_with_warning(self, mock_get_other_course_settings):
+        """A non-list value for the course setting is ignored and a WARNING is logged."""
+        mock_get_other_course_settings.return_value = _make_course_settings("bad-value")
+
+        with self.assertLogs('nau_openedx_extensions.utils.instructor_analytics', level='WARNING') as log:
+            result = get_additional_student_profile_attributes_factory(_make_original())(_make_course_key())
+
+        self.assertEqual(result, _BASE_ATTRIBUTES)
+        self.assertTrue(any("not a list" in msg for msg in log.output))

nau_openedx_extensions/utils/instructor_analytics.py

@@ -0,0 +1,110 @@
+# -*- coding: utf-8 -*-
+"""
+Utilities for instructor analytics customization in NAU openedX extensions.
+"""
+from __future__ import absolute_import, unicode_literals
+
+import logging
+
+from django.conf import settings
+
+from nau_openedx_extensions.edxapp_wrapper.course_module import get_other_course_settings
+
+logger = logging.getLogger(__name__)
+
+# Course advanced setting key that lists extra student profile fields to include in
+# the instructor Student Profile Info CSV report for a specific course.
+_COURSE_SETTING_KEY = "nau_additional_features_on_instructor_analytics_student_profile_info"
+
+# Django setting name for the global allowlist of all valid extra features.
+_DJANGO_ALLOWLIST_SETTING = "NAU_ALL_ADDITIONAL_FEATURES_ON_INSTRUCTOR_ANALYTICS_STUDENT_PROFILE_INFO"
+
+
+def get_additional_student_profile_attributes_factory(prev_get_additional_student_profile_attributes):
+    """
+    Factory function to wrap the get_additional_student_profile_attributes function.
+
+    This allows NAU to append additional student profile fields per-course from the
+    course advanced setting ``nau_additional_features_on_instructor_analytics_student_profile_info``
+    without exposing sensitive fields (e.g. VAT ID / NIF) by default.
+
+    Two layers of configuration control which fields are appended:
+
+    1. **Django allowlist** (``NAU_ALL_ADDITIONAL_FEATURES_ON_INSTRUCTOR_ANALYTICS_STUDENT_PROFILE_INFO``):
+       A site-level list of *all* field names that are ever permitted to appear in the
+       Student Profile Info CSV.  This guards against typos or misconfiguration at the
+       course level.  Example Django setting::
+
+           NAU_ALL_ADDITIONAL_FEATURES_ON_INSTRUCTOR_ANALYTICS_STUDENT_PROFILE_INFO = [
+               "nau_nif",
+               "nau_user_extended_model_cc_nic",
+           ]
+
+    2. **Course advanced setting** (``nau_additional_features_on_instructor_analytics_student_profile_info``):
+       A per-course list that selects which of the allowed fields to activate for that
+       specific course.  Only fields that also appear in the Django allowlist are included;
+       any field not in the allowlist triggers a warning and is skipped.  Example::
+
+           nau_additional_features_on_instructor_analytics_student_profile_info:
+               ["nau_nif"]
+
+    Behaviour:
+    - The original ``get_additional_student_profile_attributes(course_key)`` is called first so
+      that the existing ``additional_student_profile_attributes`` site-configuration
+      mechanism continues to work.
+    - Course-level fields are filtered against the Django allowlist before being appended.
+    - Duplicate entries are removed while preserving order.
+    - A ``WARNING`` is logged for any course-level field not present in the allowlist.
+
+    Args:
+        prev_get_additional_student_profile_attributes: The original ``get_additional_student_profile_attributes``
+            function from ``lms.djangoapps.instructor_analytics.basic``.
+
+    Returns:
+        A wrapped version of the function that includes NAU per-course extra fields.
+    """
+
+    def get_additional_student_profile_attributes_wrapper(course_key):
+        """
+        Wraps the original get_additional_student_profile_attributes to add NAU per-course extra fields.
+        """
+        # Step 1: delegate to the original function
+        attributes = prev_get_additional_student_profile_attributes(course_key)
+
+        # Step 2: read the per-course advanced setting
+        other_course_settings = get_other_course_settings(course_key)
+        extra_features = other_course_settings.get("value", {}).get(_COURSE_SETTING_KEY, [])
+
+        if not extra_features:
+            return attributes
+
+        # Step 3: validate the value is a list
+        if not isinstance(extra_features, list):
+            logger.warning(
+                "Course other settings '%s' for course '%s' is not a list (got %s), ignoring.",
+                _COURSE_SETTING_KEY, course_key, type(extra_features).__name__,
+            )
+            return attributes
+
+        # Step 4: filter against the site-level Django allowlist
+        allowlist = getattr(settings, _DJANGO_ALLOWLIST_SETTING, [])
+        existing = set(attributes)
+        new_fields = []
+        for field in extra_features:
+            if field not in allowlist:
+                logger.warning(
+                    "Field '%s' requested by course '%s' is not in the allowlist '%s', skipping.",
+                    field, course_key, _DJANGO_ALLOWLIST_SETTING,
+                )
+            elif field not in existing:
+                # Step 5: append only fields not already present
+                new_fields.append(field)
+
+        if new_fields:
+            attributes = attributes + new_fields
+
+        logger.info("Final additional student profile attributes for course '%s': %s", course_key, attributes)
+
+        return attributes
+
+    return get_additional_student_profile_attributes_wrapper