diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index e7a82496456e..da811ad4f42d 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -1433,7 +1433,8 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=red query_features = [ 'id', 'username', 'name', 'email', 'language', 'location', 'year_of_birth', 'gender', 'level_of_education', 'mailing_address', - 'goals', 'enrollment_mode', 'last_login', 'date_joined', 'external_user_key' + 'goals', 'enrollment_mode', 'last_login', 'date_joined', 'external_user_key', + 'enrollment_date' ] keep_field_private(query_features, 'year_of_birth') # protected information @@ -1456,6 +1457,7 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=red 'last_login': _('Last Login'), 'date_joined': _('Date Joined'), 'external_user_key': _('External User Key'), + 'enrollment_date': _('Enrollment Date'), } if is_course_cohorted(course.id): diff --git a/lms/djangoapps/instructor_analytics/basic.py b/lms/djangoapps/instructor_analytics/basic.py index c7bc6ca6da4c..64a93d4a834c 100644 --- a/lms/djangoapps/instructor_analytics/basic.py +++ b/lms/djangoapps/instructor_analytics/basic.py @@ -10,7 +10,6 @@ import logging from django.conf import settings -from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.core.exceptions import ObjectDoesNotExist from django.core.serializers.json import DjangoJSONEncoder from django.db.models import Count # lint-amnesty, pylint: disable=unused-import @@ -38,6 +37,7 @@ 'level_of_education', 'mailing_address', 'goals', 'meta', 'city', 'country') PROGRAM_ENROLLMENT_FEATURES = ('external_user_key', ) +ENROLLMENT_FEATURES = ('enrollment_date', ) ORDER_ITEM_FEATURES = ('list_price', 'unit_cost', 'status') ORDER_FEATURES = ('purchase_time',) @@ -49,7 +49,7 @@ 'bill_to_street2', 'bill_to_city', 'bill_to_state', 'bill_to_postalcode', 'bill_to_country', 'order_type', 'created') -AVAILABLE_FEATURES = STUDENT_FEATURES + PROFILE_FEATURES + PROGRAM_ENROLLMENT_FEATURES +AVAILABLE_FEATURES = STUDENT_FEATURES + PROFILE_FEATURES + PROGRAM_ENROLLMENT_FEATURES + ENROLLMENT_FEATURES COURSE_REGISTRATION_FEATURES = ('code', 'course_id', 'created_by', 'created_at', 'is_valid') COUPON_FEATURES = ('code', 'course_id', 'percentage_discount', 'description', 'expiration_date', 'is_active') CERTIFICATE_FEATURES = ('course_id', 'mode', 'status', 'grade', 'created_date', 'is_active', 'error_reason') @@ -84,7 +84,66 @@ def issued_certificates(course_key, features): return generated_certificates -def enrolled_students_features(course_key, features): +def get_student_features_with_custom(course_key): + """ + Allow site operators to include on the export custom fields if platform has an extending + User model. This can be used if you have an extended model that include for example + an university student number. + + Basic example of adding age: + ```python + def get_age(self): + return datetime.datetime.now().year - self.profile.year_of_birth + setattr(User, 'age', property(get_age)) + ``` + Then you have to add `age` to both site configurations: + - `student_profile_download_fields_custom_student_attributes` + - `student_profile_download_fields` site configurations` + + ```json + "student_profile_download_fields_custom_student_attributes": ["age"], + "student_profile_download_fields": [ + "id", "username", "name", "email", "language", "location", + "year_of_birth", "gender", "level_of_education", "mailing_address", + "goals", "enrollment_mode", "last_login", "date_joined", "external_user_key", + "enrollment_date", "age" + ] + ``` + + Example if the platform has a custom user extended model like a One-To-One Link + with the User Model: + ```python + def get_user_extended_model_custom_field(self): + if hasattr(self, "userextendedmodel"): + return self.userextendedmodel.custom_field + return None + setattr(User, 'user_extended_model_custom_field', property(get_user_extended_model_custom_field)) + ``` + + ```json + "student_profile_download_fields_custom_student_attributes": ["user_extended_model_custom_field"], + "student_profile_download_fields": [ + "id", "username", "name", "email", "language", "location", + "year_of_birth", "gender", "level_of_education", "mailing_address", + "goals", "enrollment_mode", "last_login", "date_joined", "external_user_key", + "enrollment_date", "user_extended_model_custom_field" + ] + ``` + """ + return STUDENT_FEATURES + tuple( + configuration_helpers.get_value_for_org( + course_key.org, + "student_profile_download_fields_custom_student_attributes", + getattr( + settings, + "STUDENT_PROFILE_DOWNLOAD_FIELDS_CUSTOM_STUDENT_ATTRIBUTES", + (), + ), + ) + ) + + +def enrolled_students_features(course_key, features): # lint-amnesty, pylint: disable=too-many-statements """ Return list of student features as dictionaries. @@ -101,18 +160,24 @@ def enrolled_students_features(course_key, features): include_enrollment_mode = 'enrollment_mode' in features include_verification_status = 'verification_status' in features include_program_enrollments = 'external_user_key' in features + include_enrollment_date = 'enrollment_date' in features external_user_key_dict = {} - students = User.objects.filter( - courseenrollment__course_id=course_key, - courseenrollment__is_active=1, - ).order_by('username').select_related('profile') + enrollments = CourseEnrollment.objects.filter( + course_id=course_key, + is_active=1, + ).select_related('user').order_by('user__username').select_related('user__profile') if include_cohort_column: - students = students.prefetch_related('course_groups') + enrollments = enrollments.prefetch_related('user__course_groups') if include_team_column: - students = students.prefetch_related('teams') + enrollments = enrollments.prefetch_related('user__teams') + + students = [enrollment.user for enrollment in enrollments] + + student_features = [x for x in get_student_features_with_custom(course_key) if x in features] + profile_features = [x for x in PROFILE_FEATURES if x in features] if include_program_enrollments and len(students) > 0: program_enrollments = fetch_program_enrollments_by_students(users=students, realized_only=True) @@ -128,10 +193,9 @@ def extract_attr(student, feature): except TypeError: return str(attr) - def extract_student(student, features): + def extract_enrollment_student(enrollment, features): """ convert student to dictionary """ - student_features = [x for x in STUDENT_FEATURES if x in features] - profile_features = [x for x in PROFILE_FEATURES if x in features] + student = enrollment.user # For data extractions on the 'meta' field # the feature name should be in the format of 'meta.foo' where @@ -189,9 +253,12 @@ def extract_student(student, features): # extra external_user_key student_dict['external_user_key'] = external_user_key_dict.get(student.id, '') + if include_enrollment_date: + student_dict['enrollment_date'] = enrollment.created + return student_dict - return [extract_student(student, features) for student in students] + return [extract_enrollment_student(enrollment, features) for enrollment in enrollments] def list_may_enroll(course_key, features): diff --git a/lms/djangoapps/instructor_analytics/tests/test_basic.py b/lms/djangoapps/instructor_analytics/tests/test_basic.py index 83f23246eaa2..fe0fb6daacb2 100644 --- a/lms/djangoapps/instructor_analytics/tests/test_basic.py +++ b/lms/djangoapps/instructor_analytics/tests/test_basic.py @@ -5,8 +5,11 @@ from unittest.mock import MagicMock, Mock, patch +import random +import datetime import ddt import json # lint-amnesty, pylint: disable=wrong-import-order +from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from edx_proctoring.api import create_exam from edx_proctoring.models import ProctoredExamStudentAttempt from opaque_keys.edx.locator import UsageKey @@ -15,6 +18,7 @@ PROFILE_FEATURES, PROGRAM_ENROLLMENT_FEATURES, STUDENT_FEATURES, + ENROLLMENT_FEATURES, StudentModule, enrolled_students_features, get_proctored_exam_results, @@ -24,6 +28,7 @@ ) from lms.djangoapps.program_enrollments.tests.factories import ProgramEnrollmentFactory from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory +from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAllowed from common.djangoapps.student.tests.factories import InstructorFactory from common.djangoapps.student.tests.factories import UserFactory @@ -131,7 +136,7 @@ def test_enrolled_students_features_keys(self): user.profile.save() for feature in query_features: assert feature in AVAILABLE_FEATURES - with self.assertNumQueries(1): + with self.assertNumQueries(2): userreports = enrolled_students_features(self.course_key, query_features) assert len(userreports) == len(self.users) @@ -159,7 +164,7 @@ def test_enrolled_students_meta_features_keys(self): Assert that we can query individual fields in the 'meta' field in the UserProfile """ query_features = ('meta.position', 'meta.company') - with self.assertNumQueries(1): + with self.assertNumQueries(2): userreports = enrolled_students_features(self.course_key, query_features) assert len(userreports) == len(self.users) for userreport in userreports: @@ -216,7 +221,7 @@ def test_enrolled_students_features_keys_cohorted(self): # enrolled_students_features. The first query comes from the call to # User.objects.filter(...), and the second comes from # prefetch_related('course_groups'). - with self.assertNumQueries(2): + with self.assertNumQueries(3): userreports = enrolled_students_features(course.id, query_features) assert len([r for r in userreports if r['username'] in cohorted_usernames]) == len(cohorted_students) assert len([r for r in userreports if r['username'] == non_cohorted_student.username]) == 1 @@ -238,7 +243,7 @@ def test_enrolled_student_features_external_user_keys(self): ProgramEnrollmentFactory.create(user=user, external_user_key=external_user_key) username_with_external_user_key_dict[user.username] = external_user_key - with self.assertNumQueries(2): + with self.assertNumQueries(3): userreports = enrolled_students_features(self.course_key, query_features) assert len(userreports) == 30 for report in userreports: @@ -250,8 +255,61 @@ def test_enrolled_student_features_external_user_keys(self): assert '' == report['external_user_key'] def test_available_features(self): - assert len(AVAILABLE_FEATURES) == len(STUDENT_FEATURES + PROFILE_FEATURES + PROGRAM_ENROLLMENT_FEATURES) - assert set(AVAILABLE_FEATURES) == set(STUDENT_FEATURES + PROFILE_FEATURES + PROGRAM_ENROLLMENT_FEATURES) + assert len(AVAILABLE_FEATURES) == len( + STUDENT_FEATURES + + PROFILE_FEATURES + + PROGRAM_ENROLLMENT_FEATURES + + ENROLLMENT_FEATURES + ) + assert set(AVAILABLE_FEATURES) == set( + STUDENT_FEATURES + + PROFILE_FEATURES + + PROGRAM_ENROLLMENT_FEATURES + + ENROLLMENT_FEATURES + ) + + def test_enrolled_students_enrollment_date(self): + query_features = ('username', 'enrollment_date',) + for feature in query_features: + assert feature in AVAILABLE_FEATURES + with self.assertNumQueries(2): + userreports = enrolled_students_features(self.course_key, query_features) + assert len(userreports) == len(self.users) + + userreports = sorted(userreports, key=lambda u: u["username"]) + users = sorted(self.users, key=lambda u: u.username) + for userreport, user in zip(userreports, users): + assert set(userreport.keys()) == set(query_features) + assert userreport['enrollment_date'] == CourseEnrollment.enrollments_for_user(user)[0].created + + def test_enrolled_students_extended_model_age(self): + SiteConfigurationFactory.create( + site_values={ + 'course_org_filter': ['robot'], + 'student_profile_download_fields_custom_student_attributes': ['age'], + } + ) + + def get_age(self): + return datetime.datetime.now().year - self.profile.year_of_birth + setattr(User, "age", property(get_age)) # lint-amnesty, pylint: disable=literal-used-as-attribute + + for user in self.users: + user.profile.year_of_birth = random.randint(1900, 2000) + user.profile.save() + + query_features = ('username', 'age',) + with self.assertNumQueries(3): + userreports = enrolled_students_features(self.course_key, query_features) + assert len(userreports) == len(self.users) + + userreports = sorted(userreports, key=lambda u: u["username"]) + users = sorted(self.users, key=lambda u: u.username) + for userreport, user in zip(userreports, users): + assert set(userreport.keys()) == set(query_features) + assert userreport['age'] == str(user.age) + + delattr(User, "age") # lint-amnesty, pylint: disable=literal-used-as-attribute def test_list_may_enroll(self): may_enroll = list_may_enroll(self.course_key, ['email'])