diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index e799bcdf9b..2faf03bba4 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1377,7 +1377,7 @@ class CourseEnrollment(models.Model): # If it is after the refundable cutoff date they should not be refunded. refund_cutoff_date = self.refund_cutoff_date() - if refund_cutoff_date and datetime.now() > refund_cutoff_date: + if refund_cutoff_date and datetime.now(UTC) > refund_cutoff_date: return False course_mode = CourseMode.mode_for_course(self.course_id, 'verified') @@ -1400,7 +1400,7 @@ class CourseEnrollment(models.Model): self.course_overview.start.replace(tzinfo=None) ) - return refund_window_start_date + EnrollmentRefundConfiguration.current().refund_window + return refund_window_start_date.replace(tzinfo=UTC) + EnrollmentRefundConfiguration.current().refund_window @property def username(self): diff --git a/common/djangoapps/student/tests/test_refunds.py b/common/djangoapps/student/tests/test_refunds.py index 6c56c9a18a..f495b81bc6 100644 --- a/common/djangoapps/student/tests/test_refunds.py +++ b/common/djangoapps/student/tests/test_refunds.py @@ -113,10 +113,10 @@ class RefundableTest(SharedModuleStoreTestCase): self.assertTrue(self.enrollment.refundable()) with patch('student.models.CourseEnrollment.refund_cutoff_date') as cutoff_date: - cutoff_date.return_value = datetime.now() - timedelta(days=1) + cutoff_date.return_value = datetime.now(pytz.UTC) - timedelta(minutes=5) self.assertFalse(self.enrollment.refundable()) - cutoff_date.return_value = datetime.now() + timedelta(days=1) + cutoff_date.return_value = datetime.now(pytz.UTC) + timedelta(minutes=5) self.assertTrue(self.enrollment.refundable()) @ddt.data( @@ -132,7 +132,7 @@ class RefundableTest(SharedModuleStoreTestCase): """ Assert that the later date is used with the configurable refund period in calculating the returned cutoff date. """ - now = datetime.now().replace(microsecond=0) + now = datetime.now(pytz.UTC).replace(microsecond=0) order_date = now + order_date_delta course_start = now + course_start_delta expected_date = now + expected_date_delta diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index 2c42339913..3438770856 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -11,6 +11,7 @@ from django.conf import settings from student.tests.factories import UserFactory, CourseEnrollmentFactory from student.models import CourseEnrollment +from student.helpers import DISABLE_UNENROLL_CERT_STATES from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -38,7 +39,10 @@ class TestStudentDashboardUnenrollments(ModuleStoreTestCase): def mock_cert(self, _user, _course_overview, _course_mode): # pylint: disable=unused-argument """ Return a preset certificate status. """ if self.cert_status is not None: - return {'status': self.cert_status} + return { + 'status': self.cert_status, + 'can_unenroll': self.cert_status not in DISABLE_UNENROLL_CERT_STATES + } else: return {} @@ -85,3 +89,17 @@ class TestStudentDashboardUnenrollments(ModuleStoreTestCase): course_enrollment.assert_called_with(self.user, self.course.id) else: course_enrollment.assert_not_called() + + def test_no_cert_status(self): + """ Assert that the dashboard loads when cert_status is None.""" + with patch('student.views.cert_info', return_value=None): + response = self.client.get(reverse('dashboard')) + + self.assertEqual(response.status_code, 200) + + def test_cant_unenroll_status(self): + """ Assert that the dashboard loads when cert_status does not allow for unenrollment""" + with patch('certificates.models.certificate_status_for_student', return_value={'status': 'ready'}): + response = self.client.get(reverse('dashboard')) + + self.assertEqual(response.status_code, 200) diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index cc03c370c3..a2c1e05ae4 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -79,6 +79,7 @@ class CourseEndingTest(TestCase): 'show_disabled_download_button': False, 'show_download_url': False, 'show_survey_button': False, + 'can_unenroll': True, } ) @@ -91,7 +92,8 @@ class CourseEndingTest(TestCase): 'show_download_url': False, 'show_survey_button': False, 'mode': None, - 'linked_in_url': None + 'linked_in_url': None, + 'can_unenroll': True, } ) @@ -106,7 +108,8 @@ class CourseEndingTest(TestCase): 'survey_url': survey_url, 'grade': '67', 'mode': 'honor', - 'linked_in_url': None + 'linked_in_url': None, + 'can_unenroll': False, } ) @@ -121,7 +124,8 @@ class CourseEndingTest(TestCase): 'survey_url': survey_url, 'grade': '67', 'mode': 'verified', - 'linked_in_url': None + 'linked_in_url': None, + 'can_unenroll': False, } ) @@ -143,7 +147,8 @@ class CourseEndingTest(TestCase): 'survey_url': survey_url, 'grade': '67', 'mode': 'honor', - 'linked_in_url': None + 'linked_in_url': None, + 'can_unenroll': False, } ) @@ -162,7 +167,8 @@ class CourseEndingTest(TestCase): 'survey_url': survey_url, 'grade': '67', 'mode': 'honor', - 'linked_in_url': None + 'linked_in_url': None, + 'can_unenroll': True, } ) @@ -181,21 +187,22 @@ class CourseEndingTest(TestCase): 'show_survey_button': False, 'grade': '67', 'mode': 'honor', - 'linked_in_url': None + 'linked_in_url': None, + 'can_unenroll': True, } ) # test when the display is unavailable or notpassing, we get the correct results out course2.certificates_display_behavior = 'early_no_info' cert_status = {'status': 'unavailable'} - self.assertIsNone(_cert_info(user, course2, cert_status, course_mode)) + self.assertEqual(_cert_info(user, course2, cert_status, course_mode), {}) cert_status = { 'status': 'notpassing', 'grade': '67', 'download_url': download_url, 'mode': 'honor' } - self.assertIsNone(_cert_info(user, course2, cert_status, course_mode)) + self.assertEqual(_cert_info(user, course2, cert_status, course_mode), {}) @ddt.ddt @@ -1033,6 +1040,32 @@ class DashboardTestXSeriesPrograms(ModuleStoreTestCase, ProgramsApiConfigMixin): else: self.assertIn('xseries-border-btn', response.content) + @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) + @ddt.data((-2, -1), (-1, 1), (1, 2)) + @ddt.unpack + def test_start_end_offsets(self, start_days_offset, end_days_offset): + """Test that the xseries upsell messaging displays whether the course + has not yet started, is in session, or has already ended. + """ + self.course_1.start = datetime.now(pytz.UTC) + timedelta(days=start_days_offset) + self.course_1.end = datetime.now(pytz.UTC) + timedelta(days=end_days_offset) + self.update_course(self.course_1, self.user.id) + CourseEnrollment.enroll(self.user, self.course_1.id, mode='verified') + + self.client.login(username="jack", password="test") + self.create_config(enabled=True, enable_student_dashboard=True) + + with patch( + 'student.views.get_course_programs_for_dashboard', + return_value=self._create_program_data([(self.course_1.id, 'active')]) + ) as mock_get_programs: + response = self.client.get(reverse('dashboard')) + # ensure that our course id was included in the API call regardless of start/end dates + __, course_ids = mock_get_programs.call_args[0] + self.assertEqual(list(course_ids), [self.course_1.id]) + # count total courses appearing on student dashboard + self._assert_responses(response, 1) + @ddt.data( ('unpublished', 'unpublished', 'unpublished', 0), ('active', 'unpublished', 'unpublished', 1), diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 89e7337114..845968856e 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -202,6 +202,7 @@ def cert_info(user, course_overview, course_mode): 'show_survey_button': bool 'survey_url': url, only if show_survey_button is True 'grade': if status is not 'processing' + 'can_unenroll': if status allows for unenrollment """ if not course_overview.may_certify(): return {} @@ -302,6 +303,7 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa 'show_disabled_download_button': False, 'show_download_url': False, 'show_survey_button': False, + 'can_unenroll': True } if cert_status is None: @@ -310,7 +312,7 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa is_hidden_status = cert_status['status'] in ('unavailable', 'processing', 'generating', 'notpassing') if course_overview.certificates_display_behavior == 'early_no_info' and is_hidden_status: - return None + return {} status = template_state.get(cert_status['status'], default_status) @@ -319,7 +321,8 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa 'show_download_url': status == 'ready', 'show_disabled_download_button': status == 'generating', 'mode': cert_status.get('mode', None), - 'linked_in_url': None + 'linked_in_url': None, + 'can_unenroll': status not in DISABLE_UNENROLL_CERT_STATES, } if (status in ('generating', 'ready', 'notpassing', 'restricted') and @@ -581,7 +584,7 @@ def dashboard(request): # program-related information on the dashboard view. course_programs = {} if is_student_dashboard_programs_enabled(): - course_programs = _get_course_programs(user, show_courseware_links_for) + course_programs = _get_course_programs(user, [enrollment.course_id for enrollment in course_enrollments]) # Construct a dictionary of course mode information # used to render the course list. We re-use the course modes dict @@ -1030,8 +1033,8 @@ def change_enrollment(request, check_access=True): if not enrollment: return HttpResponseBadRequest(_("You are not enrolled in this course")) - certicifate_info = cert_info(user, enrollment.course_overview, enrollment.mode) - if certicifate_info.get('status') in DISABLE_UNENROLL_CERT_STATES: + certificate_info = cert_info(user, enrollment.course_overview, enrollment.mode) + if certificate_info.get('status') in DISABLE_UNENROLL_CERT_STATES: return HttpResponseBadRequest(_("Your certificate prevents you from unenrolling from this course")) CourseEnrollment.unenroll(user, course_id) diff --git a/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py b/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py index 483bcad788..93f2043b2d 100644 --- a/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py +++ b/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py @@ -245,7 +245,7 @@ class ProctoredExamsTest(BaseInstructorDashboardTest): # Stop the timed exam. self.courseware_page.stop_timed_exam() - @flaky # TODO fix this SOL-1183 + @flaky # TODO fix this SOL-1182 def test_can_add_remove_allowance(self): """ Make sure that allowances can be added and removed. @@ -263,6 +263,7 @@ class ProctoredExamsTest(BaseInstructorDashboardTest): # Then I can add Allowance to that exam for a student self.assertTrue(allowance_section.is_add_allowance_button_visible) + @flaky # TODO fix this SOL-1182 def test_can_reset_attempts(self): """ Make sure that Exam attempts are visible and can be reset. diff --git a/lms/djangoapps/certificates/admin.py b/lms/djangoapps/certificates/admin.py index 38983e2988..f878e083b2 100644 --- a/lms/djangoapps/certificates/admin.py +++ b/lms/djangoapps/certificates/admin.py @@ -11,6 +11,7 @@ from certificates.models import ( BadgeImageConfiguration, CertificateTemplate, CertificateTemplateAsset, + GeneratedCertificate, ) @@ -46,8 +47,17 @@ class CertificateTemplateAssetAdmin(admin.ModelAdmin): list_display = ('description', '__unicode__') +class GeneratedCertificateAdmin(admin.ModelAdmin): + """ + Django admin customizations for GeneratedCertificate model + """ + search_fields = ('course_id', 'user__username') + list_display = ('id', 'course_id', 'mode', 'user') + + admin.site.register(CertificateGenerationConfiguration) admin.site.register(CertificateHtmlViewConfiguration, ConfigurationModelAdmin) admin.site.register(BadgeImageConfiguration) admin.site.register(CertificateTemplate, CertificateTemplateAdmin) admin.site.register(CertificateTemplateAsset, CertificateTemplateAssetAdmin) +admin.site.register(GeneratedCertificate, GeneratedCertificateAdmin) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 38d8bac62c..bf0decad3d 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -1037,7 +1037,7 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course # New Relic. The suffix is necessary for XModule handlers because the # "handler" in those cases is always just "xmodule_handler". nr_tx_name = "{}.{}".format(instance.__class__.__name__, handler) - nr_tx_name += "/{}".format(suffix) if suffix else "" + nr_tx_name += "/{}".format(suffix) if (suffix and handler == "xmodule_handler") else "" newrelic.agent.set_transaction_name(nr_tx_name, group="Python/XBlock/Handler") tracking_context_name = 'module_callback_handler' diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index 8755a7f8d2..a54bb972dd 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -87,6 +87,7 @@ import json % for dashboard_index, enrollment in enumerate(course_enrollments): <% show_courseware_link = (enrollment.course_id in show_courseware_links_for) %> <% cert_status = cert_statuses.get(enrollment.course_id) %> + <% can_unenroll = (not cert_status) or cert_status.get('can_unenroll') %> <% credit_status = credit_statuses.get(enrollment.course_id) %> <% show_email_settings = (enrollment.course_id in show_email_settings_for) %> <% course_mode_info = all_course_modes.get(enrollment.course_id) %> @@ -96,7 +97,7 @@ import json <% course_verification_status = verification_status_by_course.get(enrollment.course_id, {}) %> <% course_requirements = courses_requirements_not_met.get(enrollment.course_id) %> <% course_program_info = course_programs.get(unicode(enrollment.course_id)) %> - <%include file = 'dashboard/_dashboard_course_listing.html' args="course_overview=enrollment.course_overview, enrollment=enrollment, show_courseware_link=show_courseware_link, cert_status=cert_status, credit_status=credit_status, show_email_settings=show_email_settings, course_mode_info=course_mode_info, show_refund_option=show_refund_option, is_paid_course=is_paid_course, is_course_blocked=is_course_blocked, verification_status=course_verification_status, course_requirements=course_requirements, dashboard_index=dashboard_index, share_settings=share_settings, user=user, course_program_info=course_program_info" /> + <%include file = 'dashboard/_dashboard_course_listing.html' args="course_overview=enrollment.course_overview, enrollment=enrollment, show_courseware_link=show_courseware_link, cert_status=cert_status, can_unenroll=can_unenroll, credit_status=credit_status, show_email_settings=show_email_settings, course_mode_info=course_mode_info, show_refund_option=show_refund_option, is_paid_course=is_paid_course, is_course_blocked=is_course_blocked, verification_status=course_verification_status, course_requirements=course_requirements, dashboard_index=dashboard_index, share_settings=share_settings, user=user, course_program_info=course_program_info" /> % endfor diff --git a/lms/templates/dashboard/_dashboard_course_listing.html b/lms/templates/dashboard/_dashboard_course_listing.html index 50d45e7ae2..b17e089ca1 100644 --- a/lms/templates/dashboard/_dashboard_course_listing.html +++ b/lms/templates/dashboard/_dashboard_course_listing.html @@ -1,4 +1,4 @@ -<%page args="course_overview, enrollment, show_courseware_link, cert_status, credit_status, show_email_settings, course_mode_info, show_refund_option, is_paid_course, is_course_blocked, verification_status, course_requirements, dashboard_index, share_settings, course_program_info" /> +<%page args="course_overview, enrollment, show_courseware_link, cert_status, can_unenroll, credit_status, show_email_settings, course_mode_info, show_refund_option, is_paid_course, is_course_blocked, verification_status, course_requirements, dashboard_index, share_settings, course_program_info" /> <%! import urllib @@ -178,7 +178,7 @@ from student.helpers import (