From 9c73fb375600f951e0e86898232195090f0accb2 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Wed, 1 Apr 2026 14:25:57 -0500 Subject: [PATCH 1/7] Refactor program cert generation to support new cert requirements (#3439) --- courses/api.py | 46 ++- courses/api_test.py | 347 +++++++++++++++--- .../manage_program_certificate_test.py} | 21 +- 3 files changed, 345 insertions(+), 69 deletions(-) rename courses/management/{commands/test_manage_program_certificate.py => tests/manage_program_certificate_test.py} (90%) diff --git a/courses/api.py b/courses/api.py index 0a98c309bc..f4d5298864 100644 --- a/courses/api.py +++ b/courses/api.py @@ -1042,11 +1042,16 @@ def _has_earned_program_cert(user, program): """ program_course_ids = [course[0].id for course in program.courses] - passed_courses = Course.objects.filter( + cert_courses = Course.objects.filter( id__in=program_course_ids, courseruns__courseruncertificates__user=user, courseruns__courseruncertificates__is_revoked=False, ) + grade_courses = Course.objects.filter( + id__in=program_course_ids, + courseruns__grades__user=user, + courseruns__grades__passed=True, + ).exclude(courseruns__enrollment_modes__mode_slug=EDX_ENROLLMENT_VERIFIED_MODE) root = ProgramRequirement.get_root_nodes().get(program=program) def _has_earned(node): @@ -1059,8 +1064,8 @@ def _has_earned(node): node.operator_value ) elif node.is_course: - # has passed the reference course - return node.course in passed_courses + # has passed the referenced course + return node.course in [*cert_courses, *grade_courses] elif node.is_program: # has earned certificate for the required sub-program return ProgramCertificate.objects.filter( @@ -1073,9 +1078,8 @@ def _has_earned(node): def generate_program_certificate(user, program, force_create=False): # noqa: FBT002 """ - Create a program certificate if the user has a course certificate - for each course in the program. Also, It will create the - program enrollment if it does not exist for the user. + Create a program certificate if the user has a verified enrollment in the + program and certificates or passing grades in the program's required courses. Args: user (User): a Django user. @@ -1090,13 +1094,26 @@ def generate_program_certificate(user, program, force_create=False): # noqa: FB """ from hubspot_sync.task_helpers import sync_hubspot_user # noqa: PLC0415 + if ( + not force_create + and not program.enrollments.filter( + user=user, enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE + ).exists() + ): + log.warning( + "Skipping program enrollment generation for %s in %s: no verified enrollment", + user, + program, + ) + return ( + None, + False, + ) + existing_cert_queryset = ProgramCertificate.all_objects.filter( user=user, program=program ) if existing_cert_queryset.exists(): - ProgramEnrollment.objects.get_or_create( - program=program, user=user, defaults={"active": True, "change_status": None} - ) return existing_cert_queryset.first(), False if not force_create and not _has_earned_program_cert(user, program): @@ -1113,17 +1130,6 @@ def generate_program_certificate(user, program, force_create=False): # noqa: FB if not program_cert.verifiable_credential_id: create_verifiable_credential(program_cert) - _, created = ProgramEnrollment.objects.get_or_create( - program=program, user=user, defaults={"active": True, "change_status": None} - ) - - if created: - log.info( - "Program enrollment for [%s] in program [%s] is created.", - user.edx_username, - program.title, - ) - return program_cert, True diff --git a/courses/api_test.py b/courses/api_test.py index 54261406eb..ba3896364f 100644 --- a/courses/api_test.py +++ b/courses/api_test.py @@ -65,6 +65,7 @@ CourseRunFactory, CourseRunGradeFactory, DepartmentFactory, + EnrollmentModeFactory, ProgramCertificateFactory, ProgramEnrollmentFactory, ProgramFactory, @@ -143,6 +144,16 @@ def courses_api_logs(mocker): return mocker.patch("courses.api.log") +@pytest.fixture +def default_mode_records(): + """Returns the default modes we expect.""" + + return [ + EnrollmentModeFactory.create(mode_slug=EDX_ENROLLMENT_AUDIT_MODE), + EnrollmentModeFactory.create(mode_slug=EDX_ENROLLMENT_VERIFIED_MODE), + ] + + def _mock_edx_course_detail(coursekey, settings): """Make a fake course detail record for the given key.""" @@ -1301,16 +1312,56 @@ def test_create_run_enrollments_upgrade_edx_request_failure(mocker, user): assert successful_enrollments[0].edx_enrolled == False # noqa: E712 +@pytest.mark.parametrize( + "has_enrollment", + [ + True, + False, + ], +) +def test_generate_program_certificate_not_verified( + user, + program_with_requirements, # noqa: F811 + has_enrollment, + caplog, +): + """Test that certificate generation fails if the user doesn't have a verified enrollment.""" + + # This should fail early - before the check for course enrollments. + + assert not program_with_requirements.program.enrollments.filter(user=user).exists() + + if has_enrollment: + ProgramEnrollment.objects.create( + program=program_with_requirements.program, + user=user, + enrollment_mode=EDX_ENROLLMENT_AUDIT_MODE, + ) + + result = generate_program_certificate( + user=user, + program=program_with_requirements.program, + ) + + assert result == (None, False) + assert "Skipping program enrollment certificate" not in caplog.messages + + def test_generate_program_certificate_failure_missing_certificates( user, program_with_requirements, # noqa: F811 + default_mode_records, ): """ Test that generate_program_certificate return (None, False) and not create program certificate if there is not any course_run certificate for the given course. """ course = CourseFactory.create() - CourseRunFactory.create_batch(3, course=course) + courseruns = CourseRunFactory.create_batch(3, course=course) + for run in courseruns: + run.enrollment_modes.set(default_mode_records) + run.save() + program_with_requirements.program.add_requirement(course) result = generate_program_certificate( @@ -1321,11 +1372,13 @@ def test_generate_program_certificate_failure_missing_certificates( @patch("courses.signals.upsert_custom_properties") -def test_generate_program_certificate_failure_not_all_passed( +def test_generate_program_certificate_failure_not_all_passed( # noqa: PLR0913 mock_upsert_custom_properties, + caplog, user, program_with_requirements, # noqa: F811 mocker, + default_mode_records, ): """ Test that generate_program_certificate return (None, False) and not create program certificate @@ -1334,8 +1387,17 @@ def test_generate_program_certificate_failure_not_all_passed( mocker.patch( "hubspot_sync.api.upsert_custom_properties", ) + ProgramEnrollment.objects.create( + user=user, + program=program_with_requirements.program, + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, + ) courses = CourseFactory.create_batch(3) course_runs = CourseRunFactory.create_batch(3, course=factory.Iterator(courses)) + for run in course_runs: + run.enrollment_modes.set(default_mode_records) + run.save() + CourseRunCertificateFactory.create_batch( 2, user=user, course_run=factory.Iterator(course_runs) ) @@ -1347,11 +1409,12 @@ def test_generate_program_certificate_failure_not_all_passed( result = generate_program_certificate(user=user, program=program) assert result == (None, False) assert len(ProgramCertificate.objects.all()) == 0 + assert "Skipping program enrollment certificate" not in caplog.messages @patch("courses.signals.upsert_custom_properties") def test_generate_program_certificate_success_single_requirement_course( - mock_upsert_custom_properties, user, mocker + mock_upsert_custom_properties, user, mocker, default_mode_records ): """ Test that generate_program_certificate generates a program certificate for a Program with a single required Course. @@ -1364,6 +1427,11 @@ def test_generate_program_certificate_success_single_requirement_course( ) course = CourseFactory.create() program = ProgramFactory.create() + ProgramEnrollment.objects.create( + user=user, + program=program, + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, + ) root_node = program.requirements_root root_node.add_child( @@ -1373,6 +1441,8 @@ def test_generate_program_certificate_success_single_requirement_course( ) program.add_requirement(course) course_run = CourseRunFactory.create(course=course) + course_run.enrollment_modes.set(default_mode_records) + course_run.save() CourseRunGradeFactory.create(course_run=course_run, user=user, passed=True, grade=1) CourseRunCertificateFactory.create(user=user, course_run=course_run) @@ -1386,7 +1456,7 @@ def test_generate_program_certificate_success_single_requirement_course( @patch("courses.signals.upsert_custom_properties") def test_generate_program_certificate_success_multiple_required_courses( - mock_upsert_custom_properties, user, mocker + mock_upsert_custom_properties, user, mocker, default_mode_records ): """ Test that generate_program_certificate generate a program certificate @@ -1399,6 +1469,11 @@ def test_generate_program_certificate_success_multiple_required_courses( ) courses = CourseFactory.create_batch(3) program = ProgramFactory.create() + ProgramEnrollment.objects.create( + user=user, + program=program, + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, + ) root_node = program.requirements_root root_node.add_child( @@ -1409,6 +1484,10 @@ def test_generate_program_certificate_success_multiple_required_courses( for course in courses: program.add_requirement(course) course_runs = CourseRunFactory.create_batch(3, course=factory.Iterator(courses)) + for run in course_runs: + run.enrollment_modes.set(default_mode_records) + run.save() + CourseRunCertificateFactory.create_batch( 3, user=user, course_run=factory.Iterator(course_runs) ) @@ -1422,7 +1501,7 @@ def test_generate_program_certificate_success_multiple_required_courses( @patch("courses.signals.upsert_custom_properties") def test_generate_program_certificate_success_minimum_electives_not_met( - mock_upsert_custom_properties, user, mocker + mock_upsert_custom_properties, user, mocker, default_mode_records ): """ Test that generate_program_certificate does not generate a program certificate if minimum electives have not been met. @@ -1434,6 +1513,11 @@ def test_generate_program_certificate_success_minimum_electives_not_met( # Create Program with 2 minimum elective courses. program = ProgramFactory.create() + ProgramEnrollment.objects.create( + user=user, + program=program, + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, + ) root_node = program.requirements_root root_node.add_child( @@ -1456,7 +1540,14 @@ def test_generate_program_certificate_success_minimum_electives_not_met( required_course1_course_run = CourseRunFactory.create(course=required_course1) elective_course1_course_run = CourseRunFactory.create(course=elective_course1) - elective_course2_course_run = CourseRunFactory.create(course=elective_course2) # noqa: F841 + elective_course2_course_run = CourseRunFactory.create(course=elective_course2) + + required_course1_course_run.enrollment_modes.set(default_mode_records) + required_course1_course_run.save() + elective_course1_course_run.enrollment_modes.set(default_mode_records) + elective_course1_course_run.save() + elective_course2_course_run.enrollment_modes.set(default_mode_records) + elective_course2_course_run.save() # User has a certificate for required_course1 and elective_course1 only. No certificate for elective_course2. CourseRunCertificateFactory.create( @@ -1471,12 +1562,21 @@ def test_generate_program_certificate_success_minimum_electives_not_met( assert len(ProgramCertificate.objects.all()) == 0 +@pytest.mark.parametrize( + "has_program_enrollment", + [ + True, + False, + ], +) @patch("courses.signals.upsert_custom_properties") -def test_force_generate_program_certificate_success( +def test_force_generate_program_certificate_success( # noqa: PLR0913 mock_upsert_custom_properties, user, program_with_requirements, # noqa: F811 mocker, + has_program_enrollment, + caplog, ): """ Test that force creating a program certificate with generate_program_certificate generates @@ -1488,6 +1588,14 @@ def test_force_generate_program_certificate_success( mocker.patch( "hubspot_sync.api.upsert_custom_properties", ) + + if has_program_enrollment: + ProgramEnrollment.objects.create( + user=user, + program=program_with_requirements.program, + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, + ) + courses = CourseFactory.create_batch(3) course_runs = CourseRunFactory.create_batch(3, course=factory.Iterator(courses)) CourseRunCertificateFactory.create_batch( @@ -1506,6 +1614,9 @@ def test_force_generate_program_certificate_success( assert len(ProgramCertificate.objects.all()) == 1 patched_sync_hubspot_user.assert_called_once_with(user) + if not has_program_enrollment: + assert "Skipping program enrollment certificate" not in caplog.messages + def test_generate_program_certificate_already_exist( user, @@ -1515,6 +1626,11 @@ def test_generate_program_certificate_already_exist( Test that generate_program_certificate return (None, False) and not create program certificate if program certificate already exist. """ + ProgramEnrollment.objects.create( + user=user, + program=program_with_empty_requirements, + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, + ) program_certificate = ProgramCertificateFactory.create( program=program_with_empty_requirements, user=user ) @@ -1528,6 +1644,11 @@ def test_generate_program_certificate_already_exist( def test_program_certificates_access(): """Tests that the revoke and unrevoke for a program certificates sets the states properly""" test_certificate = ProgramCertificateFactory.create(is_revoked=False) + ProgramEnrollment.objects.create( + user=test_certificate.user, + program=test_certificate.program, + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, + ) # Revoke a program certificate manage_program_certificate_access( @@ -1554,6 +1675,7 @@ def test_generate_program_certificate_failure_not_all_passed_nested_elective_sti mock_upsert_custom_properties, user, mocker, + default_mode_records, ): """ Test that generate_program_certificate returns (None, False) and does not create a program certificate @@ -1561,11 +1683,19 @@ def test_generate_program_certificate_failure_not_all_passed_nested_elective_sti """ courses = CourseFactory.create_batch(3) course_runs = CourseRunFactory.create_batch(3, course=factory.Iterator(courses)) + for run in course_runs: + run.enrollment_modes.set(default_mode_records) + run.save() mocker.patch( "hubspot_sync.api.upsert_custom_properties", ) # Create Program program = ProgramFactory.create() + ProgramEnrollment.objects.create( + user=user, + program=program, + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, + ) root_node = program.requirements_root root_node.add_child( @@ -1608,9 +1738,26 @@ def test_generate_program_certificate_failure_not_all_passed_nested_elective_sti assert len(ProgramCertificate.objects.all()) == 0 +@pytest.mark.parametrize( + ("has_main_enroll", "has_sub_enroll"), + [ + ( + True, + True, + ), + (True, False), + (False, True), + (False, False), + ], +) @patch("courses.signals.upsert_custom_properties") -def test_generate_program_certificate_with_subprogram_requirement( - mock_upsert_custom_properties, user, mocker +def test_generate_program_certificate_with_subprogram_requirement( # noqa: PLR0913 + mock_upsert_custom_properties, + user, + mocker, + default_mode_records, + has_main_enroll, + has_sub_enroll, ): """ Test that generate_program_certificate considers sub-program (nested program) requirements @@ -1628,64 +1775,61 @@ def test_generate_program_certificate_with_subprogram_requirement( sub_course = CourseFactory.create() sub_program.add_requirement(sub_course) + if has_sub_enroll: + ProgramEnrollment.objects.create( + user=user, + program=sub_program, + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, + ) + # Create the main program that requires the sub-program main_program = ProgramFactory.create() main_program.add_program_requirement(sub_program) + if has_main_enroll: + ProgramEnrollment.objects.create( + user=user, + program=main_program, + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, + ) + # User completes the sub-program course and gets a certificate sub_course_run = CourseRunFactory.create(course=sub_course) + sub_course_run.enrollment_modes.set(default_mode_records) + sub_course_run.save() CourseRunGradeFactory.create( course_run=sub_course_run, user=user, passed=True, grade=1 ) CourseRunCertificateFactory.create(user=user, course_run=sub_course_run) - # Generate sub-program certificate (user has completed sub-program requirements) + # Test sub-program certificate (user has completed sub-program requirements) sub_certificate, sub_created = generate_program_certificate( user=user, program=sub_program ) - assert sub_created is True - assert isinstance(sub_certificate, ProgramCertificate) + if has_sub_enroll: + assert sub_created is True + assert isinstance(sub_certificate, ProgramCertificate) # Now try to generate main program certificate - # It should succeed because the user has a certificate for the required sub-program + # It should succeed if the user has a certificate for the required sub-program main_certificate, main_created = generate_program_certificate( user=user, program=main_program ) - assert main_created is True - assert isinstance(main_certificate, ProgramCertificate) - assert len(ProgramCertificate.objects.all()) == 2 - patched_sync_hubspot_user.assert_called() - - -def test_generate_program_certificate_with_subprogram_requirement_missing_certificate( - user, mocker -): - """ - Test that generate_program_certificate does NOT generate a certificate when the required - sub-program certificate is missing. - """ - mocker.patch( - "hubspot_sync.api.upsert_custom_properties", - ) - - # Create a sub-program - sub_program = ProgramFactory.create() - sub_course = CourseFactory.create() - sub_program.add_requirement(sub_course) - - # Create the main program that requires the sub-program - main_program = ProgramFactory.create() - main_program.add_program_requirement(sub_program) - - # User does NOT complete the sub-program course (no certificate) - # Try to generate main program certificate - # It should fail because the user does not have a certificate for the required sub-program - main_certificate, main_created = generate_program_certificate( - user=user, program=main_program - ) - assert main_created is False - assert main_certificate is None - assert len(ProgramCertificate.objects.all()) == 0 + if has_main_enroll and has_sub_enroll: + # Should only get a certificate if we had a cert in the sub + # So, we'd have to have been enrolled there, too + assert main_created is True + assert isinstance(main_certificate, ProgramCertificate) + assert len(ProgramCertificate.objects.all()) == 2 + patched_sync_hubspot_user.assert_called() + + if not has_main_enroll or not has_sub_enroll: + assert ( + ProgramCertificate.objects.filter( + program__in=[main_program, sub_program], user=user + ).count() + < 2 + ) @patch("courses.signals.upsert_custom_properties") @@ -1705,10 +1849,22 @@ def test_generate_program_certificate_with_revoked_subprogram_certificate( sub_course = CourseFactory.create() sub_program.add_requirement(sub_course) + ProgramEnrollment.objects.create( + user=user, + program=sub_program, + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, + ) + # Create the main program that requires the sub-program main_program = ProgramFactory.create() main_program.add_program_requirement(sub_program) + ProgramEnrollment.objects.create( + user=user, + program=main_program, + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, + ) + # User completes the sub-program and gets a certificate, but it gets revoked sub_course_run = CourseRunFactory.create(course=sub_course) CourseRunGradeFactory.create( @@ -1721,6 +1877,7 @@ def test_generate_program_certificate_with_revoked_subprogram_certificate( user=user, program=sub_program ) assert sub_created is True + assert isinstance(sub_certificate, ProgramCertificate) sub_certificate.is_revoked = True sub_certificate.save() @@ -1736,6 +1893,100 @@ def test_generate_program_certificate_with_revoked_subprogram_certificate( assert len(ProgramCertificate.all_objects.all()) == 1 +def test_generate_program_certificate_audit_courses(user, default_mode_records): + """ + Test that the certificates generate OK if the program has courses that are + audit-only. + + Worth noting: simply having a passing grade in a course that you're auditing, + when the course has a verified mode in it, should not grant you a certificate. + """ + + program = ProgramFactory.create() + program.requirements_root.add_child( + node_type=ProgramRequirementNodeType.OPERATOR, + operator=ProgramRequirement.Operator.ALL_OF, + title="Required Courses", + ) + + cert_course = CourseFactory.create() + cert_course_run = CourseRunFactory.create(course=cert_course) + cert_course_run.enrollment_modes.set(default_mode_records) + cert_course.save() + + audit_course = CourseFactory.create() + audit_course_run = CourseRunFactory.create(course=audit_course) + # Potential flakiness: if the fixture is changed and these aren't in the right + # order, this will be wrong. + audit_course_run.enrollment_modes.add(default_mode_records[0]) + audit_course.save() + + program.add_requirement(cert_course) + program.add_requirement(audit_course) + + ProgramEnrollment.objects.create( + user=user, + program=program, + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, + ) + CourseRunEnrollment.objects.create( + user=user, + run=cert_course_run, + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, + ) + CourseRunEnrollment.objects.create( + user=user, + run=audit_course_run, + enrollment_mode=EDX_ENROLLMENT_AUDIT_MODE, + ) + + CourseRunGradeFactory.create( + course_run=cert_course_run, + user=user, + passed=True, + ) + audit_grade = CourseRunGradeFactory.create( + course_run=audit_course_run, + user=user, + passed=False, + ) + + # This should fail - you have neither a cert in the cert run, nor a passing + # grade in the audit course, so you don't meet the requirements. + result = generate_program_certificate(user, program) + assert result == (None, False) + + audit_grade.passed = True + audit_grade.save() + + # This should also fail - you don't have a cert in the cert run, but a passing + # grade in the audit course. You need both, so you don't meet the requirements. + result = generate_program_certificate(user, program) + assert result == (None, False) + + audit_grade.passed = False + audit_grade.save() + + CourseRunCertificateFactory.create( + user=user, + course_run=cert_course_run, + ) + + # This should also fail - you have a cert in the cert run, but no passing + # grade in the audit course, so you still don't meet the requirements. + result = generate_program_certificate(user, program) + assert result == (None, False) + + audit_grade.passed = True + audit_grade.save() + + # This should succeed - you have a cert in the cert run, and a passing grade + # in the audit run, so you're good now. + cert, created = generate_program_certificate(user, program) + assert created + assert isinstance(cert, ProgramCertificate) + + @pytest.mark.parametrize( "mocked_api_response, expect_success", # noqa: PT006 [ diff --git a/courses/management/commands/test_manage_program_certificate.py b/courses/management/tests/manage_program_certificate_test.py similarity index 90% rename from courses/management/commands/test_manage_program_certificate.py rename to courses/management/tests/manage_program_certificate_test.py index ae02cabd10..daa3afec55 100644 --- a/courses/management/commands/test_manage_program_certificate.py +++ b/courses/management/tests/manage_program_certificate_test.py @@ -9,6 +9,7 @@ CourseRunCertificateFactory, CourseRunFactory, CourseRunGradeFactory, + EnrollmentModeFactory, ProgramCertificateFactory, ProgramFactory, ProgramRequirementFactory, # noqa: F401 @@ -16,7 +17,11 @@ program_with_requirements, # noqa: F401 ) from courses.management.commands import manage_program_certificates -from courses.models import ProgramCertificate +from courses.models import ProgramCertificate, ProgramEnrollment +from openedx.constants import ( + EDX_ENROLLMENT_AUDIT_MODE, + EDX_ENROLLMENT_VERIFIED_MODE, +) from users.factories import UserFactory pytestmark = [pytest.mark.django_db] @@ -129,6 +134,17 @@ def test_program_certificate_management_create( Test that create operation for program certificate management command creates the program certificate for a user """ + modes = [ + EnrollmentModeFactory.create(mode_slug=EDX_ENROLLMENT_AUDIT_MODE), + EnrollmentModeFactory.create(mode_slug=EDX_ENROLLMENT_VERIFIED_MODE), + ] + + ProgramEnrollment.objects.create( + user=user, + program=program_with_empty_requirements, + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, + ) + mocker.patch( "hubspot_sync.api.upsert_custom_properties", ) @@ -136,6 +152,9 @@ def test_program_certificate_management_create( program_with_empty_requirements.add_requirement(courses[0]) program_with_empty_requirements.add_elective(courses[1]) course_runs = CourseRunFactory.create_batch(2, course=factory.Iterator(courses)) + for run in course_runs: + run.enrollment_modes.set(modes) + run.save() CourseRunCertificateFactory.create_batch( 2, user=user, course_run=factory.Iterator(course_runs) ) From 1c1f5b776c05a187ca82dab4ab429cf675f2307a Mon Sep 17 00:00:00 2001 From: Muhammad Anas <88967643+Anas12091101@users.noreply.github.com> Date: Thu, 2 Apr 2026 00:49:49 +0500 Subject: [PATCH 2/7] fix: fall back to audit enrollment for non-upgradable runs in verified program enrollment API (#3451) --- courses/views/v2/__init__.py | 31 ++++++++++++------ courses/views/v2/views_test.py | 58 ++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/courses/views/v2/__init__.py b/courses/views/v2/__init__.py index fd887b5db4..393177c9d3 100644 --- a/courses/views/v2/__init__.py +++ b/courses/views/v2/__init__.py @@ -682,27 +682,38 @@ def destroy(self, request, *args, **kwargs): # noqa: ARG002 def _create_course_enrollment_from_program(request, courserun_id, program_enrollment): """Create the course enrollment based on the specified program enrollment.""" + run = CourseRun.objects.filter(courseware_id=courserun_id).get() + + in_program = run.course in [ + *program_enrollment.program.required_courses, + *program_enrollment.program.elective_courses, + ] + + should_create_audit_enrollment = ( + program_enrollment.enrollment_mode == EDX_ENROLLMENT_AUDIT_MODE + or not in_program + or not run.is_upgradable + ) + + effective_enrollment_mode = ( + EDX_ENROLLMENT_AUDIT_MODE + if should_create_audit_enrollment + else program_enrollment.enrollment_mode + ) + if CourseRunEnrollment.objects.filter( run__courseware_id=courserun_id, user=request.user, - enrollment_mode=program_enrollment.enrollment_mode, + enrollment_mode=effective_enrollment_mode, ).exists(): # Learner already has a matching enrollment, so nothing to do. return Response(status=status.HTTP_204_NO_CONTENT) - run = CourseRun.objects.filter(courseware_id=courserun_id).get() - # Check if: # .. the program enrollment is audit, # .. if the run isn't in the program - if (program_enrollment.enrollment_mode == EDX_ENROLLMENT_AUDIT_MODE) or ( - run.course - not in [ - *program_enrollment.program.required_courses, - *program_enrollment.program.elective_courses, - ] - ): + if should_create_audit_enrollment: # Audit enrollments just get created, regardless of whether or not # the course is an elective. enrollments, _ = create_run_enrollments( diff --git a/courses/views/v2/views_test.py b/courses/views/v2/views_test.py index 7f3ca5d2af..892f4c5d38 100644 --- a/courses/views/v2/views_test.py +++ b/courses/views/v2/views_test.py @@ -1773,6 +1773,64 @@ def test_add_verified_program_course_enrollment( assert resp.json()["enrollment_mode"] == EDX_ENROLLMENT_VERIFIED_MODE +@pytest.mark.skip_nplusone_check +@responses.activate +def test_add_verified_program_course_enrollment_audit_only_run_falls_back_to_audit( + user, user_drf_client +): + """Verify we fall back to audit when a program is verified but the run isn't upgradable.""" + + responses.add( + responses.GET, + f"{settings.OPENEDX_API_BASE_URL}/api/enrollment/v1/enrollments", + json={ + "results": [ + {"mode": EDX_ENROLLMENT_VERIFIED_MODE, "is_active": True}, + ], + }, + status=status.HTTP_200_OK, + ) + + prog_enrollment = ProgramEnrollmentFactory.create( + user=user, enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE + ) + program = prog_enrollment.program + + # Match existing verified setup: programs generally have a product. + program_content_type = ContentType.objects.get_for_model(program) + with reversion.create_revision(): + Product.objects.create( + price=10, + is_active=True, + object_id=program.id, + content_type=program_content_type, + ) + + course_run = CourseRunFactory.create() + program.add_requirement(course_run.course) + + # Intentionally do NOT create a Product for the course run. This makes the run + # not upgradable (audit-only), and the endpoint should create an audit enrollment + # instead of erroring while trying to generate a verified order. + + initial_order_count = user.orders.count() + + resp = user_drf_client.post( + reverse( + "v2:add_verified_program_course_enrollment", + kwargs={ + "courserun_id": course_run.courseware_id, + }, + ), + data=[program.readable_id], + ) + + assert resp.status_code == status.HTTP_201_CREATED + assert resp.json()["run"]["id"] == course_run.id + assert resp.json()["enrollment_mode"] == EDX_ENROLLMENT_AUDIT_MODE + assert user.orders.count() == initial_order_count + + @pytest.mark.skip_nplusone_check @responses.activate @pytest.mark.parametrize( From 4a68740caf49acac2c8bbb080c6732ea675385a1 Mon Sep 17 00:00:00 2001 From: annagav Date: Wed, 1 Apr 2026 16:23:18 -0400 Subject: [PATCH 3/7] Add test to the program enrollment dialog (#3445) --- .../components/ProgramProductDetailEnroll.js | 21 +++++++++------- .../ProgramProductDetailEnroll_test.js | 25 +++++++++++++++++++ 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/frontend/public/src/components/ProgramProductDetailEnroll.js b/frontend/public/src/components/ProgramProductDetailEnroll.js index 87f4227365..7398ac7f5b 100644 --- a/frontend/public/src/components/ProgramProductDetailEnroll.js +++ b/frontend/public/src/components/ProgramProductDetailEnroll.js @@ -139,15 +139,18 @@ export class ProgramProductDetailEnroll extends React.Component< this.setState({ upgradeEnrollmentDialogVisibility: !upgradeEnrollmentDialogVisibility }) - try { - //find program id - const program = programs && programs[0] - if (program) { - await createProgramEnrollment(program.id) + if (!upgradeEnrollmentDialogVisibility) { + //if opening dialog + try { + //find program id + const program = programs && programs[0] + if (program) { + await createProgramEnrollment(program.id) + } + } catch (error) { + console.error("Failed to create program enrollment", error) + // Optionally, display an error message to the user. } - } catch (error) { - console.error("Failed to create program enrollment", error) - // Optionally, display an error message to the user. } } @@ -360,7 +363,7 @@ export class ProgramProductDetailEnroll extends React.Component< return ( this.toggleAddlProfileFieldsModal()} diff --git a/frontend/public/src/components/ProgramProductDetailEnroll_test.js b/frontend/public/src/components/ProgramProductDetailEnroll_test.js index e0e39c7236..1728634bff 100644 --- a/frontend/public/src/components/ProgramProductDetailEnroll_test.js +++ b/frontend/public/src/components/ProgramProductDetailEnroll_test.js @@ -107,4 +107,29 @@ describe("ProgramProductDetailEnroll", () => { const modal = inner.find(".upgrade-enrollment-modal") assert.isOk(modal) }) + + it("calls program enrollment API when Enroll Now is clicked", async () => { + const program = programs[0] + const createProgramEnrollmentStub = helper.sandbox.stub().resolves({}) + + const { inner } = await renderPage() + inner.setProps({ createProgramEnrollment: createProgramEnrollmentStub }) + + const enrollBtn = inner.find(".enroll-now").at(0) + assert.isTrue(enrollBtn.exists()) + await enrollBtn.prop("onClick")() + + assert.isTrue(createProgramEnrollmentStub.calledOnceWithExactly(program.id)) + + const modal = inner.find("#upgrade-enrollment-dialog") + assert.isTrue(modal.find("select.form-control").exists()) + + assert.isFalse(modal.find("button.enroll-now-free").exists()) + + // toggle the dialog back and check if the program enrollment api was not called again + await modal.prop("toggle")() + inner.update() + assert.isFalse(inner.find("#upgrade-enrollment-dialog").prop("isOpen")) + assert.strictEqual(createProgramEnrollmentStub.callCount, 1) + }) }) From 168c95fa97abb22d2560e405385164c6324c9922 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 2 Apr 2026 17:29:14 +0500 Subject: [PATCH 4/7] [pre-commit.ci] pre-commit autoupdate (#3438) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f68fa446cf..cd3dc612f9 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -47,7 +47,7 @@ repos: - "config/keycloak/*" additional_dependencies: ["gibberish-detector"] - repo: https://github.com/astral-sh/ruff-pre-commit - rev: "v0.15.7" + rev: "v0.15.8" hooks: - id: ruff-format - id: ruff @@ -60,7 +60,7 @@ repos: - id: shellcheck args: ["--severity=warning"] - repo: https://github.com/rhysd/actionlint - rev: v1.7.11 + rev: v1.7.12 hooks: - id: actionlint name: actionlint From f8bfa011a31a7fd2d7fb677ffbd6497e3094d33f Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Thu, 2 Apr 2026 09:28:44 -0400 Subject: [PATCH 5/7] chore: add drf-lint pre-commit hook with baseline (#3453) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .pre-commit-config.yaml | 12 ++++++++++++ drf_lint_baseline.json | 31 +++++++++++++++++++++++++++++++ pyproject.toml | 1 + uv.lock | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+) create mode 100644 drf_lint_baseline.json diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index cd3dc612f9..1cdf9e4f68 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -65,3 +65,15 @@ repos: - id: actionlint name: actionlint description: Runs actionlint to lint GitHub Actions workflow files + - repo: local + hooks: + - id: drf-serializer-orm-check + name: DRF Serializer ORM Check + description: "Detects Django ORM queries inside DRF serializer methods (N+1 risk)" + entry: drf-lint + args: [--baseline, drf_lint_baseline.json] + language: python + files: 'serializers\.py$' + additional_dependencies: + - mitol-drf-lint + - "setuptools<82" diff --git a/drf_lint_baseline.json b/drf_lint_baseline.json new file mode 100644 index 0000000000..ae26698361 --- /dev/null +++ b/drf_lint_baseline.json @@ -0,0 +1,31 @@ +[ + "cms/serializers.py:114:16:ORM001", + "cms/serializers.py:153:41:ORM001", + "cms/serializers.py:306:12:ORM001", + "cms/serializers.py:341:20:ORM001", + "cms/serializers.py:352:39:ORM001", + "cms/serializers.py:83:12:ORM001", + "cms/serializers.py:96:12:ORM001", + "flexiblepricing/serializers.py:129:38:ORM001", + "flexiblepricing/serializers.py:132:34:ORM001", + "flexiblepricing/serializers.py:147:34:ORM001", + "flexiblepricing/serializers.py:170:34:ORM001", + "flexiblepricing/serializers.py:173:20:ORM001", + "flexiblepricing/serializers.py:204:30:ORM001", + "flexiblepricing/serializers.py:207:31:ORM001", + "flexiblepricing/serializers.py:212:16:ORM001", + "flexiblepricing/serializers.py:216:16:ORM001", + "hubspot_sync/serializers.py:156:22:ORM002", + "hubspot_sync/serializers.py:157:22:ORM001", + "hubspot_sync/serializers.py:169:25:ORM002", + "hubspot_sync/serializers.py:172:33:ORM002", + "hubspot_sync/serializers.py:298:33:ORM001", + "hubspot_sync/serializers.py:309:36:ORM001", + "hubspot_sync/serializers.py:57:22:ORM002", + "hubspot_sync/serializers.py:68:27:ORM002", + "users/serializers.py:208:16:ORM001", + "users/serializers.py:254:20:ORM001", + "users/serializers.py:301:19:ORM001", + "users/serializers.py:431:13:ORM001", + "users/serializers.py:467:11:ORM001" +] diff --git a/pyproject.toml b/pyproject.toml index a35541aa46..b2dd16f35c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -111,6 +111,7 @@ dev = [ "semantic-version>=2.10.0,<3", "wagtail-factories>=4.2,<4.3", "pytest-xdist[psutil]>=3.6.1,<4", + "mitol-drf-lint>=2025.3.27", ] [tool.uv] diff --git a/uv.lock b/uv.lock index 297c35c6ff..2f9e61bea8 100644 --- a/uv.lock +++ b/uv.lock @@ -1598,6 +1598,25 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/60/fe/31f76f5cb2579bdda208aa257ce5482653f22ab1bad3e128fe2f803fa2f1/laces-0.1.2-py3-none-any.whl", hash = "sha256:980cdaf9a31e883a2b8198132e2388931a4eb8814f5bfa5d8bba13ff9f657b7c", size = 22462, upload-time = "2025-01-14T04:37:30.636Z" }, ] +[[package]] +name = "libcst" +version = "1.8.6" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pyyaml" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/de/cd/337df968b38d94c5aabd3e1b10630f047a2b345f6e1d4456bd9fe7417537/libcst-1.8.6.tar.gz", hash = "sha256:f729c37c9317126da9475bdd06a7208eb52fcbd180a6341648b45a56b4ba708b", size = 891354, upload-time = "2025-11-03T22:33:30.621Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/dc/15/95c2ecadc0fb4af8a7057ac2012a4c0ad5921b9ef1ace6c20006b56d3b5f/libcst-1.8.6-cp311-cp311-macosx_10_12_x86_64.whl", hash = "sha256:3649a813660fbffd7bc24d3f810b1f75ac98bd40d9d6f56d1f0ee38579021073", size = 2211289, upload-time = "2025-11-03T22:32:04.673Z" }, + { url = "https://files.pythonhosted.org/packages/80/c3/7e1107acd5ed15cf60cc07c7bb64498a33042dc4821874aea3ec4942f3cd/libcst-1.8.6-cp311-cp311-macosx_11_0_arm64.whl", hash = "sha256:0cbe17067055829607c5ba4afa46bfa4d0dd554c0b5a583546e690b7367a29b6", size = 2092927, upload-time = "2025-11-03T22:32:06.209Z" }, + { url = "https://files.pythonhosted.org/packages/c1/ff/0d2be87f67e2841a4a37d35505e74b65991d30693295c46fc0380ace0454/libcst-1.8.6-cp311-cp311-manylinux_2_28_aarch64.whl", hash = "sha256:59a7e388c57d21d63722018978a8ddba7b176e3a99bd34b9b84a576ed53f2978", size = 2237002, upload-time = "2025-11-03T22:32:07.559Z" }, + { url = "https://files.pythonhosted.org/packages/69/99/8c4a1b35c7894ccd7d33eae01ac8967122f43da41325223181ca7e4738fe/libcst-1.8.6-cp311-cp311-manylinux_2_28_x86_64.whl", hash = "sha256:b6c1248cc62952a3a005792b10cdef2a4e130847be9c74f33a7d617486f7e532", size = 2301048, upload-time = "2025-11-03T22:32:08.869Z" }, + { url = "https://files.pythonhosted.org/packages/9b/8b/d1aa811eacf936cccfb386ae0585aa530ea1221ccf528d67144e041f5915/libcst-1.8.6-cp311-cp311-musllinux_1_2_aarch64.whl", hash = "sha256:6421a930b028c5ef4a943b32a5a78b7f1bf15138214525a2088f11acbb7d3d64", size = 2300675, upload-time = "2025-11-03T22:32:10.579Z" }, + { url = "https://files.pythonhosted.org/packages/c6/6b/7b65cd41f25a10c1fef2389ddc5c2b2cc23dc4d648083fa3e1aa7e0eeac2/libcst-1.8.6-cp311-cp311-musllinux_1_2_x86_64.whl", hash = "sha256:6d8b67874f2188399a71a71731e1ba2d1a2c3173b7565d1cc7ffb32e8fbaba5b", size = 2407934, upload-time = "2025-11-03T22:32:11.856Z" }, + { url = "https://files.pythonhosted.org/packages/c5/8b/401cfff374bb3b785adfad78f05225225767ee190997176b2a9da9ed9460/libcst-1.8.6-cp311-cp311-win_amd64.whl", hash = "sha256:b0d8c364c44ae343937f474b2e492c1040df96d94530377c2f9263fb77096e4f", size = 2119247, upload-time = "2025-11-03T22:32:13.279Z" }, + { url = "https://files.pythonhosted.org/packages/f1/17/085f59eaa044b6ff6bc42148a5449df2b7f0ba567307de7782fe85c39ee2/libcst-1.8.6-cp311-cp311-win_arm64.whl", hash = "sha256:5dcaaebc835dfe5755bc85f9b186fb7e2895dda78e805e577fef1011d51d5a5c", size = 2001774, upload-time = "2025-11-03T22:32:14.647Z" }, +] + [[package]] name = "lxml" version = "6.0.2" @@ -1901,6 +1920,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/71/5e/1889d00cf93919d6b34f4638b35a930942cfa95b64c3d005d22a3ea0dd5a/mitol_django_scim-2025.7.29-py3-none-any.whl", hash = "sha256:aa10c93756d15a2fd8e8f747095e53d0c9aa7510d6fad8ce559db5ad67acfb87", size = 21390, upload-time = "2025-07-29T16:01:41.267Z" }, ] +[[package]] +name = "mitol-drf-lint" +version = "2025.3.27" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "libcst" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/86/25/5dde18a64b612a1e0c68d0617436745d91c1499573c6c80d32f92344ef77/mitol_drf_lint-2025.3.27.tar.gz", hash = "sha256:1e6c872bb4bd198c262be045cabe522a00ad83d9ed7fdd53c8049b9bb2138787", size = 6356, upload-time = "2026-04-01T19:33:37.607Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/60/72/76c9a413a7376c4be572edf81623d5ce950ad5de5c65713675c6b2df33ff/mitol_drf_lint-2025.3.27-py3-none-any.whl", hash = "sha256:76467eb7b6250093ccf631f87ebc3265155997e486be49d11ee3a7b20bad2fd1", size = 10843, upload-time = "2026-04-01T19:33:36.549Z" }, +] + [[package]] name = "mitxonline" version = "0.69.1" @@ -1995,6 +2026,7 @@ dev = [ { name = "flaky" }, { name = "freezegun" }, { name = "ipdb" }, + { name = "mitol-drf-lint" }, { name = "pdbpp" }, { name = "pre-commit" }, { name = "pytest-cov" }, @@ -2103,6 +2135,7 @@ dev = [ { name = "flaky", specifier = ">=3.7.0,<4" }, { name = "freezegun", specifier = ">=1.2,<2" }, { name = "ipdb", specifier = ">=0.13.13,<0.14" }, + { name = "mitol-drf-lint", specifier = ">=2025.3.27" }, { name = "pdbpp", specifier = ">=0.11.6,<0.12" }, { name = "pre-commit", specifier = ">=4.0.0,<5" }, { name = "pytest-cov", specifier = ">=7.0.0,<8" }, From ca2c78717162d5ff4d5e383daa3ee7214bd5a6a2 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Thu, 2 Apr 2026 11:04:19 -0500 Subject: [PATCH 6/7] Add management command to check/repair B2B program enrollments (#3449) --- .../repair_b2b_program_enrollments.py | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 b2b/management/commands/repair_b2b_program_enrollments.py diff --git a/b2b/management/commands/repair_b2b_program_enrollments.py b/b2b/management/commands/repair_b2b_program_enrollments.py new file mode 100644 index 0000000000..020a242883 --- /dev/null +++ b/b2b/management/commands/repair_b2b_program_enrollments.py @@ -0,0 +1,81 @@ +"""Check and repair B2B program enrollments.""" + +import logging + +from django.core.management import BaseCommand +from django.db.models import Q + +from b2b.models import ContractProgramItem +from courses.constants import ENROLL_CHANGE_STATUS_UNENROLLED +from courses.models import Program, ProgramEnrollment + +log = logging.getLogger(__name__) + + +class Command(BaseCommand): + """Check and repair B2B program enrollments.""" + + help = """Check and repair B2B program enrollments. + +Unenroll users from B2B programs that belong to contracts that the user doesn't have an attachment for.""" + + def add_arguments(self, parser): + """Add command line arguments.""" + + parser.add_argument( + "--user", type=str, help="Check specified username or email." + ) + parser.add_argument( + "--commit", + action="store_true", + default=False, + help="Commit changes. Otherwise, this will produce output but won't make any changes.", + ) + + def handle(self, *args, **kwargs): # noqa: ARG002 + """Perform check and repair operation.""" + + specific_user = kwargs.pop("user", None) + commit = kwargs.pop("commit", False) + + if not commit: + self.stdout.write( + self.style.WARNING("Commit flag not set - no changes will be made.") + ) + + b2b_programs = Program.objects.filter( + b2b_only=True, + contract_memberships__isnull=False, + ).distinct() + + for program in b2b_programs: + program_contract_ids = ContractProgramItem.objects.filter( + program=program + ).values_list("contract_id", flat=True) + + extraneous_enrollments = ( + ProgramEnrollment.objects.filter(program=program) + .exclude(user__b2b_contracts__in=program_contract_ids) + .select_related("user", "program") + .distinct() + ) + + if specific_user: + extraneous_enrollments = extraneous_enrollments.filter( + Q(user__email=specific_user) | Q(user__username=specific_user) + ) + + count = extraneous_enrollments.count() + if count > 0: + self.stdout.write( + f"Program {program.readable_id}: unenrolling {count} user(s):" + ) + + for enrollment in extraneous_enrollments: + self.stdout.write(f"\t{enrollment.user.email}") + if commit: + enrollment.deactivate_and_save( + ENROLL_CHANGE_STATUS_UNENROLLED, no_user=True + ) + + self.stdout.write("\n") From f842362b974ee7564b79b22c70fa2d6dfcb44bb0 Mon Sep 17 00:00:00 2001 From: Doof Date: Thu, 2 Apr 2026 16:04:56 +0000 Subject: [PATCH 7/7] Release 1.144.5 --- RELEASE.rst | 10 ++++++++++ main/settings.py | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/RELEASE.rst b/RELEASE.rst index b1c7ca8bfb..1b0e148e35 100644 --- a/RELEASE.rst +++ b/RELEASE.rst @@ -1,6 +1,16 @@ Release Notes ============= +Version 1.144.5 +--------------- + +- Add management command to check/repair B2B program enrollments (#3449) +- chore: add drf-lint pre-commit hook with baseline (#3453) +- [pre-commit.ci] pre-commit autoupdate (#3438) +- Add test to the program enrollment dialog (#3445) +- fix: fall back to audit enrollment for non-upgradable runs in verified program enrollment API (#3451) +- Refactor program cert generation to support new cert requirements (#3439) + Version 1.144.4 (Released April 02, 2026) --------------- diff --git a/main/settings.py b/main/settings.py index c65a8c1ffe..a5f77f01be 100644 --- a/main/settings.py +++ b/main/settings.py @@ -37,7 +37,7 @@ from main.sentry import init_sentry from openapi.settings_spectacular import open_spectacular_settings -VERSION = "1.144.4" +VERSION = "1.144.5" log = logging.getLogger()