From caac46de68a9b77aa3fda471362ee4301e50729e Mon Sep 17 00:00:00 2001 From: Ivo Branco Date: Tue, 9 May 2023 15:31:39 +0100 Subject: [PATCH 1/2] feat: add enrollment_date to profile data csv Add `enrollment_date` column on the csv file of all students enrolled in a course. --- lms/djangoapps/instructor/views/api.py | 4 ++- lms/djangoapps/instructor_analytics/basic.py | 34 ++++++++++++------- .../instructor_analytics/tests/test_basic.py | 29 ++++++++++++++-- 3 files changed, 51 insertions(+), 16 deletions(-) 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..4f7e92f5766f 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,7 @@ def issued_certificates(course_key, features): return generated_certificates -def enrolled_students_features(course_key, features): +def enrolled_students_features(course_key, features): # lint-amnesty, pylint: disable=too-many-statements """ Return list of student features as dictionaries. @@ -101,18 +101,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 STUDENT_FEATURES 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 +134,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 +194,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..28e30683b36a 100644 --- a/lms/djangoapps/instructor_analytics/tests/test_basic.py +++ b/lms/djangoapps/instructor_analytics/tests/test_basic.py @@ -15,6 +15,7 @@ PROFILE_FEATURES, PROGRAM_ENROLLMENT_FEATURES, STUDENT_FEATURES, + ENROLLMENT_FEATURES, StudentModule, enrolled_students_features, get_proctored_exam_results, @@ -250,8 +251,32 @@ 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(1): + 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_list_may_enroll(self): may_enroll = list_may_enroll(self.course_key, ['email']) From e907ed4ef58d14b569b5f4d9d959a635dec78e7c Mon Sep 17 00:00:00 2001 From: Ivo Branco Date: Wed, 10 May 2023 11:34:55 +0100 Subject: [PATCH 2/2] feat: add custom fields on profile data csv Allow site operators to include on the export of profile information as CSV custom fields if the platform has an extending User model. This can be used if you have an extended model that include for example an university student number and site operator want to export the student number on the student profile information CSV. --- lms/djangoapps/instructor_analytics/basic.py | 61 ++++++++++++++++++- .../instructor_analytics/tests/test_basic.py | 43 +++++++++++-- 2 files changed, 98 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/instructor_analytics/basic.py b/lms/djangoapps/instructor_analytics/basic.py index 4f7e92f5766f..64a93d4a834c 100644 --- a/lms/djangoapps/instructor_analytics/basic.py +++ b/lms/djangoapps/instructor_analytics/basic.py @@ -84,6 +84,65 @@ def issued_certificates(course_key, features): return generated_certificates +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. @@ -117,7 +176,7 @@ def enrolled_students_features(course_key, features): # lint-amnesty, pylint: d students = [enrollment.user for enrollment in enrollments] - student_features = [x for x in STUDENT_FEATURES if x in features] + 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: diff --git a/lms/djangoapps/instructor_analytics/tests/test_basic.py b/lms/djangoapps/instructor_analytics/tests/test_basic.py index 28e30683b36a..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 @@ -25,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 @@ -132,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) @@ -160,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: @@ -217,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 @@ -239,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: @@ -268,7 +272,7 @@ 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(1): + with self.assertNumQueries(2): userreports = enrolled_students_features(self.course_key, query_features) assert len(userreports) == len(self.users) @@ -278,6 +282,35 @@ def test_enrolled_students_enrollment_date(self): 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']) assert len(may_enroll) == (len(self.students_who_may_enroll) - len(self.users))