Merge pull request #24922 from edx/tuchfarber/fix_var_reuse

Fix variable reuse bug in program enrollment check

openedx/core/djangoapps/demographics/api/status.py

@@ -3,7 +3,7 @@ Python API for Demographics Status
 """
 
 from openedx.features.enterprise_support.utils import is_enterprise_learner
-from openedx.core.djangoapps.programs.utils import is_user_enrolled_in_program_type
+from openedx.core.djangoapps.programs.api import is_user_enrolled_in_program_type
 from openedx.core.djangoapps.demographics.models import UserDemographics
 
 

openedx/core/djangoapps/programs/api.py

@@ -5,7 +5,8 @@ Python APIs exposed by the Programs app to other in-process apps.
 from .utils import is_user_enrolled_in_program_type as _is_user_enrolled_in_program_type
 
 
-def is_user_enrolled_in_program_type(user, program_type_slug, paid_modes=False, enrollments=None, entitlements=None):
+# `paid_modes` is deprecated in favor of `paid_modes_only`, but we need to expand and contract to prevent breakage.
+def is_user_enrolled_in_program_type(user, program_type_slug, paid_modes=False, paid_modes_only=False, enrollments=None, entitlements=None):
     """
     This method will look at the learners Enrollments and Entitlements to determine
     if a learner is enrolled in a Program of the given type.
@@ -16,7 +17,7 @@ def is_user_enrolled_in_program_type(user, program_type_slug, paid_modes=False,
     Arguments:
         user (User): The user we are looking for.
         program_type_slug (str): The slug of the Program type we are looking for.
-        paid_modes (bool): Request if the user is enrolled in a Program in a paid mode, False by default.
+        paid_modes_only (bool): Request if the user is enrolled in a Program in a paid mode, False by default.
         enrollments (List[Dict]): Takes a serialized list of CourseEnrollments linked to the user
         entitlements (List[CourseEntitlement]): Take a list of CourseEntitlement objects linked to the user
 
@@ -27,4 +28,10 @@ def is_user_enrolled_in_program_type(user, program_type_slug, paid_modes=False,
         bool: True is the user is enrolled in programs of the requested type
     """
 
-    return _is_user_enrolled_in_program_type(user, program_type_slug, paid_modes, enrollments, entitlements)
+    return _is_user_enrolled_in_program_type(
+        user,
+        program_type_slug,
+        paid_modes_only=(paid_modes or paid_modes_only),
+        enrollments=enrollments,
+        entitlements=entitlements
+    )

openedx/core/djangoapps/programs/tests/test_utils.py

@@ -4,6 +4,7 @@
 import datetime
 import json
 import uuid
+from collections import namedtuple
 from copy import deepcopy
 
 import ddt
@@ -1603,7 +1604,40 @@ class TestProgramEnrollment(SharedModuleStoreTestCase):
         CourseEnrollmentFactory.create(user=self.user, course_id=self.course_run.id, mode=CourseMode.AUDIT)
         mock_get_programs_by_type.return_value = [self.program]
         self.assertFalse(
-            is_user_enrolled_in_program_type(user=self.user, program_type_slug=self.MICROBACHELORS, paid_modes=True)
+            is_user_enrolled_in_program_type(user=self.user, program_type_slug=self.MICROBACHELORS, paid_modes_only=True)
+        )
+
+    # NEW CODE HERE
+    @mock.patch('openedx.core.djangoapps.programs.utils.get_paid_modes_for_course')
+    def test_user_enrolled_in_paid_only_with_no_matching_paid_course_modes(self, mock_get_paid_modes_for_course, mock_get_programs_by_type):
+        second_program = ProgramFactory(type=self.MICROBACHELORS)
+        second_catalog_course_run = second_program['courses'][0]['course_runs'][0]
+        second_course_key = CourseKey.from_string(second_catalog_course_run['key'])
+        second_course_run = ModuleStoreCourseFactory.create(
+            org=second_course_key.org,
+            number=second_course_key.course,
+            run=second_course_key.run,
+            modulestore=self.store,
+        )
+        CourseEnrollmentFactory.create(user=self.user, course_id=self.course_run.id, mode=CourseMode.AUDIT)
+        CourseEnrollmentFactory.create(user=self.user, course_id=second_course_run.id, mode=CourseMode.AUDIT)
+
+        mock_get_programs_by_type.return_value = [self.program, second_program]
+
+        # While most of a programs courses would likely come with a paid mode, if the course in question is now expired,
+        # then get_paid_modes_for_course would return an empty list. Even with no paid modes, if we request paid modes only
+        # we should return False
+        mock_get_paid_modes_for_course.return_value = []
+        # raise Exception((mock_get_programs_by_type, mock_get_paid_modes_for_course))
+        self.assertFalse(
+            is_user_enrolled_in_program_type(user=self.user, program_type_slug=self.MICROBACHELORS, paid_modes_only=True)
+        )
+
+        # We should continue to return false even if they do contain paid modes
+        Mode = namedtuple('Mode', ['slug'])
+        # mock_get_paid_modes_for_course.return_value = [Mode(CourseMode.VERIFIED)]
+        self.assertFalse(
+            is_user_enrolled_in_program_type(user=self.user, program_type_slug=self.MICROBACHELORS, paid_modes_only=True)
         )
 
     def test_user_with_entitlement_no_enrollment(self, mock_get_programs_by_type):

openedx/core/djangoapps/programs/utils.py

@@ -902,7 +902,7 @@ class ProgramMarketingDataExtender(ProgramDataExtender):
                     self.instructors.append(instructor)
 
 
-def is_user_enrolled_in_program_type(user, program_type_slug, paid_modes=False, enrollments=None, entitlements=None):
+def is_user_enrolled_in_program_type(user, program_type_slug, paid_modes_only=False, enrollments=None, entitlements=None):
     """
     This method will look at the learners Enrollments and Entitlements to determine
     if a learner is enrolled in a Program of the given type.
@@ -913,7 +913,7 @@ def is_user_enrolled_in_program_type(user, program_type_slug, paid_modes=False,
     Arguments:
         user (User): The user we are looking for.
         program_type_slug (str): The slug of the Program type we are looking for.
-        paid_modes (bool): Request if the user is enrolled in a Program in a paid mode, False by default.
+        paid_modes_only (bool): Request if the user is enrolled in a Program in a paid mode, False by default.
         enrollments (List[Dict]): Takes a serialized list of CourseEnrollments linked to the user
         entitlements (List[CourseEntitlement]): Take a list of CourseEntitlement objects linked to the user
 
@@ -923,25 +923,6 @@ def is_user_enrolled_in_program_type(user, program_type_slug, paid_modes=False,
     Returns:
         bool: True is the user is enrolled in programs of the requested type
     """
-    # NOTE: Used to debug production issue on 9/4/20.
-    if getattr(settings, "DEBUG_PROGRAM_ENROLLMENT_TYPE", False):
-        log.info(
-            """
-                is_user_enrolled_in_program_type called with the following parameters:
-                    User: {},
-                    program_type_slug: {},
-                    paid_modes: {},
-                    enrollments: {},
-                    entitlements: {}
-            """.format(
-                user.id,
-                program_type_slug,
-                str(paid_modes),
-                str(enrollments),
-                str(entitlements)
-            )
-        )
-
     course_runs = set()
     course_uuids = set()
     programs = get_programs_by_type(Site.objects.get_current(), program_type_slug)
@@ -959,39 +940,16 @@ def is_user_enrolled_in_program_type(user, program_type_slug, paid_modes=False,
     student_entitlements = entitlements if entitlements is not None else get_active_entitlement_list_for_user(user)
     for entitlement in student_entitlements:
         if str(entitlement.course_uuid) in course_uuids:
-            if getattr(settings, "DEBUG_PROGRAM_ENROLLMENT_TYPE", False):
-                # NOTE: Used to debug production issue on 9/4/20.
-                log.info("User {} has an entitlement for course {} which is in a {} program".format(
-                    str(user.id),
-                    str(entitlement.course_uuid),
-                    str(program_type_slug)
-                ))
             return True
 
     student_enrollments = enrollments if enrollments is not None else get_enrollments(user.username)
     for enrollment in student_enrollments:
         course_run_id = enrollment['course_details']['course_id']
-        if paid_modes:
+        if paid_modes_only:
             course_run_key = CourseKey.from_string(course_run_id)
             paid_modes = [mode.slug for mode in get_paid_modes_for_course(course_run_key)]
             if enrollment['mode'] in paid_modes and course_run_id in course_runs:
-                if getattr(settings, "DEBUG_PROGRAM_ENROLLMENT_TYPE", False):
-                    # NOTE: Used to debug production issue on 9/4/20.
-                    log.info("User {} has an enrollment mode of {} for course {} which is in a {} program and has {} paid modes".format(
-                        str(user.id),
-                        str(enrollment['mode']),
-                        str(course_run_id),
-                        str(program_type_slug),
-                        str(paid_modes)
-                    ))
                 return True
         elif course_run_id in course_runs:
-            if getattr(settings, "DEBUG_PROGRAM_ENROLLMENT_TYPE", False):
-                # NOTE: Used to debug production issue on 9/4/20.
-                log.info("User {} is enrolled in course {} which is in a {} program".format(
-                    str(user.id),
-                    str(course_run_id),
-                    str(program_type_slug)
-                ))
             return True
     return False