From e191d39f59e99b96bd1a1e82d52cfd4114323b89 Mon Sep 17 00:00:00 2001 From: "Albert (AJ) St. Aubin" Date: Mon, 19 Jul 2021 14:47:05 -0400 Subject: [PATCH] fix: Corrected issue with request cert showing when user was in mode that did not generate certs --- common/djangoapps/course_modes/models.py | 2 +- common/djangoapps/student/helpers.py | 18 +++-- common/djangoapps/student/tests/test_views.py | 17 ++-- common/djangoapps/student/tests/tests.py | 77 +++++++++++-------- common/djangoapps/student/views/dashboard.py | 2 +- common/djangoapps/student/views/management.py | 2 +- 6 files changed, 70 insertions(+), 48 deletions(-) diff --git a/common/djangoapps/course_modes/models.py b/common/djangoapps/course_modes/models.py index aee40b4de6..259514377b 100644 --- a/common/djangoapps/course_modes/models.py +++ b/common/djangoapps/course_modes/models.py @@ -776,7 +776,7 @@ class CourseMode(models.Model): """ ineligible_modes = [cls.AUDIT] - if settings.FEATURES['DISABLE_HONOR_CERTIFICATES']: + if settings.FEATURES.get('DISABLE_HONOR_CERTIFICATES', False): # Adding check so that we can regenerate the certificate for learners who have # already earned the certificate using honor mode from lms.djangoapps.certificates.data import CertificateStatuses diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index 80b65724e3..3c4b40840a 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -441,32 +441,32 @@ class AccountValidationError(Exception): self.error_code = error_code -def cert_info(user, course_overview): +def cert_info(user, enrollment): """ Get the certificate info needed to render the dashboard section for the given student and course. Arguments: user (User): A user. - course_overview (CourseOverview): A course. + enrollment (CourseEnrollment): A course enrollment. Returns: See _cert_info """ return _cert_info( user, - course_overview, - certificate_status_for_student(user, course_overview.id), + enrollment, + certificate_status_for_student(user, enrollment.course_overview.id), ) -def _cert_info(user, course_overview, cert_status): +def _cert_info(user, enrollment, cert_status): """ Implements the logic for cert_info -- split out for testing. Arguments: user (User): A user. - course_overview (CourseOverview): A course. + enrollment (CourseEnrollment): A course enrollment. cert_status (dict): dictionary containing information about certificate status for the user Returns: @@ -506,9 +506,10 @@ def _cert_info(user, course_overview, cert_status): 'can_unenroll': True, } - if cert_status is None: + if cert_status is None or enrollment is None: return default_info + course_overview = enrollment.course_overview if enrollment else None status = template_state.get(cert_status['status'], default_status) is_hidden_status = status in ('processing', 'generating', 'notpassing', 'auditing') @@ -525,6 +526,9 @@ def _cert_info(user, course_overview, cert_status): ): return default_info + if not CourseMode.is_eligible_for_certificate(enrollment.mode, status=status): + return default_info + status_dict = { 'status': status, 'mode': cert_status.get('mode', None), diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index 3ee0dbd7c7..a7e7ca9dce 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -154,10 +154,13 @@ class TestStudentDashboardUnenrollments(SharedModuleStoreTestCase): def test_course_run_refund_status_invalid_course_key(self): """ Assert that view:course_run_refund_status returns correct Json for Invalid Course Key .""" - with patch('opaque_keys.edx.keys.CourseKey.from_string') as mock_method: - mock_method.side_effect = InvalidKeyError('CourseKey', 'The course key used to get refund status caused \ - InvalidKeyError during look up.') - response = self.client.get(reverse('course_run_refund_status', kwargs={'course_id': self.course.id})) + test_url = reverse('course_run_refund_status', kwargs={'course_id': self.course.id}) + with patch('common.djangoapps.student.views.management.CourseKey.from_string') as mock_method: + mock_method.side_effect = InvalidKeyError( + 'CourseKey', + 'The course key used to get refund status caused InvalidKeyError during look up.' + ) + response = self.client.get(test_url) assert json.loads(response.content.decode('utf-8')) == {'course_refundable_status': ''} assert response.status_code == 406 @@ -229,7 +232,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, certificate_available_date=TOMORROW, lowest_passing_grade=0.3 ) - CourseEnrollmentFactory(course_id=course.id, user=self.user) + CourseEnrollmentFactory(course_id=course.id, user=self.user, mode=CourseMode.VERIFIED) GeneratedCertificateFactory( status=CertificateStatuses.downloadable, course_id=course.id, user=self.user, grade=0.45 ) @@ -244,7 +247,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, certificate_available_date=TOMORROW, lowest_passing_grade=0.3 ) - CourseEnrollmentFactory(course_id=course.id, user=self.user) + CourseEnrollmentFactory(course_id=course.id, user=self.user, mode=CourseMode.VERIFIED) GeneratedCertificateFactory( status=CertificateStatuses.downloadable, course_id=course.id, user=self.user, grade=0.45 ) @@ -259,7 +262,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, certificate_available_date=now(), lowest_passing_grade=0.3 ) - CourseEnrollmentFactory(course_id=course.id, user=self.user) + CourseEnrollmentFactory(course_id=course.id, user=self.user, mode=CourseMode.VERIFIED) GeneratedCertificateFactory( status=CertificateStatuses.downloadable, course_id=course.id, user=self.user, grade=0.45 ) diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index c1716409b9..d0cf19e097 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -85,25 +85,28 @@ class CourseEndingTest(ModuleStoreTestCase): grade='67', download_url='http://s3.edx/cert' ) + enrollment = CourseEnrollmentFactory(user=user, course_id=course.id, mode=CourseMode.VERIFIED) - assert _cert_info(user, course, None) ==\ + assert _cert_info(user, enrollment, None) ==\ {'status': 'processing', 'show_survey_button': False, 'can_unenroll': True} cert_status = {'status': 'unavailable', 'mode': 'honor', 'uuid': None} - assert _cert_info(user, course, cert_status) == {'status': 'processing', 'show_survey_button': False, - 'mode': 'honor', 'linked_in_url': None, 'can_unenroll': True} + assert _cert_info(user, enrollment, cert_status) == {'status': 'processing', 'show_survey_button': False, + 'mode': 'honor', 'linked_in_url': None, + 'can_unenroll': True} cert_status = {'status': 'generating', 'grade': '0.67', 'mode': 'honor', 'uuid': None} with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as patch_persisted_grade: patch_persisted_grade.return_value = Mock(percent=1.0) - assert _cert_info(user, course, cert_status) == {'status': 'generating', 'show_survey_button': True, - 'survey_url': survey_url, 'grade': '1.0', 'mode': 'honor', - 'linked_in_url': None, 'can_unenroll': False} + assert _cert_info(user, enrollment, cert_status) == {'status': 'generating', 'show_survey_button': True, + 'survey_url': survey_url, 'grade': '1.0', + 'mode': 'honor', 'linked_in_url': None, + 'can_unenroll': False} cert_status = {'status': 'generating', 'grade': '0.67', 'mode': 'honor', 'uuid': None} - assert _cert_info(user, course, cert_status) == {'status': 'generating', 'show_survey_button': True, - 'survey_url': survey_url, 'grade': '0.67', 'mode': 'honor', - 'linked_in_url': None, 'can_unenroll': False} + assert _cert_info(user, enrollment, cert_status) == {'status': 'generating', 'show_survey_button': True, + 'survey_url': survey_url, 'grade': '0.67', 'mode': 'honor', + 'linked_in_url': None, 'can_unenroll': False} cert_status = { 'status': 'downloadable', @@ -112,10 +115,11 @@ class CourseEndingTest(ModuleStoreTestCase): 'mode': 'honor', 'uuid': 'fakeuuidbutitsfine', } - assert _cert_info(user, course, cert_status) == {'status': 'downloadable', 'download_url': cert.download_url, - 'show_survey_button': True, 'survey_url': survey_url, - 'grade': '0.67', 'mode': 'honor', 'linked_in_url': None, - 'can_unenroll': False} + assert _cert_info(user, enrollment, cert_status) == {'status': 'downloadable', + 'download_url': cert.download_url, + 'show_survey_button': True, 'survey_url': survey_url, + 'grade': '0.67', 'mode': 'honor', 'linked_in_url': None, + 'can_unenroll': False} cert_status = { 'status': 'notpassing', 'grade': '0.67', @@ -123,25 +127,34 @@ class CourseEndingTest(ModuleStoreTestCase): 'mode': 'honor', 'uuid': 'fakeuuidbutitsfine', } - assert _cert_info(user, course, cert_status) == {'status': 'notpassing', 'show_survey_button': True, - 'survey_url': survey_url, 'grade': '0.67', 'mode': 'honor', - 'linked_in_url': None, 'can_unenroll': True} + assert _cert_info(user, enrollment, cert_status) == {'status': 'notpassing', 'show_survey_button': True, + 'survey_url': survey_url, 'grade': '0.67', 'mode': 'honor', + 'linked_in_url': None, 'can_unenroll': True} # Test a course that doesn't have a survey specified - course2 = Mock(end_of_course_survey_url=None, id=CourseLocator(org="a", course="b", run="c")) + course2 = CourseOverviewFactory.create( + end_of_course_survey_url=None, + certificates_display_behavior='end', + ) + enrollment2 = CourseEnrollmentFactory(user=user, course_id=course2.id, mode=CourseMode.VERIFIED) + cert_status = { 'status': 'notpassing', 'grade': '0.67', 'download_url': cert.download_url, 'mode': 'honor', 'uuid': 'fakeuuidbutitsfine' } - assert _cert_info(user, course2, cert_status) == {'status': 'notpassing', 'show_survey_button': False, - 'grade': '0.67', 'mode': 'honor', 'linked_in_url': None, - 'can_unenroll': True} + assert _cert_info(user, enrollment2, cert_status) == {'status': 'notpassing', 'show_survey_button': False, + 'grade': '0.67', 'mode': 'honor', 'linked_in_url': None, + 'can_unenroll': True} + course3 = CourseOverviewFactory.create( + end_of_course_survey_url=None, + certificates_display_behavior='early_no_info', + ) + enrollment3 = CourseEnrollmentFactory(user=user, course_id=course3.id, mode=CourseMode.VERIFIED) # 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', 'mode': 'honor', 'uuid': None} - assert _cert_info(user, course2, cert_status) == {'status': 'processing', 'show_survey_button': False, - 'can_unenroll': True} + assert _cert_info(user, enrollment3, cert_status) == {'status': 'processing', 'show_survey_button': False, + 'can_unenroll': True} cert_status = { 'status': 'notpassing', 'grade': '0.67', @@ -149,8 +162,8 @@ class CourseEndingTest(ModuleStoreTestCase): 'mode': 'honor', 'uuid': 'fakeuuidbutitsfine' } - assert _cert_info(user, course2, cert_status) == {'status': 'processing', 'show_survey_button': False, - 'can_unenroll': True} + assert _cert_info(user, enrollment3, cert_status) == {'status': 'processing', 'show_survey_button': False, + 'can_unenroll': True} @ddt.data( (0.70, 0.60), @@ -175,6 +188,7 @@ class CourseEndingTest(ModuleStoreTestCase): end_of_course_survey_url=survey_url, certificates_display_behavior='end', ) + enrollment = CourseEnrollmentFactory(user=user, course_id=course.id, mode=CourseMode.VERIFIED) if cert_grade is not None: cert_status = {'status': 'generating', 'grade': str(cert_grade), 'mode': 'honor', 'uuid': None} @@ -183,10 +197,10 @@ class CourseEndingTest(ModuleStoreTestCase): with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as patch_persisted_grade: patch_persisted_grade.return_value = Mock(percent=persisted_grade) - assert _cert_info(user, course, cert_status) == {'status': 'generating', 'show_survey_button': True, - 'survey_url': survey_url, 'grade': str(expected_grade), - 'mode': 'honor', 'linked_in_url': None, - 'can_unenroll': False} + assert _cert_info(user, enrollment, cert_status) == {'status': 'generating', 'show_survey_button': True, + 'survey_url': survey_url, 'grade': str(expected_grade), + 'mode': 'honor', 'linked_in_url': None, + 'can_unenroll': False} def test_cert_grade_no_grades(self): """ @@ -201,11 +215,12 @@ class CourseEndingTest(ModuleStoreTestCase): certificates_display_behavior='end', ) cert_status = {'status': 'generating', 'mode': 'honor', 'uuid': None} + enrollment = CourseEnrollmentFactory(user=user, course_id=course.id, mode=CourseMode.VERIFIED) with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as patch_persisted_grade: patch_persisted_grade.return_value = None - assert _cert_info(user, course, cert_status) == {'status': 'processing', 'show_survey_button': False, - 'can_unenroll': True} + assert _cert_info(user, enrollment, cert_status) == {'status': 'processing', 'show_survey_button': False, + 'can_unenroll': True} @ddt.ddt diff --git a/common/djangoapps/student/views/dashboard.py b/common/djangoapps/student/views/dashboard.py index 779a1320ec..cd3372c9a5 100644 --- a/common/djangoapps/student/views/dashboard.py +++ b/common/djangoapps/student/views/dashboard.py @@ -676,7 +676,7 @@ def student_dashboard(request): # lint-amnesty, pylint: disable=too-many-statem # there is no verification messaging to display. verify_status_by_course = check_verify_status_by_course(user, course_enrollments) cert_statuses = { - enrollment.course_id: cert_info(request.user, enrollment.course_overview) + enrollment.course_id: cert_info(request.user, enrollment) for enrollment in course_enrollments } diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index d43ceb8456..4505b6212d 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -401,7 +401,7 @@ def change_enrollment(request, check_access=True): if not enrollment: return HttpResponseBadRequest(_("You are not enrolled in this course")) - certificate_info = cert_info(user, enrollment.course_overview) + certificate_info = cert_info(user, enrollment) if certificate_info.get('status') in DISABLE_UNENROLL_CERT_STATES: return HttpResponseBadRequest(_("Your certificate prevents you from unenrolling from this course"))