Separate the date and link logic out of VerifiedUpgradeDeadlineBlock, so that it…

Separate the date and link logic out of VerifiedUpgradeDeadlineBlock, so that it can be called directly with prefetched data for check_and_get_upgrade_link_and_date

common/djangoapps/student/tests/test_models.py

@@ -132,7 +132,6 @@ class CourseEnrollmentTests(SharedModuleStoreTestCase):
         self.assertEqual(Schedule.objects.all().count(), 0)
         self.assertEqual(enrollment.upgrade_deadline, course_mode.expiration_datetime)
 
-
     @skip_unless_lms
     # NOTE: We mute the post_save signal to prevent Schedules from being created for new enrollments
     @factory.django.mute_signals(signals.post_save)

lms/djangoapps/ccx/tests/test_field_override_performance.py

@@ -237,18 +237,18 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase):
         #     # of sql queries to default,
         #     # of mongo queries,
         # )
-        ('no_overrides', 1, True, False): (19, 1),
-        ('no_overrides', 2, True, False): (19, 1),
-        ('no_overrides', 3, True, False): (19, 1),
-        ('ccx', 1, True, False): (19, 1),
-        ('ccx', 2, True, False): (19, 1),
-        ('ccx', 3, True, False): (19, 1),
-        ('no_overrides', 1, False, False): (19, 1),
-        ('no_overrides', 2, False, False): (19, 1),
-        ('no_overrides', 3, False, False): (19, 1),
-        ('ccx', 1, False, False): (19, 1),
-        ('ccx', 2, False, False): (19, 1),
-        ('ccx', 3, False, False): (19, 1),
+        ('no_overrides', 1, True, False): (18, 1),
+        ('no_overrides', 2, True, False): (18, 1),
+        ('no_overrides', 3, True, False): (18, 1),
+        ('ccx', 1, True, False): (18, 1),
+        ('ccx', 2, True, False): (18, 1),
+        ('ccx', 3, True, False): (18, 1),
+        ('no_overrides', 1, False, False): (18, 1),
+        ('no_overrides', 2, False, False): (18, 1),
+        ('no_overrides', 3, False, False): (18, 1),
+        ('ccx', 1, False, False): (18, 1),
+        ('ccx', 2, False, False): (18, 1),
+        ('ccx', 3, False, False): (18, 1),
     }
 
 
@@ -260,19 +260,19 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
     __test__ = True
 
     TEST_DATA = {
-        ('no_overrides', 1, True, False): (19, 3),
-        ('no_overrides', 2, True, False): (19, 3),
-        ('no_overrides', 3, True, False): (19, 3),
-        ('ccx', 1, True, False): (19, 3),
-        ('ccx', 2, True, False): (19, 3),
-        ('ccx', 3, True, False): (19, 3),
-        ('ccx', 1, True, True): (20, 3),
-        ('ccx', 2, True, True): (20, 3),
-        ('ccx', 3, True, True): (20, 3),
-        ('no_overrides', 1, False, False): (19, 3),
-        ('no_overrides', 2, False, False): (19, 3),
-        ('no_overrides', 3, False, False): (19, 3),
-        ('ccx', 1, False, False): (19, 3),
-        ('ccx', 2, False, False): (19, 3),
-        ('ccx', 3, False, False): (19, 3),
+        ('no_overrides', 1, True, False): (18, 3),
+        ('no_overrides', 2, True, False): (18, 3),
+        ('no_overrides', 3, True, False): (18, 3),
+        ('ccx', 1, True, False): (18, 3),
+        ('ccx', 2, True, False): (18, 3),
+        ('ccx', 3, True, False): (18, 3),
+        ('ccx', 1, True, True): (19, 3),
+        ('ccx', 2, True, True): (19, 3),
+        ('ccx', 3, True, True): (19, 3),
+        ('no_overrides', 1, False, False): (18, 3),
+        ('no_overrides', 2, False, False): (18, 3),
+        ('no_overrides', 3, False, False): (18, 3),
+        ('ccx', 1, False, False): (18, 3),
+        ('ccx', 2, False, False): (18, 3),
+        ('ccx', 3, False, False): (18, 3),
     }

lms/djangoapps/ccx/tests/test_views.py

@@ -36,6 +36,7 @@ from lms.djangoapps.ccx.utils import ccx_course, is_email
 from lms.djangoapps.ccx.views import get_date
 from lms.djangoapps.grades.tasks import compute_all_grades_for_course
 from lms.djangoapps.instructor.access import allow_access, list_with_level
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from request_cache.middleware import RequestCache
 from student.models import CourseEnrollment, CourseEnrollmentAllowed
 from student.roles import CourseCcxCoachRole, CourseInstructorRole, CourseStaffRole
@@ -1061,6 +1062,7 @@ class TestCCXGrades(FieldOverrideTestMixin, SharedModuleStoreTestCase, LoginEnro
     def setUpClass(cls):
         super(TestCCXGrades, cls).setUpClass()
         cls._course = course = CourseFactory.create(enable_ccx=True)
+        CourseOverview.load_from_module_store(course.id)
 
         # Create a course outline
         cls.mooc_start = start = datetime.datetime(
@@ -1122,6 +1124,7 @@ class TestCCXGrades(FieldOverrideTestMixin, SharedModuleStoreTestCase, LoginEnro
         # which emulates how a student would get access.
         self.ccx_key = CCXLocator.from_course_locator(self._course.id, unicode(ccx.id))
         self.course = get_course_by_id(self.ccx_key, depth=None)
+        CourseOverview.load_from_module_store(self.course.id)
         setup_students_and_grades(self)
         self.client.login(username=coach.username, password="test")
         self.addCleanup(RequestCache.clear_request_cache)

lms/djangoapps/courseware/date_summary.py

@@ -20,6 +20,7 @@ from course_modes.models import CourseMode, get_cosmetic_verified_display_price
 from lms.djangoapps.commerce.utils import EcommerceService
 from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification, VerificationDeadline
 from openedx.core.djangoapps.certificates.api import can_show_certificate_available_date_field
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.core.djangolib.markup import HTML, Text
 from openedx.features.course_experience import CourseHomeMessages, UPGRADE_DEADLINE_MESSAGE
 from student.models import CourseEnrollment
@@ -380,6 +381,67 @@ class CertificateAvailableDate(DateSummary):
             )
 
 
+def verified_upgrade_deadline_link(user, course=None, course_id=None):
+    """
+    Format the correct verified upgrade link for the specified ``user``
+    in a course.
+
+    One of ``course`` or ``course_id`` must be supplied. If both are specified,
+    ``course`` will take priority.
+
+    Arguments:
+        user (:class:`~django.contrib.auth.models.User`): The user to display
+            the link for.
+        course (:class:`.CourseOverview`): The course to render a link for.
+        course_id (:class:`.CourseKey`): The course_id of the course to render for.
+
+    Returns:
+        The formatted link to that will allow the user to upgrade to verified
+        in this course.
+    """
+    if course is not None:
+        course_id = course.id
+
+    ecommerce_service = EcommerceService()
+    if ecommerce_service.is_enabled(user):
+        if course is not None and isinstance(course, CourseOverview):
+            course_mode = [
+                mode
+                for mode in course.modes
+                if mode.slug == CourseMode.VERIFIED
+            ]
+        else:
+            course_mode = CourseMode.objects.get(
+                course_id=course_id, mode_slug=CourseMode.VERIFIED
+            )
+        return ecommerce_service.get_checkout_page_url(course_mode.sku)
+    return reverse('verify_student_upgrade_and_verify', args=(course_id,))
+
+
+def verified_upgrade_link_is_valid(enrollment=None):
+    """
+    Return whether this enrollment can be upgraded.
+
+    Arguments:
+        enrollment (:class:`.CourseEnrollment`): The enrollment under consideration.
+            If None, then the enrollment is considered to be upgradeable.
+    """
+    # Return `true` if user is not enrolled in course
+    if enrollment is None:
+        return False
+
+    upgrade_deadline = enrollment.upgrade_deadline
+
+    if upgrade_deadline is None:
+        return False
+
+    if datetime.datetime.now(utc).date() > upgrade_deadline.date():
+        return False
+
+    # Show the summary if user enrollment is in which allow user to upsell
+    return enrollment.is_active and enrollment.mode in CourseMode.UPSELL_TO_VERIFIED_MODES
+
+
 class VerifiedUpgradeDeadlineDate(DateSummary):
     """
     Displays the date before which learners must upgrade to the
@@ -395,7 +457,7 @@ class VerifiedUpgradeDeadlineDate(DateSummary):
 
     @property
     def link(self):
-        return EcommerceService().upgrade_url(self.user, self.course_id)
+        return verified_upgrade_deadline_link(self.user, self.course, self.course_id)
 
     @cached_property
     def enrollment(self):
@@ -413,19 +475,7 @@ class VerifiedUpgradeDeadlineDate(DateSummary):
         if not is_enabled:
             return False
 
-        enrollment_mode = None
-        is_active = None
-
-        if self.enrollment:
-            enrollment_mode = self.enrollment.mode
-            is_active = self.enrollment.is_active
-
-        # Return `true` if user is not enrolled in course
-        if enrollment_mode is None and is_active is None:
-            return True
-
-        # Show the summary if user enrollment is in which allow user to upsell
-        return is_active and enrollment_mode in CourseMode.UPSELL_TO_VERIFIED_MODES
+        return verified_upgrade_link_is_valid(self.enrollment)
 
     @lazy
     def date(self):

lms/djangoapps/courseware/tests/test_views.py

@@ -213,8 +213,8 @@ class IndexQueryTestCase(ModuleStoreTestCase):
     NUM_PROBLEMS = 20
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 10, 147),
-        (ModuleStoreEnum.Type.split, 4, 147),
+        (ModuleStoreEnum.Type.mongo, 10, 146),
+        (ModuleStoreEnum.Type.split, 4, 146),
     )
     @ddt.unpack
     def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count):
@@ -1047,6 +1047,7 @@ class BaseDueDateTests(ModuleStoreTestCase):
         course = modulestore().get_course(course.id)
         self.assertIsNotNone(course.get_children()[0].get_children()[0].due)
         CourseEnrollmentFactory(user=self.user, course_id=course.id)
+        CourseOverview.load_from_module_store(course.id)
         return course
 
     def setUp(self):
@@ -1456,13 +1457,13 @@ class ProgressPageTests(ProgressPageBaseTests):
         """Test that query counts remain the same for self-paced and instructor-paced courses."""
         SelfPacedConfiguration(enabled=self_paced_enabled).save()
         self.setup_course(self_paced=self_paced)
-        with self.assertNumQueries(36, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST), check_mongo_calls(1):
+        with self.assertNumQueries(35, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST), check_mongo_calls(1):
             self._get_progress_page()
 
     @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
     @ddt.data(
-        (False, 43, 27),
-        (True, 36, 23)
+        (False, 42, 26),
+        (True, 35, 22)
     )
     @ddt.unpack
     def test_progress_queries(self, enable_waffle, initial, subsequent):
@@ -2213,6 +2214,7 @@ class TestIndexView(ModuleStoreTestCase):
                 state=json.dumps({'state': unicode(item.scope_ids.usage_id)})
             )
 
+        CourseOverview.load_from_module_store(course.id)
         CourseEnrollmentFactory(user=user, course_id=course.id)
 
         self.assertTrue(self.client.login(username=user.username, password='test'))
@@ -2241,6 +2243,7 @@ class TestIndexView(ModuleStoreTestCase):
             vertical = ItemFactory.create(parent=section, category='vertical', display_name="Vertical")
             ItemFactory.create(parent=vertical, category='id_checker', display_name="ID Checker")
 
+        CourseOverview.load_from_module_store(course.id)
         CourseEnrollmentFactory(user=user, course_id=course.id)
 
         self.assertTrue(self.client.login(username=user.username, password='test'))
@@ -2281,6 +2284,8 @@ class TestIndexViewWithVerticalPositions(ModuleStoreTestCase):
             ItemFactory.create(parent=self.section, category='vertical', display_name="Vertical2")
             ItemFactory.create(parent=self.section, category='vertical', display_name="Vertical3")
 
+        CourseOverview.load_from_module_store(self.course.id)
+
         self.client.login(username=self.user, password='test')
         CourseEnrollmentFactory(user=self.user, course_id=self.course.id)
 
@@ -2505,6 +2510,7 @@ class EnterpriseConsentTestCase(EnterpriseTestConsentRequired, ModuleStoreTestCa
         self.user = UserFactory.create()
         self.assertTrue(self.client.login(username=self.user.username, password='test'))
         self.course = CourseFactory.create()
+        CourseOverview.load_from_module_store(self.course.id)
         CourseEnrollmentFactory(user=self.user, course_id=self.course.id)
 
     def test_consent_required(self):

lms/djangoapps/courseware/testutils.py

@@ -12,6 +12,7 @@ from mock import patch
 
 from lms.djangoapps.courseware.field_overrides import OverrideModulestoreFieldData
 from lms.djangoapps.courseware.url_helpers import get_redirect_url
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
@@ -104,6 +105,7 @@ class RenderXBlockTestMixin(object):
                 category='html',
                 data="<p>Test HTML Content<p>"
             )
+        CourseOverview.load_from_module_store(self.course.id)
 
         # block_name_to_be_tested can be `html_block` or `vertical_block`.
         # These attributes help ensure the positive and negative tests are in sync.

lms/djangoapps/experiments/utils.py

@@ -3,21 +3,41 @@ from course_modes.models import (
     get_cosmetic_verified_display_price
 )
 from courseware.date_summary import (
-    VerifiedUpgradeDeadlineDate
+    verified_upgrade_deadline_link, verified_upgrade_link_is_valid
 )
 
 
-def check_and_get_upgrade_link(user, course_id):
+def check_and_get_upgrade_link_and_date(user, enrollment=None, course=None):
     """
     For an authenticated user, return a link to allow them to upgrade
     in the specified course.
     """
-    if user.is_authenticated():
-        upgrade_data = VerifiedUpgradeDeadlineDate(None, user, course_id=course_id)
-        if upgrade_data.is_enabled:
-            return upgrade_data
+    if enrollment is None and course is None:
+        raise ValueError("Must specify either an enrollment or a course")
 
-    return None
+    if enrollment:
+        if course is None:
+            course = enrollment.course
+        elif enrollment.course_id != course.id:
+            raise ValueError("{} refers to a different course than {} which was supplied".format(
+                enrollment, course
+            ))
+
+        if enrollment.user_id != user.id:
+            raise ValueError("{} refers to a different user than {} which was supplied".format(
+                enrollment, user
+            ))
+
+    if enrollment is None:
+        enrollment = CourseEnrollment.get_enrollment(user, course.id)
+
+    if user.is_authenticated() and verified_upgrade_link_is_valid(enrollment):
+        return (
+            verified_upgrade_deadline_link(user, course),
+            enrollment.upgrade_deadline
+        )
+
+    return (None, None)
 
 
 def get_experiment_user_metadata_context(course, user):
@@ -26,23 +46,26 @@ def get_experiment_user_metadata_context(course, user):
     """
     enrollment_mode = None
     enrollment_time = None
+    enrollment = None
     try:
-        enrollment = CourseEnrollment.objects.get(user_id=user.id, course_id=course.id)
+        enrollment = CourseEnrollment.objects.select_related(
+            'course'
+        ).get(user_id=user.id, course_id=course.id)
         if enrollment.is_active:
             enrollment_mode = enrollment.mode
             enrollment_time = enrollment.created
     except CourseEnrollment.DoesNotExist:
         pass  # Not enrolled, used the default None values
 
-    upgrade_data = check_and_get_upgrade_link(user, course.id)
+    upgrade_link, upgrade_date = check_and_get_upgrade_link_and_date(user, enrollment, course)
 
     return {
-        'upgrade_link': upgrade_data and upgrade_data.link,
+        'upgrade_link': upgrade_link,
         'upgrade_price': unicode(get_cosmetic_verified_display_price(course)),
         'enrollment_mode': enrollment_mode,
         'enrollment_time': enrollment_time,
         'pacing_type': 'self_paced' if course.self_paced else 'instructor_paced',
-        'upgrade_deadline': upgrade_data and upgrade_data.date,
+        'upgrade_deadline': upgrade_date,
         'course_key': course.id,
         'course_start': course.start,
         'course_end': course.end,

openedx/core/djangoapps/schedules/management/commands/tests/test_send_recurring_nudge.py

@@ -22,6 +22,25 @@ from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_un
 from student.tests.factories import UserFactory
 
 
+# Populating recurring nudge emails requires three queries
+# 1a) Find all users whose first enrollment during a day was in the specified hour window
+# 1b) All schedules for all enrollments for that day for users in that window (with 1a as a subquery)
+# 2) Check whether debugging is enabled
+CONST_QUERIES = 2
+
+# 1) Prefetch all course modes for those schedules
+# 2) (When not cached) load the DynamicUpgradeDeadlineConfiguration
+SCHEDULE_QUERIES = 2
+
+# 1) Load the current django site
+# 2) Load the ScheduleConfig
+SEND_QUERIES = 2
+
+# 1) (When not cached) load the CourseDynamicUpgradeDeadlineConfiguration
+# 2) Load the VERIFIED course mode for the course
+PER_COURSE_QUERIES = 2
+
+
 @ddt.ddt
 @skip_unless_lms
 @skipUnless('openedx.core.djangoapps.schedules.apps.SchedulesConfig' in settings.INSTALLED_APPS,
@@ -29,6 +48,8 @@ from student.tests.factories import UserFactory
 class TestSendRecurringNudge(CacheIsolationTestCase):
     # pylint: disable=protected-access
 
+    ENABLED_CACHES = ['default']
+
     def setUp(self):
         super(TestSendRecurringNudge, self).setUp()
 
@@ -233,7 +254,7 @@ class TestSendRecurringNudge(CacheIsolationTestCase):
         patch_channels(self, [mock_channel])
 
         sent_messages = []
-        
+
         templates_override = deepcopy(settings.TEMPLATES)
         templates_override[0]['OPTIONS']['string_if_invalid'] = "TEMPLATE WARNING - MISSING VARIABLE [%s]"
         with self.settings(TEMPLATES=templates_override):

openedx/core/djangoapps/schedules/tasks.py

@@ -18,7 +18,7 @@ from edx_ace.message import Message
 from edx_ace.recipient import Recipient
 from edx_ace.utils.date import deserialize
 from opaque_keys.edx.keys import CourseKey
-from lms.djangoapps.experiments.utils import check_and_get_upgrade_link
+from lms.djangoapps.experiments.utils import check_and_get_upgrade_link_and_date
 
 from edxmako.shortcuts import marketing_link
 from openedx.core.djangoapps.schedules.message_type import ScheduleMessageType
@@ -155,6 +155,8 @@ def _gather_users_and_schedules_for_target_hour(target_hour, org_list, exclude_o
     schedules = Schedule.objects.select_related(
         'enrollment__user__profile',
         'enrollment__course',
+    ).prefetch_related(
+        'enrollment__course__modes'
     ).filter(
         enrollment__user__in=users,
         start__gte=beginning_of_day,
@@ -176,37 +178,15 @@ def _gather_users_and_schedules_for_target_hour(target_hour, org_list, exclude_o
     return users, schedules
 
 
-def _should_user_be_upsold(enrollment):
-    enrollment_mode = None
-    is_active = None
-
-    if enrollment:
-        enrollment_mode = enrollment.mode
-        is_active = enrollment.is_active
-
-    # Return `true` if user is not enrolled in course
-    if enrollment_mode is None and is_active is None:
-        return True
-
-    # Show the summary if user enrollment is in which allow user to upsell
-    return is_active and enrollment_mode in ["Verified", "Professional"]
-
-
 def _add_upsell_button_to_email_template(a_user, a_schedule, template_context):
-    show_upsell = _should_user_be_upsold(a_schedule.enrollment)
     # Check and upgrade link performs a query on CourseMode, which is triggering failures in
     # test_send_recurring_nudge.py
-    upgrade_data = check_and_get_upgrade_link(a_user, a_schedule.enrollment.course_id)
-
-    upsell_link = ''
-
-    if upgrade_data:
-        upsell_link = upgrade_data.link
-
-    template_context['show_upsell'] = show_upsell
-    template_context['upsell_link'] = upsell_link
-    template_context['user_schedule_upgrade_deadline_time'] = upgrade_data.date
+    upgrade_link, upgrade_date = check_and_get_upgrade_link_and_date(a_user, a_schedule.enrollment)
 
+    template_context['show_upsell'] = upgrade_link is not None
+    template_context['upsell_link'] = upgrade_link
+    template_context['user_schedule_upgrade_deadline_time'] = upgrade_date
+    
 
 @task(ignore_result=True, routing_key=ROUTING_KEY)
 def recurring_nudge_schedule_bin(
@@ -261,6 +241,10 @@ def _recurring_nudge_schedules_for_bin(site, target_day, bin_num, org_list, excl
             # This is used by the bulk email optout policy
             'course_ids': course_id_strs,
         })
+
+        # Information for including upsell messaging in template.
+        _add_upsell_button_to_email_template(user, first_schedule, template_context)
+
         yield (user, first_schedule.enrollment.course.language, template_context)
 
 
@@ -386,6 +370,8 @@ def get_schedules_with_target_date_by_bin_and_orgs(schedule_date_field, target_d
     schedules = Schedule.objects.select_related(
         'enrollment__user__profile',
         'enrollment__course',
+    ).prefetch_related(
+        'enrollment__course__modes'
     ).filter(
         enrollment__user__in=users,
         enrollment__is_active=True,

openedx/features/course_experience/tests/views/test_course_updates.py

@@ -126,7 +126,7 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase):
         course_updates_url(self.course)
 
         # Fetch the view and verify that the query counts haven't changed
-        with self.assertNumQueries(33, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
+        with self.assertNumQueries(32, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
             with check_mongo_calls(4):
                 url = course_updates_url(self.course)
                 self.client.get(url)