From 2c984719e9ad72f60f7e5edd07f7f4802cd885ed Mon Sep 17 00:00:00 2001 From: Anthony Mangano Date: Tue, 19 Dec 2017 15:57:45 -0500 Subject: [PATCH 1/4] ensure enrollment_end is parsed to datetime prior to making comparisons --- common/djangoapps/student/tests/test_views.py | 24 +++++++++---------- common/djangoapps/student/views.py | 3 ++- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index 0cbc643680..7a184e715c 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -361,7 +361,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin): mock_course_runs.return_value = [ { 'key': 'course-v1:FAKE+FA1-MA1.X+3T2017', - 'enrollment_end': self.TOMORROW, + 'enrollment_end': str(self.TOMORROW), 'pacing_type': 'instructor_paced', 'type': 'verified' } @@ -389,7 +389,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin): mock_course_runs.return_value = [ { 'key': 'course-v1:FAKE+FA1-MA1.X+3T2017', - 'enrollment_end': self.TOMORROW, + 'enrollment_end': str(self.TOMORROW), 'pacing_type': 'instructor_paced', 'type': 'verified' } @@ -416,8 +416,8 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin): course_enrollment = CourseEnrollmentFactory(user=self.user, course_id=unicode(mocked_course_overview.id)) mock_course_runs.return_value = [ { - 'key': mocked_course_overview.id, - 'enrollment_end': mocked_course_overview.enrollment_end, + 'key': str(mocked_course_overview.id), + 'enrollment_end': str(mocked_course_overview.enrollment_end), 'pacing_type': 'self_paced', 'type': 'verified' } @@ -434,8 +434,8 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin): mock_course_key.return_value = mocked_course_overview.id mock_course_runs.return_value = [ { - 'key': mocked_course_overview.id, - 'enrollment_end': mocked_course_overview.enrollment_end, + 'key': str(mocked_course_overview.id), + 'enrollment_end': str(mocked_course_overview.enrollment_end), 'pacing_type': 'self_paced', 'type': 'verified' } @@ -451,8 +451,8 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin): mock_course_key.return_value = mocked_course_overview.id mock_course_runs.return_value = [ { - 'key': mocked_course_overview.id, - 'enrollment_end': mocked_course_overview.enrollment_end, + 'key': str(mocked_course_overview.id), + 'enrollment_end': None, 'pacing_type': 'self_paced', 'type': 'verified' } @@ -481,8 +481,8 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin): course_enrollment = CourseEnrollmentFactory(user=self.user, course_id=unicode(mocked_course_overview.id)) mock_course_runs.return_value = [ { - 'key': mocked_course_overview.id, - 'enrollment_end': mocked_course_overview.enrollment_end, + 'key': str(mocked_course_overview.id), + 'enrollment_end': str(mocked_course_overview.enrollment_end), 'pacing_type': 'self_paced', 'type': 'verified' } @@ -517,8 +517,8 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin): course_enrollment = CourseEnrollmentFactory(user=self.user, course_id=unicode(mocked_course_overview.id), created=self.THREE_YEARS_AGO) mock_course_runs.return_value = [ { - 'key': mocked_course_overview.id, - 'enrollment_end': mocked_course_overview.enrollment_end, + 'key': str(mocked_course_overview.id), + 'enrollment_end': str(mocked_course_overview.enrollment_end), 'pacing_type': 'self_paced', 'type': 'verified' } diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 5e3bf342fe..9bfa1a9858 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -3,6 +3,7 @@ Student Views """ import datetime +import dateutil import json import logging import uuid @@ -708,7 +709,7 @@ def dashboard(request): for course_run in course_runs_for_course: enrollment_end = course_run.get('enrollment_end') - if not enrollment_end or enrollment_end > datetime.datetime.now(UTC): + if not enrollment_end or (dateutil.parser.parse(enrollment_end) > datetime.datetime.now(UTC)): enrollable_course_runs.append(course_run) course_entitlement_available_sessions[str(course_entitlement.uuid)] = enrollable_course_runs From 128486d79a159a2b4e4c36da17a369228bb8041c Mon Sep 17 00:00:00 2001 From: Michael LoTurco Date: Mon, 18 Dec 2017 12:31:38 -0500 Subject: [PATCH 2/4] Upgrade audit enrollments on entitlement purchase Adds check for existing upgradeable enrollment on user entitlement creation, if single upgradeable enrollment is present, upgrades enrollment to entitlement mode and links the entitlement to the upgraded enrollment LEARNER-3579 --- .../entitlements/api/v1/tests/test_views.py | 41 ++++++++++++++++++ .../djangoapps/entitlements/api/v1/views.py | 42 +++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/common/djangoapps/entitlements/api/v1/tests/test_views.py b/common/djangoapps/entitlements/api/v1/tests/test_views.py index 44b5086b33..03fc7dddff 100644 --- a/common/djangoapps/entitlements/api/v1/tests/test_views.py +++ b/common/djangoapps/entitlements/api/v1/tests/test_views.py @@ -7,6 +7,9 @@ from datetime import datetime, timedelta import pytz from django.conf import settings from django.core.urlresolvers import reverse + +from course_modes.models import CourseMode +from course_modes.tests.factories import CourseModeFactory from mock import patch from opaque_keys.edx.locator import CourseKey from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -34,6 +37,13 @@ class EntitlementViewSetTest(ModuleStoreTestCase): self.user = UserFactory(is_staff=True) self.client.login(username=self.user.username, password=TEST_PASSWORD) self.course = CourseFactory() + self.course_mode = CourseModeFactory( + course_id=self.course.id, + mode_slug=CourseMode.VERIFIED, + # This must be in the future to ensure it is returned by downstream code. + expiration_datetime=datetime.now(pytz.UTC) + timedelta(days=1) + ) + self.entitlements_list_url = reverse('entitlements_api:v1:entitlements-list') def _get_data_set(self, user, course_uuid): @@ -115,6 +125,37 @@ class EntitlementViewSetTest(ModuleStoreTestCase): ) assert results == CourseEntitlementSerializer(course_entitlement).data + @patch("entitlements.api.v1.views.get_course_runs_for_course") + def test_add_entitlement_and_upgrade_audit_enrollment(self, mock_get_course_runs): + """ + Verify that if an entitlement is added for a user, if the user has one upgradeable enrollment + that enrollment is upgraded to the mode of the entitlement and linked to the entitlement. + """ + course_uuid = uuid.uuid4() + entitlement_data = self._get_data_set(self.user, str(course_uuid)) + mock_get_course_runs.return_value = [{'key': str(self.course.id)}] + + # Add an audit course enrollment for user. + enrollment = CourseEnrollment.enroll(self.user, self.course.id, mode=CourseMode.AUDIT) + + response = self.client.post( + self.entitlements_list_url, + data=json.dumps(entitlement_data), + content_type='application/json', + ) + assert response.status_code == 201 + results = response.data + + course_entitlement = CourseEntitlement.objects.get( + user=self.user, + course_uuid=course_uuid + ) + # Assert that enrollment mode is now verified + enrollment_mode = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id)[0] + assert enrollment_mode == course_entitlement.mode + assert course_entitlement.enrollment_course_run == enrollment + assert results == CourseEntitlementSerializer(course_entitlement).data + def test_non_staff_get_select_entitlements(self): not_staff_user = UserFactory() self.client.login(username=not_staff_user.username, password=TEST_PASSWORD) diff --git a/common/djangoapps/entitlements/api/v1/views.py b/common/djangoapps/entitlements/api/v1/views.py index 0858fdfbee..a5fdfe22a0 100644 --- a/common/djangoapps/entitlements/api/v1/views.py +++ b/common/djangoapps/entitlements/api/v1/views.py @@ -53,6 +53,48 @@ class EntitlementViewSet(viewsets.ModelViewSet): # to Admin users return CourseEntitlement.objects.all().select_related('user').select_related('enrollment_course_run') + def create(self, request, *args, **kwargs): + serializer = self.get_serializer(data=request.data) + serializer.is_valid(raise_exception=True) + self.perform_create(serializer) + + entitlement = serializer.instance + user = entitlement.user + + # find all course_runs within the course + course_runs = get_course_runs_for_course(entitlement.course_uuid) + + # check if the user has enrollments for any of the course_runs + user_run_enrollments = [ + CourseEnrollment.get_enrollment(user, CourseKey.from_string(course_run.get('key'))) + for course_run + in course_runs + if CourseEnrollment.get_enrollment(user, CourseKey.from_string(course_run.get('key'))) + ] + + # filter to just enrollments that can be upgraded. + upgradeable_enrollments = [ + enrollment + for enrollment + in user_run_enrollments + if enrollment.upgrade_deadline and enrollment.upgrade_deadline > timezone.now() + ] + + # if there is only one upgradeable enrollment, convert it from audit to the entitlement.mode + # if there is any ambiguity about which enrollment to upgrade + # (i.e. multiple upgradeable enrollments or no available upgradeable enrollment), dont enroll + if len(upgradeable_enrollments) == 1: + enrollment = upgradeable_enrollments[0] + log.info('Upgrading enrollment [%s] from audit to [%s] while adding entitlement for user [%s] for course [%s] ', enrollment, serializer.data.get('mode'), user.username, serializer.data.get('course_uuid')) + enrollment.update_enrollment(mode=entitlement.mode) + entitlement.set_enrollment(enrollment) + else: + log.info('No enrollment upgraded while adding entitlement for user [%s] for course [%s] ', user.username, serializer.data.get('course_uuid')) + + headers = self.get_success_headers(serializer.data) + # Note, the entitlement is re-serialized before getting added to the Response, so that the 'modified' date reflects changes that occur when upgrading enrollment. + return Response(CourseEntitlementSerializer(entitlement).data, status=status.HTTP_201_CREATED, headers=headers) + def retrieve(self, request, *args, **kwargs): """ Override the retrieve method to expire a record that is past the From b177f3daabebae5543a333809dfd10b7d635c000 Mon Sep 17 00:00:00 2001 From: McKenzie Welter Date: Mon, 18 Dec 2017 13:22:18 -0500 Subject: [PATCH 3/4] include email settings in fulfilled entitlement action items --- common/djangoapps/student/tests/test_views.py | 29 +++++++++++++++++++ lms/static/js/dashboard/legacy.js | 3 +- lms/templates/dashboard.html | 7 +++-- .../dashboard/_dashboard_course_listing.html | 10 +++---- .../_dashboard_entitlement_actions.html | 17 +++++++++-- themes/edx.org/lms/templates/dashboard.html | 5 +++- 6 files changed, 59 insertions(+), 12 deletions(-) diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index 7a184e715c..6686f84e69 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -19,6 +19,7 @@ from mock import patch from opaque_keys import InvalidKeyError from pyquery import PyQuery as pq +from bulk_email.models import BulkEmailFlag from entitlements.tests.factories import CourseEntitlementFactory from openedx.core.djangoapps.catalog.tests.factories import ProgramFactory from openedx.core.djangoapps.content.course_overviews.models import CourseOverview @@ -238,6 +239,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin): Tests for the student dashboard. """ + EMAIL_SETTINGS_ELEMENT_ID = "#actions-item-email-settings-0" ENABLED_SIGNALS = ['course_published'] TOMORROW = datetime.datetime.now(pytz.utc) + datetime.timedelta(days=1) THREE_YEARS_AGO = datetime.datetime.now(pytz.utc) - datetime.timedelta(days=(365 * 3)) @@ -533,6 +535,33 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin): self.assertIn('You can no longer change sessions.', response.content) self.assertIn('Related Programs:', response.content) + @patch.object(CourseOverview, 'get_from_id') + @patch.object(BulkEmailFlag, 'feature_enabled') + def test_email_settings_fulfilled_entitlement(self, mock_email_feature, mock_course_overview): + """ + Assert that the Email Settings action is shown when the user has a fulfilled entitlement. + """ + mock_email_feature.return_value = True + mock_course_overview.return_value = CourseOverviewFactory( + start=self.TOMORROW, self_paced=True, enrollment_end=self.TOMORROW + ) + course_enrollment = CourseEnrollmentFactory(user=self.user) + CourseEntitlementFactory(user=self.user, enrollment_course_run=course_enrollment) + response = self.client.get(self.path) + self.assertEqual(pq(response.content)(self.EMAIL_SETTINGS_ELEMENT_ID).length, 1) + + @patch.object(CourseOverview, 'get_from_id') + @patch.object(BulkEmailFlag, 'feature_enabled') + def test_email_settings_unfulfilled_entitlement(self, mock_email_feature, mock_course_overview): + """ + Assert that the Email Settings action is not shown when the entitlement is not fulfilled. + """ + mock_email_feature.return_value = True + mock_course_overview.return_value = CourseOverviewFactory(start=self.TOMORROW) + CourseEntitlementFactory(user=self.user) + response = self.client.get(self.path) + self.assertEqual(pq(response.content)(self.EMAIL_SETTINGS_ELEMENT_ID).length, 0) + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @override_settings(BRANCH_IO_KEY='test_key') diff --git a/lms/static/js/dashboard/legacy.js b/lms/static/js/dashboard/legacy.js index be8df1fc54..ffe87b8318 100644 --- a/lms/static/js/dashboard/legacy.js +++ b/lms/static/js/dashboard/legacy.js @@ -131,7 +131,6 @@ if ($(event.target).data('optout') === 'False') { $('#receive_emails').prop('checked', true); } - edx.dashboard.dropdown.toggleCourseActionsDropdownMenu(event); }); $('.action-unenroll').click(function(event) { var isPaidCourse = $(event.target).data('course-is-paid-course') === 'True'; @@ -211,6 +210,7 @@ }); $('.action-email-settings').each(function(index) { + $(this).attr('id', 'email-settings-' + index); // a bit of a hack, but gets the unique selector for the modal trigger var trigger = '#' + $(this).attr('id'); accessibleModal( @@ -219,7 +219,6 @@ '#email-settings-modal', '#dashboard-main' ); - $(this).attr('id', 'email-settings-' + index); }); $('.action-unenroll').each(function(index) { diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index 820733a5f5..c0e36410eb 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -167,14 +167,17 @@ from student.models import CourseEnrollment if not next_session: continue enrollment = CourseEnrollment(user=user, course_id=next_session['key'], mode=next_session['type']) - + # We only show email settings for entitlement cards if the entitlement has an associated enrollment + show_email_settings = is_fulfilled_entitlement and (entitlement_session.course_id in show_email_settings_for) + else: + show_email_settings = (enrollment.course_id in show_email_settings_for) + session_id = enrollment.course_id show_courseware_link = (session_id in show_courseware_links_for) cert_status = cert_statuses.get(session_id) can_refund_entitlement = entitlement and entitlement.is_entitlement_refundable() can_unenroll = (not cert_status) or cert_status.get('can_unenroll') if not unfulfilled_entitlement else False credit_status = credit_statuses.get(session_id) - show_email_settings = (session_id in show_email_settings_for) course_mode_info = all_course_modes.get(session_id) is_paid_course = True if entitlement else (session_id in enrolled_courses_either_paid) is_course_blocked = (session_id in block_courses) diff --git a/lms/templates/dashboard/_dashboard_course_listing.html b/lms/templates/dashboard/_dashboard_course_listing.html index 83b211ea15..3a653e5868 100644 --- a/lms/templates/dashboard/_dashboard_course_listing.html +++ b/lms/templates/dashboard/_dashboard_course_listing.html @@ -235,11 +235,11 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_ % endif % endif - ## Right now, the gear dropdown for entitlements only contains the 'unenroll' link, so we should hide the - ## gear altogether if the user is unable to unenroll/refund their entitlement. Later, when we add more options - ## to the gear dropdown, we can remove this check. - % if entitlement and can_refund_entitlement: - <%include file='_dashboard_entitlement_actions.html' args='course_overview=course_overview,entitlement=entitlement,dashboard_index=dashboard_index, can_refund_entitlement=can_refund_entitlement'/> + ## We should only show the gear dropdown if the user is able to refund/unenroll from their entitlement + ## and/or if they have selected a course run and email_settings are enabled + ## as these are the only actions currently available + % if entitlement and (can_refund_entitlement or show_email_settings): + <%include file='_dashboard_entitlement_actions.html' args='course_overview=course_overview,entitlement=entitlement,dashboard_index=dashboard_index, can_refund_entitlement=can_refund_entitlement, show_email_settings=show_email_settings'/> % elif not entitlement:
diff --git a/themes/edx.org/lms/templates/dashboard.html b/themes/edx.org/lms/templates/dashboard.html index 577d69e42e..1da55c5130 100644 --- a/themes/edx.org/lms/templates/dashboard.html +++ b/themes/edx.org/lms/templates/dashboard.html @@ -163,6 +163,10 @@ from student.models import CourseEnrollment if not next_session: continue enrollment = CourseEnrollment(user=user, course_id=next_session['key'], mode=next_session['type']) + # We only show email settings for entitlement cards if the entitlement has an associated enrollment + show_email_settings = is_fulfilled_entitlement and (entitlement_session.course_id in show_email_settings_for) + else: + show_email_settings = (enrollment.course_id in show_email_settings_for) session_id = enrollment.course_id show_courseware_link = (session_id in show_courseware_links_for) @@ -170,7 +174,6 @@ from student.models import CourseEnrollment can_refund_entitlement = entitlement and entitlement.is_entitlement_refundable() can_unenroll = (not cert_status) or cert_status.get('can_unenroll') if not unfulfilled_entitlement else False credit_status = credit_statuses.get(session_id) - show_email_settings = (session_id in show_email_settings_for) course_mode_info = all_course_modes.get(session_id) is_paid_course = True if entitlement else (session_id in enrolled_courses_either_paid) is_course_blocked = (session_id in block_courses) From f7237cf6599ddfd2a0a72ac3287e013cdc411501 Mon Sep 17 00:00:00 2001 From: Michael Terry Date: Wed, 20 Dec 2017 10:18:05 -0500 Subject: [PATCH 4/4] Avoid a dropdown for course runs Entitlement detail pages in django admin normally show a dropdown button for the enrollment_course_run field. But on stage, that can cause a timeout because the enrollment database is so large. So instead, just show it as a raw id field. --- common/djangoapps/entitlements/admin.py | 1 + 1 file changed, 1 insertion(+) diff --git a/common/djangoapps/entitlements/admin.py b/common/djangoapps/entitlements/admin.py index dd167097c5..e06d8d346f 100644 --- a/common/djangoapps/entitlements/admin.py +++ b/common/djangoapps/entitlements/admin.py @@ -14,6 +14,7 @@ class CourseEntitlementAdmin(admin.ModelAdmin): 'mode', 'enrollment_course_run', 'order_number') + raw_id_fields = ('enrollment_course_run', 'user',) @admin.register(CourseEntitlementPolicy)