From 44e5afba1e5d14518f81ec9aa7e8c495df9ae790 Mon Sep 17 00:00:00 2001 From: Gabe Mulley Date: Tue, 24 Oct 2017 11:18:06 -0400 Subject: [PATCH 1/3] Revert "Revert "Merge pull request #16260 from edx/mulby/dynamic-deadline-upgrade-messaging"" This reverts commit b541dfa3fddf8fc4ad0cee4d6217402609c54913. --- common/djangoapps/student/models.py | 24 ++++- lms/djangoapps/courseware/course_tools.py | 73 ++++++++++++++ lms/djangoapps/courseware/date_summary.py | 64 +++++++++--- .../courseware/tests/test_course_info.py | 4 +- .../courseware/tests/test_course_tools.py | 99 +++++++++++++++++++ .../courseware/tests/test_date_summary.py | 10 ++ .../tests/views/test_course_home.py | 4 +- .../course_experience/views/course_sock.py | 33 ++----- setup.py | 3 +- 9 files changed, 266 insertions(+), 48 deletions(-) create mode 100644 lms/djangoapps/courseware/course_tools.py create mode 100644 lms/djangoapps/courseware/tests/test_course_tools.py diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 4d51f91486..39fd943eff 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1709,13 +1709,25 @@ class CourseEnrollment(models.Model): return None if self.dynamic_upgrade_deadline is not None: + # When course modes expire they aren't found any more and None would be returned. + # Replicate that behavior here by returning None if the personalized deadline is in the past. + if datetime.now(UTC) >= self.dynamic_upgrade_deadline: + return None return self.dynamic_upgrade_deadline return self.course_upgrade_deadline @cached_property def dynamic_upgrade_deadline(self): + """ + Returns the learner's personalized upgrade deadline if one exists, otherwise it returns None. + Note that this will return a value even if the deadline is in the past. This property can be used + to modify behavior for users with personalized deadlines by checking if it's None or not. + + Returns: + datetime|None + """ try: course_overview = self.course except CourseOverview.DoesNotExist: @@ -1746,13 +1758,19 @@ class CourseEnrollment(models.Model): log.debug('Schedules: No schedule exists for CourseEnrollment %d.', self.id) return None - if upgrade_deadline is None or datetime.now(UTC) >= upgrade_deadline: - return None - return upgrade_deadline @cached_property def course_upgrade_deadline(self): + """ + Returns the expiration datetime for the verified course mode. + + If the mode is already expired, return None. Also return None if the course does not have a verified + course mode. + + Returns: + datetime|None + """ try: if self.verified_mode: log.debug('Schedules: Defaulting to verified mode expiration date-time for %s.', self.course_id) diff --git a/lms/djangoapps/courseware/course_tools.py b/lms/djangoapps/courseware/course_tools.py new file mode 100644 index 0000000000..0ca6708bf4 --- /dev/null +++ b/lms/djangoapps/courseware/course_tools.py @@ -0,0 +1,73 @@ +""" +Platform plugins to support a verified upgrade tool. +""" + +import datetime +import pytz +from django.utils.translation import ugettext as _ + +from course_modes.models import CourseMode +from openedx.features.course_experience.course_tools import CourseTool +from student.models import CourseEnrollment +from courseware.date_summary import verified_upgrade_deadline_link +from request_cache import get_request + + +class VerifiedUpgradeTool(CourseTool): + """ + The verified upgrade tool. + """ + @classmethod + def analytics_id(cls): + """ + Returns an id to uniquely identify this tool in analytics events. + """ + return 'edx.tool.verified_upgrade' + + @classmethod + def is_enabled(cls, request, course_key): + """ + Show this tool to all learners who are eligible to upgrade. + """ + enrollment = CourseEnrollment.get_enrollment(request.user, course_key) + if enrollment is None: + return False + + if enrollment.dynamic_upgrade_deadline is None: + return False + + if not enrollment.is_active: + return False + + if enrollment.mode not in CourseMode.UPSELL_TO_VERIFIED_MODES: + return False + + if enrollment.course_upgrade_deadline is None: + return False + + if datetime.datetime.now(pytz.UTC) >= enrollment.course_upgrade_deadline: + return False + + return True + + @classmethod + def title(cls): + """ + Returns the title of this tool. + """ + return _('Upgrade to Verified') + + @classmethod + def icon_classes(cls): + """ + Returns the icon classes needed to represent this tool. + """ + return 'fa fa-certificate' + + @classmethod + def url(cls, course_key): + """ + Returns the URL for this tool for the specified course key. + """ + request = get_request() + return verified_upgrade_deadline_link(request.user, course_id=course_key) diff --git a/lms/djangoapps/courseware/date_summary.py b/lms/djangoapps/courseware/date_summary.py index b885beaf7c..1bf76a47ba 100644 --- a/lms/djangoapps/courseware/date_summary.py +++ b/lms/djangoapps/courseware/date_summary.py @@ -10,6 +10,7 @@ from babel.dates import format_timedelta from django.conf import settings from django.core.urlresolvers import reverse +from django.utils.formats import date_format from django.utils.functional import cached_property from django.utils.translation import get_language, to_locale, ugettext_lazy from django.utils.translation import ugettext as _ @@ -444,11 +445,6 @@ class VerifiedUpgradeDeadlineDate(DateSummary): Verified track. """ css_class = 'verified-upgrade-deadline' - title = ugettext_lazy('Verification Upgrade Deadline') - description = ugettext_lazy( - 'You are still eligible to upgrade to a Verified Certificate! ' - 'Pursue it to highlight the knowledge and skills you gain in this course.' - ) link_text = ugettext_lazy('Upgrade to Verified Certificate') @property @@ -475,12 +471,49 @@ class VerifiedUpgradeDeadlineDate(DateSummary): @lazy def date(self): - deadline = None - if self.enrollment: - deadline = self.enrollment.upgrade_deadline + return self.enrollment.upgrade_deadline + else: + return None - return deadline + @property + def title(self): + dynamic_deadline = self._dynamic_deadline() + if dynamic_deadline is not None: + return _('Upgrade to Verified Certificate') + + return _('Verification Upgrade Deadline') + + def _dynamic_deadline(self): + if not self.enrollment: + return None + + return self.enrollment.dynamic_upgrade_deadline + + @property + def description(self): + dynamic_deadline = self._dynamic_deadline() + if dynamic_deadline is not None: + return _('Don\'t miss the opportunity to highlight your new knowledge and skills by earning a verified' + ' certificate.') + + return _('You are still eligible to upgrade to a Verified Certificate! ' + 'Pursue it to highlight the knowledge and skills you gain in this course.') + + @property + def relative_datestring(self): + dynamic_deadline = self._dynamic_deadline() + if dynamic_deadline is None: + return super(VerifiedUpgradeDeadlineDate, self).relative_datestring + + if self.date is None or self.deadline_has_passed(): + return ' ' + + # Translators: This describes the time by which the user + # should upgrade to the verified track. 'date' will be + # their personalized verified upgrade deadline formatted + # according to their locale. + return _(u'by {date}') def register_alerts(self, request, course): """ @@ -491,6 +524,13 @@ class VerifiedUpgradeDeadlineDate(DateSummary): return days_left_to_upgrade = (self.date - self.current_time).days if self.date > self.current_time and days_left_to_upgrade <= settings.COURSE_MESSAGE_ALERT_DURATION_IN_DAYS: + upgrade_message = _( + "Don't forget, you have {time_remaining_string} left to upgrade to a Verified Certificate." + ).format(time_remaining_string=self.time_remaining_string) + if self._dynamic_deadline() is not None: + upgrade_message = _( + "Don't forget to upgrade to a verified certificate by {localized_date}." + ).format(localized_date=date_format(self.date)) CourseHomeMessages.register_info_message( request, Text(_( @@ -510,11 +550,7 @@ class VerifiedUpgradeDeadlineDate(DateSummary): upgrade_label=Text(_('Upgrade ({upgrade_price})')).format(upgrade_price=upgrade_price), ) ), - title=Text(_( - "Don't forget, you have {time_remaining_string} left to upgrade to a Verified Certificate." - )).format( - time_remaining_string=self.time_remaining_string, - ) + title=Text(upgrade_message) ) diff --git a/lms/djangoapps/courseware/tests/test_course_info.py b/lms/djangoapps/courseware/tests/test_course_info.py index 45edfe6419..cc4ac9fd22 100644 --- a/lms/djangoapps/courseware/tests/test_course_info.py +++ b/lms/djangoapps/courseware/tests/test_course_info.py @@ -379,7 +379,7 @@ class SelfPacedCourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTest self.assertEqual(resp.status_code, 200) def test_num_queries_instructor_paced(self): - self.fetch_course_info_with_queries(self.instructor_paced_course, 26, 3) + self.fetch_course_info_with_queries(self.instructor_paced_course, 27, 3) def test_num_queries_self_paced(self): - self.fetch_course_info_with_queries(self.self_paced_course, 26, 3) + self.fetch_course_info_with_queries(self.self_paced_course, 27, 3) diff --git a/lms/djangoapps/courseware/tests/test_course_tools.py b/lms/djangoapps/courseware/tests/test_course_tools.py new file mode 100644 index 0000000000..cbe9ff7f69 --- /dev/null +++ b/lms/djangoapps/courseware/tests/test_course_tools.py @@ -0,0 +1,99 @@ +import datetime + +from mock import patch +from nose.plugins.attrib import attr +import pytz +from django.test import RequestFactory + +from course_modes.models import CourseMode +from course_modes.tests.factories import CourseModeFactory +from courseware.course_tools import VerifiedUpgradeTool +from courseware.models import DynamicUpgradeDeadlineConfiguration +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.schedules.config import CREATE_SCHEDULE_WAFFLE_FLAG +from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag +from student.tests.factories import CourseEnrollmentFactory, UserFactory +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + + +@attr(shard=3) +class VerifiedUpgradeToolTest(SharedModuleStoreTestCase): + + @classmethod + def setUpClass(cls): + super(VerifiedUpgradeToolTest, cls).setUpClass() + cls.now = datetime.datetime.now(pytz.UTC) + + cls.course = CourseFactory.create( + org='edX', + number='test', + display_name='Test Course', + self_paced=True, + start=cls.now - datetime.timedelta(days=30), + ) + cls.course_overview = CourseOverview.get_from_id(cls.course.id) + + @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True) + def setUp(self): + super(VerifiedUpgradeToolTest, self).setUp() + + self.course_verified_mode = CourseModeFactory( + course_id=self.course.id, + mode_slug=CourseMode.VERIFIED, + expiration_datetime=self.now + datetime.timedelta(days=30), + ) + + patcher = patch('openedx.core.djangoapps.schedules.signals.get_current_site') + mock_get_current_site = patcher.start() + self.addCleanup(patcher.stop) + mock_get_current_site.return_value = SiteFactory.create() + + DynamicUpgradeDeadlineConfiguration.objects.create(enabled=True) + + self.enrollment = CourseEnrollmentFactory( + course_id=self.course.id, + mode=CourseMode.AUDIT, + course=self.course_overview, + ) + self.request = RequestFactory().request() + self.request.user = self.enrollment.user + + def test_tool_visible(self): + self.assertTrue(VerifiedUpgradeTool().is_enabled(self.request, self.course.id)) + + def test_not_visible_when_no_enrollment_exists(self): + self.enrollment.delete() + + request = RequestFactory().request() + request.user = UserFactory() + self.assertFalse(VerifiedUpgradeTool().is_enabled(self.request, self.course.id)) + + def test_not_visible_when_using_deadline_from_course_mode(self): + DynamicUpgradeDeadlineConfiguration.objects.create(enabled=False) + self.assertFalse(VerifiedUpgradeTool().is_enabled(self.request, self.course.id)) + + def test_not_visible_when_enrollment_is_inactive(self): + self.enrollment.is_active = False + self.enrollment.save() + self.assertFalse(VerifiedUpgradeTool().is_enabled(self.request, self.course.id)) + + def test_not_visible_when_already_verified(self): + self.enrollment.mode = CourseMode.VERIFIED + self.enrollment.save() + self.assertFalse(VerifiedUpgradeTool().is_enabled(self.request, self.course.id)) + + def test_not_visible_when_no_verified_track(self): + self.course_verified_mode.delete() + self.assertFalse(VerifiedUpgradeTool().is_enabled(self.request, self.course.id)) + + def test_not_visible_when_course_deadline_has_passed(self): + self.course_verified_mode.expiration_datetime = self.now - datetime.timedelta(days=1) + self.course_verified_mode.save() + self.assertFalse(VerifiedUpgradeTool().is_enabled(self.request, self.course.id)) + + def test_not_visible_when_course_mode_has_no_deadline(self): + self.course_verified_mode.expiration_datetime = None + self.course_verified_mode.save() + self.assertFalse(VerifiedUpgradeTool().is_enabled(self.request, self.course.id)) diff --git a/lms/djangoapps/courseware/tests/test_date_summary.py b/lms/djangoapps/courseware/tests/test_date_summary.py index 340e7883a6..3dcb787e39 100644 --- a/lms/djangoapps/courseware/tests/test_date_summary.py +++ b/lms/djangoapps/courseware/tests/test_date_summary.py @@ -585,6 +585,16 @@ class TestScheduleOverrides(SharedModuleStoreTestCase): enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) block = VerifiedUpgradeDeadlineDate(course, enrollment.user) self.assertEqual(block.date, expected) + self._check_text(block) + + def _check_text(self, upgrade_date_summary): + self.assertEqual(upgrade_date_summary.title, 'Upgrade to Verified Certificate') + self.assertEqual( + upgrade_date_summary.description, + 'Don\'t miss the opportunity to highlight your new knowledge and skills by earning a verified' + ' certificate.' + ) + self.assertEqual(upgrade_date_summary.relative_datestring, 'by {date}') @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True) def test_date_with_self_paced_with_enrollment_after_course_start(self): diff --git a/openedx/features/course_experience/tests/views/test_course_home.py b/openedx/features/course_experience/tests/views/test_course_home.py index 9e46e235d6..81bee98906 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -244,7 +244,7 @@ class TestCourseHomePageAccess(CourseHomePageTestCase): expected_count = 1 if (is_enrolled or is_unenrolled_staff) else 0 self.assertContains(response, TEST_CHAPTER_NAME, count=expected_count) self.assertContains(response, 'Start Course', count=expected_count) - self.assertContains(response, 'Learn About Verified Certificate', count=expected_count) + self.assertContains(response, 'Learn About Verified Certificate', count=(1 if is_enrolled else 0)) self.assertContains(response, TEST_WELCOME_MESSAGE, count=expected_count) # Verify that the expected message is shown to the user @@ -285,7 +285,7 @@ class TestCourseHomePageAccess(CourseHomePageTestCase): expected_count = 1 if (is_enrolled or is_unenrolled_staff) else 0 self.assertContains(response, TEST_CHAPTER_NAME, count=expected_count) self.assertContains(response, 'Start Course', count=expected_count) - self.assertContains(response, 'Learn About Verified Certificate', count=expected_count) + self.assertContains(response, 'Learn About Verified Certificate', count=(1 if is_enrolled else 0)) # Verify that the expected message is shown to the user self.assertContains(response, '
', count=1 if expected_message else 0) diff --git a/openedx/features/course_experience/views/course_sock.py b/openedx/features/course_experience/views/course_sock.py index 9c27c77183..e708511c1e 100644 --- a/openedx/features/course_experience/views/course_sock.py +++ b/openedx/features/course_experience/views/course_sock.py @@ -3,12 +3,10 @@ Fragment for rendering the course's sock and associated toggle button. """ from django.template.loader import render_to_string from django.utils.translation import get_language -from opaque_keys.edx.keys import CourseKey from web_fragments.fragment import Fragment -from commerce.utils import EcommerceService -from course_modes.models import CourseMode, get_cosmetic_verified_display_price -from courseware.date_summary import VerifiedUpgradeDeadlineDate +from course_modes.models import get_cosmetic_verified_display_price +from courseware.date_summary import verified_upgrade_deadline_link, verified_upgrade_link_is_valid from openedx.core.djangoapps.plugin_api.views import EdxFragmentView from student.models import CourseEnrollment @@ -25,29 +23,12 @@ class CourseSockFragmentView(EdxFragmentView): html = render_to_string('course_experience/course-sock-fragment.html', context) return Fragment(html) - def get_verification_context(self, request, course): - course_key = CourseKey.from_string(unicode(course.id)) - - # Establish whether the course has a verified mode - available_modes = CourseMode.modes_for_course_dict(unicode(course.id)) - has_verified_mode = CourseMode.has_verified_mode(available_modes) - - # Establish whether the user is already enrolled - is_already_verified = CourseEnrollment.is_enrolled_as_verified(request.user, course_key) - - # Establish whether the verification deadline has already passed - verification_deadline = VerifiedUpgradeDeadlineDate(course, request.user) - deadline_has_passed = verification_deadline.deadline_has_passed() - - # If this proves its worth, we can internationalize and display for more than English speakers. - show_course_sock = ( - has_verified_mode and not is_already_verified and - not deadline_has_passed and get_language() == 'en' - ) - - # Get information about the upgrade + @staticmethod + def get_verification_context(request, course): + enrollment = CourseEnrollment.get_enrollment(request.user, course.id) + show_course_sock = verified_upgrade_link_is_valid(enrollment) and get_language() == 'en' + upgrade_url = verified_upgrade_deadline_link(request.user, course=course) course_price = get_cosmetic_verified_display_price(course) - upgrade_url = EcommerceService().upgrade_url(request.user, course_key) context = { 'show_course_sock': show_course_sock, diff --git a/setup.py b/setup.py index 61bf07eb69..d5d37a7a42 100644 --- a/setup.py +++ b/setup.py @@ -6,7 +6,7 @@ from setuptools import setup setup( name="Open edX", - version="0.7", + version="0.8", install_requires=["setuptools"], requires=[], # NOTE: These are not the names we should be installing. This tree should @@ -40,6 +40,7 @@ setup( "course_bookmarks = openedx.features.course_bookmarks.plugins:CourseBookmarksTool", "course_updates = openedx.features.course_experience.plugins:CourseUpdatesTool", "course_reviews = openedx.features.course_experience.plugins:CourseReviewsTool", + "verified_upgrade = courseware.course_tools:VerifiedUpgradeTool", ], "openedx.user_partition_scheme": [ "random = openedx.core.djangoapps.user_api.partition_schemes:RandomUserPartitionScheme", From f90f4276f99d5b484f41e676548a5adeb8c25dfe Mon Sep 17 00:00:00 2001 From: Gabe Mulley Date: Fri, 20 Oct 2017 12:31:21 -0400 Subject: [PATCH 2/3] fix course sock for courses without verified modes --- .../tests/views/test_course_sock.py | 15 +++++---------- .../course_experience/views/course_sock.py | 8 ++++++-- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/openedx/features/course_experience/tests/views/test_course_sock.py b/openedx/features/course_experience/tests/views/test_course_sock.py index c4cebe8b1d..cc31250d45 100644 --- a/openedx/features/course_experience/tests/views/test_course_sock.py +++ b/openedx/features/course_experience/tests/views/test_course_sock.py @@ -4,6 +4,7 @@ Tests for course verification sock import ddt +from commerce.models import CommerceConfiguration from course_modes.models import CourseMode from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag from openedx.features.course_experience import DISPLAY_COURSE_SOCK_FLAG @@ -50,6 +51,8 @@ class TestCourseSockView(SharedModuleStoreTestCase): user=self.user, course_id=self.verified_course_already_enrolled.id, mode=CourseMode.VERIFIED ) + CommerceConfiguration.objects.create(enabled=True, checkout_on_ecommerce_service=True) + # Log the user in self.client.login(username=self.user.username, password=TEST_PASSWORD) @@ -90,15 +93,7 @@ class TestCourseSockView(SharedModuleStoreTestCase): self.assert_verified_sock_is_not_visible(self.verified_course_already_enrolled, response) def assert_verified_sock_is_visible(self, course, response): - return self.assertIn( - TEST_VERIFICATION_SOCK_LOCATOR, - response.content, - msg='Student should be able to see sock if they have already upgraded to verified mode.', - ) + return self.assertContains(response, TEST_VERIFICATION_SOCK_LOCATOR, html=False) def assert_verified_sock_is_not_visible(self, course, response): - return self.assertNotIn( - TEST_VERIFICATION_SOCK_LOCATOR, - response.content, - msg='Student should not be able to see sock in a unverifiable course.', - ) + return self.assertNotContains(response, TEST_VERIFICATION_SOCK_LOCATOR, html=False) diff --git a/openedx/features/course_experience/views/course_sock.py b/openedx/features/course_experience/views/course_sock.py index e708511c1e..d9c25e7724 100644 --- a/openedx/features/course_experience/views/course_sock.py +++ b/openedx/features/course_experience/views/course_sock.py @@ -27,8 +27,12 @@ class CourseSockFragmentView(EdxFragmentView): def get_verification_context(request, course): enrollment = CourseEnrollment.get_enrollment(request.user, course.id) show_course_sock = verified_upgrade_link_is_valid(enrollment) and get_language() == 'en' - upgrade_url = verified_upgrade_deadline_link(request.user, course=course) - course_price = get_cosmetic_verified_display_price(course) + if show_course_sock: + upgrade_url = verified_upgrade_deadline_link(request.user, course=course) + course_price = get_cosmetic_verified_display_price(course) + else: + upgrade_url = '' + course_price = '' context = { 'show_course_sock': show_course_sock, From 7efa805fbc38eae3f0f80dcf9fa60f9d9291e005 Mon Sep 17 00:00:00 2001 From: Gabe Mulley Date: Tue, 24 Oct 2017 13:51:22 -0400 Subject: [PATCH 3/3] fix tests --- lms/djangoapps/courseware/tests/test_views.py | 4 ++-- .../course_experience/tests/views/test_course_home.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 7dad956499..11a500dd9e 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -213,8 +213,8 @@ class IndexQueryTestCase(ModuleStoreTestCase): NUM_PROBLEMS = 20 @ddt.data( - (ModuleStoreEnum.Type.mongo, 10, 146), - (ModuleStoreEnum.Type.split, 4, 146), + (ModuleStoreEnum.Type.mongo, 10, 143), + (ModuleStoreEnum.Type.split, 4, 143), ) @ddt.unpack def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count): diff --git a/openedx/features/course_experience/tests/views/test_course_home.py b/openedx/features/course_experience/tests/views/test_course_home.py index 81bee98906..bdbdb6969c 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -175,7 +175,7 @@ class TestCourseHomePage(CourseHomePageTestCase): course_home_url(self.course) # Fetch the view and verify the query counts - with self.assertNumQueries(50, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(49, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_home_url(self.course) self.client.get(url)